Merge pull request #3599 from thaJeztah/use_local_ConvertToHostname

use local ConvertToHostname() implementation

cli/command/registry.go

@@ -11,6 +11,7 @@ import (
 
 	"github.com/distribution/reference"
 	"github.com/docker/cli/cli/config/configfile"
+	"github.com/docker/cli/cli/config/credentials"
 	configtypes "github.com/docker/cli/cli/config/types"
 	"github.com/docker/cli/cli/hints"
 	"github.com/docker/cli/cli/streams"
@@ -71,7 +72,7 @@ func ResolveAuthConfig(cfg *configfile.ConfigFile, index *registrytypes.IndexInf
 // If credentials for given serverAddress exists in the credential store, the configuration will be populated with values in it
 func GetDefaultAuthConfig(cfg *configfile.ConfigFile, checkCredStore bool, serverAddress string, isDefaultRegistry bool) (registrytypes.AuthConfig, error) {
 	if !isDefaultRegistry {
-		serverAddress = registry.ConvertToHostname(serverAddress)
+		serverAddress = credentials.ConvertToHostname(serverAddress)
 	}
 	authconfig := configtypes.AuthConfig{}
 	var err error

cli/command/registry/logout.go

@@ -6,6 +6,7 @@ import (
 
 	"github.com/docker/cli/cli"
 	"github.com/docker/cli/cli/command"
+	"github.com/docker/cli/cli/config/credentials"
 	"github.com/docker/docker/registry"
 	"github.com/spf13/cobra"
 )
@@ -46,7 +47,7 @@ func runLogout(_ context.Context, dockerCli command.Cli, serverAddress string) e
 		hostnameAddress = serverAddress
 	)
 	if !isDefaultRegistry {
-		hostnameAddress = registry.ConvertToHostname(serverAddress)
+		hostnameAddress = credentials.ConvertToHostname(serverAddress)
 		// the tries below are kept for backward compatibility where a user could have
 		// saved the registry in one of the following format.
 		regsToLogout = append(regsToLogout, hostnameAddress, "http://"+hostnameAddress, "https://"+hostnameAddress)

cli/config/credentials/file_store.go

@@ -1,6 +1,7 @@
 package credentials
 
 import (
+	"net/url"
 	"strings"
 
 	"github.com/docker/cli/cli/config/types"
@@ -68,14 +69,14 @@ func (c *fileStore) IsFileStore() bool {
 // ConvertToHostname converts a registry url which has http|https prepended
 // to just an hostname.
 // Copied from github.com/docker/docker/registry.ConvertToHostname to reduce dependencies.
-func ConvertToHostname(url string) string {
-	stripped := url
-	if strings.HasPrefix(url, "http://") {
-		stripped = strings.TrimPrefix(url, "http://")
-	} else if strings.HasPrefix(url, "https://") {
-		stripped = strings.TrimPrefix(url, "https://")
+func ConvertToHostname(maybeURL string) string {
+	stripped := maybeURL
+	if strings.Contains(stripped, "://") {
+		u, err := url.Parse(stripped)
+		if err == nil && u.Hostname() != "" {
+			return u.Hostname()
+		}
 	}
-
 	hostName, _, _ := strings.Cut(stripped, "/")
 	return hostName
 }

cli/config/credentials/file_store_test.go

@@ -134,3 +134,45 @@ func TestFileStoreErase(t *testing.T) {
 		t.Fatalf("expected empty email, got %s", a.Email)
 	}
 }
+
+func TestConvertToHostname(t *testing.T) {
+	tests := []struct{ input, expected string }{
+		{
+			input:    "example.com",
+			expected: "example.com",
+		},
+		{
+			input:    "http://example.com",
+			expected: "example.com",
+		},
+		{
+			input:    "https://example.com",
+			expected: "example.com",
+		},
+		{
+			input:    "https://example.com/",
+			expected: "example.com",
+		},
+		{
+			input:    "https://example.com/v2/",
+			expected: "example.com",
+		},
+		{
+			// FIXME(thaJeztah): should ConvertToHostname correctly handle this / fail on this?
+			input:    "unix:///var/run/docker.sock",
+			expected: "unix:",
+		},
+		{
+			// FIXME(thaJeztah): should ConvertToHostname correctly handle this?
+			input:    "ftp://example.com",
+			expected: "example.com",
+		},
+	}
+	for _, tc := range tests {
+		tc := tc
+		t.Run(tc.input, func(t *testing.T) {
+			actual := ConvertToHostname(tc.input)
+			assert.Equal(t, actual, tc.expected)
+		})
+	}
+}

cli/registry/client/client.go

@@ -101,23 +101,23 @@ func (c *client) MountBlob(ctx context.Context, sourceRef reference.Canonical, t
 func (c *client) PutManifest(ctx context.Context, ref reference.Named, manifest distribution.Manifest) (digest.Digest, error) {
 	repoEndpoint, err := newDefaultRepositoryEndpoint(ref, c.insecureRegistry)
 	if err != nil {
-		return digest.Digest(""), err
+		return "", err
 	}
 
 	repoEndpoint.actions = trust.ActionsPushAndPull
 	repo, err := c.getRepositoryForReference(ctx, ref, repoEndpoint)
 	if err != nil {
-		return digest.Digest(""), err
+		return "", err
 	}
 
 	manifestService, err := repo.Manifests(ctx)
 	if err != nil {
-		return digest.Digest(""), err
+		return "", err
 	}
 
 	_, opts, err := getManifestOptionsFromReference(ref)
 	if err != nil {
-		return digest.Digest(""), err
+		return "", err
 	}
 
 	dgst, err := manifestService.Put(ctx, manifest, opts...)

cli/registry/client/fetcher.go

@@ -270,7 +270,7 @@ func (c *client) iterateEndpoints(ctx context.Context, namedRef reference.Named,
 	return newNotFoundError(namedRef.String())
 }
 
-// allEndpoints returns a list of endpoints ordered by priority (v2, https, v1).
+// allEndpoints returns a list of endpoints ordered by priority (v2, http).
 func allEndpoints(namedRef reference.Named, insecure bool) ([]registry.APIEndpoint, error) {
 	repoInfo, err := registry.ParseRepositoryInfo(namedRef)
 	if err != nil {