From 233ad2071ca1b0a8d86b18f1b6871eb390346e8f Mon Sep 17 00:00:00 2001 From: zhengkai Date: Thu, 25 Sep 2025 20:37:59 +0800 Subject: [PATCH] refactor(serviceaccount-tokens-controller): Change the secret's MutationCache to SecretLister --- .../serviceaccount/tokens_controller.go | 54 +++++-------------- .../serviceaccount/tokens_controller_test.go | 11 ---- 2 files changed, 13 insertions(+), 52 deletions(-) diff --git a/pkg/controller/serviceaccount/tokens_controller.go b/pkg/controller/serviceaccount/tokens_controller.go index d106573a1d8..3b7594ea3a1 100644 --- a/pkg/controller/serviceaccount/tokens_controller.go +++ b/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 -} diff --git a/pkg/controller/serviceaccount/tokens_controller_test.go b/pkg/controller/serviceaccount/tokens_controller_test.go index 5b7b14901c3..e1fef112861 100644 --- a/pkg/controller/serviceaccount/tokens_controller_test.go +++ b/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)