From ec9ac0867507eaa3da11f85d8a6eb9401121979c Mon Sep 17 00:00:00 2001 From: Alano Terblanche <18033717+Benehiko@users.noreply.github.com> Date: Thu, 22 Oct 2026 11:22:09 +0200 Subject: [PATCH] cli/command: test credential precedence with Docker Desktop Check that the Docker Desktop session is used for Docker Hub and dhi.io, that other registries use the configuration file, and that DOCKER_AUTH_CONFIG takes precedence over Docker Desktop. TestRetrieveAuthTokenFromImage disables the Docker Desktop lookup. Signed-off-by: Alano Terblanche <18033717+Benehiko@users.noreply.github.com> --- cli/command/container/create.go | 2 +- cli/command/image/pull.go | 2 +- cli/command/image/push.go | 2 +- cli/command/plugin/install.go | 2 +- cli/command/plugin/push.go | 2 +- cli/command/registry.go | 26 +- cli/command/registry_internal_test.go | 156 +++++++ cli/command/registry_test.go | 142 ++----- cli/command/service/create.go | 2 +- cli/command/service/update.go | 2 +- cli/command/stack/deploy_composefile.go | 2 +- internal/registry/config.go | 7 +- internal/secretsengine/engine_test.go | 404 ------------------- internal/secretsengine/secretsengine.go | 103 ++--- internal/secretsengine/secretsengine_test.go | 234 ++++++----- internal/test/cli.go | 17 +- 16 files changed, 412 insertions(+), 693 deletions(-) create mode 100644 cli/command/registry_internal_test.go delete mode 100644 internal/secretsengine/engine_test.go diff --git a/cli/command/container/create.go b/cli/command/container/create.go index 122d15e2e1..13e7dde4d4 100644 --- a/cli/command/container/create.go +++ b/cli/command/container/create.go @@ -133,7 +133,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.RetrieveAuthTokenFromImage(dockerCLI.ConfigFile(), img) + encodedAuth, err := command.EncodedRegistryAuth(dockerCLI, img) if err != nil { return err } diff --git a/cli/command/image/pull.go b/cli/command/image/pull.go index 7485d21a68..fdbe0814c8 100644 --- a/cli/command/image/pull.go +++ b/cli/command/image/pull.go @@ -90,7 +90,7 @@ func runPull(ctx context.Context, dockerCLI command.Cli, opts pullOptions) error ociPlatforms = append(ociPlatforms, p) } - encodedAuth, err := command.RetrieveAuthTokenFromImage(dockerCLI.ConfigFile(), distributionRef.String()) + encodedAuth, err := command.EncodedRegistryAuth(dockerCLI, distributionRef.String()) if err != nil { return err } diff --git a/cli/command/image/push.go b/cli/command/image/push.go index 87e2f7dea4..6a33a91eb5 100644 --- a/cli/command/image/push.go +++ b/cli/command/image/push.go @@ -108,7 +108,7 @@ To push the complete multi-platform image, remove the --platform flag. } // Resolve the Auth config relevant for this server - encodedAuth, err := command.RetrieveAuthTokenFromImage(dockerCli.ConfigFile(), ref.String()) + encodedAuth, err := command.EncodedRegistryAuth(dockerCli, ref.String()) if err != nil { return err } diff --git a/cli/command/plugin/install.go b/cli/command/plugin/install.go index 7d2d9aafae..0d457af7bc 100644 --- a/cli/command/plugin/install.go +++ b/cli/command/plugin/install.go @@ -60,7 +60,7 @@ func buildPullConfig(dockerCLI command.Cli, opts pluginOptions) (client.PluginIn return client.PluginInstallOptions{}, err } - encodedAuth, err := command.RetrieveAuthTokenFromImage(dockerCLI.ConfigFile(), ref.String()) + encodedAuth, err := command.EncodedRegistryAuth(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 af76db49eb..5c7e84dbe6 100644 --- a/cli/command/plugin/push.go +++ b/cli/command/plugin/push.go @@ -42,7 +42,7 @@ func runPush(ctx context.Context, dockerCli command.Cli, name string) error { } named = reference.TagNameOnly(named) - encodedAuth, err := command.RetrieveAuthTokenFromImage(dockerCli.ConfigFile(), named.String()) + encodedAuth, err := command.EncodedRegistryAuth(dockerCli, named.String()) if err != nil { return err } diff --git a/cli/command/registry.go b/cli/command/registry.go index 4893a9fa71..0a3df5ee79 100644 --- a/cli/command/registry.go +++ b/cli/command/registry.go @@ -36,8 +36,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/" -var desktopAuthConfig = secretsengine.AuthConfig - func envAuthConfig(registryHostname string) (configtypes.AuthConfig, bool) { if os.Getenv(configfile.DockerEnvConfigKey) == "" { return configtypes.AuthConfig{}, false @@ -206,6 +204,25 @@ 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 (cli *DockerCli) RegistryAuthProvider() secretsengine.AuthProvider { + return secretsengine.NewDesktop(cli.ConfigFile()) +} + +func retrieveAuthTokenFromImage(auth secretsengine.AuthProvider, image string) (string, error) { registryRef, err := reference.ParseNormalizedNamed(image) if err != nil { return "", err @@ -213,10 +230,7 @@ func RetrieveAuthTokenFromImage(cfg *configfile.ConfigFile, image string) (strin domain := reference.Domain(registryRef) authConfig, ok := envAuthConfig(domain) if !ok { - authConfig, ok = desktopAuthConfig(domain) - } - if !ok { - authConfig, err = cfg.GetAuthConfig(domain) + authConfig, err = auth.GetAuthConfig(domain) if err != nil { return "", err } diff --git a/cli/command/registry_internal_test.go b/cli/command/registry_internal_test.go new file mode 100644 index 0000000000..918036fe47 --- /dev/null +++ b/cli/command/registry_internal_test.go @@ -0,0 +1,156 @@ +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 751711f363..865e72ad54 100644 --- a/cli/command/registry_test.go +++ b/cli/command/registry_test.go @@ -1,14 +1,13 @@ 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" @@ -82,6 +81,35 @@ 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" @@ -95,113 +123,3 @@ 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 45406d7b26..b95f8c9dbd 100644 --- a/cli/command/service/create.go +++ b/cli/command/service/create.go @@ -120,7 +120,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.RetrieveAuthTokenFromImage(dockerCLI.ConfigFile(), opts.image) + encodedAuth, err = command.EncodedRegistryAuth(dockerCLI, opts.image) if err != nil { return err } diff --git a/cli/command/service/update.go b/cli/command/service/update.go index 2e7a0ee25f..35ffe9fb9f 100644 --- a/cli/command/service/update.go +++ b/cli/command/service/update.go @@ -223,7 +223,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.RetrieveAuthTokenFromImage(dockerCLI.ConfigFile(), spec.TaskTemplate.ContainerSpec.Image) + encodedAuth, err = command.EncodedRegistryAuth(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 ad85765133..01041008bc 100644 --- a/cli/command/stack/deploy_composefile.go +++ b/cli/command/stack/deploy_composefile.go @@ -224,7 +224,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.RetrieveAuthTokenFromImage(dockerCLI.ConfigFile(), image) + encodedAuth, err = command.EncodedRegistryAuth(dockerCLI, image) if err != nil { return nil, err } diff --git a/internal/registry/config.go b/internal/registry/config.go index 24b5eedfe3..062940f627 100644 --- a/internal/registry/config.go +++ b/internal/registry/config.go @@ -51,7 +51,10 @@ const ( // IndexServer is used for user auth and image search IndexServer = "https://index.docker.io/v1/" // IndexName is the name of the index - IndexName = "docker.io" + IndexName = "docker.io" + DefaultRegistryHost = "registry-1.docker.io" + StagingRegistryHost = "registry-1-stage.docker.io" + DHIRegistryHost = "dhi.io" ) var ( @@ -61,7 +64,7 @@ var ( // or "ubuntu:latest"), as well as references using the "docker.io" domain // name, which is used as canonical reference for images on Docker Hub, but // does not match the domain-name of Docker Hub's registry. - DefaultV2Registry = &url.URL{Scheme: "https", Host: "registry-1.docker.io"} + DefaultV2Registry = &url.URL{Scheme: "https", Host: DefaultRegistryHost} validHostPortRegex = sync.OnceValue(func() *regexp.Regexp { return regexp.MustCompile(`^` + reference.DomainRegexp.String() + `$`) diff --git a/internal/secretsengine/engine_test.go b/internal/secretsengine/engine_test.go deleted file mode 100644 index 8518247690..0000000000 --- a/internal/secretsengine/engine_test.go +++ /dev/null @@ -1,404 +0,0 @@ -//go:build go1.26 - -package secretsengine - -import ( - "context" - "encoding/base64" - "encoding/json" - "fmt" - "net" - "net/http" - "os" - "path/filepath" - "sync" - "sync/atomic" - "testing" - "time" - - seclient "github.com/docker/secrets-engine/client" - "github.com/docker/secrets-engine/client/dockerhub" - "github.com/docker/secrets-engine/x/api/resolver" - "github.com/docker/secrets-engine/x/api/resolver/v1/resolverv1connect" - "github.com/docker/secrets-engine/x/secrets" - "gotest.tools/v3/assert" -) - -const benchSocketEnv = "SECRETSENGINE_BENCH_SOCKET" - -type fakeResolver struct { - latency time.Duration - values map[string][]byte - calls atomic.Int64 -} - -func (r *fakeResolver) GetSecrets(ctx context.Context, pattern secrets.Pattern) ([]secrets.Envelope, error) { - r.calls.Add(1) - if r.latency > 0 { - select { - case <-time.After(r.latency): - case <-ctx.Done(): - return nil, ctx.Err() - } - } - var envelopes []secrets.Envelope - for key, value := range r.values { - id := secrets.MustParseID(key) - if pattern.Match(id) { - envelopes = append(envelopes, secrets.Envelope{ID: id, Value: value, ResolvedAt: time.Now()}) - } - } - if len(envelopes) == 0 { - return nil, secrets.ErrNotFound - } - return envelopes, nil -} - -func fakeAccessToken() string { - enc := base64.RawURLEncoding.EncodeToString - return enc([]byte(`{"alg":"RS256","typ":"JWT"}`)) + "." + enc([]byte(`{"sub":"bench"}`)) + "." + enc([]byte("signature")) -} - -func signedIn(tb testing.TB, expiresAt time.Time) map[string][]byte { - tb.Helper() - const username = "hubuser" - userID := "docker/auth/hub/" + username - profile, err := json.Marshal(dockerhub.Profile{UserID: userID, Username: username, Email: username + "@example.com"}) - assert.NilError(tb, err) - s := dockerhub.UserSession{AccessToken: fakeAccessToken()} - s.Claims.Username = username - s.Claims.ExpiresAt = &dockerhub.NumericDate{Time: expiresAt} - userSession, err := json.Marshal(s) - assert.NilError(tb, err) - return map[string][]byte{ - "docker/auth/metadata/hub/default": profile, - userID: userSession, - } -} - -var socketDirs atomic.Int64 - -func socketDir(tb testing.TB) string { - tb.Helper() - dir := filepath.Join(os.TempDir(), fmt.Sprintf("se-%d-%d", os.Getpid(), socketDirs.Add(1))) - assert.NilError(tb, os.Mkdir(dir, 0o700)) - tb.Cleanup(func() { _ = os.RemoveAll(dir) }) - return dir -} - -type fakeEngine struct { - socketPath string - - mu sync.Mutex - conns map[net.Conn]struct{} -} - -func startEngine(tb testing.TB, r *fakeResolver) *fakeEngine { - tb.Helper() - e := &fakeEngine{ - socketPath: filepath.Join(socketDir(tb), "engine.sock"), - conns: map[net.Conn]struct{}{}, - } - ln, err := net.Listen("unix", e.socketPath) - assert.NilError(tb, err) - mux := http.NewServeMux() - mux.Handle(resolverv1connect.NewResolverServiceHandler(resolver.NewResolverHandler(r))) - srv := &http.Server{Handler: mux, ReadHeaderTimeout: 5 * time.Second, ConnState: e.track} - go func() { _ = srv.Serve(ln) }() - tb.Cleanup(func() { _ = srv.Close() }) - return e -} - -func (e *fakeEngine) track(c net.Conn, state http.ConnState) { - e.mu.Lock() - defer e.mu.Unlock() - switch state { - case http.StateNew: - e.conns[c] = struct{}{} - case http.StateClosed, http.StateHijacked: - delete(e.conns, c) - case http.StateActive, http.StateIdle: - } -} - -func (e *fakeEngine) dropConnections() { - e.mu.Lock() - defer e.mu.Unlock() - for c := range e.conns { - _ = c.Close() - } - clear(e.conns) -} - -func startHungEngine(tb testing.TB) string { - tb.Helper() - socketPath := filepath.Join(socketDir(tb), "engine.sock") - ln, err := net.Listen("unix", socketPath) - assert.NilError(tb, err) - var mu sync.Mutex - var held []net.Conn - go func() { - for { - c, err := ln.Accept() - if err != nil { - return - } - mu.Lock() - held = append(held, c) - mu.Unlock() - } - }() - tb.Cleanup(func() { - _ = ln.Close() - mu.Lock() - defer mu.Unlock() - for _, c := range held { - _ = c.Close() - } - }) - return socketPath -} - -func staleSocket(tb testing.TB) string { - tb.Helper() - socketPath := filepath.Join(socketDir(tb), "engine.sock") - ln, err := net.ListenUnix("unix", &net.UnixAddr{Name: socketPath, Net: "unix"}) - assert.NilError(tb, err) - ln.SetUnlinkOnClose(false) - assert.NilError(tb, ln.Close()) - _, err = os.Lstat(socketPath) - assert.NilError(tb, err) - return socketPath -} - -func socketSession(socketPath string, timeout time.Duration) *session { - return &session{ - name: "Docker Hub", - connect: func() (hubSessions, error) { - return connect(socketPath) - }, - timeout: timeout, - now: time.Now, - } -} - -func TestEngineSignedIn(t *testing.T) { - r := &fakeResolver{values: signedIn(t, time.Now().Add(time.Hour))} - e := startEngine(t, r) - p := &provider{hub: socketSession(e.socketPath, lookupTimeout)} - - got, ok := p.authConfig("docker.io") - assert.Assert(t, ok) - assert.Equal(t, got.Username, "hubuser") - assert.Equal(t, got.Password, fakeAccessToken()) - assert.Equal(t, got.ServerAddress, indexServer) - assert.Equal(t, got.IdentityToken, "") - - got, ok = p.authConfig("dhi.io") - assert.Assert(t, ok) - assert.Equal(t, got.ServerAddress, dhiRegistryHost) - assert.Equal(t, r.calls.Load(), int64(2), "the profile and the session are read once, then cached") -} - -func TestEngineSignedOut(t *testing.T) { - r := &fakeResolver{} - e := startEngine(t, r) - p := &provider{hub: socketSession(e.socketPath, lookupTimeout)} - - _, ok := p.authConfig("docker.io") - assert.Assert(t, !ok) - assert.Equal(t, r.calls.Load(), int64(1)) -} - -func TestMissingSocketFallsBackImmediately(t *testing.T) { - s := socketSession(filepath.Join(socketDir(t), "engine.sock"), lookupTimeout) - - start := time.Now() - _, ok := s.auth() - assert.Assert(t, !ok) - assert.Assert(t, time.Since(start) < time.Second) - assert.Assert(t, s.hub == nil, "no client is created without a socket") -} - -func TestStaleSocketFallsBackImmediately(t *testing.T) { - s := socketSession(staleSocket(t), lookupTimeout) - - start := time.Now() - _, ok := s.auth() - assert.Assert(t, !ok) - assert.Assert(t, time.Since(start) < time.Second) -} - -func TestHungEngineIsBounded(t *testing.T) { - const timeout = 200 * time.Millisecond - s := socketSession(startHungEngine(t), timeout) - - start := time.Now() - _, ok := s.auth() - elapsed := time.Since(start) - assert.Assert(t, !ok) - assert.Assert(t, elapsed >= timeout, elapsed) - assert.Assert(t, elapsed < timeout+2*time.Second, elapsed) -} - -func benchmarkColdLookup(b *testing.B, socketPath string, e *fakeEngine, want bool) { - b.Helper() - b.ReportAllocs() - for range b.N { - s := socketSession(socketPath, lookupTimeout) - if _, ok := s.auth(); ok != want { - b.Fatalf("got a Desktop session: %t, want %t", ok, want) - } - if e != nil { - b.StopTimer() - e.dropConnections() - b.StartTimer() - } - } -} - -func BenchmarkAuthConfig(b *testing.B) { - b.Run("other-registry", func(b *testing.B) { - p := &provider{hub: socketSession(filepath.Join(socketDir(b), "engine.sock"), lookupTimeout)} - b.ReportAllocs() - for range b.N { - if _, ok := p.authConfig("ghcr.io"); ok { - b.Fatal("unexpected Desktop session") - } - } - }) - b.Run("missing-socket", func(b *testing.B) { - benchmarkColdLookup(b, filepath.Join(socketDir(b), "engine.sock"), nil, false) - }) - b.Run("missing-socket-without-stat", func(b *testing.B) { - socketPath := filepath.Join(socketDir(b), "engine.sock") - b.ReportAllocs() - for range b.N { - c, err := seclient.New(seclient.WithSocketPath(socketPath), seclient.WithTimeout(lookupTimeout)) - if err != nil { - b.Fatal(err) - } - if _, err := c.HubAuth().GetDefaultSession(context.Background()); err == nil { - b.Fatal("unexpected Desktop session") - } - } - }) - b.Run("stale-socket", func(b *testing.B) { - benchmarkColdLookup(b, staleSocket(b), nil, false) - }) - b.Run("signed-out", func(b *testing.B) { - e := startEngine(b, &fakeResolver{}) - benchmarkColdLookup(b, e.socketPath, e, false) - }) - b.Run("signed-in", func(b *testing.B) { - e := startEngine(b, &fakeResolver{values: signedIn(b, time.Now().Add(time.Hour))}) - benchmarkColdLookup(b, e.socketPath, e, true) - }) - b.Run("signed-in-10ms-engine", func(b *testing.B) { - e := startEngine(b, &fakeResolver{latency: 10 * time.Millisecond, values: signedIn(b, time.Now().Add(time.Hour))}) - benchmarkColdLookup(b, e.socketPath, e, true) - }) - b.Run("signed-in-refetch", func(b *testing.B) { - e := startEngine(b, &fakeResolver{values: signedIn(b, time.Now().Add(time.Hour))}) - s := socketSession(e.socketPath, lookupTimeout) - if _, ok := s.auth(); !ok { - b.Fatal("no Desktop session") - } - b.ReportAllocs() - b.ResetTimer() - for range b.N { - s.cached = nil - if _, ok := s.auth(); !ok { - b.Fatal("no Desktop session") - } - } - }) - b.Run("signed-in-cached", func(b *testing.B) { - e := startEngine(b, &fakeResolver{values: signedIn(b, time.Now().Add(time.Hour))}) - s := socketSession(e.socketPath, lookupTimeout) - if _, ok := s.auth(); !ok { - b.Fatal("no Desktop session") - } - b.ReportAllocs() - b.ResetTimer() - for range b.N { - if _, ok := s.auth(); !ok { - b.Fatal("no Desktop session") - } - } - }) -} - -type trackingDialer struct { - mu sync.Mutex - conns []net.Conn -} - -func (d *trackingDialer) dial(socketPath string) func(ctx context.Context, _, _ string) (net.Conn, error) { - return func(ctx context.Context, _, _ string) (net.Conn, error) { - var nd net.Dialer - c, err := nd.DialContext(ctx, "unix", socketPath) - if err != nil { - return nil, err - } - d.mu.Lock() - d.conns = append(d.conns, c) - d.mu.Unlock() - return c, nil - } -} - -func (d *trackingDialer) closeAll() { - d.mu.Lock() - defer d.mu.Unlock() - for _, c := range d.conns { - _ = c.Close() - } - d.conns = nil -} - -func BenchmarkExternalEngine(b *testing.B) { - socketPath := os.Getenv(benchSocketEnv) - if socketPath == "" { - b.Skipf("%s is not set", benchSocketEnv) - } - b.Run("cold", func(b *testing.B) { - b.ReportAllocs() - for range b.N { - var dialer trackingDialer - s := &session{ - name: "Docker Hub", - connect: func() (hubSessions, error) { - c, err := seclient.New(seclient.WithDialContext(dialer.dial(socketPath)), seclient.WithTimeout(lookupTimeout)) - if err != nil { - return nil, err - } - return c.HubAuth(), nil - }, - timeout: lookupTimeout, - now: time.Now, - } - _, ok := s.auth() - b.StopTimer() - dialer.closeAll() - if !ok { - b.Fatal("no Docker Hub session from the engine at " + socketPath) - } - b.StartTimer() - } - }) - b.Run("refetch", func(b *testing.B) { - s := socketSession(socketPath, lookupTimeout) - if _, ok := s.auth(); !ok { - b.Fatal("no Docker Hub session from the engine at " + socketPath) - } - b.ReportAllocs() - b.ResetTimer() - for range b.N { - s.cached = nil - if _, ok := s.auth(); !ok { - b.Fatal("no Docker Hub session from the engine at " + socketPath) - } - } - }) -} diff --git a/internal/secretsengine/secretsengine.go b/internal/secretsengine/secretsengine.go index 6840c5d53c..41a322d919 100644 --- a/internal/secretsengine/secretsengine.go +++ b/internal/secretsengine/secretsengine.go @@ -9,11 +9,11 @@ import ( "io/fs" "os" "sync" - "sync/atomic" "time" "github.com/docker/cli/cli/config/credentials" "github.com/docker/cli/cli/config/types" + "github.com/docker/cli/internal/registry" seclient "github.com/docker/secrets-engine/client" "github.com/docker/secrets-engine/client/dockerhub" "github.com/docker/secrets-engine/x/api" @@ -25,10 +25,6 @@ const ( sessionExpirySkew = 30 * time.Second sessionTTL = time.Minute retryInterval = time.Minute - - indexServer = "https://index.docker.io/v1/" - dhiRegistryHost = "dhi.io" - stagingRegistryHost = "registry-1-stage.docker.io" ) var ( @@ -36,57 +32,62 @@ var ( errSessionExpired = errors.New("the session has expired (not signed in to Docker Desktop?)") ) -type hubSessions interface { +type HubClient interface { GetDefaultSession(ctx context.Context) (dockerhub.UserSession, error) GetDefaultProfile(ctx context.Context) (dockerhub.Profile, error) } -var _ hubSessions = dockerhub.ClientAuth(nil) +var _ HubClient = dockerhub.ClientAuth(nil) -type provider struct { - hub *session - staging *session +type AuthProvider interface { + GetAuthConfig(registryHostname string) (types.AuthConfig, error) } -var shared = &provider{ - hub: newSession("Docker Hub"), - staging: newSession("Docker Hub staging", dockerhub.Staging()), +type Provider struct { + fallback AuthProvider + hub *session + staging *session } -var disabled atomic.Bool - -func Disable() { - disabled.Store(true) -} - -func AuthConfig(registryHostname string) (types.AuthConfig, bool) { - if disabled.Load() { - return types.AuthConfig{}, false +func New(fallback AuthProvider, hub, staging HubClient) *Provider { + return &Provider{ + fallback: fallback, + hub: newSession("Docker Hub", hub), + staging: newSession("Docker Hub staging", staging), } - return shared.authConfig(registryHostname) } -func (p *provider) authConfig(registryHostname string) (types.AuthConfig, bool) { - s, serverAddress := p.sessionFor(registryHostname) - if s == nil { - return types.AuthConfig{}, false +var ( + desktopHubSession = newDesktopSession("Docker Hub") + desktopStagingSession = newDesktopSession("Docker Hub staging", dockerhub.Staging()) +) + +func NewDesktop(fallback AuthProvider) *Provider { + return &Provider{ + fallback: fallback, + hub: desktopHubSession, + staging: desktopStagingSession, } - auth, ok := s.auth() - if !ok { - return types.AuthConfig{}, false - } - auth.ServerAddress = serverAddress - return auth, true } -func (p *provider) sessionFor(registryHostname string) (*session, string) { +func (p *Provider) GetAuthConfig(registryHostname string) (types.AuthConfig, error) { + if s, serverAddress := p.sessionFor(registryHostname); s != nil { + if auth, ok := s.auth(); ok { + auth.ServerAddress = serverAddress + return auth, nil + } + } + return p.fallback.GetAuthConfig(registryHostname) +} + +func (p *Provider) sessionFor(registryHostname string) (*session, string) { switch credentials.ConvertToHostname(registryHostname) { - case "docker.io", "index.docker.io", "registry-1.docker.io": - return p.hub, indexServer - case dhiRegistryHost: - return p.hub, dhiRegistryHost - case stagingRegistryHost: - return p.staging, stagingRegistryHost + 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 default: return nil, "" } @@ -94,29 +95,35 @@ func (p *provider) sessionFor(registryHostname string) (*session, string) { type session struct { name string - connect func() (hubSessions, error) + connect func() (HubClient, error) timeout time.Duration now func() time.Time mu sync.Mutex - hub hubSessions + hub HubClient retryAt time.Time cached *types.AuthConfig validUntil time.Time } -func newSession(name string, opts ...dockerhub.Option) *session { +func newSession(name string, hub HubClient) *session { return &session{ - name: name, - connect: func() (hubSessions, error) { - return connect(api.DesktopSocketPath(), opts...) - }, + name: name, + hub: hub, timeout: lookupTimeout, now: time.Now, } } -func connect(socketPath string, opts ...dockerhub.Option) (hubSessions, error) { +func newDesktopSession(name string, opts ...dockerhub.Option) *session { + s := newSession(name, nil) + s.connect = func() (HubClient, error) { + return connect(api.DesktopSocketPath(), opts...) + } + return s +} + +func connect(socketPath string, opts ...dockerhub.Option) (HubClient, error) { if _, err := os.Lstat(socketPath); errors.Is(err, fs.ErrNotExist) { return nil, err } diff --git a/internal/secretsengine/secretsengine_test.go b/internal/secretsengine/secretsengine_test.go index 71b799c825..06a3bd16c6 100644 --- a/internal/secretsengine/secretsengine_test.go +++ b/internal/secretsengine/secretsengine_test.go @@ -14,6 +14,7 @@ import ( "time" "github.com/docker/cli/cli/config/types" + "github.com/docker/cli/internal/registry" seclient "github.com/docker/secrets-engine/client" "github.com/docker/secrets-engine/client/dockerhub" "gotest.tools/v3/assert" @@ -64,47 +65,47 @@ func hubSession(username string, expiresAt time.Time) dockerhub.UserSession { return s } +type recordingFallback struct { + mu sync.Mutex + hosts []string +} + +func (f *recordingFallback) GetAuthConfig(registryHostname string) (types.AuthConfig, error) { + f.mu.Lock() + defer f.mu.Unlock() + f.hosts = append(f.hosts, registryHostname) + return cliCredentials(registryHostname), nil +} + +func cliCredentials(registryHostname string) types.AuthConfig { + return types.AuthConfig{Username: "cli-user", Password: "cli-password", ServerAddress: registryHostname} +} + func desktopCredentials() types.AuthConfig { - return types.AuthConfig{Username: "hubuser", Password: "desktop-oauth-token", ServerAddress: indexServer} + return types.AuthConfig{Username: "hubuser", Password: "desktop-oauth-token", ServerAddress: registry.IndexServer} } func stagingCredentials() types.AuthConfig { - return types.AuthConfig{Username: "stageuser", Password: "staging-oauth-token", ServerAddress: stagingRegistryHost} + return types.AuthConfig{Username: "stageuser", Password: "staging-oauth-token", ServerAddress: registry.StagingRegistryHost} } type testProvider struct { - *provider + *Provider + fallback *recordingFallback clock *fakeClock - connects atomic.Int32 staging *fakeHub } -func newTestProvider(hub hubSessions, connectErr error) *testProvider { +func newTestProvider(hub HubClient) *testProvider { tp := &testProvider{ - clock: &fakeClock{t: testNow}, - staging: &fakeHub{sessionErr: dockerhub.ErrNoDefaultProfile}, + fallback: &recordingFallback{}, + clock: &fakeClock{t: testNow}, + staging: &fakeHub{sessionErr: dockerhub.ErrNoDefaultProfile}, } - tp.provider = &provider{ - hub: &session{ - name: "Docker Hub", - connect: func() (hubSessions, error) { - tp.connects.Add(1) - if connectErr != nil { - return nil, connectErr - } - return hub, nil - }, - timeout: time.Second, - now: tp.clock.now, - }, - staging: &session{ - name: "Docker Hub staging", - connect: func() (hubSessions, error) { - return tp.staging, nil - }, - timeout: time.Second, - now: tp.clock.now, - }, + tp.Provider = New(tp.fallback, hub, tp.staging) + for _, s := range []*session{tp.Provider.hub, tp.Provider.staging} { + s.timeout = time.Second + s.now = tp.clock.now } return tp } @@ -123,75 +124,68 @@ func connectError(reason seclient.ConnectReason) error { } } -func TestDisableSkipsDockerDesktop(t *testing.T) { - Disable() - - _, ok := AuthConfig("docker.io") - assert.Assert(t, !ok) - - shared.hub.mu.Lock() - defer shared.hub.mu.Unlock() - assert.Assert(t, shared.hub.retryAt.IsZero(), "Docker Desktop must not be contacted once disabled") - assert.Assert(t, shared.hub.hub == nil) -} - -func TestSharedProviderHasOneSessionPerEnvironment(t *testing.T) { - assert.Assert(t, shared.hub != nil) - assert.Assert(t, shared.staging != nil) - assert.Assert(t, shared.hub != shared.staging, "production and staging sessions must be separate") +func TestNewDesktopSharesOneSessionPerEnvironment(t *testing.T) { + a := NewDesktop(&recordingFallback{}) + b := NewDesktop(&recordingFallback{}) + 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 TestDockerHubUsesDesktopSession(t *testing.T) { - for _, host := range []string{"docker.io", "index.docker.io", "registry-1.docker.io", indexServer} { + 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))} - p := newTestProvider(hub, nil) + p := newTestProvider(hub) - got, ok := p.authConfig(host) - assert.Assert(t, ok) + got, err := p.GetAuthConfig(host) + assert.NilError(t, err) assert.DeepEqual(t, got, desktopCredentials()) assert.Equal(t, got.IdentityToken, "", "an IdentityToken is sent as a refresh token") + assert.Equal(t, len(p.fallback.hosts), 0) }) } } -func TestOtherRegistriesDoNotContactDesktop(t *testing.T) { +func TestOtherRegistriesUseFallback(t *testing.T) { hosts := []string{ "ghcr.io", "localhost:5000", "registry.example.com", "", - stagingRegistryHost + ".example.com", "example.com/" + stagingRegistryHost, - dhiRegistryHost + ".example.com", "example.com/" + dhiRegistryHost, "registry." + dhiRegistryHost, + registry.StagingRegistryHost + ".example.com", "example.com/" + registry.StagingRegistryHost, + registry.DHIRegistryHost + ".example.com", "example.com/" + registry.DHIRegistryHost, "registry." + 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))} - p := newTestProvider(hub, nil) + p := newTestProvider(hub) p.signInToStaging() - _, ok := p.authConfig(host) - assert.Assert(t, !ok) - assert.Equal(t, p.connects.Load(), int32(0), "Docker Desktop must only be contacted for Docker Hub") + got, err := p.GetAuthConfig(host) + assert.NilError(t, err) + assert.DeepEqual(t, got, cliCredentials(host)) + assert.DeepEqual(t, p.fallback.hosts, []string{host}) + assert.Equal(t, hub.sessionCalls.Load(), int32(0), "Docker Desktop must only be contacted for Docker Hub") assert.Equal(t, p.staging.sessionCalls.Load(), int32(0), "Docker Desktop must only be contacted for Docker Hub") }) } } func TestDHIUsesDockerHubSession(t *testing.T) { - for _, host := range []string{dhiRegistryHost, "https://" + dhiRegistryHost, "https://" + dhiRegistryHost + "/"} { + for _, host := range []string{registry.DHIRegistryHost, "https://" + registry.DHIRegistryHost, "https://" + registry.DHIRegistryHost + "/"} { t.Run(host, func(t *testing.T) { hub := &fakeHub{session: hubSession("hubuser", testNow.Add(time.Hour))} - p := newTestProvider(hub, nil) + p := newTestProvider(hub) p.signInToStaging() want := desktopCredentials() - want.ServerAddress = dhiRegistryHost - got, ok := p.authConfig(host) - assert.Assert(t, ok) + want.ServerAddress = registry.DHIRegistryHost + got, err := p.GetAuthConfig(host) + assert.NilError(t, err) assert.DeepEqual(t, got, want) assert.Equal(t, p.staging.sessionCalls.Load(), int32(0), "the staging session must not be read for DHI") - got, ok = p.authConfig("docker.io") - assert.Assert(t, ok) + got, err = p.GetAuthConfig("docker.io") + assert.NilError(t, err) assert.DeepEqual(t, got, desktopCredentials()) assert.Equal(t, hub.sessionCalls.Load(), int32(1)) }) @@ -199,14 +193,14 @@ func TestDHIUsesDockerHubSession(t *testing.T) { } func TestStagingRegistryUsesStagingSession(t *testing.T) { - for _, host := range []string{stagingRegistryHost, "https://" + stagingRegistryHost + "/"} { + for _, host := range []string{registry.StagingRegistryHost, "https://" + registry.StagingRegistryHost + "/"} { t.Run(host, func(t *testing.T) { hub := &fakeHub{session: hubSession("hubuser", testNow.Add(time.Hour))} - p := newTestProvider(hub, nil) + p := newTestProvider(hub) p.signInToStaging() - got, ok := p.authConfig(host) - assert.Assert(t, ok) + got, err := p.GetAuthConfig(host) + assert.NilError(t, err) assert.DeepEqual(t, got, stagingCredentials()) assert.Equal(t, hub.sessionCalls.Load(), int32(0), "the production session must not be read for the staging registry") }) @@ -215,25 +209,27 @@ func TestStagingRegistryUsesStagingSession(t *testing.T) { func TestEnvironmentsDoNotMix(t *testing.T) { t.Run("staging session is not used for docker.io", func(t *testing.T) { - p := newTestProvider(&fakeHub{sessionErr: dockerhub.ErrNoDefaultProfile}, nil) + p := newTestProvider(&fakeHub{sessionErr: dockerhub.ErrNoDefaultProfile}) p.signInToStaging() - _, ok := p.authConfig("docker.io") - assert.Assert(t, !ok) + got, err := p.GetAuthConfig("docker.io") + assert.NilError(t, err) + assert.DeepEqual(t, got, cliCredentials("docker.io")) - got, ok := p.authConfig(stagingRegistryHost) - assert.Assert(t, ok) + got, err = p.GetAuthConfig(registry.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))}, nil) + p := newTestProvider(&fakeHub{session: hubSession("hubuser", testNow.Add(time.Hour))}) - _, ok := p.authConfig(stagingRegistryHost) - assert.Assert(t, !ok) + got, err := p.GetAuthConfig(registry.StagingRegistryHost) + assert.NilError(t, err) + assert.DeepEqual(t, got, cliCredentials(registry.StagingRegistryHost)) - got, ok := p.authConfig("docker.io") - assert.Assert(t, ok) + got, err = p.GetAuthConfig("docker.io") + assert.NilError(t, err) assert.DeepEqual(t, got, desktopCredentials()) }) } @@ -243,10 +239,10 @@ func TestUsernameFromProfile(t *testing.T) { session: hubSession("", testNow.Add(time.Hour)), profile: dockerhub.Profile{Username: "hubuser"}, } - p := newTestProvider(hub, nil) + p := newTestProvider(hub) - got, ok := p.authConfig("docker.io") - assert.Assert(t, ok) + got, err := p.GetAuthConfig("docker.io") + assert.NilError(t, err) assert.DeepEqual(t, got, desktopCredentials()) } @@ -267,45 +263,55 @@ func TestFallsBackWithoutUsableSession(t *testing.T) { } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - p := newTestProvider(tc.hub, nil) + p := newTestProvider(tc.hub) for range 2 { - _, ok := p.authConfig("docker.io") - assert.Assert(t, !ok) + 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") }) } } -func TestFallsBackWhenClientCannotBeCreated(t *testing.T) { - p := newTestProvider(nil, errors.New("no socket path")) +func TestDesktopFallsBackWhenClientCannotBeCreated(t *testing.T) { + var connects atomic.Int32 + s := newSession("Docker Hub", nil) + s.connect = func() (HubClient, error) { + connects.Add(1) + return nil, errors.New("no socket path") + } + p := &Provider{fallback: &recordingFallback{}, hub: s} for range 2 { - _, ok := p.authConfig("docker.io") - assert.Assert(t, !ok) + got, err := p.GetAuthConfig("docker.io") + assert.NilError(t, err) + assert.DeepEqual(t, got, cliCredentials("docker.io")) } - assert.Equal(t, p.connects.Load(), int32(1)) + assert.Equal(t, connects.Load(), int32(1)) } func TestRetriesAfterInterval(t *testing.T) { hub := &fakeHub{sessionErr: dockerhub.ErrNoDefaultProfile} - p := newTestProvider(hub, nil) + p := newTestProvider(hub) - _, ok := p.authConfig("docker.io") - assert.Assert(t, !ok) + 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)) p.clock.advance(retryInterval - time.Second) - _, ok = p.authConfig("docker.io") - assert.Assert(t, !ok) + 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, ok := p.authConfig("docker.io") - assert.Assert(t, ok) + got, err = p.GetAuthConfig("docker.io") + assert.NilError(t, err) assert.DeepEqual(t, got, desktopCredentials()) assert.Equal(t, hub.sessionCalls.Load(), int32(2)) } @@ -313,53 +319,58 @@ func TestRetriesAfterInterval(t *testing.T) { func TestReusesSessionUntilExpiry(t *testing.T) { expiresAt := testNow.Add(10 * time.Minute) hub := &fakeHub{session: hubSession("hubuser", expiresAt)} - p := newTestProvider(hub, nil) + p := newTestProvider(hub) - _, ok := p.authConfig("docker.io") - assert.Assert(t, ok) + got, err := p.GetAuthConfig("docker.io") + assert.NilError(t, err) + assert.DeepEqual(t, got, desktopCredentials()) p.clock.advance(5 * time.Minute) - _, ok = p.authConfig("docker.io") - assert.Assert(t, ok) + 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) - got, ok := p.authConfig("docker.io") - assert.Assert(t, ok) + 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, nil) + p := newTestProvider(hub) - _, ok := p.authConfig("docker.io") - assert.Assert(t, ok) + got, err := p.GetAuthConfig("docker.io") + assert.NilError(t, err) + assert.DeepEqual(t, got, desktopCredentials()) p.clock.advance(sessionTTL - time.Second) - _, ok = p.authConfig("docker.io") - assert.Assert(t, ok) + 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) - _, ok = p.authConfig("docker.io") - assert.Assert(t, ok) + 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))} - p := newTestProvider(hub, nil) + p := newTestProvider(hub) const lookups = 20 results := make([]types.AuthConfig, lookups) var wg sync.WaitGroup for i := range lookups { wg.Go(func() { - auth, ok := p.authConfig("docker.io") - assert.Check(t, ok) + auth, err := p.GetAuthConfig("docker.io") + assert.Check(t, err) results[i] = auth }) } @@ -369,7 +380,6 @@ func TestConcurrentLookupsShareOneFetch(t *testing.T) { assert.DeepEqual(t, auth, desktopCredentials()) } assert.Equal(t, hub.sessionCalls.Load(), int32(1)) - assert.Equal(t, p.connects.Load(), int32(1)) } func TestConnectSkipsMissingSocket(t *testing.T) { diff --git a/internal/test/cli.go b/internal/test/cli.go index 151dcadad0..fdf4a4b584 100644 --- a/internal/test/cli.go +++ b/internal/test/cli.go @@ -2,6 +2,7 @@ package test import ( "bytes" + "context" "io" "strings" @@ -13,6 +14,7 @@ import ( "github.com/docker/cli/cli/streams" "github.com/docker/cli/internal/registryclient" "github.com/docker/cli/internal/secretsengine" + "github.com/docker/secrets-engine/client/dockerhub" "github.com/moby/moby/client" ) @@ -36,7 +38,6 @@ type FakeCli struct { // NewFakeCli returns a fake for the command.Cli interface func NewFakeCli(apiClient client.APIClient, opts ...func(*FakeCli)) *FakeCli { - secretsengine.Disable() outBuffer := new(bytes.Buffer) errBuffer := new(bytes.Buffer) c := &FakeCli{ @@ -178,6 +179,20 @@ func (c *FakeCli) SetRegistryClient(registryClient registryclient.RegistryClient c.registryClient = registryClient } +type signedOutHub struct{} + +func (signedOutHub) GetDefaultSession(context.Context) (dockerhub.UserSession, error) { + return dockerhub.UserSession{}, dockerhub.ErrNoSession +} + +func (signedOutHub) GetDefaultProfile(context.Context) (dockerhub.Profile, error) { + return dockerhub.Profile{}, dockerhub.ErrNoSession +} + +func (c *FakeCli) RegistryAuthProvider() secretsengine.AuthProvider { + return secretsengine.New(c.configfile, signedOutHub{}, signedOutHub{}) +} + // BuildKitEnabled on the fake cli func (*FakeCli) BuildKitEnabled() (bool, error) { return true, nil