fix: don't use docker config cache if it's empty

add one comment

test: add unit test

fix comments

fix comments

revert test change

fix comments
This commit is contained in:
andyzhangx 2020-06-20 01:34:23 +00:00
parent 403716626d
commit fe873af660
4 changed files with 62 additions and 11 deletions

View File

@ -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",
],
)

View File

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

View File

@ -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 := `
{

View File

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