diff --git a/pkg/agent/cluster/cluster.go b/pkg/agent/cluster/cluster.go index 0d75133934d..fdaacea403c 100644 --- a/pkg/agent/cluster/cluster.go +++ b/pkg/agent/cluster/cluster.go @@ -74,9 +74,9 @@ func getTokenFromAPI() ([]byte, []byte, error) { } return secret.Data[coreV1.ServiceAccountRootCAKey], secret.Data[coreV1.ServiceAccountTokenKey], nil } - secret, err := serviceaccounttoken.CreateSecretForServiceAccount(context.Background(), k8s, sa) + secret, err := serviceaccounttoken.EnsureSecretForServiceAccount(context.Background(), nil, k8s, sa) if err != nil { - return nil, nil, fmt.Errorf("failed to create secret for service account %s/%s: %w", namespace.System, "cattle", err) + return nil, nil, fmt.Errorf("failed to ensure secret for service account %s/%s: %w", namespace.System, "cattle", err) } return []byte(cm.Data["ca.crt"]), []byte(secret.Data[coreV1.ServiceAccountTokenKey]), nil } diff --git a/pkg/api/norman/customization/monitor/handler_utils.go b/pkg/api/norman/customization/monitor/handler_utils.go index 26b2d2fec42..35648e951c2 100644 --- a/pkg/api/norman/customization/monitor/handler_utils.go +++ b/pkg/api/norman/customization/monitor/handler_utils.go @@ -187,9 +187,9 @@ func getAuthToken(userContext *config.UserContext, appName, namespace string) (s return "", fmt.Errorf("get service account %s:%s for monitor failed, %v", namespace, appName, err) } - secret, err := serviceaccounttoken.CreateSecretForServiceAccount(context.TODO(), userContext.K8sClient, sa) + secret, err := serviceaccounttoken.EnsureSecretForServiceAccount(context.Background(), nil, userContext.K8sClient, sa) if err != nil { - return "", fmt.Errorf("get secret from service account %s:%s for monitor failed: %v", namespace, appName, err) + return "", fmt.Errorf("ensure secret from service account %s:%s for monitor failed: %w", namespace, appName, err) } return string(secret.Data[v1.ServiceAccountTokenKey]), nil diff --git a/pkg/controllers/dashboard/apiservice/apiservice.go b/pkg/controllers/dashboard/apiservice/apiservice.go index a738da51beb..eb11375c52b 100644 --- a/pkg/controllers/dashboard/apiservice/apiservice.go +++ b/pkg/controllers/dashboard/apiservice/apiservice.go @@ -4,6 +4,7 @@ import ( "context" "crypto/sha256" "encoding/base64" + "fmt" v3 "github.com/rancher/rancher/pkg/apis/management.cattle.io/v3" mgmtcontrollers "github.com/rancher/rancher/pkg/generated/controllers/management.cattle.io/v3" @@ -16,10 +17,12 @@ import ( "github.com/rancher/wrangler/pkg/name" "github.com/rancher/wrangler/pkg/relatedresource" corev1 "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" apierror "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/kubernetes" ) type handler struct { @@ -32,6 +35,7 @@ type handler struct { apiServices mgmtcontrollers.APIServiceCache services corev1controllers.ServiceCache embedded bool + k8s kubernetes.Interface ctx context.Context } @@ -46,6 +50,7 @@ func Register(ctx context.Context, context *wrangler.Context, embedded bool) { apiServices: context.Mgmt.APIService().Cache(), services: context.Core.Service().Cache(), embedded: embedded, + k8s: context.K8s, ctx: ctx, } @@ -142,35 +147,12 @@ func (h *handler) getToken(sa *corev1.ServiceAccount) (string, error) { return "", err } - // create a secret-based token for the service account - sName := serviceaccounttoken.ServiceAccountSecretName(sa) - secret, err := h.secretsCache.Get(sa.Namespace, sName) + // create a secret-based token for the service account if one does not exist + secret, err := serviceaccounttoken.EnsureSecretForServiceAccount(h.ctx, h.secretsCache.Get, h.k8s, sa) if err != nil { - if !apierror.IsNotFound(err) { - return "", err - } - sc := serviceaccounttoken.SecretTemplate(sa) - secret, err = h.secretsClient.Create(sc) - if err != nil { - if !apierror.IsAlreadyExists(err) { - return "", err - } - secret, err = h.secretsCache.Get(sa.Namespace, sName) - if err != nil { - if !apierror.IsNotFound(err) { - return "", err - } - secret, err = h.secretsClient.Get(sa.Namespace, sName, metav1.GetOptions{}) - if err != nil { - return "", err - } - } - } - } - token := secret.Data["token"] - if len(token) == 0 { - return "", nil + return "", fmt.Errorf("error ensuring secret for service account [%s:%s]: %w", sa.Namespace, sa.Name, err) } + token := secret.Data[v1.ServiceAccountTokenKey] hash := sha256.Sum256(token) return base64.StdEncoding.EncodeToString(hash[:]), nil diff --git a/pkg/controllers/provisioningv2/rke2/bootstrap/controller.go b/pkg/controllers/provisioningv2/rke2/bootstrap/controller.go index 060da83792e..42b3879018d 100644 --- a/pkg/controllers/provisioningv2/rke2/bootstrap/controller.go +++ b/pkg/controllers/provisioningv2/rke2/bootstrap/controller.go @@ -25,6 +25,7 @@ import ( apierror "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/kubernetes" capi "sigs.k8s.io/cluster-api/api/v1beta1" ) @@ -41,6 +42,7 @@ type handler struct { deploymentCache appcontrollers.DeploymentCache rkeControlPlanes rkecontroller.RKEControlPlaneCache rkeBootstrapClient rkecontroller.RKEBootstrapClient + k8s kubernetes.Interface } func Register(ctx context.Context, clients *wrangler.Context) { @@ -53,6 +55,7 @@ func Register(ctx context.Context, clients *wrangler.Context) { deploymentCache: clients.Apps.Deployment().Cache(), rkeControlPlanes: clients.RKE.RKEControlPlane().Cache(), rkeBootstrapClient: clients.RKE.RKEBootstrap(), + k8s: clients.K8s, } rkecontroller.RegisterRKEBootstrapGeneratingHandler(ctx, clients.RKE.RKEBootstrap(), @@ -102,23 +105,9 @@ func (h *handler) getBootstrapSecret(namespace, name string, envVars []corev1.En return nil, err } - sName := serviceaccounttoken.ServiceAccountSecretName(sa) - secret, err := h.secretCache.Get(sa.Namespace, sName) + secret, err := serviceaccounttoken.EnsureSecretForServiceAccount(context.Background(), h.secretCache.Get, h.k8s, sa) if err != nil { - if !apierror.IsNotFound(err) { - return nil, err - } - sc := serviceaccounttoken.SecretTemplate(sa) - secret, err = h.secretClient.Create(sc) - if err != nil { - if !apierror.IsAlreadyExists(err) { - return nil, err - } - secret, err = h.secretClient.Get(sa.Namespace, sName, metav1.GetOptions{}) - if err != nil { - return nil, err - } - } + return nil, err } hash := sha256.Sum256(secret.Data["token"]) diff --git a/pkg/controllers/provisioningv2/rke2/bootstrap/controller_test.go b/pkg/controllers/provisioningv2/rke2/bootstrap/controller_test.go index ea8c2684582..c238d6ed8ff 100644 --- a/pkg/controllers/provisioningv2/rke2/bootstrap/controller_test.go +++ b/pkg/controllers/provisioningv2/rke2/bootstrap/controller_test.go @@ -19,6 +19,7 @@ import ( v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" + "k8s.io/client-go/kubernetes/fake" "sigs.k8s.io/cluster-api/api/v1beta1" ) @@ -135,6 +136,7 @@ func Test_getBootstrapSecret(t *testing.T) { secretCache: getSecretCacheMock(tt.args.namespaceName, tt.args.secretName), deploymentCache: getDeploymentCacheMock(), machineCache: getMachineCacheMock(tt.args.namespaceName, tt.args.os), + k8s: fake.NewSimpleClientset(), } //act @@ -248,7 +250,7 @@ func getDeploymentCacheMock() *deploymentCacheMock { func getSecretCacheMock(namespace, secretName string) *secretCacheMock { mockSecretCache := new(secretCacheMock) - mockSecretCache.On("Get", namespace, secretName+"-token").Return(&v1.Secret{ + mockSecretCache.On("Get", namespace, secretName).Return(&v1.Secret{ TypeMeta: metav1.TypeMeta{}, ObjectMeta: metav1.ObjectMeta{ Name: secretName, diff --git a/pkg/controllers/provisioningv2/rke2/common.go b/pkg/controllers/provisioningv2/rke2/common.go index 0c55691117d..2fbe877b3ea 100644 --- a/pkg/controllers/provisioningv2/rke2/common.go +++ b/pkg/controllers/provisioningv2/rke2/common.go @@ -22,9 +22,9 @@ import ( "github.com/rancher/wrangler/pkg/generic" "github.com/rancher/wrangler/pkg/name" corev1 "k8s.io/api/core/v1" - apierror "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" + "k8s.io/client-go/kubernetes" capi "sigs.k8s.io/cluster-api/api/v1beta1" capierrors "sigs.k8s.io/cluster-api/errors" ) @@ -231,52 +231,17 @@ func GetPlanSecretName(planSA *corev1.ServiceAccount) (string, error) { // GetPlanServiceAccountTokenSecret retrieves the secret that corresponds to the plan service account that is passed in. It will create a secret if one does not // already exist for the plan service account. -func GetPlanServiceAccountTokenSecret(secretClient corecontrollers.SecretController, planSA *corev1.ServiceAccount) (*corev1.Secret, bool, error) { +func GetPlanServiceAccountTokenSecret(secretClient corecontrollers.SecretController, k8s kubernetes.Interface, planSA *corev1.ServiceAccount) (*corev1.Secret, bool, error) { if planSA == nil { return nil, false, fmt.Errorf("planSA was nil") } - sName := serviceaccounttoken.ServiceAccountSecretName(planSA) - secret, err := secretClient.Cache().Get(planSA.Namespace, sName) + secret, err := serviceaccounttoken.EnsureSecretForServiceAccount(context.Background(), secretClient.Cache().Get, k8s, planSA) if err != nil { - if !apierror.IsNotFound(err) { - return nil, false, err - } - sc := serviceaccounttoken.SecretTemplate(planSA) - secret, err = secretClient.Create(sc) - if err != nil { - if !apierror.IsAlreadyExists(err) { - return nil, false, err - } - secret, err = secretClient.Cache().Get(planSA.Namespace, sName) - if err != nil { - return nil, false, err - } - } - } - // wait for token to be populated - if !PlanServiceAccountTokenReady(planSA, secret) { - return secret, true, fmt.Errorf("planSA %s/%s token secret %s/%s was not ready for consumption yet", planSA.Namespace, planSA.Name, secret.Namespace, secret.Name) + return nil, false, fmt.Errorf("error ensuring secret for service account [%s:%s]: %w", planSA.Namespace, planSA.Name, err) } return secret, true, nil } -func PlanServiceAccountTokenReady(planSA *corev1.ServiceAccount, tokenSecret *corev1.Secret) bool { - if planSA == nil || tokenSecret == nil { - return false - } - if tokenSecret.Name != serviceaccounttoken.ServiceAccountSecretName(planSA) { - return false - } - if v, ok := tokenSecret.Data[corev1.ServiceAccountTokenKey]; ok { - if len(v) == 0 { - return false - } - } else { - return false - } - return true -} - func PlanSecretFromBootstrapName(bootstrapName string) string { return name.SafeConcatName(bootstrapName, "machine", "plan") } diff --git a/pkg/impersonation/impersonation.go b/pkg/impersonation/impersonation.go index 21e5bd38336..bb0b1d06cb9 100644 --- a/pkg/impersonation/impersonation.go +++ b/pkg/impersonation/impersonation.go @@ -2,6 +2,7 @@ package impersonation import ( + "context" "fmt" "reflect" "sort" @@ -100,28 +101,9 @@ func (i *Impersonator) SetUpImpersonation() (*corev1.ServiceAccount, error) { // GetToken accepts a service account and returns the service account's token. func (i *Impersonator) GetToken(sa *corev1.ServiceAccount) (string, error) { - name := serviceaccounttoken.ServiceAccountSecretName(sa) - secret, err := i.clusterContext.Core.Secrets(ImpersonationNamespace).Get(name, metav1.GetOptions{}) + secret, err := serviceaccounttoken.EnsureSecretForServiceAccount(context.Background(), i.clusterContext.Core.Secrets("").Controller().Lister().Get, i.clusterContext.K8sClient, sa) if err != nil { - if !apierrors.IsNotFound(err) { - logrus.Tracef("impersonation: using context for cluster %s", i.clusterContext.ClusterName) - return "", fmt.Errorf("error getting secret: %w", err) - } - // create the secret - sc := serviceaccounttoken.SecretTemplate(sa) - secret, err = i.clusterContext.Core.Secrets(ImpersonationNamespace).Create(sc) - if err != nil { - if !apierrors.IsAlreadyExists(err) { - logrus.Tracef("impersonation: using context for cluster %s", i.clusterContext.ClusterName) - return "", fmt.Errorf("error creating secret: %w", err) - } - secret, err = i.clusterContext.Core.Secrets(ImpersonationNamespace).Get(name, metav1.GetOptions{}) - if err != nil { - logrus.Tracef("impersonation: using context for cluster %s", i.clusterContext.ClusterName) - logrus.Tracef("impersonation: secrret already exists") - return "", fmt.Errorf("error getting secret: %w", err) - } - } + return "", fmt.Errorf("error getting secret: %w", err) } token, ok := secret.Data["token"] if !ok { @@ -156,6 +138,9 @@ func (i *Impersonator) getServiceAccount() (*corev1.ServiceAccount, error) { func (i *Impersonator) createServiceAccount(role *rbacv1.ClusterRole) (*corev1.ServiceAccount, error) { name := ImpersonationPrefix + i.user.GetUID() sa, err := i.clusterContext.Core.ServiceAccounts("").Controller().Lister().Get(ImpersonationNamespace, name) + if err != nil && !apierrors.IsNotFound(err) { + return nil, fmt.Errorf("impersonation: error getting service account [%s:%s]: %w", ImpersonationNamespace, name, err) + } if apierrors.IsNotFound(err) { logrus.Debugf("impersonation: creating service account %s", name) sa, err = i.clusterContext.Core.ServiceAccounts(ImpersonationNamespace).Create(&corev1.ServiceAccount{ @@ -177,16 +162,16 @@ func (i *Impersonator) createServiceAccount(role *rbacv1.ClusterRole) (*corev1.S // in case cache isn't synced yet, use raw client sa, err = i.clusterContext.Core.ServiceAccounts(ImpersonationNamespace).Get(name, metav1.GetOptions{}) } - } - if sa != nil { - // create secret for service account - sc := serviceaccounttoken.SecretTemplate(sa) - _, secretErr := i.clusterContext.Core.Secrets(ImpersonationNamespace).Create(sc) - if secretErr != nil && !apierrors.IsAlreadyExists(secretErr) { - return nil, fmt.Errorf("impersonation: error creating secret for service account %s", name) + if err != nil { + return nil, fmt.Errorf("impersonation: error getting service account [%s:%s]: %w", ImpersonationNamespace, name, err) } } - return sa, err + // create secret for service account if it was not automatically generated + _, err = serviceaccounttoken.EnsureSecretForServiceAccount(context.Background(), i.clusterContext.Core.Secrets("").Controller().Lister().Get, i.clusterContext.K8sClient, sa) + if err != nil { + return nil, fmt.Errorf("impersonation: error ensuring secret for service account %s: %w", name, err) + } + return sa, nil } func (i *Impersonator) createNamespace() error { diff --git a/pkg/kontainer-engine/drivers/util/utils.go b/pkg/kontainer-engine/drivers/util/utils.go index 30837b1d012..2bcbb5733be 100644 --- a/pkg/kontainer-engine/drivers/util/utils.go +++ b/pkg/kontainer-engine/drivers/util/utils.go @@ -3,9 +3,7 @@ package util import ( "context" "fmt" - "time" - errs "github.com/pkg/errors" "github.com/rancher/rancher/pkg/serviceaccounttoken" v3 "github.com/rancher/rke/types" "gopkg.in/yaml.v2" @@ -94,23 +92,14 @@ func GenerateServiceAccountToken(clientset kubernetes.Interface) (string, error) return "", fmt.Errorf("error creating role bindings: %v", err) } - start := time.Millisecond * 250 - for i := 0; i < 5; i++ { - time.Sleep(start) - if serviceAccount, err = clientset.CoreV1().ServiceAccounts(cattleNamespace).Get(context.TODO(), serviceAccount.Name, metav1.GetOptions{}); err != nil { - return "", fmt.Errorf("error getting service account: %v", err) - } - secret, err := serviceaccounttoken.CreateSecretForServiceAccount(context.TODO(), clientset, serviceAccount) - if err != nil { - return "", fmt.Errorf("error creating secret for service account: %v", err) - } - if token, ok := secret.Data["token"]; ok { - return string(token), nil - } - start = start * 2 + if serviceAccount, err = clientset.CoreV1().ServiceAccounts(cattleNamespace).Get(context.Background(), serviceAccount.Name, metav1.GetOptions{}); err != nil { + return "", fmt.Errorf("error getting service account: %w", err) } - - return "", errs.New("failed to fetch serviceAccountToken") + secret, err := serviceaccounttoken.EnsureSecretForServiceAccount(context.Background(), nil, clientset, serviceAccount) + if err != nil { + return "", fmt.Errorf("error ensuring secret for service account: %w", err) + } + return string(secret.Data["token"]), nil } func DeleteLegacyServiceAccountAndRoleBinding(clientset kubernetes.Interface) error { diff --git a/pkg/provisioningv2/rke2/configserver/server.go b/pkg/provisioningv2/rke2/configserver/server.go index a6db92fae48..b7e55b87a7b 100644 --- a/pkg/provisioningv2/rke2/configserver/server.go +++ b/pkg/provisioningv2/rke2/configserver/server.go @@ -28,6 +28,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/client-go/kubernetes" "k8s.io/client-go/tools/clientcmd" clientcmdapi "k8s.io/client-go/tools/clientcmd/api" capi "sigs.k8s.io/cluster-api/api/v1beta1" @@ -55,6 +56,7 @@ type RKE2ConfigServer struct { machines capicontrollers.MachineClient bootstrapCache rkecontroller.RKEBootstrapCache provisioningClusterCache provisioningcontrollers.ClusterCache + k8s kubernetes.Interface } func New(clients *wrangler.Context) *RKE2ConfigServer { @@ -82,6 +84,7 @@ func New(clients *wrangler.Context) *RKE2ConfigServer { machines: clients.CAPI.Machine(), bootstrapCache: clients.RKE.RKEBootstrap().Cache(), provisioningClusterCache: clients.Provisioning.Cluster().Cache(), + k8s: clients.K8s, } } @@ -340,7 +343,7 @@ func (r *RKE2ConfigServer) findSA(req *http.Request) (string, *corev1.Secret, er if planSecret == "" { continue } - tokenSecret, _, err := rke2.GetPlanServiceAccountTokenSecret(r.secrets, planSA) + tokenSecret, _, err := rke2.GetPlanServiceAccountTokenSecret(r.secrets, r.k8s, planSA) if err != nil { logrus.Errorf("[rke2configserver] error encountered when searching for token secret for planSA %s/%s: %v", planSA.Namespace, planSA.Name, err) continue @@ -389,7 +392,7 @@ func (r *RKE2ConfigServer) findSA(req *http.Request) (string, *corev1.Secret, er if planSecret == "" { continue } - tokenSecret, watchable, err := rke2.GetPlanServiceAccountTokenSecret(r.secrets, planSA) + tokenSecret, watchable, err := rke2.GetPlanServiceAccountTokenSecret(r.secrets, r.k8s, planSA) if err != nil || tokenSecret == nil { logrus.Debugf("[rke2configserver] %s/%s token secret for planSecret %s was nil or error received", machineNamespace, machineName, planSecret) if err != nil { @@ -425,10 +428,6 @@ func (r *RKE2ConfigServer) findSA(req *http.Request) (string, *corev1.Secret, er }() for event := range respSecret.ResultChan() { if secret, ok := event.Object.(*corev1.Secret); ok { - if !rke2.PlanServiceAccountTokenReady(planSA, secret) { - logrus.Debugf("[rke2configserver] %s/%s planSA %s/%s service account token %s/%s was not ready yet", machineNamespace, machineName, planSA.Namespace, planSA.Name, secret.Namespace, secret.Name) - continue - } logrus.Infof("[rke2configserver] %s/%s machineID: %s delivering planSecret %s with token secret %s/%s to system-agent from secret watch", machineNamespace, machineName, machineID, planSecret, secret.Namespace, secret.Name) return planSecret, secret, nil } diff --git a/pkg/serviceaccounttoken/secret.go b/pkg/serviceaccounttoken/secret.go index 2942171c018..fb24f9f2469 100644 --- a/pkg/serviceaccounttoken/secret.go +++ b/pkg/serviceaccounttoken/secret.go @@ -2,60 +2,96 @@ package serviceaccounttoken import ( "context" + "fmt" "time" - "github.com/rancher/wrangler/pkg/name" "github.com/sirupsen/logrus" v1 "k8s.io/api/core/v1" apierror "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/kubernetes" ) -// CreateSecretForServiceAccount creates a service-account-token Secret for the provided Service Account. -// If the secret already exists, the existing one is returned. -func CreateSecretForServiceAccount(ctx context.Context, clientSet kubernetes.Interface, sa *v1.ServiceAccount) (*v1.Secret, error) { - secretName := ServiceAccountSecretName(sa) +// secretGetter is an abstraction over any kind of secret getter. +// The caller can use any cache or client it has available, whether that is from norman, wrangler, or client-go, +// as long as it can wrap it in a simplified lambda with this signature. +type secretGetter func(namespace, name string) (*v1.Secret, error) + +// EnsureSecretForServiceAccount gets or creates a service account token Secret for the provided Service Account. +// For k8s <1.24, the secret is automatically generated for the service account. For >=1.24, we need to generate it explicitly. +func EnsureSecretForServiceAccount(ctx context.Context, secretGetter secretGetter, clientSet kubernetes.Interface, sa *v1.ServiceAccount) (*v1.Secret, error) { + if secretGetter == nil { + secretGetter = func(namespace, name string) (*v1.Secret, error) { + return clientSet.CoreV1().Secrets(namespace).Get(ctx, name, metav1.GetOptions{}) + } + } + if sa == nil { + return nil, fmt.Errorf("could not ensure secret for invalid service account") + } secretClient := clientSet.CoreV1().Secrets(sa.Namespace) - secret, err := secretClient.Get(ctx, secretName, metav1.GetOptions{}) - if err != nil { - if !apierror.IsNotFound(err) { - return nil, err + saClient := clientSet.CoreV1().ServiceAccounts(sa.Namespace) + secretName := ServiceAccountSecretName(sa) + var secret *v1.Secret + var err error + if secretName != "" { + secret, err = secretGetter(sa.Namespace, secretName) + if err != nil && !apierror.IsNotFound(err) { + return nil, fmt.Errorf("error ensuring secret for service account [%s:%s]: %w", sa.Namespace, sa.Name, err) } + } + if secret == nil { sc := SecretTemplate(sa) secret, err = secretClient.Create(ctx, sc, metav1.CreateOptions{}) if err != nil { - if !apierror.IsAlreadyExists(err) { - return nil, err - } - secret, err = secretClient.Get(ctx, secretName, metav1.GetOptions{}) - if err != nil { - return nil, err + return nil, fmt.Errorf("error ensuring secret for service account [%s:%s]: %w", sa.Namespace, sa.Name, err) + } + // k8s >=1.24 does not store a reference to the secret, but we need it to refer back to later + saCopy := sa.DeepCopy() + saCopy.Secrets = append(saCopy.Secrets, v1.ObjectReference{Name: secret.Name}) + saCopy, err = saClient.Update(ctx, saCopy, metav1.UpdateOptions{}) + if err != nil { + // clean up the secret we just created + cleanupErr := secretClient.Delete(ctx, secret.Name, metav1.DeleteOptions{}) + if cleanupErr != nil { + return nil, fmt.Errorf("encountered error while handling service account update error: %v, original error: %w", cleanupErr, err) } + return nil, fmt.Errorf("error ensuring secret for service account [%s:%s]: %w", sa.Namespace, sa.Name, err) } } if len(secret.Data[v1.ServiceAccountTokenKey]) > 0 { return secret, nil } - logrus.Infof("createSecretForServiceAccount: waiting for secret [%s] to be populated with token", secretName) - for { + logrus.Infof("EnsureSecretForServiceAccount: waiting for secret [%s] to be populated with token", secret.Name) + backoff := wait.Backoff{ + Duration: 2 * time.Second, + Cap: 10 * time.Second, + Steps: 5, + } + err = wait.ExponentialBackoff(backoff, func() (bool, error) { if len(secret.Data[v1.ServiceAccountTokenKey]) > 0 { - return secret, nil + return true, nil } - time.Sleep(2 * time.Second) - secret, err = secretClient.Get(ctx, secretName, metav1.GetOptions{}) + var err error + // use the secret client, rather than the secret getter, to circumvent the cache + secret, err = secretClient.Get(ctx, secret.Name, metav1.GetOptions{}) if err != nil { - return nil, err + return false, fmt.Errorf("error ensuring secret for service account [%s:%s]: %w", sa.Namespace, sa.Name, err) } + return false, nil + }) + if err != nil { + return nil, fmt.Errorf("error ensuring secret for service account [%s:%s]: %w", sa.Namespace, sa.Name, err) } + return secret, nil } // SecretTemplate generate a template of service-account-token Secret for the provided Service Account. func SecretTemplate(sa *v1.ServiceAccount) *v1.Secret { return &v1.Secret{ ObjectMeta: metav1.ObjectMeta{ - Name: ServiceAccountSecretName(sa), - Namespace: sa.Namespace, + GenerateName: serviceAccountSecretPrefix(sa), + Namespace: sa.Namespace, OwnerReferences: []metav1.OwnerReference{ { APIVersion: "v1", @@ -73,7 +109,16 @@ func SecretTemplate(sa *v1.ServiceAccount) *v1.Secret { } +// serviceAccountSecretPrefix returns the prefix that will be used to generate the secret for the given service account. +func serviceAccountSecretPrefix(sa *v1.ServiceAccount) string { + return fmt.Sprintf("%s-token-", sa.Name) +} + // ServiceAccountSecretName returns the secret name for the given Service Account. +// If there are more than one, it returns the first. func ServiceAccountSecretName(sa *v1.ServiceAccount) string { - return name.SafeConcatName(sa.Name, "token") + if len(sa.Secrets) < 1 { + return "" + } + return sa.Secrets[0].Name } diff --git a/pkg/serviceaccounttoken/secret_test.go b/pkg/serviceaccounttoken/secret_test.go new file mode 100644 index 00000000000..acd92459011 --- /dev/null +++ b/pkg/serviceaccounttoken/secret_test.go @@ -0,0 +1,128 @@ +package serviceaccounttoken + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/kubernetes/fake" + k8stesting "k8s.io/client-go/testing" +) + +func TestEnsureSecretForServiceAccount(t *testing.T) { + tests := []struct { + name string + sa *v1.ServiceAccount + wantSA *v1.ServiceAccount + existingSecret *v1.Secret + wantSecret *v1.Secret + wantErr bool + }{ + { + name: "service account with no secret generates secret", + sa: &v1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + }, + }, + wantSA: &v1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + }, + Secrets: []v1.ObjectReference{{ + Name: "test-token-abcde", + }}, + }, + wantSecret: &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-token-abcde", + Namespace: "default", + }, + Data: map[string][]byte{ + "token": []byte("abcde"), + }, + }, + }, + { + name: "service account with existing secret returns it", + sa: &v1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + }, + Secrets: []v1.ObjectReference{{ + Name: "test-token-abcde", + }}, + }, + wantSA: &v1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + }, + Secrets: []v1.ObjectReference{{ + Name: "test-token-abcde", + }}, + }, + existingSecret: &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-token-abcde", + Namespace: "default", + }, + Data: map[string][]byte{ + "token": []byte("abcde"), + }, + }, + wantSecret: &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-token-abcde", + Namespace: "default", + }, + Data: map[string][]byte{ + "token": []byte("abcde"), + }, + }, + }, + { + name: "returns error for nil service account", + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var k8sClient *fake.Clientset + objs := []runtime.Object{} + if tt.sa != nil { + objs = append(objs, tt.sa) + } + if tt.existingSecret != nil { + objs = append(objs, tt.existingSecret) + } + k8sClient = fake.NewSimpleClientset(objs...) + k8sClient.PrependReactor("create", "secrets", + func(action k8stesting.Action) (bool, runtime.Object, error) { + ret := action.(k8stesting.CreateAction).GetObject().(*v1.Secret) + ret.ObjectMeta.Name = ret.GenerateName + "abcde" + ret.Data = map[string][]byte{ + "token": []byte("abcde"), + } + return true, ret, nil + }, + ) + got, gotErr := EnsureSecretForServiceAccount(context.Background(), nil, k8sClient, tt.sa) + if tt.wantErr { + assert.Error(t, gotErr) + return + } + assert.NoError(t, gotErr) + assert.Equal(t, tt.wantSecret.Name, got.Name) + gotSA, _ := k8sClient.CoreV1().ServiceAccounts("default").Get(context.Background(), "test", metav1.GetOptions{}) + assert.Equal(t, tt.wantSA.Secrets, gotSA.Secrets) + }) + } +} diff --git a/pkg/tunnelserver/peermanager.go b/pkg/tunnelserver/peermanager.go index f607e194d0b..45827bc28e3 100644 --- a/pkg/tunnelserver/peermanager.go +++ b/pkg/tunnelserver/peermanager.go @@ -77,7 +77,7 @@ func getTokenFromToken(ctx context.Context, tokenBytes []byte) ([]byte, error) { return nil, err } - secret, err := serviceaccounttoken.CreateSecretForServiceAccount(ctx, client, sa) + secret, err := serviceaccounttoken.EnsureSecretForServiceAccount(ctx, nil, client, sa) if err != nil { return nil, err }