fix: use service management endpoint for ACR when using WI (#2913)

The `scope` parameter used to be the ACR url foobar.azurecr.io, but
this stopped working. Turns out that you need to use the management
endpoint as `scope` in order to authenticate with ACR.

Signed-off-by: Moritz Johner <beller.moritz@googlemail.com>

docs/api/generator/acr.md

@@ -18,7 +18,8 @@ You must choose one out of three authentication mechanisms:
 - managed identity
 - workload identity
 
-The generated token will inherit the permissions from the assigned policy. I.e. when you assign a read-only policy all generated tokens will be read-only.
+The generated token will inherit the permissions from the assigned policy. I.e. when you assign a read-only policy all generated tokens will be read-only. 
+You **must** [assign a Azure RBAC role](https://learn.microsoft.com/en-us/azure/role-based-access-control/role-assignments-steps), such as `AcrPush` or `AcrPull` to the service principal in order to be able to authenticate with the Azure container registry API. 
 
 You can scope tokens to a particular repository using `spec.scope`.
 

pkg/generator/acr/acr.go

@@ -131,7 +131,6 @@ func (g *Generator) generate(
 			ctx,
 			crClient,
 			kubeClient.CoreV1(),
-			res.Spec.ACRRegistry,
 			res.Spec.EnvironmentType,
 			res.Spec.Auth.WorkloadIdentity.ServiceAccountRef,
 			namespace,
@@ -228,12 +227,9 @@ func fetchACRRefreshToken(aadAccessToken, tenantID, registryURL string) (string,
 	return refreshToken, nil
 }
 
-func accessTokenForWorkloadIdentity(ctx context.Context, crClient client.Client, kubeClient kcorev1.CoreV1Interface, acrRegistry string, envType v1beta1.AzureEnvironmentType, serviceAccountRef *smmeta.ServiceAccountSelector, namespace string) (string, error) {
+func accessTokenForWorkloadIdentity(ctx context.Context, crClient client.Client, kubeClient kcorev1.CoreV1Interface, envType v1beta1.AzureEnvironmentType, serviceAccountRef *smmeta.ServiceAccountSelector, namespace string) (string, error) {
 	aadEndpoint := keyvault.AadEndpointForType(envType)
-	if !strings.HasSuffix(acrRegistry, "/") {
-		acrRegistry += "/"
-	}
-	acrResource := fmt.Sprintf("https://%s/.default", acrRegistry)
+	scope := keyvault.ServiceManagementEndpointForType(envType)
 	// if no serviceAccountRef was provided
 	// we expect certain env vars to be present.
 	// They are set by the azure workload identity webhook.
@@ -248,7 +244,7 @@ func accessTokenForWorkloadIdentity(ctx context.Context, crClient client.Client,
 		if err != nil {
 			return "", fmt.Errorf("unable to read token file %s: %w", tokenFilePath, err)
 		}
-		tp, err := keyvault.NewTokenProvider(ctx, string(token), clientID, tenantID, aadEndpoint, acrResource)
+		tp, err := keyvault.NewTokenProvider(ctx, string(token), clientID, tenantID, aadEndpoint, scope)
 		if err != nil {
 			return "", err
 		}
@@ -278,7 +274,7 @@ func accessTokenForWorkloadIdentity(ctx context.Context, crClient client.Client,
 	if err != nil {
 		return "", err
 	}
-	tp, err := keyvault.NewTokenProvider(ctx, token, clientID, tenantID, aadEndpoint, acrResource)
+	tp, err := keyvault.NewTokenProvider(ctx, token, clientID, tenantID, aadEndpoint, scope)
 	if err != nil {
 		return "", err
 	}

pkg/provider/azure/keyvault/keyvault.go

@@ -967,6 +967,21 @@ func AadEndpointForType(t esv1beta1.AzureEnvironmentType) string {
 	}
 }
 
+func ServiceManagementEndpointForType(t esv1beta1.AzureEnvironmentType) string {
+	switch t {
+	case esv1beta1.AzureEnvironmentPublicCloud:
+		return azure.PublicCloud.ServiceManagementEndpoint
+	case esv1beta1.AzureEnvironmentChinaCloud:
+		return azure.ChinaCloud.ServiceManagementEndpoint
+	case esv1beta1.AzureEnvironmentUSGovernmentCloud:
+		return azure.USGovernmentCloud.ServiceManagementEndpoint
+	case esv1beta1.AzureEnvironmentGermanCloud:
+		return azure.GermanCloud.ServiceManagementEndpoint
+	default:
+		return azure.PublicCloud.ServiceManagementEndpoint
+	}
+}
+
 func kvResourceForProviderConfig(t esv1beta1.AzureEnvironmentType) string {
 	var res string
 	switch t {