registry: move search-related code to separate files

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

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)
-}

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 {

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 := &registry.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
-}

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)
+}

registry/endpoint_v1.go → 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
+}

registry/endpoint_test.go → 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'")
+		}
+	}
+}

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 := &registry.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

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) {