Merge pull request #4712 from jackfrancis/msi-per-sub-client

enable per-sub msi client

Author: jackfrancis

azure/services/identities/client.go

@@ -51,6 +51,19 @@ func NewClient(auth azure.Authorizer) (Client, error) {
 	return &AzureClient{factory.NewUserAssignedIdentitiesClient()}, nil
 }
 
+// NewClientBySub creates a new MSI client with a given subscriptionID.
+func NewClientBySub(auth azure.Authorizer, subscriptionID string) (Client, error) {
+	opts, err := azure.ARMClientOptions(auth.CloudEnvironment())
+	if err != nil {
+		return nil, errors.Wrap(err, "failed to create identities client options")
+	}
+	factory, err := armmsi.NewClientFactory(subscriptionID, auth.Token(), opts)
+	if err != nil {
+		return nil, errors.Wrap(err, "failed to create armmsi client factory")
+	}
+	return &AzureClient{factory.NewUserAssignedIdentitiesClient()}, nil
+}
+
 // Get returns a managed service identity.
 func (ac *AzureClient) Get(ctx context.Context, resourceGroupName, name string) (armmsi.Identity, error) {
 	ctx, _, done := tele.StartSpanWithLogger(ctx, "identities.AzureClient.Get")

azure/services/virtualmachines/virtualmachines.go

@@ -176,7 +176,18 @@ func (s *Service) checkUserAssignedIdentities(ctx context.Context, specIdentitie
 
 	// Create a map of the expected identities. The ProviderID is converted to match the format of the VM identity.
 	for _, expectedIdentity := range specIdentities {
-		expectedClientID, err := s.identitiesGetter.GetClientID(ctx, expectedIdentity.ProviderID)
+		identitiesClient := s.identitiesGetter
+		parsed, err := azureutil.ParseResourceID(expectedIdentity.ProviderID)
+		if err != nil {
+			return err
+		}
+		if parsed.SubscriptionID != s.Scope.SubscriptionID() {
+			identitiesClient, err = identities.NewClientBySub(s.Scope, parsed.SubscriptionID)
+			if err != nil {
+				return errors.Wrapf(err, "failed to create identities client from subscription ID %s", parsed.SubscriptionID)
+			}
+		}
+		expectedClientID, err := identitiesClient.GetClientID(ctx, expectedIdentity.ProviderID)
 		if err != nil {
 			return errors.Wrap(err, "failed to get client ID")
 		}

azure/services/virtualmachines/virtualmachines_test.go

@@ -114,10 +114,10 @@ var (
 		},
 	}
 	fakeUserAssignedIdentity = infrav1.UserAssignedIdentity{
-		ProviderID: "fake-provider-id",
+		ProviderID: "azure:///subscriptions/123/resourceGroups/test-rg/providers/Microsoft.ManagedIdentity/userAssignedIdentities/fake-provider-id",
 	}
 	fakeUserAssignedIdentity2 = infrav1.UserAssignedIdentity{
-		ProviderID: "fake-provider-id-2",
+		ProviderID: "azure:///subscriptions/123/resourceGroups/test-rg/providers/Microsoft.ManagedIdentity/userAssignedIdentities/fake-provider-id-2",
 	}
 )
 
@@ -335,6 +335,7 @@ func TestCheckUserAssignedIdentities(t *testing.T) {
 			specIdentities:   []infrav1.UserAssignedIdentity{fakeUserAssignedIdentity},
 			actualIdentities: []infrav1.UserAssignedIdentity{fakeUserAssignedIdentity},
 			expect: func(s *mock_virtualmachines.MockVMScopeMockRecorder, i *mock_identities.MockClientMockRecorder) {
+				s.SubscriptionID().Return("123")
 				i.GetClientID(gomockinternal.AContext(), fakeUserAssignedIdentity.ProviderID).AnyTimes().Return(fakeUserAssignedIdentity.ProviderID, nil)
 			},
 			expectedError: "",
@@ -344,6 +345,7 @@ func TestCheckUserAssignedIdentities(t *testing.T) {
 			specIdentities:   []infrav1.UserAssignedIdentity{fakeUserAssignedIdentity, fakeUserAssignedIdentity2},
 			actualIdentities: []infrav1.UserAssignedIdentity{fakeUserAssignedIdentity},
 			expect: func(s *mock_virtualmachines.MockVMScopeMockRecorder, i *mock_identities.MockClientMockRecorder) {
+				s.SubscriptionID().AnyTimes().Return("123")
 				i.GetClientID(gomockinternal.AContext(), fakeUserAssignedIdentity.ProviderID).AnyTimes().Return(fakeUserAssignedIdentity.ProviderID, nil)
 				i.GetClientID(gomockinternal.AContext(), fakeUserAssignedIdentity2.ProviderID).AnyTimes().Return(fakeUserAssignedIdentity2.ProviderID, nil)
 				s.SetConditionFalse(infrav1.VMIdentitiesReadyCondition, infrav1.UserAssignedIdentityMissingReason, clusterv1.ConditionSeverityWarning, vmMissingUAI+fakeUserAssignedIdentity2.ProviderID).Times(1)
@@ -355,6 +357,7 @@ func TestCheckUserAssignedIdentities(t *testing.T) {
 			specIdentities:   []infrav1.UserAssignedIdentity{fakeUserAssignedIdentity},
 			actualIdentities: []infrav1.UserAssignedIdentity{fakeUserAssignedIdentity, fakeUserAssignedIdentity2},
 			expect: func(s *mock_virtualmachines.MockVMScopeMockRecorder, i *mock_identities.MockClientMockRecorder) {
+				s.SubscriptionID().Return("123")
 				i.GetClientID(gomockinternal.AContext(), fakeUserAssignedIdentity.ProviderID).AnyTimes().Return(fakeUserAssignedIdentity.ProviderID, nil)
 			},
 			expectedError: "",
@@ -364,6 +367,7 @@ func TestCheckUserAssignedIdentities(t *testing.T) {
 			specIdentities:   []infrav1.UserAssignedIdentity{fakeUserAssignedIdentity},
 			actualIdentities: []infrav1.UserAssignedIdentity{fakeUserAssignedIdentity2},
 			expect: func(s *mock_virtualmachines.MockVMScopeMockRecorder, i *mock_identities.MockClientMockRecorder) {
+				s.SubscriptionID().Return("123")
 				i.GetClientID(gomockinternal.AContext(), fakeUserAssignedIdentity.ProviderID).AnyTimes().Return(fakeUserAssignedIdentity.ProviderID, nil)
 				s.SetConditionFalse(infrav1.VMIdentitiesReadyCondition, infrav1.UserAssignedIdentityMissingReason, clusterv1.ConditionSeverityWarning, vmMissingUAI+fakeUserAssignedIdentity.ProviderID).Times(1)
 			},
@@ -374,6 +378,7 @@ func TestCheckUserAssignedIdentities(t *testing.T) {
 			specIdentities:   []infrav1.UserAssignedIdentity{fakeUserAssignedIdentity, fakeUserAssignedIdentity},
 			actualIdentities: []infrav1.UserAssignedIdentity{fakeUserAssignedIdentity},
 			expect: func(s *mock_virtualmachines.MockVMScopeMockRecorder, i *mock_identities.MockClientMockRecorder) {
+				s.SubscriptionID().AnyTimes().Return("123")
 				i.GetClientID(gomockinternal.AContext(), fakeUserAssignedIdentity.ProviderID).AnyTimes().Return(fakeUserAssignedIdentity.ProviderID, nil)
 			},
 			expectedError: "",
@@ -383,6 +388,7 @@ func TestCheckUserAssignedIdentities(t *testing.T) {
 			specIdentities:   []infrav1.UserAssignedIdentity{fakeUserAssignedIdentity},
 			actualIdentities: []infrav1.UserAssignedIdentity{fakeUserAssignedIdentity},
 			expect: func(s *mock_virtualmachines.MockVMScopeMockRecorder, i *mock_identities.MockClientMockRecorder) {
+				s.SubscriptionID().Return("123")
 				i.GetClientID(gomockinternal.AContext(), fakeUserAssignedIdentity.ProviderID).AnyTimes().Return("", errors.New("failed to get client id"))
 			},
 			expectedError: "failed to get client id",

controllers/azurejson_machine_controller.go

@@ -31,6 +31,7 @@ import (
 	infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1"
 	"sigs.k8s.io/cluster-api-provider-azure/azure/scope"
 	"sigs.k8s.io/cluster-api-provider-azure/azure/services/identities"
+	azureutil "sigs.k8s.io/cluster-api-provider-azure/util/azure"
 	"sigs.k8s.io/cluster-api-provider-azure/util/reconciler"
 	"sigs.k8s.io/cluster-api-provider-azure/util/tele"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
@@ -215,11 +216,22 @@ func (r *AzureJSONMachineReconciler) Reconcile(ctx context.Context, req ctrl.Req
 	// Construct secret for this machine
 	userAssignedIdentityIfExists := ""
 	if len(azureMachine.Spec.UserAssignedIdentities) > 0 {
-		idsClient, err := identities.NewClient(clusterScope)
+		var identitiesClient identities.Client
+		identitiesClient, err := identities.NewClient(clusterScope)
 		if err != nil {
 			return reconcile.Result{}, errors.Wrap(err, "failed to create identities client")
 		}
-		userAssignedIdentityIfExists, err = idsClient.GetClientID(
+		parsed, err := azureutil.ParseResourceID(azureMachine.Spec.UserAssignedIdentities[0].ProviderID)
+		if err != nil {
+			return reconcile.Result{}, errors.Wrapf(err, "failed to parse ProviderID %s", azureMachine.Spec.UserAssignedIdentities[0].ProviderID)
+		}
+		if parsed.SubscriptionID != clusterScope.SubscriptionID() {
+			identitiesClient, err = identities.NewClientBySub(clusterScope, parsed.SubscriptionID)
+			if err != nil {
+				return reconcile.Result{}, errors.Wrapf(err, "failed to create identities client from subscription ID %s", parsed.SubscriptionID)
+			}
+		}
+		userAssignedIdentityIfExists, err = identitiesClient.GetClientID(
 			ctx, azureMachine.Spec.UserAssignedIdentities[0].ProviderID)
 		if err != nil {
 			return reconcile.Result{}, errors.Wrap(err, "failed to get user-assigned identity ClientID")

controllers/azurejson_machinepool_controller.go

@@ -29,6 +29,7 @@ import (
 	infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1"
 	"sigs.k8s.io/cluster-api-provider-azure/azure/services/identities"
 	infrav1exp "sigs.k8s.io/cluster-api-provider-azure/exp/api/v1beta1"
+	azureutil "sigs.k8s.io/cluster-api-provider-azure/util/azure"
 	"sigs.k8s.io/cluster-api-provider-azure/util/reconciler"
 	"sigs.k8s.io/cluster-api-provider-azure/util/tele"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
@@ -148,11 +149,22 @@ func (r *AzureJSONMachinePoolReconciler) Reconcile(ctx context.Context, req ctrl
 	// Construct secret for this machine
 	userAssignedIdentityIfExists := ""
 	if len(azureMachinePool.Spec.UserAssignedIdentities) > 0 {
-		idsClient, err := getClient(clusterScope)
+		var identitiesClient identities.Client
+		identitiesClient, err := getClient(clusterScope)
 		if err != nil {
 			return reconcile.Result{}, errors.Wrap(err, "failed to create identities client")
 		}
-		userAssignedIdentityIfExists, err = idsClient.GetClientID(
+		parsed, err := azureutil.ParseResourceID(azureMachinePool.Spec.UserAssignedIdentities[0].ProviderID)
+		if err != nil {
+			return reconcile.Result{}, errors.Wrapf(err, "failed to parse ProviderID %s", azureMachinePool.Spec.UserAssignedIdentities[0].ProviderID)
+		}
+		if parsed.SubscriptionID != clusterScope.SubscriptionID() {
+			identitiesClient, err = identities.NewClientBySub(clusterScope, parsed.SubscriptionID)
+			if err != nil {
+				return reconcile.Result{}, errors.Wrapf(err, "failed to create identities client from subscription ID %s", parsed.SubscriptionID)
+			}
+		}
+		userAssignedIdentityIfExists, err = identitiesClient.GetClientID(
 			ctx, azureMachinePool.Spec.UserAssignedIdentities[0].ProviderID)
 		if err != nil {
 			return reconcile.Result{}, errors.Wrap(err, "failed to get user-assigned identity ClientID")

controllers/azurejson_machinepool_controller_test.go

@@ -271,7 +271,7 @@ func TestAzureJSONPoolReconcilerUserAssignedIdentities(t *testing.T) {
 		Spec: infrav1exp.AzureMachinePoolSpec{
 			UserAssignedIdentities: []infrav1.UserAssignedIdentity{
 				{
-					ProviderID: "fake-id",
+					ProviderID: "azure:///subscriptions/123/resourceGroups/test-rg/providers/Microsoft.ManagedIdentity/userAssignedIdentities/fake-provider-id",
 				},
 			},
 		},
@@ -378,7 +378,7 @@ func TestAzureJSONPoolReconcilerUserAssignedIdentities(t *testing.T) {
 		Recorder: record.NewFakeRecorder(42),
 		Timeouts: reconciler.Timeouts{},
 	}
-	id := "fake-id"
+	id := "azure:///subscriptions/123/resourceGroups/test-rg/providers/Microsoft.ManagedIdentity/userAssignedIdentities/fake-provider-id"
 	getClient = func(auth azure.Authorizer) (identities.Client, error) {
 		mockClient := mock_identities.NewMockClient(ctrlr)
 		mockClient.EXPECT().GetClientID(gomock.Any(), gomock.Any()).Return(id, nil)

controllers/azurejson_machinetemplate_controller.go

@@ -30,6 +30,7 @@ import (
 	infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1"
 	"sigs.k8s.io/cluster-api-provider-azure/azure/scope"
 	"sigs.k8s.io/cluster-api-provider-azure/azure/services/identities"
+	azureutil "sigs.k8s.io/cluster-api-provider-azure/util/azure"
 	"sigs.k8s.io/cluster-api-provider-azure/util/reconciler"
 	"sigs.k8s.io/cluster-api-provider-azure/util/tele"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
@@ -175,11 +176,22 @@ func (r *AzureJSONTemplateReconciler) Reconcile(ctx context.Context, req ctrl.Re
 	// Construct secret for this machine template
 	userAssignedIdentityIfExists := ""
 	if len(azureMachineTemplate.Spec.Template.Spec.UserAssignedIdentities) > 0 {
-		idsClient, err := identities.NewClient(clusterScope)
+		var identitiesClient identities.Client
+		identitiesClient, err := identities.NewClient(clusterScope)
 		if err != nil {
 			return reconcile.Result{}, errors.Wrap(err, "failed to create identities client")
 		}
-		userAssignedIdentityIfExists, err = idsClient.GetClientID(
+		parsed, err := azureutil.ParseResourceID(azureMachineTemplate.Spec.Template.Spec.UserAssignedIdentities[0].ProviderID)
+		if err != nil {
+			return reconcile.Result{}, errors.Wrapf(err, "failed to parse ProviderID %s", azureMachineTemplate.Spec.Template.Spec.UserAssignedIdentities[0].ProviderID)
+		}
+		if parsed.SubscriptionID != clusterScope.SubscriptionID() {
+			identitiesClient, err = identities.NewClientBySub(clusterScope, parsed.SubscriptionID)
+			if err != nil {
+				return reconcile.Result{}, errors.Wrapf(err, "failed to create identities client from subscription ID %s", parsed.SubscriptionID)
+			}
+		}
+		userAssignedIdentityIfExists, err = identitiesClient.GetClientID(
 			ctx, azureMachineTemplate.Spec.Template.Spec.UserAssignedIdentities[0].ProviderID)
 		if err != nil {
 			return reconcile.Result{}, errors.Wrap(err, "failed to get user-assigned identity ClientID")