From 79db131a358f15d4bdef37e251daf27429d116b3 Mon Sep 17 00:00:00 2001 From: Aaron Lehmann Date: Wed, 17 Feb 2016 16:53:25 -0800 Subject: [PATCH] Change APIEndpoint to contain the URL in a parsed format This allows easier URL handling in code that uses APIEndpoint. If we continued to store the URL unparsed, it would require redundant parsing whenver we want to extract information from it. Also, parsing the URL earlier should give improve validation. Signed-off-by: Aaron Lehmann --- distribution/pull.go | 15 +++------- distribution/push.go | 15 +++------- distribution/registry.go | 4 +-- distribution/registry_unit_test.go | 8 ++++- registry/config.go | 4 +-- registry/config_unix.go | 16 ++++++++-- registry/config_windows.go | 13 +++++++-- registry/endpoint.go | 47 +++++++++++++++++++----------- registry/endpoint_test.go | 2 +- registry/registry_test.go | 2 +- registry/service.go | 10 ++----- registry/service_v1.go | 11 +++++-- registry/service_v2.go | 22 +++++++++++--- 13 files changed, 104 insertions(+), 65 deletions(-) diff --git a/distribution/pull.go b/distribution/pull.go index 659675fd62..23d31d7977 100644 --- a/distribution/pull.go +++ b/distribution/pull.go @@ -2,7 +2,6 @@ package distribution import ( "fmt" - "net/url" "github.com/Sirupsen/logrus" "github.com/docker/docker/api" @@ -122,14 +121,8 @@ func Pull(ctx context.Context, ref reference.Named, imagePullConfig *ImagePullCo continue } - parsedURL, urlErr := url.Parse(endpoint.URL) - if urlErr != nil { - logrus.Errorf("Failed to parse endpoint URL %s", endpoint.URL) - continue - } - - if parsedURL.Scheme != "https" { - if _, confirmedTLS := confirmedTLSRegistries[parsedURL.Host]; confirmedTLS { + if endpoint.URL.Scheme != "https" { + if _, confirmedTLS := confirmedTLSRegistries[endpoint.URL.Host]; confirmedTLS { logrus.Debugf("Skipping non-TLS endpoint %s for host/port that appears to use TLS", endpoint.URL) continue } @@ -152,8 +145,8 @@ func Pull(ctx context.Context, ref reference.Named, imagePullConfig *ImagePullCo if fallbackErr, ok := err.(fallbackError); ok { fallback = true confirmedV2 = confirmedV2 || fallbackErr.confirmedV2 - if fallbackErr.transportOK && parsedURL.Scheme == "https" { - confirmedTLSRegistries[parsedURL.Host] = struct{}{} + if fallbackErr.transportOK && endpoint.URL.Scheme == "https" { + confirmedTLSRegistries[endpoint.URL.Host] = struct{}{} } err = fallbackErr.err } diff --git a/distribution/push.go b/distribution/push.go index 9380f5c2ba..1571bdbaa9 100644 --- a/distribution/push.go +++ b/distribution/push.go @@ -5,7 +5,6 @@ import ( "compress/gzip" "fmt" "io" - "net/url" "github.com/Sirupsen/logrus" "github.com/docker/docker/distribution/metadata" @@ -133,14 +132,8 @@ func Push(ctx context.Context, ref reference.Named, imagePushConfig *ImagePushCo continue } - parsedURL, urlErr := url.Parse(endpoint.URL) - if urlErr != nil { - logrus.Errorf("Failed to parse endpoint URL %s", endpoint.URL) - continue - } - - if parsedURL.Scheme != "https" { - if _, confirmedTLS := confirmedTLSRegistries[parsedURL.Host]; confirmedTLS { + if endpoint.URL.Scheme != "https" { + if _, confirmedTLS := confirmedTLSRegistries[endpoint.URL.Host]; confirmedTLS { logrus.Debugf("Skipping non-TLS endpoint %s for host/port that appears to use TLS", endpoint.URL) continue } @@ -161,8 +154,8 @@ func Push(ctx context.Context, ref reference.Named, imagePushConfig *ImagePushCo default: if fallbackErr, ok := err.(fallbackError); ok { confirmedV2 = confirmedV2 || fallbackErr.confirmedV2 - if fallbackErr.transportOK && parsedURL.Scheme == "https" { - confirmedTLSRegistries[parsedURL.Host] = struct{}{} + if fallbackErr.transportOK && endpoint.URL.Scheme == "https" { + confirmedTLSRegistries[endpoint.URL.Host] = struct{}{} } err = fallbackErr.err lastErr = err diff --git a/distribution/registry.go b/distribution/registry.go index f0bfd8c64f..afc9522b79 100644 --- a/distribution/registry.go +++ b/distribution/registry.go @@ -57,7 +57,7 @@ func NewV2Repository(ctx context.Context, repoInfo *registry.RepositoryInfo, end Transport: authTransport, Timeout: 15 * time.Second, } - endpointStr := strings.TrimRight(endpoint.URL, "/") + "/v2/" + endpointStr := strings.TrimRight(endpoint.URL.String(), "/") + "/v2/" req, err := http.NewRequest("GET", endpointStr, nil) if err != nil { return nil, false, fallbackError{err: err} @@ -118,7 +118,7 @@ func NewV2Repository(ctx context.Context, repoInfo *registry.RepositoryInfo, end } } - repo, err = client.NewRepository(ctx, repoNameRef, endpoint.URL, tr) + repo, err = client.NewRepository(ctx, repoNameRef, endpoint.URL.String(), tr) if err != nil { err = fallbackError{ err: err, diff --git a/distribution/registry_unit_test.go b/distribution/registry_unit_test.go index 0702232943..b60a465d78 100644 --- a/distribution/registry_unit_test.go +++ b/distribution/registry_unit_test.go @@ -3,6 +3,7 @@ package distribution import ( "net/http" "net/http/httptest" + "net/url" "os" "strings" "testing" @@ -43,9 +44,14 @@ func testTokenPassThru(t *testing.T, ts *httptest.Server) { } defer os.RemoveAll(tmp) + uri, err := url.Parse(ts.URL) + if err != nil { + t.Fatalf("could not parse url from test server: %v", err) + } + endpoint := registry.APIEndpoint{ Mirror: false, - URL: ts.URL, + URL: uri, Version: 2, Official: false, TrimHostname: false, diff --git a/registry/config.go b/registry/config.go index ec8ec271c9..ebad6f8692 100644 --- a/registry/config.go +++ b/registry/config.go @@ -19,7 +19,7 @@ type Options struct { InsecureRegistries opts.ListOpts } -const ( +var ( // DefaultNamespace is the default namespace DefaultNamespace = "docker.io" // DefaultRegistryVersionHeader is the name of the default HTTP header @@ -27,7 +27,7 @@ const ( DefaultRegistryVersionHeader = "Docker-Distribution-Api-Version" // IndexServer is the v1 registry server used for user auth + account creation - IndexServer = DefaultV1Registry + "/v1/" + IndexServer = DefaultV1Registry.String() + "/v1/" // IndexName is the name of the index IndexName = "docker.io" diff --git a/registry/config_unix.go b/registry/config_unix.go index df970181de..c3c19162f2 100644 --- a/registry/config_unix.go +++ b/registry/config_unix.go @@ -2,12 +2,22 @@ package registry -const ( +import ( + "net/url" +) + +var ( // DefaultV1Registry is the URI of the default v1 registry - DefaultV1Registry = "https://index.docker.io" + DefaultV1Registry = &url.URL{ + Scheme: "https", + Host: "index.docker.io", + } // DefaultV2Registry is the URI of the default v2 registry - DefaultV2Registry = "https://registry-1.docker.io" + DefaultV2Registry = &url.URL{ + Scheme: "https", + Host: "registry-1.docker.io", + } ) var ( diff --git a/registry/config_windows.go b/registry/config_windows.go index d01b2618af..f1ee488b1f 100644 --- a/registry/config_windows.go +++ b/registry/config_windows.go @@ -1,21 +1,28 @@ package registry import ( + "net/url" "os" "path/filepath" "strings" ) -const ( +var ( // DefaultV1Registry is the URI of the default v1 registry - DefaultV1Registry = "https://registry-win-tp3.docker.io" + DefaultV1Registry = &url.URL{ + Scheme: "https", + Host: "registry-win-tp3.docker.io", + } // DefaultV2Registry is the URI of the default (official) v2 registry. // This is the windows-specific endpoint. // // Currently it is a TEMPORARY link that allows Microsoft to continue // development of Docker Engine for Windows. - DefaultV2Registry = "https://registry-win-tp3.docker.io" + DefaultV2Registry = &url.URL{ + Scheme: "https", + Host: "registry-win-tp3.docker.io", + } ) // CertsDir is the directory where certificates are stored diff --git a/registry/endpoint.go b/registry/endpoint.go index ef00431f43..b056caf1e0 100644 --- a/registry/endpoint.go +++ b/registry/endpoint.go @@ -50,10 +50,12 @@ func NewEndpoint(index *registrytypes.IndexInfo, userAgent string, metaHeaders h if err != nil { return nil, err } - endpoint, err := newEndpoint(GetAuthConfigKey(index), tlsConfig, userAgent, metaHeaders) + + endpoint, err := newEndpointFromStr(GetAuthConfigKey(index), tlsConfig, userAgent, metaHeaders) if err != nil { return nil, err } + if v != APIVersionUnknown { endpoint.Version = v } @@ -91,24 +93,14 @@ func validateEndpoint(endpoint *Endpoint) error { return nil } -func newEndpoint(address string, tlsConfig *tls.Config, userAgent string, metaHeaders http.Header) (*Endpoint, error) { - var ( - endpoint = new(Endpoint) - trimmedAddress string - err error - ) - - if !strings.HasPrefix(address, "http") { - address = "https://" + address +func newEndpoint(address url.URL, tlsConfig *tls.Config, userAgent string, metaHeaders http.Header) (*Endpoint, error) { + endpoint := &Endpoint{ + IsSecure: (tlsConfig == nil || !tlsConfig.InsecureSkipVerify), + URL: new(url.URL), + Version: APIVersionUnknown, } - endpoint.IsSecure = (tlsConfig == nil || !tlsConfig.InsecureSkipVerify) - - trimmedAddress, endpoint.Version = scanForAPIVersion(address) - - if endpoint.URL, err = url.Parse(trimmedAddress); err != nil { - return nil, err - } + *endpoint.URL = address // TODO(tiborvass): make sure a ConnectTimeout transport is used tr := NewTransport(tlsConfig) @@ -116,6 +108,27 @@ func newEndpoint(address string, tlsConfig *tls.Config, userAgent string, metaHe return endpoint, nil } +func newEndpointFromStr(address string, tlsConfig *tls.Config, userAgent string, metaHeaders http.Header) (*Endpoint, error) { + if !strings.HasPrefix(address, "http://") && !strings.HasPrefix(address, "https://") { + address = "https://" + address + } + + trimmedAddress, detectedVersion := scanForAPIVersion(address) + + uri, err := url.Parse(trimmedAddress) + if err != nil { + return nil, err + } + + endpoint, err := newEndpoint(*uri, tlsConfig, userAgent, metaHeaders) + if err != nil { + return nil, err + } + + endpoint.Version = detectedVersion + return endpoint, nil +} + // Endpoint stores basic information about a registry endpoint. type Endpoint struct { client *http.Client diff --git a/registry/endpoint_test.go b/registry/endpoint_test.go index 4677e0c9e5..fa18eea010 100644 --- a/registry/endpoint_test.go +++ b/registry/endpoint_test.go @@ -19,7 +19,7 @@ func TestEndpointParse(t *testing.T) { {"0.0.0.0:5000", "https://0.0.0.0:5000/v0/"}, } for _, td := range testData { - e, err := newEndpoint(td.str, nil, "", nil) + e, err := newEndpointFromStr(td.str, nil, "", nil) if err != nil { t.Errorf("%q: %s", td.str, err) } diff --git a/registry/registry_test.go b/registry/registry_test.go index 98a3aa1c8b..33d8534755 100644 --- a/registry/registry_test.go +++ b/registry/registry_test.go @@ -673,7 +673,7 @@ func TestNewIndexInfo(t *testing.T) { func TestMirrorEndpointLookup(t *testing.T) { containsMirror := func(endpoints []APIEndpoint) bool { for _, pe := range endpoints { - if pe.URL == "my.mirror" { + if pe.URL.Host == "my.mirror" { return true } } diff --git a/registry/service.go b/registry/service.go index 861cdb4645..bba1e84234 100644 --- a/registry/service.go +++ b/registry/service.go @@ -121,7 +121,7 @@ func (s *Service) ResolveIndex(name string) (*registrytypes.IndexInfo, error) { // APIEndpoint represents a remote API endpoint type APIEndpoint struct { Mirror bool - URL string + URL *url.URL Version APIVersion Official bool TrimHostname bool @@ -130,7 +130,7 @@ type APIEndpoint struct { // ToV1Endpoint returns a V1 API endpoint based on the APIEndpoint func (e APIEndpoint) ToV1Endpoint(userAgent string, metaHeaders http.Header) (*Endpoint, error) { - return newEndpoint(e.URL, e.TLSConfig, userAgent, metaHeaders) + return newEndpoint(*e.URL, e.TLSConfig, userAgent, metaHeaders) } // TLSConfig constructs a client TLS configuration based on server defaults @@ -138,11 +138,7 @@ func (s *Service) TLSConfig(hostname string) (*tls.Config, error) { return newTLSConfig(hostname, isSecureIndex(s.Config, hostname)) } -func (s *Service) tlsConfigForMirror(mirror string) (*tls.Config, error) { - mirrorURL, err := url.Parse(mirror) - if err != nil { - return nil, err - } +func (s *Service) tlsConfigForMirror(mirrorURL *url.URL) (*tls.Config, error) { return s.TLSConfig(mirrorURL.Host) } diff --git a/registry/service_v1.go b/registry/service_v1.go index 340ce9576a..5328b8f129 100644 --- a/registry/service_v1.go +++ b/registry/service_v1.go @@ -2,6 +2,7 @@ package registry import ( "fmt" + "net/url" "strings" "github.com/docker/docker/reference" @@ -36,7 +37,10 @@ func (s *Service) lookupV1Endpoints(repoName reference.Named) (endpoints []APIEn endpoints = []APIEndpoint{ { - URL: "https://" + hostname, + URL: &url.URL{ + Scheme: "https", + Host: hostname, + }, Version: APIVersion1, TrimHostname: true, TLSConfig: tlsConfig, @@ -45,7 +49,10 @@ func (s *Service) lookupV1Endpoints(repoName reference.Named) (endpoints []APIEn if tlsConfig.InsecureSkipVerify { endpoints = append(endpoints, APIEndpoint{ // or this - URL: "http://" + hostname, + URL: &url.URL{ + Scheme: "http", + Host: hostname, + }, Version: APIVersion1, TrimHostname: true, // used to check if supposed to be secure via InsecureSkipVerify diff --git a/registry/service_v2.go b/registry/service_v2.go index f89326d515..4dbbb9fa94 100644 --- a/registry/service_v2.go +++ b/registry/service_v2.go @@ -2,6 +2,7 @@ package registry import ( "fmt" + "net/url" "strings" "github.com/docker/docker/reference" @@ -15,12 +16,19 @@ func (s *Service) lookupV2Endpoints(repoName reference.Named) (endpoints []APIEn if strings.HasPrefix(nameString, DefaultNamespace+"/") { // v2 mirrors for _, mirror := range s.Config.Mirrors { - mirrorTLSConfig, err := s.tlsConfigForMirror(mirror) + if !strings.HasPrefix(mirror, "http://") && !strings.HasPrefix(mirror, "https://") { + mirror = "https://" + mirror + } + mirrorURL, err := url.Parse(mirror) + if err != nil { + return nil, err + } + mirrorTLSConfig, err := s.tlsConfigForMirror(mirrorURL) if err != nil { return nil, err } endpoints = append(endpoints, APIEndpoint{ - URL: mirror, + URL: mirrorURL, // guess mirrors are v2 Version: APIVersion2, Mirror: true, @@ -53,7 +61,10 @@ func (s *Service) lookupV2Endpoints(repoName reference.Named) (endpoints []APIEn endpoints = []APIEndpoint{ { - URL: "https://" + hostname, + URL: &url.URL{ + Scheme: "https", + Host: hostname, + }, Version: APIVersion2, TrimHostname: true, TLSConfig: tlsConfig, @@ -62,7 +73,10 @@ func (s *Service) lookupV2Endpoints(repoName reference.Named) (endpoints []APIEn if tlsConfig.InsecureSkipVerify { endpoints = append(endpoints, APIEndpoint{ - URL: "http://" + hostname, + URL: &url.URL{ + Scheme: "http", + Host: hostname, + }, Version: APIVersion2, TrimHostname: true, // used to check if supposed to be secure via InsecureSkipVerify