Repository navigation
Conversation
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
/azp run |
|
Azure Pipelines: Successfully started running 3 pipeline(s). |
|
Please fix CI issues |
Production MI validation — 2026-10-12Validated commit
The dropped test backups remain under their configured retention; they were not purged. Use the original textual Still blocked: this MI reports immutability These were live smoke checks, not newly captured/re-recorded scenario cassettes. Existing recording refresh and standard full-module validation remain outstanding; keep this PR in draft. No implementation changes were required by the supported-path checks. |
|
/azp run |
|
Azure Pipelines: Successfully started running 3 pipeline(s). |
|
/azp run |
|
Azure Pipelines: Successfully started running 3 pipeline(s). |
|
/azp run |
|
Azure Pipelines: Successfully started running 3 pipeline(s). |
|
/azp run |
|
Commenter does not have sufficient privileges for PR 34207 in repo Azure/azure-cli |
|
/azp run |
|
Azure Pipelines: Successfully started running 3 pipeline(s). |
There was a problem hiding this comment.
🟡 Changes recommended
A successful MI database creation can leak a billable resource when an immediate output check fails before cleanup is enabled.
1 open finding
What changed in this PR
Adds SQL short-term backup retention immutability support for SQL Database and Managed Instance policies.
Changes:
- Adds locking, confirmation, status output, and preview API support.
- Introduces shared AAZ HTTP/LRO handling with final policy reads.
- Adds comprehensive unit, scenario, safety, and playback coverage.
| File | Description |
|---|---|
sql/_help.py |
Documents immutability workflows. |
sql/_params.py |
Defines locking and confirmation arguments. |
sql/_str_policy.py |
Implements shared STR HTTP operations. |
sql/commands.py |
Registers custom STR handlers. |
sql/custom.py |
Adds validation and confirmation orchestration. |
tests/latest/README.md |
Documents recording and safety procedures. |
recordings/test_sql_db_existing_policy_immutability.yaml |
Records existing-policy locking. |
recordings/test_sql_db_short_term_retention.yaml |
Records standard SQL DB retention. |
recordings/test_sql_db_short_term_retention_immutability.yaml |
Records SQL DB locking. |
recordings/test_sql_managed_db_short_retention.yaml |
Records standard MI retention. |
recordings/test_sql_managed_db_short_retention_immutability.yaml |
Records MI locking. |
tests/latest/test_sql_commands.py |
Removes superseded scenarios. |
tests/latest/test_sql_db_str_policy.py |
Adds SQL DB scenarios and safeguards. |
tests/latest/test_sql_mi_str_policy.py |
Adds MI scenarios and safeguards. |
tests/latest/test_sql_str_policy.py |
Adds offline HTTP and CLI tests. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| self.cmd('sql midb create --subscription {subscription} -g {rg} --mi {mi} -n {db}', checks=[ | ||
| self.check('name', '{db}'), self.check('status', 'Online')]) | ||
| self.database_created = True |
|
🔔 Routing this PR to @Azure/act-observability-squad. |
|
🔔 Routing this PR to @Azure/act-codegen-extensibility-squad. |
|
/azp run |
|
Azure Pipelines: Successfully started running 3 pipeline(s). |
| ) | ||
|
|
||
|
|
||
| class _ShortTermRetentionPolicy(AAZCommand): |
There was a problem hiding this comment.
- Please generate this with AAZ, this should generally end up in a
/aazsub directory. You can refer to the following for more information https://azure.github.io/aaz-dev-tools/ - Provide the appropriate upstream PR after completing step 1
There was a problem hiding this comment.
Addressed in e2b0117fd6 (AAZ migration commit 9451dbe62e). The six STR GET/PUT bases are now generated with aaz-dev-tools 4.7.1 under the SQL aaz directory. The remaining custom layer preserves the existing public handlers and the already-tested final policy GET behavior; request/response schemas are generated.
Upstream command models: Azure/aaz#1130. Both PR descriptions are cross-linked.
Validation: 47 offline tests passed, including all five recorded scenarios and execution of all four generated public-command examples; SQL AAZ metadata verification, flake8, compilation, and adapter pylint passed. No new live resource operations were performed. Full legacy-module pylint has no new diagnostics versus its parent; local environment limitations are documented in the PR.
X Engineering Agent ReportReview — Comments Reviewed head Confirmed finding — [P2] Record database ownership before checking creation output
The ordering is confirmed by Remediation: Separate successful Focused validation: Add failure-path tests where successful creation is followed by failed name/status checks or JSON parsing. Assert cleanup deletes the owned database unless explicitly retained. Evidence and remaining validationTests and recordings establish Enabled-before-lock preconditions, unchanged-retention locking, SET Metadata identifies Azure/aaz#1130, generation with aaz-dev-tools 4.7.1, and pinned specification inputs; generated ownership is documented, not presumed hand-editing. The author reports current-head 50 offline tests and eight live interval updates; these are author evidence, not Agent runs. CI succeeded, but cleanup-on-successful-create/output-validation-failure scenario coverage is not verified. Please add the focused regressions above and rerun both scenario safety suites and the five recorded scenarios. Test validation
Risk assessment — 83/100 · CriticalThe Critical rating is driven by security-sensitive behavior, public CLI behavior, failure-handling behavior and 1 additional signal(s).
Posted by x-engineering-agent (Reviewer) |
|
🔔 Routing this PR to @Azure/act-observability-squad. |
|
🔔 Routing this PR to @Azure/act-codegen-extensibility-squad. |
Generate SQL DB, live MI database, and dropped MI database STR GET/PUT command bases. Preserve existing custom command surfaces and ARM final-policy polling, and expand offline regression coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Keep generated examples aligned with the existing public wrappers, which supply the default policy name internally, and exercise all four examples through the CLI. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
/azp run |
|
Azure Pipelines: Successfully started running 3 pipeline(s). |
Normalize the SQL Database differential interval in the custom operation adapter without editing generated code. Add exact JSON type and fractional-input regressions. Verified 50 offline tests and eight live interval updates on retained disposable databases with settings restored. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
/azp run |
|
Azure Pipelines: Successfully started running 3 pipeline(s). |

