[Network] Fix #27579: az network dns zone export: Fix export of empty TXT record sets - #34088
Aditya Pujara (a0x1ab) wants to merge 6 commits into
Conversation
…f empty TXT record sets AI-generated by X Engineering Agent via Azure AI Foundry. Foundry-Job: 5acb833b4854a53d4787b397556ad96a6e6708ffb41eabf63a5392f72a400c69
x-engineering-agent
Bot
added
the
Request X Engineering Agent
microsoft-github-policy-service
Bot
added
Network
|
Network |
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
🟢 Approval recommended
The reviewed changes are fully covered and no blocking issues were identified.
Pull request overview
Fixes az network dns zone export for empty TXT record sets and preserves them during import.
Changes:
- Emits
TXT ""for empty TXT record sets. - Preserves empty TXT values during parsing.
- Adds export, import, and round-trip regression tests.
File summaries
| File | Changes |
|---|---|
src/azure-cli/azure/cli/command_modules/network/zone_file/parse_zone_file.py |
Preserves empty TXT fragments. |
src/azure-cli/azure/cli/command_modules/network/tests/latest/test_dns_commands.py |
Adds TXT export/import regression coverage. |
src/azure-cli/azure/cli/command_modules/network/custom.py |
Emits empty TXT values during export. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
/azp run |
|
Azure Pipelines: Successfully started running 3 pipeline(s). |
Live test results —
|
![x-engineering-agent[bot]](https://avatars.githubusercontent.com/in/4205534?s=60&v=4){kind=small}
There was a problem hiding this comment.
Reviewed e50e67811bc39eeab2e24be93e618cf6d62472ea from the managed a0x1ab/azure-cli fork and agent-assist/azure-azure-cli-issue-27579-5acb833b4854 branch. The live-test gate failed; this is not an all-green validation.
The bounded release-artifact, test-strength, scope-consistency, and domain-edge-case questions produced no additional confirmed semantic defect. The empty-TXT export branch supplies the missing txt field, and the TXT-specific parser change preserves quoted empty strings without changing NAPTR handling. The six added deterministic tests assert output, request mappings, round trips, empty/missing/null collections, multi-value and 255-character boundaries, and malformed-record exception propagation.
Upstream CI
All 53 completed upstream checks passed and matched current head e50e67811bc39eeab2e24be93e618cf6d62472ea in the completed one-shot gate. They do not override the separate live-test failure.
Test validation
- Live test: Failed with conclusion
failure. Workflow run - Regression coverage: Present for
network: 1 focused test file(s) changed.
Task-level evidence: Job azdev test --live (PR 34088), step 12, Run azdev test (live, series) — changed test files only, ran src/azure-cli/azure/cli/command_modules/network/tests/latest/test_dns_commands.py::DnsZoneImportTest::test_dns_zone10_import. Its log reports 1 failed, 1 passed, 1 skipped in 45.81s and stopping after 1 failures. Step 12 captures the test exit status; step 18, Set workflow conclusion, then reports azdev tests failed (exit 1). and fails the workflow.
The failing operation is the second import after deleting the zone, reaching the SOA lookup. The diagnostic is azure.core.exceptions.ResourceNotFoundError: (ParentResourceNotFound) and Failed to perform 'read' on resource(s) of type 'dnszones/SOA'; the response is 404 Not Found. The first import reports 7/7 RECORDS IMPORTED SUCCESSFULLY.
Relevance: not PR-related to the TXT change. The path-based failure helper marks the test relevant because its file changed, but the actual src/azure-cli/azure/cli/command_modules/network/tests/latest/zone_files/zone10.txt and captured export contain only SOA, NS, DS, and TLSA records: neither new TXT branch applies. The failing delete/re-create/SOA-read path is unchanged. The logs establish the missing-parent failure, not its underlying service/lifecycle cause; they do not prove a DNS propagation race or a regression introduced by this PR.
Coverage details: src/azure-cli/azure/cli/command_modules/network/tests/latest/test_dns_commands.py adds DnsZoneExportTest; the coverage helper reports no changed-file coverage gap. No recording changes are required merely for these mocked deterministic tests. The existing scenario recording src/azure-cli/azure/cli/command_modules/network/tests/latest/recordings/test_dns_zone10_import.yaml is unchanged. This live run stopped after the third of 38 collected items and provides no execution evidence for the new empty-TXT unit class or the remaining scenarios.
Required follow-up and focused verification: Investigate the existing DnsZoneImportTest._test_zone delete/re-create lifecycle and the zone-create/SOA-read responses before changing behavior. Reproduce the failure independently of TXT changes; fix only a confirmed lifecycle/readiness cause, without masking the 404, skipping the scenario, or changing unrelated TXT handling on speculation. If the scenario changes, refresh its recording after a successful authorized live run. Verify the six DnsZoneExportTest unit tests, then have the authorized tester rerun DnsZoneImportTest::test_dns_zone10_import live and the focused test_dns_commands.py selection. No test or rerun was started during this review. The agent-review iteration state is 0/3, so this feedback is eligible for one durable Foundry follow-up rather than an exhausted-cap handoff.
Risk assessment
45/100 · Medium · High confidence
The Medium rating is driven by security-sensitive behavior, public CLI behavior.
- Change scope: 3 changed files, 309 changed lines (
+309/-0), including 2 production files. - Affected components:
network - Risk drivers: security-sensitive behavior (+28); public CLI behavior (+18)
- 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
networkbefore merge.
x-engineering-agent
Bot
added
the
X Engineering Agent Reviewed
|
🔔 Routing this PR to @Azure/act-quality-productivity-squad. |
|
@Azure/x-engineering-agent The remediation execution for the current agent review was canceled by the agent deployment before model work began. Retry the existing live-test remediation against the unchanged current head without expanding its scope. |
|
Automated processing requires maintainer follow-up for this request. |
Live test results —
|
There was a problem hiding this comment.
Live validation requires follow-up
Upstream CI passed, but live validation failed. This is not an all-green result; the observed failure does not exercise the changed TXT behavior.
Summary
- Reviewed head:
8190326d657a44c9973386c61b0594bdfebf16ec. - Upstream CI: 50/50 completed checks passed, with no pending or failed checks in the supplied snapshot.
- Remaining failure groups by relevance to this diff: 0 PR-related, 1 not PR-related, 0 uncertain. These counts classify relevance, not whether the underlying service/lifecycle cause is fully established.
- Static inspection found no evidence-confirmed semantic defect in the empty-TXT changes. The existing non-TXT scenario and its lifecycle helpers are unchanged from base
10f0350504f3911fd0955b1111093673efc136cb.
Test validation
-
Live test: Failed with conclusion
failure. Workflow run -
Regression coverage: Present for
network: 1 focused test file(s) changed. -
Executed coverage is incomplete: the task collected 38 tests, then stopped after one failure: 1 failed, 2 passed, 1 skipped. None of the six new
DnsZoneExportTestmethods ran in this live attempt. The failed wrapper conclusion is not an additional test failure. -
Recording attention: no recording file changed. The new tests mock service commands and exercise serialization, import arguments, empty/null/missing TXT collections, multiple values, round-trips and malformed input; they do not establish a live service round-trip. Because public TXT output/import behavior changes, the Network owner should review recording risk and require a focused re-recording plus playback if service integration evidence is needed.
-
Single failing live task — Not PR-related to the TXT diff: Run azdev test, step 12, followed by exit-code propagation, step 19. The failure is
DnsZoneImportTest::test_dns_zone11_import. Its unchangedzone_files/zone11.txtfixture contains SOA, NS and NAPTR, no TXT.- Smallest relevant task-log excerpts (capability diagnostic and terminal exception are distinct):
<<<UNTRUSTED:live-test-task-log>>>
(BadRequest) The feature NAPTR is not available.
Code: ParentResourceNotFound
Failed to perform 'read' on resource(s) of type 'dnszones/SOA'
1 failed, 2 passed, 1 skipped in 75.69s (0:01:15)
<<<END:live-test-task-log>>>
- Capability evidence: initial import logs the NAPTR service rejection, imports 5/7 records and exports SOA/NS. The existing importer catches that record-creation error; NAPTR unavailability is not the terminal exception.
- Terminal traceback: the unchanged import/export/delete/reimport helper, line 61 fails during its second import at
import_zone's SOA read, line 2832, because the parent zone cannot be found. - The additions only affect empty TXT export and TXT-token preservation; neither applies to this fixture or its SOA/NS export. The NAPTR rejection is a service-capability issue, not a TXT defect. The fatal missing-parent cause remains unproven: these logs do not distinguish service consistency from pre-existing lifecycle behavior, or establish that NAPTR caused the 404.
Bounded next action and focused verification
- Keep the empty-TXT scope intact. Do not add a speculative NAPTR/lifecycle fix, disable the existing scenario, weaken assertions, or change unrelated modules to make this run green.
- Foundry follow-up: verify the new deterministic export regression class separately and assess only evidence-backed remediation relevant to this PR. If the remaining failure is environmental or pre-existing, leave product source unchanged and return the precise blocker rather than inventing a source fix.
- In the configured repository test environment, run
python -m pytest src/azure-cli/azure/cli/command_modules/network/tests/latest/test_dns_commands.py::DnsZoneExportTest -qseparately so fail-fast in the earlier scenario cannot hide its result. - Network/live-test owners: confirm NAPTR availability in the validation environment, reproduce
DnsZoneImportTest::test_dns_zone11_importat the base and reviewed head, and capture delete/create/SOA-read diagnostics if the parent-resource 404 persists. Escalate the capability/lifecycle evidence to the service or test-infrastructure owner as appropriate; do not assume an unawaited deletion. - After that prerequisite is resolved, the authorized test owner should rerun the existing changed-file validation (
azdev test test_dns_commands --live --series --discover). Verify the focused export class and the named import scenario; if recordings are updated, verify their playback without--livebefore publication. This review did not dispatch or rerun tests.
Automated follow-up
The agent_review cycle is below its cap (1/3 before this request). This COMMENT requests one bounded durable Foundry follow-up, not approval or a new live-test dispatch. It does not repeat the already-handled human-feedback request.
Risk assessment
45/100 · Medium · High confidence
The Medium rating is driven by security-sensitive behavior, public CLI behavior.
- Change scope: 3 changed files, 309 changed lines (
+309/-0), including 2 production files. - Affected components:
network - Risk drivers: security-sensitive behavior (+28); public CLI behavior (+18)
- 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
networkbefore merge.
|
Automated processing requires maintainer follow-up for this request. |
There was a problem hiding this comment.
Focused review: unrelated DNS validation failure
Nonblocking review: no source change is requested for the observed failure. Live validation remains incomplete.
Summary
- Reviewed head:
8190326d657a44c9973386c61b0594bdfebf16ec. - Failed test cases: 0 PR-related, 1 not PR-related, 0 uncertain, in one workflow group.
- Upstream CI: 50/50 passed in the supplied snapshot; the separate live-test workflow failed.
- All seven repository review skills were considered; no deterministic or confirmed changed-line semantic finding was identified.
Test validation
- Live test: Failed with conclusion
failure. Workflow run - Regression coverage: Present for
network: 1 focused test file(s) changed. - Out-of-scope live-test failures:
src/azure-cli/azure/cli/command_modules/network/tests/latest/test_dns_commands.py::DnsZoneImportTest::test_dns_zone11_import— Not PR-related: this existing SOA/NS/NAPTR-only scenario hit unavailable NAPTR and then a missing SOA parent during delete/reimport. Neither changed TXT-only branch is exercised by its fixture or captured export.- Task evidence: Run azdev test (live, series), step 12, log timestamp 16:36:57 UTC. The final “Set workflow conclusion” step propagates this same failure, not a second failure.
<<<UNTRUSTED:live-test-task-log>>> (BadRequest) The feature NAPTR is not available. Code: ParentResourceNotFound 1 failed, 2 passed, 1 skipped in 75.69s (0:01:15) <<<END_UNTRUSTED:live-test-task-log>>> - Attribution:
zone11.txt:1–8contains only SOA, NS and NAPTR; the captured export contains only SOA/NS. The traceback reaches the existing delete/reimport sequence,test_dns_commands.py:53–61, thencustom.py:2832, where the SOA read returns HTTP 404.import_zoneis unchanged from the PR base. The production additions are restricted to empty TXT export,custom.py:2672–2673, and TXT-only empty-token preservation,parse_zone_file.py:311–313. The logs do not establish the underlying cause of the missing parent; they do establish that these TXT changes are not on the failed path.
- Task evidence: Run azdev test (live, series), step 12, log timestamp 16:36:57 UTC. The final “Set workflow conclusion” step propagates this same failure, not a second failure.
- Execution gap: 38 tests were collected, but fail-fast stopped after four results (1 failed, 2 passed, 1 skipped), leaving 34 unexecuted. The six new
DnsZoneExportTestmethods were not reached. Playback verification and recording publication were skipped; this run is not evidence that the new tests passed. - Exact added tests: in
src/azure-cli/azure/cli/command_modules/network/tests/latest/test_dns_commands.py::DnsZoneExportTest:test_export_empty_txt_collections,test_export_valid_txt_values,test_parse_empty_txt_fragments_and_naptr_regexp,test_export_mixed_records_and_import_supported_types,test_export_import_and_reexport_txt, andtest_export_malformed_nonempty_txt_is_not_masked. They cover console/file output, missing/null/empty collections, valid/multiple/chunked values, mixed/alias records, mocked import arguments, re-export and malformed-value exceptions. - Output/request/recording risk: empty TXT sets now export as
IN TXT ""; the parser preserves empty TXT values rather than converting them to the NAPTREMPTYmarker, affecting the TXT values sent on re-import. No transport, API-version or response-schema change is present. No recordings changed. The new tests mock service calls, so they do not verify live service acceptance. The failing scenario's existing cassette issrc/azure-cli/azure/cli/command_modules/network/tests/latest/recordings/test_dns_zone11_import.yaml; it is unchanged and has no verified replacement from this run. Do not rewrite that cassette or change NAPTR/SOA code to hide this unrelated failure.
Focused validation and escalation
- In a prepared Azure CLI development environment, run
python -m unittest azure.cli.command_modules.network.tests.latest.test_dns_commands.DnsZoneExportTestindependently so the existing scenario cannot mask these regressions. - The live-test runner/Network DNS owners should rerun the existing workflow with NAPTR available, or run the focused TXT tests separately. If the SOA parent lookup still fails after zone recreation, escalate this job's diagnostics to those owners as a separate service/lifecycle investigation; do not broaden this PR.
- After restoring the live-test prerequisite, validate
azdev test test_dns_commands --live --series, thenazdev test test_dns_commands --seriesfor playback. This review has not dispatched a rerun or executed local tests.
Risk assessment
45/100 · Medium · High confidence
The Medium rating is driven by security-sensitive behavior, public CLI behavior.
- Change scope: 3 changed files, 309 changed lines (
+309/-0), including 2 production files. - Affected components:
network - Risk drivers: security-sensitive behavior (+28); public CLI behavior (+18)
- 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
networkbefore merge.
x-engineering-agent
Bot
added
Request X Engineering Agent
|
🔔 Routing this PR to @Azure/act-quality-productivity-squad. |
Live test results —
|
There was a problem hiding this comment.
Automated follow-up reached its limit after 3 attempts for the feedback at #34088 (comment). Leaving this PR for manual follow-up.
x-engineering-agent
Bot
removed
the
Request X Engineering Agent
|
Automated processing requires maintainer follow-up for this request. |
|
Automated processing requires maintainer follow-up for this request. |
x-engineering-agent
Bot
added
the
Request X Engineering Agent
There was a problem hiding this comment.
Review: live regression needs attribution
Reviewed head 8190326d657a44c9973386c61b0594bdfebf16ec. The empty-TXT export and parser changes have focused regression coverage, but the current-head live run failed. This is not a passing live-validation summary.
Upstream CI
The current-head snapshot has 51 passing checks, zero failures and zero pending checks. There is no upstream CI source failure to repair.
Test validation
- Live test: Failed with conclusion
failure. Workflow run - Regression coverage: Present for
network: 1 focused test file(s) changed. - Unattributed live-test failures:
src/azure-cli/azure/cli/command_modules/network/tests/latest/test_dns_commands.py::DnsZoneImportTest.test_dns_zone10_import— The current-head run reports ResourceNotFoundError (ParentResourceNotFound) on the SOA read after deleting and re-importing the zone; PR causality has not been established.
Live failure evidence and focused follow-up
Classification: uncertain. The latest workflow result reports 1 failed, 1 passed, 1 skipped, with DnsZoneImportTest.test_dns_zone10_import failing during re-import at test_dns_commands.py:61. The quoted error is ResourceNotFoundError: (ParentResourceNotFound) on dnszones/SOA, after a 404 Not Found response. The earlier attempt stopped in test_dns_zone11_import on the same operation. These are one unresolved zone-lifecycle validation concern, not separate TXT defects.
The inspected import path invokes zone creation and then reads the root SOA. Neither call is changed by this PR, and the failure occurs before TXT record writes. The available evidence does not prove that the TXT changes caused the missing parent zone; do not guess a service-timing cause or make unrelated production changes.
For this bounded follow-up, establish whether the create/delete/re-import failure reproduces independently of the patch using the authorized workflow and the retained live-test-pr-34088 artifacts. Check the create/delete responses and the failing SOA request. If a PR-related cause is demonstrated, make the smallest corrective change and cover that exact runtime path. If it is pre-existing or environmental, retain the source unchanged and document the evidence for the workflow/network owners instead of masking the error with broad retries or weakening assertions.
Focused validation is src/azure-cli/azure/cli/command_modules/network/tests/latest/test_dns_commands.py: preserve the new DnsZoneExportTest cases, exercise test_dns_zone10_import and test_dns_zone11_import through the authorized live workflow, and verify missing/null/empty TXT collections, mixed valid TXT values, TTL zero, console/file output, and export/import/re-export behavior. The failed run's regenerated cassette is an artifact only; do not publish it until live validation and playback succeed. No generated AAZ files need a speculative edit.
Changed-line review
No deterministic repository-policy finding or evidence-confirmed semantic defect was found in the changed lines. The tests exercise one-shot record iteration, output/request mappings, empty values, long/multiple TXT values, malformed input propagation and NAPTR compatibility. The CI result does not replace the unresolved live evidence above.
Risk assessment
45/100 · Medium · High confidence
The Medium rating is driven by security-sensitive behavior, public CLI behavior.
- Change scope: 3 changed files, 309 changed lines (
+309/-0), including 2 production files. - Affected components:
network - Risk drivers: security-sensitive behavior (+28); public CLI behavior (+18)
- 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
networkbefore merge.
|
Automated processing requires maintainer follow-up for this request. |
Live test results —
|
Live test results —
|
There was a problem hiding this comment.
Automated follow-up reached its limit after 3 attempts for the feedback at #34088 (comment). Leaving this PR for manual follow-up.
x-engineering-agent
Bot
removed
the
Request X Engineering Agent
Live test results —
|
There was a problem hiding this comment.
Automated review passed
No deterministic or confirmed changed-line findings at 18eb51698ff43f899c42f4992823ab71fbf08b9b. This COMMENT is not an approval; owning-squad review remains required.
Upstream CI
50 / 50 checks passed in the supplied exact-head snapshot. No local tests were run for this review.
Review evidence
- Release and scope: The
[Network]title and History Notes describe the empty-TXT export fix. Changes are confined to handwritten network export/parser code and focused regressions; no generated artifacts, API versions or endpoints change. - Runtime and domain: The empty-record fallback now supplies the TXT value required by the existing quoting serializer. TXT-specific parsing retains empty strings without applying NAPTR's
EMPTYconvention. The AAZ list response uses the flattened fields represented by the fixtures; its paging and request/error paths are unchanged. - Test strength: The new regressions exercise actual export, parsing and import code, checking console/file parity, missing/null/empty collections, zero TTL, multiple and long values, NAPTR isolation, import arguments, re-export and malformed-value errors. The export fixture is a one-shot iterator rather than an assumed reusable list.
- Seven-skill accounting: Release-artifact, test-strength, scope-consistency and domain-edge-case targets were inspected. Generated-ownership, command/help and new user-intent-interface checks are not applicable to this diff; existing file-output arguments and TXT data flow were traced above.
Test validation
- Live test: Passed. Workflow run
- Regression coverage: Present for
network: 1 focused test file(s) changed.
Recording caveat: No recordings changed. The focused unit tests establish local serialization and import-argument behavior, not service acceptance of re-imported empty TXT values. Because zone-file output changes, the network squad should confirm DNS import/export recording realism before merge; deterministic coverage being present is not a claim of complete integration coverage.
Risk assessment
45/100 · Medium · High confidence
The Medium rating is driven by security-sensitive behavior, public CLI behavior.
- Change scope: 3 changed files, 309 changed lines (
+309/-0), including 2 production files. - Affected components:
network - Risk drivers: security-sensitive behavior (+28); public CLI behavior (+18)
- 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
networkbefore merge.
Live test results —
|
There was a problem hiding this comment.
Review
No actionable findings across all seven review lenses at f7cc58b18a4dff2f9e817f1637b91a3de7f1839b (0 deterministic; 0 confirmed semantic). The supplied round snapshot has 50/50 upstream checks passed. No tests were executed locally. This COMMENT is not approval or merge authorization.
custom.py:2672–2673supplies the empty TXT value after missing/null/empty collections normalize to an empty list; populated TXT joining and non-TXT branches remain unchanged.parse_zone_file.py:311–313preserves empty TXT strings without changing NAPTR’sEMPTYconvention.DnsZoneExportTestasserts exact console/file output, apex/zero TTL, multiple/chunked values, import request mappings, re-export equality, and malformed-record exceptions.
Test validation
- Live test: Passed. Workflow run
- Regression coverage: Present for
network: 1 focused test file(s) changed.
Output and recording attention
No recordings changed. The fix changes empty-TXT serialization/import behavior, not transport or endpoints; Network reviewers should confirm existing DNS playback expectations remain compatible. The focused unit tests exercise the real serializer/parser with mocked service boundaries.
Risk assessment
45/100 · Medium · High confidence
The Medium rating is driven by security-sensitive behavior, public CLI behavior.
- Change scope: 3 changed files, 309 changed lines (
+309/-0), including 2 production files. - Affected components:
network - Risk drivers: security-sensitive behavior (+28); public CLI behavior (+18)
- 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
networkbefore merge.
🤖 PR Validation — ️✔️ All clear
Related command
az network dns zone exportDescription
Fixes #27579.
Correct DNS zone export so empty TXT record sets serialize without indexing a nonexistent value.
Expected behavior: exporting a zone containing an empty TXT record set must not crash, while retaining valid TXT data and the existing zone-file conventions. Both console output and optional file output should work.
Testing Guide
az network dns zone export --resource-group <resource-group> --name <dns-zone> --file-name zone.txt.src/azure-cli/azure/cli/command_modules/network/tests/latest/test_dns_commands.py.History Notes
[Network]
az network dns zone export: Fix export of empty TXT record setsThis 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.