Merge pull request #31163 from hashicorp/jbardin/plan-destroy

Use plan graph builder for destroy

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

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,

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,

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
 }

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,
+			skip:     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,
+			removeRootOutputs: b.destroy,
+		},
 
 		// Add orphan resources
 		&OrphanResourceInstanceTransformer{
 			Concrete: b.ConcreteResourceOrphan,
 			State:    b.State,
 			Config:   b.Config,
+			skip:     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,
+		}
+	}
+
 }

internal/terraform/transform_config.go

@@ -28,9 +28,16 @@ type ConfigTransformer struct {
 	// Mode will only add resources that match the given mode
 	ModeFilter bool
 	Mode       addrs.ResourceMode
+
+	// Do not apply this transformer.
+	skip bool
 }
 
 func (t *ConfigTransformer) Transform(g *Graph) error {
+	if t.skip {
+		return nil
+	}
+
 	// If no configuration is available, we don't do anything
 	if t.Config == nil {
 		return nil

internal/terraform/transform_destroy_edge.go

@@ -4,9 +4,6 @@ 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 +37,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 +81,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)

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

internal/terraform/transform_orphan_resource.go

@@ -26,9 +26,16 @@ 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
+
+	// Do not apply this transformer
+	skip bool
 }
 
 func (t *OrphanResourceInstanceTransformer) Transform(g *Graph) error {
+	if t.skip {
+		return nil
+	}
+
 	if t.State == nil {
 		// If the entire state is nil, there can't be any orphans
 		return nil

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
+	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.Destroy
+		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.Destroy,
+				Destroy:     t.removeRootOutputs,
 				RefreshOnly: t.RefreshOnly,
 			}
 		}

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 {