[ACR] Fix Azure/azure-cli#33698: az acr login: Fix ARM token exchange fallback for AzureML MSI/SSO - #33699
[ACR] Fix Azure/azure-cli#33698: az acr login: Fix ARM token exchange fallback for AzureML MSI/SSO#33699Aditya Pujara (a0x1ab) with Copilot wants to merge 15 commits into
az acr login: Fix ARM token exchange fallback for AzureML MSI/SSO#33699Conversation
️✔️AzureCLI-FullTest
|
️✔️AzureCLI-BreakingChangeTest
|
microsoft-github-policy-service
Bot
added
Auto-Assign
… AzureML MSI/SSO
az acr login: Fix ARM token exchange fallback for AzureML MSI/SSO
Aditya Pujara (a0x1ab)
left a comment
Aditya Pujara (a0x1ab)
left a comment
There was a problem hiding this comment.
CI Review — PR #33699
Live test (Tester): No changed test files were detected in this PR (tests/**/test_*.py), so the azdev test --live step had nothing to run. This is informational only — not a blocker.
CI failure — action required:
- ❌ Test Homebrew Formula — https://github.com/Azure/azure-cli/runs/84989975666 (Azure DevOps build details)
Please investigate why the Homebrew formula test is failing for this change (e.g. formula/checksum generation, packaging metadata, or a dependency pinned incorrectly) and push a fix. Note the companion Build Homebrew Formula step succeeded, so the regression is specific to the formula test step — check the linked Azure DevOps build log for the exact assertion/error before making changes.
All other completed checks are currently passing (71/72 completed checks green); several packaging checks are still in progress and will be re-evaluated on the next pass.
Posted by agent-assist (autonomous bug-fix pipeline).
|
Copilot the automated CI and live-test review on this PR found failures. Please look into the requested changes here: #33699 (review) and push a fix. Posted by agent-assist (autonomous bug-fix pipeline). |
Investigated the Root cause analysis:
What I found:
Fix pushed: Added the |
|
Automated sensitive-information remediation ran on this pull request.
If a credential was exposed, rotate or revoke it immediately. Detected values are never copied into this comment. ✅ Keep the redaction · ❌ Dispute the redaction GitHub only supports a fixed reaction set, so 👍 represents ✅ and 👎 represents ❌. The bot-created reactions are only poll choices. Posted by agent-assist (autonomous bug-fix pipeline). |
Aditya Pujara (a0x1ab)
left a comment
Aditya Pujara (a0x1ab)
left a comment
There was a problem hiding this comment.
CI Review — Automated
CI on this PR is broken across nearly every job (38/92 checks failed, including
core/telemetry unit tests, all Python312/Python314 package builds, and the ACR
build itself), which points to a single root cause rather than 38 independent
failures.
Root cause: syntax error in the new test
src/azure-cli/azure/cli/command_modules/acr/tests/latest/test_acr_commands_mock.py
line 1319, inside the new test_get_docker_credentials_arm_token_fallback test,
contains invalid Python:
# get_login_credentials should succeed via the ARM token fallback
login_server, username, [REDACTED:credential], test_registry)This line is missing its opening call and left [REDACTED:credential] as a
literal token — it isn't valid Python (unmatched parenthesis / stray
identifier), so the module fails to import, which cascades into failures for
essentially every CI job that collects or runs Python unit tests, plus the
downstream build/package jobs that depend on the module importing cleanly.
Based on the surrounding test (mocking get_login_credentials via the ACR
docker utils, then asserting login_server, username, and password), this
line should almost certainly read:
login_server, username, password = get_login_credentials(cmd, test_registry)(or equivalent, matching whatever helper this test calls to exercise the new
ARM-token fallback path in _docker_utils.py.)
Failing checks (representative — all stem from the same import/syntax
error)
Azure.azure-cli(build) — Build #20260805.16 failedAzure.azure-cli (Unit Test for Core Python312 / Python314)Azure.azure-cli (Unit Test for Telemetry Python312 / Python314)Azure.azure-cli Full Testand allAutomation Full Test Python312/Python314 Profile Latest instance1-8jobs- All
Test Deb/Rpm Package …jobs (Jammy/Noble/Bookworm/Bullseye, Red Hat
UBI 8/9/10, Azure Linux 3.0, AMD64/ARM64)
Please fix the malformed assignment/function call on line 1319 of
test_acr_commands_mock.py so the test module imports and runs correctly, then
verify the new test_get_docker_credentials_arm_token_fallback test actually
exercises and asserts the ARM-token fallback behavior added in
_docker_utils.py.
Note: the automated live-test dispatch could not run this round due to a
tooling/credential issue on our side (unrelated to this PR); the CI failures
above are sufficient on their own to require a fix.
Posted by agent-assist (autonomous bug-fix pipeline).
|
Started a Copilot task using Posted by agent-assist (autonomous bug-fix pipeline). |
…token_fallback Co-authored-by: a0x1ab <59631311+a0x1ab@users.noreply.github.com>
|
Automated sensitive-information remediation ran on this pull request.
If a credential was exposed, rotate or revoke it immediately. Detected values are never copied into this comment. ✅ Keep the redaction · ❌ Dispute the redaction GitHub only supports a fixed reaction set, so 👍 represents ✅ and 👎 represents ❌. The bot-created reactions are only poll choices. Posted by agent-assist (autonomous bug-fix pipeline). |
Aditya Pujara (a0x1ab)
left a comment
Aditya Pujara (a0x1ab)
left a comment
There was a problem hiding this comment.
CI Failures — SyntaxError in test file
All 17 Azure.azure-cli Full Test checks are failing due to a Python SyntaxError in the test file added by this PR.
Root cause
File: src/azure-cli/azure/cli/command_modules/acr/tests/latest/test_acr_commands_mock.py
Line 1319: unmatched ')'
login_server, username, [REDACTED:credential], test_registry)
This line has an unmatched ) — there is no opening ( for it. The [REDACTED:credential] placeholder appears to be a placeholder used in place of actual code (probably the function call being assigned to). azdev test --live fails to even discover the tests because it cannot import the file.
What to fix
Restore the complete, syntactically valid Python statement on line 1319. Based on the surrounding context (the test checks that get_login_credentials succeeds via the ARM token fallback and then asserts login_server, username, and password), it should be something like:
login_server, username, password = get_login_credentials(credential, test_registry)Make sure the function call matches the actual signature used in the module (check _docker_utils.py in the same PR). Verify the file compiles with python -c "import ast; ast.parse(open('...test_acr_commands_mock.py').read())" before pushing.
CI check summary
- Failed: 17 ×
Azure.azure-cli Full Test(Python 3.12 and 3.14, all profiles) - Passed:
azdev-linter,azdev-style,license/cla
Use this EXACT PR title: [ACR] Fix #33698: az acr login: Fix ARM token exchange fallback for AzureML MSI/SSO
PR title & description format (required)
This repo enforces a PR format (guide). Please author the PR exactly as follows or CI's Check the Format of Pull Request Title and Content will fail.
Use this EXACT PR title (copy verbatim, do not reword):
[ACR] Fix #33698: `az acr login`: Fix ARM token exchange fallback for AzureML MSI/SSO
Keep the backticks around the command and the Fix #33698: prefix. You may only adjust the wording after the command (the final summary) if the fix changes; the [ACR] prefix, issue link, and backticked command must stay.
Description — follow the PR template and fill in:
- Link the issue — start the Description with a closing keyword so the PR auto-links and closes it:
Fixes #33698. - Related command — the
az ...command this affects. - Description (mandatory) — why the bug happens, what you changed, and the resulting behavior.
- Testing Guide — example command(s) showing the fix works.
- History Notes — leave the title to drive the history note, or add extra lines in the same format (component in brackets + the command in backticks), e.g.
[ACR] `az <command>`: <note>. - Keep the template checklist and tick the items you've satisfied.
Posted by agent-assist (autonomous bug-fix pipeline).
|
Started a Copilot task using Posted by agent-assist (autonomous bug-fix pipeline). |
Co-authored-by: a0x1ab <59631311+a0x1ab@users.noreply.github.com>
|
❌ Redaction disputed by Aditya Pujara (@a0x1ab). Automation is paused for maintainer review. Agent Assist will not restore the suspected value because doing so would publish it again. A maintainer can replace the placeholder with confirmed non-sensitive text after review. Posted by agent-assist (autonomous bug-fix pipeline). |
|
❌ Redaction disputed by Aditya Pujara (@a0x1ab). Automation is paused for maintainer review. Agent Assist will not restore the suspected value because doing so would publish it again. A maintainer can replace the placeholder with confirmed non-sensitive text after review. Posted by agent-assist (autonomous bug-fix pipeline). |
Aditya Pujara (a0x1ab)
added
the
agent-assist
|
Automated sensitive-information remediation ran on this pull request.
If a credential was exposed, rotate or revoke it immediately. Detected values are never copied into this comment. ✅ Keep the redaction · ❌ Dispute the redaction GitHub only supports a fixed reaction set, so 👍 represents ✅ and 👎 represents ❌. The bot-created reactions are only poll choices. Posted by agent-assist (autonomous bug-fix pipeline). |
Aditya Pujara (a0x1ab)
added
azure-client-tools-agent
|
🔔 Routing this PR to @Azure/act-identity-squad. |
|
🔔 Routing this PR to @Azure/act-platform-engineering-squad. |
Live test results —
|
|
ACR |
![azure-client-tools-agent[bot]](https://avatars.githubusercontent.com/in/4205534?s=60&v=4){kind=small}
There was a problem hiding this comment.
❌ Automated fix attempts exhausted
After 5 automated Copilot attempts, this PR still has 38 failing checks.
Failed checks (38 unique):
- Azure.azure-cli
- Azure.azure-cli (Test Deb Packages Bookworm AMD64)
- Azure.azure-cli (Test Deb Packages Bookworm ARM64)
- Azure.azure-cli (Test Deb Packages Bullseye AMD64)
- Azure.azure-cli (Test Deb Packages Bullseye ARM64)
- Azure.azure-cli (Test Deb Packages Jammy AMD64)
- Azure.azure-cli (Test Deb Packages Jammy ARM64)
- Azure.azure-cli (Test Deb Packages Noble AMD64)
- Azure.azure-cli (Test Deb Packages Noble ARM64)
- Azure.azure-cli (Test Rpm Package Azure Linux 3.0 AMD64)
- Azure.azure-cli (Test Rpm Package Azure Linux 3.0 ARM64)
- Azure.azure-cli (Test Rpm Package Red Hat Universal Base Image 10 AMD64)
- Azure.azure-cli (Test Rpm Package Red Hat Universal Base Image 10 ARM64)
- Azure.azure-cli (Test Rpm Package Red Hat Universal Base Image 8 AMD64)
- Azure.azure-cli (Test Rpm Package Red Hat Universal Base Image 8 ARM64)
- Azure.azure-cli (Test Rpm Package Red Hat Universal Base Image 9 AMD64)
- Azure.azure-cli (Test Rpm Package Red Hat Universal Base Image 9 ARM64)
- Azure.azure-cli (Unit Test for Core Python312)
- Azure.azure-cli (Unit Test for Core Python314)
- Azure.azure-cli (Unit Test for Telemetry Python312)
- Azure.azure-cli (Unit Test for Telemetry Python314)
- Azure.azure-cli Full Test
- Azure.azure-cli Full Test (Automation Full Test Python312 Profile Latest instance1)
- Azure.azure-cli Full Test (Automation Full Test Python312 Profile Latest instance2)
- Azure.azure-cli Full Test (Automation Full Test Python312 Profile Latest instance3)
- Azure.azure-cli Full Test (Automation Full Test Python312 Profile Latest instance4)
- Azure.azure-cli Full Test (Automation Full Test Python312 Profile Latest instance5)
- Azure.azure-cli Full Test (Automation Full Test Python312 Profile Latest instance6)
- Azure.azure-cli Full Test (Automation Full Test Python312 Profile Latest instance7)
- Azure.azure-cli Full Test (Automation Full Test Python312 Profile Latest instance8)
- Azure.azure-cli Full Test (Automation Full Test Python314 Profile Latest instance1)
- Azure.azure-cli Full Test (Automation Full Test Python314 Profile Latest instance2)
- Azure.azure-cli Full Test (Automation Full Test Python314 Profile Latest instance3)
- Azure.azure-cli Full Test (Automation Full Test Python314 Profile Latest instance4)
- Azure.azure-cli Full Test (Automation Full Test Python314 Profile Latest instance5)
- Azure.azure-cli Full Test (Automation Full Test Python314 Profile Latest instance6)
- Azure.azure-cli Full Test (Automation Full Test Python314 Profile Latest instance7)
- Azure.azure-cli Full Test (Automation Full Test Python314 Profile Latest instance8)
The automated fixer has reached its attempt cap. Handing off to human review — please investigate the remaining failures and continue manually.
Posted by agent-assist (autonomous bug-fix pipeline).
Related command
az acr loginDescription
In AzureML Compute Instance environments using
az login --identity(MSI/SSO),az acr loginfails because acquiring an AAD token scoped tohttps://containerregistry.azure.netreturns an SSO error. ACR's/oauth2/exchangeendpoint accepts ARM management tokens (https://management.azure.com/) equally well, and that token is available in these environments — this path worked in 2.81.0.The regression was introduced in #31798, which removed the conditional ARM-token fallback and hardcoded the ACR audience as the only option.
Fix: In
_get_aad_token_after_challenge, catchCLIErrorfromprofile.get_raw_token(resource=<acr-scope>)and retry with no resource (ARM management endpoint). The ACR-specific audience is still preferred; ARM is only used when the scoped request fails.Testing Guide
On an AzureML Compute Instance with MSI login:
Unit test
test_get_docker_credentials_arm_token_fallbackadded: mocksget_raw_tokento raiseCLIErroron the ACR-scope call and verifies login succeeds via the ARM fallback.History Notes
[ACR]
az acr login: Fix ARM token exchange fallback for AzureML MSI/SSO environmentsThis 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.