[Role] az role assignment create: Do not invoke Graph API if --assignee-principal-type is provided
#19219
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jiasli
commented
Aug 13, 2021
| @@ -147,7 +147,7 @@ def create_role_assignment(cmd, role, assignee=None, assignee_object_id=None, re | |||
| raise CLIError('usage error: --assignee STRING | --assignee-object-id GUID') | |||
|
|
|||
| if assignee_principal_type and not assignee_object_id: | |||
| raise CLIError('usage error: --assignee-object-id GUID [--assignee-principal-type]') | |||
| raise CLIError('usage error: --assignee-object-id GUID --assignee-principal-type TYPE') | |||
There was a problem hiding this comment.
As --assignee-object-id is initially designed to bypass Graph call, we encourage the user to use --assignee-object-id along with --assignee-principal-type. Specifying --assignee-object-id without --assignee-principal-type is only for backward compatibility.
jiasli
commented
Aug 13, 2021
Comment on lines
+168
to
+170
| # Try best to get principal type | ||
| logger.warning('RBAC service might reject creating role assignment without --assignee-principal-type ' | ||
| 'in the future. Better to specify --assignee-principal-type manually.') |
There was a problem hiding this comment.
Encourage the user to specify --assignee-principal-type.
|
Role |
evelyn-ys
approved these changes
Aug 18, 2021
jiasli
commented
Aug 27, 2021
Comment on lines
+1816
to
+1818
| logger.warning('Failed to query %s by invoking Graph API. ' | ||
| 'If you don\'t have permission to query Graph API, please ' | ||
| 'specify --assignee-object-id and --assignee-principal-type.', assignee) |
There was a problem hiding this comment.
Even though --assignee accepts object ID, we want to avoid such usage by encouraging --assignee-object-id and --assignee-principal-type.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix #19217, Refine #17669
Symptom
--assignee-principal-typeis not honored if AD Graph query for--assignee-object-idsucceeds. See--assignee-principal-typeis not honored if Graph query for--assignee-object-idsucceeds #19217for more details.
Change
Do not invoke Graph API if
--assignee-principal-typeis provided.As
--assignee-object-idis initially designed to bypass Graph call, we recommend the user to provide--assignee-principal-typeto avoid Graph call. Otherwise if--assignee-principal-typeis not provided, CLI will try best to query Graph API to auto-complete--assignee-principal-type.Testing Guide
Create a service principal that doesn't have permission to query Graph API. Then log in with that service principal.
Specify Object ID as
--assignee:Specify Object ID as
--assignee-object-id:Specify Object ID as
--assignee-object-idand--assignee-principal-type: