make the dockerkeyring handle mutiple matching credentials

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)

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)

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,32 +116,37 @@ 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'
 		if !strings.HasPrefix(image, k) {
 			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
 	if auth, ok := dk.creds[defaultRegistryHost]; ok && isDefaultRegistryMatch(image) {
 		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
 }

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)

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: [<registry>/]<repository>/<image>[:<version] so we count '/'
+		explicitRegistry := (strings.Count(image, "/") == 2)
+		// Hack, look for a private registry, and decorate the error with the lack of
+		// credentials.  This is heuristic, and really probably could be done better
+		// by talking to the registry API directly from the kubelet here.
+		if explicitRegistry {
+			return fmt.Errorf("image pull failed for %s, this may be because there are no credentials on this request.  details: (%v)", image, err)
+		}
+
 		return err
 	}
-	// Image spec: [<registry>/]<repository>/<image>[:<version] so we count '/'
-	explicitRegistry := (strings.Count(image, "/") == 2)
-	// Hack, look for a private registry, and decorate the error with the lack of
-	// credentials.  This is heuristic, and really probably could be done better
-	// by talking to the registry API directly from the kubelet here.
-	if explicitRegistry {
-		return fmt.Errorf("image pull failed for %s, this may be because there are no credentials on this request.  details: (%v)", image, err)
+
+	var pullErrs []error
+	for _, currentCreds := range creds {
+		err := p.client.PullImage(opts, currentCreds)
+		// If there was no error, return success
+		if err == nil {
+			return nil
+		}
+
+		pullErrs = append(pullErrs, err)
 	}
-	return err
+
+	return utilerrors.NewAggregate(pullErrs)
 }
 
 func (p throttledDockerPuller) Pull(image string) error {