Split the serviceaccount package into two parts

Public utility methods and JWT parsing, and controller specific logic.
Also remove the coupling between ServiceAccountTokenGetter and the
authenticator class.

cmd/kube-apiserver/app/server.go

@@ -45,10 +45,12 @@ import (
 	"k8s.io/kubernetes/pkg/capabilities"
 	client "k8s.io/kubernetes/pkg/client/unversioned"
 	"k8s.io/kubernetes/pkg/cloudprovider"
+	serviceaccountcontroller "k8s.io/kubernetes/pkg/controller/serviceaccount"
 	"k8s.io/kubernetes/pkg/genericapiserver"
 	kubeletclient "k8s.io/kubernetes/pkg/kubelet/client"
 	"k8s.io/kubernetes/pkg/master"
 	"k8s.io/kubernetes/pkg/master/ports"
+	"k8s.io/kubernetes/pkg/serviceaccount"
 	"k8s.io/kubernetes/pkg/storage"
 	etcdstorage "k8s.io/kubernetes/pkg/storage/etcd"
 	"k8s.io/kubernetes/pkg/util"
@@ -492,18 +494,26 @@ func (s *APIServer) Run(_ []string) error {
 			glog.Warning("No RSA key provided, service account token authentication disabled")
 		}
 	}
+
+	var serviceAccountGetter serviceaccount.ServiceAccountTokenGetter
+	if s.ServiceAccountLookup {
+		// If we need to look up service accounts and tokens,
+		// go directly to etcd to avoid recursive auth insanity
+		serviceAccountGetter = serviceaccountcontroller.NewGetterFromStorageInterface(etcdStorage)
+	}
+
 	authenticator, err := authenticator.New(authenticator.AuthenticatorConfig{
-		BasicAuthFile:         s.BasicAuthFile,
+		BasicAuthFile:             s.BasicAuthFile,
-		ClientCAFile:          s.ClientCAFile,
+		ClientCAFile:              s.ClientCAFile,
-		TokenAuthFile:         s.TokenAuthFile,
+		TokenAuthFile:             s.TokenAuthFile,
-		OIDCIssuerURL:         s.OIDCIssuerURL,
+		OIDCIssuerURL:             s.OIDCIssuerURL,
-		OIDCClientID:          s.OIDCClientID,
+		OIDCClientID:              s.OIDCClientID,
-		OIDCCAFile:            s.OIDCCAFile,
+		OIDCCAFile:                s.OIDCCAFile,
-		OIDCUsernameClaim:     s.OIDCUsernameClaim,
+		OIDCUsernameClaim:         s.OIDCUsernameClaim,
-		ServiceAccountKeyFile: s.ServiceAccountKeyFile,
+		ServiceAccountKeyFile:     s.ServiceAccountKeyFile,
-		ServiceAccountLookup:  s.ServiceAccountLookup,
+		ServiceAccountLookup:      s.ServiceAccountLookup,
-		Storage:               etcdStorage,
+		ServiceAccountTokenGetter: serviceAccountGetter,
-		KeystoneURL:           s.KeystoneURL,
+		KeystoneURL:               s.KeystoneURL,
 	})
 
 	if err != nil {

cmd/kube-controller-manager/app/controllermanager.go

@@ -51,9 +51,10 @@ import (
 	resourcequotacontroller "k8s.io/kubernetes/pkg/controller/resourcequota"
 	routecontroller "k8s.io/kubernetes/pkg/controller/route"
 	servicecontroller "k8s.io/kubernetes/pkg/controller/service"
-	"k8s.io/kubernetes/pkg/controller/serviceaccount"
+	serviceaccountcontroller "k8s.io/kubernetes/pkg/controller/serviceaccount"
 	"k8s.io/kubernetes/pkg/healthz"
 	"k8s.io/kubernetes/pkg/master/ports"
+	"k8s.io/kubernetes/pkg/serviceaccount"
 	"k8s.io/kubernetes/pkg/util"
 	"k8s.io/kubernetes/pkg/util/wait"
 
@@ -433,9 +434,9 @@ func (s *CMServer) Run(_ []string) error {
 		if err != nil {
 			glog.Errorf("Error reading key for service account token controller: %v", err)
 		} else {
-			serviceaccount.NewTokensController(
+			serviceaccountcontroller.NewTokensController(
 				clientForUserAgentOrDie(*kubeconfig, "tokens-controller"),
-				serviceaccount.TokensControllerOptions{
+				serviceaccountcontroller.TokensControllerOptions{
 					TokenGenerator: serviceaccount.JWTTokenGenerator(privateKey),
 					RootCA:         rootCA,
 				},
@@ -443,9 +444,9 @@ func (s *CMServer) Run(_ []string) error {
 		}
 	}
 
-	serviceaccount.NewServiceAccountsController(
+	serviceaccountcontroller.NewServiceAccountsController(
 		clientForUserAgentOrDie(*kubeconfig, "service-account-controller"),
-		serviceaccount.DefaultServiceAccountsControllerOptions(),
+		serviceaccountcontroller.DefaultServiceAccountsControllerOptions(),
 	).Run()
 
 	select {}

contrib/mesos/pkg/controllermanager/controllermanager.go

@@ -43,8 +43,9 @@ import (
 	resourcequotacontroller "k8s.io/kubernetes/pkg/controller/resourcequota"
 	routecontroller "k8s.io/kubernetes/pkg/controller/route"
 	servicecontroller "k8s.io/kubernetes/pkg/controller/service"
-	"k8s.io/kubernetes/pkg/controller/serviceaccount"
+	serviceaccountcontroller "k8s.io/kubernetes/pkg/controller/serviceaccount"
 	"k8s.io/kubernetes/pkg/healthz"
+	"k8s.io/kubernetes/pkg/serviceaccount"
 	"k8s.io/kubernetes/pkg/util"
 
 	"k8s.io/kubernetes/contrib/mesos/pkg/profile"
@@ -209,9 +210,9 @@ func (s *CMServer) Run(_ []string) error {
 		if err != nil {
 			glog.Errorf("Error reading key for service account token controller: %v", err)
 		} else {
-			serviceaccount.NewTokensController(
+			serviceaccountcontroller.NewTokensController(
 				kubeClient,
-				serviceaccount.TokensControllerOptions{
+				serviceaccountcontroller.TokensControllerOptions{
 					TokenGenerator: serviceaccount.JWTTokenGenerator(privateKey),
 					RootCA:         rootCA,
 				},
@@ -219,9 +220,9 @@ func (s *CMServer) Run(_ []string) error {
 		}
 	}
 
-	serviceaccount.NewServiceAccountsController(
+	serviceaccountcontroller.NewServiceAccountsController(
 		kubeClient,
-		serviceaccount.DefaultServiceAccountsControllerOptions(),
+		serviceaccountcontroller.DefaultServiceAccountsControllerOptions(),
 	).Run()
 
 	select {}

pkg/apiserver/authenticator/authn.go

@@ -21,8 +21,7 @@ import (
 
 	"k8s.io/kubernetes/pkg/auth/authenticator"
 	"k8s.io/kubernetes/pkg/auth/authenticator/bearertoken"
-	"k8s.io/kubernetes/pkg/controller/serviceaccount"
+	"k8s.io/kubernetes/pkg/serviceaccount"
-	"k8s.io/kubernetes/pkg/storage"
 	"k8s.io/kubernetes/pkg/util"
 	"k8s.io/kubernetes/plugin/pkg/auth/authenticator/password/passwordfile"
 	"k8s.io/kubernetes/plugin/pkg/auth/authenticator/request/basicauth"
@@ -34,17 +33,17 @@ import (
 )
 
 type AuthenticatorConfig struct {
-	BasicAuthFile         string
+	BasicAuthFile             string
-	ClientCAFile          string
+	ClientCAFile              string
-	TokenAuthFile         string
+	TokenAuthFile             string
-	OIDCIssuerURL         string
+	OIDCIssuerURL             string
-	OIDCClientID          string
+	OIDCClientID              string
-	OIDCCAFile            string
+	OIDCCAFile                string
-	OIDCUsernameClaim     string
+	OIDCUsernameClaim         string
-	ServiceAccountKeyFile string
+	ServiceAccountKeyFile     string
-	ServiceAccountLookup  bool
+	ServiceAccountLookup      bool
-	Storage               storage.Interface
+	ServiceAccountTokenGetter serviceaccount.ServiceAccountTokenGetter
-	KeystoneURL           string
+	KeystoneURL               string
 }
 
 // New returns an authenticator.Request or an error that supports the standard
@@ -85,7 +84,7 @@ func New(config AuthenticatorConfig) (authenticator.Request, error) {
 	}
 
 	if len(config.ServiceAccountKeyFile) > 0 {
-		serviceAccountAuth, err := newServiceAccountAuthenticator(config.ServiceAccountKeyFile, config.ServiceAccountLookup, config.Storage)
+		serviceAccountAuth, err := newServiceAccountAuthenticator(config.ServiceAccountKeyFile, config.ServiceAccountLookup, config.ServiceAccountTokenGetter)
 		if err != nil {
 			return nil, err
 		}
@@ -147,19 +146,12 @@ func newAuthenticatorFromOIDCIssuerURL(issuerURL, clientID, caFile, usernameClai
 }
 
 // newServiceAccountAuthenticator returns an authenticator.Request or an error
-func newServiceAccountAuthenticator(keyfile string, lookup bool, storage storage.Interface) (authenticator.Request, error) {
+func newServiceAccountAuthenticator(keyfile string, lookup bool, serviceAccountGetter serviceaccount.ServiceAccountTokenGetter) (authenticator.Request, error) {
 	publicKey, err := serviceaccount.ReadPublicKey(keyfile)
 	if err != nil {
 		return nil, err
 	}
 
-	var serviceAccountGetter serviceaccount.ServiceAccountTokenGetter
-	if lookup {
-		// If we need to look up service accounts and tokens,
-		// go directly to etcd to avoid recursive auth insanity
-		serviceAccountGetter = serviceaccount.NewGetterFromStorageInterface(storage)
-	}
-
 	tokenAuthenticator := serviceaccount.JWTTokenAuthenticator([]*rsa.PublicKey{publicKey}, lookup, serviceAccountGetter)
 	return bearertoken.New(tokenAuthenticator), nil
 }

pkg/controller/serviceaccount/tokengetter.go

@@ -22,17 +22,12 @@ import (
 	"k8s.io/kubernetes/pkg/registry/generic"
 	"k8s.io/kubernetes/pkg/registry/secret"
 	secretetcd "k8s.io/kubernetes/pkg/registry/secret/etcd"
-	"k8s.io/kubernetes/pkg/registry/serviceaccount"
+	serviceaccountregistry "k8s.io/kubernetes/pkg/registry/serviceaccount"
 	serviceaccountetcd "k8s.io/kubernetes/pkg/registry/serviceaccount/etcd"
+	"k8s.io/kubernetes/pkg/serviceaccount"
 	"k8s.io/kubernetes/pkg/storage"
 )
 
-// ServiceAccountTokenGetter defines functions to retrieve a named service account and secret
-type ServiceAccountTokenGetter interface {
-	GetServiceAccount(namespace, name string) (*api.ServiceAccount, error)
-	GetSecret(namespace, name string) (*api.Secret, error)
-}
-
 // clientGetter implements ServiceAccountTokenGetter using a client.Interface
 type clientGetter struct {
 	client client.Interface
@@ -42,7 +37,7 @@ type clientGetter struct {
 // 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 client.Interface) ServiceAccountTokenGetter {
+func NewGetterFromClient(c client.Interface) serviceaccount.ServiceAccountTokenGetter {
 	return clientGetter{c}
 }
 func (c clientGetter) GetServiceAccount(namespace, name string) (*api.ServiceAccount, error) {
@@ -54,13 +49,13 @@ func (c clientGetter) GetSecret(namespace, name string) (*api.Secret, error) {
 
 // registryGetter implements ServiceAccountTokenGetter using a service account and secret registry
 type registryGetter struct {
-	serviceAccounts serviceaccount.Registry
+	serviceAccounts serviceaccountregistry.Registry
 	secrets         secret.Registry
 }
 
 // NewGetterFromRegistries returns a ServiceAccountTokenGetter that
 // uses the specified registries to retrieve service accounts and secrets.
-func NewGetterFromRegistries(serviceAccounts serviceaccount.Registry, secrets secret.Registry) ServiceAccountTokenGetter {
+func NewGetterFromRegistries(serviceAccounts serviceaccountregistry.Registry, secrets secret.Registry) serviceaccount.ServiceAccountTokenGetter {
 	return &registryGetter{serviceAccounts, secrets}
 }
 func (r *registryGetter) GetServiceAccount(namespace, name string) (*api.ServiceAccount, error) {
@@ -74,9 +69,9 @@ func (r *registryGetter) GetSecret(namespace, name string) (*api.Secret, error)
 
 // NewGetterFromStorageInterface returns a ServiceAccountTokenGetter that
 // uses the specified storage to retrieve service accounts and secrets.
-func NewGetterFromStorageInterface(s storage.Interface) ServiceAccountTokenGetter {
+func NewGetterFromStorageInterface(s storage.Interface) serviceaccount.ServiceAccountTokenGetter {
 	return NewGetterFromRegistries(
-		serviceaccount.NewRegistry(serviceaccountetcd.NewREST(s, generic.UndecoratedStorage)),
+		serviceaccountregistry.NewRegistry(serviceaccountetcd.NewREST(s, generic.UndecoratedStorage)),
 		secret.NewRegistry(secretetcd.NewREST(s, generic.UndecoratedStorage)),
 	)
 }

pkg/controller/serviceaccount/tokens_controller.go

@@ -30,6 +30,7 @@ import (
 	"k8s.io/kubernetes/pkg/fields"
 	"k8s.io/kubernetes/pkg/registry/secret"
 	"k8s.io/kubernetes/pkg/runtime"
+	"k8s.io/kubernetes/pkg/serviceaccount"
 	"k8s.io/kubernetes/pkg/util/sets"
 	"k8s.io/kubernetes/pkg/util/wait"
 	"k8s.io/kubernetes/pkg/watch"
@@ -40,7 +41,7 @@ const NumServiceAccountRemoveReferenceRetries = 10
 // TokensControllerOptions contains options for the TokensController
 type TokensControllerOptions struct {
 	// TokenGenerator is the generator to use to create new tokens
-	TokenGenerator TokenGenerator
+	TokenGenerator serviceaccount.TokenGenerator
 	// ServiceAccountResync is the time.Duration at which to fully re-list service accounts.
 	// If zero, re-list will be delayed as long as possible
 	ServiceAccountResync time.Duration
@@ -111,7 +112,7 @@ type TokensController struct {
 	stopChan chan struct{}
 
 	client client.Interface
-	token  TokenGenerator
+	token  serviceaccount.TokenGenerator
 
 	rootCA []byte
 
@@ -451,7 +452,7 @@ func (e *TokensController) getServiceAccount(secret *api.Secret, fetchOnCacheMis
 	for _, obj := range namespaceAccounts {
 		serviceAccount := obj.(*api.ServiceAccount)
 
-		if IsServiceAccountToken(secret, serviceAccount) {
+		if serviceaccount.IsServiceAccountToken(secret, serviceAccount) {
 			return serviceAccount, nil
 		}
 	}
@@ -465,7 +466,7 @@ func (e *TokensController) getServiceAccount(secret *api.Secret, fetchOnCacheMis
 			return nil, err
 		}
 
-		if IsServiceAccountToken(secret, serviceAccount) {
+		if serviceaccount.IsServiceAccountToken(secret, serviceAccount) {
 			return serviceAccount, nil
 		}
 	}
@@ -486,7 +487,7 @@ func (e *TokensController) listTokenSecrets(serviceAccount *api.ServiceAccount)
 	for _, obj := range namespaceSecrets {
 		secret := obj.(*api.Secret)
 
-		if IsServiceAccountToken(secret, serviceAccount) {
+		if serviceaccount.IsServiceAccountToken(secret, serviceAccount) {
 			items = append(items, secret)
 		}
 	}

pkg/serviceaccount/jwt.go

@@ -42,6 +42,12 @@ const (
 	NamespaceClaim          = "kubernetes.io/serviceaccount/namespace"
 )
 
+// ServiceAccountTokenGetter defines functions to retrieve a named service account and secret
+type ServiceAccountTokenGetter interface {
+	GetServiceAccount(namespace, name string) (*api.ServiceAccount, error)
+	GetSecret(namespace, name string) (*api.Secret, error)
+}
+
 type TokenGenerator interface {
 	// GenerateToken generates a token which will identify the given ServiceAccount.
 	// The returned token will be stored in the given (and yet-unpersisted) Secret.

pkg/serviceaccount/jwt_test.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package serviceaccount
+package serviceaccount_test
 
 import (
 	"crypto/rsa"
@@ -24,9 +24,12 @@ import (
 	"testing"
 
 	"github.com/dgrijalva/jwt-go"
+
 	"k8s.io/kubernetes/pkg/api"
 	client "k8s.io/kubernetes/pkg/client/unversioned"
 	"k8s.io/kubernetes/pkg/client/unversioned/testclient"
+	serviceaccountcontroller "k8s.io/kubernetes/pkg/controller/serviceaccount"
+	"k8s.io/kubernetes/pkg/serviceaccount"
 )
 
 const otherPublicKey = `-----BEGIN PUBLIC KEY-----
@@ -100,7 +103,7 @@ func TestReadPrivateKey(t *testing.T) {
 		t.Fatalf("error creating tmpfile: %v", err)
 	}
 
-	if _, err := ReadPrivateKey(f.Name()); err != nil {
+	if _, err := serviceaccount.ReadPrivateKey(f.Name()); err != nil {
 		t.Fatalf("error reading key: %v", err)
 	}
 }
@@ -116,7 +119,7 @@ func TestReadPublicKey(t *testing.T) {
 		t.Fatalf("error creating tmpfile: %v", err)
 	}
 
-	if _, err := ReadPublicKey(f.Name()); err != nil {
+	if _, err := serviceaccount.ReadPublicKey(f.Name()); err != nil {
 		t.Fatalf("error reading key: %v", err)
 	}
 }
@@ -141,7 +144,7 @@ func TestTokenGenerateAndValidate(t *testing.T) {
 	}
 
 	// Generate the token
-	generator := JWTTokenGenerator(getPrivateKey(privateKey))
+	generator := serviceaccount.JWTTokenGenerator(getPrivateKey(privateKey))
 	token, err := generator.GenerateToken(*serviceAccount, *secret)
 	if err != nil {
 		t.Fatalf("error generating token: %v", err)
@@ -219,8 +222,8 @@ func TestTokenGenerateAndValidate(t *testing.T) {
 	}
 
 	for k, tc := range testCases {
-		getter := NewGetterFromClient(tc.Client)
+		getter := serviceaccountcontroller.NewGetterFromClient(tc.Client)
-		authenticator := JWTTokenAuthenticator(tc.Keys, tc.Client != nil, getter)
+		authenticator := serviceaccount.JWTTokenAuthenticator(tc.Keys, tc.Client != nil, getter)
 
 		user, ok, err := authenticator.AuthenticateToken(token)
 		if (err != nil) != tc.ExpectedErr {
@@ -253,8 +256,8 @@ func TestTokenGenerateAndValidate(t *testing.T) {
 }
 
 func TestMakeSplitUsername(t *testing.T) {
-	username := MakeUsername("ns", "name")
+	username := serviceaccount.MakeUsername("ns", "name")
-	ns, name, err := SplitUsername(username)
+	ns, name, err := serviceaccount.SplitUsername(username)
 	if err != nil {
 		t.Errorf("Unexpected error %v", err)
 	}
@@ -264,7 +267,7 @@ func TestMakeSplitUsername(t *testing.T) {
 
 	invalid := []string{"test", "system:serviceaccount", "system:serviceaccount:", "system:serviceaccount:ns", "system:serviceaccount:ns:name:extra"}
 	for _, n := range invalid {
-		_, _, err := SplitUsername("test")
+		_, _, err := serviceaccount.SplitUsername("test")
 		if err == nil {
 			t.Errorf("Expected error for %s", n)
 		}

plugin/pkg/admission/serviceaccount/admission.go

@@ -28,10 +28,10 @@ import (
 	"k8s.io/kubernetes/pkg/api/errors"
 	"k8s.io/kubernetes/pkg/client/cache"
 	client "k8s.io/kubernetes/pkg/client/unversioned"
-	"k8s.io/kubernetes/pkg/controller/serviceaccount"
 	"k8s.io/kubernetes/pkg/fields"
 	kubelet "k8s.io/kubernetes/pkg/kubelet/types"
 	"k8s.io/kubernetes/pkg/runtime"
+	"k8s.io/kubernetes/pkg/serviceaccount"
 	"k8s.io/kubernetes/pkg/util/sets"
 	"k8s.io/kubernetes/pkg/watch"
 )

test/integration/service_account_test.go

@@ -39,8 +39,9 @@ import (
 	"k8s.io/kubernetes/pkg/auth/authorizer"
 	"k8s.io/kubernetes/pkg/auth/user"
 	client "k8s.io/kubernetes/pkg/client/unversioned"
-	"k8s.io/kubernetes/pkg/controller/serviceaccount"
+	serviceaccountcontroller "k8s.io/kubernetes/pkg/controller/serviceaccount"
 	"k8s.io/kubernetes/pkg/master"
+	"k8s.io/kubernetes/pkg/serviceaccount"
 	"k8s.io/kubernetes/pkg/util/sets"
 	"k8s.io/kubernetes/pkg/util/wait"
 	serviceaccountadmission "k8s.io/kubernetes/plugin/pkg/admission/serviceaccount"
@@ -358,7 +359,7 @@ func startServiceAccountTestServer(t *testing.T) (*client.Client, client.Config,
 		return nil, false, nil
 	})
 	serviceAccountKey, _ := rsa.GenerateKey(rand.Reader, 2048)
-	serviceAccountTokenGetter := serviceaccount.NewGetterFromClient(rootClient)
+	serviceAccountTokenGetter := serviceaccountcontroller.NewGetterFromClient(rootClient)
 	serviceAccountTokenAuth := serviceaccount.JWTTokenAuthenticator([]*rsa.PublicKey{&serviceAccountKey.PublicKey}, true, serviceAccountTokenGetter)
 	authenticator := union.New(
 		bearertoken.New(rootTokenAuth),
@@ -410,9 +411,9 @@ func startServiceAccountTestServer(t *testing.T) (*client.Client, client.Config,
 	m = master.New(masterConfig)
 
 	// Start the service account and service account token controllers
-	tokenController := serviceaccount.NewTokensController(rootClient, serviceaccount.TokensControllerOptions{TokenGenerator: serviceaccount.JWTTokenGenerator(serviceAccountKey)})
+	tokenController := serviceaccountcontroller.NewTokensController(rootClient, serviceaccountcontroller.TokensControllerOptions{TokenGenerator: serviceaccount.JWTTokenGenerator(serviceAccountKey)})
 	tokenController.Run()
-	serviceAccountController := serviceaccount.NewServiceAccountsController(rootClient, serviceaccount.DefaultServiceAccountsControllerOptions())
+	serviceAccountController := serviceaccountcontroller.NewServiceAccountsController(rootClient, serviceaccountcontroller.DefaultServiceAccountsControllerOptions())
 	serviceAccountController.Run()
 	// Start the admission plugin reflectors
 	serviceAccountAdmission.Run()