Allow lazy binding in credential providers; don't use it in AWS yet

This is step one for cross-region ECR support and has no visible effects yet.
I'm not crazy about the name LazyProvide. Perhaps the interface method could
remain like that and the package method of the same name could become
LateBind(). I still don't understand why the credential provider has a
DockerConfigEntry that has the same fields but is distinct from
docker.AuthConfiguration. I had to write a converter now that we do that in
more than one place.

In step two, I'll add another intermediate, lazy provider for each AWS region,
whose empty LazyAuthConfiguration will have a refresh time of months or years.
Behind the scenes, it'll use an actual ecrProvider with the usual ~12 hour
credentials, that will get created (and later refreshed) only when kubelet is
attempting to pull an image. If we simply turned ecrProvider directly into a
lazy provider, we would bypass all the caching and get new credentials for
each image pulled.

pkg/credentialprovider/aws/aws_credentials.go

@@ -119,6 +119,11 @@ func (p *ecrProvider) Enabled() bool {
 	return true
 }
 
+// LazyProvide implements DockerConfigProvider.LazyProvide. Should never be called.
+func (p *ecrProvider) LazyProvide() *credentialprovider.DockerConfigEntry {
+	return nil
+}
+
 // Provide implements DockerConfigProvider.Provide, refreshing ECR tokens on demand
 func (p *ecrProvider) Provide() credentialprovider.DockerConfig {
 	cfg := credentialprovider.DockerConfig{}

pkg/credentialprovider/config.go

@@ -46,6 +46,7 @@ type DockerConfigEntry struct {
 	Username string
 	Password string
 	Email    string
+	Provider DockerConfigProvider
 }
 
 var (

pkg/credentialprovider/gcp/jwt.go

@@ -82,6 +82,11 @@ func (j *jwtProvider) Enabled() bool {
 	return true
 }
 
+// LazyProvide implements DockerConfigProvider. Should never be called.
+func (j *jwtProvider) LazyProvide() *credentialprovider.DockerConfigEntry {
+	return nil
+}
+
 // Provide implements DockerConfigProvider
 func (j *jwtProvider) Provide() credentialprovider.DockerConfig {
 	cfg := credentialprovider.DockerConfig{}

pkg/credentialprovider/gcp/metadata.go

@@ -104,6 +104,11 @@ func (g *metadataProvider) Enabled() bool {
 	return err == nil
 }
 
+// LazyProvide implements DockerConfigProvider. Should never be called.
+func (g *dockerConfigKeyProvider) LazyProvide() *credentialprovider.DockerConfigEntry {
+	return nil
+}
+
 // Provide implements DockerConfigProvider
 func (g *dockerConfigKeyProvider) Provide() credentialprovider.DockerConfig {
 	// Read the contents of the google-dockercfg metadata key and
@@ -117,6 +122,11 @@ func (g *dockerConfigKeyProvider) Provide() credentialprovider.DockerConfig {
 	return credentialprovider.DockerConfig{}
 }
 
+// LazyProvide implements DockerConfigProvider. Should never be called.
+func (g *dockerConfigUrlKeyProvider) LazyProvide() *credentialprovider.DockerConfigEntry {
+	return nil
+}
+
 // Provide implements DockerConfigProvider
 func (g *dockerConfigUrlKeyProvider) Provide() credentialprovider.DockerConfig {
 	// Read the contents of the google-dockercfg-url key and load a .dockercfg from there
@@ -166,6 +176,11 @@ type tokenBlob struct {
 	AccessToken string `json:"access_token"`
 }
 
+// LazyProvide implements DockerConfigProvider. Should never be called.
+func (g *containerRegistryProvider) LazyProvide() *credentialprovider.DockerConfigEntry {
+	return nil
+}
+
 // Provide implements DockerConfigProvider
 func (g *containerRegistryProvider) Provide() credentialprovider.DockerConfig {
 	cfg := credentialprovider.DockerConfig{}

pkg/credentialprovider/keyring.go

@@ -39,13 +39,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) ([]LazyAuthConfiguration, bool)
 }
 
 // BasicDockerKeyring is a trivial map-backed implementation of DockerKeyring
 type BasicDockerKeyring struct {
 	index []string
-	creds map[string][]docker.AuthConfiguration
+	creds map[string][]LazyAuthConfiguration
 }
 
 // lazyDockerKeyring is an implementation of DockerKeyring that lazily
@@ -54,17 +54,38 @@ type lazyDockerKeyring struct {
 	Providers []DockerConfigProvider
 }
 
-func (dk *BasicDockerKeyring) Add(cfg DockerConfig) {
+// LazyAuthConfiguration wraps AuthConfiguration, potentially deferring its
-	if dk.index == nil {
+// binding. If Provider is non-nil, it will be used to obtain new credentials
-		dk.index = make([]string, 0)
+// by calling LazyProvide() on it.
-		dk.creds = make(map[string][]docker.AuthConfiguration)
+type LazyAuthConfiguration struct {
-	}
+	docker.AuthConfiguration
-	for loc, ident := range cfg {
+	Provider DockerConfigProvider
+}
 
-		creds := docker.AuthConfiguration{
+func DockerConfigEntryToLazyAuthConfiguration(ident DockerConfigEntry) LazyAuthConfiguration {
+	return LazyAuthConfiguration{
+		AuthConfiguration: docker.AuthConfiguration{
 			Username: ident.Username,
 			Password: ident.Password,
 			Email:    ident.Email,
+		},
+	}
+}
+
+func (dk *BasicDockerKeyring) Add(cfg DockerConfig) {
+	if dk.index == nil {
+		dk.index = make([]string, 0)
+		dk.creds = make(map[string][]LazyAuthConfiguration)
+	}
+	for loc, ident := range cfg {
+
+		var creds LazyAuthConfiguration
+		if ident.Provider != nil {
+			creds = LazyAuthConfiguration{
+				Provider: ident.Provider,
+			}
+		} else {
+			creds = DockerConfigEntryToLazyAuthConfiguration(ident)
 		}
 
 		value := loc
@@ -215,9 +236,9 @@ func urlsMatch(globUrl *url.URL, targetUrl *url.URL) (bool, error) {
 // 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) {
+func (dk *BasicDockerKeyring) Lookup(image string) ([]LazyAuthConfiguration, bool) {
 	// range over the index as iterating over a map does not provide a predictable ordering
-	ret := []docker.AuthConfiguration{}
+	ret := []LazyAuthConfiguration{}
 	for _, k := range dk.index {
 		// both k and image are schemeless URLs because even though schemes are allowed
 		// in the credential configurations, we remove them in Add.
@@ -239,12 +260,12 @@ func (dk *BasicDockerKeyring) Lookup(image string) ([]docker.AuthConfiguration,
 		}
 	}
 
-	return []docker.AuthConfiguration{}, false
+	return []LazyAuthConfiguration{}, 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) ([]LazyAuthConfiguration, bool) {
 	keyring := &BasicDockerKeyring{}
 
 	for _, p := range dk.Providers {
@@ -255,11 +276,11 @@ func (dk *lazyDockerKeyring) Lookup(image string) ([]docker.AuthConfiguration, b
 }
 
 type FakeKeyring struct {
-	auth []docker.AuthConfiguration
+	auth []LazyAuthConfiguration
 	ok   bool
 }
 
-func (f *FakeKeyring) Lookup(image string) ([]docker.AuthConfiguration, bool) {
+func (f *FakeKeyring) Lookup(image string) ([]LazyAuthConfiguration, bool) {
 	return f.auth, f.ok
 }
 
@@ -268,8 +289,8 @@ type unionDockerKeyring struct {
 	keyrings []DockerKeyring
 }
 
-func (k *unionDockerKeyring) Lookup(image string) ([]docker.AuthConfiguration, bool) {
+func (k *unionDockerKeyring) Lookup(image string) ([]LazyAuthConfiguration, bool) {
-	authConfigs := []docker.AuthConfiguration{}
+	authConfigs := []LazyAuthConfiguration{}
 
 	for _, subKeyring := range k.keyrings {
 		if subKeyring == nil {

pkg/credentialprovider/keyring_test.go

@@ -462,6 +462,11 @@ func (d *testProvider) Enabled() bool {
 	return true
 }
 
+// LazyProvide implements dockerConfigProvider. Should never be called.
+func (d *testProvider) LazyProvide() *DockerConfigEntry {
+	return nil
+}
+
 // Provide implements dockerConfigProvider
 func (d *testProvider) Provide() DockerConfig {
 	d.Count += 1

pkg/credentialprovider/provider.go

@@ -22,6 +22,7 @@ import (
 	"sync"
 	"time"
 
+	docker "github.com/fsouza/go-dockerclient"
 	"github.com/golang/glog"
 )
 
@@ -30,6 +31,19 @@ import (
 type DockerConfigProvider interface {
 	Enabled() bool
 	Provide() DockerConfig
+	// LazyProvide() gets called after URL matches have been performed, so the
+	// location used as the key in DockerConfig would be redundant.
+	LazyProvide() *DockerConfigEntry
+}
+
+func LazyProvide(creds LazyAuthConfiguration) docker.AuthConfiguration {
+	if creds.Provider != nil {
+		entry := *creds.Provider.LazyProvide()
+		return DockerConfigEntryToLazyAuthConfiguration(entry).AuthConfiguration
+	} else {
+		return creds.AuthConfiguration
+	}
+
 }
 
 // A DockerConfigProvider that simply reads the .dockercfg file
@@ -73,11 +87,21 @@ func (d *defaultDockerConfigProvider) Provide() DockerConfig {
 	return DockerConfig{}
 }
 
+// LazyProvide implements dockerConfigProvider. Should never be called.
+func (d *defaultDockerConfigProvider) LazyProvide() *DockerConfigEntry {
+	return nil
+}
+
 // Enabled implements dockerConfigProvider
 func (d *CachingDockerConfigProvider) Enabled() bool {
 	return d.Provider.Enabled()
 }
 
+// LazyProvide implements dockerConfigProvider. Should never be called.
+func (d *CachingDockerConfigProvider) LazyProvide() *DockerConfigEntry {
+	return nil
+}
+
 // Provide implements dockerConfigProvider
 func (d *CachingDockerConfigProvider) Provide() DockerConfig {
 	d.mu.Lock()

pkg/kubelet/dockertools/docker.go

@@ -187,7 +187,7 @@ func (p dockerPuller) Pull(image string, secrets []api.Secret) error {
 
 	var pullErrs []error
 	for _, currentCreds := range creds {
-		err := p.client.PullImage(opts, currentCreds)
+		err := p.client.PullImage(opts, credentialprovider.LazyProvide(currentCreds))
 		// If there was no error, return success
 		if err == nil {
 			return nil

pkg/kubelet/dockertools/docker_test.go

@@ -334,25 +334,25 @@ func TestPullWithSecrets(t *testing.T) {
 		"default keyring secrets": {
 			"ubuntu",
 			[]api.Secret{},
-			credentialprovider.DockerConfig(map[string]credentialprovider.DockerConfigEntry{"index.docker.io/v1/": {"built-in", "password", "email"}}),
+			credentialprovider.DockerConfig(map[string]credentialprovider.DockerConfigEntry{"index.docker.io/v1/": {"built-in", "password", "email", nil}}),
 			[]string{`ubuntu:latest using {"username":"built-in","password":"password","email":"email"}`},
 		},
 		"default keyring secrets unused": {
 			"ubuntu",
 			[]api.Secret{},
-			credentialprovider.DockerConfig(map[string]credentialprovider.DockerConfigEntry{"extraneous": {"built-in", "password", "email"}}),
+			credentialprovider.DockerConfig(map[string]credentialprovider.DockerConfigEntry{"extraneous": {"built-in", "password", "email", nil}}),
 			[]string{`ubuntu:latest using {}`},
 		},
 		"builtin keyring secrets, but use passed": {
 			"ubuntu",
 			[]api.Secret{{Type: api.SecretTypeDockercfg, Data: map[string][]byte{api.DockerConfigKey: dockercfgContent}}},
-			credentialprovider.DockerConfig(map[string]credentialprovider.DockerConfigEntry{"index.docker.io/v1/": {"built-in", "password", "email"}}),
+			credentialprovider.DockerConfig(map[string]credentialprovider.DockerConfigEntry{"index.docker.io/v1/": {"built-in", "password", "email", nil}}),
 			[]string{`ubuntu:latest using {"username":"passed-user","password":"passed-password","email":"passed-email"}`},
 		},
 		"builtin keyring secrets, but use passed with new docker config": {
 			"ubuntu",
 			[]api.Secret{{Type: api.SecretTypeDockerConfigJson, Data: map[string][]byte{api.DockerConfigJsonKey: dockerConfigJsonContent}}},
-			credentialprovider.DockerConfig(map[string]credentialprovider.DockerConfigEntry{"index.docker.io/v1/": {"built-in", "password", "email"}}),
+			credentialprovider.DockerConfig(map[string]credentialprovider.DockerConfigEntry{"index.docker.io/v1/": {"built-in", "password", "email", nil}}),
 			[]string{`ubuntu:latest using {"username":"passed-user","password":"passed-password","email":"passed-email"}`},
 		},
 	}
@@ -407,16 +407,20 @@ func TestDockerKeyringLookupFails(t *testing.T) {
 
 func TestDockerKeyringLookup(t *testing.T) {
 
-	ada := docker.AuthConfiguration{
+	ada := credentialprovider.LazyAuthConfiguration{
+		AuthConfiguration: docker.AuthConfiguration{
 			Username: "ada",
 			Password: "smash",
 			Email:    "ada@example.com",
+		},
 	}
 
-	grace := docker.AuthConfiguration{
+	grace := credentialprovider.LazyAuthConfiguration{
+		AuthConfiguration: docker.AuthConfiguration{
 			Username: "grace",
 			Password: "squash",
 			Email:    "grace@example.com",
+		},
 	}
 
 	dk := &credentialprovider.BasicDockerKeyring{}
@@ -435,27 +439,27 @@ func TestDockerKeyringLookup(t *testing.T) {
 
 	tests := []struct {
 		image string
-		match []docker.AuthConfiguration
+		match []credentialprovider.LazyAuthConfiguration
 		ok    bool
 	}{
 		// direct match
-		{"bar.example.com", []docker.AuthConfiguration{ada}, true},
+		{"bar.example.com", []credentialprovider.LazyAuthConfiguration{ada}, true},
 
 		// direct match deeper than other possible matches
-		{"bar.example.com/pong", []docker.AuthConfiguration{grace, ada}, true},
+		{"bar.example.com/pong", []credentialprovider.LazyAuthConfiguration{grace, ada}, true},
 
 		// no direct match, deeper path ignored
-		{"bar.example.com/ping", []docker.AuthConfiguration{ada}, true},
+		{"bar.example.com/ping", []credentialprovider.LazyAuthConfiguration{ada}, true},
 
 		// match first part of path token
-		{"bar.example.com/pongz", []docker.AuthConfiguration{grace, ada}, true},
+		{"bar.example.com/pongz", []credentialprovider.LazyAuthConfiguration{grace, ada}, true},
 
 		// match regardless of sub-path
-		{"bar.example.com/pong/pang", []docker.AuthConfiguration{grace, ada}, true},
+		{"bar.example.com/pong/pang", []credentialprovider.LazyAuthConfiguration{grace, ada}, true},
 
 		// no host match
-		{"example.com", []docker.AuthConfiguration{}, false},
+		{"example.com", []credentialprovider.LazyAuthConfiguration{}, false},
-		{"foo.example.com", []docker.AuthConfiguration{}, false},
+		{"foo.example.com", []credentialprovider.LazyAuthConfiguration{}, false},
 	}
 
 	for i, tt := range tests {
@@ -474,10 +478,12 @@ func TestDockerKeyringLookup(t *testing.T) {
 // by images that only match the hostname.
 // NOTE: the above covers the case of a more specific match trumping just hostname.
 func TestIssue3797(t *testing.T) {
-	rex := docker.AuthConfiguration{
+	rex := credentialprovider.LazyAuthConfiguration{
+		AuthConfiguration: docker.AuthConfiguration{
 			Username: "rex",
 			Password: "tiny arms",
 			Email:    "rex@example.com",
+		},
 	}
 
 	dk := &credentialprovider.BasicDockerKeyring{}
@@ -491,15 +497,15 @@ func TestIssue3797(t *testing.T) {
 
 	tests := []struct {
 		image string
-		match []docker.AuthConfiguration
+		match []credentialprovider.LazyAuthConfiguration
 		ok    bool
 	}{
 		// direct match
-		{"quay.io", []docker.AuthConfiguration{rex}, true},
+		{"quay.io", []credentialprovider.LazyAuthConfiguration{rex}, true},
 
 		// partial matches
-		{"quay.io/foo", []docker.AuthConfiguration{rex}, true},
+		{"quay.io/foo", []credentialprovider.LazyAuthConfiguration{rex}, true},
-		{"quay.io/foo/bar", []docker.AuthConfiguration{rex}, true},
+		{"quay.io/foo/bar", []credentialprovider.LazyAuthConfiguration{rex}, true},
 	}
 
 	for i, tt := range tests {

pkg/kubelet/rkt/image.go

@@ -178,7 +178,7 @@ func (r *Runtime) getImageManifest(image string) (*appcschema.ImageManifest, err
 
 // TODO(yifan): This is very racy, unefficient, and unsafe, we need to provide
 // different namespaces. See: https://github.com/coreos/rkt/issues/836.
-func (r *Runtime) writeDockerAuthConfig(image string, credsSlice []docker.AuthConfiguration) error {
+func (r *Runtime) writeDockerAuthConfig(image string, credsSlice []credentialprovider.LazyAuthConfiguration) error {
 	if len(credsSlice) == 0 {
 		return nil
 	}
@@ -186,7 +186,7 @@ func (r *Runtime) writeDockerAuthConfig(image string, credsSlice []docker.AuthCo
 	creds := docker.AuthConfiguration{}
 	// TODO handle multiple creds
 	if len(credsSlice) >= 1 {
-		creds = credsSlice[0]
+		creds = credentialprovider.LazyProvide(credsSlice[0])
 	}
 
 	registry := "index.docker.io"