Skip to content

Commit

Permalink
remove owner passed in to RemoveControlleReference only when that own…
Browse files Browse the repository at this point in the history
…er controller equals true

Signed-off-by: Troy Connor <troy0820@users.noreply.github.com>
  • Loading branch information
troy0820 committed Nov 27, 2023
1 parent 25bf123 commit 0ed0fc1
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 2 deletions.
8 changes: 6 additions & 2 deletions pkg/controller/controllerutil/controllerutil.go
Expand Up @@ -184,11 +184,16 @@ func RemoveControllerReference(owner, object metav1.Object, scheme *runtime.Sche
APIVersion: gvk.GroupVersion().String(),
Name: owner.GetName(),
Kind: gvk.Kind,
Controller: ptr.To(true),
})

if index == -1 {
return fmt.Errorf("%T does not have an controller reference for %T", object, owner)
}

if ownerRefs[index].Controller != ptr.To(true) && owner.GetName() != ownerRefs[index].Name {
return fmt.Errorf("%T owner is not the controller reference for %T", object, owner)
}

ownerRefs = append(ownerRefs[:index], ownerRefs[index+1:]...)
object.SetOwnerReferences(ownerRefs)
return nil
Expand Down Expand Up @@ -239,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
}

Expand Down
15 changes: 15 additions & 0 deletions pkg/controller/controllerutil/controllerutil_test.go
Expand Up @@ -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{},
Expand Down Expand Up @@ -229,6 +230,20 @@ 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.RemoveControllerReference(dep2, rs, scheme.Scheme)).To(HaveOccurred())
})

It("should not error when RemoveControllerReference owner's controller is set to true", func() {
rs := &appsv1.ReplicaSet{
ObjectMeta: metav1.ObjectMeta{},
Expand Down

0 comments on commit 0ed0fc1

Please sign in to comment.