Merge pull request #2819 from thaJeztah/remove_registry_negotiation

registry: don't call "/info" API endpoint to get default registry
This commit is contained in:
Sebastiaan van Stijn 2022-03-15 17:26:56 +01:00 committed by GitHub
commit 1f4111d2bf
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 11 additions and 85 deletions

View File

@ -12,7 +12,6 @@ import (
"strings" "strings"
configtypes "github.com/docker/cli/cli/config/types" configtypes "github.com/docker/cli/cli/config/types"
"github.com/docker/cli/cli/debug"
"github.com/docker/cli/cli/streams" "github.com/docker/cli/cli/streams"
"github.com/docker/distribution/reference" "github.com/docker/distribution/reference"
"github.com/docker/docker/api/types" "github.com/docker/docker/api/types"
@ -22,28 +21,10 @@ import (
"github.com/pkg/errors" "github.com/pkg/errors"
) )
// ElectAuthServer returns the default registry to use (by asking the daemon) // ElectAuthServer returns the default registry to use
func ElectAuthServer(ctx context.Context, cli Cli) string { // Deprecated: use registry.IndexServer instead
// The daemon `/info` endpoint informs us of the default registry being func ElectAuthServer(_ context.Context, _ Cli) string {
// used. This is essential in cross-platforms environment, where for
// example a Linux client might be interacting with a Windows daemon, hence
// the default registry URL might be Windows specific.
info, err := cli.Client().Info(ctx)
if err != nil {
// Daemon is not responding so use system default.
if debug.IsEnabled() {
// Only report the warning if we're in debug mode to prevent nagging during engine initialization workflows
fmt.Fprintf(cli.Err(), "Warning: failed to get default registry endpoint from daemon (%v). Using system default: %s\n", err, registry.IndexServer)
}
return registry.IndexServer return registry.IndexServer
}
if info.IndexServerAddress == "" {
if debug.IsEnabled() {
fmt.Fprintf(cli.Err(), "Warning: Empty registry endpoint from daemon. Using system default: %s\n", registry.IndexServer)
}
return registry.IndexServer
}
return info.IndexServerAddress
} }
// EncodeAuthToBase64 serializes the auth configuration as JSON base64 payload // EncodeAuthToBase64 serializes the auth configuration as JSON base64 payload
@ -61,7 +42,7 @@ func RegistryAuthenticationPrivilegedFunc(cli Cli, index *registrytypes.IndexInf
return func() (string, error) { return func() (string, error) {
fmt.Fprintf(cli.Out(), "\nPlease login prior to %s:\n", cmdName) fmt.Fprintf(cli.Out(), "\nPlease login prior to %s:\n", cmdName)
indexServer := registry.GetAuthConfigKey(index) indexServer := registry.GetAuthConfigKey(index)
isDefaultRegistry := indexServer == ElectAuthServer(context.Background(), cli) isDefaultRegistry := indexServer == registry.IndexServer
authConfig, err := GetDefaultAuthConfig(cli, true, indexServer, isDefaultRegistry) authConfig, err := GetDefaultAuthConfig(cli, true, indexServer, isDefaultRegistry)
if err != nil { if err != nil {
fmt.Fprintf(cli.Err(), "Unable to retrieve stored credentials for %s, error: %s.\n", indexServer, err) 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
// ResolveAuthConfig is like registry.ResolveAuthConfig, but if using the // ResolveAuthConfig is like registry.ResolveAuthConfig, but if using the
// default index, it uses the default index name for the daemon's platform, // default index, it uses the default index name for the daemon's platform,
// not the client's platform. // not the client's platform.
func ResolveAuthConfig(ctx context.Context, cli Cli, index *registrytypes.IndexInfo) types.AuthConfig { func ResolveAuthConfig(_ context.Context, cli Cli, index *registrytypes.IndexInfo) types.AuthConfig {
configKey := index.Name configKey := index.Name
if index.Official { if index.Official {
configKey = ElectAuthServer(ctx, cli) configKey = registry.IndexServer
} }
a, _ := cli.ConfigFile().GetAuthConfig(configKey) a, _ := cli.ConfigFile().GetAuthConfig(configKey)

View File

@ -103,16 +103,15 @@ func runLogin(dockerCli command.Cli, opts loginOptions) error { //nolint: gocycl
} }
var ( var (
serverAddress string serverAddress string
authServer = command.ElectAuthServer(ctx, dockerCli) response registrytypes.AuthenticateOKBody
) )
if opts.serverAddress != "" && opts.serverAddress != registry.DefaultNamespace { if opts.serverAddress != "" && opts.serverAddress != registry.DefaultNamespace {
serverAddress = opts.serverAddress serverAddress = opts.serverAddress
} else { } else {
serverAddress = authServer serverAddress = registry.IndexServer
} }
var response registrytypes.AuthenticateOKBody isDefaultRegistry := serverAddress == registry.IndexServer
isDefaultRegistry := serverAddress == authServer
authConfig, err := command.GetDefaultAuthConfig(dockerCli, opts.user == "" && opts.password == "", serverAddress, isDefaultRegistry) authConfig, err := command.GetDefaultAuthConfig(dockerCli, opts.user == "" && opts.password == "", serverAddress, isDefaultRegistry)
if err == nil && authConfig.Username != "" && authConfig.Password != "" { if err == nil && authConfig.Username != "" && authConfig.Password != "" {
response, err = loginWithCredStoreCreds(ctx, dockerCli, &authConfig) response, err = loginWithCredStoreCreds(ctx, dockerCli, &authConfig)

View File

@ -1,7 +1,6 @@
package registry package registry
import ( import (
"context"
"fmt" "fmt"
"github.com/docker/cli/cli" "github.com/docker/cli/cli"
@ -30,11 +29,10 @@ func NewLogoutCommand(dockerCli command.Cli) *cobra.Command {
} }
func runLogout(dockerCli command.Cli, serverAddress string) error { func runLogout(dockerCli command.Cli, serverAddress string) error {
ctx := context.Background()
var isDefaultRegistry bool var isDefaultRegistry bool
if serverAddress == "" { if serverAddress == "" {
serverAddress = command.ElectAuthServer(ctx, dockerCli) serverAddress = registry.IndexServer
isDefaultRegistry = true isDefaultRegistry = true
} }

View File

@ -6,7 +6,6 @@ import (
"fmt" "fmt"
"testing" "testing"
"github.com/pkg/errors"
"gotest.tools/v3/assert" "gotest.tools/v3/assert"
is "gotest.tools/v3/assert/cmp" is "gotest.tools/v3/assert/cmp"
@ -14,7 +13,6 @@ import (
. "github.com/docker/cli/cli/command" . "github.com/docker/cli/cli/command"
configtypes "github.com/docker/cli/cli/config/types" configtypes "github.com/docker/cli/cli/config/types"
"github.com/docker/cli/cli/debug"
"github.com/docker/cli/internal/test" "github.com/docker/cli/internal/test"
"github.com/docker/docker/api/types" "github.com/docker/docker/api/types"
"github.com/docker/docker/client" "github.com/docker/docker/client"
@ -45,56 +43,6 @@ func (cli *fakeClient) Info(_ context.Context) (types.Info, error) {
return types.Info{}, nil return types.Info{}, nil
} }
func TestElectAuthServer(t *testing.T) {
testCases := []struct {
expectedAuthServer string
expectedWarning string
infoFunc func() (types.Info, error)
}{
{
expectedAuthServer: "https://index.docker.io/v1/",
expectedWarning: "",
infoFunc: func() (types.Info, error) {
return types.Info{IndexServerAddress: "https://index.docker.io/v1/"}, nil
},
},
{
expectedAuthServer: "https://index.docker.io/v1/",
expectedWarning: "Empty registry endpoint from daemon",
infoFunc: func() (types.Info, error) {
return types.Info{IndexServerAddress: ""}, nil
},
},
{
expectedAuthServer: "https://foo.example.com",
expectedWarning: "",
infoFunc: func() (types.Info, error) {
return types.Info{IndexServerAddress: "https://foo.example.com"}, nil
},
},
{
expectedAuthServer: "https://index.docker.io/v1/",
expectedWarning: "failed to get default registry endpoint from daemon",
infoFunc: func() (types.Info, error) {
return types.Info{}, errors.Errorf("error getting info")
},
},
}
// Enable debug to see warnings we're checking for
debug.Enable()
for _, tc := range testCases {
cli := test.NewFakeCli(&fakeClient{infoFunc: tc.infoFunc})
server := ElectAuthServer(context.Background(), cli)
assert.Check(t, is.Equal(tc.expectedAuthServer, server))
actual := cli.ErrBuffer().String()
if tc.expectedWarning == "" {
assert.Check(t, is.Len(actual, 0))
} else {
assert.Check(t, is.Contains(actual, tc.expectedWarning))
}
}
}
func TestGetDefaultAuthConfig(t *testing.T) { func TestGetDefaultAuthConfig(t *testing.T) {
testCases := []struct { testCases := []struct {
checkCredStore bool checkCredStore bool