Merge pull request #4270 from DevTable/fixregistryauth

Fix registry auth and remove other hidden ping commands from client code

api/client.go

@@ -992,13 +992,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
@@ -1029,10 +1029,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
@@ -1057,8 +1057,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
 	}
@@ -1066,7 +1066,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)
@@ -1088,10 +1088,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
@@ -1768,8 +1768,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
 		}
@@ -1778,7 +1778,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{}
+
+		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{}
 }

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

@@ -92,7 +92,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!
@@ -118,11 +118,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

@@ -1331,7 +1331,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)
 	}
@@ -1534,7 +1539,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)
 	}