From a9dc919f82b8b268990006a622157acf2cd80a9e Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Thu, 6 Dec 2018 14:00:53 -0500 Subject: [PATCH] Look up service accounts from informer before trying live lookup --- cmd/kube-apiserver/app/server.go | 7 +++- pkg/controller/serviceaccount/tokengetter.go | 19 ++++++++-- pkg/serviceaccount/BUILD | 2 ++ pkg/serviceaccount/jwt_test.go | 36 ++++++++++++++++++- test/integration/auth/BUILD | 2 ++ test/integration/auth/svcaccttoken_test.go | 35 +++++++++++++++++- .../serviceaccount/service_account_test.go | 13 +++++-- 7 files changed, 105 insertions(+), 9 deletions(-) diff --git a/cmd/kube-apiserver/app/server.go b/cmd/kube-apiserver/app/server.go index 0151fa1b7f..2b17c20d8b 100644 --- a/cmd/kube-apiserver/app/server.go +++ b/cmd/kube-apiserver/app/server.go @@ -504,7 +504,12 @@ func buildGenericConfig( func BuildAuthenticator(s *options.ServerRunOptions, extclient clientgoclientset.Interface, versionedInformer clientgoinformers.SharedInformerFactory) (authenticator.Request, *spec.SecurityDefinitions, error) { authenticatorConfig := s.Authentication.ToAuthenticationConfig() if s.Authentication.ServiceAccounts.Lookup { - authenticatorConfig.ServiceAccountTokenGetter = serviceaccountcontroller.NewGetterFromClient(extclient) + authenticatorConfig.ServiceAccountTokenGetter = serviceaccountcontroller.NewGetterFromClient( + extclient, + versionedInformer.Core().V1().Secrets().Lister(), + versionedInformer.Core().V1().ServiceAccounts().Lister(), + versionedInformer.Core().V1().Pods().Lister(), + ) } authenticatorConfig.BootstrapTokenAuthenticator = bootstrap.NewTokenAuthenticator( versionedInformer.Core().V1().Secrets().Lister().Secrets(v1.NamespaceSystem), diff --git a/pkg/controller/serviceaccount/tokengetter.go b/pkg/controller/serviceaccount/tokengetter.go index ed86489639..25baf01d8d 100644 --- a/pkg/controller/serviceaccount/tokengetter.go +++ b/pkg/controller/serviceaccount/tokengetter.go @@ -20,30 +20,43 @@ import ( "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" clientset "k8s.io/client-go/kubernetes" + v1listers "k8s.io/client-go/listers/core/v1" "k8s.io/kubernetes/pkg/serviceaccount" ) // clientGetter implements ServiceAccountTokenGetter using a clientset.Interface type clientGetter struct { - client clientset.Interface + client clientset.Interface + secretLister v1listers.SecretLister + serviceAccountLister v1listers.ServiceAccountLister + podLister v1listers.PodLister } // NewGetterFromClient returns a ServiceAccountTokenGetter that // uses the specified client to retrieve service accounts and secrets. // The client should NOT authenticate using a service account token // the returned getter will be used to retrieve, or recursion will result. -func NewGetterFromClient(c clientset.Interface) serviceaccount.ServiceAccountTokenGetter { - return clientGetter{c} +func NewGetterFromClient(c clientset.Interface, secretLister v1listers.SecretLister, serviceAccountLister v1listers.ServiceAccountLister, podLister v1listers.PodLister) serviceaccount.ServiceAccountTokenGetter { + return clientGetter{c, secretLister, serviceAccountLister, podLister} } func (c clientGetter) GetServiceAccount(namespace, name string) (*v1.ServiceAccount, error) { + if serviceAccount, err := c.serviceAccountLister.ServiceAccounts(namespace).Get(name); err == nil { + return serviceAccount, nil + } return c.client.CoreV1().ServiceAccounts(namespace).Get(name, metav1.GetOptions{}) } func (c clientGetter) GetPod(namespace, name string) (*v1.Pod, error) { + if pod, err := c.podLister.Pods(namespace).Get(name); err == nil { + return pod, nil + } return c.client.CoreV1().Pods(namespace).Get(name, metav1.GetOptions{}) } func (c clientGetter) GetSecret(namespace, name string) (*v1.Secret, error) { + if secret, err := c.secretLister.Secrets(namespace).Get(name); err == nil { + return secret, nil + } return c.client.CoreV1().Secrets(namespace).Get(name, metav1.GetOptions{}) } diff --git a/pkg/serviceaccount/BUILD b/pkg/serviceaccount/BUILD index d0ad133b62..5639e80f19 100644 --- a/pkg/serviceaccount/BUILD +++ b/pkg/serviceaccount/BUILD @@ -57,6 +57,8 @@ go_test( "//staging/src/k8s.io/apiserver/pkg/authentication/authenticator:go_default_library", "//staging/src/k8s.io/client-go/kubernetes:go_default_library", "//staging/src/k8s.io/client-go/kubernetes/fake:go_default_library", + "//staging/src/k8s.io/client-go/listers/core/v1:go_default_library", + "//staging/src/k8s.io/client-go/tools/cache:go_default_library", "//staging/src/k8s.io/client-go/util/cert:go_default_library", "//vendor/gopkg.in/square/go-jose.v2/jwt:go_default_library", ], diff --git a/pkg/serviceaccount/jwt_test.go b/pkg/serviceaccount/jwt_test.go index 36be027c09..6a74099ce2 100644 --- a/pkg/serviceaccount/jwt_test.go +++ b/pkg/serviceaccount/jwt_test.go @@ -19,6 +19,7 @@ package serviceaccount_test import ( "context" "reflect" + "strings" "testing" "k8s.io/api/core/v1" @@ -26,6 +27,8 @@ import ( "k8s.io/apiserver/pkg/authentication/authenticator" clientset "k8s.io/client-go/kubernetes" "k8s.io/client-go/kubernetes/fake" + v1listers "k8s.io/client-go/listers/core/v1" + "k8s.io/client-go/tools/cache" certutil "k8s.io/client-go/util/cert" serviceaccountcontroller "k8s.io/kubernetes/pkg/controller/serviceaccount" "k8s.io/kubernetes/pkg/serviceaccount" @@ -277,7 +280,18 @@ func TestTokenGenerateAndValidate(t *testing.T) { for k, tc := range testCases { auds := authenticator.Audiences{"api"} - getter := serviceaccountcontroller.NewGetterFromClient(tc.Client) + getter := serviceaccountcontroller.NewGetterFromClient( + tc.Client, + v1listers.NewSecretLister(newIndexer(func(namespace, name string) (interface{}, error) { + return tc.Client.CoreV1().Secrets(namespace).Get(name, metav1.GetOptions{}) + })), + v1listers.NewServiceAccountLister(newIndexer(func(namespace, name string) (interface{}, error) { + return tc.Client.CoreV1().ServiceAccounts(namespace).Get(name, metav1.GetOptions{}) + })), + v1listers.NewPodLister(newIndexer(func(namespace, name string) (interface{}, error) { + return tc.Client.CoreV1().Pods(namespace).Get(name, metav1.GetOptions{}) + })), + ) authn := serviceaccount.JWTTokenAuthenticator(serviceaccount.LegacyIssuer, tc.Keys, auds, serviceaccount.NewLegacyValidator(tc.Client != nil, getter)) // An invalid, non-JWT token should always fail @@ -316,3 +330,23 @@ func TestTokenGenerateAndValidate(t *testing.T) { } } } + +func newIndexer(get func(namespace, name string) (interface{}, error)) cache.Indexer { + return &fakeIndexer{get: get} +} + +type fakeIndexer struct { + cache.Indexer + get func(namespace, name string) (interface{}, error) +} + +func (f *fakeIndexer) GetByKey(key string) (interface{}, bool, error) { + parts := strings.SplitN(key, "/", 2) + namespace := parts[0] + name := "" + if len(parts) == 2 { + name = parts[1] + } + obj, err := f.get(namespace, name) + return obj, err == nil, err +} diff --git a/test/integration/auth/BUILD b/test/integration/auth/BUILD index af4345306b..4a537465fd 100644 --- a/test/integration/auth/BUILD +++ b/test/integration/auth/BUILD @@ -78,7 +78,9 @@ go_test( "//staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/tokentest:go_default_library", "//staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/webhook:go_default_library", "//staging/src/k8s.io/client-go/kubernetes:go_default_library", + "//staging/src/k8s.io/client-go/listers/core/v1:go_default_library", "//staging/src/k8s.io/client-go/rest:go_default_library", + "//staging/src/k8s.io/client-go/tools/cache:go_default_library", "//staging/src/k8s.io/client-go/tools/clientcmd/api/v1:go_default_library", "//staging/src/k8s.io/client-go/tools/watch:go_default_library", "//staging/src/k8s.io/client-go/transport:go_default_library", diff --git a/test/integration/auth/svcaccttoken_test.go b/test/integration/auth/svcaccttoken_test.go index 65fcbba58c..0963ffeea4 100644 --- a/test/integration/auth/svcaccttoken_test.go +++ b/test/integration/auth/svcaccttoken_test.go @@ -39,6 +39,8 @@ import ( utilfeature "k8s.io/apiserver/pkg/util/feature" utilfeaturetesting "k8s.io/apiserver/pkg/util/feature/testing" clientset "k8s.io/client-go/kubernetes" + v1listers "k8s.io/client-go/listers/core/v1" + "k8s.io/client-go/tools/cache" certutil "k8s.io/client-go/util/cert" "k8s.io/kubernetes/pkg/apis/core" serviceaccountgetter "k8s.io/kubernetes/pkg/controller/serviceaccount" @@ -83,7 +85,18 @@ func TestServiceAccountTokenCreate(t *testing.T) { iss, []interface{}{&pk}, aud, - serviceaccount.NewValidator(serviceaccountgetter.NewGetterFromClient(gcs)), + serviceaccount.NewValidator(serviceaccountgetter.NewGetterFromClient( + gcs, + v1listers.NewSecretLister(newIndexer(func(namespace, name string) (interface{}, error) { + return gcs.CoreV1().Secrets(namespace).Get(name, metav1.GetOptions{}) + })), + v1listers.NewServiceAccountLister(newIndexer(func(namespace, name string) (interface{}, error) { + return gcs.CoreV1().ServiceAccounts(namespace).Get(name, metav1.GetOptions{}) + })), + v1listers.NewPodLister(newIndexer(func(namespace, name string) (interface{}, error) { + return gcs.CoreV1().Pods(namespace).Get(name, metav1.GetOptions{}) + })), + )), ), ) tokenGenerator, err := serviceaccount.JWTTokenGenerator(iss, sk) @@ -683,3 +696,23 @@ func createDeleteSecret(t *testing.T, cs clientset.Interface, sec *v1.Secret) (* } } } + +func newIndexer(get func(namespace, name string) (interface{}, error)) cache.Indexer { + return &fakeIndexer{get: get} +} + +type fakeIndexer struct { + cache.Indexer + get func(namespace, name string) (interface{}, error) +} + +func (f *fakeIndexer) GetByKey(key string) (interface{}, bool, error) { + parts := strings.SplitN(key, "/", 2) + namespace := parts[0] + name := "" + if len(parts) == 2 { + name = parts[1] + } + obj, err := f.get(namespace, name) + return obj, err == nil, err +} diff --git a/test/integration/serviceaccount/service_account_test.go b/test/integration/serviceaccount/service_account_test.go index 5139911db6..70f4aceaf9 100644 --- a/test/integration/serviceaccount/service_account_test.go +++ b/test/integration/serviceaccount/service_account_test.go @@ -363,6 +363,10 @@ func startServiceAccountTestServer(t *testing.T) (*clientset.Clientset, restclie // TODO: remove rootClient after we refactor pkg/admission to use the clientset. rootClientset := clientset.NewForConfigOrDie(&restclient.Config{Host: apiServer.URL, ContentConfig: restclient.ContentConfig{GroupVersion: &schema.GroupVersion{Group: "", Version: "v1"}}, BearerToken: rootToken}) externalRootClientset := kubernetes.NewForConfigOrDie(&restclient.Config{Host: apiServer.URL, ContentConfig: restclient.ContentConfig{GroupVersion: &schema.GroupVersion{Group: "", Version: "v1"}}, BearerToken: rootToken}) + + externalInformers := informers.NewSharedInformerFactory(externalRootClientset, controller.NoResyncPeriodFunc()) + informers := informers.NewSharedInformerFactory(rootClientset, controller.NoResyncPeriodFunc()) + // Set up two authenticators: // 1. A token authenticator that maps the rootToken to the "root" user // 2. A ServiceAccountToken authenticator that validates ServiceAccount tokens @@ -373,7 +377,12 @@ func startServiceAccountTestServer(t *testing.T) (*clientset.Clientset, restclie return nil, false, nil }) serviceAccountKey, _ := rsa.GenerateKey(rand.Reader, 2048) - serviceAccountTokenGetter := serviceaccountcontroller.NewGetterFromClient(rootClientset) + serviceAccountTokenGetter := serviceaccountcontroller.NewGetterFromClient( + rootClientset, + externalInformers.Core().V1().Secrets().Lister(), + externalInformers.Core().V1().ServiceAccounts().Lister(), + externalInformers.Core().V1().Pods().Lister(), + ) serviceAccountTokenAuth := serviceaccount.JWTTokenAuthenticator(serviceaccount.LegacyIssuer, []interface{}{&serviceAccountKey.PublicKey}, nil, serviceaccount.NewLegacyValidator(true, serviceAccountTokenGetter)) authenticator := union.New( bearertoken.New(rootTokenAuth), @@ -418,9 +427,7 @@ func startServiceAccountTestServer(t *testing.T) (*clientset.Clientset, restclie // Set up admission plugin to auto-assign serviceaccounts to pods serviceAccountAdmission := serviceaccountadmission.NewServiceAccount() serviceAccountAdmission.SetExternalKubeClientSet(externalRootClientset) - externalInformers := informers.NewSharedInformerFactory(externalRootClientset, controller.NoResyncPeriodFunc()) serviceAccountAdmission.SetExternalKubeInformerFactory(externalInformers) - informers := informers.NewSharedInformerFactory(rootClientset, controller.NoResyncPeriodFunc()) masterConfig := framework.NewMasterConfig() masterConfig.GenericConfig.EnableIndex = true