From 8376b3e42826fb37bdc6994d5bb9affbf13f7b43 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 26 Feb 2022 20:10:38 +0100 Subject: [PATCH] use local ConvertToHostname() implementation Commit 27b2797f7deb3ca5b7f80371d825113deb1faca1 added a local implementation of this function, so let's use the local variant to (slightly) reduce the dependency on moby's registry package. Also made some minor cleanups. Signed-off-by: Sebastiaan van Stijn --- cli/command/registry.go | 3 +- cli/command/registry/logout.go | 3 +- cli/config/credentials/file_store.go | 15 ++++---- cli/config/credentials/file_store_test.go | 42 +++++++++++++++++++++++ cli/registry/client/client.go | 8 ++--- cli/registry/client/fetcher.go | 2 +- 6 files changed, 59 insertions(+), 14 deletions(-) diff --git a/cli/command/registry.go b/cli/command/registry.go index 08602c3725..ba97861a6b 100644 --- a/cli/command/registry.go +++ b/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 diff --git a/cli/command/registry/logout.go b/cli/command/registry/logout.go index 6f47327cdf..580a3b8746 100644 --- a/cli/command/registry/logout.go +++ b/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) diff --git a/cli/config/credentials/file_store.go b/cli/config/credentials/file_store.go index ea30fc3006..99631bbf4c 100644 --- a/cli/config/credentials/file_store.go +++ b/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 } diff --git a/cli/config/credentials/file_store_test.go b/cli/config/credentials/file_store_test.go index f04fe22e1c..74d63afcd6 100644 --- a/cli/config/credentials/file_store_test.go +++ b/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) + }) + } +} diff --git a/cli/registry/client/client.go b/cli/registry/client/client.go index b2955d1bce..bbc7f4c584 100644 --- a/cli/registry/client/client.go +++ b/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...) diff --git a/cli/registry/client/fetcher.go b/cli/registry/client/fetcher.go index 093211d89a..3e4b36d309 100644 --- a/cli/registry/client/fetcher.go +++ b/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 {