Merge pull request kubernetes#90478 from andyzhangx/automated-cherry-…

…pick-of-#90425-upstream-release-1.18 Automated cherry pick of kubernetes#90425: fix: ACR auth fails in private azure clouds
vboulineau · May 11, 2020 · 209a8b0 · 209a8b0
2 parents ec280c2 + 6cb5bd3
commit 209a8b0
Show file tree

Hide file tree

Showing 3 changed files with 70 additions and 3 deletions.
diff --git a/pkg/credentialprovider/azure/BUILD b/pkg/credentialprovider/azure/BUILD
@@ -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",
     ],
 )

diff --git a/pkg/credentialprovider/azure/azure_credentials.go b/pkg/credentialprovider/azure/azure_credentials.go
@@ -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"
@@ -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 {
@@ -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
@@ -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 ""
 }
diff --git a/pkg/credentialprovider/azure/azure_credentials_test.go b/pkg/credentialprovider/azure/azure_credentials_test.go
@@ -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"
 )
 
@@ -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
@@ -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)
 		}
 	}