backport secret deletion fix to v1.0.0

kubernetes-sigs · Mar 28, 2022 · fbe0971 · fbe0971
1 parent 72cf6fd
commit fbe0971
Show file tree

Hide file tree

Showing 3 changed files with 132 additions and 6 deletions.
diff --git a/controllers/vspherecluster_controller.go b/controllers/vspherecluster_controller.go
@@ -55,6 +55,10 @@ import (
 	infrautilv1 "sigs.k8s.io/cluster-api-provider-vsphere/pkg/util"
 )
 
+// legacyIdentityFinalizer is deprecated and should be used only while upgrading the cluster
+// from v1alpha3(v.0.7).
+const legacyIdentityFinalizer string = "identity/infrastructure.cluster.x-k8s.io"
+
 var (
 	clusterControlledType     = &infrav1.VSphereCluster{}
 	clusterControlledTypeName = reflect.TypeOf(clusterControlledType).Elem().Name()
@@ -268,8 +272,13 @@ func (r clusterReconciler) reconcileDelete(ctx *context.ClusterContext) (reconci
 			}
 			return reconcile.Result{}, err
 		}
-		r.Logger.Info(fmt.Sprintf("Removing finalizer form Secret %s/%s", secret.Namespace, secret.Name))
+		r.Logger.Info(fmt.Sprintf("Removing finalizer from Secret %s/%s", secret.Namespace, secret.Name))
 		ctrlutil.RemoveFinalizer(secret, infrav1.SecretIdentitySetFinalizer)
+		// Check if the old finalizer(from v0.7) is present, if yes, delete it
+		// For more context, please refer: https://github.com/kubernetes-sigs/cluster-api-provider-vsphere/issues/1482
+		if ctrlutil.ContainsFinalizer(secret, legacyIdentityFinalizer) {
+			ctrlutil.RemoveFinalizer(secret, legacyIdentityFinalizer)
+		}
 		if err := ctx.Client.Update(ctx, secret); err != nil {
 			return reconcile.Result{}, err
 		}

diff --git a/controllers/vspherecluster_controller_test.go b/controllers/vspherecluster_controller_test.go
@@ -25,7 +25,6 @@ import (
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 	"github.com/vmware/govmomi/simulator"
-
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -238,6 +237,118 @@ var _ = Describe("ClusterReconciler", func() {
 		})
 	})
 
