diff --git a/plugin/pkg/admission/serviceaccount/admission.go b/plugin/pkg/admission/serviceaccount/admission.go index 80322a557ea..1d87c65a4cf 100644 --- 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 index ca482b93b31..0c96dc1467d 100644 --- 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 index bb9da381957..d3a80409545 100644 --- 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 index 9746a51724e..47a8cb51552 100644 --- 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)