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 <aaron.lehmann@docker.com>

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
 				}

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

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,

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,

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"
 

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 (

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

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,28 +93,39 @@ 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
-	)
+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,
+	}
 
-	if !strings.HasPrefix(address, "http") {
+	*endpoint.URL = address
+
+	// TODO(tiborvass): make sure a ConnectTimeout transport is used
+	tr := NewTransport(tlsConfig)
+	endpoint.client = HTTPClient(transport.NewTransport(tr, DockerHeaders(userAgent, metaHeaders)...))
+	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
 	}
 
-	endpoint.IsSecure = (tlsConfig == nil || !tlsConfig.InsecureSkipVerify)
+	trimmedAddress, detectedVersion := scanForAPIVersion(address)
 
-	trimmedAddress, endpoint.Version = scanForAPIVersion(address)
+	uri, err := url.Parse(trimmedAddress)
+	if err != nil {
+		return nil, err
+	}
 
-	if endpoint.URL, err = url.Parse(trimmedAddress); err != nil {
+	endpoint, err := newEndpoint(*uri, tlsConfig, userAgent, metaHeaders)
+	if err != nil {
 		return nil, err
 	}
 
-	// TODO(tiborvass): make sure a ConnectTimeout transport is used
-	tr := NewTransport(tlsConfig)
-	endpoint.client = HTTPClient(transport.NewTransport(tr, DockerHeaders(userAgent, metaHeaders)...))
+	endpoint.Version = detectedVersion
 	return endpoint, nil
 }
 

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

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

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

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

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