Merge pull request #1665 from srm09/automated-cherry-pick-of-#1653-release-1.3

Automated cherry pick of #1653: Creates token secret for service account

Author: k8s-ci-robot

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)