Skip to content

fix(recipes): restore Terraform inputs for recipe deletion - #13343

Closed
nicolejms wants to merge 3 commits into
mainfrom
nicolejms-terraform-deletion-properties
Closed

nicolejms wants to merge 3 commits into
mainfrom
nicolejms-terraform-deletion-properties

Conversation

@nicolejms

@nicolejms nicolejms commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Summary

This change saves the resolved Terraform module inputs during deployment. Radius writes the inputs to a reserved, sensitive Terraform output named _radius_recipe_inputs. Before terraform destroy, Radius reads this output and restores the module inputs. Thus, deletion does not use the redacted null values of sensitive properties.

Restore rules:

  • Radius fills only current inputs that are missing or null. Current non-null values stay unchanged.
  • Radius merges nested maps key by key. Radius merges lists item by item only when the two lists have the same length.
  • Radius restores only top-level inputs that the current config contains. If a new module version removes a variable, Radius does not send it. If a new module version adds a variable, the current value is used.

If terraform destroy fails after a restore, Radius runs terraform apply -refresh-only. Terraform removes root outputs during a destroy apply, also when the destroy fails. The refresh-only apply writes the saved output back to state, so a retry can restore the inputs again. If this step fails, Radius logs the error and returns the original destroy error.

Create and update operations do not restore inputs. A missing required value still causes those operations to fail.

The unit-test workflow now installs Terraform and sets RADIUS_REQUIRE_TERRAFORM_TESTS=true. Thus, the real Terraform lifecycle test runs in CI and fails if Terraform is missing.

Reason for change

Radius stores sensitive resource properties redacted as null. During deletion, Terraform validates the recipe config with these null values. When a required module argument comes from a sensitive property, terraform destroy fails and the resource cannot be deleted.

Fixes #13330

Notes:

  • Decision for maintainers: This change keeps the plaintext of sensitive inputs in the Terraform state backend. Base64 encoding stops Terraform from interpolating ${} and %{} text in the saved data. It is not encryption. The Terraform sensitive flag does not encrypt state. Access control on the state backend protects the saved values. This also removes the benefit of Terraform write-only and ephemeral attributes for these inputs. An alternative design keeps the ciphertext on the Radius resource and decrypts it only for delete. Please confirm which design to use.
  • State from older deployments does not contain the saved output. For those resources, deletion behaves as before. Redeploy such resources with valid values before you delete them. The temporary workaround in Fix Azure Terraform secret deletion resource-types-contrib#401 must stay for older state. Live Azure deployment and deletion pass with that workaround (run).

How to test

Prerequisites: Go toolchain and Terraform CLI (make install-terraform). If RADIUS_REQUIRE_TERRAFORM_TESTS is not true and Terraform is not installed, the lifecycle test is skipped.

  1. Run the unit and regression tests:

    go test ./pkg/recipes/terraform/config/... ./pkg/recipes/driver/terraform -count=1
    RADIUS_REQUIRE_TERRAFORM_TESTS=true go test ./pkg/recipes/terraform -run 'TestRecipeInputs|TestInitAndDestroyRestoresRecipeInputs|TestGenerateConfig' -count=1

    Expected result: all tests pass. TestRecipeInputsTerraformLifecycle deploys module v1 with real Terraform, changes to module v2 (one variable removed, one required variable added), and redacts the sensitive input. The first destroy fails on purpose and the test confirms that the saved output stays in state. The retry succeeds.

  2. End-to-end check on a cluster:

    1. Register a Terraform recipe that has a required module variable. Map this variable to a sensitive resource property.
    2. Deploy a resource that uses the recipe. Expected result: the deployment succeeds.
    3. Delete the resource. Expected result: the deletion succeeds. Before this change, the deletion failed with a Terraform config validation error for the required variable.

Validation that I ran (Windows, Terraform 1.16.6):

