fix: Make Cluster the owner of image registry credential Secret

This Secret is specific to the Cluster, and should be deleted together
with the Cluster.

Author: dlipovetsky

pkg/handlers/generic/mutation/imageregistries/credentials/inject.go

@@ -12,11 +12,13 @@ import (
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
+	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	bootstrapv1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1beta1"
 	controlplanev1 "sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1beta1"
 	runtimehooksv1 "sigs.k8s.io/cluster-api/exp/runtime/hooks/api/v1alpha1"
 	ctrl "sigs.k8s.io/controller-runtime"
 	ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 
 	"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/v1alpha1"
 	"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/common/pkg/capi/clustertopology/handlers/mutation"
@@ -69,7 +71,7 @@ func (h *imageRegistriesPatchHandler) Mutate(
 	vars map[string]apiextensionsv1.JSON,
 	holderRef runtimehooksv1.HolderReference,
 	clusterKey ctrlclient.ObjectKey,
-	_ mutation.ClusterGetter,
+	clusterGetter mutation.ClusterGetter,
 ) error {
 	log := ctrl.LoggerFrom(ctx).WithValues(
 		"holderRef", holderRef,
@@ -172,7 +174,16 @@ func (h *imageRegistriesPatchHandler) Mutate(
 				commands...,
 			)
 
-			generateErr = createSecretIfNeeded(ctx, h.client, registriesWithOptionalCredentials, clusterKey)
+			cluster, err := clusterGetter(ctx)
+			if err != nil {
+				log.Error(
+					err,
+					"failed to get cluster from Image Registry Credentials mutation handler",
+				)
+				return err
+			}
+
+			generateErr = createSecretIfNeeded(ctx, h.client, registriesWithOptionalCredentials, cluster)
 			if generateErr != nil {
 				return generateErr
 			}
@@ -216,7 +227,16 @@ func (h *imageRegistriesPatchHandler) Mutate(
 			).Info("adding PreKubeadmCommands to worker node kubeadm config template")
 			obj.Spec.Template.Spec.PreKubeadmCommands = append(obj.Spec.Template.Spec.PreKubeadmCommands, commands...)
 
-			generateErr := createSecretIfNeeded(ctx, h.client, registriesWithOptionalCredentials, clusterKey)
+			cluster, err := clusterGetter(ctx)
+			if err != nil {
+				log.Error(
+					err,
+					"failed to get cluster from Image Registry Credentials mutation handler",
+				)
+				return err
+			}
+
+			generateErr := createSecretIfNeeded(ctx, h.client, registriesWithOptionalCredentials, cluster)
 			if generateErr != nil {
 				return generateErr
 			}
@@ -335,12 +355,12 @@ func createSecretIfNeeded(
 	ctx context.Context,
 	c ctrlclient.Client,
 	registriesWithOptionalCredentials []providerConfig,
-	clusterKey ctrlclient.ObjectKey,
+	cluster *clusterv1.Cluster,
 ) error {
 	credentialsSecret, err := generateCredentialsSecret(
 		registriesWithOptionalCredentials,
-		clusterKey.Name,
-		clusterKey.Namespace,
+		cluster.Name,
+		cluster.Namespace,
 	)
 	if err != nil {
 		return fmt.Errorf(
@@ -352,6 +372,13 @@ func createSecretIfNeeded(
 		if err := client.ServerSideApply(ctx, c, credentialsSecret, client.ForceOwnership); err != nil {
 			return fmt.Errorf("failed to apply Image Registry Credentials Secret: %w", err)
 		}
+
+		if err = controllerutil.SetOwnerReference(cluster, credentialsSecret, c.Scheme()); err != nil {
+			return fmt.Errorf(
+				"failed to set owner reference on Image Registry Credentials Secret: %w",
+				err,
+			)
+		}
 	}
 
 	return nil

pkg/handlers/generic/mutation/imageregistries/credentials/inject_test.go

@@ -11,7 +11,11 @@ import (
 	"github.com/stretchr/testify/assert"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	"k8s.io/apiserver/pkg/storage/names"
+	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
+	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	runtimehooksv1 "sigs.k8s.io/cluster-api/exp/runtime/hooks/api/v1alpha1"
 
 	"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/v1alpha1"
@@ -126,15 +130,13 @@ func TestImageRegistriesPatch(t *testing.T) {
 }
 
 var _ = Describe("Generate Image registry patches", func() {
+	clientScheme := runtime.NewScheme()
+	utilruntime.Must(clientgoscheme.AddToScheme(clientScheme))
+	utilruntime.Must(clusterv1.AddToScheme(clientScheme))
+
 	patchGenerator := func() mutation.GeneratePatches {
-		// Always initialize the testEnv variable in the closure.
-		// This will allow ginkgo to initialize testEnv variable during test execution time.
-		testEnv := helpers.TestEnv
-		// use direct client instead of controller client. This will allow the patch handler to read k8s object
-		// that are written by the tests.
-		// Test cases writes credentials secret that the mutator handler reads.
-		// Using direct client will enable reading it immediately.
-		client, err := testEnv.GetK8sClient()
+		// Use direct client to allow patch handler to read objects created by tests.
+		client, err := helpers.TestEnv.GetK8sClientWithScheme(clientScheme)
 		gomega.Expect(err).To(gomega.BeNil())
 		return mutation.NewMetaGeneratePatchesHandler("", client, NewPatch(client)).(mutation.GeneratePatches)
 	}
@@ -392,22 +394,44 @@ var _ = Describe("Generate Image registry patches", func() {
 
 	// Create credentials secret before each test
 	BeforeEach(func(ctx SpecContext) {
-		client, err := helpers.TestEnv.GetK8sClient()
+		client, err := helpers.TestEnv.GetK8sClientWithScheme(clientScheme)
 		gomega.Expect(err).To(gomega.BeNil())
+
 		gomega.Expect(client.Create(
 			ctx,
 			newRegistryCredentialsSecret(validSecretName, request.Namespace),
 		)).To(gomega.BeNil())
+
+		gomega.Expect(client.Create(
+			ctx,
+			&clusterv1.Cluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      request.ClusterName,
+					Namespace: request.Namespace,
+				},
+			},
+		)).To(gomega.BeNil())
 	})
 
 	// Delete credentials secret after each test
 	AfterEach(func(ctx SpecContext) {
-		client, err := helpers.TestEnv.GetK8sClient()
+		client, err := helpers.TestEnv.GetK8sClientWithScheme(clientScheme)
 		gomega.Expect(err).To(gomega.BeNil())
+
 		gomega.Expect(client.Delete(
 			ctx,
 			newRegistryCredentialsSecret(validSecretName, request.Namespace),
 		)).To(gomega.BeNil())
+
+		gomega.Expect(client.Delete(
+			ctx,
+			&clusterv1.Cluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      request.ClusterName,
+					Namespace: request.Namespace,
+				},
+			},
+		)).To(gomega.BeNil())
 	})
 	// create test node for each case
 	for testIdx := range testDefs {