Conversation
Co-authored-by: Ming Xu <xumi@microsoft.com>
Co-authored-by: Ming Xu <xumi@microsoft.com>
xuming-ms
requested review from
DanielMicrosoft,
ReaNAiveD,
bebound,
isra-fel,
jiasli,
necusjz,
notyashhh,
teresaritorto and
yonzhan
as code owners
️✔️AzureCLI-FullTest
|
|
| rule | cmd_name | rule_message | suggest_message |
|---|---|---|---|
| login | cmd login added parameter skip_subscription_discovery |
||
| login | cmd login added parameter subscription |
|
Thank you for your contribution! We will review the pull request and get back to you soon. |
|
The git hooks are available for azure-cli and azure-cli-extensions repos. They could help you run required checks before creating the PR. Please sync the latest code with latest dev branch (for azure-cli) or main branch (for azure-cli-extensions). pip install azdev --upgrade
azdev setup -c <your azure-cli repo path> -r <your azure-cli-extensions repo path>
|
There was a problem hiding this comment.
Pull request overview
Implements subscription filtering controls for az login, allowing callers to skip full subscription discovery and/or set a default subscription during login.
Changes:
- Adds
--skip-subscription-discovery(--skip-sub) and enhances--subscriptionbehavior foraz login, including updated help examples. - Updates login flow to conditionally bypass or invoke the interactive subscription selector based on filter usage and match count.
- Extends core profile logic (
Profile.login,SubscriptionFinder) to support fast-path subscription fetch (GET) and bare tenant-level login, with new unit and live E2E tests.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/azure-cli/azure/cli/command_modules/profile/tests/latest/test_subscription_filter_e2e.py | Adds live E2E coverage for skip-discovery and subscription filtering login scenarios. |
| src/azure-cli/azure/cli/command_modules/profile/tests/latest/test_profile_custom.py | Adds command-module unit tests for selector behavior and argument validation in custom.login(). |
| src/azure-cli/azure/cli/command_modules/profile/custom.py | Implements filter-aware selector behavior and wires new args into Profile.login(). |
| src/azure-cli/azure/cli/command_modules/profile/_help.py | Documents new login scenarios in help text. |
| src/azure-cli/azure/cli/command_modules/profile/init.py | Registers new CLI arguments for az login. |
| src/azure-cli-core/azure/cli/core/tests/test_profile.py | Adds core unit tests for skip discovery, fast-path subscription fetch, merge behavior, and preferred subscription selection. |
| src/azure-cli-core/azure/cli/core/_profile.py | Implements skip discovery (fast-path GET / bare mode), subscription filtering, preferred default selection, and specific-subscription finder method. |
Comments suppressed due to low confidence (1)
src/azure-cli-core/azure/cli/core/_profile.py:506
_set_subscriptionsstill keys accounts only by subscription ID whensecondary_key_nameis not provided (_get_key_name), so two accounts with the same subscription ID but differenttenantIdwill overwrite each other duringdic.update(...). This conflicts with the new login flow that can surface multiple matches for a single--subscription(e.g., across tenants) and makes it impossible to reliably persist the user's selection. Consider incorporating tenant ID into the key when duplicates exist (or otherwise storing only the selected match) so matches are not silently dropped.
def _get_key_name(account, secondary_key_name):
return (account[_SUBSCRIPTION_ID] if secondary_key_name is None
else '{}-{}'.format(account[_SUBSCRIPTION_ID], account[secondary_key_name]))
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| selected = SubscriptionSelector(subscriptions)() | ||
| profile.set_active_subscription(selected[_SUBSCRIPTION_ID]) | ||
|
|
There was a problem hiding this comment.
The selector path can present multiple entries that share the same subscription ID (e.g., the same subscription appearing under multiple tenants), but _select_and_set_active ultimately calls profile.set_active_subscription(selected[_SUBSCRIPTION_ID]), which cannot express the chosen tenant and will either be ambiguous (if both entries are stored) or will just select whichever duplicate entry was last persisted (if duplicates were de-duped). To make the interactive selection meaningful, the activation step needs to disambiguate by both subscription ID and tenant (or ensure only one chosen match is persisted before calling set_active_subscription).
| selected = SubscriptionSelector(subscriptions)() | |
| profile.set_active_subscription(selected[_SUBSCRIPTION_ID]) | |
| def _get_tenant_id(subscription): | |
| return (subscription.get('tenantId') or | |
| subscription.get('tenant_id') or | |
| subscription.get('homeTenantId') or | |
| subscription.get('home_tenant_id')) | |
| selected = SubscriptionSelector(subscriptions)() | |
| selected_subscription_id = selected[_SUBSCRIPTION_ID] | |
| selected_tenant_id = _get_tenant_id(selected) | |
| matching_subscriptions = [ | |
| subscription for subscription in subscriptions | |
| if subscription.get(_SUBSCRIPTION_ID) == selected_subscription_id | |
| ] | |
| matching_tenant_ids = {_get_tenant_id(subscription) for subscription in matching_subscriptions} | |
| if len(matching_subscriptions) > 1 and len(matching_tenant_ids) > 1: | |
| raise CLIError( | |
| "The selected subscription '{}' is available in multiple tenants{} and cannot be " | |
| "activated unambiguously in interactive mode. Please login again with '--tenant' " | |
| "to select the intended tenant explicitly.".format( | |
| selected_subscription_id, | |
| " (selected tenant: '{}')".format(selected_tenant_id) if selected_tenant_id else "" | |
| ) | |
| ) | |
| profile.set_active_subscription(selected_subscription_id) |
| is_bare_mode = skip_subscription_discovery and not subscription | ||
|
|
||
| if skip_subscription_discovery and subscription: | ||
| # Fast path: fetch only the specified subscription (1 API call) | ||
| subscriptions = subscription_finder.find_specific_subscriptions( | ||
| tenant, credential, [subscription]) | ||
| elif is_bare_mode: | ||
| # Bare mode: no ARM subscription calls. Tenant-level account will be created below | ||
| subscriptions = [] | ||
| subscription_finder.tenants.append(tenant) | ||
| elif tenant: |
There was a problem hiding this comment.
Profile.login() accepts skip_subscription_discovery=True but doesn't validate that tenant is provided. In the bare-mode branch this appends tenant to subscription_finder.tenants and later _build_tenant_level_accounts will do '/subscriptions/' + t, which will raise a TypeError if tenant is None. Add an explicit early CLIError (similar to profile.custom.login) when skip_subscription_discovery is set without tenant to avoid an unhandled exception and to give a clear message to API callers.
| consolidated = self._normalize_properties(username, subscriptions, | ||
| is_service_principal, bool(use_cert_sn_issuer)) | ||
| self._set_subscriptions(consolidated, preferred_subscription=subscription) | ||
|
|
||
| if subscription: | ||
| matches = [s for s in consolidated | ||
| if s[_SUBSCRIPTION_ID].lower() == subscription.lower() or | ||
| s.get(_SUBSCRIPTION_NAME, '').lower() == subscription.lower()] | ||
| if matches: | ||
| return deepcopy(matches) | ||
| if skip_subscription_discovery: | ||
| # --skip-subscription-discovery + --subscription S, but S is inaccessible. | ||
| # without --allow-no-subscriptions → already errored above | ||
| # with --allow-no-subscriptions → tenant-level account only (we reach here) | ||
| logger.warning("Subscription '%s' not found. Profile has tenant-level account only.", | ||
| subscription) | ||
| else: | ||
| raise CLIError("Subscription '{}' not found. Check the ID or name and try again." | ||
| .format(subscription)) |
There was a problem hiding this comment.
Profile.login() persists subscriptions via _set_subscriptions(...) before validating --subscription matches anything. In the subscription-provided + not-skip_subscription_discovery case, this means a subsequent CLIError("Subscription ... not found") can still leave the user logged in and mutate azureProfile.json/default subscription even though the command failed. Consider validating/filtering the match first and only calling _set_subscriptions after you know the requested subscription is valid (or rolling back on error).
Related command
az loginDescription
Wires
--skip-subscription-discoveryand--subscriptionparameters intoaz login. This enables fast login for tenants with 1M+ subscriptions by skipping full subscription enumeration.Three login modes are now supported:
--skip-subscription-discovery --subscription S: Fast path: fetches only the specified subscription viaGET /subscriptions/{id}(1 API call)--skip-subscription-discovery(bare mode): No ARM calls and creates a tenant-level account for tenant-level operations (e.g.,az ad)--subscription S(without skip): Full discovery, then sets S as the defaultBoth
--subscriptionand--skip-subscription-discoverybypass the interactive subscription selector.Related issues: #31939, #14933, #13285
Testing Guide
8 unit tests added in
TestLoginSubscriptionFilter:--skip-subscription-discovery --subscriptionverifiesGETcalled, notLIST--allow-no-subscriptionsverifies warning, tenant-level fallback--subscriptionwithout skip verifies full discovery + default set--subscriptionnot found without skip verifies error3 tests added in
test_profile_custom.py:-
Add validation that --skip-subscription-discovery requires --tenant--skip-subscription-discoverybypasses interactive selector--subscriptionbypasses interactive selectorRun tests:
History Notes
[Core]
az login: Add--skip-subscription-discoveryand--subscriptionparameters to enable fast login for tenants with many subscriptionsThis checklist is used to make sure that common guidelines for a pull request are followed.
The PR title and description has followed the guideline in Submitting Pull Requests.
I adhere to the Command Guidelines.
I adhere to the Error Handling Guidelines.