Merge pull request kubernetes#77245 from norshtein/acr-msi-fix

Fix ACR MSI cross-subscription authentication error

Author: k8s-ci-robot

pkg/credentialprovider/azure/azure_credentials.go

@@ -22,6 +22,7 @@ import (
 	"io"
 	"io/ioutil"
 	"os"
+	"regexp"
 	"time"
 
 	"github.com/Azure/azure-sdk-for-go/services/containerregistry/mgmt/2017-10-01/containerregistry"
@@ -44,7 +45,10 @@ const (
 	maxReadLength      = 10 * 1 << 20 // 10MB
 )
 
-var containerRegistryUrls = []string{"*.azurecr.io", "*.azurecr.cn", "*.azurecr.de", "*.azurecr.us"}
+var (
+	containerRegistryUrls = []string{"*.azurecr.io", "*.azurecr.cn", "*.azurecr.de", "*.azurecr.us"}
+	acrRE                 = regexp.MustCompile(`.*\.azurecr\.io|.*\.azurecr\.cn|.*\.azurecr\.de|.*\.azurecr\.us`)
+)
 
 // init registers the various means by which credentials may
 // be resolved on Azure.
@@ -182,26 +186,16 @@ func (a *acrProvider) Enabled() bool {
 }
 
 func (a *acrProvider) Provide(image string) credentialprovider.DockerConfig {
+	klog.V(4).Infof("try to provide secret for image %s", image)
 	cfg := credentialprovider.DockerConfig{}
-	ctx, cancel := getContextWithCancel()
-	defer cancel()
 
 	if a.config.UseManagedIdentityExtension {
-		klog.V(4).Infof("listing registries")
-		result, err := a.registryClient.List(ctx)
-		if err != nil {
-			klog.Errorf("Failed to list registries: %v", err)
-			return cfg
-		}
-
-		for ix := range result {
-			loginServer := getLoginServer(result[ix])
-			klog.V(2).Infof("loginServer: %s", loginServer)
-			cred, err := getACRDockerEntryFromARMToken(a, loginServer)
-			if err != nil {
-				continue
+		if loginServer := 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 {
+				cfg[loginServer] = *cred
 			}
-			cfg[loginServer] = *cred
 		}
 	} else {
 		// Add our entry for each of the supported container registry URLs
@@ -229,6 +223,11 @@ func getLoginServer(registry containerregistry.Registry) string {
 }
 
 func getACRDockerEntryFromARMToken(a *acrProvider, loginServer string) (*credentialprovider.DockerConfigEntry, error) {
+	// Run EnsureFresh to make sure the token is valid and does not expire
+	if err := a.servicePrincipalToken.EnsureFresh(); err != nil {
+		klog.Errorf("Failed to ensure fresh service principal token: %v", err)
+		return nil, err
+	}
 	armAccessToken := a.servicePrincipalToken.OAuthToken()
 
 	klog.V(4).Infof("discovering auth redirects for: %s", loginServer)
@@ -254,6 +253,16 @@ func getACRDockerEntryFromARMToken(a *acrProvider, loginServer string) (*credent
 	}, nil
 }
 
+// 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 {
+	match := acrRE.FindAllString(image, -1)
+	if len(match) == 1 {
+		return match[0]
+	}
+	return ""
+}
 func (a *acrProvider) LazyProvide(image string) *credentialprovider.DockerConfigEntry {
 	return nil
 }

pkg/credentialprovider/azure/azure_credentials_test.go

@@ -94,3 +94,40 @@ func Test(t *testing.T) {
 		}
 	}
 }
+
+func TestParseACRLoginServerFromImage(t *testing.T) {
+	tests := []struct {
+		image    string
+		expected string
+	}{
+		{
+			image:    "invalidImage",
+			expected: "",
+		},
+		{
+			image:    "docker.io/library/busybox:latest",
+			expected: "",
+		},
+		{
+			image:    "foo.azurecr.io/bar/image:version",
+			expected: "foo.azurecr.io",
+		},
+		{
+			image:    "foo.azurecr.cn/bar/image:version",
+			expected: "foo.azurecr.cn",
+		},
+		{
+			image:    "foo.azurecr.de/bar/image:version",
+			expected: "foo.azurecr.de",
+		},
+		{
+			image:    "foo.azurecr.us/bar/image:version",
+			expected: "foo.azurecr.us",
+		},
+	}
+	for _, test := range tests {
+		if loginServer := parseACRLoginServerFromImage(test.image); loginServer != test.expected {
+			t.Errorf("function parseACRLoginServerFromImage returns \"%s\" for image %s, expected \"%s\"", loginServer, test.image, test.expected)
+		}
+	}
+}