Skip to content

Fix #2274: Honour AZURE_TENANT_ID and caller-supplied subscription_id in azure_rm workload-identity auth#2276

Open
zunyangc wants to merge 4 commits into
devfrom
2274-fix
Open

Fix #2274: Honour AZURE_TENANT_ID and caller-supplied subscription_id in azure_rm workload-identity auth#2276
zunyangc wants to merge 4 commits into
devfrom
2274-fix

Conversation

@zunyangc

@zunyangc zunyangc commented Jun 9, 2026

Copy link
Copy Markdown
Collaborator
SUMMARY

Fix #2274.

The azure_rm modules and the azure_rm inventory plugin did not pick up the environment variables injected by the AKS workload-identity webhook (and used by the azure-identity SDK), so OIDC auth failed out-of-the-box on AKS workloads until the user manually re-exported AZURE_TENANT=$AZURE_TENANT_ID.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

plugins/module_utils/azure_rm_common.py
plugins/doc_fragments/azure_plugin.py

ADDITIONAL INFORMATION
  1. AZURE_TENANT_ID honoured as a fallback for tenant
    • New AZURE_CREDENTIAL_ENV_MAPPING_FALLBACK = {'tenant': 'AZURE_TENANT_ID'}.
    • _get_env() returns the primary env var when set, otherwise the fallback.
    • _get_env_credentials() now routes through _get_env() so the alias applies there too.
    • AZURE_TENANT remains the primary name (back-compat with existing pipelines and ~/.azure/credentials profiles).
  2. _get_env_credentials() early-return reordered
    • Caller-supplied subscription_id is merged into env_credentials before the "no subscription return None" check, so a value from an azure_rm.yml inventory config or ~/.azure/credentials profile is honoured under workload identity (where the webhook does not inject AZURE_SUBSCRIPTION_ID).
  3. Auto cascade guard against env without auth material
    • _get_env_credentials() now returns None when the env has neither client_id (SP / OIDC paths) nor ad_user (user/password path), even if a caller-supplied subscription_id is present.
    • Without this guard, the change in (2) would cause auth_source=auto to short-circuit on a subscription-only env dict, skipping the documented credential_file -> az-cli fallbacks.
    • The guard preserves the azure_rm: workload identity auth reads AZURE_TENANT but not AZURE_TENANT_ID #2274 fix because the AKS webhook always injects AZURE_CLIENT_ID alongside AZURE_TENANT_ID and AZURE_FEDERATED_TOKEN_FILE.

@zunyangc zunyangc added bug Something isn't working working In trying to solve, or in working with contributors labels Jun 9, 2026
zunyangc and others added 3 commits June 9, 2026 09:46
…h material

The previous fix moved the caller-supplied subscription_id merge above the
'no subscription_id -> return None' early-return in _get_env_credentials.
That unblocked workload-identity flows (the original #2274 bug), but it
introduced a regression: when auth_source=auto and the env has no AZURE_*
auth material yet the caller supplies subscription_id (e.g. via
module_defaults, inventory, or a credentials profile), _get_env_credentials
now returns a truthy dict containing only subscription_id. The auto
cascade in _get_credentials short-circuits on that truthy value, skipping
the credential_file and az-cli fallbacks, and _set_credentials then fails
with 'Failed to authenticate with provided credentials. Some attributes
were missing.'

Add a guard before the subscription_id merge: env counts as usable auth
only when client_id (SP / OIDC paths) or ad_user (user/password path) is
set. Otherwise return None so the auto cascade falls through to
credential_file -> az-cli as documented.

This restores auto-cli-fallback and auto-default-credfile while preserving
the #2274 workload-identity fix (AKS webhook always sets AZURE_CLIENT_ID
alongside AZURE_TENANT_ID and AZURE_FEDERATED_TOKEN_FILE, so the guard
passes).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sanity test E501 (line too long, max 160) failed on the comment lines
introduced in commit da07669. Shorten them to fit.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@michielvha

Copy link
Copy Markdown

Hi @zunyangc thanks for the quick turnaround on this, really appreciate it !

i checked out the branch and ran the patched _get_env / _get_env_credentials against the exact env the AKS workload-identity webhook injects (AZURE_CLIENT_ID + AZURE_TENANT_ID + AZURE_FEDERATED_TOKEN_FILE, no AZURE_TENANT and no AZURE_SUBSCRIPTION_ID), passing the subscription the way an azure_rm.yml would. before the fix it returns None so auth fails, after the fix it resolves tenant from AZURE_TENANT_ID and merges the supplied subscription_id, so OIDC goes through.

the extra guard requiring client_id / ad_user before treating env as usable auth looks right too, the webhook always injects AZURE_CLIENT_ID so the WI path stays intact.

this is exactly what I needed downstream, I'd been bridging AZURE_TENANT_ID to AZURE_TENANT and can drop that once it's in a release. thanks again !

@zunyangc zunyangc added ready-for-review The PR is ready to be reviewed and merged. and removed working In trying to solve, or in working with contributors labels Jun 10, 2026
@zunyangc zunyangc marked this pull request as ready for review June 10, 2026 23:05
@zunyangc zunyangc requested review from Vieran and removed request for Vieran June 10, 2026 23:05
@zunyangc zunyangc added working In trying to solve, or in working with contributors and removed ready-for-review The PR is ready to be reviewed and merged. labels Jun 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working working In trying to solve, or in working with contributors

Projects

None yet

Development

Successfully merging this pull request may close these issues.

azure_rm: workload identity auth reads AZURE_TENANT but not AZURE_TENANT_ID

2 participants