Merge pull request #22937 from hashicorp/jbardin/cbd-modules

create_before_destroy across modules

terraform/context_apply_test.go

@@ -10602,3 +10602,225 @@ func TestContext2Apply_invalidIndexRef(t *testing.T) {
 		t.Fatalf("missing expected error\ngot: %s\n\nwant: error containing %q", gotErr, wantErr)
 	}
 }
+
+func TestContext2Apply_moduleReplaceCycle(t *testing.T) {
+	for _, mode := range []string{"normal", "cbd"} {
+		var m *configs.Config
+
+		switch mode {
+		case "normal":
+			m = testModule(t, "apply-module-replace-cycle")
+		case "cbd":
+			m = testModule(t, "apply-module-replace-cycle-cbd")
+		}
+
+		p := testProvider("aws")
+		p.DiffFn = testDiffFn
+		p.ApplyFn = testApplyFn
+
+		instanceSchema := &configschema.Block{
+			Attributes: map[string]*configschema.Attribute{
+				"id":          {Type: cty.String, Computed: true},
+				"require_new": {Type: cty.String, Optional: true},
+			},
+		}
+
+		p.GetSchemaReturn = &ProviderSchema{
+			ResourceTypes: map[string]*configschema.Block{
+				"aws_instance": instanceSchema,
+			},
+		}
+
+		state := states.NewState()
+		modA := state.EnsureModule(addrs.RootModuleInstance.Child("a", addrs.NoKey))
+		modA.SetResourceInstanceCurrent(
+			addrs.Resource{
+				Mode: addrs.ManagedResourceMode,
+				Type: "aws_instance",
+				Name: "a",
+			}.Instance(addrs.NoKey),
+			&states.ResourceInstanceObjectSrc{
+				Status:    states.ObjectReady,
+				AttrsJSON: []byte(`{"id":"a","require_new":"old"}`),
+			},
+			addrs.ProviderConfig{
+				Type: "aws",
+			}.Absolute(addrs.RootModuleInstance),
+		)
+
+		modB := state.EnsureModule(addrs.RootModuleInstance.Child("b", addrs.NoKey))
+		modB.SetResourceInstanceCurrent(
+			addrs.Resource{
+				Mode: addrs.ManagedResourceMode,
+				Type: "aws_instance",
+				Name: "b",
+			}.Instance(addrs.IntKey(0)),
+			&states.ResourceInstanceObjectSrc{
+				Status:    states.ObjectReady,
+				AttrsJSON: []byte(`{"id":"b","require_new":"old"}`),
+			},
+			addrs.ProviderConfig{
+				Type: "aws",
+			}.Absolute(addrs.RootModuleInstance),
+		)
+
+		aBefore, _ := plans.NewDynamicValue(
+			cty.ObjectVal(map[string]cty.Value{
+				"id":          cty.StringVal("a"),
+				"require_new": cty.StringVal("old"),
+			}), instanceSchema.ImpliedType())
+		aAfter, _ := plans.NewDynamicValue(
+			cty.ObjectVal(map[string]cty.Value{
+				"id":          cty.UnknownVal(cty.String),
+				"require_new": cty.StringVal("new"),
+			}), instanceSchema.ImpliedType())
+		bBefore, _ := plans.NewDynamicValue(
+			cty.ObjectVal(map[string]cty.Value{
+				"id":          cty.StringVal("b"),
+				"require_new": cty.StringVal("old"),
+			}), instanceSchema.ImpliedType())
+		bAfter, _ := plans.NewDynamicValue(
+			cty.ObjectVal(map[string]cty.Value{
+				"id":          cty.UnknownVal(cty.String),
+				"require_new": cty.UnknownVal(cty.String),
+			}), instanceSchema.ImpliedType())
+
+		var aAction plans.Action
+		switch mode {
+		case "normal":
+			aAction = plans.DeleteThenCreate
+		case "cbd":
+			aAction = plans.CreateThenDelete
+		}
+
+		changes := &plans.Changes{
+			Resources: []*plans.ResourceInstanceChangeSrc{
+				{
+					Addr: addrs.Resource{
+						Mode: addrs.ManagedResourceMode,
+						Type: "aws_instance",
+						Name: "a",
+					}.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance.Child("a", addrs.NoKey)),
+					ProviderAddr: addrs.ProviderConfig{
+						Type: "aws",
+					}.Absolute(addrs.RootModuleInstance),
+					ChangeSrc: plans.ChangeSrc{
+						Action: aAction,
+						Before: aBefore,
+						After:  aAfter,
+					},
+				},
+				{
+					Addr: addrs.Resource{
+						Mode: addrs.ManagedResourceMode,
+						Type: "aws_instance",
+						Name: "b",
+					}.Instance(addrs.IntKey(0)).Absolute(addrs.RootModuleInstance.Child("b", addrs.NoKey)),
+					ProviderAddr: addrs.ProviderConfig{
+						Type: "aws",
+					}.Absolute(addrs.RootModuleInstance),
+					ChangeSrc: plans.ChangeSrc{
+						Action: plans.DeleteThenCreate,
+						Before: bBefore,
+						After:  bAfter,
+					},
+				},
+			},
+		}
+
+		ctx := testContext2(t, &ContextOpts{
+			Config: m,
+			ProviderResolver: providers.ResolverFixed(
+				map[string]providers.Factory{
+					"aws": testProviderFuncFixed(p),
+				},
+			),
+			State:   state,
+			Changes: changes,
+		})
+
+		t.Run(mode, func(t *testing.T) {
+			_, diags := ctx.Apply()
+			if diags.HasErrors() {
+				t.Fatal(diags.Err())
+			}
+		})
+	}
+}
+
+func TestContext2Apply_destroyDataCycle(t *testing.T) {
+	m, snap := testModuleWithSnapshot(t, "apply-destroy-data-cycle")
+	p := testProvider("null")
+	p.ApplyFn = testApplyFn
+	p.DiffFn = testDiffFn
+
+	state := states.NewState()
+	root := state.EnsureModule(addrs.RootModuleInstance)
+	root.SetResourceInstanceCurrent(
+		addrs.Resource{
+			Mode: addrs.ManagedResourceMode,
+			Type: "null_resource",
+			Name: "a",
+		}.Instance(addrs.IntKey(0)),
+		&states.ResourceInstanceObjectSrc{
+			Status:    states.ObjectReady,
+			AttrsJSON: []byte(`{"id":"a"}`),
+		},
+		addrs.ProviderConfig{
+			Type: "null",
+		}.Absolute(addrs.RootModuleInstance),
+	)
+	root.SetResourceInstanceCurrent(
+		addrs.Resource{
+			Mode: addrs.DataResourceMode,
+			Type: "null_data_source",
+			Name: "d",
+		}.Instance(addrs.NoKey),
+		&states.ResourceInstanceObjectSrc{
+			Status:    states.ObjectReady,
+			AttrsJSON: []byte(`{"id":"data"}`),
+		},
+		addrs.ProviderConfig{
+			Type: "null",
+		}.Absolute(addrs.RootModuleInstance),
+	)
+
+	providerResolver := providers.ResolverFixed(
+		map[string]providers.Factory{
+			"null": testProviderFuncFixed(p),
+		},
+	)
+
+	hook := &testHook{}
+	ctx := testContext2(t, &ContextOpts{
+		Config:           m,
+		ProviderResolver: providerResolver,
+		State:            state,
+		Destroy:          true,
+		Hooks:            []Hook{hook},
+	})
+
+	plan, diags := ctx.Plan()
+	diags.HasErrors()
+	if diags.HasErrors() {
+		t.Fatalf("diags: %s", diags.Err())
+	}
+
+	// We'll marshal and unmarshal the plan here, to ensure that we have
+	// a clean new context as would be created if we separately ran
+	// terraform plan -out=tfplan && terraform apply tfplan
+	ctxOpts, err := contextOptsForPlanViaFile(snap, state, plan)
+	if err != nil {
+		t.Fatal(err)
+	}
+	ctxOpts.ProviderResolver = providerResolver
+	ctx, diags = NewContext(ctxOpts)
+	if diags.HasErrors() {
+		t.Fatalf("failed to create context for plan: %s", diags.Err())
+	}
+
+	_, diags = ctx.Apply()
+	if diags.HasErrors() {
+		t.Fatalf("diags: %s", diags.Err())
+	}
+}

