From 7582e8f0572a5bd2171ea55095b8ebc013adea34 Mon Sep 17 00:00:00 2001 From: maxpetrusenkoagent Date: Mon, 15 Jun 2026 07:00:56 -0400 Subject: [PATCH] fix: prefer docker login credentials in GitLab CI Signed-off-by: maxpetrusenkoagent --- cli/command/registry/warning.go | 2 +- cli/command/registry/warning_test.go | 33 ++++++++++++++++++ cli/config/configfile/file.go | 13 +++++--- cli/config/configfile/file_test.go | 35 +++++++++++++++++++ cli/config/memorystore/store.go | 43 ++++++++++++++++++++++-- cli/config/memorystore/store_test.go | 50 ++++++++++++++++++++++++++++ 6 files changed, 169 insertions(+), 7 deletions(-) create mode 100644 cli/command/registry/warning_test.go diff --git a/cli/command/registry/warning.go b/cli/command/registry/warning.go index 22a8c8655be6..f97f498fc2be 100644 --- a/cli/command/registry/warning.go +++ b/cli/command/registry/warning.go @@ -11,7 +11,7 @@ import ( // maybePrintEnvAuthWarning if the `DOCKER_AUTH_CONFIG` environment variable is // set this function will output a warning to stdErr func maybePrintEnvAuthWarning(out command.Streams) { - if os.Getenv(configfile.DockerEnvConfigKey) != "" { + if os.Getenv(configfile.DockerEnvConfigKey) != "" && os.Getenv("GITLAB_CI") != "true" { tui.NewOutput(out.Err()). PrintWarning("%[1]s is set and takes precedence.\nUnset %[1]s to restore the CLI auth behaviour.\n", configfile.DockerEnvConfigKey) } diff --git a/cli/command/registry/warning_test.go b/cli/command/registry/warning_test.go new file mode 100644 index 000000000000..21a36b5cbb5d --- /dev/null +++ b/cli/command/registry/warning_test.go @@ -0,0 +1,33 @@ +package registry + +import ( + "testing" + + "github.com/docker/cli/cli/config/configfile" + "github.com/docker/cli/internal/test" + "gotest.tools/v3/assert" + is "gotest.tools/v3/assert/cmp" +) + +const envAuthConfig = `{"auths":{"env.example.test":{"auth":"ZW52X3VzZXI6ZW52X3Bhc3M="}}}` + +func TestMaybePrintEnvAuthWarning(t *testing.T) { + t.Run("warns when environment credentials take precedence", func(t *testing.T) { + cli := test.NewFakeCli(&fakeClient{}) + t.Setenv(configfile.DockerEnvConfigKey, envAuthConfig) + + maybePrintEnvAuthWarning(cli) + + assert.Check(t, is.Contains(cli.ErrBuffer().String(), "DOCKER_AUTH_CONFIG is set and takes precedence")) + }) + + t.Run("does not warn in GitLab CI", func(t *testing.T) { + cli := test.NewFakeCli(&fakeClient{}) + t.Setenv(configfile.DockerEnvConfigKey, envAuthConfig) + t.Setenv("GITLAB_CI", "true") + + maybePrintEnvAuthWarning(cli) + + assert.Check(t, is.Equal(cli.ErrBuffer().String(), "")) + }) +} diff --git a/cli/config/configfile/file.go b/cli/config/configfile/file.go index 26e148f05987..e7fff10f17d8 100644 --- a/cli/config/configfile/file.go +++ b/cli/config/configfile/file.go @@ -336,12 +336,17 @@ func (c *ConfigFile) GetCredentialsStore(registryHostname string) credentials.St return store } - // use DOCKER_AUTH_CONFIG if set - // it uses the native or file store as a fallback to fetch and store credentials - envStore, err := memorystore.New( + storeOptions := []memorystore.Options{ memorystore.WithAuthConfig(authConfig), memorystore.WithFallbackStore(store), - ) + } + if os.Getenv("GITLAB_CI") == "true" { + storeOptions = append(storeOptions, memorystore.WithPreferFallback()) + } + + // use DOCKER_AUTH_CONFIG if set + // it uses the native or file store as a fallback to fetch and store credentials + envStore, err := memorystore.New(storeOptions...) if err != nil { _, _ = fmt.Fprintln(os.Stderr, "Failed to create credential store from DOCKER_AUTH_CONFIG: ", err) return store diff --git a/cli/config/configfile/file_test.go b/cli/config/configfile/file_test.go index 92df02c74352..1dae9a74d5ba 100644 --- a/cli/config/configfile/file_test.go +++ b/cli/config/configfile/file_test.go @@ -584,6 +584,41 @@ func TestGetAllCredentialsFromEnvironment(t *testing.T) { }) } +func TestGetAuthConfigPrefersConfigFileInGitLabCI(t *testing.T) { + configFile := New("filename") + configFile.AuthConfigs["env.example.test"] = types.AuthConfig{ + Username: "login_user", + Password: "login_pass", + ServerAddress: "env.example.test", + } + + t.Setenv("DOCKER_AUTH_CONFIG", envTestAuthConfig) + t.Setenv("GITLAB_CI", "true") + + authConfig, err := configFile.GetAuthConfig("env.example.test") + assert.NilError(t, err) + assert.Check(t, is.DeepEqual(authConfig, types.AuthConfig{ + Username: "login_user", + Password: "login_pass", + ServerAddress: "env.example.test", + })) +} + +func TestGetAuthConfigUsesEnvironmentInGitLabCIWhenConfigFileHasNoCredentials(t *testing.T) { + configFile := New("filename") + + t.Setenv("DOCKER_AUTH_CONFIG", envTestAuthConfig) + t.Setenv("GITLAB_CI", "true") + + authConfig, err := configFile.GetAuthConfig("env.example.test") + assert.NilError(t, err) + assert.Check(t, is.DeepEqual(authConfig, types.AuthConfig{ + Username: "env_user", + Password: "env_pass", + ServerAddress: "env.example.test", + })) +} + func TestParseEnvConfig(t *testing.T) { t.Run("should error on unexpected fields", func(t *testing.T) { _, err := parseEnvConfig(envTestUserPassConfig) diff --git a/cli/config/memorystore/store.go b/cli/config/memorystore/store.go index 44523d392ba8..e8da2c9fce05 100644 --- a/cli/config/memorystore/store.go +++ b/cli/config/memorystore/store.go @@ -28,6 +28,7 @@ type Config struct { lock sync.RWMutex memoryCredentials map[string]types.AuthConfig fallbackStore credentials.Store + preferFallback bool } func (e *Config) Erase(serverAddress string) error { @@ -48,6 +49,12 @@ func (e *Config) Erase(serverAddress string) error { func (e *Config) Get(serverAddress string) (types.AuthConfig, error) { e.lock.RLock() defer e.lock.RUnlock() + if e.preferFallback && e.fallbackStore != nil { + if authConfig, err := e.fallbackStore.Get(serverAddress); err == nil && hasAuthConfig(authConfig) { + return authConfig, nil + } + } + authConfig, ok := e.memoryCredentials[serverAddress] if !ok { if e.fallbackStore != nil { @@ -63,16 +70,22 @@ func (e *Config) GetAll() (map[string]types.AuthConfig, error) { defer e.lock.RUnlock() creds := make(map[string]types.AuthConfig) + if e.preferFallback { + maps.Copy(creds, e.memoryCredentials) + } + if e.fallbackStore != nil { fileCredentials, err := e.fallbackStore.GetAll() if err != nil { _, _ = fmt.Fprintln(os.Stderr, "memorystore: ", err) } else { - creds = fileCredentials + copyAuthConfigs(creds, fileCredentials, e.preferFallback) } } - maps.Copy(creds, e.memoryCredentials) + if !e.preferFallback { + maps.Copy(creds, e.memoryCredentials) + } return creds, nil } @@ -87,6 +100,23 @@ func (e *Config) Store(authConfig types.AuthConfig) error { return nil } +func hasAuthConfig(authConfig types.AuthConfig) bool { + return authConfig.Username != "" || + authConfig.Password != "" || + authConfig.Auth != "" || + authConfig.IdentityToken != "" || + authConfig.RegistryToken != "" +} + +func copyAuthConfigs(dst, src map[string]types.AuthConfig, skipEmpty bool) { + for serverAddress, authConfig := range src { + if skipEmpty && !hasAuthConfig(authConfig) { + continue + } + dst[serverAddress] = authConfig + } +} + // WithFallbackStore sets a fallback store. // // Write operations will be performed on both the memory store and the @@ -107,6 +137,15 @@ func WithFallbackStore(store credentials.Store) Options { } } +// WithPreferFallback configures the store to prefer reading credentials from +// the fallback store. Write operations are still performed on both stores. +func WithPreferFallback() Options { + return func(s *Config) error { + s.preferFallback = true + return nil + } +} + // WithAuthConfig allows to set the initial credentials in the memory store. func WithAuthConfig(config map[string]types.AuthConfig) Options { return func(s *Config) error { diff --git a/cli/config/memorystore/store_test.go b/cli/config/memorystore/store_test.go index a92a170a3b6b..ad1a3fb98e4d 100644 --- a/cli/config/memorystore/store_test.go +++ b/cli/config/memorystore/store_test.go @@ -129,3 +129,53 @@ func TestMemoryStoreWithoutFallback(t *testing.T) { assert.Check(t, is.ErrorIs(err, errValueNotFound)) }) } + +func TestMemoryStoreWithPreferFallback(t *testing.T) { + config := map[string]types.AuthConfig{ + "https://example.test": { + Username: "memory-user", + ServerAddress: "https://example.test", + }, + "https://only-in-memory.example.test": { + Username: "memory-only-user", + ServerAddress: "https://only-in-memory.example.test", + }, + } + + fallbackConfig := map[string]types.AuthConfig{ + "https://example.test": { + Username: "fallback-user", + ServerAddress: "https://example.test", + }, + "https://empty-fallback.example.test": { + ServerAddress: "https://empty-fallback.example.test", + }, + } + + fallbackStore, err := New(WithAuthConfig(fallbackConfig)) + assert.NilError(t, err) + + memoryStore, err := New(WithAuthConfig(config), WithFallbackStore(fallbackStore), WithPreferFallback()) + assert.NilError(t, err) + + t.Run("get credentials from fallback store first", func(t *testing.T) { + c, err := memoryStore.Get("https://example.test") + assert.NilError(t, err) + assert.Equal(t, c, fallbackConfig["https://example.test"]) + }) + + t.Run("get credentials from memory store when fallback has no credentials", func(t *testing.T) { + c, err := memoryStore.Get("https://only-in-memory.example.test") + assert.NilError(t, err) + assert.Equal(t, c, config["https://only-in-memory.example.test"]) + }) + + t.Run("get all credentials prefers fallback credentials", func(t *testing.T) { + c, err := memoryStore.GetAll() + assert.NilError(t, err) + assert.Check(t, is.DeepEqual(c, map[string]types.AuthConfig{ + "https://example.test": fallbackConfig["https://example.test"], + "https://only-in-memory.example.test": config["https://only-in-memory.example.test"], + })) + }) +}