Service account controller: Wire through context

This change enables structured logging and cancellation in the service
account controller by replacing the usage of context.TODO with an actual
context.
This commit is contained in:
Alvaro Aleman 2025-12-04 19:12:59 -05:00
parent 9293f9326d
commit 3d6a5d471f
8 changed files with 53 additions and 52 deletions

View file

@ -17,7 +17,6 @@ limitations under the License.
package serviceaccount
import (
"context"
"encoding/json"
"reflect"
"testing"
@ -396,7 +395,7 @@ func TestLegacyServiceAccountTokenCleanUp(t *testing.T) {
}
secrets.Add(tc.ExistingSecret)
ctx := context.TODO()
ctx := t.Context()
cleaner.evaluateSATokens(ctx)
actions := client.Actions()

View file

@ -18,7 +18,8 @@ package serviceaccount
import (
"context"
"k8s.io/api/core/v1"
v1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
clientset "k8s.io/client-go/kubernetes"
@ -43,28 +44,28 @@ func NewGetterFromClient(c clientset.Interface, secretLister v1listers.SecretLis
return clientGetter{c, secretLister, serviceAccountLister, podLister, nodeLister}
}
func (c clientGetter) GetServiceAccount(namespace, name string) (*v1.ServiceAccount, error) {
func (c clientGetter) GetServiceAccount(ctx context.Context, namespace, name string) (*v1.ServiceAccount, error) {
if serviceAccount, err := c.serviceAccountLister.ServiceAccounts(namespace).Get(name); err == nil {
return serviceAccount, nil
}
return c.client.CoreV1().ServiceAccounts(namespace).Get(context.TODO(), name, metav1.GetOptions{})
return c.client.CoreV1().ServiceAccounts(namespace).Get(ctx, name, metav1.GetOptions{})
}
func (c clientGetter) GetPod(namespace, name string) (*v1.Pod, error) {
func (c clientGetter) GetPod(ctx context.Context, namespace, name string) (*v1.Pod, error) {
if pod, err := c.podLister.Pods(namespace).Get(name); err == nil {
return pod, nil
}
return c.client.CoreV1().Pods(namespace).Get(context.TODO(), name, metav1.GetOptions{})
return c.client.CoreV1().Pods(namespace).Get(ctx, name, metav1.GetOptions{})
}
func (c clientGetter) GetSecret(namespace, name string) (*v1.Secret, error) {
func (c clientGetter) GetSecret(ctx context.Context, namespace, name string) (*v1.Secret, error) {
if secret, err := c.secretLister.Secrets(namespace).Get(name); err == nil {
return secret, nil
}
return c.client.CoreV1().Secrets(namespace).Get(context.TODO(), name, metav1.GetOptions{})
return c.client.CoreV1().Secrets(namespace).Get(ctx, name, metav1.GetOptions{})
}
func (c clientGetter) GetNode(name string) (*v1.Node, error) {
func (c clientGetter) GetNode(ctx context.Context, name string) (*v1.Node, error) {
// handle the case where the node lister isn't set due to feature being disabled
if c.nodeLister == nil {
return nil, apierrors.NewNotFound(v1.Resource("nodes"), name)
@ -72,5 +73,5 @@ func (c clientGetter) GetNode(name string) (*v1.Node, error) {
if node, err := c.nodeLister.Get(name); err == nil {
return node, nil
}
return c.client.CoreV1().Nodes().Get(context.TODO(), name, metav1.GetOptions{})
return c.client.CoreV1().Nodes().Get(ctx, name, metav1.GetOptions{})
}

View file

@ -251,7 +251,7 @@ func (e *TokensController) syncServiceAccount(ctx context.Context) {
return
}
sa, err := e.getServiceAccount(saInfo.namespace, saInfo.name, saInfo.uid, false)
sa, err := e.getServiceAccount(ctx, saInfo.namespace, saInfo.name, saInfo.uid, false)
switch {
case err != nil:
logger.Error(err, "Getting service account")
@ -260,7 +260,7 @@ func (e *TokensController) syncServiceAccount(ctx context.Context) {
// service account no longer exists, so delete related tokens
logger.V(4).Info("Service account deleted, removing tokens", "namespace", saInfo.namespace, "serviceaccount", saInfo.name)
sa = &v1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Namespace: saInfo.namespace, Name: saInfo.name, UID: saInfo.uid}}
retry, err = e.deleteTokens(sa)
retry, err = e.deleteTokens(ctx, sa)
if err != nil {
logger.Error(err, "Error deleting serviceaccount tokens", "namespace", saInfo.namespace, "serviceaccount", saInfo.name)
}
@ -287,24 +287,24 @@ func (e *TokensController) syncSecret(ctx context.Context) {
return
}
secret, err := e.getSecret(secretInfo.namespace, secretInfo.name, secretInfo.uid)
secret, err := e.getSecret(ctx, secretInfo.namespace, secretInfo.name, secretInfo.uid)
switch {
case err != nil:
logger.Error(err, "Getting secret")
retry = true
case secret == nil:
// If the service account exists
if sa, saErr := e.getServiceAccount(secretInfo.namespace, secretInfo.saName, secretInfo.saUID, false); saErr == nil && sa != nil {
if sa, saErr := e.getServiceAccount(ctx, secretInfo.namespace, secretInfo.saName, secretInfo.saUID, false); saErr == nil && sa != nil {
// secret no longer exists, so delete references to this secret from the service account
if err := clientretry.RetryOnConflict(RemoveTokenBackoff, func() error {
return e.removeSecretReference(secretInfo.namespace, secretInfo.saName, secretInfo.saUID, secretInfo.name)
return e.removeSecretReference(ctx, secretInfo.namespace, secretInfo.saName, secretInfo.saUID, secretInfo.name)
}); err != nil {
logger.Error(err, "Removing secret reference")
}
}
default:
// Ensure service account exists
sa, saErr := e.getServiceAccount(secretInfo.namespace, secretInfo.saName, secretInfo.saUID, true)
sa, saErr := e.getServiceAccount(ctx, secretInfo.namespace, secretInfo.saName, secretInfo.saUID, true)
switch {
case saErr != nil:
logger.Error(saErr, "Getting service account")
@ -312,13 +312,13 @@ func (e *TokensController) syncSecret(ctx context.Context) {
case sa == nil:
// Delete token
logger.V(4).Info("Service account does not exist, deleting token", "secret", klog.KRef(secretInfo.namespace, secretInfo.name))
if retriable, err := e.deleteToken(secretInfo.namespace, secretInfo.name, secretInfo.uid); err != nil {
if retriable, err := e.deleteToken(ctx, secretInfo.namespace, secretInfo.name, secretInfo.uid); err != nil {
logger.Error(err, "Deleting serviceaccount token", "secret", klog.KRef(secretInfo.namespace, secretInfo.name), "serviceAccount", klog.KRef(secretInfo.namespace, secretInfo.saName))
retry = retriable
}
default:
// Update token if needed
if retriable, err := e.generateTokenIfNeeded(logger, sa, secret); err != nil {
if retriable, err := e.generateTokenIfNeeded(ctx, logger, sa, secret); err != nil {
logger.Error(err, "Populating serviceaccount token", "secret", klog.KRef(secretInfo.namespace, secretInfo.name), "serviceAccount", klog.KRef(secretInfo.namespace, secretInfo.saName))
retry = retriable
}
@ -326,7 +326,7 @@ func (e *TokensController) syncSecret(ctx context.Context) {
}
}
func (e *TokensController) deleteTokens(serviceAccount *v1.ServiceAccount) ( /*retry*/ bool, error) {
func (e *TokensController) deleteTokens(ctx context.Context, serviceAccount *v1.ServiceAccount) ( /*retry*/ bool, error) {
tokens, err := e.listTokenSecrets(serviceAccount)
if err != nil {
// don't retry on cache lookup errors
@ -335,7 +335,7 @@ func (e *TokensController) deleteTokens(serviceAccount *v1.ServiceAccount) ( /*r
retry := false
errs := []error{}
for _, token := range tokens {
r, err := e.deleteToken(token.Namespace, token.Name, token.UID)
r, err := e.deleteToken(ctx, token.Namespace, token.Name, token.UID)
if err != nil {
errs = append(errs, err)
}
@ -346,12 +346,12 @@ func (e *TokensController) deleteTokens(serviceAccount *v1.ServiceAccount) ( /*r
return retry, utilerrors.NewAggregate(errs)
}
func (e *TokensController) deleteToken(ns, name string, uid types.UID) ( /*retry*/ bool, error) {
func (e *TokensController) deleteToken(ctx context.Context, ns, name string, uid types.UID) ( /*retry*/ bool, error) {
var opts metav1.DeleteOptions
if len(uid) > 0 {
opts.Preconditions = &metav1.Preconditions{UID: &uid}
}
err := e.client.CoreV1().Secrets(ns).Delete(context.TODO(), name, opts)
err := e.client.CoreV1().Secrets(ns).Delete(ctx, name, opts)
// NotFound doesn't need a retry (it's already been deleted)
// Conflict doesn't need a retry (the UID precondition failed)
if err == nil || apierrors.IsNotFound(err) || apierrors.IsConflict(err) {
@ -374,7 +374,7 @@ func (e *TokensController) secretUpdateNeeded(secret *v1.Secret) (bool, bool, bo
}
// generateTokenIfNeeded populates the token data for the given Secret if not already set
func (e *TokensController) generateTokenIfNeeded(logger klog.Logger, serviceAccount *v1.ServiceAccount, cachedSecret *v1.Secret) ( /* retry */ bool, error) {
func (e *TokensController) generateTokenIfNeeded(ctx context.Context, logger klog.Logger, serviceAccount *v1.ServiceAccount, cachedSecret *v1.Secret) ( /* retry */ bool, error) {
// Check the cached secret to see if changes are needed
if needsCA, needsNamespace, needsToken := e.secretUpdateNeeded(cachedSecret); !needsCA && !needsToken && !needsNamespace {
return false, nil
@ -383,7 +383,7 @@ func (e *TokensController) generateTokenIfNeeded(logger klog.Logger, serviceAcco
// We don't want to update the cache's copy of the secret
// so add the token to a freshly retrieved copy of the secret
secrets := e.client.CoreV1().Secrets(cachedSecret.Namespace)
liveSecret, err := secrets.Get(context.TODO(), cachedSecret.Name, metav1.GetOptions{})
liveSecret, err := secrets.Get(ctx, cachedSecret.Name, metav1.GetOptions{})
if err != nil {
// Retry for any error other than a NotFound
return !apierrors.IsNotFound(err), err
@ -419,8 +419,7 @@ func (e *TokensController) generateTokenIfNeeded(logger klog.Logger, serviceAcco
// Generate the token
if needsToken {
c, pc := serviceaccount.LegacyClaims(*serviceAccount, *liveSecret)
// TODO: need to plumb context if using external signer ever becomes a posibility.
token, err := e.token.GenerateToken(context.TODO(), c, pc)
token, err := e.token.GenerateToken(ctx, c, pc)
if err != nil {
return false, err
}
@ -432,7 +431,7 @@ func (e *TokensController) generateTokenIfNeeded(logger klog.Logger, serviceAcco
liveSecret.Annotations[v1.ServiceAccountUIDKey] = string(serviceAccount.UID)
// Save the secret
_, err = secrets.Update(context.TODO(), liveSecret, metav1.UpdateOptions{})
_, err = secrets.Update(ctx, liveSecret, metav1.UpdateOptions{})
if apierrors.IsConflict(err) || apierrors.IsNotFound(err) {
// if we got a Conflict error, the secret was updated by someone else, and we'll get an update notification later
// if we got a NotFound error, the secret no longer exists, and we don't need to populate a token
@ -445,11 +444,11 @@ func (e *TokensController) generateTokenIfNeeded(logger klog.Logger, serviceAcco
}
// removeSecretReference updates the given ServiceAccount to remove a reference to the given secretName if needed.
func (e *TokensController) removeSecretReference(saNamespace string, saName string, saUID types.UID, secretName string) error {
func (e *TokensController) removeSecretReference(ctx context.Context, saNamespace string, saName string, saUID types.UID, secretName string) error {
// We don't want to update the cache's copy of the service account
// so remove the secret from a freshly retrieved copy of the service account
serviceAccounts := e.client.CoreV1().ServiceAccounts(saNamespace)
serviceAccount, err := serviceAccounts.Get(context.TODO(), saName, metav1.GetOptions{})
serviceAccount, err := serviceAccounts.Get(ctx, saName, metav1.GetOptions{})
// Ignore NotFound errors when attempting to remove a reference
if apierrors.IsNotFound(err) {
return nil
@ -476,7 +475,7 @@ func (e *TokensController) removeSecretReference(saNamespace string, saName stri
}
}
serviceAccount.Secrets = secrets
_, err = serviceAccounts.Update(context.TODO(), serviceAccount, metav1.UpdateOptions{})
_, err = serviceAccounts.Update(ctx, serviceAccount, metav1.UpdateOptions{})
// Ignore NotFound errors when attempting to remove a reference
if apierrors.IsNotFound(err) {
return nil
@ -484,7 +483,7 @@ func (e *TokensController) removeSecretReference(saNamespace string, saName stri
return err
}
func (e *TokensController) getServiceAccount(ns string, name string, uid types.UID, fetchOnCacheMiss bool) (*v1.ServiceAccount, error) {
func (e *TokensController) getServiceAccount(ctx context.Context, ns string, name string, uid types.UID, fetchOnCacheMiss bool) (*v1.ServiceAccount, error) {
// Look up in cache
sa, err := e.serviceAccounts.ServiceAccounts(ns).Get(name)
if err != nil && !apierrors.IsNotFound(err) {
@ -502,7 +501,7 @@ func (e *TokensController) getServiceAccount(ns string, name string, uid types.U
}
// Live lookup
sa, err = e.client.CoreV1().ServiceAccounts(ns).Get(context.TODO(), name, metav1.GetOptions{})
sa, err = e.client.CoreV1().ServiceAccounts(ns).Get(ctx, name, metav1.GetOptions{})
if apierrors.IsNotFound(err) {
return nil, nil
}
@ -516,7 +515,7 @@ 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) (*v1.Secret, error) {
func (e *TokensController) getSecret(ctx context.Context, ns string, name string, uid types.UID) (*v1.Secret, error) {
// Look up in cache
secret, err := e.secrets.Secrets(ns).Get(name)
if err != nil {

View file

@ -17,6 +17,8 @@ limitations under the License.
package server
import (
"context"
v1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/client-go/informers"
@ -36,18 +38,18 @@ func genericTokenGetter(factory informers.SharedInformerFactory) serviceaccount.
return clientGetter{secretLister: factory.Core().V1().Secrets().Lister(), serviceAccountLister: factory.Core().V1().ServiceAccounts().Lister()}
}
func (c clientGetter) GetServiceAccount(namespace, name string) (*v1.ServiceAccount, error) {
func (c clientGetter) GetServiceAccount(ctx context.Context, namespace, name string) (*v1.ServiceAccount, error) {
return c.serviceAccountLister.ServiceAccounts(namespace).Get(name)
}
func (c clientGetter) GetPod(namespace, name string) (*v1.Pod, error) {
func (c clientGetter) GetPod(ctx context.Context, namespace, name string) (*v1.Pod, error) {
return nil, apierrors.NewNotFound(v1.Resource("pods"), name)
}
func (c clientGetter) GetSecret(namespace, name string) (*v1.Secret, error) {
func (c clientGetter) GetSecret(ctx context.Context, namespace, name string) (*v1.Secret, error) {
return c.secretLister.Secrets(namespace).Get(name)
}
func (c clientGetter) GetNode(name string) (*v1.Node, error) {
func (c clientGetter) GetNode(ctx context.Context, name string) (*v1.Node, error) {
return nil, apierrors.NewNotFound(v1.Resource("nodes"), name)
}

View file

@ -177,7 +177,7 @@ func (v *validator) Validate(ctx context.Context, _ string, public *jwt.Claims,
noderef := private.Kubernetes.Node
secref := private.Kubernetes.Secret
// Make sure service account still exists (name and UID)
serviceAccount, err := v.getter.GetServiceAccount(namespace, saref.Name)
serviceAccount, err := v.getter.GetServiceAccount(ctx, namespace, saref.Name)
if err != nil {
klog.V(4).Infof("Could not retrieve service account %s/%s: %v", namespace, saref.Name, err)
return nil, err
@ -194,7 +194,7 @@ func (v *validator) Validate(ctx context.Context, _ string, public *jwt.Claims,
if secref != nil {
// Make sure token hasn't been invalidated by deletion of the secret
secret, err := v.getter.GetSecret(namespace, secref.Name)
secret, err := v.getter.GetSecret(ctx, namespace, secref.Name)
if err != nil {
klog.V(4).Infof("Could not retrieve bound secret %s/%s for service account %s/%s: %v", namespace, secref.Name, namespace, saref.Name, err)
return nil, errors.New("service account token has been invalidated")
@ -212,7 +212,7 @@ func (v *validator) Validate(ctx context.Context, _ string, public *jwt.Claims,
var podName, podUID string
if podref != nil {
// Make sure token hasn't been invalidated by deletion of the pod
pod, err := v.getter.GetPod(namespace, podref.Name)
pod, err := v.getter.GetPod(ctx, namespace, podref.Name)
if err != nil {
klog.V(4).Infof("Could not retrieve bound pod %s/%s for service account %s/%s: %v", namespace, podref.Name, namespace, saref.Name, err)
return nil, errors.New("service account token has been invalidated")
@ -244,7 +244,7 @@ func (v *validator) Validate(ctx context.Context, _ string, public *jwt.Claims,
return nil, fmt.Errorf("token is bound to a Node object but the %s feature gate is disabled", features.ServiceAccountTokenNodeBindingValidation)
}
node, err := v.getter.GetNode(noderef.Name)
node, err := v.getter.GetNode(ctx, noderef.Name)
if err != nil {
klog.V(4).Infof("Could not retrieve node object %q for service account %s/%s: %v", noderef.Name, namespace, saref.Name, err)
return nil, errors.New("service account token has been invalidated")

View file

@ -504,25 +504,25 @@ type fakeGetter struct {
node *v1.Node
}
func (f fakeGetter) GetServiceAccount(namespace, name string) (*v1.ServiceAccount, error) {
func (f fakeGetter) GetServiceAccount(ctx context.Context, namespace, name string) (*v1.ServiceAccount, error) {
if f.serviceAccount == nil {
return nil, apierrors.NewNotFound(schema.GroupResource{Group: "", Resource: "serviceaccounts"}, name)
}
return f.serviceAccount, nil
}
func (f fakeGetter) GetPod(namespace, name string) (*v1.Pod, error) {
func (f fakeGetter) GetPod(ctx context.Context, namespace, name string) (*v1.Pod, error) {
if f.pod == nil {
return nil, apierrors.NewNotFound(schema.GroupResource{Group: "", Resource: "pods"}, name)
}
return f.pod, nil
}
func (f fakeGetter) GetSecret(namespace, name string) (*v1.Secret, error) {
func (f fakeGetter) GetSecret(ctx context.Context, namespace, name string) (*v1.Secret, error) {
if f.secret == nil {
return nil, apierrors.NewNotFound(schema.GroupResource{Group: "", Resource: "secrets"}, name)
}
return f.secret, nil
}
func (f fakeGetter) GetNode(name string) (*v1.Node, error) {
func (f fakeGetter) GetNode(ctx context.Context, name string) (*v1.Node, error) {
if f.node == nil {
return nil, apierrors.NewNotFound(schema.GroupResource{Group: "", Resource: "nodes"}, name)
}

View file

@ -40,10 +40,10 @@ import (
// ServiceAccountTokenGetter defines functions to retrieve a named service account and secret
type ServiceAccountTokenGetter interface {
GetServiceAccount(namespace, name string) (*v1.ServiceAccount, error)
GetPod(namespace, name string) (*v1.Pod, error)
GetSecret(namespace, name string) (*v1.Secret, error)
GetNode(name string) (*v1.Node, error)
GetServiceAccount(ctx context.Context, namespace, name string) (*v1.ServiceAccount, error)
GetPod(ctx context.Context, namespace, name string) (*v1.Pod, error)
GetSecret(ctx context.Context, namespace, name string) (*v1.Secret, error)
GetNode(ctx context.Context, name string) (*v1.Node, error)
}
type TokenGenerator interface {

View file

@ -113,7 +113,7 @@ func (v *legacyValidator) Validate(ctx context.Context, tokenData string, public
if v.lookup {
// Make sure token hasn't been invalidated by deletion of the secret
secret, err := v.getter.GetSecret(namespace, secretName)
secret, err := v.getter.GetSecret(ctx, namespace, secretName)
if err != nil {
klog.V(4).Infof("Could not retrieve token %s/%s for service account %s/%s: %v", namespace, secretName, namespace, serviceAccountName, err)
return nil, errors.New("Token has been invalidated")
@ -128,7 +128,7 @@ func (v *legacyValidator) Validate(ctx context.Context, tokenData string, public
}
// Make sure service account still exists (name and UID)
serviceAccount, err := v.getter.GetServiceAccount(namespace, serviceAccountName)
serviceAccount, err := v.getter.GetServiceAccount(ctx, namespace, serviceAccountName)
if err != nil {
klog.V(4).Infof("Could not retrieve service account %s/%s: %v", namespace, serviceAccountName, err)
return nil, err