Skip to content

Commit 06b3e84

Browse files
authored
Merge pull request #17478 from hakman/azure-fix-role-assignment
azure: Fix small issues related to role assignments
2 parents c5881f5 + 5018458 commit 06b3e84

File tree

5 files changed

+27
-6
lines changed

5 files changed

+27
-6
lines changed

pkg/model/azuremodel/vmscaleset.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ func (b *VMScaleSetModelBuilder) Build(c *fi.CloudupModelBuilderContext) error {
6767
// See https://docs.microsoft.com/en-us/azure/role-based-access-control/built-in-roles
6868
resourceGroupID := fmt.Sprintf("/subscriptions/%s/resourceGroups/%s",
6969
b.Cluster.Spec.CloudProvider.Azure.SubscriptionID,
70-
b.Cluster.Spec.CloudProvider.Azure.ResourceGroupName,
70+
b.Cluster.AzureResourceGroupName(),
7171
)
7272
c.AddTask(&azuretasks.RoleAssignment{
7373
Name: to.Ptr(fmt.Sprintf("%s-%s", *vmss.Name, "owner")),

pkg/resources/azure/azure.go

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,14 @@ func (g *resourceGetter) resourceGroupName() string {
6363
return g.clusterInfo.AzureResourceGroupName
6464
}
6565

66+
func (g *resourceGetter) resourceGroupID() string {
67+
return fmt.Sprintf("/subscriptions/%s/resourceGroups/%s", g.clusterInfo.AzureSubscriptionID, g.clusterInfo.AzureResourceGroupName)
68+
}
69+
70+
func (g *resourceGetter) storageAccountID() string {
71+
return g.clusterInfo.AzureStorageAccountID
72+
}
73+
6674
func (g *resourceGetter) listResourcesAzure() (map[string]*resources.Resource, error) {
6775
rs, err := g.listAll()
6876
if err != nil {
@@ -397,11 +405,17 @@ func (g *resourceGetter) listVMScaleSetsAndRoleAssignments(ctx context.Context)
397405
principalIDs[*vmss.Identity.PrincipalID] = vmss
398406
}
399407

400-
ras, err := g.listRoleAssignments(ctx, principalIDs)
408+
resourceGroupRAs, err := g.listRoleAssignments(ctx, principalIDs, g.resourceGroupID())
409+
if err != nil {
410+
return nil, err
411+
}
412+
rs = append(rs, resourceGroupRAs...)
413+
414+
storageAccountRAs, err := g.listRoleAssignments(ctx, principalIDs, g.storageAccountID())
401415
if err != nil {
402416
return nil, err
403417
}
404-
rs = append(rs, ras...)
418+
rs = append(rs, storageAccountRAs...)
405419

406420
return rs, nil
407421
}
@@ -509,8 +523,8 @@ func (g *resourceGetter) deleteDisk(_ fi.Cloud, r *resources.Resource) error {
509523
return g.cloud.Disk().Delete(context.TODO(), g.resourceGroupName(), r.Name)
510524
}
511525

512-
func (g *resourceGetter) listRoleAssignments(ctx context.Context, principalIDs map[string]*compute.VirtualMachineScaleSet) ([]*resources.Resource, error) {
513-
ras, err := g.cloud.RoleAssignment().List(ctx, g.resourceGroupName())
526+
func (g *resourceGetter) listRoleAssignments(ctx context.Context, principalIDs map[string]*compute.VirtualMachineScaleSet, scope string) ([]*resources.Resource, error) {
527+
ras, err := g.cloud.RoleAssignment().List(ctx, scope)
514528
if err != nil {
515529
return nil, err
516530
}

pkg/resources/clusterinfo.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ type ClusterInfo struct {
2020
Name string
2121
UsesNoneDNS bool
2222
// Azure specific
23+
AzureStorageAccountID string
24+
AzureSubscriptionID string
2325
AzureResourceGroupName string
2426
AzureResourceGroupShared bool
2527
AzureNetworkShared bool

pkg/resources/ops/collector.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,8 @@ func ListResources(cloud fi.Cloud, cluster *kops.Cluster) (map[string]*resources
5757
case kops.CloudProviderOpenstack:
5858
return openstack.ListResources(cloud.(cloudopenstack.OpenstackCloud), clusterInfo)
5959
case kops.CloudProviderAzure:
60+
clusterInfo.AzureStorageAccountID = cluster.Spec.CloudProvider.Azure.StorageAccountID
61+
clusterInfo.AzureSubscriptionID = cluster.Spec.CloudProvider.Azure.SubscriptionID
6062
clusterInfo.AzureResourceGroupName = cluster.AzureResourceGroupName()
6163
clusterInfo.AzureResourceGroupShared = cluster.IsSharedAzureResourceGroup()
6264
clusterInfo.AzureNetworkShared = cluster.SharedVPC()

upup/pkg/fi/cloudup/azure/roleassignment.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,10 @@ func (c *roleAssignmentsClientImpl) Create(
4444
parameters authz.RoleAssignmentCreateParameters,
4545
) (*authz.RoleAssignment, error) {
4646
resp, err := c.c.Create(ctx, scope, roleAssignmentName, parameters, nil)
47-
return &resp.RoleAssignment, err
47+
if err != nil {
48+
return nil, err
49+
}
50+
return &resp.RoleAssignment, nil
4851
}
4952

5053
func (c *roleAssignmentsClientImpl) List(ctx context.Context, scope string) ([]*authz.RoleAssignment, error) {

0 commit comments

Comments
 (0)