-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
✨ Add HasOwnerReference #2882
base: main
Are you sure you want to change the base?
✨ Add HasOwnerReference #2882
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: sivchari The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
// HasOwnerReference returns true if the owners list contains an owner reference | ||
// that matches the object's group, kind, and name. | ||
func HasOwnerReference(owners []metav1.OwnerReference, obj client.Object) bool { | ||
gvk := obj.GetObjectKind().GroupVersionKind() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use the indexOwnerRefs
function?
// HasOwnerReference returns true if the owners list contains an owner reference | ||
// that matches the object's group, kind, and name. | ||
func HasOwnerReference(owners []metav1.OwnerReference, obj client.Object) bool { | ||
gvk := obj.GetObjectKind().GroupVersionKind() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a function apiutil.GVKForObject()
that would be useful here. You would have to interface assert this to a runtime object.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1, TypeMeta
may not always exist in the object, even c-r has made some workaround for it
kubernetes/kubernetes#3030
kubernetes/kubernetes#80609
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I rewrote like this.
Is it right, I want the feedback, thanks.
@@ -181,6 +181,18 @@ func HasControllerReference(object metav1.Object) bool { | |||
return false | |||
} | |||
|
|||
// HasOwnerReference returns true if the owners list contains an owner reference | |||
// that matches the object's group, kind, and name. | |||
func HasOwnerReference(owners []metav1.OwnerReference, obj client.Object) bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add a test for this?
func HasOwnerReference(ownerRefs []metav1.OwnerReference, obj client.Object, scheme *runtime.Scheme) bool { | ||
gvk, err := apiutil.GVKForObject(obj, scheme) | ||
if err != nil { | ||
return false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we return an error here, please?
Kind: gvk.Kind, | ||
}) | ||
return idx != -1 | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add tests
Signed-off-by: sivchari <shibuuuu5@gmail.com>
Signed-off-by: sivchari <shibuuuu5@gmail.com>
Signed-off-by: sivchari <shibuuuu5@gmail.com>
c5352f7
to
5aa3f3e
Compare
f564288
to
1d21e60
Compare
I added HasOwnerReference API. This API check if owners contains specific owner.