credentialprovider: track service account cred source in ext provider keyring

Signed-off-by: Anish Ramasekar <anish.ramasekar@gmail.com>

pkg/credentialprovider/keyring.go

@@ -87,7 +87,8 @@ func NewTrackedAuthConfig(c *AuthConfig, src *CredentialSource) *TrackedAuthConf
 }
 
 type CredentialSource struct {
-	Secret SecretCoordinates
+	Secret         *SecretCoordinates
+	ServiceAccount *ServiceAccountCoordinates
 }
 
 type SecretCoordinates struct {
@@ -96,6 +97,12 @@ type SecretCoordinates struct {
 	Name      string
 }
 
+type ServiceAccountCoordinates struct {
+	UID       string
+	Namespace string
+	Name      string
+}
+
 // AuthConfig contains authorization information for connecting to a Registry
 // This type mirrors "github.com/docker/docker/api/types.AuthConfig"
 type AuthConfig struct {
@@ -316,8 +323,6 @@ func (dk *providersDockerKeyring) Lookup(image string) ([]TrackedAuthConfig, boo
 	keyring := &BasicDockerKeyring{}
 
 	for _, p := range dk.Providers {
-		// TODO: the source should probably change once we depend on service accounts (KEP-4412).
-		//       Perhaps `Provide()` should return the source modified to accommodate this?
 		keyring.Add(nil, p.Provide(image))
 	}
 

pkg/credentialprovider/plugin/plugin.go

@@ -401,20 +401,22 @@ type perPodPluginProvider struct {
 	serviceAccountName string
 }
 
-// Enabled always returns true since registration of the plugin via kubelet implies it should be enabled.
-func (p *perPodPluginProvider) Enabled() bool {
-	return true
-}
-
-func (p *perPodPluginProvider) Provide(image string) credentialprovider.DockerConfig {
+// provideWithCoordinates returns the DockerConfig and, if available, the ServiceAccountCoordinates
+// used for credential resolution. If ServiceAccountCoordinates is nil, it means no service account
+// context was used (e.g., the plugin is not operating in service account token mode or no service
+// account was provided for the request).
+func (p *perPodPluginProvider) provideWithCoordinates(image string) (credentialprovider.DockerConfig, *credentialprovider.ServiceAccountCoordinates) {
 	return p.provider.provide(image, p.podNamespace, p.podName, p.podUID, p.serviceAccountName)
 }
 
 // provide returns a credentialprovider.DockerConfig based on the credentials returned
-// from cache or the exec plugin.
-func (p *pluginProvider) provide(image, podNamespace, podName string, podUID types.UID, serviceAccountName string) credentialprovider.DockerConfig {
+// from cache or the exec plugin. The returned ServiceAccountCoordinates may be nil.
+// If ServiceAccountCoordinates is nil, it means no service account context was used
+// (e.g., the plugin is not operating in service account token mode or no service account
+// was provided for the request).
+func (p *pluginProvider) provide(image, podNamespace, podName string, podUID types.UID, serviceAccountName string) (credentialprovider.DockerConfig, *credentialprovider.ServiceAccountCoordinates) {
 	if !p.isImageAllowed(image) {
-		return credentialprovider.DockerConfig{}
+		return credentialprovider.DockerConfig{}, nil
 	}
 
 	var serviceAccountUID types.UID
@@ -423,11 +425,12 @@ func (p *pluginProvider) provide(image, podNamespace, podName string, podUID typ
 	var err error
 	var serviceAccountCacheKey string
 	var serviceAccountTokenHash string
+	var serviceAccountCoordinates *credentialprovider.ServiceAccountCoordinates
 
 	if p.serviceAccountProvider != nil {
 		if len(serviceAccountName) == 0 && p.serviceAccountProvider.requireServiceAccount {
 			klog.V(5).Infof("Service account name is empty for pod %s/%s", podNamespace, podName)
-			return credentialprovider.DockerConfig{}
+			return credentialprovider.DockerConfig{}, nil
 		}
 
 		// If the service account name is empty and the plugin has indicated that invoking the plugin
@@ -441,16 +444,16 @@ func (p *pluginProvider) provide(image, podNamespace, podName string, podUID typ
 					// for image pull. If any of the required annotation is missing, we will not invoke the plugin. We will log the error
 					// at higher verbosity level as it could be noisy.
 					klog.V(5).Infof("Failed to get service account data %s/%s: %v", podNamespace, serviceAccountName, err)
-					return credentialprovider.DockerConfig{}
+					return credentialprovider.DockerConfig{}, nil
 				}
 
 				klog.Errorf("Failed to get service account %s/%s: %v", podNamespace, serviceAccountName, err)
-				return credentialprovider.DockerConfig{}
+				return credentialprovider.DockerConfig{}, 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{}
+				return credentialprovider.DockerConfig{}, nil
 			}
 			serviceAccountTokenHash = getHashIfNotEmpty(serviceAccountToken)
 
@@ -468,7 +471,13 @@ func (p *pluginProvider) provide(image, podNamespace, podName string, podUID typ
 			serviceAccountCacheKey, err = generateServiceAccountCacheKey(c)
 			if err != nil {
 				klog.Errorf("Error generating service account cache key: %v", err)
-				return credentialprovider.DockerConfig{}
+				return credentialprovider.DockerConfig{}, nil
+			}
+
+			serviceAccountCoordinates = &credentialprovider.ServiceAccountCoordinates{
+				UID:       string(serviceAccountUID),
+				Namespace: podNamespace,
+				Name:      serviceAccountName,
 			}
 		}
 	}
@@ -477,11 +486,11 @@ func (p *pluginProvider) provide(image, podNamespace, podName string, podUID typ
 	cachedConfig, found, errCache := p.getCachedCredentials(image, serviceAccountCacheKey)
 	if errCache != nil {
 		klog.Errorf("Failed to get cached docker config: %v", err)
-		return credentialprovider.DockerConfig{}
+		return credentialprovider.DockerConfig{}, nil
 	}
 
 	if found {
-		return cachedConfig
+		return cachedConfig, serviceAccountCoordinates
 	}
 
 	// ExecPlugin is wrapped in single flight to exec plugin once for concurrent same image request.
@@ -500,7 +509,7 @@ func (p *pluginProvider) provide(image, podNamespace, podName string, podUID typ
 		// and the workload is using the same service account.
 		if singleFlightKey, err = generateSingleFlightKey(image, serviceAccountTokenHash, saAnnotations); err != nil {
 			klog.Errorf("Error generating singleflight key: %v", err)
-			return credentialprovider.DockerConfig{}
+			return credentialprovider.DockerConfig{}, nil
 		}
 	}
 	res, err, _ := p.group.Do(singleFlightKey, func() (interface{}, error) {
@@ -509,13 +518,13 @@ func (p *pluginProvider) provide(image, podNamespace, podName string, podUID typ
 
 	if err != nil {
 		klog.Errorf("Failed getting credential from external registry credential provider: %v", err)
-		return credentialprovider.DockerConfig{}
+		return credentialprovider.DockerConfig{}, nil
 	}
 
 	response, ok := res.(*credentialproviderapi.CredentialProviderResponse)
 	if !ok {
 		klog.Errorf("Invalid response type returned by external credential provider")
-		return credentialprovider.DockerConfig{}
+		return credentialprovider.DockerConfig{}, nil
 	}
 	if len(serviceAccountToken) > 0 && p.serviceAccountProvider.cacheType != kubeletconfig.TokenServiceAccountTokenCacheType {
 		// validate that the response credentials are not the echoed token back verbatim when cache
@@ -524,7 +533,7 @@ func (p *pluginProvider) provide(image, podNamespace, podName string, podUID typ
 		for _, authConfig := range response.Auth {
 			if authConfig.Password == serviceAccountToken {
 				klog.Errorf("Credential provider plugin returned the service account token as the password for image %q, which is not allowed when service account cache type is not set to 'Token'", image)
-				return credentialprovider.DockerConfig{}
+				return credentialprovider.DockerConfig{}, nil
 			}
 		}
 	}
@@ -540,7 +549,7 @@ func (p *pluginProvider) provide(image, podNamespace, podName string, podUID typ
 		cacheKey = globalCacheKey
 	default:
 		klog.Errorf("credential provider plugin did not return a valid cacheKeyType: %q", cacheKeyType)
-		return credentialprovider.DockerConfig{}
+		return credentialprovider.DockerConfig{}, nil
 	}
 
 	dockerConfig := make(credentialprovider.DockerConfig, len(response.Auth))
@@ -553,14 +562,14 @@ func (p *pluginProvider) provide(image, podNamespace, podName string, podUID typ
 
 	// cache duration was explicitly 0 so don't cache this response at all.
 	if response.CacheDuration != nil && response.CacheDuration.Duration == 0 {
-		return dockerConfig
+		return dockerConfig, serviceAccountCoordinates
 	}
 
 	var expiresAt time.Time
 	// nil cache duration means use the default cache duration
 	if response.CacheDuration == nil {
 		if p.defaultCacheDuration == 0 {
-			return dockerConfig
+			return dockerConfig, serviceAccountCoordinates
 		}
 		expiresAt = p.clock.Now().Add(p.defaultCacheDuration)
 	} else {
@@ -570,7 +579,7 @@ func (p *pluginProvider) provide(image, podNamespace, podName string, podUID typ
 	cacheKey, err = generateCacheKey(cacheKey, serviceAccountCacheKey)
 	if err != nil {
 		klog.Errorf("Error generating cache key: %v", err)
-		return credentialprovider.DockerConfig{}
+		return credentialprovider.DockerConfig{}, nil
 	}
 
 	cachedEntry := &cacheEntry{
@@ -583,12 +592,7 @@ func (p *pluginProvider) provide(image, podNamespace, podName string, podUID typ
 		klog.Errorf("Error adding auth entry to cache: %v", err)
 	}
 
-	return dockerConfig
-}
-
-// Enabled always returns true since registration of the plugin via kubelet implies it should be enabled.
-func (p *pluginProvider) Enabled() bool {
-	return true
+	return dockerConfig, serviceAccountCoordinates
 }
 
 // isImageAllowed returns true if the image matches against the list of allowed matches by the plugin.

pkg/credentialprovider/plugin/plugin_test.go

@@ -125,7 +125,7 @@ func TestSingleflightProvide(t *testing.T) {
 		wg.Add(1)
 		go func(i int) {
 			defer wg.Done()
-			result := dynamicProvider.Provide(image)
+			result, _ := dynamicProvider.provideWithCoordinates(image)
 			results[i] = result
 		}(i)
 	}
@@ -158,7 +158,7 @@ func TestSingleflightProvide(t *testing.T) {
 		wg.Add(1)
 		go func(i int) {
 			defer wg.Done()
-			result := dynamicProvider.Provide(image)
+			result, _ := dynamicProvider.provideWithCoordinates(image)
 			results[i] = result
 		}(i)
 	}
@@ -179,7 +179,7 @@ func TestSingleflightProvide(t *testing.T) {
 		wg.Add(1)
 		go func(i int) {
 			defer wg.Done()
-			result := dynamicProvider.Provide(image)
+			result, _ := dynamicProvider.provideWithCoordinates(image)
 			results[i] = result
 		}(i)
 	}
@@ -191,7 +191,7 @@ func TestSingleflightProvide(t *testing.T) {
 	}
 }
 
-func Test_Provide(t *testing.T) {
+func Test_ProvideWithCoordinates(t *testing.T) {
 	klog.InitFlags(nil)
 	if err := flag.Set("v", "6"); err != nil {
 		t.Fatalf("failed to set log level: %v", err)
@@ -200,11 +200,12 @@ func Test_Provide(t *testing.T) {
 
 	tclock := clock.RealClock{}
 	testcases := []struct {
-		name           string
-		pluginProvider *perPodPluginProvider
-		image          string
-		dockerconfig   credentialprovider.DockerConfig
-		wantLog        string
+		name                   string
+		pluginProvider         *perPodPluginProvider
+		image                  string
+		dockerconfig           credentialprovider.DockerConfig
+		wantLog                string
+		expectedServiceAccount *credentialprovider.ServiceAccountCoordinates
 	}{
 		{
 			name: "exact image match, with Registry cache key",
@@ -529,6 +530,7 @@ func Test_Provide(t *testing.T) {
 								ObjectMeta: metav1.ObjectMeta{
 									Name:      "sa-name",
 									Namespace: "ns",
+									UID:       "sa-uid",
 									Annotations: map[string]string{
 										"domain.io/identity-id":   "id",
 										"domain.io/identity-type": "type",
@@ -552,6 +554,11 @@ func Test_Provide(t *testing.T) {
 					Password: "password",
 				},
 			},
+			expectedServiceAccount: &credentialprovider.ServiceAccountCoordinates{
+				Namespace: "ns",
+				Name:      "sa-name",
+				UID:       "sa-uid",
+			},
 		},
 	}
 
@@ -563,8 +570,17 @@ func Test_Provide(t *testing.T) {
 			klog.LogToStderr(false)
 			defer klog.LogToStderr(true)
 
-			if got := testcase.pluginProvider.Provide(testcase.image); !reflect.DeepEqual(got, testcase.dockerconfig) {
-				t.Errorf("unexpected docker config: %v, expected: %v", got, testcase.dockerconfig)
+			gotConfig, gotCoordinates := testcase.pluginProvider.provideWithCoordinates(testcase.image)
+			if !reflect.DeepEqual(gotConfig, testcase.dockerconfig) {
+				t.Errorf("unexpected docker config from ProvideWithCoordinates: %v, expected: %v", gotConfig, testcase.dockerconfig)
+			}
+
+			if testcase.expectedServiceAccount != nil {
+				if !reflect.DeepEqual(gotCoordinates, testcase.expectedServiceAccount) {
+					t.Errorf("unexpected service account coordinates: %v, expected: %v", gotCoordinates, testcase.expectedServiceAccount)
+				}
+			} else if gotCoordinates != nil {
+				t.Errorf("expected nil service account coordinates but got: %v", gotCoordinates)
 			}
 
 			klog.Flush()
@@ -641,11 +657,9 @@ func Test_ProvideParallel(t *testing.T) {
 			for i := 0; i < 5; i++ {
 				go func(w *sync.WaitGroup) {
 					image := fmt.Sprintf(testcase.registry+"/%s", rand.String(5))
-					dockerconfigResponse := pluginProvider.Provide(image)
+					dockerconfigResponse, _ := pluginProvider.provideWithCoordinates(image)
 					if !reflect.DeepEqual(dockerconfigResponse, dockerconfig) {
-						t.Logf("actual docker config: %v", dockerconfigResponse)
-						t.Logf("expected docker config: %v", dockerconfig)
-						t.Error("unexpected docker config")
+						t.Errorf("unexpected docker config for image %s: %v, expected: %v", image, dockerconfigResponse, dockerconfig)
 					}
 					w.Done()
 				}(&wg)
@@ -1110,7 +1124,7 @@ func Test_NoCacheResponse(t *testing.T) {
 		},
 	}
 
-	dockerConfig := pluginProvider.Provide("test.registry.io/foo/bar")
+	dockerConfig, _ := pluginProvider.provideWithCoordinates("test.registry.io/foo/bar")
 	if !reflect.DeepEqual(dockerConfig, expectedDockerConfig) {
 		t.Logf("actual docker config: %v", dockerConfig)
 		t.Logf("expected docker config: %v", expectedDockerConfig)
@@ -1544,7 +1558,7 @@ func Test_CacheKeyGeneration(t *testing.T) {
 			if tc.serviceAccountCacheType != "" {
 				saTokenCacheType = &tc.serviceAccountCacheType
 			}
-			pluginProvider := createTestPluginProvider(t, tc.pluginCacheKeyType, saTokenCacheType)
+			pluginProvider := createTestPerPodPluginProvider(t, tc.pluginCacheKeyType, saTokenCacheType)
 
 			// Test image and expected SA parameters
 			testImage := "test.registry.io/foo/bar"
@@ -1566,7 +1580,7 @@ func Test_CacheKeyGeneration(t *testing.T) {
 				cacheType:   cacheTypeValue,
 			}
 
-			dockerConfig := pluginProvider.Provide(testImage)
+			dockerConfig, _ := pluginProvider.provideWithCoordinates(testImage)
 			verifyDockerConfig(t, dockerConfig)
 
 			// Verify the cache key is as expected
@@ -1579,13 +1593,13 @@ func Test_CacheKeyGeneration(t *testing.T) {
 
 			// Test cache hit (nil out plugin to ensure cache is used)
 			pluginProvider.provider.plugin = nil
-			cachedConfig := pluginProvider.Provide(testImage)
+			cachedConfig, _ := pluginProvider.provideWithCoordinates(testImage)
 			verifyDockerConfig(t, cachedConfig)
 		})
 	}
 }
 
-func createTestPluginProvider(t *testing.T, pluginCacheKeyType credentialproviderapi.PluginCacheKeyType, saTokenCacheType *kubeletconfig.ServiceAccountTokenCacheType) *perPodPluginProvider {
+func createTestPerPodPluginProvider(t *testing.T, pluginCacheKeyType credentialproviderapi.PluginCacheKeyType, saTokenCacheType *kubeletconfig.ServiceAccountTokenCacheType) *perPodPluginProvider {
 	t.Helper()
 
 	tclock := clock.RealClock{}

pkg/credentialprovider/plugin/plugins.go

@@ -17,6 +17,7 @@ limitations under the License.
 package plugin
 
 import (
+	"fmt"
 	"sync"
 
 	"k8s.io/apimachinery/pkg/types"
@@ -27,6 +28,11 @@ import (
 	"k8s.io/kubernetes/pkg/features"
 )
 
+type dockerConfigProviderWithCoordinates interface {
+	// provideWithCoordinates returns the DockerConfig and service account coordinates for the given image.
+	provideWithCoordinates(image string) (credentialprovider.DockerConfig, *credentialprovider.ServiceAccountCoordinates)
+}
+
 type provider struct {
 	name string
 	impl *pluginProvider
@@ -41,7 +47,7 @@ func registerCredentialProviderPlugin(name string, p *pluginProvider) {
 	defer providersMutex.Unlock()
 
 	if seenProviderNames.Has(name) {
-		klog.Fatalf("Credential provider %q was registered twice", name)
+		panic(fmt.Sprintf("Credential provider %q was registered twice", name))
 	}
 	seenProviderNames.Insert(name)
 
@@ -50,7 +56,7 @@ func registerCredentialProviderPlugin(name string, p *pluginProvider) {
 }
 
 type externalCredentialProviderKeyring struct {
-	providers []credentialprovider.DockerConfigProvider
+	providers []dockerConfigProviderWithCoordinates
 }
 
 func NewExternalCredentialProviderDockerKeyring(podNamespace, podName, podUID, serviceAccountName string) credentialprovider.DockerKeyring {
@@ -58,14 +64,10 @@ func NewExternalCredentialProviderDockerKeyring(podNamespace, podName, podUID, s
 	defer providersMutex.RUnlock()
 
 	keyring := &externalCredentialProviderKeyring{
-		providers: make([]credentialprovider.DockerConfigProvider, 0, len(providers)),
+		providers: make([]dockerConfigProviderWithCoordinates, 0, len(providers)),
 	}
 
 	for _, p := range providers {
-		if !p.impl.Enabled() {
-			continue
-		}
-
 		pp := &perPodPluginProvider{
 			name:     p.name,
 			provider: p.impl,
@@ -91,8 +93,12 @@ func (k *externalCredentialProviderKeyring) Lookup(image string) ([]credentialpr
 	keyring := &credentialprovider.BasicDockerKeyring{}
 
 	for _, p := range k.providers {
-		// TODO: modify the credentialprovider.CredentialSource to contain the SA/pod information
-		keyring.Add(nil, p.Provide(image))
+		dockerConfig, saCoords := p.provideWithCoordinates(image)
+		if saCoords != nil {
+			keyring.Add(&credentialprovider.CredentialSource{ServiceAccount: saCoords}, dockerConfig)
+		} else {
+			keyring.Add(nil, dockerConfig)
+		}
 	}
 
 	return keyring.Lookup(image)

pkg/credentialprovider/plugin/plugins_test.go

@@ -0,0 +1,517 @@
+/*
+Copyright 2025 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package plugin
+
+import (
+	"sync"
+	"testing"
+	"time"
+
+	"k8s.io/apimachinery/pkg/util/sets"
+	utilfeature "k8s.io/apiserver/pkg/util/feature"
+	"k8s.io/client-go/tools/cache"
+	featuregatetesting "k8s.io/component-base/featuregate/testing"
+	credentialproviderapi "k8s.io/kubelet/pkg/apis/credentialprovider"
+	"k8s.io/kubernetes/pkg/credentialprovider"
+	"k8s.io/kubernetes/pkg/features"
+	testingclock "k8s.io/utils/clock/testing"
+)
+
+// fakedockerConfigProviderWithCoordinates implements dockerConfigProviderWithCoordinates for testing
+type fakedockerConfigProviderWithCoordinates struct {
+	dockerConfig         credentialprovider.DockerConfig
+	serviceAccountCoords *credentialprovider.ServiceAccountCoordinates
+	callCount            int
+	lastImageRequested   string
+	mu                   sync.Mutex
+}
+
+func (f *fakedockerConfigProviderWithCoordinates) provideWithCoordinates(image string) (credentialprovider.DockerConfig, *credentialprovider.ServiceAccountCoordinates) {
+	f.mu.Lock()
+	defer f.mu.Unlock()
+	f.callCount++
+	f.lastImageRequested = image
+
+	return f.dockerConfig, f.serviceAccountCoords
+}
+
+func (f *fakedockerConfigProviderWithCoordinates) getCallCount() int {
+	f.mu.Lock()
+	defer f.mu.Unlock()
+	return f.callCount
+}
+
+func (f *fakedockerConfigProviderWithCoordinates) getLastImageRequested() string {
+	f.mu.Lock()
+	defer f.mu.Unlock()
+	return f.lastImageRequested
+}
+
+// createTestPluginProvider creates a pluginProvider for testing
+func createTestPluginProvider(dockerConfig credentialprovider.DockerConfig) *pluginProvider {
+	testClock := testingclock.NewFakeClock(time.Now())
+
+	authConfigMap := make(map[string]credentialproviderapi.AuthConfig)
+	for registry, config := range dockerConfig {
+		authConfigMap[registry] = credentialproviderapi.AuthConfig{
+			Username: config.Username,
+			Password: config.Password,
+		}
+	}
+
+	mockPlugin := &fakeExecPlugin{
+		cacheKeyType:  credentialproviderapi.ImagePluginCacheKeyType,
+		cacheDuration: time.Hour,
+		auth:          authConfigMap,
+	}
+
+	provider := &pluginProvider{
+		clock:                testClock,
+		matchImages:          []string{"*"},
+		cache:                cache.NewExpirationStore(cacheKeyFunc, &cacheExpirationPolicy{clock: testClock}),
+		defaultCacheDuration: time.Hour,
+		lastCachePurge:       testClock.Now(),
+		plugin:               mockPlugin,
+	}
+
+	return provider
+}
+
+// setupTestProviders sets up test providers and cleans up after the test
+func setupTestProviders(t *testing.T) {
+	t.Helper()
+
+	// Save original state
+	providersMutex.Lock()
+	originalProviders := providers
+	originalSeenProviderNames := seenProviderNames
+
+	// Reset to clean state
+	providers = make([]provider, 0)
+	seenProviderNames = sets.NewString()
+	providersMutex.Unlock()
+
+	t.Cleanup(func() {
+		// Restore original state
+		providersMutex.Lock()
+		providers = originalProviders
+		seenProviderNames = originalSeenProviderNames
+		providersMutex.Unlock()
+	})
+}
+
+func TestRegisterCredentialProviderPlugin(t *testing.T) {
+	testCases := []struct {
+		name           string
+		firstProvider  string
+		secondProvider string // empty means no second provider
+		shouldPanic    bool
+	}{
+		{
+			name:          "successful registration",
+			firstProvider: "test-provider",
+			shouldPanic:   false,
+		},
+		{
+			name:           "duplicate registration should panic",
+			firstProvider:  "duplicate-provider",
+			secondProvider: "duplicate-provider",
+			shouldPanic:    true,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			setupTestProviders(t)
+
+			mockProvider1 := createTestPluginProvider(credentialprovider.DockerConfig{
+				"test.registry.io": credentialprovider.DockerConfigEntry{
+					Username: "user",
+					Password: "pass",
+				},
+			})
+
+			// Register first provider
+			registerCredentialProviderPlugin(tc.firstProvider, mockProvider1)
+
+			// Verify first registration
+			providersMutex.RLock()
+			if len(providers) != 1 {
+				t.Errorf("Expected 1 provider after first registration, got %d", len(providers))
+			}
+			if providers[0].name != tc.firstProvider {
+				t.Errorf("Expected provider name '%s', got %s", tc.firstProvider, providers[0].name)
+			}
+			if providers[0].impl != mockProvider1 {
+				t.Errorf("Expected provider implementation to match")
+			}
+			if !seenProviderNames.Has(tc.firstProvider) {
+				t.Errorf("Expected '%s' to be in seenProviderNames", tc.firstProvider)
+			}
+			providersMutex.RUnlock()
+
+			// If we have a second provider to test, register it
+			if tc.secondProvider != "" {
+				mockProvider2 := createTestPluginProvider(credentialprovider.DockerConfig{})
+
+				if tc.shouldPanic {
+					// Test that registering duplicate provider name panics
+					defer func() {
+						if r := recover(); r == nil {
+							t.Errorf("Expected panic for duplicate provider registration")
+						}
+					}()
+				}
+
+				registerCredentialProviderPlugin(tc.secondProvider, mockProvider2)
+
+				if !tc.shouldPanic {
+					// If we didn't expect a panic, verify the second provider was registered
+					providersMutex.RLock()
+					if len(providers) != 2 {
+						t.Errorf("Expected 2 providers after second registration, got %d", len(providers))
+					}
+					providersMutex.RUnlock()
+				}
+			}
+		})
+	}
+}
+
+func TestNewExternalCredentialProviderDockerKeyring(t *testing.T) {
+	testCases := []struct {
+		name                         string
+		setupProviders               func()
+		featureGateEnabled           bool
+		expectedProviderCount        int
+		expectedPodNamespace         string
+		expectedPodName              string
+		expectedPodUID               string
+		expectedServiceAccountName   string
+		validatePerPodProviderFields bool
+	}{
+		{
+			name:                  "no providers registered",
+			setupProviders:        func() {},
+			expectedProviderCount: 0,
+		},
+		{
+			name: "multiple providers registered",
+			setupProviders: func() {
+				mockProvider1 := createTestPluginProvider(credentialprovider.DockerConfig{})
+				mockProvider2 := createTestPluginProvider(credentialprovider.DockerConfig{})
+				registerCredentialProviderPlugin("enabled-provider-1", mockProvider1)
+				registerCredentialProviderPlugin("enabled-provider-2", mockProvider2)
+			},
+			expectedProviderCount: 2,
+		},
+		{
+			name: "feature gate enabled - pod information should be set",
+			setupProviders: func() {
+				mockProvider := createTestPluginProvider(credentialprovider.DockerConfig{})
+				registerCredentialProviderPlugin("test-provider", mockProvider)
+			},
+			featureGateEnabled:           true,
+			expectedProviderCount:        1,
+			expectedPodNamespace:         "test-namespace",
+			expectedPodName:              "test-pod",
+			expectedPodUID:               "test-uid",
+			expectedServiceAccountName:   "test-sa",
+			validatePerPodProviderFields: true,
+		},
+		{
+			name: "feature gate disabled - pod information should be empty",
+			setupProviders: func() {
+				mockProvider := createTestPluginProvider(credentialprovider.DockerConfig{})
+				registerCredentialProviderPlugin("test-provider", mockProvider)
+			},
+			featureGateEnabled:           false,
+			expectedProviderCount:        1,
+			expectedPodNamespace:         "",
+			expectedPodName:              "",
+			expectedPodUID:               "",
+			expectedServiceAccountName:   "",
+			validatePerPodProviderFields: true,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			setupTestProviders(t)
+
+			featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.KubeletServiceAccountTokenForCredentialProviders, tc.featureGateEnabled)
+
+			tc.setupProviders()
+
+			keyring := NewExternalCredentialProviderDockerKeyring("test-namespace", "test-pod", "test-uid", "test-sa")
+
+			externalKeyring, ok := keyring.(*externalCredentialProviderKeyring)
+			if !ok {
+				t.Fatalf("Expected externalCredentialProviderKeyring, got %T", keyring)
+			}
+
+			if len(externalKeyring.providers) != tc.expectedProviderCount {
+				t.Errorf("Expected %d providers, got %d", tc.expectedProviderCount, len(externalKeyring.providers))
+			}
+
+			if tc.validatePerPodProviderFields && len(externalKeyring.providers) > 0 {
+				perPodProvider, ok := externalKeyring.providers[0].(*perPodPluginProvider)
+				if !ok {
+					t.Fatalf("Expected perPodPluginProvider, got %T", externalKeyring.providers[0])
+				}
+
+				if perPodProvider.podNamespace != tc.expectedPodNamespace {
+					t.Errorf("Expected podNamespace '%s', got %s", tc.expectedPodNamespace, perPodProvider.podNamespace)
+				}
+				if perPodProvider.podName != tc.expectedPodName {
+					t.Errorf("Expected podName '%s', got %s", tc.expectedPodName, perPodProvider.podName)
+				}
+				if string(perPodProvider.podUID) != tc.expectedPodUID {
+					t.Errorf("Expected podUID '%s', got %s", tc.expectedPodUID, string(perPodProvider.podUID))
+				}
+				if perPodProvider.serviceAccountName != tc.expectedServiceAccountName {
+					t.Errorf("Expected serviceAccountName '%s', got %s", tc.expectedServiceAccountName, perPodProvider.serviceAccountName)
+				}
+			}
+		})
+	}
+}
+
+func TestExternalCredentialProviderKeyringLookupNoProviders(t *testing.T) {
+	keyring := &externalCredentialProviderKeyring{
+		providers: []dockerConfigProviderWithCoordinates{},
+	}
+
+	configs, found := keyring.Lookup("test.registry.io/image:tag")
+
+	if found {
+		t.Errorf("Expected not found, got found=true")
+	}
+	if len(configs) != 0 {
+		t.Errorf("Expected 0 configs, got %d", len(configs))
+	}
+}
+
+func TestExternalCredentialProviderKeyringLookupWithProviders(t *testing.T) {
+	featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.KubeletEnsureSecretPulledImages, true)
+
+	testCases := []struct {
+		name                     string
+		image                    string
+		providers                []dockerConfigProviderWithCoordinates
+		expectedConfigCount      int
+		expectedFound            bool
+		expectedServiceAccountOK bool
+	}{
+		{
+			name:  "single provider with config",
+			image: "test.registry.io/image:tag",
+			providers: []dockerConfigProviderWithCoordinates{
+				&fakedockerConfigProviderWithCoordinates{
+					dockerConfig: credentialprovider.DockerConfig{
+						"test.registry.io": credentialprovider.DockerConfigEntry{
+							Username: "user1",
+							Password: "pass1",
+						},
+					},
+					serviceAccountCoords: nil,
+				},
+			},
+			expectedConfigCount:      1,
+			expectedFound:            true,
+			expectedServiceAccountOK: false,
+		},
+		{
+			name:  "single provider with service account coordinates",
+			image: "test.registry.io/image:tag",
+			providers: []dockerConfigProviderWithCoordinates{
+				&fakedockerConfigProviderWithCoordinates{
+					dockerConfig: credentialprovider.DockerConfig{
+						"test.registry.io": credentialprovider.DockerConfigEntry{
+							Username: "user1",
+							Password: "pass1",
+						},
+					},
+					serviceAccountCoords: &credentialprovider.ServiceAccountCoordinates{
+						Namespace: "test-namespace",
+						Name:      "test-sa",
+						UID:       "test-uid",
+					},
+				},
+			},
+			expectedConfigCount:      1,
+			expectedFound:            true,
+			expectedServiceAccountOK: true,
+		},
+		{
+			name:  "multiple providers",
+			image: "test.registry.io/image:tag",
+			providers: []dockerConfigProviderWithCoordinates{
+				&fakedockerConfigProviderWithCoordinates{
+					dockerConfig: credentialprovider.DockerConfig{
+						"test.registry.io": credentialprovider.DockerConfigEntry{
+							Username: "user1",
+							Password: "pass1",
+						},
+					},
+					serviceAccountCoords: &credentialprovider.ServiceAccountCoordinates{
+						Namespace: "test-namespace",
+						Name:      "test-sa-1",
+						UID:       "test-uid-1",
+					},
+				},
+				&fakedockerConfigProviderWithCoordinates{
+					dockerConfig: credentialprovider.DockerConfig{
+						"test.registry.io": credentialprovider.DockerConfigEntry{
+							Username: "user2",
+							Password: "pass2",
+						},
+					},
+					serviceAccountCoords: &credentialprovider.ServiceAccountCoordinates{
+						Namespace: "test-namespace",
+						Name:      "test-sa-2",
+						UID:       "test-uid-2",
+					},
+				},
+			},
+			expectedConfigCount:      2,
+			expectedFound:            true,
+			expectedServiceAccountOK: true,
+		},
+		{
+			name:  "provider with empty config",
+			image: "test.registry.io/image:tag",
+			providers: []dockerConfigProviderWithCoordinates{
+				&fakedockerConfigProviderWithCoordinates{
+					dockerConfig:         credentialprovider.DockerConfig{},
+					serviceAccountCoords: nil,
+				},
+			},
+			expectedConfigCount:      0,
+			expectedFound:            false,
+			expectedServiceAccountOK: false,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			keyring := &externalCredentialProviderKeyring{
+				providers: tc.providers,
+			}
+
+			configs, found := keyring.Lookup(tc.image)
+
+			if found != tc.expectedFound {
+				t.Errorf("Expected found=%v, got found=%v", tc.expectedFound, found)
+			}
+
+			if len(configs) != tc.expectedConfigCount {
+				t.Errorf("Expected %d configs, got %d", tc.expectedConfigCount, len(configs))
+			}
+
+			// Verify that each provider was called with the correct image
+			for i, provider := range tc.providers {
+				mockProvider := provider.(*fakedockerConfigProviderWithCoordinates)
+				if mockProvider.getCallCount() != 1 {
+					t.Errorf("Provider %d expected 1 call, got %d", i, mockProvider.getCallCount())
+				}
+				if mockProvider.getLastImageRequested() != tc.image {
+					t.Errorf("Provider %d expected image %s, got %s", i, tc.image, mockProvider.getLastImageRequested())
+				}
+			}
+
+			// Verify service account coordinates in TrackedAuthConfig
+			if tc.expectedServiceAccountOK && len(configs) > 0 {
+				foundServiceAccountCoords := false
+				for _, config := range configs {
+					if config.Source != nil && config.Source.ServiceAccount != nil {
+						foundServiceAccountCoords = true
+						break
+					}
+				}
+				if !foundServiceAccountCoords {
+					t.Errorf("Expected to find service account coordinates in TrackedAuthConfig")
+				}
+			}
+		})
+	}
+}
+
+func TestExternalCredentialProviderKeyringLookupConcurrency(t *testing.T) {
+	// Test concurrent access to the keyring
+	mockProvider := &fakedockerConfigProviderWithCoordinates{
+		dockerConfig: credentialprovider.DockerConfig{
+			"test.registry.io": credentialprovider.DockerConfigEntry{
+				Username: "user1",
+				Password: "pass1",
+			},
+		},
+		serviceAccountCoords: &credentialprovider.ServiceAccountCoordinates{
+			Namespace: "test-namespace",
+			Name:      "test-sa",
+			UID:       "test-uid",
+		},
+	}
+
+	keyring := &externalCredentialProviderKeyring{
+		providers: []dockerConfigProviderWithCoordinates{mockProvider},
+	}
+
+	const numGoroutines = 10
+	const numCallsPerGoroutine = 5
+
+	var wg sync.WaitGroup
+	var errorOccurred bool
+	var mu sync.Mutex
+
+	wg.Add(numGoroutines)
+
+	for i := range numGoroutines {
+		go func(goroutineID int) {
+			defer wg.Done()
+			for j := range numCallsPerGoroutine {
+				image := "test.registry.io/image:tag"
+				configs, found := keyring.Lookup(image)
+
+				if !found {
+					mu.Lock()
+					errorOccurred = true
+					t.Errorf("Goroutine %d call %d: Expected found=true, got found=false", goroutineID, j)
+					mu.Unlock()
+					return
+				}
+				if len(configs) != 1 {
+					mu.Lock()
+					errorOccurred = true
+					t.Errorf("Goroutine %d call %d: Expected 1 config, got %d", goroutineID, j, len(configs))
+					mu.Unlock()
+					return
+				}
+			}
+		}(i)
+	}
+
+	wg.Wait()
+
+	if !errorOccurred {
+		expectedTotalCalls := numGoroutines * numCallsPerGoroutine
+		actualCalls := mockProvider.getCallCount()
+		if actualCalls != expectedTotalCalls {
+			t.Errorf("Expected %d total calls, got %d", expectedTotalCalls, actualCalls)
+		}
+	}
+}

pkg/credentialprovider/secrets/secrets.go

@@ -49,7 +49,7 @@ func secretsToTrackedDockerConfigs(secrets []v1.Secret) (credentialprovider.Dock
 				return nil, err
 			}
 
-			coords := credentialprovider.SecretCoordinates{
+			coords := &credentialprovider.SecretCoordinates{
 				UID:       string(passedSecret.UID),
 				Namespace: passedSecret.Namespace,
 				Name:      passedSecret.Name}
@@ -62,7 +62,7 @@ func secretsToTrackedDockerConfigs(secrets []v1.Secret) (credentialprovider.Dock
 				return nil, err
 			}
 
-			coords := credentialprovider.SecretCoordinates{
+			coords := &credentialprovider.SecretCoordinates{
 				UID:       string(passedSecret.UID),
 				Namespace: passedSecret.Namespace,
 				Name:      passedSecret.Name}

pkg/credentialprovider/secrets/secrets_test.go

@@ -67,7 +67,7 @@ func Test_MakeDockerKeyring(t *testing.T) {
 			expectedAuthConfigs: []credentialprovider.TrackedAuthConfig{
 				{
 					Source: &credentialprovider.CredentialSource{
-						Secret: credentialprovider.SecretCoordinates{
+						Secret: &credentialprovider.SecretCoordinates{
 							Name: "s1", Namespace: "ns1", UID: "uid1"},
 					},
 					AuthConfig: credentialprovider.AuthConfig{
@@ -95,7 +95,7 @@ func Test_MakeDockerKeyring(t *testing.T) {
 			expectedAuthConfigs: []credentialprovider.TrackedAuthConfig{
 				{
 					Source: &credentialprovider.CredentialSource{
-						Secret: credentialprovider.SecretCoordinates{
+						Secret: &credentialprovider.SecretCoordinates{
 							Name: "s1", Namespace: "ns1", UID: "uid1"},
 					},
 					AuthConfig: credentialprovider.AuthConfig{
@@ -123,7 +123,7 @@ func Test_MakeDockerKeyring(t *testing.T) {
 			expectedAuthConfigs: []credentialprovider.TrackedAuthConfig{
 				{
 					Source: &credentialprovider.CredentialSource{
-						Secret: credentialprovider.SecretCoordinates{
+						Secret: &credentialprovider.SecretCoordinates{
 							Name: "s1", Namespace: "ns1", UID: "uid1"},
 					},
 					AuthConfig: credentialprovider.AuthConfig{
@@ -151,7 +151,7 @@ func Test_MakeDockerKeyring(t *testing.T) {
 			expectedAuthConfigs: []credentialprovider.TrackedAuthConfig{
 				{
 					Source: &credentialprovider.CredentialSource{
-						Secret: credentialprovider.SecretCoordinates{
+						Secret: &credentialprovider.SecretCoordinates{
 							Name: "s1", Namespace: "ns1", UID: "uid1"},
 					},
 					AuthConfig: credentialprovider.AuthConfig{
@@ -314,7 +314,7 @@ func Test_MakeDockerKeyring(t *testing.T) {
 			expectedAuthConfigs: []credentialprovider.TrackedAuthConfig{
 				{
 					Source: &credentialprovider.CredentialSource{
-						Secret: credentialprovider.SecretCoordinates{
+						Secret: &credentialprovider.SecretCoordinates{
 							Name: "s1", Namespace: "ns1", UID: "uid1"},
 					},
 					AuthConfig: credentialprovider.AuthConfig{