Skip to content

Commit

Permalink
Fix ACR MSI cross-subscription authentication error
Browse files Browse the repository at this point in the history
  • Loading branch information
norshtein committed Apr 30, 2019
1 parent 16193d8 commit b5cdb78
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 17 deletions.
43 changes: 26 additions & 17 deletions pkg/credentialprovider/azure/azure_credentials.go
Expand Up @@ -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"
Expand All @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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
}
25 changes: 25 additions & 0 deletions pkg/credentialprovider/azure/azure_credentials_test.go
Expand Up @@ -94,3 +94,28 @@ 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",
},
}
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)
}
}
}

0 comments on commit b5cdb78

Please sign in to comment.