Enhance ownerReferences for Secrets used by CAPX (nutanix-cloud-nativ…

…e#340)

* Remove limitation of only allowing one ownerRef for the credential secret

* Add Kind constants for CAPX

api/v1beta1/nutanixcluster_types.go

@@ -24,6 +24,9 @@ import (
 )
 
 const (
+	// NutanixClusterKind represents the Kind of NutanixCluster
+	NutanixClusterKind = "NutanixClusterKind"
+
 	// NutanixClusterFinalizer allows NutanixClusterReconciler to clean up AHV
 	// resources associated with NutanixCluster before removing it from the
 	// API Server.

api/v1beta1/nutanixmachine_types.go

@@ -28,6 +28,9 @@ import (
 // NOTE: json tags are required.  Any new fields you add must have json tags for the fields to be serialized.
 
 const (
+	// NutanixMachineKind represents the Kind of NutanixMachine
+	NutanixMachineKind = "NutanixMachineKind"
+
 	// NutanixMachineFinalizer allows NutanixMachineReconciler to clean up AHV
 	// resources associated with NutanixMachine before removing it from the
 	// API Server.

api/v1beta1/nutanixmachinetemplate_types.go

@@ -23,6 +23,11 @@ import (
 
 // NOTE: json tags are required.  Any new fields you add must have json tags for the fields to be serialized.
 
+const (
+	// NutanixMachineTemplateKind represents the Kind of NutanixMachineTemplate
+	NutanixMachineTemplateKind = "NutanixMachineTemplateKind"
+)
+
 // NutanixMachineTemplateSpec defines the desired state of NutanixMachineTemplate
 type NutanixMachineTemplateSpec struct {
 	Template NutanixMachineTemplateResource `json:"template"`

controllers/nutanixcluster_controller.go

@@ -389,16 +389,21 @@ func (r *NutanixClusterReconciler) reconcileCredentialRef(ctx context.Context, n
 		log.Error(errorMsg, "error occurred fetching cluster")
 		return errorMsg
 	}
+	// Check if ownerRef is already set on nutanixCluster object
 	if !capiutil.IsOwnedByObject(secret, nutanixCluster) {
-		if len(secret.GetOwnerReferences()) > 0 {
-			return fmt.Errorf("secret for cluster %s already has other owners set", nutanixCluster.Name)
+		// Check if another nutanixCluster already has set ownerRef. Secret can only be owned by one nutanixCluster object
+		if capiutil.HasOwner(secret.OwnerReferences, infrav1.GroupVersion.String(), []string{
+			nutanixCluster.Kind,
+		}) {
+			return fmt.Errorf("secret %s already owned by another nutanixCluster object", secret.Name)
 		}
-		secret.SetOwnerReferences([]metav1.OwnerReference{{
+		// Set nutanixCluster ownerRef on the secret
+		secret.OwnerReferences = capiutil.EnsureOwnerRef(secret.OwnerReferences, metav1.OwnerReference{
 			APIVersion: infrav1.GroupVersion.String(),
 			Kind:       nutanixCluster.Kind,
 			UID:        nutanixCluster.UID,
 			Name:       nutanixCluster.Name,
-		}})
+		})
 	}
 	if !ctrlutil.ContainsFinalizer(secret, infrav1.NutanixClusterCredentialFinalizer) {
 		ctrlutil.AddFinalizer(secret, infrav1.NutanixClusterCredentialFinalizer)

controllers/nutanixcluster_controller_test.go

@@ -24,9 +24,13 @@ import (
 	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/uuid"
+	capiv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	"sigs.k8s.io/cluster-api/util"
+	capiutil "sigs.k8s.io/cluster-api/util"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	ctrlutil "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 
 	infrav1 "github.com/nutanix-cloud-native/cluster-api-provider-nutanix/api/v1beta1"
 	nctx "github.com/nutanix-cloud-native/cluster-api-provider-nutanix/pkg/context"
@@ -43,23 +47,40 @@ func TestNutanixClusterReconciler(t *testing.T) {
 		const (
 			fd1Name = "fd-1"
 			fd2Name = "fd-2"
+			// To be replaced with capiv1.ClusterKind
+			clusterKind = "Cluster"
 		)
 
 		var (
 			ntnxCluster *infrav1.NutanixCluster
 			ctx         context.Context
 			fd1         infrav1.NutanixFailureDomain
 			reconciler  *NutanixClusterReconciler
+			ntnxSecret  *corev1.Secret
 			r           string
 		)
 
 		BeforeEach(func() {
 			ctx = context.Background()
-			r := util.RandomString(10)
+			r = util.RandomString(10)
+			ntnxSecret = &corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      r,
+					Namespace: corev1.NamespaceDefault,
+				},
+				StringData: map[string]string{
+					r: r,
+				},
+			}
 			ntnxCluster = &infrav1.NutanixCluster{
+				TypeMeta: metav1.TypeMeta{
+					Kind:       infrav1.NutanixClusterKind,
+					APIVersion: infrav1.GroupVersion.String(),
+				},
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      "test",
-					Namespace: "default",
+					Namespace: corev1.NamespaceDefault,
+					UID:       utilruntime.NewUUID(),
 				},
 				Spec: infrav1.NutanixClusterSpec{
 					PrismCentral: &credentialTypes.NutanixPrismEndpoint{
@@ -88,8 +109,8 @@ func TestNutanixClusterReconciler(t *testing.T) {
 		})
 
 		AfterEach(func() {
-			err := k8sClient.Delete(ctx, ntnxCluster)
-			Expect(err).NotTo(HaveOccurred())
+			// Delete ntnxCluster if exists.
+			k8sClient.Delete(ctx, ntnxCluster)
 		})
 
 		Context("Reconcile an NutanixCluster", func() {
@@ -110,7 +131,7 @@ func TestNutanixClusterReconciler(t *testing.T) {
 		})
 
 		Context("ReconcileNormal for a NutanixCluster", func() {
-			It("should not requeue if failure message is set on nutanixCluster", func() {
+			It("should not requeue if failure message is set on NutanixCluster", func() {
 				g.Expect(k8sClient.Create(ctx, ntnxCluster)).To(Succeed())
 				ntnxCluster.Status.FailureMessage = &r
 				result, err := reconciler.reconcileNormal(&nctx.ClusterContext{
@@ -215,5 +236,161 @@ func TestNutanixClusterReconciler(t *testing.T) {
 				g.Expect(appliedNtnxCluster.Status.FailureDomains).To(BeEmpty())
 			})
 		})
+		Context("Reconcile credentialRef for a NutanixCluster", func() {
+			It("should return error if already owned by another NutanixCluster", func() {
+				// Create an additional NutanixCluster object
+				additionalNtnxCluster := &infrav1.NutanixCluster{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      r,
+						Namespace: corev1.NamespaceDefault,
+					},
+					Spec: infrav1.NutanixClusterSpec{
+						PrismCentral: &credentialTypes.NutanixPrismEndpoint{
+							// Adding port info to override default value (0)
+							Port: 9440,
+						},
+					},
+				}
+				g.Expect(k8sClient.Create(ctx, additionalNtnxCluster)).To(Succeed())
+
+				// Add credential ref to the ntnxCluster resource
+				ntnxCluster.Spec.PrismCentral.CredentialRef = &credentialTypes.NutanixCredentialReference{
+					Kind:      credentialTypes.SecretKind,
+					Name:      ntnxSecret.Name,
+					Namespace: ntnxSecret.Namespace,
+				}
+
+				// Add an ownerReference for the additional NutanixCluster object
+				ntnxSecret.OwnerReferences = []metav1.OwnerReference{
+					{
+						APIVersion: infrav1.GroupVersion.String(),
+						Kind:       infrav1.NutanixClusterKind,
+						UID:        additionalNtnxCluster.UID,
+						Name:       additionalNtnxCluster.Name,
+					},
+				}
+				g.Expect(k8sClient.Create(ctx, ntnxSecret)).To(Succeed())
+
+				// Reconcile credentialRef
+				err := reconciler.reconcileCredentialRef(ctx, ntnxCluster)
+				g.Expect(err).To(HaveOccurred())
+			})
+			It("should add credentialRef and finalizer if not owned by other cluster", func() {
+				// Add credential ref to the ntnxCluster resource
+				ntnxCluster.Spec.PrismCentral.CredentialRef = &credentialTypes.NutanixCredentialReference{
+					Kind:      credentialTypes.SecretKind,
+					Name:      ntnxSecret.Name,
+					Namespace: ntnxSecret.Namespace,
+				}
+
+				// Create secret
+				g.Expect(k8sClient.Create(ctx, ntnxSecret)).To(Succeed())
+
+				// Reconcile credentialRef
+				err := reconciler.reconcileCredentialRef(ctx, ntnxCluster)
+				g.Expect(err).ToNot(HaveOccurred())
+
+				// Get latest secret status
+				g.Expect(k8sClient.Get(ctx, client.ObjectKey{
+					Namespace: ntnxSecret.Namespace,
+					Name:      ntnxSecret.Name,
+				}, ntnxSecret)).To(Succeed())
+
+				// Check if secret is owned by the NutanixCluster
+				g.Expect(capiutil.IsOwnedByObject(ntnxSecret, ntnxCluster)).To(BeTrue())
+
+				// check finalizer
+				g.Expect(ctrlutil.ContainsFinalizer(ntnxSecret, infrav1.NutanixClusterCredentialFinalizer))
+			})
+			It("does not add another credentialRef if it is already set", func() {
+				// Add credential ref to the ntnxCluster resource
+				ntnxCluster.Spec.PrismCentral.CredentialRef = &credentialTypes.NutanixCredentialReference{
+					Kind:      credentialTypes.SecretKind,
+					Name:      ntnxSecret.Name,
+					Namespace: ntnxSecret.Namespace,
+				}
+
+				// Add an ownerReference for the  NutanixCluster object
+				ntnxSecret.OwnerReferences = []metav1.OwnerReference{
+					{
+						APIVersion: infrav1.GroupVersion.String(),
+						Kind:       infrav1.NutanixClusterKind,
+						UID:        ntnxCluster.UID,
+						Name:       ntnxCluster.Name,
+					},
+				}
+
+				g.Expect(k8sClient.Create(ctx, ntnxSecret)).To(Succeed())
+
+				// Reconcile credentialRef
+				err := reconciler.reconcileCredentialRef(ctx, ntnxCluster)
+				g.Expect(err).ToNot(HaveOccurred())
+
+				// Get latest secret status
+				g.Expect(k8sClient.Get(ctx, client.ObjectKey{
+					Namespace: ntnxSecret.Namespace,
+					Name:      ntnxSecret.Name,
+				}, ntnxSecret)).To(Succeed())
+
+				// Check if secret is owned by the NutanixCluster
+				g.Expect(capiutil.IsOwnedByObject(ntnxSecret, ntnxCluster)).To(BeTrue())
+
+				// check if only one ownerReference has been added
+				g.Expect(len(ntnxSecret.OwnerReferences)).To(Equal(1))
+			})
+
+			It("allows multiple ownerReferences with different kinds", func() {
+				// Add credential ref to the ntnxCluster resource
+				ntnxCluster.Spec.PrismCentral.CredentialRef = &credentialTypes.NutanixCredentialReference{
+					Kind:      credentialTypes.SecretKind,
+					Name:      ntnxSecret.Name,
+					Namespace: ntnxSecret.Namespace,
+				}
+
+				// Add an ownerReference for a fake object
+				ntnxSecret.OwnerReferences = []metav1.OwnerReference{
+					{
+						APIVersion: capiv1.GroupVersion.String(),
+						Kind:       clusterKind,
+						UID:        ntnxCluster.UID,
+						Name:       r,
+					},
+				}
+
+				g.Expect(k8sClient.Create(ctx, ntnxSecret)).To(Succeed())
+
+				// Reconcile credentialRef
+				err := reconciler.reconcileCredentialRef(ctx, ntnxCluster)
+				g.Expect(err).ToNot(HaveOccurred())
+
+				// Get latest secret status
+				g.Expect(k8sClient.Get(ctx, client.ObjectKey{
+					Namespace: ntnxSecret.Namespace,
+					Name:      ntnxSecret.Name,
+				}, ntnxSecret)).To(Succeed())
+
+				// Check if secret is owned by the NutanixCluster
+				g.Expect(capiutil.IsOwnedByObject(ntnxSecret, ntnxCluster)).To(BeTrue())
+
+				g.Expect(len(ntnxSecret.OwnerReferences)).To(Equal(2))
+			})
+			It("should error if secret does not exist", func() {
+				// Add credential ref to the ntnxCluster resource
+				ntnxCluster.Spec.PrismCentral.CredentialRef = &credentialTypes.NutanixCredentialReference{
+					Kind:      credentialTypes.SecretKind,
+					Name:      ntnxSecret.Name,
+					Namespace: ntnxSecret.Namespace,
+				}
+
+				// Reconcile credentialRef
+				err := reconciler.reconcileCredentialRef(ctx, ntnxCluster)
+				g.Expect(err).To(HaveOccurred())
+			})
+			It("should error if NutanixCluster is nil", func() {
+				// Reconcile credentialRef
+				err := reconciler.reconcileCredentialRef(ctx, nil)
+				g.Expect(err).To(HaveOccurred())
+			})
+		})
 	})
 }