Add option to require API tokens to exist in admission

nikhiljindal · Jun 30, 2015 · ecebac9 · ecebac9
1 parent 96828f2
commit ecebac9
Show file tree

Hide file tree

Showing 4 changed files with 63 additions and 20 deletions.
diff --git a/plugin/pkg/admission/serviceaccount/admission.go b/plugin/pkg/admission/serviceaccount/admission.go
@@ -57,6 +57,8 @@ type serviceAccount struct {
 
 	// LimitSecretReferences rejects pods that reference secrets their service accounts do not reference
 	LimitSecretReferences bool
+	// RequireAPIToken determines whether pod creation attempts are rejected if no API token exists for the pod's service account
+	RequireAPIToken bool
 	// MountServiceAccountToken creates Volume and VolumeMounts for the first referenced ServiceAccountToken for the pod's service account
 	MountServiceAccountToken bool
 
@@ -110,6 +112,8 @@ func NewServiceAccount(cl client.Interface) *serviceAccount {
 		LimitSecretReferences: false,
 		// Auto mount service account API token secrets
 		MountServiceAccountToken: true,
+		// Reject pod creation until a service account token is available
+		RequireAPIToken: true,
 
 		client:                   cl,
 		serviceAccounts:          serviceAccountsIndexer,
@@ -172,7 +176,8 @@ func (s *serviceAccount) Admit(a admission.Attributes) (err error) {
 		return admission.NewForbidden(a, fmt.Errorf("Error looking up service account %s/%s: %v", a.GetNamespace(), pod.Spec.ServiceAccountName, err))
 	}
 	if serviceAccount == nil {
-		return admission.NewForbidden(a, fmt.Errorf("Missing service account %s/%s: %v", a.GetNamespace(), pod.Spec.ServiceAccountName, err))
+		// TODO: convert to a ServerTimeout error (or other error that sends a Retry-After header)
+		return admission.NewForbidden(a, fmt.Errorf("service account %s/%s was not found, retry after the service account is created", a.GetNamespace(), pod.Spec.ServiceAccountName))
 	}
 
 	if s.LimitSecretReferences {
@@ -320,10 +325,15 @@ func (s *serviceAccount) mountServiceAccountToken(serviceAccount *api.ServiceAcc
 	// Find the name of a referenced ServiceAccountToken secret we can mount
 	serviceAccountToken, err := s.getReferencedServiceAccountToken(serviceAccount)
 	if err != nil {
-		fmt.Errorf("Error looking up service account token for %s/%s: %v", serviceAccount.Namespace, serviceAccount.Name, err)
+		return fmt.Errorf("Error looking up service account token for %s/%s: %v", serviceAccount.Namespace, serviceAccount.Name, err)
 	}
 	if len(serviceAccountToken) == 0 {
 		// We don't have an API token to mount, so return
+		if s.RequireAPIToken {
+			// If a token is required, this is considered an error
+			// TODO: convert to a ServerTimeout error (or other error that sends a Retry-After header)
+			return fmt.Errorf("no API token found for service account %s/%s, retry after the token is automatically created and added to the service account", serviceAccount.Namespace, serviceAccount.Name)
+		}
 		return nil
 	}
 

diff --git a/plugin/pkg/admission/serviceaccount/admission_test.go b/plugin/pkg/admission/serviceaccount/admission_test.go
@@ -128,6 +128,7 @@ func TestAssignsDefaultServiceAccountAndToleratesMissingAPIToken(t *testing.T) {
 
 	admit := NewServiceAccount(nil)
 	admit.MountServiceAccountToken = true
+	admit.RequireAPIToken = false
 
 	// Add the default service account for the ns into the cache
 	admit.serviceAccounts.Add(&api.ServiceAccount{
@@ -148,6 +149,29 @@ func TestAssignsDefaultServiceAccountAndToleratesMissingAPIToken(t *testing.T) {
 	}
 }
 
+func TestAssignsDefaultServiceAccountAndRejectsMissingAPIToken(t *testing.T) {
+	ns := "myns"
+
+	admit := NewServiceAccount(nil)
+	admit.MountServiceAccountToken = true
+	admit.RequireAPIToken = true
+
+	// Add the default service account for the ns into the cache
+	admit.serviceAccounts.Add(&api.ServiceAccount{
+		ObjectMeta: api.ObjectMeta{
+			Name:      DefaultServiceAccountName,
+			Namespace: ns,
+		},
+	})
+
+	pod := &api.Pod{}
+	attrs := admission.NewAttributesRecord(pod, "Pod", ns, "myname", string(api.ResourcePods), "", admission.Create, nil)
+	err := admit.Admit(attrs)
+	if err == nil {
+		t.Errorf("Expected admission error for missing API token")
+	}
+}
+
 func TestFetchesUncachedServiceAccount(t *testing.T) {
 	ns := "myns"
 
@@ -160,6 +184,7 @@ func TestFetchesUncachedServiceAccount(t *testing.T) {
 	})
 
 	admit := NewServiceAccount(client)
+	admit.RequireAPIToken = false
 
 	pod := &api.Pod{}
 	attrs := admission.NewAttributesRecord(pod, "Pod", ns, "myname", string(api.ResourcePods), "", admission.Create, nil)
@@ -208,6 +233,7 @@ func TestAutomountsAPIToken(t *testing.T) {
 
 	admit := NewServiceAccount(nil)
 	admit.MountServiceAccountToken = true
+	admit.RequireAPIToken = true
 
 	// Add the default service account for the ns with a token into the cache
 	admit.serviceAccounts.Add(&api.ServiceAccount{
@@ -279,6 +305,7 @@ func TestRespectsExistingMount(t *testing.T) {
 
 	admit := NewServiceAccount(nil)
 	admit.MountServiceAccountToken = true
+	admit.RequireAPIToken = true
 
 	// Add the default service account for the ns with a token into the cache
 	admit.serviceAccounts.Add(&api.ServiceAccount{
@@ -345,6 +372,7 @@ func TestAllowsReferencedSecretVolumes(t *testing.T) {
 
 	admit := NewServiceAccount(nil)
 	admit.LimitSecretReferences = true
+	admit.RequireAPIToken = false
 
 	// Add the default service account for the ns with a secret reference into the cache
 	admit.serviceAccounts.Add(&api.ServiceAccount{
@@ -376,6 +404,7 @@ func TestRejectsUnreferencedSecretVolumes(t *testing.T) {
 
 	admit := NewServiceAccount(nil)
 	admit.LimitSecretReferences = true
+	admit.RequireAPIToken = false
 
 	// Add the default service account for the ns into the cache
 	admit.serviceAccounts.Add(&api.ServiceAccount{
@@ -404,6 +433,7 @@ func TestAllowsReferencedImagePullSecrets(t *testing.T) {
 
 	admit := NewServiceAccount(nil)
 	admit.LimitSecretReferences = true
+	admit.RequireAPIToken = false
 
 	// Add the default service account for the ns with a secret reference into the cache
 	admit.serviceAccounts.Add(&api.ServiceAccount{
@@ -433,6 +463,7 @@ func TestRejectsUnreferencedImagePullSecrets(t *testing.T) {
 
 	admit := NewServiceAccount(nil)
 	admit.LimitSecretReferences = true
+	admit.RequireAPIToken = false
 
 	// Add the default service account for the ns into the cache
 	admit.serviceAccounts.Add(&api.ServiceAccount{
@@ -459,6 +490,7 @@ func TestDoNotAddImagePullSecrets(t *testing.T) {
 
 	admit := NewServiceAccount(nil)
 	admit.LimitSecretReferences = true
+	admit.RequireAPIToken = false
 
 	// Add the default service account for the ns with a secret reference into the cache
 	admit.serviceAccounts.Add(&api.ServiceAccount{
@@ -493,6 +525,7 @@ func TestAddImagePullSecrets(t *testing.T) {
 
 	admit := NewServiceAccount(nil)
 	admit.LimitSecretReferences = true
+	admit.RequireAPIToken = false
 
 	sa := &api.ServiceAccount{
 		ObjectMeta: api.ObjectMeta{

diff --git a/test/e2e/pods.go b/test/e2e/pods.go
@@ -35,19 +35,10 @@ import (
 	. "github.com/onsi/gomega"
 )
 
-// createNamespaceIfDoesNotExist ensures that the namespace with specified name exists, or returns an error
-func createNamespaceIfDoesNotExist(c *client.Client, name string) (*api.Namespace, error) {
-	namespace, err := c.Namespaces().Get(name)
-	if err != nil {
-		namespace, err = c.Namespaces().Create(&api.Namespace{ObjectMeta: api.ObjectMeta{Name: name}})
-	}
-	return namespace, err
-}
-
 func runLivenessTest(c *client.Client, podDescr *api.Pod, expectRestart bool) {
-	ns := "e2e-test-" + string(util.NewUUID())
-	_, err := createNamespaceIfDoesNotExist(c, ns)
-	expectNoError(err, fmt.Sprintf("creating namespace %s", ns))
+	namespace, err := createTestingNS("pods-liveness", c)
+	Expect(err).NotTo(HaveOccurred())
+	ns := namespace.Name
 
 	By(fmt.Sprintf("Creating pod %s in namespace %s", podDescr.Name, ns))
 	_, err = c.Pods(ns).Create(podDescr)
@@ -96,9 +87,9 @@ func runLivenessTest(c *client.Client, podDescr *api.Pod, expectRestart bool) {
 
 // testHostIP tests that a pod gets a host IP
 func testHostIP(c *client.Client, pod *api.Pod) {
-	ns := "e2e-test-" + string(util.NewUUID())
-	_, err := createNamespaceIfDoesNotExist(c, ns)
-	expectNoError(err, fmt.Sprintf("creating namespace %s", ns))
+	namespace, err := createTestingNS("pods-host-ip", c)
+	Expect(err).NotTo(HaveOccurred())
+	ns := namespace.Name
 
 	podClient := c.Pods(ns)
 	By("creating pod")

diff --git a/test/e2e/util.go b/test/e2e/util.go
@@ -32,6 +32,7 @@ import (
 	"time"
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
+	apierrs "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/client"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/client/cache"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/client/clientcmd"
@@ -317,12 +318,20 @@ func waitForPodsRunningReady(ns string, minPods int, timeout time.Duration) erro
 func waitForServiceAccountInNamespace(c *client.Client, ns, serviceAccountName string, timeout time.Duration) error {
 	Logf("Waiting up to %v for service account %s to be provisioned in ns %s", timeout, serviceAccountName, ns)
 	for start := time.Now(); time.Since(start) < timeout; time.Sleep(poll) {
-		_, err := c.ServiceAccounts(ns).Get(serviceAccountName)
-		if err != nil {
+		sa, err := c.ServiceAccounts(ns).Get(serviceAccountName)
+		if apierrs.IsNotFound(err) {
 			Logf("Get service account %s in ns %s failed, ignoring for %v: %v", serviceAccountName, ns, poll, err)
 			continue
 		}
-		Logf("Service account %s in ns %s found. (%v)", serviceAccountName, ns, time.Since(start))
+		if err != nil {
+			Logf("Get service account %s in ns %s failed: %v", serviceAccountName, ns, err)
+			return err
+		}
+		if len(sa.Secrets) == 0 {
+			Logf("Service account %s in ns %s had 0 secrets, ignoring for %v: %v", serviceAccountName, ns, poll, err)
+			continue
+		}
+		Logf("Service account %s in ns %s with secrets found. (%v)", serviceAccountName, ns, time.Since(start))
 		return nil
 	}
 	return fmt.Errorf("Service account %s in namespace %s not ready within %v", serviceAccountName, ns, timeout)