diff --git a/pkg/serviceaccount/tokens_controller.go b/pkg/serviceaccount/tokens_controller.go index cb034d967f0..a800ec9840a 100644 --- a/pkg/serviceaccount/tokens_controller.go +++ b/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 { diff --git a/pkg/serviceaccount/tokens_controller_test.go b/pkg/serviceaccount/tokens_controller_test.go index e34cab5809d..ebd96845aba 100644 --- a/pkg/serviceaccount/tokens_controller_test.go +++ b/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)