From a30a596da9c101063d2d89fae31eb8935da0ced9 Mon Sep 17 00:00:00 2001 From: Colleen Murphy Date: Thu, 22 Sep 2022 15:46:52 -0700 Subject: [PATCH] Ensure cattle token secret has unique name The "cattle" service account on a downstream cluster is the account that Rancher uses to connect as an admin to the downstream cluster. Without this change, if the cattle service account's token is deleted, the cluster agent will regenerate it identically. This is a problem because it makes rotation of the token nontrivial. We can't craft the JWT ourselves or influence what claims are included in it, that is done within kubernetes. The only way to change the resulting JWT is to change the values kubernetes uses for claims. The only option is to make the secret name unique[1]. All other claims come from the service account, which we do not want to have to change in order to rotate the token. This change addresses the problem by using GenerateName when creating the secret so that it will be unique every time. However, since the name is no longer predictable, this causes problems when Rancher tries to look up the token. We now need to look up the name of the secret from the service account object. A further complication is that for kubernetes 1.24, the secret name is no longer stored on the service account, so now we set it explicitly. An extra benefit of this approach is that we no longer create multiple tokens for service accounts on k8s <1.24, since creating the token is skipped if it is found on the service account. This change refactors any code that was creating a service account token to use the serviceaccounttoken.EnsureSecretForServiceAccount function in order to be consistent everywhere. The function is updated to use a backoff routine instead of an infinite loop to check the state of the secret. It is flexible enough to use controller caches for callers with access to that, and falls back to regular clients for remote callers such as the agent. See also the change that introduced this functionality in Rancher[2]. [1] https://github.com/kubernetes/kubernetes/blob/v1.25.2/pkg/serviceaccount/legacy.go#L39 [2] https://github.com/rancher/rancher/pull/38113 --- pkg/agent/cluster/cluster.go | 4 +- .../customization/monitor/handler_utils.go | 4 +- .../dashboard/apiservice/apiservice.go | 36 ++--- .../rke2/bootstrap/controller.go | 21 +-- .../rke2/bootstrap/controller_test.go | 4 +- pkg/controllers/provisioningv2/rke2/common.go | 43 +----- pkg/impersonation/impersonation.go | 43 ++---- pkg/kontainer-engine/drivers/util/utils.go | 25 +--- .../rke2/configserver/server.go | 11 +- pkg/serviceaccounttoken/secret.go | 93 +++++++++---- pkg/serviceaccounttoken/secret_test.go | 128 ++++++++++++++++++ pkg/tunnelserver/peermanager.go | 2 +- 12 files changed, 249 insertions(+), 165 deletions(-) create mode 100644 pkg/serviceaccounttoken/secret_test.go 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 }