浏览代码

registry/search: pass User-Agent through headers

Commit 3991faf4640a46412a8af000ede78fc5cba76d0a moved search into the registry
package, which also made the `dockerversion` package a dependency for registry,
which brings additional (indirect) dependencies, such as `pkg/parsers/kernel`,
and `golang.org/x/sys/windows/registry`.

Client code, such as used in docker/cli may depend on the `registry` package,
but should not depend on those additional dependencies.

This patch moves setting the userAgent to the API router, and instead of
passing it as a separate argument, includes it into the "headers".

As these headers now not only contain the `X-Meta-...` headers, the variables
were renamed accordingly.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Sebastiaan van Stijn 2 年之前
父节点
当前提交
cb76638699

+ 1 - 1
api/server/router/image/backend.go

@@ -42,5 +42,5 @@ type registryBackend interface {
 }
 }
 
 
 type Searcher interface {
 type Searcher interface {
-	Search(ctx context.Context, searchFilters filters.Args, term string, limit int, authConfig *registry.AuthConfig, metaHeaders map[string][]string) ([]registry.SearchResult, error)
+	Search(ctx context.Context, searchFilters filters.Args, term string, limit int, authConfig *registry.AuthConfig, headers map[string][]string) ([]registry.SearchResult, error)
 }
 }

+ 10 - 7
api/server/router/image/image_routes.go

@@ -18,6 +18,7 @@ import (
 	"github.com/docker/docker/api/types/registry"
 	"github.com/docker/docker/api/types/registry"
 	"github.com/docker/docker/api/types/versions"
 	"github.com/docker/docker/api/types/versions"
 	"github.com/docker/docker/builder/remotecontext"
 	"github.com/docker/docker/builder/remotecontext"
+	"github.com/docker/docker/dockerversion"
 	"github.com/docker/docker/errdefs"
 	"github.com/docker/docker/errdefs"
 	"github.com/docker/docker/image"
 	"github.com/docker/docker/image"
 	"github.com/docker/docker/pkg/ioutils"
 	"github.com/docker/docker/pkg/ioutils"
@@ -392,13 +393,6 @@ func (ir *imageRouter) getImagesSearch(ctx context.Context, w http.ResponseWrite
 		return err
 		return err
 	}
 	}
 
 
