diff --git a/pkg/credentialprovider/keyring.go b/pkg/credentialprovider/keyring.go index 0ea079aee5d..2378156dcd3 100644 --- a/pkg/credentialprovider/keyring.go +++ b/pkg/credentialprovider/keyring.go @@ -67,7 +67,11 @@ func (dk *BasicDockerKeyring) Add(cfg DockerConfig) { Email: ident.Email, } - parsed, err := url.Parse(loc) + value := loc + if !strings.HasPrefix(value, "https://") && !strings.HasPrefix(value, "http://") { + value = "https://" + value + } + parsed, err := url.Parse(value) if err != nil { glog.Errorf("Entry %q in dockercfg invalid (%v), ignoring", loc, err) continue @@ -77,17 +81,20 @@ func (dk *BasicDockerKeyring) Add(cfg DockerConfig) { // foo.bar.com/namespace // Or hostname matches: // foo.bar.com + // It also considers /v2/ and /v1/ equivalent to the hostname // See ResolveAuthConfig in docker/registry/auth.go. - if parsed.Host != "" { - // NOTE: foo.bar.com comes through as Path. - dk.creds[parsed.Host] = append(dk.creds[parsed.Host], creds) - dk.index = append(dk.index, parsed.Host) + effectivePath := parsed.Path + if strings.HasPrefix(effectivePath, "/v2/") || strings.HasPrefix(effectivePath, "/v1/") { + effectivePath = effectivePath[3:] } - if (len(parsed.Path) > 0) && (parsed.Path != "/") { - key := parsed.Host + parsed.Path - dk.creds[key] = append(dk.creds[key], creds) - dk.index = append(dk.index, key) + var key string + if (len(effectivePath) > 0) && (effectivePath != "/") { + key = parsed.Host + effectivePath + } else { + key = parsed.Host } + dk.creds[key] = append(dk.creds[key], creds) + dk.index = append(dk.index, key) } eliminateDupes := sets.NewString(dk.index...) @@ -100,7 +107,10 @@ func (dk *BasicDockerKeyring) Add(cfg DockerConfig) { sort.Sort(sort.Reverse(sort.StringSlice(dk.index))) } -const defaultRegistryHost = "index.docker.io/v1/" +const ( + defaultRegistryHost = "index.docker.io" + defaultRegistryKey = defaultRegistryHost + "/v1/" +) // isDefaultRegistryMatch determines whether the given image will // pull from the default registry (DockerHub) based on the @@ -223,8 +233,10 @@ func (dk *BasicDockerKeyring) Lookup(image string) ([]docker.AuthConfiguration, } // Use credentials for the default registry if provided, and appropriate - if auth, ok := dk.creds[defaultRegistryHost]; ok && isDefaultRegistryMatch(image) { - return auth, true + if isDefaultRegistryMatch(image) { + if auth, ok := dk.creds[defaultRegistryHost]; ok { + return auth, true + } } return []docker.AuthConfiguration{}, false diff --git a/pkg/credentialprovider/keyring_test.go b/pkg/credentialprovider/keyring_test.go index cd92d79b20a..72db024be49 100644 --- a/pkg/credentialprovider/keyring_test.go +++ b/pkg/credentialprovider/keyring_test.go @@ -125,65 +125,77 @@ func TestDockerKeyringForGlob(t *testing.T) { targetUrl string }{ { - globUrl: "hello.kubernetes.io", + globUrl: "https://hello.kubernetes.io", targetUrl: "hello.kubernetes.io", }, { - globUrl: "*.docker.io", + globUrl: "https://*.docker.io", targetUrl: "prefix.docker.io", }, { - globUrl: "prefix.*.io", + globUrl: "https://prefix.*.io", targetUrl: "prefix.docker.io", }, { - globUrl: "prefix.docker.*", + globUrl: "https://prefix.docker.*", targetUrl: "prefix.docker.io", }, { - globUrl: "*.docker.io/path", + globUrl: "https://*.docker.io/path", targetUrl: "prefix.docker.io/path", }, { - globUrl: "prefix.*.io/path", + globUrl: "https://prefix.*.io/path", targetUrl: "prefix.docker.io/path/subpath", }, { - globUrl: "prefix.docker.*/path", + globUrl: "https://prefix.docker.*/path", targetUrl: "prefix.docker.io/path", }, { - globUrl: "*.docker.io:8888", + globUrl: "https://*.docker.io:8888", targetUrl: "prefix.docker.io:8888", }, { - globUrl: "prefix.*.io:8888", + globUrl: "https://prefix.*.io:8888", targetUrl: "prefix.docker.io:8888", }, { - globUrl: "prefix.docker.*:8888", + globUrl: "https://prefix.docker.*:8888", targetUrl: "prefix.docker.io:8888", }, { - globUrl: "*.docker.io/path:1111", + globUrl: "https://*.docker.io/path:1111", targetUrl: "prefix.docker.io/path:1111", }, { - globUrl: "prefix.*.io/path:1111", - targetUrl: "prefix.docker.io/path/subpath:1111", + globUrl: "https://*.docker.io/v1/", + targetUrl: "prefix.docker.io/path:1111", }, { - globUrl: "prefix.docker.*/path:1111", + globUrl: "https://*.docker.io/v2/", targetUrl: "prefix.docker.io/path:1111", }, + { + globUrl: "https://prefix.docker.*/path:1111", + targetUrl: "prefix.docker.io/path:1111", + }, + { + globUrl: "prefix.docker.io:1111", + targetUrl: "prefix.docker.io:1111/path", + }, + { + globUrl: "*.docker.io:1111", + targetUrl: "prefix.docker.io:1111/path", + }, } - for _, test := range tests { + for i, test := range tests { email := "foo@bar.baz" username := "foo" password := "bar" auth := base64.StdEncoding.EncodeToString([]byte(fmt.Sprintf("%s:%s", username, password))) sampleDockerConfig := fmt.Sprintf(`{ - "https://%s": { + "%s": { "email": %q, "auth": %q } @@ -198,8 +210,8 @@ func TestDockerKeyringForGlob(t *testing.T) { creds, ok := keyring.Lookup(test.targetUrl + "/foo/bar") if !ok { - t.Errorf("Didn't find expected URL: %s", test.targetUrl) - return + t.Errorf("%d: Didn't find expected URL: %s", i, test.targetUrl) + continue } val := creds[0] @@ -221,21 +233,29 @@ func TestKeyringMiss(t *testing.T) { lookupUrl string }{ { - globUrl: "hello.kubernetes.io", + globUrl: "https://hello.kubernetes.io", lookupUrl: "world.mesos.org/foo/bar", }, { - globUrl: "*.docker.com", + globUrl: "https://*.docker.com", lookupUrl: "prefix.docker.io", }, + { + globUrl: "https://suffix.*.io", + lookupUrl: "prefix.docker.io", + }, + { + globUrl: "https://prefix.docker.c*", + lookupUrl: "prefix.docker.io", + }, + { + globUrl: "https://prefix.*.io/path:1111", + lookupUrl: "prefix.docker.io/path/subpath:1111", + }, { globUrl: "suffix.*.io", lookupUrl: "prefix.docker.io", }, - { - globUrl: "prefix.docker.c*", - lookupUrl: "prefix.docker.io", - }, } for _, test := range tests { email := "foo@bar.baz" @@ -243,7 +263,7 @@ func TestKeyringMiss(t *testing.T) { password := "bar" auth := base64.StdEncoding.EncodeToString([]byte(fmt.Sprintf("%s:%s", username, password))) sampleDockerConfig := fmt.Sprintf(`{ - "https://%s": { + "%s": { "email": %q, "auth": %q } @@ -265,7 +285,7 @@ func TestKeyringMiss(t *testing.T) { } func TestKeyringMissWithDockerHubCredentials(t *testing.T) { - url := defaultRegistryHost + url := defaultRegistryKey email := "foo@bar.baz" username := "foo" password := "bar" @@ -291,7 +311,7 @@ func TestKeyringMissWithDockerHubCredentials(t *testing.T) { } func TestKeyringHitWithUnqualifiedDockerHub(t *testing.T) { - url := defaultRegistryHost + url := defaultRegistryKey email := "foo@bar.baz" username := "foo" password := "bar" @@ -332,7 +352,7 @@ func TestKeyringHitWithUnqualifiedDockerHub(t *testing.T) { } func TestKeyringHitWithUnqualifiedLibraryDockerHub(t *testing.T) { - url := defaultRegistryHost + url := defaultRegistryKey email := "foo@bar.baz" username := "foo" password := "bar" @@ -373,7 +393,7 @@ func TestKeyringHitWithUnqualifiedLibraryDockerHub(t *testing.T) { } func TestKeyringHitWithQualifiedDockerHub(t *testing.T) { - url := defaultRegistryHost + url := defaultRegistryKey email := "foo@bar.baz" username := "foo" password := "bar" diff --git a/pkg/kubelet/dockertools/docker_test.go b/pkg/kubelet/dockertools/docker_test.go index bcfef0d5bf4..2edbd846add 100644 --- a/pkg/kubelet/dockertools/docker_test.go +++ b/pkg/kubelet/dockertools/docker_test.go @@ -354,7 +354,7 @@ func TestPullWithSecrets(t *testing.T) { []string{`ubuntu:latest using {"username":"passed-user","password":"passed-password","email":"passed-email"}`}, }, } - for _, test := range tests { + for i, test := range tests { builtInKeyRing := &credentialprovider.BasicDockerKeyring{} builtInKeyRing.Add(test.builtInDockerConfig) @@ -367,17 +367,17 @@ func TestPullWithSecrets(t *testing.T) { err := dp.Pull(test.imageName, test.passedSecrets) if err != nil { - t.Errorf("unexpected non-nil err: %s", err) + t.Errorf("%s: unexpected non-nil err: %s", i, err) continue } if e, a := 1, len(fakeClient.pulled); e != a { - t.Errorf("%s: expected 1 pulled image, got %d: %v", test.imageName, a, fakeClient.pulled) + t.Errorf("%s: expected 1 pulled image, got %d: %v", i, a, fakeClient.pulled) continue } if e, a := test.expectedPulls, fakeClient.pulled; !reflect.DeepEqual(e, a) { - t.Errorf("%s: expected pull of %v, but got %v", test.imageName, e, a) + t.Errorf("%s: expected pull of %v, but got %v", i, e, a) } } }