From 4455f517605f1fd7279bbe9547915f15c037997d Mon Sep 17 00:00:00 2001 From: Tibor Vass Date: Tue, 11 Nov 2014 17:37:44 -0500 Subject: [PATCH 1/3] registry: refactor registry.IsSecure calls into registry.NewEndpoint Signed-off-by: Tibor Vass --- graph/pull.go | 4 +--- graph/push.go | 4 +--- registry/endpoint.go | 16 +++++++++------- registry/endpoint_test.go | 2 +- registry/registry_test.go | 4 ++-- registry/service.go | 6 ++---- 6 files changed, 16 insertions(+), 20 deletions(-) diff --git a/graph/pull.go b/graph/pull.go index a7394a4b92..716a27c909 100644 --- a/graph/pull.go +++ b/graph/pull.go @@ -113,9 +113,7 @@ func (s *TagStore) CmdPull(job *engine.Job) engine.Status { return job.Error(err) } - secure := registry.IsSecure(hostname, s.insecureRegistries) - - endpoint, err := registry.NewEndpoint(hostname, secure) + endpoint, err := registry.NewEndpoint(hostname, s.insecureRegistries) if err != nil { return job.Error(err) } diff --git a/graph/push.go b/graph/push.go index 4cda8914b3..29fc4a066d 100644 --- a/graph/push.go +++ b/graph/push.go @@ -214,9 +214,7 @@ func (s *TagStore) CmdPush(job *engine.Job) engine.Status { return job.Error(err) } - secure := registry.IsSecure(hostname, s.insecureRegistries) - - endpoint, err := registry.NewEndpoint(hostname, secure) + endpoint, err := registry.NewEndpoint(hostname, s.insecureRegistries) if err != nil { return job.Error(err) } diff --git a/registry/endpoint.go b/registry/endpoint.go index 0d0749d7a2..390eec2e6a 100644 --- a/registry/endpoint.go +++ b/registry/endpoint.go @@ -34,12 +34,15 @@ func scanForAPIVersion(hostname string) (string, APIVersion) { return hostname, DefaultAPIVersion } -func NewEndpoint(hostname string, secure bool) (*Endpoint, error) { - endpoint, err := newEndpoint(hostname, secure) +func NewEndpoint(hostname string, insecureRegistries []string) (*Endpoint, error) { + endpoint, err := newEndpoint(hostname) if err != nil { return nil, err } + secure := isSecure(endpoint.URL.Host, insecureRegistries) + endpoint.secure = secure + // Try HTTPS ping to registry endpoint.URL.Scheme = "https" if _, err := endpoint.Ping(); err != nil { @@ -65,9 +68,9 @@ func NewEndpoint(hostname string, secure bool) (*Endpoint, error) { return endpoint, nil } -func newEndpoint(hostname string, secure bool) (*Endpoint, error) { +func newEndpoint(hostname string) (*Endpoint, error) { var ( - endpoint = Endpoint{secure: secure} + endpoint = Endpoint{secure: true} trimmedHostname string err error ) @@ -149,10 +152,9 @@ func (e Endpoint) Ping() (RegistryInfo, error) { return info, nil } -// IsSecure returns false if the provided hostname is part of the list of insecure registries. +// isSecure returns false if the provided hostname is part of the list of insecure registries. // Insecure registries accept HTTP and/or accept HTTPS with certificates from unknown CAs. -func IsSecure(hostname string, insecureRegistries []string) bool { - +func isSecure(hostname string, insecureRegistries []string) bool { if hostname == IndexServerAddress() { return true } diff --git a/registry/endpoint_test.go b/registry/endpoint_test.go index def5e0d7ae..0ec1220d9c 100644 --- a/registry/endpoint_test.go +++ b/registry/endpoint_test.go @@ -12,7 +12,7 @@ func TestEndpointParse(t *testing.T) { {"0.0.0.0:5000", "https://0.0.0.0:5000/v1/"}, } for _, td := range testData { - e, err := newEndpoint(td.str, true) + e, err := newEndpoint(td.str) if err != nil { t.Errorf("%q: %s", td.str, err) } diff --git a/registry/registry_test.go b/registry/registry_test.go index 032c9fbf03..8bc6a35166 100644 --- a/registry/registry_test.go +++ b/registry/registry_test.go @@ -343,8 +343,8 @@ func TestIsSecure(t *testing.T) { {"127.0.0.1:5000", []string{"example.com"}, false}, } for _, tt := range tests { - if sec := IsSecure(tt.addr, tt.insecureRegistries); sec != tt.expected { - t.Errorf("IsSecure failed for %q %v, expected %v got %v", tt.addr, tt.insecureRegistries, tt.expected, sec) + if sec := isSecure(tt.addr, tt.insecureRegistries); sec != tt.expected { + t.Errorf("isSecure failed for %q %v, expected %v got %v", tt.addr, tt.insecureRegistries, tt.expected, sec) } } } diff --git a/registry/service.go b/registry/service.go index 7051d93430..53e8278b04 100644 --- a/registry/service.go +++ b/registry/service.go @@ -40,7 +40,7 @@ func (s *Service) Auth(job *engine.Job) engine.Status { job.GetenvJson("authConfig", authConfig) if addr := authConfig.ServerAddress; addr != "" && addr != IndexServerAddress() { - endpoint, err := NewEndpoint(addr, IsSecure(addr, s.insecureRegistries)) + endpoint, err := NewEndpoint(addr, s.insecureRegistries) if err != nil { return job.Error(err) } @@ -92,9 +92,7 @@ func (s *Service) Search(job *engine.Job) engine.Status { return job.Error(err) } - secure := IsSecure(hostname, s.insecureRegistries) - - endpoint, err := NewEndpoint(hostname, secure) + endpoint, err := NewEndpoint(hostname, s.insecureRegistries) if err != nil { return job.Error(err) } From 78e859f3c35d1f31e7d6f3ded9a414dc0fbb8eaa Mon Sep 17 00:00:00 2001 From: Tibor Vass Date: Tue, 11 Nov 2014 20:08:59 -0500 Subject: [PATCH 2/3] Put mock registry address in insecureRegistries for unit tests Signed-off-by: Tibor Vass --- registry/registry_mock_test.go | 10 ++++++++-- registry/registry_test.go | 4 ++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/registry/registry_mock_test.go b/registry/registry_mock_test.go index 02884c6224..1c710e21e9 100644 --- a/registry/registry_mock_test.go +++ b/registry/registry_mock_test.go @@ -19,8 +19,9 @@ import ( ) var ( - testHTTPServer *httptest.Server - testLayers = map[string]map[string]string{ + testHTTPServer *httptest.Server + insecureRegistries []string + testLayers = map[string]map[string]string{ "77dbf71da1d00e3fbddc480176eac8994025630c6590d11cfc8fe1209c2a1d20": { "json": `{"id":"77dbf71da1d00e3fbddc480176eac8994025630c6590d11cfc8fe1209c2a1d20", "comment":"test base image","created":"2013-03-23T12:53:11.10432-07:00", @@ -100,6 +101,11 @@ func init() { r.HandleFunc("/v2/version", handlerGetPing).Methods("GET") testHTTPServer = httptest.NewServer(handlerAccessLog(r)) + URL, err := url.Parse(testHTTPServer.URL) + if err != nil { + panic(err) + } + insecureRegistries = []string{URL.Host} } func handlerAccessLog(handler http.Handler) http.Handler { diff --git a/registry/registry_test.go b/registry/registry_test.go index 8bc6a35166..37dedc2acc 100644 --- a/registry/registry_test.go +++ b/registry/registry_test.go @@ -21,7 +21,7 @@ const ( func spawnTestRegistrySession(t *testing.T) *Session { authConfig := &AuthConfig{} - endpoint, err := NewEndpoint(makeURL("/v1/"), false) + endpoint, err := NewEndpoint(makeURL("/v1/"), insecureRegistries) if err != nil { t.Fatal(err) } @@ -33,7 +33,7 @@ func spawnTestRegistrySession(t *testing.T) *Session { } func TestPingRegistryEndpoint(t *testing.T) { - ep, err := NewEndpoint(makeURL("/v1/"), false) + ep, err := NewEndpoint(makeURL("/v1/"), insecureRegistries) if err != nil { t.Fatal(err) } From fbe10c83d81843412fd3485a8d6bb75849de97d4 Mon Sep 17 00:00:00 2001 From: Tibor Vass Date: Thu, 13 Nov 2014 06:56:36 -0800 Subject: [PATCH 3/3] registry: parse INDEXSERVERADDRESS into a URL for easier check in isSecure Signed-off-by: Tibor Vass --- registry/auth.go | 10 ++++++++++ registry/endpoint.go | 14 ++++++-------- registry/endpoint_test.go | 2 +- registry/registry_test.go | 1 + 4 files changed, 18 insertions(+), 9 deletions(-) diff --git a/registry/auth.go b/registry/auth.go index 1b11179533..a22d0b881f 100644 --- a/registry/auth.go +++ b/registry/auth.go @@ -7,6 +7,7 @@ import ( "fmt" "io/ioutil" "net/http" + "net/url" "os" "path" "strings" @@ -27,8 +28,17 @@ const ( var ( ErrConfigFileMissing = errors.New("The Auth config file is missing") + IndexServerURL *url.URL ) +func init() { + url, err := url.Parse(INDEXSERVER) + if err != nil { + panic(err) + } + IndexServerURL = url +} + type AuthConfig struct { Username string `json:"username,omitempty"` Password string `json:"password,omitempty"` diff --git a/registry/endpoint.go b/registry/endpoint.go index 390eec2e6a..bd23c30299 100644 --- a/registry/endpoint.go +++ b/registry/endpoint.go @@ -35,21 +35,18 @@ func scanForAPIVersion(hostname string) (string, APIVersion) { } func NewEndpoint(hostname string, insecureRegistries []string) (*Endpoint, error) { - endpoint, err := newEndpoint(hostname) + endpoint, err := newEndpoint(hostname, insecureRegistries) if err != nil { return nil, err } - secure := isSecure(endpoint.URL.Host, insecureRegistries) - endpoint.secure = secure - // Try HTTPS ping to registry endpoint.URL.Scheme = "https" if _, err := endpoint.Ping(); err != nil { //TODO: triggering highland build can be done there without "failing" - if secure { + if endpoint.secure { // If registry is secure and HTTPS failed, show user the error and tell them about `--insecure-registry` // in case that's what they need. DO NOT accept unknown CA certificates, and DO NOT fallback to HTTP. return nil, fmt.Errorf("Invalid registry endpoint %s: %v. If this private registry supports only HTTP or HTTPS with an unknown CA certificate, please add `--insecure-registry %s` to the daemon's arguments. In the case of HTTPS, if you have access to the registry's CA certificate, no need for the flag; simply place the CA certificate at /etc/docker/certs.d/%s/ca.crt", endpoint, err, endpoint.URL.Host, endpoint.URL.Host) @@ -68,9 +65,9 @@ func NewEndpoint(hostname string, insecureRegistries []string) (*Endpoint, error return endpoint, nil } -func newEndpoint(hostname string) (*Endpoint, error) { +func newEndpoint(hostname string, insecureRegistries []string) (*Endpoint, error) { var ( - endpoint = Endpoint{secure: true} + endpoint = Endpoint{} trimmedHostname string err error ) @@ -82,6 +79,7 @@ func newEndpoint(hostname string) (*Endpoint, error) { if err != nil { return nil, err } + endpoint.secure = isSecure(endpoint.URL.Host, insecureRegistries) return &endpoint, nil } @@ -155,7 +153,7 @@ func (e Endpoint) Ping() (RegistryInfo, error) { // isSecure returns false if the provided hostname is part of the list of insecure registries. // Insecure registries accept HTTP and/or accept HTTPS with certificates from unknown CAs. func isSecure(hostname string, insecureRegistries []string) bool { - if hostname == IndexServerAddress() { + if hostname == IndexServerURL.Host { return true } diff --git a/registry/endpoint_test.go b/registry/endpoint_test.go index 0ec1220d9c..54105ec174 100644 --- a/registry/endpoint_test.go +++ b/registry/endpoint_test.go @@ -12,7 +12,7 @@ func TestEndpointParse(t *testing.T) { {"0.0.0.0:5000", "https://0.0.0.0:5000/v1/"}, } for _, td := range testData { - e, err := newEndpoint(td.str) + e, err := newEndpoint(td.str, insecureRegistries) if err != nil { t.Errorf("%q: %s", td.str, err) } diff --git a/registry/registry_test.go b/registry/registry_test.go index 37dedc2acc..3e0950efe0 100644 --- a/registry/registry_test.go +++ b/registry/registry_test.go @@ -326,6 +326,7 @@ func TestIsSecure(t *testing.T) { insecureRegistries []string expected bool }{ + {IndexServerURL.Host, nil, true}, {"example.com", []string{}, true}, {"example.com", []string{"example.com"}, false}, {"localhost", []string{"localhost:5000"}, false},