refactor(serviceaccount-tokens-controller): Change the secret's MutationCache to SecretLister

pkg/controller/serviceaccount/tokens_controller.go

@@ -25,6 +25,7 @@ import (
 	v1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/types"
 	utilerrors "k8s.io/apimachinery/pkg/util/errors"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
@@ -103,8 +104,7 @@ func NewTokensController(logger klog.Logger, serviceAccounts informers.ServiceAc
 		options.ServiceAccountResync,
 	)
 
-	secretCache := secrets.Informer().GetIndexer()
-	e.updatedSecrets = cache.NewIntegerResourceVersionMutationCache(logger, secretCache, secretCache, 60*time.Second, true)
+	e.secrets = secrets.Lister()
 	e.secretSynced = secrets.Informer().HasSynced
 	secrets.Informer().AddEventHandlerWithOptions(
 		cache.FilteringResourceEventHandler{
@@ -140,10 +140,7 @@ type TokensController struct {
 	rootCA []byte
 
 	serviceAccounts listersv1.ServiceAccountLister
-	// updatedSecrets is a wrapper around the shared cache which allows us to record
-	// and return our local mutations (since we're very likely to act on an updated
-	// secret before the watch reports it).
-	updatedSecrets cache.MutationCache
+	secrets         listersv1.SecretLister
 
 	// Since we join two objects, we'll watch both of them with controllers.
 	serviceAccountSynced cache.InformerSynced
@@ -281,7 +278,7 @@ func (e *TokensController) syncSecret(ctx context.Context) {
 		return
 	}
 
-	secret, err := e.getSecret(secretInfo.namespace, secretInfo.name, secretInfo.uid, false)
+	secret, err := e.getSecret(secretInfo.namespace, secretInfo.name, secretInfo.uid)
 	switch {
 	case err != nil:
 		logger.Error(err, "Getting secret")
@@ -510,54 +507,34 @@ func (e *TokensController) getServiceAccount(ns string, name string, uid types.U
 	return nil, nil
 }
 
-func (e *TokensController) getSecret(ns string, name string, uid types.UID, fetchOnCacheMiss bool) (*v1.Secret, error) {
+func (e *TokensController) getSecret(ns string, name string, uid types.UID) (*v1.Secret, error) {
 	// Look up in cache
-	obj, exists, err := e.updatedSecrets.GetByKey(makeCacheKey(ns, name))
+	secret, err := e.secrets.Secrets(ns).Get(name)
 	if err != nil {
+		if apierrors.IsNotFound(err) {
+			return nil, nil
+		}
 		return nil, err
 	}
-	if exists {
-		secret, ok := obj.(*v1.Secret)
-		if !ok {
-			return nil, fmt.Errorf("expected *v1.Secret, got %#v", secret)
-		}
-		// Ensure UID matches if given
-		if len(uid) == 0 || uid == secret.UID {
-			return secret, nil
-		}
-	}
 
-	if !fetchOnCacheMiss {
-		return nil, nil
-	}
-
-	// Live lookup
-	secret, err := e.client.CoreV1().Secrets(ns).Get(context.TODO(), name, metav1.GetOptions{})
-	if apierrors.IsNotFound(err) {
-		return nil, nil
-	}
-	if err != nil {
-		return nil, err
-	}
 	// Ensure UID matches if given
 	if len(uid) == 0 || uid == secret.UID {
 		return secret, nil
 	}
+
 	return nil, nil
 }
 
-// listTokenSecrets returns a list of all of the ServiceAccountToken secrets that
+// listTokenSecrets returns a list of all the ServiceAccountToken secrets that
 // reference the given service account's name and uid
 func (e *TokensController) listTokenSecrets(serviceAccount *v1.ServiceAccount) ([]*v1.Secret, error) {
-	namespaceSecrets, err := e.updatedSecrets.ByIndex("namespace", serviceAccount.Namespace)
+	namespaceSecrets, err := e.secrets.Secrets(serviceAccount.Namespace).List(labels.Everything())
 	if err != nil {
 		return nil, err
 	}
 
 	items := []*v1.Secret{}
-	for _, obj := range namespaceSecrets {
-		secret := obj.(*v1.Secret)
-
+	for _, secret := range namespaceSecrets {
 		if apiserverserviceaccount.IsServiceAccountToken(secret, serviceAccount) {
 			items = append(items, secret)
 		}
@@ -627,8 +604,3 @@ func parseSecretQueueKey(key interface{}) (secretQueueKey, error) {
 	}
 	return queueKey, nil
 }
-
-// produce the same key format as cache.MetaNamespaceKeyFunc
-func makeCacheKey(namespace, name string) string {
-	return namespace + "/" + name
-}

pkg/controller/serviceaccount/tokens_controller_test.go

@@ -180,7 +180,6 @@ func TestTokenCreation(t *testing.T) {
 		UpdatedServiceAccount *v1.ServiceAccount
 		DeletedServiceAccount *v1.ServiceAccount
 		AddedSecret           *v1.Secret
-		AddedSecretLocal      *v1.Secret
 		UpdatedSecret         *v1.Secret
 		DeletedSecret         *v1.Secret
 
@@ -198,13 +197,6 @@ func TestTokenCreation(t *testing.T) {
 			AddedServiceAccount: serviceAccount(missingSecretReferences()),
 			ExpectedActions:     []core.Action{},
 		},
-		"new serviceaccount with missing secrets and a local secret in the cache": {
-			ClientObjects: []runtime.Object{serviceAccount(missingSecretReferences())},
-
-			AddedServiceAccount: serviceAccount(tokenSecretReferences()),
-			AddedSecretLocal:    serviceAccountTokenSecret(),
-			ExpectedActions:     []core.Action{},
-		},
 		"new serviceaccount with non-token secrets": {
 			ClientObjects: []runtime.Object{serviceAccount(regularSecretReferences()), opaqueSecret()},
 
@@ -483,9 +475,6 @@ func TestTokenCreation(t *testing.T) {
 				secrets.Add(tc.AddedSecret)
 				controller.queueSecretSync(tc.AddedSecret)
 			}
-			if tc.AddedSecretLocal != nil {
-				controller.updatedSecrets.Mutation(tc.AddedSecretLocal)
-			}
 			if tc.UpdatedSecret != nil {
 				secrets.Add(tc.UpdatedSecret)
 				controller.queueSecretUpdateSync(nil, tc.UpdatedSecret)