Command Result
RADIUS_REQUIRE_TERRAFORM_TESTS=true go test ./pkg/recipes/terraform -run 'TestRecipeInputs|TestInitAndDestroyRestoresRecipeInputs|TestGenerateConfig|TestValidateOutputMappings|TestUsesOutputMappings|TestRetryTerraformInit|TestIsTransientTerraformInitError' -count=1 Passed
go test ./pkg/recipes/terraform/config/... ./pkg/recipes/driver/terraform -skip 'Test_Save_ConfigFileReadOnly|Test_Save_InvalidWorkingDir' -count=1 Passed
Mutation check: remove the refresh-only call, then run TestRecipeInputsTerraformLifecycle Failed as expected, then passed after I restored the call
Mutation check: turn off UseNumber, then run TestRecipeInputsPreserveNumericPrecision Failed as expected, then passed after I restored the call
gofmt -l pkg/recipes/terraform and go vet ./pkg/recipes/terraform/... Passed
markdownlint-cli2 docs/architecture/extensibility.md --config ./.github/linters/.markdownlint-cli2.yaml Passed, 0 issues
markdown-table-formatter docs/architecture/extensibility.md --check Passed
make spellcheck Passed, 0 issues
git diff --check Passed
shellcheck -x build/scripts/install-terraform.sh (script used by the new workflow step) Passed
actionlint .github/workflows/unit-tests.yaml 2 issues on lines 186 and 208. These lines existed before this change.

Not run on Windows: tests that create symlinks (they need extra OS privileges), Test_Save_ConfigFileReadOnly and Test_Save_InvalidWorkingDir (they fail on main on Windows because of OS error text), and go test -race (not supported on windows/arm64). CI runs these tests on Linux.

File change summary

File Summary of change
pkg/recipes/terraform/config/inputs.go Adds AddRecipeInputs and RestoreRecipeInputs. They save the module inputs to a reserved sensitive output and restore them. Restore fills only missing or null values and only for inputs in the current config. It keeps numbers exact (UseNumber) and rejects trailing data. It sets the output to the saved value. It does not put secret data in error messages.
pkg/recipes/terraform/config/inputs_test.go Adds unit tests for merge rules, typed values, integers larger than 2^53, trailing data, invalid saved data, a missing module, and an output name collision.
pkg/recipes/terraform/execute.go Adds the saved inputs output during config generation. Removes the internal output from the returned state. Restores the inputs before destroy. Runs a refresh-only apply after a failed destroy. Older state without the output keeps the previous behavior.
pkg/recipes/terraform/execute_test.go Adds regression tests for the restore path, destroy failure, older state, invalid data, output errors, and log redaction. Adds a real Terraform lifecycle test with a module version change and a failed first destroy.
.github/workflows/unit-tests.yaml Installs Terraform and sets RADIUS_REQUIRE_TERRAFORM_TESTS=true, so the lifecycle test runs in CI.
docs/architecture/extensibility.md Explains the saved inputs, the merge rules, the write-back after a failed destroy, the state security limit, and the behavior for older state.

Save resolved Terraform module inputs in a reserved sensitive output
during deployment. Restore them before destroy so that redacted
sensitive properties do not cause config validation failures.

Fixes #13330

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Signed-off-by: nicolejms <nicolej@microsoft.com>
@nicolejms
nicolejms requested review from a team as code owners October 19, 2026 17:31
Copilot AI balanced review requested due to automatic review settings October 19, 2026 17:31
@nicolejms nicolejms added the pr:standard Ongoing maintenance, minor improvements, documentation updates, and routine development work label Oct 19, 2026
@github-actions

Copy link
Copy Markdown

Dependency Review

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

Scanned Files

None

@github-actions 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 AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Saved inputs can be lost after failed destruction, and numeric restoration can change input values.

2 open issues
What changed in this PR

Restores Terraform recipe inputs before deletion to address failures caused by redacted sensitive properties.

Changes:

  • Saves module inputs in a reserved sensitive state output and restores them only for destroy.
  • Adds validation, regression tests, and an optional Terraform lifecycle test.
  • Documents state security and compatibility with older deployments.
File Description
pkg/​recipes/​terraform/​execute.go Integrates input persistence and restoration.
pkg/​recipes/​terraform/​execute_test.go Tests restoration, failures, logging, and lifecycle behavior.
pkg/​recipes/​terraform/​config/​inputs.go Encodes, validates, and restores saved inputs.
pkg/​recipes/​terraform/​config/​inputs_test.go Tests round trips and invalid saved data.
docs/​architecture/​extensibility.md Explains retention, security, and legacy-state behavior.

Review effort: Balanced

