From d9a1c3f9e474646b13abf6929c75ea6701eb5a02 Mon Sep 17 00:00:00 2001 From: Wei Fu Date: Sat, 4 Apr 2020 14:09:35 +0800 Subject: [PATCH 1/2] bugfix: add default host config if not set If there is not specific host config, like ctr does, the resolver will fail to get host path. And this patch is to add default host config if needs. And default config host config should have all caps for pull and push. Signed-off-by: Wei Fu --- remotes/docker/config/hosts.go | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/remotes/docker/config/hosts.go b/remotes/docker/config/hosts.go index b9736cafb..f93fd1b02 100644 --- a/remotes/docker/config/hosts.go +++ b/remotes/docker/config/hosts.go @@ -88,21 +88,20 @@ func ConfigureHosts(ctx context.Context, options HostOptions) docker.RegistryHos if hosts == nil { hosts = make([]hostConfig, 1) } - if len(hosts) > 1 { - if hosts[len(hosts)-1].host == "" { - if host == "docker.io" { - hosts[len(hosts)-1].scheme = "https" - hosts[len(hosts)-1].host = "https://registry-1.docker.io" + if len(hosts) >= 1 && hosts[len(hosts)-1].host == "" { + if host == "docker.io" { + hosts[len(hosts)-1].scheme = "https" + hosts[len(hosts)-1].host = "registry-1.docker.io" + } else { + hosts[len(hosts)-1].host = host + if options.DefaultScheme != "" { + hosts[len(hosts)-1].scheme = options.DefaultScheme } else { - hosts[len(hosts)-1].host = host - if options.DefaultScheme != "" { - hosts[len(hosts)-1].scheme = options.DefaultScheme - } else { - hosts[len(hosts)-1].scheme = "https" - } + hosts[len(hosts)-1].scheme = "https" } - hosts[len(hosts)-1].path = "/v2" } + hosts[len(hosts)-1].path = "/v2" + hosts[len(hosts)-1].capabilities = docker.HostCapabilityPull | docker.HostCapabilityResolve | docker.HostCapabilityPush } defaultTransport := &http.Transport{ From 067aba732e2250575ff25ef662950a27d235ce68 Mon Sep 17 00:00:00 2001 From: Derek McGowan Date: Mon, 6 Apr 2020 14:36:15 -0700 Subject: [PATCH 2/2] Add test for default setup for host configuration Signed-off-by: Derek McGowan --- remotes/docker/config/hosts.go | 3 +- remotes/docker/config/hosts_test.go | 64 ++++++++++++++++++++++++++++- 2 files changed, 64 insertions(+), 3 deletions(-) diff --git a/remotes/docker/config/hosts.go b/remotes/docker/config/hosts.go index f93fd1b02..8268e2267 100644 --- a/remotes/docker/config/hosts.go +++ b/remotes/docker/config/hosts.go @@ -74,6 +74,7 @@ func ConfigureHosts(ctx context.Context, options HostOptions) docker.RegistryHos return nil, err } if dir != "" { + log.G(ctx).WithField("dir", dir).Debug("loading host directory") hosts, err = loadHostDir(ctx, dir) if err != nil { return nil, err @@ -88,7 +89,7 @@ func ConfigureHosts(ctx context.Context, options HostOptions) docker.RegistryHos if hosts == nil { hosts = make([]hostConfig, 1) } - if len(hosts) >= 1 && hosts[len(hosts)-1].host == "" { + if len(hosts) > 0 && hosts[len(hosts)-1].host == "" { if host == "docker.io" { hosts[len(hosts)-1].scheme = "https" hosts[len(hosts)-1].host = "registry-1.docker.io" diff --git a/remotes/docker/config/hosts_test.go b/remotes/docker/config/hosts_test.go index 936f25246..35cab475a 100644 --- a/remotes/docker/config/hosts_test.go +++ b/remotes/docker/config/hosts_test.go @@ -23,11 +23,55 @@ import ( "path/filepath" "testing" + "github.com/containerd/containerd/log/logtest" "github.com/containerd/containerd/remotes/docker" ) +const allCaps = docker.HostCapabilityPull | docker.HostCapabilityResolve | docker.HostCapabilityPush + +func TestDefaultHosts(t *testing.T) { + ctx := logtest.WithT(context.Background(), t) + resolve := ConfigureHosts(ctx, HostOptions{}) + + for _, tc := range []struct { + host string + expected []docker.RegistryHost + }{ + { + host: "docker.io", + expected: []docker.RegistryHost{ + { + Scheme: "https", + Host: "registry-1.docker.io", + Path: "/v2", + Capabilities: allCaps, + }, + }, + }, + } { + hosts, err := resolve(tc.host) + if err != nil { + t.Errorf("[%s] resolve failed: %v", tc.host, err) + continue + } + if len(hosts) != len(tc.expected) { + t.Errorf("[%s] unexpected number of hosts %d, expected %d", tc.host, len(hosts), len(tc.expected)) + continue + } + for j := range hosts { + if !compareRegistryHost(hosts[j], tc.expected[j]) { + + t.Errorf("[%s] [%d] unexpected host %v, expected %v", tc.host, j, hosts[j], tc.expected[j]) + break + } + + } + + } +} + func TestParseHostFile(t *testing.T) { - ctx := context.Background() + ctx := logtest.WithT(context.Background(), t) const testtoml = ` server = "https://test-default.registry" @@ -56,7 +100,6 @@ ca = "/etc/path/default" client = ["/etc/certs/client-1.pem", "/etc/certs/client-2.pem"] ` var tb, fb = true, false - var allCaps = docker.HostCapabilityPull | docker.HostCapabilityResolve | docker.HostCapabilityPush expected := []hostConfig{ { scheme: "https", @@ -142,6 +185,23 @@ ca = "/etc/path/default" } } +func compareRegistryHost(j, k docker.RegistryHost) bool { + if j.Scheme != k.Scheme { + return false + } + if j.Host != k.Host { + return false + } + if j.Path != k.Path { + return false + } + if j.Capabilities != k.Capabilities { + return false + } + // Not comparing TLS configs or authorizations + return true +} + func compareHostConfig(j, k hostConfig) bool { if j.scheme != k.scheme { return false