Cleanup legacy mirror string to registry host

Move the conversion to its own function and add unit tests. Signed-off-by: Derek McGowan <derek@mcg.dev>
moby · Apr 27, 2024 · 9d15d56 · 9d15d56
1 parent de8337d
commit 9d15d56
Show file tree

Hide file tree

Showing 2 changed files with 169 additions and 33 deletions.
diff --git a/daemon/hosts.go b/daemon/hosts.go
@@ -50,29 +50,7 @@ func (daemon *Daemon) mergeLegacyConfig(host string, hosts []docker.RegistryHost
 	}
 	sc := daemon.registryService.ServiceConfig()
 	if host == "docker.io" && len(sc.Mirrors) > 0 {
-		var mirrorHosts []docker.RegistryHost
-		for _, mirror := range sc.Mirrors {
-			h := hosts[0]
-			h.Capabilities = docker.HostCapabilityPull | docker.HostCapabilityResolve
-
-			u, err := url.Parse(mirror)
-			if err != nil || u.Host == "" {
-				u, err = url.Parse(fmt.Sprintf("//%s", mirror))
-			}
-			if err == nil && u.Host != "" {
-				h.Host = u.Host
-				h.Path = strings.TrimRight(u.Path, "/")
-				if !strings.HasSuffix(h.Path, defaultPath) {
-					h.Path = path.Join(defaultPath, h.Path)
-				}
-			} else {
-				h.Host = mirror
-				h.Path = defaultPath
-			}
-
-			mirrorHosts = append(mirrorHosts, h)
-		}
-		hosts = append(mirrorHosts, hosts[0])
+		hosts = mirrorsToRegistryHosts(sc.Mirrors, hosts[0])
 	}
 	hostDir := hostconfig.HostDirFromRoot(registry.CertsDir())
 	for i := range hosts {
@@ -93,20 +71,50 @@ func (daemon *Daemon) mergeLegacyConfig(host string, hosts []docker.RegistryHost
 			}
 		}
 		if daemon.registryService.IsInsecureRegistry(hosts[i].Host) {
-			if t.TLSClientConfig != nil {
-				t.TLSClientConfig.InsecureSkipVerify = true
-			} else {
-				t.TLSClientConfig = &tls.Config{
-					InsecureSkipVerify: true,
-				}
+			if t.TLSClientConfig == nil {
+				t.TLSClientConfig = &tls.Config{}
 			}
+			t.TLSClientConfig.InsecureSkipVerify = true
 
 			hosts[i].Client.Transport = docker.NewHTTPFallback(hosts[i].Client.Transport)
 		}
 	}
 	return hosts, nil
 }
 
