fix(recipes): delete Key Vault secrets through managed parent vaults by nicolejms · Pull Request #13342 · radius-project/radius

nicolejms · 2026-10-19T17:25:20Z

Summary

Delete Azure Key Vault secrets through their parent vault when the same Bicep recipe deletion batch contains that Radius-managed vault. Do not send unsupported ARM DELETE requests for those secret IDs. Wait for the vault delete to finish and report its errors.

Keep direct deletion unchanged when the vault is retained, unmanaged, or absent from the batch. Restrict the rule to Azure Microsoft.KeyVault/vaults/secrets, not extension resources, other child types, or other planes. No HTTP 405 errors are ignored.

Reason for change

Refs radius-project/resource-types-contrib#400 and radius-project/resource-types-contrib#401.

The declarative Azure secrets recipe tracks both a vault and its secrets. Azure ARM rejects secret DELETE with 405 DeleteNotSupported; Radius retries those requests even while deleting the vault. The vault delete already removes the active secrets, so separate child DELETE requests make app deletion fail without removing more resources.

This fix requires no secret migration, new vault, URI change, deployment script, data-plane credential, or ownership transfer. Azure soft-delete retention remains in effect; nothing is purged. Removing a single tracked secret while keeping its vault is a separate, pre-existing limitation and is not changed here.

How to test

With the repository Go toolchain installed, run go test ./pkg/recipes/driver/bicep -run 'Test_Bicep_Delete|Test_GetGCOutputResources' -count=1. Expect all tests to pass. The new tests check multiple declarative secret IDs, waiting for parent deletion, parent errors, unmanaged parents, exact scope matching, other planes/types, and retained-vault updates that must still report the direct-delete error.

For Azure end-to-end validation, install a control-plane build that contains this commit, deploy the declarative Azure Key Vault secrets recipe, and delete the Radius app. Expect vault deletion to finish without separate ARM secret DELETE retries. Confirm the vault and secrets are not available as active resources. Soft-deleted resources can remain under Azure retention rules. This Azure check was not run locally because no test subscription is available.

Local results:

Command	Result
`go test ./pkg/recipes/driver/bicep -run 'Test_Bicep_Delete	Test_GetGCOutputResources' -count=1`
`go test ./pkg/recipes/driver/bicep -run 'KeyVault' -count=25`	Passed
`go test ./pkg/recipes/driver/bicep -count=1`	Failed only in unchanged `Test_WriteDockerConfig_ReadsDecodedSecretFromTargetCluster`: Windows reports directory mode 0777, but the test expects 0700
`go test ./pkg/recipes/driver/bicep -skip '^Test_WriteDockerConfig_ReadsDecodedSecretFromTargetCluster$' -count=1`	Passed
`go vet ./pkg/recipes/driver/bicep`	Passed
`npm exec --yes --package=markdownlint-cli2@0.23.3 -- markdownlint-cli2 docs\architecture\extensibility.md --config .github\linters\.markdownlint-cli2.yaml`	Passed
`npm exec --yes --package=markdown-table-formatter@1.7.0 -- markdown-table-formatter docs\architecture\extensibility.md --check`	Passed

The documented pnpm commands could not start because local pnpm rejected the pinned package-manager resolution. The checks above use the same pinned tool versions through npm. ShellCheck is not applicable: no shell scripts changed.

File change summary

File	Summary of change
`pkg/recipes/driver/bicep/bicep.go`	Use managed parent-vault deletion for secret IDs in the same batch; preserve the batch wait and error path.
`pkg/recipes/driver/bicep/keyvault_test.go`	Cover declarative inventory deletion, parent completion and errors, ownership and scope exclusions, and retained-vault behavior.
`docs/architecture/extensibility.md`	Document the narrow deletion rule, soft-delete retention, and unchanged retained-vault behavior.

Wait for parent vault deletion instead of sending unsupported ARM secret deletes when both resources are in the same deletion batch. Preserve direct deletion and errors when the vault is retained or unmanaged. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: nicolejms <nicolej@microsoft.com>

github-actions · 2026-10-19T17:25:34Z

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

Copilot

🟢 Approval recommended

The narrow change preserves deletion waiting and error propagation, with regression coverage and no unresolved findings.

No open issues 🎉

What changed in this PR

Fixes Bicep recipe deletion so Azure Key Vault secrets are deleted through their Radius-managed parent vault when both appear in the same deletion batch.

Changes:

Skips separate secret DELETE requests while preserving vault completion and error handling.
Adds regression tests for ownership, scope, parent failures, and retained vaults.
Documents the deletion rule and unchanged soft-delete retention.

File	Description
`pkg/recipes/driver/bicep/keyvault_test.go`	Tests parent-vault deletion and excluded cases.
`pkg/recipes/driver/bicep/bicep.go`	Adds narrowly scoped parent-vault matching.
`docs/architecture/extensibility.md`	Documents deletion behavior and limitations.

_{Review effort: Balanced}

github-actions · 2026-10-19T17:46:20Z

Unit Tests

2 files ± 0 422 suites ±0 17m 45s ⏱️ + 2m 24s
7 223 tests +23 7 221 ✅ +23 2 💤 ±0 0 ❌ ±0
8 406 runs +23 8 404 ✅ +23 2 💤 ±0 0 ❌ ±0

Results for commit ac1e6ac. ± Comparison against base commit 90e68a2.

♻️ This comment has been updated with latest results.

codecov · 2026-10-19T17:47:04Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 60.29%. Comparing base (90e68a2) to head (ac1e6ac).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #13342      +/-   ##
==========================================
+ Coverage   60.27%   60.29%   +0.01%     
==========================================
  Files         748      748              
  Lines       44255    44272      +17     
==========================================
+ Hits        26676    26692      +16     
- Misses      17579    17580       +1

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

nithyatsu

LGTM, this fixes the Azure Bicep app-delete hang seen in resource-types-contrib (radius-project/resource-types-contrib#396). There, RADIUS.SECURITY/SECRETS|DELETE retried DELETE .../vaults/kv-app-sec-*/secrets/{apikey,username,password} and got 405 DeleteNotSupported every 60 seconds until the 30-minute job timed out (run 39051743295, dynamic-rp.log). The skip applies only to Azure Key Vault secrets whose Radius-managed parent vault is in the same batch, and vault errors are still reported. CI is green.

Notes:

This doesn't address the Azure Terraform secrets delete failure (#13330), so the resource-types-contrib workaround for that stays.
I'd still like to see the end-to-end Azure check from the PR description run against an image that includes this change.
Three advisory comments are inline; none of them block.

Compare parent vaults using the same ARM-form path as Azure resource deletion. Keep non-Azure planes and resource scopes distinct, and clarify that the child secret DELETE is skipped. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: nicolejms <nicolej@microsoft.com>

radius-functional-tests · 2026-10-19T19:14:45Z

Radius functional test overview

🔍 Go to test action run

Click here to see the test run details

Name	Value
Repository	radius-project/radius
Commit ref	`ac1e6ac`
Unique ID	funcf875970638
Image tag	test-funcf875970638-39309040814-1

Dapr: 1.14.4
Azure KeyVault CSI driver: 1.4.2
Azure Workload identity webhook: 1.3.0
Bicep recipe location ghcr.io/radius-project/dev/test/testrecipes/test-bicep-recipes/<name>:test-funcf875970638-39309040814-1
Terraform recipe location http://tf-module-server.radius-test-tf-module-server.svc.cluster.local/<name>.zip (in cluster)
applications-rp test image location: ghcr.io/radius-project/dev/applications-rp:test-funcf875970638-39309040814-1
dynamic-rp test image location: ghcr.io/radius-project/dev/dynamic-rp:test-funcf875970638-39309040814-1
controller test image location: ghcr.io/radius-project/dev/controller:test-funcf875970638-39309040814-1
ucp test image location: ghcr.io/radius-project/dev/ucpd:test-funcf875970638-39309040814-1
deployment-engine test image location: ghcr.io/radius-project/deployment-engine:latest

Test Status

⌛ Building Radius and pushing container images for functional tests...
✅ Container images build succeeded
⌛ Publishing Bicep Recipes for functional tests...
✅ Recipe publishing succeeded
⌛ Starting corerp-cloud functional tests...
⌛ Starting ucp-cloud functional tests...
✅ ucp-cloud functional tests succeeded
✅ corerp-cloud functional tests succeeded

## Summary  Fix the Azure Terraform secret recipe so destroy can run when Radius redacts secret values, with focused regression coverage in CI. - Supply an empty value to keep the destroy configuration valid. Destroy does not write that value. - Reject missing/null data and values on create/update through a lifecycle precondition, preserving valid deployment behavior. - Grant Purge/Recover permissions needed by the Azure provider's secret lifecycle. - Run all eleven input tests through `make test-azure-secret-inputs`, locally and in CI. The target uses a temporary recipe copy so provider downloads and lock files cannot enter the source recipe package. No leftover-resource cleanup, Azure workflow overrides, or scheduled monitoring is included. The Bicep recipe is unchanged. ## Reason for change  Refs radius-project#400  The provider requires `value` or `value_wo` even when Terraform evaluates a destroy configuration. Radius supplies null for sensitive values, causing destroy to fail before deletion. Keep this workaround for older Terraform state while radius-project/radius#13330 fixes saved-input handling in the runtime. The separate Bicep HTTP 405 problem needs radius-project/radius#13342, not a recipe rewrite. Neither a secret migration nor a vault URI change is introduced here. The focused tests must run automatically: a green deployment test alone does not cover invalid create/update inputs. The resource-type workflow now installs Terraform 1.16.6 using SHA-pinned `hashicorp/setup-terraform` v3.1.2 and calls the make target without Azure credentials. The README uses that same target rather than initializing Terraform in the packaged source folder. ## How to test  | Command or check | Result | | --- | --- | | `make test-azure-secret-inputs` | Passed locally and in [current-head CI](https://github.com/radius-project/resource-types-contrib/actions/runs/39321579100/job/118296687907): 11 passed, 0 failed | | Source recipe hashes and generated-file checks before/after the make target | Passed: source unchanged; no `.terraform/` or `.terraform.lock.hcl` left in the recipe folder | | Deliberate Terraform init failure | Passed: the target failed and removed its temporary copy | | `make -n test-azure-secret-inputs \| tr -d '\r' \| shellcheck --shell=bash -` | Passed locally; carriage-return normalization is for Windows make output | | `go run github.com/rhysd/actionlint/cmd/actionlint@v1.7.7 -shellcheck= -pyflakes= .github/workflows/validate-resource-types.yaml` | Passed locally | | `git diff --check` | Passed locally | | `RECIPE_PLATFORM_FILTER=azure make test ENVIRONMENT="$AZURE_ENVIRONMENT_NAME" RECIPE_TYPE=terraform` | Passed live at bb3c7b8: [Azure Terraform job](https://github.com/radius-project/resource-types-contrib/actions/runs/39311104096/job/118261214269), including secret deploy and application delete | Current head is `f9b66e04d3fc2ba96fda50dae387a8a42f5004b1`. Production recipe code and its eleven test cases are unchanged from the passing live Azure revision; this follow-up restores CI execution and corrects the README command. Native mock-provider tests require Terraform 1.7 or later. The separate Bicep runtime problem is not claimed fixed by this PR. ## File change summary  | File | Summary of change | | ---- | ----------------- | | `Security/secrets/recipes/azure/terraform/main.tf` | Allow redacted destroy inputs, guard create/update values, and grant secret lifecycle permissions. | | `Security/secrets/recipes/azure/terraform/tests/inputs.tftest.hcl` | Add eleven regression tests for input guards and preserved valid values. | | `.github/build/test.mk` | Add the temporary-copy test target, with failure propagation and cleanup. | | `.github/workflows/validate-resource-types.yaml` | Install pinned Terraform and run the focused tests on PRs. | | `Security/secrets/README.md` | Explain the workaround and document the safe make target. | --------- Signed-off-by: nicolejms <nicolej@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

nicolejms requested review from a team as code owners October 19, 2026 17:25

Copilot AI balanced review requested due to automatic review settings October 19, 2026 17:25

nicolejms added the pr:standard Ongoing maintenance, minor improvements, documentation updates, and routine development work label Oct 19, 2026

github-actions Bot assigned nicolejms Oct 19, 2026

github-actions Bot added the pr:waiting-for-review A reviewer owns the next action; required approval is pending or unverifiable label Oct 19, 2026

Copilot started reviewing on behalf of nicolejms October 19, 2026 17:26

View session

Copilot AI reviewed Oct 19, 2026

View reviewed changes

nicolejms mentioned this pull request Oct 19, 2026

Fix Azure Terraform secret deletion radius-project/resource-types-contrib#401

Merged

nithyatsu previously approved these changes Oct 19, 2026

View reviewed changes

Comment thread pkg/recipes/driver/bicep/bicep.go

Comment thread pkg/recipes/driver/bicep/bicep.go Outdated

Comment thread pkg/recipes/driver/bicep/bicep.go Outdated

nicolejms dismissed nithyatsu’s stale review via ac1e6ac October 19, 2026 19:12

nicolejms added pr:important High-impact changes needing special attention during release and in release notes and removed pr:standard Ongoing maintenance, minor improvements, documentation updates, and routine development work labels Oct 19, 2026

nicolejms enabled auto-merge October 19, 2026 21:43

brooke-hamilton approved these changes Oct 19, 2026

View reviewed changes

nicolejms added this pull request to the merge queue Oct 19, 2026

github-actions Bot added pr:review-approved Required reviews are approved; checks may still be pending and removed pr:waiting-for-review A reviewer owns the next action; required approval is pending or unverifiable labels Oct 19, 2026

Merged via the queue into main with commit dbe0605 Oct 19, 2026
110 of 111 checks passed

nicolejms deleted the nicolejms-key-vault-recipe-migration branch October 19, 2026 23:49

github-actions Bot removed the pr:review-approved Required reviews are approved; checks may still be pending label Oct 19, 2026

nicolejms mentioned this pull request Oct 20, 2026

Azure recipe validation fails: secrets Terraform destroy error and containers delete hang radius-project/resource-types-contrib#400

Closed

Navigation Menu

Repository navigation

fix(recipes): delete Key Vault secrets through managed parent vaults - #13342

fix(recipes): delete Key Vault secrets through managed parent vaults#13342
nicolejms merged 2 commits into
mainfrom
nicolejms-key-vault-recipe-migration

nicolejms commented Oct 19, 2026

Uh oh!

github-actions Bot commented Oct 19, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

github-actions Bot commented Oct 19, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Oct 19, 2026 •

edited

Loading

Uh oh!

nithyatsu left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

radius-functional-tests Bot commented Oct 19, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

nicolejms commented Oct 19, 2026

Summary

Reason for change

How to test

File change summary

Uh oh!

github-actions Bot commented Oct 19, 2026

Dependency Review

Scanned Files

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

github-actions Bot commented Oct 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Unit Tests

Uh oh!

codecov Bot commented Oct 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

nithyatsu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

radius-functional-tests Bot commented Oct 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Radius functional test overview

Test Status

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

github-actions Bot commented Oct 19, 2026 •

edited

Loading

codecov Bot commented Oct 19, 2026 •

edited

Loading

radius-functional-tests Bot commented Oct 19, 2026 •

edited

Loading