From 1ee557814676fcf69c5c7c9e8d77ccb684939009 Mon Sep 17 00:00:00 2001
From: Jordan Liggitt <jliggitt@redhat.com>
Date: Thu, 18 Jun 2015 10:42:41 -0400
Subject: [PATCH] Ensure service account does not exist before deleting
 added/updated tokens

---
 pkg/serviceaccount/tokens_controller.go      | 31 ++++++++++++--------
 pkg/serviceaccount/tokens_controller_test.go | 18 ++----------
 2 files changed, 21 insertions(+), 28 deletions(-)

diff --git a/pkg/serviceaccount/tokens_controller.go b/pkg/serviceaccount/tokens_controller.go
index ef3cca65e33..562fdccee8e 100644
--- a/pkg/serviceaccount/tokens_controller.go
+++ b/pkg/serviceaccount/tokens_controller.go
@@ -181,16 +181,12 @@ func (e *TokensController) serviceAccountDeleted(obj interface{}) {
 // secretAdded reacts to a Secret create by ensuring the referenced ServiceAccount exists, and by adding a token to the secret if needed
 func (e *TokensController) secretAdded(obj interface{}) {
 	secret := obj.(*api.Secret)
-	serviceAccount, err := e.getServiceAccount(secret)
+	serviceAccount, err := e.getServiceAccount(secret, true)
 	if err != nil {
 		glog.Error(err)
 		return
 	}
 	if serviceAccount == nil {
-		// We shouldn't delete a secret based on an invalid serviceAccount reference until the serviceAccount store is synced
-		if !e.serviceAccountsSynced() {
-			return
-		}
 		glog.V(2).Infof(
 			"Deleting new secret %s/%s because service account %s (uid=%s) was not found",
 			secret.Namespace, secret.Name,
@@ -206,16 +202,12 @@ func (e *TokensController) secretAdded(obj interface{}) {
 // secretUpdated reacts to a Secret update (or re-list) by deleting the secret (if the referenced ServiceAccount does not exist)
 func (e *TokensController) secretUpdated(oldObj interface{}, newObj interface{}) {
 	newSecret := newObj.(*api.Secret)
-	newServiceAccount, err := e.getServiceAccount(newSecret)
+	newServiceAccount, err := e.getServiceAccount(newSecret, true)
 	if err != nil {
 		glog.Error(err)
 		return
 	}
 	if newServiceAccount == nil {
-		// We shouldn't delete a secret based on an invalid serviceAccount reference until the serviceAccount store is synced
-		if !e.serviceAccountsSynced() {
-			return
-		}
 		glog.V(2).Infof(
 			"Deleting updated secret %s/%s because service account %s (uid=%s) was not found",
 			newSecret.Namespace, newSecret.Name,
@@ -237,7 +229,7 @@ func (e *TokensController) secretDeleted(obj interface{}) {
 		return
 	}
 
-	serviceAccount, err := e.getServiceAccount(secret)
+	serviceAccount, err := e.getServiceAccount(secret, false)
 	if err != nil {
 		glog.Error(err)
 		return
@@ -408,7 +400,7 @@ func (e *TokensController) removeSecretReferenceIfNeeded(serviceAccount *api.Ser
 
 // getServiceAccount returns the ServiceAccount referenced by the given secret. If the secret is not
 // of type ServiceAccountToken, or if the referenced ServiceAccount does not exist, nil is returned
-func (e *TokensController) getServiceAccount(secret *api.Secret) (*api.ServiceAccount, error) {
+func (e *TokensController) getServiceAccount(secret *api.Secret, fetchOnCacheMiss bool) (*api.ServiceAccount, error) {
 	name, uid := serviceAccountNameAndUID(secret)
 	if len(name) == 0 {
 		return nil, nil
@@ -433,6 +425,21 @@ func (e *TokensController) getServiceAccount(secret *api.Secret) (*api.ServiceAc
 		return serviceAccount, nil
 	}
 
+	if fetchOnCacheMiss {
+		serviceAccount, err := e.client.ServiceAccounts(secret.Namespace).Get(name)
+		if apierrors.IsNotFound(err) {
+			return nil, nil
+		}
+		if err != nil {
+			return nil, err
+		}
+		if len(uid) > 0 && uid != string(serviceAccount.UID) {
+			// If UID is specified, it must match
+			return nil, nil
+		}
+		return serviceAccount, nil
+	}
+
 	return nil, nil
 }
 
diff --git a/pkg/serviceaccount/tokens_controller_test.go b/pkg/serviceaccount/tokens_controller_test.go
index ebd96845aba..16ac25a4fa0 100644
--- a/pkg/serviceaccount/tokens_controller_test.go
+++ b/pkg/serviceaccount/tokens_controller_test.go
@@ -302,17 +302,10 @@ func TestTokenCreation(t *testing.T) {
 
 			AddedSecret: serviceAccountTokenSecret(),
 			ExpectedActions: []testclient.FakeAction{
+				{Action: "get-serviceaccount", Value: "default"},
 				{Action: "delete-secret", Value: "token-secret-1"},
 			},
 		},
-		"added secret without serviceaccount with unsynced service account store": {
-			ClientObjects: []runtime.Object{serviceAccountTokenSecret()},
-
-			ServiceAccountsSyncPending: true,
-
-			AddedSecret:     serviceAccountTokenSecret(),
-			ExpectedActions: []testclient.FakeAction{},
-		},
 		"added secret with serviceaccount": {
 			ExistingServiceAccount: serviceAccount(tokenSecretReferences()),
 
@@ -334,17 +327,10 @@ func TestTokenCreation(t *testing.T) {
 
 			UpdatedSecret: serviceAccountTokenSecret(),
 			ExpectedActions: []testclient.FakeAction{
+				{Action: "get-serviceaccount", Value: "default"},
 				{Action: "delete-secret", Value: "token-secret-1"},
 			},
 		},
-		"updated secret without serviceaccount with unsynced service account store": {
-			ClientObjects: []runtime.Object{serviceAccountTokenSecret()},
-
-			ServiceAccountsSyncPending: true,
-
-			UpdatedSecret:   serviceAccountTokenSecret(),
-			ExpectedActions: []testclient.FakeAction{},
-		},
 		"updated secret with serviceaccount": {
 			ExistingServiceAccount: serviceAccount(tokenSecretReferences()),