diff --git a/pkg/credentialprovider/azure/BUILD b/pkg/credentialprovider/azure/BUILD index fcce639e64c..ba9ec88167b 100644 --- a/pkg/credentialprovider/azure/BUILD +++ b/pkg/credentialprovider/azure/BUILD @@ -34,6 +34,7 @@ go_test( "//vendor/github.com/Azure/azure-sdk-for-go/services/containerregistry/mgmt/2019-05-01/containerregistry:go_default_library", "//vendor/github.com/Azure/go-autorest/autorest/azure:go_default_library", "//vendor/github.com/Azure/go-autorest/autorest/to:go_default_library", + "//vendor/github.com/stretchr/testify/assert:go_default_library", ], ) diff --git a/pkg/credentialprovider/azure/azure_credentials.go b/pkg/credentialprovider/azure/azure_credentials.go index c7ea2c3fc3d..74670a3d1f6 100644 --- a/pkg/credentialprovider/azure/azure_credentials.go +++ b/pkg/credentialprovider/azure/azure_credentials.go @@ -56,8 +56,9 @@ var ( func init() { credentialprovider.RegisterCredentialProvider("azure", &credentialprovider.CachingDockerConfigProvider{ - Provider: NewACRProvider(flagConfigFile), - Lifetime: 1 * time.Minute, + Provider: NewACRProvider(flagConfigFile), + Lifetime: 1 * time.Minute, + ShouldCache: func(d credentialprovider.DockerConfig) bool { return len(d) > 0 }, }) } @@ -186,6 +187,12 @@ func (a *acrProvider) Provide(image string) credentialprovider.DockerConfig { klog.V(4).Infof("try to provide secret for image %s", image) cfg := credentialprovider.DockerConfig{} + defaultConfigEntry := credentialprovider.DockerConfigEntry{ + Username: "", + Password: "", + Email: dummyRegistryEmail, + } + if a.config.UseManagedIdentityExtension { if loginServer := a.parseACRLoginServerFromImage(image); loginServer == "" { klog.V(4).Infof("image(%s) is not from ACR, skip MSI authentication", image) @@ -193,6 +200,8 @@ func (a *acrProvider) Provide(image string) credentialprovider.DockerConfig { if cred, err := getACRDockerEntryFromARMToken(a, loginServer); err == nil { cfg[loginServer] = *cred } + // add ACR anonymous repo support: use empty username and password for anonymous access + cfg["*.azurecr.*"] = defaultConfigEntry } } else { // Add our entry for each of the supported container registry URLs @@ -226,13 +235,9 @@ func (a *acrProvider) Provide(image string) credentialprovider.DockerConfig { cfg[customAcrSuffix] = *cred } } - } - // add ACR anonymous repo support: use empty username and password for anonymous access - cfg["*.azurecr.*"] = credentialprovider.DockerConfigEntry{ - Username: "", - Password: "", - Email: dummyRegistryEmail, + // add ACR anonymous repo support: use empty username and password for anonymous access + cfg["*.azurecr.*"] = defaultConfigEntry } return cfg } diff --git a/pkg/credentialprovider/azure/azure_credentials_test.go b/pkg/credentialprovider/azure/azure_credentials_test.go index fffddea3f64..987c748c1c0 100644 --- a/pkg/credentialprovider/azure/azure_credentials_test.go +++ b/pkg/credentialprovider/azure/azure_credentials_test.go @@ -24,6 +24,8 @@ import ( "github.com/Azure/azure-sdk-for-go/services/containerregistry/mgmt/2019-05-01/containerregistry" "github.com/Azure/go-autorest/autorest/azure" "github.com/Azure/go-autorest/autorest/to" + + "github.com/stretchr/testify/assert" ) type fakeClient struct { @@ -96,6 +98,42 @@ func Test(t *testing.T) { } } +func TestProvide(t *testing.T) { + testCases := []struct { + desc string + configStr string + expectedCredsLength int + }{ + { + desc: "return multiple credentials using Service Principal", + configStr: ` + { + "aadClientId": "foo", + "aadClientSecret": "bar" + }`, + expectedCredsLength: 5, + }, + { + desc: "retuen 0 credential for non-ACR image using Managed Identity", + configStr: ` + { + "UseManagedIdentityExtension": true + }`, + expectedCredsLength: 0, + }, + } + + for i, test := range testCases { + provider := &acrProvider{ + registryClient: &fakeClient{}, + } + provider.loadConfig(bytes.NewBufferString(test.configStr)) + + creds := provider.Provide("busybox") + assert.Equal(t, test.expectedCredsLength, len(creds), "TestCase[%d]: %s", i, test.desc) + } +} + func TestParseACRLoginServerFromImage(t *testing.T) { configStr := ` { diff --git a/pkg/credentialprovider/provider.go b/pkg/credentialprovider/provider.go index 72a706b5c74..d575ca9155f 100644 --- a/pkg/credentialprovider/provider.go +++ b/pkg/credentialprovider/provider.go @@ -58,6 +58,10 @@ type CachingDockerConfigProvider struct { Provider DockerConfigProvider Lifetime time.Duration + // ShouldCache is an optional function that returns true if the specific config should be cached. + // If nil, all configs are treated as cacheable. + ShouldCache func(DockerConfig) bool + // cache fields cacheDockerConfig DockerConfig expiration time.Time @@ -96,7 +100,10 @@ func (d *CachingDockerConfigProvider) Provide(image string) DockerConfig { } klog.V(2).Infof("Refreshing cache for provider: %v", reflect.TypeOf(d.Provider).String()) - d.cacheDockerConfig = d.Provider.Provide(image) - d.expiration = time.Now().Add(d.Lifetime) - return d.cacheDockerConfig + config := d.Provider.Provide(image) + if d.ShouldCache == nil || d.ShouldCache(config) { + d.cacheDockerConfig = config + d.expiration = time.Now().Add(d.Lifetime) + } + return config }