[ACR] Fix #33660: az acr network-rule list: Fix null virtualNetworkResourceId and add virtualNetworkSubnetResourceId#33685
Open
johnsonshi wants to merge 1 commit into
Conversation
…Rules showing null subnet id The ACR registry REST API returns the virtual network rule's subnet resource ID under the `virtualNetworkSubnetResourceId` key (confirmed against the live 2021-08-01-preview pin and the 2026-03-01-preview re-GA API). The CLI read the stale `id` key, so `az acr network-rule list/add` emitted `virtualNetworkResourceId: null`, and `az acr network-rule remove` never matched a rule to delete. - _format_registry_response: read `virtualNetworkSubnetResourceId` (fallback `id`) and surface both `virtualNetworkResourceId` (kept for backward compat) and `virtualNetworkSubnetResourceId`. - acr_network_rule_add: send `virtualNetworkSubnetResourceId`. - acr_network_rule_remove: match on `virtualNetworkSubnetResourceId` (fallback `id`) so rules are actually removed. - Update scenario test assertions and regenerate the recording to reflect the current API field name. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
️✔️AzureCLI-FullTest
|
️✔️AzureCLI-BreakingChangeTest
|
microsoft-github-policy-service
Bot
requested review from
jsntcy,
necusjz,
northtyphoon,
terencet-dev and
yonzhan
microsoft-github-policy-service
Bot
added
Network
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes ACR az acr network-rule list/add/remove to correctly read/write virtual network rule subnet IDs when using direct REST calls (since networkRuleSet.virtualNetworkRules isn’t modeled in the current SDK for the pinned API version).
Changes:
- Normalize virtual network rule parsing to read
virtualNetworkSubnetResourceId(fallback to legacyid) and surface bothvirtualNetworkResourceId(back-compat) andvirtualNetworkSubnetResourceIdin CLI output. - Update
acr network-rule addto send the subnet undervirtualNetworkSubnetResourceId, andacr network-rule removeto match/remove rules using that key (fallbackid). - Extend the live/playback scenario test to assert
virtualNetworkSubnetResourceIdis present and correct.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/azure-cli/azure/cli/command_modules/acr/network_rule.py | Fixes field mapping for VNet rules in REST payloads/responses and ensures remove matches the correct key (with legacy fallback). |
| src/azure-cli/azure/cli/command_modules/acr/tests/latest/test_acr_network_rule_commands.py | Adds assertions to cover the new/expected virtualNetworkSubnetResourceId output in both registry and list command responses. |
| src/azure-cli/HISTORY.rst | Documents the behavior fix in the release history. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Collaborator
|
ACR |
Contributor
Author
yonzhan
removed
Network
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.
Fixes #33660
Related command
az acr network-rule list(root cause also affectsaz acr network-rule addandaz acr network-rule remove)Description
az acr network-rule listreturned every virtual network rule withvirtualNetworkResourceId: null, and the expectedvirtualNetworkSubnetResourceIdfield was missing (reported on 2.85.0 / 2.86.0 / 2.87.0).Root cause:
network_rule.pytalks to the registry REST API directly (thenetworkRuleSet.virtualNetworkRulesproperty is not modeled in the currentazure-mgmt-containerregistrySDK, so the module is pinned to2021-08-01-preview). The code read the subnet resource ID fromrule.get('id'), but the API returns it under the keyvirtualNetworkSubnetResourceId. As a result:list/addemittedvirtualNetworkResourceId: null(theidkey does not exist in the response).removefiltered onx.get('id'), which was alwaysNone, so a matching rule was never actually removed.I verified the real field name directly against the ARM REST API (
GET/PATCHon a Premium registry). ThevirtualNetworkRulesarray is exposed by2021-08-01-preview(the current pin) and, after being absent from the intervening API versions, is present again in the latest2026-03-01-preview; in both versions the subnet id is returned undervirtualNetworkSubnetResourceId:virtualNetworkRulesvirtualNetworkSubnetResourceIdvirtualNetworkSubnetResourceIdChanges (
src/azure-cli/azure/cli/command_modules/acr/network_rule.py):_format_registry_responsenow readsvirtualNetworkSubnetResourceId(falling back toidfor older response shapes) and surfaces bothvirtualNetworkResourceId(kept for backward compatibility with existing scripts) andvirtualNetworkSubnetResourceIdin the output.acr_network_rule_addsends the subnet undervirtualNetworkSubnetResourceId(the API accepts it and echoes it back).acr_network_rule_removematches onvirtualNetworkSubnetResourceId(fallbackid) so rules are removed correctly.Resulting behavior:
The pinned API version is intentionally left unchanged — this is a surgical fix for the field mapping. Reading
virtualNetworkSubnetResourceIdis forward-compatible with the newer API version above.Out of scope / future work (service endpoint GA): The virtual-network (service endpoint) rules capability is being re-introduced in newer preview APIs — it is present again in
2026-03-01-preview— and is progressing toward a future GA. Bumping this module's pinned API version (or, onceazure-mgmt-containerregistrymodelsnetworkRuleSet.virtualNetworkRulesagain, migrating these calls back to the SDK) is best done as part of that GA work and is intentionally out of scope for this bug fix. The fix is version-independent:virtualNetworkSubnetResourceIdis identical on the pinned2021-08-01-previewand on2026-03-01-preview, so it holds across a future bump.Field naming and compatibility (
virtualNetworkResourceIdvsvirtualNetworkSubnetResourceId)The output intentionally carries both keys, both populated with the same subnet resource ID. This is deliberate for forward and backward compatibility, and it lines up with how the property is encoded in the API:
In the
2021-08-01-previewswagger, the property is the wire fieldidwithx-ms-client-name: VirtualNetworkResourceId, and its description is literally "Resource ID of a subnet, for example:/subscriptions/{…}/virtualNetworks/{vnetName}/subnets/{subnetName}." In other words,virtualNetworkResourceIdis a documented misnomer — the name says "virtual network", but the value has always been a subnet id:The deployed service — and the re-GA
2026-03-01-preview— returns that same subnet id under the accurately-namedvirtualNetworkSubnetResourceId. So the CLI surfaces both:virtualNetworkSubnetResourceId(correct, matches the current/re-GA API — forward compatible) andvirtualNetworkResourceId(the name the CLI has always emitted — backward compatible).Why not just replace it?
virtualNetworkResourceIdis the existing output contract: user scripts,--queryexpressions, and the module's own scenario-test assertions read it. Dropping it or renaming it would be a breaking change for anyone parsingaz acr network-rule listoutput today, for no functional gain. Emitting both keeps existing consumers working while giving new consumers the correctly-named field.How the commands read/write the property (code walkthrough)
All three subcommands go through two thin REST helpers in
network_rule.py, both targeting.../registries/{name}?api-version=2021-08-01-preview:_get_registry(...)→send_raw_request(cli_ctx, "GET", url)— reads the registry JSON._update_registry(...)→send_raw_request(cli_ctx, "PATCH", url, body=...)— writes back a{"properties": {"networkRuleSet": ...}}payload.addandremoveare read-modify-write: GET the currentnetworkRuleSet, mutate thevirtualNetworkRuleslist in memory, PATCH the whole set back, then format the PATCH response with the same read path aslist.az acr network-rule list— read path_get_registryGETs the registry, then_format_registry_responseruns_format_virtual_network_ruleon each entry.{"action": "Allow", "virtualNetworkSubnetResourceId": "<subnet-id>"}. The helper doessubnet_id = rule.get('virtualNetworkSubnetResourceId') or rule.get('id')and emits bothvirtualNetworkResourceIdandvirtualNetworkSubnetResourceId.rule.get('id')— a key the response does not contain — sovirtualNetworkResourceIdcame backnull.az acr network-rule add— write, then readvirtual_network_rules.append({'virtualNetworkSubnetResourceId': subnet_id, 'action': 'Allow'}).networkRuleSet, then format the PATCH response (same read path aslist).idorvirtualNetworkSubnetResourceIdon the request but always echoesvirtualNetworkSubnetResourceId; sending the same key we read keeps write/read symmetric.az acr network-rule remove— read, filter, write(x.get('virtualNetworkSubnetResourceId') or x.get('id') or '').lower() != subnet_id— then PATCH the result.x.get('id')(alwaysNone), so nothing ever matched and the rule was never removed.Request/response shape (
2021-08-01-preview):Testing Guide
Automated:
test_acr_network_rule(AcrNetworkRuleCommandsTests).azdev test test_acr_network_rule --live→1 passed in 68.27s. The scenario creates a VNet + subnet with aMicrosoft.ContainerRegistryservice endpoint, a Premium registry with--default-action Deny, then adds/lists/removes VNet and IP rules and asserts bothvirtualNetworkResourceIdand the newvirtualNetworkSubnetResourceIdequal the subnet id. The HTTP recording was regenerated from this live run so CI playback exercises the current API field name.azdev test test_acr_network_rule→1 passed in 6.27s.azdev style acrandazdev linter acrboth PASSED.Live validation transcript (actual runs; registry/VNet/subnet names genericized and subscription id scrubbed to
00000000-…).(1) Raw ARM ground truth — why the old code failed. Send the rule under
id(exactly what the pre-fix CLI sent) and read back the raw response:{ "defaultAction": "Deny", "ipRules": [], "virtualNetworkRules": [ { "action": "Allow", "virtualNetworkSubnetResourceId": "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myrg/providers/Microsoft.Network/virtualNetworks/myvnet/subnets/mysubnet" } ] }The request used
id, but the service stored and returnedvirtualNetworkSubnetResourceId— neverid. The pre-fix CLI readrule.get('id'), hence thenull.(2) End-to-end with the fixed CLI against a Premium registry (
--default-action Deny) whose subnet has aMicrosoft.ContainerRegistryservice endpoint:{ "ipRules": [], "virtualNetworkRules": [ { "action": "Allow", "virtualNetworkResourceId": "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myrg/providers/Microsoft.Network/virtualNetworks/myvnet/subnets/mysubnet", "virtualNetworkSubnetResourceId": "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myrg/providers/Microsoft.Network/virtualNetworks/myvnet/subnets/mysubnet" } ] }Both keys are populated with the subnet id (pre-fix:
virtualNetworkResourceId: null, novirtualNetworkSubnetResourceId). Then remove actually deletes the rule (pre-fix: silent no-op):{ "ipRules": [], "virtualNetworkRules": [] }History Notes
[ACR]
az acr network-rule list/add/remove: FixvirtualNetworkRulesentries returningvirtualNetworkResourceIdas null and surface thevirtualNetworkSubnetResourceIdfieldThis 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.