🤖 PR Validation —⚠️ Review suggested
Related command
az sql db str-policy set,show, andwaitaz sql midb short-term-retention-policy setandshow(live and restorable dropped databases)Description
Add short-term backup immutability support using
2026-08-01-preview, with the final async SET result aligned with Azure PowerShell PR 30215.--lock-immutabilityand--yes/-yto both SET commands. Onlytrueprompts, with an irreversible-action warning. Decline/non-interactive confirmation failure sends no update and emits no result;--yesbypasses confirmation.--lock-immutabilitywith--deleted-time, including explicitfalse, before HTTP. Never serialize that property for dropped MI databases.immutabilityStatuson SET/show for SQL Database and live/dropped MI databases, retaining flattened CLI output and nestedsystemData.--ids, wait and--no-wait, with a small compatibility adapter for omitted values and completion polling.azure-mgmt-sql==4.0.0b22is unchanged; a whole-module migration to the hybrid models in5.0.0b2is out of scope.Integer differential-backup interval serialization (2026-10-16)
Commit
03307dd24188df4fce24d1223c51430d19eb144ffixes a regression found by live SQL Database validation: AAZ's generated numeric schema serialized--diffbackup-hours 12/24as JSON12.0/24.0, which the service rejected withInvalidResourceIdSegment. An integer-payload REST control was accepted.The SQL Database custom operation now normalizes only a present, integral
diffBackupIntervalInHoursvalue after generated serialization, and explicitly rejects fractional input instead of truncating it. Omitted fields stay omitted; the other properties, MI operations, response schemas, confirmation, and completion polling are unchanged. Generated Python files and the upstream AAZ models were not hand-edited or changed; this compatibility override survives regeneration.Validation for this fix:
Lockedand oneEnabled: 12/24 hours with normal completion and--no-wait, including explicit retention/lock preservation. A transport guard checked the actual outgoing JSON integer type and permitted writes only to these two policies.The earlier broad SQL live scenarios failed before this correction; they are not being relabeled as passes. The post-fix live validation targeted the affected field and its combinations. The MI and isolated SQL locking scenarios had already passed live and offline replay.
Exact additional validation commands (from the CLI worktree root; all exited 0):
The full offline command below was rerun unchanged and now reports 50 passing tests. New hosted CI results must be checked for this commit before merging.
AAZ generation (2026-10-16)
Commit
9451dbe62e486904492d957059b857d154b702a0replaces the handwritten STR request/response schemas with six command bases generated by aaz-dev-tools 4.7.1:sql/aaz/latest/sql/db/str_policy.sql/aaz/latest/sql/midb/short_term_retention_policy.restorable_droppedchild package.The bases remain unregistered: the existing custom handlers preserve public command names, aliases, required MI retention, database-ID handling, conditional irreversible-action confirmation, and rejection of either lock flag value for dropped databases. No new public command group is introduced. The dropped-policy model hides the shared-schema lock argument.
_str_policy.pycontains compatibility adapters, including the integer wire-type correction above, not manually maintained API schemas. Its shared PUT operation override intentionally retains the final policy GET described below: the current REST specifications declarefinal-state-via: location, which must not undo the already-tested PowerShell alignment when regenerating.Generation inputs:
Azure/azure-rest-api-specscommit2e2b290fd446b03bbddcdc31098ff97e7218f0cf, SQL2026-08-01-preview; AAZ base commitf4676b10d373ec9f5c0b63b986fe2a1832682814. Generated files were not hand-edited. An unrelated generator change to the existing MI start-stop-schedule registration was excluded.Upstream models: Azure/aaz#1130.
Final policy GET alignment (2026-10-13)
Commit
93e16b8f2253fbdccb1a8337cc2dca763d6cae83removes the explicitfinal-state-via: locationoverride from the shared STR PUT operation. For theAzure-AsyncOperationflow recorded for SQL Database and MI:Succeeded.This matches the PowerShell recordings, whose final policy GET belongs to the SET operation. It replaces the previous OperationResults GET; it does not add a second final read, synthesize
Locked, retry until a desired immutability status, add fixed delays, or fill omitted request properties.--no-waitstill submits once, without polling or a final GET, and emits no result.200responses without polling headers and Location-only responses retain standard ARM behavior: return their response without an additional policy GET. Unit tests cover these distinct paths; no unconditional readback is claimed.Lockedon SET itself, as well as through independent policy GETs. All five affected scenarios were genuinely re-recorded live on 2026-10-13; response statuses and URLs were not hand-edited to pass playback.Wait-command compatibility
The earlier fix in
3333619a720b9deb77c784134f1e276500dda0c4remains intact:az sql db str-policy wait --policy-name defaultis accepted and--policy-nameremains required, matching the exact PR basecc010d2104b3ab0089c33fb7eac8816c260a11ac. A dedicated getter forwards and URL-escapes the supplied name; show/set and MI commands still use the default policy without a new public option. Regression coverage includes--ids, encoded names and missing-option rejection without HTTP. No compatibility-rule suppression or exception is requested.Remaining service limitations and release checks
Disabledwhile the actual policy GET reportsLocked. Earlier raw SQL DB recordings and 24 paired MI diagnostic reads over approximately 144 seconds demonstrated this backend inconsistency. The CLI now uses the standard final policy GET for the recorded async flow; this does not claim the backend endpoint is fixed.Disabledbefore becomingEnabled. The new-DB locking test retains its 900-second readiness timeout and refuses to lock beforeEnabled; independent post-lock checks retain their 300-second timeout. A previous Oct13 initialization failure is not counted as passing. The refreshed new-DB locking recording passed.InvalidParameterValue; including current retention succeeds. On a locked policy, retention updates with the lock flag omitted can be interpreted as unlocking. Help now documents explicit retention when locking and preserving--lock-immutability truewhen increasing retention. No automatic payload completion was introduced.Testing Guide
Latest local result (2026-10-16): 50 tests passed, zero failures/errors/skips (34 CLI/HTTP unit tests + 11 scenario safety tests + 5 recorded scenarios). Offline execution used this checkout's CLI/core/test SDK source with the installed CLI libraries on Python 3.14.6, an empty Azure configuration, live-target flags removed, socket connections blocked, and only the unrelated release-update connectivity probe disabled.
The new original-resource final-GET regressions failed against the previous Location override and passed after the fix. A combined-suite run also exposed and fixed the unit-test session-cache isolation issue; the complete combined suite then passed in 70.238 seconds.
AAZ migration validation (2026-10-16)
custom.pyhas 27 import-analysis diagnostics in this local SDK environment, all also present in the 31-diagnostic parent baseline; the migration adds zero pylint diagnostics. A clean full-module pylint result is not claimed.--idsand required retention, lack of public registration for dropped-policy bases, nestedsystemDataacross all three generated resource types, and execution of all four generated examples through the public CLI wrappers.Exact local validation commands and results
Run from the CLI worktree root. The installed SDK uses Python 3.14.6; the separate test dependencies are in
E:\CLI\str-env.The broader AAZ repository check was stopped in favor of the focused SQL verification above. Hosted CI for this new commit is not claimed as passed; prior pipeline approvals/results do not validate this regenerated head.
SQL Database scenarios
Source: SQL Database STR tests.
test_sql_db_short_term_retentionDisabled/Enabled.test_sql_db_short_term_retention_immutabilityEnabled; initialize 7/12. Decline/no-TTY cause no HTTP. Accepted confirmation locks at 7 days; SET returnsLockedand GET independently verifies it.--yesincreases to 8 days with SET and GETLocked. Explicit-lock decrease and false unlock returnInvalidImmutableBackupRetentionPeriodandCannotRemoveLockedPitrImmutability. Differential-only no-wait preserves Locked at 8/24.test_sql_db_existing_policy_immutabilityEnabled. Locks with unchanged 7-day retention and preserves 24-hour differential interval; SET and GET reportLocked. Rejects reduction to 6 days and unlocking. Repeated lock with no-wait has no output/prompt; independent GET remains Locked7/24. Policy-only transport guard; no create/delete.All three refreshed SQL DB scenarios passed live on 2026-10-13 (new DBs in West US 2; policy-only in West Central US), and passed offline playback. The two new SQL DBs were retained by request. There are now eight retained SQL test databases across the earlier and current validation runs; they continue to incur charges. The policy-only run modifies only its expressly authorized retained test DB, not an original database.
SQL Managed Instance scenarios
Source: SQL MI STR tests.
test_sql_managed_db_short_retentiontest_sql_managed_db_short_retention_immutabilityEnabled. Ordinary updates/false before locking; decline/no-TTY send no HTTP. Accepted lock at 7 and--yesincrease to 8 both return SETLocked, with independent GET verification. Service rejects reduction/unlocking. After dropping only its own DB, policy remains Locked8; both dropped lock-flag values fail locally, and the service rejects dropped-retention reduction.Both refreshed MI recordings passed live in West Central US on 2026-10-13 and offline playback. Earlier attempts encountered a non-JSON response and an interrupted backup-readiness wait; these are not counted as successful recordings and were superseded by complete passing captures. All MI test databases from these attempts were cleaned up, including failed-attempt databases. The seven original MI database names remain unchanged; locked dropped backups are not purged and remain for their retention period.
Offline regressions, audits and CI
systemData; required wait policy-name compatibility; IDs and help. Polling coverage includes async-only/both headers for all three resource types, InProgress → Succeeded → original policy GET, synchronous200, Location-only, no-wait request counts, Failed/Canceled without final read, final-GET403/404 propagation, and actual CLI SET JSON output. Lock-only serialization coverage does not imply live support.LockedSET results across the three locking scenarios. No STR OperationResults GET remains in these captures. No-wait SETs have no polling/final reads.git diff --check, and editor diagnostics passed. The shared STR operation module passed Pylint at 10.00/10. Full local SQL-module Pylint is not claimed because the installed runtime has compiled-only dependencies and previously reported unrelated findings.3333619a.... Checks for this new head93e16b8f2253fbdccb1a8337cc2dca763d6cae83must run independently; earlier green checks or bot summaries are not validation of this commit.Hosted pipeline gate (prior head): On October 13, requested
/azp runfor the then-current head, but the Azure Pipelines bot reported insufficient commenter privileges. A maintainer must trigger the main/full-test/breaking-change pipelines. The independent azdev-linter/style workflows are running/queued; their results are pending.Replay with
python -m unittest azure.cli.command_modules.sql.tests.latest.test_sql_str_policy azure.cli.command_modules.sql.tests.latest.test_sql_mi_str_policy azure.cli.command_modules.sql.tests.latest.test_sql_db_str_policyin a configured developer checkout.Live recording requirements and safety guards: SQL STR recording guidance.
AZURE_CLI_TEST_SQL_MI_RESOURCE_ID; irreversible locking additionally requiresAZURE_CLI_TEST_SQL_MI_ALLOW_IMMUTABILITY_LOCK=true.AZURE_CLI_TEST_SQL_SERVER_RESOURCE_ID; locking additionally requiresAZURE_CLI_TEST_SQL_DB_ALLOW_IMMUTABILITY_LOCK=true. These runs usedAZURE_CLI_TEST_SQL_DB_KEEP_DATABASES=true.AZURE_CLI_TEST_SQL_DB_RESOURCE_IDplus separate irreversible-lock authorization. RequiresEnabledand retention above one day; always retains the database.History Notes
[SQL]
az sql db str-policy: Add short-term backup immutability locking with confirmation and expose immutability status[SQL]
az sql midb short-term-retention-policy: Add immutability locking for live databases and expose status for live and dropped databasesThis checklist is used to make sure that common guidelines for a pull request are followed.