From e204fb3426b40d01fd347b98087aa9d653dc4906 Mon Sep 17 00:00:00 2001 From: Troy Connor Date: Fri, 24 Nov 2023 10:09:24 -0500 Subject: [PATCH] remove owner passed in to RemoveControlleReference only when that owner controller equals true Signed-off-by: Troy Connor --- .../controllerutil/controllerutil.go | 28 +++++++++++++++++-- .../controllerutil/controllerutil_test.go | 17 +++++++++++ 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/pkg/controller/controllerutil/controllerutil.go b/pkg/controller/controllerutil/controllerutil.go index 7b10d78582..708d56cbc4 100644 --- a/pkg/controller/controllerutil/controllerutil.go +++ b/pkg/controller/controllerutil/controllerutil.go @@ -171,7 +171,32 @@ func RemoveControllerReference(owner, object metav1.Object, scheme *runtime.Sche if ok := HasControllerReference(object); !ok { return fmt.Errorf("%T does not have a owner reference with controller equals true", object) } - return RemoveOwnerReference(owner, object, scheme) + ro, ok := owner.(runtime.Object) + if !ok { + return fmt.Errorf("%T is not a runtime.Object, cannot call RemoveControllerReference", owner) + } + gvk, err := apiutil.GVKForObject(ro, scheme) + if err != nil { + 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) + } + + ownerRefs = append(ownerRefs[:index], ownerRefs[index+1:]...) + object.SetOwnerReferences(ownerRefs) + return nil } func upsertOwnerRef(ref metav1.OwnerReference, object metav1.Object) { @@ -219,7 +244,6 @@ func referSameObject(a, b metav1.OwnerReference) bool { if err != nil { return false } - return aGV.Group == bGV.Group && a.Kind == b.Kind && a.Name == b.Name } diff --git a/pkg/controller/controllerutil/controllerutil_test.go b/pkg/controller/controllerutil/controllerutil_test.go index 7e9d9f96c4..a72dd01e09 100644 --- a/pkg/controller/controllerutil/controllerutil_test.go +++ b/pkg/controller/controllerutil/controllerutil_test.go @@ -181,6 +181,7 @@ var _ = Describe("Controllerutil", func() { Expect(controllerutil.RemoveOwnerReference(obj, rs, scheme.Scheme)).To(HaveOccurred()) Expect(rs.GetOwnerReferences()).To(HaveLen(1)) }) + It("should error when trying to remove an owner that doesn't exist", func() { rs := &appsv1.ReplicaSet{ ObjectMeta: metav1.ObjectMeta{}, @@ -229,6 +230,22 @@ var _ = Describe("Controllerutil", func() { Expect(controllerutil.RemoveControllerReference(dep, rs, scheme.Scheme)).To(HaveOccurred()) }) + It("should error when RemoveControllerReference passed in owner is not the owner", func() { + rs := &appsv1.ReplicaSet{ + ObjectMeta: metav1.ObjectMeta{}, + } + dep := &extensionsv1beta1.Deployment{ + ObjectMeta: metav1.ObjectMeta{Name: "foo", UID: "foo-uid-2"}, + } + dep2 := &extensionsv1beta1.Deployment{ + ObjectMeta: metav1.ObjectMeta{Name: "foo-2", UID: "foo-uid-42"}, + } + Expect(controllerutil.SetControllerReference(dep, rs, scheme.Scheme)).ToNot(HaveOccurred()) + Expect(controllerutil.SetOwnerReference(dep2, rs, scheme.Scheme)).ToNot(HaveOccurred()) + Expect(controllerutil.RemoveControllerReference(dep2, rs, scheme.Scheme)).To(HaveOccurred()) + Expect(rs.GetOwnerReferences()).To(HaveLen(2)) + }) + It("should not error when RemoveControllerReference owner's controller is set to true", func() { rs := &appsv1.ReplicaSet{ ObjectMeta: metav1.ObjectMeta{},