From ca6bdba014f0a98efe0e0dd4e15f57d1c121d6c9 Mon Sep 17 00:00:00 2001 From: Rudi Chiarito Date: Tue, 29 Mar 2016 14:27:59 -0400 Subject: [PATCH] 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 | 5 ++ pkg/credentialprovider/config.go | 1 + pkg/credentialprovider/gcp/jwt.go | 5 ++ pkg/credentialprovider/gcp/metadata.go | 15 +++++ pkg/credentialprovider/keyring.go | 55 +++++++++++----- pkg/credentialprovider/keyring_test.go | 5 ++ pkg/credentialprovider/provider.go | 24 +++++++ pkg/kubelet/dockertools/docker.go | 2 +- pkg/kubelet/dockertools/docker_test.go | 62 ++++++++++--------- pkg/kubelet/rkt/image.go | 4 +- 10 files changed, 130 insertions(+), 48 deletions(-) diff --git a/pkg/credentialprovider/aws/aws_credentials.go b/pkg/credentialprovider/aws/aws_credentials.go index aceda3853a..aeb7316b32 100644 --- a/pkg/credentialprovider/aws/aws_credentials.go +++ b/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{} diff --git a/pkg/credentialprovider/config.go b/pkg/credentialprovider/config.go index f03bd26c31..b80fa5945c 100644 --- a/pkg/credentialprovider/config.go +++ b/pkg/credentialprovider/config.go @@ -46,6 +46,7 @@ type DockerConfigEntry struct { Username string Password string Email string + Provider DockerConfigProvider } var ( diff --git a/pkg/credentialprovider/gcp/jwt.go b/pkg/credentialprovider/gcp/jwt.go index 3c1a05d4ba..e4c16afa85 100644 --- a/pkg/credentialprovider/gcp/jwt.go +++ b/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{} diff --git a/pkg/credentialprovider/gcp/metadata.go b/pkg/credentialprovider/gcp/metadata.go index 8ab929315f..fb89b38c0c 100644 --- a/pkg/credentialprovider/gcp/metadata.go +++ b/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{} diff --git a/pkg/credentialprovider/keyring.go b/pkg/credentialprovider/keyring.go index 2378156dcd..09e6b687f1 100644 --- a/pkg/credentialprovider/keyring.go +++ b/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) { - if dk.index == nil { - dk.index = make([]string, 0) - dk.creds = make(map[string][]docker.AuthConfiguration) - } - for loc, ident := range cfg { +// LazyAuthConfiguration wraps AuthConfiguration, potentially deferring its +// binding. If Provider is non-nil, it will be used to obtain new credentials +// by calling LazyProvide() on it. +type LazyAuthConfiguration struct { + docker.AuthConfiguration + 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) { - authConfigs := []docker.AuthConfiguration{} +func (k *unionDockerKeyring) Lookup(image string) ([]LazyAuthConfiguration, bool) { + authConfigs := []LazyAuthConfiguration{} for _, subKeyring := range k.keyrings { if subKeyring == nil { diff --git a/pkg/credentialprovider/keyring_test.go b/pkg/credentialprovider/keyring_test.go index 72db024be4..376e080170 100644 --- a/pkg/credentialprovider/keyring_test.go +++ b/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 diff --git a/pkg/credentialprovider/provider.go b/pkg/credentialprovider/provider.go index ede5765a3c..c8832d1870 100644 --- a/pkg/credentialprovider/provider.go +++ b/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() diff --git a/pkg/kubelet/dockertools/docker.go b/pkg/kubelet/dockertools/docker.go index c41bcc1a95..5366c2b8e9 100644 --- a/pkg/kubelet/dockertools/docker.go +++ b/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 diff --git a/pkg/kubelet/dockertools/docker_test.go b/pkg/kubelet/dockertools/docker_test.go index 1a7366560e..bfc9f17a74 100644 --- a/pkg/kubelet/dockertools/docker_test.go +++ b/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{ - Username: "ada", - Password: "smash", - Email: "ada@example.com", + ada := credentialprovider.LazyAuthConfiguration{ + AuthConfiguration: docker.AuthConfiguration{ + Username: "ada", + Password: "smash", + Email: "ada@example.com", + }, } - grace := docker.AuthConfiguration{ - Username: "grace", - Password: "squash", - Email: "grace@example.com", + 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}, - {"foo.example.com", []docker.AuthConfiguration{}, false}, + {"example.com", []credentialprovider.LazyAuthConfiguration{}, 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{ - Username: "rex", - Password: "tiny arms", - Email: "rex@example.com", + 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/bar", []docker.AuthConfiguration{rex}, true}, + {"quay.io/foo", []credentialprovider.LazyAuthConfiguration{rex}, true}, + {"quay.io/foo/bar", []credentialprovider.LazyAuthConfiguration{rex}, true}, } for i, tt := range tests { diff --git a/pkg/kubelet/rkt/image.go b/pkg/kubelet/rkt/image.go index 2a949042ea..379e54aea5 100644 --- a/pkg/kubelet/rkt/image.go +++ b/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"