diff --git a/pkg/credentialprovider/plugin/plugin.go b/pkg/credentialprovider/plugin/plugin.go index 41ca908c997..bd87aecf139 100644 --- a/pkg/credentialprovider/plugin/plugin.go +++ b/pkg/credentialprovider/plugin/plugin.go @@ -34,6 +34,7 @@ import ( authenticationv1 "k8s.io/api/authentication/v1" v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/runtime/serializer" @@ -279,8 +280,11 @@ func (s *serviceAccountProvider) getServiceAccountData(namespace, name string) ( } // getServiceAccountToken returns a service account token for the service account. -func (s *serviceAccountProvider) getServiceAccountToken(podNamespace, podName, serviceAccountName string, podUID types.UID) (string, error) { +func (s *serviceAccountProvider) getServiceAccountToken(podNamespace, podName, serviceAccountName string, serviceAccountUID, podUID types.UID) (string, error) { tr, err := s.getServiceAccountTokenFunc(podNamespace, serviceAccountName, &authenticationv1.TokenRequest{ + ObjectMeta: metav1.ObjectMeta{ + UID: serviceAccountUID, + }, Spec: authenticationv1.TokenRequestSpec{ Audiences: []string{s.audience}, // expirationSeconds is not set explicitly here. It has the same default value of "ExpirationSeconds" in the TokenRequestSpec. @@ -387,7 +391,7 @@ func (p *pluginProvider) provide(image, podNamespace, podName string, podUID typ return credentialprovider.DockerConfig{} } - if serviceAccountToken, err = p.serviceAccountProvider.getServiceAccountToken(podNamespace, podName, serviceAccountName, podUID); err != nil { + if serviceAccountToken, err = p.serviceAccountProvider.getServiceAccountToken(podNamespace, podName, serviceAccountName, serviceAccountUID, podUID); err != nil { klog.Errorf("Error getting service account token %s/%s: %v", podNamespace, serviceAccountName, err) return credentialprovider.DockerConfig{} } diff --git a/pkg/kubelet/token/token_manager.go b/pkg/kubelet/token/token_manager.go index 3accadef0b8..11ee24b24be 100644 --- a/pkg/kubelet/token/token_manager.go +++ b/pkg/kubelet/token/token_manager.go @@ -206,5 +206,14 @@ func keyFunc(name, namespace string, tr *authenticationv1.TokenRequest) string { ref = *tr.Spec.BoundObjectRef } - return fmt.Sprintf("%q/%q/%#v/%#v/%#v", name, namespace, tr.Spec.Audiences, exp, ref) + var uid types.UID + if len(tr.UID) > 0 { + // If UID is set in the token request it is used as a precondition + // to ensure that the token request is for the same service account. + // This is useful to prevent stale tokens from being returned after a service account + // is deleted and recreated with the same name. + uid = tr.UID + } + + return fmt.Sprintf("%q/%q/%#v/%#v/%#v/%q", name, namespace, tr.Spec.Audiences, exp, ref, uid) } diff --git a/pkg/kubelet/token/token_manager_test.go b/pkg/kubelet/token/token_manager_test.go index 9512ccaf3f5..4ece42d9170 100644 --- a/pkg/kubelet/token/token_manager_test.go +++ b/pkg/kubelet/token/token_manager_test.go @@ -84,6 +84,90 @@ func TestTokenCachingAndExpiration(t *testing.T) { } }, }, + { + name: "service account recreated - cache miss due to different UID", + exp: time.Hour, + f: func(t *testing.T, s *suite) { + // First, get a token for service account with UID-1 + tr1 := &authenticationv1.TokenRequest{ + ObjectMeta: metav1.ObjectMeta{ + UID: "service-account-uid-1", + }, + Spec: authenticationv1.TokenRequestSpec{ + Audiences: []string{"foo1", "foo2"}, + ExpirationSeconds: getInt64Point(3600), + BoundObjectRef: &authenticationv1.BoundObjectReference{ + Kind: "pod", + Name: "foo-pod", + UID: "foo-uid", + }, + }, + } + + if _, err := s.mgr.GetServiceAccountToken("a", "b", tr1); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if s.tg.count != 2 { // First call from setup + this call + t.Fatalf("expected first token request: call count was %d", s.tg.count) + } + + // Now request token for "recreated" service account with UID-2 + tr2 := &authenticationv1.TokenRequest{ + ObjectMeta: metav1.ObjectMeta{ + UID: "service-account-uid-2", + }, + Spec: authenticationv1.TokenRequestSpec{ + Audiences: []string{"foo1", "foo2"}, + ExpirationSeconds: getInt64Point(3600), + BoundObjectRef: &authenticationv1.BoundObjectReference{ + Kind: "pod", + Name: "foo-pod", + UID: "foo-uid", + }, + }, + } + + if _, err := s.mgr.GetServiceAccountToken("a", "b", tr2); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if s.tg.count != 3 { // Should be 3 calls total (no cache hit) + t.Fatalf("expected cache miss due to different service account UID: call count was %d", s.tg.count) + } + }, + }, + { + name: "service account UID consistent - cache hit", + exp: time.Hour, + f: func(t *testing.T, s *suite) { + // Request token twice with same service account UID + tr := &authenticationv1.TokenRequest{ + ObjectMeta: metav1.ObjectMeta{ + UID: "consistent-service-account-uid", + }, + Spec: authenticationv1.TokenRequestSpec{ + Audiences: []string{"foo1", "foo2"}, + ExpirationSeconds: getInt64Point(3600), + BoundObjectRef: &authenticationv1.BoundObjectReference{ + Kind: "pod", + Name: "foo-pod", + UID: "foo-uid", + }, + }, + } + + if _, err := s.mgr.GetServiceAccountToken("a", "b", tr); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if _, err := s.mgr.GetServiceAccountToken("a", "b", tr); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if s.tg.count != 2 { // Setup call + first call, second should be cache hit + t.Fatalf("expected cache hit with same service account UID: call count was %d", s.tg.count) + } + }, + }, } for _, c := range cases { @@ -565,6 +649,168 @@ func TestKeyFunc(t *testing.T) { }, shouldHit: false, }, + { + name: "not hit due to different service account UID", + trus: []tokenRequestUnit{ + { + name: "foo-sa", + namespace: "foo-ns", + tr: &authenticationv1.TokenRequest{ + ObjectMeta: metav1.ObjectMeta{ + UID: "old-service-account-uid-123", + }, + Spec: authenticationv1.TokenRequestSpec{ + Audiences: []string{"foo1", "foo2"}, + ExpirationSeconds: getInt64Point(2000), + BoundObjectRef: &authenticationv1.BoundObjectReference{ + Kind: "pod", + Name: "foo-pod", + UID: "foo-uid", + }, + }, + }, + }, + }, + target: tokenRequestUnit{ + name: "foo-sa", + namespace: "foo-ns", + tr: &authenticationv1.TokenRequest{ + ObjectMeta: metav1.ObjectMeta{ + UID: "new-service-account-uid-456", // Different service account UID + }, + Spec: authenticationv1.TokenRequestSpec{ + Audiences: []string{"foo1", "foo2"}, + ExpirationSeconds: getInt64Point(2000), + BoundObjectRef: &authenticationv1.BoundObjectReference{ + Kind: "pod", + Name: "foo-pod", + UID: "foo-uid", + }, + }, + }, + }, + shouldHit: false, + }, + { + name: "hit with same service account UID", + trus: []tokenRequestUnit{ + { + name: "foo-sa", + namespace: "foo-ns", + tr: &authenticationv1.TokenRequest{ + ObjectMeta: metav1.ObjectMeta{ + UID: "same-service-account-uid-123", + }, + Spec: authenticationv1.TokenRequestSpec{ + Audiences: []string{"foo1", "foo2"}, + ExpirationSeconds: getInt64Point(2000), + BoundObjectRef: &authenticationv1.BoundObjectReference{ + Kind: "pod", + Name: "foo-pod", + UID: "foo-uid", + }, + }, + }, + }, + }, + target: tokenRequestUnit{ + name: "foo-sa", + namespace: "foo-ns", + tr: &authenticationv1.TokenRequest{ + ObjectMeta: metav1.ObjectMeta{ + UID: "same-service-account-uid-123", // Same service account UID + }, + Spec: authenticationv1.TokenRequestSpec{ + Audiences: []string{"foo1", "foo2"}, + ExpirationSeconds: getInt64Point(2000), + BoundObjectRef: &authenticationv1.BoundObjectReference{ + Kind: "pod", + Name: "foo-pod", + UID: "foo-uid", + }, + }, + }, + }, + shouldHit: true, + }, + { + name: "hit with empty UID (backward compatibility)", + trus: []tokenRequestUnit{ + { + name: "foo-sa", + namespace: "foo-ns", + tr: &authenticationv1.TokenRequest{ + // No UID set + Spec: authenticationv1.TokenRequestSpec{ + Audiences: []string{"foo1", "foo2"}, + ExpirationSeconds: getInt64Point(2000), + BoundObjectRef: &authenticationv1.BoundObjectReference{ + Kind: "pod", + Name: "foo-pod", + UID: "foo-uid", + }, + }, + }, + }, + }, + target: tokenRequestUnit{ + name: "foo-sa", + namespace: "foo-ns", + tr: &authenticationv1.TokenRequest{ + // No UID set + Spec: authenticationv1.TokenRequestSpec{ + Audiences: []string{"foo1", "foo2"}, + ExpirationSeconds: getInt64Point(2000), + BoundObjectRef: &authenticationv1.BoundObjectReference{ + Kind: "pod", + Name: "foo-pod", + UID: "foo-uid", + }, + }, + }, + }, + shouldHit: true, + }, + { + name: "not hit when one has UID and other doesn't", + trus: []tokenRequestUnit{ + { + name: "foo-sa", + namespace: "foo-ns", + tr: &authenticationv1.TokenRequest{ + ObjectMeta: metav1.ObjectMeta{ + UID: "service-account-uid-123", + }, + Spec: authenticationv1.TokenRequestSpec{ + Audiences: []string{"foo1", "foo2"}, + ExpirationSeconds: getInt64Point(2000), + BoundObjectRef: &authenticationv1.BoundObjectReference{ + Kind: "pod", + Name: "foo-pod", + UID: "foo-uid", + }, + }, + }, + }, + }, + target: tokenRequestUnit{ + name: "foo-sa", + namespace: "foo-ns", + tr: &authenticationv1.TokenRequest{ + // No UID set - should not hit cached entry with UID + Spec: authenticationv1.TokenRequestSpec{ + Audiences: []string{"foo1", "foo2"}, + ExpirationSeconds: getInt64Point(2000), + BoundObjectRef: &authenticationv1.BoundObjectReference{ + Kind: "pod", + Name: "foo-pod", + UID: "foo-uid", + }, + }, + }, + }, + shouldHit: false, + }, } for _, c := range cases { @@ -586,7 +832,95 @@ func TestKeyFunc(t *testing.T) { } }) } +} +func TestServiceAccountRecreationCacheInvalidation(t *testing.T) { + mgr := NewManager(nil) + mgr.clock = testingclock.NewFakeClock(time.Time{}.Add(30 * 24 * time.Hour)) + + callCount := 0 + mgr.getToken = func(name, namespace string, tr *authenticationv1.TokenRequest) (*authenticationv1.TokenRequest, error) { + callCount++ + expSecs := int64(3600) + return &authenticationv1.TokenRequest{ + ObjectMeta: tr.ObjectMeta, // Preserve the UID from request + Spec: authenticationv1.TokenRequestSpec{ + ExpirationSeconds: &expSecs, + }, + Status: authenticationv1.TokenRequestStatus{ + Token: fmt.Sprintf("token-%d", callCount), + ExpirationTimestamp: metav1.Time{Time: mgr.clock.Now().Add(time.Hour)}, + }, + }, nil + } + + // 1. Get token for service account with original UID + originalTR := &authenticationv1.TokenRequest{ + ObjectMeta: metav1.ObjectMeta{ + UID: "original-sa-uid-123", + }, + Spec: authenticationv1.TokenRequestSpec{ + Audiences: []string{"test-audience"}, + ExpirationSeconds: getInt64Point(3600), + }, + } + + token1, err := mgr.GetServiceAccountToken("test-ns", "test-sa", originalTR) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if callCount != 1 { + t.Fatalf("expected 1 API call, got %d", callCount) + } + if token1.Status.Token != "token-1" { + t.Fatalf("unexpected token: %s", token1.Status.Token) + } + + // 2. Request same token again - should be cache hit + token2, err := mgr.GetServiceAccountToken("test-ns", "test-sa", originalTR) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if callCount != 1 { + t.Fatalf("expected cache hit, but got %d API calls", callCount) + } + if token2.Status.Token != "token-1" { + t.Fatalf("unexpected token from cache: %s", token2.Status.Token) + } + + // 3. Service account recreated with new UID - should be cache miss + recreatedTR := &authenticationv1.TokenRequest{ + ObjectMeta: metav1.ObjectMeta{ + UID: "recreated-sa-uid-456", + }, + Spec: authenticationv1.TokenRequestSpec{ + Audiences: []string{"test-audience"}, + ExpirationSeconds: getInt64Point(3600), + }, + } + + token3, err := mgr.GetServiceAccountToken("test-ns", "test-sa", recreatedTR) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if callCount != 2 { + t.Fatalf("expected cache miss due to UID change, but got %d API calls", callCount) + } + if token3.Status.Token != "token-2" { + t.Fatalf("unexpected token for recreated SA: %s", token3.Status.Token) + } + + // 4. Request for recreated SA again - should be cache hit + token4, err := mgr.GetServiceAccountToken("test-ns", "test-sa", recreatedTR) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if callCount != 2 { + t.Fatalf("expected cache hit for recreated SA, but got %d API calls", callCount) + } + if token4.Status.Token != "token-2" { + t.Fatalf("unexpected token from cache for recreated SA: %s", token4.Status.Token) + } } func getTokenRequest() *authenticationv1.TokenRequest {