-
Notifications
You must be signed in to change notification settings - Fork 3.3k
[Role] az role assignment list/delete: Add --assignee-object-id
#30469
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
Conversation
️✔️AzureCLI-FullTest
|
|
| rule | cmd_name | rule_message | suggest_message |
|---|---|---|---|
| role assignment delete | cmd role assignment delete added parameter assignee_object_id |
||
| role assignment list | cmd role assignment list added parameter assignee_object_id |
|
Thank you for your contribution! We will review the pull request and get back to you soon. |
|
|
|
In azure-cli/src/azure-cli/azure/cli/command_modules/role/custom.py Lines 244 to 245 in 1d63a5f
Classic administrator support should be dropped: #29470 |
|
In azure-cli/src/azure-cli/azure/cli/command_modules/role/custom.py Lines 268 to 270 in 464a79c
I feel this behavior should be skipped if Update: #30693 added |
1d63a5f to
0088515
Compare
| if assignee_object_id and include_classic_administrators: | ||
| raise CLIError('Usage error: --assignee-object-id cannot be used with --include-classic-administrators. ' | ||
| 'Use --assignee instead.') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Classic admin assignments use email_address (UPN), instead of principal ID:
| co_admins = [x for x in co_admins if assignee == x.email_address.lower()] |
When --assignee-object-id is provided, it is necessary to convert object ID to UPN:
azure-cli/src/azure-cli/azure/cli/command_modules/role/custom.py
Lines 471 to 478 in 7fbe2fe
| if is_guid(assignee): | |
| try: | |
| result = _get_object_stubs(graph_client, [assignee]) | |
| if not result: | |
| return [] | |
| assignee = _get_displayable_name(result[0]).lower() | |
| except ValueError: | |
| pass |
This will trigger Graph call which is against the initial purpose.
Moreover, as --include-classic-administrator will be removed soon (#29470). Updating the legacy code is worthless.
az role assignment list/delete: Support --assignee-object-idaz role assignment list/delete: Add --assignee-object-id to bypass Graph query
| ''' | ||
| :param include_groups: include extra assignments to the groups of which the user is a | ||
| member(transitively). | ||
| ''' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
include_groups should be declared in _params.py, not here. #602 (comment)
az role assignment list/delete: Add --assignee-object-id to bypass Graph queryaz role assignment list/delete: Add --assignee-object-id
| self.not_exists("[0].principalName") | ||
| ]) | ||
| # Manually verify the recording file that no HTTP request to https://graph.microsoft.com is made | ||
| self.cmd('role assignment delete --scope {rg_id} --assignee-object-id {uami_object_id}') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
az role assignment delete doesn't support --all, so it can only query role assignments via
azure-cli/src/azure-cli/azure/cli/command_modules/role/custom.py
Lines 582 to 586 in d4147a7
| if scope: | |
| f = 'atScope()' # atScope() excludes role assignments at subscopes | |
| if assignee_object_id and include_groups: | |
| f = f + " and assignedTo('{}')".format(assignee_object_id) | |
| assignments = list(assignments_client.list_for_scope(scope=scope, filter=f)) |
This still makes the recording big. #31179 will solve this.
| self.cmd('role assignment list --all --assignee-object-id {uami_object_id} ' | ||
| '--fill-role-definition-name false --fill-principal-name false', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using --all --assignee-object-id reduces az role assignment list's recording file size (#31152 (comment)).
bbfb909 to
933506c
Compare
933506c to
56123d6
Compare
| except Exception as ex: # pylint: disable=broad-except | ||
| if _error_caused_by_role_assignment_exists(ex): # for idempotent | ||
| return list_role_assignments(cmd, assignee=assignee, role=role, scope=scope)[0] | ||
| return list_role_assignments(cmd, assignee_object_id=object_id, role=role, scope=scope)[0] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using object_id instead of assignee saves one Graph query for az role assignment create.
Related command
az role assignment listaz role assignment deleteDescription
Close #30436
Fix #20349, #30428, #30672
With more encouragement on managed identity and more strict permission/access control, supporting
--assignee-object-idto bypass Graph query is a necessary feature for role assignment’s management experience.Similar to #5273 which added
--assignee-object-idtoaz role assignment createto bypass Graph query, this PR adds--assignee-object-idtoaz role assignment list/delete.Testing Guide
History Notes
[Role]
az role assignment list/delete: Add--assignee-object-idargument. Use this argument instead of--assigneeto bypass Microsoft Graph query