Skip to content
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 controllerutil.EnsureOwnerReference #816

Merged
merged 1 commit into from
Mar 2, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 81 additions & 25 deletions pkg/controller/controllerutil/controllerutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/utils/pointer"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
)
Expand Down Expand Up @@ -55,47 +56,102 @@ func newAlreadyOwnedError(Object metav1.Object, Owner metav1.OwnerReference) *Al
// Since only one OwnerReference can be a controller, it returns an error if
// there is another OwnerReference with Controller flag set.
func SetControllerReference(owner, controlled metav1.Object, scheme *runtime.Scheme) error {
// Validate the owner.
ro, ok := owner.(runtime.Object)
if !ok {
return fmt.Errorf("%T is not a runtime.Object, cannot call SetControllerReference", owner)
}

ownerNs := owner.GetNamespace()
if ownerNs != "" {
objNs := controlled.GetNamespace()
if objNs == "" {
return fmt.Errorf("cluster-scoped resource must not have a namespace-scoped owner, owner's namespace %s", ownerNs)
}
if ownerNs != objNs {
return fmt.Errorf("cross-namespace owner references are disallowed, owner's namespace %s, obj's namespace %s", owner.GetNamespace(), controlled.GetNamespace())
}
if err := validateOwner(owner, controlled); err != nil {
return err
}

// Create a new controller ref.
gvk, err := apiutil.GVKForObject(ro, scheme)
if err != nil {
return err
}
ref := metav1.OwnerReference{
APIVersion: gvk.GroupVersion().String(),
Kind: gvk.Kind,
Name: owner.GetName(),
UID: owner.GetUID(),
BlockOwnerDeletion: pointer.BoolPtr(true),
Controller: pointer.BoolPtr(true),
}

// Return early with an error if the object is already controlled.
if existing := metav1.GetControllerOf(controlled); existing != nil && !referSameObject(*existing, ref) {
return newAlreadyOwnedError(controlled, *existing)
}

// Create a new ref
ref := *metav1.NewControllerRef(owner, schema.GroupVersionKind{Group: gvk.Group, Version: gvk.Version, Kind: gvk.Kind})
// Update owner references and return.
upsertOwnerRef(ref, controlled)
return nil
}

existingRefs := controlled.GetOwnerReferences()
fi := -1
for i, r := range existingRefs {
if referSameObject(ref, r) {
fi = i
} else if r.Controller != nil && *r.Controller {
return newAlreadyOwnedError(controlled, r)
}
// EnsureOwnerReference is a helper method to make sure the given object contains
// an object reference to the object provided.
// If a reference already exists, it'll be overwritten with the newly provided version.
func EnsureOwnerReference(owner, object metav1.Object, scheme *runtime.Scheme) error {
// Validate the owner.
ro, ok := owner.(runtime.Object)
if !ok {
return fmt.Errorf("%T is not a runtime.Object, cannot call SetControllerReference", owner)
vincepri marked this conversation as resolved.
Show resolved Hide resolved
}
if err := validateOwner(owner, object); err != nil {
return err
}

// Create a new owner ref.
gvk, err := apiutil.GVKForObject(ro, scheme)
if err != nil {
return err
}
if fi == -1 {
existingRefs = append(existingRefs, ref)
ref := metav1.OwnerReference{
vincepri marked this conversation as resolved.
Show resolved Hide resolved
APIVersion: gvk.GroupVersion().String(),
Kind: gvk.Kind,
UID: owner.GetUID(),
Name: owner.GetName(),
}

// Update owner references and return.
upsertOwnerRef(ref, object)
return nil

}

func upsertOwnerRef(ref metav1.OwnerReference, object metav1.Object) {
owners := object.GetOwnerReferences()
idx := indexOwnerRef(owners, ref)
if idx == -1 {
owners = append(owners, ref)
} else {
existingRefs[fi] = ref
owners[idx] = ref
}
object.SetOwnerReferences(owners)
}

// indexOwnerRef returns the index of the owner reference in the slice if found, or -1.
func indexOwnerRef(ownerReferences []metav1.OwnerReference, ref metav1.OwnerReference) int {
vincepri marked this conversation as resolved.
Show resolved Hide resolved
for index, r := range ownerReferences {
if referSameObject(r, ref) {
return index
}
}
return -1
}

// Update owner references
controlled.SetOwnerReferences(existingRefs)
func validateOwner(owner, object metav1.Object) error {
ownerNs := owner.GetNamespace()
if ownerNs != "" {
objNs := object.GetNamespace()
if objNs == "" {
return fmt.Errorf("cluster-scoped resource must not have a namespace-scoped owner, owner's namespace %s", ownerNs)
}
if ownerNs != objNs {
return fmt.Errorf("cross-namespace owner references are disallowed, owner's namespace %s, obj's namespace %s", owner.GetNamespace(), object.GetNamespace())
}
}
return nil
}

Expand Down
98 changes: 96 additions & 2 deletions pkg/controller/controllerutil/controllerutil_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ import (
"fmt"
"math/rand"

"sigs.k8s.io/controller-runtime/pkg/client"

. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
appsv1 "k8s.io/api/apps/v1"
Expand All @@ -32,10 +30,80 @@ import (
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/kubernetes/scheme"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
)

var _ = Describe("Controllerutil", func() {
Describe("EnsureOwnerReference", func() {
It("should set ownerRef on an empty list", func() {
rs := &appsv1.ReplicaSet{}
dep := &extensionsv1beta1.Deployment{
ObjectMeta: metav1.ObjectMeta{Name: "foo", UID: "foo-uid"},
}
Expect(controllerutil.EnsureOwnerReference(dep, rs, scheme.Scheme)).ToNot(HaveOccurred())
Expect(rs.OwnerReferences).To(ConsistOf(metav1.OwnerReference{
Name: "foo",
Kind: "Deployment",
APIVersion: "extensions/v1beta1",
vincepri marked this conversation as resolved.
Show resolved Hide resolved
UID: "foo-uid",
}))
})

It("should not duplicate owner references", func() {
rs := &appsv1.ReplicaSet{
ObjectMeta: metav1.ObjectMeta{
OwnerReferences: []metav1.OwnerReference{
{
Name: "foo",
Kind: "Deployment",
APIVersion: "extensions/v1beta1",
UID: "foo-uid",
},
},
},
}
dep := &extensionsv1beta1.Deployment{
ObjectMeta: metav1.ObjectMeta{Name: "foo", UID: "foo-uid"},
}

Expect(controllerutil.EnsureOwnerReference(dep, rs, scheme.Scheme)).ToNot(HaveOccurred())
Expect(rs.OwnerReferences).To(ConsistOf(metav1.OwnerReference{
Name: "foo",
Kind: "Deployment",
APIVersion: "extensions/v1beta1",
UID: "foo-uid",
}))
})

It("should update the reference", func() {
rs := &appsv1.ReplicaSet{
ObjectMeta: metav1.ObjectMeta{
OwnerReferences: []metav1.OwnerReference{
{
Name: "foo",
Kind: "Deployment",
APIVersion: "extensions/v1alpha1",
UID: "foo-uid-1",
},
},
},
}
dep := &extensionsv1beta1.Deployment{
ObjectMeta: metav1.ObjectMeta{Name: "foo", UID: "foo-uid-2"},
}

Expect(controllerutil.EnsureOwnerReference(dep, rs, scheme.Scheme)).ToNot(HaveOccurred())
Expect(rs.OwnerReferences).To(ConsistOf(metav1.OwnerReference{
Name: "foo",
Kind: "Deployment",
APIVersion: "extensions/v1beta1",
UID: "foo-uid-2",
}))

})
})

Describe("SetControllerReference", func() {
It("should set the OwnerReference if it can find the group version kind", func() {
rs := &appsv1.ReplicaSet{}
Expand Down Expand Up @@ -116,6 +184,32 @@ var _ = Describe("Controllerutil", func() {
}))
})

It("should replace the owner reference if it's already present", func() {
t := true
rsOwners := []metav1.OwnerReference{
{
Name: "foo",
Kind: "Deployment",
APIVersion: "extensions/v1alpha1",
UID: "foo-uid",
Controller: &t,
BlockOwnerDeletion: &t,
},
}
rs := &appsv1.ReplicaSet{ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default", OwnerReferences: rsOwners}}
dep := &extensionsv1beta1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default", UID: "foo-uid"}}

Expect(controllerutil.SetControllerReference(dep, rs, scheme.Scheme)).NotTo(HaveOccurred())
Expect(rs.OwnerReferences).To(ConsistOf(metav1.OwnerReference{
Name: "foo",
Kind: "Deployment",
APIVersion: "extensions/v1beta1",
UID: "foo-uid",
Controller: &t,
BlockOwnerDeletion: &t,
}))
})

It("should return an error if it's setting a cross-namespace owner reference", func() {
rs := &appsv1.ReplicaSet{ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "namespace1"}}
dep := &extensionsv1beta1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "namespace2", UID: "foo-uid"}}
Expand Down