From 15b95beac77fe6ed1397ca46145bf8a7dee9438f Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 26 Mar 2025 14:25:50 +0100 Subject: [PATCH 1/3] cli/command: add unit-test for RetrieveAuthTokenFromImage It's currently slower because it calls registry.ParseRepositoryInfo, which does a DNS lookup for hostnames to determine if they're a loopback address (and marked "insecure"); go test -v -run TestRetrieveAuthTokenFromImage === RUN TestRetrieveAuthTokenFromImage === RUN TestRetrieveAuthTokenFromImage/no-prefix === RUN TestRetrieveAuthTokenFromImage/docker.io === RUN TestRetrieveAuthTokenFromImage/index.docker.io === RUN TestRetrieveAuthTokenFromImage/registry-1.docker.io === RUN TestRetrieveAuthTokenFromImage/registry.hub.docker.com === RUN TestRetrieveAuthTokenFromImage/[::1] === RUN TestRetrieveAuthTokenFromImage/[::1]:5000 === RUN TestRetrieveAuthTokenFromImage/127.0.0.1 === RUN TestRetrieveAuthTokenFromImage/localhost === RUN TestRetrieveAuthTokenFromImage/localhost:5000 === RUN TestRetrieveAuthTokenFromImage/no-auth.example.com --- PASS: TestRetrieveAuthTokenFromImage (0.35s) --- PASS: TestRetrieveAuthTokenFromImage/no-prefix (0.00s) --- PASS: TestRetrieveAuthTokenFromImage/docker.io (0.00s) --- PASS: TestRetrieveAuthTokenFromImage/index.docker.io (0.00s) --- PASS: TestRetrieveAuthTokenFromImage/registry-1.docker.io (0.08s) --- PASS: TestRetrieveAuthTokenFromImage/registry.hub.docker.com (0.12s) --- PASS: TestRetrieveAuthTokenFromImage/[::1] (0.13s) --- PASS: TestRetrieveAuthTokenFromImage/[::1]:5000 (0.00s) --- PASS: TestRetrieveAuthTokenFromImage/127.0.0.1 (0.00s) --- PASS: TestRetrieveAuthTokenFromImage/localhost (0.00s) --- PASS: TestRetrieveAuthTokenFromImage/localhost:5000 (0.00s) --- PASS: TestRetrieveAuthTokenFromImage/no-auth.example.com (0.01s) PASS ok github.com/docker/cli/cli/command 1.367s Signed-off-by: Sebastiaan van Stijn --- cli/command/registry_test.go | 112 +++++++++++++++++++++++++++++++++++ 1 file changed, 112 insertions(+) diff --git a/cli/command/registry_test.go b/cli/command/registry_test.go index 65acd069e2bb..4c6bb9f89abb 100644 --- a/cli/command/registry_test.go +++ b/cli/command/registry_test.go @@ -1,6 +1,8 @@ package command_test import ( + "bytes" + "path" "testing" "github.com/docker/cli/cli/command" @@ -80,3 +82,113 @@ func TestGetDefaultAuthConfig_HelperError(t *testing.T) { assert.Check(t, is.DeepEqual(expectedAuthConfig, authconfig)) assert.Check(t, is.ErrorContains(err, "docker-credential-fake-does-not-exist")) } + +func TestRetrieveAuthTokenFromImage(t *testing.T) { + // configFileContent contains a plain-text "username:password", as stored by + // the plain-text store; + // https://github.com/docker/cli/blob/v28.0.4/cli/config/configfile/file.go#L218-L229 + const configFileContent = `{"auths": { + "https://index.docker.io/v1/": {"auth": "dXNlcm5hbWU6cGFzc3dvcmQ="}, + "[::1]": {"auth": "dXNlcm5hbWU6cGFzc3dvcmQ="}, + "[::1]:5000": {"auth": "dXNlcm5hbWU6cGFzc3dvcmQ="}, + "127.0.0.1": {"auth": "dXNlcm5hbWU6cGFzc3dvcmQ="}, + "127.0.0.1:5000": {"auth": "dXNlcm5hbWU6cGFzc3dvcmQ="}, + "localhost": {"auth": "dXNlcm5hbWU6cGFzc3dvcmQ="}, + "localhost:5000": {"auth": "dXNlcm5hbWU6cGFzc3dvcmQ="}, + "registry-1.docker.io": {"auth": "dXNlcm5hbWU6cGFzc3dvcmQ="}, + "registry.hub.docker.com": {"auth": "dXNlcm5hbWU6cGFzc3dvcmQ="} + } +}` + cfg := configfile.ConfigFile{} + err := cfg.LoadFromReader(bytes.NewReader([]byte(configFileContent))) + assert.NilError(t, err) + + remoteRefs := []string{ + "ubuntu", + "ubuntu:latest", + "ubuntu:latest@sha256:72297848456d5d37d1262630108ab308d3e9ec7ed1c3286a32fe09856619a782", + "ubuntu@sha256:72297848456d5d37d1262630108ab308d3e9ec7ed1c3286a32fe09856619a782", + "library/ubuntu", + "library/ubuntu:latest", + "library/ubuntu:latest@sha256:72297848456d5d37d1262630108ab308d3e9ec7ed1c3286a32fe09856619a782", + "library/ubuntu@sha256:72297848456d5d37d1262630108ab308d3e9ec7ed1c3286a32fe09856619a782", + } + + tests := []struct { + prefix string + expectedAddress string + expectedAuthCfg registry.AuthConfig + }{ + { + prefix: "", + expectedAddress: "https://index.docker.io/v1/", + expectedAuthCfg: registry.AuthConfig{Username: "username", Password: "password", ServerAddress: "https://index.docker.io/v1/"}, + }, + { + prefix: "docker.io", + expectedAddress: "https://index.docker.io/v1/", + expectedAuthCfg: registry.AuthConfig{Username: "username", Password: "password", ServerAddress: "https://index.docker.io/v1/"}, + }, + { + prefix: "index.docker.io", + expectedAddress: "https://index.docker.io/v1/", + expectedAuthCfg: registry.AuthConfig{Username: "username", Password: "password", ServerAddress: "https://index.docker.io/v1/"}, + }, + { + // FIXME(thaJeztah): registry-1.docker.io (the actual registry) is the odd one out, and is stored separate from other URLs used for docker hub's registry + prefix: "registry-1.docker.io", + expectedAuthCfg: registry.AuthConfig{Username: "username", Password: "password", ServerAddress: "registry-1.docker.io"}, + }, + { + // FIXME(thaJeztah): registry.hub.docker.com is stored separate from other URLs used for docker hub's registry + prefix: "registry.hub.docker.com", + expectedAuthCfg: registry.AuthConfig{Username: "username", Password: "password", ServerAddress: "registry.hub.docker.com"}, + }, + { + prefix: "[::1]", + expectedAddress: "[::1]", + expectedAuthCfg: registry.AuthConfig{Username: "username", Password: "password", ServerAddress: "[::1]"}, + }, + { + prefix: "[::1]:5000", + expectedAddress: "[::1]:5000", + expectedAuthCfg: registry.AuthConfig{Username: "username", Password: "password", ServerAddress: "[::1]:5000"}, + }, + { + prefix: "127.0.0.1", + expectedAddress: "127.0.0.1", + expectedAuthCfg: registry.AuthConfig{Username: "username", Password: "password", ServerAddress: "127.0.0.1"}, + }, + { + prefix: "localhost", + expectedAddress: "localhost", + expectedAuthCfg: registry.AuthConfig{Username: "username", Password: "password", ServerAddress: "localhost"}, + }, + { + prefix: "localhost:5000", + expectedAddress: "localhost:5000", + expectedAuthCfg: registry.AuthConfig{Username: "username", Password: "password", ServerAddress: "localhost:5000"}, + }, + { + prefix: "no-auth.example.com", + expectedAuthCfg: registry.AuthConfig{}, + }, + } + + for _, tc := range tests { + tcName := tc.prefix + if tc.prefix == "" { + tcName = "no-prefix" + } + t.Run(tcName, func(t *testing.T) { + for _, remoteRef := range remoteRefs { + imageRef := path.Join(tc.prefix, remoteRef) + actual, err := command.RetrieveAuthTokenFromImage(&cfg, imageRef) + assert.NilError(t, err) + ac, err := registry.DecodeAuthConfig(actual) + assert.NilError(t, err) + assert.Check(t, is.DeepEqual(*ac, tc.expectedAuthCfg)) + } + }) + } +} From 9f4165ccb8873826f3f7bdc09afee1056546d294 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 26 Mar 2025 11:42:25 +0100 Subject: [PATCH 2/3] Reapply "cli/command: remove uses of GetAuthConfigKey, ParseRepositoryInfo" This reverts commit f5962021251c127326d654a440c8b6b675e2ba0d, and reapplies 79141ce5eb8f7c341b03aa50f7ce4b819de20720. > cli/command: remove uses of GetAuthConfigKey, ParseRepositoryInfo > > Re-implement locally, based on the code in github.com/docker/docker/registry, > but leaving out bits that are not used on the client-side, such as > configuration of Mirrors, and configurable insecure-registry, which > are not used on the client side. This commit contains a regression due to a typo in `authConfigKey`; const authConfigKey = "https:/index.docker.io/v1/" Which is missing a `/` after the scheme. Which currently fails the TestRetrieveAuthTokenFromImage test. Signed-off-by: Sebastiaan van Stijn --- cli/command/registry.go | 41 +++++++++++++++++++++++++++++++---------- 1 file changed, 31 insertions(+), 10 deletions(-) diff --git a/cli/command/registry.go b/cli/command/registry.go index e2581d574f93..cf7243aabe5d 100644 --- a/cli/command/registry.go +++ b/cli/command/registry.go @@ -15,7 +15,6 @@ import ( "github.com/docker/cli/cli/streams" "github.com/docker/cli/internal/tui" registrytypes "github.com/docker/docker/api/types/registry" - "github.com/docker/docker/registry" "github.com/morikuni/aec" "github.com/pkg/errors" ) @@ -28,16 +27,22 @@ const ( "for organizations using SSO. Learn more at https://docs.docker.com/go/access-tokens/" ) +// authConfigKey is the key used to store credentials for Docker Hub. It is +// a copy of [registry.IndexServer]. +// +// [registry.IndexServer]: https://pkg.go.dev/github.com/docker/docker/registry#IndexServer +const authConfigKey = "https:/index.docker.io/v1/" + // RegistryAuthenticationPrivilegedFunc returns a RequestPrivilegeFunc from the specified registry index info // for the given command. func RegistryAuthenticationPrivilegedFunc(cli Cli, index *registrytypes.IndexInfo, cmdName string) registrytypes.RequestAuthConfig { + configKey := getAuthConfigKey(index.Name) + isDefaultRegistry := configKey == authConfigKey || index.Official return func(ctx context.Context) (string, error) { _, _ = fmt.Fprintf(cli.Out(), "\nLogin prior to %s:\n", cmdName) - indexServer := registry.GetAuthConfigKey(index) - isDefaultRegistry := indexServer == registry.IndexServer - authConfig, err := GetDefaultAuthConfig(cli.ConfigFile(), true, indexServer, isDefaultRegistry) + authConfig, err := GetDefaultAuthConfig(cli.ConfigFile(), true, configKey, isDefaultRegistry) if err != nil { - _, _ = fmt.Fprintf(cli.Err(), "Unable to retrieve stored credentials for %s, error: %s.\n", indexServer, err) + _, _ = fmt.Fprintf(cli.Err(), "Unable to retrieve stored credentials for %s, error: %s.\n", authConfigKey, err) } select { @@ -46,7 +51,7 @@ func RegistryAuthenticationPrivilegedFunc(cli Cli, index *registrytypes.IndexInf default: } - authConfig, err = PromptUserForCredentials(ctx, cli, "", "", authConfig.Username, indexServer) + authConfig, err = PromptUserForCredentials(ctx, cli, "", "", authConfig.Username, authConfigKey) if err != nil { return "", err } @@ -63,7 +68,7 @@ func RegistryAuthenticationPrivilegedFunc(cli Cli, index *registrytypes.IndexInf func ResolveAuthConfig(cfg *configfile.ConfigFile, index *registrytypes.IndexInfo) registrytypes.AuthConfig { configKey := index.Name if index.Official { - configKey = registry.IndexServer + configKey = authConfigKey } a, _ := cfg.GetAuthConfig(configKey) @@ -132,7 +137,7 @@ func PromptUserForCredentials(ctx context.Context, cli Cli, argUser, argPassword argUser = strings.TrimSpace(argUser) if argUser == "" { - if serverAddress == registry.IndexServer { + if serverAddress == authConfigKey { // When signing in to the default (Docker Hub) registry, we display // hints for creating an account, and (if hints are enabled), using // a token instead of a password. @@ -225,9 +230,25 @@ func resolveAuthConfigFromImage(cfg *configfile.ConfigFile, image string) (regis if err != nil { return registrytypes.AuthConfig{}, err } - repoInfo, err := registry.ParseRepositoryInfo(registryRef) + configKey := getAuthConfigKey(reference.Domain(registryRef)) + a, err := cfg.GetAuthConfig(configKey) if err != nil { return registrytypes.AuthConfig{}, err } - return ResolveAuthConfig(cfg, repoInfo.Index), nil + return registrytypes.AuthConfig(a), nil +} + +// getAuthConfigKey special-cases using the full index address of the official +// index as the AuthConfig key, and uses the (host)name[:port] for private indexes. +// +// It is similar to [registry.GetAuthConfigKey], but does not require on +// [registrytypes.IndexInfo] as intermediate. +// +// [registry.GetAuthConfigKey]: https://pkg.go.dev/github.com/docker/docker/registry#GetAuthConfigKey +// [registrytypes.IndexInfo]:https://pkg.go.dev/github.com/docker/docker/api/types/registry#IndexInfo +func getAuthConfigKey(domainName string) string { + if domainName == "docker.io" || domainName == "index.docker.io" { + return authConfigKey + } + return domainName } From 0e32baf1153ed299d2387a5ccbd3308bba15f4f3 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 26 Mar 2025 14:43:57 +0100 Subject: [PATCH 3/3] cli/command: fix regression in resolving auth from config MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This was introduced in 79141ce5eb8f7c341b03aa50f7ce4b819de20720, which was reverted in f5962021251c127326d654a440c8b6b675e2ba0d, and re-applied in the previous commit. Before this patch, saving credentials worked correctly; docker login -u thajeztah Password: Login Succeeded cat ~/.docker/config.json { "auths": { "https://index.docker.io/v1/": { "auth": "REDACTED" } } } But when resolving the credentials, the credentials stored would not be found; docker pull -q thajeztah/private-test-image Error response from daemon: pull access denied for thajeztah/private-test-image, repository does not exist or may require 'docker login': denied: requested access to the resource is denied With this patch applied: docker pull -q thajeztah/private-test-image docker.io/thajeztah/private-test-image:latest Thanks to mtrmac (Miloslav Trmač) for spotting this mistake! Suggested-by: Miloslav Trmač Signed-off-by: Sebastiaan van Stijn --- cli/command/registry.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/command/registry.go b/cli/command/registry.go index cf7243aabe5d..acf0c7e63553 100644 --- a/cli/command/registry.go +++ b/cli/command/registry.go @@ -31,7 +31,7 @@ const ( // a copy of [registry.IndexServer]. // // [registry.IndexServer]: https://pkg.go.dev/github.com/docker/docker/registry#IndexServer -const authConfigKey = "https:/index.docker.io/v1/" +const authConfigKey = "https://index.docker.io/v1/" // RegistryAuthenticationPrivilegedFunc returns a RequestPrivilegeFunc from the specified registry index info // for the given command.