Remove lazy provide from credential provider and kubelet (#79674)

* Remove LazyProvide from kubelet

* Remove LazyProvide from cloud providers

* Remove LazyProvide from credential provider keyring and provider
This commit is contained in:
tiffany jernigan 2019-07-03 13:52:53 -07:00 committed by Kubernetes Prow Robot
parent a9b3d7d821
commit 27a0d91f2d
9 changed files with 56 additions and 141 deletions

View File

@ -80,12 +80,6 @@ func (p *ecrProvider) Enabled() bool {
return true return true
} }
// LazyProvide is lazy
// TODO: the LazyProvide methods will be removed in a future PR
func (p *ecrProvider) LazyProvide(image string) *credentialprovider.DockerConfigEntry {
return nil
}
// Provide returns a DockerConfig with credentials from the cache if they are // Provide returns a DockerConfig with credentials from the cache if they are
// found, or from ECR // found, or from ECR
func (p *ecrProvider) Provide(image string) credentialprovider.DockerConfig { func (p *ecrProvider) Provide(image string) credentialprovider.DockerConfig {

View File

@ -259,6 +259,3 @@ func parseACRLoginServerFromImage(image string) string {
} }
return "" return ""
} }
func (a *acrProvider) LazyProvide(image string) *credentialprovider.DockerConfigEntry {
return nil
}

View File

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

View File