+func mirrorsToRegistryHosts(mirrors []string, dHost docker.RegistryHost) []docker.RegistryHost {
+	var mirrorHosts []docker.RegistryHost
+	for _, mirror := range mirrors {
+		h := dHost
+		h.Capabilities = docker.HostCapabilityPull | docker.HostCapabilityResolve
+
+		u, err := url.Parse(mirror)
+		if err != nil || u.Host == "" {
+			u, err = url.Parse(fmt.Sprintf("dummy://%s", mirror))
+		}
+		if err == nil && u.Host != "" {
+			h.Host = u.Host
+			h.Path = strings.TrimSuffix(u.Path, "/")
+
+			// For compatibility with legacy mirrors, ensure ends with /v2
+			// NOTE: Use newer configuration to completely override the path
+			if !strings.HasSuffix(h.Path, defaultPath) {
+				h.Path = path.Join(h.Path, defaultPath)
+			}
+			if u.Scheme != "dummy" {
+				h.Scheme = u.Scheme
+			}
+		} else {
+			h.Host = mirror
+			h.Path = defaultPath
+		}
+
+		mirrorHosts = append(mirrorHosts, h)
+	}
+	return append(mirrorHosts, dHost)
+
+}
+
 func loadTLSConfig(d string) (*tls.Config, error) {
 	fs, err := os.ReadDir(d)
 	if err != nil && !errors.Is(err, os.ErrNotExist) && !errors.Is(err, os.ErrPermission) {
@@ -121,10 +129,10 @@ func loadTLSConfig(d string) (*tls.Config, error) {
 		keyPairs []keyPair
 	)
 	for _, f := range fs {
-		if strings.HasSuffix(f.Name(), ".crt") {
+		switch filepath.Ext(f.Name()) {
+		case ".crt":
 			rootCAs = append(rootCAs, filepath.Join(d, f.Name()))
-		}
-		if strings.HasSuffix(f.Name(), ".cert") {
+		case ".cert":
 			keyPairs = append(keyPairs, keyPair{
 				Certificate: filepath.Join(d, f.Name()),
 				Key:         filepath.Join(d, strings.TrimSuffix(f.Name(), ".cert")+".key"),
@@ -150,7 +158,7 @@ func loadTLSConfig(d string) (*tls.Config, error) {
 	for _, p := range rootCAs {
 		dt, err := os.ReadFile(p)
 		if err != nil {
-			return nil, errors.Wrapf(err, "failed to read %s", p)
+			return nil, err
 		}
 		tc.RootCAs.AppendCertsFromPEM(dt)
 	}

diff --git a/daemon/hosts_test.go b/daemon/hosts_test.go
@@ -0,0 +1,128 @@
+package daemon // import "github.com/docker/docker/daemon"
+
+import (
+	"testing"
+
+	"github.com/containerd/containerd/remotes/docker"
+)
+
+func TestMirrorsToHosts(t *testing.T) {
+	pullCaps := docker.HostCapabilityPull | docker.HostCapabilityResolve
+	allCaps := docker.HostCapabilityPull | docker.HostCapabilityResolve | docker.HostCapabilityPush
+	defaultRegistry := testRegistryHost("https", "registry-1.docker.com", "/v2", allCaps)
+	for ti, tc := range []struct {
+		mirrors  []string
+		dhost    docker.RegistryHost
+		expected []docker.RegistryHost
+	}{
+		{
+			mirrors: []string{"https://localhost:5000"},
+			dhost:   defaultRegistry,
+			expected: []docker.RegistryHost{
+				testRegistryHost("https", "localhost:5000", "/v2", pullCaps),
+				defaultRegistry,
+			},
+		},
+		{
+			mirrors: []string{"http://localhost:5000"},
+			dhost:   defaultRegistry,
+			expected: []docker.RegistryHost{
+				testRegistryHost("http", "localhost:5000", "/v2", pullCaps),
+				defaultRegistry,
+			},
+		},
+		{
+			mirrors: []string{"http://localhost:5000/v2"},
+			dhost:   defaultRegistry,
+			expected: []docker.RegistryHost{
+				testRegistryHost("http", "localhost:5000", "/v2", pullCaps),
+				defaultRegistry,
+			},
+		},
+		{
+			mirrors: []string{"localhost:5000"},
+			dhost:   defaultRegistry,
+			expected: []docker.RegistryHost{
+				testRegistryHost("https", "localhost:5000", "/v2", pullCaps),
+				defaultRegistry,
+			},
+		},
+		{
+			mirrors: []string{"localhost:5000/trailingslash/"},
+			dhost:   defaultRegistry,
+			expected: []docker.RegistryHost{
+				testRegistryHost("https", "localhost:5000", "/trailingslash/v2", pullCaps),
+				defaultRegistry,
+			},
+		},
+		{
+			mirrors: []string{"localhost:5000/2trailingslash//"},
+			dhost:   defaultRegistry,
+			expected: []docker.RegistryHost{
+				testRegistryHost("https", "localhost:5000", "/2trailingslash/v2", pullCaps),
+				defaultRegistry,
+			},
+		},
+		{
+			mirrors: []string{"localhost:5000/v2/"},
+			dhost:   defaultRegistry,
+			expected: []docker.RegistryHost{
+				testRegistryHost("https", "localhost:5000", "/v2", pullCaps),
+				defaultRegistry,
+			},
+		},
+		{
+			mirrors: []string{"localhost:5000/base"},
+			dhost:   defaultRegistry,
+			expected: []docker.RegistryHost{
+				testRegistryHost("https", "localhost:5000", "/base/v2", pullCaps),
+				defaultRegistry,
+			},
+		},
+		{
+			// Legacy mirror configuration always appended /v2, keep functionality the same
+			mirrors: []string{"localhost:5000/v2/base"},
+			dhost:   defaultRegistry,
+			expected: []docker.RegistryHost{
+				testRegistryHost("https", "localhost:5000", "/v2/base/v2", pullCaps),
+				defaultRegistry,
+			},
+		},
+	} {
+		actual := mirrorsToRegistryHosts(tc.mirrors, tc.dhost)
+
+		if len(actual) != len(tc.expected) {
+			t.Fatalf("[%d] Did not match hosts\nExpected:\n\t%v\nActual:\n\t%v", ti, tc.expected, actual)
+		}
+		for i := range actual {
+			if !isEqualHosts(actual[i], tc.expected[i]) {
+				t.Errorf("[%d] Host at %d did not match\nExpected:\n\t%v\nActual:\n\t%v", ti, i, tc.expected[i], actual[i])
+			}
+		}
+	}
+}
+
+func testRegistryHost(scheme, host, path string, caps docker.HostCapabilities) docker.RegistryHost {
+	return docker.RegistryHost{
+		Host:         host,
+		Scheme:       scheme,
+		Path:         path,
+		Capabilities: caps,
+	}
+}
+
+func isEqualHosts(a, b docker.RegistryHost) bool {
+	if a.Host != b.Host {
+		return false
+	}
+	if a.Scheme != b.Scheme {
+		return false
+	}
+	if a.Path != b.Path {
+		return false
+	}
+	if a.Capabilities != b.Capabilities {
+		return false
+	}
+	return true
+}