Merge pull request #9328 from liggitt/token_controller_process_new

pkg/serviceaccount/tokens_controller.go

@@ -141,9 +141,6 @@ func (e *TokensController) Stop() {
 
 // serviceAccountAdded reacts to a ServiceAccount creation by creating a corresponding ServiceAccountToken Secret
 func (e *TokensController) serviceAccountAdded(obj interface{}) {
-	if !e.secretsSynced() {
-		return
-	}
 	serviceAccount := obj.(*api.ServiceAccount)
 	err := e.createSecretIfNeeded(serviceAccount)
 	if err != nil {
@@ -153,9 +150,6 @@ func (e *TokensController) serviceAccountAdded(obj interface{}) {
 
 // serviceAccountUpdated reacts to a ServiceAccount update (or re-list) by ensuring a corresponding ServiceAccountToken Secret exists
 func (e *TokensController) serviceAccountUpdated(oldObj interface{}, newObj interface{}) {
-	if !e.secretsSynced() {
-		return
-	}
 	newServiceAccount := newObj.(*api.ServiceAccount)
 	err := e.createSecretIfNeeded(newServiceAccount)
 	if err != nil {
@@ -185,9 +179,6 @@ 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{}) {
-	if !e.serviceAccountsSynced() {
-		return
-	}
 	secret := obj.(*api.Secret)
 	serviceAccount, err := e.getServiceAccount(secret)
 	if err != nil {
@@ -195,6 +186,10 @@ func (e *TokensController) secretAdded(obj interface{}) {
 		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
+		}
 		if err := e.deleteSecret(secret); err != nil {
 			glog.Errorf("Error deleting secret %s/%s: %v", secret.Namespace, secret.Name, err)
 		}
@@ -205,9 +200,6 @@ 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{}) {
-	if !e.serviceAccountsSynced() {
-		return
-	}
 	newSecret := newObj.(*api.Secret)
 	newServiceAccount, err := e.getServiceAccount(newSecret)
 	if err != nil {
@@ -215,6 +207,10 @@ func (e *TokensController) secretUpdated(oldObj interface{}, newObj interface{})
 		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
+		}
 		if err := e.deleteSecret(newSecret); err != nil {
 			glog.Errorf("Error deleting secret %s/%s: %v", newSecret.Namespace, newSecret.Name, err)
 		}
@@ -253,6 +249,11 @@ func (e *TokensController) createSecretIfNeeded(serviceAccount *api.ServiceAccou
 		return e.createSecret(serviceAccount)
 	}
 
+	// We shouldn't try to validate secret references until the secrets store is synced
+	if !e.secretsSynced() {
+		return nil
+	}
+
 	// If any existing token secrets are referenced by the service account, return
 	allSecrets, err := e.listTokenSecrets(serviceAccount)
 	if err != nil {

pkg/serviceaccount/tokens_controller_test.go

@@ -143,6 +143,9 @@ func TestTokenCreation(t *testing.T) {
 	testcases := map[string]struct {
 		ClientObjects []runtime.Object
 
+		SecretsSyncPending         bool
+		ServiceAccountsSyncPending bool
+
 		ExistingServiceAccount *api.ServiceAccount
 		ExistingSecrets        []*api.Secret
 
@@ -165,6 +168,18 @@ func TestTokenCreation(t *testing.T) {
 				{Action: "update-serviceaccount", Value: serviceAccount(addTokenSecretReference(emptySecretReferences()))},
 			},
 		},
+		"new serviceaccount with no secrets with unsynced secret store": {
+			ClientObjects: []runtime.Object{serviceAccount(emptySecretReferences()), createdTokenSecret()},
+
+			SecretsSyncPending: true,
+
+			AddedServiceAccount: serviceAccount(emptySecretReferences()),
+			ExpectedActions: []testclient.FakeAction{
+				{Action: "create-secret", Value: createdTokenSecret()},
+				{Action: "get-serviceaccount", Value: "default"},
+				{Action: "update-serviceaccount", Value: serviceAccount(addTokenSecretReference(emptySecretReferences()))},
+			},
+		},
 		"new serviceaccount with missing secrets": {
 			ClientObjects: []runtime.Object{serviceAccount(missingSecretReferences()), createdTokenSecret()},
 
@@ -175,6 +190,14 @@ func TestTokenCreation(t *testing.T) {
 				{Action: "update-serviceaccount", Value: serviceAccount(addTokenSecretReference(missingSecretReferences()))},
 			},
 		},
+		"new serviceaccount with missing secrets with unsynced secret store": {
+			ClientObjects: []runtime.Object{serviceAccount(missingSecretReferences()), createdTokenSecret()},
+
+			SecretsSyncPending: true,
+
+			AddedServiceAccount: serviceAccount(missingSecretReferences()),
+			ExpectedActions:     []testclient.FakeAction{},
+		},
 		"new serviceaccount with non-token secrets": {
 			ClientObjects: []runtime.Object{serviceAccount(regularSecretReferences()), createdTokenSecret(), opaqueSecret()},
 
@@ -203,6 +226,18 @@ func TestTokenCreation(t *testing.T) {
 				{Action: "update-serviceaccount", Value: serviceAccount(addTokenSecretReference(emptySecretReferences()))},
 			},
 		},
+		"updated serviceaccount with no secrets with unsynced secret store": {
+			ClientObjects: []runtime.Object{serviceAccount(emptySecretReferences()), createdTokenSecret()},
+
+			SecretsSyncPending: true,
+
+			UpdatedServiceAccount: serviceAccount(emptySecretReferences()),
+			ExpectedActions: []testclient.FakeAction{
+				{Action: "create-secret", Value: createdTokenSecret()},
+				{Action: "get-serviceaccount", Value: "default"},
+				{Action: "update-serviceaccount", Value: serviceAccount(addTokenSecretReference(emptySecretReferences()))},
+			},
+		},
 		"updated serviceaccount with missing secrets": {
 			ClientObjects: []runtime.Object{serviceAccount(missingSecretReferences()), createdTokenSecret()},
 
@@ -213,6 +248,14 @@ func TestTokenCreation(t *testing.T) {
 				{Action: "update-serviceaccount", Value: serviceAccount(addTokenSecretReference(missingSecretReferences()))},
 			},
 		},
+		"updated serviceaccount with missing secrets with unsynced secret store": {
+			ClientObjects: []runtime.Object{serviceAccount(missingSecretReferences()), createdTokenSecret()},
+
+			SecretsSyncPending: true,
+
+			UpdatedServiceAccount: serviceAccount(missingSecretReferences()),
+			ExpectedActions:       []testclient.FakeAction{},
+		},
 		"updated serviceaccount with non-token secrets": {
 			ClientObjects: []runtime.Object{serviceAccount(regularSecretReferences()), createdTokenSecret(), opaqueSecret()},
 
@@ -262,6 +305,14 @@ func TestTokenCreation(t *testing.T) {
 				{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()),
 
@@ -286,6 +337,14 @@ func TestTokenCreation(t *testing.T) {
 				{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()),
 
@@ -335,9 +394,9 @@ func TestTokenCreation(t *testing.T) {
 
 		controller := NewTokensController(client, DefaultTokenControllerOptions(generator))
 
-		// Tell the token controller its stores have been synced
-		controller.serviceAccountsSynced = func() bool { return true }
-		controller.secretsSynced = func() bool { return true }
+		// Tell the token controller whether its stores have been synced
+		controller.serviceAccountsSynced = func() bool { return !tc.ServiceAccountsSyncPending }
+		controller.secretsSynced = func() bool { return !tc.SecretsSyncPending }
 
 		if tc.ExistingServiceAccount != nil {
 			controller.serviceAccounts.Add(tc.ExistingServiceAccount)