From 59e40e63479576e8e40b185c317e23baeda0c34b Mon Sep 17 00:00:00 2001 From: Alano Terblanche <18033717+Benehiko@users.noreply.github.com> Date: Fri, 23 Oct 2026 08:29:09 +0200 Subject: [PATCH] internal/secretsengine: unexport what only tests use Provider, HubClient, and New were only used inside the package and by FakeCli. FakeCli now returns its configuration file as the auth provider, which is what the signed-out Docker Hub stub fell back to. NewDesktop returns an AuthProvider. Remove DockerCli.RegistryAuthProvider: EncodedRegistryAuth already falls back to RetrieveAuthTokenFromImage, which uses the Docker Desktop session, so the method only added to the public API. Signed-off-by: Alano Terblanche <18033717+Benehiko@users.noreply.github.com> --- cli/command/registry.go | 4 --- internal/secretsengine/secretsengine.go | 28 ++++++++++---------- internal/secretsengine/secretsengine_test.go | 16 +++++------ internal/test/cli.go | 14 +--------- 4 files changed, 23 insertions(+), 39 deletions(-) diff --git a/cli/command/registry.go b/cli/command/registry.go index 0a3df5ee79..41ebc6d8ae 100644 --- a/cli/command/registry.go +++ b/cli/command/registry.go @@ -218,10 +218,6 @@ func EncodedRegistryAuth(dockerCLI Cli, image string) (string, error) { 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 { diff --git a/internal/secretsengine/secretsengine.go b/internal/secretsengine/secretsengine.go index 41a322d919..f614a02293 100644 --- a/internal/secretsengine/secretsengine.go +++ b/internal/secretsengine/secretsengine.go @@ -32,25 +32,25 @@ var ( errSessionExpired = errors.New("the session has expired (not signed in to Docker Desktop?)") ) -type HubClient interface { +type hubClient interface { GetDefaultSession(ctx context.Context) (dockerhub.UserSession, error) GetDefaultProfile(ctx context.Context) (dockerhub.Profile, error) } -var _ HubClient = dockerhub.ClientAuth(nil) +var _ hubClient = dockerhub.ClientAuth(nil) type AuthProvider interface { GetAuthConfig(registryHostname string) (types.AuthConfig, error) } -type Provider struct { +type provider struct { fallback AuthProvider hub *session staging *session } -func New(fallback AuthProvider, hub, staging HubClient) *Provider { - return &Provider{ +func newProvider(fallback AuthProvider, hub, staging hubClient) *provider { + return &provider{ fallback: fallback, hub: newSession("Docker Hub", hub), staging: newSession("Docker Hub staging", staging), @@ -62,15 +62,15 @@ var ( desktopStagingSession = newDesktopSession("Docker Hub staging", dockerhub.Staging()) ) -func NewDesktop(fallback AuthProvider) *Provider { - return &Provider{ +func NewDesktop(fallback AuthProvider) AuthProvider { + return &provider{ fallback: fallback, hub: desktopHubSession, staging: desktopStagingSession, } } -func (p *Provider) GetAuthConfig(registryHostname string) (types.AuthConfig, error) { +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 @@ -80,7 +80,7 @@ func (p *Provider) GetAuthConfig(registryHostname string) (types.AuthConfig, err return p.fallback.GetAuthConfig(registryHostname) } -func (p *Provider) sessionFor(registryHostname string) (*session, string) { +func (p *provider) sessionFor(registryHostname string) (*session, string) { switch credentials.ConvertToHostname(registryHostname) { case registry.IndexName, registry.IndexHostname, registry.DefaultRegistryHost: return p.hub, registry.IndexServer @@ -95,18 +95,18 @@ func (p *Provider) sessionFor(registryHostname string) (*session, string) { type session struct { name string - connect func() (HubClient, error) + connect func() (hubClient, error) timeout time.Duration now func() time.Time mu sync.Mutex - hub HubClient + hub hubClient retryAt time.Time cached *types.AuthConfig validUntil time.Time } -func newSession(name string, hub HubClient) *session { +func newSession(name string, hub hubClient) *session { return &session{ name: name, hub: hub, @@ -117,13 +117,13 @@ func newSession(name string, hub HubClient) *session { func newDesktopSession(name string, opts ...dockerhub.Option) *session { s := newSession(name, nil) - s.connect = func() (HubClient, error) { + s.connect = func() (hubClient, error) { return connect(api.DesktopSocketPath(), opts...) } return s } -func connect(socketPath string, opts ...dockerhub.Option) (HubClient, error) { +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 06a3bd16c6..c185e53cc2 100644 --- a/internal/secretsengine/secretsengine_test.go +++ b/internal/secretsengine/secretsengine_test.go @@ -90,20 +90,20 @@ func stagingCredentials() types.AuthConfig { } type testProvider struct { - *Provider + *provider fallback *recordingFallback clock *fakeClock staging *fakeHub } -func newTestProvider(hub HubClient) *testProvider { +func newTestProvider(hub hubClient) *testProvider { tp := &testProvider{ fallback: &recordingFallback{}, clock: &fakeClock{t: testNow}, staging: &fakeHub{sessionErr: dockerhub.ErrNoDefaultProfile}, } - tp.Provider = New(tp.fallback, hub, tp.staging) - for _, s := range []*session{tp.Provider.hub, tp.Provider.staging} { + 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 } @@ -125,8 +125,8 @@ func connectError(reason seclient.ConnectReason) error { } func TestNewDesktopSharesOneSessionPerEnvironment(t *testing.T) { - a := NewDesktop(&recordingFallback{}) - b := NewDesktop(&recordingFallback{}) + 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") @@ -278,11 +278,11 @@ func TestFallsBackWithoutUsableSession(t *testing.T) { func TestDesktopFallsBackWhenClientCannotBeCreated(t *testing.T) { var connects atomic.Int32 s := newSession("Docker Hub", nil) - s.connect = func() (HubClient, error) { + s.connect = func() (hubClient, error) { connects.Add(1) return nil, errors.New("no socket path") } - p := &Provider{fallback: &recordingFallback{}, hub: s} + p := &provider{fallback: &recordingFallback{}, hub: s} for range 2 { got, err := p.GetAuthConfig("docker.io") diff --git a/internal/test/cli.go b/internal/test/cli.go index fdf4a4b584..5b431437b7 100644 --- a/internal/test/cli.go +++ b/internal/test/cli.go @@ -2,7 +2,6 @@ package test import ( "bytes" - "context" "io" "strings" @@ -14,7 +13,6 @@ 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" ) @@ -179,18 +177,8 @@ 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{}) + return c.configfile } // BuildKitEnabled on the fake cli