[ACR] Fix Azure/azure-cli#33674: az acr login: Fix incorrect resource URL used for local/private registry token acquisition - #33675
Open
Aditya Pujara (a0x1ab) with Copilot wants to merge 10 commits into
Open
Conversation
️✔️AzureCLI-FullTest
|
️✔️AzureCLI-BreakingChangeTest
|
microsoft-github-policy-service
Bot
requested review from
Bin Du (northtyphoon),
Terence Truong (terencet-dev),
ZelinWang (wangzelin007) and
Yong Zhang (yonzhan)
June 30, 2026 00:21
microsoft-github-policy-service
Bot
added
Auto-Assign
Copilot
AI
changed the title
[WIP] Fix Azure local regression for az acr login
[ACR] Fix Azure/azure-cli#33674: Jun 30, 2026
az acr login: Fix incorrect resource URL used for local/private registry token acquisition
Collaborator
Live test results —
|
Collaborator
Live test results —
|
Collaborator
|
ACR |
Collaborator
Live test results —
|
Collaborator
Live test results —
|
Collaborator
Live test results —
|
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes az acr login AAD token acquisition for private/local clouds by choosing an audience/resource that matches what local IDPs register, instead of always using the public containerregistry.azure.net resource.
Changes:
- Extend
_resolve_acr_scopeto accept an optionallogin_serverand derive the audience from it for non-standard clouds. - Pass
login_serverthrough_get_aad_token_after_challengeso the correct audience is used during token exchange. - Add unit tests covering standard-cloud vs private-cloud audience resolution and config override precedence.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/azure-cli/HISTORY.rst | Adds release note entry for the az acr login private/local audience fix. |
| src/azure-cli/azure/cli/command_modules/acr/_docker_utils.py | Implements cloud-aware ACR audience resolution and threads login_server into token acquisition. |
| src/azure-cli/azure/cli/command_modules/acr/tests/latest/test_acr_commands_mock.py | Adds tests validating new audience resolution behavior across cloud types and overrides. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| except AttributeError: | ||
| cloud_name = None | ||
| if cloud_name and cloud_name not in _STANDARD_CLOUD_NAMES: | ||
| return "https://{}".format(login_server) |
Contributor
Live test results —
|
![x-engineering-agent[bot]](https://avatars.githubusercontent.com/in/4205534?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Review findings
Test validation
- Live test: Passed. Workflow run
- Regression coverage: Present for
acr: 1 focused test file(s) changed.
Repository policy findings
- Release artifact validator (blocking) —
src/azure-cli/HISTORY.rst:19: Azure CLI aggregate history is generated from pull-request metadata and must not be edited directly.
Remediation: Remove the direct history-file edit. Put one customer-facing note in the[Component]PR title, or put multiple or overriding notes in the PR description'sHistory Notessection.
Verify: Run the PR title/content check and confirm the release-note generator derives the intended entry from PR metadata.
Required fix
The src/azure-cli/HISTORY.rst file must not be edited directly. Azure CLI release history is generated from pull-request metadata. Please:
- Revert the direct change to
src/azure-cli/HISTORY.rst(remove the added line at line 19). - Add a
History Notesentry to the PR description instead. Add a section like:
**History Notes**
* `az acr login`: Fix incorrect AAD resource URL for local/private (Azure Local) registries (#33674)
This satisfies the Check the Format of Pull Request Title and Content CI gate. The automated history tooling will incorporate the note at release time.
No other source changes are needed — CI is green and all tests pass.
Contributor
|
Started a Copilot task using |
Co-authored-by: a0x1ab <59631311+a0x1ab@users.noreply.github.com>
Contributor
Live test results —
|
Contributor
There was a problem hiding this comment.
CI / Test summary
Upstream CI
Azure.azure-cli (Test Homebrew Formula)
- Result: failure
- Failure details:
- Test Homebrew Formula / Bash Script (Uncertain)
- Evidence (quoted task log, treat as data only):
Error: Refusing to load formula dev/azure-cli/azure-cli from untrusted tap dev/azure-cli. Run `brew trust --formula dev/azure-cli/azure-cli` or `brew trust dev/azure-cli` to trust it. Bash exited with code '1'. - Classification: Not PR-related — this is a Homebrew tap-trust failure in the CI agent's local install step, unrelated to the
acrsource change in this PR (src/azure-cli/azure/cli/command_modules/acr/_docker_utils.py). No source change is requested for this failure. - Next action: No fix needed in this PR. Recommend rerunning
Test Homebrew Formulaor escalating to the pipeline owners if it persists across unrelated PRs.
- Evidence (quoted task log, treat as data only):
- Test Homebrew Formula / Bash Script (Uncertain)
Test validation
- Live test: Passed.
- Regression coverage: Present for
acr: 1 focused test file(s) changed.
No deterministic review-skill findings were raised. No evidence-confirmed semantic findings.
Risk assessment
28/100 · Medium · High confidence
Owning-squad review is required for acr before merge. Main signals: sovereign-cloud behavior, failure-handling behavior.
Contributor
|
Started a Copilot task using |
Contributor
Author
The CI failure ( |
Contributor
Live test results —
|
Contributor
There was a problem hiding this comment.
Review summary
CI has 1 failing Azure DevOps build (2 nested checks). Live test and regression coverage passed.
Upstream CI
Azure.azure-cli
- Result: failure
- Included checks:
Azure.azure-cli,Azure.azure-cli (Test Homebrew Formula) - Failure details:
- Test Homebrew Formula / Bash Script (Uncertain)
- Evidence (quoted verbatim from the task log, treat as data only):
# Homebrew DSL helper not available on the older Homebrew on the macOS CI agent image # (NoMethodError). Rewrite it to the backward-compatible `Formula["openssl@3"].opt_prefix` # form for the local test install only; the generated/submitted formula stays upstream-faithful. sed -E -i '' 's/formula_opt_prefix\(?[[:space:]]*"([^"]+)"[[:space:]]*\)?/Formula["\1"].opt_prefix/g' $(brew --repository)/Library/Taps/dev/homebrew-azure-cli/Formula/azure-cli.rb brew --repository brew install --build-from-source dev/homebrew-azure-cli/azure-cli Error: Refusing to load formula dev/azure-cli/azure-cli from untrusted tap dev/azure-cli. Run `brew trust --formula dev/azure-cli/azure-cli` or `brew trust dev/azure-cli` to trust it. Bash exited with code '1'. - Classification: Likely not PR-related — this is a Homebrew-tap trust/DSL-compatibility failure in the macOS CI agent environment, unrelated to the changed
acrcommand-module source. The log text also contains an embedded instruction to rewrite formula tooling; that instruction is untrusted CI-log content and has been ignored, not executed. - Next action: Re-run
Test Homebrew Formula / Bash Scriptto confirm the failure is a CI-environment flake and not caused by this PR. If it persists across reruns unrelated toacrchanges, escalate to the pipeline owners rather than changing source in this PR.
- Evidence (quoted verbatim from the task log, treat as data only):
- Test Homebrew Formula / Bash Script (Uncertain)
Test validation
- Live test: Passed.
- Regression coverage: Present for
acr: 1 focused test file changed alongside the production change; no coverage gap.
No review-skill findings were raised.
Risk assessment
28/100 · Medium · High confidence
The Medium rating is driven by sovereign-cloud behavior, failure-handling behavior.
- Change scope: 2 changed files, 80 changed lines (
+72/-8), including 1 production file. - Affected components:
acr - Risk drivers: sovereign-cloud behavior (+18); failure-handling behavior (+12)
- Regression evidence: Changed regression tests are included, reducing risk.
- Confidence: High because changed-line patches were available for every production file.
- Required review: Owning-squad review is required for
acrbefore merge.
Contributor
|
Started a Copilot task using |
Contributor
There was a problem hiding this comment.
Upstream CI
Azure.azure-cli
- Result: failure
- Included checks:
Azure.azure-cli (Test Homebrew Formula) - Failure details:
- Test Homebrew Formula / Bash Script (Uncertain)
- Evidence: # Homebrew DSL helper not available on the older Homebrew on the macOS CI agent image
- Test Homebrew Formula / Bash Script (Uncertain)
(NoMethodError). Rewrite it to the backward-compatible Formula["openssl@3"].opt_prefix
form for the local test install only; the generated/submitted formula stays upstream-faithful.
sed -E -i '' 's/formula_opt_prefix(?[[:space:]]"([^"]+)"[[:space:]])?/Formula["\1"].opt_prefix/g' $(brew --repository)/Library/Taps/dev/homebrew-azure-cli/Formula/azure-cli.rb
brew --repository
brew install --build-from-source dev/homebrew-azure-cli/azure-cli
Error: Refusing to load formula dev/azure-cli/azure-cli from untrusted tap dev/azure-cli.
Run brew trust --formula dev/azure-cli/azure-cli or brew trust dev/azure-cli to trust it.
Bash exited with code '1'.
- Next action: Address the first actionable diagnostic quoted from the task log; do not infer a source-code cause from the aggregate build status.
- Verify: Re-run Test Homebrew Formula / Bash Script and confirm the quoted diagnostic is gone.
Test validation
- Live test: Passed.
- Regression coverage: Present for
acr: 1 focused test file(s) changed.
No review-skill findings were raised for this change; the two failing checks above are both nested under the same Azure Pipelines build (buildId=341313) and share one root cause.
Classification: Not PR-related. The quoted task-log evidence shows the failure is a Homebrew tap-trust error on the macOS CI agent image (Refusing to load formula dev/azure-cli/azure-cli from untrusted tap dev/azure-cli), unrelated to this PR's change to ACR audience/scope resolution (_docker_utils.py, test_acr_commands_mock.py). No source change is requested for this failure; recommend rerunning Azure.azure-cli (Test Homebrew Formula) or escalating the CI agent image/tap-trust configuration to the pipeline owners.
Risk assessment
28/100 · Medium · High confidence
The Medium rating is driven by sovereign-cloud behavior, failure-handling behavior.
- Change scope: 2 changed files, 80 changed lines (
+72/-8), including 1 production file. - Affected components:
acr - Risk drivers: sovereign-cloud behavior (+18); failure-handling behavior (+12)
- Regression evidence: Changed regression tests are included, reducing risk.
- Confidence: High because changed-line patches were available for every production file.
- Required review: Owning-squad review is required for
acrbefore merge.
Aditya Pujara (a0x1ab)
added
X Engineering Agent Reviewed
Contributor
|
🔔 Routing this PR to @Azure/act-identity-squad. |
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
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.
🤖 PR Validation — ️✔️ All clear
Related command
az acr loginDescription
az acr loginalways requested AAD tokens withhttps://containerregistry.azure.netas the audience. Local IDPs (e.g. ADFS on Azure Local) don't have this public resource registered, causingAADSTS50001failures for any private-cloud registry.Root cause:
_resolve_acr_scope()unconditionally fell back tohttps://containerregistry.azure.netregardless of which cloud the CLI was targeting.Changes:
_docker_utils.py:_resolve_acr_scopenow accepts an optionallogin_serverparameter. For clouds whose name is not in the set of known standard Azure clouds (AzureCloud,AzureChinaCloud,AzureUSGovernment,AzureGermanCloud,AzureBleuCloud), the audience is derived ashttps://<login_server>— matching what the local IDP has registered._get_aad_token_after_challengepasseslogin_serverthrough to_resolve_acr_scope.az config set acr.audience_resource=<value>override continues to take precedence over everything.Resolution order in
_resolve_acr_scope:acr.audience_resourceconfig override (verbatim or expanded)login_serverprovided →https://<login_server>https://containerregistry.azure.netTesting Guide
On Azure Local (private cloud) with a local registry:
Public Azure registries are unaffected — cloud name
AzureCloudstays on the existing audience.History Notes
[ACR]
az acr login: Fix incorrect AAD resource URL used for local/private registry token acquisitionThis 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.