dockerconfig: fix credential refresh for expired auth cache entries

util/dockerutil/dockerconfig/configprovider.go

@@ -42,12 +42,14 @@ type authConfigProvider struct {
 	alternativeConfigs []*alternativeConfig
 }
 
+func defaultCacheExpireCheck(created time.Time, _ string) bool {
+	// Tokens for Google Artifact Registry via Workload Identity expire after 5 minutes.
+	return time.Since(created) > 4*time.Minute+50*time.Second
+}
+
 func (ap *authConfigProvider) load(ctx context.Context, host string, scopes []string, cacheExpireCheck authprovider.ExpireCachedAuthCheck) (types.AuthConfig, error) {
 	if cacheExpireCheck == nil {
-		cacheExpireCheck = func(created time.Time, _ string) bool {
-			// Tokens for Google Artifact Registry via Workload Identity expire after 5 minutes.
-			return time.Since(created) > 4*time.Minute+50*time.Second
-		}
+		cacheExpireCheck = defaultCacheExpireCheck
 	}
 
 	ac, err := ap.loadHost(ctx, host, scopes, cacheExpireCheck)
@@ -78,6 +80,10 @@ func (ap *authConfigProvider) loadHost(_ context.Context, host string, scopes []
 	ap.mu.Lock()
 	defer ap.mu.Unlock()
 
+	if cacheExpireCheck == nil {
+		cacheExpireCheck = defaultCacheExpireCheck
+	}
+
 	candidates := []*alternativeConfig{}
 	parsedScopes := parseScopes(scopes)
 
@@ -115,8 +121,11 @@ func (ap *authConfigProvider) loadHost(_ context.Context, host string, scopes []
 	}
 
 	entry, exists := ap.authConfigCache[key]
-	if exists && (cacheExpireCheck == nil || !cacheExpireCheck(entry.Created, key)) {
-		return *entry.Auth, nil
+	if exists {
+		if !cacheExpireCheck(entry.Created, key) {
+			return *entry.Auth, nil
+		}
+		delete(ap.authConfigCache, key)
 	}
 
 	hostKey := host

util/dockerutil/dockerconfig/configprovider_test.go

@@ -3,6 +3,11 @@ package dockerconfig
 import (
 	"testing"
 
+	"context"
+	"time"
+
+	"github.com/docker/cli/cli/config/configfile"
+	"github.com/docker/cli/cli/config/types"
 	"github.com/stretchr/testify/require"
 )
 
@@ -198,3 +203,145 @@ func TestMatchesScopes(t *testing.T) {
 		})
 	}
 }
