Merge pull request #5969 from thaJeztah/simplify_auth_fixed

cli/command: Reapply "remove uses of GetAuthConfigKey, ParseRepositoryInfo" and add test

Author: thaJeztah

cli/command/registry.go

@@ -15,7 +15,6 @@ import (
 	"github.com/docker/cli/cli/streams"
 	"github.com/docker/cli/internal/tui"
 	registrytypes "github.com/docker/docker/api/types/registry"
-	"github.com/docker/docker/registry"
 	"github.com/morikuni/aec"
 	"github.com/pkg/errors"
 )
@@ -28,16 +27,22 @@ const (
 		"for organizations using SSO. Learn more at https://docs.docker.com/go/access-tokens/"
 )
 
+// authConfigKey is the key used to store credentials for Docker Hub. It is
+// a copy of [registry.IndexServer].
+//
+// [registry.IndexServer]: https://pkg.go.dev/github.com/docker/docker/registry#IndexServer
+const authConfigKey = "https://index.docker.io/v1/"
+
 // RegistryAuthenticationPrivilegedFunc returns a RequestPrivilegeFunc from the specified registry index info
 // for the given command.
 func RegistryAuthenticationPrivilegedFunc(cli Cli, index *registrytypes.IndexInfo, cmdName string) registrytypes.RequestAuthConfig {
+	configKey := getAuthConfigKey(index.Name)
+	isDefaultRegistry := configKey == authConfigKey || index.Official
 	return func(ctx context.Context) (string, error) {
 		_, _ = fmt.Fprintf(cli.Out(), "\nLogin prior to %s:\n", cmdName)
-		indexServer := registry.GetAuthConfigKey(index)
-		isDefaultRegistry := indexServer == registry.IndexServer
-		authConfig, err := GetDefaultAuthConfig(cli.ConfigFile(), true, indexServer, isDefaultRegistry)
+		authConfig, err := GetDefaultAuthConfig(cli.ConfigFile(), true, configKey, isDefaultRegistry)
 		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", authConfigKey, err)
 		}
 
 		select {
@@ -46,7 +51,7 @@ func RegistryAuthenticationPrivilegedFunc(cli Cli, index *registrytypes.IndexInf
 		default:
 		}
 
-		authConfig, err = PromptUserForCredentials(ctx, cli, "", "", authConfig.Username, indexServer)
+		authConfig, err = PromptUserForCredentials(ctx, cli, "", "", authConfig.Username, authConfigKey)
 		if err != nil {
 			return "", err
 		}
@@ -63,7 +68,7 @@ func RegistryAuthenticationPrivilegedFunc(cli Cli, index *registrytypes.IndexInf
 func ResolveAuthConfig(cfg *configfile.ConfigFile, index *registrytypes.IndexInfo) registrytypes.AuthConfig {
 	configKey := index.Name
 	if index.Official {
-		configKey = registry.IndexServer
+		configKey = authConfigKey
 	}
 
 	a, _ := cfg.GetAuthConfig(configKey)
@@ -132,7 +137,7 @@ func PromptUserForCredentials(ctx context.Context, cli Cli, argUser, argPassword
 
 	argUser = strings.TrimSpace(argUser)
 	if argUser == "" {
-		if serverAddress == registry.IndexServer {
+		if serverAddress == authConfigKey {
 			// When signing in to the default (Docker Hub) registry, we display
 			// hints for creating an account, and (if hints are enabled), using
 			// a token instead of a password.
@@ -225,9 +230,25 @@ func resolveAuthConfigFromImage(cfg *configfile.ConfigFile, image string) (regis
 	if err != nil {
 		return registrytypes.AuthConfig{}, err
 	}
-	repoInfo, err := registry.ParseRepositoryInfo(registryRef)
+	configKey := getAuthConfigKey(reference.Domain(registryRef))
+	a, err := cfg.GetAuthConfig(configKey)
 	if err != nil {
 		return registrytypes.AuthConfig{}, err
 	}
-	return ResolveAuthConfig(cfg, repoInfo.Index), nil
+	return registrytypes.AuthConfig(a), nil
+}
+
+// getAuthConfigKey special-cases using the full index address of the official
+// index as the AuthConfig key, and uses the (host)name[:port] for private indexes.
+//
+// It is similar to [registry.GetAuthConfigKey], but does not require on
+// [registrytypes.IndexInfo] as intermediate.
+//
+// [registry.GetAuthConfigKey]: https://pkg.go.dev/github.com/docker/docker/registry#GetAuthConfigKey
+// [registrytypes.IndexInfo]:https://pkg.go.dev/github.com/docker/docker/api/types/registry#IndexInfo
+func getAuthConfigKey(domainName string) string {
+	if domainName == "docker.io" || domainName == "index.docker.io" {
+		return authConfigKey
+	}
+	return domainName
 }

cli/command/registry_test.go

@@ -1,6 +1,8 @@
 package command_test
 
 import (
+	"bytes"
+	"path"
 	"testing"
 
 	"github.com/docker/cli/cli/command"
@@ -80,3 +82,113 @@ func TestGetDefaultAuthConfig_HelperError(t *testing.T) {
 	assert.Check(t, is.DeepEqual(expectedAuthConfig, authconfig))
 	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)
+				ac, err := registry.DecodeAuthConfig(actual)
+				assert.NilError(t, err)
+				assert.Check(t, is.DeepEqual(*ac, tc.expectedAuthCfg))
+			}
+		})
+	}
+}