Skip to content

Commit

Permalink
Merge pull request #4716 from k8s-infra-cherrypick-robot/cherry-pick-…
Browse files Browse the repository at this point in the history
…4712-to-release-1.13

[release-1.13] enable per-sub msi client
  • Loading branch information
jackfrancis authored Apr 10, 2024
2 parents fcb6264 + ef3d7bf commit eaef537
Show file tree
Hide file tree
Showing 7 changed files with 77 additions and 11 deletions.
13 changes: 13 additions & 0 deletions azure/services/identities/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
13 changes: 12 additions & 1 deletion azure/services/virtualmachines/virtualmachines.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,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")
}
Expand Down
10 changes: 8 additions & 2 deletions azure/services/virtualmachines/virtualmachines_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
}
)

Expand Down Expand Up @@ -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: "",
Expand All @@ -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, "VM is missing expected user assigned identity with client ID: "+fakeUserAssignedIdentity2.ProviderID).Times(1)
Expand All @@ -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: "",
Expand All @@ -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, "VM is missing expected user assigned identity with client ID: "+fakeUserAssignedIdentity.ProviderID).Times(1)
},
Expand All @@ -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: "",
Expand All @@ -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",
Expand Down
16 changes: 14 additions & 2 deletions controllers/azurejson_machine_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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")
Expand Down
16 changes: 14 additions & 2 deletions controllers/azurejson_machinepool_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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")
Expand Down
4 changes: 2 additions & 2 deletions controllers/azurejson_machinepool_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
},
},
},
Expand Down Expand Up @@ -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)
Expand Down
16 changes: 14 additions & 2 deletions controllers/azurejson_machinetemplate_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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")
Expand Down

0 comments on commit eaef537

Please sign in to comment.