+
+func TestAuthConfigCache_Hit(t *testing.T) {
+	cfg := configfile.New("test")
+	cfg.AuthConfigs["europe-west1-docker.pkg.dev"] = types.AuthConfig{
+		Username: "_dcgcloud_token",
+		Password: "token-1",
+	}
+
+	ap := &authConfigProvider{
+		defaultConfig:   cfg,
+		authConfigCache: map[string]authConfigCacheEntry{},
+	}
+	ap.initOnce.Do(func() {})
+
+	ctx := context.Background()
+	ac, err := ap.load(ctx, "europe-west1-docker.pkg.dev", nil, func(created time.Time, serverURL string) bool {
+		return false
+	})
+	require.NoError(t, err)
+	require.Equal(t, "token-1", ac.Password)
+
+	// Update config in underlying store
+	cfg.AuthConfigs["europe-west1-docker.pkg.dev"] = types.AuthConfig{
+		Username: "_dcgcloud_token",
+		Password: "token-2",
+	}
+
+	// Should still hit cache and return token-1
+	ac, err = ap.load(ctx, "europe-west1-docker.pkg.dev", nil, func(created time.Time, serverURL string) bool {
+		return false
+	})
+	require.NoError(t, err)
+	require.Equal(t, "token-1", ac.Password)
+}
+
+func TestAuthConfigCache_Expired(t *testing.T) {
+	cfg := configfile.New("test")
+	cfg.AuthConfigs["europe-west1-docker.pkg.dev"] = types.AuthConfig{
+		Username: "_dcgcloud_token",
+		Password: "token-1",
+	}
+
+	ap := &authConfigProvider{
+		defaultConfig:   cfg,
+		authConfigCache: map[string]authConfigCacheEntry{},
+	}
+	ap.initOnce.Do(func() {})
+
+	ctx := context.Background()
+	ac, err := ap.load(ctx, "europe-west1-docker.pkg.dev", nil, nil)
+	require.NoError(t, err)
+	require.Equal(t, "token-1", ac.Password)
+
+	// Update config in underlying store
+	cfg.AuthConfigs["europe-west1-docker.pkg.dev"] = types.AuthConfig{
+		Username: "_dcgcloud_token",
+		Password: "token-2",
+	}
+
+	// Force expire check to return true
+	ac, err = ap.load(ctx, "europe-west1-docker.pkg.dev", nil, func(created time.Time, serverURL string) bool {
+		return true
+	})
+	require.NoError(t, err)
+	require.Equal(t, "token-2", ac.Password)
+}
+
+func TestAuthConfigCache_DefaultExpireCheck_Nil(t *testing.T) {
+	cfg := configfile.New("test")
+	cfg.AuthConfigs["europe-west1-docker.pkg.dev"] = types.AuthConfig{
+		Username: "_dcgcloud_token",
+		Password: "token-1",
+	}
+
+	ap := &authConfigProvider{
+		defaultConfig:   cfg,
+		authConfigCache: map[string]authConfigCacheEntry{},
+	}
+	ap.initOnce.Do(func() {})
+
+	ctx := context.Background()
+	// Initial load
+	ac, err := ap.load(ctx, "europe-west1-docker.pkg.dev", nil, nil)
+	require.NoError(t, err)
+	require.Equal(t, "token-1", ac.Password)
+
+	// Change underlying config
+	cfg.AuthConfigs["europe-west1-docker.pkg.dev"] = types.AuthConfig{
+		Username: "_dcgcloud_token",
+		Password: "token-2",
+	}
+
+	// Recent cache entry should not expire with nil check
+	ac, err = ap.load(ctx, "europe-west1-docker.pkg.dev", nil, nil)
+	require.NoError(t, err)
+	require.Equal(t, "token-1", ac.Password)
+
+	// Age the cached entry past 4m50s
+	entry := ap.authConfigCache["europe-west1-docker.pkg.dev"]
+	entry.Created = time.Now().Add(-5 * time.Minute)
+	ap.authConfigCache["europe-west1-docker.pkg.dev"] = entry
+
+	// Should expire with default check and reload token-2
+	ac, err = ap.load(ctx, "europe-west1-docker.pkg.dev", nil, nil)
+	require.NoError(t, err)
+	require.Equal(t, "token-2", ac.Password)
+}
+
+func TestAuthConfigCache_LoadHost_DirectNilCheck(t *testing.T) {
+	cfg := configfile.New("test")
+	cfg.AuthConfigs["europe-west1-docker.pkg.dev"] = types.AuthConfig{
+		Username: "_dcgcloud_token",
+		Password: "token-1",
+	}
+
+	ap := &authConfigProvider{
+		defaultConfig:   cfg,
+		authConfigCache: map[string]authConfigCacheEntry{},
+	}
+	ap.initOnce.Do(func() {})
+
+	ctx := context.Background()
+	// Direct call to loadHost with nil cacheExpireCheck
+	ac, err := ap.loadHost(ctx, "europe-west1-docker.pkg.dev", nil, nil)
+	require.NoError(t, err)
+	require.Equal(t, "token-1", ac.Password)
+
+	cfg.AuthConfigs["europe-west1-docker.pkg.dev"] = types.AuthConfig{
+		Username: "_dcgcloud_token",
+		Password: "token-2",
+	}
+
+	// Age the entry
+	entry := ap.authConfigCache["europe-west1-docker.pkg.dev"]
+	entry.Created = time.Now().Add(-5 * time.Minute)
+	ap.authConfigCache["europe-west1-docker.pkg.dev"] = entry
+
+	// loadHost directly with nil should expire and reload
+	ac, err = ap.loadHost(ctx, "europe-west1-docker.pkg.dev", nil, nil)
+	require.NoError(t, err)
+	require.Equal(t, "token-2", ac.Password)
+}