From f6b90bc2535042dfcdbca4b3167e2533f78a51a0 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 24 Jul 2025 01:06:19 +0200 Subject: [PATCH 01/12] add internal fork of docker/docker/registry This adds an internal fork of [github.com/docker/docker/registry], taken at commit [moby@f651a5d]. Git history was not preserved in this fork, but can be found using the URLs provided. This fork was created to remove the dependency on the "Moby" codebase, and because the CLI only needs a subset of its features. The original package was written specifically for use in the daemon code, and includes functionality that cannot be used in the CLI. [github.com/docker/docker/registry]: https://pkg.go.dev/github.com/docker/docker@v28.3.2+incompatible/registry [moby@49306c6]: https://github.com/moby/moby/tree/49306c607b72c5bf0a8e426f5a9760fa5ef96ea0/registry Signed-off-by: Sebastiaan van Stijn --- cli/command/image/push.go | 2 +- cli/command/image/trust.go | 2 +- cli/command/plugin/install.go | 2 +- cli/command/plugin/push.go | 2 +- cli/command/registry/login.go | 2 +- cli/command/registry/login_test.go | 2 +- cli/command/registry/logout.go | 2 +- cli/command/service/trust.go | 2 +- cli/command/system/info.go | 2 +- cli/registry/client/endpoint.go | 2 +- cli/registry/client/fetcher.go | 2 +- cli/trust/trust.go | 2 +- cli/trust/trust_push.go | 2 +- internal/oauth/manager/manager.go | 2 +- .../docker => internal}/registry/auth.go | 0 internal/registry/auth_test.go | 106 +++ .../docker => internal}/registry/config.go | 0 internal/registry/config_test.go | 340 ++++++++++ internal/registry/doc.go | 12 + .../docker => internal}/registry/errors.go | 0 .../docker => internal}/registry/registry.go | 0 internal/registry/registry_mock_test.go | 120 ++++ internal/registry/registry_test.go | 637 ++++++++++++++++++ .../docker => internal}/registry/search.go | 0 .../registry/search_endpoint_v1.go | 0 internal/registry/search_endpoint_v1_test.go | 237 +++++++ .../registry/search_session.go | 0 internal/registry/search_test.go | 418 ++++++++++++ .../docker => internal}/registry/service.go | 0 .../registry/service_v2.go | 0 .../docker => internal}/registry/types.go | 0 vendor.mod | 4 +- vendor/modules.txt | 1 - 33 files changed, 1886 insertions(+), 17 deletions(-) rename {vendor/github.com/docker/docker => internal}/registry/auth.go (100%) create mode 100644 internal/registry/auth_test.go rename {vendor/github.com/docker/docker => internal}/registry/config.go (100%) create mode 100644 internal/registry/config_test.go create mode 100644 internal/registry/doc.go rename {vendor/github.com/docker/docker => internal}/registry/errors.go (100%) rename {vendor/github.com/docker/docker => internal}/registry/registry.go (100%) create mode 100644 internal/registry/registry_mock_test.go create mode 100644 internal/registry/registry_test.go rename {vendor/github.com/docker/docker => internal}/registry/search.go (100%) rename {vendor/github.com/docker/docker => internal}/registry/search_endpoint_v1.go (100%) create mode 100644 internal/registry/search_endpoint_v1_test.go rename {vendor/github.com/docker/docker => internal}/registry/search_session.go (100%) create mode 100644 internal/registry/search_test.go rename {vendor/github.com/docker/docker => internal}/registry/service.go (100%) rename {vendor/github.com/docker/docker => internal}/registry/service_v2.go (100%) rename {vendor/github.com/docker/docker => internal}/registry/types.go (100%) diff --git a/cli/command/image/push.go b/cli/command/image/push.go index 8b78ef0e118e..c3fd7611d937 100644 --- a/cli/command/image/push.go +++ b/cli/command/image/push.go @@ -16,8 +16,8 @@ import ( "github.com/docker/cli/cli/command/completion" "github.com/docker/cli/cli/streams" "github.com/docker/cli/internal/jsonstream" + "github.com/docker/cli/internal/registry" "github.com/docker/cli/internal/tui" - "github.com/docker/docker/registry" "github.com/moby/moby/api/types/auxprogress" "github.com/moby/moby/api/types/image" registrytypes "github.com/moby/moby/api/types/registry" diff --git a/cli/command/image/trust.go b/cli/command/image/trust.go index b15d537d6231..5381c1b55bb5 100644 --- a/cli/command/image/trust.go +++ b/cli/command/image/trust.go @@ -11,7 +11,7 @@ import ( "github.com/docker/cli/cli/streams" "github.com/docker/cli/cli/trust" "github.com/docker/cli/internal/jsonstream" - "github.com/docker/docker/registry" + "github.com/docker/cli/internal/registry" "github.com/moby/moby/api/types/image" registrytypes "github.com/moby/moby/api/types/registry" "github.com/opencontainers/go-digest" diff --git a/cli/command/plugin/install.go b/cli/command/plugin/install.go index f48f940d5cdb..b17c23ba5b5a 100644 --- a/cli/command/plugin/install.go +++ b/cli/command/plugin/install.go @@ -11,7 +11,7 @@ import ( "github.com/docker/cli/cli/command/image" "github.com/docker/cli/internal/jsonstream" "github.com/docker/cli/internal/prompt" - "github.com/docker/docker/registry" + "github.com/docker/cli/internal/registry" "github.com/moby/moby/api/types" registrytypes "github.com/moby/moby/api/types/registry" "github.com/pkg/errors" diff --git a/cli/command/plugin/push.go b/cli/command/plugin/push.go index 78d93136666a..f7fe2cdee9dc 100644 --- a/cli/command/plugin/push.go +++ b/cli/command/plugin/push.go @@ -8,7 +8,7 @@ import ( "github.com/docker/cli/cli/command" "github.com/docker/cli/cli/trust" "github.com/docker/cli/internal/jsonstream" - "github.com/docker/docker/registry" + "github.com/docker/cli/internal/registry" registrytypes "github.com/moby/moby/api/types/registry" "github.com/pkg/errors" "github.com/spf13/cobra" diff --git a/cli/command/registry/login.go b/cli/command/registry/login.go index dd1b99514aaa..f5853fe6a3fc 100644 --- a/cli/command/registry/login.go +++ b/cli/command/registry/login.go @@ -15,8 +15,8 @@ import ( "github.com/docker/cli/cli/config/configfile" configtypes "github.com/docker/cli/cli/config/types" "github.com/docker/cli/internal/oauth/manager" + "github.com/docker/cli/internal/registry" "github.com/docker/cli/internal/tui" - "github.com/docker/docker/registry" registrytypes "github.com/moby/moby/api/types/registry" "github.com/moby/moby/client" "github.com/pkg/errors" diff --git a/cli/command/registry/login_test.go b/cli/command/registry/login_test.go index 6d24011ec09c..20ca3d929901 100644 --- a/cli/command/registry/login_test.go +++ b/cli/command/registry/login_test.go @@ -13,8 +13,8 @@ import ( configtypes "github.com/docker/cli/cli/config/types" "github.com/docker/cli/cli/streams" "github.com/docker/cli/internal/prompt" + "github.com/docker/cli/internal/registry" "github.com/docker/cli/internal/test" - "github.com/docker/docker/registry" registrytypes "github.com/moby/moby/api/types/registry" "github.com/moby/moby/api/types/system" "github.com/moby/moby/client" diff --git a/cli/command/registry/logout.go b/cli/command/registry/logout.go index 2af2cdad3f3f..34498871a557 100644 --- a/cli/command/registry/logout.go +++ b/cli/command/registry/logout.go @@ -8,7 +8,7 @@ import ( "github.com/docker/cli/cli/command" "github.com/docker/cli/cli/config/credentials" "github.com/docker/cli/internal/oauth/manager" - "github.com/docker/docker/registry" + "github.com/docker/cli/internal/registry" "github.com/spf13/cobra" ) diff --git a/cli/command/service/trust.go b/cli/command/service/trust.go index 3bf81845f4bf..f9c8bae33c11 100644 --- a/cli/command/service/trust.go +++ b/cli/command/service/trust.go @@ -6,7 +6,7 @@ import ( "github.com/distribution/reference" "github.com/docker/cli/cli/command" "github.com/docker/cli/cli/trust" - "github.com/docker/docker/registry" + "github.com/docker/cli/internal/registry" "github.com/moby/moby/api/types/swarm" "github.com/opencontainers/go-digest" "github.com/pkg/errors" diff --git a/cli/command/system/info.go b/cli/command/system/info.go index 07306160c862..101622157236 100644 --- a/cli/command/system/info.go +++ b/cli/command/system/info.go @@ -19,8 +19,8 @@ import ( "github.com/docker/cli/cli/debug" flagsHelper "github.com/docker/cli/cli/flags" "github.com/docker/cli/internal/lazyregexp" + "github.com/docker/cli/internal/registry" "github.com/docker/cli/templates" - "github.com/docker/docker/registry" "github.com/docker/go-units" "github.com/moby/moby/api/types/swarm" "github.com/moby/moby/api/types/system" diff --git a/cli/registry/client/endpoint.go b/cli/registry/client/endpoint.go index 9690e7d71646..f08de5d14ae5 100644 --- a/cli/registry/client/endpoint.go +++ b/cli/registry/client/endpoint.go @@ -6,9 +6,9 @@ import ( "time" "github.com/distribution/reference" + "github.com/docker/cli/internal/registry" "github.com/docker/distribution/registry/client/auth" "github.com/docker/distribution/registry/client/transport" - "github.com/docker/docker/registry" registrytypes "github.com/moby/moby/api/types/registry" "github.com/pkg/errors" ) diff --git a/cli/registry/client/fetcher.go b/cli/registry/client/fetcher.go index f270d494324f..e169179d1670 100644 --- a/cli/registry/client/fetcher.go +++ b/cli/registry/client/fetcher.go @@ -6,6 +6,7 @@ import ( "github.com/distribution/reference" "github.com/docker/cli/cli/manifest/types" + "github.com/docker/cli/internal/registry" "github.com/docker/distribution" "github.com/docker/distribution/manifest/manifestlist" "github.com/docker/distribution/manifest/ocischema" @@ -13,7 +14,6 @@ import ( "github.com/docker/distribution/registry/api/errcode" v2 "github.com/docker/distribution/registry/api/v2" distclient "github.com/docker/distribution/registry/client" - "github.com/docker/docker/registry" "github.com/opencontainers/go-digest" ocispec "github.com/opencontainers/image-spec/specs-go/v1" "github.com/pkg/errors" diff --git a/cli/trust/trust.go b/cli/trust/trust.go index b64bf25e9a43..b8525f051325 100644 --- a/cli/trust/trust.go +++ b/cli/trust/trust.go @@ -14,10 +14,10 @@ import ( "github.com/distribution/reference" "github.com/docker/cli/cli/config" + "github.com/docker/cli/internal/registry" "github.com/docker/distribution/registry/client/auth" "github.com/docker/distribution/registry/client/auth/challenge" "github.com/docker/distribution/registry/client/transport" - "github.com/docker/docker/registry" "github.com/docker/go-connections/tlsconfig" registrytypes "github.com/moby/moby/api/types/registry" "github.com/opencontainers/go-digest" diff --git a/cli/trust/trust_push.go b/cli/trust/trust_push.go index 02432383f43c..b7b7b5ec892d 100644 --- a/cli/trust/trust_push.go +++ b/cli/trust/trust_push.go @@ -11,7 +11,7 @@ import ( "github.com/distribution/reference" "github.com/docker/cli/cli/streams" "github.com/docker/cli/internal/jsonstream" - "github.com/docker/docker/registry" + "github.com/docker/cli/internal/registry" "github.com/moby/moby/api/types" registrytypes "github.com/moby/moby/api/types/registry" "github.com/opencontainers/go-digest" diff --git a/internal/oauth/manager/manager.go b/internal/oauth/manager/manager.go index 115006436404..96deb61b1325 100644 --- a/internal/oauth/manager/manager.go +++ b/internal/oauth/manager/manager.go @@ -14,8 +14,8 @@ import ( "github.com/docker/cli/cli/streams" "github.com/docker/cli/internal/oauth" "github.com/docker/cli/internal/oauth/api" + "github.com/docker/cli/internal/registry" "github.com/docker/cli/internal/tui" - "github.com/docker/docker/registry" "github.com/morikuni/aec" "github.com/sirupsen/logrus" diff --git a/vendor/github.com/docker/docker/registry/auth.go b/internal/registry/auth.go similarity index 100% rename from vendor/github.com/docker/docker/registry/auth.go rename to internal/registry/auth.go diff --git a/internal/registry/auth_test.go b/internal/registry/auth_test.go new file mode 100644 index 000000000000..39a29d90805e --- /dev/null +++ b/internal/registry/auth_test.go @@ -0,0 +1,106 @@ +package registry + +import ( + "testing" + + "github.com/moby/moby/api/types/registry" + "gotest.tools/v3/assert" +) + +func buildAuthConfigs() map[string]registry.AuthConfig { + authConfigs := map[string]registry.AuthConfig{} + + for _, reg := range []string{"testIndex", IndexServer} { + authConfigs[reg] = registry.AuthConfig{ + Username: "docker-user", + Password: "docker-pass", + } + } + + return authConfigs +} + +func TestResolveAuthConfigIndexServer(t *testing.T) { + authConfigs := buildAuthConfigs() + indexConfig := authConfigs[IndexServer] + + officialIndex := ®istry.IndexInfo{ + Official: true, + } + privateIndex := ®istry.IndexInfo{ + Official: false, + } + + resolved := ResolveAuthConfig(authConfigs, officialIndex) + assert.Equal(t, resolved, indexConfig, "Expected ResolveAuthConfig to return IndexServer") + + resolved = ResolveAuthConfig(authConfigs, privateIndex) + assert.Check(t, resolved != indexConfig, "Expected ResolveAuthConfig to not return IndexServer") +} + +func TestResolveAuthConfigFullURL(t *testing.T) { + authConfigs := buildAuthConfigs() + + registryAuth := registry.AuthConfig{ + Username: "foo-user", + Password: "foo-pass", + } + localAuth := registry.AuthConfig{ + Username: "bar-user", + Password: "bar-pass", + } + officialAuth := registry.AuthConfig{ + Username: "baz-user", + Password: "baz-pass", + } + authConfigs[IndexServer] = officialAuth + + expectedAuths := map[string]registry.AuthConfig{ + "registry.example.com": registryAuth, + "localhost:8000": localAuth, + "example.com": localAuth, + } + + validRegistries := map[string][]string{ + "registry.example.com": { + "https://registry.example.com/v1/", + "http://registry.example.com/v1/", + "registry.example.com", + "registry.example.com/v1/", + }, + "localhost:8000": { + "https://localhost:8000/v1/", + "http://localhost:8000/v1/", + "localhost:8000", + "localhost:8000/v1/", + }, + "example.com": { + "https://example.com/v1/", + "http://example.com/v1/", + "example.com", + "example.com/v1/", + }, + } + + for configKey, registries := range validRegistries { + configured, ok := expectedAuths[configKey] + if !ok { + t.Fail() + } + index := ®istry.IndexInfo{ + Name: configKey, + } + for _, reg := range registries { + authConfigs[reg] = configured + resolved := ResolveAuthConfig(authConfigs, index) + if resolved.Username != configured.Username || resolved.Password != configured.Password { + t.Errorf("%s -> %v != %v\n", reg, resolved, configured) + } + delete(authConfigs, reg) + resolved = ResolveAuthConfig(authConfigs, index) + if resolved.Username == configured.Username || resolved.Password == configured.Password { + t.Errorf("%s -> %v == %v\n", reg, resolved, configured) + } + } + } +} diff --git a/vendor/github.com/docker/docker/registry/config.go b/internal/registry/config.go similarity index 100% rename from vendor/github.com/docker/docker/registry/config.go rename to internal/registry/config.go diff --git a/internal/registry/config_test.go b/internal/registry/config_test.go new file mode 100644 index 000000000000..699e763be6cf --- /dev/null +++ b/internal/registry/config_test.go @@ -0,0 +1,340 @@ +package registry + +import ( + "testing" + + cerrdefs "github.com/containerd/errdefs" + "gotest.tools/v3/assert" + is "gotest.tools/v3/assert/cmp" +) + +func TestValidateMirror(t *testing.T) { + tests := []struct { + input string + output string + expectedErr string + }{ + // Valid cases + { + input: "http://mirror-1.example.com", + output: "http://mirror-1.example.com/", + }, + { + input: "http://mirror-1.example.com/", + output: "http://mirror-1.example.com/", + }, + { + input: "https://mirror-1.example.com", + output: "https://mirror-1.example.com/", + }, + { + input: "https://mirror-1.example.com/", + output: "https://mirror-1.example.com/", + }, + { + input: "http://localhost", + output: "http://localhost/", + }, + { + input: "https://localhost", + output: "https://localhost/", + }, + { + input: "http://localhost:5000", + output: "http://localhost:5000/", + }, + { + input: "https://localhost:5000", + output: "https://localhost:5000/", + }, + { + input: "http://127.0.0.1", + output: "http://127.0.0.1/", + }, + { + input: "https://127.0.0.1", + output: "https://127.0.0.1/", + }, + { + input: "http://127.0.0.1:5000", + output: "http://127.0.0.1:5000/", + }, + { + input: "https://127.0.0.1:5000", + output: "https://127.0.0.1:5000/", + }, + { + input: "http://mirror-1.example.com/v1/", + output: "http://mirror-1.example.com/v1/", + }, + { + input: "https://mirror-1.example.com/v1/", + output: "https://mirror-1.example.com/v1/", + }, + + // Invalid cases + { + input: "!invalid!://%as%", + expectedErr: `invalid mirror: "!invalid!://%as%" is not a valid URI: parse "!invalid!://%as%": first path segment in URL cannot contain colon`, + }, + { + input: "mirror-1.example.com", + expectedErr: `invalid mirror: no scheme specified for "mirror-1.example.com": must use either 'https://' or 'http://'`, + }, + { + input: "mirror-1.example.com:5000", + expectedErr: `invalid mirror: no scheme specified for "mirror-1.example.com:5000": must use either 'https://' or 'http://'`, + }, + { + input: "ftp://mirror-1.example.com", + expectedErr: `invalid mirror: unsupported scheme "ftp" in "ftp://mirror-1.example.com": must use either 'https://' or 'http://'`, + }, + { + input: "http://mirror-1.example.com/?q=foo", + expectedErr: `invalid mirror: query or fragment at end of the URI "http://mirror-1.example.com/?q=foo"`, + }, + { + input: "http://mirror-1.example.com/v1/?q=foo", + expectedErr: `invalid mirror: query or fragment at end of the URI "http://mirror-1.example.com/v1/?q=foo"`, + }, + { + input: "http://mirror-1.example.com/v1/?q=foo#frag", + expectedErr: `invalid mirror: query or fragment at end of the URI "http://mirror-1.example.com/v1/?q=foo#frag"`, + }, + { + input: "http://mirror-1.example.com?q=foo", + expectedErr: `invalid mirror: query or fragment at end of the URI "http://mirror-1.example.com?q=foo"`, + }, + { + input: "https://mirror-1.example.com#frag", + expectedErr: `invalid mirror: query or fragment at end of the URI "https://mirror-1.example.com#frag"`, + }, + { + input: "https://mirror-1.example.com/#frag", + expectedErr: `invalid mirror: query or fragment at end of the URI "https://mirror-1.example.com/#frag"`, + }, + { + input: "http://foo:bar@mirror-1.example.com/", + expectedErr: `invalid mirror: username/password not allowed in URI "http://foo:xxxxx@mirror-1.example.com/"`, + }, + { + input: "https://mirror-1.example.com/v1/#frag", + expectedErr: `invalid mirror: query or fragment at end of the URI "https://mirror-1.example.com/v1/#frag"`, + }, + { + input: "https://mirror-1.example.com?q", + expectedErr: `invalid mirror: query or fragment at end of the URI "https://mirror-1.example.com?q"`, + }, + } + + for _, tc := range tests { + t.Run(tc.input, func(t *testing.T) { + out, err := ValidateMirror(tc.input) + if tc.expectedErr != "" { + assert.Error(t, err, tc.expectedErr) + } else { + assert.NilError(t, err) + } + assert.Check(t, is.Equal(out, tc.output)) + }) + } +} + +func TestLoadInsecureRegistries(t *testing.T) { + testCases := []struct { + registries []string + index string + err string + }{ + { + registries: []string{"127.0.0.1"}, + index: "127.0.0.1", + }, + { + registries: []string{"127.0.0.1:8080"}, + index: "127.0.0.1:8080", + }, + { + registries: []string{"2001:db8::1"}, + index: "2001:db8::1", + }, + { + registries: []string{"[2001:db8::1]:80"}, + index: "[2001:db8::1]:80", + }, + { + registries: []string{"http://myregistry.example.com"}, + index: "myregistry.example.com", + }, + { + registries: []string{"https://myregistry.example.com"}, + index: "myregistry.example.com", + }, + { + registries: []string{"HTTP://myregistry.example.com"}, + index: "myregistry.example.com", + }, + { + registries: []string{"svn://myregistry.example.com"}, + err: "insecure registry svn://myregistry.example.com should not contain '://'", + }, + { + registries: []string{"-invalid-registry"}, + err: "Cannot begin or end with a hyphen", + }, + { + registries: []string{`mytest-.com`}, + err: `insecure registry mytest-.com is not valid: invalid host "mytest-.com"`, + }, + { + registries: []string{`1200:0000:AB00:1234:0000:2552:7777:1313:8080`}, + err: `insecure registry 1200:0000:AB00:1234:0000:2552:7777:1313:8080 is not valid: invalid host "1200:0000:AB00:1234:0000:2552:7777:1313:8080"`, + }, + { + registries: []string{`myregistry.example.com:500000`}, + err: `insecure registry myregistry.example.com:500000 is not valid: invalid port "500000"`, + }, + { + registries: []string{`"myregistry.example.com"`}, + err: `insecure registry "myregistry.example.com" is not valid: invalid host "\"myregistry.example.com\""`, + }, + { + registries: []string{`"myregistry.example.com:5000"`}, + err: `insecure registry "myregistry.example.com:5000" is not valid: invalid host "\"myregistry.example.com"`, + }, + } + for _, testCase := range testCases { + config := &serviceConfig{} + err := config.loadInsecureRegistries(testCase.registries) + if testCase.err == "" { + if err != nil { + t.Fatalf("expect no error, got '%s'", err) + } + match := false + for index := range config.IndexConfigs { + if index == testCase.index { + match = true + } + } + if !match { + t.Fatalf("expect index configs to contain '%s', got %+v", testCase.index, config.IndexConfigs) + } + } else { + if err == nil { + t.Fatalf("expect error '%s', got no error", testCase.err) + } + assert.ErrorContains(t, err, testCase.err) + assert.Check(t, cerrdefs.IsInvalidArgument(err)) + } + } +} + +func TestNewServiceConfig(t *testing.T) { + tests := []struct { + doc string + opts ServiceOptions + errStr string + }{ + { + doc: "empty config", + }, + { + doc: "invalid mirror", + opts: ServiceOptions{ + Mirrors: []string{"example.com:5000"}, + }, + errStr: `invalid mirror: no scheme specified for "example.com:5000": must use either 'https://' or 'http://'`, + }, + { + doc: "valid mirror", + opts: ServiceOptions{ + Mirrors: []string{"https://example.com:5000"}, + }, + }, + { + doc: "invalid insecure registry", + opts: ServiceOptions{ + InsecureRegistries: []string{"[fe80::]/64"}, + }, + errStr: `insecure registry [fe80::]/64 is not valid: invalid host "[fe80::]/64"`, + }, + { + doc: "valid insecure registry", + opts: ServiceOptions{ + InsecureRegistries: []string{"102.10.8.1/24"}, + }, + }, + } + + for _, tc := range tests { + t.Run(tc.doc, func(t *testing.T) { + _, err := newServiceConfig(tc.opts) + if tc.errStr != "" { + assert.Check(t, is.Error(err, tc.errStr)) + assert.Check(t, cerrdefs.IsInvalidArgument(err)) + } else { + assert.Check(t, err) + } + }) + } +} + +func TestValidateIndexName(t *testing.T) { + valid := []struct { + index string + expect string + }{ + { + index: "index.docker.io", + expect: "docker.io", + }, + { + index: "example.com", + expect: "example.com", + }, + { + index: "127.0.0.1:8080", + expect: "127.0.0.1:8080", + }, + { + index: "mytest-1.com", + expect: "mytest-1.com", + }, + { + index: "mirror-1.example.com/v1/?q=foo", + expect: "mirror-1.example.com/v1/?q=foo", + }, + } + + for _, testCase := range valid { + result, err := ValidateIndexName(testCase.index) + if assert.Check(t, err) { + assert.Check(t, is.Equal(testCase.expect, result)) + } + } +} + +func TestValidateIndexNameWithError(t *testing.T) { + invalid := []struct { + index string + err string + }{ + { + index: "docker.io-", + err: "invalid index name (docker.io-). Cannot begin or end with a hyphen", + }, + { + index: "-example.com", + err: "invalid index name (-example.com). Cannot begin or end with a hyphen", + }, + { + index: "mirror-1.example.com/v1/?q=foo-", + err: "invalid index name (mirror-1.example.com/v1/?q=foo-). Cannot begin or end with a hyphen", + }, + } + for _, testCase := range invalid { + _, err := ValidateIndexName(testCase.index) + assert.Check(t, is.Error(err, testCase.err)) + assert.Check(t, cerrdefs.IsInvalidArgument(err)) + } +} diff --git a/internal/registry/doc.go b/internal/registry/doc.go new file mode 100644 index 000000000000..0b6a24767c84 --- /dev/null +++ b/internal/registry/doc.go @@ -0,0 +1,12 @@ +// Package registry is a fork of [github.com/docker/docker/registry], taken +// at commit [moby@49306c6]. Git history was not preserved in this fork, +// but can be found using the URLs provided. +// +// This fork was created to remove the dependency on the "Moby" codebase, +// and because the CLI only needs a subset of its features. The original +// package was written specifically for use in the daemon code, and includes +// functionality that cannot be used in the CLI. +// +// [github.com/docker/docker/registry]: https://pkg.go.dev/github.com/docker/docker@v28.3.2+incompatible/registry +// [moby@49306c6]: https://github.com/moby/moby/tree/49306c607b72c5bf0a8e426f5a9760fa5ef96ea0/registry +package registry diff --git a/vendor/github.com/docker/docker/registry/errors.go b/internal/registry/errors.go similarity index 100% rename from vendor/github.com/docker/docker/registry/errors.go rename to internal/registry/errors.go diff --git a/vendor/github.com/docker/docker/registry/registry.go b/internal/registry/registry.go similarity index 100% rename from vendor/github.com/docker/docker/registry/registry.go rename to internal/registry/registry.go diff --git a/internal/registry/registry_mock_test.go b/internal/registry/registry_mock_test.go new file mode 100644 index 000000000000..986e7ee64939 --- /dev/null +++ b/internal/registry/registry_mock_test.go @@ -0,0 +1,120 @@ +package registry + +import ( + "context" + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "testing" + + "github.com/containerd/log" + "github.com/moby/moby/api/types/registry" + "gotest.tools/v3/assert" +) + +var ( + testHTTPServer *httptest.Server + testHTTPSServer *httptest.Server +) + +func init() { + r := http.NewServeMux() + + // /v1/ + r.HandleFunc("/v1/_ping", handlerGetPing) + r.HandleFunc("/v1/search", handlerSearch) + + // /v2/ + r.HandleFunc("/v2/version", handlerGetPing) + + testHTTPServer = httptest.NewServer(handlerAccessLog(r)) + testHTTPSServer = httptest.NewTLSServer(handlerAccessLog(r)) +} + +func handlerAccessLog(handler http.Handler) http.Handler { + logHandler := func(w http.ResponseWriter, r *http.Request) { + log.G(context.TODO()).Debugf(`%s "%s %s"`, r.RemoteAddr, r.Method, r.URL) + handler.ServeHTTP(w, r) + } + return http.HandlerFunc(logHandler) +} + +func makeURL(req string) string { + return testHTTPServer.URL + req +} + +func makeHTTPSURL(req string) string { + return testHTTPSServer.URL + req +} + +func makeIndex(req string) *registry.IndexInfo { + return ®istry.IndexInfo{ + Name: makeURL(req), + } +} + +func makeHTTPSIndex(req string) *registry.IndexInfo { + return ®istry.IndexInfo{ + Name: makeHTTPSURL(req), + } +} + +func makePublicIndex() *registry.IndexInfo { + return ®istry.IndexInfo{ + Name: IndexServer, + Secure: true, + Official: true, + } +} + +func writeHeaders(w http.ResponseWriter) { + h := w.Header() + h.Add("Server", "docker-tests/mock") + h.Add("Expires", "-1") + h.Add("Content-Type", "application/json") + h.Add("Pragma", "no-cache") + h.Add("Cache-Control", "no-cache") +} + +func writeResponse(w http.ResponseWriter, message interface{}, code int) { + writeHeaders(w) + w.WriteHeader(code) + body, err := json.Marshal(message) + if err != nil { + _, _ = io.WriteString(w, err.Error()) + return + } + _, _ = w.Write(body) +} + +func handlerGetPing(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodGet { + writeResponse(w, http.StatusText(http.StatusMethodNotAllowed), http.StatusMethodNotAllowed) + return + } + writeResponse(w, true, http.StatusOK) +} + +func handlerSearch(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodGet { + writeResponse(w, http.StatusText(http.StatusMethodNotAllowed), http.StatusMethodNotAllowed) + return + } + result := ®istry.SearchResults{ + Query: "fakequery", + NumResults: 1, + Results: []registry.SearchResult{{Name: "fakeimage", StarCount: 42}}, + } + writeResponse(w, result, http.StatusOK) +} + +func TestPing(t *testing.T) { + res, err := http.Get(makeURL("/v1/_ping")) + if err != nil { + t.Fatal(err) + } + assert.Equal(t, res.StatusCode, http.StatusOK, "") + assert.Equal(t, res.Header.Get("Server"), "docker-tests/mock") + _ = res.Body.Close() +} diff --git a/internal/registry/registry_test.go b/internal/registry/registry_test.go new file mode 100644 index 000000000000..79c17fa3666e --- /dev/null +++ b/internal/registry/registry_test.go @@ -0,0 +1,637 @@ +package registry + +import ( + "errors" + "net" + "testing" + + "github.com/distribution/reference" + "github.com/moby/moby/api/types/registry" + "gotest.tools/v3/assert" + is "gotest.tools/v3/assert/cmp" +) + +// overrideLookupIP overrides net.LookupIP for testing. +func overrideLookupIP(t *testing.T) { + t.Helper() + restoreLookup := lookupIP + + // override net.LookupIP + lookupIP = func(host string) ([]net.IP, error) { + mockHosts := map[string][]net.IP{ + "": {net.ParseIP("0.0.0.0")}, + "localhost": {net.ParseIP("127.0.0.1"), net.ParseIP("::1")}, + "example.com": {net.ParseIP("42.42.42.42")}, + "other.com": {net.ParseIP("43.43.43.43")}, + } + if addrs, ok := mockHosts[host]; ok { + return addrs, nil + } + return nil, errors.New("lookup: no such host") + } + t.Cleanup(func() { + lookupIP = restoreLookup + }) +} + +func TestParseRepositoryInfo(t *testing.T) { + type staticRepositoryInfo struct { + Index *registry.IndexInfo + RemoteName string + CanonicalName string + LocalName string + } + + tests := map[string]staticRepositoryInfo{ + "fooo/bar": { + Index: ®istry.IndexInfo{ + Name: IndexName, + Mirrors: []string{}, + Official: true, + Secure: true, + }, + RemoteName: "fooo/bar", + LocalName: "fooo/bar", + CanonicalName: "docker.io/fooo/bar", + }, + "library/ubuntu": { + Index: ®istry.IndexInfo{ + Name: IndexName, + Mirrors: []string{}, + Official: true, + Secure: true, + }, + RemoteName: "library/ubuntu", + LocalName: "ubuntu", + CanonicalName: "docker.io/library/ubuntu", + }, + "nonlibrary/ubuntu": { + Index: ®istry.IndexInfo{ + Name: IndexName, + Mirrors: []string{}, + Official: true, + Secure: true, + }, + RemoteName: "nonlibrary/ubuntu", + LocalName: "nonlibrary/ubuntu", + CanonicalName: "docker.io/nonlibrary/ubuntu", + }, + "ubuntu": { + Index: ®istry.IndexInfo{ + Name: IndexName, + Mirrors: []string{}, + Official: true, + Secure: true, + }, + RemoteName: "library/ubuntu", + LocalName: "ubuntu", + CanonicalName: "docker.io/library/ubuntu", + }, + "other/library": { + Index: ®istry.IndexInfo{ + Name: IndexName, + Mirrors: []string{}, + Official: true, + Secure: true, + }, + RemoteName: "other/library", + LocalName: "other/library", + CanonicalName: "docker.io/other/library", + }, + "127.0.0.1:8000/private/moonbase": { + Index: ®istry.IndexInfo{ + Name: "127.0.0.1:8000", + Mirrors: []string{}, + Official: false, + Secure: false, + }, + RemoteName: "private/moonbase", + LocalName: "127.0.0.1:8000/private/moonbase", + CanonicalName: "127.0.0.1:8000/private/moonbase", + }, + "127.0.0.1:8000/privatebase": { + Index: ®istry.IndexInfo{ + Name: "127.0.0.1:8000", + Mirrors: []string{}, + Official: false, + Secure: false, + }, + RemoteName: "privatebase", + LocalName: "127.0.0.1:8000/privatebase", + CanonicalName: "127.0.0.1:8000/privatebase", + }, + "[::1]:8000/private/moonbase": { + Index: ®istry.IndexInfo{ + Name: "[::1]:8000", + Mirrors: []string{}, + Official: false, + Secure: false, + }, + RemoteName: "private/moonbase", + LocalName: "[::1]:8000/private/moonbase", + CanonicalName: "[::1]:8000/private/moonbase", + }, + "[::1]:8000/privatebase": { + Index: ®istry.IndexInfo{ + Name: "[::1]:8000", + Mirrors: []string{}, + Official: false, + Secure: false, + }, + RemoteName: "privatebase", + LocalName: "[::1]:8000/privatebase", + CanonicalName: "[::1]:8000/privatebase", + }, + // IPv6 only has a single loopback address, so ::2 is not a loopback, + // hence not marked "insecure". + "[::2]:8000/private/moonbase": { + Index: ®istry.IndexInfo{ + Name: "[::2]:8000", + Mirrors: []string{}, + Official: false, + Secure: true, + }, + RemoteName: "private/moonbase", + LocalName: "[::2]:8000/private/moonbase", + CanonicalName: "[::2]:8000/private/moonbase", + }, + // IPv6 only has a single loopback address, so ::2 is not a loopback, + // hence not marked "insecure". + "[::2]:8000/privatebase": { + Index: ®istry.IndexInfo{ + Name: "[::2]:8000", + Mirrors: []string{}, + Official: false, + Secure: true, + }, + RemoteName: "privatebase", + LocalName: "[::2]:8000/privatebase", + CanonicalName: "[::2]:8000/privatebase", + }, + "localhost:8000/private/moonbase": { + Index: ®istry.IndexInfo{ + Name: "localhost:8000", + Mirrors: []string{}, + Official: false, + Secure: false, + }, + RemoteName: "private/moonbase", + LocalName: "localhost:8000/private/moonbase", + CanonicalName: "localhost:8000/private/moonbase", + }, + "localhost:8000/privatebase": { + Index: ®istry.IndexInfo{ + Name: "localhost:8000", + Mirrors: []string{}, + Official: false, + Secure: false, + }, + RemoteName: "privatebase", + LocalName: "localhost:8000/privatebase", + CanonicalName: "localhost:8000/privatebase", + }, + "example.com/private/moonbase": { + Index: ®istry.IndexInfo{ + Name: "example.com", + Mirrors: []string{}, + Official: false, + Secure: true, + }, + RemoteName: "private/moonbase", + LocalName: "example.com/private/moonbase", + CanonicalName: "example.com/private/moonbase", + }, + "example.com/privatebase": { + Index: ®istry.IndexInfo{ + Name: "example.com", + Mirrors: []string{}, + Official: false, + Secure: true, + }, + RemoteName: "privatebase", + LocalName: "example.com/privatebase", + CanonicalName: "example.com/privatebase", + }, + "example.com:8000/private/moonbase": { + Index: ®istry.IndexInfo{ + Name: "example.com:8000", + Mirrors: []string{}, + Official: false, + Secure: true, + }, + RemoteName: "private/moonbase", + LocalName: "example.com:8000/private/moonbase", + CanonicalName: "example.com:8000/private/moonbase", + }, + "example.com:8000/privatebase": { + Index: ®istry.IndexInfo{ + Name: "example.com:8000", + Mirrors: []string{}, + Official: false, + Secure: true, + }, + RemoteName: "privatebase", + LocalName: "example.com:8000/privatebase", + CanonicalName: "example.com:8000/privatebase", + }, + "localhost/private/moonbase": { + Index: ®istry.IndexInfo{ + Name: "localhost", + Mirrors: []string{}, + Official: false, + Secure: false, + }, + RemoteName: "private/moonbase", + LocalName: "localhost/private/moonbase", + CanonicalName: "localhost/private/moonbase", + }, + "localhost/privatebase": { + Index: ®istry.IndexInfo{ + Name: "localhost", + Mirrors: []string{}, + Official: false, + Secure: false, + }, + RemoteName: "privatebase", + LocalName: "localhost/privatebase", + CanonicalName: "localhost/privatebase", + }, + IndexName + "/public/moonbase": { + Index: ®istry.IndexInfo{ + Name: IndexName, + Mirrors: []string{}, + Official: true, + Secure: true, + }, + RemoteName: "public/moonbase", + LocalName: "public/moonbase", + CanonicalName: "docker.io/public/moonbase", + }, + "index." + IndexName + "/public/moonbase": { + Index: ®istry.IndexInfo{ + Name: IndexName, + Mirrors: []string{}, + Official: true, + Secure: true, + }, + RemoteName: "public/moonbase", + LocalName: "public/moonbase", + CanonicalName: "docker.io/public/moonbase", + }, + "ubuntu-12.04-base": { + Index: ®istry.IndexInfo{ + Name: IndexName, + Mirrors: []string{}, + Official: true, + Secure: true, + }, + RemoteName: "library/ubuntu-12.04-base", + LocalName: "ubuntu-12.04-base", + CanonicalName: "docker.io/library/ubuntu-12.04-base", + }, + IndexName + "/ubuntu-12.04-base": { + Index: ®istry.IndexInfo{ + Name: IndexName, + Mirrors: []string{}, + Official: true, + Secure: true, + }, + RemoteName: "library/ubuntu-12.04-base", + LocalName: "ubuntu-12.04-base", + CanonicalName: "docker.io/library/ubuntu-12.04-base", + }, + "index." + IndexName + "/ubuntu-12.04-base": { + Index: ®istry.IndexInfo{ + Name: IndexName, + Mirrors: []string{}, + Official: true, + Secure: true, + }, + RemoteName: "library/ubuntu-12.04-base", + LocalName: "ubuntu-12.04-base", + CanonicalName: "docker.io/library/ubuntu-12.04-base", + }, + } + + for reposName, expected := range tests { + t.Run(reposName, func(t *testing.T) { + named, err := reference.ParseNormalizedNamed(reposName) + assert.NilError(t, err) + + repoInfo, err := ParseRepositoryInfo(named) + assert.NilError(t, err) + + assert.Check(t, is.DeepEqual(repoInfo.Index, expected.Index)) + assert.Check(t, is.Equal(reference.Path(repoInfo.Name), expected.RemoteName)) + assert.Check(t, is.Equal(reference.FamiliarName(repoInfo.Name), expected.LocalName)) + assert.Check(t, is.Equal(repoInfo.Name.Name(), expected.CanonicalName)) + }) + } +} + +func TestNewIndexInfo(t *testing.T) { + overrideLookupIP(t) + + // 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). It 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: []string{}, + Secure: true, + Official: true, + }, + }, + InsecureRegistryCIDRs: []*registry.NetIPNet{ + (*registry.NetIPNet)(ipv6Loopback), + (*registry.NetIPNet)(ipv4Loopback), + }, + } + + expectedIndexInfos := map[string]*registry.IndexInfo{ + IndexName: { + Name: IndexName, + Official: true, + Secure: true, + Mirrors: []string{}, + }, + "index." + IndexName: { + Name: IndexName, + Official: true, + Secure: true, + Mirrors: []string{}, + }, + "example.com": { + Name: "example.com", + Official: false, + Secure: true, + Mirrors: []string{}, + }, + "127.0.0.1:5000": { + Name: "127.0.0.1:5000", + Official: false, + Secure: false, + Mirrors: []string{}, + }, + } + t.Run("no mirrors", func(t *testing.T) { + for indexName, expected := range expectedIndexInfos { + t.Run(indexName, func(t *testing.T) { + actual := newIndexInfo(emptyServiceConfig, indexName) + assert.Check(t, is.DeepEqual(actual, expected)) + }) + } + }) + + expectedIndexInfos = map[string]*registry.IndexInfo{ + IndexName: { + Name: IndexName, + Official: true, + Secure: true, + Mirrors: []string{"http://mirror1.local/", "http://mirror2.local/"}, + }, + "index." + IndexName: { + Name: IndexName, + Official: true, + Secure: true, + Mirrors: []string{"http://mirror1.local/", "http://mirror2.local/"}, + }, + "example.com": { + Name: "example.com", + Official: false, + Secure: false, + Mirrors: []string{}, + }, + "example.com:5000": { + Name: "example.com:5000", + Official: false, + Secure: true, + Mirrors: []string{}, + }, + "127.0.0.1": { + Name: "127.0.0.1", + Official: false, + Secure: false, + Mirrors: []string{}, + }, + "127.0.0.1:5000": { + Name: "127.0.0.1:5000", + Official: false, + Secure: false, + Mirrors: []string{}, + }, + "127.255.255.255": { + Name: "127.255.255.255", + Official: false, + Secure: false, + Mirrors: []string{}, + }, + "127.255.255.255:5000": { + Name: "127.255.255.255:5000", + Official: false, + Secure: false, + Mirrors: []string{}, + }, + "::1": { + Name: "::1", + Official: false, + Secure: false, + Mirrors: []string{}, + }, + "[::1]:5000": { + Name: "[::1]:5000", + Official: false, + Secure: false, + Mirrors: []string{}, + }, + // IPv6 only has a single loopback address, so ::2 is not a loopback, + // hence not marked "insecure". + "::2": { + Name: "::2", + Official: false, + Secure: true, + Mirrors: []string{}, + }, + // IPv6 only has a single loopback address, so ::2 is not a loopback, + // hence not marked "insecure". + "[::2]:5000": { + Name: "[::2]:5000", + Official: false, + Secure: true, + Mirrors: []string{}, + }, + "other.com": { + Name: "other.com", + Official: false, + Secure: true, + Mirrors: []string{}, + }, + } + t.Run("mirrors", func(t *testing.T) { + // Note that newServiceConfig calls ValidateMirror internally, which normalizes + // mirror-URLs to have a trailing slash. + config, err := newServiceConfig(ServiceOptions{ + Mirrors: []string{"http://mirror1.local", "http://mirror2.local"}, + InsecureRegistries: []string{"example.com"}, + }) + assert.NilError(t, err) + for indexName, expected := range expectedIndexInfos { + t.Run(indexName, func(t *testing.T) { + actual := newIndexInfo(config, indexName) + assert.Check(t, is.DeepEqual(actual, expected)) + }) + } + }) + + expectedIndexInfos = map[string]*registry.IndexInfo{ + "example.com": { + Name: "example.com", + Official: false, + Secure: false, + Mirrors: []string{}, + }, + "example.com:5000": { + Name: "example.com:5000", + Official: false, + Secure: false, + Mirrors: []string{}, + }, + "127.0.0.1": { + Name: "127.0.0.1", + Official: false, + Secure: false, + Mirrors: []string{}, + }, + "127.0.0.1:5000": { + Name: "127.0.0.1:5000", + Official: false, + Secure: false, + Mirrors: []string{}, + }, + "42.42.0.1:5000": { + Name: "42.42.0.1:5000", + Official: false, + Secure: false, + Mirrors: []string{}, + }, + "42.43.0.1:5000": { + Name: "42.43.0.1:5000", + Official: false, + Secure: true, + Mirrors: []string{}, + }, + "other.com": { + Name: "other.com", + Official: false, + Secure: true, + Mirrors: []string{}, + }, + } + t.Run("custom insecure", func(t *testing.T) { + config, err := newServiceConfig(ServiceOptions{ + InsecureRegistries: []string{"42.42.0.0/16"}, + }) + assert.NilError(t, err) + for indexName, expected := range expectedIndexInfos { + t.Run(indexName, func(t *testing.T) { + actual := newIndexInfo(config, indexName) + assert.Check(t, is.DeepEqual(actual, expected)) + }) + } + }) +} + +func TestMirrorEndpointLookup(t *testing.T) { + containsMirror := func(endpoints []APIEndpoint) bool { + for _, pe := range endpoints { + if pe.URL.Host == "my.mirror" { + return true + } + } + return false + } + cfg, err := newServiceConfig(ServiceOptions{ + Mirrors: []string{"https://my.mirror"}, + }) + assert.NilError(t, err) + s := Service{config: cfg} + + imageName, err := reference.WithName(IndexName + "/test/image") + if err != nil { + t.Error(err) + } + pushAPIEndpoints, err := s.LookupPushEndpoints(reference.Domain(imageName)) + if err != nil { + t.Fatal(err) + } + if containsMirror(pushAPIEndpoints) { + t.Fatal("Push endpoint should not contain mirror") + } + + pullAPIEndpoints, err := s.LookupPullEndpoints(reference.Domain(imageName)) + if err != nil { + t.Fatal(err) + } + if !containsMirror(pullAPIEndpoints) { + t.Fatal("Pull endpoint should contain mirror") + } +} + +func TestIsSecureIndex(t *testing.T) { + overrideLookupIP(t) + tests := []struct { + addr string + insecureRegistries []string + expected bool + }{ + {IndexName, nil, true}, + {"example.com", []string{}, true}, + {"example.com", []string{"example.com"}, false}, + {"localhost", []string{"localhost:5000"}, false}, + {"localhost:5000", []string{"localhost:5000"}, false}, + {"localhost", []string{"example.com"}, false}, + {"127.0.0.1:5000", []string{"127.0.0.1:5000"}, false}, + {"localhost", nil, false}, + {"localhost:5000", nil, false}, + {"127.0.0.1", nil, false}, + {"localhost", []string{"example.com"}, false}, + {"127.0.0.1", []string{"example.com"}, false}, + {"example.com", nil, true}, + {"example.com", []string{"example.com"}, false}, + {"127.0.0.1", []string{"example.com"}, false}, + {"127.0.0.1:5000", []string{"example.com"}, false}, + {"example.com:5000", []string{"42.42.0.0/16"}, false}, + {"example.com", []string{"42.42.0.0/16"}, false}, + {"example.com:5000", []string{"42.42.42.42/8"}, false}, + {"127.0.0.1:5000", []string{"127.0.0.0/8"}, false}, + {"42.42.42.42:5000", []string{"42.1.1.1/8"}, false}, + {"invalid.example.com", []string{"42.42.0.0/16"}, true}, + {"invalid.example.com", []string{"invalid.example.com"}, false}, + {"invalid.example.com:5000", []string{"invalid.example.com"}, true}, + {"invalid.example.com:5000", []string{"invalid.example.com:5000"}, false}, + } + for _, tc := range tests { + config, err := newServiceConfig(ServiceOptions{ + InsecureRegistries: tc.insecureRegistries, + }) + assert.NilError(t, err) + + sec := config.isSecureIndex(tc.addr) + assert.Equal(t, sec, tc.expected, "isSecureIndex failed for %q %v, expected %v got %v", tc.addr, tc.insecureRegistries, tc.expected, sec) + } +} diff --git a/vendor/github.com/docker/docker/registry/search.go b/internal/registry/search.go similarity index 100% rename from vendor/github.com/docker/docker/registry/search.go rename to internal/registry/search.go diff --git a/vendor/github.com/docker/docker/registry/search_endpoint_v1.go b/internal/registry/search_endpoint_v1.go similarity index 100% rename from vendor/github.com/docker/docker/registry/search_endpoint_v1.go rename to internal/registry/search_endpoint_v1.go diff --git a/internal/registry/search_endpoint_v1_test.go b/internal/registry/search_endpoint_v1_test.go new file mode 100644 index 000000000000..137e2a60fc84 --- /dev/null +++ b/internal/registry/search_endpoint_v1_test.go @@ -0,0 +1,237 @@ +package registry + +import ( + "context" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/moby/moby/api/types/registry" + "gotest.tools/v3/assert" + is "gotest.tools/v3/assert/cmp" +) + +func TestV1EndpointPing(t *testing.T) { + testPing := func(index *registry.IndexInfo, expectedStandalone bool, assertMessage string) { + ep, err := newV1Endpoint(context.Background(), index, nil) + if err != nil { + t.Fatal(err) + } + regInfo, err := ep.ping(context.Background()) + if err != nil { + t.Fatal(err) + } + + assert.Equal(t, regInfo.Standalone, expectedStandalone, assertMessage) + } + + testPing(makeIndex("/v1/"), true, "Expected standalone to be true (default)") + testPing(makeHTTPSIndex("/v1/"), true, "Expected standalone to be true (default)") + testPing(makePublicIndex(), false, "Expected standalone to be false for public index") +} + +func TestV1Endpoint(t *testing.T) { + // Simple wrapper to fail test if err != nil + expandEndpoint := func(index *registry.IndexInfo) *v1Endpoint { + endpoint, err := newV1Endpoint(context.Background(), index, nil) + if err != nil { + t.Fatal(err) + } + return endpoint + } + + assertInsecureIndex := func(index *registry.IndexInfo) { + index.Secure = true + _, err := newV1Endpoint(context.Background(), index, nil) + assert.ErrorContains(t, err, "insecure-registry", index.Name+": Expected insecure-registry error for insecure index") + index.Secure = false + } + + assertSecureIndex := func(index *registry.IndexInfo) { + index.Secure = true + _, err := newV1Endpoint(context.Background(), index, nil) + assert.ErrorContains(t, err, "certificate signed by unknown authority", index.Name+": Expected cert error for secure index") + index.Secure = false + } + + index := ®istry.IndexInfo{} + index.Name = makeURL("/v1/") + endpoint := expandEndpoint(index) + assert.Equal(t, endpoint.String(), index.Name, "Expected endpoint to be "+index.Name) + assertInsecureIndex(index) + + index.Name = makeURL("") + endpoint = expandEndpoint(index) + assert.Equal(t, endpoint.String(), index.Name+"/v1/", index.Name+": Expected endpoint to be "+index.Name+"/v1/") + assertInsecureIndex(index) + + httpURL := makeURL("") + index.Name = strings.SplitN(httpURL, "://", 2)[1] + endpoint = expandEndpoint(index) + assert.Equal(t, endpoint.String(), httpURL+"/v1/", index.Name+": Expected endpoint to be "+httpURL+"/v1/") + assertInsecureIndex(index) + + index.Name = makeHTTPSURL("/v1/") + endpoint = expandEndpoint(index) + assert.Equal(t, endpoint.String(), index.Name, "Expected endpoint to be "+index.Name) + assertSecureIndex(index) + + index.Name = makeHTTPSURL("") + endpoint = expandEndpoint(index) + assert.Equal(t, endpoint.String(), index.Name+"/v1/", index.Name+": Expected endpoint to be "+index.Name+"/v1/") + assertSecureIndex(index) + + httpsURL := makeHTTPSURL("") + index.Name = strings.SplitN(httpsURL, "://", 2)[1] + endpoint = expandEndpoint(index) + assert.Equal(t, endpoint.String(), httpsURL+"/v1/", index.Name+": Expected endpoint to be "+httpsURL+"/v1/") + assertSecureIndex(index) + + badEndpoints := []string{ + "http://127.0.0.1/v1/", + "https://127.0.0.1/v1/", + "http://127.0.0.1", + "https://127.0.0.1", + "127.0.0.1", + } + for _, address := range badEndpoints { + index.Name = address + _, err := newV1Endpoint(context.Background(), index, nil) + assert.Check(t, err != nil, "Expected error while expanding bad endpoint: %s", address) + } +} + +func TestV1EndpointParse(t *testing.T) { + tests := []struct { + address string + expected string + expectedErr string + }{ + { + address: IndexServer, + expected: IndexServer, + }, + { + address: "https://0.0.0.0:5000/v1/", + expected: "https://0.0.0.0:5000/v1/", + }, + { + address: "https://0.0.0.0:5000", + expected: "https://0.0.0.0:5000/v1/", + }, + { + address: "0.0.0.0:5000", + expected: "https://0.0.0.0:5000/v1/", + }, + { + address: "https://0.0.0.0:5000/nonversion/", + expected: "https://0.0.0.0:5000/nonversion/v1/", + }, + { + address: "https://0.0.0.0:5000/v0/", + expected: "https://0.0.0.0:5000/v0/v1/", + }, + { + address: "https://0.0.0.0:5000/v2/", + expectedErr: "search is not supported on v2 endpoints: https://0.0.0.0:5000/v2/", + }, + } + for _, tc := range tests { + t.Run(tc.address, func(t *testing.T) { + ep, err := newV1EndpointFromStr(tc.address, nil, nil) + if tc.expectedErr != "" { + assert.Check(t, is.Error(err, tc.expectedErr)) + assert.Check(t, is.Nil(ep)) + } else { + assert.NilError(t, err) + assert.Check(t, is.Equal(ep.String(), tc.expected)) + } + }) + } +} + +// Ensure that a registry endpoint that responds with a 401 only is determined +// to be a valid v1 registry endpoint +func TestV1EndpointValidate(t *testing.T) { + requireBasicAuthHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Add("WWW-Authenticate", `Basic realm="localhost"`) + w.WriteHeader(http.StatusUnauthorized) + }) + + // Make a test server which should validate as a v1 server. + testServer := httptest.NewServer(requireBasicAuthHandler) + defer testServer.Close() + + testEndpoint, err := newV1Endpoint(context.Background(), ®istry.IndexInfo{Name: testServer.URL}, nil) + if err != nil { + t.Fatal(err) + } + + if testEndpoint.URL.Scheme != "http" { + t.Fatalf("expecting to validate endpoint as http, got url %s", testEndpoint.String()) + } +} + +func TestTrustedLocation(t *testing.T) { + for _, u := range []string{"http://example.com", "https://example.com:7777", "http://docker.io", "http://test.docker.com", "https://fakedocker.com"} { + req, _ := http.NewRequest(http.MethodGet, u, http.NoBody) + assert.Check(t, !trustedLocation(req)) + } + + for _, u := range []string{"https://docker.io", "https://test.docker.com:80"} { + req, _ := http.NewRequest(http.MethodGet, u, http.NoBody) + assert.Check(t, trustedLocation(req)) + } +} + +func TestAddRequiredHeadersToRedirectedRequests(t *testing.T) { + for _, urls := range [][]string{ + {"http://docker.io", "https://docker.com"}, + {"https://foo.docker.io:7777", "http://bar.docker.com"}, + {"https://foo.docker.io", "https://example.com"}, + } { + reqFrom, _ := http.NewRequest(http.MethodGet, urls[0], http.NoBody) + reqFrom.Header.Add("Content-Type", "application/json") + reqFrom.Header.Add("Authorization", "super_secret") + reqTo, _ := http.NewRequest(http.MethodGet, urls[1], http.NoBody) + + _ = addRequiredHeadersToRedirectedRequests(reqTo, []*http.Request{reqFrom}) + + if len(reqTo.Header) != 1 { + t.Fatalf("Expected 1 headers, got %d", len(reqTo.Header)) + } + + if reqTo.Header.Get("Content-Type") != "application/json" { + t.Fatal("'Content-Type' should be 'application/json'") + } + + if reqTo.Header.Get("Authorization") != "" { + t.Fatal("'Authorization' should be empty") + } + } + + for _, urls := range [][]string{ + {"https://docker.io", "https://docker.com"}, + {"https://foo.docker.io:7777", "https://bar.docker.com"}, + } { + reqFrom, _ := http.NewRequest(http.MethodGet, urls[0], http.NoBody) + reqFrom.Header.Add("Content-Type", "application/json") + reqFrom.Header.Add("Authorization", "super_secret") + reqTo, _ := http.NewRequest(http.MethodGet, urls[1], http.NoBody) + + _ = addRequiredHeadersToRedirectedRequests(reqTo, []*http.Request{reqFrom}) + + if len(reqTo.Header) != 2 { + t.Fatalf("Expected 2 headers, got %d", len(reqTo.Header)) + } + + if reqTo.Header.Get("Content-Type") != "application/json" { + t.Fatal("'Content-Type' should be 'application/json'") + } + + if reqTo.Header.Get("Authorization") != "super_secret" { + t.Fatal("'Authorization' should be 'super_secret'") + } + } +} diff --git a/vendor/github.com/docker/docker/registry/search_session.go b/internal/registry/search_session.go similarity index 100% rename from vendor/github.com/docker/docker/registry/search_session.go rename to internal/registry/search_session.go diff --git a/internal/registry/search_test.go b/internal/registry/search_test.go new file mode 100644 index 000000000000..7af875767faa --- /dev/null +++ b/internal/registry/search_test.go @@ -0,0 +1,418 @@ +package registry + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "net/http/httputil" + "testing" + + cerrdefs "github.com/containerd/errdefs" + "github.com/docker/distribution/registry/client/transport" + "github.com/moby/moby/api/types/filters" + "github.com/moby/moby/api/types/registry" + "gotest.tools/v3/assert" +) + +func spawnTestRegistrySession(t *testing.T) *session { + t.Helper() + authConfig := ®istry.AuthConfig{} + endpoint, err := newV1Endpoint(context.Background(), makeIndex("/v1/"), nil) + if err != nil { + t.Fatal(err) + } + userAgent := "docker test client" + var tr http.RoundTripper = debugTransport{newTransport(nil), t.Log} + tr = transport.NewTransport(newAuthTransport(tr, authConfig, false), Headers(userAgent, nil)...) + client := httpClient(tr) + + if err := authorizeClient(context.Background(), client, authConfig, endpoint); err != nil { + t.Fatal(err) + } + r := newSession(client, endpoint) + + // In a normal scenario for the v1 registry, the client should send a `X-Docker-Token: true` + // header while authenticating, in order to retrieve a token that can be later used to + // perform authenticated actions. + // + // The mock v1 registry does not support that, (TODO(tiborvass): support it), instead, + // it will consider authenticated any request with the header `X-Docker-Token: fake-token`. + // + // Because we know that the client's transport is an `*authTransport` we simply cast it, + // in order to set the internal cached token to the fake token, and thus send that fake token + // upon every subsequent requests. + r.client.Transport.(*authTransport).token = []string{"fake-token"} + return r +} + +type debugTransport struct { + http.RoundTripper + log func(...interface{}) +} + +func (tr debugTransport) RoundTrip(req *http.Request) (*http.Response, error) { + dump, err := httputil.DumpRequestOut(req, false) + if err != nil { + tr.log("could not dump request") + } + tr.log(string(dump)) + resp, err := tr.RoundTripper.RoundTrip(req) + if err != nil { + return nil, err + } + dump, err = httputil.DumpResponse(resp, false) + if err != nil { + tr.log("could not dump response") + } + tr.log(string(dump)) + return resp, err +} + +func TestSearchRepositories(t *testing.T) { + r := spawnTestRegistrySession(t) + results, err := r.searchRepositories(context.Background(), "fakequery", 25) + if err != nil { + t.Fatal(err) + } + if results == nil { + t.Fatal("Expected non-nil SearchResults object") + } + assert.Equal(t, results.NumResults, 1, "Expected 1 search results") + assert.Equal(t, results.Query, "fakequery", "Expected 'fakequery' as query") + assert.Equal(t, results.Results[0].StarCount, 42, "Expected 'fakeimage' to have 42 stars") +} + +func TestSearchErrors(t *testing.T) { + errorCases := []struct { + filtersArgs filters.Args + shouldReturnError bool + expectedError string + }{ + { + expectedError: "unexpected status code 500", + shouldReturnError: true, + }, + { + filtersArgs: filters.NewArgs(filters.Arg("type", "custom")), + expectedError: "invalid filter 'type'", + }, + { + filtersArgs: filters.NewArgs(filters.Arg("is-automated", "invalid")), + expectedError: "invalid filter 'is-automated=[invalid]'", + }, + { + filtersArgs: filters.NewArgs( + filters.Arg("is-automated", "true"), + filters.Arg("is-automated", "false"), + ), + expectedError: "invalid filter 'is-automated", + }, + { + filtersArgs: filters.NewArgs(filters.Arg("is-official", "invalid")), + expectedError: "invalid filter 'is-official=[invalid]'", + }, + { + filtersArgs: filters.NewArgs( + filters.Arg("is-official", "true"), + filters.Arg("is-official", "false"), + ), + expectedError: "invalid filter 'is-official", + }, + { + filtersArgs: filters.NewArgs(filters.Arg("stars", "invalid")), + expectedError: "invalid filter 'stars=invalid'", + }, + { + filtersArgs: filters.NewArgs( + filters.Arg("stars", "1"), + filters.Arg("stars", "invalid"), + ), + expectedError: "invalid filter 'stars=invalid'", + }, + } + for _, tc := range errorCases { + t.Run(tc.expectedError, func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if !tc.shouldReturnError { + t.Errorf("unexpected HTTP request") + } + http.Error(w, "no search for you", http.StatusInternalServerError) + })) + defer srv.Close() + + // Construct the search term by cutting the 'http://' prefix off srv.URL. + term := srv.URL[7:] + "/term" + + reg, err := NewService(ServiceOptions{}) + assert.NilError(t, err) + _, err = reg.Search(context.Background(), tc.filtersArgs, term, 0, nil, map[string][]string{}) + assert.ErrorContains(t, err, tc.expectedError) + if tc.shouldReturnError { + assert.Check(t, cerrdefs.IsUnknown(err), "got: %T: %v", err, err) + return + } + assert.Check(t, cerrdefs.IsInvalidArgument(err), "got: %T: %v", err, err) + }) + } +} + +func TestSearch(t *testing.T) { + const term = "term" + successCases := []struct { + name string + filtersArgs filters.Args + registryResults []registry.SearchResult + expectedResults []registry.SearchResult + }{ + { + name: "empty results", + registryResults: []registry.SearchResult{}, + expectedResults: []registry.SearchResult{}, + }, + { + name: "no filter", + registryResults: []registry.SearchResult{ + { + Name: "name", + Description: "description", + }, + }, + expectedResults: []registry.SearchResult{ + { + Name: "name", + Description: "description", + }, + }, + }, + { + name: "is-automated=true, no results", + filtersArgs: filters.NewArgs(filters.Arg("is-automated", "true")), + registryResults: []registry.SearchResult{ + { + Name: "name", + Description: "description", + }, + }, + expectedResults: []registry.SearchResult{}, + }, + { + name: "is-automated=true", + filtersArgs: filters.NewArgs(filters.Arg("is-automated", "true")), + registryResults: []registry.SearchResult{ + { + Name: "name", + Description: "description", + IsAutomated: true, //nolint:staticcheck // ignore SA1019 (field is deprecated). + }, + }, + expectedResults: []registry.SearchResult{}, + }, + { + name: "is-automated=false, IsAutomated reset to false", + filtersArgs: filters.NewArgs(filters.Arg("is-automated", "false")), + registryResults: []registry.SearchResult{ + { + Name: "name", + Description: "description", + IsAutomated: true, //nolint:staticcheck // ignore SA1019 (field is deprecated). + }, + }, + expectedResults: []registry.SearchResult{ + { + Name: "name", + Description: "description", + IsAutomated: false, //nolint:staticcheck // ignore SA1019 (field is deprecated). + }, + }, + }, + { + name: "is-automated=false", + filtersArgs: filters.NewArgs(filters.Arg("is-automated", "false")), + registryResults: []registry.SearchResult{ + { + Name: "name", + Description: "description", + }, + }, + expectedResults: []registry.SearchResult{ + { + Name: "name", + Description: "description", + }, + }, + }, + { + name: "is-official=true, no results", + filtersArgs: filters.NewArgs(filters.Arg("is-official", "true")), + registryResults: []registry.SearchResult{ + { + Name: "name", + Description: "description", + }, + }, + expectedResults: []registry.SearchResult{}, + }, + { + name: "is-official=true", + filtersArgs: filters.NewArgs(filters.Arg("is-official", "true")), + registryResults: []registry.SearchResult{ + { + Name: "name", + Description: "description", + IsOfficial: true, + }, + }, + expectedResults: []registry.SearchResult{ + { + Name: "name", + Description: "description", + IsOfficial: true, + }, + }, + }, + { + name: "is-official=false, no results", + filtersArgs: filters.NewArgs(filters.Arg("is-official", "false")), + registryResults: []registry.SearchResult{ + { + Name: "name", + Description: "description", + IsOfficial: true, + }, + }, + expectedResults: []registry.SearchResult{}, + }, + { + name: "is-official=false", + filtersArgs: filters.NewArgs(filters.Arg("is-official", "false")), + registryResults: []registry.SearchResult{ + { + Name: "name", + Description: "description", + IsOfficial: false, + }, + }, + expectedResults: []registry.SearchResult{ + { + Name: "name", + Description: "description", + IsOfficial: false, + }, + }, + }, + { + name: "stars=0", + filtersArgs: filters.NewArgs(filters.Arg("stars", "0")), + registryResults: []registry.SearchResult{ + { + Name: "name", + Description: "description", + StarCount: 0, + }, + }, + expectedResults: []registry.SearchResult{ + { + Name: "name", + Description: "description", + StarCount: 0, + }, + }, + }, + { + name: "stars=0, no results", + filtersArgs: filters.NewArgs(filters.Arg("stars", "1")), + registryResults: []registry.SearchResult{ + { + Name: "name", + Description: "description", + StarCount: 0, + }, + }, + expectedResults: []registry.SearchResult{}, + }, + { + name: "stars=1", + filtersArgs: filters.NewArgs(filters.Arg("stars", "1")), + registryResults: []registry.SearchResult{ + { + Name: "name0", + Description: "description0", + StarCount: 0, + }, + { + Name: "name1", + Description: "description1", + StarCount: 1, + }, + }, + expectedResults: []registry.SearchResult{ + { + Name: "name1", + Description: "description1", + StarCount: 1, + }, + }, + }, + { + name: "stars=1, is-official=true, is-automated=true", + filtersArgs: filters.NewArgs( + filters.Arg("stars", "1"), + filters.Arg("is-official", "true"), + filters.Arg("is-automated", "true"), + ), + registryResults: []registry.SearchResult{ + { + Name: "name0", + Description: "description0", + StarCount: 0, + IsOfficial: true, + IsAutomated: true, //nolint:staticcheck // ignore SA1019 (field is deprecated). + }, + { + Name: "name1", + Description: "description1", + StarCount: 1, + IsOfficial: false, + IsAutomated: true, //nolint:staticcheck // ignore SA1019 (field is deprecated). + }, + { + Name: "name2", + Description: "description2", + StarCount: 1, + IsOfficial: true, + }, + { + Name: "name3", + Description: "description3", + StarCount: 2, + IsOfficial: true, + IsAutomated: true, //nolint:staticcheck // ignore SA1019 (field is deprecated). + }, + }, + expectedResults: []registry.SearchResult{}, + }, + } + for _, tc := range successCases { + t.Run(tc.name, func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-type", "application/json") + json.NewEncoder(w).Encode(registry.SearchResults{ + Query: term, + NumResults: len(tc.registryResults), + Results: tc.registryResults, + }) + })) + defer srv.Close() + + // Construct the search term by cutting the 'http://' prefix off srv.URL. + searchTerm := srv.URL[7:] + "/" + term + + reg, err := NewService(ServiceOptions{}) + assert.NilError(t, err) + results, err := reg.Search(context.Background(), tc.filtersArgs, searchTerm, 0, nil, map[string][]string{}) + assert.NilError(t, err) + assert.DeepEqual(t, results, tc.expectedResults) + }) + } +} diff --git a/vendor/github.com/docker/docker/registry/service.go b/internal/registry/service.go similarity index 100% rename from vendor/github.com/docker/docker/registry/service.go rename to internal/registry/service.go diff --git a/vendor/github.com/docker/docker/registry/service_v2.go b/internal/registry/service_v2.go similarity index 100% rename from vendor/github.com/docker/docker/registry/service_v2.go rename to internal/registry/service_v2.go diff --git a/vendor/github.com/docker/docker/registry/types.go b/internal/registry/types.go similarity index 100% rename from vendor/github.com/docker/docker/registry/types.go rename to internal/registry/types.go diff --git a/vendor.mod b/vendor.mod index 87396832c4aa..5a1e7cbeb359 100644 --- a/vendor.mod +++ b/vendor.mod @@ -15,6 +15,7 @@ replace ( require ( dario.cat/mergo v1.0.1 github.com/containerd/errdefs v1.0.0 + github.com/containerd/log v0.1.0 github.com/containerd/platforms v1.0.0-rc.1 github.com/cpuguy83/go-md2man/v2 v2.0.7 github.com/creack/pty v1.1.24 @@ -55,6 +56,7 @@ require ( github.com/theupdateframework/notary v0.7.1-0.20210315103452-bf96a202a09a github.com/tonistiigi/go-rosetta v0.0.0-20220804170347-3f4430f2d346 github.com/xeipuuv/gojsonschema v1.2.0 + go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.60.0 go.opentelemetry.io/otel v1.35.0 go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc v1.35.0 go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.35.0 @@ -78,7 +80,6 @@ require ( github.com/cenkalti/backoff/v4 v4.3.0 // indirect github.com/cespare/xxhash/v2 v2.3.0 // indirect github.com/containerd/errdefs/pkg v0.3.0 // indirect - github.com/containerd/log v0.1.0 // indirect github.com/docker/go v1.5.1-1.0.20160303222718-d30aec9fd63c // indirect github.com/docker/go-events v0.0.0-20190806004212-e31b211e4f1c // indirect github.com/docker/go-metrics v0.0.1 // indirect @@ -105,7 +106,6 @@ require ( github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415 // indirect go.etcd.io/etcd/raft/v3 v3.5.16 // indirect go.opentelemetry.io/auto/sdk v1.1.0 // indirect - go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.60.0 // indirect go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.35.0 // indirect go.opentelemetry.io/proto/otlp v1.5.0 // indirect golang.org/x/crypto v0.37.0 // indirect diff --git a/vendor/modules.txt b/vendor/modules.txt index 2177350f88d8..f4115c073f7d 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -71,7 +71,6 @@ github.com/docker/docker/pkg/jsonmessage github.com/docker/docker/pkg/process github.com/docker/docker/pkg/progress github.com/docker/docker/pkg/streamformatter -github.com/docker/docker/registry # github.com/docker/docker-credential-helpers v0.9.3 ## explicit; go 1.21 github.com/docker/docker-credential-helpers/client From 7716219e17d935a6327e244333c1deeddc3574aa Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 24 Jul 2025 01:36:57 +0200 Subject: [PATCH 02/12] internal/registry: remove dead code Signed-off-by: Sebastiaan van Stijn --- internal/registry/auth.go | 38 -- internal/registry/auth_test.go | 106 ----- internal/registry/config.go | 45 -- internal/registry/errors.go | 16 - internal/registry/registry_mock_test.go | 30 +- internal/registry/registry_test.go | 17 + internal/registry/search.go | 170 -------- internal/registry/search_endpoint_v1.go | 213 ---------- internal/registry/search_endpoint_v1_test.go | 237 ----------- internal/registry/search_session.go | 248 ----------- internal/registry/search_test.go | 418 ------------------- internal/registry/service.go | 58 --- 12 files changed, 18 insertions(+), 1578 deletions(-) delete mode 100644 internal/registry/auth_test.go delete mode 100644 internal/registry/search.go delete mode 100644 internal/registry/search_endpoint_v1.go delete mode 100644 internal/registry/search_endpoint_v1_test.go delete mode 100644 internal/registry/search_session.go delete mode 100644 internal/registry/search_test.go diff --git a/internal/registry/auth.go b/internal/registry/auth.go index a361a04fe2f3..72192e5ad7a9 100644 --- a/internal/registry/auth.go +++ b/internal/registry/auth.go @@ -127,44 +127,6 @@ func v2AuthHTTPClient(endpoint *url.URL, authTransport http.RoundTripper, modifi }, nil } -// ConvertToHostname normalizes a registry URL which has http|https prepended -// to just its hostname. It is used to match credentials, which may be either -// stored as hostname or as hostname including scheme (in legacy configuration -// files). -func ConvertToHostname(maybeURL string) string { - stripped := maybeURL - if scheme, remainder, ok := strings.Cut(stripped, "://"); ok { - switch scheme { - case "http", "https": - stripped = remainder - default: - // unknown, or no scheme; doing nothing for now, as we never did. - } - } - stripped, _, _ = strings.Cut(stripped, "/") - return stripped -} - -// ResolveAuthConfig matches an auth configuration to a server address or a URL -func ResolveAuthConfig(authConfigs map[string]registry.AuthConfig, index *registry.IndexInfo) registry.AuthConfig { - configKey := GetAuthConfigKey(index) - // First try the happy case - if c, found := authConfigs[configKey]; found || index.Official { - return c - } - - // Maybe they have a legacy config file, we will iterate the keys converting - // them to the new format and testing - for registryURL, ac := range authConfigs { - if configKey == ConvertToHostname(registryURL) { - return ac - } - } - - // When all else fails, return an empty auth config - return registry.AuthConfig{} -} - // PingResponseError is used when the response from a ping // was received but invalid. type PingResponseError struct { diff --git a/internal/registry/auth_test.go b/internal/registry/auth_test.go deleted file mode 100644 index 39a29d90805e..000000000000 --- a/internal/registry/auth_test.go +++ /dev/null @@ -1,106 +0,0 @@ -package registry - -import ( - "testing" - - "github.com/moby/moby/api/types/registry" - "gotest.tools/v3/assert" -) - -func buildAuthConfigs() map[string]registry.AuthConfig { - authConfigs := map[string]registry.AuthConfig{} - - for _, reg := range []string{"testIndex", IndexServer} { - authConfigs[reg] = registry.AuthConfig{ - Username: "docker-user", - Password: "docker-pass", - } - } - - return authConfigs -} - -func TestResolveAuthConfigIndexServer(t *testing.T) { - authConfigs := buildAuthConfigs() - indexConfig := authConfigs[IndexServer] - - officialIndex := ®istry.IndexInfo{ - Official: true, - } - privateIndex := ®istry.IndexInfo{ - Official: false, - } - - resolved := ResolveAuthConfig(authConfigs, officialIndex) - assert.Equal(t, resolved, indexConfig, "Expected ResolveAuthConfig to return IndexServer") - - resolved = ResolveAuthConfig(authConfigs, privateIndex) - assert.Check(t, resolved != indexConfig, "Expected ResolveAuthConfig to not return IndexServer") -} - -func TestResolveAuthConfigFullURL(t *testing.T) { - authConfigs := buildAuthConfigs() - - registryAuth := registry.AuthConfig{ - Username: "foo-user", - Password: "foo-pass", - } - localAuth := registry.AuthConfig{ - Username: "bar-user", - Password: "bar-pass", - } - officialAuth := registry.AuthConfig{ - Username: "baz-user", - Password: "baz-pass", - } - authConfigs[IndexServer] = officialAuth - - expectedAuths := map[string]registry.AuthConfig{ - "registry.example.com": registryAuth, - "localhost:8000": localAuth, - "example.com": localAuth, - } - - validRegistries := map[string][]string{ - "registry.example.com": { - "https://registry.example.com/v1/", - "http://registry.example.com/v1/", - "registry.example.com", - "registry.example.com/v1/", - }, - "localhost:8000": { - "https://localhost:8000/v1/", - "http://localhost:8000/v1/", - "localhost:8000", - "localhost:8000/v1/", - }, - "example.com": { - "https://example.com/v1/", - "http://example.com/v1/", - "example.com", - "example.com/v1/", - }, - } - - for configKey, registries := range validRegistries { - configured, ok := expectedAuths[configKey] - if !ok { - t.Fail() - } - index := ®istry.IndexInfo{ - Name: configKey, - } - for _, reg := range registries { - authConfigs[reg] = configured - resolved := ResolveAuthConfig(authConfigs, index) - if resolved.Username != configured.Username || resolved.Password != configured.Password { - t.Errorf("%s -> %v != %v\n", reg, resolved, configured) - } - delete(authConfigs, reg) - resolved = ResolveAuthConfig(authConfigs, index) - if resolved.Username == configured.Username || resolved.Password == configured.Password { - t.Errorf("%s -> %v == %v\n", reg, resolved, configured) - } - } - } -} diff --git a/internal/registry/config.go b/internal/registry/config.go index baa078abdd07..a4817f89a5f4 100644 --- a/internal/registry/config.go +++ b/internal/registry/config.go @@ -106,19 +106,6 @@ func newServiceConfig(options ServiceOptions) (*serviceConfig, error) { return config, nil } -// copy constructs a new ServiceConfig with a copy of the configuration in config. -func (config *serviceConfig) copy() *registry.ServiceConfig { - ic := make(map[string]*registry.IndexInfo) - for key, value := range config.IndexConfigs { - ic[key] = value - } - return ®istry.ServiceConfig{ - InsecureRegistryCIDRs: append([]*registry.NetIPNet(nil), config.InsecureRegistryCIDRs...), - IndexConfigs: ic, - Mirrors: append([]string(nil), config.Mirrors...), - } -} - // loadMirrors loads mirrors to config, after removing duplicates. // Returns an error if mirrors contains an invalid mirror. func (config *serviceConfig) loadMirrors(mirrors []string) error { @@ -320,18 +307,12 @@ func ValidateIndexName(val string) (string, error) { } 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, "://") -} - func validateHostPort(s string) error { // Split host and port, and in case s can not be split, assume host only host, port, err := net.SplitHostPort(s) @@ -356,32 +337,6 @@ func validateHostPort(s string) error { return nil } -// newIndexInfo returns IndexInfo configuration from indexName -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 - } - - // Construct a non-configured index info. - return ®istry.IndexInfo{ - Name: indexName, - Mirrors: []string{}, - Secure: config.isSecureIndex(indexName), - } -} - -// GetAuthConfigKey special-cases using the full index address of the official -// index as the AuthConfig key, and uses the (host)name[:port] for private indexes. -func GetAuthConfigKey(index *registry.IndexInfo) string { - if index.Official { - return IndexServer - } - return index.Name -} - // ParseRepositoryInfo performs the breakdown of a repository name into a // [RepositoryInfo], but lacks registry configuration. // diff --git a/internal/registry/errors.go b/internal/registry/errors.go index d37155a789de..4174c91d15b4 100644 --- a/internal/registry/errors.go +++ b/internal/registry/errors.go @@ -49,19 +49,3 @@ func (invalidParameterErr) InvalidParameter() {} func (e invalidParameterErr) Unwrap() error { return e.error } - -type systemErr struct{ error } - -func (systemErr) System() {} - -func (e systemErr) Unwrap() error { - return e.error -} - -type errUnknown struct{ error } - -func (errUnknown) Unknown() {} - -func (e errUnknown) Unwrap() error { - return e.error -} diff --git a/internal/registry/registry_mock_test.go b/internal/registry/registry_mock_test.go index 986e7ee64939..8bed2f02bb00 100644 --- a/internal/registry/registry_mock_test.go +++ b/internal/registry/registry_mock_test.go @@ -13,10 +13,7 @@ import ( "gotest.tools/v3/assert" ) -var ( - testHTTPServer *httptest.Server - testHTTPSServer *httptest.Server -) +var testHTTPServer *httptest.Server func init() { r := http.NewServeMux() @@ -29,7 +26,6 @@ func init() { r.HandleFunc("/v2/version", handlerGetPing) testHTTPServer = httptest.NewServer(handlerAccessLog(r)) - testHTTPSServer = httptest.NewTLSServer(handlerAccessLog(r)) } func handlerAccessLog(handler http.Handler) http.Handler { @@ -44,30 +40,6 @@ func makeURL(req string) string { return testHTTPServer.URL + req } -func makeHTTPSURL(req string) string { - return testHTTPSServer.URL + req -} - -func makeIndex(req string) *registry.IndexInfo { - return ®istry.IndexInfo{ - Name: makeURL(req), - } -} - -func makeHTTPSIndex(req string) *registry.IndexInfo { - return ®istry.IndexInfo{ - Name: makeHTTPSURL(req), - } -} - -func makePublicIndex() *registry.IndexInfo { - return ®istry.IndexInfo{ - Name: IndexServer, - Secure: true, - Official: true, - } -} - func writeHeaders(w http.ResponseWriter) { h := w.Header() h.Add("Server", "docker-tests/mock") diff --git a/internal/registry/registry_test.go b/internal/registry/registry_test.go index 79c17fa3666e..55205f81622e 100644 --- a/internal/registry/registry_test.go +++ b/internal/registry/registry_test.go @@ -34,6 +34,23 @@ func overrideLookupIP(t *testing.T) { }) } +// newIndexInfo returns IndexInfo configuration from indexName +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 + } + + // Construct a non-configured index info. + return ®istry.IndexInfo{ + Name: indexName, + Mirrors: []string{}, + Secure: config.isSecureIndex(indexName), + } +} + func TestParseRepositoryInfo(t *testing.T) { type staticRepositoryInfo struct { Index *registry.IndexInfo diff --git a/internal/registry/search.go b/internal/registry/search.go deleted file mode 100644 index 24dc91ab022c..000000000000 --- a/internal/registry/search.go +++ /dev/null @@ -1,170 +0,0 @@ -package registry - -import ( - "context" - "net/http" - "strconv" - "strings" - - "github.com/containerd/log" - "github.com/docker/distribution/registry/client/auth" - "github.com/moby/moby/api/types/filters" - "github.com/moby/moby/api/types/registry" - "github.com/pkg/errors" -) - -var acceptedSearchFilterTags = map[string]bool{ - "is-automated": true, // Deprecated: the "is_automated" field is deprecated and will always be false in the future. - "is-official": true, - "stars": true, -} - -// Search queries the public registry for repositories matching the specified -// search term and filters. -func (s *Service) Search(ctx context.Context, searchFilters filters.Args, term string, limit int, authConfig *registry.AuthConfig, headers map[string][]string) ([]registry.SearchResult, error) { - if err := searchFilters.Validate(acceptedSearchFilterTags); err != nil { - return nil, err - } - - isAutomated, err := searchFilters.GetBoolOrDefault("is-automated", false) - if err != nil { - return nil, err - } - - // "is-automated" is deprecated and filtering for `true` will yield no results. - if isAutomated { - return []registry.SearchResult{}, nil - } - - isOfficial, err := searchFilters.GetBoolOrDefault("is-official", false) - if err != nil { - return nil, err - } - - hasStarFilter := 0 - if searchFilters.Contains("stars") { - hasStars := searchFilters.Get("stars") - for _, hasStar := range hasStars { - iHasStar, err := strconv.Atoi(hasStar) - if err != nil { - return nil, invalidParameterErr{errors.Wrapf(err, "invalid filter 'stars=%s'", hasStar)} - } - if iHasStar > hasStarFilter { - hasStarFilter = iHasStar - } - } - } - - unfilteredResult, err := s.searchUnfiltered(ctx, term, limit, authConfig, headers) - if err != nil { - return nil, err - } - - filteredResults := []registry.SearchResult{} - for _, result := range unfilteredResult.Results { - if searchFilters.Contains("is-official") { - if isOfficial != result.IsOfficial { - continue - } - } - if searchFilters.Contains("stars") { - if result.StarCount < hasStarFilter { - continue - } - } - // "is-automated" is deprecated and the value in Docker Hub search - // results is untrustworthy. Force it to false so as to not mislead our - // clients. - result.IsAutomated = false //nolint:staticcheck // ignore SA1019 (field is deprecated) - filteredResults = append(filteredResults, result) - } - - return filteredResults, nil -} - -func (s *Service) searchUnfiltered(ctx context.Context, term string, limit int, authConfig *registry.AuthConfig, headers http.Header) (*registry.SearchResults, error) { - if hasScheme(term) { - return nil, invalidParamf("invalid repository name: repository name (%s) should not have a scheme", term) - } - - indexName, remoteName := splitReposSearchTerm(term) - - // Search is a long-running operation, just lock s.config to avoid block others. - s.mu.RLock() - index := newIndexInfo(s.config, indexName) - s.mu.RUnlock() - if index.Official { - // If pull "library/foo", it's stored locally under "foo" - remoteName = strings.TrimPrefix(remoteName, "library/") - } - - endpoint, err := newV1Endpoint(ctx, index, headers) - if err != nil { - return nil, err - } - - var client *http.Client - if authConfig != nil && authConfig.IdentityToken != "" && authConfig.Username != "" { - creds := NewStaticCredentialStore(authConfig) - - // TODO(thaJeztah); is there a reason not to include other headers here? (originally added in 19d48f0b8ba59eea9f2cac4ad1c7977712a6b7ac) - modifiers := Headers(headers.Get("User-Agent"), nil) - v2Client, err := v2AuthHTTPClient(endpoint.URL, endpoint.client.Transport, modifiers, creds, []auth.Scope{ - auth.RegistryScope{Name: "catalog", Actions: []string{"search"}}, - }) - if err != nil { - return nil, err - } - // Copy non transport http client features - v2Client.Timeout = endpoint.client.Timeout - v2Client.CheckRedirect = endpoint.client.CheckRedirect - v2Client.Jar = endpoint.client.Jar - - log.G(ctx).Debugf("using v2 client for search to %s", endpoint.URL) - client = v2Client - } else { - client = endpoint.client - if err := authorizeClient(ctx, client, authConfig, endpoint); err != nil { - return nil, err - } - } - - return newSession(client, endpoint).searchRepositories(ctx, remoteName, limit) -} - -// splitReposSearchTerm breaks a search term into an index name and remote name -func splitReposSearchTerm(reposName string) (string, string) { - nameParts := strings.SplitN(reposName, "/", 2) - if len(nameParts) == 1 || (!strings.Contains(nameParts[0], ".") && - !strings.Contains(nameParts[0], ":") && nameParts[0] != "localhost") { - // This is a Docker Hub repository (ex: samalba/hipache or ubuntu), - // use the default Docker Hub registry (docker.io) - return IndexName, reposName - } - return nameParts[0], nameParts[1] -} - -// ParseSearchIndexInfo will use repository name to get back an indexInfo. -// -// TODO(thaJeztah) this function is only used by the CLI, and used to get -// information of the registry (to provide credentials if needed). We should -// move this function (or equivalent) to the CLI, as it's doing too much just -// for that. -func ParseSearchIndexInfo(reposName string) (*registry.IndexInfo, error) { - indexName, _ := splitReposSearchTerm(reposName) - indexName = normalizeIndexName(indexName) - if indexName == IndexName { - return ®istry.IndexInfo{ - Name: IndexName, - Mirrors: []string{}, - Secure: true, - Official: true, - }, nil - } - - return ®istry.IndexInfo{ - Name: indexName, - Mirrors: []string{}, - Secure: !isInsecure(indexName), - }, nil -} diff --git a/internal/registry/search_endpoint_v1.go b/internal/registry/search_endpoint_v1.go deleted file mode 100644 index d6a663012516..000000000000 --- a/internal/registry/search_endpoint_v1.go +++ /dev/null @@ -1,213 +0,0 @@ -package registry - -import ( - "context" - "crypto/tls" - "encoding/json" - "errors" - "fmt" - "net/http" - "net/url" - "strings" - - "github.com/containerd/log" - "github.com/docker/distribution/registry/client/transport" - "github.com/moby/moby/api/types/registry" -) - -// v1PingResult contains the information returned when pinging a registry. It -// indicates whether the registry claims to be a standalone registry. -type v1PingResult struct { - // Standalone is set to true if the registry indicates it is a - // standalone registry in the X-Docker-Registry-Standalone - // header - Standalone bool `json:"standalone"` -} - -// v1Endpoint stores basic information about a V1 registry endpoint. -type v1Endpoint struct { - client *http.Client - URL *url.URL - IsSecure bool -} - -// newV1Endpoint parses the given address to return a registry endpoint. -// TODO: remove. This is only used by search. -func newV1Endpoint(ctx context.Context, index *registry.IndexInfo, headers http.Header) (*v1Endpoint, error) { - tlsConfig, err := newTLSConfig(ctx, index.Name, index.Secure) - if err != nil { - return nil, err - } - - endpoint, err := newV1EndpointFromStr(GetAuthConfigKey(index), tlsConfig, headers) - if err != nil { - return nil, err - } - - if endpoint.String() == IndexServer { - // Skip the check, we know this one is valid - // (and we never want to fall back to http in case of error) - return endpoint, nil - } - - // Try HTTPS ping to registry - endpoint.URL.Scheme = "https" - if _, err := endpoint.ping(ctx); err != nil { - if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) { - return nil, err - } - if endpoint.IsSecure { - // If registry is secure and HTTPS failed, show user the error and tell them about `--insecure-registry` - // in case that's what they need. DO NOT accept unknown CA certificates, and DO NOT fall back to HTTP. - hint := fmt.Sprintf( - ". If this private registry supports only HTTP or HTTPS with an unknown CA certificate, add `--insecure-registry %[1]s` to the daemon's arguments. "+ - "In the case of HTTPS, if you have access to the registry's CA certificate, no need for the flag; place the CA certificate at /etc/docker/certs.d/%[1]s/ca.crt", - endpoint.URL.Host, - ) - return nil, invalidParamf("invalid registry endpoint %s: %v%s", endpoint, err, hint) - } - - // registry is insecure and HTTPS failed, fallback to HTTP. - log.G(ctx).WithError(err).Debugf("error from registry %q marked as insecure - insecurely falling back to HTTP", endpoint) - endpoint.URL.Scheme = "http" - if _, err2 := endpoint.ping(ctx); err2 != nil { - return nil, invalidParamf("invalid registry endpoint %q. HTTPS attempt: %v. HTTP attempt: %v", endpoint, err, err2) - } - } - - return endpoint, nil -} - -// trimV1Address trims the "v1" version suffix off the address and returns -// the trimmed address. It returns an error on "v2" endpoints. -func trimV1Address(address string) (string, error) { - trimmed := strings.TrimSuffix(address, "/") - if strings.HasSuffix(trimmed, "/v2") { - return "", invalidParamf("search is not supported on v2 endpoints: %s", address) - } - return strings.TrimSuffix(trimmed, "/v1"), nil -} - -func newV1EndpointFromStr(address string, tlsConfig *tls.Config, headers http.Header) (*v1Endpoint, error) { - if !strings.HasPrefix(address, "http://") && !strings.HasPrefix(address, "https://") { - address = "https://" + address - } - - address, err := trimV1Address(address) - if err != nil { - return nil, err - } - - uri, err := url.Parse(address) - if err != nil { - return nil, invalidParam(err) - } - - // TODO(tiborvass): make sure a ConnectTimeout transport is used - tr := newTransport(tlsConfig) - - return &v1Endpoint{ - IsSecure: tlsConfig == nil || !tlsConfig.InsecureSkipVerify, - URL: uri, - client: httpClient(transport.NewTransport(tr, Headers("", headers)...)), - }, nil -} - -// Get the formatted URL for the root of this registry Endpoint -func (e *v1Endpoint) String() string { - return e.URL.String() + "/v1/" -} - -// ping returns a v1PingResult which indicates whether the registry is standalone or not. -func (e *v1Endpoint) ping(ctx context.Context) (v1PingResult, error) { - if e.String() == IndexServer { - // Skip the check, we know this one is valid - // (and we never want to fallback to http in case of error) - return v1PingResult{}, nil - } - - pingURL := e.String() + "_ping" - log.G(ctx).WithField("url", pingURL).Debug("attempting v1 ping for registry endpoint") - req, err := http.NewRequestWithContext(ctx, http.MethodGet, pingURL, http.NoBody) - if err != nil { - return v1PingResult{}, invalidParam(err) - } - - resp, err := e.client.Do(req) - if err != nil { - if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) { - return v1PingResult{}, err - } - return v1PingResult{}, invalidParam(err) - } - - defer resp.Body.Close() - - if v := resp.Header.Get("X-Docker-Registry-Standalone"); v != "" { - info := v1PingResult{} - // Accepted values are "1", and "true" (case-insensitive). - if v == "1" || strings.EqualFold(v, "true") { - info.Standalone = true - } - log.G(ctx).Debugf("v1PingResult.Standalone (from X-Docker-Registry-Standalone header): %t", info.Standalone) - return info, nil - } - - // If the header is absent, we assume true for compatibility with earlier - // versions of the registry. default to true - info := v1PingResult{ - Standalone: true, - } - if err := json.NewDecoder(resp.Body).Decode(&info); err != nil { - log.G(ctx).WithError(err).Debug("error unmarshaling _ping response") - // don't stop here. Just assume sane defaults - } - - log.G(ctx).Debugf("v1PingResult.Standalone: %t", info.Standalone) - return info, nil -} - -// httpClient returns an HTTP client structure which uses the given transport -// and contains the necessary headers for redirected requests -func httpClient(tr http.RoundTripper) *http.Client { - return &http.Client{ - Transport: tr, - CheckRedirect: addRequiredHeadersToRedirectedRequests, - } -} - -func trustedLocation(req *http.Request) bool { - var ( - trusteds = []string{"docker.com", "docker.io"} - hostname = strings.SplitN(req.Host, ":", 2)[0] - ) - if req.URL.Scheme != "https" { - return false - } - - for _, trusted := range trusteds { - if hostname == trusted || strings.HasSuffix(hostname, "."+trusted) { - return true - } - } - return false -} - -// addRequiredHeadersToRedirectedRequests adds the necessary redirection headers -// for redirected requests -func addRequiredHeadersToRedirectedRequests(req *http.Request, via []*http.Request) error { - if len(via) != 0 && via[0] != nil { - if trustedLocation(req) && trustedLocation(via[0]) { - req.Header = via[0].Header - return nil - } - for k, v := range via[0].Header { - if k != "Authorization" { - for _, vv := range v { - req.Header.Add(k, vv) - } - } - } - } - return nil -} diff --git a/internal/registry/search_endpoint_v1_test.go b/internal/registry/search_endpoint_v1_test.go deleted file mode 100644 index 137e2a60fc84..000000000000 --- a/internal/registry/search_endpoint_v1_test.go +++ /dev/null @@ -1,237 +0,0 @@ -package registry - -import ( - "context" - "net/http" - "net/http/httptest" - "strings" - "testing" - - "github.com/moby/moby/api/types/registry" - "gotest.tools/v3/assert" - is "gotest.tools/v3/assert/cmp" -) - -func TestV1EndpointPing(t *testing.T) { - testPing := func(index *registry.IndexInfo, expectedStandalone bool, assertMessage string) { - ep, err := newV1Endpoint(context.Background(), index, nil) - if err != nil { - t.Fatal(err) - } - regInfo, err := ep.ping(context.Background()) - if err != nil { - t.Fatal(err) - } - - assert.Equal(t, regInfo.Standalone, expectedStandalone, assertMessage) - } - - testPing(makeIndex("/v1/"), true, "Expected standalone to be true (default)") - testPing(makeHTTPSIndex("/v1/"), true, "Expected standalone to be true (default)") - testPing(makePublicIndex(), false, "Expected standalone to be false for public index") -} - -func TestV1Endpoint(t *testing.T) { - // Simple wrapper to fail test if err != nil - expandEndpoint := func(index *registry.IndexInfo) *v1Endpoint { - endpoint, err := newV1Endpoint(context.Background(), index, nil) - if err != nil { - t.Fatal(err) - } - return endpoint - } - - assertInsecureIndex := func(index *registry.IndexInfo) { - index.Secure = true - _, err := newV1Endpoint(context.Background(), index, nil) - assert.ErrorContains(t, err, "insecure-registry", index.Name+": Expected insecure-registry error for insecure index") - index.Secure = false - } - - assertSecureIndex := func(index *registry.IndexInfo) { - index.Secure = true - _, err := newV1Endpoint(context.Background(), index, nil) - assert.ErrorContains(t, err, "certificate signed by unknown authority", index.Name+": Expected cert error for secure index") - index.Secure = false - } - - index := ®istry.IndexInfo{} - index.Name = makeURL("/v1/") - endpoint := expandEndpoint(index) - assert.Equal(t, endpoint.String(), index.Name, "Expected endpoint to be "+index.Name) - assertInsecureIndex(index) - - index.Name = makeURL("") - endpoint = expandEndpoint(index) - assert.Equal(t, endpoint.String(), index.Name+"/v1/", index.Name+": Expected endpoint to be "+index.Name+"/v1/") - assertInsecureIndex(index) - - httpURL := makeURL("") - index.Name = strings.SplitN(httpURL, "://", 2)[1] - endpoint = expandEndpoint(index) - assert.Equal(t, endpoint.String(), httpURL+"/v1/", index.Name+": Expected endpoint to be "+httpURL+"/v1/") - assertInsecureIndex(index) - - index.Name = makeHTTPSURL("/v1/") - endpoint = expandEndpoint(index) - assert.Equal(t, endpoint.String(), index.Name, "Expected endpoint to be "+index.Name) - assertSecureIndex(index) - - index.Name = makeHTTPSURL("") - endpoint = expandEndpoint(index) - assert.Equal(t, endpoint.String(), index.Name+"/v1/", index.Name+": Expected endpoint to be "+index.Name+"/v1/") - assertSecureIndex(index) - - httpsURL := makeHTTPSURL("") - index.Name = strings.SplitN(httpsURL, "://", 2)[1] - endpoint = expandEndpoint(index) - assert.Equal(t, endpoint.String(), httpsURL+"/v1/", index.Name+": Expected endpoint to be "+httpsURL+"/v1/") - assertSecureIndex(index) - - badEndpoints := []string{ - "http://127.0.0.1/v1/", - "https://127.0.0.1/v1/", - "http://127.0.0.1", - "https://127.0.0.1", - "127.0.0.1", - } - for _, address := range badEndpoints { - index.Name = address - _, err := newV1Endpoint(context.Background(), index, nil) - assert.Check(t, err != nil, "Expected error while expanding bad endpoint: %s", address) - } -} - -func TestV1EndpointParse(t *testing.T) { - tests := []struct { - address string - expected string - expectedErr string - }{ - { - address: IndexServer, - expected: IndexServer, - }, - { - address: "https://0.0.0.0:5000/v1/", - expected: "https://0.0.0.0:5000/v1/", - }, - { - address: "https://0.0.0.0:5000", - expected: "https://0.0.0.0:5000/v1/", - }, - { - address: "0.0.0.0:5000", - expected: "https://0.0.0.0:5000/v1/", - }, - { - address: "https://0.0.0.0:5000/nonversion/", - expected: "https://0.0.0.0:5000/nonversion/v1/", - }, - { - address: "https://0.0.0.0:5000/v0/", - expected: "https://0.0.0.0:5000/v0/v1/", - }, - { - address: "https://0.0.0.0:5000/v2/", - expectedErr: "search is not supported on v2 endpoints: https://0.0.0.0:5000/v2/", - }, - } - for _, tc := range tests { - t.Run(tc.address, func(t *testing.T) { - ep, err := newV1EndpointFromStr(tc.address, nil, nil) - if tc.expectedErr != "" { - assert.Check(t, is.Error(err, tc.expectedErr)) - assert.Check(t, is.Nil(ep)) - } else { - assert.NilError(t, err) - assert.Check(t, is.Equal(ep.String(), tc.expected)) - } - }) - } -} - -// Ensure that a registry endpoint that responds with a 401 only is determined -// to be a valid v1 registry endpoint -func TestV1EndpointValidate(t *testing.T) { - requireBasicAuthHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Add("WWW-Authenticate", `Basic realm="localhost"`) - w.WriteHeader(http.StatusUnauthorized) - }) - - // Make a test server which should validate as a v1 server. - testServer := httptest.NewServer(requireBasicAuthHandler) - defer testServer.Close() - - testEndpoint, err := newV1Endpoint(context.Background(), ®istry.IndexInfo{Name: testServer.URL}, nil) - if err != nil { - t.Fatal(err) - } - - if testEndpoint.URL.Scheme != "http" { - t.Fatalf("expecting to validate endpoint as http, got url %s", testEndpoint.String()) - } -} - -func TestTrustedLocation(t *testing.T) { - for _, u := range []string{"http://example.com", "https://example.com:7777", "http://docker.io", "http://test.docker.com", "https://fakedocker.com"} { - req, _ := http.NewRequest(http.MethodGet, u, http.NoBody) - assert.Check(t, !trustedLocation(req)) - } - - for _, u := range []string{"https://docker.io", "https://test.docker.com:80"} { - req, _ := http.NewRequest(http.MethodGet, u, http.NoBody) - assert.Check(t, trustedLocation(req)) - } -} - -func TestAddRequiredHeadersToRedirectedRequests(t *testing.T) { - for _, urls := range [][]string{ - {"http://docker.io", "https://docker.com"}, - {"https://foo.docker.io:7777", "http://bar.docker.com"}, - {"https://foo.docker.io", "https://example.com"}, - } { - reqFrom, _ := http.NewRequest(http.MethodGet, urls[0], http.NoBody) - reqFrom.Header.Add("Content-Type", "application/json") - reqFrom.Header.Add("Authorization", "super_secret") - reqTo, _ := http.NewRequest(http.MethodGet, urls[1], http.NoBody) - - _ = addRequiredHeadersToRedirectedRequests(reqTo, []*http.Request{reqFrom}) - - if len(reqTo.Header) != 1 { - t.Fatalf("Expected 1 headers, got %d", len(reqTo.Header)) - } - - if reqTo.Header.Get("Content-Type") != "application/json" { - t.Fatal("'Content-Type' should be 'application/json'") - } - - if reqTo.Header.Get("Authorization") != "" { - t.Fatal("'Authorization' should be empty") - } - } - - for _, urls := range [][]string{ - {"https://docker.io", "https://docker.com"}, - {"https://foo.docker.io:7777", "https://bar.docker.com"}, - } { - reqFrom, _ := http.NewRequest(http.MethodGet, urls[0], http.NoBody) - reqFrom.Header.Add("Content-Type", "application/json") - reqFrom.Header.Add("Authorization", "super_secret") - reqTo, _ := http.NewRequest(http.MethodGet, urls[1], http.NoBody) - - _ = addRequiredHeadersToRedirectedRequests(reqTo, []*http.Request{reqFrom}) - - if len(reqTo.Header) != 2 { - t.Fatalf("Expected 2 headers, got %d", len(reqTo.Header)) - } - - if reqTo.Header.Get("Content-Type") != "application/json" { - t.Fatal("'Content-Type' should be 'application/json'") - } - - if reqTo.Header.Get("Authorization") != "super_secret" { - t.Fatal("'Authorization' should be 'super_secret'") - } - } -} diff --git a/internal/registry/search_session.go b/internal/registry/search_session.go deleted file mode 100644 index 51d3e990abd8..000000000000 --- a/internal/registry/search_session.go +++ /dev/null @@ -1,248 +0,0 @@ -package registry - -import ( - // this is required for some certificates - "context" - _ "crypto/sha512" - "encoding/json" - "fmt" - "io" - "net/http" - "net/http/cookiejar" - "net/url" - "strconv" - "strings" - "sync" - - "github.com/containerd/log" - "github.com/moby/moby/api/types/registry" - "github.com/pkg/errors" -) - -// A session is used to communicate with a V1 registry -type session struct { - indexEndpoint *v1Endpoint - client *http.Client -} - -type authTransport struct { - base http.RoundTripper - authConfig *registry.AuthConfig - - alwaysSetBasicAuth bool - token []string - - mu sync.Mutex // guards modReq - modReq map[*http.Request]*http.Request // original -> modified -} - -// newAuthTransport handles the auth layer when communicating with a v1 registry (private or official) -// -// For private v1 registries, set alwaysSetBasicAuth to true. -// -// For the official v1 registry, if there isn't already an Authorization header in the request, -// but there is an X-Docker-Token header set to true, then Basic Auth will be used to set the Authorization header. -// After sending the request with the provided base http.RoundTripper, if an X-Docker-Token header, representing -// a token, is present in the response, then it gets cached and sent in the Authorization header of all subsequent -// requests. -// -// If the server sends a token without the client having requested it, it is ignored. -// -// This RoundTripper also has a CancelRequest method important for correct timeout handling. -func newAuthTransport(base http.RoundTripper, authConfig *registry.AuthConfig, alwaysSetBasicAuth bool) *authTransport { - if base == nil { - base = http.DefaultTransport - } - return &authTransport{ - base: base, - authConfig: authConfig, - alwaysSetBasicAuth: alwaysSetBasicAuth, - modReq: make(map[*http.Request]*http.Request), - } -} - -// cloneRequest returns a clone of the provided *http.Request. -// The clone is a shallow copy of the struct and its Header map. -func cloneRequest(r *http.Request) *http.Request { - // shallow copy of the struct - r2 := new(http.Request) - *r2 = *r - // deep copy of the Header - r2.Header = make(http.Header, len(r.Header)) - for k, s := range r.Header { - r2.Header[k] = append([]string(nil), s...) - } - - return r2 -} - -// onEOFReader wraps an io.ReadCloser and a function -// the function will run at the end of file or close the file. -type onEOFReader struct { - Rc io.ReadCloser - Fn func() -} - -func (r *onEOFReader) Read(p []byte) (int, error) { - n, err := r.Rc.Read(p) - if err == io.EOF { - r.runFunc() - } - return n, err -} - -// Close closes the file and run the function. -func (r *onEOFReader) Close() error { - err := r.Rc.Close() - r.runFunc() - return err -} - -func (r *onEOFReader) runFunc() { - if fn := r.Fn; fn != nil { - fn() - r.Fn = nil - } -} - -// RoundTrip changes an HTTP request's headers to add the necessary -// authentication-related headers -func (tr *authTransport) RoundTrip(orig *http.Request) (*http.Response, error) { - // Authorization should not be set on 302 redirect for untrusted locations. - // This logic mirrors the behavior in addRequiredHeadersToRedirectedRequests. - // As the authorization logic is currently implemented in RoundTrip, - // a 302 redirect is detected by looking at the Referrer header as go http package adds said header. - // This is safe as Docker doesn't set Referrer in other scenarios. - if orig.Header.Get("Referer") != "" && !trustedLocation(orig) { - return tr.base.RoundTrip(orig) - } - - req := cloneRequest(orig) - tr.mu.Lock() - tr.modReq[orig] = req - tr.mu.Unlock() - - if tr.alwaysSetBasicAuth { - if tr.authConfig == nil { - return nil, errors.New("unexpected error: empty auth config") - } - req.SetBasicAuth(tr.authConfig.Username, tr.authConfig.Password) - return tr.base.RoundTrip(req) - } - - // Don't override - if req.Header.Get("Authorization") == "" { - if req.Header.Get("X-Docker-Token") == "true" && tr.authConfig != nil && tr.authConfig.Username != "" { - req.SetBasicAuth(tr.authConfig.Username, tr.authConfig.Password) - } else if len(tr.token) > 0 { - req.Header.Set("Authorization", "Token "+strings.Join(tr.token, ",")) - } - } - resp, err := tr.base.RoundTrip(req) - if err != nil { - tr.mu.Lock() - delete(tr.modReq, orig) - tr.mu.Unlock() - return nil, err - } - if len(resp.Header["X-Docker-Token"]) > 0 { - tr.token = resp.Header["X-Docker-Token"] - } - resp.Body = &onEOFReader{ - Rc: resp.Body, - Fn: func() { - tr.mu.Lock() - delete(tr.modReq, orig) - tr.mu.Unlock() - }, - } - return resp, nil -} - -// CancelRequest cancels an in-flight request by closing its connection. -func (tr *authTransport) CancelRequest(req *http.Request) { - type canceler interface { - CancelRequest(*http.Request) - } - if cr, ok := tr.base.(canceler); ok { - tr.mu.Lock() - modReq := tr.modReq[req] - delete(tr.modReq, req) - tr.mu.Unlock() - cr.CancelRequest(modReq) - } -} - -func authorizeClient(ctx context.Context, client *http.Client, authConfig *registry.AuthConfig, endpoint *v1Endpoint) error { - var alwaysSetBasicAuth bool - - // If we're working with a standalone private registry over HTTPS, send Basic Auth headers - // alongside all our requests. - if endpoint.String() != IndexServer && endpoint.URL.Scheme == "https" { - info, err := endpoint.ping(ctx) - if err != nil { - return err - } - if info.Standalone && authConfig != nil { - log.G(ctx).WithField("endpoint", endpoint.String()).Debug("Endpoint is eligible for private registry; enabling alwaysSetBasicAuth") - alwaysSetBasicAuth = true - } - } - - // Annotate the transport unconditionally so that v2 can - // properly fallback on v1 when an image is not found. - client.Transport = newAuthTransport(client.Transport, authConfig, alwaysSetBasicAuth) - - jar, err := cookiejar.New(nil) - if err != nil { - return systemErr{errors.New("cookiejar.New is not supposed to return an error")} - } - client.Jar = jar - - return nil -} - -func newSession(client *http.Client, endpoint *v1Endpoint) *session { - return &session{ - client: client, - indexEndpoint: endpoint, - } -} - -// defaultSearchLimit is the default value for maximum number of returned search results. -const defaultSearchLimit = 25 - -// searchRepositories performs a search against the remote repository -func (r *session) searchRepositories(ctx context.Context, term string, limit int) (*registry.SearchResults, error) { - if limit == 0 { - limit = defaultSearchLimit - } - if limit < 1 || limit > 100 { - return nil, invalidParamf("limit %d is outside the range of [1, 100]", limit) - } - u := r.indexEndpoint.String() + "search?q=" + url.QueryEscape(term) + "&n=" + url.QueryEscape(strconv.Itoa(limit)) - log.G(ctx).WithField("url", u).Debug("searchRepositories") - - req, err := http.NewRequestWithContext(ctx, http.MethodGet, u, http.NoBody) - if err != nil { - return nil, invalidParamWrapf(err, "error building request") - } - // Have the AuthTransport send authentication, when logged in. - req.Header.Set("X-Docker-Token", "true") - res, err := r.client.Do(req) - if err != nil { - return nil, systemErr{err} - } - defer res.Body.Close() - if res.StatusCode != http.StatusOK { - // TODO(thaJeztah): return upstream response body for errors (see https://github.com/moby/moby/issues/27286). - // TODO(thaJeztah): handle other status-codes to return correct error-type - return nil, errUnknown{fmt.Errorf("unexpected status code %d", res.StatusCode)} - } - result := ®istry.SearchResults{} - err = json.NewDecoder(res.Body).Decode(result) - if err != nil { - return nil, systemErr{errors.Wrap(err, "error decoding registry search results")} - } - return result, nil -} diff --git a/internal/registry/search_test.go b/internal/registry/search_test.go deleted file mode 100644 index 7af875767faa..000000000000 --- a/internal/registry/search_test.go +++ /dev/null @@ -1,418 +0,0 @@ -package registry - -import ( - "context" - "encoding/json" - "net/http" - "net/http/httptest" - "net/http/httputil" - "testing" - - cerrdefs "github.com/containerd/errdefs" - "github.com/docker/distribution/registry/client/transport" - "github.com/moby/moby/api/types/filters" - "github.com/moby/moby/api/types/registry" - "gotest.tools/v3/assert" -) - -func spawnTestRegistrySession(t *testing.T) *session { - t.Helper() - authConfig := ®istry.AuthConfig{} - endpoint, err := newV1Endpoint(context.Background(), makeIndex("/v1/"), nil) - if err != nil { - t.Fatal(err) - } - userAgent := "docker test client" - var tr http.RoundTripper = debugTransport{newTransport(nil), t.Log} - tr = transport.NewTransport(newAuthTransport(tr, authConfig, false), Headers(userAgent, nil)...) - client := httpClient(tr) - - if err := authorizeClient(context.Background(), client, authConfig, endpoint); err != nil { - t.Fatal(err) - } - r := newSession(client, endpoint) - - // In a normal scenario for the v1 registry, the client should send a `X-Docker-Token: true` - // header while authenticating, in order to retrieve a token that can be later used to - // perform authenticated actions. - // - // The mock v1 registry does not support that, (TODO(tiborvass): support it), instead, - // it will consider authenticated any request with the header `X-Docker-Token: fake-token`. - // - // Because we know that the client's transport is an `*authTransport` we simply cast it, - // in order to set the internal cached token to the fake token, and thus send that fake token - // upon every subsequent requests. - r.client.Transport.(*authTransport).token = []string{"fake-token"} - return r -} - -type debugTransport struct { - http.RoundTripper - log func(...interface{}) -} - -func (tr debugTransport) RoundTrip(req *http.Request) (*http.Response, error) { - dump, err := httputil.DumpRequestOut(req, false) - if err != nil { - tr.log("could not dump request") - } - tr.log(string(dump)) - resp, err := tr.RoundTripper.RoundTrip(req) - if err != nil { - return nil, err - } - dump, err = httputil.DumpResponse(resp, false) - if err != nil { - tr.log("could not dump response") - } - tr.log(string(dump)) - return resp, err -} - -func TestSearchRepositories(t *testing.T) { - r := spawnTestRegistrySession(t) - results, err := r.searchRepositories(context.Background(), "fakequery", 25) - if err != nil { - t.Fatal(err) - } - if results == nil { - t.Fatal("Expected non-nil SearchResults object") - } - assert.Equal(t, results.NumResults, 1, "Expected 1 search results") - assert.Equal(t, results.Query, "fakequery", "Expected 'fakequery' as query") - assert.Equal(t, results.Results[0].StarCount, 42, "Expected 'fakeimage' to have 42 stars") -} - -func TestSearchErrors(t *testing.T) { - errorCases := []struct { - filtersArgs filters.Args - shouldReturnError bool - expectedError string - }{ - { - expectedError: "unexpected status code 500", - shouldReturnError: true, - }, - { - filtersArgs: filters.NewArgs(filters.Arg("type", "custom")), - expectedError: "invalid filter 'type'", - }, - { - filtersArgs: filters.NewArgs(filters.Arg("is-automated", "invalid")), - expectedError: "invalid filter 'is-automated=[invalid]'", - }, - { - filtersArgs: filters.NewArgs( - filters.Arg("is-automated", "true"), - filters.Arg("is-automated", "false"), - ), - expectedError: "invalid filter 'is-automated", - }, - { - filtersArgs: filters.NewArgs(filters.Arg("is-official", "invalid")), - expectedError: "invalid filter 'is-official=[invalid]'", - }, - { - filtersArgs: filters.NewArgs( - filters.Arg("is-official", "true"), - filters.Arg("is-official", "false"), - ), - expectedError: "invalid filter 'is-official", - }, - { - filtersArgs: filters.NewArgs(filters.Arg("stars", "invalid")), - expectedError: "invalid filter 'stars=invalid'", - }, - { - filtersArgs: filters.NewArgs( - filters.Arg("stars", "1"), - filters.Arg("stars", "invalid"), - ), - expectedError: "invalid filter 'stars=invalid'", - }, - } - for _, tc := range errorCases { - t.Run(tc.expectedError, func(t *testing.T) { - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if !tc.shouldReturnError { - t.Errorf("unexpected HTTP request") - } - http.Error(w, "no search for you", http.StatusInternalServerError) - })) - defer srv.Close() - - // Construct the search term by cutting the 'http://' prefix off srv.URL. - term := srv.URL[7:] + "/term" - - reg, err := NewService(ServiceOptions{}) - assert.NilError(t, err) - _, err = reg.Search(context.Background(), tc.filtersArgs, term, 0, nil, map[string][]string{}) - assert.ErrorContains(t, err, tc.expectedError) - if tc.shouldReturnError { - assert.Check(t, cerrdefs.IsUnknown(err), "got: %T: %v", err, err) - return - } - assert.Check(t, cerrdefs.IsInvalidArgument(err), "got: %T: %v", err, err) - }) - } -} - -func TestSearch(t *testing.T) { - const term = "term" - successCases := []struct { - name string - filtersArgs filters.Args - registryResults []registry.SearchResult - expectedResults []registry.SearchResult - }{ - { - name: "empty results", - registryResults: []registry.SearchResult{}, - expectedResults: []registry.SearchResult{}, - }, - { - name: "no filter", - registryResults: []registry.SearchResult{ - { - Name: "name", - Description: "description", - }, - }, - expectedResults: []registry.SearchResult{ - { - Name: "name", - Description: "description", - }, - }, - }, - { - name: "is-automated=true, no results", - filtersArgs: filters.NewArgs(filters.Arg("is-automated", "true")), - registryResults: []registry.SearchResult{ - { - Name: "name", - Description: "description", - }, - }, - expectedResults: []registry.SearchResult{}, - }, - { - name: "is-automated=true", - filtersArgs: filters.NewArgs(filters.Arg("is-automated", "true")), - registryResults: []registry.SearchResult{ - { - Name: "name", - Description: "description", - IsAutomated: true, //nolint:staticcheck // ignore SA1019 (field is deprecated). - }, - }, - expectedResults: []registry.SearchResult{}, - }, - { - name: "is-automated=false, IsAutomated reset to false", - filtersArgs: filters.NewArgs(filters.Arg("is-automated", "false")), - registryResults: []registry.SearchResult{ - { - Name: "name", - Description: "description", - IsAutomated: true, //nolint:staticcheck // ignore SA1019 (field is deprecated). - }, - }, - expectedResults: []registry.SearchResult{ - { - Name: "name", - Description: "description", - IsAutomated: false, //nolint:staticcheck // ignore SA1019 (field is deprecated). - }, - }, - }, - { - name: "is-automated=false", - filtersArgs: filters.NewArgs(filters.Arg("is-automated", "false")), - registryResults: []registry.SearchResult{ - { - Name: "name", - Description: "description", - }, - }, - expectedResults: []registry.SearchResult{ - { - Name: "name", - Description: "description", - }, - }, - }, - { - name: "is-official=true, no results", - filtersArgs: filters.NewArgs(filters.Arg("is-official", "true")), - registryResults: []registry.SearchResult{ - { - Name: "name", - Description: "description", - }, - }, - expectedResults: []registry.SearchResult{}, - }, - { - name: "is-official=true", - filtersArgs: filters.NewArgs(filters.Arg("is-official", "true")), - registryResults: []registry.SearchResult{ - { - Name: "name", - Description: "description", - IsOfficial: true, - }, - }, - expectedResults: []registry.SearchResult{ - { - Name: "name", - Description: "description", - IsOfficial: true, - }, - }, - }, - { - name: "is-official=false, no results", - filtersArgs: filters.NewArgs(filters.Arg("is-official", "false")), - registryResults: []registry.SearchResult{ - { - Name: "name", - Description: "description", - IsOfficial: true, - }, - }, - expectedResults: []registry.SearchResult{}, - }, - { - name: "is-official=false", - filtersArgs: filters.NewArgs(filters.Arg("is-official", "false")), - registryResults: []registry.SearchResult{ - { - Name: "name", - Description: "description", - IsOfficial: false, - }, - }, - expectedResults: []registry.SearchResult{ - { - Name: "name", - Description: "description", - IsOfficial: false, - }, - }, - }, - { - name: "stars=0", - filtersArgs: filters.NewArgs(filters.Arg("stars", "0")), - registryResults: []registry.SearchResult{ - { - Name: "name", - Description: "description", - StarCount: 0, - }, - }, - expectedResults: []registry.SearchResult{ - { - Name: "name", - Description: "description", - StarCount: 0, - }, - }, - }, - { - name: "stars=0, no results", - filtersArgs: filters.NewArgs(filters.Arg("stars", "1")), - registryResults: []registry.SearchResult{ - { - Name: "name", - Description: "description", - StarCount: 0, - }, - }, - expectedResults: []registry.SearchResult{}, - }, - { - name: "stars=1", - filtersArgs: filters.NewArgs(filters.Arg("stars", "1")), - registryResults: []registry.SearchResult{ - { - Name: "name0", - Description: "description0", - StarCount: 0, - }, - { - Name: "name1", - Description: "description1", - StarCount: 1, - }, - }, - expectedResults: []registry.SearchResult{ - { - Name: "name1", - Description: "description1", - StarCount: 1, - }, - }, - }, - { - name: "stars=1, is-official=true, is-automated=true", - filtersArgs: filters.NewArgs( - filters.Arg("stars", "1"), - filters.Arg("is-official", "true"), - filters.Arg("is-automated", "true"), - ), - registryResults: []registry.SearchResult{ - { - Name: "name0", - Description: "description0", - StarCount: 0, - IsOfficial: true, - IsAutomated: true, //nolint:staticcheck // ignore SA1019 (field is deprecated). - }, - { - Name: "name1", - Description: "description1", - StarCount: 1, - IsOfficial: false, - IsAutomated: true, //nolint:staticcheck // ignore SA1019 (field is deprecated). - }, - { - Name: "name2", - Description: "description2", - StarCount: 1, - IsOfficial: true, - }, - { - Name: "name3", - Description: "description3", - StarCount: 2, - IsOfficial: true, - IsAutomated: true, //nolint:staticcheck // ignore SA1019 (field is deprecated). - }, - }, - expectedResults: []registry.SearchResult{}, - }, - } - for _, tc := range successCases { - t.Run(tc.name, func(t *testing.T) { - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-type", "application/json") - json.NewEncoder(w).Encode(registry.SearchResults{ - Query: term, - NumResults: len(tc.registryResults), - Results: tc.registryResults, - }) - })) - defer srv.Close() - - // Construct the search term by cutting the 'http://' prefix off srv.URL. - searchTerm := srv.URL[7:] + "/" + term - - reg, err := NewService(ServiceOptions{}) - assert.NilError(t, err) - results, err := reg.Search(context.Background(), tc.filtersArgs, searchTerm, 0, nil, map[string][]string{}) - assert.NilError(t, err) - assert.DeepEqual(t, results, tc.expectedResults) - }) - } -} diff --git a/internal/registry/service.go b/internal/registry/service.go index 50291798b35e..6230cc42bbd4 100644 --- a/internal/registry/service.go +++ b/internal/registry/service.go @@ -6,11 +6,9 @@ import ( "errors" "net/url" "strings" - "sync" cerrdefs "github.com/containerd/errdefs" "github.com/containerd/log" - "github.com/distribution/reference" "github.com/moby/moby/api/types/registry" ) @@ -18,7 +16,6 @@ import ( // of mirrors. type Service struct { config *serviceConfig - mu sync.RWMutex } // NewService returns a new instance of [Service] ready to be installed into @@ -32,27 +29,6 @@ func NewService(options ServiceOptions) (*Service, error) { return &Service{config: config}, err } -// ServiceConfig returns a copy of the public registry service's configuration. -func (s *Service) ServiceConfig() *registry.ServiceConfig { - s.mu.RLock() - defer s.mu.RUnlock() - return s.config.copy() -} - -// ReplaceConfig prepares a transaction which will atomically replace the -// registry service's configuration when the returned commit function is called. -func (s *Service) ReplaceConfig(options ServiceOptions) (commit func(), _ error) { - config, err := newServiceConfig(options) - if err != nil { - return nil, err - } - return func() { - s.mu.Lock() - defer s.mu.Unlock() - s.config = config - }, nil -} - // Auth contacts the public registry with the provided credentials, // and returns OK if authentication was successful. // It can be used to verify the validity of a client's credentials. @@ -74,9 +50,7 @@ func (s *Service) Auth(ctx context.Context, authConfig *registry.AuthConfig, use // Lookup endpoints for authentication but exclude mirrors to prevent // sending credentials of the upstream registry to a mirror. - s.mu.RLock() endpoints, err := s.lookupV2Endpoints(ctx, registryHostName, false) - s.mu.RUnlock() if err != nil { if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) { return "", "", err @@ -108,24 +82,6 @@ func (s *Service) Auth(ctx context.Context, authConfig *registry.AuthConfig, use return "", "", lastErr } -// ResolveAuthConfig looks up authentication for the given reference from the -// given authConfigs. -// -// IMPORTANT: This function is for internal use and should not be used by external projects. -func (s *Service) ResolveAuthConfig(authConfigs map[string]registry.AuthConfig, ref reference.Named) registry.AuthConfig { - s.mu.RLock() - defer s.mu.RUnlock() - // Simplified version of "newIndexInfo" without handling of insecure - // registries and mirrors, as we don't need that information to resolve - // the auth-config. - indexName := normalizeIndexName(reference.Domain(ref)) - registryInfo, ok := s.config.IndexConfigs[indexName] - if !ok { - registryInfo = ®istry.IndexInfo{Name: indexName} - } - return ResolveAuthConfig(authConfigs, registryInfo) -} - // APIEndpoint represents a remote API endpoint type APIEndpoint struct { Mirror bool @@ -136,25 +92,11 @@ type APIEndpoint struct { // LookupPullEndpoints creates a list of v2 endpoints to try to pull from, in order of preference. // It gives preference to mirrors over the actual registry, and HTTPS over plain HTTP. func (s *Service) LookupPullEndpoints(hostname string) ([]APIEndpoint, error) { - s.mu.RLock() - defer s.mu.RUnlock() - return s.lookupV2Endpoints(context.TODO(), hostname, true) } // LookupPushEndpoints creates a list of v2 endpoints to try to push to, in order of preference. // It gives preference to HTTPS over plain HTTP. Mirrors are not included. func (s *Service) LookupPushEndpoints(hostname string) ([]APIEndpoint, error) { - s.mu.RLock() - defer s.mu.RUnlock() - return s.lookupV2Endpoints(context.TODO(), hostname, false) } - -// IsInsecureRegistry returns true if the registry at given host is configured as -// insecure registry. -func (s *Service) IsInsecureRegistry(host string) bool { - s.mu.RLock() - defer s.mu.RUnlock() - return !s.config.isSecureIndex(host) -} From e0b351b3d9160b03cbc4ed388def82673b3d71f0 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 24 Jul 2025 01:57:57 +0200 Subject: [PATCH 03/12] internal/registry: remove code related to mirrors The CLI does not have information about mirrors, and doesn't configure them, so we can remove these parts. Signed-off-by: Sebastiaan van Stijn --- internal/registry/config.go | 83 +------ internal/registry/config_test.go | 182 -------------- internal/registry/registry_test.go | 373 ----------------------------- internal/registry/service.go | 21 +- internal/registry/service_v2.go | 27 +-- 5 files changed, 13 insertions(+), 673 deletions(-) diff --git a/internal/registry/config.go b/internal/registry/config.go index a4817f89a5f4..6abc299bebb2 100644 --- a/internal/registry/config.go +++ b/internal/registry/config.go @@ -22,7 +22,6 @@ import ( // ServiceOptions holds command line options. type ServiceOptions struct { - Mirrors []string `json:"registry-mirrors,omitempty"` InsecureRegistries []string `json:"insecure-registries,omitempty"` } @@ -93,51 +92,6 @@ func CertsDir() string { return certsDir } -// newServiceConfig returns a new instance of ServiceConfig -func newServiceConfig(options ServiceOptions) (*serviceConfig, error) { - config := &serviceConfig{} - if err := config.loadMirrors(options.Mirrors); err != nil { - return nil, err - } - if err := config.loadInsecureRegistries(options.InsecureRegistries); err != nil { - return nil, err - } - - return config, nil -} - -// loadMirrors loads mirrors to config, after removing duplicates. -// Returns an error if mirrors contains an invalid mirror. -func (config *serviceConfig) loadMirrors(mirrors []string) error { - mMap := map[string]struct{}{} - unique := []string{} - - for _, mirror := range mirrors { - m, err := ValidateMirror(mirror) - if err != nil { - return err - } - if _, exist := mMap[m]; !exist { - mMap[m] = struct{}{} - unique = append(unique, m) - } - } - - config.Mirrors = unique - - // Configure public registry since mirrors may have changed. - config.IndexConfigs = map[string]*registry.IndexInfo{ - IndexName: { - Name: IndexName, - Mirrors: unique, - Secure: true, - Official: true, - }, - } - - return nil -} - // loadInsecureRegistries loads insecure registries to config func (config *serviceConfig) loadInsecureRegistries(registries []string) error { // Localhost is by default considered as an insecure registry. This is a @@ -184,7 +138,6 @@ skip: // Assume `host:port` if not CIDR. indexConfigs[r] = ®istry.IndexInfo{ Name: r, - Mirrors: []string{}, Secure: false, Official: false, } @@ -194,7 +147,6 @@ skip: // Configure public registry. indexConfigs[IndexName] = ®istry.IndexInfo{ Name: IndexName, - Mirrors: config.Mirrors, Secure: true, Official: true, } @@ -267,35 +219,6 @@ func isCIDRMatch(cidrs []*registry.NetIPNet, urlHost string) bool { return false } -// ValidateMirror validates and normalizes an HTTP(S) registry mirror. It -// returns an error if the given mirrorURL is invalid, or the normalized -// format for the URL otherwise. -// -// It is used by the daemon to validate the daemon configuration. -func ValidateMirror(mirrorURL string) (string, error) { - // Fast path for missing scheme, as url.Parse splits by ":", which can - // cause the hostname to be considered the "scheme" when using "hostname:port". - if scheme, _, ok := strings.Cut(mirrorURL, "://"); !ok || scheme == "" { - return "", invalidParamf("invalid mirror: no scheme specified for %q: must use either 'https://' or 'http://'", mirrorURL) - } - uri, err := url.Parse(mirrorURL) - if err != nil { - return "", invalidParamWrapf(err, "invalid mirror: %q is not a valid URI", mirrorURL) - } - if uri.Scheme != "http" && uri.Scheme != "https" { - return "", invalidParamf("invalid mirror: unsupported scheme %q in %q: must use either 'https://' or 'http://'", uri.Scheme, uri) - } - if uri.RawQuery != "" || uri.Fragment != "" { - return "", invalidParamf("invalid mirror: query or fragment at end of the URI %q", uri) - } - if uri.User != nil { - // strip password from output - uri.User = url.UserPassword(uri.User.Username(), "xxxxx") - return "", invalidParamf("invalid mirror: username/password not allowed in URI %q", uri) - } - return strings.TrimSuffix(mirrorURL, "/") + "/", nil -} - // ValidateIndexName validates an index name. It is used by the daemon to // validate the daemon configuration. func ValidateIndexName(val string) (string, error) { @@ -348,7 +271,6 @@ func ParseRepositoryInfo(reposName reference.Named) (*RepositoryInfo, error) { Name: reference.TrimNamed(reposName), Index: ®istry.IndexInfo{ Name: IndexName, - Mirrors: []string{}, Secure: true, Official: true, }, @@ -358,9 +280,8 @@ func ParseRepositoryInfo(reposName reference.Named) (*RepositoryInfo, error) { return &RepositoryInfo{ Name: reference.TrimNamed(reposName), Index: ®istry.IndexInfo{ - Name: indexName, - Mirrors: []string{}, - Secure: !isInsecure(indexName), + Name: indexName, + Secure: !isInsecure(indexName), }, }, nil } diff --git a/internal/registry/config_test.go b/internal/registry/config_test.go index 699e763be6cf..f3d8b4c44a2e 100644 --- a/internal/registry/config_test.go +++ b/internal/registry/config_test.go @@ -8,138 +8,6 @@ import ( is "gotest.tools/v3/assert/cmp" ) -func TestValidateMirror(t *testing.T) { - tests := []struct { - input string - output string - expectedErr string - }{ - // Valid cases - { - input: "http://mirror-1.example.com", - output: "http://mirror-1.example.com/", - }, - { - input: "http://mirror-1.example.com/", - output: "http://mirror-1.example.com/", - }, - { - input: "https://mirror-1.example.com", - output: "https://mirror-1.example.com/", - }, - { - input: "https://mirror-1.example.com/", - output: "https://mirror-1.example.com/", - }, - { - input: "http://localhost", - output: "http://localhost/", - }, - { - input: "https://localhost", - output: "https://localhost/", - }, - { - input: "http://localhost:5000", - output: "http://localhost:5000/", - }, - { - input: "https://localhost:5000", - output: "https://localhost:5000/", - }, - { - input: "http://127.0.0.1", - output: "http://127.0.0.1/", - }, - { - input: "https://127.0.0.1", - output: "https://127.0.0.1/", - }, - { - input: "http://127.0.0.1:5000", - output: "http://127.0.0.1:5000/", - }, - { - input: "https://127.0.0.1:5000", - output: "https://127.0.0.1:5000/", - }, - { - input: "http://mirror-1.example.com/v1/", - output: "http://mirror-1.example.com/v1/", - }, - { - input: "https://mirror-1.example.com/v1/", - output: "https://mirror-1.example.com/v1/", - }, - - // Invalid cases - { - input: "!invalid!://%as%", - expectedErr: `invalid mirror: "!invalid!://%as%" is not a valid URI: parse "!invalid!://%as%": first path segment in URL cannot contain colon`, - }, - { - input: "mirror-1.example.com", - expectedErr: `invalid mirror: no scheme specified for "mirror-1.example.com": must use either 'https://' or 'http://'`, - }, - { - input: "mirror-1.example.com:5000", - expectedErr: `invalid mirror: no scheme specified for "mirror-1.example.com:5000": must use either 'https://' or 'http://'`, - }, - { - input: "ftp://mirror-1.example.com", - expectedErr: `invalid mirror: unsupported scheme "ftp" in "ftp://mirror-1.example.com": must use either 'https://' or 'http://'`, - }, - { - input: "http://mirror-1.example.com/?q=foo", - expectedErr: `invalid mirror: query or fragment at end of the URI "http://mirror-1.example.com/?q=foo"`, - }, - { - input: "http://mirror-1.example.com/v1/?q=foo", - expectedErr: `invalid mirror: query or fragment at end of the URI "http://mirror-1.example.com/v1/?q=foo"`, - }, - { - input: "http://mirror-1.example.com/v1/?q=foo#frag", - expectedErr: `invalid mirror: query or fragment at end of the URI "http://mirror-1.example.com/v1/?q=foo#frag"`, - }, - { - input: "http://mirror-1.example.com?q=foo", - expectedErr: `invalid mirror: query or fragment at end of the URI "http://mirror-1.example.com?q=foo"`, - }, - { - input: "https://mirror-1.example.com#frag", - expectedErr: `invalid mirror: query or fragment at end of the URI "https://mirror-1.example.com#frag"`, - }, - { - input: "https://mirror-1.example.com/#frag", - expectedErr: `invalid mirror: query or fragment at end of the URI "https://mirror-1.example.com/#frag"`, - }, - { - input: "http://foo:bar@mirror-1.example.com/", - expectedErr: `invalid mirror: username/password not allowed in URI "http://foo:xxxxx@mirror-1.example.com/"`, - }, - { - input: "https://mirror-1.example.com/v1/#frag", - expectedErr: `invalid mirror: query or fragment at end of the URI "https://mirror-1.example.com/v1/#frag"`, - }, - { - input: "https://mirror-1.example.com?q", - expectedErr: `invalid mirror: query or fragment at end of the URI "https://mirror-1.example.com?q"`, - }, - } - - for _, tc := range tests { - t.Run(tc.input, func(t *testing.T) { - out, err := ValidateMirror(tc.input) - if tc.expectedErr != "" { - assert.Error(t, err, tc.expectedErr) - } else { - assert.NilError(t, err) - } - assert.Check(t, is.Equal(out, tc.output)) - }) - } -} - func TestLoadInsecureRegistries(t *testing.T) { testCases := []struct { registries []string @@ -229,56 +97,6 @@ func TestLoadInsecureRegistries(t *testing.T) { } } -func TestNewServiceConfig(t *testing.T) { - tests := []struct { - doc string - opts ServiceOptions - errStr string - }{ - { - doc: "empty config", - }, - { - doc: "invalid mirror", - opts: ServiceOptions{ - Mirrors: []string{"example.com:5000"}, - }, - errStr: `invalid mirror: no scheme specified for "example.com:5000": must use either 'https://' or 'http://'`, - }, - { - doc: "valid mirror", - opts: ServiceOptions{ - Mirrors: []string{"https://example.com:5000"}, - }, - }, - { - doc: "invalid insecure registry", - opts: ServiceOptions{ - InsecureRegistries: []string{"[fe80::]/64"}, - }, - errStr: `insecure registry [fe80::]/64 is not valid: invalid host "[fe80::]/64"`, - }, - { - doc: "valid insecure registry", - opts: ServiceOptions{ - InsecureRegistries: []string{"102.10.8.1/24"}, - }, - }, - } - - for _, tc := range tests { - t.Run(tc.doc, func(t *testing.T) { - _, err := newServiceConfig(tc.opts) - if tc.errStr != "" { - assert.Check(t, is.Error(err, tc.errStr)) - assert.Check(t, cerrdefs.IsInvalidArgument(err)) - } else { - assert.Check(t, err) - } - }) - } -} - func TestValidateIndexName(t *testing.T) { valid := []struct { index string diff --git a/internal/registry/registry_test.go b/internal/registry/registry_test.go index 55205f81622e..31c4483c40f4 100644 --- a/internal/registry/registry_test.go +++ b/internal/registry/registry_test.go @@ -1,8 +1,6 @@ package registry import ( - "errors" - "net" "testing" "github.com/distribution/reference" @@ -11,46 +9,6 @@ import ( is "gotest.tools/v3/assert/cmp" ) -// overrideLookupIP overrides net.LookupIP for testing. -func overrideLookupIP(t *testing.T) { - t.Helper() - restoreLookup := lookupIP - - // override net.LookupIP - lookupIP = func(host string) ([]net.IP, error) { - mockHosts := map[string][]net.IP{ - "": {net.ParseIP("0.0.0.0")}, - "localhost": {net.ParseIP("127.0.0.1"), net.ParseIP("::1")}, - "example.com": {net.ParseIP("42.42.42.42")}, - "other.com": {net.ParseIP("43.43.43.43")}, - } - if addrs, ok := mockHosts[host]; ok { - return addrs, nil - } - return nil, errors.New("lookup: no such host") - } - t.Cleanup(func() { - lookupIP = restoreLookup - }) -} - -// newIndexInfo returns IndexInfo configuration from indexName -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 - } - - // Construct a non-configured index info. - return ®istry.IndexInfo{ - Name: indexName, - Mirrors: []string{}, - Secure: config.isSecureIndex(indexName), - } -} - func TestParseRepositoryInfo(t *testing.T) { type staticRepositoryInfo struct { Index *registry.IndexInfo @@ -63,7 +21,6 @@ func TestParseRepositoryInfo(t *testing.T) { "fooo/bar": { Index: ®istry.IndexInfo{ Name: IndexName, - Mirrors: []string{}, Official: true, Secure: true, }, @@ -74,7 +31,6 @@ func TestParseRepositoryInfo(t *testing.T) { "library/ubuntu": { Index: ®istry.IndexInfo{ Name: IndexName, - Mirrors: []string{}, Official: true, Secure: true, }, @@ -85,7 +41,6 @@ func TestParseRepositoryInfo(t *testing.T) { "nonlibrary/ubuntu": { Index: ®istry.IndexInfo{ Name: IndexName, - Mirrors: []string{}, Official: true, Secure: true, }, @@ -96,7 +51,6 @@ func TestParseRepositoryInfo(t *testing.T) { "ubuntu": { Index: ®istry.IndexInfo{ Name: IndexName, - Mirrors: []string{}, Official: true, Secure: true, }, @@ -107,7 +61,6 @@ func TestParseRepositoryInfo(t *testing.T) { "other/library": { Index: ®istry.IndexInfo{ Name: IndexName, - Mirrors: []string{}, Official: true, Secure: true, }, @@ -118,7 +71,6 @@ func TestParseRepositoryInfo(t *testing.T) { "127.0.0.1:8000/private/moonbase": { Index: ®istry.IndexInfo{ Name: "127.0.0.1:8000", - Mirrors: []string{}, Official: false, Secure: false, }, @@ -129,7 +81,6 @@ func TestParseRepositoryInfo(t *testing.T) { "127.0.0.1:8000/privatebase": { Index: ®istry.IndexInfo{ Name: "127.0.0.1:8000", - Mirrors: []string{}, Official: false, Secure: false, }, @@ -140,7 +91,6 @@ func TestParseRepositoryInfo(t *testing.T) { "[::1]:8000/private/moonbase": { Index: ®istry.IndexInfo{ Name: "[::1]:8000", - Mirrors: []string{}, Official: false, Secure: false, }, @@ -151,7 +101,6 @@ func TestParseRepositoryInfo(t *testing.T) { "[::1]:8000/privatebase": { Index: ®istry.IndexInfo{ Name: "[::1]:8000", - Mirrors: []string{}, Official: false, Secure: false, }, @@ -164,7 +113,6 @@ func TestParseRepositoryInfo(t *testing.T) { "[::2]:8000/private/moonbase": { Index: ®istry.IndexInfo{ Name: "[::2]:8000", - Mirrors: []string{}, Official: false, Secure: true, }, @@ -177,7 +125,6 @@ func TestParseRepositoryInfo(t *testing.T) { "[::2]:8000/privatebase": { Index: ®istry.IndexInfo{ Name: "[::2]:8000", - Mirrors: []string{}, Official: false, Secure: true, }, @@ -188,7 +135,6 @@ func TestParseRepositoryInfo(t *testing.T) { "localhost:8000/private/moonbase": { Index: ®istry.IndexInfo{ Name: "localhost:8000", - Mirrors: []string{}, Official: false, Secure: false, }, @@ -199,7 +145,6 @@ func TestParseRepositoryInfo(t *testing.T) { "localhost:8000/privatebase": { Index: ®istry.IndexInfo{ Name: "localhost:8000", - Mirrors: []string{}, Official: false, Secure: false, }, @@ -210,7 +155,6 @@ func TestParseRepositoryInfo(t *testing.T) { "example.com/private/moonbase": { Index: ®istry.IndexInfo{ Name: "example.com", - Mirrors: []string{}, Official: false, Secure: true, }, @@ -221,7 +165,6 @@ func TestParseRepositoryInfo(t *testing.T) { "example.com/privatebase": { Index: ®istry.IndexInfo{ Name: "example.com", - Mirrors: []string{}, Official: false, Secure: true, }, @@ -232,7 +175,6 @@ func TestParseRepositoryInfo(t *testing.T) { "example.com:8000/private/moonbase": { Index: ®istry.IndexInfo{ Name: "example.com:8000", - Mirrors: []string{}, Official: false, Secure: true, }, @@ -243,7 +185,6 @@ func TestParseRepositoryInfo(t *testing.T) { "example.com:8000/privatebase": { Index: ®istry.IndexInfo{ Name: "example.com:8000", - Mirrors: []string{}, Official: false, Secure: true, }, @@ -254,7 +195,6 @@ func TestParseRepositoryInfo(t *testing.T) { "localhost/private/moonbase": { Index: ®istry.IndexInfo{ Name: "localhost", - Mirrors: []string{}, Official: false, Secure: false, }, @@ -265,7 +205,6 @@ func TestParseRepositoryInfo(t *testing.T) { "localhost/privatebase": { Index: ®istry.IndexInfo{ Name: "localhost", - Mirrors: []string{}, Official: false, Secure: false, }, @@ -276,7 +215,6 @@ func TestParseRepositoryInfo(t *testing.T) { IndexName + "/public/moonbase": { Index: ®istry.IndexInfo{ Name: IndexName, - Mirrors: []string{}, Official: true, Secure: true, }, @@ -287,7 +225,6 @@ func TestParseRepositoryInfo(t *testing.T) { "index." + IndexName + "/public/moonbase": { Index: ®istry.IndexInfo{ Name: IndexName, - Mirrors: []string{}, Official: true, Secure: true, }, @@ -298,7 +235,6 @@ func TestParseRepositoryInfo(t *testing.T) { "ubuntu-12.04-base": { Index: ®istry.IndexInfo{ Name: IndexName, - Mirrors: []string{}, Official: true, Secure: true, }, @@ -309,7 +245,6 @@ func TestParseRepositoryInfo(t *testing.T) { IndexName + "/ubuntu-12.04-base": { Index: ®istry.IndexInfo{ Name: IndexName, - Mirrors: []string{}, Official: true, Secure: true, }, @@ -320,7 +255,6 @@ func TestParseRepositoryInfo(t *testing.T) { "index." + IndexName + "/ubuntu-12.04-base": { Index: ®istry.IndexInfo{ Name: IndexName, - Mirrors: []string{}, Official: true, Secure: true, }, @@ -345,310 +279,3 @@ func TestParseRepositoryInfo(t *testing.T) { }) } } - -func TestNewIndexInfo(t *testing.T) { - overrideLookupIP(t) - - // 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). It 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: []string{}, - Secure: true, - Official: true, - }, - }, - InsecureRegistryCIDRs: []*registry.NetIPNet{ - (*registry.NetIPNet)(ipv6Loopback), - (*registry.NetIPNet)(ipv4Loopback), - }, - } - - expectedIndexInfos := map[string]*registry.IndexInfo{ - IndexName: { - Name: IndexName, - Official: true, - Secure: true, - Mirrors: []string{}, - }, - "index." + IndexName: { - Name: IndexName, - Official: true, - Secure: true, - Mirrors: []string{}, - }, - "example.com": { - Name: "example.com", - Official: false, - Secure: true, - Mirrors: []string{}, - }, - "127.0.0.1:5000": { - Name: "127.0.0.1:5000", - Official: false, - Secure: false, - Mirrors: []string{}, - }, - } - t.Run("no mirrors", func(t *testing.T) { - for indexName, expected := range expectedIndexInfos { - t.Run(indexName, func(t *testing.T) { - actual := newIndexInfo(emptyServiceConfig, indexName) - assert.Check(t, is.DeepEqual(actual, expected)) - }) - } - }) - - expectedIndexInfos = map[string]*registry.IndexInfo{ - IndexName: { - Name: IndexName, - Official: true, - Secure: true, - Mirrors: []string{"http://mirror1.local/", "http://mirror2.local/"}, - }, - "index." + IndexName: { - Name: IndexName, - Official: true, - Secure: true, - Mirrors: []string{"http://mirror1.local/", "http://mirror2.local/"}, - }, - "example.com": { - Name: "example.com", - Official: false, - Secure: false, - Mirrors: []string{}, - }, - "example.com:5000": { - Name: "example.com:5000", - Official: false, - Secure: true, - Mirrors: []string{}, - }, - "127.0.0.1": { - Name: "127.0.0.1", - Official: false, - Secure: false, - Mirrors: []string{}, - }, - "127.0.0.1:5000": { - Name: "127.0.0.1:5000", - Official: false, - Secure: false, - Mirrors: []string{}, - }, - "127.255.255.255": { - Name: "127.255.255.255", - Official: false, - Secure: false, - Mirrors: []string{}, - }, - "127.255.255.255:5000": { - Name: "127.255.255.255:5000", - Official: false, - Secure: false, - Mirrors: []string{}, - }, - "::1": { - Name: "::1", - Official: false, - Secure: false, - Mirrors: []string{}, - }, - "[::1]:5000": { - Name: "[::1]:5000", - Official: false, - Secure: false, - Mirrors: []string{}, - }, - // IPv6 only has a single loopback address, so ::2 is not a loopback, - // hence not marked "insecure". - "::2": { - Name: "::2", - Official: false, - Secure: true, - Mirrors: []string{}, - }, - // IPv6 only has a single loopback address, so ::2 is not a loopback, - // hence not marked "insecure". - "[::2]:5000": { - Name: "[::2]:5000", - Official: false, - Secure: true, - Mirrors: []string{}, - }, - "other.com": { - Name: "other.com", - Official: false, - Secure: true, - Mirrors: []string{}, - }, - } - t.Run("mirrors", func(t *testing.T) { - // Note that newServiceConfig calls ValidateMirror internally, which normalizes - // mirror-URLs to have a trailing slash. - config, err := newServiceConfig(ServiceOptions{ - Mirrors: []string{"http://mirror1.local", "http://mirror2.local"}, - InsecureRegistries: []string{"example.com"}, - }) - assert.NilError(t, err) - for indexName, expected := range expectedIndexInfos { - t.Run(indexName, func(t *testing.T) { - actual := newIndexInfo(config, indexName) - assert.Check(t, is.DeepEqual(actual, expected)) - }) - } - }) - - expectedIndexInfos = map[string]*registry.IndexInfo{ - "example.com": { - Name: "example.com", - Official: false, - Secure: false, - Mirrors: []string{}, - }, - "example.com:5000": { - Name: "example.com:5000", - Official: false, - Secure: false, - Mirrors: []string{}, - }, - "127.0.0.1": { - Name: "127.0.0.1", - Official: false, - Secure: false, - Mirrors: []string{}, - }, - "127.0.0.1:5000": { - Name: "127.0.0.1:5000", - Official: false, - Secure: false, - Mirrors: []string{}, - }, - "42.42.0.1:5000": { - Name: "42.42.0.1:5000", - Official: false, - Secure: false, - Mirrors: []string{}, - }, - "42.43.0.1:5000": { - Name: "42.43.0.1:5000", - Official: false, - Secure: true, - Mirrors: []string{}, - }, - "other.com": { - Name: "other.com", - Official: false, - Secure: true, - Mirrors: []string{}, - }, - } - t.Run("custom insecure", func(t *testing.T) { - config, err := newServiceConfig(ServiceOptions{ - InsecureRegistries: []string{"42.42.0.0/16"}, - }) - assert.NilError(t, err) - for indexName, expected := range expectedIndexInfos { - t.Run(indexName, func(t *testing.T) { - actual := newIndexInfo(config, indexName) - assert.Check(t, is.DeepEqual(actual, expected)) - }) - } - }) -} - -func TestMirrorEndpointLookup(t *testing.T) { - containsMirror := func(endpoints []APIEndpoint) bool { - for _, pe := range endpoints { - if pe.URL.Host == "my.mirror" { - return true - } - } - return false - } - cfg, err := newServiceConfig(ServiceOptions{ - Mirrors: []string{"https://my.mirror"}, - }) - assert.NilError(t, err) - s := Service{config: cfg} - - imageName, err := reference.WithName(IndexName + "/test/image") - if err != nil { - t.Error(err) - } - pushAPIEndpoints, err := s.LookupPushEndpoints(reference.Domain(imageName)) - if err != nil { - t.Fatal(err) - } - if containsMirror(pushAPIEndpoints) { - t.Fatal("Push endpoint should not contain mirror") - } - - pullAPIEndpoints, err := s.LookupPullEndpoints(reference.Domain(imageName)) - if err != nil { - t.Fatal(err) - } - if !containsMirror(pullAPIEndpoints) { - t.Fatal("Pull endpoint should contain mirror") - } -} - -func TestIsSecureIndex(t *testing.T) { - overrideLookupIP(t) - tests := []struct { - addr string - insecureRegistries []string - expected bool - }{ - {IndexName, nil, true}, - {"example.com", []string{}, true}, - {"example.com", []string{"example.com"}, false}, - {"localhost", []string{"localhost:5000"}, false}, - {"localhost:5000", []string{"localhost:5000"}, false}, - {"localhost", []string{"example.com"}, false}, - {"127.0.0.1:5000", []string{"127.0.0.1:5000"}, false}, - {"localhost", nil, false}, - {"localhost:5000", nil, false}, - {"127.0.0.1", nil, false}, - {"localhost", []string{"example.com"}, false}, - {"127.0.0.1", []string{"example.com"}, false}, - {"example.com", nil, true}, - {"example.com", []string{"example.com"}, false}, - {"127.0.0.1", []string{"example.com"}, false}, - {"127.0.0.1:5000", []string{"example.com"}, false}, - {"example.com:5000", []string{"42.42.0.0/16"}, false}, - {"example.com", []string{"42.42.0.0/16"}, false}, - {"example.com:5000", []string{"42.42.42.42/8"}, false}, - {"127.0.0.1:5000", []string{"127.0.0.0/8"}, false}, - {"42.42.42.42:5000", []string{"42.1.1.1/8"}, false}, - {"invalid.example.com", []string{"42.42.0.0/16"}, true}, - {"invalid.example.com", []string{"invalid.example.com"}, false}, - {"invalid.example.com:5000", []string{"invalid.example.com"}, true}, - {"invalid.example.com:5000", []string{"invalid.example.com:5000"}, false}, - } - for _, tc := range tests { - config, err := newServiceConfig(ServiceOptions{ - InsecureRegistries: tc.insecureRegistries, - }) - assert.NilError(t, err) - - sec := config.isSecureIndex(tc.addr) - assert.Equal(t, sec, tc.expected, "isSecureIndex failed for %q %v, expected %v got %v", tc.addr, tc.insecureRegistries, tc.expected, sec) - } -} diff --git a/internal/registry/service.go b/internal/registry/service.go index 6230cc42bbd4..11ae5f68b5f9 100644 --- a/internal/registry/service.go +++ b/internal/registry/service.go @@ -21,12 +21,13 @@ type Service struct { // NewService returns a new instance of [Service] ready to be installed into // an engine. func NewService(options ServiceOptions) (*Service, error) { - config, err := newServiceConfig(options) - if err != nil { - return nil, err + config := &serviceConfig{} + if len(options.InsecureRegistries) > 0 { + if err := config.loadInsecureRegistries(options.InsecureRegistries); err != nil { + return nil, err + } } - - return &Service{config: config}, err + return &Service{config: config}, nil } // Auth contacts the public registry with the provided credentials, @@ -48,9 +49,8 @@ func (s *Service) Auth(ctx context.Context, authConfig *registry.AuthConfig, use registryHostName = u.Host } - // Lookup endpoints for authentication but exclude mirrors to prevent - // sending credentials of the upstream registry to a mirror. - endpoints, err := s.lookupV2Endpoints(ctx, registryHostName, false) + // Lookup endpoints for authentication. + endpoints, err := s.lookupV2Endpoints(ctx, registryHostName) if err != nil { if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) { return "", "", err @@ -84,7 +84,6 @@ func (s *Service) Auth(ctx context.Context, authConfig *registry.AuthConfig, use // APIEndpoint represents a remote API endpoint type APIEndpoint struct { - Mirror bool URL *url.URL TLSConfig *tls.Config } @@ -92,11 +91,11 @@ type APIEndpoint struct { // LookupPullEndpoints creates a list of v2 endpoints to try to pull from, in order of preference. // It gives preference to mirrors over the actual registry, and HTTPS over plain HTTP. func (s *Service) LookupPullEndpoints(hostname string) ([]APIEndpoint, error) { - return s.lookupV2Endpoints(context.TODO(), hostname, true) + return s.lookupV2Endpoints(context.TODO(), hostname) } // LookupPushEndpoints creates a list of v2 endpoints to try to push to, in order of preference. // It gives preference to HTTPS over plain HTTP. Mirrors are not included. func (s *Service) LookupPushEndpoints(hostname string) ([]APIEndpoint, error) { - return s.lookupV2Endpoints(context.TODO(), hostname, false) + return s.lookupV2Endpoints(context.TODO(), hostname) } diff --git a/internal/registry/service_v2.go b/internal/registry/service_v2.go index df343a155dec..0f0059533022 100644 --- a/internal/registry/service_v2.go +++ b/internal/registry/service_v2.go @@ -3,38 +3,13 @@ package registry import ( "context" "net/url" - "strings" "github.com/docker/go-connections/tlsconfig" ) -func (s *Service) lookupV2Endpoints(ctx context.Context, hostname string, includeMirrors bool) ([]APIEndpoint, error) { +func (s *Service) lookupV2Endpoints(ctx context.Context, hostname string) ([]APIEndpoint, error) { var endpoints []APIEndpoint if hostname == DefaultNamespace || hostname == IndexHostname { - if includeMirrors { - for _, mirror := range s.config.Mirrors { - if ctx.Err() != nil { - return nil, ctx.Err() - } - if !strings.HasPrefix(mirror, "http://") && !strings.HasPrefix(mirror, "https://") { - mirror = "https://" + mirror - } - mirrorURL, err := url.Parse(mirror) - if err != nil { - return nil, invalidParam(err) - } - // TODO(thaJeztah); this should all be memoized when loading the config. We're resolving mirrors and loading TLS config every time. - mirrorTLSConfig, err := newTLSConfig(ctx, mirrorURL.Host, s.config.isSecureIndex(mirrorURL.Host)) - if err != nil { - return nil, err - } - endpoints = append(endpoints, APIEndpoint{ - URL: mirrorURL, - Mirror: true, - TLSConfig: mirrorTLSConfig, - }) - } - } endpoints = append(endpoints, APIEndpoint{ URL: DefaultV2Registry, TLSConfig: tlsconfig.ServerDefault(), From 7cf245d2f7ab5338bc325913f961f6f4aec88be6 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 24 Jul 2025 02:04:37 +0200 Subject: [PATCH 04/12] internal/registry: Service.Auth remove unused statusmessage return Signed-off-by: Sebastiaan van Stijn --- cli/command/registry/login.go | 2 +- internal/registry/service.go | 16 +++++++--------- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/cli/command/registry/login.go b/cli/command/registry/login.go index f5853fe6a3fc..7083aaf932cf 100644 --- a/cli/command/registry/login.go +++ b/cli/command/registry/login.go @@ -288,7 +288,7 @@ func loginClientSide(ctx context.Context, auth registrytypes.AuthConfig) (*regis return nil, err } - _, token, err := svc.Auth(ctx, &auth, command.UserAgent()) + token, err := svc.Auth(ctx, &auth, command.UserAgent()) if err != nil { return nil, err } diff --git a/internal/registry/service.go b/internal/registry/service.go index 11ae5f68b5f9..91b624462763 100644 --- a/internal/registry/service.go +++ b/internal/registry/service.go @@ -33,8 +33,7 @@ func NewService(options ServiceOptions) (*Service, error) { // Auth contacts the public registry with the provided credentials, // and returns OK if authentication was successful. // It can be used to verify the validity of a client's credentials. -func (s *Service) Auth(ctx context.Context, authConfig *registry.AuthConfig, userAgent string) (statusMessage, token string, _ error) { - // TODO Use ctx when searching for repositories +func (s *Service) Auth(ctx context.Context, authConfig *registry.AuthConfig, userAgent string) (token string, _ error) { registryHostName := IndexHostname if authConfig.ServerAddress != "" { @@ -44,7 +43,7 @@ func (s *Service) Auth(ctx context.Context, authConfig *registry.AuthConfig, use } u, err := url.Parse(serverAddress) if err != nil { - return "", "", invalidParamWrapf(err, "unable to parse server address") + return "", invalidParamWrapf(err, "unable to parse server address") } registryHostName = u.Host } @@ -53,9 +52,9 @@ func (s *Service) Auth(ctx context.Context, authConfig *registry.AuthConfig, use endpoints, err := s.lookupV2Endpoints(ctx, registryHostName) if err != nil { if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) { - return "", "", err + return "", err } - return "", "", invalidParam(err) + return "", invalidParam(err) } var lastErr error @@ -64,7 +63,7 @@ func (s *Service) Auth(ctx context.Context, authConfig *registry.AuthConfig, use if err != nil { if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) || cerrdefs.IsUnauthorized(err) { // Failed to authenticate; don't continue with (non-TLS) endpoints. - return "", "", err + return "", err } // Try next endpoint log.G(ctx).WithFields(log.Fields{ @@ -75,11 +74,10 @@ func (s *Service) Auth(ctx context.Context, authConfig *registry.AuthConfig, use continue } - // TODO(thaJeztah): move the statusMessage to the API endpoint; we don't need to produce that here? - return "Login Succeeded", authToken, nil + return authToken, nil } - return "", "", lastErr + return "", lastErr } // APIEndpoint represents a remote API endpoint From dad2e67860ee4685e46a4664457d071de5b5ec86 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 24 Jul 2025 18:49:43 +0200 Subject: [PATCH 05/12] internal/registry: remove PingResponseError It's not matched anywhere, so we can just return a plain error. Signed-off-by: Sebastiaan van Stijn --- internal/registry/auth.go | 22 +++++----------------- 1 file changed, 5 insertions(+), 17 deletions(-) diff --git a/internal/registry/auth.go b/internal/registry/auth.go index 72192e5ad7a9..be6f2c2c7492 100644 --- a/internal/registry/auth.go +++ b/internal/registry/auth.go @@ -127,29 +127,19 @@ func v2AuthHTTPClient(endpoint *url.URL, authTransport http.RoundTripper, modifi }, nil } -// PingResponseError is used when the response from a ping -// was received but invalid. -type PingResponseError struct { - Err error -} - -func (err PingResponseError) Error() string { - return err.Err.Error() -} - // PingV2Registry attempts to ping a v2 registry and on success return a // challenge manager for the supported authentication types. // If a response is received but cannot be interpreted, a PingResponseError will be returned. func PingV2Registry(endpoint *url.URL, authTransport http.RoundTripper) (challenge.Manager, error) { - pingClient := &http.Client{ - Transport: authTransport, - Timeout: 15 * time.Second, - } endpointStr := strings.TrimRight(endpoint.String(), "/") + "/v2/" req, err := http.NewRequest(http.MethodGet, endpointStr, http.NoBody) if err != nil { return nil, err } + pingClient := &http.Client{ + Transport: authTransport, + Timeout: 15 * time.Second, + } resp, err := pingClient.Do(req) if err != nil { return nil, err @@ -158,9 +148,7 @@ func PingV2Registry(endpoint *url.URL, authTransport http.RoundTripper) (challen challengeManager := challenge.NewSimpleManager() if err := challengeManager.AddResponse(resp); err != nil { - return nil, PingResponseError{ - Err: err, - } + return nil, err } return challengeManager, nil From dc41365b562062ce32ba912e8cdafeb04f16d5c0 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 24 Jul 2025 19:05:02 +0200 Subject: [PATCH 06/12] internal/registry: remove NewStaticCredentialStore It was only used in a single place; inline it there. Signed-off-by: Sebastiaan van Stijn --- cli/registry/client/endpoint.go | 23 ++++++++++++++++++++++- internal/registry/auth.go | 29 ----------------------------- 2 files changed, 22 insertions(+), 30 deletions(-) diff --git a/cli/registry/client/endpoint.go b/cli/registry/client/endpoint.go index f08de5d14ae5..631a5bdf44ae 100644 --- a/cli/registry/client/endpoint.go +++ b/cli/registry/client/endpoint.go @@ -3,6 +3,7 @@ package client import ( "net" "net/http" + "net/url" "time" "github.com/distribution/reference" @@ -97,7 +98,7 @@ func getHTTPTransport(authConfig registrytypes.AuthConfig, endpoint registry.API if len(actions) == 0 { actions = []string{"pull"} } - creds := registry.NewStaticCredentialStore(&authConfig) + creds := &staticCredentialStore{authConfig: &authConfig} tokenHandler := auth.NewTokenHandler(authTransport, creds, repoName, actions...) basicHandler := auth.NewBasicHandler(creds) modifiers = append(modifiers, auth.NewAuthorizer(challengeManager, tokenHandler, basicHandler)) @@ -117,3 +118,23 @@ func (th *existingTokenHandler) AuthorizeRequest(req *http.Request, _ map[string func (*existingTokenHandler) Scheme() string { return "bearer" } + +type staticCredentialStore struct { + authConfig *registrytypes.AuthConfig +} + +func (scs staticCredentialStore) Basic(*url.URL) (string, string) { + if scs.authConfig == nil { + return "", "" + } + return scs.authConfig.Username, scs.authConfig.Password +} + +func (scs staticCredentialStore) RefreshToken(*url.URL, string) string { + if scs.authConfig == nil { + return "" + } + return scs.authConfig.IdentityToken +} + +func (staticCredentialStore) SetRefreshToken(*url.URL, string, string) {} diff --git a/internal/registry/auth.go b/internal/registry/auth.go index be6f2c2c7492..e3ad9613d8ef 100644 --- a/internal/registry/auth.go +++ b/internal/registry/auth.go @@ -34,35 +34,6 @@ func (lcs loginCredentialStore) SetRefreshToken(u *url.URL, service, token strin lcs.authConfig.IdentityToken = token } -type staticCredentialStore struct { - auth *registry.AuthConfig -} - -// NewStaticCredentialStore returns a credential store -// which always returns the same credential values. -func NewStaticCredentialStore(ac *registry.AuthConfig) auth.CredentialStore { - return staticCredentialStore{ - auth: ac, - } -} - -func (scs staticCredentialStore) Basic(*url.URL) (string, string) { - if scs.auth == nil { - return "", "" - } - return scs.auth.Username, scs.auth.Password -} - -func (scs staticCredentialStore) RefreshToken(*url.URL, string) string { - if scs.auth == nil { - return "" - } - return scs.auth.IdentityToken -} - -func (staticCredentialStore) SetRefreshToken(*url.URL, string, string) { -} - // loginV2 tries to login to the v2 registry server. The given registry // endpoint will be pinged to get authorization challenges. These challenges // will be used to authenticate against the registry to validate credentials. From 5322affc9f6f02f51b926ea2e650935c37ff2bd7 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 24 Jul 2025 20:07:22 +0200 Subject: [PATCH 07/12] internal/registry: remove duplicate endpoint methods now that we no longer need to account for mirrors, these were identical, so just use a single one. Signed-off-by: Sebastiaan van Stijn --- cli/registry/client/endpoint.go | 3 ++- cli/registry/client/fetcher.go | 4 ++-- internal/registry/service.go | 14 +------------- internal/registry/service_v2.go | 27 ++++++++------------------- 4 files changed, 13 insertions(+), 35 deletions(-) diff --git a/cli/registry/client/endpoint.go b/cli/registry/client/endpoint.go index 631a5bdf44ae..5b6cf4a5ad06 100644 --- a/cli/registry/client/endpoint.go +++ b/cli/registry/client/endpoint.go @@ -1,6 +1,7 @@ package client import ( + "context" "net" "net/http" "net/url" @@ -55,7 +56,7 @@ func getDefaultEndpoint(repoName reference.Named, insecure bool) (registry.APIEn if err != nil { return registry.APIEndpoint{}, err } - endpoints, err := registryService.LookupPushEndpoints(reference.Domain(repoName)) + endpoints, err := registryService.Endpoints(context.TODO(), reference.Domain(repoName)) if err != nil { return registry.APIEndpoint{}, err } diff --git a/cli/registry/client/fetcher.go b/cli/registry/client/fetcher.go index e169179d1670..c8687d9d91c8 100644 --- a/cli/registry/client/fetcher.go +++ b/cli/registry/client/fetcher.go @@ -283,10 +283,10 @@ func allEndpoints(namedRef reference.Named, insecure bool) ([]registry.APIEndpoi } registryService, err := registry.NewService(serviceOpts) if err != nil { - return []registry.APIEndpoint{}, err + return nil, err } repoInfo, _ := registry.ParseRepositoryInfo(namedRef) - endpoints, err := registryService.LookupPullEndpoints(reference.Domain(repoInfo.Name)) + endpoints, err := registryService.Endpoints(context.TODO(), reference.Domain(repoInfo.Name)) logrus.Debugf("endpoints for %s: %v", namedRef, endpoints) return endpoints, err } diff --git a/internal/registry/service.go b/internal/registry/service.go index 91b624462763..ed5b3e00e549 100644 --- a/internal/registry/service.go +++ b/internal/registry/service.go @@ -49,7 +49,7 @@ func (s *Service) Auth(ctx context.Context, authConfig *registry.AuthConfig, use } // Lookup endpoints for authentication. - endpoints, err := s.lookupV2Endpoints(ctx, registryHostName) + endpoints, err := s.Endpoints(ctx, registryHostName) if err != nil { if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) { return "", err @@ -85,15 +85,3 @@ type APIEndpoint struct { URL *url.URL TLSConfig *tls.Config } - -// LookupPullEndpoints creates a list of v2 endpoints to try to pull from, in order of preference. -// It gives preference to mirrors over the actual registry, and HTTPS over plain HTTP. -func (s *Service) LookupPullEndpoints(hostname string) ([]APIEndpoint, error) { - return s.lookupV2Endpoints(context.TODO(), hostname) -} - -// LookupPushEndpoints creates a list of v2 endpoints to try to push to, in order of preference. -// It gives preference to HTTPS over plain HTTP. Mirrors are not included. -func (s *Service) LookupPushEndpoints(hostname string) ([]APIEndpoint, error) { - return s.lookupV2Endpoints(context.TODO(), hostname) -} diff --git a/internal/registry/service_v2.go b/internal/registry/service_v2.go index 0f0059533022..ccfb5ac50959 100644 --- a/internal/registry/service_v2.go +++ b/internal/registry/service_v2.go @@ -7,15 +7,12 @@ import ( "github.com/docker/go-connections/tlsconfig" ) -func (s *Service) lookupV2Endpoints(ctx context.Context, hostname string) ([]APIEndpoint, error) { - var endpoints []APIEndpoint +func (s *Service) Endpoints(ctx context.Context, hostname string) ([]APIEndpoint, error) { if hostname == DefaultNamespace || hostname == IndexHostname { - endpoints = append(endpoints, APIEndpoint{ + return []APIEndpoint{{ URL: DefaultV2Registry, TLSConfig: tlsconfig.ServerDefault(), - }) - - return endpoints, nil + }}, nil } tlsConfig, err := newTLSConfig(ctx, hostname, s.config.isSecureIndex(hostname)) @@ -23,22 +20,14 @@ func (s *Service) lookupV2Endpoints(ctx context.Context, hostname string) ([]API return nil, err } - endpoints = []APIEndpoint{ - { - URL: &url.URL{ - Scheme: "https", - Host: hostname, - }, - TLSConfig: tlsConfig, - }, - } + endpoints := []APIEndpoint{{ + URL: &url.URL{Scheme: "https", Host: hostname}, + TLSConfig: tlsConfig, + }} if tlsConfig.InsecureSkipVerify { endpoints = append(endpoints, APIEndpoint{ - URL: &url.URL{ - Scheme: "http", - Host: hostname, - }, + URL: &url.URL{Scheme: "http", Host: hostname}, // used to check if supposed to be secure via InsecureSkipVerify TLSConfig: tlsConfig, }) From 2607ba8062b3fccd21b4ffe7c2ec3a27911bc3b5 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 24 Jul 2025 22:15:08 +0200 Subject: [PATCH 08/12] internal/registry: remove ValidateIndexName It was written to be used as validate-func for command-line flags, which we don't use it for (which for CLI-flags includes normalizing the value). The validation itself didn't add much; it only checked the registry didn't start or end with a hyphen (which would still fail when parsing). Signed-off-by: Sebastiaan van Stijn --- internal/registry/config.go | 14 ------- internal/registry/config_test.go | 65 -------------------------------- 2 files changed, 79 deletions(-) diff --git a/internal/registry/config.go b/internal/registry/config.go index 6abc299bebb2..fab9e8ab5ddd 100644 --- a/internal/registry/config.go +++ b/internal/registry/config.go @@ -105,10 +105,6 @@ func (config *serviceConfig) loadInsecureRegistries(registries []string) error { skip: for _, r := range registries { - // validate insecure registry - if _, err := ValidateIndexName(r); err != nil { - return err - } if scheme, host, ok := strings.Cut(r, "://"); ok { switch strings.ToLower(scheme) { case "http", "https": @@ -219,16 +215,6 @@ func isCIDRMatch(cidrs []*registry.NetIPNet, urlHost string) bool { return false } -// ValidateIndexName validates an index name. It is used by the daemon to -// validate the daemon configuration. -func ValidateIndexName(val string) (string, error) { - 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 { if val == "index.docker.io" { return "docker.io" diff --git a/internal/registry/config_test.go b/internal/registry/config_test.go index f3d8b4c44a2e..7e0838fe59c6 100644 --- a/internal/registry/config_test.go +++ b/internal/registry/config_test.go @@ -5,7 +5,6 @@ import ( cerrdefs "github.com/containerd/errdefs" "gotest.tools/v3/assert" - is "gotest.tools/v3/assert/cmp" ) func TestLoadInsecureRegistries(t *testing.T) { @@ -46,10 +45,6 @@ func TestLoadInsecureRegistries(t *testing.T) { registries: []string{"svn://myregistry.example.com"}, err: "insecure registry svn://myregistry.example.com should not contain '://'", }, - { - registries: []string{"-invalid-registry"}, - err: "Cannot begin or end with a hyphen", - }, { registries: []string{`mytest-.com`}, err: `insecure registry mytest-.com is not valid: invalid host "mytest-.com"`, @@ -96,63 +91,3 @@ func TestLoadInsecureRegistries(t *testing.T) { } } } - -func TestValidateIndexName(t *testing.T) { - valid := []struct { - index string - expect string - }{ - { - index: "index.docker.io", - expect: "docker.io", - }, - { - index: "example.com", - expect: "example.com", - }, - { - index: "127.0.0.1:8080", - expect: "127.0.0.1:8080", - }, - { - index: "mytest-1.com", - expect: "mytest-1.com", - }, - { - index: "mirror-1.example.com/v1/?q=foo", - expect: "mirror-1.example.com/v1/?q=foo", - }, - } - - for _, testCase := range valid { - result, err := ValidateIndexName(testCase.index) - if assert.Check(t, err) { - assert.Check(t, is.Equal(testCase.expect, result)) - } - } -} - -func TestValidateIndexNameWithError(t *testing.T) { - invalid := []struct { - index string - err string - }{ - { - index: "docker.io-", - err: "invalid index name (docker.io-). Cannot begin or end with a hyphen", - }, - { - index: "-example.com", - err: "invalid index name (-example.com). Cannot begin or end with a hyphen", - }, - { - index: "mirror-1.example.com/v1/?q=foo-", - err: "invalid index name (mirror-1.example.com/v1/?q=foo-). Cannot begin or end with a hyphen", - }, - } - for _, testCase := range invalid { - _, err := ValidateIndexName(testCase.index) - assert.Check(t, is.Error(err, testCase.err)) - assert.Check(t, cerrdefs.IsInvalidArgument(err)) - } -} From 219cfc8b7da53ec0e725a0ce31368d13f7dd8f5f Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 24 Jul 2025 22:36:47 +0200 Subject: [PATCH 09/12] internal/registry: define local serviceConfig The registry.ServiceConfig struct in the API types was meant for the registry configuration on the daemon side; it has variuos fields we don't use, defines methods for (un)marshaling JSON, and a custom version of `net.IPNet`, also to (un)marshal JSON. None of that is needed, so let's change it to a local type, and implement a constructor (as we now only have "insecure registries" to care about). Signed-off-by: Sebastiaan van Stijn --- internal/registry/config.go | 63 +++++++++++++++++--------------- internal/registry/config_test.go | 7 ++-- internal/registry/service.go | 8 ++-- 3 files changed, 40 insertions(+), 38 deletions(-) diff --git a/internal/registry/config.go b/internal/registry/config.go index fab9e8ab5ddd..72529f75df40 100644 --- a/internal/registry/config.go +++ b/internal/registry/config.go @@ -21,12 +21,19 @@ import ( ) // ServiceOptions holds command line options. +// +// TODO(thaJeztah): add CertsDir as option to replace the [CertsDir] function, which sets the location magically. type ServiceOptions struct { InsecureRegistries []string `json:"insecure-registries,omitempty"` } // serviceConfig holds daemon configuration for the registry service. -type serviceConfig registry.ServiceConfig +// +// It's a reduced version of [registry.ServiceConfig] for the CLI. +type serviceConfig struct { + insecureRegistryCIDRs []*net.IPNet + indexConfigs map[string]*registry.IndexInfo +} // TODO(thaJeztah) both the "index.docker.io" and "registry-1.docker.io" domains // are here for historic reasons and backward-compatibility. These domains @@ -38,27 +45,22 @@ type serviceConfig registry.ServiceConfig const ( // DefaultNamespace is the default namespace DefaultNamespace = "docker.io" - // DefaultRegistryHost is the hostname for the default (Docker Hub) registry - // used for pushing and pulling images. This hostname is hard-coded to handle - // the conversion from image references without registry name (e.g. "ubuntu", - // or "ubuntu:latest"), as well as references using the "docker.io" domain - // name, which is used as canonical reference for images on Docker Hub, but - // does not match the domain-name of Docker Hub's registry. - DefaultRegistryHost = "registry-1.docker.io" // IndexHostname is the index hostname, used for authentication and image search. IndexHostname = "index.docker.io" // IndexServer is used for user auth and image search - IndexServer = "https://" + IndexHostname + "/v1/" + IndexServer = "https://index.docker.io/v1/" // IndexName is the name of the index IndexName = "docker.io" ) var ( - // DefaultV2Registry is the URI of the default (Docker Hub) registry. - DefaultV2Registry = &url.URL{ - Scheme: "https", - Host: DefaultRegistryHost, - } + // DefaultV2Registry is the URI of the default (Docker Hub) registry + // used for pushing and pulling images. This hostname is hard-coded to handle + // the conversion from image references without registry name (e.g. "ubuntu", + // or "ubuntu:latest"), as well as references using the "docker.io" domain + // name, which is used as canonical reference for images on Docker Hub, but + // does not match the domain-name of Docker Hub's registry. + DefaultV2Registry = &url.URL{Scheme: "https", Host: "registry-1.docker.io"} validHostPortRegex = sync.OnceValue(func() *regexp.Regexp { return regexp.MustCompile(`^` + reference.DomainRegexp.String() + `$`) @@ -92,14 +94,17 @@ func CertsDir() string { return certsDir } -// loadInsecureRegistries loads insecure registries to config -func (config *serviceConfig) loadInsecureRegistries(registries []string) error { +// newServiceConfig creates a new service config with the given options. +func newServiceConfig(registries []string) (*serviceConfig, error) { + if len(registries) == 0 { + return &serviceConfig{}, nil + } // Localhost is by default considered as an insecure registry. This is a // stop-gap for people who are running a private registry on localhost. registries = append(registries, "::1/128", "127.0.0.0/8") var ( - insecureRegistryCIDRs = make([]*registry.NetIPNet, 0) + insecureRegistryCIDRs = make([]*net.IPNet, 0) indexConfigs = make(map[string]*registry.IndexInfo) ) @@ -112,24 +117,23 @@ skip: r = host default: // unsupported scheme - return invalidParamf("insecure registry %s should not contain '://'", r) + return nil, invalidParamf("insecure registry %s should not contain '://'", r) } } // Check if CIDR was passed to --insecure-registry _, ipnet, err := net.ParseCIDR(r) if err == nil { // Valid CIDR. If ipnet is already in config.InsecureRegistryCIDRs, skip. - data := (*registry.NetIPNet)(ipnet) for _, value := range insecureRegistryCIDRs { - if value.IP.String() == data.IP.String() && value.Mask.String() == data.Mask.String() { + if value.IP.String() == ipnet.IP.String() && value.Mask.String() == ipnet.Mask.String() { continue skip } } // ipnet is not found, add it in config.InsecureRegistryCIDRs - insecureRegistryCIDRs = append(insecureRegistryCIDRs, data) + insecureRegistryCIDRs = append(insecureRegistryCIDRs, ipnet) } else { if err := validateHostPort(r); err != nil { - return invalidParamWrapf(err, "insecure registry %s is not valid", r) + return nil, invalidParamWrapf(err, "insecure registry %s is not valid", r) } // Assume `host:port` if not CIDR. indexConfigs[r] = ®istry.IndexInfo{ @@ -146,10 +150,11 @@ skip: Secure: true, Official: true, } - config.InsecureRegistryCIDRs = insecureRegistryCIDRs - config.IndexConfigs = indexConfigs - return nil + return &serviceConfig{ + indexConfigs: indexConfigs, + insecureRegistryCIDRs: insecureRegistryCIDRs, + }, nil } // isSecureIndex returns false if the provided indexName is part of the list of insecure registries @@ -166,11 +171,11 @@ skip: func (config *serviceConfig) isSecureIndex(indexName string) bool { // Check for configured index, first. This is needed in case isSecureIndex // is called from anything besides newIndexInfo, in order to honor per-index configurations. - if index, ok := config.IndexConfigs[indexName]; ok { + if index, ok := config.indexConfigs[indexName]; ok { return index.Secure } - return !isCIDRMatch(config.InsecureRegistryCIDRs, indexName) + return !isCIDRMatch(config.insecureRegistryCIDRs, indexName) } // for mocking in unit tests. @@ -179,7 +184,7 @@ var lookupIP = net.LookupIP // isCIDRMatch returns true if urlHost matches an element of cidrs. urlHost is a URL.Host ("host:port" or "host") // where the `host` part can be either a domain name or an IP address. If it is a domain name, then it will be // resolved to IP addresses for matching. If resolution fails, false is returned. -func isCIDRMatch(cidrs []*registry.NetIPNet, urlHost string) bool { +func isCIDRMatch(cidrs []*net.IPNet, urlHost string) bool { if len(cidrs) == 0 { return false } @@ -206,7 +211,7 @@ func isCIDRMatch(cidrs []*registry.NetIPNet, urlHost string) bool { for _, addr := range addresses { for _, ipnet := range cidrs { // check if the addr falls in the subnet - if (*net.IPNet)(ipnet).Contains(addr) { + if ipnet.Contains(addr) { return true } } diff --git a/internal/registry/config_test.go b/internal/registry/config_test.go index 7e0838fe59c6..a5574a298d45 100644 --- a/internal/registry/config_test.go +++ b/internal/registry/config_test.go @@ -67,20 +67,19 @@ func TestLoadInsecureRegistries(t *testing.T) { }, } for _, testCase := range testCases { - config := &serviceConfig{} - err := config.loadInsecureRegistries(testCase.registries) + config, err := newServiceConfig(testCase.registries) if testCase.err == "" { if err != nil { t.Fatalf("expect no error, got '%s'", err) } match := false - for index := range config.IndexConfigs { + for index := range config.indexConfigs { if index == testCase.index { match = true } } if !match { - t.Fatalf("expect index configs to contain '%s', got %+v", testCase.index, config.IndexConfigs) + t.Fatalf("expect index configs to contain '%s', got %+v", testCase.index, config.indexConfigs) } } else { if err == nil { diff --git a/internal/registry/service.go b/internal/registry/service.go index ed5b3e00e549..51576387ae49 100644 --- a/internal/registry/service.go +++ b/internal/registry/service.go @@ -21,11 +21,9 @@ type Service struct { // NewService returns a new instance of [Service] ready to be installed into // an engine. func NewService(options ServiceOptions) (*Service, error) { - config := &serviceConfig{} - if len(options.InsecureRegistries) > 0 { - if err := config.loadInsecureRegistries(options.InsecureRegistries); err != nil { - return nil, err - } + config, err := newServiceConfig(options.InsecureRegistries) + if err != nil { + return nil, err } return &Service{config: config}, nil } From c297770d2df1d4438d0e0b5f3d3b57b293c96ffa Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 24 Jul 2025 23:15:40 +0200 Subject: [PATCH 10/12] internal/registry: remove pkg/errors Signed-off-by: Sebastiaan van Stijn --- internal/registry/auth.go | 4 ++-- internal/registry/config.go | 5 +++-- internal/registry/errors.go | 9 +++------ internal/registry/registry.go | 3 ++- internal/registry/service.go | 3 ++- 5 files changed, 12 insertions(+), 12 deletions(-) diff --git a/internal/registry/auth.go b/internal/registry/auth.go index e3ad9613d8ef..e15a139cb471 100644 --- a/internal/registry/auth.go +++ b/internal/registry/auth.go @@ -2,6 +2,7 @@ package registry import ( "context" + "fmt" "net/http" "net/url" "strings" @@ -12,7 +13,6 @@ import ( "github.com/docker/distribution/registry/client/auth/challenge" "github.com/docker/distribution/registry/client/transport" "github.com/moby/moby/api/types/registry" - "github.com/pkg/errors" ) // AuthClientID is used the ClientID used for the token server @@ -67,7 +67,7 @@ func loginV2(ctx context.Context, authConfig *registry.AuthConfig, endpoint APIE if resp.StatusCode != http.StatusOK { // TODO(dmcgowan): Attempt to further interpret result, status code and error code string - return "", errors.Errorf("login attempt to %s failed with status: %d %s", endpointStr, resp.StatusCode, http.StatusText(resp.StatusCode)) + return "", fmt.Errorf("login attempt to %s failed with status: %d %s", endpointStr, resp.StatusCode, http.StatusText(resp.StatusCode)) } return credentialAuthConfig.IdentityToken, nil diff --git a/internal/registry/config.go b/internal/registry/config.go index 72529f75df40..b2f3070b1c4e 100644 --- a/internal/registry/config.go +++ b/internal/registry/config.go @@ -5,6 +5,7 @@ package registry import ( "context" + "fmt" "net" "net/url" "os" @@ -117,7 +118,7 @@ skip: r = host default: // unsupported scheme - return nil, invalidParamf("insecure registry %s should not contain '://'", r) + return nil, invalidParam(fmt.Errorf("insecure registry %s should not contain '://'", r)) } } // Check if CIDR was passed to --insecure-registry @@ -133,7 +134,7 @@ skip: insecureRegistryCIDRs = append(insecureRegistryCIDRs, ipnet) } else { if err := validateHostPort(r); err != nil { - return nil, invalidParamWrapf(err, "insecure registry %s is not valid", r) + return nil, invalidParam(fmt.Errorf("insecure registry %s is not valid: %w", r, err)) } // Assume `host:port` if not CIDR. indexConfigs[r] = ®istry.IndexInfo{ diff --git a/internal/registry/errors.go b/internal/registry/errors.go index 4174c91d15b4..9b321ab0159d 100644 --- a/internal/registry/errors.go +++ b/internal/registry/errors.go @@ -1,10 +1,11 @@ package registry import ( + "errors" + "fmt" "net/url" "github.com/docker/distribution/registry/api/errcode" - "github.com/pkg/errors" ) func translateV2AuthError(err error) error { @@ -23,11 +24,7 @@ func invalidParam(err error) error { } func invalidParamf(format string, args ...interface{}) error { - return invalidParameterErr{errors.Errorf(format, args...)} -} - -func invalidParamWrapf(err error, format string, args ...interface{}) error { - return invalidParameterErr{errors.Wrapf(err, format, args...)} + return invalidParameterErr{fmt.Errorf(format, args...)} } type unauthorizedErr struct{ error } diff --git a/internal/registry/registry.go b/internal/registry/registry.go index d69c1d9ec7b3..97946d7fd879 100644 --- a/internal/registry/registry.go +++ b/internal/registry/registry.go @@ -4,6 +4,7 @@ package registry import ( "context" "crypto/tls" + "fmt" "net" "net/http" "os" @@ -82,7 +83,7 @@ func loadTLSConfig(ctx context.Context, directory string, tlsConfig *tls.Config) if tlsConfig.RootCAs == nil { systemPool, err := tlsconfig.SystemCertPool() if err != nil { - return invalidParamWrapf(err, "unable to get system cert pool") + return invalidParam(fmt.Errorf("unable to get system cert pool: %w", err)) } tlsConfig.RootCAs = systemPool } diff --git a/internal/registry/service.go b/internal/registry/service.go index 51576387ae49..0e29b98b0411 100644 --- a/internal/registry/service.go +++ b/internal/registry/service.go @@ -4,6 +4,7 @@ import ( "context" "crypto/tls" "errors" + "fmt" "net/url" "strings" @@ -41,7 +42,7 @@ func (s *Service) Auth(ctx context.Context, authConfig *registry.AuthConfig, use } u, err := url.Parse(serverAddress) if err != nil { - return "", invalidParamWrapf(err, "unable to parse server address") + return "", invalidParam(fmt.Errorf("unable to parse server address: %w", err)) } registryHostName = u.Host } From cd277a5815ace9b7de0e116c8d243a13e2105213 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 24 Jul 2025 23:24:22 +0200 Subject: [PATCH 11/12] cli/command/system: remove use of Mirrors field in test Signed-off-by: Sebastiaan van Stijn --- cli/command/system/info_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/cli/command/system/info_test.go b/cli/command/system/info_test.go index 2b028cde33f4..1591ed833cb6 100644 --- a/cli/command/system/info_test.go +++ b/cli/command/system/info_test.go @@ -78,7 +78,6 @@ var sampleInfoNoSwarm = system.Info{ IndexConfigs: map[string]*registrytypes.IndexInfo{ "docker.io": { Name: "docker.io", - Mirrors: nil, Secure: true, Official: true, }, From f907c7a4b03898455d024920f38fef3545a29c05 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 25 Jul 2025 00:08:26 +0200 Subject: [PATCH 12/12] internal/registry: fix linting issues (revive) internal/registry/errors.go:26:43: use-any: since Go 1.18 'interface{}' can be replaced by 'any' (revive) func invalidParamf(format string, args ...interface{}) error { ^ internal/registry/registry_mock_test.go:52:51: use-any: since Go 1.18 'interface{}' can be replaced by 'any' (revive) func writeResponse(w http.ResponseWriter, message interface{}, code int) { ^ Signed-off-by: Sebastiaan van Stijn --- internal/registry/errors.go | 5 ++++- internal/registry/registry_mock_test.go | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/internal/registry/errors.go b/internal/registry/errors.go index 9b321ab0159d..e27eb3e7a682 100644 --- a/internal/registry/errors.go +++ b/internal/registry/errors.go @@ -1,3 +1,6 @@ +// FIXME(thaJeztah): remove once we are a module; the go:build directive prevents go from downgrading language version to go1.16: +//go:build go1.23 + package registry import ( @@ -23,7 +26,7 @@ func invalidParam(err error) error { return invalidParameterErr{err} } -func invalidParamf(format string, args ...interface{}) error { +func invalidParamf(format string, args ...any) error { return invalidParameterErr{fmt.Errorf(format, args...)} } diff --git a/internal/registry/registry_mock_test.go b/internal/registry/registry_mock_test.go index 8bed2f02bb00..024f6a83f00d 100644 --- a/internal/registry/registry_mock_test.go +++ b/internal/registry/registry_mock_test.go @@ -49,7 +49,7 @@ func writeHeaders(w http.ResponseWriter) { h.Add("Cache-Control", "no-cache") } -func writeResponse(w http.ResponseWriter, message interface{}, code int) { +func writeResponse(w http.ResponseWriter, message any, code int) { writeHeaders(w) w.WriteHeader(code) body, err := json.Marshal(message)