From 35426b59f63fb56239c037342d6c2bc8bbe3c069 Mon Sep 17 00:00:00 2001 From: Ashutosh Kumar Date: Wed, 17 Nov 2021 14:29:26 +0530 Subject: [PATCH] chore(role_assignment): make roleassignment reconcile async Signed-off-by: Ashutosh Kumar --- azure/scope/machine.go | 13 +++- azure/services/roleassignments/client.go | 72 +++++++++++++++---- .../roleassignments/roleassignments.go | 48 ++++++------- azure/services/roleassignments/spec.go | 58 +++++++++++++++ 4 files changed, 148 insertions(+), 43 deletions(-) create mode 100644 azure/services/roleassignments/spec.go diff --git a/azure/scope/machine.go b/azure/scope/machine.go index 27ee66467035..53685e92ac9a 100644 --- a/azure/scope/machine.go +++ b/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,22 @@ func (m *MachineScope) DiskSpecs() []azure.DiskSpec { } // RoleAssignmentSpecs returns the role assignment specs. -func (m *MachineScope) RoleAssignmentSpecs() []azure.RoleAssignmentSpec { +// ToDO: Complete the function +func (m *MachineScope) RoleAssignmentSpecs() []azure.ResourceSpecGetter { + roles := make([]azure.ResourceSpecGetter, 1) if m.AzureMachine.Spec.Identity == infrav1.VMIdentitySystemAssigned { - return []azure.RoleAssignmentSpec{ + //spec := &roleassignments.RoleAssignmentSpec{} + rm := []roleassignments.RoleAssignmentSpec{ { MachineName: m.Name(), Name: m.AzureMachine.Spec.RoleAssignmentName, ResourceType: azure.VirtualMachine, }, } + roles[0] = &rm[0] + return roles } - return []azure.RoleAssignmentSpec{} + return []azure.ResourceSpecGetter{} } // VMExtensionSpecs returns the vm extension specs. diff --git a/azure/services/roleassignments/client.go b/azure/services/roleassignments/client.go index 7c8fb032f2c6..bbae0c28a7d5 100644 --- a/azure/services/roleassignments/client.go +++ b/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,59 @@ 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.RoleAssignmentCreateParameters) + 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) + } + + result, err := ac.roleassignments.Create(ctx, spec.ResourceGroupName(), spec.ResourceName(), roleassignment) + 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 resource groups as only Delete operations return a future. + return nil, nil } diff --git a/azure/services/roleassignments/roleassignments.go b/azure/services/roleassignments/roleassignments.go index e0a51c5718d7..334f0337fa64 100644 --- a/azure/services/roleassignments/roleassignments.go +++ b/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 } diff --git a/azure/services/roleassignments/spec.go b/azure/services/roleassignments/spec.go new file mode 100644 index 000000000000..402e3568ca52 --- /dev/null +++ b/azure/services/roleassignments/spec.go @@ -0,0 +1,58 @@ +package roleassignments + +import ( + "fmt" + + "github.com/Azure/azure-sdk-for-go/profiles/2019-03-01/authorization/mgmt/authorization" + "github.com/Azure/go-autorest/autorest/to" + "github.com/pkg/errors" +) + +// RoleAssignmentSpec defines the specification for a Role Assignment. +type RoleAssignmentSpec struct { + RoleAssignmentName string + ResourceGroup string + MachineName string + Name string + ResourceType string + SubscriptionID string + PrincipalID *string +} + +// ResourceName returns the name of the role assignment. +func (s *RoleAssignmentSpec) ResourceName() string { + return s.RoleAssignmentName +} + +// ResourceGroupName returns the name of the resource group. +func (s *RoleAssignmentSpec) ResourceGroupName() string { + return s.ResourceGroup +} + +// OwnerResourceName is a no-op for role assignment. +func (s *RoleAssignmentSpec) OwnerResourceName() string { + return "" +} + +// Parameters returns the parameters for the RoleAssignmentSpec. +func (s *RoleAssignmentSpec) Parameters(existing interface{}) (interface{}, error) { + if existing != nil { + if _, ok := existing.(authorization.RoleAssignment); !ok { + return nil, errors.Errorf("%T is not a authorization.RoleAssignment", existing) + } + // RoleAssignmentSpec already exists + return nil, nil + } + scope := fmt.Sprintf("/subscriptions/%s/", s.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.SubscriptionID, azureBuiltInContributorID) + properties := &authorization.RoleAssignmentPropertiesWithScope{ + Scope: to.StringPtr(scope), + RoleDefinitionID: to.StringPtr(contributorRoleDefinitionID), + PrincipalID: s.PrincipalID, + } + return authorization.RoleAssignment{ + Name: to.StringPtr(s.RoleAssignmentName), + Properties: properties, + }, nil +}