registry: split normalizing index name from validating

ValidateIndexName is used by the docker daemon CLI to validate options
passed through CLI flags and daemon.json. However, it also handled
normalizing the registry name ("index.docker.io" -> "docker.io").

This patch splits the normalization code to a separate function. It
is currently not exported, but could be considered in the future;
if we do so, we may want to look for a better place for that function
to not have it in the same package as the registry code.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

registry/config.go

@@ -318,16 +318,22 @@ func ValidateMirror(val string) (string, error) {
 // ValidateIndexName validates an index name. It is used by the daemon to
 // validate the daemon configuration.
 func ValidateIndexName(val string) (string, error) {
-	// TODO: upstream this to check to reference package
-	if val == "index.docker.io" {
-		val = "docker.io"
-	}
+	val = normalizeIndexName(val)
 	if strings.HasPrefix(val, "-") || strings.HasSuffix(val, "-") {
 		return "", invalidParamf("invalid index name (%s). Cannot begin or end with a hyphen", val)
 	}
 	return val, nil
 }
 
+func normalizeIndexName(val string) string {
+	// TODO(thaJeztah): consider normalizing other known options, such as "(https://)registry-1.docker.io", "https://index.docker.io/v1/".
+	// TODO: upstream this to check to reference package
+	if val == "index.docker.io" {
+		return "docker.io"
+	}
+	return val
+}
+
 func hasScheme(reposName string) bool {
 	return strings.Contains(reposName, "://")
 }
@@ -357,25 +363,20 @@ func validateHostPort(s string) error {
 }
 
 // newIndexInfo returns IndexInfo configuration from indexName
-func newIndexInfo(config *serviceConfig, indexName string) (*registry.IndexInfo, error) {
-	var err error
-	indexName, err = ValidateIndexName(indexName)
-	if err != nil {
-		return nil, err
-	}
+func newIndexInfo(config *serviceConfig, indexName string) *registry.IndexInfo {
+	indexName = normalizeIndexName(indexName)
 
 	// Return any configured index info, first.
 	if index, ok := config.IndexConfigs[indexName]; ok {
-		return index, nil
+		return index
 	}
 
 	// Construct a non-configured index info.
 	return &registry.IndexInfo{
-		Name:     indexName,
-		Mirrors:  make([]string, 0),
-		Secure:   config.isSecureIndex(indexName),
-		Official: false,
-	}, nil
+		Name:    indexName,
+		Mirrors: make([]string, 0),
+		Secure:  config.isSecureIndex(indexName),
+	}
 }
 
 // GetAuthConfigKey special-cases using the full index address of the official
@@ -388,11 +389,8 @@ func GetAuthConfigKey(index *registry.IndexInfo) string {
 }
 
 // newRepositoryInfo validates and breaks down a repository name into a RepositoryInfo
-func newRepositoryInfo(config *serviceConfig, name reference.Named) (*RepositoryInfo, error) {
-	index, err := newIndexInfo(config, reference.Domain(name))
-	if err != nil {
-		return nil, err
-	}
+func newRepositoryInfo(config *serviceConfig, name reference.Named) *RepositoryInfo {
+	index := newIndexInfo(config, reference.Domain(name))
 	var officialRepo bool
 	if index.Official {
 		// RepositoryInfo.Official indicates whether the image repository
@@ -406,7 +404,7 @@ func newRepositoryInfo(config *serviceConfig, name reference.Named) (*Repository
 		Name:     reference.TrimNamed(name),
 		Index:    index,
 		Official: officialRepo,
-	}, nil
+	}
 }
 
 // ParseRepositoryInfo performs the breakdown of a repository name into a
@@ -414,5 +412,5 @@ func newRepositoryInfo(config *serviceConfig, name reference.Named) (*Repository
 //
 // It is used by the Docker cli to interact with registry-related endpoints.
 func ParseRepositoryInfo(reposName reference.Named) (*RepositoryInfo, error) {
-	return newRepositoryInfo(emptyServiceConfig, reposName)
+	return newRepositoryInfo(emptyServiceConfig, reposName), nil
 }

registry/registry_test.go

@@ -270,15 +270,11 @@ func TestNewIndexInfo(t *testing.T) {
 	overrideLookupIP(t)
 	testIndexInfo := func(config *serviceConfig, expectedIndexInfos map[string]*registry.IndexInfo) {
 		for indexName, expectedIndexInfo := range expectedIndexInfos {
-			index, err := newIndexInfo(config, indexName)
-			if err != nil {
-				t.Fatal(err)
-			} else {
-				assert.Check(t, is.Equal(index.Name, expectedIndexInfo.Name), indexName+" name")
-				assert.Check(t, is.Equal(index.Official, expectedIndexInfo.Official), indexName+" is official")
-				assert.Check(t, is.Equal(index.Secure, expectedIndexInfo.Secure), indexName+" is secure")
-				assert.Check(t, is.Equal(len(index.Mirrors), len(expectedIndexInfo.Mirrors)), indexName+" mirrors")
-			}
+			index := newIndexInfo(config, indexName)
+			assert.Check(t, is.Equal(index.Name, expectedIndexInfo.Name), indexName+" name")
+			assert.Check(t, is.Equal(index.Official, expectedIndexInfo.Official), indexName+" is official")
+			assert.Check(t, is.Equal(index.Secure, expectedIndexInfo.Secure), indexName+" is secure")
+			assert.Check(t, is.Equal(len(index.Mirrors), len(expectedIndexInfo.Mirrors)), indexName+" mirrors")
 		}
 	}
 

registry/search.go

@@ -93,12 +93,8 @@ func (s *Service) searchUnfiltered(ctx context.Context, term string, limit int,
 
 	// Search is a long-running operation, just lock s.config to avoid block others.
 	s.mu.RLock()
-	index, err := newIndexInfo(s.config, indexName)
+	index := newIndexInfo(s.config, indexName)
 	s.mu.RUnlock()
-
-	if err != nil {
-		return nil, err
-	}
 	if index.Official {
 		// If pull "library/foo", it's stored locally under "foo"
 		remoteName = strings.TrimPrefix(remoteName, "library/")
@@ -158,5 +154,5 @@ func splitReposSearchTerm(reposName string) (string, string) {
 // for that.
 func ParseSearchIndexInfo(reposName string) (*registry.IndexInfo, error) {
 	indexName, _ := splitReposSearchTerm(reposName)
-	return newIndexInfo(emptyServiceConfig, indexName)
+	return newIndexInfo(emptyServiceConfig, indexName), nil
 }

registry/service.go

@@ -97,7 +97,8 @@ func (s *Service) Auth(ctx context.Context, authConfig *registry.AuthConfig, use
 func (s *Service) ResolveRepository(name reference.Named) (*RepositoryInfo, error) {
 	s.mu.RLock()
 	defer s.mu.RUnlock()
-	return newRepositoryInfo(s.config, name)
+	// TODO(thaJeztah): remove error return as it's no longer used.
+	return newRepositoryInfo(s.config, name), nil
 }
 
 // APIEndpoint represents a remote API endpoint