diff --git a/backend/local/backend_plan_test.go b/backend/local/backend_plan_test.go index 5c0545380..3f72168f1 100644 --- a/backend/local/backend_plan_test.go +++ b/backend/local/backend_plan_test.go @@ -559,7 +559,7 @@ func TestLocal_planDestroy(t *testing.T) { b, cleanup := TestLocal(t) defer cleanup() - p := TestLocalProvider(t, b, "test", planFixtureSchema()) + TestLocalProvider(t, b, "test", planFixtureSchema()) testStateFile(t, b.StatePath, testPlanState()) outDir := testTempDir(t) @@ -593,10 +593,6 @@ func TestLocal_planDestroy(t *testing.T) { t.Fatalf("plan operation failed") } - if p.ReadResourceCalled { - t.Fatal("ReadResource should not be called") - } - if run.PlanEmpty { t.Fatal("plan should not be empty") } @@ -613,7 +609,7 @@ func TestLocal_planDestroy_withDataSources(t *testing.T) { b, cleanup := TestLocal(t) defer cleanup() - p := TestLocalProvider(t, b, "test", planFixtureSchema()) + TestLocalProvider(t, b, "test", planFixtureSchema()) testStateFile(t, b.StatePath, testPlanState_withDataSource()) b.CLI = cli.NewMockUi() @@ -649,14 +645,6 @@ func TestLocal_planDestroy_withDataSources(t *testing.T) { t.Fatalf("plan operation failed") } - if p.ReadResourceCalled { - t.Fatal("ReadResource should not be called") - } - - if p.ReadDataSourceCalled { - t.Fatal("ReadDataSourceCalled should not be called") - } - if run.PlanEmpty { t.Fatal("plan should not be empty") } diff --git a/backend/local/testdata/destroy-with-ds/main.tf b/backend/local/testdata/destroy-with-ds/main.tf index 4ee80ea3d..7062d896b 100644 --- a/backend/local/testdata/destroy-with-ds/main.tf +++ b/backend/local/testdata/destroy-with-ds/main.tf @@ -1,4 +1,5 @@ resource "test_instance" "foo" { + count = 1 ami = "bar" } diff --git a/terraform/context.go b/terraform/context.go index 7e24de341..a7fa0fb3e 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -530,6 +530,20 @@ The -target option is not for routine use, and is provided only for exceptional )) } + var plan *plans.Plan + var planDiags tfdiags.Diagnostics + switch { + case c.destroy: + plan, planDiags = c.destroyPlan() + default: + plan, planDiags = c.plan() + } + diags = diags.Append(planDiags) + if diags.HasErrors() { + return nil, diags + } + + // convert the variables into the format expected for the plan varVals := make(map[string]plans.DynamicValue, len(c.variables)) for k, iv := range c.variables { // We use cty.DynamicPseudoType here so that we'll save both the @@ -547,44 +561,85 @@ The -target option is not for routine use, and is provided only for exceptional varVals[k] = dv } - p := &plans.Plan{ - VariableValues: varVals, - TargetAddrs: c.targets, - ProviderSHA256s: c.providerSHA256s, - } + // insert the run-specific data from the context into the plan; variables, + // targets and provider SHAs. + plan.VariableValues = varVals + plan.TargetAddrs = c.targets + plan.ProviderSHA256s = c.providerSHA256s - operation := walkPlan - graphType := GraphTypePlan - if c.destroy { - operation = walkPlanDestroy - graphType = GraphTypePlanDestroy - } + return plan, diags +} - graph, graphDiags := c.Graph(graphType, nil) +func (c *Context) plan() (*plans.Plan, tfdiags.Diagnostics) { + var diags tfdiags.Diagnostics + + graph, graphDiags := c.Graph(GraphTypePlan, nil) diags = diags.Append(graphDiags) if graphDiags.HasErrors() { return nil, diags } // Do the walk - walker, walkDiags := c.walk(graph, operation) + walker, walkDiags := c.walk(graph, walkPlan) diags = diags.Append(walker.NonFatalDiagnostics) diags = diags.Append(walkDiags) if walkDiags.HasErrors() { return nil, diags } - p.Changes = c.changes + plan := &plans.Plan{ + Changes: c.changes, + } c.refreshState.SyncWrapper().RemovePlannedResourceInstanceObjects() refreshedState := c.refreshState.DeepCopy() - p.State = refreshedState + plan.State = refreshedState // replace the working state with the updated state, so that immediate calls // to Apply work as expected. c.state = refreshedState - return p, diags + return plan, diags +} + +func (c *Context) destroyPlan() (*plans.Plan, tfdiags.Diagnostics) { + var diags tfdiags.Diagnostics + destroyPlan := &plans.Plan{} + c.changes = plans.NewChanges() + + // A destroy plan starts by running Refresh to read any pending data + // sources, and remove missing managed resources. This is required because + // a "destroy plan" is only creating delete changes, and is essentially a + // local operation. + if !c.skipRefresh { + refreshPlan, refreshDiags := c.plan() + diags = diags.Append(refreshDiags) + if diags.HasErrors() { + return nil, diags + } + + // insert the refreshed state into the destroy plan result, and discard + // the changes recorded from the refresh. + destroyPlan.State = refreshPlan.State + c.changes = plans.NewChanges() + } + + graph, graphDiags := c.Graph(GraphTypePlanDestroy, nil) + diags = diags.Append(graphDiags) + if graphDiags.HasErrors() { + return nil, diags + } + + // Do the walk + walker, walkDiags := c.walk(graph, walkPlan) + diags = diags.Append(walker.NonFatalDiagnostics) + diags = diags.Append(walkDiags) + if walkDiags.HasErrors() { + return nil, diags + } + + destroyPlan.Changes = c.changes + return destroyPlan, diags } // Refresh goes through all the resources in the state and refreshes them diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index a29ba8730..4d1c80f7c 100644 --- a/terraform/context_apply_test.go +++ b/terraform/context_apply_test.go @@ -1275,6 +1275,7 @@ func TestContext2Apply_destroyDependsOnStateOnly(t *testing.T) { } func testContext2Apply_destroyDependsOnStateOnly(t *testing.T, state *states.State) { + state = state.DeepCopy() m := testModule(t, "empty") p := testProvider("aws") p.ApplyResourceChangeFn = testApplyFn @@ -1371,6 +1372,7 @@ func TestContext2Apply_destroyDependsOnStateOnlyModule(t *testing.T) { } func testContext2Apply_destroyDependsOnStateOnlyModule(t *testing.T, state *states.State) { + state = state.DeepCopy() m := testModule(t, "empty") p := testProvider("aws") p.ApplyResourceChangeFn = testApplyFn @@ -1451,6 +1453,12 @@ func TestContext2Apply_destroyData(t *testing.T) { p := testProvider("null") p.ApplyResourceChangeFn = testApplyFn p.PlanResourceChangeFn = testDiffFn + p.ReadDataSourceFn = func(req providers.ReadDataSourceRequest) providers.ReadDataSourceResponse { + return providers.ReadDataSourceResponse{ + State: req.Config, + } + } + state := states.NewState() root := state.EnsureModule(addrs.RootModuleInstance) root.SetResourceInstanceCurrent( @@ -1493,6 +1501,8 @@ func TestContext2Apply_destroyData(t *testing.T) { } wantHookCalls := []*testHookCall{ + {"PreDiff", "data.null_data_source.testing"}, + {"PostDiff", "data.null_data_source.testing"}, {"PreDiff", "data.null_data_source.testing"}, {"PostDiff", "data.null_data_source.testing"}, {"PostStateUpdate", ""}, @@ -9561,6 +9571,18 @@ func TestContext2Apply_destroyDataCycle(t *testing.T) { p := testProvider("null") p.ApplyResourceChangeFn = testApplyFn p.PlanResourceChangeFn = testDiffFn + p.ReadDataSourceFn = func(req providers.ReadDataSourceRequest) providers.ReadDataSourceResponse { + return providers.ReadDataSourceResponse{ + State: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("new"), + "foo": cty.NullVal(cty.String), + }), + } + } + + tp := testProvider("test") + tp.ApplyResourceChangeFn = testApplyFn + tp.PlanResourceChangeFn = testDiffFn state := states.NewState() root := state.EnsureModule(addrs.RootModuleInstance) @@ -9579,6 +9601,31 @@ func TestContext2Apply_destroyDataCycle(t *testing.T) { Module: addrs.RootModule, }, ) + root.SetResourceInstanceCurrent( + addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "test_resource", + Name: "a", + }.Instance(addrs.IntKey(0)), + &states.ResourceInstanceObjectSrc{ + Status: states.ObjectReady, + AttrsJSON: []byte(`{"id":"a"}`), + Dependencies: []addrs.ConfigResource{ + addrs.ConfigResource{ + Resource: addrs.Resource{ + Mode: addrs.DataResourceMode, + Type: "null_data_source", + Name: "d", + }, + Module: addrs.RootModule, + }, + }, + }, + addrs.AbsProviderConfig{ + Provider: addrs.NewDefaultProvider("test"), + Module: addrs.RootModule, + }, + ) root.SetResourceInstanceCurrent( addrs.Resource{ Mode: addrs.DataResourceMode, @@ -9587,7 +9634,7 @@ func TestContext2Apply_destroyDataCycle(t *testing.T) { }.Instance(addrs.NoKey), &states.ResourceInstanceObjectSrc{ Status: states.ObjectReady, - AttrsJSON: []byte(`{"id":"data"}`), + AttrsJSON: []byte(`{"id":"old"}`), }, addrs.AbsProviderConfig{ Provider: addrs.NewDefaultProvider("null"), @@ -9597,15 +9644,14 @@ func TestContext2Apply_destroyDataCycle(t *testing.T) { Providers := map[addrs.Provider]providers.Factory{ addrs.NewDefaultProvider("null"): testProviderFuncFixed(p), + addrs.NewDefaultProvider("test"): testProviderFuncFixed(tp), } - hook := &testHook{} ctx := testContext2(t, &ContextOpts{ Config: m, Providers: Providers, State: state, Destroy: true, - Hooks: []Hook{hook}, }) plan, diags := ctx.Plan() @@ -9627,6 +9673,19 @@ func TestContext2Apply_destroyDataCycle(t *testing.T) { t.Fatalf("failed to create context for plan: %s", diags.Err()) } + tp.ConfigureFn = func(req providers.ConfigureRequest) (resp providers.ConfigureResponse) { + foo := req.Config.GetAttr("foo") + if !foo.IsKnown() { + resp.Diagnostics = resp.Diagnostics.Append(fmt.Errorf("unknown config value foo")) + return resp + } + + if foo.AsString() != "new" { + resp.Diagnostics = resp.Diagnostics.Append(fmt.Errorf("wrong config value: %q", foo.AsString())) + } + return resp + } + _, diags = ctx.Apply() if diags.HasErrors() { t.Fatalf("diags: %s", diags.Err()) diff --git a/terraform/graph_builder_destroy_plan.go b/terraform/graph_builder_destroy_plan.go index d82162282..a16729507 100644 --- a/terraform/graph_builder_destroy_plan.go +++ b/terraform/graph_builder_destroy_plan.go @@ -12,7 +12,10 @@ import ( // planning a pure-destroy. // // Planning a pure destroy operation is simple because we can ignore most -// ordering configuration and simply reverse the state. +// 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 @@ -72,6 +75,7 @@ func (b *DestroyPlanGraphBuilder) Steps() []GraphTransformer { State: b.State, }, + // Create the delete changes for root module outputs. &OutputTransformer{ Config: b.Config, Destroy: true, @@ -93,8 +97,6 @@ func (b *DestroyPlanGraphBuilder) Steps() []GraphTransformer { Schemas: b.Schemas, }, - // Target. Note we don't set "Destroy: true" here since we already - // created proper destroy ordering. &TargetsTransformer{Targets: b.Targets}, // Close opened plugin connections diff --git a/terraform/node_resource_abstract_instance.go b/terraform/node_resource_abstract_instance.go index 417d94bf1..3278759e5 100644 --- a/terraform/node_resource_abstract_instance.go +++ b/terraform/node_resource_abstract_instance.go @@ -1401,6 +1401,13 @@ func (n *NodeAbstractResourceInstance) planDataSource(ctx EvalContext, currentSt return plannedChange, plannedNewState, diags } + // While this isn't a "diff", continue to call this for data sources. + diags = diags.Append(ctx.Hook(func(h Hook) (HookAction, error) { + return h.PreDiff(n.Addr, states.CurrentGen, priorVal, configVal) + })) + if diags.HasErrors() { + return nil, nil, diags + } // We have a complete configuration with no dependencies to wait on, so we // can read the data source into the state. newVal, readDiags := n.readDataSource(ctx, configVal) diff --git a/terraform/testdata/apply-destroy-data-cycle/main.tf b/terraform/testdata/apply-destroy-data-cycle/main.tf index bd72a47e3..591af8200 100644 --- a/terraform/testdata/apply-destroy-data-cycle/main.tf +++ b/terraform/testdata/apply-destroy-data-cycle/main.tf @@ -8,3 +8,7 @@ data "null_data_source" "d" { resource "null_resource" "a" { count = local.l == "NONE" ? 1 : 0 } + +provider "test" { + foo = data.null_data_source.d.id +} diff --git a/terraform/testdata/apply-destroy-data-resource/main.tf b/terraform/testdata/apply-destroy-data-resource/main.tf index cb16d9f34..0d941a707 100644 --- a/terraform/testdata/apply-destroy-data-resource/main.tf +++ b/terraform/testdata/apply-destroy-data-resource/main.tf @@ -1,5 +1,3 @@ data "null_data_source" "testing" { - inputs = { - test = "yes" - } + foo = "yes" } diff --git a/terraform/testdata/plan-module-destroy-gh-1835/b/main.tf b/terraform/testdata/plan-module-destroy-gh-1835/b/main.tf index c3b0270b0..3b0cc6664 100644 --- a/terraform/testdata/plan-module-destroy-gh-1835/b/main.tf +++ b/terraform/testdata/plan-module-destroy-gh-1835/b/main.tf @@ -1,5 +1,5 @@ variable "a_id" {} resource "aws_instance" "b" { - command = "echo ${var.a_id}" + foo = "echo ${var.a_id}" }