From e50a1ac824a34b43f2feafc2f14b145fffc2810a Mon Sep 17 00:00:00 2001 From: Daniel Schmidt Date: Tue, 17 Sep 2024 10:55:41 +0200 Subject: [PATCH] stacks: fix dependency handling in component forget --- internal/stacks/stackruntime/apply_test.go | 190 ++++++++++++++++++ internal/stacks/stackruntime/helper_test.go | 7 +- .../internal/stackeval/component_instance.go | 3 +- .../internal/stackeval/main_apply.go | 2 +- .../forget_with_dependency.tf | 11 + .../forget_with_dependency.tfstack.hcl | 33 +++ .../forget_with_dependency_to_forget.tf | 11 + ...rget_with_dependency_to_forget.tfstack.hcl | 34 ++++ 8 files changed, 288 insertions(+), 3 deletions(-) create mode 100644 internal/stacks/stackruntime/testdata/mainbundle/test/forget_with_dependency/forget_with_dependency.tf create mode 100644 internal/stacks/stackruntime/testdata/mainbundle/test/forget_with_dependency/forget_with_dependency.tfstack.hcl create mode 100644 internal/stacks/stackruntime/testdata/mainbundle/test/forget_with_dependency_to_forget/forget_with_dependency_to_forget.tf create mode 100644 internal/stacks/stackruntime/testdata/mainbundle/test/forget_with_dependency_to_forget/forget_with_dependency_to_forget.tfstack.hcl diff --git a/internal/stacks/stackruntime/apply_test.go b/internal/stacks/stackruntime/apply_test.go index 4e3cb384cc..3c2f173d8a 100644 --- a/internal/stacks/stackruntime/apply_test.go +++ b/internal/stacks/stackruntime/apply_test.go @@ -273,6 +273,196 @@ func TestApply(t *testing.T) { }, }, }, + "forget with dependency": { + path: "forget_with_dependency", + state: stackstate.NewStateBuilder(). + AddComponentInstance(stackstate.NewComponentInstanceBuilder(mustAbsComponentInstance("component.one")). + AddDependent(mustAbsComponent("component.two")). + AddInputVariable("value", cty.StringVal("bar")). + AddOutputValue("id", cty.StringVal("foo"))). + AddResourceInstance(stackstate.NewResourceInstanceBuilder(). + SetAddr(mustAbsResourceInstanceObject("component.one.testing_resource.resource")). + SetProviderAddr(mustDefaultRootProvider("testing")). + SetResourceInstanceObjectSrc(states.ResourceInstanceObjectSrc{ + AttrsJSON: mustMarshalJSONAttrs(map[string]interface{}{ + "id": "foo", + "value": "bar", + }), + Status: states.ObjectReady, + })). + AddComponentInstance(stackstate.NewComponentInstanceBuilder(mustAbsComponentInstance("component.two")). + AddDependency(mustAbsComponent("component.one")). + AddInputVariable("value", cty.StringVal("foo")). + AddOutputValue("id", cty.StringVal("baz"))). + AddResourceInstance(stackstate.NewResourceInstanceBuilder(). + SetAddr(mustAbsResourceInstanceObject("component.two.testing_resource.resource")). + SetProviderAddr(mustDefaultRootProvider("testing")). + SetResourceInstanceObjectSrc(states.ResourceInstanceObjectSrc{ + AttrsJSON: mustMarshalJSONAttrs(map[string]interface{}{ + "id": "baz", + "value": "foo", + }), + Status: states.ObjectReady, + })). + Build(), + store: stacks_testing_provider.NewResourceStoreBuilder(). + AddResource("foo", cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("foo"), + "value": cty.StringVal("bar"), + })). + AddResource("baz", cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("baz"), + "value": cty.StringVal("foo"), + })). + Build(), + cycles: []TestCycle{ + { + planMode: plans.NormalMode, + wantPlannedDiags: tfdiags.Diagnostics{ + tfdiags.Sourceless(tfdiags.Warning, "Some objects will no longer be managed by Terraform", `If you apply this plan, Terraform will discard its tracking information for the following objects, but it will not delete them: + - testing_resource.resource + +After applying this plan, Terraform will no longer manage these objects. You will need to import them into Terraform to manage them again.`), + }, + wantAppliedChanges: []stackstate.AppliedChange{ + &stackstate.AppliedChangeComponentInstance{ + ComponentAddr: mustAbsComponent("component.one"), + ComponentInstanceAddr: mustAbsComponentInstance("component.one"), + OutputValues: map[addrs.OutputValue]cty.Value{ + addrs.OutputValue{Name: "id"}: cty.StringVal("foo"), + }, + InputVariables: map[addrs.InputVariable]cty.Value{ + addrs.InputVariable{Name: "value"}: cty.StringVal("bar"), + }, + Dependents: collections.NewSet(mustAbsComponent("component.two")), + }, + &stackstate.AppliedChangeResourceInstanceObject{ + ResourceInstanceObjectAddr: mustAbsResourceInstanceObject("component.one.testing_resource.resource"), + NewStateSrc: &states.ResourceInstanceObjectSrc{ + AttrsJSON: mustMarshalJSONAttrs(map[string]interface{}{ + "id": "foo", + "value": "bar", + }), + Status: states.ObjectReady, + AttrSensitivePaths: make([]cty.Path, 0), + }, + ProviderConfigAddr: addrs.AbsProviderConfig{ + Provider: addrs.MustParseProviderSourceString("hashicorp/testing"), + }, + Schema: stacks_testing_provider.TestingResourceSchema, + }, + &stackstate.AppliedChangeComponentInstance{ + ComponentAddr: mustAbsComponent("component.two"), + ComponentInstanceAddr: mustAbsComponentInstance("component.two"), + OutputValues: map[addrs.OutputValue]cty.Value{}, + InputVariables: map[addrs.InputVariable]cty.Value{ + addrs.InputVariable{Name: "value"}: cty.StringVal("foo"), + }, + Dependencies: collections.NewSet(mustAbsComponent("component.one")), + }, + &stackstate.AppliedChangeResourceInstanceObject{ + ResourceInstanceObjectAddr: mustAbsResourceInstanceObject("component.two.testing_resource.resource"), + NewStateSrc: nil, // Resource is forgotten + ProviderConfigAddr: addrs.AbsProviderConfig{ + Provider: addrs.MustParseProviderSourceString("hashicorp/testing"), + }, + }, + }, + }, + }, + }, + "forget with dependency on component to forget": { + path: "forget_with_dependency_to_forget", + state: stackstate.NewStateBuilder(). + AddComponentInstance(stackstate.NewComponentInstanceBuilder(mustAbsComponentInstance("component.one")). + AddDependent(mustAbsComponent("component.two")). + AddInputVariable("value", cty.StringVal("bar")). + AddOutputValue("id", cty.StringVal("foo"))). + AddResourceInstance(stackstate.NewResourceInstanceBuilder(). + SetAddr(mustAbsResourceInstanceObject("component.one.testing_resource.resource")). + SetProviderAddr(mustDefaultRootProvider("testing")). + SetResourceInstanceObjectSrc(states.ResourceInstanceObjectSrc{ + AttrsJSON: mustMarshalJSONAttrs(map[string]interface{}{ + "id": "foo", + "value": "bar", + }), + Status: states.ObjectReady, + })). + AddComponentInstance(stackstate.NewComponentInstanceBuilder(mustAbsComponentInstance("component.two")). + AddDependency(mustAbsComponent("component.one")). + AddInputVariable("value", cty.StringVal("foo")). + AddOutputValue("id", cty.StringVal("baz"))). + AddResourceInstance(stackstate.NewResourceInstanceBuilder(). + SetAddr(mustAbsResourceInstanceObject("component.two.testing_resource.resource")). + SetProviderAddr(mustDefaultRootProvider("testing")). + SetResourceInstanceObjectSrc(states.ResourceInstanceObjectSrc{ + AttrsJSON: mustMarshalJSONAttrs(map[string]interface{}{ + "id": "baz", + "value": "foo", + }), + Status: states.ObjectReady, + })). + Build(), + store: stacks_testing_provider.NewResourceStoreBuilder(). + AddResource("foo", cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("foo"), + "value": cty.StringVal("bar"), + })). + AddResource("baz", cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("baz"), + "value": cty.StringVal("foo"), + })). + Build(), + cycles: []TestCycle{ + { + planMode: plans.NormalMode, + wantPlannedDiags: tfdiags.Diagnostics{ + tfdiags.Sourceless(tfdiags.Warning, "Some objects will no longer be managed by Terraform", `If you apply this plan, Terraform will discard its tracking information for the following objects, but it will not delete them: + - testing_resource.resource + +After applying this plan, Terraform will no longer manage these objects. You will need to import them into Terraform to manage them again.`), + tfdiags.Sourceless(tfdiags.Warning, "Some objects will no longer be managed by Terraform", `If you apply this plan, Terraform will discard its tracking information for the following objects, but it will not delete them: + - testing_resource.resource + +After applying this plan, Terraform will no longer manage these objects. You will need to import them into Terraform to manage them again.`), + }, + wantAppliedChanges: []stackstate.AppliedChange{ + &stackstate.AppliedChangeComponentInstance{ + ComponentAddr: mustAbsComponent("component.one"), + ComponentInstanceAddr: mustAbsComponentInstance("component.one"), + OutputValues: map[addrs.OutputValue]cty.Value{}, // No output being read, we rely on the plan data + InputVariables: map[addrs.InputVariable]cty.Value{ + addrs.InputVariable{Name: "value"}: cty.StringVal("bar"), + }, + Dependents: collections.NewSet(mustAbsComponent("component.two")), + }, + &stackstate.AppliedChangeResourceInstanceObject{ + ResourceInstanceObjectAddr: mustAbsResourceInstanceObject("component.one.testing_resource.resource"), + NewStateSrc: nil, // Resource is forgotten + ProviderConfigAddr: addrs.AbsProviderConfig{ + Provider: addrs.MustParseProviderSourceString("hashicorp/testing"), + }, + }, + &stackstate.AppliedChangeComponentInstance{ + ComponentAddr: mustAbsComponent("component.two"), + ComponentInstanceAddr: mustAbsComponentInstance("component.two"), + OutputValues: map[addrs.OutputValue]cty.Value{}, + InputVariables: map[addrs.InputVariable]cty.Value{ + addrs.InputVariable{Name: "value"}: cty.StringVal("foo"), + }, + Dependencies: collections.NewSet(mustAbsComponent("component.one")), + }, + &stackstate.AppliedChangeResourceInstanceObject{ + ResourceInstanceObjectAddr: mustAbsResourceInstanceObject("component.two.testing_resource.resource"), + NewStateSrc: nil, // Resource is forgotten + ProviderConfigAddr: addrs.AbsProviderConfig{ + Provider: addrs.MustParseProviderSourceString("hashicorp/testing"), + }, + }, + }, + }, + }, + }, } for name, tc := range tcs { diff --git a/internal/stacks/stackruntime/helper_test.go b/internal/stacks/stackruntime/helper_test.go index df4880ad29..595c69cb51 100644 --- a/internal/stacks/stackruntime/helper_test.go +++ b/internal/stacks/stackruntime/helper_test.go @@ -468,7 +468,12 @@ func diagnosticSortFunc(diags tfdiags.Diagnostics) func(i, j int) bool { if !cmp.Equal(id.Description(), jd.Description()) { return sortDescription(id.Description(), jd.Description()) } - return sortRange(id.Source().Subject, jd.Source().Subject) + if id.Source().Subject != nil && jd.Source().Subject != nil { + + return sortRange(id.Source().Subject, jd.Source().Subject) + } + + return false } } diff --git a/internal/stacks/stackruntime/internal/stackeval/component_instance.go b/internal/stacks/stackruntime/internal/stackeval/component_instance.go index 38ad5ef649..5c53bb03b2 100644 --- a/internal/stacks/stackruntime/internal/stackeval/component_instance.go +++ b/internal/stacks/stackruntime/internal/stackeval/component_instance.go @@ -519,7 +519,8 @@ func (c *ComponentInstance) ResultValue(ctx context.Context, phase EvalPhase) ct // Weird, but we'll tolerate it. return cty.DynamicVal } - if ourPlan.PlannedAction == plans.Delete { + + if ourPlan.PlannedAction == plans.Delete || ourPlan.PlannedAction == plans.Forget { // In this case our result was already decided during the // planning phase, because we can't block on anything else // here to make sure we don't create a self-dependency diff --git a/internal/stacks/stackruntime/internal/stackeval/main_apply.go b/internal/stacks/stackruntime/internal/stackeval/main_apply.go index fc23110c11..c1aba53bfb 100644 --- a/internal/stacks/stackruntime/internal/stackeval/main_apply.go +++ b/internal/stacks/stackruntime/internal/stackeval/main_apply.go @@ -182,7 +182,7 @@ func ApplyPlan(ctx context.Context, config *stackconfig.Config, plan *stackplan. var waitForComponents collections.Set[stackaddrs.AbsComponent] var waitForRemoveds collections.Set[stackaddrs.AbsComponent] - if action == plans.Delete { + if action == plans.Delete || action == plans.Forget { // If the effect of this apply will be to destroy this // component instance then we need to wait for all // of our dependents to be destroyed first, because diff --git a/internal/stacks/stackruntime/testdata/mainbundle/test/forget_with_dependency/forget_with_dependency.tf b/internal/stacks/stackruntime/testdata/mainbundle/test/forget_with_dependency/forget_with_dependency.tf new file mode 100644 index 0000000000..8e7a20e79c --- /dev/null +++ b/internal/stacks/stackruntime/testdata/mainbundle/test/forget_with_dependency/forget_with_dependency.tf @@ -0,0 +1,11 @@ +variable "value" { + type = string +} + +resource "testing_resource" "resource" { + value = var.value +} + +output "id" { + value = testing_resource.resource.id +} \ No newline at end of file diff --git a/internal/stacks/stackruntime/testdata/mainbundle/test/forget_with_dependency/forget_with_dependency.tfstack.hcl b/internal/stacks/stackruntime/testdata/mainbundle/test/forget_with_dependency/forget_with_dependency.tfstack.hcl new file mode 100644 index 0000000000..aa90910c02 --- /dev/null +++ b/internal/stacks/stackruntime/testdata/mainbundle/test/forget_with_dependency/forget_with_dependency.tfstack.hcl @@ -0,0 +1,33 @@ +required_providers { + testing = { + source = "hashicorp/testing" + version = "0.1.0" + } +} + +provider "testing" "default" {} + +component "one" { + source = "./" + + providers = { + testing = provider.testing.default + } + + inputs = { + value = "bar" + } +} + +removed { + source = "./" + from = component.two + + providers = { + testing = provider.testing.default + } + + lifecycle { + destroy = false + } +} \ No newline at end of file diff --git a/internal/stacks/stackruntime/testdata/mainbundle/test/forget_with_dependency_to_forget/forget_with_dependency_to_forget.tf b/internal/stacks/stackruntime/testdata/mainbundle/test/forget_with_dependency_to_forget/forget_with_dependency_to_forget.tf new file mode 100644 index 0000000000..8e7a20e79c --- /dev/null +++ b/internal/stacks/stackruntime/testdata/mainbundle/test/forget_with_dependency_to_forget/forget_with_dependency_to_forget.tf @@ -0,0 +1,11 @@ +variable "value" { + type = string +} + +resource "testing_resource" "resource" { + value = var.value +} + +output "id" { + value = testing_resource.resource.id +} \ No newline at end of file diff --git a/internal/stacks/stackruntime/testdata/mainbundle/test/forget_with_dependency_to_forget/forget_with_dependency_to_forget.tfstack.hcl b/internal/stacks/stackruntime/testdata/mainbundle/test/forget_with_dependency_to_forget/forget_with_dependency_to_forget.tfstack.hcl new file mode 100644 index 0000000000..0cbfe5801f --- /dev/null +++ b/internal/stacks/stackruntime/testdata/mainbundle/test/forget_with_dependency_to_forget/forget_with_dependency_to_forget.tfstack.hcl @@ -0,0 +1,34 @@ +required_providers { + testing = { + source = "hashicorp/testing" + version = "0.1.0" + } +} + +provider "testing" "default" {} + +removed { + source = "./" + from = component.one + + providers = { + testing = provider.testing.default + } + + lifecycle { + destroy = false + } +} + +removed { + source = "./" + from = component.two + + providers = { + testing = provider.testing.default + } + + lifecycle { + destroy = false + } +} \ No newline at end of file