Make controller-manager resilient to stale serviceaccount tokens

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

@@ -135,9 +135,10 @@ func Run(s *options.CloudControllerManagerServer, cloud cloudprovider.Interface)
 		var clientBuilder controller.ControllerClientBuilder
 		if len(s.ServiceAccountKeyFile) > 0 && s.UseServiceAccountCredentials {
 			clientBuilder = controller.SAControllerClientBuilder{
-				ClientConfig: restclient.AnonymousClientConfig(kubeconfig),
-				CoreClient:   kubeClient.Core(),
-				Namespace:    "kube-system",
+				ClientConfig:         restclient.AnonymousClientConfig(kubeconfig),
+				CoreClient:           kubeClient.Core(),
+				AuthenticationClient: kubeClient.Authentication(),
+				Namespace:            "kube-system",
 			}
 		} else {
 			clientBuilder = rootClientBuilder

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

@@ -159,9 +159,10 @@ func Run(s *options.CMServer) error {
 		var clientBuilder controller.ControllerClientBuilder
 		if len(s.ServiceAccountKeyFile) > 0 && s.UseServiceAccountCredentials {
 			clientBuilder = controller.SAControllerClientBuilder{
-				ClientConfig: restclient.AnonymousClientConfig(kubeconfig),
-				CoreClient:   kubeClient.Core(),
-				Namespace:    "kube-system",
+				ClientConfig:         restclient.AnonymousClientConfig(kubeconfig),
+				CoreClient:           kubeClient.Core(),
+				AuthenticationClient: kubeClient.Authentication(),
+				Namespace:            "kube-system",
 			}
 		} else {
 			clientBuilder = rootClientBuilder

pkg/controller/BUILD

@@ -22,8 +22,10 @@ go_library(
         "//pkg/api:go_default_library",
         "//pkg/api/v1:go_default_library",
         "//pkg/api/validation:go_default_library",
+        "//pkg/apis/authentication/v1:go_default_library",
         "//pkg/apis/extensions/v1beta1:go_default_library",
         "//pkg/client/clientset_generated/clientset:go_default_library",
+        "//pkg/client/clientset_generated/clientset/typed/authentication/v1:go_default_library",
         "//pkg/client/clientset_generated/clientset/typed/core/v1:go_default_library",
         "//pkg/client/retry:go_default_library",
         "//pkg/serviceaccount:go_default_library",

pkg/controller/client_builder.go

@@ -31,7 +31,9 @@ import (
 	"k8s.io/client-go/tools/cache"
 	"k8s.io/kubernetes/pkg/api"
 	"k8s.io/kubernetes/pkg/api/v1"
+	v1authenticationapi "k8s.io/kubernetes/pkg/apis/authentication/v1"
 	"k8s.io/kubernetes/pkg/client/clientset_generated/clientset"
+	v1authentication "k8s.io/kubernetes/pkg/client/clientset_generated/clientset/typed/authentication/v1"
 	v1core "k8s.io/kubernetes/pkg/client/clientset_generated/clientset/typed/core/v1"
 	"k8s.io/kubernetes/pkg/serviceaccount"
 
@@ -109,6 +111,10 @@ type SAControllerClientBuilder struct {
 	// to construct a controller client
 	CoreClient v1core.CoreV1Interface
 
+	// AuthenticationClient is used to check API tokens to make sure they are valid before
+	// building a controller client from them
+	AuthenticationClient v1authentication.AuthenticationV1Interface
+
 	// Namespace is the namespace used to host the service accounts that will back the
 	// controllers.  It must be highly privileged namespace which normal users cannot inspect.
 	Namespace string
@@ -117,29 +123,13 @@ type SAControllerClientBuilder struct {
 // config returns a complete clientConfig for constructing clients.  This is separate in anticipation of composition
 // which means that not all clientsets are known here
 func (b SAControllerClientBuilder) Config(name string) (*restclient.Config, error) {
-	clientConfig := restclient.AnonymousClientConfig(b.ClientConfig)
-
-	// we need the SA UID to find a matching SA token
-	sa, err := b.CoreClient.ServiceAccounts(b.Namespace).Get(name, metav1.GetOptions{})
-	if err != nil && !apierrors.IsNotFound(err) {
+	sa, err := b.getOrCreateServiceAccount(name)
+	if err != nil {
 		return nil, err
-	} else if apierrors.IsNotFound(err) {
-		// check to see if the namespace exists.  If it isn't a NotFound, just try to create the SA.
-		// It'll probably fail, but perhaps that will have a better message.
-		if _, err := b.CoreClient.Namespaces().Get(b.Namespace, metav1.GetOptions{}); apierrors.IsNotFound(err) {
-			_, err = b.CoreClient.Namespaces().Create(&v1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: b.Namespace}})
-			if err != nil && !apierrors.IsAlreadyExists(err) {
-				return nil, err
-			}
-		}
-
-		sa, err = b.CoreClient.ServiceAccounts(b.Namespace).Create(
-			&v1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Namespace: b.Namespace, Name: name}})
-		if err != nil {
-			return nil, err
-		}
 	}
 
+	var clientConfig *restclient.Config
+
 	lw := &cache.ListWatch{
 		ListFunc: func(options metav1.ListOptions) (runtime.Object, error) {
 			options.FieldSelector = fields.SelectorFromSet(map[string]string{api.SecretTypeField: string(v1.SecretTypeServiceAccountToken)}).String()
@@ -159,14 +149,32 @@ func (b SAControllerClientBuilder) Config(name string) (*restclient.Config, erro
 				return false, fmt.Errorf("error watching")
 
 			case watch.Added, watch.Modified:
-				secret := event.Object.(*v1.Secret)
-				if !serviceaccount.IsServiceAccountToken(secret, sa) ||
-					len(secret.Data[v1.ServiceAccountTokenKey]) == 0 {
+				secret, ok := event.Object.(*v1.Secret)
+				if !ok {
+					return false, fmt.Errorf("unexpected object type: %T", event.Object)
+				}
+				if !serviceaccount.IsServiceAccountToken(secret, sa) {
+					return false, nil
+				}
+				if len(secret.Data[v1.ServiceAccountTokenKey]) == 0 {
+					return false, nil
+				}
+				validConfig, valid, err := b.getAuthenticatedConfig(sa, string(secret.Data[v1.ServiceAccountTokenKey]))
+				if err != nil {
+					glog.Warningf("error validating API token for %s/%s in secret %s: %v", sa.Name, sa.Namespace, secret.Name, err)
+					// continue watching for good tokens
 					return false, nil
 				}
-				// TODO maybe verify the token is valid
-				clientConfig.BearerToken = string(secret.Data[v1.ServiceAccountTokenKey])
-				restclient.AddUserAgent(clientConfig, apiserverserviceaccount.MakeUsername(b.Namespace, name))
+				if !valid {
+					glog.Warningf("secret %s contained an invalid API token for %s/%s", secret.Name, sa.Name, sa.Namespace)
+					// try to delete the secret containing the invalid token
+					if err := b.CoreClient.Secrets(secret.Namespace).Delete(secret.Name, &metav1.DeleteOptions{}); err != nil && !apierrors.IsNotFound(err) {
+						glog.Warningf("error deleting secret %s containing invalid API token for %s/%s: %v", secret.Name, sa.Name, sa.Namespace, err)
+					}
+					// continue watching for good tokens
+					return false, nil
+				}
+				clientConfig = validConfig
 				return true, nil
 
 			default:
@@ -180,6 +188,69 @@ func (b SAControllerClientBuilder) Config(name string) (*restclient.Config, erro
 	return clientConfig, nil
 }
 
+func (b SAControllerClientBuilder) getOrCreateServiceAccount(name string) (*v1.ServiceAccount, error) {
+	sa, err := b.CoreClient.ServiceAccounts(b.Namespace).Get(name, metav1.GetOptions{})
+	if err == nil {
+		return sa, nil
+	}
+	if !apierrors.IsNotFound(err) {
+		return nil, err
+	}
+
+	// Create the namespace if we can't verify it exists.
+	// Tolerate errors, since we don't know whether this component has namespace creation permissions.
+	if _, err := b.CoreClient.Namespaces().Get(b.Namespace, metav1.GetOptions{}); err != nil {
+		b.CoreClient.Namespaces().Create(&v1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: b.Namespace}})
+	}
+
+	// Create the service account
+	sa, err = b.CoreClient.ServiceAccounts(b.Namespace).Create(&v1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Namespace: b.Namespace, Name: name}})
+	if apierrors.IsAlreadyExists(err) {
+		// If we're racing to init and someone else already created it, re-fetch
+		return b.CoreClient.ServiceAccounts(b.Namespace).Get(name, metav1.GetOptions{})
+	}
+	return sa, err
+}
+
+func (b SAControllerClientBuilder) getAuthenticatedConfig(sa *v1.ServiceAccount, token string) (*restclient.Config, bool, error) {
+	username := apiserverserviceaccount.MakeUsername(sa.Namespace, sa.Name)
+
+	clientConfig := restclient.AnonymousClientConfig(b.ClientConfig)
+	clientConfig.BearerToken = token
+	restclient.AddUserAgent(clientConfig, username)
+
+	// Try token review first
+	tokenReview := &v1authenticationapi.TokenReview{Spec: v1authenticationapi.TokenReviewSpec{Token: token}}
+	if tokenResult, err := b.AuthenticationClient.TokenReviews().Create(tokenReview); err == nil {
+		if !tokenResult.Status.Authenticated {
+			glog.Warningf("Token for %s/%s did not authenticate correctly", sa.Name, sa.Namespace)
+			return nil, false, nil
+		}
+		if tokenResult.Status.User.Username != username {
+			glog.Warningf("Token for %s/%s authenticated as unexpected username: %s", sa.Name, sa.Namespace, tokenResult.Status.User.Username)
+			return nil, false, nil
+		}
+		glog.V(4).Infof("Verified credential for %s/%s", sa.Name, sa.Namespace)
+		return clientConfig, true, nil
+	}
+
+	// If we couldn't run the token review, the API might be disabled or we might not have permission.
+	// Try to make a request to /apis with the token. If we get a 401 we should consider the token invalid.
+	clientConfigCopy := *clientConfig
+	clientConfigCopy.NegotiatedSerializer = api.Codecs
+	client, err := restclient.UnversionedRESTClientFor(&clientConfigCopy)
+	if err != nil {
+		return nil, false, err
+	}
+	err = client.Get().AbsPath("/apis").Do().Error()
+	if apierrors.IsUnauthorized(err) {
+		glog.Warningf("Token for %s/%s did not authenticate correctly: %v", sa.Name, sa.Namespace, err)
+		return nil, false, nil
+	}
+
+	return clientConfig, true, nil
+}
+
 func (b SAControllerClientBuilder) ConfigOrDie(name string) *restclient.Config {
 	clientConfig, err := b.Config(name)
 	if err != nil {

plugin/pkg/auth/authorizer/rbac/bootstrappolicy/policy.go

@@ -294,6 +294,8 @@ func ClusterRoles() []rbac.ClusterRole {
 				rbac.NewRule("delete").Groups(legacyGroup).Resources("secrets").RuleOrDie(),
 				rbac.NewRule("get").Groups(legacyGroup).Resources("endpoints", "namespaces", "serviceaccounts").RuleOrDie(),
 				rbac.NewRule("update").Groups(legacyGroup).Resources("endpoints", "serviceaccounts").RuleOrDie(),
+				// Needed to check API access.  These creates are non-mutating
+				rbac.NewRule("create").Groups(authenticationGroup).Resources("tokenreviews").RuleOrDie(),
 
 				rbac.NewRule("list", "watch").Groups(legacyGroup).Resources(
 					"configmaps",

plugin/pkg/auth/authorizer/rbac/bootstrappolicy/testdata/cluster-roles.yaml

@@ -462,6 +462,12 @@ items:
     verbs:
     - update
   - apiGroups:
+    - authentication.k8s.io
+    resources:
+    - tokenreviews
+    verbs:
+    - create
+  - apiGroups:
     - ""
     resources:
     - configmaps