diff --git a/cmd/kube-controller-manager/app/controllermanager.go b/cmd/kube-controller-manager/app/controllermanager.go index 0b52ca8231d..81a53398a17 100644 --- a/cmd/kube-controller-manager/app/controllermanager.go +++ b/cmd/kube-controller-manager/app/controllermanager.go @@ -64,6 +64,7 @@ import ( "k8s.io/controller-manager/pkg/informerfactory" "k8s.io/controller-manager/pkg/leadermigration" "k8s.io/klog/v2" + kubefeatures "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/cmd/kube-controller-manager/app/config" "k8s.io/kubernetes/cmd/kube-controller-manager/app/options" @@ -631,6 +632,7 @@ 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 8ee524afee0..e3efbc6163f 100644 --- a/pkg/controller/serviceaccount/tokens_controller.go +++ b/pkg/controller/serviceaccount/tokens_controller.go @@ -22,7 +22,7 @@ import ( "fmt" "time" - "k8s.io/api/core/v1" + 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/types" @@ -68,6 +68,9 @@ 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. @@ -85,7 +88,8 @@ func NewTokensController(serviceAccounts informers.ServiceAccountInformer, secre syncServiceAccountQueue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "serviceaccount_tokens_service"), syncSecretQueue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "serviceaccount_tokens_secret"), - maxRetries: maxRetries, + maxRetries: maxRetries, + autoGenerate: options.AutoGenerate, } if cl != nil && cl.CoreV1().RESTClient().GetRateLimiter() != nil { if err := ratelimiter.RegisterMetricAndTrackRateLimiterUsage("serviceaccount_tokens_controller", cl.CoreV1().RESTClient().GetRateLimiter()); err != nil { @@ -160,7 +164,8 @@ type TokensController struct { // key is a secretQueueKey{} syncSecretQueue workqueue.RateLimitingInterface - maxRetries int + maxRetries int + autoGenerate bool } // Run runs controller blocks until stopCh is closed @@ -255,7 +260,7 @@ func (e *TokensController) syncServiceAccount() { if err != nil { klog.Errorf("error deleting serviceaccount tokens for %s/%s: %v", saInfo.namespace, saInfo.name, err) } - default: + case e.autoGenerate: // ensure a token exists and is referenced by this service account retry, err = e.ensureReferencedToken(sa) if err != nil { diff --git a/pkg/controller/serviceaccount/tokens_controller_test.go b/pkg/controller/serviceaccount/tokens_controller_test.go index f2055320759..ffe02ced1fd 100644 --- a/pkg/controller/serviceaccount/tokens_controller_test.go +++ b/pkg/controller/serviceaccount/tokens_controller_test.go @@ -24,19 +24,20 @@ import ( "github.com/davecgh/go-spew/spew" "gopkg.in/square/go-jose.v2/jwt" - "k8s.io/klog/v2" - - "k8s.io/api/core/v1" + 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 { @@ -207,8 +208,9 @@ func TestTokenCreation(t *testing.T) { testcases := map[string]struct { ClientObjects []runtime.Object - IsAsync bool - MaxRetries int + IsAsync bool + MaxRetries int + autoGenerateDisabled bool Reactors []reaction @@ -235,6 +237,14 @@ func TestTokenCreation(t *testing.T) { 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, @@ -306,6 +316,14 @@ func TestTokenCreation(t *testing.T) { 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{}, + }, "new serviceaccount with missing secrets and a local secret in the cache": { ClientObjects: []runtime.Object{serviceAccount(missingSecretReferences())}, @@ -313,6 +331,15 @@ 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()}, @@ -323,6 +350,14 @@ func TestTokenCreation(t *testing.T) { 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{}, + }, "new serviceaccount with token secrets": { ClientObjects: []runtime.Object{serviceAccount(tokenSecretReferences()), serviceAccountTokenSecret()}, ExistingSecrets: []*v1.Secret{serviceAccountTokenSecret()}, @@ -330,6 +365,15 @@ 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, @@ -577,124 +621,126 @@ func TestTokenCreation(t *testing.T) { } for k, tc := range testcases { - klog.Infof(k) + 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) + // Re-seed to reset name generation + utilrand.Seed(1) - generator := &testGenerator{Token: "ABC"} + generator := &testGenerator{Token: "ABC"} - client := fake.NewSimpleClientset(tc.ClientObjects...) - for _, reactor := range tc.Reactors { - client.Fake.PrependReactor(reactor.verb, reactor.resource, reactor.reactor(t)) - } - informers := informers.NewSharedInformerFactory(client, controller.NoResyncPeriodFunc()) - 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}) - if err != nil { - t.Fatalf("error creating Tokens controller: %v", err) - } - - if tc.ExistingServiceAccount != nil { - serviceAccounts.Add(tc.ExistingServiceAccount) - } - for _, s := range tc.ExistingSecrets { - secrets.Add(s) - } - - if tc.AddedServiceAccount != nil { - serviceAccounts.Add(tc.AddedServiceAccount) - controller.queueServiceAccountSync(tc.AddedServiceAccount) - } - if tc.UpdatedServiceAccount != nil { - serviceAccounts.Add(tc.UpdatedServiceAccount) - controller.queueServiceAccountUpdateSync(nil, tc.UpdatedServiceAccount) - } - if tc.DeletedServiceAccount != nil { - serviceAccounts.Delete(tc.DeletedServiceAccount) - controller.queueServiceAccountSync(tc.DeletedServiceAccount) - } - if tc.AddedSecret != nil { - secrets.Add(tc.AddedSecret) - controller.queueSecretSync(tc.AddedSecret) - } - if tc.AddedSecretLocal != nil { - controller.updatedSecrets.Mutation(tc.AddedSecretLocal) - } - if tc.UpdatedSecret != nil { - secrets.Add(tc.UpdatedSecret) - controller.queueSecretUpdateSync(nil, tc.UpdatedSecret) - } - if tc.DeletedSecret != nil { - secrets.Delete(tc.DeletedSecret) - controller.queueSecretSync(tc.DeletedSecret) - } - - // This is the longest we'll wait for async tests - timeout := time.Now().Add(30 * time.Second) - waitedForAdditionalActions := false - - for { - if controller.syncServiceAccountQueue.Len() > 0 { - controller.syncServiceAccount() + client := fake.NewSimpleClientset(tc.ClientObjects...) + for _, reactor := range tc.Reactors { + client.Fake.PrependReactor(reactor.verb, reactor.resource, reactor.reactor(t)) } - if controller.syncSecretQueue.Len() > 0 { - controller.syncSecret() + informers := informers.NewSharedInformerFactory(client, controller.NoResyncPeriodFunc()) + 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}) + if err != nil { + t.Fatalf("error creating Tokens controller: %v", err) } - // The queues still have things to work on - if controller.syncServiceAccountQueue.Len() > 0 || controller.syncSecretQueue.Len() > 0 { - continue + if tc.ExistingServiceAccount != nil { + serviceAccounts.Add(tc.ExistingServiceAccount) + } + for _, s := range tc.ExistingSecrets { + secrets.Add(s) } - // If we expect this test to work asynchronously... - if tc.IsAsync { - // if we're still missing expected actions within our test timeout - if len(client.Actions()) < len(tc.ExpectedActions) && time.Now().Before(timeout) { - // wait for the expected actions (without hotlooping) - time.Sleep(time.Millisecond) + if tc.AddedServiceAccount != nil { + serviceAccounts.Add(tc.AddedServiceAccount) + controller.queueServiceAccountSync(tc.AddedServiceAccount) + } + if tc.UpdatedServiceAccount != nil { + serviceAccounts.Add(tc.UpdatedServiceAccount) + controller.queueServiceAccountUpdateSync(nil, tc.UpdatedServiceAccount) + } + if tc.DeletedServiceAccount != nil { + serviceAccounts.Delete(tc.DeletedServiceAccount) + controller.queueServiceAccountSync(tc.DeletedServiceAccount) + } + if tc.AddedSecret != nil { + secrets.Add(tc.AddedSecret) + controller.queueSecretSync(tc.AddedSecret) + } + if tc.AddedSecretLocal != nil { + controller.updatedSecrets.Mutation(tc.AddedSecretLocal) + } + if tc.UpdatedSecret != nil { + secrets.Add(tc.UpdatedSecret) + controller.queueSecretUpdateSync(nil, tc.UpdatedSecret) + } + if tc.DeletedSecret != nil { + secrets.Delete(tc.DeletedSecret) + controller.queueSecretSync(tc.DeletedSecret) + } + + // This is the longest we'll wait for async tests + timeout := time.Now().Add(30 * time.Second) + waitedForAdditionalActions := false + + for { + if controller.syncServiceAccountQueue.Len() > 0 { + controller.syncServiceAccount() + } + if controller.syncSecretQueue.Len() > 0 { + controller.syncSecret() + } + + // The queues still have things to work on + if controller.syncServiceAccountQueue.Len() > 0 || controller.syncSecretQueue.Len() > 0 { continue } - // if we exactly match our expected actions, wait a bit to make sure no other additional actions show up - if len(client.Actions()) == len(tc.ExpectedActions) && !waitedForAdditionalActions { - time.Sleep(time.Second) - waitedForAdditionalActions = true - continue + // If we expect this test to work asynchronously... + if tc.IsAsync { + // if we're still missing expected actions within our test timeout + if len(client.Actions()) < len(tc.ExpectedActions) && time.Now().Before(timeout) { + // wait for the expected actions (without hotlooping) + time.Sleep(time.Millisecond) + continue + } + + // if we exactly match our expected actions, wait a bit to make sure no other additional actions show up + if len(client.Actions()) == len(tc.ExpectedActions) && !waitedForAdditionalActions { + time.Sleep(time.Second) + waitedForAdditionalActions = true + continue + } } - } - break - } - - if controller.syncServiceAccountQueue.Len() > 0 { - t.Errorf("%s: unexpected items in service account queue: %d", k, controller.syncServiceAccountQueue.Len()) - } - if controller.syncSecretQueue.Len() > 0 { - t.Errorf("%s: unexpected items in secret queue: %d", k, controller.syncSecretQueue.Len()) - } - - actions := client.Actions() - for i, action := range actions { - if len(tc.ExpectedActions) < i+1 { - t.Errorf("%s: %d unexpected actions: %+v", k, len(actions)-len(tc.ExpectedActions), actions[i:]) break } - expectedAction := tc.ExpectedActions[i] - if !reflect.DeepEqual(expectedAction, action) { - t.Errorf("%s:\nExpected:\n%s\ngot:\n%s", k, spew.Sdump(expectedAction), spew.Sdump(action)) - continue + if controller.syncServiceAccountQueue.Len() > 0 { + t.Errorf("%s: unexpected items in service account queue: %d", k, controller.syncServiceAccountQueue.Len()) + } + if controller.syncSecretQueue.Len() > 0 { + t.Errorf("%s: unexpected items in secret queue: %d", k, controller.syncSecretQueue.Len()) } - } - if len(tc.ExpectedActions) > len(actions) { - t.Errorf("%s: %d additional expected actions", k, len(tc.ExpectedActions)-len(actions)) - for _, a := range tc.ExpectedActions[len(actions):] { - t.Logf(" %+v", a) + actions := client.Actions() + for i, action := range actions { + if len(tc.ExpectedActions) < i+1 { + t.Errorf("%s: %d unexpected actions: %+v", k, len(actions)-len(tc.ExpectedActions), actions[i:]) + break + } + + expectedAction := tc.ExpectedActions[i] + if !reflect.DeepEqual(expectedAction, action) { + t.Errorf("%s:\nExpected:\n%s\ngot:\n%s", k, spew.Sdump(expectedAction), spew.Sdump(action)) + continue + } } - } + + if len(tc.ExpectedActions) > len(actions) { + t.Errorf("%s: %d additional expected actions", k, len(tc.ExpectedActions)-len(actions)) + for _, a := range tc.ExpectedActions[len(actions):] { + t.Logf(" %+v", a) + } + } + }) } } diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index f41fd7230ba..9145653ac17 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -819,6 +819,13 @@ const ( // // Enables GRPC probe method for {Liveness,Readiness,Startup}Probe. GRPCContainerProbe featuregate.Feature = "GRPCContainerProbe" + + // owner: @zshihang + // kep: http://kep.k8s.io/2800 + // beta: v1.24 + // + // Stop auto-generation of secret-based service account tokens. + LegacyServiceAccountTokenNoAutoGeneration featuregate.Feature = "LegacyServiceAccountTokenNoAutoGeneration" ) func init() { @@ -939,6 +946,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS HonorPVReclaimPolicy: {Default: false, PreRelease: featuregate.Alpha}, RecoverVolumeExpansionFailure: {Default: false, PreRelease: featuregate.Alpha}, GRPCContainerProbe: {Default: false, PreRelease: featuregate.Alpha}, + LegacyServiceAccountTokenNoAutoGeneration: {Default: true, PreRelease: featuregate.Beta}, // inherited features from generic apiserver, relisted here to get a conflict if it is changed // unintentionally on either side: diff --git a/test/e2e/auth/node_authz.go b/test/e2e/auth/node_authz.go index 77b472adaa3..12035ede272 100644 --- a/test/e2e/auth/node_authz.go +++ b/test/e2e/auth/node_authz.go @@ -45,7 +45,6 @@ var _ = SIGDescribe("[Feature:NodeAuthorizer]", func() { var c clientset.Interface var ns string var asUser string - var defaultSaSecret string var nodeName string ginkgo.BeforeEach(func() { ns = f.Namespace.Name @@ -55,11 +54,6 @@ var _ = SIGDescribe("[Feature:NodeAuthorizer]", func() { framework.ExpectNotEqual(len(nodeList.Items), 0) nodeName = nodeList.Items[0].Name asUser = nodeNamePrefix + nodeName - saName := "default" - sa, err := f.ClientSet.CoreV1().ServiceAccounts(ns).Get(context.TODO(), saName, metav1.GetOptions{}) - framework.ExpectNotEqual(len(sa.Secrets), 0) - framework.ExpectNoError(err, "failed to retrieve service account (%s:%s)", ns, saName) - defaultSaSecret = sa.Secrets[0].Name ginkgo.By("Creating a kubernetes client that impersonates a node") config, err := framework.LoadConfig() framework.ExpectNoError(err, "failed to load kubernetes client config") @@ -77,7 +71,17 @@ var _ = SIGDescribe("[Feature:NodeAuthorizer]", func() { }) ginkgo.It("Getting an existing secret should exit with the Forbidden error", func() { - _, err := c.CoreV1().Secrets(ns).Get(context.TODO(), defaultSaSecret, metav1.GetOptions{}) + ginkgo.By("Create a secret for testing") + secret := &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns, + Name: "node-auth-secret", + }, + StringData: map[string]string{}, + } + _, err := f.ClientSet.CoreV1().Secrets(ns).Create(context.TODO(), secret, metav1.CreateOptions{}) + framework.ExpectNoError(err, "failed to create secret (%s:%s) %+v", ns, secret.Name, *secret) + _, err = c.CoreV1().Secrets(ns).Get(context.TODO(), secret.Name, metav1.GetOptions{}) framework.ExpectEqual(apierrors.IsForbidden(err), true) }) diff --git a/test/e2e/auth/service_accounts.go b/test/e2e/auth/service_accounts.go index 2dbfb78dea4..d8b29ffbd80 100644 --- a/test/e2e/auth/service_accounts.go +++ b/test/e2e/auth/service_accounts.go @@ -46,121 +46,18 @@ import ( "github.com/onsi/ginkgo" ) +const rootCAConfigMapName = "kube-root-ca.crt" + var _ = SIGDescribe("ServiceAccounts", func() { f := framework.NewDefaultFramework("svcaccounts") - ginkgo.It("should ensure a single API token exists", func() { - // wait for the service account to reference a single secret - var secrets []v1.ObjectReference - framework.ExpectNoError(wait.Poll(time.Millisecond*500, time.Second*10, func() (bool, error) { - ginkgo.By("waiting for a single token reference") - sa, err := f.ClientSet.CoreV1().ServiceAccounts(f.Namespace.Name).Get(context.TODO(), "default", metav1.GetOptions{}) - if apierrors.IsNotFound(err) { - framework.Logf("default service account was not found") - return false, nil - } - if err != nil { - framework.Logf("error getting default service account: %v", err) - return false, err - } - switch len(sa.Secrets) { - case 0: - framework.Logf("default service account has no secret references") - return false, nil - case 1: - framework.Logf("default service account has a single secret reference") - secrets = sa.Secrets - return true, nil - default: - return false, fmt.Errorf("default service account has too many secret references: %#v", sa.Secrets) - } - })) - - // make sure the reference doesn't flutter + ginkgo.It("no secret-based service account token should be auto-generated", func() { { - ginkgo.By("ensuring the single token reference persists") - time.Sleep(2 * time.Second) + ginkgo.By("ensuring no secret-based service account token exists") + time.Sleep(10 * time.Second) sa, err := f.ClientSet.CoreV1().ServiceAccounts(f.Namespace.Name).Get(context.TODO(), "default", metav1.GetOptions{}) framework.ExpectNoError(err) - framework.ExpectEqual(sa.Secrets, secrets) - } - - // delete the referenced secret - ginkgo.By("deleting the service account token") - framework.ExpectNoError(f.ClientSet.CoreV1().Secrets(f.Namespace.Name).Delete(context.TODO(), secrets[0].Name, metav1.DeleteOptions{})) - - // wait for the referenced secret to be removed, and another one autocreated - framework.ExpectNoError(wait.Poll(time.Millisecond*500, framework.ServiceAccountProvisionTimeout, func() (bool, error) { - ginkgo.By("waiting for a new token reference") - sa, err := f.ClientSet.CoreV1().ServiceAccounts(f.Namespace.Name).Get(context.TODO(), "default", metav1.GetOptions{}) - if err != nil { - framework.Logf("error getting default service account: %v", err) - return false, err - } - switch len(sa.Secrets) { - case 0: - framework.Logf("default service account has no secret references") - return false, nil - case 1: - if sa.Secrets[0] == secrets[0] { - framework.Logf("default service account still has the deleted secret reference") - return false, nil - } - framework.Logf("default service account has a new single secret reference") - secrets = sa.Secrets - return true, nil - default: - return false, fmt.Errorf("default service account has too many secret references: %#v", sa.Secrets) - } - })) - - // make sure the reference doesn't flutter - { - ginkgo.By("ensuring the single token reference persists") - time.Sleep(2 * time.Second) - sa, err := f.ClientSet.CoreV1().ServiceAccounts(f.Namespace.Name).Get(context.TODO(), "default", metav1.GetOptions{}) - framework.ExpectNoError(err) - framework.ExpectEqual(sa.Secrets, secrets) - } - - // delete the reference from the service account - ginkgo.By("deleting the reference to the service account token") - { - sa, err := f.ClientSet.CoreV1().ServiceAccounts(f.Namespace.Name).Get(context.TODO(), "default", metav1.GetOptions{}) - framework.ExpectNoError(err) - sa.Secrets = nil - _, updateErr := f.ClientSet.CoreV1().ServiceAccounts(f.Namespace.Name).Update(context.TODO(), sa, metav1.UpdateOptions{}) - framework.ExpectNoError(updateErr) - } - - // wait for another one to be autocreated - framework.ExpectNoError(wait.Poll(time.Millisecond*500, framework.ServiceAccountProvisionTimeout, func() (bool, error) { - ginkgo.By("waiting for a new token to be created and added") - sa, err := f.ClientSet.CoreV1().ServiceAccounts(f.Namespace.Name).Get(context.TODO(), "default", metav1.GetOptions{}) - if err != nil { - framework.Logf("error getting default service account: %v", err) - return false, err - } - switch len(sa.Secrets) { - case 0: - framework.Logf("default service account has no secret references") - return false, nil - case 1: - framework.Logf("default service account has a new single secret reference") - secrets = sa.Secrets - return true, nil - default: - return false, fmt.Errorf("default service account has too many secret references: %#v", sa.Secrets) - } - })) - - // make sure the reference doesn't flutter - { - ginkgo.By("ensuring the single token reference persists") - time.Sleep(2 * time.Second) - sa, err := f.ClientSet.CoreV1().ServiceAccounts(f.Namespace.Name).Get(context.TODO(), "default", metav1.GetOptions{}) - framework.ExpectNoError(err) - framework.ExpectEqual(sa.Secrets, secrets) + framework.ExpectEmpty(sa.Secrets) } }) @@ -174,43 +71,9 @@ var _ = SIGDescribe("ServiceAccounts", func() { Account mount path MUST be auto mounted to the Container. */ framework.ConformanceIt("should mount an API token into pods ", func() { - var rootCAContent string - sa, err := f.ClientSet.CoreV1().ServiceAccounts(f.Namespace.Name).Create(context.TODO(), &v1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Name: "mount-test"}}, metav1.CreateOptions{}) framework.ExpectNoError(err) - // Standard get, update retry loop - framework.ExpectNoError(wait.Poll(time.Millisecond*500, framework.ServiceAccountProvisionTimeout, func() (bool, error) { - ginkgo.By("getting the auto-created API token") - sa, err := f.ClientSet.CoreV1().ServiceAccounts(f.Namespace.Name).Get(context.TODO(), "mount-test", metav1.GetOptions{}) - if apierrors.IsNotFound(err) { - framework.Logf("mount-test service account was not found") - return false, nil - } - if err != nil { - framework.Logf("error getting mount-test service account: %v", err) - return false, err - } - if len(sa.Secrets) == 0 { - framework.Logf("mount-test service account has no secret references") - return false, nil - } - for _, secretRef := range sa.Secrets { - secret, err := f.ClientSet.CoreV1().Secrets(f.Namespace.Name).Get(context.TODO(), secretRef.Name, metav1.GetOptions{}) - if err != nil { - framework.Logf("Error getting secret %s: %v", secretRef.Name, err) - continue - } - if secret.Type == v1.SecretTypeServiceAccountToken { - rootCAContent = string(secret.Data[v1.ServiceAccountRootCAKey]) - return true, nil - } - } - - framework.Logf("default service account has no secret references to valid service account tokens") - return false, nil - })) - zero := int64(0) pod, err := f.ClientSet.CoreV1().Pods(f.Namespace.Name).Create(context.TODO(), &v1.Pod{ ObjectMeta: metav1.ObjectMeta{ @@ -239,7 +102,10 @@ var _ = SIGDescribe("ServiceAccounts", func() { framework.ExpectNoError(err) // CA and namespace should be identical - framework.ExpectEqual(mountedCA, rootCAContent) + rootCA, err := f.ClientSet.CoreV1().ConfigMaps(f.Namespace.Name).Get(context.TODO(), rootCAConfigMapName, metav1.GetOptions{}) + framework.ExpectNoError(err) + framework.Logf("Got root ca configmap in namespace %q", f.Namespace.Name) + framework.ExpectEqual(mountedCA, rootCA.Data["ca.crt"]) framework.ExpectEqual(mountedNamespace, f.Namespace.Name) // Token should be a valid credential that identifies the pod's service account tokenReview := &authenticationv1.TokenReview{Spec: authenticationv1.TokenReviewSpec{Token: mountedToken}} @@ -291,37 +157,6 @@ var _ = SIGDescribe("ServiceAccounts", func() { nomountSA, err = f.ClientSet.CoreV1().ServiceAccounts(f.Namespace.Name).Create(context.TODO(), nomountSA, metav1.CreateOptions{}) framework.ExpectNoError(err) - // Standard get, update retry loop - framework.ExpectNoError(wait.Poll(time.Millisecond*500, framework.ServiceAccountProvisionTimeout, func() (bool, error) { - ginkgo.By("getting the auto-created API token") - sa, err := f.ClientSet.CoreV1().ServiceAccounts(f.Namespace.Name).Get(context.TODO(), mountSA.Name, metav1.GetOptions{}) - if apierrors.IsNotFound(err) { - framework.Logf("mount service account was not found") - return false, nil - } - if err != nil { - framework.Logf("error getting mount service account: %v", err) - return false, err - } - if len(sa.Secrets) == 0 { - framework.Logf("mount service account has no secret references") - return false, nil - } - for _, secretRef := range sa.Secrets { - secret, err := f.ClientSet.CoreV1().Secrets(f.Namespace.Name).Get(context.TODO(), secretRef.Name, metav1.GetOptions{}) - if err != nil { - framework.Logf("Error getting secret %s: %v", secretRef.Name, err) - continue - } - if secret.Type == v1.SecretTypeServiceAccountToken { - return true, nil - } - } - - framework.Logf("default service account has no secret references to valid service account tokens") - return false, nil - })) - testcases := []struct { PodName string ServiceAccountName string @@ -883,8 +718,6 @@ var _ = SIGDescribe("ServiceAccounts", func() { 3. Reconciled if modified */ framework.ConformanceIt("should guarantee kube-root-ca.crt exist in any namespace", func() { - const rootCAConfigMapName = "kube-root-ca.crt" - framework.ExpectNoError(wait.PollImmediate(500*time.Millisecond, wait.ForeverTestTimeout, func() (bool, error) { _, err := f.ClientSet.CoreV1().ConfigMaps(f.Namespace.Name).Get(context.TODO(), rootCAConfigMapName, metav1.GetOptions{}) if err == nil { diff --git a/test/integration/serviceaccount/service_account_test.go b/test/integration/serviceaccount/service_account_test.go index b097aceda25..f5f0adacbf1 100644 --- a/test/integration/serviceaccount/service_account_test.go +++ b/test/integration/serviceaccount/service_account_test.go @@ -43,11 +43,15 @@ import ( serviceaccountapiserver "k8s.io/apiserver/pkg/authentication/serviceaccount" "k8s.io/apiserver/pkg/authentication/user" "k8s.io/apiserver/pkg/authorization/authorizer" + utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/informers" clientset "k8s.io/client-go/kubernetes" restclient "k8s.io/client-go/rest" + featuregatetesting "k8s.io/component-base/featuregate/testing" "k8s.io/kubernetes/pkg/controller" serviceaccountcontroller "k8s.io/kubernetes/pkg/controller/serviceaccount" + "k8s.io/kubernetes/pkg/features" + kubefeatures "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/serviceaccount" serviceaccountadmission "k8s.io/kubernetes/plugin/pkg/admission/serviceaccount" "k8s.io/kubernetes/test/integration/framework" @@ -99,6 +103,7 @@ func TestServiceAccountAutoCreate(t *testing.T) { } func TestServiceAccountTokenAutoCreate(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.LegacyServiceAccountTokenNoAutoGeneration, false)() c, _, stopFunc, err := startServiceAccountTestServer(t) defer stopFunc() if err != nil { @@ -261,16 +266,18 @@ func TestServiceAccountTokenAuthentication(t *testing.T) { } // Create "ro" user in myns - _, err = c.CoreV1().ServiceAccounts(myns).Create(context.TODO(), &v1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Name: readOnlyServiceAccountName}}, metav1.CreateOptions{}) + roSA, err := c.CoreV1().ServiceAccounts(myns).Create(context.TODO(), &v1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Name: readOnlyServiceAccountName}}, metav1.CreateOptions{}) if err != nil { t.Fatalf("Service Account not created: %v", err) } - roTokenName, roToken, err := getReferencedServiceAccountToken(c, myns, readOnlyServiceAccountName, true) + + roTokenName := "ro-test-token" + secret, err := createServiceAccountToken(c, roSA, myns, roTokenName) if err != nil { - t.Fatal(err) + t.Fatalf("Secret not created: %v", err) } roClientConfig := config - roClientConfig.BearerToken = roToken + roClientConfig.BearerToken = string(secret.Data[v1.ServiceAccountTokenKey]) roClient := clientset.NewForConfigOrDie(&roClientConfig) doServiceAccountAPIRequests(t, roClient, myns, true, true, false) doServiceAccountAPIRequests(t, roClient, otherns, true, false, false) @@ -278,45 +285,23 @@ func TestServiceAccountTokenAuthentication(t *testing.T) { if err != nil { t.Fatalf("could not delete token: %v", err) } - // wait for delete to be observed and reacted to via watch - wait.PollImmediate(100*time.Millisecond, 30*time.Second, func() (bool, error) { - sa, err := c.CoreV1().ServiceAccounts(myns).Get(context.TODO(), readOnlyServiceAccountName, metav1.GetOptions{}) - if err != nil { - return false, err - } - for _, secretRef := range sa.Secrets { - if secretRef.Name == roTokenName { - return false, nil - } - } - return true, nil - }) doServiceAccountAPIRequests(t, roClient, myns, false, false, false) // Create "rw" user in myns - _, err = c.CoreV1().ServiceAccounts(myns).Create(context.TODO(), &v1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Name: readWriteServiceAccountName}}, metav1.CreateOptions{}) + rwSA, err := c.CoreV1().ServiceAccounts(myns).Create(context.TODO(), &v1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Name: readWriteServiceAccountName}}, metav1.CreateOptions{}) if err != nil { t.Fatalf("Service Account not created: %v", err) } - _, rwToken, err := getReferencedServiceAccountToken(c, myns, readWriteServiceAccountName, true) + rwTokenName := "rw-test-token" + secret, err = createServiceAccountToken(c, rwSA, myns, rwTokenName) if err != nil { - t.Fatal(err) + t.Fatalf("Secret not created: %v", err) } rwClientConfig := config - rwClientConfig.BearerToken = rwToken + rwClientConfig.BearerToken = string(secret.Data[v1.ServiceAccountTokenKey]) rwClient := clientset.NewForConfigOrDie(&rwClientConfig) doServiceAccountAPIRequests(t, rwClient, myns, true, true, true) doServiceAccountAPIRequests(t, rwClient, otherns, true, false, false) - - // Get default user and token which should have been automatically created - _, defaultToken, err := getReferencedServiceAccountToken(c, myns, "default", true) - if err != nil { - t.Fatalf("could not get default user and token: %v", err) - } - defaultClientConfig := config - defaultClientConfig.BearerToken = defaultToken - defaultClient := clientset.NewForConfigOrDie(&defaultClientConfig) - doServiceAccountAPIRequests(t, defaultClient, myns, true, false, false) } // startServiceAccountTestServer returns a started server @@ -424,7 +409,10 @@ func startServiceAccountTestServer(t *testing.T) (*clientset.Clientset, restclie informers.Core().V1().ServiceAccounts(), informers.Core().V1().Secrets(), rootClientset, - serviceaccountcontroller.TokensControllerOptions{TokenGenerator: tokenGenerator}, + serviceaccountcontroller.TokensControllerOptions{ + TokenGenerator: tokenGenerator, + AutoGenerate: !utilfeature.DefaultFeatureGate.Enabled(kubefeatures.LegacyServiceAccountTokenNoAutoGeneration), + }, ) if err != nil { return rootClientset, clientConfig, stop, err @@ -467,6 +455,36 @@ func getServiceAccount(c *clientset.Clientset, ns string, name string, shouldWai return user, err } +func createServiceAccountToken(c *clientset.Clientset, sa *v1.ServiceAccount, ns string, name string) (*v1.Secret, error) { + secret := &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: ns, + Annotations: map[string]string{ + v1.ServiceAccountNameKey: sa.GetName(), + v1.ServiceAccountUIDKey: string(sa.UID), + }, + }, + Type: v1.SecretTypeServiceAccountToken, + Data: map[string][]byte{}, + } + secret, err := c.CoreV1().Secrets(ns).Create(context.TODO(), secret, metav1.CreateOptions{}) + if err != nil { + return nil, fmt.Errorf("failed to create secret (%s:%s) %+v, err: %v", ns, secret.Name, *secret, err) + } + err = wait.Poll(time.Second, 10*time.Second, func() (bool, error) { + if len(secret.Data[v1.ServiceAccountTokenKey]) != 0 { + return false, nil + } + secret, err = c.CoreV1().Secrets(ns).Get(context.TODO(), name, metav1.GetOptions{}) + if err != nil { + return true, fmt.Errorf("failed to get secret (%s:%s) %+v, err: %v", ns, secret.Name, *secret, err) + } + return true, nil + }) + return secret, nil +} + func getReferencedServiceAccountToken(c *clientset.Clientset, ns string, name string, shouldWait bool) (string, string, error) { tokenName := "" token := ""