From e3dbd850ced6a37ddefb0bb5e55dab7d32d94c90 Mon Sep 17 00:00:00 2001 From: Hiranmoy Das Chowdhury Date: Mon, 23 Sep 2024 21:31:23 +0600 Subject: [PATCH 1/4] clean up and fixed referSameObject function Signed-off-by: Hiranmoy Das Chowdhury --- .../controllerutil/controllerutil.go | 91 ++++++------------- 1 file changed, 27 insertions(+), 64 deletions(-) diff --git a/pkg/controller/controllerutil/controllerutil.go b/pkg/controller/controllerutil/controllerutil.go index ba3f931e47..16e2ea73db 100644 --- a/pkg/controller/controllerutil/controllerutil.go +++ b/pkg/controller/controllerutil/controllerutil.go @@ -27,8 +27,6 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/utils/ptr" - "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/apiutil" ) @@ -82,25 +80,20 @@ func SetControllerReference(owner, controlled metav1.Object, scheme *runtime.Sch if err != nil { return err } - ref := metav1.OwnerReference{ - APIVersion: gvk.GroupVersion().String(), - Kind: gvk.Kind, - Name: owner.GetName(), - UID: owner.GetUID(), - BlockOwnerDeletion: ptr.To(true), - Controller: ptr.To(true), - } + + ref := metav1.NewControllerRef(owner, gvk) + for _, opt := range opts { - opt(&ref) + opt(ref) } // Return early with an error if the object is already controlled. - if existing := metav1.GetControllerOf(controlled); existing != nil && !referSameObject(*existing, ref) { + if existing := metav1.GetControllerOf(controlled); existing != nil && !referSameObject(*existing, *ref) { return newAlreadyOwnedError(controlled, *existing) } // Update owner references and return. - upsertOwnerRef(ref, controlled) + upsertOwnerRef(*ref, controlled) return nil } @@ -122,18 +115,13 @@ func SetOwnerReference(owner, object metav1.Object, scheme *runtime.Scheme, opts if err != nil { return err } - ref := metav1.OwnerReference{ - APIVersion: gvk.GroupVersion().String(), - Kind: gvk.Kind, - UID: owner.GetUID(), - Name: owner.GetName(), - } + ref := NewOwnerRef(owner, gvk) for _, opt := range opts { - opt(&ref) + opt(ref) } // Update owner references and return. - upsertOwnerRef(ref, object) + upsertOwnerRef(*ref, object) return nil } @@ -154,11 +142,7 @@ func RemoveOwnerReference(owner, object metav1.Object, scheme *runtime.Scheme) e return err } - index := indexOwnerRef(owners, metav1.OwnerReference{ - APIVersion: gvk.GroupVersion().String(), - Name: owner.GetName(), - Kind: gvk.Kind, - }) + index := indexOwnerRef(owners, *NewOwnerRef(owner, gvk)) if index == -1 { return fmt.Errorf("%T does not have an owner reference for %T", object, owner) } @@ -171,14 +155,7 @@ func RemoveOwnerReference(owner, object metav1.Object, scheme *runtime.Scheme) e // HasControllerReference returns true if the object // has an owner ref with controller equal to true func HasControllerReference(object metav1.Object) bool { - owners := object.GetOwnerReferences() - for _, owner := range owners { - isTrue := owner.Controller - if owner.Controller != nil && *isTrue { - return true - } - } - return false + return metav1.GetControllerOfNoCopy(object) != nil } // HasOwnerReference returns true if the owners list contains an owner reference @@ -188,20 +165,17 @@ func HasOwnerReference(ownerRefs []metav1.OwnerReference, obj client.Object, sch if err != nil { return false, err } - idx := indexOwnerRef(ownerRefs, metav1.OwnerReference{ - APIVersion: gvk.GroupVersion().String(), - Name: obj.GetName(), - Kind: gvk.Kind, - }) + idx := indexOwnerRef(ownerRefs, *NewOwnerRef(obj, gvk)) return idx != -1, nil } // RemoveControllerReference removes an owner reference where the controller // equals true func RemoveControllerReference(owner, object metav1.Object, scheme *runtime.Scheme) error { - if ok := HasControllerReference(object); !ok { - return fmt.Errorf("%T does not have a owner reference with controller equals true", object) + if !metav1.IsControlledBy(object, owner) { + return fmt.Errorf("%T owner is not the controller reference for %T", owner, object) } + ro, ok := owner.(runtime.Object) if !ok { return fmt.Errorf("%T is not a runtime.Object, cannot call RemoveControllerReference", owner) @@ -211,19 +185,7 @@ func RemoveControllerReference(owner, object metav1.Object, scheme *runtime.Sche return err } ownerRefs := object.GetOwnerReferences() - index := indexOwnerRef(ownerRefs, metav1.OwnerReference{ - APIVersion: gvk.GroupVersion().String(), - Name: owner.GetName(), - Kind: gvk.Kind, - }) - - if index == -1 { - return fmt.Errorf("%T does not have an controller reference for %T", object, owner) - } - - if ownerRefs[index].Controller == nil || !*ownerRefs[index].Controller { - return fmt.Errorf("%T owner is not the controller reference for %T", owner, object) - } + index := indexOwnerRef(ownerRefs, *metav1.NewControllerRef(owner, gvk)) ownerRefs = append(ownerRefs[:index], ownerRefs[index+1:]...) object.SetOwnerReferences(ownerRefs) @@ -250,6 +212,16 @@ func indexOwnerRef(ownerReferences []metav1.OwnerReference, ref metav1.OwnerRefe return -1 } +// NewOwnerRef creates an OwnerReference pointing to the given owner. +func NewOwnerRef(owner metav1.Object, gvk schema.GroupVersionKind) *metav1.OwnerReference { + return &metav1.OwnerReference{ + APIVersion: gvk.GroupVersion().String(), + Kind: gvk.Kind, + Name: owner.GetName(), + UID: owner.GetUID(), + } +} + func validateOwner(owner, object metav1.Object) error { ownerNs := owner.GetNamespace() if ownerNs != "" { @@ -266,16 +238,7 @@ func validateOwner(owner, object metav1.Object) error { // Returns true if a and b point to the same object. func referSameObject(a, b metav1.OwnerReference) bool { - aGV, err := schema.ParseGroupVersion(a.APIVersion) - if err != nil { - return false - } - - bGV, err := schema.ParseGroupVersion(b.APIVersion) - if err != nil { - return false - } - return aGV.Group == bGV.Group && a.Kind == b.Kind && a.Name == b.Name + return a.UID == b.UID } // OperationResult is the action result of a CreateOrUpdate call. From 361186dd221c423dd6e03bc7468abd8e3e4db6a8 Mon Sep 17 00:00:00 2001 From: Hiranmoy Das Chowdhury Date: Tue, 24 Sep 2024 13:19:37 +0600 Subject: [PATCH 2/4] validating Namespace added while removing ownerReference and controller reference and fix error messages for not having controller reference Signed-off-by: Hiranmoy Das Chowdhury --- .../controllerutil/controllerutil.go | 38 +++++++++++++++---- 1 file changed, 30 insertions(+), 8 deletions(-) diff --git a/pkg/controller/controllerutil/controllerutil.go b/pkg/controller/controllerutil/controllerutil.go index 16e2ea73db..f16a71b364 100644 --- a/pkg/controller/controllerutil/controllerutil.go +++ b/pkg/controller/controllerutil/controllerutil.go @@ -71,7 +71,7 @@ func SetControllerReference(owner, controlled metav1.Object, scheme *runtime.Sch if !ok { return fmt.Errorf("%T is not a runtime.Object, cannot call SetControllerReference", owner) } - if err := validateOwner(owner, controlled); err != nil { + if err := validateOwnerWithNS(owner, controlled); err != nil { return err } @@ -106,7 +106,7 @@ func SetOwnerReference(owner, object metav1.Object, scheme *runtime.Scheme, opts if !ok { return fmt.Errorf("%T is not a runtime.Object, cannot call SetOwnerReference", owner) } - if err := validateOwner(owner, object); err != nil { + if err := validateOwnerWithNS(owner, object); err != nil { return err } @@ -133,10 +133,16 @@ func RemoveOwnerReference(owner, object metav1.Object, scheme *runtime.Scheme) e if length < 1 { return fmt.Errorf("%T does not have any owner references", object) } + ro, ok := owner.(runtime.Object) if !ok { return fmt.Errorf("%T is not a runtime.Object, cannot call RemoveOwnerReference", owner) } + + if err := validateOwnerWithNS(owner, object); err != nil { + return err + } + gvk, err := apiutil.GVKForObject(ro, scheme) if err != nil { return err @@ -172,10 +178,6 @@ func HasOwnerReference(ownerRefs []metav1.OwnerReference, obj client.Object, sch // RemoveControllerReference removes an owner reference where the controller // equals true func RemoveControllerReference(owner, object metav1.Object, scheme *runtime.Scheme) error { - if !metav1.IsControlledBy(object, owner) { - return fmt.Errorf("%T owner is not the controller reference for %T", owner, object) - } - ro, ok := owner.(runtime.Object) if !ok { return fmt.Errorf("%T is not a runtime.Object, cannot call RemoveControllerReference", owner) @@ -184,6 +186,17 @@ func RemoveControllerReference(owner, object metav1.Object, scheme *runtime.Sche if err != nil { return err } + + if controller := metav1.GetControllerOfNoCopy(object); controller == nil { + return fmt.Errorf("%T does not have a owner reference with controller equals true", object) + } else if !referSameObject(*controller, *NewOwnerRef(owner, gvk)) { + return fmt.Errorf("%T owner is not the controller reference for %T", owner, object) + } + + if err := validateOwnerWithNS(owner, object); err != nil { + return err + } + ownerRefs := object.GetOwnerReferences() index := indexOwnerRef(ownerRefs, *metav1.NewControllerRef(owner, gvk)) @@ -222,7 +235,7 @@ func NewOwnerRef(owner metav1.Object, gvk schema.GroupVersionKind) *metav1.Owner } } -func validateOwner(owner, object metav1.Object) error { +func validateOwnerWithNS(owner, object metav1.Object) error { ownerNs := owner.GetNamespace() if ownerNs != "" { objNs := object.GetNamespace() @@ -238,7 +251,16 @@ func validateOwner(owner, object metav1.Object) error { // Returns true if a and b point to the same object. func referSameObject(a, b metav1.OwnerReference) bool { - return a.UID == b.UID + aGV, err := schema.ParseGroupVersion(a.APIVersion) + if err != nil { + return false + } + + bGV, err := schema.ParseGroupVersion(b.APIVersion) + if err != nil { + return false + } + return aGV.Group == bGV.Group && a.Kind == b.Kind && a.Name == b.Name } // OperationResult is the action result of a CreateOrUpdate call. From 14c9e2f7add2ea02b67e6d88894bb33a4592afd3 Mon Sep 17 00:00:00 2001 From: Hiranmoy Das Chowdhury Date: Tue, 24 Sep 2024 18:57:48 +0600 Subject: [PATCH 3/4] newOwnerRef to newControllerRef for removing controller Signed-off-by: Hiranmoy Das Chowdhury --- pkg/controller/controllerutil/controllerutil.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pkg/controller/controllerutil/controllerutil.go b/pkg/controller/controllerutil/controllerutil.go index f16a71b364..8b2669a8a7 100644 --- a/pkg/controller/controllerutil/controllerutil.go +++ b/pkg/controller/controllerutil/controllerutil.go @@ -82,7 +82,6 @@ func SetControllerReference(owner, controlled metav1.Object, scheme *runtime.Sch } ref := metav1.NewControllerRef(owner, gvk) - for _, opt := range opts { opt(ref) } @@ -138,7 +137,6 @@ func RemoveOwnerReference(owner, object metav1.Object, scheme *runtime.Scheme) e if !ok { return fmt.Errorf("%T is not a runtime.Object, cannot call RemoveOwnerReference", owner) } - if err := validateOwnerWithNS(owner, object); err != nil { return err } @@ -189,7 +187,7 @@ func RemoveControllerReference(owner, object metav1.Object, scheme *runtime.Sche if controller := metav1.GetControllerOfNoCopy(object); controller == nil { return fmt.Errorf("%T does not have a owner reference with controller equals true", object) - } else if !referSameObject(*controller, *NewOwnerRef(owner, gvk)) { + } else if !referSameObject(*controller, *metav1.NewControllerRef(owner, gvk)) { return fmt.Errorf("%T owner is not the controller reference for %T", owner, object) } From 618116341874cbdca30c0cb35a3a0f039f98edd6 Mon Sep 17 00:00:00 2001 From: Hiranmoy Das Chowdhury Date: Fri, 27 Sep 2024 21:45:09 +0600 Subject: [PATCH 4/4] newOwnerRef modified and some clean up Signed-off-by: Hiranmoy Das Chowdhury --- .../controllerutil/controllerutil.go | 41 +++++++++++-------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/pkg/controller/controllerutil/controllerutil.go b/pkg/controller/controllerutil/controllerutil.go index 8b2669a8a7..6c7bb0138f 100644 --- a/pkg/controller/controllerutil/controllerutil.go +++ b/pkg/controller/controllerutil/controllerutil.go @@ -19,6 +19,7 @@ package controllerutil import ( "context" "fmt" + "k8s.io/utils/ptr" "reflect" "k8s.io/apimachinery/pkg/api/equality" @@ -81,10 +82,8 @@ func SetControllerReference(owner, controlled metav1.Object, scheme *runtime.Sch return err } - ref := metav1.NewControllerRef(owner, gvk) - for _, opt := range opts { - opt(ref) - } + ref := NewOwnerRef(owner, gvk, opts...) + ref.Controller = ptr.To(true) // Return early with an error if the object is already controlled. if existing := metav1.GetControllerOf(controlled); existing != nil && !referSameObject(*existing, *ref) { @@ -114,10 +113,7 @@ func SetOwnerReference(owner, object metav1.Object, scheme *runtime.Scheme, opts if err != nil { return err } - ref := NewOwnerRef(owner, gvk) - for _, opt := range opts { - opt(ref) - } + ref := NewOwnerRef(owner, gvk, opts...) // Update owner references and return. upsertOwnerRef(*ref, object) @@ -185,19 +181,28 @@ func RemoveControllerReference(owner, object metav1.Object, scheme *runtime.Sche return err } - if controller := metav1.GetControllerOfNoCopy(object); controller == nil { + hasController := false + index := -1 + ownerRefs := object.GetOwnerReferences() + ctrlRefToBeRemove := *metav1.NewControllerRef(owner, gvk) + for i, ownerRef := range ownerRefs { + if ownerRef.Controller != nil { + hasController = *ownerRef.Controller || hasController + } + if referSameObject(ownerRef, ctrlRefToBeRemove) { + index = i + } + } + + if !hasController { return fmt.Errorf("%T does not have a owner reference with controller equals true", object) - } else if !referSameObject(*controller, *metav1.NewControllerRef(owner, gvk)) { + } else if index == -1 || ownerRefs[index].Controller == nil || !*ownerRefs[index].Controller { return fmt.Errorf("%T owner is not the controller reference for %T", owner, object) } - if err := validateOwnerWithNS(owner, object); err != nil { return err } - ownerRefs := object.GetOwnerReferences() - index := indexOwnerRef(ownerRefs, *metav1.NewControllerRef(owner, gvk)) - ownerRefs = append(ownerRefs[:index], ownerRefs[index+1:]...) object.SetOwnerReferences(ownerRefs) return nil @@ -224,13 +229,17 @@ func indexOwnerRef(ownerReferences []metav1.OwnerReference, ref metav1.OwnerRefe } // NewOwnerRef creates an OwnerReference pointing to the given owner. -func NewOwnerRef(owner metav1.Object, gvk schema.GroupVersionKind) *metav1.OwnerReference { - return &metav1.OwnerReference{ +func NewOwnerRef(owner metav1.Object, gvk schema.GroupVersionKind, opts ...OwnerReferenceOption) *metav1.OwnerReference { + ref := &metav1.OwnerReference{ APIVersion: gvk.GroupVersion().String(), Kind: gvk.Kind, Name: owner.GetName(), UID: owner.GetUID(), } + for _, opt := range opts { + opt(ref) + } + return ref } func validateOwnerWithNS(owner, object metav1.Object) error {