Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(role_assignment): make roleassignment reconcile async #1873

Merged
merged 1 commit into from
Mar 28, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions azure/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
CecileRobertMichon marked this conversation as resolved.
Show resolved Hide resolved
)

const (
Expand All @@ -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")
Expand Down
34 changes: 25 additions & 9 deletions azure/scope/machine.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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{
sonasingh46 marked this conversation as resolved.
Show resolved Hide resolved
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.
Expand Down
35 changes: 27 additions & 8 deletions azure/scope/machine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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",
Expand All @@ -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",
Expand All @@ -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"),
sonasingh46 marked this conversation as resolved.
Show resolved Hide resolved
},
},
},
}
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)
}
})
Expand Down
32 changes: 23 additions & 9 deletions azure/scope/machinepool.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"
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"
Expand Down Expand Up @@ -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.
Expand Down
60 changes: 41 additions & 19 deletions azure/services/roleassignments/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand All @@ -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
}
46 changes: 0 additions & 46 deletions azure/services/roleassignments/mock_roleassignments/client_mock.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading