From 2bdaac5594a96a6e6250e33cc26088642fb156dd Mon Sep 17 00:00:00 2001
From: Eric Chiang <eric.chiang.m@gmail.com>
Date: Wed, 25 Jan 2017 10:11:47 -0800
Subject: [PATCH] plugin/pkg/admission/serviceaccount: prefer first referenced
 secret

When a pod uses a service account that references multiple secrets,
prefer the secrets in the order they're listed.

Without this change, the added test fails:

    --- FAIL: TestMultipleReferencedSecrets (0.00s)
            admission_test.go:832: expected first referenced secret to be mounted, got "token2"
---
 .../pkg/admission/serviceaccount/admission.go | 13 +--
 .../serviceaccount/admission_test.go          | 79 +++++++++++++++++++
 2 files changed, 86 insertions(+), 6 deletions(-)

diff --git a/plugin/pkg/admission/serviceaccount/admission.go b/plugin/pkg/admission/serviceaccount/admission.go
index b589ded9ccc..679036f8f1e 100644
--- a/plugin/pkg/admission/serviceaccount/admission.go
+++ b/plugin/pkg/admission/serviceaccount/admission.go
@@ -303,13 +303,14 @@ func (s *serviceAccount) getReferencedServiceAccountToken(serviceAccount *api.Se
 		return "", err
 	}
 
-	references := sets.NewString()
-	for _, secret := range serviceAccount.Secrets {
-		references.Insert(secret.Name)
-	}
+	accountTokens := sets.NewString()
 	for _, token := range tokens {
-		if references.Has(token.Name) {
-			return token.Name, nil
+		accountTokens.Insert(token.Name)
+	}
+	// Prefer secrets in the order they're referenced.
+	for _, secret := range serviceAccount.Secrets {
+		if accountTokens.Has(secret.Name) {
+			return secret.Name, nil
 		}
 	}
 
diff --git a/plugin/pkg/admission/serviceaccount/admission_test.go b/plugin/pkg/admission/serviceaccount/admission_test.go
index 1447054c90d..dcbc5b59b23 100644
--- a/plugin/pkg/admission/serviceaccount/admission_test.go
+++ b/plugin/pkg/admission/serviceaccount/admission_test.go
@@ -753,3 +753,82 @@ func TestAddImagePullSecrets(t *testing.T) {
 		t.Errorf("accidentally mutated the ServiceAccount.ImagePullSecrets: %v", sa.ImagePullSecrets)
 	}
 }
+
+func TestMultipleReferencedSecrets(t *testing.T) {
+	var (
+		ns                 = "myns"
+		serviceAccountName = "mysa"
+		serviceAccountUID  = "mysauid"
+		token1             = "token1"
+		token2             = "token2"
+	)
+
+	admit := NewServiceAccount()
+	admit.SetInternalClientSet(nil)
+	admit.MountServiceAccountToken = true
+	admit.RequireAPIToken = true
+
+	sa := &api.ServiceAccount{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      serviceAccountName,
+			UID:       types.UID(serviceAccountUID),
+			Namespace: ns,
+		},
+		Secrets: []api.ObjectReference{
+			{Name: token1},
+			{Name: token2},
+		},
+	}
+	admit.serviceAccounts.Add(sa)
+
+	// Add two tokens for the service account into the cache.
+	admit.secrets.Add(&api.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      token2,
+			Namespace: ns,
+			Annotations: map[string]string{
+				api.ServiceAccountNameKey: serviceAccountName,
+				api.ServiceAccountUIDKey:  serviceAccountUID,
+			},
+		},
+		Type: api.SecretTypeServiceAccountToken,
+		Data: map[string][]byte{
+			api.ServiceAccountTokenKey: []byte("token-data"),
+		},
+	})
+	admit.secrets.Add(&api.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      token1,
+			Namespace: ns,
+			Annotations: map[string]string{
+				api.ServiceAccountNameKey: serviceAccountName,
+				api.ServiceAccountUIDKey:  serviceAccountUID,
+			},
+		},
+		Type: api.SecretTypeServiceAccountToken,
+		Data: map[string][]byte{
+			api.ServiceAccountTokenKey: []byte("token-data"),
+		},
+	})
+
+	pod := &api.Pod{
+		Spec: api.PodSpec{
+			ServiceAccountName: serviceAccountName,
+			Containers: []api.Container{
+				{Name: "container-1"},
+			},
+		},
+	}
+
+	attrs := admission.NewAttributesRecord(pod, nil, api.Kind("Pod").WithVersion("version"), ns, "myname", api.Resource("pods").WithVersion("version"), "", admission.Create, nil)
+	if err := admit.Admit(attrs); err != nil {
+		t.Fatal(err)
+	}
+
+	if n := len(pod.Spec.Volumes); n != 1 {
+		t.Fatalf("expected 1 volume mount, got %d", n)
+	}
+	if name := pod.Spec.Volumes[0].Name; name != token1 {
+		t.Errorf("expected first referenced secret to be mounted, got %q", name)
+	}
+}