From a195d24d1f5369051bf344dabe31fd1e3ee54e53 Mon Sep 17 00:00:00 2001 From: Alano Terblanche <18033717+Benehiko@users.noreply.github.com> Date: Fri, 23 Oct 2026 14:53:47 +0200 Subject: [PATCH] cli/command: look up registry credentials through internal/registryauth RetrieveAuthTokenFromImage is exported and imported by other projects, so it keeps reading only the configuration file. The commands that pull and push call registryauth.EncodedAuth instead, which asks DOCKER_AUTH_CONFIG, then the Docker Desktop session, then the configuration file. This drops command.EncodedRegistryAuth from the public API. Each DockerCli owns its Docker Desktop sessions through RegistryAuthProvider, created on first use, instead of package-level sessions shared by every caller. A command.Cli without RegistryAuthProvider uses only its configuration file. The fallback reads the current configuration file, since Initialize replaces it. Remove the session fields and the constructors that only tests used, move the staging and dhi.io hostnames into internal/secretsengine, and use testing/synctest instead of a fake clock, so that the tests run in parallel. Only the DOCKER_AUTH_CONFIG test still sets the environment. Signed-off-by: Alano Terblanche <18033717+Benehiko@users.noreply.github.com> --- cli/command/cli.go | 19 ++ cli/command/cli_test.go | 15 ++ cli/command/container/create.go | 3 +- cli/command/image/pull.go | 3 +- cli/command/image/push.go | 3 +- cli/command/plugin/install.go | 3 +- cli/command/plugin/push.go | 3 +- cli/command/registry.go | 34 +-- cli/command/registry_internal_test.go | 156 ----------- cli/command/registry_test.go | 142 +++++++--- cli/command/service/create.go | 3 +- cli/command/service/update.go | 3 +- cli/command/stack/deploy_composefile.go | 3 +- internal/registry/config.go | 2 - internal/registryauth/registryauth.go | 62 +++++ internal/registryauth/registryauth_test.go | 131 ++++++++++ internal/secretsengine/secretsengine.go | 63 ++--- internal/secretsengine/secretsengine_test.go | 257 ++++++++++--------- 18 files changed, 515 insertions(+), 390 deletions(-) delete mode 100644 cli/command/registry_internal_test.go create mode 100644 internal/registryauth/registryauth.go create mode 100644 internal/registryauth/registryauth_test.go diff --git a/cli/command/cli.go b/cli/command/cli.go index 0d2f4b506f..308cbbd18b 100644 --- a/cli/command/cli.go +++ b/cli/command/cli.go @@ -16,6 +16,7 @@ import ( "github.com/docker/cli/cli/config" "github.com/docker/cli/cli/config/configfile" + configtypes "github.com/docker/cli/cli/config/types" dcontext "github.com/docker/cli/cli/context" "github.com/docker/cli/cli/context/docker" "github.com/docker/cli/cli/context/store" @@ -23,6 +24,7 @@ import ( cliflags "github.com/docker/cli/cli/flags" "github.com/docker/cli/cli/streams" "github.com/docker/cli/cli/version" + "github.com/docker/cli/internal/secretsengine" dopts "github.com/docker/cli/opts" "github.com/moby/moby/api/types/build" "github.com/moby/moby/client" @@ -75,6 +77,8 @@ type DockerCli struct { contextStoreConfig *store.Config initTimeout time.Duration res telemetryResource + registryAuthOnce sync.Once + registryAuth secretsengine.AuthProvider // baseCtx is the base context used for internal operations. In the future // this may be replaced by explicitly passing a context to functions that @@ -141,6 +145,21 @@ func (cli *DockerCli) ConfigFile() *configfile.ConfigFile { return cli.configFile } +func (cli *DockerCli) RegistryAuthProvider() secretsengine.AuthProvider { + cli.registryAuthOnce.Do(func() { + cli.registryAuth = secretsengine.NewDesktop(configFileAuth{cli}) + }) + return cli.registryAuth +} + +type configFileAuth struct { + cli *DockerCli +} + +func (c configFileAuth) GetAuthConfig(registryHostname string) (configtypes.AuthConfig, error) { + return c.cli.ConfigFile().GetAuthConfig(registryHostname) +} + // ServerInfo returns the server version details for the host this client is // connected to func (cli *DockerCli) ServerInfo() ServerInfo { diff --git a/cli/command/cli_test.go b/cli/command/cli_test.go index 09c093d5dd..fc0413f50d 100644 --- a/cli/command/cli_test.go +++ b/cli/command/cli_test.go @@ -18,6 +18,7 @@ import ( "github.com/docker/cli/cli/config" "github.com/docker/cli/cli/config/configfile" + configtypes "github.com/docker/cli/cli/config/types" "github.com/docker/cli/cli/context/store" "github.com/docker/cli/cli/flags" "github.com/moby/moby/client" @@ -501,3 +502,17 @@ func TestNewDockerCliWithAPIClientOptions(t *testing.T) { assert.NilError(t, err) assert.DeepEqual(t, received, "fake-agent/0.0.1") } + +func TestRegistryAuthProvider(t *testing.T) { + t.Parallel() + cli := &DockerCli{configFile: &configfile.ConfigFile{}} + p := cli.RegistryAuthProvider() + assert.Assert(t, p == cli.RegistryAuthProvider(), "the provider must be created once per DockerCli") + + cli.configFile = &configfile.ConfigFile{AuthConfigs: map[string]configtypes.AuthConfig{ + "ghcr.io": {Username: "user", Password: "pass"}, + }} + got, err := p.GetAuthConfig("ghcr.io") + assert.NilError(t, err) + assert.DeepEqual(t, got, configtypes.AuthConfig{Username: "user", Password: "pass"}) +} diff --git a/cli/command/container/create.go b/cli/command/container/create.go index 13e7dde4d4..14ab92d573 100644 --- a/cli/command/container/create.go +++ b/cli/command/container/create.go @@ -21,6 +21,7 @@ import ( "github.com/docker/cli/cli/config/types" "github.com/docker/cli/cli/streams" "github.com/docker/cli/internal/jsonstream" + "github.com/docker/cli/internal/registryauth" "github.com/docker/cli/opts" "github.com/moby/moby/api/types/mount" "github.com/moby/moby/client" @@ -133,7 +134,7 @@ func runCreate(ctx context.Context, dockerCLI command.Cli, flags *pflag.FlagSet, } func pullImage(ctx context.Context, dockerCLI command.Cli, img string, options *createOptions) error { - encodedAuth, err := command.EncodedRegistryAuth(dockerCLI, img) + encodedAuth, err := registryauth.EncodedAuth(dockerCLI, img) if err != nil { return err } diff --git a/cli/command/image/pull.go b/cli/command/image/pull.go index fdbe0814c8..27aca9e75a 100644 --- a/cli/command/image/pull.go +++ b/cli/command/image/pull.go @@ -14,6 +14,7 @@ import ( "github.com/docker/cli/cli/command/completion" "github.com/docker/cli/cli/streams" "github.com/docker/cli/internal/jsonstream" + "github.com/docker/cli/internal/registryauth" "github.com/moby/moby/client" ocispec "github.com/opencontainers/image-spec/specs-go/v1" "github.com/spf13/cobra" @@ -90,7 +91,7 @@ func runPull(ctx context.Context, dockerCLI command.Cli, opts pullOptions) error ociPlatforms = append(ociPlatforms, p) } - encodedAuth, err := command.EncodedRegistryAuth(dockerCLI, distributionRef.String()) + encodedAuth, err := registryauth.EncodedAuth(dockerCLI, distributionRef.String()) if err != nil { return err } diff --git a/cli/command/image/push.go b/cli/command/image/push.go index 6a33a91eb5..fdd1f52f47 100644 --- a/cli/command/image/push.go +++ b/cli/command/image/push.go @@ -17,6 +17,7 @@ import ( "github.com/docker/cli/cli/command/completion" "github.com/docker/cli/cli/streams" "github.com/docker/cli/internal/jsonstream" + "github.com/docker/cli/internal/registryauth" "github.com/docker/cli/internal/tui" "github.com/moby/moby/api/types/auxprogress" "github.com/moby/moby/client" @@ -108,7 +109,7 @@ To push the complete multi-platform image, remove the --platform flag. } // Resolve the Auth config relevant for this server - encodedAuth, err := command.EncodedRegistryAuth(dockerCli, ref.String()) + encodedAuth, err := registryauth.EncodedAuth(dockerCli, ref.String()) if err != nil { return err } diff --git a/cli/command/plugin/install.go b/cli/command/plugin/install.go index 0d457af7bc..eaffc5eae9 100644 --- a/cli/command/plugin/install.go +++ b/cli/command/plugin/install.go @@ -9,6 +9,7 @@ import ( "github.com/docker/cli/cli/command" "github.com/docker/cli/internal/jsonstream" "github.com/docker/cli/internal/prompt" + "github.com/docker/cli/internal/registryauth" "github.com/moby/moby/api/types/plugin" "github.com/moby/moby/client" "github.com/spf13/cobra" @@ -60,7 +61,7 @@ func buildPullConfig(dockerCLI command.Cli, opts pluginOptions) (client.PluginIn return client.PluginInstallOptions{}, err } - encodedAuth, err := command.EncodedRegistryAuth(dockerCLI, ref.String()) + encodedAuth, err := registryauth.EncodedAuth(dockerCLI, ref.String()) if err != nil { return client.PluginInstallOptions{}, err } diff --git a/cli/command/plugin/push.go b/cli/command/plugin/push.go index 5c7e84dbe6..ca7f7139fa 100644 --- a/cli/command/plugin/push.go +++ b/cli/command/plugin/push.go @@ -8,6 +8,7 @@ import ( "github.com/docker/cli/cli" "github.com/docker/cli/cli/command" "github.com/docker/cli/internal/jsonstream" + "github.com/docker/cli/internal/registryauth" "github.com/moby/moby/client" "github.com/spf13/cobra" ) @@ -42,7 +43,7 @@ func runPush(ctx context.Context, dockerCli command.Cli, name string) error { } named = reference.TagNameOnly(named) - encodedAuth, err := command.EncodedRegistryAuth(dockerCli, named.String()) + encodedAuth, err := registryauth.EncodedAuth(dockerCli, named.String()) if err != nil { return err } diff --git a/cli/command/registry.go b/cli/command/registry.go index 41ebc6d8ae..2e767d5b72 100644 --- a/cli/command/registry.go +++ b/cli/command/registry.go @@ -15,7 +15,6 @@ import ( "github.com/docker/cli/cli/hints" "github.com/docker/cli/cli/streams" "github.com/docker/cli/internal/prompt" - "github.com/docker/cli/internal/secretsengine" "github.com/docker/cli/internal/tui" "github.com/moby/moby/api/pkg/authconfig" registrytypes "github.com/moby/moby/api/types/registry" @@ -36,14 +35,6 @@ const ( // [registry.IndexServer]: https://pkg.go.dev/github.com/docker/docker@v28.3.3+incompatible/registry#IndexServer const authConfigKey = "https://index.docker.io/v1/" -func envAuthConfig(registryHostname string) (configtypes.AuthConfig, bool) { - if os.Getenv(configfile.DockerEnvConfigKey) == "" { - return configtypes.AuthConfig{}, false - } - authConfig, err := configfile.New("").GetAuthConfig(registryHostname) - return authConfig, err == nil && authConfig.Username != "" -} - // ResolveAuthConfig returns auth-config for the given registry from the // credential-store. It returns an empty AuthConfig if no credentials were // found. @@ -204,32 +195,13 @@ func PromptUserForCredentials(ctx context.Context, cli Streams, argUser, argPass // // [RFC 4648, Section 5]: https://tools.ietf.org/html/rfc4648#section-5 func RetrieveAuthTokenFromImage(cfg *configfile.ConfigFile, image string) (string, error) { - return retrieveAuthTokenFromImage(secretsengine.NewDesktop(cfg), image) -} - -type registryAuthProvider interface { - RegistryAuthProvider() secretsengine.AuthProvider -} - -func EncodedRegistryAuth(dockerCLI Cli, image string) (string, error) { - if p, ok := dockerCLI.(registryAuthProvider); ok { - return retrieveAuthTokenFromImage(p.RegistryAuthProvider(), image) - } - return RetrieveAuthTokenFromImage(dockerCLI.ConfigFile(), image) -} - -func retrieveAuthTokenFromImage(auth secretsengine.AuthProvider, image string) (string, error) { registryRef, err := reference.ParseNormalizedNamed(image) if err != nil { return "", err } - domain := reference.Domain(registryRef) - authConfig, ok := envAuthConfig(domain) - if !ok { - authConfig, err = auth.GetAuthConfig(domain) - if err != nil { - return "", err - } + authConfig, err := cfg.GetAuthConfig(reference.Domain(registryRef)) + if err != nil { + return "", err } return authconfig.Encode(registrytypes.AuthConfig{ diff --git a/cli/command/registry_internal_test.go b/cli/command/registry_internal_test.go deleted file mode 100644 index 918036fe47..0000000000 --- a/cli/command/registry_internal_test.go +++ /dev/null @@ -1,156 +0,0 @@ -package command - -import ( - "bytes" - "path" - "testing" - - "github.com/docker/cli/cli/config/configfile" - configtypes "github.com/docker/cli/cli/config/types" - "github.com/moby/moby/api/pkg/authconfig" - registrytypes "github.com/moby/moby/api/types/registry" - "gotest.tools/v3/assert" - is "gotest.tools/v3/assert/cmp" -) - -type recordingAuthProvider struct { - hosts []string -} - -func (p *recordingAuthProvider) GetAuthConfig(registryHostname string) (configtypes.AuthConfig, error) { - p.hosts = append(p.hosts, registryHostname) - return configtypes.AuthConfig{Username: "hubuser", Password: "desktop-token", ServerAddress: registryHostname}, nil -} - -func TestRetrieveAuthTokenFromImageUsesAuthProvider(t *testing.T) { - auth := &recordingAuthProvider{} - - actual, err := retrieveAuthTokenFromImage(auth, "dhi.io/python") - assert.NilError(t, err) - expected, err := authconfig.Encode(registrytypes.AuthConfig{Username: "hubuser", Password: "desktop-token", ServerAddress: "dhi.io"}) - assert.NilError(t, err) - assert.Check(t, is.Equal(actual, expected)) - assert.Check(t, is.DeepEqual(auth.hosts, []string{"dhi.io"})) -} - -func TestRetrieveAuthTokenFromImageEnvTakesPrecedence(t *testing.T) { - t.Setenv(configfile.DockerEnvConfigKey, `{"auths":{"https://index.docker.io/v1/":{"auth":"ZW52dXNlcjplbnZwYXNz"}}}`) - auth := &recordingAuthProvider{} - - actual, err := retrieveAuthTokenFromImage(auth, "ubuntu") - assert.NilError(t, err) - expected, err := authconfig.Encode(registrytypes.AuthConfig{Username: "envuser", Password: "envpass", ServerAddress: "https://index.docker.io/v1/"}) - assert.NilError(t, err) - assert.Check(t, is.Equal(actual, expected)) - assert.Check(t, is.Len(auth.hosts, 0), "the auth provider must not be asked for a registry in DOCKER_AUTH_CONFIG") -} - -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 registrytypes.AuthConfig - }{ - { - prefix: "", - expectedAddress: "https://index.docker.io/v1/", - expectedAuthCfg: registrytypes.AuthConfig{Username: "username", Password: "password", ServerAddress: "https://index.docker.io/v1/"}, - }, - { - prefix: "docker.io", - expectedAddress: "https://index.docker.io/v1/", - expectedAuthCfg: registrytypes.AuthConfig{Username: "username", Password: "password", ServerAddress: "https://index.docker.io/v1/"}, - }, - { - prefix: "index.docker.io", - expectedAddress: "https://index.docker.io/v1/", - expectedAuthCfg: registrytypes.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: registrytypes.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: registrytypes.AuthConfig{Username: "username", Password: "password", ServerAddress: "registry.hub.docker.com"}, - }, - { - prefix: "[::1]", - expectedAddress: "[::1]", - expectedAuthCfg: registrytypes.AuthConfig{Username: "username", Password: "password", ServerAddress: "[::1]"}, - }, - { - prefix: "[::1]:5000", - expectedAddress: "[::1]:5000", - expectedAuthCfg: registrytypes.AuthConfig{Username: "username", Password: "password", ServerAddress: "[::1]:5000"}, - }, - { - prefix: "127.0.0.1", - expectedAddress: "127.0.0.1", - expectedAuthCfg: registrytypes.AuthConfig{Username: "username", Password: "password", ServerAddress: "127.0.0.1"}, - }, - { - prefix: "localhost", - expectedAddress: "localhost", - expectedAuthCfg: registrytypes.AuthConfig{Username: "username", Password: "password", ServerAddress: "localhost"}, - }, - { - prefix: "localhost:5000", - expectedAddress: "localhost:5000", - expectedAuthCfg: registrytypes.AuthConfig{Username: "username", Password: "password", ServerAddress: "localhost:5000"}, - }, - { - prefix: "no-auth.example.com", - expectedAuthCfg: registrytypes.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 := retrieveAuthTokenFromImage(&cfg, imageRef) - assert.NilError(t, err) - expectedAuthCfg, err := authconfig.Encode(tc.expectedAuthCfg) - assert.NilError(t, err) - assert.Equal(t, actual, expectedAuthCfg) - } - }) - } -} diff --git a/cli/command/registry_test.go b/cli/command/registry_test.go index 865e72ad54..751711f363 100644 --- a/cli/command/registry_test.go +++ b/cli/command/registry_test.go @@ -1,13 +1,14 @@ package command_test import ( + "bytes" + "path" "path/filepath" "testing" "github.com/docker/cli/cli/command" "github.com/docker/cli/cli/config/configfile" configtypes "github.com/docker/cli/cli/config/types" - "github.com/docker/cli/internal/secretsengine" "github.com/moby/moby/api/pkg/authconfig" "github.com/moby/moby/api/types/registry" "gotest.tools/v3/assert" @@ -81,35 +82,6 @@ func TestGetDefaultAuthConfig(t *testing.T) { } } -type fakeAuthProvider struct { - hosts []string -} - -func (p *fakeAuthProvider) GetAuthConfig(registryHostname string) (configtypes.AuthConfig, error) { - p.hosts = append(p.hosts, registryHostname) - return configtypes.AuthConfig{Username: "hubuser", Password: "desktop-token", ServerAddress: registryHostname}, nil -} - -type authProviderCli struct { - command.Cli - auth *fakeAuthProvider -} - -func (c *authProviderCli) RegistryAuthProvider() secretsengine.AuthProvider { - return c.auth -} - -func TestEncodedRegistryAuthUsesCLIAuthProvider(t *testing.T) { - dockerCLI := &authProviderCli{auth: &fakeAuthProvider{}} - - actual, err := command.EncodedRegistryAuth(dockerCLI, "dhi.io/python") - assert.NilError(t, err) - expected, err := authconfig.Encode(registry.AuthConfig{Username: "hubuser", Password: "desktop-token", ServerAddress: "dhi.io"}) - assert.NilError(t, err) - assert.Check(t, is.Equal(actual, expected)) - assert.Check(t, is.DeepEqual(dockerCLI.auth.hosts, []string{"dhi.io"})) -} - func TestGetDefaultAuthConfig_HelperError(t *testing.T) { cfg := configfile.New("filename") cfg.CredentialsStore = "fake-does-not-exist" @@ -123,3 +95,113 @@ func TestGetDefaultAuthConfig_HelperError(t *testing.T) { assert.Check(t, is.DeepEqual(expectedAuthConfig, authCfg)) 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) + expectedAuthCfg, err := authconfig.Encode(tc.expectedAuthCfg) + assert.NilError(t, err) + assert.Equal(t, actual, expectedAuthCfg) + } + }) + } +} diff --git a/cli/command/service/create.go b/cli/command/service/create.go index b95f8c9dbd..d2c1e0ee68 100644 --- a/cli/command/service/create.go +++ b/cli/command/service/create.go @@ -7,6 +7,7 @@ import ( "github.com/docker/cli/cli" "github.com/docker/cli/cli/command" "github.com/docker/cli/cli/command/completion" + "github.com/docker/cli/internal/registryauth" cliopts "github.com/docker/cli/opts" "github.com/moby/moby/api/types/swarm" "github.com/moby/moby/client" @@ -120,7 +121,7 @@ func runCreate(ctx context.Context, dockerCLI command.Cli, flags *pflag.FlagSet, if opts.registryAuth { // Retrieve encoded auth token from the image reference var err error - encodedAuth, err = command.EncodedRegistryAuth(dockerCLI, opts.image) + encodedAuth, err = registryauth.EncodedAuth(dockerCLI, opts.image) if err != nil { return err } diff --git a/cli/command/service/update.go b/cli/command/service/update.go index 35ffe9fb9f..7da58245aa 100644 --- a/cli/command/service/update.go +++ b/cli/command/service/update.go @@ -17,6 +17,7 @@ import ( "github.com/docker/cli/cli" "github.com/docker/cli/cli/command" "github.com/docker/cli/cli/command/completion" + "github.com/docker/cli/internal/registryauth" "github.com/docker/cli/opts" "github.com/docker/cli/opts/swarmopts" "github.com/moby/moby/api/types/container" @@ -223,7 +224,7 @@ func runUpdate(ctx context.Context, dockerCLI command.Cli, flags *pflag.FlagSet, // Retrieve encoded auth token from the image reference // This would be the old image if it didn't change in this update var err error - encodedAuth, err = command.EncodedRegistryAuth(dockerCLI, spec.TaskTemplate.ContainerSpec.Image) + encodedAuth, err = registryauth.EncodedAuth(dockerCLI, spec.TaskTemplate.ContainerSpec.Image) if err != nil { return err } diff --git a/cli/command/stack/deploy_composefile.go b/cli/command/stack/deploy_composefile.go index 01041008bc..82f8c74073 100644 --- a/cli/command/stack/deploy_composefile.go +++ b/cli/command/stack/deploy_composefile.go @@ -10,6 +10,7 @@ import ( "github.com/docker/cli/cli/command/service" "github.com/docker/cli/cli/compose/convert" composetypes "github.com/docker/cli/cli/compose/types" + "github.com/docker/cli/internal/registryauth" "github.com/moby/moby/api/types/container" "github.com/moby/moby/api/types/network" "github.com/moby/moby/api/types/swarm" @@ -224,7 +225,7 @@ func deployServices(ctx context.Context, dockerCLI command.Cli, services map[str if sendAuth { // Retrieve encoded auth token from the image reference - encodedAuth, err = command.EncodedRegistryAuth(dockerCLI, image) + encodedAuth, err = registryauth.EncodedAuth(dockerCLI, image) if err != nil { return nil, err } diff --git a/internal/registry/config.go b/internal/registry/config.go index 062940f627..aab0b7ae28 100644 --- a/internal/registry/config.go +++ b/internal/registry/config.go @@ -53,8 +53,6 @@ const ( // IndexName is the name of the index IndexName = "docker.io" DefaultRegistryHost = "registry-1.docker.io" - StagingRegistryHost = "registry-1-stage.docker.io" - DHIRegistryHost = "dhi.io" ) var ( diff --git a/internal/registryauth/registryauth.go b/internal/registryauth/registryauth.go new file mode 100644 index 0000000000..657387492a --- /dev/null +++ b/internal/registryauth/registryauth.go @@ -0,0 +1,62 @@ +package registryauth + +import ( + "os" + + "github.com/distribution/reference" + "github.com/docker/cli/cli/config/configfile" + configtypes "github.com/docker/cli/cli/config/types" + "github.com/docker/cli/internal/secretsengine" + "github.com/moby/moby/api/pkg/authconfig" + registrytypes "github.com/moby/moby/api/types/registry" +) + +type configProvider interface { + ConfigFile() *configfile.ConfigFile +} + +type authProvider interface { + RegistryAuthProvider() secretsengine.AuthProvider +} + +type envAuthFunc func(registryHostname string) (configtypes.AuthConfig, bool) + +func EncodedAuth(dockerCLI configProvider, image string) (string, error) { + var auth secretsengine.AuthProvider = dockerCLI.ConfigFile() + if p, ok := dockerCLI.(authProvider); ok { + auth = p.RegistryAuthProvider() + } + return encodedAuth(envAuthConfig, auth, image) +} + +func encodedAuth(envAuth envAuthFunc, auth secretsengine.AuthProvider, image string) (string, error) { + registryRef, err := reference.ParseNormalizedNamed(image) + if err != nil { + return "", err + } + domain := reference.Domain(registryRef) + authConfig, ok := envAuth(domain) + if !ok { + authConfig, err = auth.GetAuthConfig(domain) + if err != nil { + return "", err + } + } + + return authconfig.Encode(registrytypes.AuthConfig{ + Username: authConfig.Username, + Password: authConfig.Password, + ServerAddress: authConfig.ServerAddress, + Auth: authConfig.Auth, + IdentityToken: authConfig.IdentityToken, + RegistryToken: authConfig.RegistryToken, + }) +} + +func envAuthConfig(registryHostname string) (configtypes.AuthConfig, bool) { + if os.Getenv(configfile.DockerEnvConfigKey) == "" { + return configtypes.AuthConfig{}, false + } + authConfig, err := configfile.New("").GetAuthConfig(registryHostname) + return authConfig, err == nil && authConfig.Username != "" +} diff --git a/internal/registryauth/registryauth_test.go b/internal/registryauth/registryauth_test.go new file mode 100644 index 0000000000..941fc9de5c --- /dev/null +++ b/internal/registryauth/registryauth_test.go @@ -0,0 +1,131 @@ +package registryauth + +import ( + "bytes" + "sync" + "testing" + + "github.com/docker/cli/cli/config/configfile" + configtypes "github.com/docker/cli/cli/config/types" + "github.com/docker/cli/internal/secretsengine" + "github.com/moby/moby/api/pkg/authconfig" + registrytypes "github.com/moby/moby/api/types/registry" + "gotest.tools/v3/assert" + is "gotest.tools/v3/assert/cmp" +) + +type recordingAuthProvider struct { + mu sync.Mutex + hosts []string +} + +func (p *recordingAuthProvider) GetAuthConfig(registryHostname string) (configtypes.AuthConfig, error) { + p.mu.Lock() + defer p.mu.Unlock() + p.hosts = append(p.hosts, registryHostname) + return configtypes.AuthConfig{Username: "hubuser", Password: "desktop-token", ServerAddress: registryHostname}, nil +} + +type configOnlyCli struct { + cfg *configfile.ConfigFile +} + +func (c *configOnlyCli) ConfigFile() *configfile.ConfigFile { + return c.cfg +} + +type fakeCli struct { + configOnlyCli + auth *recordingAuthProvider +} + +func (c *fakeCli) RegistryAuthProvider() secretsengine.AuthProvider { + return c.auth +} + +func noEnvAuth(string) (configtypes.AuthConfig, bool) { + return configtypes.AuthConfig{}, false +} + +func loadConfig(t *testing.T) *configfile.ConfigFile { + t.Helper() + const configFileContent = `{"auths": { + "https://index.docker.io/v1/": {"auth": "dXNlcm5hbWU6cGFzc3dvcmQ="}, + "localhost:5000": {"auth": "dXNlcm5hbWU6cGFzc3dvcmQ="} + } +}` + cfg := &configfile.ConfigFile{} + assert.NilError(t, cfg.LoadFromReader(bytes.NewReader([]byte(configFileContent)))) + return cfg +} + +func TestEncodedAuthUsesCLIAuthProvider(t *testing.T) { + t.Parallel() + dockerCLI := &fakeCli{configOnlyCli: configOnlyCli{cfg: configfile.New("")}, auth: &recordingAuthProvider{}} + + actual, err := EncodedAuth(dockerCLI, "dhi.io/python") + assert.NilError(t, err) + expected, err := authconfig.Encode(registrytypes.AuthConfig{Username: "hubuser", Password: "desktop-token", ServerAddress: "dhi.io"}) + assert.NilError(t, err) + assert.Check(t, is.Equal(actual, expected)) + assert.Check(t, is.DeepEqual(dockerCLI.auth.hosts, []string{"dhi.io"})) +} + +func TestEncodedAuthWithoutAuthProviderUsesConfigFile(t *testing.T) { + t.Parallel() + actual, err := EncodedAuth(&configOnlyCli{cfg: loadConfig(t)}, "ubuntu") + assert.NilError(t, err) + expected, err := authconfig.Encode(registrytypes.AuthConfig{Username: "username", Password: "password", ServerAddress: "https://index.docker.io/v1/"}) + assert.NilError(t, err) + assert.Check(t, is.Equal(actual, expected)) +} + +func TestEncodedAuthEnvTakesPrecedence(t *testing.T) { + t.Parallel() + envAuth := func(registryHostname string) (configtypes.AuthConfig, bool) { + return configtypes.AuthConfig{Username: "envuser", Password: "envpass", ServerAddress: registryHostname}, true + } + auth := &recordingAuthProvider{} + + actual, err := encodedAuth(envAuth, auth, "ubuntu") + assert.NilError(t, err) + expected, err := authconfig.Encode(registrytypes.AuthConfig{Username: "envuser", Password: "envpass", ServerAddress: "docker.io"}) + assert.NilError(t, err) + assert.Check(t, is.Equal(actual, expected)) + assert.Check(t, is.Len(auth.hosts, 0), "the auth provider must not be asked for a registry in DOCKER_AUTH_CONFIG") +} + +func TestEnvAuthConfig(t *testing.T) { + t.Setenv(configfile.DockerEnvConfigKey, `{"auths":{"https://index.docker.io/v1/":{"auth":"ZW52dXNlcjplbnZwYXNz"}}}`) + + got, ok := envAuthConfig("docker.io") + assert.Check(t, ok) + assert.Check(t, is.DeepEqual(got, configtypes.AuthConfig{Username: "envuser", Password: "envpass", ServerAddress: "https://index.docker.io/v1/"})) + + _, ok = envAuthConfig("ghcr.io") + assert.Check(t, !ok) +} + +func TestEncodedAuthFromConfigFile(t *testing.T) { + t.Parallel() + cfg := loadConfig(t) + + tests := []struct { + image string + expected registrytypes.AuthConfig + }{ + {image: "ubuntu", expected: registrytypes.AuthConfig{Username: "username", Password: "password", ServerAddress: "https://index.docker.io/v1/"}}, + {image: "localhost:5000/ubuntu", expected: registrytypes.AuthConfig{Username: "username", Password: "password", ServerAddress: "localhost:5000"}}, + {image: "no-auth.example.com/ubuntu", expected: registrytypes.AuthConfig{}}, + } + for _, tc := range tests { + t.Run(tc.image, func(t *testing.T) { + t.Parallel() + actual, err := encodedAuth(noEnvAuth, cfg, tc.image) + assert.NilError(t, err) + expected, err := authconfig.Encode(tc.expected) + assert.NilError(t, err) + assert.Check(t, is.Equal(actual, expected)) + }) + } +} diff --git a/internal/secretsengine/secretsengine.go b/internal/secretsengine/secretsengine.go index f614a02293..61ea71e409 100644 --- a/internal/secretsengine/secretsengine.go +++ b/internal/secretsengine/secretsengine.go @@ -25,12 +25,12 @@ const ( sessionExpirySkew = 30 * time.Second sessionTTL = time.Minute retryInterval = time.Minute + + stagingRegistryHost = "registry-1-stage.docker.io" + dhiRegistryHost = "dhi.io" ) -var ( - errNoUsername = errors.New("no username for the session") - errSessionExpired = errors.New("the session has expired (not signed in to Docker Desktop?)") -) +var errSessionExpired = errors.New("the session has expired (not signed in to Docker Desktop?)") type hubClient interface { GetDefaultSession(ctx context.Context) (dockerhub.UserSession, error) @@ -49,24 +49,11 @@ type provider struct { staging *session } -func newProvider(fallback AuthProvider, hub, staging hubClient) *provider { - return &provider{ - fallback: fallback, - hub: newSession("Docker Hub", hub), - staging: newSession("Docker Hub staging", staging), - } -} - -var ( - desktopHubSession = newDesktopSession("Docker Hub") - desktopStagingSession = newDesktopSession("Docker Hub staging", dockerhub.Staging()) -) - func NewDesktop(fallback AuthProvider) AuthProvider { return &provider{ fallback: fallback, - hub: desktopHubSession, - staging: desktopStagingSession, + hub: newDesktopSession("Docker Hub"), + staging: newDesktopSession("Docker Hub staging", dockerhub.Staging()), } } @@ -84,10 +71,10 @@ func (p *provider) sessionFor(registryHostname string) (*session, string) { switch credentials.ConvertToHostname(registryHostname) { case registry.IndexName, registry.IndexHostname, registry.DefaultRegistryHost: return p.hub, registry.IndexServer - case registry.DHIRegistryHost: - return p.hub, registry.DHIRegistryHost - case registry.StagingRegistryHost: - return p.staging, registry.StagingRegistryHost + case dhiRegistryHost: + return p.hub, dhiRegistryHost + case stagingRegistryHost: + return p.staging, stagingRegistryHost default: return nil, "" } @@ -96,8 +83,6 @@ func (p *provider) sessionFor(registryHostname string) (*session, string) { type session struct { name string connect func() (hubClient, error) - timeout time.Duration - now func() time.Time mu sync.Mutex hub hubClient @@ -106,21 +91,13 @@ type session struct { validUntil time.Time } -func newSession(name string, hub hubClient) *session { - return &session{ - name: name, - hub: hub, - timeout: lookupTimeout, - now: time.Now, - } -} - func newDesktopSession(name string, opts ...dockerhub.Option) *session { - s := newSession(name, nil) - s.connect = func() (hubClient, error) { - return connect(api.DesktopSocketPath(), opts...) + return &session{ + name: name, + connect: func() (hubClient, error) { + return connect(api.DesktopSocketPath(), opts...) + }, } - return s } func connect(socketPath string, opts ...dockerhub.Option) (hubClient, error) { @@ -141,7 +118,7 @@ func (s *session) auth() (types.AuthConfig, bool) { s.mu.Lock() defer s.mu.Unlock() - now := s.now() + now := time.Now() if now.Before(s.retryAt) { return types.AuthConfig{}, false } @@ -150,7 +127,7 @@ func (s *session) auth() (types.AuthConfig, bool) { } auth, validUntil, err := s.fetch() if err != nil { - s.retryAt = s.now().Add(retryInterval) + s.retryAt = time.Now().Add(retryInterval) s.cached = nil logFallback(s.name, err) return types.AuthConfig{}, false @@ -169,7 +146,7 @@ func (s *session) fetch() (types.AuthConfig, time.Time, error) { s.hub = hub } - ctx, cancel := context.WithTimeout(context.Background(), s.timeout) + ctx, cancel := context.WithTimeout(context.Background(), lookupTimeout) defer cancel() userSession, err := s.hub.GetDefaultSession(ctx) @@ -186,10 +163,10 @@ func (s *session) fetch() (types.AuthConfig, time.Time, error) { username = profile.Username } if username == "" { - return types.AuthConfig{}, time.Time{}, errNoUsername + return types.AuthConfig{}, time.Time{}, errors.New("no username for the session") } - now := s.now() + now := time.Now() validUntil := now.Add(sessionTTL) if exp := userSession.Claims.ExpiresAt; exp != nil { validUntil = exp.Add(-sessionExpirySkew) diff --git a/internal/secretsengine/secretsengine_test.go b/internal/secretsengine/secretsengine_test.go index c185e53cc2..c6f0a71eb0 100644 --- a/internal/secretsengine/secretsengine_test.go +++ b/internal/secretsengine/secretsengine_test.go @@ -11,6 +11,7 @@ import ( "sync" "sync/atomic" "testing" + "testing/synctest" "time" "github.com/docker/cli/cli/config/types" @@ -20,8 +21,6 @@ import ( "gotest.tools/v3/assert" ) -var testNow = time.Date(2026, 10, 7, 12, 0, 0, 0, time.UTC) - type fakeHub struct { session dockerhub.UserSession sessionErr error @@ -39,23 +38,6 @@ func (f *fakeHub) GetDefaultProfile(context.Context) (dockerhub.Profile, error) return f.profile, f.profileErr } -type fakeClock struct { - mu sync.Mutex - t time.Time -} - -func (c *fakeClock) now() time.Time { - c.mu.Lock() - defer c.mu.Unlock() - return c.t -} - -func (c *fakeClock) advance(d time.Duration) { - c.mu.Lock() - defer c.mu.Unlock() - c.t = c.t.Add(d) -} - func hubSession(username string, expiresAt time.Time) dockerhub.UserSession { s := dockerhub.UserSession{AccessToken: "desktop-oauth-token"} s.Claims.Username = username @@ -65,6 +47,10 @@ func hubSession(username string, expiresAt time.Time) dockerhub.UserSession { return s } +func inAnHour() time.Time { + return time.Now().Add(time.Hour) +} + type recordingFallback struct { mu sync.Mutex hosts []string @@ -86,33 +72,31 @@ func desktopCredentials() types.AuthConfig { } func stagingCredentials() types.AuthConfig { - return types.AuthConfig{Username: "stageuser", Password: "staging-oauth-token", ServerAddress: registry.StagingRegistryHost} + return types.AuthConfig{Username: "stageuser", Password: "staging-oauth-token", ServerAddress: stagingRegistryHost} } type testProvider struct { *provider fallback *recordingFallback - clock *fakeClock staging *fakeHub } func newTestProvider(hub hubClient) *testProvider { tp := &testProvider{ fallback: &recordingFallback{}, - clock: &fakeClock{t: testNow}, staging: &fakeHub{sessionErr: dockerhub.ErrNoDefaultProfile}, } - tp.provider = newProvider(tp.fallback, hub, tp.staging) - for _, s := range []*session{tp.provider.hub, tp.provider.staging} { - s.timeout = time.Second - s.now = tp.clock.now + tp.provider = &provider{ + fallback: tp.fallback, + hub: &session{name: "Docker Hub", hub: hub}, + staging: &session{name: "Docker Hub staging", hub: tp.staging}, } return tp } func (tp *testProvider) signInToStaging() { tp.staging.sessionErr = nil - tp.staging.session = hubSession("stageuser", testNow.Add(time.Hour)) + tp.staging.session = hubSession("stageuser", inAnHour()) tp.staging.session.AccessToken = "staging-oauth-token" } @@ -124,18 +108,18 @@ func connectError(reason seclient.ConnectReason) error { } } -func TestNewDesktopSharesOneSessionPerEnvironment(t *testing.T) { - a := NewDesktop(&recordingFallback{}).(*provider) - b := NewDesktop(&recordingFallback{}).(*provider) - assert.Assert(t, a.hub == b.hub, "providers must share the process-wide Docker Hub session") - assert.Assert(t, a.staging == b.staging, "providers must share the process-wide Docker Hub staging session") - assert.Assert(t, a.hub != a.staging, "production and staging sessions must be separate") +func TestNewDesktopUsesSeparateSessions(t *testing.T) { + t.Parallel() + p := NewDesktop(&recordingFallback{}).(*provider) + assert.Assert(t, p.hub != p.staging, "production and staging sessions must be separate") } func TestDockerHubUsesDesktopSession(t *testing.T) { + t.Parallel() for _, host := range []string{"docker.io", "index.docker.io", "registry-1.docker.io", registry.IndexServer} { t.Run(host, func(t *testing.T) { - hub := &fakeHub{session: hubSession("hubuser", testNow.Add(time.Hour))} + t.Parallel() + hub := &fakeHub{session: hubSession("hubuser", inAnHour())} p := newTestProvider(hub) got, err := p.GetAuthConfig(host) @@ -148,15 +132,17 @@ func TestDockerHubUsesDesktopSession(t *testing.T) { } func TestOtherRegistriesUseFallback(t *testing.T) { + t.Parallel() hosts := []string{ "ghcr.io", "localhost:5000", "registry.example.com", "", - registry.StagingRegistryHost + ".example.com", "example.com/" + registry.StagingRegistryHost, - registry.DHIRegistryHost + ".example.com", "example.com/" + registry.DHIRegistryHost, "registry." + registry.DHIRegistryHost, + stagingRegistryHost + ".example.com", "example.com/" + stagingRegistryHost, + dhiRegistryHost + ".example.com", "example.com/" + dhiRegistryHost, "registry." + dhiRegistryHost, "docker.io.example.com", } for _, host := range hosts { t.Run(host, func(t *testing.T) { - hub := &fakeHub{session: hubSession("hubuser", testNow.Add(time.Hour))} + t.Parallel() + hub := &fakeHub{session: hubSession("hubuser", inAnHour())} p := newTestProvider(hub) p.signInToStaging() @@ -171,14 +157,16 @@ func TestOtherRegistriesUseFallback(t *testing.T) { } func TestDHIUsesDockerHubSession(t *testing.T) { - for _, host := range []string{registry.DHIRegistryHost, "https://" + registry.DHIRegistryHost, "https://" + registry.DHIRegistryHost + "/"} { + t.Parallel() + for _, host := range []string{dhiRegistryHost, "https://" + dhiRegistryHost, "https://" + dhiRegistryHost + "/"} { t.Run(host, func(t *testing.T) { - hub := &fakeHub{session: hubSession("hubuser", testNow.Add(time.Hour))} + t.Parallel() + hub := &fakeHub{session: hubSession("hubuser", inAnHour())} p := newTestProvider(hub) p.signInToStaging() want := desktopCredentials() - want.ServerAddress = registry.DHIRegistryHost + want.ServerAddress = dhiRegistryHost got, err := p.GetAuthConfig(host) assert.NilError(t, err) assert.DeepEqual(t, got, want) @@ -193,9 +181,11 @@ func TestDHIUsesDockerHubSession(t *testing.T) { } func TestStagingRegistryUsesStagingSession(t *testing.T) { - for _, host := range []string{registry.StagingRegistryHost, "https://" + registry.StagingRegistryHost + "/"} { + t.Parallel() + for _, host := range []string{stagingRegistryHost, "https://" + stagingRegistryHost + "/"} { t.Run(host, func(t *testing.T) { - hub := &fakeHub{session: hubSession("hubuser", testNow.Add(time.Hour))} + t.Parallel() + hub := &fakeHub{session: hubSession("hubuser", inAnHour())} p := newTestProvider(hub) p.signInToStaging() @@ -208,7 +198,9 @@ func TestStagingRegistryUsesStagingSession(t *testing.T) { } func TestEnvironmentsDoNotMix(t *testing.T) { + t.Parallel() t.Run("staging session is not used for docker.io", func(t *testing.T) { + t.Parallel() p := newTestProvider(&fakeHub{sessionErr: dockerhub.ErrNoDefaultProfile}) p.signInToStaging() @@ -216,17 +208,18 @@ func TestEnvironmentsDoNotMix(t *testing.T) { assert.NilError(t, err) assert.DeepEqual(t, got, cliCredentials("docker.io")) - got, err = p.GetAuthConfig(registry.StagingRegistryHost) + got, err = p.GetAuthConfig(stagingRegistryHost) assert.NilError(t, err) assert.DeepEqual(t, got, stagingCredentials()) }) t.Run("production session is not used for the staging registry", func(t *testing.T) { - p := newTestProvider(&fakeHub{session: hubSession("hubuser", testNow.Add(time.Hour))}) + t.Parallel() + p := newTestProvider(&fakeHub{session: hubSession("hubuser", inAnHour())}) - got, err := p.GetAuthConfig(registry.StagingRegistryHost) + got, err := p.GetAuthConfig(stagingRegistryHost) assert.NilError(t, err) - assert.DeepEqual(t, got, cliCredentials(registry.StagingRegistryHost)) + assert.DeepEqual(t, got, cliCredentials(stagingRegistryHost)) got, err = p.GetAuthConfig("docker.io") assert.NilError(t, err) @@ -235,8 +228,9 @@ func TestEnvironmentsDoNotMix(t *testing.T) { } func TestUsernameFromProfile(t *testing.T) { + t.Parallel() hub := &fakeHub{ - session: hubSession("", testNow.Add(time.Hour)), + session: hubSession("", inAnHour()), profile: dockerhub.Profile{Username: "hubuser"}, } p := newTestProvider(hub) @@ -247,40 +241,51 @@ func TestUsernameFromProfile(t *testing.T) { } func TestFallsBackWithoutUsableSession(t *testing.T) { - future := testNow.Add(time.Hour) + t.Parallel() tests := []struct { name string - hub *fakeHub + hub func() *fakeHub }{ - {name: "secrets engine not running", hub: &fakeHub{sessionErr: connectError(seclient.ReasonNotRunning)}}, - {name: "secrets engine socket not accessible", hub: &fakeHub{sessionErr: connectError(seclient.ReasonPermissionDenied)}}, - {name: "nobody signed in", hub: &fakeHub{sessionErr: dockerhub.ErrNoDefaultProfile}}, - {name: "access denied", hub: &fakeHub{sessionErr: seclient.ErrAccessDenied}}, - {name: "no username", hub: &fakeHub{session: hubSession("", future)}}, - {name: "profile lookup fails", hub: &fakeHub{session: hubSession("", future), profileErr: errors.New("boom")}}, - {name: "expired", hub: &fakeHub{session: hubSession("hubuser", testNow.Add(-time.Minute))}}, - {name: "expires within the skew", hub: &fakeHub{session: hubSession("hubuser", testNow.Add(sessionExpirySkew/2))}}, + {name: "secrets engine not running", hub: func() *fakeHub { return &fakeHub{sessionErr: connectError(seclient.ReasonNotRunning)} }}, + {name: "secrets engine socket not accessible", hub: func() *fakeHub { return &fakeHub{sessionErr: connectError(seclient.ReasonPermissionDenied)} }}, + {name: "nobody signed in", hub: func() *fakeHub { return &fakeHub{sessionErr: dockerhub.ErrNoDefaultProfile} }}, + {name: "access denied", hub: func() *fakeHub { return &fakeHub{sessionErr: seclient.ErrAccessDenied} }}, + {name: "no username", hub: func() *fakeHub { return &fakeHub{session: hubSession("", inAnHour())} }}, + {name: "profile lookup fails", hub: func() *fakeHub { + return &fakeHub{session: hubSession("", inAnHour()), profileErr: errors.New("boom")} + }}, + {name: "expired", hub: func() *fakeHub { return &fakeHub{session: hubSession("hubuser", time.Now().Add(-time.Minute))} }}, + {name: "expires within the skew", hub: func() *fakeHub { + return &fakeHub{session: hubSession("hubuser", time.Now().Add(sessionExpirySkew/2))} + }}, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - p := newTestProvider(tc.hub) + t.Parallel() + synctest.Test(t, func(t *testing.T) { + hub := tc.hub() + p := newTestProvider(hub) - for range 2 { - got, err := p.GetAuthConfig("docker.io") - assert.NilError(t, err) - assert.DeepEqual(t, got, cliCredentials("docker.io")) - } - assert.Equal(t, tc.hub.sessionCalls.Load(), int32(1), "Desktop must not be asked again within retryInterval") + for range 2 { + got, err := p.GetAuthConfig("docker.io") + assert.NilError(t, err) + assert.DeepEqual(t, got, cliCredentials("docker.io")) + } + assert.Equal(t, hub.sessionCalls.Load(), int32(1), "Desktop must not be asked again within retryInterval") + }) }) } } func TestDesktopFallsBackWhenClientCannotBeCreated(t *testing.T) { + t.Parallel() var connects atomic.Int32 - s := newSession("Docker Hub", nil) - s.connect = func() (hubClient, error) { - connects.Add(1) - return nil, errors.New("no socket path") + s := &session{ + name: "Docker Hub", + connect: func() (hubClient, error) { + connects.Add(1) + return nil, errors.New("no socket path") + }, } p := &provider{fallback: &recordingFallback{}, hub: s} @@ -293,75 +298,85 @@ func TestDesktopFallsBackWhenClientCannotBeCreated(t *testing.T) { } func TestRetriesAfterInterval(t *testing.T) { - hub := &fakeHub{sessionErr: dockerhub.ErrNoDefaultProfile} - p := newTestProvider(hub) + t.Parallel() + synctest.Test(t, func(t *testing.T) { + hub := &fakeHub{sessionErr: dockerhub.ErrNoDefaultProfile} + p := newTestProvider(hub) - got, err := p.GetAuthConfig("docker.io") - assert.NilError(t, err) - assert.DeepEqual(t, got, cliCredentials("docker.io")) + got, err := p.GetAuthConfig("docker.io") + assert.NilError(t, err) + assert.DeepEqual(t, got, cliCredentials("docker.io")) - hub.sessionErr = nil - hub.session = hubSession("hubuser", testNow.Add(time.Hour)) + hub.sessionErr = nil + hub.session = hubSession("hubuser", inAnHour()) - p.clock.advance(retryInterval - time.Second) - got, err = p.GetAuthConfig("docker.io") - assert.NilError(t, err) - assert.DeepEqual(t, got, cliCredentials("docker.io")) - assert.Equal(t, hub.sessionCalls.Load(), int32(1)) + time.Sleep(retryInterval - time.Second) + got, err = p.GetAuthConfig("docker.io") + assert.NilError(t, err) + assert.DeepEqual(t, got, cliCredentials("docker.io")) + assert.Equal(t, hub.sessionCalls.Load(), int32(1)) - p.clock.advance(time.Second) - got, err = p.GetAuthConfig("docker.io") - assert.NilError(t, err) - assert.DeepEqual(t, got, desktopCredentials()) - assert.Equal(t, hub.sessionCalls.Load(), int32(2)) + time.Sleep(time.Second) + got, err = p.GetAuthConfig("docker.io") + assert.NilError(t, err) + assert.DeepEqual(t, got, desktopCredentials()) + assert.Equal(t, hub.sessionCalls.Load(), int32(2)) + }) } func TestReusesSessionUntilExpiry(t *testing.T) { - expiresAt := testNow.Add(10 * time.Minute) - hub := &fakeHub{session: hubSession("hubuser", expiresAt)} - p := newTestProvider(hub) + t.Parallel() + synctest.Test(t, func(t *testing.T) { + expiresAt := time.Now().Add(10 * time.Minute) + hub := &fakeHub{session: hubSession("hubuser", expiresAt)} + p := newTestProvider(hub) - got, err := p.GetAuthConfig("docker.io") - assert.NilError(t, err) - assert.DeepEqual(t, got, desktopCredentials()) - p.clock.advance(5 * time.Minute) - got, err = p.GetAuthConfig("docker.io") - assert.NilError(t, err) - assert.DeepEqual(t, got, desktopCredentials()) - assert.Equal(t, hub.sessionCalls.Load(), int32(1)) + got, err := p.GetAuthConfig("docker.io") + assert.NilError(t, err) + assert.DeepEqual(t, got, desktopCredentials()) + time.Sleep(5 * time.Minute) + got, err = p.GetAuthConfig("docker.io") + assert.NilError(t, err) + assert.DeepEqual(t, got, desktopCredentials()) + assert.Equal(t, hub.sessionCalls.Load(), int32(1)) - hub.session = hubSession("hubuser", expiresAt.Add(time.Hour)) - hub.session.AccessToken = "refreshed-token" - p.clock.advance(5*time.Minute - sessionExpirySkew) + hub.session = hubSession("hubuser", expiresAt.Add(time.Hour)) + hub.session.AccessToken = "refreshed-token" + time.Sleep(5*time.Minute - sessionExpirySkew) - got, err = p.GetAuthConfig("docker.io") - assert.NilError(t, err) - assert.Equal(t, got.Password, "refreshed-token") - assert.Equal(t, hub.sessionCalls.Load(), int32(2)) + got, err = p.GetAuthConfig("docker.io") + assert.NilError(t, err) + assert.Equal(t, got.Password, "refreshed-token") + assert.Equal(t, hub.sessionCalls.Load(), int32(2)) + }) } func TestRefetchesSessionWithoutExpiry(t *testing.T) { - hub := &fakeHub{session: hubSession("hubuser", time.Time{})} - p := newTestProvider(hub) + t.Parallel() + synctest.Test(t, func(t *testing.T) { + hub := &fakeHub{session: hubSession("hubuser", time.Time{})} + p := newTestProvider(hub) - got, err := p.GetAuthConfig("docker.io") - assert.NilError(t, err) - assert.DeepEqual(t, got, desktopCredentials()) - p.clock.advance(sessionTTL - time.Second) - got, err = p.GetAuthConfig("docker.io") - assert.NilError(t, err) - assert.DeepEqual(t, got, desktopCredentials()) - assert.Equal(t, hub.sessionCalls.Load(), int32(1)) + got, err := p.GetAuthConfig("docker.io") + assert.NilError(t, err) + assert.DeepEqual(t, got, desktopCredentials()) + time.Sleep(sessionTTL - time.Second) + got, err = p.GetAuthConfig("docker.io") + assert.NilError(t, err) + assert.DeepEqual(t, got, desktopCredentials()) + assert.Equal(t, hub.sessionCalls.Load(), int32(1)) - p.clock.advance(time.Second) - got, err = p.GetAuthConfig("docker.io") - assert.NilError(t, err) - assert.DeepEqual(t, got, desktopCredentials()) - assert.Equal(t, hub.sessionCalls.Load(), int32(2)) + time.Sleep(time.Second) + got, err = p.GetAuthConfig("docker.io") + assert.NilError(t, err) + assert.DeepEqual(t, got, desktopCredentials()) + assert.Equal(t, hub.sessionCalls.Load(), int32(2)) + }) } func TestConcurrentLookupsShareOneFetch(t *testing.T) { - hub := &fakeHub{session: hubSession("hubuser", testNow.Add(time.Hour))} + t.Parallel() + hub := &fakeHub{session: hubSession("hubuser", inAnHour())} p := newTestProvider(hub) const lookups = 20 @@ -383,12 +398,14 @@ func TestConcurrentLookupsShareOneFetch(t *testing.T) { } func TestConnectSkipsMissingSocket(t *testing.T) { + t.Parallel() _, err := connect(filepath.Join(t.TempDir(), "engine.sock")) assert.Assert(t, errors.Is(err, fs.ErrNotExist), err) assert.Assert(t, isSessionUnavailable(err)) } func TestIsSessionUnavailable(t *testing.T) { + t.Parallel() for _, err := range []error{ connectError(seclient.ReasonNotRunning), fmt.Errorf("get session: %w", connectError(seclient.ReasonNotRunning)), @@ -404,7 +421,7 @@ func TestIsSessionUnavailable(t *testing.T) { } for _, err := range []error{ seclient.ErrAccessDenied, - errNoUsername, + errors.New("no username for the session"), context.DeadlineExceeded, } { assert.Assert(t, !isSessionUnavailable(err), err)