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)