chore(roleassignment): make roleassignments service async

Signed-off-by: Ashutosh Kumar <[email protected]>

azure/defaults.go

@@ -82,6 +82,9 @@ const (
 	// ProviderIDPrefix will be appended to the beginning of Azure resource IDs to form the Kubernetes Provider ID.
 	// NOTE: this format matches the 2 slashes format used in cloud-provider and cluster-autoscaler.
 	ProviderIDPrefix = "azure://"
+	// azureBuiltInContributorID the ID of the Contributor role in Azure
+	// Ref: https://docs.microsoft.com/en-us/azure/role-based-access-control/built-in-roles
+	azureBuiltInContributorID = "b24988ac-6180-42a0-ab88-20f7382dd24c"
 )
 
 const (
@@ -105,6 +108,16 @@ func GenerateBackendAddressPoolName(lbName string) string {
 	return fmt.Sprintf("%s-%s", lbName, "backendPool")
 }
 
+// GenerateSubscriptionScope generates a role assignment scope that applies to all resources in the subscription.
+func GenerateSubscriptionScope(subscriptionID string) string {
+	return fmt.Sprintf("/subscriptions/%s/", subscriptionID)
+}
+
+// GenerateContributorRoleDefinitionID generates the contributor role definition ID.
+func GenerateContributorRoleDefinitionID(subscriptionID string) string {
+	return fmt.Sprintf("/subscriptions/%s/providers/Microsoft.Authorization/roleDefinitions/%s", subscriptionID, azureBuiltInContributorID)
+}
+
 // GenerateOutboundBackendAddressPoolName generates a load balancer outbound backend address pool name.
 func GenerateOutboundBackendAddressPoolName(lbName string) string {
 	return fmt.Sprintf("%s-%s", lbName, "outboundBackendPool")

azure/scope/machine.go

@@ -35,6 +35,7 @@ import (
 	"sigs.k8s.io/cluster-api-provider-azure/azure/services/inboundnatrules"
 	"sigs.k8s.io/cluster-api-provider-azure/azure/services/networkinterfaces"
 	"sigs.k8s.io/cluster-api-provider-azure/azure/services/resourceskus"
+	"sigs.k8s.io/cluster-api-provider-azure/azure/services/roleassignments"
 	"sigs.k8s.io/cluster-api-provider-azure/azure/services/virtualmachines"
 	"sigs.k8s.io/cluster-api-provider-azure/util/futures"
 	"sigs.k8s.io/cluster-api-provider-azure/util/tele"
@@ -288,17 +289,32 @@ func (m *MachineScope) DiskSpecs() []azure.ResourceSpecGetter {
 }
 
 // RoleAssignmentSpecs returns the role assignment specs.
-func (m *MachineScope) RoleAssignmentSpecs() []azure.RoleAssignmentSpec {
-	if m.AzureMachine.Spec.Identity == infrav1.VMIdentitySystemAssigned {
-		return []azure.RoleAssignmentSpec{
-			{
-				MachineName:  m.Name(),
-				Name:         m.AzureMachine.Spec.RoleAssignmentName,
-				ResourceType: azure.VirtualMachine,
-			},
+func (m *MachineScope) RoleAssignmentSpecs(principalID *string) []azure.ResourceSpecGetter {
+	roles := make([]azure.ResourceSpecGetter, 1)
+	if m.HasSystemAssignedIdentity() {
+		roles[0] = &roleassignments.RoleAssignmentSpec{
+			Name:             m.AzureMachine.Spec.RoleAssignmentName,
+			MachineName:      m.Name(),
+			ResourceType:     azure.VirtualMachine,
+			ResourceGroup:    m.ResourceGroup(),
+			Scope:            azure.GenerateSubscriptionScope(m.SubscriptionID()),
+			RoleDefinitionID: azure.GenerateContributorRoleDefinitionID(m.SubscriptionID()),
+			PrincipalID:      principalID,
 		}
+		return roles
 	}
-	return []azure.RoleAssignmentSpec{}
+	return []azure.ResourceSpecGetter{}
+}
+
+// RoleAssignmentResourceType returns the role assignment resource type.
+func (m *MachineScope) RoleAssignmentResourceType() string {
+	return azure.VirtualMachine
+}
+
+// HasSystemAssignedIdentity returns true if the azure machine has
+// system assigned identity.
+func (m *MachineScope) HasSystemAssignedIdentity() bool {
+	return m.AzureMachine.Spec.Identity == infrav1.VMIdentitySystemAssigned
 }
 
 // VMExtensionSpecs returns the vm extension specs.

azure/scope/machine_test.go

@@ -35,6 +35,7 @@ import (
 	"sigs.k8s.io/cluster-api-provider-azure/azure/services/inboundnatrules"
 	"sigs.k8s.io/cluster-api-provider-azure/azure/services/networkinterfaces"
 	"sigs.k8s.io/cluster-api-provider-azure/azure/services/resourceskus"
+	"sigs.k8s.io/cluster-api-provider-azure/azure/services/roleassignments"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 )
 
@@ -381,7 +382,7 @@ func TestMachineScope_RoleAssignmentSpecs(t *testing.T) {
 	tests := []struct {
 		name         string
 		machineScope MachineScope
-		want         []azure.RoleAssignmentSpec
+		want         []azure.ResourceSpecGetter
 	}{
 		{
 			name: "returns empty if VM identity is system assigned",
@@ -393,7 +394,7 @@ func TestMachineScope_RoleAssignmentSpecs(t *testing.T) {
 					},
 				},
 			},
-			want: []azure.RoleAssignmentSpec{},
+			want: []azure.ResourceSpecGetter{},
 		},
 		{
 			name: "returns RoleAssignmentSpec if VM identity is not system assigned",
@@ -408,19 +409,37 @@ func TestMachineScope_RoleAssignmentSpecs(t *testing.T) {
 						RoleAssignmentName: "azure-role-assignment-name",
 					},
 				},
+				ClusterScoper: &ClusterScope{
+					AzureClients: AzureClients{
+						EnvironmentSettings: auth.EnvironmentSettings{
+							Values: map[string]string{
+								auth.SubscriptionID: "123",
+							},
+						},
+					},
+					AzureCluster: &infrav1.AzureCluster{
+						Spec: infrav1.AzureClusterSpec{
+							ResourceGroup: "my-rg",
+						},
+					},
+				},
 			},
-			want: []azure.RoleAssignmentSpec{
-				{
-					MachineName:  "machine-name",
-					Name:         "azure-role-assignment-name",
-					ResourceType: azure.VirtualMachine,
+			want: []azure.ResourceSpecGetter{
+				&roleassignments.RoleAssignmentSpec{
+					ResourceType:     azure.VirtualMachine,
+					MachineName:      "machine-name",
+					Name:             "azure-role-assignment-name",
+					ResourceGroup:    "my-rg",
+					Scope:            azure.GenerateSubscriptionScope("123"),
+					RoleDefinitionID: azure.GenerateContributorRoleDefinitionID("123"),
+					PrincipalID:      to.StringPtr("fakePrincipalID"),
 				},
 			},
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			if got := tt.machineScope.RoleAssignmentSpecs(); !reflect.DeepEqual(got, tt.want) {
+			if got := tt.machineScope.RoleAssignmentSpecs(to.StringPtr("fakePrincipalID")); !reflect.DeepEqual(got, tt.want) {
 				t.Errorf("RoleAssignmentSpecs() = %v, want %v", got, tt.want)
 			}
 		})

azure/scope/machinepool.go

@@ -31,6 +31,7 @@ import (
 	infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1"
 	"sigs.k8s.io/cluster-api-provider-azure/azure"
 	machinepool "sigs.k8s.io/cluster-api-provider-azure/azure/scope/strategies/machinepool_deployments"
+	"sigs.k8s.io/cluster-api-provider-azure/azure/services/roleassignments"
 	infrav1exp "sigs.k8s.io/cluster-api-provider-azure/exp/api/v1beta1"
 	"sigs.k8s.io/cluster-api-provider-azure/util/futures"
 	"sigs.k8s.io/cluster-api-provider-azure/util/tele"
@@ -569,17 +570,30 @@ func (m *MachinePoolScope) SaveVMImageToStatus(image *infrav1.Image) {
 }
 
 // RoleAssignmentSpecs returns the role assignment specs.
-func (m *MachinePoolScope) RoleAssignmentSpecs() []azure.RoleAssignmentSpec {
-	if m.AzureMachinePool.Spec.Identity == infrav1.VMIdentitySystemAssigned {
-		return []azure.RoleAssignmentSpec{
-			{
-				MachineName:  m.Name(),
-				Name:         m.AzureMachinePool.Spec.RoleAssignmentName,
-				ResourceType: azure.VirtualMachineScaleSet,
-			},
+func (m *MachinePoolScope) RoleAssignmentSpecs(principalID *string) []azure.ResourceSpecGetter {
+	roles := make([]azure.ResourceSpecGetter, 1)
+	if m.HasSystemAssignedIdentity() {
+		roles[0] = &roleassignments.RoleAssignmentSpec{
+			Name:          m.AzureMachinePool.Spec.RoleAssignmentName,
+			MachineName:   m.Name(),
+			ResourceGroup: m.ResourceGroup(),
+			ResourceType:  azure.VirtualMachineScaleSet,
+			PrincipalID:   principalID,
 		}
+		return roles
 	}
-	return []azure.RoleAssignmentSpec{}
+	return []azure.ResourceSpecGetter{}
+}
+
+// RoleAssignmentResourceType returns the role assignment resource type.
+func (m *MachinePoolScope) RoleAssignmentResourceType() string {
+	return azure.VirtualMachineScaleSet
+}
+
+// HasSystemAssignedIdentity returns true if the azure machine pool has system
+// assigned identity.
+func (m *MachinePoolScope) HasSystemAssignedIdentity() bool {
+	return m.AzureMachinePool.Spec.Identity == infrav1.VMIdentitySystemAssigned
 }
 
 // VMSSExtensionSpecs returns the vmss extension specs.

azure/services/roleassignments/client.go

@@ -21,22 +21,17 @@ import (
 
 	"github.com/Azure/azure-sdk-for-go/profiles/2019-03-01/authorization/mgmt/authorization"
 	"github.com/Azure/go-autorest/autorest"
+	azureautorest "github.com/Azure/go-autorest/autorest/azure"
+	"github.com/pkg/errors"
 	"sigs.k8s.io/cluster-api-provider-azure/azure"
 	"sigs.k8s.io/cluster-api-provider-azure/util/tele"
 )
 
-// client wraps go-sdk.
-type client interface {
-	Create(context.Context, string, string, authorization.RoleAssignmentCreateParameters) (authorization.RoleAssignment, error)
-}
-
 // azureClient contains the Azure go-sdk Client.
 type azureClient struct {
 	roleassignments authorization.RoleAssignmentsClient
 }
 
-var _ client = (*azureClient)(nil)
-
 // newClient creates a new role assignment client from subscription ID.
 func newClient(auth azure.Authorizer) *azureClient {
 	c := newRoleAssignmentClient(auth.SubscriptionID(), auth.BaseURI(), auth.Authorizer())
@@ -50,18 +45,45 @@ func newRoleAssignmentClient(subscriptionID string, baseURI string, authorizer a
 	return roleClient
 }
 
-// Create creates a role assignment.
-// Parameters:
-// scope - the scope of the role assignment to create. The scope can be any REST resource instance. For
-// example, use '/subscriptions/{subscription-id}/' for a subscription,
-// '/subscriptions/{subscription-id}/resourceGroups/{resource-group-name}' for a resource group, and
-// '/subscriptions/{subscription-id}/resourceGroups/{resource-group-name}/providers/{resource-provider}/{resource-type}/{resource-name}'
-// for a resource.
-// roleAssignmentName - the name of the role assignment to create. It can be any valid GUID.
-// parameters - parameters for the role assignment.
-func (ac *azureClient) Create(ctx context.Context, scope string, roleAssignmentName string, parameters authorization.RoleAssignmentCreateParameters) (authorization.RoleAssignment, error) {
-	ctx, _, done := tele.StartSpanWithLogger(ctx, "roleassignments.AzureClient.Create")
+// Get gets the specified role assignment by the role assignment name.
+func (ac *azureClient) Get(ctx context.Context, spec azure.ResourceSpecGetter) (interface{}, error) {
+	ctx, span := tele.Tracer().Start(ctx, "roleassignments.AzureClient.Get")
+	defer span.End()
+	return ac.roleassignments.Get(ctx, spec.OwnerResourceName(), spec.ResourceName())
+}
+
+// CreateOrUpdateAsync creates a roleassignment.
+// Creating a roleassignment is not a long running operation, so we don't ever return a future.
+func (ac *azureClient) CreateOrUpdateAsync(ctx context.Context, spec azure.ResourceSpecGetter, parameters interface{}) (interface{}, azureautorest.FutureAPI, error) {
+	ctx, _, done := tele.StartSpanWithLogger(ctx, "groups.AzureClient.CreateOrUpdate")
+	defer done()
+	createParams, ok := parameters.(authorization.RoleAssignmentCreateParameters)
+	if !ok {
+		return nil, nil, errors.Errorf("%T is not a authorization.RoleAssignmentCreateParameters", parameters)
+	}
+	result, err := ac.roleassignments.Create(ctx, spec.OwnerResourceName(), spec.ResourceName(), createParams)
+	return result, nil, err
+}
+
+// IsDone returns true if the long-running operation has completed.
+func (ac *azureClient) IsDone(ctx context.Context, future azureautorest.FutureAPI) (bool, error) {
+	ctx, _, done := tele.StartSpanWithLogger(ctx, "roleassignments.AzureClient.IsDone")
 	defer done()
 
-	return ac.roleassignments.Create(ctx, scope, roleAssignmentName, parameters)
+	isDone, err := future.DoneWithContext(ctx, ac.roleassignments)
+	if err != nil {
+		return false, errors.Wrap(err, "failed checking if the operation was complete")
+	}
+	return isDone, nil
+}
+
+// Result fetches the result of a long-running operation future.
+func (ac *azureClient) Result(ctx context.Context, futureData azureautorest.FutureAPI, futureType string) (interface{}, error) {
+	// Result is a no-op for role assignment as only Delete operations return a future.
+	return nil, nil
+}
+
+// DeleteAsync is no-op for role assignments. It gets deleted as part of the VM deletion.
+func (ac *azureClient) DeleteAsync(ctx context.Context, spec azure.ResourceSpecGetter) (azureautorest.FutureAPI, error) {
+	return nil, nil
 }