From b8e362d24c3a501c6e606c979d6ce711212dca99 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Fri, 27 May 2022 10:50:01 -0400 Subject: [PATCH 1/8] do not connect references _to_ destroyers either Destroy nodes should never participate in references. These edges didn't come up before, because we weren't building a complete graph including all temporary values. --- internal/terraform/transform_reference.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/internal/terraform/transform_reference.go b/internal/terraform/transform_reference.go index bd07161ca02e..7c45955b86f8 100644 --- a/internal/terraform/transform_reference.go +++ b/internal/terraform/transform_reference.go @@ -114,6 +114,7 @@ func (t *ReferenceTransformer) Transform(g *Graph) error { // use their own state. continue } + parents := m.References(v) parentsDbg := make([]string, len(parents)) for i, v := range parents { @@ -124,6 +125,14 @@ func (t *ReferenceTransformer) Transform(g *Graph) error { dag.VertexName(v), parentsDbg) for _, parent := range parents { + // A destroy plan relies solely on the state, so we only need to + // ensure that temporary values are connected to get the evaluation + // order correct. Any references to destroy nodes will cause + // cycles, because they are connected in reverse order. + if _, ok := parent.(GraphNodeDestroyer); ok { + continue + } + if !graphNodesAreResourceInstancesInDifferentInstancesOfSameModule(v, parent) { g.Connect(dag.BasicEdge(v, parent)) } else { From e3a6e1f6e8ae746af9b710a89d15e5794011fa13 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Fri, 27 May 2022 10:54:58 -0400 Subject: [PATCH 2/8] remove unused fields from DestroyEdgeTransformer --- internal/terraform/graph_builder_apply.go | 5 +-- internal/terraform/transform_destroy_edge.go | 11 ++----- .../terraform/transform_destroy_edge_test.go | 31 ++++--------------- 3 files changed, 9 insertions(+), 38 deletions(-) diff --git a/internal/terraform/graph_builder_apply.go b/internal/terraform/graph_builder_apply.go index eea3235cd1ab..619e2e14ff1e 100644 --- a/internal/terraform/graph_builder_apply.go +++ b/internal/terraform/graph_builder_apply.go @@ -136,10 +136,7 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer { &ForcedCBDTransformer{}, // Destruction ordering - &DestroyEdgeTransformer{ - Config: b.Config, - State: b.State, - }, + &DestroyEdgeTransformer{}, &CBDEdgeTransformer{ Config: b.Config, State: b.State, diff --git a/internal/terraform/transform_destroy_edge.go b/internal/terraform/transform_destroy_edge.go index 521acced02e7..213289716289 100644 --- a/internal/terraform/transform_destroy_edge.go +++ b/internal/terraform/transform_destroy_edge.go @@ -4,9 +4,7 @@ import ( "log" "github.com/hashicorp/terraform/internal/addrs" - "github.com/hashicorp/terraform/internal/states" - "github.com/hashicorp/terraform/internal/configs" "github.com/hashicorp/terraform/internal/dag" ) @@ -40,12 +38,7 @@ type GraphNodeCreator interface { // dependent resources will block parent resources from deleting. Concrete // example: VPC with subnets, the VPC can't be deleted while there are // still subnets. -type DestroyEdgeTransformer struct { - // These are needed to properly build the graph of dependencies - // to determine what a destroy node depends on. Any of these can be nil. - Config *configs.Config - State *states.State -} +type DestroyEdgeTransformer struct{} func (t *DestroyEdgeTransformer) Transform(g *Graph) error { // Build a map of what is being destroyed (by address string) to @@ -89,7 +82,7 @@ func (t *DestroyEdgeTransformer) Transform(g *Graph) error { return nil } - // Connect destroy despendencies as stored in the state + // Connect destroy dependencies as stored in the state for _, ds := range destroyers { for _, des := range ds { ri, ok := des.(GraphNodeResourceInstance) diff --git a/internal/terraform/transform_destroy_edge_test.go b/internal/terraform/transform_destroy_edge_test.go index a5176a3cc7d3..b1a0407ccb63 100644 --- a/internal/terraform/transform_destroy_edge_test.go +++ b/internal/terraform/transform_destroy_edge_test.go @@ -37,9 +37,7 @@ func TestDestroyEdgeTransformer_basic(t *testing.T) { t.Fatal(err) } - tf := &DestroyEdgeTransformer{ - Config: testModule(t, "transform-destroy-edge-basic"), - } + tf := &DestroyEdgeTransformer{} if err := tf.Transform(&g); err != nil { t.Fatalf("err: %s", err) } @@ -93,9 +91,7 @@ func TestDestroyEdgeTransformer_multi(t *testing.T) { t.Fatal(err) } - tf := &DestroyEdgeTransformer{ - Config: testModule(t, "transform-destroy-edge-multi"), - } + tf := &DestroyEdgeTransformer{} if err := tf.Transform(&g); err != nil { t.Fatalf("err: %s", err) } @@ -110,9 +106,7 @@ func TestDestroyEdgeTransformer_multi(t *testing.T) { func TestDestroyEdgeTransformer_selfRef(t *testing.T) { g := Graph{Path: addrs.RootModuleInstance} g.Add(testDestroyNode("test_object.A")) - tf := &DestroyEdgeTransformer{ - Config: testModule(t, "transform-destroy-edge-self-ref"), - } + tf := &DestroyEdgeTransformer{} if err := tf.Transform(&g); err != nil { t.Fatalf("err: %s", err) } @@ -153,9 +147,7 @@ func TestDestroyEdgeTransformer_module(t *testing.T) { t.Fatal(err) } - tf := &DestroyEdgeTransformer{ - Config: testModule(t, "transform-destroy-edge-module"), - } + tf := &DestroyEdgeTransformer{} if err := tf.Transform(&g); err != nil { t.Fatalf("err: %s", err) } @@ -214,9 +206,7 @@ func TestDestroyEdgeTransformer_moduleOnly(t *testing.T) { t.Fatal(err) } - tf := &DestroyEdgeTransformer{ - Config: testModule(t, "transform-destroy-edge-module-only"), - } + tf := &DestroyEdgeTransformer{} if err := tf.Transform(&g); err != nil { t.Fatalf("err: %s", err) } @@ -284,16 +274,7 @@ func TestDestroyEdgeTransformer_destroyThenUpdate(t *testing.T) { t.Fatal(err) } - m := testModuleInline(t, map[string]string{ - "main.tf": ` -resource "test_instance" "a" { - test_string = "udpated" -} -`, - }) - tf := &DestroyEdgeTransformer{ - Config: m, - } + tf := &DestroyEdgeTransformer{} if err := tf.Transform(&g); err != nil { t.Fatalf("err: %s", err) } From d6f0d1ea57c64f30c581845bf26f775bf540a6ee Mon Sep 17 00:00:00 2001 From: James Bardin Date: Fri, 27 May 2022 10:55:38 -0400 Subject: [PATCH 3/8] don't add config nodes during destroy plan We want to use the normal plan graph for destroy, so we need to flag off configuration for that process. --- internal/terraform/transform_config.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/internal/terraform/transform_config.go b/internal/terraform/transform_config.go index 6cb42b620002..f404c80d0a31 100644 --- a/internal/terraform/transform_config.go +++ b/internal/terraform/transform_config.go @@ -28,6 +28,9 @@ type ConfigTransformer struct { // Mode will only add resources that match the given mode ModeFilter bool Mode addrs.ResourceMode + + // destroyPlan indicated this is being called from a destroy plan. + destroyPlan bool } func (t *ConfigTransformer) Transform(g *Graph) error { @@ -36,6 +39,11 @@ func (t *ConfigTransformer) Transform(g *Graph) error { return nil } + // Configured resource are not added for a destroy plan + if t.destroyPlan { + return nil + } + // Start the transformation process return t.transform(g, t.Config) } From 6860596b681762a59d72cfb44cb02ffb5b837b25 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Fri, 27 May 2022 10:56:40 -0400 Subject: [PATCH 4/8] don't add orphan nodes during destroy All instances in state are being removed for destroy, so we can skip checking for orphans. Because we want to use the normal plan graph, we need to be able to still call this during destroy, so flag it off. --- internal/terraform/transform_orphan_resource.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/internal/terraform/transform_orphan_resource.go b/internal/terraform/transform_orphan_resource.go index af8302c4925c..e218d8970e70 100644 --- a/internal/terraform/transform_orphan_resource.go +++ b/internal/terraform/transform_orphan_resource.go @@ -26,9 +26,17 @@ type OrphanResourceInstanceTransformer struct { // Config is the root node in the configuration tree. We'll look up // the appropriate note in this tree using the path in each node. Config *configs.Config + + // There are no orphans when doing a full destroy + destroyPlan bool } func (t *OrphanResourceInstanceTransformer) Transform(g *Graph) error { + if t.destroyPlan { + // everything is being destroyed, so don't worry about orphaned instances + return nil + } + if t.State == nil { // If the entire state is nil, there can't be any orphans return nil From 77d13808d5e0f251743b5ecce6cffa63895e1446 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Fri, 27 May 2022 10:58:28 -0400 Subject: [PATCH 5/8] rename field to destroyPlan for consistency --- internal/terraform/transform_output.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/terraform/transform_output.go b/internal/terraform/transform_output.go index 73419aea0b2e..6805a465a997 100644 --- a/internal/terraform/transform_output.go +++ b/internal/terraform/transform_output.go @@ -21,7 +21,7 @@ type OutputTransformer struct { // If this is a planned destroy, root outputs are still in the configuration // so we need to record that we wish to remove them - Destroy bool + destroyPlan bool // Refresh-only mode means that any failing output preconditions are // reported as warnings rather than errors @@ -66,7 +66,7 @@ func (t *OutputTransformer) transform(g *Graph, c *configs.Config) error { } } - destroy := t.Destroy + destroy := t.destroyPlan if rootChange != nil { destroy = rootChange.Action == plans.Delete } @@ -95,7 +95,7 @@ func (t *OutputTransformer) transform(g *Graph, c *configs.Config) error { Addr: addr, Module: c.Path, Config: o, - Destroy: t.Destroy, + Destroy: t.destroyPlan, RefreshOnly: t.RefreshOnly, } } From 8fed14fc59bc86a2c71f16c6e946bbf4916365b3 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Fri, 27 May 2022 10:59:11 -0400 Subject: [PATCH 6/8] use the PlanGraphBuilder for destroy Rather than maintain a separate graph builder for destroy, use the normal plan graph with some extra options. Utilize the same pattern as the validate graph for now, where we take the normal plan graph builder and inject a new concrete function for the destroy nodes. --- internal/terraform/context_plan.go | 2 +- .../terraform/graph_builder_destroy_plan.go | 108 +----------------- internal/terraform/graph_builder_plan.go | 70 +++++++----- 3 files changed, 47 insertions(+), 133 deletions(-) diff --git a/internal/terraform/context_plan.go b/internal/terraform/context_plan.go index 12168901baab..3364bca0190b 100644 --- a/internal/terraform/context_plan.go +++ b/internal/terraform/context_plan.go @@ -574,7 +574,7 @@ func (c *Context) planGraph(config *configs.Config, prevRunState *states.State, }).Build(addrs.RootModuleInstance) return graph, walkPlan, diags case plans.DestroyMode: - graph, diags := (&DestroyPlanGraphBuilder{ + graph, diags := DestroyPlanGraphBuilder(&PlanGraphBuilder{ Config: config, State: prevRunState, RootVariableValues: opts.SetVariables, diff --git a/internal/terraform/graph_builder_destroy_plan.go b/internal/terraform/graph_builder_destroy_plan.go index ade01c629bdc..9e29d7ceb662 100644 --- a/internal/terraform/graph_builder_destroy_plan.go +++ b/internal/terraform/graph_builder_destroy_plan.go @@ -1,115 +1,17 @@ package terraform import ( - "github.com/hashicorp/terraform/internal/addrs" - "github.com/hashicorp/terraform/internal/configs" "github.com/hashicorp/terraform/internal/dag" - "github.com/hashicorp/terraform/internal/states" - "github.com/hashicorp/terraform/internal/tfdiags" ) -// DestroyPlanGraphBuilder implements GraphBuilder and is responsible for -// planning a pure-destroy. -// -// Planning a pure destroy operation is simple because we can ignore most -// ordering configuration and simply reverse the state. This graph mainly -// exists for targeting, because we need to walk the destroy dependencies to -// ensure we plan the required resources. Without the requirement for -// targeting, the plan could theoretically be created directly from the state. -type DestroyPlanGraphBuilder struct { - // Config is the configuration tree to build the plan from. - Config *configs.Config - - // State is the current state - State *states.State - - // RootVariableValues are the raw input values for root input variables - // given by the caller, which we'll resolve into final values as part - // of the plan walk. - RootVariableValues InputValues - - // Plugins is a library of plug-in components (providers and - // provisioners) available for use. - Plugins *contextPlugins - - // Targets are resources to target - Targets []addrs.Targetable - - // If set, skipRefresh will cause us stop skip refreshing any existing - // resource instances as part of our planning. This will cause us to fail - // to detect if an object has already been deleted outside of Terraform. - skipRefresh bool -} - -// See GraphBuilder -func (b *DestroyPlanGraphBuilder) Build(path addrs.ModuleInstance) (*Graph, tfdiags.Diagnostics) { - return (&BasicGraphBuilder{ - Steps: b.Steps(), - Name: "DestroyPlanGraphBuilder", - }).Build(path) -} - -// See GraphBuilder -func (b *DestroyPlanGraphBuilder) Steps() []GraphTransformer { - concreteResourceInstance := func(a *NodeAbstractResourceInstance) dag.Vertex { +func DestroyPlanGraphBuilder(p *PlanGraphBuilder) GraphBuilder { + p.ConcreteResourceInstance = func(a *NodeAbstractResourceInstance) dag.Vertex { return &NodePlanDestroyableResourceInstance{ NodeAbstractResourceInstance: a, - skipRefresh: b.skipRefresh, - } - } - concreteResourceInstanceDeposed := func(a *NodeAbstractResourceInstance, key states.DeposedKey) dag.Vertex { - return &NodePlanDeposedResourceInstanceObject{ - NodeAbstractResourceInstance: a, - DeposedKey: key, - skipRefresh: b.skipRefresh, - } - } - - concreteProvider := func(a *NodeAbstractProvider) dag.Vertex { - return &NodeApplyableProvider{ - NodeAbstractProvider: a, + skipRefresh: p.skipRefresh, } } + p.destroy = true - steps := []GraphTransformer{ - // Creates nodes for the resource instances tracked in the state. - &StateTransformer{ - ConcreteCurrent: concreteResourceInstance, - ConcreteDeposed: concreteResourceInstanceDeposed, - State: b.State, - }, - - // Create the delete changes for root module outputs. - &OutputTransformer{ - Config: b.Config, - Destroy: true, - }, - - // Attach the state - &AttachStateTransformer{State: b.State}, - - // Attach the configuration to any resources - &AttachResourceConfigTransformer{Config: b.Config}, - - transformProviders(concreteProvider, b.Config), - - // Destruction ordering. We require this only so that - // targeting below will prune the correct things. - &DestroyEdgeTransformer{ - Config: b.Config, - State: b.State, - }, - - &TargetsTransformer{Targets: b.Targets}, - - // Close opened plugin connections - &CloseProviderTransformer{}, - - // Close the root module - &CloseRootModuleTransformer{}, - - &TransitiveReductionTransformer{}, - } - - return steps + return p } diff --git a/internal/terraform/graph_builder_plan.go b/internal/terraform/graph_builder_plan.go index b5e080526e25..6879664d3138 100644 --- a/internal/terraform/graph_builder_plan.go +++ b/internal/terraform/graph_builder_plan.go @@ -1,8 +1,6 @@ package terraform import ( - "sync" - "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/configs" "github.com/hashicorp/terraform/internal/dag" @@ -57,13 +55,16 @@ type PlanGraphBuilder struct { // CustomConcrete can be set to customize the node types created // for various parts of the plan. This is useful in order to customize // the plan behavior. - CustomConcrete bool - ConcreteProvider ConcreteProviderNodeFunc - ConcreteResource ConcreteResourceNodeFunc - ConcreteResourceOrphan ConcreteResourceInstanceNodeFunc - ConcreteModule ConcreteModuleNodeFunc - - once sync.Once + CustomConcrete bool + ConcreteProvider ConcreteProviderNodeFunc + ConcreteResource ConcreteResourceNodeFunc + ConcreteResourceInstance ConcreteResourceInstanceNodeFunc + ConcreteResourceOrphan ConcreteResourceInstanceNodeFunc + ConcreteResourceInstanceDeposed ConcreteResourceInstanceDeposedNodeFunc + ConcreteModule ConcreteModuleNodeFunc + + // destroy is set to true when create a full destroy plan. + destroy bool } // See GraphBuilder @@ -76,36 +77,32 @@ func (b *PlanGraphBuilder) Build(path addrs.ModuleInstance) (*Graph, tfdiags.Dia // See GraphBuilder func (b *PlanGraphBuilder) Steps() []GraphTransformer { - b.once.Do(b.init) - - concreteResourceInstanceDeposed := func(a *NodeAbstractResourceInstance, key states.DeposedKey) dag.Vertex { - return &NodePlanDeposedResourceInstanceObject{ - NodeAbstractResourceInstance: a, - DeposedKey: key, - - skipRefresh: b.skipRefresh, - skipPlanChanges: b.skipPlanChanges, - } - } + b.init() steps := []GraphTransformer{ // Creates all the resources represented in the config &ConfigTransformer{ - Concrete: b.ConcreteResource, - Config: b.Config, + Concrete: b.ConcreteResource, + Config: b.Config, + destroyPlan: b.destroy, }, // Add dynamic values &RootVariableTransformer{Config: b.Config, RawValues: b.RootVariableValues}, &ModuleVariableTransformer{Config: b.Config}, &LocalTransformer{Config: b.Config}, - &OutputTransformer{Config: b.Config, RefreshOnly: b.skipPlanChanges}, + &OutputTransformer{ + Config: b.Config, + RefreshOnly: b.skipPlanChanges, + destroyPlan: b.destroy, + }, // Add orphan resources &OrphanResourceInstanceTransformer{ - Concrete: b.ConcreteResourceOrphan, - State: b.State, - Config: b.Config, + Concrete: b.ConcreteResourceOrphan, + State: b.State, + Config: b.Config, + destroyPlan: b.destroy, }, // We also need nodes for any deposed instance objects present in the @@ -113,7 +110,8 @@ func (b *PlanGraphBuilder) Steps() []GraphTransformer { // skips creating nodes for _current_ objects, since ConfigTransformer // created nodes that will do that during DynamicExpand.) &StateTransformer{ - ConcreteDeposed: concreteResourceInstanceDeposed, + ConcreteCurrent: b.ConcreteResourceInstance, + ConcreteDeposed: b.ConcreteResourceInstanceDeposed, State: b.State, }, @@ -141,15 +139,18 @@ func (b *PlanGraphBuilder) Steps() []GraphTransformer { // objects that can belong to modules. &ModuleExpansionTransformer{Concrete: b.ConcreteModule, Config: b.Config}, - // Connect so that the references are ready for targeting. We'll - // have to connect again later for providers and so on. &ReferenceTransformer{}, + &AttachDependenciesTransformer{}, // Make sure data sources are aware of any depends_on from the // configuration &attachDataResourceDependsOnTransformer{}, + // DestroyEdgeTransformer is only required during a plan so that the + // TargetsTransformer can determine which nodes to keep in the graph. + &DestroyEdgeTransformer{}, + // Target &TargetsTransformer{Targets: b.Targets}, @@ -199,4 +200,15 @@ func (b *PlanGraphBuilder) init() { skipPlanChanges: b.skipPlanChanges, } } + + b.ConcreteResourceInstanceDeposed = func(a *NodeAbstractResourceInstance, key states.DeposedKey) dag.Vertex { + return &NodePlanDeposedResourceInstanceObject{ + NodeAbstractResourceInstance: a, + DeposedKey: key, + + skipRefresh: b.skipRefresh, + skipPlanChanges: b.skipPlanChanges, + } + } + } From ec476af655044487fcc3f28a88555c9c78e20fae Mon Sep 17 00:00:00 2001 From: James Bardin Date: Fri, 27 May 2022 11:56:33 -0400 Subject: [PATCH 7/8] test for configured destroy plan provider --- internal/terraform/context_apply2_test.go | 138 ++++++++++++++++++++++ 1 file changed, 138 insertions(+) diff --git a/internal/terraform/context_apply2_test.go b/internal/terraform/context_apply2_test.go index f2cc27de3588..a9610b725cf6 100644 --- a/internal/terraform/context_apply2_test.go +++ b/internal/terraform/context_apply2_test.go @@ -3,6 +3,7 @@ package terraform import ( "errors" "fmt" + "strings" "sync" "testing" "time" @@ -912,3 +913,140 @@ resource "test_resource" "c" { } }) } + +// pass an input through some expanded values, and back to a provider to make +// sure we can fully evaluate a provider configuration during a destroy plan. +func TestContext2Apply_destroyWithConfiguredProvider(t *testing.T) { + m := testModuleInline(t, map[string]string{ + "main.tf": ` +variable "in" { + type = map(string) + default = { + "a" = "first" + "b" = "second" + } +} + +module "mod" { + source = "./mod" + for_each = var.in + in = each.value +} + +locals { + config = [for each in module.mod : each.out] +} + +provider "other" { + output = [for each in module.mod : each.out] + local = local.config + var = var.in +} + +resource "other_object" "other" { +} +`, + "./mod/main.tf": ` +variable "in" { + type = string +} + +data "test_object" "d" { + test_string = var.in +} + +resource "test_object" "a" { + test_string = var.in +} + +output "out" { + value = data.test_object.d.output +} +`}) + + testProvider := &MockProvider{ + GetProviderSchemaResponse: &providers.GetProviderSchemaResponse{ + Provider: providers.Schema{Block: simpleTestSchema()}, + ResourceTypes: map[string]providers.Schema{ + "test_object": providers.Schema{Block: simpleTestSchema()}, + }, + DataSources: map[string]providers.Schema{ + "test_object": providers.Schema{ + Block: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "test_string": { + Type: cty.String, + Optional: true, + }, + "output": { + Type: cty.String, + Computed: true, + }, + }, + }, + }, + }, + }, + } + + testProvider.ReadDataSourceFn = func(req providers.ReadDataSourceRequest) (resp providers.ReadDataSourceResponse) { + cfg := req.Config.AsValueMap() + s := cfg["test_string"].AsString() + if !strings.Contains("firstsecond", s) { + resp.Diagnostics = resp.Diagnostics.Append(fmt.Errorf("expected 'first' or 'second', got %s", s)) + return resp + } + + cfg["output"] = cty.StringVal(s + "-ok") + resp.State = cty.ObjectVal(cfg) + return resp + } + + otherProvider := &MockProvider{ + GetProviderSchemaResponse: &providers.GetProviderSchemaResponse{ + Provider: providers.Schema{ + Block: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "output": { + Type: cty.List(cty.String), + Optional: true, + }, + "local": { + Type: cty.List(cty.String), + Optional: true, + }, + "var": { + Type: cty.Map(cty.String), + Optional: true, + }, + }, + }, + }, + ResourceTypes: map[string]providers.Schema{ + "other_object": providers.Schema{Block: simpleTestSchema()}, + }, + }, + } + + ctx := testContext2(t, &ContextOpts{ + Providers: map[addrs.Provider]providers.Factory{ + addrs.NewDefaultProvider("test"): testProviderFuncFixed(testProvider), + addrs.NewDefaultProvider("other"): testProviderFuncFixed(otherProvider), + }, + }) + + opts := SimplePlanOpts(plans.NormalMode, testInputValuesUnset(m.Module.Variables)) + plan, diags := ctx.Plan(m, states.NewState(), opts) + assertNoErrors(t, diags) + + state, diags := ctx.Apply(plan, m) + assertNoErrors(t, diags) + + // TODO: extend this to ensure the otherProvider is always properly + // configured during the destroy plan + + opts.Mode = plans.DestroyMode + // destroy only a single instance not included in the moved statements + _, diags = ctx.Plan(m, state, opts) + assertNoErrors(t, diags) +} From e36ee757a5c631456aa823d6f154e9ed23c961a9 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 1 Jun 2022 15:29:59 -0400 Subject: [PATCH 8/8] minor cleanup from review --- internal/terraform/context_apply2_test.go | 2 +- internal/terraform/graph_builder_plan.go | 20 +++++++++---------- internal/terraform/transform_config.go | 11 +++++----- internal/terraform/transform_destroy_edge.go | 1 - .../terraform/transform_orphan_resource.go | 7 +++---- internal/terraform/transform_output.go | 6 +++--- 6 files changed, 22 insertions(+), 25 deletions(-) diff --git a/internal/terraform/context_apply2_test.go b/internal/terraform/context_apply2_test.go index a9610b725cf6..afbe9db044ec 100644 --- a/internal/terraform/context_apply2_test.go +++ b/internal/terraform/context_apply2_test.go @@ -923,7 +923,7 @@ variable "in" { type = map(string) default = { "a" = "first" - "b" = "second" + "b" = "second" } } diff --git a/internal/terraform/graph_builder_plan.go b/internal/terraform/graph_builder_plan.go index 6879664d3138..ed63675d2252 100644 --- a/internal/terraform/graph_builder_plan.go +++ b/internal/terraform/graph_builder_plan.go @@ -82,9 +82,9 @@ func (b *PlanGraphBuilder) Steps() []GraphTransformer { steps := []GraphTransformer{ // Creates all the resources represented in the config &ConfigTransformer{ - Concrete: b.ConcreteResource, - Config: b.Config, - destroyPlan: b.destroy, + Concrete: b.ConcreteResource, + Config: b.Config, + skip: b.destroy, }, // Add dynamic values @@ -92,17 +92,17 @@ func (b *PlanGraphBuilder) Steps() []GraphTransformer { &ModuleVariableTransformer{Config: b.Config}, &LocalTransformer{Config: b.Config}, &OutputTransformer{ - Config: b.Config, - RefreshOnly: b.skipPlanChanges, - destroyPlan: b.destroy, + Config: b.Config, + RefreshOnly: b.skipPlanChanges, + removeRootOutputs: b.destroy, }, // Add orphan resources &OrphanResourceInstanceTransformer{ - Concrete: b.ConcreteResourceOrphan, - State: b.State, - Config: b.Config, - destroyPlan: b.destroy, + Concrete: b.ConcreteResourceOrphan, + State: b.State, + Config: b.Config, + skip: b.destroy, }, // We also need nodes for any deposed instance objects present in the diff --git a/internal/terraform/transform_config.go b/internal/terraform/transform_config.go index f404c80d0a31..3895efebb26a 100644 --- a/internal/terraform/transform_config.go +++ b/internal/terraform/transform_config.go @@ -29,18 +29,17 @@ type ConfigTransformer struct { ModeFilter bool Mode addrs.ResourceMode - // destroyPlan indicated this is being called from a destroy plan. - destroyPlan bool + // Do not apply this transformer. + skip bool } func (t *ConfigTransformer) Transform(g *Graph) error { - // If no configuration is available, we don't do anything - if t.Config == nil { + if t.skip { return nil } - // Configured resource are not added for a destroy plan - if t.destroyPlan { + // If no configuration is available, we don't do anything + if t.Config == nil { return nil } diff --git a/internal/terraform/transform_destroy_edge.go b/internal/terraform/transform_destroy_edge.go index 213289716289..c6cc59397a52 100644 --- a/internal/terraform/transform_destroy_edge.go +++ b/internal/terraform/transform_destroy_edge.go @@ -4,7 +4,6 @@ import ( "log" "github.com/hashicorp/terraform/internal/addrs" - "github.com/hashicorp/terraform/internal/dag" ) diff --git a/internal/terraform/transform_orphan_resource.go b/internal/terraform/transform_orphan_resource.go index e218d8970e70..974fdf0de3ed 100644 --- a/internal/terraform/transform_orphan_resource.go +++ b/internal/terraform/transform_orphan_resource.go @@ -27,13 +27,12 @@ type OrphanResourceInstanceTransformer struct { // the appropriate note in this tree using the path in each node. Config *configs.Config - // There are no orphans when doing a full destroy - destroyPlan bool + // Do not apply this transformer + skip bool } func (t *OrphanResourceInstanceTransformer) Transform(g *Graph) error { - if t.destroyPlan { - // everything is being destroyed, so don't worry about orphaned instances + if t.skip { return nil } diff --git a/internal/terraform/transform_output.go b/internal/terraform/transform_output.go index 6805a465a997..107330ec2d3d 100644 --- a/internal/terraform/transform_output.go +++ b/internal/terraform/transform_output.go @@ -21,7 +21,7 @@ type OutputTransformer struct { // If this is a planned destroy, root outputs are still in the configuration // so we need to record that we wish to remove them - destroyPlan bool + removeRootOutputs bool // Refresh-only mode means that any failing output preconditions are // reported as warnings rather than errors @@ -66,7 +66,7 @@ func (t *OutputTransformer) transform(g *Graph, c *configs.Config) error { } } - destroy := t.destroyPlan + destroy := t.removeRootOutputs if rootChange != nil { destroy = rootChange.Action == plans.Delete } @@ -95,7 +95,7 @@ func (t *OutputTransformer) transform(g *Graph, c *configs.Config) error { Addr: addr, Module: c.Path, Config: o, - Destroy: t.destroyPlan, + Destroy: t.removeRootOutputs, RefreshOnly: t.RefreshOnly, } }