From 849f344eccae5bcee908b49640f093bc09be8b6d Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 1 Mar 2025 16:46:03 +0100 Subject: [PATCH] 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 --- registry/config.go | 44 +++++++++++++++++++-------------------- registry/registry_test.go | 14 +++++-------- registry/search.go | 8 ++----- registry/service.go | 3 ++- 4 files changed, 30 insertions(+), 39 deletions(-) diff --git a/registry/config.go b/registry/config.go index 61047e53ba..3d3cddfc6b 100644 --- a/registry/config.go +++ b/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 ®istry.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 } diff --git a/registry/registry_test.go b/registry/registry_test.go index 935cf23f85..6237628f6b 100644 --- a/registry/registry_test.go +++ b/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") } } diff --git a/registry/search.go b/registry/search.go index 4ce90f55d4..da77a2f42d 100644 --- a/registry/search.go +++ b/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 } diff --git a/registry/service.go b/registry/service.go index 4d66523c61..d8870b731d 100644 --- a/registry/service.go +++ b/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