{Profile} Update help message for arguments#30090
Merged
Conversation
️✔️AzureCLI-FullTest
|
️✔️AzureCLI-BreakingChangeTest
|
Collaborator
|
az login refinement |
microsoft-github-policy-service
bot
requested review from
wangzelin007 and
yonzhan
jiasli
commented
Oct 15, 2024
| with self.argument_context('login') as c: | ||
| c.argument('password', options_list=['--password', '-p'], help="Credentials like user password, or for a service principal, provide client secret or a pem file with key and public certificate. Will prompt if not given.") | ||
| c.argument('service_principal', action='store_true', help='The credential representing a service principal.') | ||
| c.argument('username', options_list=['--username', '-u'], help='user name, service principal, or managed service identity ID') |
Member
Author
There was a problem hiding this comment.
"service principal" is a resource. It should be "service principal client ID".
jiasli
commented
Oct 15, 2024
| help='Credentials like password for user, or for a service principal, provide client ' | ||
| 'secret or a PEM file with key and public certificate. Will prompt if not given.') | ||
| c.argument('tenant', options_list=['--tenant', '-t'], validator=validate_tenant, | ||
| help='The Microsoft Entra tenant, must provide when using a service principal.') |
Member
Author
There was a problem hiding this comment.
Renamed from "AAD" to "Microsoft Entra".
jiasli
commented
Oct 15, 2024
|
|
||
| # Device code flow | ||
| c.argument('use_device_code', action='store_true', | ||
| help="Use CLI's old authentication flow based on device code. CLI will also use this if it can't launch a browser in your behalf, e.g. in remote SSH or Cloud Shell") |
Member
Author
There was a problem hiding this comment.
Device code is a currently support flow. It is not "old".
Member
Author
There was a problem hiding this comment.
"In your behalf" is wrong. It should be "on your behalf", but this phrase is not needed at all.
jiasli
commented
Oct 15, 2024
|
|
||
| # Service principal | ||
| c.argument('service_principal', action='store_true', | ||
| help='Log in with a service principal.') |
Member
Author
There was a problem hiding this comment.
Changed from "The credential representing a service principal." to make it more readable.
jiasli
commented
Oct 15, 2024
Comment on lines
+69
to
+71
| help='Use Subject Name + Issuer (SN+I) authentication in order to support automatic ' | ||
| 'certificate rolls.') |
Member
Author
There was a problem hiding this comment.
Use the official name "Subject Name + Issuer (SN+I)".
jiasli
commented
Oct 15, 2024
|
|
||
| # Managed identity | ||
| c.argument('identity', options_list=('-i', '--identity'), action='store_true', | ||
| help="Log in using managed identity", arg_group='Managed Identity') |
Member
Author
There was a problem hiding this comment.
Changed from "Log in using the Virtual Machine's identity". CLI not only supports Virtual Machine's managed identity, but also App Service and other services' managed identities as well.
Member
Author
There was a problem hiding this comment.
Changed arg_group from "Managed Service Identity" to "Managed Identity".
jiasli
commented
Oct 15, 2024
|
|
||
| with self.argument_context('account') as c: | ||
| c.argument('subscription', options_list=['--subscription', '-s', '--name', '-n'], arg_group='', help='Name or ID of subscription.', completer=get_subscription_id_list) | ||
| c.argument('subscription', options_list=['--subscription', '-s', '--name', '-n'], |
Member
Author
There was a problem hiding this comment.
An empty arg_group is meaningless.
jiasli
commented
Oct 15, 2024
| c.argument('resource_type', get_enum_type(cloud_resource_types), options_list=['--resource-type'], | ||
| help='Type of well-known resource.') | ||
| c.argument('resource', options_list=['--resource'], | ||
| help='Azure resource endpoints in Microsoft Entra v1.0.') |
Member
Author
There was a problem hiding this comment.
Renamed from "AAD" to "Microsoft Entra".
jiasli
commented
Oct 15, 2024
| c.argument('resource', options_list=['--resource'], | ||
| help='Azure resource endpoints in Microsoft Entra v1.0.') | ||
| c.argument('scopes', options_list=['--scope'], nargs='*', | ||
| help='Space-separated scopes in Microsoft Entra v2.0. Default to Azure Resource Manager.') |
Member
Author
There was a problem hiding this comment.
Renamed from "AAD" to "Microsoft Entra".
jiasli
commented
Oct 15, 2024
| help='Space-separated scopes in Microsoft Entra v2.0. Default to Azure Resource Manager.') | ||
| c.argument('tenant', options_list=['--tenant', '-t'], | ||
| help='Tenant ID for which the token is acquired. Only available for user and service principal ' | ||
| 'account, not for managed identity or Cloud Shell account') |
Member
Author
There was a problem hiding this comment.
Renamed from "MSI" to "managed identity".
jiasli
commented
Oct 15, 2024
| c.argument('tenant', options_list=['--tenant', '-t'], help='The AAD tenant, must provide when using service principals.', validator=validate_tenant) | ||
| c.argument('allow_no_subscriptions', action='store_true', help="Support access tenants without subscriptions. It's uncommon but useful to run tenant level commands, such as 'az ad'") | ||
| c.argument('username', options_list=['--username', '-u'], | ||
| help='User name, service principal client ID, or managed identity ID.') |
Member
Author
There was a problem hiding this comment.
Renamed from "managed service identity" to "managed identity".
jiasli
commented
Oct 15, 2024
jiasli
commented
Oct 15, 2024
Comment on lines
+56
to
+58
| help="Support accessing tenants without subscriptions. It's uncommon but useful to run " | ||
| "tenant-level commands, such as 'az ad'.") |
Member
Author
There was a problem hiding this comment.
Changed from "Support access tenants" to "Support accessing tenants" to fix the wrong grammar.
Changed from "tenant level commands" to "tenant-level commands".
jiasli
commented
Oct 15, 2024
| c.argument('resource', options_list=['--resource'], help='Azure resource endpoints in AAD v1.0.') | ||
| c.argument('scopes', options_list=['--scope'], nargs='*', help='Space-separated AAD scopes in AAD v2.0. Default to Azure Resource Manager.') | ||
| c.argument('tenant', options_list=['--tenant', '-t'], help='Tenant ID for which the token is acquired. Only available for user and service principal account, not for MSI or Cloud Shell account') | ||
| c.argument('resource_type', get_enum_type(cloud_resource_types), options_list=['--resource-type'], |
Member
Author
There was a problem hiding this comment.
An empty arg_group is meaningless.
dbradish-microsoft
requested changes
Oct 15, 2024
bebound
approved these changes
Oct 24, 2024
evelyn-ys
approved these changes
Oct 25, 2024
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
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.
Related command
az loginDescription
Update help message for arguments, mainly including
az ad: Rename Azure Active Directory to Microsoft Entra ID #27756Testing Guide
az login -h