From a9b84948ba6a14434d838bf451c8f0bea6eadc3f Mon Sep 17 00:00:00 2001 From: Liam Cervante Date: Mon, 13 Jan 2025 10:17:14 +0100 Subject: [PATCH] stacks: allow unknown variables during apply operations --- .../stacks/stackruntime/apply_destroy_test.go | 133 ++++++++++++++++++ internal/stacks/stackruntime/apply_test.go | 15 +- .../internal/stackeval/component_instance.go | 13 +- .../internal/stackeval/input_variable.go | 4 +- .../test/removed-offline/child/child.tf | 16 +++ .../test/removed-offline/parent/parent.tf | 14 ++ .../removed-offline.tfstack.hcl | 28 ++++ 7 files changed, 205 insertions(+), 18 deletions(-) create mode 100644 internal/stacks/stackruntime/testdata/mainbundle/test/removed-offline/child/child.tf create mode 100644 internal/stacks/stackruntime/testdata/mainbundle/test/removed-offline/parent/parent.tf create mode 100644 internal/stacks/stackruntime/testdata/mainbundle/test/removed-offline/removed-offline.tfstack.hcl diff --git a/internal/stacks/stackruntime/apply_destroy_test.go b/internal/stacks/stackruntime/apply_destroy_test.go index d4a4307c430d..feb29c35faac 100644 --- a/internal/stacks/stackruntime/apply_destroy_test.go +++ b/internal/stacks/stackruntime/apply_destroy_test.go @@ -1025,6 +1025,139 @@ func TestApplyDestroy(t *testing.T) { }, }, }, + "destroy after manual removal": { + path: "removed-offline", + state: stackstate.NewStateBuilder(). + AddComponentInstance(stackstate.NewComponentInstanceBuilder(mustAbsComponentInstance("component.parent")). + AddDependent(mustAbsComponent("component.child")). + AddOutputValue("value", cty.StringVal("hello"))). + AddResourceInstance(stackstate.NewResourceInstanceBuilder(). + SetAddr(mustAbsResourceInstanceObject("component.parent.testing_resource.resource")). + SetProviderAddr(mustDefaultRootProvider("testing")). + SetResourceInstanceObjectSrc(states.ResourceInstanceObjectSrc{ + AttrsJSON: mustMarshalJSONAttrs(map[string]interface{}{ + "id": "parent", + "value": "hello", + }), + Status: states.ObjectReady, + })). + AddComponentInstance(stackstate.NewComponentInstanceBuilder(mustAbsComponentInstance("component.child")). + AddDependency(mustAbsComponent("component.parent")). + AddInputVariable("value", cty.StringVal("hello"))). + AddResourceInstance(stackstate.NewResourceInstanceBuilder(). + SetAddr(mustAbsResourceInstanceObject("component.child.testing_resource.resource")). + SetProviderAddr(mustDefaultRootProvider("testing")). + SetResourceInstanceObjectSrc(states.ResourceInstanceObjectSrc{ + AttrsJSON: mustMarshalJSONAttrs(map[string]interface{}{ + "id": "child", + "value": "hello", + }), + Status: states.ObjectReady, + })). + Build(), + store: stacks_testing_provider.NewResourceStoreBuilder(). + AddResource("child", cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("child"), + "value": cty.StringVal("hello"), + })).Build(), + cycles: []TestCycle{ + { + planMode: plans.DestroyMode, + wantPlannedChanges: []stackplan.PlannedChange{ + &stackplan.PlannedChangeApplyable{ + Applyable: true, + }, + &stackplan.PlannedChangeComponentInstance{ + Addr: mustAbsComponentInstance("component.child"), + Action: plans.Delete, + Mode: plans.DestroyMode, + PlanComplete: true, + PlanApplyable: true, + RequiredComponents: collections.NewSet(mustAbsComponent("component.parent")), + PlannedInputValues: map[string]plans.DynamicValue{ + "value": mustPlanDynamicValueDynamicType(cty.UnknownVal(cty.String)), + }, + PlannedInputValueMarks: map[string][]cty.PathValueMarks{ + "value": nil, + }, + PlannedOutputValues: make(map[string]cty.Value), + PlannedCheckResults: &states.CheckResults{}, + PlanTimestamp: fakePlanTimestamp, + }, + &stackplan.PlannedChangeResourceInstancePlanned{ + ResourceInstanceObjectAddr: mustAbsResourceInstanceObject("component.child.testing_resource.resource"), + ChangeSrc: &plans.ResourceInstanceChangeSrc{ + Addr: mustAbsResourceInstance("testing_resource.resource"), + PrevRunAddr: mustAbsResourceInstance("testing_resource.resource"), + ProviderAddr: mustDefaultRootProvider("testing"), + ChangeSrc: plans.ChangeSrc{ + Action: plans.Delete, + Before: mustPlanDynamicValue(cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("child"), + "value": cty.StringVal("hello"), + })), + After: mustPlanDynamicValue(cty.NullVal(cty.Object(map[string]cty.Type{ + "id": cty.String, + "value": cty.String, + }))), + }, + }, + PriorStateSrc: &states.ResourceInstanceObjectSrc{ + AttrsJSON: mustMarshalJSONAttrs(map[string]interface{}{ + "id": "child", + "value": "hello", + }), + Status: states.ObjectReady, + Dependencies: make([]addrs.ConfigResource, 0), + }, + ProviderConfigAddr: mustDefaultRootProvider("testing"), + Schema: stacks_testing_provider.TestingResourceSchema, + }, + &stackplan.PlannedChangeComponentInstance{ + Addr: mustAbsComponentInstance("component.parent"), + Action: plans.Delete, + Mode: plans.DestroyMode, + PlanComplete: true, + PlanApplyable: false, + PlannedInputValues: make(map[string]plans.DynamicValue), + PlannedOutputValues: map[string]cty.Value{ + "value": cty.DynamicVal, + }, + PlannedCheckResults: &states.CheckResults{}, + PlanTimestamp: fakePlanTimestamp, + }, + &stackplan.PlannedChangeResourceInstancePlanned{ + ResourceInstanceObjectAddr: mustAbsResourceInstanceObject("component.parent.testing_resource.resource"), + ProviderConfigAddr: mustDefaultRootProvider("testing"), + }, + &stackplan.PlannedChangeHeader{ + TerraformVersion: version.SemVer, + }, + &stackplan.PlannedChangePlannedTimestamp{ + PlannedTimestamp: fakePlanTimestamp, + }, + }, + wantAppliedChanges: []stackstate.AppliedChange{ + &stackstate.AppliedChangeComponentInstanceRemoved{ + ComponentAddr: mustAbsComponent("component.child"), + ComponentInstanceAddr: mustAbsComponentInstance("component.child"), + }, + &stackstate.AppliedChangeResourceInstanceObject{ + ResourceInstanceObjectAddr: mustAbsResourceInstanceObject("component.child.testing_resource.resource"), + ProviderConfigAddr: mustDefaultRootProvider("testing"), + }, + &stackstate.AppliedChangeComponentInstanceRemoved{ + ComponentAddr: mustAbsComponent("component.parent"), + ComponentInstanceAddr: mustAbsComponentInstance("component.parent"), + }, + &stackstate.AppliedChangeResourceInstanceObject{ + ResourceInstanceObjectAddr: mustAbsResourceInstanceObject("component.parent.testing_resource.resource"), + ProviderConfigAddr: mustDefaultRootProvider("testing"), + }, + }, + }, + }, + }, "partial destroy recovery": { path: "component-chain", description: "this test simulates a partial destroy recovery", diff --git a/internal/stacks/stackruntime/apply_test.go b/internal/stacks/stackruntime/apply_test.go index 0c223358e9b5..ca6f44e1baed 100644 --- a/internal/stacks/stackruntime/apply_test.go +++ b/internal/stacks/stackruntime/apply_test.go @@ -1463,7 +1463,11 @@ After applying this plan, Terraform will no longer manage these objects. You wil ComponentInstanceAddr: mustAbsComponentInstance("component.main"), Dependencies: collections.NewSet(mustAbsComponent("component.self")), OutputValues: make(map[addrs.OutputValue]cty.Value), - InputVariables: make(map[addrs.InputVariable]cty.Value), + InputVariables: map[addrs.InputVariable]cty.Value{ + mustInputVariable("input"): cty.UnknownVal(cty.Map(cty.Object(map[string]cty.Type{ + "output": cty.String, + }))), + }, }, &stackstate.AppliedChangeInputVariable{ Addr: mustStackInputVariable("inputs"), @@ -2358,7 +2362,10 @@ func TestApplyWithFailedComponent(t *testing.T) { ComponentInstanceAddr: mustAbsComponentInstance("component.self"), Dependencies: collections.NewSet(mustAbsComponent("component.parent")), OutputValues: make(map[addrs.OutputValue]cty.Value), - InputVariables: make(map[addrs.InputVariable]cty.Value), + InputVariables: map[addrs.InputVariable]cty.Value{ + mustInputVariable("id"): cty.NullVal(cty.String), + mustInputVariable("input"): cty.UnknownVal(cty.String), + }, }, } @@ -3110,9 +3117,6 @@ func TestApplyWithChangedInputValues(t *testing.T) { go Apply(ctx, &applyReq, &applyResp) applyChanges, applyDiags := collectApplyOutput(applyChangesCh, diagsCh) - if len(applyDiags) != 1 { - t.Fatalf("expected exactly two diagnostics, got %s", applyDiags.ErrWithWarnings()) - } sort.SliceStable(applyDiags, diagnosticSortFunc(applyDiags)) expectDiagnosticsForTest(t, applyDiags, @@ -3120,6 +3124,7 @@ func TestApplyWithChangedInputValues(t *testing.T) { tfdiags.Error, "Inconsistent value for input variable during apply", "The value for non-ephemeral input variable \"input\" was set to a different value during apply than was set during plan. Only ephemeral input variables can change between the plan and apply phases."), + expectDiagnostic(tfdiags.Error, "Invalid inputs for component", "Invalid input variable definition object: attribute \"input\": string required."), ) wantChanges := []stackstate.AppliedChange{ diff --git a/internal/stacks/stackruntime/internal/stackeval/component_instance.go b/internal/stacks/stackruntime/internal/stackeval/component_instance.go index dd689d91f038..e84c04b40835 100644 --- a/internal/stacks/stackruntime/internal/stackeval/component_instance.go +++ b/internal/stacks/stackruntime/internal/stackeval/component_instance.go @@ -96,18 +96,9 @@ func (c *ComponentInstance) CheckInputVariableValues(ctx context.Context, phase // We actually checked the errors statically already, so we only care about // the value here. val, diags := EvalComponentInputVariables(ctx, varDecls, wantTy, defs, decl, phase, c) - if phase == ApplyPhase { - if !val.IsWhollyKnown() { - // We can't apply a configuration that has unknown values in it. - // This means an error has occured somewhere else, while gathering - // the input variables. We return a nil value here, whatever caused - // the error should have raised an error diagnostic separately. - return cty.NilVal, diags - } - - // Note, that unknown values during the planning phase are totally fine. + if diags.HasErrors() { + return cty.NilVal, diags } - return val, diags } diff --git a/internal/stacks/stackruntime/internal/stackeval/input_variable.go b/internal/stacks/stackruntime/internal/stackeval/input_variable.go index 7c86ecb83974..b19c8f5e127e 100644 --- a/internal/stacks/stackruntime/internal/stackeval/input_variable.go +++ b/internal/stacks/stackruntime/internal/stackeval/input_variable.go @@ -107,7 +107,7 @@ func (v *InputVariable) Value(ctx context.Context, phase EvalPhase) cty.Value { } func (v *InputVariable) CheckValue(ctx context.Context, phase EvalPhase) (cty.Value, tfdiags.Diagnostics) { - return withCtyDynamicValPlaceholder(doOnceWithDiags( + return doOnceWithDiags( ctx, v.value.For(phase), v.main, func(ctx context.Context) (cty.Value, tfdiags.Diagnostics) { var diags tfdiags.Diagnostics @@ -211,7 +211,7 @@ func (v *InputVariable) CheckValue(ctx context.Context, phase EvalPhase) (cty.Va return cfg.markValue(val), diags } }, - )) + ) } // ExprReferenceValue implements Referenceable. diff --git a/internal/stacks/stackruntime/testdata/mainbundle/test/removed-offline/child/child.tf b/internal/stacks/stackruntime/testdata/mainbundle/test/removed-offline/child/child.tf new file mode 100644 index 000000000000..302855cac6a2 --- /dev/null +++ b/internal/stacks/stackruntime/testdata/mainbundle/test/removed-offline/child/child.tf @@ -0,0 +1,16 @@ +terraform { + required_providers { + testing = { + source = "hashicorp/testing" + version = "0.1.0" + } + } +} + +variable "value" { + type = string +} + +resource "testing_resource" "resource" { + value = var.value +} diff --git a/internal/stacks/stackruntime/testdata/mainbundle/test/removed-offline/parent/parent.tf b/internal/stacks/stackruntime/testdata/mainbundle/test/removed-offline/parent/parent.tf new file mode 100644 index 000000000000..30b6659dc5bc --- /dev/null +++ b/internal/stacks/stackruntime/testdata/mainbundle/test/removed-offline/parent/parent.tf @@ -0,0 +1,14 @@ +terraform { + required_providers { + testing = { + source = "hashicorp/testing" + version = "0.1.0" + } + } +} + +resource "testing_resource" "resource" {} + +output "value" { + value = testing_resource.resource.id +} diff --git a/internal/stacks/stackruntime/testdata/mainbundle/test/removed-offline/removed-offline.tfstack.hcl b/internal/stacks/stackruntime/testdata/mainbundle/test/removed-offline/removed-offline.tfstack.hcl new file mode 100644 index 000000000000..4e4713515140 --- /dev/null +++ b/internal/stacks/stackruntime/testdata/mainbundle/test/removed-offline/removed-offline.tfstack.hcl @@ -0,0 +1,28 @@ +required_providers { + testing = { + source = "hashicorp/testing" + version = "0.1.0" + } +} + +provider "testing" "default" {} + +component "parent" { + source = "./parent" + + providers = { + testing = provider.testing.default + } +} + +component "child" { + source = "./child" + + providers = { + testing = provider.testing.default + } + + inputs = { + value = component.parent.value + } +}