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 <jake@devtable.com> (github: jakedt)

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

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{}
-	}
 
-	// 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)
+		nameParts := strings.SplitN(stripped, "/", 2)
+
+		return nameParts[0]
 	}
 
-	url := "https://" + registry
-	if !strings.Contains(registry, "/") {
-		url = url + "/v1/"
+	// 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
+		}
 	}
-	return resolveIgnoringProtocol(url)
+
+	// When all else fails, return an empty auth config
+	return AuthConfig{}
 }

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 {

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

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")
 }
 

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)
 	}