From 2ecb0ebd7345144e38462cc8190eb57f9aa6145a Mon Sep 17 00:00:00 2001 From: deads2k Date: Fri, 8 May 2015 13:30:59 -0400 Subject: [PATCH] make the dockerkeyring handle mutiple matching credentials --- pkg/credentialprovider/gcp/jwt_test.go | 7 ++- pkg/credentialprovider/gcp/metadata_test.go | 21 +++++++-- pkg/credentialprovider/keyring.go | 46 ++++++++++++------- pkg/credentialprovider/keyring_test.go | 28 +++++++++-- pkg/kubelet/dockertools/docker.go | 43 +++++++++++------ pkg/kubelet/dockertools/docker_test.go | 41 ++++++++--------- pkg/kubelet/dockertools/fake_docker_client.go | 7 ++- pkg/kubelet/dockertools/manager.go | 4 +- pkg/kubelet/rkt/rkt.go | 10 +++- 9 files changed, 142 insertions(+), 65 deletions(-) diff --git a/pkg/credentialprovider/gcp/jwt_test.go b/pkg/credentialprovider/gcp/jwt_test.go index cab575062e..a99a16f762 100644 --- a/pkg/credentialprovider/gcp/jwt_test.go +++ b/pkg/credentialprovider/gcp/jwt_test.go @@ -104,10 +104,15 @@ func TestJwtProvider(t *testing.T) { // Verify that we get the expected username/password combo for // a gcr.io image name. registryUrl := "gcr.io/foo/bar" - val, ok := keyring.Lookup(registryUrl) + creds, ok := keyring.Lookup(registryUrl) if !ok { t.Errorf("Didn't find expected URL: %s", registryUrl) + return } + if len(creds) > 1 { + t.Errorf("Got more hits than expected: %s", creds) + } + val := creds[0] if "_token" != val.Username { t.Errorf("Unexpected username value, want: _token, got: %s", val.Username) diff --git a/pkg/credentialprovider/gcp/metadata_test.go b/pkg/credentialprovider/gcp/metadata_test.go index 3e1b8724d3..5c41315bdc 100644 --- a/pkg/credentialprovider/gcp/metadata_test.go +++ b/pkg/credentialprovider/gcp/metadata_test.go @@ -76,10 +76,15 @@ func TestDockerKeyringFromGoogleDockerConfigMetadata(t *testing.T) { keyring.Add(provider.Provide()) - val, ok := keyring.Lookup(registryUrl) + creds, ok := keyring.Lookup(registryUrl) if !ok { t.Errorf("Didn't find expected URL: %s", registryUrl) + return } + if len(creds) > 1 { + t.Errorf("Got more hits than expected: %s", creds) + } + val := creds[0] if username != val.Username { t.Errorf("Unexpected username value, want: %s, got: %s", username, val.Username) @@ -143,10 +148,15 @@ func TestDockerKeyringFromGoogleDockerConfigMetadataUrl(t *testing.T) { keyring.Add(provider.Provide()) - val, ok := keyring.Lookup(registryUrl) + creds, ok := keyring.Lookup(registryUrl) if !ok { t.Errorf("Didn't find expected URL: %s", registryUrl) + return } + if len(creds) > 1 { + t.Errorf("Got more hits than expected: %s", creds) + } + val := creds[0] if username != val.Username { t.Errorf("Unexpected username value, want: %s, got: %s", username, val.Username) @@ -211,10 +221,15 @@ func TestContainerRegistryBasics(t *testing.T) { keyring.Add(provider.Provide()) - val, ok := keyring.Lookup(registryUrl) + creds, ok := keyring.Lookup(registryUrl) if !ok { t.Errorf("Didn't find expected URL: %s", registryUrl) + return } + if len(creds) > 1 { + t.Errorf("Got more hits than expected: %s", creds) + } + val := creds[0] if "_token" != val.Username { t.Errorf("Unexpected username value, want: %s, got: %s", "_token", val.Username) diff --git a/pkg/credentialprovider/keyring.go b/pkg/credentialprovider/keyring.go index 8bbeda548c..94855b8a71 100644 --- a/pkg/credentialprovider/keyring.go +++ b/pkg/credentialprovider/keyring.go @@ -23,6 +23,8 @@ import ( docker "github.com/fsouza/go-dockerclient" "github.com/golang/glog" + + "github.com/GoogleCloudPlatform/kubernetes/pkg/util" ) // DockerKeyring tracks a set of docker registry credentials, maintaining a @@ -33,13 +35,13 @@ import ( // most specific match for a given image // - iterating a map does not yield predictable results type DockerKeyring interface { - Lookup(image string) (docker.AuthConfiguration, bool) + Lookup(image string) ([]docker.AuthConfiguration, bool) } // BasicDockerKeyring is a trivial map-backed implementation of DockerKeyring type BasicDockerKeyring struct { index []string - creds map[string]docker.AuthConfiguration + creds map[string][]docker.AuthConfiguration } // lazyDockerKeyring is an implementation of DockerKeyring that lazily @@ -51,9 +53,10 @@ type lazyDockerKeyring struct { func (dk *BasicDockerKeyring) Add(cfg DockerConfig) { if dk.index == nil { dk.index = make([]string, 0) - dk.creds = make(map[string]docker.AuthConfiguration) + dk.creds = make(map[string][]docker.AuthConfiguration) } for loc, ident := range cfg { + creds := docker.AuthConfiguration{ Username: ident.Username, Password: ident.Password, @@ -73,15 +76,19 @@ func (dk *BasicDockerKeyring) Add(cfg DockerConfig) { // See ResolveAuthConfig in docker/registry/auth.go. if parsed.Host != "" { // NOTE: foo.bar.com comes through as Path. - dk.creds[parsed.Host] = creds + dk.creds[parsed.Host] = append(dk.creds[parsed.Host], creds) dk.index = append(dk.index, parsed.Host) } - if parsed.Path != "/" { - dk.creds[parsed.Host+parsed.Path] = creds - dk.index = append(dk.index, parsed.Host+parsed.Path) + 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) } } + eliminateDupes := util.NewStringSet(dk.index...) + dk.index = eliminateDupes.List() + // Update the index used to identify which credentials to use for a given // image. The index is reverse-sorted so more specific paths are matched // first. For example, if for the given image "quay.io/coreos/etcd", @@ -109,11 +116,12 @@ func isDefaultRegistryMatch(image string) bool { return !strings.ContainsAny(parts[0], ".:") } -// Lookup implements the DockerKeyring method for fetching credentials -// based on image name. -func (dk *BasicDockerKeyring) Lookup(image string) (docker.AuthConfiguration, bool) { - // range over the index as iterating over a map does not provide - // a predictable ordering +// Lookup implements the DockerKeyring method for fetching credentials based on image name. +// Multiple credentials may be returned if there are multiple potentially valid credentials +// available. This allows for rotation. +func (dk *BasicDockerKeyring) Lookup(image string) ([]docker.AuthConfiguration, bool) { + // range over the index as iterating over a map does not provide a predictable ordering + ret := []docker.AuthConfiguration{} for _, k := range dk.index { // NOTE: prefix is a sufficient check because while scheme is allowed, // it is stripped as part of 'Add' @@ -121,7 +129,11 @@ func (dk *BasicDockerKeyring) Lookup(image string) (docker.AuthConfiguration, bo continue } - return dk.creds[k], true + ret = append(ret, dk.creds[k]...) + } + + if len(ret) > 0 { + return ret, true } // Use credentials for the default registry if provided, and appropriate @@ -129,12 +141,12 @@ func (dk *BasicDockerKeyring) Lookup(image string) (docker.AuthConfiguration, bo return auth, true } - return docker.AuthConfiguration{}, false + return []docker.AuthConfiguration{}, false } // Lookup implements the DockerKeyring method for fetching credentials // based on image name. -func (dk *lazyDockerKeyring) Lookup(image string) (docker.AuthConfiguration, bool) { +func (dk *lazyDockerKeyring) Lookup(image string) ([]docker.AuthConfiguration, bool) { keyring := &BasicDockerKeyring{} for _, p := range dk.Providers { @@ -145,10 +157,10 @@ func (dk *lazyDockerKeyring) Lookup(image string) (docker.AuthConfiguration, boo } type FakeKeyring struct { - auth docker.AuthConfiguration + auth []docker.AuthConfiguration ok bool } -func (f *FakeKeyring) Lookup(image string) (docker.AuthConfiguration, bool) { +func (f *FakeKeyring) Lookup(image string) ([]docker.AuthConfiguration, bool) { return f.auth, f.ok } diff --git a/pkg/credentialprovider/keyring_test.go b/pkg/credentialprovider/keyring_test.go index 52584f7584..77bb78b4d6 100644 --- a/pkg/credentialprovider/keyring_test.go +++ b/pkg/credentialprovider/keyring_test.go @@ -42,10 +42,15 @@ func TestDockerKeyringFromBytes(t *testing.T) { keyring.Add(cfg) } - val, ok := keyring.Lookup(url + "/foo/bar") + creds, ok := keyring.Lookup(url + "/foo/bar") if !ok { t.Errorf("Didn't find expected URL: %s", url) + return } + if len(creds) > 1 { + t.Errorf("Got more hits than expected: %s", creds) + } + val := creds[0] if username != val.Username { t.Errorf("Unexpected username value, want: %s, got: %s", username, val.Username) @@ -130,10 +135,15 @@ func TestKeyringHitWithUnqualifiedDockerHub(t *testing.T) { keyring.Add(cfg) } - val, ok := keyring.Lookup("google/docker-registry") + creds, ok := keyring.Lookup("google/docker-registry") if !ok { t.Errorf("Didn't find expected URL: %s", url) + return } + if len(creds) > 1 { + t.Errorf("Got more hits than expected: %s", creds) + } + val := creds[0] if username != val.Username { t.Errorf("Unexpected username value, want: %s, got: %s", username, val.Username) @@ -166,10 +176,15 @@ func TestKeyringHitWithUnqualifiedLibraryDockerHub(t *testing.T) { keyring.Add(cfg) } - val, ok := keyring.Lookup("jenkins") + creds, ok := keyring.Lookup("jenkins") if !ok { t.Errorf("Didn't find expected URL: %s", url) + return } + if len(creds) > 1 { + t.Errorf("Got more hits than expected: %s", creds) + } + val := creds[0] if username != val.Username { t.Errorf("Unexpected username value, want: %s, got: %s", username, val.Username) @@ -202,10 +217,15 @@ func TestKeyringHitWithQualifiedDockerHub(t *testing.T) { keyring.Add(cfg) } - val, ok := keyring.Lookup(url + "/google/docker-registry") + creds, ok := keyring.Lookup(url + "/google/docker-registry") if !ok { t.Errorf("Didn't find expected URL: %s", url) + return } + if len(creds) > 2 { + t.Errorf("Got more hits than expected: %s", creds) + } + val := creds[0] if username != val.Username { t.Errorf("Unexpected username value, want: %s, got: %s", username, val.Username) diff --git a/pkg/kubelet/dockertools/docker.go b/pkg/kubelet/dockertools/docker.go index 5b54e545aa..5d5e1e7145 100644 --- a/pkg/kubelet/dockertools/docker.go +++ b/pkg/kubelet/dockertools/docker.go @@ -30,6 +30,7 @@ import ( kubeletTypes "github.com/GoogleCloudPlatform/kubernetes/pkg/kubelet/types" "github.com/GoogleCloudPlatform/kubernetes/pkg/types" "github.com/GoogleCloudPlatform/kubernetes/pkg/util" + utilerrors "github.com/GoogleCloudPlatform/kubernetes/pkg/util/errors" "github.com/docker/docker/pkg/parsers" docker "github.com/fsouza/go-dockerclient" "github.com/golang/glog" @@ -124,25 +125,39 @@ func (p dockerPuller) Pull(image string) error { Tag: tag, } - creds, ok := p.keyring.Lookup(repoToPull) - if !ok { + creds, haveCredentials := p.keyring.Lookup(repoToPull) + if !haveCredentials { glog.V(1).Infof("Pulling image %s without credentials", image) - } - err := p.client.PullImage(opts, creds) - // If there was no error, or we had credentials, just return the error. - if err == nil || ok { + err := p.client.PullImage(opts, docker.AuthConfiguration{}) + if err == nil { + return nil + } + + // Image spec: [/]/[:/]/[:= 1 { + creds = credsSlice[0] + } + registry := "index.docker.io" // Image spec: [/]/[: