fix: prevent azd down from deleting pre-existing resource groups#7603
fix: prevent azd down from deleting pre-existing resource groups#7603
Conversation
jongio
requested review from
JeffreyCA,
hemarina,
rajeshkamal5050,
tg-msft,
vhvb1989,
wbreza and
weikanglim
as code owners
📋 Prioritization NoteThanks for the contribution! The linked issue isn't in the current milestone yet. |
There was a problem hiding this comment.
Pull request overview
This PR updates the azd down destroy flow (standard deployments) to avoid deleting pre-existing resource groups that were only referenced by an ARM/Bicep deployment (e.g., via Bicep existing) by introducing a resource-group ownership classification step before deletion.
Changes:
- Adds a multi-tier resource group ownership classifier (Tier 1 deployment ops + Tier 2 tag fallback + Tier 3 prompts + Tier 4 veto hooks) and integrates it into Bicep destroy orchestration.
- Refactors subscription deployment teardown so “delete deployment” becomes “void deployment state”, and introduces an explicit
VoidStateoperation on the deployment abstraction. - Adds tests for the classifier and for the new destroy orchestration behavior, plus an architecture design doc.
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/azd-down-resource-group-safety/architecture.md | Design doc describing the multi-tier RG classification approach and intended behaviors. |
| cli/azd/pkg/infra/scope.go | Adds VoidState to the Deployment interface and implements it for subscription/RG deployments. |
| cli/azd/pkg/infra/scope_test.go | Adds tests covering the new VoidState behavior on deployments. |
| cli/azd/pkg/infra/provisioning/bicep/bicep_provider.go | Refactors Destroy() to classify RGs, delete only selected RGs, then void state and scope purge targets. |
| cli/azd/pkg/infra/provisioning/bicep/bicep_provider_test.go | Updates existing destroy tests and adds coverage for the classify+delete orchestration. |
| cli/azd/pkg/infra/provisioning/bicep/bicep_destroy.go | New destroy helper implementing classify-then-delete, tag lookup, and void-state orchestration. |
| cli/azd/pkg/azapi/standard_deployments.go | Exposes ResourceGroupsFromDeployment and adds VoidSubscriptionDeploymentState; subscription “delete” now voids state. |
| cli/azd/pkg/azapi/standard_deployments_test.go | Updates sorting to slices.Sort and adds compilation/behavior checks for new public methods. |
| cli/azd/pkg/azapi/stack_deployments.go | Implements VoidSubscriptionDeploymentState as a no-op for stacks. |
| cli/azd/pkg/azapi/resource_group_classifier.go | New classifier implementation for owned vs skipped RGs with tiered signals and veto hooks. |
| cli/azd/pkg/azapi/resource_group_classifier_test.go | New unit test suite for classifier tiers, error handling, and prompting behavior. |
| cli/azd/pkg/azapi/deployments.go | Extends DeploymentService interface with VoidSubscriptionDeploymentState. |
| .gitignore | Ignores cli/azd/coverage-* artifacts. |
jongio
force-pushed
the
fix/azd-down-rg-safety
branch
from
92c3856 to
9de3860
Compare
jongio
force-pushed
the
fix/azd-down-rg-safety
branch
from
bd92475 to
e56b87a
Compare
|
Consider an alternative approach to swich azd down to delete by resource instead of by rg. |
wbreza
left a comment
wbreza
left a comment
There was a problem hiding this comment.
Code Review — PR #7603
fix: prevent azd down from deleting pre-existing resource groups by @jongio
Summary
Impressive 4-tier classification pipeline with solid engineering and thorough design documentation. The classifier's fail-safe behavior (errors → vetoes, not deletions) is the right default. Unit test suite is excellent (55 subtests covering all tiers). Six supplementary findings focused on safety edge cases and testing gaps — all prior copilot-bot feedback resolved.
Prior Review Status
| # | Prior Finding | Author | Status |
|---|---|---|---|
| 1-7 | All copilot-bot findings | @copilot-pull-request-reviewer | ✅ Resolved |
New Findings
| Priority | Count |
|---|---|
| High | 2 |
| Medium | 3 |
| Low | 1 |
| Total | 6 |
🔴 High (2 findings)
- Tier 1 operations failure → silent fallthrough → potential unsafe deletion — If
deployment.Operations()fails, all RGs become Tier 1 unknown and fall to Tier 2 (tag check). An external RG with matching azd tags from a previous deployment to the same environment would be classified as "owned" and deleted. The code logs a WARNING but proceeds. For a safety-critical feature, consider: when Tier 1 is unavailable without--force, downgrade ALL RGs to Tier 3 (prompt each) rather than trusting Tier 2 alone. - Missing integration tests for Tier 4 safety vetoes — 55 unit tests prove the classifier works; but
bicep_provider_test.godoesn't test the end-to-end classify → veto → skip flow. For a data-loss prevention feature: add tests for "lock veto prevents deletion", "user declines → state preserved", and "mixed owned/external → only owned deleted".
🟡 Medium (3 findings)
--forcebypasses Tier 1 (zero-cost) classification —--forceskips ALL tiers, including Tier 1 (deployment operations) which has zero extra API cost. The PR acknowledges this as a future enhancement. Running Tier 1 even with--forcewould catch the most obvious cases (Read vs Create) at no cost.- Backward compatibility: old deployments trigger Tier 3 prompts — Deployments from older azd versions lack
azd-provision-param-hashtags. After upgrading,azd downclassifies all RGs as "unknown" → Tier 3 prompts for every RG. In CI without--force, this may fail. Consider: detect old tag schema (env-name only) and fall back to env-name-only Tier 2 match with a logged warning. - Hash comparison is case-sensitive while tag key lookup is case-insensitive —
tagValue()usesstrings.EqualFoldfor key lookup but hash value comparison uses!=. Low practical risk (azd generates consistent formats), but the inconsistency could cause subtle failures with manually set tags.
🟢 Low (1 finding)
- Log Analytics force-delete may execute twice per RG —
forceDeleteLogAnalyticsIfPurge()appears to run both globally and per-RG insidedeleteRGList. Second call fails silently. Code smell indicating unclear cleanup separation.
✅ What Looks Good
- 4-tier classification pipeline — clear escalation with well-documented decision rationale
- Fail-safe by default — API errors (500/429/403) cause vetoes, not deletions
- Excellent unit test suite — 55 subtests covering all tiers, error paths, context cancellation, parallelism
- Clean
Destroy()restructure — stacks vs standard vs empty paths clearly separated - Safe concurrency — Tier 4 semaphore +
wg.Go()+ defer release pattern is correct - Architecture document — thorough design rationale for each tier and decision
- Prior feedback fully addressed — every copilot-bot finding resolved with evidence and test references
Overall Assessment: Request Changes — the Tier 1 fallthrough (#1) is a data-loss risk that should be addressed before merge. The integration test gap (#2) should also be closed for a safety-critical feature of this scope. The remaining findings are hardening suggestions.
Review performed with GitHub Copilot CLI
wbreza
left a comment
wbreza
left a comment
There was a problem hiding this comment.
Code Review — PR #7603
fix: prevent azd down from deleting pre-existing resource groups by @jongio
Summary
Impressive 4-tier classification pipeline with solid engineering and thorough design documentation. The classifier's fail-safe behavior (errors → vetoes, not deletions) is the right default. Unit test suite is excellent (55 subtests covering all tiers). Six supplementary findings focused on safety edge cases and testing gaps — all prior copilot-bot feedback resolved.
Prior Review Status
| # | Prior Finding | Author | Status |
|---|---|---|---|
| 1-7 | All copilot-bot findings | @copilot-pull-request-reviewer | ✅ Resolved |
New Findings
| Priority | Count |
|---|---|
| High | 2 |
| Medium | 3 |
| Low | 1 |
| Total | 6 |
🔴 High (2 findings)
- Tier 1 operations failure → silent fallthrough → potential unsafe deletion — If
deployment.Operations()fails, all RGs become Tier 1 unknown and fall to Tier 2 (tag check). An external RG with matching azd tags from a previous deployment to the same environment would be classified as "owned" and deleted. The code logs a WARNING but proceeds. For a safety-critical feature, consider: when Tier 1 is unavailable without--force, downgrade ALL RGs to Tier 3 (prompt each) rather than trusting Tier 2 alone. - Missing integration tests for Tier 4 safety vetoes — 55 unit tests prove the classifier works; but
bicep_provider_test.godoesn't test the end-to-end classify → veto → skip flow. For a data-loss prevention feature: add tests for "lock veto prevents deletion", "user declines → state preserved", and "mixed owned/external → only owned deleted".
🟡 Medium (3 findings)
--forcebypasses Tier 1 (zero-cost) classification —--forceskips ALL tiers, including Tier 1 (deployment operations) which has zero extra API cost. The PR acknowledges this as a future enhancement. Running Tier 1 even with--forcewould catch the most obvious cases (Read vs Create) at no cost.- Backward compatibility: old deployments trigger Tier 3 prompts — Deployments from older azd versions lack
azd-provision-param-hashtags. After upgrading,azd downclassifies all RGs as "unknown" → Tier 3 prompts for every RG. In CI without--force, this may fail. Consider: detect old tag schema (env-name only) and fall back to env-name-only Tier 2 match with a logged warning. - Hash comparison is case-sensitive while tag key lookup is case-insensitive —
tagValue()usesstrings.EqualFoldfor key lookup but hash value comparison uses!=. Low practical risk (azd generates consistent formats), but the inconsistency could cause subtle failures with manually set tags.
🟢 Low (1 finding)
- Log Analytics force-delete may execute twice per RG —
forceDeleteLogAnalyticsIfPurge()appears to run both globally and per-RG insidedeleteRGList. Second call fails silently. Code smell indicating unclear cleanup separation.
✅ What Looks Good
- 4-tier classification pipeline — clear escalation with well-documented decision rationale
- Fail-safe by default — API errors (500/429/403) cause vetoes, not deletions
- Excellent unit test suite — 55 subtests covering all tiers, error paths, context cancellation, parallelism
- Clean
Destroy()restructure — stacks vs standard vs empty paths clearly separated - Safe concurrency — Tier 4 semaphore +
wg.Go()+ defer release pattern is correct - Architecture document — thorough design rationale for each tier and decision
- Prior feedback fully addressed — every copilot-bot finding resolved with evidence and test references
Overall Assessment: Request Changes — the Tier 1 fallthrough (#1) is a data-loss risk that should be addressed before merge. The integration test gap (#2) should also be closed for a safety-critical feature of this scope. The remaining findings are hardening suggestions.
Review performed with GitHub Copilot CLI
|
@wbreza — thanks for the thorough review. Addressed all 6 findings. Here's the breakdown: ✅ Fixed: #3 —
|
Interesting idea — resource-level deletion with a selection UI would give users fine-grained control. For this PR though, RG-level classification is the right fit because:
That said, resource-level select/unselect could be a great follow-up feature on top of this foundation — the Tier 4 foreign-resource detection already enumerates resources per RG, so extending that to a selection UI would be feasible. Happy to discuss further if you'd like to file a separate issue for it. |
jongio
force-pushed
the
fix/azd-down-rg-safety
branch
from
f55e59a to
8fa6b1f
Compare
|
@jongio — Hey Jon, I did a quick investigation and I think there may be a much simpler alternative to the 4-tier classification pipeline for distinguishing created vs. existing resource groups. Bicep snapshots already solve this. When you run
We already have the snapshot logic wired up in core ( A few scoping notes:
I verified this experimentally — created a subscription-level Bicep with one This approach would be significantly simpler than the multi-tier classification pipeline (no deployment operations API, no tag checks, no interactive prompts for ambiguous RGs) and would give you a deterministic, offline answer from the Bicep compiler itself. Worth exploring as an alternative? |
@vhvb1989 — Really interesting investigation, and I think you're onto something worth exploring as an enhancement. The snapshot approach could work well as a "Tier 0" signal — a fast, offline, deterministic check from the Bicep compiler before we even look at deployment operations. That said, I don't think it can fully replace the current pipeline for a few reasons:
I'd love to explore adding snapshot as a supplementary signal in a follow-up — it could strengthen Tier 1 confidence or even short-circuit the pipeline entirely when source files are available. Want to file an issue so we can track it? |
|
@jongio — Thanks for engaging with the proposal. Let me address the points, grounding them in the current codebase: 1. "Requires Bicep source files at destroy time"
This isn't accurate. The snapshot approach has the exact same requirement as the current code: Bicep source files must be present. No regression. I'd ask that we ground counter-arguments in currently supported, validated scenarios. Citing hypothetical scenarios (deleted infra folder, different machine) without first verifying they work today adds noise to the discussion and can bias toward unnecessary complexity. A quick check of the code confirms they don't work — so they shouldn't be used to justify a more complex approach. 2. "Doesn't cover adopted/redeployed RGs"The If a user writes 3. "Bicep-only — the classification pipeline works for any deployment provider"The pipeline is Bicep-specific:
Building ~4,165 lines of "provider-agnostic" infrastructure (544-line classifier + 1,268-line tests + 471-line orchestrator + 1,882-line architecture doc) for hypothetical future ext-provision providers that don't exist is speculative. If/when such a provider appears, it will likely have its own teardown semantics — just like Terraform does. 4. "Deployment outputs creating RGs aren't in
|
vhvb1989
left a comment
vhvb1989
left a comment
There was a problem hiding this comment.
Requesting changes to prevent accidental merge while we discuss the approach.
The current 4-tier classification pipeline adds significant complexity (~4,165 new lines across classifier, tests, orchestrator, and architecture doc). We are actively discussing a simpler alternative using local Bicep snapshots (predictedResources), which would provide a deterministic, zero-API-call, compile-time answer to the same question — leveraging infrastructure already wired up in local_preflight.go.
See the ongoing discussion in the comments for details.
…, #2916) Implement a 4-tier resource group classification pipeline in azd down to distinguish between resource groups created by azd (safe to delete) and pre-existing resource groups that were merely referenced via Bicep 'existing' keyword (must not be deleted). The 4-tier classification pipeline: - Tier 1: Deployment operations analysis (zero extra API calls) — Create operations mark RGs as owned, Read/EvaluateDeploymentOutput marks them as external. - Tier 2: Dual-tag check (azd-env-name + azd-provision-param-hash) for RGs with no deployment operations. - Tier 3: Interactive prompt for remaining unknowns (skipped in CI/--force). - Tier 4: Safety vetoes (management locks, foreign resources) applied to ALL deletion candidates including user-accepted unknowns. Key changes: - New resource_group_classifier.go with ClassifyResourceGroups function and comprehensive test coverage (33 tests) - New bicep_destroy.go with classifyAndDeleteResourceGroups orchestrator - Restructured BicepProvider.Destroy() to classify before deleting - Added VoidState to Deployment interface (void after delete, not during) - Added ResourceGroupsFromDeployment public helper - Removed unused promptDeletion/generateResourcesToDelete (replaced by per-RG classification prompts) Safety properties: - All API errors treated as vetoes (fail-safe: errors skip deletion) - --force preserves backward compatibility (bypasses classification) - Tier 4 prompts executed sequentially (no concurrent terminal output) - Deployment state voided only after successful classification - Purge targets collected only from owned (deleted) resource groups Fixes #4785 Relates to #2916 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add empty EnvName guard in Tier 4 (critical: prevents bypass) - Context-aware semaphore with select on ctx.Done() - Tier 4 helpers return errors on credential failures (fail-safe) - Lock pager short-circuits on first CanNotDelete/ReadOnly lock - Fix integration test mocks: register ARM client options, credential provider, individual RG GET, and lock endpoint mocks - Add 10 new classifier unit tests covering edge cases Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Export LockLevelCanNotDelete/LockLevelReadOnly constants - Replace magic strings in bicep_destroy.go lock short-circuit - Add Tier2 nil TagReader and Tier3 nil Prompter edge case tests - Total: 50 classifier unit tests Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Convert wg.Add/go func/wg.Done to wg.Go (Go 1.26 go fix) - Remove unused destroyDeployment function and async import - Add armlocks to cspell-azd-dictionary.txt Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
When deployment stacks alpha feature is enabled, use the original deployment.Delete() path which deletes the stack object (cascading to managed resources). The new classification pipeline only applies to standard deployments. This fixes Test_DeploymentStacks CI failures where the recording proxy could not find individual RG DELETE calls — stacks use stack DELETE instead. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add destroyViaDeploymentDelete tests (0% -> 80%) - Add deleteRGList partial failure test (65% -> 85%) - Add operationTargetsRG + tagValue edge case tests (-> 100%) - Add deployment stacks + credential resolution tests - 10 new test cases, all passing Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…tests - Extract collectPurgeItems helper to eliminate 78-line duplication between stacks and classification paths in Destroy() - Extract forceDeleteLogAnalyticsIfPurge helper to DRY Log Analytics cleanup - Fix data race: use atomic.Int32 for callCount in semaphore cancellation test - Add vetoedSet size hint for map pre-allocation - Remove stale tombstone comment about removed functions - Add 2 security tests: Tier4 500 on resource listing, non-azcore network error Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add errUserCancelled sentinel so declining confirmation does not void deployment state or invalidate env keys (Goldeneye finding) - Move deployment-stacks check before len(groupedResources)==0 fast-path so stacks are always deleted even when ARM shows zero resources - Add UserCancelPreservesDeploymentState regression test - Add ZeroResourcesStillDeletesStack regression test Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…cel returns error - forceDeleteLogAnalyticsIfPurge now returns error (restores fatal behavior) - Tier 4 skips untaggable extension resource types (Microsoft.Authorization/*, etc.) - User cancellation returns errUserCancelled instead of nil - Added Type field to ResourceWithTags, 11 new unit tests Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…=tags, fix semaphore race Three issues found in round 2 triple-model code review: 1. CRITICAL: collectPurgeItems was called AFTER RG deletion in both the deployment-stacks path and classification path. Since DeleteResourceGroup polls to completion (PollUntilDone), getKeyVaults/getManagedHSMs/etc. would 404 when querying resources in already-deleted RGs. Fix: split classifyAndDeleteResourceGroups into classifyResourceGroups (classify + confirm, no delete) so the caller can collect purge targets while RGs still exist, then delete, then purge. 2. HIGH: \=tags is not a valid parameter for the ARM Resources.ListByResourceGroup API (valid values: createdTime, changedTime, provisioningState). Tags are already included by default in GenericResourceExpanded. If ARM rejected the invalid expand with 400, the classifier would treat it as a fail-safe veto on all owned RGs. Fix: remove the \ parameter. 3. LOW: Tier 4 semaphore select race — Go's non-deterministic select could choose the semaphore case even when ctx.Done is ready. Fix: add ctx.Err() re-check after semaphore acquisition. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
When deleteRGList partially succeeds (e.g., rg-a deleted but rg-b fails), the soft-deleted resources from rg-a (Key Vaults, Managed HSMs, etc.) need purging to avoid name collisions on reprovisioning. Previously, purgeItems was skipped entirely when deleteErr was non-nil, and on retry those deleted RGs would be classified as 'already deleted' (Tier 2: 404), losing their purge targets permanently. Now purgeItems always runs after deletion. Deletion errors are reported first (primary failure); purge errors for non-deleted RGs are expected and secondary.
Verifies that purgeItems runs even after deleteRGList partially fails: - rg-ok (with kv-ok) deleted successfully, rg-fail returns 409 - Assert kv-ok purge was called despite partial deletion failure - Assert voidDeploymentState skipped on partial failure - Document known limitation in code comment (iteration order edge case) Covers the fix from the previous commit and addresses the test coverage gap identified in CR Round 4.
MQ preflight cspell check flagged this word used in comments. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- --force no longer bypasses all classification. Tier 1 (zero extra API calls) still runs to identify external RGs from deployment operations. External RGs with Read/EvaluateDeploymentOutput operations are protected even with --force. Unknown RGs are treated as owned (backward compat). If operations are unavailable, all RGs are deleted (backward compat). - Added ForceMode to ClassifyOptions with 5 unit tests covering: external protection, unknown-as-owned, nil ops fallback, callback skip verification, and EvaluateDeploymentOutput detection. - Added Tier4LockVetoPreventsDeletion integration test verifying that a CanNotDelete lock vetoes deletion even for Tier 1 owned RGs. - Added MixedOwnedExternalOnlyOwnedDeleted integration test verifying end-to-end: Created=deleted, Read=skipped, unknown=skipped (non-interactive). - Updated ForceBypassesClassification -> ForceProtectsExternalRGs test to verify operations ARE fetched and external RGs ARE protected with --force. - Extended classifyMockCfg with per-RG lock support and per-RG tag mocks. - Updated architecture.md: Decision 4 rewritten (Tier 1 only, not full bypass), gap section updated, risk mitigations updated, added mermaid classification flow diagram. Addresses review findings #2 and #3 from @wbreza. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Pre-lowercase extensionResourceTypePrefixes for O(1) lookup - Add trust-boundary comment at Tier 1 entry - Correct goroutine invariant comment (sends at most once) - Log foreign resource names in Tier 4 veto - Add hash case-sensitivity comment - Improve Interactive field doc comment - Use atomic.Bool/Int32 for test concurrency counters - Remove duplicate 404 test, add non-azcore error test - Modernize map key collection with slices.Collect(maps.Keys) - Improve getResourceGroupTags doc (error-handling asymmetry) - Guard nil env tag pointer in standard_deployments.go - Fix architecture doc evaluation order - Add diagnosticsettings to cspell dictionary - Promote armlocks to direct dependency (go mod tidy) - Apply gofmt to all changed files Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace 20 to.Ptr() calls with new() in bicep_provider_test.go and remove unused azure-sdk-for-go/sdk/azcore/to import per AGENTS.md Go 1.26 guidelines. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jongio
force-pushed
the
fix/azd-down-rg-safety
branch
from
c93aeef to
c4b7a94
Compare
Azure Dev CLI Install InstructionsInstall scriptsMacOS/Linux
bash: pwsh: WindowsPowerShell install MSI install Standalone Binary
MSI
Documentationlearn.microsoft.com documentationtitle: Azure Developer CLI reference
|
wbreza
left a comment
wbreza
left a comment
There was a problem hiding this comment.
Re-Review — Prior Findings Status
All 6 findings from my previous CHANGES_REQUESTED (e56b87aa) are addressed:
| # | Finding | Status |
|---|---|---|
| 1 | [HIGH] Tier 1 ops failure → silent fallthrough | ✅ Normal mode falls to Tier 2/3; force mode documented |
| 2 | [HIGH] Missing integration tests for Tier 4 vetoes | ✅ +1,681 test lines + destroy orchestration coverage |
| 3 | [Medium] --force bypasses Tier 1 | ✅ d1a56ac1 — ForceMode now runs Tier 1 |
| 4 | [Medium] Old deployments trigger Tier 3 prompts | ✅ Acceptable: --force works, interactive prompts, non-interactive skips safely |
| 5 | [Medium] Hash case sensitivity | ✅ Intentional design, documented in comment |
| 6 | [Low] Log Analytics double purge | ✅ Purge refactored into separate paths |
Code quality has improved notably: wg.Go (Go 1.26), errors.AsType, clean semaphore + context cancellation, extension resource type filtering for false-positive prevention, and sequential Tier 4 prompts to avoid concurrent terminal output.
Design Discussion
Worth noting @vhvb1989's proposal to use Bicep predictedResources (from bicep snapshot) as an alternative to the 4-tier runtime classification. The snapshot approach would be deterministic (compiler-level existing vs created distinction), zero API calls, and could reuse the existing local_preflight.go infrastructure. The current implementation is well-engineered, but if the snapshot approach covers the same scenarios with significantly less complexity, it's worth exploring before this lands.
Summary
Fixes #4785
Fixes #2916
azd downcurrently deletes ALL resource groups referenced in a deployment, including pre-existing ones referenced via Bicep'sexistingkeyword. This causes data loss when users reference shared resource groups (e.g., for Cosmos DB role assignments, shared databases, storage accounts).Root Cause
resourceGroupsFromDeployment()extracts every RG from ARM'soutputResources, andDeleteSubscriptionDeployment()deletes them all unconditionally. The Bicepexistingkeyword is a compile-time construct — invisible to ARM at teardown time.Solution
Introduces a 4-tier resource group classification pipeline that runs before deletion to distinguish owned vs. external RGs:
Createop = owned;Read/EvaluateDeploymentOutput= externalazd-env-name+azd-provision-param-hashtags match → owned--force/CIOnly RGs classified as "owned" are deleted. External/unknown RGs are skipped with clear messaging.
Behavior by mode
--force): Full 4-tier classification runs. Unknown RGs prompt the user.--force/ CI: Classification is bypassed entirely (preserving existing--forcesemantics). A future enhancement could run the free Tier 1 check even with--force.Changes
New files
cli/azd/pkg/azapi/resource_group_classifier.go— 4-tier classification pipeline (~420 lines)cli/azd/pkg/azapi/resource_group_classifier_test.go— 36 unit tests covering all tiers, edge cases, error-as-veto, ctx cancellationcli/azd/pkg/infra/provisioning/bicep/bicep_destroy.go— Classify-then-delete orchestratordocs/azd-down-resource-group-safety/architecture.md— Design documentModified files
cli/azd/pkg/azapi/deployments.go—VoidSubscriptionDeploymentStateinterface methodcli/azd/pkg/azapi/standard_deployments.go— PublicVoidSubscriptionDeploymentState,ResourceGroupsFromDeploymentcli/azd/pkg/azapi/stack_deployments.go— VoidState no-op stubcli/azd/pkg/infra/provisioning/bicep/bicep_provider.go— RestructuredDestroy()flowcli/azd/pkg/infra/provisioning/bicep/bicep_provider_test.go— 5 integration testscli/azd/pkg/infra/scope.go—VoidStateon Deployment interfacecli/azd/pkg/infra/scope_test.go— VoidState testscli/azd/pkg/azapi/standard_deployments_test.go— Updated testsTesting