Skip to content

Implement subscription filter on az login #33122

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.

Sign up for GitHub

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

Open
xuming-ms wants to merge 1 commit into
base: feat/subscription-filtering
Choose a base branch
from
Open

Implement subscription filter on az login #33122

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 38 additions & 4 deletions src/azure-cli-core/azure/cli/core/_profile.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,9 @@ def login(self,
allow_no_subscriptions=False,
use_cert_sn_issuer=None,
show_progress=False,
claims_challenge=None):
claims_challenge=None,
skip_subscription_discovery=False,
default_subscription=None):
"""
For service principal, `password` is a dict returned by ServicePrincipalAuth.build_credential
"""
Expand Down Expand Up @@ -198,15 +200,25 @@ def login(self,
else:
credential = identity.get_service_principal_credential(username)

if tenant:
is_bare_mode = skip_subscription_discovery and not default_subscription

if skip_subscription_discovery and default_subscription:
# Fast path: fetch only the specified subscription (1 API call)
subscriptions = subscription_finder.find_specific_subscriptions(
tenant, credential, [default_subscription])
elif is_bare_mode:
# Bare mode: no ARM subscription calls. Tenant-level account will be creatd below
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

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

Typo in comment: "creatd" should be "created".

Suggested change
# Bare mode: no ARM subscription calls. Tenant-level account will be creatd below
# Bare mode: no ARM subscription calls. Tenant-level account will be created below

Copilot uses AI. Check for mistakes.
subscriptions = []
subscription_finder.tenants.append(tenant)
elif tenant:
Comment on lines +203 to +213
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

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

Profile.login accepts skip_subscription_discovery=True but doesn't validate that tenant is provided. If tenant is None, the bare-mode branch appends None to subscription_finder.tenants and _build_tenant_level_accounts will be called with [None], and the fast-path branch will call find_specific_subscriptions(None, ...). Add a core-level usage check (similar to command module custom.login) to raise a CLIError when skip_subscription_discovery is set without tenant to keep behavior consistent for programmatic callers.

Copilot uses AI. Check for mistakes.
subscriptions = subscription_finder.find_using_specific_tenant(tenant, credential)
else:
subscriptions = subscription_finder.find_using_common_tenant(username, credential)

if not subscriptions and not allow_no_subscriptions:
if not subscriptions and not allow_no_subscriptions and not is_bare_mode:
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

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

When skip_subscription_discovery=True and a default_subscription is provided but cannot be retrieved, find_specific_subscriptions returns an empty list and this code raises CLIError("No subscriptions found for <username>."). This is misleading because the user asked for a specific subscription. Consider special-casing the fast-path to raise an error that mentions the requested subscription ID (and ideally distinguishes not-found/unauthorized vs transient errors), rather than the generic "No subscriptions found" message.

Suggested change
if not subscriptions and not allow_no_subscriptions and not is_bare_mode:
if not subscriptions and not allow_no_subscriptions and not is_bare_mode:
if skip_subscription_discovery and default_subscription:
# Fast-path error: a specific subscription was requested but could not be retrieved
raise CLIError(
"The subscription '{}' could not be retrieved for '{}'. "
"Ensure the subscription exists and that you have access to it.".format(
default_subscription, username))

Copilot uses AI. Check for mistakes.
raise CLIError("No subscriptions found for {}.".format(username))

if allow_no_subscriptions:
if allow_no_subscriptions or is_bare_mode:
t_list = [s.tenant_id for s in subscriptions]
bare_tenants = [t for t in subscription_finder.tenants if t not in t_list]
tenant_accounts = self._build_tenant_level_accounts(bare_tenants)
Expand All @@ -218,6 +230,27 @@ def login(self,
is_service_principal, bool(use_cert_sn_issuer))

self._set_subscriptions(consolidated)

# Validate default_subscription exists before calling set_active_subscription,
# so the caller can handle "not found" differently based on context.
if default_subscription:
match = next((s for s in consolidated
if s[_SUBSCRIPTION_ID].lower() == default_subscription.lower() or
s.get(_SUBSCRIPTION_NAME, '').lower() == default_subscription.lower()), None)
if match:
self.set_active_subscription(match[_SUBSCRIPTION_ID])
# Refresh consolidated from storage so the returned list reflects the new default
consolidated = self.load_cached_subscriptions()
elif 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.",
default_subscription)
else:
raise CLIError("Subscription '{}' not found. Check the ID or name and try again."
.format(default_subscription))

return deepcopy(consolidated)

def login_with_managed_identity(self, client_id=None, object_id=None, resource_id=None,
Expand Down Expand Up @@ -507,6 +540,7 @@ def _match_account(account, subscription_id, secondary_key_name, secondary_key_v

set_cloud_subscription(self.cli_ctx, active_cloud.name, default_sub_id)
self._storage[_SUBSCRIPTIONS] = subscriptions
return subscriptions

@staticmethod
def _pick_working_subscription(subscriptions):
Expand Down
Loading
Loading