@ -36,18 +36,18 @@ import (
// most specific match for a given image // most specific match for a given image
// - iterating a map does not yield predictable results // - iterating a map does not yield predictable results
type DockerKeyring interface { type DockerKeyring interface {
Lookup(image string) ([]LazyAuthConfiguration, bool) Lookup(image string) ([]AuthConfig, bool)
} }
// BasicDockerKeyring is a trivial map-backed implementation of DockerKeyring // BasicDockerKeyring is a trivial map-backed implementation of DockerKeyring
type BasicDockerKeyring struct { type BasicDockerKeyring struct {
index []string index []string
creds map[string][]LazyAuthConfiguration creds map[string][]AuthConfig
} }
// lazyDockerKeyring is an implementation of DockerKeyring that lazily // providersDockerKeyring is an implementation of DockerKeyring that
// materializes its dockercfg based on a set of dockerConfigProviders. // materializes its dockercfg based on a set of dockerConfigProviders.
type lazyDockerKeyring struct { type providersDockerKeyring struct {
Providers []DockerConfigProvider Providers []DockerConfigProvider
} }
@ -73,38 +73,16 @@ type AuthConfig struct {
RegistryToken string `json:"registrytoken,omitempty"` RegistryToken string `json:"registrytoken,omitempty"`
} }
// LazyAuthConfiguration wraps dockertypes.AuthConfig, 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 {
AuthConfig
Provider DockerConfigProvider
}
func DockerConfigEntryToLazyAuthConfiguration(ident DockerConfigEntry) LazyAuthConfiguration {
return LazyAuthConfiguration{
AuthConfig: AuthConfig{
Username: ident.Username,
Password: ident.Password,
Email: ident.Email,
},
}
}
func (dk *BasicDockerKeyring) Add(cfg DockerConfig) { func (dk *BasicDockerKeyring) Add(cfg DockerConfig) {
if dk.index == nil { if dk.index == nil {
dk.index = make([]string, 0) dk.index = make([]string, 0)
dk.creds = make(map[string][]LazyAuthConfiguration) dk.creds = make(map[string][]AuthConfig)
} }
for loc, ident := range cfg { for loc, ident := range cfg {
creds := AuthConfig{
var creds LazyAuthConfiguration Username: ident.Username,
if ident.Provider != nil { Password: ident.Password,
creds = LazyAuthConfiguration{ Email: ident.Email,
Provider: ident.Provider,
}
} else {
creds = DockerConfigEntryToLazyAuthConfiguration(ident)
} }
value := loc value := loc
@ -255,9 +233,9 @@ func urlsMatch(globUrl *url.URL, targetUrl *url.URL) (bool, error) {
// Lookup implements the DockerKeyring method for fetching credentials based on image name. // Lookup implements the DockerKeyring method for fetching credentials based on image name.
// Multiple credentials may be returned if there are multiple potentially valid credentials // Multiple credentials may be returned if there are multiple potentially valid credentials
// available. This allows for rotation. // available. This allows for rotation.
func (dk *BasicDockerKeyring) Lookup(image string) ([]LazyAuthConfiguration, bool) { func (dk *BasicDockerKeyring) Lookup(image string) ([]AuthConfig, bool) {
// range over the index as iterating over a map does not provide a predictable ordering // range over the index as iterating over a map does not provide a predictable ordering
ret := []LazyAuthConfiguration{} ret := []AuthConfig{}
for _, k := range dk.index { for _, k := range dk.index {
// both k and image are schemeless URLs because even though schemes are allowed // both k and image are schemeless URLs because even though schemes are allowed
// in the credential configurations, we remove them in Add. // in the credential configurations, we remove them in Add.
@ -277,12 +255,12 @@ func (dk *BasicDockerKeyring) Lookup(image string) ([]LazyAuthConfiguration, boo
} }
} }
return []LazyAuthConfiguration{}, false return []AuthConfig{}, false
} }
// Lookup implements the DockerKeyring method for fetching credentials // Lookup implements the DockerKeyring method for fetching credentials
// based on image name. // based on image name.
func (dk *lazyDockerKeyring) Lookup(image string) ([]LazyAuthConfiguration, bool) { func (dk *providersDockerKeyring) Lookup(image string) ([]AuthConfig, bool) {
keyring := &BasicDockerKeyring{} keyring := &BasicDockerKeyring{}
for _, p := range dk.Providers { for _, p := range dk.Providers {
@ -293,19 +271,19 @@ func (dk *lazyDockerKeyring) Lookup(image string) ([]LazyAuthConfiguration, bool
} }
type FakeKeyring struct { type FakeKeyring struct {
auth []LazyAuthConfiguration auth []AuthConfig
ok bool ok bool
} }
func (f *FakeKeyring) Lookup(image string) ([]LazyAuthConfiguration, bool) { func (f *FakeKeyring) Lookup(image string) ([]AuthConfig, bool) {
return f.auth, f.ok return f.auth, f.ok
} }
// UnionDockerKeyring delegates to a set of keyrings. // UnionDockerKeyring delegates to a set of keyrings.
type UnionDockerKeyring []DockerKeyring type UnionDockerKeyring []DockerKeyring
func (k UnionDockerKeyring) Lookup(image string) ([]LazyAuthConfiguration, bool) { func (k UnionDockerKeyring) Lookup(image string) ([]AuthConfig, bool) {
authConfigs := []LazyAuthConfiguration{} authConfigs := []AuthConfig{}
for _, subKeyring := range k { for _, subKeyring := range k {
if subKeyring == nil { if subKeyring == nil {
continue continue

View File

@ -463,22 +463,17 @@ func (d *testProvider) Enabled() bool {
return true return true
} }
// LazyProvide implements dockerConfigProvider. Should never be called.
func (d *testProvider) LazyProvide(image string) *DockerConfigEntry {
return nil
}
// Provide implements dockerConfigProvider // Provide implements dockerConfigProvider
func (d *testProvider) Provide(image string) DockerConfig { func (d *testProvider) Provide(image string) DockerConfig {
d.Count++ d.Count++
return DockerConfig{} return DockerConfig{}
} }
func TestLazyKeyring(t *testing.T) { func TestProvidersDockerKeyring(t *testing.T) {
provider := &testProvider{ provider := &testProvider{
Count: 0, Count: 0,
} }
lazy := &lazyDockerKeyring{ keyring := &providersDockerKeyring{
Providers: []DockerConfigProvider{ Providers: []DockerConfigProvider{
provider, provider,
}, },
@ -487,35 +482,31 @@ func TestLazyKeyring(t *testing.T) {
if provider.Count != 0 { if provider.Count != 0 {
t.Errorf("Unexpected number of Provide calls: %v", provider.Count) t.Errorf("Unexpected number of Provide calls: %v", provider.Count)
} }
lazy.Lookup("foo") keyring.Lookup("foo")
if provider.Count != 1 { if provider.Count != 1 {
t.Errorf("Unexpected number of Provide calls: %v", provider.Count) t.Errorf("Unexpected number of Provide calls: %v", provider.Count)
} }
lazy.Lookup("foo") keyring.Lookup("foo")
if provider.Count != 2 { if provider.Count != 2 {
t.Errorf("Unexpected number of Provide calls: %v", provider.Count) t.Errorf("Unexpected number of Provide calls: %v", provider.Count)
} }
lazy.Lookup("foo") keyring.Lookup("foo")
if provider.Count != 3 { if provider.Count != 3 {
t.Errorf("Unexpected number of Provide calls: %v", provider.Count) t.Errorf("Unexpected number of Provide calls: %v", provider.Count)
} }
} }
func TestDockerKeyringLookup(t *testing.T) { func TestDockerKeyringLookup(t *testing.T) {
ada := LazyAuthConfiguration{ ada := AuthConfig{
AuthConfig: AuthConfig{ Username: "ada",
Username: "ada", Password: "smash",
Password: "smash", Email: "ada@example.com",
Email: "ada@example.com",
},
} }
grace := LazyAuthConfiguration{ grace := AuthConfig{
AuthConfig: AuthConfig{ Username: "grace",
Username: "grace", Password: "squash",
Password: "squash", Email: "grace@example.com",
Email: "grace@example.com",
},
} }
dk := &BasicDockerKeyring{} dk := &BasicDockerKeyring{}
@ -534,27 +525,27 @@ func TestDockerKeyringLookup(t *testing.T) {
tests := []struct { tests := []struct {
image string image string
match []LazyAuthConfiguration match []AuthConfig
ok bool ok bool
}{ }{
// direct match // direct match
{"bar.example.com", []LazyAuthConfiguration{ada}, true}, {"bar.example.com", []AuthConfig{ada}, true},
// direct match deeper than other possible matches // direct match deeper than other possible matches
{"bar.example.com/pong", []LazyAuthConfiguration{grace, ada}, true}, {"bar.example.com/pong", []AuthConfig{grace, ada}, true},
// no direct match, deeper path ignored // no direct match, deeper path ignored
{"bar.example.com/ping", []LazyAuthConfiguration{ada}, true}, {"bar.example.com/ping", []AuthConfig{ada}, true},
// match first part of path token // match first part of path token
{"bar.example.com/pongz", []LazyAuthConfiguration{grace, ada}, true}, {"bar.example.com/pongz", []AuthConfig{grace, ada}, true},
// match regardless of sub-path // match regardless of sub-path
{"bar.example.com/pong/pang", []LazyAuthConfiguration{grace, ada}, true}, {"bar.example.com/pong/pang", []AuthConfig{grace, ada}, true},
// no host match // no host match
{"example.com", []LazyAuthConfiguration{}, false}, {"example.com", []AuthConfig{}, false},
{"foo.example.com", []LazyAuthConfiguration{}, false}, {"foo.example.com", []AuthConfig{}, false},
} }
for i, tt := range tests { for i, tt := range tests {
@ -573,12 +564,10 @@ func TestDockerKeyringLookup(t *testing.T) {
// by images that only match the hostname. // by images that only match the hostname.
// NOTE: the above covers the case of a more specific match trumping just hostname. // NOTE: the above covers the case of a more specific match trumping just hostname.
func TestIssue3797(t *testing.T) { func TestIssue3797(t *testing.T) {
rex := LazyAuthConfiguration{ rex := AuthConfig{
AuthConfig: AuthConfig{ Username: "rex",
Username: "rex", Password: "tiny arms",
Password: "tiny arms", Email: "rex@example.com",
Email: "rex@example.com",
},
} }
dk := &BasicDockerKeyring{} dk := &BasicDockerKeyring{}
@ -592,15 +581,15 @@ func TestIssue3797(t *testing.T) {
tests := []struct { tests := []struct {
image string image string
match []LazyAuthConfiguration match []AuthConfig
ok bool ok bool
}{ }{
// direct match // direct match
{"quay.io", []LazyAuthConfiguration{rex}, true}, {"quay.io", []AuthConfig{rex}, true},
// partial matches // partial matches
{"quay.io/foo", []LazyAuthConfiguration{rex}, true}, {"quay.io/foo", []AuthConfig{rex}, true},
{"quay.io/foo/bar", []LazyAuthConfiguration{rex}, true}, {"quay.io/foo/bar", []AuthConfig{rex}, true},
} }
for i, tt := range tests { for i, tt := range tests {

View File

@ -45,9 +45,9 @@ func RegisterCredentialProvider(name string, provider DockerConfigProvider) {
} }
// NewDockerKeyring creates a DockerKeyring to use for resolving credentials, // NewDockerKeyring creates a DockerKeyring to use for resolving credentials,
// which lazily draws from the set of registered credential providers. // which draws from the set of registered credential providers.
func NewDockerKeyring() DockerKeyring { func NewDockerKeyring() DockerKeyring {
keyring := &lazyDockerKeyring{ keyring := &providersDockerKeyring{
Providers: make([]DockerConfigProvider, 0), Providers: make([]DockerConfigProvider, 0),
} }

View File

@ -37,22 +37,6 @@ type DockerConfigProvider interface {
// implementation depends on information in the image name to return // implementation depends on information in the image name to return
// credentials; implementations are safe to ignore the image. // credentials; implementations are safe to ignore the image.
Provide(image string) DockerConfig Provide(image string) DockerConfig
// LazyProvide gets called after URL matches have been
// performed, so the location used as the key in DockerConfig would be
// redundant.
// The image is passed in as context in the event that the
// implementation depends on information in the image name to return
// credentials; implementations are safe to ignore the image.
LazyProvide(image string) *DockerConfigEntry
}
//LazyProvide returns an Lazy AuthConfig
func LazyProvide(creds LazyAuthConfiguration, image string) AuthConfig {
if creds.Provider != nil {
entry := *creds.Provider.LazyProvide(image)
return DockerConfigEntryToLazyAuthConfiguration(entry).AuthConfig
}
return creds.AuthConfig
} }
// A DockerConfigProvider that simply reads the .dockercfg file // A DockerConfigProvider that simply reads the .dockercfg file
@ -85,7 +69,7 @@ func (d *defaultDockerConfigProvider) Enabled() bool {
return true return true
} }
// LazyProvide implements dockerConfigProvider // Provide implements dockerConfigProvider
func (d *defaultDockerConfigProvider) Provide(image string) DockerConfig { func (d *defaultDockerConfigProvider) Provide(image string) DockerConfig {
// Read the standard Docker credentials from .dockercfg // Read the standard Docker credentials from .dockercfg
if cfg, err := ReadDockerConfigFile(); err == nil { if cfg, err := ReadDockerConfigFile(); err == nil {
@ -96,21 +80,11 @@ func (d *defaultDockerConfigProvider) Provide(image string) DockerConfig {
return DockerConfig{} return DockerConfig{}
} }
// LazyProvide implements dockerConfigProvider. Should never be called.
func (d *defaultDockerConfigProvider) LazyProvide(image string) *DockerConfigEntry {
return nil
}
// Enabled implements dockerConfigProvider // Enabled implements dockerConfigProvider
func (d *CachingDockerConfigProvider) Enabled() bool { func (d *CachingDockerConfigProvider) Enabled() bool {
return d.Provider.Enabled() return d.Provider.Enabled()
} }
// LazyProvide implements dockerConfigProvider. Should never be called.
func (d *CachingDockerConfigProvider) LazyProvide(image string) *DockerConfigEntry {
return nil
}
// Provide implements dockerConfigProvider // Provide implements dockerConfigProvider
func (d *CachingDockerConfigProvider) Provide(image string) DockerConfig { func (d *CachingDockerConfigProvider) Provide(image string) DockerConfig {
d.mu.Lock() d.mu.Lock()

View File

@ -344,7 +344,7 @@ func ensureSandboxImageExists(client libdocker.Interface, image string) error {
var pullErrs []error var pullErrs []error
for _, currentCreds := range creds { for _, currentCreds := range creds {
authConfig := dockertypes.AuthConfig(credentialprovider.LazyProvide(currentCreds, repoToPull)) authConfig := dockertypes.AuthConfig(currentCreds)
err := client.PullImage(image, authConfig, dockertypes.ImagePullOptions{}) err := client.PullImage(image, authConfig, dockertypes.ImagePullOptions{})
// If there was no error, return success // If there was no error, return success
if err == nil { if err == nil {

View File

@ -21,7 +21,6 @@ import (
utilerrors "k8s.io/apimachinery/pkg/util/errors" utilerrors "k8s.io/apimachinery/pkg/util/errors"
runtimeapi "k8s.io/cri-api/pkg/apis/runtime/v1alpha2" runtimeapi "k8s.io/cri-api/pkg/apis/runtime/v1alpha2"
"k8s.io/klog" "k8s.io/klog"
"k8s.io/kubernetes/pkg/credentialprovider"
credentialprovidersecrets "k8s.io/kubernetes/pkg/credentialprovider/secrets" credentialprovidersecrets "k8s.io/kubernetes/pkg/credentialprovider/secrets"
kubecontainer "k8s.io/kubernetes/pkg/kubelet/container" kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
"k8s.io/kubernetes/pkg/util/parsers" "k8s.io/kubernetes/pkg/util/parsers"
@ -57,14 +56,13 @@ func (m *kubeGenericRuntimeManager) PullImage(image kubecontainer.ImageSpec, pul
var pullErrs []error var pullErrs []error
for _, currentCreds := range creds { for _, currentCreds := range creds {
authConfig := credentialprovider.LazyProvide(currentCreds, repoToPull)
auth := &runtimeapi.AuthConfig{ auth := &runtimeapi.AuthConfig{
Username: authConfig.Username, Username: currentCreds.Username,
Password: authConfig.Password, Password: currentCreds.Password,
Auth: authConfig.Auth, Auth: currentCreds.Auth,
ServerAddress: authConfig.ServerAddress, ServerAddress: currentCreds.ServerAddress,
IdentityToken: authConfig.IdentityToken, IdentityToken: currentCreds.IdentityToken,
RegistryToken: authConfig.RegistryToken, RegistryToken: currentCreds.RegistryToken,
} }
imageRef, err := m.imageService.PullImage(imgSpec, auth, podSandboxConfig) imageRef, err := m.imageService.PullImage(imgSpec, auth, podSandboxConfig)