From 90b0cce07b4f68d8099903f7e1470f79541f45d0 Mon Sep 17 00:00:00 2001 From: Jake Moshenko Date: Thu, 20 Feb 2014 17:57:58 -0500 Subject: [PATCH] Fix registry auth by storing the string passed on the command line, and allowing for credential selection by normalizing on hostname. Also, remove remote ping calls from CmdPush and CmdPull. Docker-DCO-1.1-Signed-off-by: Jake Moshenko (github: jakedt) --- api/client.go | 26 ++++++++--------- auth/auth.go | 59 ++++++++++++++++----------------------- auth/auth_test.go | 7 +++++ registry/registry.go | 9 ++---- registry/registry_test.go | 2 +- server.go | 14 ++++++++-- 6 files changed, 60 insertions(+), 57 deletions(-) diff --git a/api/client.go b/api/client.go index eb345ae40b..f421c6366c 100644 --- a/api/client.go +++ b/api/client.go @@ -977,13 +977,13 @@ func (cli *DockerCli) CmdPush(args ...string) error { cli.LoadConfigFile() - // Resolve the Repository name from fqn to endpoint + name - endpoint, _, err := registry.ResolveRepositoryName(name) + // Resolve the Repository name from fqn to hostname + name + hostname, _, err := registry.ResolveRepositoryName(name) if err != nil { return err } // Resolve the Auth config relevant for this server - authConfig := cli.configFile.ResolveAuthConfig(endpoint) + authConfig := cli.configFile.ResolveAuthConfig(hostname) // If we're not using a custom registry, we know the restrictions // applied to repository names and can warn the user in advance. // Custom repositories can have different rules, and we must also @@ -1014,10 +1014,10 @@ func (cli *DockerCli) CmdPush(args ...string) error { if err := push(authConfig); err != nil { if strings.Contains(err.Error(), "Status 401") { fmt.Fprintln(cli.out, "\nPlease login prior to push:") - if err := cli.CmdLogin(endpoint); err != nil { + if err := cli.CmdLogin(hostname); err != nil { return err } - authConfig := cli.configFile.ResolveAuthConfig(endpoint) + authConfig := cli.configFile.ResolveAuthConfig(hostname) return push(authConfig) } return err @@ -1042,8 +1042,8 @@ func (cli *DockerCli) CmdPull(args ...string) error { *tag = parsedTag } - // Resolve the Repository name from fqn to endpoint + name - endpoint, _, err := registry.ResolveRepositoryName(remote) + // Resolve the Repository name from fqn to hostname + name + hostname, _, err := registry.ResolveRepositoryName(remote) if err != nil { return err } @@ -1051,7 +1051,7 @@ func (cli *DockerCli) CmdPull(args ...string) error { cli.LoadConfigFile() // Resolve the Auth config relevant for this server - authConfig := cli.configFile.ResolveAuthConfig(endpoint) + authConfig := cli.configFile.ResolveAuthConfig(hostname) v := url.Values{} v.Set("fromImage", remote) v.Set("tag", *tag) @@ -1073,10 +1073,10 @@ func (cli *DockerCli) CmdPull(args ...string) error { if err := pull(authConfig); err != nil { if strings.Contains(err.Error(), "Status 401") { fmt.Fprintln(cli.out, "\nPlease login prior to pull:") - if err := cli.CmdLogin(endpoint); err != nil { + if err := cli.CmdLogin(hostname); err != nil { return err } - authConfig := cli.configFile.ResolveAuthConfig(endpoint) + authConfig := cli.configFile.ResolveAuthConfig(hostname) return pull(authConfig) } return err @@ -1753,8 +1753,8 @@ func (cli *DockerCli) CmdRun(args ...string) error { v.Set("fromImage", repos) v.Set("tag", tag) - // Resolve the Repository name from fqn to endpoint + name - endpoint, _, err := registry.ResolveRepositoryName(repos) + // Resolve the Repository name from fqn to hostname + name + hostname, _, err := registry.ResolveRepositoryName(repos) if err != nil { return err } @@ -1763,7 +1763,7 @@ func (cli *DockerCli) CmdRun(args ...string) error { cli.LoadConfigFile() // Resolve the Auth config relevant for this server - authConfig := cli.configFile.ResolveAuthConfig(endpoint) + authConfig := cli.configFile.ResolveAuthConfig(hostname) buf, err := json.Marshal(authConfig) if err != nil { return err diff --git a/auth/auth.go b/auth/auth.go index cbca81f3e3..4417dd0f7a 100644 --- a/auth/auth.go +++ b/auth/auth.go @@ -252,50 +252,39 @@ func Login(authConfig *AuthConfig, factory *utils.HTTPRequestFactory) (string, e } // this method matches a auth configuration to a server address or a url -func (config *ConfigFile) ResolveAuthConfig(registry string) AuthConfig { - if registry == IndexServerAddress() || len(registry) == 0 { +func (config *ConfigFile) ResolveAuthConfig(hostname string) AuthConfig { + if hostname == IndexServerAddress() || len(hostname) == 0 { // default to the index server return config.Configs[IndexServerAddress()] } - // if it's not the index server there are three cases: - // - // 1. a full config url -> it should be used as is - // 2. a full url, but with the wrong protocol - // 3. a hostname, with an optional port - // - // as there is only one auth entry which is fully qualified we need to start - // parsing and matching - swapProtocol := func(url string) string { - if strings.HasPrefix(url, "http:") { - return strings.Replace(url, "http:", "https:", 1) - } - if strings.HasPrefix(url, "https:") { - return strings.Replace(url, "https:", "http:", 1) - } - return url + // First try the happy case + if c, found := config.Configs[hostname]; found { + return c } - resolveIgnoringProtocol := func(url string) AuthConfig { - if c, found := config.Configs[url]; found { - return c + convertToHostname := func(url string) string { + stripped := url + if strings.HasPrefix(url, "http://") { + stripped = strings.Replace(url, "http://", "", 1) + } else if strings.HasPrefix(url, "https://") { + stripped = strings.Replace(url, "https://", "", 1) } - registrySwappedProtocol := swapProtocol(url) - // now try to match with the different protocol - if c, found := config.Configs[registrySwappedProtocol]; found { - return c - } - return AuthConfig{} + + nameParts := strings.SplitN(stripped, "/", 2) + + return nameParts[0] } - // match both protocols as it could also be a server name like httpfoo - if strings.HasPrefix(registry, "http:") || strings.HasPrefix(registry, "https:") { - return resolveIgnoringProtocol(registry) + // Maybe they have a legacy config file, we will iterate the keys converting + // them to the new format and testing + normalizedHostename := convertToHostname(hostname) + for registry, config := range config.Configs { + if registryHostname := convertToHostname(registry); registryHostname == normalizedHostename { + return config + } } - url := "https://" + registry - if !strings.Contains(registry, "/") { - url = url + "/v1/" - } - return resolveIgnoringProtocol(url) + // When all else fails, return an empty auth config + return AuthConfig{} } diff --git a/auth/auth_test.go b/auth/auth_test.go index 5f2d3b85fd..2335072609 100644 --- a/auth/auth_test.go +++ b/auth/auth_test.go @@ -108,6 +108,7 @@ func TestResolveAuthConfigFullURL(t *testing.T) { } configFile.Configs["https://registry.example.com/v1/"] = registryAuth configFile.Configs["http://localhost:8000/v1/"] = localAuth + configFile.Configs["registry.com"] = registryAuth validRegistries := map[string][]string{ "https://registry.example.com/v1/": { @@ -122,6 +123,12 @@ func TestResolveAuthConfigFullURL(t *testing.T) { "localhost:8000", "localhost:8000/v1/", }, + "registry.com": { + "https://registry.com/v1/", + "http://registry.com/v1/", + "registry.com", + "registry.com/v1/", + }, } for configKey, registries := range validRegistries { diff --git a/registry/registry.go b/registry/registry.go index df94302305..37e107fad4 100644 --- a/registry/registry.go +++ b/registry/registry.go @@ -91,7 +91,7 @@ func validateRepositoryName(repositoryName string) error { return nil } -// Resolves a repository name to a endpoint + name +// Resolves a repository name to a hostname + name func ResolveRepositoryName(reposName string) (string, string, error) { if strings.Contains(reposName, "://") { // It cannot contain a scheme! @@ -117,11 +117,8 @@ func ResolveRepositoryName(reposName string) (string, string, error) { if err := validateRepositoryName(reposName); err != nil { return "", "", err } - endpoint, err := ExpandAndVerifyRegistryUrl(hostname) - if err != nil { - return "", "", err - } - return endpoint, reposName, err + + return hostname, reposName, nil } // this method expands the registry name as used in the prefix of a repo diff --git a/registry/registry_test.go b/registry/registry_test.go index 16bc431e55..5e398f9933 100644 --- a/registry/registry_test.go +++ b/registry/registry_test.go @@ -145,7 +145,7 @@ func TestResolveRepositoryName(t *testing.T) { if err != nil { t.Fatal(err) } - assertEqual(t, ep, "http://"+u+"/v1/", "Expected endpoint to be "+u) + assertEqual(t, ep, u, "Expected endpoint to be "+u) assertEqual(t, repo, "private/moonbase", "Expected endpoint to be private/moonbase") } diff --git a/server.go b/server.go index 0ed96fee31..7555830c0a 100644 --- a/server.go +++ b/server.go @@ -1336,7 +1336,12 @@ func (srv *Server) ImagePull(job *engine.Job) engine.Status { defer srv.poolRemove("pull", localName+":"+tag) // Resolve the Repository name from fqn to endpoint + name - endpoint, remoteName, err := registry.ResolveRepositoryName(localName) + hostname, remoteName, err := registry.ResolveRepositoryName(localName) + if err != nil { + return job.Error(err) + } + + endpoint, err := registry.ExpandAndVerifyRegistryUrl(hostname) if err != nil { return job.Error(err) } @@ -1538,7 +1543,12 @@ func (srv *Server) ImagePush(job *engine.Job) engine.Status { defer srv.poolRemove("push", localName) // Resolve the Repository name from fqn to endpoint + name - endpoint, remoteName, err := registry.ResolveRepositoryName(localName) + hostname, remoteName, err := registry.ResolveRepositoryName(localName) + if err != nil { + return job.Error(err) + } + + endpoint, err := registry.ExpandAndVerifyRegistryUrl(hostname) if err != nil { return job.Error(err) }