+	Context("Reconcile delete", func() {
+		var (
+			ctx             context.Context
+			secret          *corev1.Secret
+			capiCluster     *clusterv1.Cluster
+			instance        *infrav1.VSphereCluster
+			legacyFinalizer = "identity/infrastructure.cluster.x-k8s.io"
+			key             client.ObjectKey
+		)
+		It("should remove legacy finalizer if present during the cluster deletion", func() {
+			ctx = context.Background()
+			capiCluster = &clusterv1.Cluster{
+				ObjectMeta: metav1.ObjectMeta{
+					GenerateName: "test1-",
+					Namespace:    "default",
+				},
+				Spec: clusterv1.ClusterSpec{
+					InfrastructureRef: &corev1.ObjectReference{
+						APIVersion: infrav1.GroupVersion.String(),
+						Kind:       "VsphereCluster",
+						Name:       "vsphere-test1",
+					},
+				},
+			}
+			defer func() {
+				Expect(testEnv.Cleanup(ctx, capiCluster)).To(Succeed())
+			}()
+
+			// Create the CAPI cluster (owner) object
+			Expect(testEnv.Create(ctx, capiCluster)).To(Succeed())
+
+			fakeVCenter := startVcenter()
+			vcURL := fakeVCenter.ServerURL()
+			defer fakeVCenter.Destroy()
+			// Create the secret containing the credentials
+			password, _ := vcURL.User.Password()
+
+			secret = &corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					GenerateName: "secret-",
+					Namespace:    "default",
+					Finalizers:   []string{legacyFinalizer},
+				},
+				Data: map[string][]byte{
+					identity.UsernameKey: []byte(vcURL.User.Username()),
+					identity.PasswordKey: []byte(password),
+				},
+			}
+			Expect(testEnv.Create(ctx, secret)).To(Succeed())
+			Eventually(func() error {
+				secretKey := client.ObjectKey{Namespace: secret.Namespace, Name: secret.Name}
+				return testEnv.Get(ctx, secretKey, secret)
+			}).Should(BeNil())
+
+			// Create the VSphereCluster object
+			instance = &infrav1.VSphereCluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "vsphere-test1",
+					Namespace: "default",
+				},
+				Spec: infrav1.VSphereClusterSpec{
+					IdentityRef: &infrav1.VSphereIdentityReference{
+						Kind: infrav1.SecretKind,
+						Name: secret.Name,
+					},
+					Server: fmt.Sprintf("%s://%s", vcURL.Scheme, vcURL.Host),
+				},
+			}
+			Expect(testEnv.Create(ctx, instance)).To(Succeed())
+
+			// Make sure the VSphereCluster exists.
+			key = client.ObjectKey{Namespace: instance.Namespace, Name: instance.Name}
+			Eventually(func() error {
+				return testEnv.Get(ctx, key, instance)
+			}, timeout).Should(BeNil())
+
+			By("setting the OwnerRef on the VSphereCluster")
+			Eventually(func() bool {
+				ph, err := patch.NewHelper(instance, testEnv)
+				Expect(err).ShouldNot(HaveOccurred())
+				instance.OwnerReferences = append(instance.OwnerReferences, metav1.OwnerReference{Kind: "Cluster", APIVersion: clusterv1.GroupVersion.String(), Name: capiCluster.Name, UID: "blah"})
+				Expect(ph.Patch(ctx, instance, patch.WithStatusObservedGeneration{})).ShouldNot(HaveOccurred())
+				return true
+			}, timeout).Should(BeTrue())
+
+			By("setting the VSphereCluster's VCenterAvailableCondition to true")
+			Eventually(func() bool {
+				if err := testEnv.Get(ctx, key, instance); err != nil {
+					return false
+				}
+				return conditions.IsTrue(instance, infrav1.VCenterAvailableCondition)
+			}, timeout).Should(BeTrue())
+
+			By("deleting the vspherecluster which has the secret with legacy finalizer")
+			Eventually(func() error {
+				return testEnv.Delete(ctx, instance)
+			}, timeout).Should(BeNil())
+			// confirm that the VSphereCluster is deleted
+			Eventually(func() bool {
+				err := testEnv.Get(ctx, key, instance)
+				return apierrors.IsNotFound(err)
+			}, timeout).Should(BeTrue())
+
+			By("checking that the secret is deleted")
+			secretKey := client.ObjectKey{Namespace: secret.Namespace, Name: secret.Name}
+			Eventually(func() bool {
+				err := testEnv.Get(ctx, secretKey, secret)
+				return apierrors.IsNotFound(err)
+			}, timeout).Should(BeTrue())
+		})
+	})
+
 	It("should remove vspherecluster finalizer if the secret does not exist", func() {
 		ctx := context.Background()
 

diff --git a/controllers/vsphereclusteridentity_controller.go b/controllers/vsphereclusteridentity_controller.go
@@ -26,9 +26,6 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	infrav1 "sigs.k8s.io/cluster-api-provider-vsphere/api/v1beta1"
-	"sigs.k8s.io/cluster-api-provider-vsphere/pkg/context"
-	"sigs.k8s.io/cluster-api-provider-vsphere/pkg/record"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	clusterutilv1 "sigs.k8s.io/cluster-api/util"
 	"sigs.k8s.io/cluster-api/util/conditions"
@@ -39,6 +36,10 @@ import (
 	ctrlutil "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
+
+	infrav1 "sigs.k8s.io/cluster-api-provider-vsphere/api/v1beta1"
+	"sigs.k8s.io/cluster-api-provider-vsphere/pkg/context"
+	"sigs.k8s.io/cluster-api-provider-vsphere/pkg/record"
 )
 
 var (
@@ -169,7 +170,12 @@ func (r clusterIdentityReconciler) reconcileDelete(ctx _context.Context, identit
 		}
 		return reconcile.Result{}, err
 	}
-	r.Logger.Info(fmt.Sprintf("Removing finalizer form Secret %s/%s", secret.Namespace, secret.Name))
+	r.Logger.Info(fmt.Sprintf("Removing finalizer from Secret %s/%s", secret.Namespace, secret.Name))
+	// Check if the old finalizer(from v0.7) is present, if yes, delete it
+	// For more context, please refer: https://github.com/kubernetes-sigs/cluster-api-provider-vsphere/issues/1482
+	if ctrlutil.ContainsFinalizer(secret, legacyIdentityFinalizer) {
+		ctrlutil.RemoveFinalizer(secret, legacyIdentityFinalizer)
+	}
 	ctrlutil.RemoveFinalizer(secret, infrav1.SecretIdentitySetFinalizer)
 	if err := r.Client.Update(ctx, secret); err != nil {
 		return reconcile.Result{}, err