diff --git a/cmd/kube-controller-manager/app/controllermanager.go b/cmd/kube-controller-manager/app/controllermanager.go index bb33636c016..1eb5afde855 100644 --- a/cmd/kube-controller-manager/app/controllermanager.go +++ b/cmd/kube-controller-manager/app/controllermanager.go @@ -669,7 +669,6 @@ func (c serviceAccountTokenControllerStarter) startServiceAccountTokenController serviceaccountcontroller.TokensControllerOptions{ TokenGenerator: tokenGenerator, RootCA: rootCA, - AutoGenerate: !utilfeature.DefaultFeatureGate.Enabled(kubefeatures.LegacyServiceAccountTokenNoAutoGeneration), }, ) if err != nil { diff --git a/pkg/controller/serviceaccount/tokens_controller.go b/pkg/controller/serviceaccount/tokens_controller.go index eee7a0ad9e4..b11b66ecba7 100644 --- a/pkg/controller/serviceaccount/tokens_controller.go +++ b/pkg/controller/serviceaccount/tokens_controller.go @@ -38,7 +38,6 @@ import ( clientretry "k8s.io/client-go/util/retry" "k8s.io/client-go/util/workqueue" "k8s.io/klog/v2" - "k8s.io/kubernetes/pkg/registry/core/secret" "k8s.io/kubernetes/pkg/serviceaccount" ) @@ -67,9 +66,6 @@ type TokensControllerOptions struct { // MaxRetries controls the maximum number of times a particular key is retried before giving up // If zero, a default max is used MaxRetries int - - // AutoGenerate decides the auto-generation of secret-based token for service accounts. - AutoGenerate bool } // NewTokensController returns a new *TokensController. @@ -87,8 +83,7 @@ func NewTokensController(serviceAccounts informers.ServiceAccountInformer, secre syncServiceAccountQueue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "serviceaccount_tokens_service"), syncSecretQueue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "serviceaccount_tokens_secret"), - maxRetries: maxRetries, - autoGenerate: options.AutoGenerate, + maxRetries: maxRetries, } e.serviceAccounts = serviceAccounts.Lister() @@ -146,7 +141,6 @@ type TokensController struct { secretSynced cache.InformerSynced // syncServiceAccountQueue handles service account events: - // * ensures a referenced token exists for service accounts which still exist // * ensures tokens are removed for service accounts which no longer exist // key is "//" syncServiceAccountQueue workqueue.RateLimitingInterface @@ -158,8 +152,7 @@ type TokensController struct { // key is a secretQueueKey{} syncSecretQueue workqueue.RateLimitingInterface - maxRetries int - autoGenerate bool + maxRetries int } // Run runs controller blocks until stopCh is closed @@ -254,12 +247,6 @@ func (e *TokensController) syncServiceAccount() { if err != nil { klog.Errorf("error deleting serviceaccount tokens for %s/%s: %v", saInfo.namespace, saInfo.name, err) } - case e.autoGenerate: - // ensure a token exists and is referenced by this service account - retry, err = e.ensureReferencedToken(sa) - if err != nil { - klog.Errorf("error synchronizing serviceaccount %s/%s: %v", saInfo.namespace, saInfo.name, err) - } } } @@ -356,148 +343,6 @@ func (e *TokensController) deleteToken(ns, name string, uid types.UID) ( /*retry return true, err } -// ensureReferencedToken makes sure at least one ServiceAccountToken secret exists, and is included in the serviceAccount's Secrets list -func (e *TokensController) ensureReferencedToken(serviceAccount *v1.ServiceAccount) ( /* retry */ bool, error) { - if hasToken, err := e.hasReferencedToken(serviceAccount); err != nil { - // Don't retry cache lookup errors - return false, err - } else if hasToken { - // A service account token already exists, and is referenced, short-circuit - return false, nil - } - - // We don't want to update the cache's copy of the service account - // so add the secret to a freshly retrieved copy of the service account - serviceAccounts := e.client.CoreV1().ServiceAccounts(serviceAccount.Namespace) - liveServiceAccount, err := serviceAccounts.Get(context.TODO(), serviceAccount.Name, metav1.GetOptions{}) - if err != nil { - // Retry if we cannot fetch the live service account (for a NotFound error, either the live lookup or our cache are stale) - return true, err - } - if liveServiceAccount.ResourceVersion != serviceAccount.ResourceVersion { - // Retry if our liveServiceAccount doesn't match our cache's resourceVersion (either the live lookup or our cache are stale) - klog.V(4).Infof("liveServiceAccount.ResourceVersion (%s) does not match cache (%s), retrying", liveServiceAccount.ResourceVersion, serviceAccount.ResourceVersion) - return true, nil - } - - // Build the secret - secret := &v1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: secret.Strategy.GenerateName(fmt.Sprintf("%s-token-", serviceAccount.Name)), - Namespace: serviceAccount.Namespace, - Annotations: map[string]string{ - v1.ServiceAccountNameKey: serviceAccount.Name, - v1.ServiceAccountUIDKey: string(serviceAccount.UID), - }, - }, - Type: v1.SecretTypeServiceAccountToken, - Data: map[string][]byte{}, - } - - // Generate the token - token, err := e.token.GenerateToken(serviceaccount.LegacyClaims(*serviceAccount, *secret)) - if err != nil { - // retriable error - return true, err - } - secret.Data[v1.ServiceAccountTokenKey] = []byte(token) - secret.Data[v1.ServiceAccountNamespaceKey] = []byte(serviceAccount.Namespace) - if e.rootCA != nil && len(e.rootCA) > 0 { - secret.Data[v1.ServiceAccountRootCAKey] = e.rootCA - } - - // Save the secret - createdToken, err := e.client.CoreV1().Secrets(serviceAccount.Namespace).Create(context.TODO(), secret, metav1.CreateOptions{}) - if err != nil { - // if the namespace is being terminated, create will fail no matter what - if apierrors.HasStatusCause(err, v1.NamespaceTerminatingCause) { - return false, err - } - // retriable error - return true, err - } - // Manually add the new token to the cache store. - // This prevents the service account update (below) triggering another token creation, if the referenced token couldn't be found in the store - e.updatedSecrets.Mutation(createdToken) - - // Try to add a reference to the newly created token to the service account - addedReference := false - err = clientretry.RetryOnConflict(clientretry.DefaultRetry, func() error { - // refresh liveServiceAccount on every retry - defer func() { liveServiceAccount = nil }() - - // fetch the live service account if needed, and verify the UID matches and that we still need a token - if liveServiceAccount == nil { - liveServiceAccount, err = serviceAccounts.Get(context.TODO(), serviceAccount.Name, metav1.GetOptions{}) - if err != nil { - return err - } - - if liveServiceAccount.UID != serviceAccount.UID { - // If we don't have the same service account, stop trying to add a reference to the token made for the old service account. - return nil - } - - if hasToken, err := e.hasReferencedToken(liveServiceAccount); err != nil { - // Don't retry cache lookup errors - return nil - } else if hasToken { - // A service account token already exists, and is referenced, short-circuit - return nil - } - } - - // Try to add a reference to the token - liveServiceAccount.Secrets = append(liveServiceAccount.Secrets, v1.ObjectReference{Name: secret.Name}) - if _, err := serviceAccounts.Update(context.TODO(), liveServiceAccount, metav1.UpdateOptions{}); err != nil { - return err - } - - addedReference = true - return nil - }) - - if !addedReference { - // we weren't able to use the token, try to clean it up. - klog.V(2).Infof("deleting secret %s/%s because reference couldn't be added (%v)", secret.Namespace, secret.Name, err) - deleteOpts := metav1.DeleteOptions{Preconditions: &metav1.Preconditions{UID: &createdToken.UID}} - if err := e.client.CoreV1().Secrets(createdToken.Namespace).Delete(context.TODO(), createdToken.Name, deleteOpts); err != nil { - klog.Error(err) // if we fail, just log it - } - } - - if err != nil { - if apierrors.IsConflict(err) || apierrors.IsNotFound(err) { - // if we got a Conflict error, the service account was updated by someone else, and we'll get an update notification later - // if we got a NotFound error, the service account no longer exists, and we don't need to create a token for it - return false, nil - } - // retry in all other cases - return true, err - } - - // success! - return false, nil -} - -// hasReferencedToken returns true if the serviceAccount references a service account token secret -func (e *TokensController) hasReferencedToken(serviceAccount *v1.ServiceAccount) (bool, error) { - if len(serviceAccount.Secrets) == 0 { - return false, nil - } - allSecrets, err := e.listTokenSecrets(serviceAccount) - if err != nil { - return false, err - } - referencedSecrets := getSecretReferences(serviceAccount) - for _, secret := range allSecrets { - if referencedSecrets.Has(secret.Name) { - return true, nil - } - } - return false, nil -} - func (e *TokensController) secretUpdateNeeded(secret *v1.Secret) (bool, bool, bool) { caData := secret.Data[v1.ServiceAccountRootCAKey] needsCA := len(e.rootCA) > 0 && !bytes.Equal(caData, e.rootCA) diff --git a/pkg/controller/serviceaccount/tokens_controller_test.go b/pkg/controller/serviceaccount/tokens_controller_test.go index ffe02ced1fd..8fc55b82fb4 100644 --- a/pkg/controller/serviceaccount/tokens_controller_test.go +++ b/pkg/controller/serviceaccount/tokens_controller_test.go @@ -17,7 +17,6 @@ limitations under the License. package serviceaccount import ( - "errors" "reflect" "testing" "time" @@ -25,19 +24,14 @@ import ( "github.com/davecgh/go-spew/spew" "gopkg.in/square/go-jose.v2/jwt" v1 "k8s.io/api/core/v1" - apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" utilrand "k8s.io/apimachinery/pkg/util/rand" - utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes/fake" core "k8s.io/client-go/testing" - featuregatetesting "k8s.io/component-base/featuregate/testing" - api "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/controller" - "k8s.io/kubernetes/pkg/features" ) type testGenerator struct { @@ -69,16 +63,6 @@ func tokenSecretReferences() []v1.ObjectReference { return []v1.ObjectReference{{Name: "token-secret-1"}} } -// addTokenSecretReference adds a reference to the ServiceAccountToken that will be created -func addTokenSecretReference(refs []v1.ObjectReference) []v1.ObjectReference { - return addNamedTokenSecretReference(refs, "default-token-xn8fg") -} - -// addNamedTokenSecretReference adds a reference to the named ServiceAccountToken -func addNamedTokenSecretReference(refs []v1.ObjectReference, name string) []v1.ObjectReference { - return append(refs, v1.ObjectReference{Name: name}) -} - // serviceAccount returns a service account with the given secret refs func serviceAccount(secretRefs []v1.ObjectReference) *v1.ServiceAccount { return &v1.ServiceAccount{ @@ -115,32 +99,6 @@ func opaqueSecret() *v1.Secret { } } -// createdTokenSecret returns the ServiceAccountToken secret posted when creating a new token secret. -// Named "default-token-xn8fg", since that is the first generated name after rand.Seed(1) -func createdTokenSecret(overrideName ...string) *v1.Secret { - return namedCreatedTokenSecret("default-token-xn8fg") -} - -// namedTokenSecret returns the ServiceAccountToken secret posted when creating a new token secret with the given name. -func namedCreatedTokenSecret(name string) *v1.Secret { - return &v1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: "default", - Annotations: map[string]string{ - v1.ServiceAccountNameKey: "default", - v1.ServiceAccountUIDKey: "12345", - }, - }, - Type: v1.SecretTypeServiceAccountToken, - Data: map[string][]byte{ - "token": []byte("ABC"), - "ca.crt": []byte("CA Data"), - "namespace": []byte("default"), - }, - } -} - // serviceAccountTokenSecret returns an existing ServiceAccountToken secret named "token-secret-1" func serviceAccountTokenSecret() *v1.Secret { return &v1.Secret{ @@ -208,9 +166,8 @@ func TestTokenCreation(t *testing.T) { testcases := map[string]struct { ClientObjects []runtime.Object - IsAsync bool - MaxRetries int - autoGenerateDisabled bool + IsAsync bool + MaxRetries int Reactors []reaction @@ -230,97 +187,12 @@ func TestTokenCreation(t *testing.T) { "new serviceaccount with no secrets": { ClientObjects: []runtime.Object{serviceAccount(emptySecretReferences())}, - AddedServiceAccount: serviceAccount(emptySecretReferences()), - ExpectedActions: []core.Action{ - core.NewGetAction(schema.GroupVersionResource{Version: "v1", Resource: "serviceaccounts"}, metav1.NamespaceDefault, "default"), - core.NewCreateAction(schema.GroupVersionResource{Version: "v1", Resource: "secrets"}, metav1.NamespaceDefault, createdTokenSecret()), - core.NewUpdateAction(schema.GroupVersionResource{Version: "v1", Resource: "serviceaccounts"}, metav1.NamespaceDefault, serviceAccount(addTokenSecretReference(emptySecretReferences()))), - }, - }, - "new serviceaccount with no secrets [autogenerate disabled]": { - autoGenerateDisabled: true, - - ClientObjects: []runtime.Object{serviceAccount(emptySecretReferences())}, - AddedServiceAccount: serviceAccount(emptySecretReferences()), ExpectedActions: []core.Action{}, }, - "new serviceaccount with no secrets encountering create error": { - ClientObjects: []runtime.Object{serviceAccount(emptySecretReferences())}, - MaxRetries: 10, - IsAsync: true, - Reactors: []reaction{{ - verb: "create", - resource: "secrets", - reactor: func(t *testing.T) core.ReactionFunc { - i := 0 - return func(core.Action) (bool, runtime.Object, error) { - i++ - if i < 3 { - return true, nil, apierrors.NewForbidden(api.Resource("secrets"), "foo", errors.New("no can do")) - } - return false, nil, nil - } - }, - }}, - AddedServiceAccount: serviceAccount(emptySecretReferences()), - ExpectedActions: []core.Action{ - // Attempt 1 - core.NewGetAction(schema.GroupVersionResource{Version: "v1", Resource: "serviceaccounts"}, metav1.NamespaceDefault, "default"), - core.NewCreateAction(schema.GroupVersionResource{Version: "v1", Resource: "secrets"}, metav1.NamespaceDefault, createdTokenSecret()), - - // Attempt 2 - core.NewGetAction(schema.GroupVersionResource{Version: "v1", Resource: "serviceaccounts"}, metav1.NamespaceDefault, "default"), - core.NewCreateAction(schema.GroupVersionResource{Version: "v1", Resource: "secrets"}, metav1.NamespaceDefault, namedCreatedTokenSecret("default-token-txhzt")), - - // Attempt 3 - core.NewGetAction(schema.GroupVersionResource{Version: "v1", Resource: "serviceaccounts"}, metav1.NamespaceDefault, "default"), - core.NewCreateAction(schema.GroupVersionResource{Version: "v1", Resource: "secrets"}, metav1.NamespaceDefault, namedCreatedTokenSecret("default-token-vnmz7")), - core.NewUpdateAction(schema.GroupVersionResource{Version: "v1", Resource: "serviceaccounts"}, metav1.NamespaceDefault, serviceAccount(addNamedTokenSecretReference(emptySecretReferences(), "default-token-vnmz7"))), - }, - }, - "new serviceaccount with no secrets encountering unending create error": { - ClientObjects: []runtime.Object{serviceAccount(emptySecretReferences()), createdTokenSecret()}, - MaxRetries: 2, - IsAsync: true, - Reactors: []reaction{{ - verb: "create", - resource: "secrets", - reactor: func(t *testing.T) core.ReactionFunc { - return func(core.Action) (bool, runtime.Object, error) { - return true, nil, apierrors.NewForbidden(api.Resource("secrets"), "foo", errors.New("no can do")) - } - }, - }}, - - AddedServiceAccount: serviceAccount(emptySecretReferences()), - ExpectedActions: []core.Action{ - // Attempt - core.NewGetAction(schema.GroupVersionResource{Version: "v1", Resource: "serviceaccounts"}, metav1.NamespaceDefault, "default"), - core.NewCreateAction(schema.GroupVersionResource{Version: "v1", Resource: "secrets"}, metav1.NamespaceDefault, createdTokenSecret()), - // Retry 1 - core.NewGetAction(schema.GroupVersionResource{Version: "v1", Resource: "serviceaccounts"}, metav1.NamespaceDefault, "default"), - core.NewCreateAction(schema.GroupVersionResource{Version: "v1", Resource: "secrets"}, metav1.NamespaceDefault, namedCreatedTokenSecret("default-token-txhzt")), - // Retry 2 - core.NewGetAction(schema.GroupVersionResource{Version: "v1", Resource: "serviceaccounts"}, metav1.NamespaceDefault, "default"), - core.NewCreateAction(schema.GroupVersionResource{Version: "v1", Resource: "secrets"}, metav1.NamespaceDefault, namedCreatedTokenSecret("default-token-vnmz7")), - }, - }, "new serviceaccount with missing secrets": { ClientObjects: []runtime.Object{serviceAccount(missingSecretReferences())}, - AddedServiceAccount: serviceAccount(missingSecretReferences()), - ExpectedActions: []core.Action{ - core.NewGetAction(schema.GroupVersionResource{Version: "v1", Resource: "serviceaccounts"}, metav1.NamespaceDefault, "default"), - core.NewCreateAction(schema.GroupVersionResource{Version: "v1", Resource: "secrets"}, metav1.NamespaceDefault, createdTokenSecret()), - core.NewUpdateAction(schema.GroupVersionResource{Version: "v1", Resource: "serviceaccounts"}, metav1.NamespaceDefault, serviceAccount(addTokenSecretReference(missingSecretReferences()))), - }, - }, - "new serviceaccount with missing secrets [autogenerate disabled]": { - autoGenerateDisabled: true, - - ClientObjects: []runtime.Object{serviceAccount(missingSecretReferences())}, - AddedServiceAccount: serviceAccount(missingSecretReferences()), ExpectedActions: []core.Action{}, }, @@ -331,30 +203,9 @@ func TestTokenCreation(t *testing.T) { AddedSecretLocal: serviceAccountTokenSecret(), ExpectedActions: []core.Action{}, }, - "new serviceaccount with missing secrets and a local secret in the cache [autogenerate disabled]": { - autoGenerateDisabled: true, - - ClientObjects: []runtime.Object{serviceAccount(missingSecretReferences())}, - - AddedServiceAccount: serviceAccount(tokenSecretReferences()), - AddedSecretLocal: serviceAccountTokenSecret(), - ExpectedActions: []core.Action{}, - }, "new serviceaccount with non-token secrets": { ClientObjects: []runtime.Object{serviceAccount(regularSecretReferences()), opaqueSecret()}, - AddedServiceAccount: serviceAccount(regularSecretReferences()), - ExpectedActions: []core.Action{ - core.NewGetAction(schema.GroupVersionResource{Version: "v1", Resource: "serviceaccounts"}, metav1.NamespaceDefault, "default"), - core.NewCreateAction(schema.GroupVersionResource{Version: "v1", Resource: "secrets"}, metav1.NamespaceDefault, createdTokenSecret()), - core.NewUpdateAction(schema.GroupVersionResource{Version: "v1", Resource: "serviceaccounts"}, metav1.NamespaceDefault, serviceAccount(addTokenSecretReference(regularSecretReferences()))), - }, - }, - "new serviceaccount with non-token secrets [autogenerate disabled]": { - autoGenerateDisabled: true, - - ClientObjects: []runtime.Object{serviceAccount(regularSecretReferences()), opaqueSecret()}, - AddedServiceAccount: serviceAccount(regularSecretReferences()), ExpectedActions: []core.Action{}, }, @@ -365,55 +216,23 @@ func TestTokenCreation(t *testing.T) { AddedServiceAccount: serviceAccount(tokenSecretReferences()), ExpectedActions: []core.Action{}, }, - "new serviceaccount with token secrets [autogenerate disabled]": { - autoGenerateDisabled: false, - - ClientObjects: []runtime.Object{serviceAccount(tokenSecretReferences()), serviceAccountTokenSecret()}, - ExistingSecrets: []*v1.Secret{serviceAccountTokenSecret()}, - - AddedServiceAccount: serviceAccount(tokenSecretReferences()), - ExpectedActions: []core.Action{}, - }, - "new serviceaccount with no secrets with resource conflict": { - ClientObjects: []runtime.Object{updatedServiceAccount(emptySecretReferences()), createdTokenSecret()}, - IsAsync: true, - MaxRetries: 1, - - AddedServiceAccount: serviceAccount(emptySecretReferences()), - ExpectedActions: []core.Action{ - core.NewGetAction(schema.GroupVersionResource{Version: "v1", Resource: "serviceaccounts"}, metav1.NamespaceDefault, "default"), - core.NewGetAction(schema.GroupVersionResource{Version: "v1", Resource: "serviceaccounts"}, metav1.NamespaceDefault, "default"), - }, - }, "updated serviceaccount with no secrets": { ClientObjects: []runtime.Object{serviceAccount(emptySecretReferences())}, UpdatedServiceAccount: serviceAccount(emptySecretReferences()), - ExpectedActions: []core.Action{ - core.NewGetAction(schema.GroupVersionResource{Version: "v1", Resource: "serviceaccounts"}, metav1.NamespaceDefault, "default"), - core.NewCreateAction(schema.GroupVersionResource{Version: "v1", Resource: "secrets"}, metav1.NamespaceDefault, createdTokenSecret()), - core.NewUpdateAction(schema.GroupVersionResource{Version: "v1", Resource: "serviceaccounts"}, metav1.NamespaceDefault, serviceAccount(addTokenSecretReference(emptySecretReferences()))), - }, + ExpectedActions: []core.Action{}, }, "updated serviceaccount with missing secrets": { ClientObjects: []runtime.Object{serviceAccount(missingSecretReferences())}, UpdatedServiceAccount: serviceAccount(missingSecretReferences()), - ExpectedActions: []core.Action{ - core.NewGetAction(schema.GroupVersionResource{Version: "v1", Resource: "serviceaccounts"}, metav1.NamespaceDefault, "default"), - core.NewCreateAction(schema.GroupVersionResource{Version: "v1", Resource: "secrets"}, metav1.NamespaceDefault, createdTokenSecret()), - core.NewUpdateAction(schema.GroupVersionResource{Version: "v1", Resource: "serviceaccounts"}, metav1.NamespaceDefault, serviceAccount(addTokenSecretReference(missingSecretReferences()))), - }, + ExpectedActions: []core.Action{}, }, "updated serviceaccount with non-token secrets": { ClientObjects: []runtime.Object{serviceAccount(regularSecretReferences()), opaqueSecret()}, UpdatedServiceAccount: serviceAccount(regularSecretReferences()), - ExpectedActions: []core.Action{ - core.NewGetAction(schema.GroupVersionResource{Version: "v1", Resource: "serviceaccounts"}, metav1.NamespaceDefault, "default"), - core.NewCreateAction(schema.GroupVersionResource{Version: "v1", Resource: "secrets"}, metav1.NamespaceDefault, createdTokenSecret()), - core.NewUpdateAction(schema.GroupVersionResource{Version: "v1", Resource: "serviceaccounts"}, metav1.NamespaceDefault, serviceAccount(addTokenSecretReference(regularSecretReferences()))), - }, + ExpectedActions: []core.Action{}, }, "updated serviceaccount with token secrets": { ExistingSecrets: []*v1.Secret{serviceAccountTokenSecret()}, @@ -427,10 +246,7 @@ func TestTokenCreation(t *testing.T) { MaxRetries: 1, UpdatedServiceAccount: serviceAccount(emptySecretReferences()), - ExpectedActions: []core.Action{ - core.NewGetAction(schema.GroupVersionResource{Version: "v1", Resource: "serviceaccounts"}, metav1.NamespaceDefault, "default"), - core.NewGetAction(schema.GroupVersionResource{Version: "v1", Resource: "serviceaccounts"}, metav1.NamespaceDefault, "default"), - }, + ExpectedActions: []core.Action{}, }, "deleted serviceaccount with no secrets": { @@ -622,8 +438,6 @@ func TestTokenCreation(t *testing.T) { for k, tc := range testcases { t.Run(k, func(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.LegacyServiceAccountTokenNoAutoGeneration, tc.autoGenerateDisabled)() - // Re-seed to reset name generation utilrand.Seed(1) @@ -637,7 +451,7 @@ func TestTokenCreation(t *testing.T) { secretInformer := informers.Core().V1().Secrets().Informer() secrets := secretInformer.GetStore() serviceAccounts := informers.Core().V1().ServiceAccounts().Informer().GetStore() - controller, err := NewTokensController(informers.Core().V1().ServiceAccounts(), informers.Core().V1().Secrets(), client, TokensControllerOptions{TokenGenerator: generator, RootCA: []byte("CA Data"), MaxRetries: tc.MaxRetries, AutoGenerate: !tc.autoGenerateDisabled}) + controller, err := NewTokensController(informers.Core().V1().ServiceAccounts(), informers.Core().V1().Secrets(), client, TokensControllerOptions{TokenGenerator: generator, RootCA: []byte("CA Data"), MaxRetries: tc.MaxRetries}) if err != nil { t.Fatalf("error creating Tokens controller: %v", err) } diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index 1de2193da44..786d2d1efa8 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -471,6 +471,7 @@ const ( // owner: @zshihang // kep: https://kep.k8s.io/2800 // beta: v1.24 + // ga: v1.26 // // Stop auto-generation of secret-based service account tokens. LegacyServiceAccountTokenNoAutoGeneration featuregate.Feature = "LegacyServiceAccountTokenNoAutoGeneration" @@ -956,7 +957,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS KubeletTracing: {Default: false, PreRelease: featuregate.Alpha}, - LegacyServiceAccountTokenNoAutoGeneration: {Default: true, PreRelease: featuregate.GA}, + LegacyServiceAccountTokenNoAutoGeneration: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.29 LegacyServiceAccountTokenTracking: {Default: false, PreRelease: featuregate.Alpha}, diff --git a/test/integration/serviceaccount/service_account_test.go b/test/integration/serviceaccount/service_account_test.go index dc3b5034dd1..4074a3a44f2 100644 --- a/test/integration/serviceaccount/service_account_test.go +++ b/test/integration/serviceaccount/service_account_test.go @@ -30,7 +30,6 @@ import ( v1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/wait" serviceaccountapiserver "k8s.io/apiserver/pkg/authentication/serviceaccount" "k8s.io/apiserver/pkg/authorization/authorizer" @@ -96,105 +95,6 @@ func TestServiceAccountAutoCreate(t *testing.T) { } } -func TestServiceAccountTokenAutoCreate(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, kubefeatures.LegacyServiceAccountTokenNoAutoGeneration, false)() - c, _, stopFunc, err := startServiceAccountTestServerAndWaitForCaches(t) - defer stopFunc() - if err != nil { - t.Fatalf("failed to setup ServiceAccounts server: %v", err) - } - - ns := "test-service-account-token-creation" - name := "my-service-account" - - // Create namespace - _, err = c.CoreV1().Namespaces().Create(context.TODO(), &v1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: ns}}, metav1.CreateOptions{}) - if err != nil { - t.Fatalf("could not create namespace: %v", err) - } - - // Create service account - _, err = c.CoreV1().ServiceAccounts(ns).Create(context.TODO(), &v1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Name: name}}, metav1.CreateOptions{}) - if err != nil { - t.Fatalf("Service Account not created: %v", err) - } - - // Get token - token1Name, token1, err := getReferencedServiceAccountToken(c, ns, name, true) - if err != nil { - t.Fatal(err) - } - - // Delete token - err = c.CoreV1().Secrets(ns).Delete(context.TODO(), token1Name, metav1.DeleteOptions{}) - if err != nil { - t.Fatalf("Could not delete token: %v", err) - } - - // Get recreated token - token2Name, token2, err := getReferencedServiceAccountToken(c, ns, name, true) - if err != nil { - t.Fatal(err) - } - if token1Name == token2Name { - t.Fatalf("Expected new auto-created token name") - } - if token1 == token2 { - t.Fatalf("Expected new auto-created token value") - } - - // Trigger creation of a new referenced token - serviceAccount, err := c.CoreV1().ServiceAccounts(ns).Get(context.TODO(), name, metav1.GetOptions{}) - if err != nil { - t.Fatal(err) - } - serviceAccount.Secrets = []v1.ObjectReference{} - _, err = c.CoreV1().ServiceAccounts(ns).Update(context.TODO(), serviceAccount, metav1.UpdateOptions{}) - if err != nil { - t.Fatal(err) - } - - // Get rotated token - token3Name, token3, err := getReferencedServiceAccountToken(c, ns, name, true) - if err != nil { - t.Fatal(err) - } - if token3Name == token2Name { - t.Fatalf("Expected new auto-created token name") - } - if token3 == token2 { - t.Fatalf("Expected new auto-created token value") - } - - // Delete service account - err = c.CoreV1().ServiceAccounts(ns).Delete(context.TODO(), name, metav1.DeleteOptions{}) - if err != nil { - t.Fatal(err) - } - - // Wait for tokens to be deleted - tokensToCleanup := sets.NewString(token1Name, token2Name, token3Name) - err = wait.Poll(time.Second, 10*time.Second, func() (bool, error) { - // Get all secrets in the namespace - secrets, err := c.CoreV1().Secrets(ns).List(context.TODO(), metav1.ListOptions{}) - // Retrieval errors should fail - if err != nil { - return false, err - } - for _, s := range secrets.Items { - if tokensToCleanup.Has(s.Name) { - // Still waiting for tokens to be cleaned up - return false, nil - } - } - // All clean - return true, nil - }) - if err != nil { - t.Fatalf("Error waiting for tokens to be deleted: %v", err) - } -} - func TestServiceAccountTokenAutoMount(t *testing.T) { c, _, stopFunc, err := startServiceAccountTestServerAndWaitForCaches(t) defer stopFunc() @@ -315,7 +215,6 @@ func TestServiceAccountTokenAuthentication(t *testing.T) { } func TestLegacyServiceAccountTokenTracking(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, kubefeatures.LegacyServiceAccountTokenNoAutoGeneration, false)() defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, kubefeatures.LegacyServiceAccountTokenTracking, true)() c, config, stopFunc, err := startServiceAccountTestServerAndWaitForCaches(t) defer stopFunc() @@ -339,8 +238,13 @@ func TestLegacyServiceAccountTokenTracking(t *testing.T) { t.Fatalf("Secret not created: %v", err) } - autoSecretName, autoSecretTokenData, err := getReferencedServiceAccountToken(c, myns, readOnlyServiceAccountName, true) + // manually craft an auto created token + autoSecretName := "auto-token" + autoSecret, err := createServiceAccountToken(c, mysa, myns, autoSecretName) if err != nil { + t.Fatalf("Secret not created: %v", err) + } + if err := addReferencedServiceAccountToken(c, myns, readOnlyServiceAccountName, autoSecret); err != nil { t.Fatal(err) } @@ -359,7 +263,7 @@ func TestLegacyServiceAccountTokenTracking(t *testing.T) { { name: "auto created legacy token", secretName: autoSecretName, - secretTokenData: autoSecretTokenData, + secretTokenData: string(autoSecret.Data[v1.ServiceAccountTokenKey]), expectWarning: true, }, } @@ -494,7 +398,6 @@ func startServiceAccountTestServerAndWaitForCaches(t *testing.T) (clientset.Inte rootClientset, serviceaccountcontroller.TokensControllerOptions{ TokenGenerator: tokenGenerator, - AutoGenerate: !utilfeature.DefaultFeatureGate.Enabled(kubefeatures.LegacyServiceAccountTokenNoAutoGeneration), }, ) if err != nil { @@ -573,58 +476,22 @@ func createServiceAccountToken(c clientset.Interface, sa *v1.ServiceAccount, ns return secret, nil } -func getReferencedServiceAccountToken(c clientset.Interface, ns string, name string, shouldWait bool) (string, string, error) { - tokenName := "" - token := "" - - findToken := func() (bool, error) { - user, err := c.CoreV1().ServiceAccounts(ns).Get(context.TODO(), name, metav1.GetOptions{}) - if apierrors.IsNotFound(err) { - return false, nil - } - if err != nil { - return false, err - } - - for _, ref := range user.Secrets { - secret, err := c.CoreV1().Secrets(ns).Get(context.TODO(), ref.Name, metav1.GetOptions{}) - if apierrors.IsNotFound(err) { - continue - } - if err != nil { - return false, err - } - if secret.Type != v1.SecretTypeServiceAccountToken { - continue - } - name := secret.Annotations[v1.ServiceAccountNameKey] - uid := secret.Annotations[v1.ServiceAccountUIDKey] - tokenData := secret.Data[v1.ServiceAccountTokenKey] - if name == user.Name && uid == string(user.UID) && len(tokenData) > 0 { - tokenName = secret.Name - token = string(tokenData) - return true, nil - } - } - - return false, nil +func addReferencedServiceAccountToken(c clientset.Interface, ns string, name string, secret *v1.Secret) error { + sa, err := c.CoreV1().ServiceAccounts(ns).Get(context.TODO(), name, metav1.GetOptions{}) + if err != nil { + return err } - - if shouldWait { - err := wait.Poll(time.Second, 10*time.Second, findToken) - if err != nil { - return "", "", err - } - } else { - ok, err := findToken() - if err != nil { - return "", "", err - } - if !ok { - return "", "", fmt.Errorf("No token found for %s/%s", ns, name) - } + sa.Secrets = append(sa.Secrets, v1.ObjectReference{ + APIVersion: secret.APIVersion, + Kind: secret.Kind, + Namespace: secret.Namespace, + Name: secret.Name, + ResourceVersion: secret.ResourceVersion, + }) + if _, err = c.CoreV1().ServiceAccounts(ns).Update(context.TODO(), sa, metav1.UpdateOptions{}); err != nil { + return err } - return tokenName, token, nil + return nil } type testOperation func() error