Merge pull request #9853 from hashicorp/jbardin/cbd-datasource

fix CreateBeforeDestroy with datasources

terraform/context_plan_test.go

@@ -2703,3 +2703,39 @@ func TestContext2Plan_moduleVariableFromSplat(t *testing.T) {
 		t.Fatalf("bad:\n%s\n\nexpected\n\n%s", actual, expected)
 	}
 }
+
+func TestContext2Plan_createBeforeDestroy_depends_datasource(t *testing.T) {
+	m := testModule(t, "plan-cdb-depends-datasource")
+	p := testProvider("aws")
+	p.DiffFn = testDiffFn
+	ctx := testContext2(t, &ContextOpts{
+		Module: m,
+		Providers: map[string]ResourceProviderFactory{
+			"aws": testProviderFuncFixed(p),
+		},
+	})
+
+	plan, err := ctx.Plan()
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	if got := len(plan.Diff.Modules); got != 1 {
+		t.Fatalf("got %d modules; want 1", got)
+	}
+
+	moduleDiff := plan.Diff.Modules[0]
+
+	if _, ok := moduleDiff.Resources["aws_instance.foo.0"]; !ok {
+		t.Fatalf("missing diff for aws_instance.foo.0")
+	}
+	if _, ok := moduleDiff.Resources["aws_instance.foo.1"]; !ok {
+		t.Fatalf("missing diff for aws_instance.foo.1")
+	}
+	if _, ok := moduleDiff.Resources["data.aws_vpc.bar.0"]; !ok {
+		t.Fatalf("missing diff for data.aws_vpc.bar.0")
+	}
+	if _, ok := moduleDiff.Resources["data.aws_vpc.bar.1"]; !ok {
+		t.Fatalf("missing diff for data.aws_vpc.bar.1")
+	}
+}

terraform/graph_builder_test.go

@@ -173,6 +173,7 @@ func TestBuiltinGraphBuilder_cbdDepNonCbd(t *testing.T) {
 	}
 }
 
+// This now returns no errors due to a general fix while building the graph
 func TestBuiltinGraphBuilder_cbdDepNonCbd_errorsWhenVerbose(t *testing.T) {
 	b := &BuiltinGraphBuilder{
 		Root:     testModule(t, "graph-builder-cbd-non-cbd"),
@@ -181,8 +182,8 @@ func TestBuiltinGraphBuilder_cbdDepNonCbd_errorsWhenVerbose(t *testing.T) {
 	}
 
 	_, err := b.Build(RootModulePath)
-	if err == nil {
-		t.Fatalf("expected err, got none")
+	if err != nil {
+		t.Fatalf("err: %s", err)
 	}
 }
 

terraform/test-fixtures/plan-cdb-depends-datasource/main.tf

@@ -0,0 +1,11 @@
+resource "aws_instance" "foo" {
+  count = 2
+  num     = "2"
+  compute = "${element(data.aws_vpc.bar.*.id, count.index)}"
+  lifecycle { create_before_destroy = true }
+}
+
+data "aws_vpc" "bar" {
+  count = 2
+  foo = "${count.index}"
+}

terraform/transform_destroy.go

@@ -1,8 +1,6 @@
 package terraform
 
-import (
-	"github.com/hashicorp/terraform/dag"
-)
+import "github.com/hashicorp/terraform/dag"
 
 // GraphNodeDestroyable is the interface that nodes that can be destroyed
 // must implement. This is used to automatically handle the creation of
@@ -153,7 +151,7 @@ func (t *CreateBeforeDestroyTransformer) Transform(g *Graph) error {
 		}
 
 		// If the node doesn't need to create before destroy, then continue
-		if !dn.CreateBeforeDestroy() {
+		if !dn.CreateBeforeDestroy() && noCreateBeforeDestroyAncestors(g, dn) {
 			continue
 		}
 
@@ -200,6 +198,30 @@ func (t *CreateBeforeDestroyTransformer) Transform(g *Graph) error {
 	return nil
 }
 
+// noCreateBeforeDestroyAncestors verifies that a vertex has no ancestors that
+// are CreateBeforeDestroy.
+// If this vertex has an ancestor with CreateBeforeDestroy, we will need to
+// inherit that behavior and re-order the edges even if this node type doesn't
+// directly implement CreateBeforeDestroy.
+func noCreateBeforeDestroyAncestors(g *Graph, v dag.Vertex) bool {
+	s, _ := g.Ancestors(v)
+	if s == nil {
+		return true
+	}
+	for _, v := range s.List() {
+		dn, ok := v.(GraphNodeDestroy)
+		if !ok {
+			continue
+		}
+
+		if dn.CreateBeforeDestroy() {
+			// some ancestor is CreateBeforeDestroy, so we need to follow suit
+			return false
+		}
+	}
+	return true
+}
+
 // PruneDestroyTransformer is a GraphTransformer that removes the destroy
 // nodes that aren't in the diff.
 type PruneDestroyTransformer struct {