diff --git a/internal/plans/changes_sync.go b/internal/plans/changes_sync.go index 95920c1c6b98..2b4254b04494 100644 --- a/internal/plans/changes_sync.go +++ b/internal/plans/changes_sync.go @@ -21,24 +21,6 @@ type ChangesSync struct { changes *Changes } -// IsFullDestroy returns true if the set of changes indicates we are doing a -// destroy of all resources. -func (cs *ChangesSync) IsFullDestroy() bool { - if cs == nil { - panic("FullDestroy on nil ChangesSync") - } - cs.lock.Lock() - defer cs.lock.Unlock() - - for _, c := range cs.changes.Resources { - if c.Action != Delete { - return false - } - } - - return true -} - // AppendResourceInstanceChange records the given resource instance change in // the set of planned resource changes. // diff --git a/internal/plans/plan.go b/internal/plans/plan.go index 98f462390c2a..47909b1b0688 100644 --- a/internal/plans/plan.go +++ b/internal/plans/plan.go @@ -28,6 +28,11 @@ type Plan struct { // to the end-user, and so it must not be used to influence apply-time // behavior. The actions during apply must be described entirely by // the Changes field, regardless of how the plan was created. + // + // FIXME: destroy operations still rely on DestroyMode being set, because + // there is no other source of this information in the plan. New behavior + // should not be added based on this flag, and changing the flag should be + // checked carefully against existing destroy behaviors. UIMode Mode VariableValues map[string]DynamicValue diff --git a/internal/terraform/context.go b/internal/terraform/context.go index 115f62276cba..18b3f4b37b54 100644 --- a/internal/terraform/context.go +++ b/internal/terraform/context.go @@ -15,8 +15,6 @@ import ( "github.com/hashicorp/terraform/internal/states" "github.com/hashicorp/terraform/internal/tfdiags" "github.com/zclconf/go-cty/cty" - - _ "github.com/hashicorp/terraform/internal/logging" ) // InputMode defines what sort of input will be asked for when Input diff --git a/internal/terraform/context_apply.go b/internal/terraform/context_apply.go index 1d66628ad773..4630f66a9193 100644 --- a/internal/terraform/context_apply.go +++ b/internal/terraform/context_apply.go @@ -111,6 +111,18 @@ func (c *Context) applyGraph(plan *plans.Plan, config *configs.Config, validate } } + operation := walkApply + if plan.UIMode == plans.DestroyMode { + // FIXME: Due to differences in how objects must be handled in the + // graph and evaluated during a complete destroy, we must continue to + // use plans.DestroyMode to switch on this behavior. If all objects + // which require special destroy handling can be tracked in the plan, + // then this switch will no longer be needed and we can remove the + // walkDestroy operation mode. + // TODO: Audit that and remove walkDestroy as an operation mode. + operation = walkDestroy + } + graph, moreDiags := (&ApplyGraphBuilder{ Config: config, Changes: plan.Changes, @@ -119,22 +131,13 @@ func (c *Context) applyGraph(plan *plans.Plan, config *configs.Config, validate Plugins: c.plugins, Targets: plan.TargetAddrs, ForceReplace: plan.ForceReplaceAddrs, + Operation: operation, }).Build(addrs.RootModuleInstance) diags = diags.Append(moreDiags) if moreDiags.HasErrors() { return nil, walkApply, diags } - operation := walkApply - if plan.UIMode == plans.DestroyMode { - // NOTE: This is a vestigial violation of the rule that we mustn't - // use plan.UIMode to affect apply-time behavior. It's a design error - // if anything downstream switches behavior when operation is set - // to walkDestroy, but we've not yet fully audited that. - // TODO: Audit that and remove walkDestroy as an operation mode. - operation = walkDestroy - } - return graph, operation, diags } diff --git a/internal/terraform/context_apply2_test.go b/internal/terraform/context_apply2_test.go index c0e734f68bc8..100e75def83d 100644 --- a/internal/terraform/context_apply2_test.go +++ b/internal/terraform/context_apply2_test.go @@ -1485,3 +1485,159 @@ resource "test_object" "y" { _, diags = ctx.Apply(plan, m) assertNoErrors(t, diags) } + +// Outputs should not cause evaluation errors during destroy +// Check eval from both root level outputs and module outputs, which are +// handled differently during apply. +func TestContext2Apply_outputsNotToEvaluate(t *testing.T) { + m := testModuleInline(t, map[string]string{ + "main.tf": ` +module "mod" { + source = "./mod" + cond = false +} + +output "from_resource" { + value = module.mod.from_resource +} + +output "from_data" { + value = module.mod.from_data +} +`, + + "./mod/main.tf": ` +variable "cond" { + type = bool +} + +module "mod" { + source = "../mod2/" + cond = var.cond +} + +output "from_resource" { + value = module.mod.resource +} + +output "from_data" { + value = module.mod.data +} +`, + + "./mod2/main.tf": ` +variable "cond" { + type = bool +} + +resource "test_object" "x" { + count = var.cond ? 0:1 +} + +data "test_object" "d" { + count = var.cond ? 0:1 +} + +output "resource" { + value = var.cond ? null : test_object.x.*.test_string[0] +} + +output "data" { + value = one(data.test_object.d[*].test_string) +} +`}) + + p := simpleMockProvider() + p.ReadDataSourceFn = func(req providers.ReadDataSourceRequest) (resp providers.ReadDataSourceResponse) { + resp.State = req.Config + return resp + } + + ctx := testContext2(t, &ContextOpts{ + Providers: map[addrs.Provider]providers.Factory{ + addrs.NewDefaultProvider("test"): testProviderFuncFixed(p), + }, + }) + + // apply the state + opts := SimplePlanOpts(plans.NormalMode, nil) + plan, diags := ctx.Plan(m, states.NewState(), opts) + assertNoErrors(t, diags) + + state, diags := ctx.Apply(plan, m) + assertNoErrors(t, diags) + + // and destroy + opts = SimplePlanOpts(plans.DestroyMode, nil) + plan, diags = ctx.Plan(m, state, opts) + assertNoErrors(t, diags) + + state, diags = ctx.Apply(plan, m) + assertNoErrors(t, diags) + + // and destroy again with no state + if !state.Empty() { + t.Fatal("expected empty state, got", state) + } + + opts = SimplePlanOpts(plans.DestroyMode, nil) + plan, diags = ctx.Plan(m, state, opts) + assertNoErrors(t, diags) + + _, diags = ctx.Apply(plan, m) + assertNoErrors(t, diags) +} + +// don't evaluate conditions on outputs when destroying +func TestContext2Apply_noOutputChecksOnDestroy(t *testing.T) { + m := testModuleInline(t, map[string]string{ + "main.tf": ` +module "mod" { + source = "./mod" +} + +output "from_resource" { + value = module.mod.from_resource +} +`, + + "./mod/main.tf": ` +resource "test_object" "x" { + test_string = "wrong val" +} + +output "from_resource" { + value = test_object.x.test_string + precondition { + condition = test_object.x.test_string == "ok" + error_message = "resource error" + } +} +`}) + + p := simpleMockProvider() + + state := states.NewState() + mod := state.EnsureModule(addrs.RootModuleInstance.Child("mod", addrs.NoKey)) + mod.SetResourceInstanceCurrent( + mustResourceInstanceAddr("test_object.x").Resource, + &states.ResourceInstanceObjectSrc{ + Status: states.ObjectReady, + AttrsJSON: []byte(`{"test_string":"wrong_val"}`), + }, + mustProviderConfig(`provider["registry.terraform.io/hashicorp/test"]`), + ) + + ctx := testContext2(t, &ContextOpts{ + Providers: map[addrs.Provider]providers.Factory{ + addrs.NewDefaultProvider("test"): testProviderFuncFixed(p), + }, + }) + + opts := SimplePlanOpts(plans.DestroyMode, nil) + plan, diags := ctx.Plan(m, state, opts) + assertNoErrors(t, diags) + + _, diags = ctx.Apply(plan, m) + assertNoErrors(t, diags) +} diff --git a/internal/terraform/context_eval_test.go b/internal/terraform/context_eval_test.go index 0bd752935ecb..ac828dbcc424 100644 --- a/internal/terraform/context_eval_test.go +++ b/internal/terraform/context_eval_test.go @@ -85,3 +85,46 @@ func TestContextEval(t *testing.T) { }) } } + +// ensure that we can execute a console when outputs have preconditions +func TestContextEval_outputsWithPreconditions(t *testing.T) { + m := testModuleInline(t, map[string]string{ + "main.tf": ` +module "mod" { + source = "./mod" + input = "ok" +} + +output "out" { + value = module.mod.out +} +`, + + "./mod/main.tf": ` +variable "input" { + type = string +} + +output "out" { + value = var.input + + precondition { + condition = var.input != "" + error_message = "error" + } +} +`, + }) + + p := simpleMockProvider() + ctx := testContext2(t, &ContextOpts{ + Providers: map[addrs.Provider]providers.Factory{ + addrs.NewDefaultProvider("test"): testProviderFuncFixed(p), + }, + }) + + _, diags := ctx.Eval(m, states.NewState(), addrs.RootModuleInstance, &EvalOpts{ + SetVariables: testInputValuesUnset(m.Module.Variables), + }) + assertNoErrors(t, diags) +} diff --git a/internal/terraform/context_plan.go b/internal/terraform/context_plan.go index abd8d2adbe24..9b35ea756237 100644 --- a/internal/terraform/context_plan.go +++ b/internal/terraform/context_plan.go @@ -334,11 +334,11 @@ func (c *Context) destroyPlan(config *configs.Config, prevRunState *states.State // must coordinate with this by taking that action only when c.skipRefresh // _is_ set. This coupling between the two is unfortunate but necessary // to work within our current structure. - if !opts.SkipRefresh { + if !opts.SkipRefresh && !prevRunState.Empty() { log.Printf("[TRACE] Context.destroyPlan: calling Context.plan to get the effect of refreshing the prior state") - normalOpts := *opts - normalOpts.Mode = plans.NormalMode - refreshPlan, refreshDiags := c.plan(config, prevRunState, &normalOpts) + refreshOpts := *opts + refreshOpts.Mode = plans.RefreshOnlyMode + refreshPlan, refreshDiags := c.refreshOnlyPlan(config, prevRunState, &refreshOpts) if refreshDiags.HasErrors() { // NOTE: Normally we'd append diagnostics regardless of whether // there are errors, just in case there are warnings we'd want to diff --git a/internal/terraform/evaluate.go b/internal/terraform/evaluate.go index b841dbd863e2..d521443472f6 100644 --- a/internal/terraform/evaluate.go +++ b/internal/terraform/evaluate.go @@ -728,7 +728,7 @@ func (d *evaluationStateData) GetResource(addr addrs.Resource, rng tfdiags.Sourc // Decode all instances in the current state instances := map[addrs.InstanceKey]cty.Value{} - pendingDestroy := d.Evaluator.Changes.IsFullDestroy() + pendingDestroy := d.Operation == walkDestroy for key, is := range rs.Instances { if is == nil || is.Current == nil { // Assume we're dealing with an instance that hasn't been created yet. diff --git a/internal/terraform/graph.go b/internal/terraform/graph.go index 242ac0429b7c..38d0fad6ed5c 100644 --- a/internal/terraform/graph.go +++ b/internal/terraform/graph.go @@ -106,7 +106,7 @@ func (g *Graph) walk(walker GraphWalker) tfdiags.Diagnostics { diags = diags.Append(tfdiags.Sourceless( tfdiags.Error, "Graph node has invalid dynamic subgraph", - fmt.Sprintf("The internal logic for %q generated an invalid dynamic subgraph: the root node is %T, which is not a suitable root node type.\n\nThis is a bug in Terraform. Please report it!", dag.VertexName(v), v), + fmt.Sprintf("The internal logic for %q generated an invalid dynamic subgraph: the root node is %T, which is not a suitable root node type.\n\nThis is a bug in Terraform. Please report it!", dag.VertexName(v), n), )) return } diff --git a/internal/terraform/graph_builder_apply.go b/internal/terraform/graph_builder_apply.go index 619e2e14ff1e..4d7763d2a5f3 100644 --- a/internal/terraform/graph_builder_apply.go +++ b/internal/terraform/graph_builder_apply.go @@ -46,6 +46,9 @@ type ApplyGraphBuilder struct { // The apply step refers to these as part of verifying that the planned // actions remain consistent between plan and apply. ForceReplace []addrs.AbsResourceInstance + + // Plan Operation this graph will be used for. + Operation walkOperation } // See GraphBuilder @@ -92,7 +95,10 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer { &RootVariableTransformer{Config: b.Config, RawValues: b.RootVariableValues}, &ModuleVariableTransformer{Config: b.Config}, &LocalTransformer{Config: b.Config}, - &OutputTransformer{Config: b.Config, Changes: b.Changes}, + &OutputTransformer{ + Config: b.Config, + ApplyDestroy: b.Operation == walkDestroy, + }, // Creates all the resource instances represented in the diff, along // with dependency edges against the whole-resource nodes added by @@ -136,7 +142,9 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer { &ForcedCBDTransformer{}, // Destruction ordering - &DestroyEdgeTransformer{}, + &DestroyEdgeTransformer{ + Changes: b.Changes, + }, &CBDEdgeTransformer{ Config: b.Config, State: b.State, diff --git a/internal/terraform/graph_builder_eval.go b/internal/terraform/graph_builder_eval.go index 4e205045f7a3..cb3606d62c9f 100644 --- a/internal/terraform/graph_builder_eval.go +++ b/internal/terraform/graph_builder_eval.go @@ -67,7 +67,10 @@ func (b *EvalGraphBuilder) Steps() []GraphTransformer { &RootVariableTransformer{Config: b.Config, RawValues: b.RootVariableValues}, &ModuleVariableTransformer{Config: b.Config}, &LocalTransformer{Config: b.Config}, - &OutputTransformer{Config: b.Config}, + &OutputTransformer{ + Config: b.Config, + Planning: true, + }, // Attach the configuration to any resources &AttachResourceConfigTransformer{Config: b.Config}, diff --git a/internal/terraform/graph_builder_plan.go b/internal/terraform/graph_builder_plan.go index 3a4a457e45fb..4195a7445a79 100644 --- a/internal/terraform/graph_builder_plan.go +++ b/internal/terraform/graph_builder_plan.go @@ -110,9 +110,9 @@ func (b *PlanGraphBuilder) Steps() []GraphTransformer { &ModuleVariableTransformer{Config: b.Config}, &LocalTransformer{Config: b.Config}, &OutputTransformer{ - Config: b.Config, - RefreshOnly: b.skipPlanChanges, - removeRootOutputs: b.Operation == walkPlanDestroy, + Config: b.Config, + RefreshOnly: b.skipPlanChanges, + PlanDestroy: b.Operation == walkPlanDestroy, // NOTE: We currently treat anything built with the plan graph // builder as "planning" for our purposes here, because we share diff --git a/internal/terraform/node_output.go b/internal/terraform/node_output.go index c2948ccff3c5..1079a59df36f 100644 --- a/internal/terraform/node_output.go +++ b/internal/terraform/node_output.go @@ -20,11 +20,12 @@ import ( // nodeExpandOutput is the placeholder for a non-root module output that has // not yet had its module path expanded. type nodeExpandOutput struct { - Addr addrs.OutputValue - Module addrs.Module - Config *configs.Output - Destroy bool - RefreshOnly bool + Addr addrs.OutputValue + Module addrs.Module + Config *configs.Output + PlanDestroy bool + ApplyDestroy bool + RefreshOnly bool // Planning is set to true when this node is in a graph that was produced // by the plan graph builder, as opposed to the apply graph builder. @@ -52,13 +53,6 @@ func (n *nodeExpandOutput) temporaryValue() bool { } func (n *nodeExpandOutput) DynamicExpand(ctx EvalContext) (*Graph, error) { - if n.Destroy { - // if we're planning a destroy, we only need to handle the root outputs. - // The destroy plan doesn't evaluate any other config, so we can skip - // the rest of the outputs. - return n.planDestroyRootOutput(ctx) - } - expander := ctx.InstanceExpander() changes := ctx.Changes() @@ -104,14 +98,29 @@ func (n *nodeExpandOutput) DynamicExpand(ctx EvalContext) (*Graph, error) { } } - o := &NodeApplyableOutput{ - Addr: absAddr, - Config: n.Config, - Change: change, - RefreshOnly: n.RefreshOnly, + var node dag.Vertex + switch { + case module.IsRoot() && (n.PlanDestroy || n.ApplyDestroy): + node = &NodeDestroyableOutput{ + Addr: absAddr, + } + + case n.PlanDestroy: + // nothing is done here for non-root outputs + continue + + default: + node = &NodeApplyableOutput{ + Addr: absAddr, + Config: n.Config, + Change: change, + RefreshOnly: n.RefreshOnly, + DestroyApply: n.ApplyDestroy, + } } - log.Printf("[TRACE] Expanding output: adding %s as %T", o.Addr.String(), o) - g.Add(o) + + log.Printf("[TRACE] Expanding output: adding %s as %T", absAddr.String(), node) + g.Add(node) } addRootNodeToGraph(&g) @@ -123,27 +132,6 @@ func (n *nodeExpandOutput) DynamicExpand(ctx EvalContext) (*Graph, error) { return &g, nil } -// if we're planing a destroy operation, add a destroy node for any root output -func (n *nodeExpandOutput) planDestroyRootOutput(ctx EvalContext) (*Graph, error) { - if !n.Module.IsRoot() { - return nil, nil - } - state := ctx.State() - if state == nil { - return nil, nil - } - - var g Graph - o := &NodeDestroyableOutput{ - Addr: n.Addr.Absolute(addrs.RootModuleInstance), - Config: n.Config, - } - log.Printf("[TRACE] Expanding output: adding %s as %T", o.Addr.String(), o) - g.Add(o) - - return &g, nil -} - func (n *nodeExpandOutput) Name() string { path := n.Module.String() addr := n.Addr.String() + " (expand)" @@ -192,8 +180,11 @@ func (n *nodeExpandOutput) ReferenceOutside() (selfPath, referencePath addrs.Mod // GraphNodeReferencer func (n *nodeExpandOutput) References() []*addrs.Reference { - // root outputs might be destroyable, and may not reference anything in - // that case + // DestroyNodes do not reference anything. + if n.Module.IsRoot() && n.ApplyDestroy { + return nil + } + return referencesForOutput(n.Config) } @@ -208,6 +199,10 @@ type NodeApplyableOutput struct { // Refresh-only mode means that any failing output preconditions are // reported as warnings rather than errors RefreshOnly bool + + // DestroyApply indicates that we are applying a destroy plan, and do not + // need to account for conditional blocks. + DestroyApply bool } var ( @@ -321,19 +316,23 @@ func (n *NodeApplyableOutput) Execute(ctx EvalContext, op walkOperation) (diags } } - checkRuleSeverity := tfdiags.Error - if n.RefreshOnly { - checkRuleSeverity = tfdiags.Warning - } - checkDiags := evalCheckRules( - addrs.OutputPrecondition, - n.Config.Preconditions, - ctx, n.Addr, EvalDataForNoInstanceKey, - checkRuleSeverity, - ) - diags = diags.Append(checkDiags) - if diags.HasErrors() { - return diags // failed preconditions prevent further evaluation + // Checks are not evaluated during a destroy. The checks may fail, may not + // be valid, or may not have been registered at all. + if !n.DestroyApply { + checkRuleSeverity := tfdiags.Error + if n.RefreshOnly { + checkRuleSeverity = tfdiags.Warning + } + checkDiags := evalCheckRules( + addrs.OutputPrecondition, + n.Config.Preconditions, + ctx, n.Addr, EvalDataForNoInstanceKey, + checkRuleSeverity, + ) + diags = diags.Append(checkDiags) + if diags.HasErrors() { + return diags // failed preconditions prevent further evaluation + } } // If there was no change recorded, or the recorded change was not wholly @@ -421,8 +420,7 @@ func (n *NodeApplyableOutput) DotNode(name string, opts *dag.DotOpts) *dag.DotNo // NodeDestroyableOutput represents an output that is "destroyable": // its application will remove the output from the state. type NodeDestroyableOutput struct { - Addr addrs.AbsOutputValue - Config *configs.Output // Config is the output in the config + Addr addrs.AbsOutputValue } var ( diff --git a/internal/terraform/transform_destroy_edge.go b/internal/terraform/transform_destroy_edge.go index 14b421b7581d..aac39fd4d7cd 100644 --- a/internal/terraform/transform_destroy_edge.go +++ b/internal/terraform/transform_destroy_edge.go @@ -5,6 +5,7 @@ import ( "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/dag" + "github.com/hashicorp/terraform/internal/plans" ) // GraphNodeDestroyer must be implemented by nodes that destroy resources. @@ -37,7 +38,15 @@ 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{} +type DestroyEdgeTransformer struct { + // FIXME: GraphNodeCreators are not always applying changes, and should not + // participate in the destroy graph if there are no operations which could + // interract with destroy nodes. We need Changes for now to detect the + // action type, but perhaps this should be indicated somehow by the + // DiffTransformer which was intended to be the only transformer operating + // from the change set. + Changes *plans.Changes +} // tryInterProviderDestroyEdge checks if we're inserting a destroy edge // across a provider boundary, and only adds the edge if it results in no cycles. @@ -144,8 +153,20 @@ func (t *DestroyEdgeTransformer) Transform(g *Graph) error { resAddr := addr.ContainingResource().Config().String() destroyersByResource[resAddr] = append(destroyersByResource[resAddr], n) case GraphNodeCreator: - addr := n.CreateAddr().ContainingResource().Config().String() - creators[addr] = append(creators[addr], n) + addr := n.CreateAddr() + cfgAddr := addr.ContainingResource().Config().String() + + if t.Changes == nil { + // unit tests may not have changes + creators[cfgAddr] = append(creators[cfgAddr], n) + break + } + + // NoOp changes should not participate in the destroy dependencies. + rc := t.Changes.ResourceInstance(*addr) + if rc != nil && rc.Action != plans.NoOp { + creators[cfgAddr] = append(creators[cfgAddr], n) + } } } diff --git a/internal/terraform/transform_destroy_edge_test.go b/internal/terraform/transform_destroy_edge_test.go index a3690994018b..b4fc351be2ec 100644 --- a/internal/terraform/transform_destroy_edge_test.go +++ b/internal/terraform/transform_destroy_edge_test.go @@ -383,8 +383,7 @@ func TestPruneUnusedNodesTransformer_rootModuleOutputValues(t *testing.T) { Config: config, }, &OutputTransformer{ - Config: config, - Changes: changes, + Config: config, }, &DiffTransformer{ Concrete: concreteResourceInstance, @@ -441,6 +440,75 @@ func TestPruneUnusedNodesTransformer_rootModuleOutputValues(t *testing.T) { } } +// NoOp changes should not be participating in the destroy sequence +func TestDestroyEdgeTransformer_noOp(t *testing.T) { + g := Graph{Path: addrs.RootModuleInstance} + g.Add(testDestroyNode("test_object.A")) + g.Add(testUpdateNode("test_object.B")) + g.Add(testDestroyNode("test_object.C")) + + state := states.NewState() + root := state.EnsureModule(addrs.RootModuleInstance) + root.SetResourceInstanceCurrent( + mustResourceInstanceAddr("test_object.A").Resource, + &states.ResourceInstanceObjectSrc{ + Status: states.ObjectReady, + AttrsJSON: []byte(`{"id":"A"}`), + }, + mustProviderConfig(`provider["registry.terraform.io/hashicorp/test"]`), + ) + root.SetResourceInstanceCurrent( + mustResourceInstanceAddr("test_object.B").Resource, + &states.ResourceInstanceObjectSrc{ + Status: states.ObjectReady, + AttrsJSON: []byte(`{"id":"B","test_string":"x"}`), + Dependencies: []addrs.ConfigResource{mustConfigResourceAddr("test_object.A")}, + }, + mustProviderConfig(`provider["registry.terraform.io/hashicorp/test"]`), + ) + root.SetResourceInstanceCurrent( + mustResourceInstanceAddr("test_object.C").Resource, + &states.ResourceInstanceObjectSrc{ + Status: states.ObjectReady, + AttrsJSON: []byte(`{"id":"C","test_string":"x"}`), + Dependencies: []addrs.ConfigResource{mustConfigResourceAddr("test_object.A"), + mustConfigResourceAddr("test_object.B")}, + }, + mustProviderConfig(`provider["registry.terraform.io/hashicorp/test"]`), + ) + + if err := (&AttachStateTransformer{State: state}).Transform(&g); err != nil { + t.Fatal(err) + } + + tf := &DestroyEdgeTransformer{ + // We only need a minimal object to indicate GraphNodeCreator change is + // a NoOp here. + Changes: &plans.Changes{ + Resources: []*plans.ResourceInstanceChangeSrc{ + { + Addr: mustResourceInstanceAddr("test_object.B"), + ChangeSrc: plans.ChangeSrc{Action: plans.NoOp}, + }, + }, + }, + } + if err := tf.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + + expected := strings.TrimSpace(` +test_object.A (destroy) + test_object.C (destroy) +test_object.B +test_object.C (destroy)`) + + actual := strings.TrimSpace(g.String()) + if actual != expected { + t.Fatalf("wrong result\n\ngot:\n%s\n\nwant:\n%s", actual, expected) + } +} + func testDestroyNode(addrString string) GraphNodeDestroyer { instAddr := mustResourceInstanceAddr(addrString) inst := NewNodeAbstractResourceInstance(instAddr) diff --git a/internal/terraform/transform_output.go b/internal/terraform/transform_output.go index ffed7df19681..214bc3ff10ec 100644 --- a/internal/terraform/transform_output.go +++ b/internal/terraform/transform_output.go @@ -5,8 +5,6 @@ import ( "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/configs" - "github.com/hashicorp/terraform/internal/dag" - "github.com/hashicorp/terraform/internal/plans" ) // OutputTransformer is a GraphTransformer that adds all the outputs @@ -16,12 +14,7 @@ import ( // aren't changing since there is no downside: the state will be available // even if the dependent items aren't changing. type OutputTransformer struct { - Config *configs.Config - Changes *plans.Changes - - // If this is a planned destroy, root outputs are still in the configuration - // so we need to record that we wish to remove them - removeRootOutputs bool + Config *configs.Config // Refresh-only mode means that any failing output preconditions are // reported as warnings rather than errors @@ -30,6 +23,14 @@ type OutputTransformer struct { // Planning must be set to true only when we're building a planning graph. // It must be set to false whenever we're building an apply graph. Planning bool + + // If this is a planned destroy, root outputs are still in the configuration + // so we need to record that we wish to remove them + PlanDestroy bool + + // ApplyDestroy indicates that this is being added to an apply graph, which + // is the result of a destroy plan. + ApplyDestroy bool } func (t *OutputTransformer) Transform(g *Graph) error { @@ -51,50 +52,17 @@ func (t *OutputTransformer) transform(g *Graph, c *configs.Config) error { } } - // Add outputs to the graph, which will be dynamically expanded - // into NodeApplyableOutputs to reflect possible expansion - // through the presence of "count" or "for_each" on the modules. - - var changes []*plans.OutputChangeSrc - if t.Changes != nil { - changes = t.Changes.Outputs - } - for _, o := range c.Module.Outputs { addr := addrs.OutputValue{Name: o.Name} - var rootChange *plans.OutputChangeSrc - for _, c := range changes { - if c.Addr.Module.IsRoot() && c.Addr.OutputValue.Name == o.Name { - rootChange = c - } - } - - destroy := t.removeRootOutputs - if rootChange != nil { - destroy = rootChange.Action == plans.Delete - } - - // If this is a root output and we're destroying, we add the destroy - // node directly, as there is no need to expand. - - var node dag.Vertex - switch { - case c.Path.IsRoot() && destroy: - node = &NodeDestroyableOutput{ - Addr: addr.Absolute(addrs.RootModuleInstance), - Config: o, - } - - default: - node = &nodeExpandOutput{ - Addr: addr, - Module: c.Path, - Config: o, - Destroy: t.removeRootOutputs, - RefreshOnly: t.RefreshOnly, - Planning: t.Planning, - } + node := &nodeExpandOutput{ + Addr: addr, + Module: c.Path, + Config: o, + PlanDestroy: t.PlanDestroy, + ApplyDestroy: t.ApplyDestroy, + RefreshOnly: t.RefreshOnly, + Planning: t.Planning, } log.Printf("[TRACE] OutputTransformer: adding %s as %T", o.Name, node)