Skip to content

Conversation

@gerrytan
Copy link
Member

Adds a new parameter --include-role-assignment in res mode to include resource scope RBAC role assignments.

Example usage:

aztfexport res -n --include-role-assignment \
"/subscriptions/12345678-1234-1234-1234-123456789012/resourceGroups/my-rg/providers/Microsoft.KeyVault/vaults/myvault" \
"/subscriptions/12345678-1234-1234-1234-123456789012/resourceGroups/my-rg/providers/Microsoft.Network/virtualNetworks/myvnet"

Following cases are tested manually:

  1. aztfexport res with and without --include-role-assignment over 1 and multiple resources
  2. aztfexport rg with --include-role-assignment

@gerrytan gerrytan requested a review from magodo July 10, 2025 01:53
Copy link
Collaborator

@magodo magodo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this PR!
I've taken a look through and left some comments inline, but this is mostly looking good to me 👍

Comment on lines 167 to 170
var ids []string
for _, res := range resources {
ids = append(ids, res.Id.String())
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we combine this loop with the one at L185, and put it right above where it was used (L188)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed


rset := &resourceset.AzureResourceSet{
Resources: resources,
rset, err := meta.queryResourceSet(ctx, resources)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is only necessary if includeRoleAssignment = true, otherwise, we don't need to call azlist, to save one or more APIs.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

@@ -0,0 +1,163 @@
package meta
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo in the file name.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

- renamed meta_res_test.go
- meta_res.go: remove redundant loop
- meta_res.go: avoid querying azlist if role assignment not included
@gerrytan gerrytan requested a review from magodo July 10, 2025 05:13
@magodo magodo merged commit 1bc2421 into Azure:main Jul 10, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants