chore(role_assignment): make roleassignment reconcile async

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

azure/scope/machine.go

@@ -23,6 +23,8 @@ import (
 	"strings"
 	"time"
 
+	"sigs.k8s.io/cluster-api-provider-azure/azure/services/roleassignments"
+
 	"github.com/Azure/go-autorest/autorest/to"
 	"github.com/go-logr/logr"
 	"github.com/pkg/errors"
@@ -265,17 +267,17 @@ func (m *MachineScope) DiskSpecs() []azure.DiskSpec {
 }
 
 // RoleAssignmentSpecs returns the role assignment specs.
-func (m *MachineScope) RoleAssignmentSpecs() []azure.RoleAssignmentSpec {
+func (m *MachineScope) RoleAssignmentSpecs() []azure.ResourceSpecGetter {
+	roles := make([]azure.ResourceSpecGetter, 1)
+	spec := &roleassignments.RoleAssignmentSpec{}
 	if m.AzureMachine.Spec.Identity == infrav1.VMIdentitySystemAssigned {
-		return []azure.RoleAssignmentSpec{
-			{
-				MachineName:  m.Name(),
-				Name:         m.AzureMachine.Spec.RoleAssignmentName,
-				ResourceType: azure.VirtualMachine,
-			},
-		}
+		spec.MachineName = m.Name()
+		spec.Name = m.AzureMachine.Spec.RoleAssignmentName
+		spec.ResourceType = azure.VirtualMachine
+		return roles
 	}
-	return []azure.RoleAssignmentSpec{}
+	roles[0] = spec
+	return roles
 }
 
 // VMExtensionSpecs returns the vm extension specs.

azure/scope/machinepool.go

@@ -22,6 +22,8 @@ import (
 	"strings"
 	"time"
 
+	"sigs.k8s.io/cluster-api-provider-azure/azure/services/roleassignments"
+
 	"sigs.k8s.io/cluster-api-provider-azure/util/futures"
 
 	"github.com/Azure/go-autorest/autorest/to"
@@ -585,17 +587,17 @@ func (m *MachinePoolScope) SaveVMImageToStatus(image *infrav1.Image) {
 }
 
 // RoleAssignmentSpecs returns the role assignment specs.
-func (m *MachinePoolScope) RoleAssignmentSpecs() []azure.RoleAssignmentSpec {
+func (m *MachinePoolScope) RoleAssignmentSpecs() []azure.ResourceSpecGetter {
+	roles := make([]azure.ResourceSpecGetter, 1)
+	spec := &roleassignments.RoleAssignmentSpec{}
 	if m.AzureMachinePool.Spec.Identity == infrav1.VMIdentitySystemAssigned {
-		return []azure.RoleAssignmentSpec{
-			{
-				MachineName:  m.Name(),
-				Name:         m.AzureMachinePool.Spec.RoleAssignmentName,
-				ResourceType: azure.VirtualMachineScaleSet,
-			},
-		}
+		spec.MachineName = m.Name()
+		spec.Name = m.AzureMachinePool.Spec.RoleAssignmentName
+		spec.ResourceType = azure.VirtualMachineScaleSet
+		return roles
 	}
-	return []azure.RoleAssignmentSpec{}
+	roles[0] = spec
+	return roles
 }
 
 // VMSSExtensionSpecs returns the vmss extension specs.

azure/services/roleassignments/client.go

@@ -19,6 +19,10 @@ package roleassignments
 import (
 	"context"
 
+	"github.com/pkg/errors"
+
+	azureautorest "github.com/Azure/go-autorest/autorest/azure"
+
 	"github.com/Azure/azure-sdk-for-go/profiles/2019-03-01/authorization/mgmt/authorization"
 	"github.com/Azure/go-autorest/autorest"
 
@@ -28,7 +32,10 @@ import (
 
 // client wraps go-sdk.
 type client interface {
-	Create(context.Context, string, string, authorization.RoleAssignmentCreateParameters) (authorization.RoleAssignment, error)
+	Get(context.Context, string, string) (authorization.RoleAssignment, error)
+	CreateOrUpdateAsync(context.Context, azure.ResourceSpecGetter) (interface{}, azureautorest.FutureAPI, error)
+	IsDone(context.Context, azureautorest.FutureAPI) (bool, error)
+	Result(context.Context, azureautorest.FutureAPI, string) (interface{}, error)
 }
 
 // azureClient contains the Azure go-sdk Client.
@@ -51,18 +58,65 @@ 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) {
+// Get gets the specified role assignment by the role assignment name.
+func (ac *azureClient) Get(ctx context.Context, scope, roleAssignment string) (authorization.RoleAssignment, error) {
+	ctx, span := tele.Tracer().Start(ctx, "roleassignments.AzureClient.Get")
+	defer span.End()
+	return ac.roleassignments.Get(ctx, scope, roleAssignment)
+}
+
+// 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) (interface{}, azureautorest.FutureAPI, error) {
 	ctx, _, done := tele.StartSpanWithLogger(ctx, "roleassignments.AzureClient.Create")
 	defer done()
+	var existingRoleassignment interface{}
+	if existing, err := ac.Get(ctx, spec.ResourceGroupName(), spec.ResourceName()); err != nil && !azure.ResourceNotFound(err) {
+		return nil, nil, errors.Wrapf(err, "failed to get virtual network peering %s for %s in %s", spec.ResourceName(), spec.OwnerResourceName(), spec.ResourceGroupName())
+	} else if err == nil {
+		existingRoleassignment = existing
+	}
+
+	params, err := spec.Parameters(existingRoleassignment)
+	if err != nil {
+		return nil, nil, errors.Wrapf(err, "failed to get desired parameters for virtual network peering %s", spec.ResourceName())
+	}
+
+	roleassignment, ok := params.(authorization.RoleAssignmentPropertiesWithScope)
+	if !ok {
+		if params == nil {
+			// nothing to do here.
+			return existingRoleassignment, nil, nil
+		}
+		return nil, nil, errors.Errorf("%T is not a authorization.RoleAssignment", params)
+	}
+	scopeName := roleassignment.Scope
+
+	roleAssignmentCreateParams := authorization.RoleAssignmentCreateParameters{
+		Properties: &authorization.RoleAssignmentProperties{
+			RoleDefinitionID: roleassignment.RoleDefinitionID,
+			PrincipalID:      roleassignment.PrincipalID,
+		},
+	}
+	result, err := ac.roleassignments.Create(ctx, *scopeName, spec.ResourceName(), roleAssignmentCreateParams)
+	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()
+
+	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
+}
 
-	return ac.roleassignments.Create(ctx, scope, roleAssignmentName, parameters)
+// 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
 }

azure/services/roleassignments/roleassignments.go

@@ -18,10 +18,9 @@ package roleassignments
 
 import (
 	"context"
-	"fmt"
 
-	"github.com/Azure/azure-sdk-for-go/profiles/2019-03-01/authorization/mgmt/authorization"
-	"github.com/Azure/go-autorest/autorest/to"
+	"sigs.k8s.io/cluster-api-provider-azure/azure/services/async"
+
 	"github.com/go-logr/logr"
 	"github.com/pkg/errors"
 
@@ -32,12 +31,14 @@ import (
 )
 
 const azureBuiltInContributorID = "b24988ac-6180-42a0-ab88-20f7382dd24c"
+const serviceName = "roleassignments"
 
 // RoleAssignmentScope defines the scope interface for a role assignment service.
 type RoleAssignmentScope interface {
 	logr.Logger
 	azure.ClusterDescriber
-	RoleAssignmentSpecs() []azure.RoleAssignmentSpec
+	azure.AsyncStatusUpdater
+	RoleAssignmentSpecs() []azure.ResourceSpecGetter
 }
 
 // Service provides operations on Azure resources.
@@ -64,71 +65,62 @@ func (s *Service) Reconcile(ctx context.Context) error {
 	defer done()
 
 	for _, roleSpec := range s.Scope.RoleAssignmentSpecs() {
-		switch roleSpec.ResourceType {
+		rs := roleSpec.(*RoleAssignmentSpec)
+		switch rs.ResourceType {
 		case azure.VirtualMachine:
-			return s.reconcileVM(ctx, roleSpec)
+			return s.reconcileVM(ctx, rs.MachineName, roleSpec)
 		case azure.VirtualMachineScaleSet:
-			return s.reconcileVMSS(ctx, roleSpec)
+			return s.reconcileVMSS(ctx, rs.MachineName, roleSpec)
 		default:
-			return errors.Errorf("unexpected resource type %q. Expected one of [%s, %s]", roleSpec.ResourceType,
+			return errors.Errorf("unexpected resource type %q. Expected one of [%s, %s]", rs.ResourceType,
 				azure.VirtualMachine, azure.VirtualMachineScaleSet)
 		}
 	}
 	return nil
 }
 
-func (s *Service) reconcileVM(ctx context.Context, roleSpec azure.RoleAssignmentSpec) error {
+func (s *Service) reconcileVM(ctx context.Context, machineName string, roleSpec azure.ResourceSpecGetter) error {
 	ctx, _, done := tele.StartSpanWithLogger(ctx, "roleassignments.Service.reconcileVM")
 	defer done()
 
-	resultVM, err := s.virtualMachinesClient.Get(ctx, s.Scope.ResourceGroup(), roleSpec.MachineName)
+	resultVM, err := s.virtualMachinesClient.Get(ctx, s.Scope.ResourceGroup(), machineName)
 	if err != nil {
 		return errors.Wrap(err, "cannot get VM to assign role to system assigned identity")
 	}
 
-	err = s.assignRole(ctx, roleSpec.Name, resultVM.Identity.PrincipalID)
+	err = s.assignRole(ctx, roleSpec, resultVM.Identity.PrincipalID)
 	if err != nil {
 		return errors.Wrap(err, "cannot assign role to VM system assigned identity")
 	}
 
-	s.Scope.V(2).Info("successfully created role assignment for generated Identity for VM", "virtual machine", roleSpec.MachineName)
+	s.Scope.V(2).Info("successfully created role assignment for generated Identity for VM", "virtual machine", machineName)
 
 	return nil
 }
 
-func (s *Service) reconcileVMSS(ctx context.Context, roleSpec azure.RoleAssignmentSpec) error {
+func (s *Service) reconcileVMSS(ctx context.Context, machineName string, roleSpec azure.ResourceSpecGetter) error {
 	ctx, _, done := tele.StartSpanWithLogger(ctx, "roleassignments.Service.reconcileVMSS")
 	defer done()
 
-	resultVMSS, err := s.virtualMachineScaleSetClient.Get(ctx, s.Scope.ResourceGroup(), roleSpec.MachineName)
+	resultVMSS, err := s.virtualMachineScaleSetClient.Get(ctx, s.Scope.ResourceGroup(), machineName)
 	if err != nil {
 		return errors.Wrap(err, "cannot get VMSS to assign role to system assigned identity")
 	}
 
-	err = s.assignRole(ctx, roleSpec.Name, resultVMSS.Identity.PrincipalID)
+	err = s.assignRole(ctx, roleSpec, resultVMSS.Identity.PrincipalID)
 	if err != nil {
 		return errors.Wrap(err, "cannot assign role to VMSS system assigned identity")
 	}
 
-	s.Scope.V(2).Info("successfully created role assignment for generated Identity for VMSS", "virtual machine scale set", roleSpec.MachineName)
+	s.Scope.V(2).Info("successfully created role assignment for generated Identity for VMSS", "virtual machine scale set", machineName)
 
 	return nil
 }
 
-func (s *Service) assignRole(ctx context.Context, roleAssignmentName string, principalID *string) error {
+func (s *Service) assignRole(ctx context.Context, roleSpec azure.ResourceSpecGetter, principalID *string) error {
 	ctx, _, done := tele.StartSpanWithLogger(ctx, "roleassignments.Service.assignRole")
 	defer done()
-
-	scope := fmt.Sprintf("/subscriptions/%s/", s.Scope.SubscriptionID())
-	// Azure built-in roles https://docs.microsoft.com/en-us/azure/role-based-access-control/built-in-roles
-	contributorRoleDefinitionID := fmt.Sprintf("/subscriptions/%s/providers/Microsoft.Authorization/roleDefinitions/%s", s.Scope.SubscriptionID(), azureBuiltInContributorID)
-	params := authorization.RoleAssignmentCreateParameters{
-		Properties: &authorization.RoleAssignmentProperties{
-			RoleDefinitionID: to.StringPtr(contributorRoleDefinitionID),
-			PrincipalID:      principalID,
-		},
-	}
-	_, err := s.client.Create(ctx, scope, roleAssignmentName, params)
+	_, err := async.CreateResource(ctx, s.Scope, s.client, roleSpec, serviceName)
 	return err
 }