From c4755e7e0d5540f9b7e2359af636c2d821ab1a24 Mon Sep 17 00:00:00 2001 From: nicolejms Date: Mon, 19 Oct 2026 10:31:20 -0700 Subject: [PATCH 1/3] fix(recipes): restore Terraform inputs for recipe deletion 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 --- docs/architecture/extensibility.md | 4 +- pkg/recipes/terraform/config/inputs.go | 95 ++++++++ pkg/recipes/terraform/config/inputs_test.go | 109 +++++++++ pkg/recipes/terraform/execute.go | 52 ++++- pkg/recipes/terraform/execute_test.go | 231 ++++++++++++++++++++ 5 files changed, 485 insertions(+), 6 deletions(-) create mode 100644 pkg/recipes/terraform/config/inputs.go create mode 100644 pkg/recipes/terraform/config/inputs_test.go diff --git a/docs/architecture/extensibility.md b/docs/architecture/extensibility.md index c748d9746a3..7f4f66ded04 100644 --- a/docs/architecture/extensibility.md +++ b/docs/architecture/extensibility.md @@ -375,7 +375,9 @@ with `x-radius-sensitive`, and encrypts those property values before the accepted resource is stored. During backend processing, `CreateOrUpdateResource` decrypts a recipe-only copy of those values, persists a redacted resource, and passes the decrypted copy to the recipe engine. This -keeps recipe input usable without storing sensitive plaintext. +keeps recipe input usable without storing sensitive plaintext in the Radius resource record. + +Terraform recipes also retain module inputs in the reserved, sensitive `_radius_recipe_inputs` state output. Before destroy, the executor reads this output and restores the prior inputs. Terraform can then validate required arguments even when the Radius resource properties have been redacted. This applies to wrapped recipes and direct modules on all state backends. Create and update do not restore prior inputs, so missing required values still fail validation. The executor removes this internal output from the deployment response; it is not a recipe output or a managed secret. Terraform's `sensitive` flag hides the output in normal CLI logs but does not encrypt state. Protect access to the state backend. Older state files have no saved inputs and keep the previous deletion behavior; redeploy with the required values to save inputs before deletion. ### How The Recipe Runs diff --git a/pkg/recipes/terraform/config/inputs.go b/pkg/recipes/terraform/config/inputs.go new file mode 100644 index 00000000000..b95556a13c1 --- /dev/null +++ b/pkg/recipes/terraform/config/inputs.go @@ -0,0 +1,95 @@ +/* +Copyright 2026 The Radius Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package config + +import ( + "encoding/base64" + "encoding/json" + "errors" + "fmt" +) + +// RecipeInputsOutputName is reserved for inputs retained in Terraform state for deletion. +const RecipeInputsOutputName = "_radius_recipe_inputs" + +// AddRecipeInputs retains module inputs in a sensitive state output for destroy. +func (cfg *TerraformConfig) AddRecipeInputs(moduleName string) error { + module, ok := cfg.Module[moduleName] + if !ok { + return fmt.Errorf("module %q not found in the initialized terraform config", moduleName) + } + if _, exists := cfg.Output[RecipeInputsOutputName]; exists { + return fmt.Errorf("output %q is reserved for Radius recipe inputs", RecipeInputsOutputName) + } + + inputs := RecipeParams{} + for name, value := range module { + if !isModuleMetadata(name) { + inputs[name] = value + } + } + data, err := json.Marshal(inputs) + if err != nil { + return fmt.Errorf("encoding terraform recipe inputs: %w", err) + } + if cfg.Output == nil { + cfg.Output = map[string]any{} + } + // Base64 prevents Terraform from evaluating interpolation syntax inside the saved JSON. + // It is not encryption; the state backend must protect this sensitive output. + cfg.Output[RecipeInputsOutputName] = map[string]any{ + "value": base64.StdEncoding.EncodeToString(data), + "sensitive": true, + } + return nil +} + +// RestoreRecipeInputs replaces module inputs with those saved during deployment. +// It must only be used for destroy, never for create or update. +func (cfg *TerraformConfig) RestoreRecipeInputs(moduleName, encoded string) error { + module, ok := cfg.Module[moduleName] + if !ok { + return fmt.Errorf("module %q not found in the initialized terraform config", moduleName) + } + data, err := base64.StdEncoding.DecodeString(encoded) + if err != nil { + return errors.New("invalid saved terraform recipe inputs: expected base64") + } + var inputs RecipeParams + if err := json.Unmarshal(data, &inputs); err != nil || inputs == nil { + // Do not include the decoder error: it can contain secret input data. + return errors.New("invalid saved terraform recipe inputs: expected a JSON object") + } + for name := range inputs { + if isModuleMetadata(name) { + return errors.New("invalid saved terraform recipe inputs: module metadata is not allowed") + } + } + for name := range module { + if !isModuleMetadata(name) { + delete(module, name) + } + } + for name, value := range inputs { + module[name] = value + } + return nil +} + +func isModuleMetadata(name string) bool { + return name == moduleSourceKey || name == moduleVersionKey || name == moduleProvidersKey +} diff --git a/pkg/recipes/terraform/config/inputs_test.go b/pkg/recipes/terraform/config/inputs_test.go new file mode 100644 index 00000000000..0194d33f113 --- /dev/null +++ b/pkg/recipes/terraform/config/inputs_test.go @@ -0,0 +1,109 @@ +/* +Copyright 2026 The Radius Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package config + +import ( + "encoding/base64" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestRecipeInputsRoundTrip(t *testing.T) { + t.Parallel() + inputs := RecipeParams{ + "context": map[string]any{ + "resource": map[string]any{ + "properties": map[string]any{ + "data": map[string]any{ + "password": map[string]any{"value": "secret-${literal}-%{literal}"}, + }, + }, + }, + }, + "enabled": true, + "counted": float64(2), + "list": []any{"first", nil}, + "null": nil, + } + cfg := &TerraformConfig{Module: map[string]TFModuleConfig{ + "recipe": {moduleSourceKey: "original", moduleVersionKey: "1.0", moduleProvidersKey: map[string]string{"aws": "aws"}}, + }} + // Include an explicit null to verify that restoration preserves the saved input exactly. + for name, value := range inputs { + cfg.Module["recipe"][name] = value + } + require.NoError(t, cfg.AddRecipeInputs("recipe")) + output := cfg.Output[RecipeInputsOutputName].(map[string]any) + require.Equal(t, true, output["sensitive"]) + encoded := output["value"].(string) + require.NotContains(t, encoded, "secret") + require.NotContains(t, encoded, "${") + require.NotContains(t, encoded, "%{") + + cfg.Module["recipe"] = TFModuleConfig{ + moduleSourceKey: "current", moduleVersionKey: "2.0", + moduleProvidersKey: map[string]string{"aws": "aws.current"}, + "context": nil, + "newInput": "must not survive", + } + require.NoError(t, cfg.RestoreRecipeInputs("recipe", encoded)) + expected := TFModuleConfig{ + moduleSourceKey: "current", moduleVersionKey: "2.0", + moduleProvidersKey: map[string]string{"aws": "aws.current"}, + } + for name, value := range inputs { + expected[name] = value + } + require.Equal(t, expected, cfg.Module["recipe"]) +} + +func TestRecipeInputsRejectInvalidState(t *testing.T) { + t.Parallel() + tests := []struct { + name string + encoded string + }{ + {name: "not base64", encoded: "invalid secret"}, + {name: "not JSON", encoded: base64.StdEncoding.EncodeToString([]byte("secret"))}, + {name: "null", encoded: base64.StdEncoding.EncodeToString([]byte("null"))}, + {name: "array", encoded: base64.StdEncoding.EncodeToString([]byte("[]"))}, + {name: "source override", encoded: base64.StdEncoding.EncodeToString([]byte(`{"source":"secret"}`))}, + {name: "version override", encoded: base64.StdEncoding.EncodeToString([]byte(`{"version":"secret"}`))}, + {name: "provider override", encoded: base64.StdEncoding.EncodeToString([]byte(`{"providers":"secret"}`))}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + cfg := &TerraformConfig{Module: map[string]TFModuleConfig{"recipe": {"input": "unchanged"}}} + err := cfg.RestoreRecipeInputs("recipe", tt.encoded) + require.ErrorContains(t, err, "invalid saved terraform recipe inputs") + require.NotContains(t, err.Error(), "secret") + require.Equal(t, TFModuleConfig{"input": "unchanged"}, cfg.Module["recipe"]) + }) + } +} + +func TestRecipeInputsRequireModuleAndReservedOutput(t *testing.T) { + t.Parallel() + cfg := &TerraformConfig{Module: map[string]TFModuleConfig{"recipe": {}}} + require.ErrorContains(t, cfg.AddRecipeInputs("missing"), `module "missing" not found`) + require.ErrorContains(t, cfg.RestoreRecipeInputs("missing", ""), `module "missing" not found`) + cfg.Output = map[string]any{RecipeInputsOutputName: map[string]any{"value": "user output"}} + require.ErrorContains(t, cfg.AddRecipeInputs("recipe"), "reserved") + require.Equal(t, map[string]any{"value": "user output"}, cfg.Output[RecipeInputsOutputName]) +} diff --git a/pkg/recipes/terraform/execute.go b/pkg/recipes/terraform/execute.go index fbf01ca2218..63565805636 100644 --- a/pkg/recipes/terraform/execute.go +++ b/pkg/recipes/terraform/execute.go @@ -18,6 +18,7 @@ package terraform import ( "context" + "encoding/json" "errors" "fmt" "io" @@ -101,6 +102,8 @@ type executor struct { // backendResult carries the extra lifecycle operations each backend needs after Terraform runs. type backendResult struct { + config *config.TerraformConfig + // kubernetesSecretName is set only for the Kubernetes backend, whose state secret Radius manages. kubernetesSecretName string @@ -191,7 +194,7 @@ func (e *executor) Delete(ctx context.Context, options Options) error { stateLockTimeout := getStateLockTimeout(options.StateLockTimeout) if backend.kubernetesSecretName == "" { // Native cloud backends handle missing state, locking and permissions. - if err := initAndDestroy(ctx, tf, stateLockTimeout); err != nil { + if err := initAndDestroy(ctx, tf, stateLockTimeout, backend.config, options.EnvRecipe.Name); err != nil { return err } @@ -223,7 +226,7 @@ func (e *executor) Delete(ctx context.Context, options Options) error { } // Run TF Destroy in the working directory to delete the resources deployed by the recipe - err = initAndDestroy(ctx, tf, stateLockTimeout) + err = initAndDestroy(ctx, tf, stateLockTimeout, backend.config, options.EnvRecipe.Name) if err != nil { return err } @@ -391,7 +394,7 @@ func (e *executor) generateConfig(ctx context.Context, tf *tfexec.Terraform, opt return nil, err } - result := &backendResult{} + result := &backendResult{config: tfConfig} if backendDetails, ok := backendConfig[backends.BackendKubernetes]; ok { backendMap := backendDetails.(map[string]any) if secret, ok := backendMap["secret_suffix"]; ok { @@ -478,6 +481,10 @@ func (e *executor) generateConfig(ctx context.Context, tf *tfexec.Terraform, opt // Add more configurations here. + if err := tfConfig.AddRecipeInputs(options.EnvRecipe.Name); err != nil { + return nil, err + } + // Ensure that we need to save the configuration after adding providers and recipecontext. if err := tfConfig.Save(ctx, workingDir); err != nil { return nil, err @@ -582,11 +589,19 @@ func initAndApply(ctx context.Context, tf *tfexec.Terraform, stateLockTimeout st tf.SetStdout(io.Discard) defer tf.SetStdout(&tfLogWrapper{logger: logger}) - return tf.Show(ctx) + state, err := tf.Show(ctx) + if err != nil { + return nil, err + } + // Internal inputs must not reach recipe outputs or managed secret materialization. + if state != nil && state.Values != nil { + delete(state.Values.Outputs, config.RecipeInputsOutputName) + } + return state, nil } // initAndDestroy runs Terraform init and destroy in the provided working directory. -func initAndDestroy(ctx context.Context, tf *tfexec.Terraform, stateLockTimeout string) error { +func initAndDestroy(ctx context.Context, tf *tfexec.Terraform, stateLockTimeout string, cfg *config.TerraformConfig, moduleName string) error { logger := ucplog.FromContextOrDiscard(ctx) // Initialize Terraform @@ -600,6 +615,10 @@ func initAndDestroy(ctx context.Context, tf *tfexec.Terraform, stateLockTimeout } metrics.DefaultRecipeEngineMetrics.RecordTerraformInitializationDuration(ctx, terraformInitStartTime, nil) + if err := restoreRecipeInputs(ctx, tf, cfg, moduleName); err != nil { + return err + } + // Destroy Terraform configuration with state lock timeout logger.Info("Running Terraform destroy with state lock timeout: " + stateLockTimeout) if err := tf.Destroy(ctx, tfexec.Lock(true), tfexec.LockTimeout(stateLockTimeout)); err != nil { @@ -609,6 +628,29 @@ func initAndDestroy(ctx context.Context, tf *tfexec.Terraform, stateLockTimeout return nil } +func restoreRecipeInputs(ctx context.Context, tf *tfexec.Terraform, cfg *config.TerraformConfig, moduleName string) error { + // Output -json exposes sensitive values. Keep them out of the Radius logs. + tf.SetStdout(io.Discard) + outputs, err := tf.Output(ctx) + tf.SetStdout(&tfLogWrapper{logger: ucplog.FromContextOrDiscard(ctx)}) + if err != nil { + return fmt.Errorf("reading terraform recipe inputs for deletion: %w", err) + } + output, ok := outputs[config.RecipeInputsOutputName] + if !ok { + // Older state has no saved inputs. Preserve its existing deletion behavior. + return nil + } + var encoded string + if err := json.Unmarshal(output.Value, &encoded); err != nil { + return errors.New("invalid saved terraform recipe inputs: expected a string") + } + if err := cfg.RestoreRecipeInputs(moduleName, encoded); err != nil { + return err + } + return cfg.Save(ctx, tf.WorkingDir()) +} + func initializeTerraform(ctx context.Context, tf *tfexec.Terraform) error { backoff := goretry.WithMaxRetries(2, goretry.NewExponential(time.Second)) retryer := retry.NewRetryer(&retry.RetryConfig{BackoffStrategy: backoff}) diff --git a/pkg/recipes/terraform/execute_test.go b/pkg/recipes/terraform/execute_test.go index aa73806ab09..a2121410d3d 100644 --- a/pkg/recipes/terraform/execute_test.go +++ b/pkg/recipes/terraform/execute_test.go @@ -18,14 +18,19 @@ package terraform import ( "context" + "encoding/base64" + "encoding/json" "errors" "fmt" "os" + "os/exec" "path/filepath" reflect "reflect" "testing" "time" + "github.com/go-logr/logr" + "github.com/go-logr/logr/funcr" "github.com/hashicorp/terraform-exec/tfexec" "github.com/radius-project/radius/pkg/components/kubernetesclient/kubernetesclientprovider" dm "github.com/radius-project/radius/pkg/corerp/datamodel" @@ -90,6 +95,11 @@ func runTerraformTestHelper() error { 0644); err != nil { return fmt.Errorf("writing test module outputs: %w", err) } + if os.Getenv("RADIUS_TERRAFORM_CONTEXT_VARIABLE") == "1" { + if err := os.WriteFile(filepath.Join(moduleDir, "variables.tf"), []byte(`variable "context" { type = any }`), 0600); err != nil { + return fmt.Errorf("writing test module context variable: %w", err) + } + } if provider := os.Getenv("RADIUS_TERRAFORM_REQUIRED_PROVIDER"); provider != "" { content := fmt.Sprintf("terraform {\n required_providers {\n %s = {\n source = \"hashicorp/%s\"\n", provider, provider) if alias := os.Getenv("RADIUS_TERRAFORM_PROVIDER_ALIAS"); alias != "" { @@ -106,6 +116,12 @@ func runTerraformTestHelper() error { fmt.Fprint(os.Stdout, `{"terraform_version":"1.15.8","platform":"test","provider_selections":{}}`) case "show": fmt.Fprint(os.Stdout, `{"format_version":"1.0","terraform_version":"1.15.8","values":{"outputs":{"endpoint":{"value":"declared","type":"string","sensitive":false}}}}`) + case "output": + output := os.Getenv("RADIUS_TERRAFORM_TEST_OUTPUT") + if output == "" { + output = "{}" + } + fmt.Fprint(os.Stdout, output) } } return nil @@ -243,6 +259,172 @@ func TestDeleteSkipsOutputMappingValidation(t *testing.T) { require.Contains(t, string(commands), "destroy\n") } +func TestInitAndDestroyRestoresRecipeInputs(t *testing.T) { + inputs := map[string]any{ + "context": map[string]any{ + "resource": map[string]any{ + "properties": map[string]any{ + "data": map[string]any{ + "password": map[string]any{"value": "previous-secret"}, + }, + }, + }, + }, + } + saved := &config.TerraformConfig{Module: map[string]config.TFModuleConfig{"recipe": config.TFModuleConfig(inputs)}} + require.NoError(t, saved.AddRecipeInputs("recipe")) + encoded := saved.Output[config.RecipeInputsOutputName].(map[string]any)["value"] + + for _, tt := range []struct { + name string + outputs map[string]any + failOutput bool + wantError string + restored bool + }{ + {name: "saved inputs", outputs: map[string]any{ + config.RecipeInputsOutputName: map[string]any{"value": encoded, "type": "string", "sensitive": true}, + }, restored: true}, + {name: "legacy state", outputs: map[string]any{}}, + {name: "invalid output type", outputs: map[string]any{ + config.RecipeInputsOutputName: map[string]any{"value": 42, "type": "number"}, + }, wantError: "expected a string"}, + {name: "invalid saved inputs", outputs: map[string]any{ + config.RecipeInputsOutputName: map[string]any{"value": base64.StdEncoding.EncodeToString([]byte("secret")), "type": "string"}, + }, wantError: "expected a JSON object"}, + {name: "output failure", outputs: map[string]any{}, failOutput: true, wantError: "reading terraform recipe inputs for deletion"}, + } { + t.Run(tt.name, func(t *testing.T) { + workingDir := t.TempDir() + cfg := &config.TerraformConfig{Module: map[string]config.TFModuleConfig{ + "recipe": { + "source": "test/module", + "context": map[string]any{ + "resource": map[string]any{ + "properties": map[string]any{ + "data": map[string]any{"password": map[string]any{"value": nil}}, + }, + }, + }, + }, + }} + require.NoError(t, cfg.AddRecipeInputs("recipe")) + require.NoError(t, cfg.Save(t.Context(), workingDir)) + snapshotsPath := filepath.Join(t.TempDir(), "executions.jsonl") + outputs, err := json.Marshal(tt.outputs) + require.NoError(t, err) + tf, err := tfexec.NewTerraform(workingDir, os.Args[0]) + require.NoError(t, err) + env := map[string]string{ + terraformGetTestHelper: "1", + terraformBackendSnapshots: snapshotsPath, + "RADIUS_TERRAFORM_TEST_OUTPUT": string(outputs), + } + if tt.failOutput { + env["RADIUS_TERRAFORM_FAIL_COMMAND"] = "output" + } + require.NoError(t, tf.SetEnv(env)) + + var logs string + logger := funcr.New(func(_, message string) { logs += message }, funcr.Options{}) + ctx := logr.NewContext(t.Context(), logger) + tf.SetStdout(&tfLogWrapper{logger: logger}) + err = initAndDestroy(ctx, tf, "37s", cfg, "recipe") + require.NotContains(t, logs, encoded.(string)) + require.NotContains(t, logs, "previous-secret") + if tt.wantError != "" { + require.ErrorContains(t, err, tt.wantError) + } else { + require.NoError(t, err) + } + var destroyed bool + for _, snapshot := range readBackendSnapshots(t, snapshotsPath) { + if snapshot.Command != "destroy" { + continue + } + destroyed = true + var actual config.TerraformConfig + require.NoError(t, json.Unmarshal(snapshot.Config, &actual)) + require.Equal(t, "test/module", actual.Module["recipe"]["source"]) + if tt.restored { + require.Equal(t, inputs["context"], actual.Module["recipe"]["context"]) + } else { + require.NotContains(t, string(snapshot.Config), "previous-secret") + } + require.Contains(t, snapshot.Args, "-lock-timeout=37s") + } + require.Equal(t, tt.wantError == "", destroyed) + }) + } +} + +func TestRecipeInputsTerraformLifecycle(t *testing.T) { + terraformPath, err := exec.LookPath("terraform") + if err != nil { + t.Skip("Terraform is not installed") + } + workingDir := t.TempDir() + moduleDir := filepath.Join(workingDir, "module") + require.NoError(t, os.Mkdir(moduleDir, 0700)) + require.NoError(t, os.WriteFile(filepath.Join(moduleDir, "main.tf"), []byte(` +variable "context" { + type = any + validation { + condition = var.context.resource.properties.data.password.value != null + error_message = "A secret value is required." + } +} +resource "terraform_data" "secret" { + input = var.context.resource.properties.data.password.value +} +output "result" { + value = {} +} +`), 0600)) + tf, err := tfexec.NewTerraform(workingDir, terraformPath) + require.NoError(t, err) + newConfig := func(value any) *config.TerraformConfig { + t.Helper() + cfg := &config.TerraformConfig{Module: map[string]config.TFModuleConfig{ + "recipe": { + "source": "./module", + "context": map[string]any{ + "resource": map[string]any{ + "properties": map[string]any{ + "data": map[string]any{"password": map[string]any{"value": value}}, + }, + }, + }, + }, + }} + require.NoError(t, cfg.AddRecipeInputs("recipe")) + require.NoError(t, cfg.Save(t.Context(), workingDir)) + return cfg + } + + newConfig(nil) + _, err = initAndApply(t.Context(), tf, "37s") + require.ErrorContains(t, err, "A secret value is required") + + newConfig("previous-secret") + state, err := initAndApply(t.Context(), tf, "37s") + require.NoError(t, err) + require.NotContains(t, state.Values.Outputs, config.RecipeInputsOutputName) + outputs, err := tf.Output(t.Context()) + require.NoError(t, err) + require.True(t, outputs[config.RecipeInputsOutputName].Sensitive) + + redacted := newConfig(nil) + _, err = initAndApply(t.Context(), tf, "37s") + require.ErrorContains(t, err, "A secret value is required") + require.ErrorContains(t, tf.Destroy(t.Context()), "A secret value is required") + + require.NoError(t, initAndDestroy(t.Context(), tf, "37s", redacted, "recipe")) + state, err = tf.Show(t.Context()) + require.NoError(t, err) + require.Nil(t, state.Values) +} + func TestGenerateConfig(t *testing.T) { configTests := []struct { name string @@ -311,6 +493,55 @@ func TestGenerateConfigRejectsInvalidOutputMappingBeforeProviderSetup(t *testing require.EqualError(t, err, `recipe "test-recipe" for resource type "Test.Resources/widgets": invalid outputs mapping: no declared module output matches outputs["host"] -> "missing"; available module outputs: "endpoint"`) } +func TestGenerateConfigRetainsRecipeInputs(t *testing.T) { + for _, wrapped := range []bool{true, false} { + t.Run(fmt.Sprintf("context_variable_%t", wrapped), func(t *testing.T) { + workingDir := t.TempDir() + tf, err := tfexec.NewTerraform(workingDir, os.Args[0]) + require.NoError(t, err) + env := map[string]string{terraformGetTestHelper: "1"} + if wrapped { + env["RADIUS_TERRAFORM_CONTEXT_VARIABLE"] = "1" + } + require.NoError(t, tf.SetEnv(env)) + kubernetesClients := kubernetesclientprovider.KubernetesClientProvider{} + kubernetesClients.SetClientGoClient(fake.NewSimpleClientset()) + e := executor{kubernetesClients: kubernetesClients} + options := Options{ + EnvConfig: &recipes.Configuration{}, + EnvRecipe: &recipes.EnvironmentDefinition{ + Name: "test-recipe", TemplatePath: "test/module/source", + }, + ResourceRecipe: &recipes.ResourceMetadata{ + ResourceID: "/planes/radius/local/resourceGroups/test-rg/providers/Test.Resources/widgets/widget", + EnvironmentID: "/planes/radius/local/resourceGroups/test-rg/providers/Applications.Core/environments/env", + Properties: map[string]any{ + "data": map[string]any{"password": map[string]any{"value": "previous-secret"}}, + }, + }, + } + if !wrapped { + options.EnvRecipe.Parameters = map[string]any{ + "secret_data": "{{context.resource.properties.data}}", + } + } + + backend, err := e.generateConfig(t.Context(), tf, options, backends.CloudBackendAuth{}, requireValidOutputMappings) + require.NoError(t, err) + output := backend.config.Output[config.RecipeInputsOutputName].(map[string]any) + require.Equal(t, true, output["sensitive"]) + require.NoError(t, backend.config.RestoreRecipeInputs("test-recipe", output["value"].(string))) + if wrapped { + recipeContext := backend.config.Module["test-recipe"]["context"].(map[string]any) + resource := recipeContext["resource"].(map[string]any) + require.Equal(t, options.ResourceRecipe.Properties, resource["properties"]) + } else { + require.Equal(t, options.ResourceRecipe.Properties["data"], backend.config.Module["test-recipe"]["secret_data"]) + } + }) + } +} + func TestValidateOutputMappings(t *testing.T) { definition := &recipes.EnvironmentDefinition{ Name: "service-bus", From 666765567c21a86df0ac05652609988c64c713be Mon Sep 17 00:00:00 2001 From: nicolejms Date: Mon, 19 Oct 2026 10:46:57 -0700 Subject: [PATCH 2/3] fix(recipes): merge restored Terraform inputs and preserve them on destroy 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 --- .github/workflows/unit-tests.yaml | 7 ++ docs/architecture/extensibility.md | 2 +- pkg/recipes/terraform/config/inputs.go | 70 +++++++++++++- pkg/recipes/terraform/config/inputs_test.go | 88 +++++++++++++++-- pkg/recipes/terraform/execute.go | 36 +++++-- pkg/recipes/terraform/execute_test.go | 100 +++++++++++++++----- 6 files changed, 256 insertions(+), 47 deletions(-) diff --git a/.github/workflows/unit-tests.yaml b/.github/workflows/unit-tests.yaml index 634093fdf5d..3297b32c80e 100644 --- a/.github/workflows/unit-tests.yaml +++ b/.github/workflows/unit-tests.yaml @@ -122,6 +122,11 @@ jobs: # Version pinned in build/tools.yaml (HELM_VERSION). run: make install-helm + # Terraform recipe lifecycle tests run the real Terraform CLI. + - name: Install Terraform + # Version defaults to .terraform-version; checksums pinned in build/tools.yaml. + run: make install-terraform + - name: Run cluster diagnostics tests # Ubuntu provides jq and GNU timeout; keep these optional for make test. run: make test-cluster-diagnostics @@ -130,6 +135,8 @@ jobs: env: GOTESTSUM_OPTS: --junitfile ./dist/unit_test/results.xml GOTEST_OPTS: -race -coverprofile ./dist/unit_test/ut_coverage.out + # Fail instead of skipping tests that need the Terraform CLI. + RADIUS_REQUIRE_TERRAFORM_TESTS: "true" run: | mkdir -p ./dist/unit_test make test diff --git a/docs/architecture/extensibility.md b/docs/architecture/extensibility.md index 7f4f66ded04..027c670751b 100644 --- a/docs/architecture/extensibility.md +++ b/docs/architecture/extensibility.md @@ -377,7 +377,7 @@ accepted resource is stored. During backend processing, a redacted resource, and passes the decrypted copy to the recipe engine. This keeps recipe input usable without storing sensitive plaintext in the Radius resource record. -Terraform recipes also retain module inputs in the reserved, sensitive `_radius_recipe_inputs` state output. Before destroy, the executor reads this output and restores the prior inputs. Terraform can then validate required arguments even when the Radius resource properties have been redacted. This applies to wrapped recipes and direct modules on all state backends. Create and update do not restore prior inputs, so missing required values still fail validation. The executor removes this internal output from the deployment response; it is not a recipe output or a managed secret. Terraform's `sensitive` flag hides the output in normal CLI logs but does not encrypt state. Protect access to the state backend. Older state files have no saved inputs and keep the previous deletion behavior; redeploy with the required values to save inputs before deletion. +Terraform recipes also retain module inputs in the reserved, sensitive `_radius_recipe_inputs` state output. Before destroy, the executor reads this output and fills only the `null` or missing values of the current module inputs from it. Current values always take precedence, and the executor restores only arguments that the current config contains. Terraform can then validate required arguments even when the Radius resource properties have been redacted, and deletion still works after the recipe's module version adds or removes variables. This applies to wrapped recipes and direct modules on all state backends. Create and update do not restore prior inputs, so missing required values still fail validation. Terraform removes root outputs during a destroy apply, even when the apply fails. If destroy fails after the executor restored inputs, the executor runs a refresh-only apply to write the saved output back to state, so a retry can restore the inputs again. This step is best effort; if it fails, the executor logs the failure and returns the destroy error. The executor removes this internal output from the deployment response; it is not a recipe output or a managed secret. Terraform's `sensitive` flag hides the output in normal CLI logs but does not encrypt state. Protect access to the state backend. Older state files have no saved inputs and keep the previous deletion behavior; redeploy with the required values to save inputs before deletion. ### How The Recipe Runs diff --git a/pkg/recipes/terraform/config/inputs.go b/pkg/recipes/terraform/config/inputs.go index b95556a13c1..f71f67d4e3d 100644 --- a/pkg/recipes/terraform/config/inputs.go +++ b/pkg/recipes/terraform/config/inputs.go @@ -58,7 +58,10 @@ func (cfg *TerraformConfig) AddRecipeInputs(moduleName string) error { return nil } -// RestoreRecipeInputs replaces module inputs with those saved during deployment. +// RestoreRecipeInputs fills missing or null module input values with those saved during deployment. +// Current inputs take precedence, and only arguments in the current config are restored, so the +// config stays valid when the module version changed after deployment. The saved value is also +// kept in the reserved output so that a retry after a failed destroy can restore it again. // It must only be used for destroy, never for create or update. func (cfg *TerraformConfig) RestoreRecipeInputs(moduleName, encoded string) error { module, ok := cfg.Module[moduleName] @@ -79,17 +82,74 @@ func (cfg *TerraformConfig) RestoreRecipeInputs(moduleName, encoded string) erro return errors.New("invalid saved terraform recipe inputs: module metadata is not allowed") } } - for name := range module { - if !isModuleMetadata(name) { - delete(module, name) + restored := map[string]any{} + for name, value := range module { + if isModuleMetadata(name) { + continue } + // Convert typed values such as the recipe context struct to JSON values so they can be merged. + normalized, err := normalizeInput(value) + if err != nil { + return fmt.Errorf("encoding current terraform recipe input %q: %w", name, err) + } + saved, ok := inputs[name] + if ok { + normalized = fillMissingValues(normalized, saved) + } + restored[name] = normalized } - for name, value := range inputs { + for name, value := range restored { module[name] = value } + if cfg.Output == nil { + cfg.Output = map[string]any{} + } + cfg.Output[RecipeInputsOutputName] = map[string]any{ + "value": encoded, + "sensitive": true, + } return nil } +func normalizeInput(value any) (any, error) { + data, err := json.Marshal(value) + if err != nil { + return nil, err + } + var normalized any + if err := json.Unmarshal(data, &normalized); err != nil { + return nil, err + } + return normalized, nil +} + +// fillMissingValues returns current with null values and missing object keys taken from saved. +// Radius redacts sensitive values to null, so non-null current values are never replaced. +func fillMissingValues(current, saved any) any { + if current == nil { + return saved + } + switch currentValue := current.(type) { + case map[string]any: + savedValue, ok := saved.(map[string]any) + if !ok { + return current + } + for key, value := range savedValue { + currentValue[key] = fillMissingValues(currentValue[key], value) + } + case []any: + savedValue, ok := saved.([]any) + if !ok || len(savedValue) != len(currentValue) { + return current + } + for i := range currentValue { + currentValue[i] = fillMissingValues(currentValue[i], savedValue[i]) + } + } + return current +} + func isModuleMetadata(name string) bool { return name == moduleSourceKey || name == moduleVersionKey || name == moduleProvidersKey } diff --git a/pkg/recipes/terraform/config/inputs_test.go b/pkg/recipes/terraform/config/inputs_test.go index 0194d33f113..7a9e1d0688a 100644 --- a/pkg/recipes/terraform/config/inputs_test.go +++ b/pkg/recipes/terraform/config/inputs_test.go @@ -25,7 +25,7 @@ import ( func TestRecipeInputsRoundTrip(t *testing.T) { t.Parallel() - inputs := RecipeParams{ + saved := RecipeParams{ "context": map[string]any{ "resource": map[string]any{ "properties": map[string]any{ @@ -37,14 +37,14 @@ func TestRecipeInputsRoundTrip(t *testing.T) { }, "enabled": true, "counted": float64(2), - "list": []any{"first", nil}, + "list": []any{"first", "second-secret"}, "null": nil, + "removed": "must not be restored", } cfg := &TerraformConfig{Module: map[string]TFModuleConfig{ "recipe": {moduleSourceKey: "original", moduleVersionKey: "1.0", moduleProvidersKey: map[string]string{"aws": "aws"}}, }} - // Include an explicit null to verify that restoration preserves the saved input exactly. - for name, value := range inputs { + for name, value := range saved { cfg.Module["recipe"][name] = value } require.NoError(t, cfg.AddRecipeInputs("recipe")) @@ -55,21 +55,89 @@ func TestRecipeInputsRoundTrip(t *testing.T) { require.NotContains(t, encoded, "${") require.NotContains(t, encoded, "%{") + // Simulate deletion with a newer module version and redacted sensitive values. cfg.Module["recipe"] = TFModuleConfig{ moduleSourceKey: "current", moduleVersionKey: "2.0", moduleProvidersKey: map[string]string{"aws": "aws.current"}, - "context": nil, - "newInput": "must not survive", + "context": map[string]any{ + "resource": map[string]any{ + "properties": map[string]any{ + "data": map[string]any{"password": map[string]any{"value": nil}}, + "current": "kept", + }, + }, + }, + "enabled": nil, + "counted": float64(3), + "list": []any{"first", nil}, + "null": nil, + "newInput": "kept", } + cfg.Output[RecipeInputsOutputName] = map[string]any{"value": "redacted", "sensitive": true} require.NoError(t, cfg.RestoreRecipeInputs("recipe", encoded)) - expected := TFModuleConfig{ + require.Equal(t, TFModuleConfig{ moduleSourceKey: "current", moduleVersionKey: "2.0", moduleProvidersKey: map[string]string{"aws": "aws.current"}, + "context": map[string]any{ + "resource": map[string]any{ + "properties": map[string]any{ + "data": map[string]any{"password": map[string]any{"value": "secret-${literal}-%{literal}"}}, + "current": "kept", + }, + }, + }, + "enabled": true, + "counted": float64(3), + "list": []any{"first", "second-secret"}, + "null": nil, + "newInput": "kept", + }, cfg.Module["recipe"]) + require.Equal(t, map[string]any{"value": encoded, "sensitive": true}, cfg.Output[RecipeInputsOutputName]) +} + +func TestRecipeInputsRestoreTypedValues(t *testing.T) { + t.Parallel() + type secret struct { + Value any `json:"value"` + } + type recipeContext struct { + Secret secret `json:"secret"` + Name string `json:"name"` } - for name, value := range inputs { - expected[name] = value + cfg := &TerraformConfig{Module: map[string]TFModuleConfig{ + "recipe": {"context": recipeContext{Secret: secret{Value: "saved"}, Name: "old"}}, + }} + require.NoError(t, cfg.AddRecipeInputs("recipe")) + encoded := cfg.Output[RecipeInputsOutputName].(map[string]any)["value"].(string) + + cfg.Module["recipe"]["context"] = &recipeContext{Name: "current"} + require.NoError(t, cfg.RestoreRecipeInputs("recipe", encoded)) + require.Equal(t, map[string]any{ + "secret": map[string]any{"value": "saved"}, + "name": "current", + }, cfg.Module["recipe"]["context"]) +} + +func TestFillMissingValues(t *testing.T) { + t.Parallel() + tests := []struct { + name string + current any + saved any + expected any + }{ + {name: "null filled", current: nil, saved: "saved", expected: "saved"}, + {name: "value kept", current: "current", saved: "saved", expected: "current"}, + {name: "missing key filled", current: map[string]any{}, saved: map[string]any{"key": "saved"}, expected: map[string]any{"key": "saved"}}, + {name: "type change kept", current: map[string]any{"key": nil}, saved: "saved", expected: map[string]any{"key": nil}}, + {name: "list length change kept", current: []any{nil}, saved: []any{"one", "two"}, expected: []any{nil}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + require.Equal(t, tt.expected, fillMissingValues(tt.current, tt.saved)) + }) } - require.Equal(t, expected, cfg.Module["recipe"]) } func TestRecipeInputsRejectInvalidState(t *testing.T) { diff --git a/pkg/recipes/terraform/execute.go b/pkg/recipes/terraform/execute.go index 63565805636..ef818cd642e 100644 --- a/pkg/recipes/terraform/execute.go +++ b/pkg/recipes/terraform/execute.go @@ -615,40 +615,62 @@ func initAndDestroy(ctx context.Context, tf *tfexec.Terraform, stateLockTimeout } metrics.DefaultRecipeEngineMetrics.RecordTerraformInitializationDuration(ctx, terraformInitStartTime, nil) - if err := restoreRecipeInputs(ctx, tf, cfg, moduleName); err != nil { + restored, err := restoreRecipeInputs(ctx, tf, cfg, moduleName) + if err != nil { return err } // Destroy Terraform configuration with state lock timeout logger.Info("Running Terraform destroy with state lock timeout: " + stateLockTimeout) if err := tf.Destroy(ctx, tfexec.Lock(true), tfexec.LockTimeout(stateLockTimeout)); err != nil { + if restored { + preserveRecipeInputs(ctx, tf, stateLockTimeout) + } return fmt.Errorf("terraform destroy failure: %w", err) } return nil } -func restoreRecipeInputs(ctx context.Context, tf *tfexec.Terraform, cfg *config.TerraformConfig, moduleName string) error { +// preserveRecipeInputs writes the saved inputs back to state after a failed destroy. +// Terraform removes root outputs during a destroy apply, even when it fails. Without this, +// a retry has no saved inputs and fails validation on the redacted values. +// This is best effort: the destroy error is the actionable failure. +func preserveRecipeInputs(ctx context.Context, tf *tfexec.Terraform, stateLockTimeout string) { + logger := ucplog.FromContextOrDiscard(ctx) + logger.Info("Restoring saved Terraform recipe inputs in state after destroy failure") + err := tf.Apply(ctx, tfexec.RefreshOnly(true), tfexec.Lock(true), tfexec.LockTimeout(stateLockTimeout)) + if err != nil { + logger.Error(err, "Failed to restore saved Terraform recipe inputs in state; a retry of the deletion may fail validation") + } +} + +// restoreRecipeInputs fills redacted module inputs from the saved state output. It reports +// whether saved inputs were found. +func restoreRecipeInputs(ctx context.Context, tf *tfexec.Terraform, cfg *config.TerraformConfig, moduleName string) (bool, error) { // Output -json exposes sensitive values. Keep them out of the Radius logs. tf.SetStdout(io.Discard) outputs, err := tf.Output(ctx) tf.SetStdout(&tfLogWrapper{logger: ucplog.FromContextOrDiscard(ctx)}) if err != nil { - return fmt.Errorf("reading terraform recipe inputs for deletion: %w", err) + return false, fmt.Errorf("reading terraform recipe inputs for deletion: %w", err) } output, ok := outputs[config.RecipeInputsOutputName] if !ok { // Older state has no saved inputs. Preserve its existing deletion behavior. - return nil + return false, nil } var encoded string if err := json.Unmarshal(output.Value, &encoded); err != nil { - return errors.New("invalid saved terraform recipe inputs: expected a string") + return false, errors.New("invalid saved terraform recipe inputs: expected a string") } if err := cfg.RestoreRecipeInputs(moduleName, encoded); err != nil { - return err + return false, err + } + if err := cfg.Save(ctx, tf.WorkingDir()); err != nil { + return false, err } - return cfg.Save(ctx, tf.WorkingDir()) + return true, nil } func initializeTerraform(ctx context.Context, tf *tfexec.Terraform) error { diff --git a/pkg/recipes/terraform/execute_test.go b/pkg/recipes/terraform/execute_test.go index a2121410d3d..fbc944ecec2 100644 --- a/pkg/recipes/terraform/execute_test.go +++ b/pkg/recipes/terraform/execute_test.go @@ -26,6 +26,8 @@ import ( "os/exec" "path/filepath" reflect "reflect" + goruntime "runtime" + "strconv" "testing" "time" @@ -275,24 +277,30 @@ func TestInitAndDestroyRestoresRecipeInputs(t *testing.T) { require.NoError(t, saved.AddRecipeInputs("recipe")) encoded := saved.Output[config.RecipeInputsOutputName].(map[string]any)["value"] + savedOutputs := map[string]any{ + config.RecipeInputsOutputName: map[string]any{"value": encoded, "type": "string", "sensitive": true}, + } + for _, tt := range []struct { - name string - outputs map[string]any - failOutput bool - wantError string - restored bool + name string + outputs map[string]any + failCommand string + wantError string + restored bool + noDestroy bool + wantRefresh bool }{ - {name: "saved inputs", outputs: map[string]any{ - config.RecipeInputsOutputName: map[string]any{"value": encoded, "type": "string", "sensitive": true}, - }, restored: true}, + {name: "saved inputs", outputs: savedOutputs, restored: true}, {name: "legacy state", outputs: map[string]any{}}, {name: "invalid output type", outputs: map[string]any{ config.RecipeInputsOutputName: map[string]any{"value": 42, "type": "number"}, - }, wantError: "expected a string"}, + }, wantError: "expected a string", noDestroy: true}, {name: "invalid saved inputs", outputs: map[string]any{ config.RecipeInputsOutputName: map[string]any{"value": base64.StdEncoding.EncodeToString([]byte("secret")), "type": "string"}, - }, wantError: "expected a JSON object"}, - {name: "output failure", outputs: map[string]any{}, failOutput: true, wantError: "reading terraform recipe inputs for deletion"}, + }, wantError: "expected a JSON object", noDestroy: true}, + {name: "output failure", outputs: map[string]any{}, failCommand: "output", wantError: "reading terraform recipe inputs for deletion", noDestroy: true}, + {name: "destroy failure preserves saved inputs", outputs: savedOutputs, failCommand: "destroy", wantError: "terraform destroy failure", restored: true, wantRefresh: true}, + {name: "destroy failure with legacy state", outputs: map[string]any{}, failCommand: "destroy", wantError: "terraform destroy failure"}, } { t.Run(tt.name, func(t *testing.T) { workingDir := t.TempDir() @@ -320,8 +328,8 @@ func TestInitAndDestroyRestoresRecipeInputs(t *testing.T) { terraformBackendSnapshots: snapshotsPath, "RADIUS_TERRAFORM_TEST_OUTPUT": string(outputs), } - if tt.failOutput { - env["RADIUS_TERRAFORM_FAIL_COMMAND"] = "output" + if tt.failCommand != "" { + env["RADIUS_TERRAFORM_FAIL_COMMAND"] = tt.failCommand } require.NoError(t, tf.SetEnv(env)) @@ -337,8 +345,18 @@ func TestInitAndDestroyRestoresRecipeInputs(t *testing.T) { } else { require.NoError(t, err) } - var destroyed bool + var destroyed, refreshed bool for _, snapshot := range readBackendSnapshots(t, snapshotsPath) { + if snapshot.Command == "apply" { + require.Contains(t, snapshot.Args, "-refresh-only") + require.Contains(t, snapshot.Args, "-lock-timeout=37s") + var actual config.TerraformConfig + require.NoError(t, json.Unmarshal(snapshot.Config, &actual)) + output := actual.Output[config.RecipeInputsOutputName].(map[string]any) + require.Equal(t, encoded, output["value"]) + refreshed = true + continue + } if snapshot.Command != "destroy" { continue } @@ -353,7 +371,8 @@ func TestInitAndDestroyRestoresRecipeInputs(t *testing.T) { } require.Contains(t, snapshot.Args, "-lock-timeout=37s") } - require.Equal(t, tt.wantError == "", destroyed) + require.Equal(t, !tt.noDestroy, destroyed) + require.Equal(t, tt.wantRefresh, refreshed) }) } } @@ -361,12 +380,26 @@ func TestInitAndDestroyRestoresRecipeInputs(t *testing.T) { func TestRecipeInputsTerraformLifecycle(t *testing.T) { terraformPath, err := exec.LookPath("terraform") if err != nil { + if os.Getenv("RADIUS_REQUIRE_TERRAFORM_TESTS") == "true" { + t.Fatal("Terraform is required but not installed") + } t.Skip("Terraform is not installed") } workingDir := t.TempDir() moduleDir := filepath.Join(workingDir, "module") require.NoError(t, os.Mkdir(moduleDir, 0700)) - require.NoError(t, os.WriteFile(filepath.Join(moduleDir, "main.tf"), []byte(` + + // The destroy provisioner fails once, so the test can retry after a partial destroy failure. + marker := filepath.Join(t.TempDir(), "destroy-attempted") + interpreter := `["/bin/sh", "-c"]` + failOnce := fmt.Sprintf(`if [ -f %[1]q ]; then exit 0; fi; touch %[1]q; exit 1`, marker) + if goruntime.GOOS == "windows" { + interpreter = `["powershell", "-NoProfile", "-NonInteractive", "-Command"]` + failOnce = fmt.Sprintf(`if (Test-Path -LiteralPath '%[1]s') { exit 0 }; New-Item -ItemType File -Path '%[1]s' | Out-Null; exit 1`, marker) + } + writeModule := func(inputName string) { + t.Helper() + require.NoError(t, os.WriteFile(filepath.Join(moduleDir, "main.tf"), fmt.Appendf(nil, ` variable "context" { type = any validation { @@ -374,16 +407,25 @@ variable "context" { error_message = "A secret value is required." } } +variable %q { + type = string +} resource "terraform_data" "secret" { input = var.context.resource.properties.data.password.value + provisioner "local-exec" { + when = destroy + interpreter = %s + command = %s + } } output "result" { value = {} } -`), 0600)) +`, inputName, interpreter, strconv.Quote(failOnce)), 0600)) + } tf, err := tfexec.NewTerraform(workingDir, terraformPath) require.NoError(t, err) - newConfig := func(value any) *config.TerraformConfig { + newConfig := func(value any, inputName string) *config.TerraformConfig { t.Helper() cfg := &config.TerraformConfig{Module: map[string]config.TFModuleConfig{ "recipe": { @@ -395,6 +437,7 @@ output "result" { }, }, }, + inputName: inputName + "-value", }, }} require.NoError(t, cfg.AddRecipeInputs("recipe")) @@ -402,11 +445,12 @@ output "result" { return cfg } - newConfig(nil) + writeModule("removed_input") + newConfig(nil, "removed_input") _, err = initAndApply(t.Context(), tf, "37s") require.ErrorContains(t, err, "A secret value is required") - newConfig("previous-secret") + newConfig("previous-secret", "removed_input") state, err := initAndApply(t.Context(), tf, "37s") require.NoError(t, err) require.NotContains(t, state.Values.Outputs, config.RecipeInputsOutputName) @@ -414,12 +458,20 @@ output "result" { require.NoError(t, err) require.True(t, outputs[config.RecipeInputsOutputName].Sensitive) - redacted := newConfig(nil) - _, err = initAndApply(t.Context(), tf, "37s") - require.ErrorContains(t, err, "A secret value is required") + // A new module version removes one input and adds a required input. Deletion must use the + // current arguments and only fill the redacted secret from the saved inputs. + writeModule("added_input") + newConfig(nil, "added_input") require.ErrorContains(t, tf.Destroy(t.Context()), "A secret value is required") - require.NoError(t, initAndDestroy(t.Context(), tf, "37s", redacted, "recipe")) + err = initAndDestroy(t.Context(), tf, "37s", newConfig(nil, "added_input"), "recipe") + require.ErrorContains(t, err, "terraform destroy failure") + require.FileExists(t, marker) + outputs, err = tf.Output(t.Context()) + require.NoError(t, err) + require.Contains(t, outputs, config.RecipeInputsOutputName, "a failed destroy must keep the saved inputs for a retry") + + require.NoError(t, initAndDestroy(t.Context(), tf, "37s", newConfig(nil, "added_input"), "recipe")) state, err = tf.Show(t.Context()) require.NoError(t, err) require.Nil(t, state.Values) From 8f2c8a2615ce0965198b4dbf21ac0bfe0a6a3694 Mon Sep 17 00:00:00 2001 From: nicolejms Date: Mon, 19 Oct 2026 15:19:57 -0700 Subject: [PATCH 3/3] fix(recipes): preserve numeric precision when restoring Terraform inputs 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 --- pkg/recipes/terraform/config/inputs.go | 20 +++++++++-- pkg/recipes/terraform/config/inputs_test.go | 40 ++++++++++++++++++++- 2 files changed, 57 insertions(+), 3 deletions(-) diff --git a/pkg/recipes/terraform/config/inputs.go b/pkg/recipes/terraform/config/inputs.go index f71f67d4e3d..bd5f4b8b042 100644 --- a/pkg/recipes/terraform/config/inputs.go +++ b/pkg/recipes/terraform/config/inputs.go @@ -17,10 +17,12 @@ limitations under the License. package config import ( + "bytes" "encoding/base64" "encoding/json" "errors" "fmt" + "io" ) // RecipeInputsOutputName is reserved for inputs retained in Terraform state for deletion. @@ -73,7 +75,7 @@ func (cfg *TerraformConfig) RestoreRecipeInputs(moduleName, encoded string) erro return errors.New("invalid saved terraform recipe inputs: expected base64") } var inputs RecipeParams - if err := json.Unmarshal(data, &inputs); err != nil || inputs == nil { + if err := decodeJSON(data, &inputs); err != nil || inputs == nil { // Do not include the decoder error: it can contain secret input data. return errors.New("invalid saved terraform recipe inputs: expected a JSON object") } @@ -117,12 +119,26 @@ func normalizeInput(value any) (any, error) { return nil, err } var normalized any - if err := json.Unmarshal(data, &normalized); err != nil { + if err := decodeJSON(data, &normalized); err != nil { return nil, err } return normalized, nil } +// decodeJSON decodes a single JSON value and keeps numbers as json.Number, so that integers +// larger than 2^53 are not rounded through float64 when the config is written again. +func decodeJSON(data []byte, v any) error { + decoder := json.NewDecoder(bytes.NewReader(data)) + decoder.UseNumber() + if err := decoder.Decode(v); err != nil { + return err + } + if _, err := decoder.Token(); !errors.Is(err, io.EOF) { + return errors.New("unexpected data after JSON value") + } + return nil +} + // fillMissingValues returns current with null values and missing object keys taken from saved. // Radius redacts sensitive values to null, so non-null current values are never replaced. func fillMissingValues(current, saved any) any { diff --git a/pkg/recipes/terraform/config/inputs_test.go b/pkg/recipes/terraform/config/inputs_test.go index 7a9e1d0688a..8b8a38e0f29 100644 --- a/pkg/recipes/terraform/config/inputs_test.go +++ b/pkg/recipes/terraform/config/inputs_test.go @@ -18,6 +18,7 @@ package config import ( "encoding/base64" + "encoding/json" "testing" "github.com/stretchr/testify/require" @@ -87,7 +88,7 @@ func TestRecipeInputsRoundTrip(t *testing.T) { }, }, "enabled": true, - "counted": float64(3), + "counted": json.Number("3"), "list": []any{"first", "second-secret"}, "null": nil, "newInput": "kept", @@ -95,6 +96,41 @@ func TestRecipeInputsRoundTrip(t *testing.T) { require.Equal(t, map[string]any{"value": encoded, "sensitive": true}, cfg.Output[RecipeInputsOutputName]) } +func TestRecipeInputsPreserveNumericPrecision(t *testing.T) { + t.Parallel() + // 2^53 + 1 cannot be represented exactly as float64. + const large = "9007199254740993" + cfg := &TerraformConfig{Module: map[string]TFModuleConfig{ + "recipe": { + moduleSourceKey: "source", + "saved": int64(9007199254740993), + "current": uint64(9007199254740993), + "nested": map[string]any{"value": int64(9007199254740993)}, + }, + }} + require.NoError(t, cfg.AddRecipeInputs("recipe")) + encoded := cfg.Output[RecipeInputsOutputName].(map[string]any)["value"].(string) + data, err := base64.StdEncoding.DecodeString(encoded) + require.NoError(t, err) + require.JSONEq(t, `{"current":`+large+`,"nested":{"value":`+large+`},"saved":`+large+`}`, string(data)) + + cfg.Module["recipe"] = TFModuleConfig{ + moduleSourceKey: "source", + "saved": nil, + "current": uint64(9007199254740993), + "nested": map[string]any{"value": nil}, + } + require.NoError(t, cfg.RestoreRecipeInputs("recipe", encoded)) + + // The config is written with json.Marshal, so check the JSON that Terraform receives. + written, err := json.Marshal(cfg.Module["recipe"]) + require.NoError(t, err) + require.JSONEq(t, `{"source":"source","current":`+large+`,"nested":{"value":`+large+`},"saved":`+large+`}`, string(written)) + require.Contains(t, string(written), `"saved":`+large) + require.Contains(t, string(written), `"current":`+large) + require.Contains(t, string(written), `"value":`+large) +} + func TestRecipeInputsRestoreTypedValues(t *testing.T) { t.Parallel() type secret struct { @@ -150,6 +186,8 @@ func TestRecipeInputsRejectInvalidState(t *testing.T) { {name: "not JSON", encoded: base64.StdEncoding.EncodeToString([]byte("secret"))}, {name: "null", encoded: base64.StdEncoding.EncodeToString([]byte("null"))}, {name: "array", encoded: base64.StdEncoding.EncodeToString([]byte("[]"))}, + {name: "trailing invalid data", encoded: base64.StdEncoding.EncodeToString([]byte(`{"input":"secret"} secret`))}, + {name: "trailing JSON value", encoded: base64.StdEncoding.EncodeToString([]byte(`{"input":"secret"} {}`))}, {name: "source override", encoded: base64.StdEncoding.EncodeToString([]byte(`{"source":"secret"}`))}, {name: "version override", encoded: base64.StdEncoding.EncodeToString([]byte(`{"version":"secret"}`))}, {name: "provider override", encoded: base64.StdEncoding.EncodeToString([]byte(`{"providers":"secret"}`))},