terraform/graph_builder_apply.go

@@ -127,21 +127,6 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer {
 		// Attach the state
 		&AttachStateTransformer{State: b.State},
 
-		// Destruction ordering
-		&DestroyEdgeTransformer{
-			Config:  b.Config,
-			State:   b.State,
-			Schemas: b.Schemas,
-		},
-		GraphTransformIf(
-			func() bool { return !b.Destroy },
-			&CBDEdgeTransformer{
-				Config:  b.Config,
-				State:   b.State,
-				Schemas: b.Schemas,
-			},
-		),
-
 		// Provisioner-related transformations
 		&MissingProvisionerTransformer{Provisioners: b.Components.ResourceProvisioners()},
 		&ProvisionerTransformer{},
@@ -171,6 +156,20 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer {
 		// Connect references so ordering is correct
 		&ReferenceTransformer{},
 
+		// Destruction ordering
+		&DestroyEdgeTransformer{
+			Config:  b.Config,
+			State:   b.State,
+			Schemas: b.Schemas,
+		},
+
+		&CBDEdgeTransformer{
+			Config:  b.Config,
+			State:   b.State,
+			Schemas: b.Schemas,
+			Destroy: b.Destroy,
+		},
+
 		// Handle destroy time transformations for output and local values.
 		// Reverse the edges from outputs and locals, so that
 		// interpolations don't fail during destroy.

terraform/graph_builder_apply_test.go

@@ -522,8 +522,9 @@ root
 test_object.A (prepare state)
   provider.test
 test_object.A[1] (destroy)
-  test_object.A (prepare state)
+  provider.test
 test_object.B
+  test_object.A (prepare state)
   test_object.A[1] (destroy)
   test_object.B (prepare state)
 test_object.B (prepare state)

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

@@ -0,0 +1,10 @@
+locals {
+    l = data.null_data_source.d.id
+}
+
+data "null_data_source" "d" {
+}
+
+resource "null_resource" "a" {
+    count = local.l == "NONE" ? 1 : 0
+}

terraform/testdata/apply-module-replace-cycle-cbd/main.tf

@@ -0,0 +1,8 @@
+module "a" {
+  source = "./mod1"
+}
+
+module "b" {
+  source = "./mod2"
+  ids = module.a.ids
+}

terraform/testdata/apply-module-replace-cycle-cbd/mod1/main.tf

@@ -0,0 +1,10 @@
+resource "aws_instance" "a" {
+  require_new = "new"
+  lifecycle {
+    create_before_destroy = true
+  }
+}
+
+output "ids" {
+  value = [aws_instance.a.id]
+}

terraform/testdata/apply-module-replace-cycle-cbd/mod2/main.tf

@@ -0,0 +1,8 @@
+resource "aws_instance" "b" {
+  count    = length(var.ids)
+  require_new = var.ids[count.index]
+}
+
+variable "ids" {
+  type = list(string)
+}

terraform/testdata/apply-module-replace-cycle/main.tf

@@ -0,0 +1,8 @@
+module "a" {
+  source = "./mod1"
+}
+
+module "b" {
+  source = "./mod2"
+  ids = module.a.ids
+}

terraform/testdata/apply-module-replace-cycle/mod1/main.tf

@@ -0,0 +1,10 @@
+resource "aws_instance" "a" {
+  require_new = "new"
+  lifecycle {
+    create_before_destroy = true
+  }
+}
+
+output "ids" {
+  value = [aws_instance.a.id]
+}

terraform/testdata/apply-module-replace-cycle/mod2/main.tf

@@ -0,0 +1,8 @@
+resource "aws_instance" "b" {
+  count    = length(var.ids)
+  require_new = var.ids[count.index]
+}
+
+variable "ids" {
+  type = list(string)
+}

terraform/testdata/transform-cbd-destroy-edge-both-count/main.tf

@@ -0,0 +1,11 @@
+resource "test_object" "A" {
+  count = 2
+  lifecycle {
+    create_before_destroy = true
+  }
+}
+
+resource "test_object" "B" {
+  count       = 2
+  test_string = test_object.A[*].test_string[count.index]
+}

terraform/testdata/transform-cbd-destroy-edge-count/main.tf

@@ -0,0 +1,10 @@
+resource "test_object" "A" {
+  lifecycle {
+    create_before_destroy = true
+  }
+}
+
+resource "test_object" "B" {
+  count       = 2
+  test_string = test_object.A.test_string
+}

terraform/testdata/transform-destroy-cbd-edge-basic/main.tf

@@ -0,0 +1,9 @@
+resource "test_object" "A" {
+  lifecycle {
+    create_before_destroy = true
+  }
+}
+
+resource "test_object" "B" {
+  test_string = "${test_object.A.id}"
+}

terraform/transform_config_flat.go

@@ -1,71 +0,0 @@
-package terraform
-
-import (
-	"github.com/hashicorp/terraform/configs"
-	"github.com/hashicorp/terraform/dag"
-)
-
-// FlatConfigTransformer is a GraphTransformer that adds the configuration
-// to the graph. The module used to configure this transformer must be
-// the root module.
-//
-// This transform adds the nodes but doesn't connect any of the references.
-// The ReferenceTransformer should be used for that.
-//
-// NOTE: In relation to ConfigTransformer: this is a newer generation config
-// transformer. It puts the _entire_ config into the graph (there is no
-// "flattening" step as before).
-type FlatConfigTransformer struct {
-	Concrete ConcreteResourceNodeFunc // What to turn resources into
-
-	Config *configs.Config
-}
-
-func (t *FlatConfigTransformer) Transform(g *Graph) error {
-	// We have nothing to do if there is no configuration.
-	if t.Config == nil {
-		return nil
-	}
-
-	return t.transform(g, t.Config)
-}
-
-func (t *FlatConfigTransformer) transform(g *Graph, config *configs.Config) error {
-	// If we have no configuration then there's nothing to do.
-	if config == nil {
-		return nil
-	}
-
-	// Transform all the children.
-	for _, c := range config.Children {
-		if err := t.transform(g, c); err != nil {
-			return err
-		}
-	}
-
-	module := config.Module
-	// For now we assume that each module call produces only one module
-	// instance with no key, since we don't yet support "count" and "for_each"
-	// on modules.
-	// FIXME: As part of supporting "count" and "for_each" on modules, rework
-	// this so that we'll "expand" the module call first and then create graph
-	// nodes for each module instance separately.
-	instPath := config.Path.UnkeyedInstanceShim()
-
-	for _, r := range module.ManagedResources {
-		addr := r.Addr().Absolute(instPath)
-		abstract := &NodeAbstractResource{
-			Addr:   addr,
-			Config: r,
-		}
-		// Grab the address for this resource
-		var node dag.Vertex = abstract
-		if f := t.Concrete; f != nil {
-			node = f(abstract)
-		}
-
-		g.Add(node)
-	}
-
-	return nil
-}

terraform/transform_config_flat_test.go

@@ -1,42 +0,0 @@
-package terraform
-
-import (
-	"strings"
-	"testing"
-
-	"github.com/hashicorp/terraform/addrs"
-)
-
-func TestFlatConfigTransformer_nilModule(t *testing.T) {
-	g := Graph{Path: addrs.RootModuleInstance}
-	tf := &FlatConfigTransformer{}
-	if err := tf.Transform(&g); err != nil {
-		t.Fatalf("err: %s", err)
-	}
-
-	if len(g.Vertices()) > 0 {
-		t.Fatal("graph should be empty")
-	}
-}
-
-func TestFlatConfigTransformer(t *testing.T) {
-	g := Graph{Path: addrs.RootModuleInstance}
-	tf := &FlatConfigTransformer{
-		Config: testModule(t, "transform-flat-config-basic"),
-	}
-	if err := tf.Transform(&g); err != nil {
-		t.Fatalf("err: %s", err)
-	}
-
-	actual := strings.TrimSpace(g.String())
-	expected := strings.TrimSpace(testTransformFlatConfigBasicStr)
-	if actual != expected {
-		t.Fatalf("bad:\n\n%s", actual)
-	}
-}
-
-const testTransformFlatConfigBasicStr = `
-aws_instance.bar
-aws_instance.foo
-module.child.aws_instance.baz
-`

terraform/transform_destroy_cbd.go

@@ -134,9 +134,16 @@ type CBDEdgeTransformer struct {
 	// obtain schema information from providers and provisioners so we can
 	// properly resolve implicit dependencies.
 	Schemas *Schemas
+
+	// If the operation is a simple destroy, no transformation is done.
+	Destroy bool
 }
 
 func (t *CBDEdgeTransformer) Transform(g *Graph) error {
+	if t.Destroy {
+		return nil
+	}
+
 	// Go through and reverse any destroy edges
 	destroyMap := make(map[string][]dag.Vertex)
 	for _, v := range g.Vertices() {
@@ -169,6 +176,7 @@ func (t *CBDEdgeTransformer) Transform(g *Graph) error {
 			applyNode := de.Source()
 			destroyNode := de.Target()
 			g.Connect(&DestroyEdge{S: destroyNode, T: applyNode})
+			break
 		}
 
 		// If the address has an index, we strip that. Our depMap creation
@@ -201,12 +209,7 @@ func (t *CBDEdgeTransformer) Transform(g *Graph) error {
 	// They key here is that B happens before A is destroyed. This is to
 	// facilitate the primary purpose for CBD: making sure that downstreams
 	// are properly updated to avoid downtime before the resource is destroyed.
-	//
-	// We can't trust that the resource being destroyed or anything that
-	// depends on it is actually in our current graph so we make a new
-	// graph in order to determine those dependencies and add them in.
-	log.Printf("[TRACE] CBDEdgeTransformer: building graph to find dependencies...")
-	depMap, err := t.depMap(destroyMap)
+	depMap, err := t.depMap(g, destroyMap)
 	if err != nil {
 		return err
 	}
@@ -248,26 +251,10 @@ func (t *CBDEdgeTransformer) Transform(g *Graph) error {
 	return nil
 }
 
-func (t *CBDEdgeTransformer) depMap(destroyMap map[string][]dag.Vertex) (map[string][]dag.Vertex, error) {
-	// Build the graph of our config, this ensures that all resources
-	// are present in the graph.
-	g, diags := (&BasicGraphBuilder{
-		Steps: []GraphTransformer{
-			&FlatConfigTransformer{Config: t.Config},
-			&AttachResourceConfigTransformer{Config: t.Config},
-			&AttachStateTransformer{State: t.State},
-			&AttachSchemaTransformer{Schemas: t.Schemas},
-			&ReferenceTransformer{},
-		},
-		Name: "CBDEdgeTransformer",
-	}).Build(nil)
-	if diags.HasErrors() {
-		return nil, diags.Err()
-	}
-
-	// Using this graph, build the list of destroy nodes that each resource
-	// address should depend on. For example, when we find B, we map the
-	// address of B to A_d in the "depMap" variable below.
+func (t *CBDEdgeTransformer) depMap(g *Graph, destroyMap map[string][]dag.Vertex) (map[string][]dag.Vertex, error) {
+	// Build the list of destroy nodes that each resource address should depend
+	// on. For example, when we find B, we map the address of B to A_d in the
+	// "depMap" variable below.
 	depMap := make(map[string][]dag.Vertex)
 	for _, v := range g.Vertices() {
 		// We're looking for resources.
@@ -289,8 +276,10 @@ func (t *CBDEdgeTransformer) depMap(destroyMap map[string][]dag.Vertex) (map[str
 		}
 
 		// Get the nodes that depend on this on. In the example above:
-		// finding B in A => B.
-		for _, v := range g.UpEdges(v).List() {
+		// finding B in A => B. Since dependencies can span modules, walk all
+		// descendents of the resource.
+		des, _ := g.Descendents(v)
+		for _, v := range des.List() {
 			// We're looking for resources.
 			rn, ok := v.(GraphNodeResource)
 			if !ok {

terraform/transform_destroy_cbd_test.go

@@ -1,198 +1,198 @@
 package terraform
 
 import (
+	"regexp"
 	"strings"
 	"testing"
 
 	"github.com/hashicorp/terraform/addrs"
+	"github.com/hashicorp/terraform/plans"
 )
 
+func cbdTestGraph(t *testing.T, mod string, changes *plans.Changes) *Graph {
+	module := testModule(t, mod)
+
+	applyBuilder := &ApplyGraphBuilder{
+		Config:     module,
+		Changes:    changes,
+		Components: simpleMockComponentFactory(),
+		Schemas:    simpleTestSchemas(),
+	}
+	g, err := (&BasicGraphBuilder{
+		Steps: cbdTestSteps(applyBuilder.Steps()),
+		Name:  "ApplyGraphBuilder",
+	}).Build(addrs.RootModuleInstance)
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	return filterInstances(g)
+}
+
+// override the apply graph builder to halt the process after CBD
+func cbdTestSteps(steps []GraphTransformer) []GraphTransformer {
+	found := false
+	var i int
+	var t GraphTransformer
+	for i, t = range steps {
+		if _, ok := t.(*CBDEdgeTransformer); ok {
+			found = true
+			break
+		}
+	}
+
+	if !found {
+		panic("CBDEdgeTransformer not found")
+	}
+
+	return steps[:i+1]
+}
+
+// remove extra nodes for easier test comparisons
+func filterInstances(g *Graph) *Graph {
+	for _, v := range g.Vertices() {
+		if _, ok := v.(GraphNodeResourceInstance); !ok {
+			g.Remove(v)
+		}
+
+	}
+	return g
+}
+
 func TestCBDEdgeTransformer(t *testing.T) {
-	g := Graph{Path: addrs.RootModuleInstance}
-	g.Add(&graphNodeCreatorTest{AddrString: "test_object.A"})
-	g.Add(&graphNodeCreatorTest{AddrString: "test_object.B"})
-	g.Add(&graphNodeDestroyerTest{AddrString: "test_object.A", CBD: true})
-
-	module := testModule(t, "transform-destroy-edge-basic")
-
-	{
-		tf := &DestroyEdgeTransformer{
-			Config:  module,
-			Schemas: simpleTestSchemas(),
-		}
-		if err := tf.Transform(&g); err != nil {
-			t.Fatalf("err: %s", err)
-		}
+	changes := &plans.Changes{
+		Resources: []*plans.ResourceInstanceChangeSrc{
+			{
+				Addr: mustResourceInstanceAddr("test_object.A"),
+				ChangeSrc: plans.ChangeSrc{
+					Action: plans.CreateThenDelete,
+				},
+			},
+			{
+				Addr: mustResourceInstanceAddr("test_object.B"),
+				ChangeSrc: plans.ChangeSrc{
+					Action: plans.Update,
+				},
+			},
+		},
 	}
 
-	{
-		tf := &CBDEdgeTransformer{
-			Config:  module,
-			Schemas: simpleTestSchemas(),
-		}
-		if err := tf.Transform(&g); err != nil {
-			t.Fatalf("err: %s", err)
-		}
-	}
+	g := cbdTestGraph(t, "transform-destroy-cbd-edge-basic", changes)
+	g = filterInstances(g)
 
 	actual := strings.TrimSpace(g.String())
-	expected := strings.TrimSpace(testTransformCBDEdgeBasicStr)
-	if actual != expected {
+	expected := regexp.MustCompile(strings.TrimSpace(`
+(?m)test_object.A
+test_object.A \(destroy deposed \w+\)
+  test_object.A
+  test_object.B
+test_object.B
+  test_object.A
+`))
+
+	if !expected.MatchString(actual) {
 		t.Fatalf("wrong result\n\ngot:\n%s\n\nwant:\n%s", actual, expected)
 	}
 }
 
-// FIXME: see if there is a worthwhile test to create from this.
-// CBD is marked on created nodes during the plan phase now, and the
-// CBDEdgeTransformer only takes care of the final edge reversal.
-/*
-func TestCBDEdgeTransformer_depNonCBD(t *testing.T) {
-	g := Graph{Path: addrs.RootModuleInstance}
-	g.Add(&graphNodeCreatorTest{AddrString: "test_object.A"})
-	g.Add(&graphNodeCreatorTest{AddrString: "test_object.B"})
-	g.Add(&graphNodeDestroyerTest{AddrString: "test_object.A"})
-	g.Add(&graphNodeDestroyerTest{AddrString: "test_object.B", CBD: true})
-
-	module := testModule(t, "transform-destroy-edge-basic")
-
-	{
-		tf := &DestroyEdgeTransformer{
-			Config:  module,
-			Schemas: simpleTestSchemas(),
-		}
-		if err := tf.Transform(&g); err != nil {
-			t.Fatalf("err: %s", err)
-		}
-	}
-
-	{
-		tf := &CBDEdgeTransformer{
-			Config:  module,
-			Schemas: simpleTestSchemas(),
-		}
-		if err := tf.Transform(&g); err != nil {
-			t.Fatalf("err: %s", err)
-		}
-	}
-
-	actual := strings.TrimSpace(g.String())
-	expected := strings.TrimSpace(testTransformCBDEdgeDepNonCBDStr)
-	if actual != expected {
-		t.Fatalf("wrong result\n\ngot:\n%s\n\nwant:\n%s", actual, expected)
-	}
-}
-*/
-
 func TestCBDEdgeTransformer_depNonCBDCount(t *testing.T) {
-	g := Graph{Path: addrs.RootModuleInstance}
-	g.Add(&graphNodeCreatorTest{AddrString: "test_object.A"})
-	g.Add(&graphNodeCreatorTest{AddrString: "test_object.B[0]"})
-	g.Add(&graphNodeCreatorTest{AddrString: "test_object.B[1]"})
-	g.Add(&graphNodeDestroyerTest{AddrString: "test_object.A", CBD: true})
-
-	module := testModule(t, "transform-destroy-edge-splat")
-
-	{
-		tf := &DestroyEdgeTransformer{
-			Config:  module,
-			Schemas: simpleTestSchemas(),
-		}
-		if err := tf.Transform(&g); err != nil {
-			t.Fatalf("err: %s", err)
-		}
+	changes := &plans.Changes{
+		Resources: []*plans.ResourceInstanceChangeSrc{
+			{
+				Addr: mustResourceInstanceAddr("test_object.A"),
+				ChangeSrc: plans.ChangeSrc{
+					Action: plans.CreateThenDelete,
+				},
+			},
+			{
+				Addr: mustResourceInstanceAddr("test_object.B[0]"),
+				ChangeSrc: plans.ChangeSrc{
+					Action: plans.Update,
+				},
+			},
+			{
+				Addr: mustResourceInstanceAddr("test_object.B[1]"),
+				ChangeSrc: plans.ChangeSrc{
+					Action: plans.Update,
+				},
+			},
+		},
 	}
 
-	{
-		tf := &CBDEdgeTransformer{
-			Config:  module,
-			Schemas: simpleTestSchemas(),
-		}
-		if err := tf.Transform(&g); err != nil {
-			t.Fatalf("err: %s", err)
-		}
-	}
+	g := cbdTestGraph(t, "transform-cbd-destroy-edge-count", changes)
 
 	actual := strings.TrimSpace(g.String())
-	expected := strings.TrimSpace(`
-test_object.A
-test_object.A (destroy)
+	expected := regexp.MustCompile(strings.TrimSpace(`
+(?m)test_object.A
+test_object.A \(destroy deposed \w+\)
   test_object.A
-  test_object.B[0]
-  test_object.B[1]
-test_object.B[0]
-test_object.B[1]
-	`)
-	if actual != expected {
+  test_object.B\[0\]
+  test_object.B\[1\]
+test_object.B\[0\]
+  test_object.A
+test_object.B\[1\]
+  test_object.A`))
+
+	if !expected.MatchString(actual) {
 		t.Fatalf("wrong result\n\ngot:\n%s\n\nwant:\n%s", actual, expected)
 	}
 }
 
 func TestCBDEdgeTransformer_depNonCBDCountBoth(t *testing.T) {
-	g := Graph{Path: addrs.RootModuleInstance}
-	g.Add(&graphNodeCreatorTest{AddrString: "test_object.A[0]"})
-	g.Add(&graphNodeCreatorTest{AddrString: "test_object.A[1]"})
-	g.Add(&graphNodeCreatorTest{AddrString: "test_object.B[0]"})
-	g.Add(&graphNodeCreatorTest{AddrString: "test_object.B[1]"})
-	g.Add(&graphNodeDestroyerTest{AddrString: "test_object.A[0]", CBD: true})
-	g.Add(&graphNodeDestroyerTest{AddrString: "test_object.A[1]", CBD: true})
-
-	module := testModule(t, "transform-destroy-edge-splat")
-
-	{
-		tf := &DestroyEdgeTransformer{
-			Config:  module,
-			Schemas: simpleTestSchemas(),
-		}
-		if err := tf.Transform(&g); err != nil {
-			t.Fatalf("err: %s", err)
-		}
+	changes := &plans.Changes{
+		Resources: []*plans.ResourceInstanceChangeSrc{
+			{
+				Addr: mustResourceInstanceAddr("test_object.A[0]"),
+				ChangeSrc: plans.ChangeSrc{
+					Action: plans.CreateThenDelete,
+				},
+			},
+			{
+				Addr: mustResourceInstanceAddr("test_object.A[1]"),
+				ChangeSrc: plans.ChangeSrc{
+					Action: plans.CreateThenDelete,
+				},
+			},
+			{
+				Addr: mustResourceInstanceAddr("test_object.B[0]"),
+				ChangeSrc: plans.ChangeSrc{
+					Action: plans.Update,
+				},
+			},
+			{
+				Addr: mustResourceInstanceAddr("test_object.B[1]"),
+				ChangeSrc: plans.ChangeSrc{
+					Action: plans.Update,
+				},
+			},
+		},
 	}
 
-	{
-		tf := &CBDEdgeTransformer{
-			Config:  module,
-			Schemas: simpleTestSchemas(),
-		}
-		if err := tf.Transform(&g); err != nil {
-			t.Fatalf("err: %s", err)
-		}
-	}
+	g := cbdTestGraph(t, "transform-cbd-destroy-edge-both-count", changes)
 
 	actual := strings.TrimSpace(g.String())
-	expected := strings.TrimSpace(`
-test_object.A[0]
-test_object.A[0] (destroy)
-  test_object.A[0]
-  test_object.B[0]
-  test_object.B[1]
-test_object.A[1]
-test_object.A[1] (destroy)
-  test_object.A[1]
-  test_object.B[0]
-  test_object.B[1]
-test_object.B[0]
-test_object.B[1]
-	`)
-	if actual != expected {
+	expected := regexp.MustCompile(strings.TrimSpace(`
+test_object.A \(destroy deposed \w+\)
+  test_object.A\[0\]
+  test_object.A\[1\]
+  test_object.B\[0\]
+  test_object.B\[1\]
+test_object.A \(destroy deposed \w+\)
+  test_object.A\[0\]
+  test_object.A\[1\]
+  test_object.B\[0\]
+  test_object.B\[1\]
+test_object.A\[0\]
+test_object.A\[1\]
+test_object.B\[0\]
+  test_object.A\[0\]
+  test_object.A\[1\]
+test_object.B\[1\]
+  test_object.A\[0\]
+  test_object.A\[1\]
+`))
+
+	if !expected.MatchString(actual) {
 		t.Fatalf("wrong result\n\ngot:\n%s\n\nwant:\n%s", actual, expected)
 	}
 }
-
-const testTransformCBDEdgeBasicStr = `
-test_object.A
-test_object.A (destroy)
-  test_object.A
-  test_object.B
-test_object.B
-`
-
-const testTransformCBDEdgeDepNonCBDStr = `
-test_object.A
-test_object.A (destroy) (modified)
-  test_object.A
-  test_object.B
-  test_object.B (destroy)
-test_object.B
-test_object.B (destroy)
-  test_object.B
-`

terraform/transform_destroy_edge.go

@@ -277,5 +277,47 @@ func (t *DestroyEdgeTransformer) Transform(g *Graph) error {
 		}
 	}
 
+	return t.pruneResources(g)
+}
+
+// If there are only destroy instances for a particular resource, there's no
+// reason for the resource node to prepare the state. Remove Resource nodes so
+// that they don't fail by trying to evaluate a resource that is only being
+// destroyed along with its dependencies.
+func (t *DestroyEdgeTransformer) pruneResources(g *Graph) error {
+	for _, v := range g.Vertices() {
+		n, ok := v.(*NodeApplyableResource)
+		if !ok {
+			continue
+		}
+
+		// if there are only destroy dependencies, we don't need this node
+		des, err := g.Descendents(n)
+		if err != nil {
+			return err
+		}
+
+		descendents := des.List()
+		nonDestroyInstanceFound := false
+		for _, v := range descendents {
+			if _, ok := v.(*NodeApplyableResourceInstance); ok {
+				nonDestroyInstanceFound = true
+				break
+			}
+		}
+
+		if nonDestroyInstanceFound {
+			continue
+		}
+
+		// connect all the through-edges, then delete the node
+		for _, d := range g.DownEdges(n).List() {
+			for _, u := range g.UpEdges(n).List() {
+				g.Connect(dag.BasicEdge(u, d))
+			}
+		}
+		log.Printf("DestroyEdgeTransformer: pruning unused resource node %s", dag.VertexName(n))
+		g.Remove(n)
+	}
 	return nil
 }

terraform/transform_diff.go

@@ -174,14 +174,6 @@ func (t *DiffTransformer) Transform(g *Graph) error {
 				log.Printf("[TRACE] DiffTransformer: %s deposed object %s will be represented for destruction by %s", addr, dk, dag.VertexName(node))
 			}
 			g.Add(node)
-			rsrcAddr := addr.ContainingResource().String()
-			for _, rsrcNode := range resourceNodes[rsrcAddr] {
-				// We connect this edge "forwards" (even though destroy dependencies
-				// are often inverted) because evaluating the resource node
-				// after the destroy node could cause an unnecessary husk of
-				// a resource state to be re-added.
-				g.Connect(dag.BasicEdge(node, rsrcNode))
-			}
 		}
 
 	}