Comment thread pkg/recipes/terraform/config/inputs.go
Comment thread pkg/recipes/terraform/config/inputs.go Outdated
…stroy failure

Fill only missing or null current inputs from the saved values, so module version changes do not break deletion. After a failed destroy, run a refresh-only apply to write the saved inputs output back to state for a retry. Install Terraform in the unit-test workflow so the lifecycle test runs in CI.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Signed-off-by: nicolejms <nicolej@microsoft.com>
@nicolejms
nicolejms requested a review from a team as a code owner October 19, 2026 17:47
@github-actions

github-actions Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

Unit Tests

    2 files  ± 0    422 suites  ±0   19m 23s ⏱️ + 4m 2s
7 232 tests +32  7 230 ✅ +32  2 💤 ±0  0 ❌ ±0 
8 415 runs  +32  8 413 ✅ +32  2 💤 ±0  0 ❌ ±0 

Results for commit 8f2c8a2. ± Comparison against base commit 90e68a2.

♻️ This comment has been updated with latest results.

@codecov

codecov Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.17391% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 60.37%. Comparing base (90e68a2) to head (8f2c8a2).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
pkg/recipes/terraform/config/inputs.go 93.58% 5 Missing ⚠️
pkg/recipes/terraform/execute.go 89.18% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #13343      +/-   ##
==========================================
+ Coverage   60.27%   60.37%   +0.09%     
==========================================
  Files         748      749       +1     
  Lines       44255    44366     +111     
==========================================
+ Hits        26676    26784     +108     
- Misses      17579    17582       +3     

☔ 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.

@github-actions github-actions Bot added pr:waiting-for-author The author needs to address human review feedback or human-endorsed inline feedback and removed pr:waiting-for-review A reviewer owns the next action; required approval is pending or unverifiable labels Oct 19, 2026
@nicolejms 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
Decode saved and current inputs with UseNumber so integers above 2^53 are not rounded through float64. Keep rejecting trailing data after the saved JSON value.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Signed-off-by: nicolejms <nicolej@microsoft.com>
@radius-functional-tests

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

Copy link
Copy Markdown

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 8f2c8a2
Unique ID func217c069b64
Image tag test-func217c069b64-39334192698-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-func217c069b64-39334192698-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-func217c069b64-39334192698-1
  • dynamic-rp test image location: ghcr.io/radius-project/dev/dynamic-rp:test-func217c069b64-39334192698-1
  • controller test image location: ghcr.io/radius-project/dev/controller:test-func217c069b64-39334192698-1
  • ucp test image location: ghcr.io/radius-project/dev/ucpd:test-func217c069b64-39334192698-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 ucp-cloud functional tests...
✅ ucp-cloud functional tests succeeded
✅ corerp-cloud functional tests succeeded

@nicolejms nicolejms removed the pr:waiting-for-author The author needs to address human review feedback or human-endorsed inline feedback label Oct 19, 2026
@github-actions github-actions Bot added pr:waiting-for-review A reviewer owns the next action; required approval is pending or unverifiable pr:waiting-for-author The author needs to address human review feedback or human-endorsed inline feedback and removed pr:waiting-for-review A reviewer owns the next action; required approval is pending or unverifiable pr:waiting-for-author The author needs to address human review feedback or human-endorsed inline feedback labels Oct 19, 2026
@lakshmimsft

Copy link
Copy Markdown
Contributor

Let us re-consider this approach.. We should not persist the original sensitive inputs in Terraform state to support deletion. Redacting them on the Radius resource while retaining a recoverable copy in  _radius_recipe_inputs  is ideally not the behavior we want. We need a deletion approach that does not retain those values. The guarded recipe-side placeholder appears appropriate for this secret recipe because destroy does not need the original secret value; create/update must continue rejecting missing values. Please reconsider the runtime design on that basis.

@nicolejms

Copy link
Copy Markdown
Contributor Author

making the fix in the recipe is the correct way to do this. closing this pr

@nicolejms nicolejms closed this Oct 20, 2026
@github-actions github-actions Bot removed the pr:waiting-for-review A reviewer owns the next action; required approval is pending or unverifiable label Oct 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr:important High-impact changes needing special attention during release and in release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Terraform recipe deletion loses sensitive resource properties

3 participants