fix: create serviceaccount token for v1.24 clusters (#9546)

* fix: create serviceaccount token for v1.24 clusters Signed-off-by: Daniel Helfand <[email protected]> * change create to get in err Signed-off-by: Daniel Helfand <[email protected]>
argoproj · Jun 8, 2022 · 67bc5ee · 67bc5ee
1 parent ab1b0ec
commit 67bc5ee
Show file tree

Hide file tree

Showing 8 changed files with 236 additions and 43 deletions.
diff --git a/cmd/argocd/commands/admin/cluster.go b/cmd/argocd/commands/admin/cluster.go
@@ -609,7 +609,7 @@ func GenerateToken(clusterOpts cmdutil.ClusterOptions, conf *rest.Config) (strin
 	clientset, err := kubernetes.NewForConfig(conf)
 	errors.CheckError(err)
 
-	bearerToken, err := clusterauth.GetServiceAccountBearerToken(clientset, clusterOpts.SystemNamespace, clusterOpts.ServiceAccount)
+	bearerToken, err := clusterauth.GetServiceAccountBearerToken(clientset, clusterOpts.SystemNamespace, clusterOpts.ServiceAccount, common.BearerTokenTimeout)
 	if err != nil {
 		return "", err
 	}

diff --git a/cmd/argocd/commands/cluster.go b/cmd/argocd/commands/cluster.go
@@ -101,7 +101,7 @@ func NewClusterAddCommand(clientOpts *argocdclient.ClientOptions, pathOpts *clie
 				clientset, err := kubernetes.NewForConfig(conf)
 				errors.CheckError(err)
 				if clusterOpts.ServiceAccount != "" {
-					managerBearerToken, err = clusterauth.GetServiceAccountBearerToken(clientset, clusterOpts.SystemNamespace, clusterOpts.ServiceAccount)
+					managerBearerToken, err = clusterauth.GetServiceAccountBearerToken(clientset, clusterOpts.SystemNamespace, clusterOpts.ServiceAccount, common.BearerTokenTimeout)
 				} else {
 					isTerminal := isatty.IsTerminal(os.Stdout.Fd()) || isatty.IsCygwinTerminal(os.Stdout.Fd())
 
@@ -115,7 +115,7 @@ func NewClusterAddCommand(clientOpts *argocdclient.ClientOptions, pathOpts *clie
 							os.Exit(1)
 						}
 					}
-					managerBearerToken, err = clusterauth.InstallClusterManagerRBAC(clientset, clusterOpts.SystemNamespace, clusterOpts.Namespaces)
+					managerBearerToken, err = clusterauth.InstallClusterManagerRBAC(clientset, clusterOpts.SystemNamespace, clusterOpts.Namespaces, common.BearerTokenTimeout)
 				}
 				errors.CheckError(err)
 			}

diff --git a/common/common.go b/common/common.go
@@ -229,6 +229,12 @@ const (
 	CacheVersion = "1.8.3"
 )
 
+// Constants used by util/clusterauth package
+const (
+	ClusterAuthRequestTimeout = 10 * time.Second
+	BearerTokenTimeout        = 30 * time.Second
+)
+
 const (
 	DefaultGitRetryMaxDuration time.Duration = time.Second * 5        // 5s
 	DefaultGitRetryDuration    time.Duration = time.Millisecond * 250 // 0.25s

diff --git a/go.mod b/go.mod
@@ -217,7 +217,7 @@ require (
 	gopkg.in/square/go-jose.v2 v2.2.2 // indirect
 	gopkg.in/warnings.v0 v0.1.2 // indirect
 	gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b // indirect
-	k8s.io/apiserver v0.23.1 // indirect
+	k8s.io/apiserver v0.23.1
 	k8s.io/cli-runtime v0.23.1 // indirect
 	k8s.io/component-base v0.23.1 // indirect
 	k8s.io/component-helpers v0.23.1 // indirect

diff --git a/test/e2e/cluster_test.go b/test/e2e/cluster_test.go
@@ -184,8 +184,6 @@ func TestClusterURLInRestAPI(t *testing.T) {
 }
 
 func TestClusterDeleteDenied(t *testing.T) {
-	EnsureCleanState(t)
-
 	accountFixture.Given(t).
 		Name("test").
 		When().

diff --git a/test/e2e/fixture/cluster/actions.go b/test/e2e/fixture/cluster/actions.go
@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"log"
 
+	"github.com/argoproj/argo-cd/v2/common"
 	"github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
 	"github.com/argoproj/argo-cd/v2/util/clusterauth"
 	"k8s.io/client-go/kubernetes"
@@ -81,7 +82,7 @@ func (a *Actions) CreateWithRBAC(args ...string) *Actions {
 	}
 	client := kubernetes.NewForConfigOrDie(conf)
 
-	_, err = clusterauth.InstallClusterManagerRBAC(client, "kube-system", []string{})
+	_, err = clusterauth.InstallClusterManagerRBAC(client, "kube-system", []string{}, common.BearerTokenTimeout)
 	if err != nil {
 		a.lastError = err
 		return a

diff --git a/util/clusterauth/clusterauth.go b/util/clusterauth/clusterauth.go
@@ -2,16 +2,19 @@ package clusterauth
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
 	"strings"
 	"time"
 
+	"github.com/argoproj/argo-cd/v2/common"
 	jwt "github.com/golang-jwt/jwt/v4"
 	log "github.com/sirupsen/logrus"
 	corev1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
 	apierr "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/client-go/kubernetes"
 )
@@ -173,7 +176,7 @@ func upsertRoleBinding(clientset kubernetes.Interface, name string, roleName str
 }
 
 // InstallClusterManagerRBAC installs RBAC resources for a cluster manager to operate a cluster. Returns a token
-func InstallClusterManagerRBAC(clientset kubernetes.Interface, ns string, namespaces []string) (string, error) {
+func InstallClusterManagerRBAC(clientset kubernetes.Interface, ns string, namespaces []string, bearerTokenTimeout time.Duration) (string, error) {
 
 	err := CreateServiceAccount(clientset, ArgoCDManagerServiceAccount, ns)
 	if err != nil {
@@ -212,42 +215,123 @@ func InstallClusterManagerRBAC(clientset kubernetes.Interface, ns string, namesp
 		}
 	}
 
-	return GetServiceAccountBearerToken(clientset, ns, ArgoCDManagerServiceAccount)
+	return GetServiceAccountBearerToken(clientset, ns, ArgoCDManagerServiceAccount, bearerTokenTimeout)
 }
 
-// GetServiceAccountBearerToken will attempt to get the provided service account until it
-// exists, iterate the secrets associated with it looking for one of type
-// kubernetes.io/service-account-token, and return it's token if found.
-func GetServiceAccountBearerToken(clientset kubernetes.Interface, ns string, sa string) (string, error) {
-	var serviceAccount *corev1.ServiceAccount
+// GetServiceAccountBearerToken determines if a ServiceAccount has a
+// bearer token secret to use or if a secret should be created. It then
+// waits for the secret to have a bearer token if a secret needs to
+// be created and returns the token in encoded base64.
+func GetServiceAccountBearerToken(clientset kubernetes.Interface, ns string, sa string, timeout time.Duration) (string, error) {
+	secretName, err := getOrCreateServiceAccountTokenSecret(clientset, sa, ns)
+	if err != nil {
+		return "", err
+	}
+
 	var secret *corev1.Secret
-	var err error
-	err = wait.Poll(500*time.Millisecond, 30*time.Second, func() (bool, error) {
-		serviceAccount, err = clientset.CoreV1().ServiceAccounts(ns).Get(context.Background(), sa, metav1.GetOptions{})
+	err = wait.PollImmediate(500*time.Millisecond, timeout, func() (bool, error) {
+		ctx, cancel := context.WithTimeout(context.Background(), common.ClusterAuthRequestTimeout)
+		defer cancel()
+		secret, err = clientset.CoreV1().Secrets(ns).Get(ctx, secretName, metav1.GetOptions{})
 		if err != nil {
-			return false, err
+			return false, fmt.Errorf("failed to get secret %q for serviceaccount %q: %w", secretName, sa, err)
 		}
-		// Scan all secrets looking for one of the correct type:
-		for _, oRef := range serviceAccount.Secrets {
-			var getErr error
-			secret, err = clientset.CoreV1().Secrets(ns).Get(context.Background(), oRef.Name, metav1.GetOptions{})
-			if err != nil {
-				return false, fmt.Errorf("Failed to retrieve secret %q: %v", oRef.Name, getErr)
-			}
-			if secret.Type == corev1.SecretTypeServiceAccountToken {
-				return true, nil
-			}
+
+		_, ok := secret.Data["token"]
+		if !ok {
+			return false, nil
 		}
-		return false, nil
+
+		return true, nil
 	})
 	if err != nil {
-		return "", fmt.Errorf("Failed to wait for service account secret: %v", err)
+		return "", fmt.Errorf("failed to get token for serviceaccount %q: %w", sa, err)
 	}
-	token, ok := secret.Data["token"]
-	if !ok {
-		return "", fmt.Errorf("Secret %q for service account %q did not have a token", secret.Name, serviceAccount)
+
+	return string(secret.Data["token"]), nil
+}
+
+// getOrCreateServiceAccountTokenSecret will check if a ServiceAccount
+// already has a kubernetes.io/service-account-token secret associated
+// with it or creates one if the ServiceAccount doesn't have one. This
+// was added to help add k8s v1.24+ clusters.
+func getOrCreateServiceAccountTokenSecret(clientset kubernetes.Interface, sa, ns string) (string, error) {
+	// Wait for sa to have secret, but don't wait too
+	// long for 1.24+ clusters
+	var serviceAccount *corev1.ServiceAccount
+	err := wait.PollImmediate(500*time.Millisecond, 5*time.Second, func() (bool, error) {
+		ctx, cancel := context.WithTimeout(context.Background(), common.ClusterAuthRequestTimeout)
+		defer cancel()
+		var getErr error
+		serviceAccount, getErr = clientset.CoreV1().ServiceAccounts(ns).Get(ctx, sa, metav1.GetOptions{})
+		if getErr != nil {
+			return false, fmt.Errorf("failed to get serviceaccount %q: %w", sa, getErr)
+		}
+		if len(serviceAccount.Secrets) == 0 {
+			return false, nil
+		}
+		return true, nil
+	})
+	if err != nil && err != wait.ErrWaitTimeout {
+		return "", fmt.Errorf("failed to get serviceaccount token secret: %w", err)
 	}
-	return string(token), nil
+	if serviceAccount == nil {
+		log.Errorf("Unexpected nil serviceaccount '%s/%s' with no error returned", ns, sa)
+		return "", fmt.Errorf("failed to create serviceaccount token secret: nil serviceaccount returned for '%s/%s' with no error", ns, sa)
+	}
+
+	outerCtx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
+	defer cancel()
+	for _, s := range serviceAccount.Secrets {
+		innerCtx, cancel := context.WithTimeout(outerCtx, common.ClusterAuthRequestTimeout)
+		defer cancel()
+		existingSecret, err := clientset.CoreV1().Secrets(ns).Get(innerCtx, s.Name, metav1.GetOptions{})
+		if err != nil {
+			return "", fmt.Errorf("failed to retrieve secret %q: %w", s.Name, err)
+		}
+		if existingSecret.Type == corev1.SecretTypeServiceAccountToken {
+			return existingSecret.Name, nil
+		}
+	}
+
+	return createServiceAccountToken(clientset, serviceAccount)
+}
+
+func createServiceAccountToken(clientset kubernetes.Interface, sa *corev1.ServiceAccount) (string, error) {
+	secret := &corev1.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			GenerateName: sa.Name + "-token-",
+			Namespace:    sa.Namespace,
+			Annotations: map[string]string{
+				corev1.ServiceAccountNameKey: sa.Name,
+			},
+		},
+		Type: corev1.SecretTypeServiceAccountToken,
+	}
+
+	ctx, cancel := context.WithTimeout(context.Background(), common.ClusterAuthRequestTimeout)
+	defer cancel()
+	secret, err := clientset.CoreV1().Secrets(sa.Namespace).Create(ctx, secret, metav1.CreateOptions{})
+	if err != nil {
+		return "", fmt.Errorf("failed to create secret for serviceaccount %q: %w", sa, err)
+	}
+
+	log.Infof("Created bearer token secret for ServiceAccount %q", sa)
+	sa.Secrets = []corev1.ObjectReference{{
+		Name:      secret.Name,
+		Namespace: secret.Namespace,
+	}}
+	patch, err := json.Marshal(sa)
+	if err != nil {
+		return "", fmt.Errorf("failed marshaling patch for serviceaccount %q: %w", sa, err)
+	}
+
+	_, err = clientset.CoreV1().ServiceAccounts(sa.Namespace).Patch(ctx, sa.Name, types.StrategicMergePatchType, patch, metav1.PatchOptions{})
+	if err != nil {
+		return "", fmt.Errorf("failed to patch serviceaccount %q with bearer token secret: %w", sa, err)
+	}
+
+	return secret.Name, nil
 }
 
 // UninstallClusterManagerRBAC removes RBAC resources for a cluster manager to operate a cluster