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

Automated cherry pick of #90425: fix: ACR auth fails in private azure clouds #90478

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
1 change: 1 addition & 0 deletions pkg/credentialprovider/azure/BUILD
Expand Up @@ -32,6 +32,7 @@ go_test(
embed = [":go_default_library"],
deps = [
"//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",
],
)
Expand Down
41 changes: 39 additions & 2 deletions pkg/credentialprovider/azure/azure_credentials.go
Expand Up @@ -23,6 +23,7 @@ import (
"io/ioutil"
"os"
"regexp"
"strings"
"time"

"github.com/Azure/azure-sdk-for-go/services/containerregistry/mgmt/2019-05-01/containerregistry"
Expand Down Expand Up @@ -186,7 +187,7 @@ func (a *acrProvider) Provide(image string) credentialprovider.DockerConfig {
cfg := credentialprovider.DockerConfig{}

if a.config.UseManagedIdentityExtension {
if loginServer := parseACRLoginServerFromImage(image); loginServer == "" {
if loginServer := a.parseACRLoginServerFromImage(image); loginServer == "" {
klog.V(4).Infof("image(%s) is not from ACR, skip MSI authentication", image)
} else {
if cred, err := getACRDockerEntryFromARMToken(a, loginServer); err == nil {
Expand All @@ -203,6 +204,28 @@ func (a *acrProvider) Provide(image string) credentialprovider.DockerConfig {
}
cfg[url] = *cred
}

// Handle the custom cloud case
// In clouds where ACR is not yet deployed, the string will be empty
if a.environment != nil && strings.Contains(a.environment.ContainerRegistryDNSSuffix, ".azurecr.") {
customAcrSuffix := "*" + a.environment.ContainerRegistryDNSSuffix
hasBeenAdded := false
for _, url := range containerRegistryUrls {
if strings.EqualFold(url, customAcrSuffix) {
hasBeenAdded = true
break
}
}

if !hasBeenAdded {
cred := &credentialprovider.DockerConfigEntry{
Username: a.config.AADClientID,
Password: a.config.AADClientSecret,
Email: dummyRegistryEmail,
}
cfg[customAcrSuffix] = *cred
}
}
}

// add ACR anonymous repo support: use empty username and password for anonymous access
Expand Down Expand Up @@ -252,10 +275,24 @@ func getACRDockerEntryFromARMToken(a *acrProvider, loginServer string) (*credent
// parseACRLoginServerFromImage takes image as parameter and returns login server of it.
// Parameter `image` is expected in following format: foo.azurecr.io/bar/imageName:version
// If the provided image is not an acr image, this function will return an empty string.
func parseACRLoginServerFromImage(image string) string {
func (a *acrProvider) parseACRLoginServerFromImage(image string) string {
match := acrRE.FindAllString(image, -1)
if len(match) == 1 {
return match[0]
}

// handle the custom cloud case
if a != nil && a.environment != nil {
cloudAcrSuffix := a.environment.ContainerRegistryDNSSuffix
cloudAcrSuffixLength := len(cloudAcrSuffix)
if cloudAcrSuffixLength > 0 {
customAcrSuffixIndex := strings.Index(image, cloudAcrSuffix)
if customAcrSuffixIndex != -1 {
endIndex := customAcrSuffixIndex + cloudAcrSuffixLength
return image[0:endIndex]
}
}
}

return ""
}
31 changes: 30 additions & 1 deletion pkg/credentialprovider/azure/azure_credentials_test.go
Expand Up @@ -22,6 +22,7 @@ import (
"testing"

"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"
)

Expand Down Expand Up @@ -96,6 +97,30 @@ func Test(t *testing.T) {
}

func TestParseACRLoginServerFromImage(t *testing.T) {
configStr := `
{
"aadClientId": "foo",
"aadClientSecret": "bar"
}`
result := []containerregistry.Registry{
{
Name: to.StringPtr("foo"),
RegistryProperties: &containerregistry.RegistryProperties{
LoginServer: to.StringPtr("*.azurecr.io"),
},
},
}
fakeClient := &fakeClient{
results: result,
}

provider := &acrProvider{
registryClient: fakeClient,
}
provider.loadConfig(bytes.NewBufferString(configStr))
provider.environment = &azure.Environment{
ContainerRegistryDNSSuffix: ".azurecr.my.cloud",
}
tests := []struct {
image string
expected string
Expand Down Expand Up @@ -124,9 +149,13 @@ func TestParseACRLoginServerFromImage(t *testing.T) {
image: "foo.azurecr.us/bar/image:version",
expected: "foo.azurecr.us",
},
{
image: "foo.azurecr.my.cloud/bar/image:version",
expected: "foo.azurecr.my.cloud",
},
}
for _, test := range tests {
if loginServer := parseACRLoginServerFromImage(test.image); loginServer != test.expected {
if loginServer := provider.parseACRLoginServerFromImage(test.image); loginServer != test.expected {
t.Errorf("function parseACRLoginServerFromImage returns \"%s\" for image %s, expected \"%s\"", loginServer, test.image, test.expected)
}
}
Expand Down