From a3583b4b58d8848abe320ac0896a6dc5a7f8fc34 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 1 Mar 2025 14:45:48 +0100 Subject: [PATCH 1/3] registry: newRepositoryInfo only check for official images for Docker Hub RepositoryInfo.Official indicates whether the image repository is an official (docker library official images) repository. We only need to check this if the image-repository is on Docker Hub. This patch renames the variable to make it more transparent that this boolean is for the repository, and not to be confused for IndexInfo.Official, which indicates if the _registry_ is the "Official" (Docker Hub) registry. Signed-off-by: Sebastiaan van Stijn --- registry/config.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/registry/config.go b/registry/config.go index f8d94ce806..178a9b5796 100644 --- a/registry/config.go +++ b/registry/config.go @@ -363,12 +363,19 @@ func newRepositoryInfo(config *serviceConfig, name reference.Named) (*Repository if err != nil { return nil, err } - official := !strings.ContainsRune(reference.FamiliarName(name), '/') + var officialRepo bool + if index.Official { + // RepositoryInfo.Official indicates whether the image repository + // is an official (docker library official images) repository. + // + // We only need to check this if the image-repository is on Docker Hub. + officialRepo = !strings.ContainsRune(reference.FamiliarName(name), '/') + } return &RepositoryInfo{ Name: reference.TrimNamed(name), Index: index, - Official: official, + Official: officialRepo, }, nil } From fee40a9333d70870bedeabf473ef72567babb520 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 1 Mar 2025 14:13:20 +0100 Subject: [PATCH 2/3] registry: create emptyServiceConfig without parsing emptyServiceConfig is a default service-config for situations where no config-file is available (e.g. when used in the CLI). If won't have mirrors configured, but does have the default insecure registry CIDRs for loopback interfaces configured. Before this patch, this config was constructeed using the same code that handled constructing the config with a config present, but this involved parsing CIDR masks, and much more. With this patch, the service config is constructed as a literal, making it more transparent that it does not depend on any config or state. Signed-off-by: Sebastiaan van Stijn --- registry/config.go | 34 ++++++++++++++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) diff --git a/registry/config.go b/registry/config.go index 178a9b5796..61047e53ba 100644 --- a/registry/config.go +++ b/registry/config.go @@ -56,8 +56,38 @@ var ( Host: DefaultRegistryHost, } - emptyServiceConfig, _ = newServiceConfig(ServiceOptions{}) - validHostPortRegex = lazyregexp.New(`^` + reference.DomainRegexp.String() + `$`) + // ipv6Loopback is the CIDR for the IPv6 loopback address ("::1"); "::1/128" + ipv6Loopback = &net.IPNet{ + IP: net.IPv6loopback, + Mask: net.CIDRMask(128, 128), + } + + // ipv4Loopback is the CIDR for IPv4 loopback addresses ("127.0.0.0/8") + ipv4Loopback = &net.IPNet{ + IP: net.IPv4(127, 0, 0, 0), + Mask: net.CIDRMask(8, 32), + } + + // emptyServiceConfig is a default service-config for situations where + // no config-file is available (e.g. when used in the CLI). If won't + // have mirrors configured, but does have the default insecure registry + // CIDRs for loopback interfaces configured. + emptyServiceConfig = &serviceConfig{ + IndexConfigs: map[string]*registry.IndexInfo{ + IndexName: { + Name: IndexName, + Mirrors: make([]string, 0), + Secure: true, + Official: true, + }, + }, + InsecureRegistryCIDRs: []*registry.NetIPNet{ + (*registry.NetIPNet)(ipv6Loopback), + (*registry.NetIPNet)(ipv4Loopback), + }, + } + + validHostPortRegex = lazyregexp.New(`^` + reference.DomainRegexp.String() + `$`) // certsDir is used to override defaultCertsDir. certsDir string From 849f344eccae5bcee908b49640f093bc09be8b6d Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 1 Mar 2025 16:46:03 +0100 Subject: [PATCH 3/3] 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