From 5da2dd98e95983d062b9f27d609109dd99d90539 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 25 Aug 2022 10:13:03 +0200 Subject: [PATCH] registry: move search-related code to separate files Signed-off-by: Sebastiaan van Stijn --- registry/config.go | 11 -- registry/registry.go | 45 ------ registry/registry_test.go | 133 ------------------ registry/search.go | 30 +++- .../{endpoint_v1.go => search_endpoint_v1.go} | 45 ++++++ ...int_test.go => search_endpoint_v1_test.go} | 63 +++++++++ registry/{session.go => search_session.go} | 0 registry/search_test.go | 71 +++++++++- registry/service.go | 12 -- 9 files changed, 204 insertions(+), 206 deletions(-) rename registry/{endpoint_v1.go => search_endpoint_v1.go} (83%) rename registry/{endpoint_test.go => search_endpoint_v1_test.go} (71%) rename registry/{session.go => search_session.go} (100%) diff --git a/registry/config.go b/registry/config.go index 45bb6482a7..72b7cb2bea 100644 --- a/registry/config.go +++ b/registry/config.go @@ -435,14 +435,3 @@ func newRepositoryInfo(config *serviceConfig, name reference.Named) (*Repository func ParseRepositoryInfo(reposName reference.Named) (*RepositoryInfo, error) { return newRepositoryInfo(emptyServiceConfig, reposName) } - -// 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) - return newIndexInfo(emptyServiceConfig, indexName) -} diff --git a/registry/registry.go b/registry/registry.go index 83f712a0a0..9b42d9fe95 100644 --- a/registry/registry.go +++ b/registry/registry.go @@ -113,51 +113,6 @@ func Headers(userAgent string, metaHeaders http.Header) []transport.RequestModif return modifiers } -// httpClient returns an HTTP client structure which uses the given transport -// and contains the necessary headers for redirected requests -func httpClient(transport http.RoundTripper) *http.Client { - return &http.Client{ - Transport: transport, - 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 -} - // newTransport returns a new HTTP transport. If tlsConfig is nil, it uses the // default TLS configuration. func newTransport(tlsConfig *tls.Config) *http.Transport { diff --git a/registry/registry_test.go b/registry/registry_test.go index 37ab99ded4..50406fcf3f 100644 --- a/registry/registry_test.go +++ b/registry/registry_test.go @@ -1,49 +1,16 @@ package registry // import "github.com/docker/docker/registry" import ( - "net/http" - "net/http/httputil" "os" "testing" "github.com/docker/distribution/reference" - "github.com/docker/distribution/registry/client/transport" "github.com/docker/docker/api/types/registry" "gotest.tools/v3/assert" is "gotest.tools/v3/assert/cmp" "gotest.tools/v3/skip" ) -func spawnTestRegistrySession(t *testing.T) *session { - authConfig := ®istry.AuthConfig{} - endpoint, err := newV1Endpoint(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(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 -} - func TestParseRepositoryInfo(t *testing.T) { type staticRepositoryInfo struct { Index *registry.IndexInfo @@ -450,83 +417,6 @@ func TestMirrorEndpointLookup(t *testing.T) { } } -func TestSearchRepositories(t *testing.T) { - r := spawnTestRegistrySession(t) - results, err := r.searchRepositories("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 TestTrustedLocation(t *testing.T) { - for _, url := range []string{"http://example.com", "https://example.com:7777", "http://docker.io", "http://test.docker.com", "https://fakedocker.com"} { - req, _ := http.NewRequest(http.MethodGet, url, nil) - assert.Check(t, !trustedLocation(req)) - } - - for _, url := range []string{"https://docker.io", "https://test.docker.com:80"} { - req, _ := http.NewRequest(http.MethodGet, url, nil) - 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], nil) - reqFrom.Header.Add("Content-Type", "application/json") - reqFrom.Header.Add("Authorization", "super_secret") - reqTo, _ := http.NewRequest(http.MethodGet, urls[1], nil) - - _ = 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], nil) - reqFrom.Header.Add("Content-Type", "application/json") - reqFrom.Header.Add("Authorization", "super_secret") - reqTo, _ := http.NewRequest(http.MethodGet, urls[1], nil) - - _ = 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'") - } - } -} - func TestAllowNondistributableArtifacts(t *testing.T) { tests := []struct { addr string @@ -614,26 +504,3 @@ func TestIsSecureIndex(t *testing.T) { } } } - -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 -} diff --git a/registry/search.go b/registry/search.go index d2c3559905..8324203fc4 100644 --- a/registry/search.go +++ b/registry/search.go @@ -1,4 +1,4 @@ -package registry // import "github.com/docker/docker/registry" +package registry import ( "context" @@ -6,12 +6,11 @@ import ( "strconv" "strings" + "github.com/containerd/containerd/log" + "github.com/docker/distribution/registry/client/auth" "github.com/docker/docker/api/types/filters" "github.com/docker/docker/api/types/registry" "github.com/docker/docker/errdefs" - - "github.com/containerd/containerd/log" - "github.com/docker/distribution/registry/client/auth" "github.com/pkg/errors" ) @@ -139,3 +138,26 @@ func (s *Service) searchUnfiltered(ctx context.Context, term string, limit int, return newSession(client, endpoint).searchRepositories(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) + return newIndexInfo(emptyServiceConfig, indexName) +} diff --git a/registry/endpoint_v1.go b/registry/search_endpoint_v1.go similarity index 83% rename from registry/endpoint_v1.go rename to registry/search_endpoint_v1.go index 1fdf659ae0..6d2c331043 100644 --- a/registry/endpoint_v1.go +++ b/registry/search_endpoint_v1.go @@ -175,3 +175,48 @@ func (e *v1Endpoint) ping() (v1PingResult, error) { log.G(context.TODO()).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(transport http.RoundTripper) *http.Client { + return &http.Client{ + Transport: transport, + 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/registry/endpoint_test.go b/registry/search_endpoint_v1_test.go similarity index 71% rename from registry/endpoint_test.go rename to registry/search_endpoint_v1_test.go index f76b1e94d5..81d4f0b5a4 100644 --- a/registry/endpoint_test.go +++ b/registry/search_endpoint_v1_test.go @@ -186,3 +186,66 @@ func TestV1EndpointValidate(t *testing.T) { 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, nil) + assert.Check(t, !trustedLocation(req)) + } + + for _, u := range []string{"https://docker.io", "https://test.docker.com:80"} { + req, _ := http.NewRequest(http.MethodGet, u, nil) + 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], nil) + reqFrom.Header.Add("Content-Type", "application/json") + reqFrom.Header.Add("Authorization", "super_secret") + reqTo, _ := http.NewRequest(http.MethodGet, urls[1], nil) + + _ = 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], nil) + reqFrom.Header.Add("Content-Type", "application/json") + reqFrom.Header.Add("Authorization", "super_secret") + reqTo, _ := http.NewRequest(http.MethodGet, urls[1], nil) + + _ = 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/registry/session.go b/registry/search_session.go similarity index 100% rename from registry/session.go rename to registry/search_session.go diff --git a/registry/search_test.go b/registry/search_test.go index 26cad78cd6..f9e1bd95ed 100644 --- a/registry/search_test.go +++ b/registry/search_test.go @@ -1,18 +1,87 @@ -package registry // import "github.com/docker/docker/registry" +package registry import ( "context" "encoding/json" "net/http" "net/http/httptest" + "net/http/httputil" "testing" + "github.com/docker/distribution/registry/client/transport" "github.com/docker/docker/api/types/filters" "github.com/docker/docker/api/types/registry" "github.com/docker/docker/errdefs" "gotest.tools/v3/assert" ) +func spawnTestRegistrySession(t *testing.T) *session { + authConfig := ®istry.AuthConfig{} + endpoint, err := newV1Endpoint(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(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("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 diff --git a/registry/service.go b/registry/service.go index 8e3c95c074..75b6dde06c 100644 --- a/registry/service.go +++ b/registry/service.go @@ -91,18 +91,6 @@ func (s *Service) Auth(ctx context.Context, authConfig *registry.AuthConfig, use return "", "", err } -// 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] -} - // ResolveRepository splits a repository name into its components // and configuration of the associated registry. func (s *Service) ResolveRepository(name reference.Named) (*RepositoryInfo, error) {