Merge pull request #27408 from hashicorp/jbardin/destroy-plan-refresh

Refresh during destroy

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")
 	}

backend/local/testdata/destroy-with-ds/main.tf

@@ -1,4 +1,5 @@
 resource "test_instance" "foo" {
+  count = 1
   ami = "bar"
 }
 

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{
+	// insert the run-specific data from the context into the plan; variables,
-		VariableValues:  varVals,
+	// targets and provider SHAs.
-		TargetAddrs:     c.targets,
+	plan.VariableValues = varVals
-		ProviderSHA256s: c.providerSHA256s,
+	plan.TargetAddrs = c.targets
-	}
+	plan.ProviderSHA256s = c.providerSHA256s
 
-	operation := walkPlan
+	return plan, diags
-	graphType := GraphTypePlan
+}
-	if c.destroy {
-		operation = walkPlanDestroy
-		graphType = GraphTypePlanDestroy
-	}
 
-	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

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())

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

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)

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
+}

terraform/testdata/apply-destroy-data-resource/main.tf

@@ -1,5 +1,3 @@
 data "null_data_source" "testing" {
-  inputs = {
+  foo = "yes"
-    test = "yes"
-  }
 }

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}"
 }