refactor: extract setting owner reference to a helper

Change-Id: Iadd6a6ec887c6b70f7d931a4f0983f6e9b7d8d52

pkg/controllers/helpers.go

@@ -0,0 +1,24 @@
+package controllers
+
+import (
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+)
+
+func p[T any](v T) *T {
+	return &v
+}
+
+func setOwnerRef(ownee, owner client.Object) {
+	ownee.SetOwnerReferences(
+		[]metav1.OwnerReference{
+			{
+				APIVersion: owner.GetObjectKind().GroupVersionKind().GroupVersion().String(),
+				Kind:       owner.GetObjectKind().GroupVersionKind().Kind,
+				Name:       owner.GetName(),
+				UID:        owner.GetUID(),
+				Controller: p(true),
+			},
+		},
+	)
+}

pkg/controllers/helpers_internal_test.go

@@ -0,0 +1,40 @@
+package controllers
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	v1 "k8s.io/api/core/v1"
+	netv1 "k8s.io/api/networking/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+func TestSetOwnerRef(t *testing.T) {
+	obj := v1.Pod{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "test-pod",
+			Namespace: "test-namespace",
+		},
+	}
+	ingress := &netv1.Ingress{
+		TypeMeta: metav1.TypeMeta{
+			Kind:       "Ingress",
+			APIVersion: "networking.k8s.io/v1",
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "test-ingress",
+			Namespace: "test-namespace",
+			UID:       "test-uid",
+		},
+	}
+	setOwnerRef(&obj, ingress)
+
+	require.Len(t, obj.GetOwnerReferences(), 1)
+	assert.EqualValues(t, "Ingress", obj.GetOwnerReferences()[0].Kind)
+	assert.EqualValues(t, "networking.k8s.io/v1", obj.GetOwnerReferences()[0].APIVersion)
+	assert.EqualValues(t, "test-ingress", obj.GetOwnerReferences()[0].Name)
+	assert.EqualValues(t, "test-uid", obj.GetOwnerReferences()[0].UID)
+	require.NotNil(t, obj.GetOwnerReferences()[0].Controller)
+	assert.True(t, *obj.GetOwnerReferences()[0].Controller)
+}

pkg/controllers/ingress_controller.go

@@ -133,13 +133,15 @@ func (r *IngressReconciler) calculateIngressWeight(ingress netv1.Ingress) (uint,
 	return desiredWeight, err
 }
 