-	var headers = map[string][]string{}
-	for k, v := range r.Header {
-		if strings.HasPrefix(k, "X-Meta-") {
-			headers[k] = v
-		}
-	}
-
 	var limit int
 	var limit int
 	if r.Form.Get("limit") != "" {
 	if r.Form.Get("limit") != "" {
 		var err error
 		var err error
@@ -415,6 +409,15 @@ func (ir *imageRouter) getImagesSearch(ctx context.Context, w http.ResponseWrite
 	// For a search it is not an error if no auth was given. Ignore invalid
 	// For a search it is not an error if no auth was given. Ignore invalid
 	// AuthConfig to increase compatibility with the existing API.
 	// AuthConfig to increase compatibility with the existing API.
 	authConfig, _ := registry.DecodeAuthConfig(r.Header.Get(registry.AuthHeader))
 	authConfig, _ := registry.DecodeAuthConfig(r.Header.Get(registry.AuthHeader))
+
+	var headers = http.Header{}
+	for k, v := range r.Header {
+		k = http.CanonicalHeaderKey(k)
+		if strings.HasPrefix(k, "X-Meta-") {
+			headers[k] = v
+		}
+	}
+	headers.Set("User-Agent", dockerversion.DockerUserAgent(ctx))
 	res, err := ir.searcher.Search(ctx, searchFilters, r.Form.Get("term"), limit, authConfig, headers)
 	res, err := ir.searcher.Search(ctx, searchFilters, r.Form.Get("term"), limit, authConfig, headers)
 	if err != nil {
 	if err != nil {
 		return err
 		return err

+ 2 - 2
registry/endpoint_test.go

@@ -20,7 +20,7 @@ func TestEndpointParse(t *testing.T) {
 		{"http://0.0.0.0:5000/v0/", "http://0.0.0.0:5000/v0/v1/"},
 		{"http://0.0.0.0:5000/v0/", "http://0.0.0.0:5000/v0/v1/"},
 	}
 	}
 	for _, td := range testData {
 	for _, td := range testData {
-		e, err := newV1EndpointFromStr(td.str, nil, "", nil)
+		e, err := newV1EndpointFromStr(td.str, nil, nil)
 		if err != nil {
 		if err != nil {
 			t.Errorf("%q: %s", td.str, err)
 			t.Errorf("%q: %s", td.str, err)
 		}
 		}
@@ -39,7 +39,7 @@ func TestEndpointParseInvalid(t *testing.T) {
 		"http://0.0.0.0:5000/v2/",
 		"http://0.0.0.0:5000/v2/",
 	}
 	}
 	for _, td := range testData {
 	for _, td := range testData {
-		e, err := newV1EndpointFromStr(td, nil, "", nil)
+		e, err := newV1EndpointFromStr(td, nil, nil)
 		if err == nil {
 		if err == nil {
 			t.Errorf("expected error parsing %q: parsed as %q", td, e)
 			t.Errorf("expected error parsing %q: parsed as %q", td, e)
 		}
 		}

+ 4 - 4
registry/endpoint_v1.go

@@ -35,13 +35,13 @@ type v1Endpoint struct {
 
 
 // newV1Endpoint parses the given address to return a registry endpoint.
 // newV1Endpoint parses the given address to return a registry endpoint.
 // TODO: remove. This is only used by search.
 // TODO: remove. This is only used by search.
-func newV1Endpoint(index *registry.IndexInfo, userAgent string, metaHeaders http.Header) (*v1Endpoint, error) {
+func newV1Endpoint(index *registry.IndexInfo, headers http.Header) (*v1Endpoint, error) {
 	tlsConfig, err := newTLSConfig(index.Name, index.Secure)
 	tlsConfig, err := newTLSConfig(index.Name, index.Secure)
 	if err != nil {
 	if err != nil {
 		return nil, err
 		return nil, err
 	}
 	}
 
 
-	endpoint, err := newV1EndpointFromStr(GetAuthConfigKey(index), tlsConfig, userAgent, metaHeaders)
+	endpoint, err := newV1EndpointFromStr(GetAuthConfigKey(index), tlsConfig, headers)
 	if err != nil {
 	if err != nil {
 		return nil, err
 		return nil, err
 	}
 	}
@@ -100,7 +100,7 @@ func trimV1Address(address string) (string, error) {
 	return address, nil
 	return address, nil
 }
 }
 
 
-func newV1EndpointFromStr(address string, tlsConfig *tls.Config, userAgent string, metaHeaders http.Header) (*v1Endpoint, error) {
+func newV1EndpointFromStr(address string, tlsConfig *tls.Config, headers http.Header) (*v1Endpoint, error) {
 	if !strings.HasPrefix(address, "http://") && !strings.HasPrefix(address, "https://") {
 	if !strings.HasPrefix(address, "http://") && !strings.HasPrefix(address, "https://") {
 		address = "https://" + address
 		address = "https://" + address
 	}
 	}
@@ -121,7 +121,7 @@ func newV1EndpointFromStr(address string, tlsConfig *tls.Config, userAgent strin
 	return &v1Endpoint{
 	return &v1Endpoint{
 		IsSecure: tlsConfig == nil || !tlsConfig.InsecureSkipVerify,
 		IsSecure: tlsConfig == nil || !tlsConfig.InsecureSkipVerify,
 		URL:      uri,
 		URL:      uri,
-		client:   httpClient(transport.NewTransport(tr, Headers(userAgent, metaHeaders)...)),
+		client:   httpClient(transport.NewTransport(tr, Headers("", headers)...)),
 	}, nil
 	}, nil
 }
 }
 
 

+ 6 - 6
registry/registry_test.go

@@ -17,7 +17,7 @@ import (
 
 
 func spawnTestRegistrySession(t *testing.T) *session {
 func spawnTestRegistrySession(t *testing.T) *session {
 	authConfig := &registry.AuthConfig{}
 	authConfig := &registry.AuthConfig{}
-	endpoint, err := newV1Endpoint(makeIndex("/v1/"), "", nil)
+	endpoint, err := newV1Endpoint(makeIndex("/v1/"), nil)
 	if err != nil {
 	if err != nil {
 		t.Fatal(err)
 		t.Fatal(err)
 	}
 	}
@@ -48,7 +48,7 @@ func spawnTestRegistrySession(t *testing.T) *session {
 func TestPingRegistryEndpoint(t *testing.T) {
 func TestPingRegistryEndpoint(t *testing.T) {
 	skip.If(t, os.Getuid() != 0, "skipping test that requires root")
 	skip.If(t, os.Getuid() != 0, "skipping test that requires root")
 	testPing := func(index *registry.IndexInfo, expectedStandalone bool, assertMessage string) {
 	testPing := func(index *registry.IndexInfo, expectedStandalone bool, assertMessage string) {
-		ep, err := newV1Endpoint(index, "", nil)
+		ep, err := newV1Endpoint(index, nil)
 		if err != nil {
 		if err != nil {
 			t.Fatal(err)
 			t.Fatal(err)
 		}
 		}
@@ -69,7 +69,7 @@ func TestEndpoint(t *testing.T) {
 	skip.If(t, os.Getuid() != 0, "skipping test that requires root")
 	skip.If(t, os.Getuid() != 0, "skipping test that requires root")
 	// Simple wrapper to fail test if err != nil
 	// Simple wrapper to fail test if err != nil
 	expandEndpoint := func(index *registry.IndexInfo) *v1Endpoint {
 	expandEndpoint := func(index *registry.IndexInfo) *v1Endpoint {
-		endpoint, err := newV1Endpoint(index, "", nil)
+		endpoint, err := newV1Endpoint(index, nil)
 		if err != nil {
 		if err != nil {
 			t.Fatal(err)
 			t.Fatal(err)
 		}
 		}
@@ -78,14 +78,14 @@ func TestEndpoint(t *testing.T) {
 
 
 	assertInsecureIndex := func(index *registry.IndexInfo) {
 	assertInsecureIndex := func(index *registry.IndexInfo) {
 		index.Secure = true
 		index.Secure = true
-		_, err := newV1Endpoint(index, "", nil)
+		_, err := newV1Endpoint(index, nil)
 		assert.ErrorContains(t, err, "insecure-registry", index.Name+": Expected insecure-registry  error for insecure index")
 		assert.ErrorContains(t, err, "insecure-registry", index.Name+": Expected insecure-registry  error for insecure index")
 		index.Secure = false
 		index.Secure = false
 	}
 	}
 
 
 	assertSecureIndex := func(index *registry.IndexInfo) {
 	assertSecureIndex := func(index *registry.IndexInfo) {
 		index.Secure = true
 		index.Secure = true
-		_, err := newV1Endpoint(index, "", nil)
+		_, err := newV1Endpoint(index, nil)
 		assert.ErrorContains(t, err, "certificate signed by unknown authority", index.Name+": Expected cert error for secure index")
 		assert.ErrorContains(t, err, "certificate signed by unknown authority", index.Name+": Expected cert error for secure index")
 		index.Secure = false
 		index.Secure = false
 	}
 	}
@@ -132,7 +132,7 @@ func TestEndpoint(t *testing.T) {
 	}
 	}
 	for _, address := range badEndpoints {
 	for _, address := range badEndpoints {
 		index.Name = address
 		index.Name = address
-		_, err := newV1Endpoint(index, "", nil)
+		_, err := newV1Endpoint(index, nil)
 		assert.Check(t, err != nil, "Expected error while expanding bad endpoint: %s", address)
 		assert.Check(t, err != nil, "Expected error while expanding bad endpoint: %s", address)
 	}
 	}
 }
 }

+ 5 - 5
registry/search.go

@@ -8,7 +8,6 @@ import (
 
 
 	"github.com/docker/docker/api/types/filters"
 	"github.com/docker/docker/api/types/filters"
 	"github.com/docker/docker/api/types/registry"
 	"github.com/docker/docker/api/types/registry"
-	"github.com/docker/docker/dockerversion"
 	"github.com/docker/docker/errdefs"
 	"github.com/docker/docker/errdefs"
 
 
 	"github.com/docker/distribution/registry/client/auth"
 	"github.com/docker/distribution/registry/client/auth"
@@ -52,7 +51,7 @@ func (s *Service) Search(ctx context.Context, searchFilters filters.Args, term s
 		}
 		}
 	}
 	}
 
 
-	unfilteredResult, err := s.searchUnfiltered(ctx, term, limit, authConfig, dockerversion.DockerUserAgent(ctx), headers)
+	unfilteredResult, err := s.searchUnfiltered(ctx, term, limit, authConfig, headers)
 	if err != nil {
 	if err != nil {
 		return nil, err
 		return nil, err
 	}
 	}
@@ -80,7 +79,7 @@ func (s *Service) Search(ctx context.Context, searchFilters filters.Args, term s
 	return filteredResults, nil
 	return filteredResults, nil
 }
 }
 
 
-func (s *Service) searchUnfiltered(ctx context.Context, term string, limit int, authConfig *registry.AuthConfig, userAgent string, headers map[string][]string) (*registry.SearchResults, error) {
+func (s *Service) searchUnfiltered(ctx context.Context, term string, limit int, authConfig *registry.AuthConfig, headers http.Header) (*registry.SearchResults, error) {
 	// TODO Use ctx when searching for repositories
 	// TODO Use ctx when searching for repositories
 	if hasScheme(term) {
 	if hasScheme(term) {
 		return nil, invalidParamf("invalid repository name: repository name (%s) should not have a scheme", term)
 		return nil, invalidParamf("invalid repository name: repository name (%s) should not have a scheme", term)
@@ -101,7 +100,7 @@ func (s *Service) searchUnfiltered(ctx context.Context, term string, limit int,
 		remoteName = strings.TrimPrefix(remoteName, "library/")
 		remoteName = strings.TrimPrefix(remoteName, "library/")
 	}
 	}
 
 
-	endpoint, err := newV1Endpoint(index, userAgent, headers)
+	endpoint, err := newV1Endpoint(index, headers)
 	if err != nil {
 	if err != nil {
 		return nil, err
 		return nil, err
 	}
 	}
@@ -116,7 +115,8 @@ func (s *Service) searchUnfiltered(ctx context.Context, term string, limit int,
 			},
 			},
 		}
 		}
 
 
-		modifiers := Headers(userAgent, nil)
+		// 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, scopes)
 		v2Client, err := v2AuthHTTPClient(endpoint.URL, endpoint.client.Transport, modifiers, creds, scopes)
 		if err != nil {
 		if err != nil {
 			return nil, err
 			return nil, err