Creates token secret for service account

This patch changes the default behavior of waiting for the generation of
a default secret for the service account and propagate that over to the
guest cluster.
Instead, alongwith the service account a secret of type
"kubernetes.io/service-account-token" is created to host the token for
the service account. The controller waits for the realization of this
secret and then propagates this data over to the guest cluster.

This is required since 1.24 disables the auto generation of a default
token secret for every service account created.

Signed-off-by: Sagar Muchhal <[email protected]>

Author: srm09

controllers/serviceaccount_controller.go

@@ -89,9 +89,10 @@ func AddServiceAccountProviderControllerToManager(ctx *context.ControllerManager
 			&source.Kind{Type: &vmwarev1.ProviderServiceAccount{}},
 			handler.EnqueueRequestsFromMapFunc(r.providerServiceAccountToVSphereCluster),
 		).
+		// Watches the secrets to re-enqueue once the service account token is set
 		Watches(
-			&source.Kind{Type: &corev1.ServiceAccount{}},
-			handler.EnqueueRequestsFromMapFunc(r.serviceAccountToVSphereCluster),
+			&source.Kind{Type: &corev1.Secret{}},
+			handler.EnqueueRequestsFromMapFunc(r.secretToVSphereCluster),
 		).
 		Complete(r)
 }
@@ -228,18 +229,22 @@ func (r ServiceAccountReconciler) ensureProviderServiceAccounts(ctx *vmwareconte
 		if err := r.ensureServiceAccountConfigMap(ctx.ClusterContext, pSvcAccount); err != nil {
 			return errors.Wrapf(err, "unable to sync configmap for provider serviceaccount %s", pSvcAccount.Name)
 		}
+		// 3. Create secret of Service account token type for the service account
+		if err := r.ensureServiceAccountSecret(ctx.ClusterContext, pSvcAccount); err != nil {
+			return errors.Wrapf(err, "unable to create provider serviceaccount secret %s", getServiceAccountSecretName(pSvcAccount))
+		}
 
-		// 3. Create the associated role for the service account
+		// 4. Create the associated role for the service account
 		if err := r.ensureRole(ctx.ClusterContext, pSvcAccount); err != nil {
 			return errors.Wrapf(err, "unable to create role for provider serviceaccount %s", pSvcAccount.Name)
 		}
 
-		// 4. Create the associated roleBinding for the service account
+		// 5. Create the associated roleBinding for the service account
 		if err := r.ensureRoleBinding(ctx.ClusterContext, pSvcAccount); err != nil {
 			return errors.Wrapf(err, "unable to create rolebinding for provider serviceaccount %s", pSvcAccount.Name)
 		}
 
-		// 5. Sync the service account with the target
+		// 6. Sync the service account with the target
 		if err := r.syncServiceAccountSecret(ctx, pSvcAccount); err != nil {
 			return errors.Wrapf(err, "unable to sync secret for provider serviceaccount %s", pSvcAccount.Name)
 		}
@@ -269,6 +274,34 @@ func (r ServiceAccountReconciler) ensureServiceAccount(ctx *vmwarecontext.Cluste
 	return nil
 }
 
+func (r ServiceAccountReconciler) ensureServiceAccountSecret(ctx *vmwarecontext.ClusterContext, pSvcAccount vmwarev1.ProviderServiceAccount) error {
+	secret := corev1.Secret{
+		Type: corev1.SecretTypeServiceAccountToken,
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      getServiceAccountSecretName(pSvcAccount),
+			Namespace: pSvcAccount.Namespace,
+			Annotations: map[string]string{
+				// denotes that this secret holds the token for the service account
+				corev1.ServiceAccountNameKey: getServiceAccountName(pSvcAccount),
+			},
+		},
+	}
+
+	logger := ctx.Logger.WithValues("providerserviceaccount", pSvcAccount.Name, "secret", secret.Name)
+	err := controllerutil.SetControllerReference(&pSvcAccount, &secret, ctx.Scheme)
+	if err != nil {
+		return err
+	}
+	logger.V(4).Info("Creating service account secret")
+	err = ctx.Client.Create(ctx, &secret)
+	if err != nil && !apierrors.IsAlreadyExists(err) {
+		// Note: We skip updating the service account because the token controller updates the service account with a
+		// secret and we don't want to overwrite it with an empty secret.
+		return err
+	}
+	return nil
+}
+
 func (r ServiceAccountReconciler) ensureRole(ctx *vmwarecontext.ClusterContext, pSvcAccount vmwarev1.ProviderServiceAccount) error {
 	role := rbacv1.Role{
 		ObjectMeta: metav1.ObjectMeta{
@@ -323,29 +356,23 @@ func (r ServiceAccountReconciler) ensureRoleBinding(ctx *vmwarecontext.ClusterCo
 
 func (r ServiceAccountReconciler) syncServiceAccountSecret(ctx *vmwarecontext.GuestClusterContext, pSvcAccount vmwarev1.ProviderServiceAccount) error {
 	logger := ctx.Logger.WithValues("providerserviceaccount", pSvcAccount.Name)
-	logger.V(4).Info("Attempting to sync secret for provider service account")
-	var svcAccount corev1.ServiceAccount
-	err := ctx.Client.Get(ctx, types.NamespacedName{Name: getServiceAccountName(pSvcAccount), Namespace: pSvcAccount.Namespace}, &svcAccount)
+	logger.V(4).Info("Attempting to sync token secret for provider service account")
+
+	secretName := getServiceAccountSecretName(pSvcAccount)
+	logger.V(4).Info("Fetching secret for service account token details", "secret", secretName)
+	var svcAccountTokenSecret corev1.Secret
+	err := ctx.Client.Get(ctx, types.NamespacedName{Name: secretName, Namespace: pSvcAccount.Namespace}, &svcAccountTokenSecret)
 	if err != nil {
 		return err
 	}
-	// Check if token secret exists
-	if len(svcAccount.Secrets) == 0 {
-		// Note: We don't have to requeue here because we have a watch on the service account and the cluster should be reconciled
-		// when a secret is added to the service account by the token controller.
-		logger.Info("Skipping sync secret for provider service account: serviceaccount has no secrets", "serviceaccount", svcAccount.Name)
+	// Check if token data exists
+	if len(svcAccountTokenSecret.Data) == 0 {
+		// Note: We don't have to requeue here because we have a watch on the secret and the cluster should be reconciled
+		// when a secret has token data populated.
+		logger.Info("Skipping sync secret for provider service account: secret has no data", "secret", secretName)
 		return nil
 	}
 
-	// Choose the default secret
-	secretRef := svcAccount.Secrets[0]
-	logger.V(4).Info("Fetching secret for provider service account", "secret", secretRef.Name)
-	var sourceSecret corev1.Secret
-	err = ctx.Client.Get(ctx, types.NamespacedName{Name: secretRef.Name, Namespace: svcAccount.Namespace}, &sourceSecret)
-	if err != nil {
-		return err
-	}
-
 	// Create the target namespace if it is not existing
 	targetNamespace := &corev1.Namespace{
 		ObjectMeta: metav1.ObjectMeta{
@@ -372,7 +399,7 @@ func (r ServiceAccountReconciler) syncServiceAccountSecret(ctx *vmwarecontext.Gu
 	}
 	logger.V(4).Info("Creating or updating secret in cluster", "namespace", targetSecret.Namespace, "name", targetSecret.Name)
 	_, err = controllerutil.CreateOrPatch(ctx, ctx.GuestClient, targetSecret, func() error {
-		targetSecret.Data = sourceSecret.Data
+		targetSecret.Data = svcAccountTokenSecret.Data
 		return nil
 	})
 	return err
@@ -468,6 +495,10 @@ func getServiceAccountName(pSvcAccount vmwarev1.ProviderServiceAccount) string {
 	return pSvcAccount.Name
 }
 
+func getServiceAccountSecretName(pSvcAccount vmwarev1.ProviderServiceAccount) string {
+	return fmt.Sprintf("%s-secret", pSvcAccount.Name)
+}
+
 func getSystemServiceAccountFullName(pSvcAccount vmwarev1.ProviderServiceAccount) string {
 	return fmt.Sprintf("%s.%s.%s", systemServiceAccountPrefix, getServiceAccountNamespace(pSvcAccount), getServiceAccountName(pSvcAccount))
 }
@@ -484,6 +515,33 @@ func GetCMNamespaceName() types.NamespacedName {
 	}
 }
 
+// secretToVSphereCluster is a mapper function used to enqueue reconcile.Request objects.
+// It accepts a Secret object owned by the controller and fetches the service account
+// that contains the token and creates a reconcile.Request for the vmwarev1.VSphereCluster object.
+func (r ServiceAccountReconciler) secretToVSphereCluster(o client.Object) []reconcile.Request {
+	secret, ok := o.(*corev1.Secret)
+	if !ok {
+		return nil
+	}
+
+	ownerRef := metav1.GetControllerOf(secret)
+	if ownerRef != nil && ownerRef.Kind == kindProviderServiceAccount {
+		if !metav1.HasAnnotation(secret.ObjectMeta, corev1.ServiceAccountNameKey) {
+			return nil
+		}
+		svcAccountName := secret.GetAnnotations()[corev1.ServiceAccountNameKey]
+		svcAccount := &corev1.ServiceAccount{}
+		if err := r.Client.Get(r.Context, client.ObjectKey{
+			Namespace: secret.Namespace,
+			Name:      svcAccountName,
+		}, svcAccount); err != nil {
+			return nil
+		}
+		return r.serviceAccountToVSphereCluster(svcAccount)
+	}
+	return nil
+}
+
 // serviceAccountToVSphereCluster is a mapper function used to enqueue reconcile.Request objects.
 // From the watched object owned by this controller, it creates reconcile.Request object
 // for the vmwarev1.VSphereCluster object that owns the watched object.

controllers/serviceaccount_controller_suite_test.go

@@ -18,6 +18,7 @@ package controllers
 
 import (
 	goctx "context"
+	"fmt"
 	"strings"
 	"time"
 
@@ -35,13 +36,10 @@ import (
 )
 
 const (
-	testProviderSvcAccountName = "test-pvcsi"
-
-	testTargetNS             = "test-pvcsi-system"
-	testTargetSecret         = "test-pvcsi-secret" // nolint:gosec
-	testSvcAccountSecretName = testProviderSvcAccountName + "-token-abcdef"
-	testSystemSvcAcctNs      = "test-system-svc-acct-namespace"
-	testSystemSvcAcctCM      = "test-system-svc-acct-cm"
+	testTargetNS        = "test-pvcsi-system"
+	testTargetSecret    = "test-pvcsi-secret" //nolint:gosec
+	testSystemSvcAcctNs = "test-system-svc-acct-namespace"
+	testSystemSvcAcctCM = "test-system-svc-acct-cm"
 
 	testSecretToken = "ZXlKaGJHY2lPaUpTVXpJMU5pSXNJbXRwWkNJNklp" // nolint:gosec
 )
@@ -94,15 +92,14 @@ func assertNoEntities(ctx goctx.Context, ctrlClient client.Client, namespace str
 func assertServiceAccountAndUpdateSecret(ctx goctx.Context, ctrlClient client.Client, namespace, name string) {
 	svcAccount := &corev1.ServiceAccount{}
 	assertEventuallyExistsInNamespace(ctx, ctrlClient, namespace, name, svcAccount)
-	// Update the service account with a prototype secret
-	secret := getTestSvcAccountSecret(namespace, testSvcAccountSecretName)
-	Expect(ctrlClient.Create(ctx, secret)).To(Succeed())
-	svcAccount.Secrets = []corev1.ObjectReference{
-		{
-			Name: testSvcAccountSecretName,
-		},
+	secret := &corev1.Secret{}
+	assertEventuallyExistsInNamespace(ctx, ctrlClient, namespace, fmt.Sprintf("%s-secret", name), secret)
+
+	// Update the data on the secret
+	secret.Data = map[string][]byte{
+		"token": []byte(testSecretToken),
 	}
-	Expect(ctrlClient.Update(ctx, svcAccount)).To(Succeed())
+	Expect(ctrlClient.Update(ctx, secret)).To(Succeed())
 }
 
 func assertTargetSecret(ctx goctx.Context, guestClient client.Client, namespace, name string) { // nolint
@@ -159,9 +156,8 @@ func assertRoleBinding(_ *helpers.UnitTestContextForController, ctrlClient clien
 	}))
 }
 
-// nolint
-func assertProviderServiceAccountsCondition(vCluster *vmwarev1.VSphereCluster, status corev1.ConditionStatus,
-	message string, reason string, severity clusterv1.ConditionSeverity) {
+// assertProviderServiceAccountsCondition asserts the condition on the ProviderServiceAccount CR.
+func assertProviderServiceAccountsCondition(vCluster *vmwarev1.VSphereCluster, status corev1.ConditionStatus, message string, reason string, severity clusterv1.ConditionSeverity) { //nolint
 	c := conditions.Get(vCluster, vmwarev1.ProviderServiceAccountsReadyCondition)
 	Expect(c).NotTo(BeNil())
 	Expect(c.Status).To(Equal(status))
@@ -246,18 +242,6 @@ func getSystemServiceAccountsConfigMap(namespace, name string) *corev1.ConfigMap
 	}
 }
 
-func getTestSvcAccountSecret(namespace, name string) *corev1.Secret {
-	return &corev1.Secret{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      name,
-			Namespace: namespace,
-		},
-		Data: map[string][]byte{
-			"token": []byte(testSecretToken),
-		},
-	}
-}
-
 func getTestRoleWithGetPod(namespace, name string) *rbacv1.Role {
 	return &rbacv1.Role{
 		ObjectMeta: metav1.ObjectMeta{

controllers/serviceaccount_controller_unit_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 package controllers
 
 import (
+	"fmt"
 	"os"
 
 	. "github.com/onsi/ginkgo"
@@ -79,6 +80,16 @@ func unitTestsReconcileNormal() {
 				getTestProviderServiceAccount(namespace, vsphereCluster, false),
 			}
 		})
+		It("should create a service account and a secret", func() {
+			_, err := reconciler.ReconcileNormal(ctx.GuestClusterContext)
+			Expect(err).NotTo(HaveOccurred())
+
+			svcAccount := &corev1.ServiceAccount{}
+			assertEventuallyExistsInNamespace(ctx, ctx.Client, namespace, vsphereCluster.GetName(), svcAccount)
+
+			secret := &corev1.Secret{}
+			assertEventuallyExistsInNamespace(ctx, ctx.Client, namespace, fmt.Sprintf("%s-secret", vsphereCluster.GetName()), secret)
+		})
 		Context("When serviceaccount secret is created", func() {
 			It("Should reconcile", func() {
 				assertTargetNamespace(ctx, ctx.GuestClient, testTargetNS, false)