Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Apr 15, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions pkg/credentialprovider/aws/aws_credentials.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand Down
1 change: 1 addition & 0 deletions pkg/credentialprovider/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ type DockerConfigEntry struct {
Username string
Password string
Email string
Provider DockerConfigProvider
}

var (
Expand Down
5 changes: 5 additions & 0 deletions pkg/credentialprovider/gcp/jwt.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand Down
15 changes: 15 additions & 0 deletions pkg/credentialprovider/gcp/metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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{}
Expand Down
51 changes: 36 additions & 15 deletions pkg/credentialprovider/keyring.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -54,17 +54,38 @@ type lazyDockerKeyring struct {
Providers []DockerConfigProvider
}

// 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
}

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][]docker.AuthConfiguration)
dk.creds = make(map[string][]LazyAuthConfiguration)
}
for loc, ident := range cfg {

creds := docker.AuthConfiguration{
Username: ident.Username,
Password: ident.Password,
Email: ident.Email,
var creds LazyAuthConfiguration
if ident.Provider != nil {
creds = LazyAuthConfiguration{
Provider: ident.Provider,
}
} else {
creds = DockerConfigEntryToLazyAuthConfiguration(ident)
}

value := loc
Expand Down Expand Up @@ -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.
Expand All @@ -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 {
Expand All @@ -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
}

Expand All @@ -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 {
Expand Down
5 changes: 5 additions & 0 deletions pkg/credentialprovider/keyring_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
24 changes: 24 additions & 0 deletions pkg/credentialprovider/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"sync"
"time"

docker "github.com/fsouza/go-dockerclient"
"github.com/golang/glog"
)

Expand All @@ -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
Expand Down Expand Up @@ -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()
Expand Down
2 changes: 1 addition & 1 deletion pkg/kubelet/dockertools/docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
62 changes: 34 additions & 28 deletions pkg/kubelet/dockertools/docker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"}`},
},
}
Expand Down Expand Up @@ -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{}
Expand All @@ -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 {
Expand All @@ -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{}
Expand All @@ -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 {
Expand Down