Skip to content

Commit b4ca1c7

Browse files
committed
registry: don't call "/info" API endpoint to get default registry
The CLI currenly calls the `/info` endpoint to get the address of the default registry to use. This functionality was added as part of the initial Windows implementation of the engine. For legal reasons, Microsoft Windows (and thus Docker images based on Windows) were not allowed to be distributed through non-Microsoft infrastructure. As a temporary solution, a dedicated "registry-win-tp3.docker.io" registry was created to serve Windows images. As a result, the default registry was no longer "fixed", so a helper function (`ElectAuthServer`) was added to allow the CLI to get the correct registry address from the daemon. (docker/docker PR's/issues 18019, 19891, 19973) Using separate registries was not an ideal solution, and a more permanent solution was created by introducing "foreign image layers" in the distribution spec, after which the "registry-win-tp3.docker.io" ceased to exist, and removed from the engine through docker/docker PR 21100. However, the `ElectAuthServer` was left in place, quoting from that PR; > make the client check which default registry the daemon uses is still > more correct than leaving it up to the client, even if it won't technically > matter after this PR. There may be some backward compatibility scenarios > where `ElectAuthServer` [sic] is still helpful. That comment was 5 years ago, and given that the engine and cli are released in tandem, and the default registry is not configurable, we can save the extra roundtrip to the daemon by using a fixed value. This patch deprecates the `ElectAuthServer` function, and makes it return the default registry without calling (potentially expensie) `/info` API endpoint. Signed-off-by: Sebastiaan van Stijn <[email protected]>
1 parent 3304c49 commit b4ca1c7

File tree

4 files changed

+11
-85
lines changed

4 files changed

+11
-85
lines changed

cli/command/registry.go

Lines changed: 7 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import (
1212
"strings"
1313

1414
configtypes "github.com/docker/cli/cli/config/types"
15-
"github.com/docker/cli/cli/debug"
1615
"github.com/docker/cli/cli/streams"
1716
"github.com/docker/distribution/reference"
1817
"github.com/docker/docker/api/types"
@@ -22,28 +21,10 @@ import (
2221
"github.com/pkg/errors"
2322
)
2423

25-
// ElectAuthServer returns the default registry to use (by asking the daemon)
26-
func ElectAuthServer(ctx context.Context, cli Cli) string {
27-
// The daemon `/info` endpoint informs us of the default registry being
28-
// used. This is essential in cross-platforms environment, where for
29-
// example a Linux client might be interacting with a Windows daemon, hence
30-
// the default registry URL might be Windows specific.
31-
info, err := cli.Client().Info(ctx)
32-
if err != nil {
33-
// Daemon is not responding so use system default.
34-
if debug.IsEnabled() {
35-
// Only report the warning if we're in debug mode to prevent nagging during engine initialization workflows
36-
fmt.Fprintf(cli.Err(), "Warning: failed to get default registry endpoint from daemon (%v). Using system default: %s\n", err, registry.IndexServer)
37-
}
38-
return registry.IndexServer
39-
}
40-
if info.IndexServerAddress == "" {
41-
if debug.IsEnabled() {
42-
fmt.Fprintf(cli.Err(), "Warning: Empty registry endpoint from daemon. Using system default: %s\n", registry.IndexServer)
43-
}
44-
return registry.IndexServer
45-
}
46-
return info.IndexServerAddress
24+
// ElectAuthServer returns the default registry to use
25+
// Deprecated: use registry.IndexServer instead
26+
func ElectAuthServer(_ context.Context, _ Cli) string {
27+
return registry.IndexServer
4728
}
4829

4930
// EncodeAuthToBase64 serializes the auth configuration as JSON base64 payload
@@ -61,7 +42,7 @@ func RegistryAuthenticationPrivilegedFunc(cli Cli, index *registrytypes.IndexInf
6142
return func() (string, error) {
6243
fmt.Fprintf(cli.Out(), "\nPlease login prior to %s:\n", cmdName)
6344
indexServer := registry.GetAuthConfigKey(index)
64-
isDefaultRegistry := indexServer == ElectAuthServer(context.Background(), cli)
45+
isDefaultRegistry := indexServer == registry.IndexServer
6546
authConfig, err := GetDefaultAuthConfig(cli, true, indexServer, isDefaultRegistry)
6647
if err != nil {
6748
fmt.Fprintf(cli.Err(), "Unable to retrieve stored credentials for %s, error: %s.\n", indexServer, err)
@@ -77,10 +58,10 @@ func RegistryAuthenticationPrivilegedFunc(cli Cli, index *registrytypes.IndexInf
7758
// ResolveAuthConfig is like registry.ResolveAuthConfig, but if using the
7859
// default index, it uses the default index name for the daemon's platform,
7960
// not the client's platform.
80-
func ResolveAuthConfig(ctx context.Context, cli Cli, index *registrytypes.IndexInfo) types.AuthConfig {
61+
func ResolveAuthConfig(_ context.Context, cli Cli, index *registrytypes.IndexInfo) types.AuthConfig {
8162
configKey := index.Name
8263
if index.Official {
83-
configKey = ElectAuthServer(ctx, cli)
64+
configKey = registry.IndexServer
8465
}
8566

8667
a, _ := cli.ConfigFile().GetAuthConfig(configKey)

cli/command/registry/login.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -103,16 +103,15 @@ func runLogin(dockerCli command.Cli, opts loginOptions) error { //nolint: gocycl
103103
}
104104
var (
105105
serverAddress string
106-
authServer = command.ElectAuthServer(ctx, dockerCli)
106+
response registrytypes.AuthenticateOKBody
107107
)
108108
if opts.serverAddress != "" && opts.serverAddress != registry.DefaultNamespace {
109109
serverAddress = opts.serverAddress
110110
} else {
111-
serverAddress = authServer
111+
serverAddress = registry.IndexServer
112112
}
113113

114-
var response registrytypes.AuthenticateOKBody
115-
isDefaultRegistry := serverAddress == authServer
114+
isDefaultRegistry := serverAddress == registry.IndexServer
116115
authConfig, err := command.GetDefaultAuthConfig(dockerCli, opts.user == "" && opts.password == "", serverAddress, isDefaultRegistry)
117116
if err == nil && authConfig.Username != "" && authConfig.Password != "" {
118117
response, err = loginWithCredStoreCreds(ctx, dockerCli, &authConfig)

cli/command/registry/logout.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package registry
22

33
import (
4-
"context"
54
"fmt"
65

76
"github.com/docker/cli/cli"
@@ -30,11 +29,10 @@ func NewLogoutCommand(dockerCli command.Cli) *cobra.Command {
3029
}
3130

3231
func runLogout(dockerCli command.Cli, serverAddress string) error {
33-
ctx := context.Background()
3432
var isDefaultRegistry bool
3533

3634
if serverAddress == "" {
37-
serverAddress = command.ElectAuthServer(ctx, dockerCli)
35+
serverAddress = registry.IndexServer
3836
isDefaultRegistry = true
3937
}
4038

cli/command/registry_test.go

Lines changed: 0 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,13 @@ import (
66
"fmt"
77
"testing"
88

9-
"github.com/pkg/errors"
109
"gotest.tools/v3/assert"
1110
is "gotest.tools/v3/assert/cmp"
1211

1312
// Prevents a circular import with "github.com/docker/cli/internal/test"
1413

1514
. "github.com/docker/cli/cli/command"
1615
configtypes "github.com/docker/cli/cli/config/types"
17-
"github.com/docker/cli/cli/debug"
1816
"github.com/docker/cli/internal/test"
1917
"github.com/docker/docker/api/types"
2018
"github.com/docker/docker/client"
@@ -45,56 +43,6 @@ func (cli *fakeClient) Info(_ context.Context) (types.Info, error) {
4543
return types.Info{}, nil
4644
}
4745

48-
func TestElectAuthServer(t *testing.T) {
49-
testCases := []struct {
50-
expectedAuthServer string
51-
expectedWarning string
52-
infoFunc func() (types.Info, error)
53-
}{
54-
{
55-
expectedAuthServer: "https://index.docker.io/v1/",
56-
expectedWarning: "",
57-
infoFunc: func() (types.Info, error) {
58-
return types.Info{IndexServerAddress: "https://index.docker.io/v1/"}, nil
59-
},
60-
},
61-
{
62-
expectedAuthServer: "https://index.docker.io/v1/",
63-
expectedWarning: "Empty registry endpoint from daemon",
64-
infoFunc: func() (types.Info, error) {
65-
return types.Info{IndexServerAddress: ""}, nil
66-
},
67-
},
68-
{
69-
expectedAuthServer: "https://foo.example.com",
70-
expectedWarning: "",
71-
infoFunc: func() (types.Info, error) {
72-
return types.Info{IndexServerAddress: "https://foo.example.com"}, nil
73-
},
74-
},
75-
{
76-
expectedAuthServer: "https://index.docker.io/v1/",
77-
expectedWarning: "failed to get default registry endpoint from daemon",
78-
infoFunc: func() (types.Info, error) {
79-
return types.Info{}, errors.Errorf("error getting info")
80-
},
81-
},
82-
}
83-
// Enable debug to see warnings we're checking for
84-
debug.Enable()
85-
for _, tc := range testCases {
86-
cli := test.NewFakeCli(&fakeClient{infoFunc: tc.infoFunc})
87-
server := ElectAuthServer(context.Background(), cli)
88-
assert.Check(t, is.Equal(tc.expectedAuthServer, server))
89-
actual := cli.ErrBuffer().String()
90-
if tc.expectedWarning == "" {
91-
assert.Check(t, is.Len(actual, 0))
92-
} else {
93-
assert.Check(t, is.Contains(actual, tc.expectedWarning))
94-
}
95-
}
96-
}
97-
9846
func TestGetDefaultAuthConfig(t *testing.T) {
9947
testCases := []struct {
10048
checkCredStore bool

0 commit comments

Comments
 (0)