-func (r *IngressReconciler) newDnsEndpoint(ctx context.Context, dnsEndpoint *externaldnsk8siov1alpha1.DNSEndpoint, target string, ingress netv1.Ingress, owner metav1.OwnerReference) {
+func (r *IngressReconciler) newDnsEndpoint(ctx context.Context, dnsEndpoint *externaldnsk8siov1alpha1.DNSEndpoint, target string, ingress netv1.Ingress) {
 	var desiredWeight uint
 	var err error
 	var healthCheckProperty *externaldnsk8siov1alpha1.ProviderSpecificProperty
+
+	setOwnerRef(dnsEndpoint, &ingress)
+
 	dnsEndpoint.Name = ingress.ObjectMeta.Name
 	dnsEndpoint.Namespace = ingress.ObjectMeta.Namespace
-	dnsEndpoint.SetOwnerReferences([]metav1.OwnerReference{owner})
 	desiredWeight = uint(trafficweight.Store.DesiredWeight)
 	if trafficweight.Store.AWSHealthCheckID != "" {
 		healthCheckProperty = &externaldnsk8siov1alpha1.ProviderSpecificProperty{
@@ -187,7 +189,7 @@ func (r *IngressReconciler) newDnsEndpoint(ctx context.Context, dnsEndpoint *ext
 	}
 }
 
-func (r *IngressReconciler) reconcileDNSEntries(ctx context.Context, ingress netv1.Ingress, ownerRef metav1.OwnerReference) error {
+func (r *IngressReconciler) reconcileDNSEntries(ctx context.Context, ingress netv1.Ingress) error {
 	log := r.Log.WithValues("IngressName", ingress.ObjectMeta.Name).WithValues("IngressNamespace", ingress.ObjectMeta.Namespace)
 
 	if !r.ingressAnnotationMatchFilter(ingress) {
@@ -210,7 +212,7 @@ func (r *IngressReconciler) reconcileDNSEntries(ctx context.Context, ingress net
 		},
 	}
 	var f controllerutil.MutateFn = func() error {
-		r.newDnsEndpoint(ctx, dnsEndpoint, target, ingress, ownerRef)
+		r.newDnsEndpoint(ctx, dnsEndpoint, target, ingress)
 		return nil
 	}
 	_, err = ctrl.CreateOrUpdate(ctx, r.Client, dnsEndpoint, f)
@@ -294,8 +296,6 @@ func (r *IngressReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
 	trafficStoreMetrics.CurrentWeight.Set(float64(trafficweight.Store.CurrentWeight))
 
 	var ingress netv1.Ingress
-	controller := true
-	var ownerRef metav1.OwnerReference
 
 	if err := r.Get(ctx, req.NamespacedName, &ingress); err != nil {
 		if apierrors.IsNotFound(err) {
@@ -322,13 +322,6 @@ func (r *IngressReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
 		// on deleted requests.
 		return ctrl.Result{}, client.IgnoreNotFound(err)
 	}
-	ownerRef = metav1.OwnerReference{
-		APIVersion: ingress.APIVersion,
-		Kind:       ingress.Kind,
-		Name:       ingress.GetName(),
-		UID:        ingress.GetUID(),
-		Controller: &controller,
-	}
 
 	if ingress.ObjectMeta.DeletionTimestamp.IsZero() { // Not being deleted
 		// There maybe situations in which we receive an reconciliation event while
@@ -338,7 +331,7 @@ func (r *IngressReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
 			log.Info("DNS endpoint being removed, requeuing notification")
 			return ctrl.Result{Requeue: true, RequeueAfter: 5 * time.Second}, nil
 		}
-		err := r.reconcileDNSEntries(ctx, ingress, ownerRef)
+		err := r.reconcileDNSEntries(ctx, ingress)
 		if err != nil {
 			log.Error(err, "Could not reconcile DNS endpoints")
 			return ctrl.Result{}, err

pkg/controllers/ingress_controller_test.go

@@ -127,14 +127,14 @@ func TestIngressController(t *testing.T) {
 
 	t.Run("Should forge DNS Endpoints", func(t *testing.T) {
 		forged := &externaldnsk8siov1alpha1.DNSEndpoint{}
-		reconciler.newDnsEndpoint(context.Background(), forged, "bar-celona", ing, ownerRef)
+		reconciler.newDnsEndpoint(context.Background(), forged, "bar-celona", ing)
 		assert.Equal(t, expected, *forged)
 	})
 
 	t.Run("Should create a DNSentry that does not exist during reconcile", func(t *testing.T) {
 		reconciler.Client = fake.NewClientBuilder().WithScheme(scheme).WithObjects(&ing).Build()
 
-		err := reconciler.reconcileDNSEntries(context.Background(), ing, ownerRef)
+		err := reconciler.reconcileDNSEntries(context.Background(), ing)
 		assert.NoError(t, err)
 
 		ep := externaldnsk8siov1alpha1.DNSEndpoint{}
@@ -150,7 +150,7 @@ func TestIngressController(t *testing.T) {
 		oldEndpoint.Spec.Endpoints[0].DNSName = "randomChange"
 
 		reconciler.Client = fake.NewClientBuilder().WithScheme(scheme).WithObjects(&ing, oldEndpoint).Build()
-		err := reconciler.reconcileDNSEntries(context.Background(), ing, ownerRef)
+		err := reconciler.reconcileDNSEntries(context.Background(), ing)
 		assert.NoError(t, err)
 
 		ep := externaldnsk8siov1alpha1.DNSEndpoint{}
@@ -165,7 +165,7 @@ func TestIngressController(t *testing.T) {
 		reconciler.Client = fake.NewClientBuilder().WithScheme(scheme).WithObjects(&ing).Build()
 		reconciler.AnnotationFilter = NewAnnotationFilter("foo=bar")
 
-		err := reconciler.reconcileDNSEntries(context.Background(), ing, ownerRef)
+		err := reconciler.reconcileDNSEntries(context.Background(), ing)
 		assert.NoError(t, err)
 
 		ep := externaldnsk8siov1alpha1.DNSEndpoint{}
@@ -180,7 +180,7 @@ func TestIngressController(t *testing.T) {
 		reconciler.Client = fake.NewClientBuilder().WithScheme(scheme).WithObjects(&ing).Build()
 		reconciler.AnnotationFilter = NewAnnotationFilter("foo=notbar")
 
-		err := reconciler.reconcileDNSEntries(context.Background(), ing, ownerRef)
+		err := reconciler.reconcileDNSEntries(context.Background(), ing)
 
 		ep := externaldnsk8siov1alpha1.DNSEndpoint{}
 		err = reconciler.Client.Get(context.Background(), client.ObjectKeyFromObject(&expected), &ep)
@@ -191,7 +191,7 @@ func TestIngressController(t *testing.T) {
 	t.Run("if we dont set --aws-health-check-id ingress shouldnt have health property", func(t *testing.T) {
 		trafficweight.Store.AWSHealthCheckID = ""
 		forged := &externaldnsk8siov1alpha1.DNSEndpoint{}
-		reconciler.newDnsEndpoint(context.Background(), forged, "bar-celona", ing, ownerRef)
+		reconciler.newDnsEndpoint(context.Background(), forged, "bar-celona", ing)
 		assert.Equal(t, expected, *forged)
 	})
 
@@ -226,7 +226,7 @@ func TestIngressController(t *testing.T) {
 		}
 
 		forged := &externaldnsk8siov1alpha1.DNSEndpoint{}
-		reconciler.newDnsEndpoint(context.Background(), forged, "bar-celona", ing, ownerRef)
+		reconciler.newDnsEndpoint(context.Background(), forged, "bar-celona", ing)
 		assert.Equal(t, expected, *forged)
 		ing.Spec.Rules = oldRules
 	})