terraform: don't include variables in destroy node requirements

terraform/context_test.go

@@ -1155,6 +1155,58 @@ func TestContext2Plan_moduleDestroy(t *testing.T) {
 	}
 }
 
+// GH-1835
+func TestContext2Plan_moduleDestroyCycle(t *testing.T) {
+	m := testModule(t, "plan-module-destroy-gh-1835")
+	p := testProvider("aws")
+	p.DiffFn = testDiffFn
+	s := &State{
+		Modules: []*ModuleState{
+			&ModuleState{
+				Path: []string{"root", "a_module"},
+				Resources: map[string]*ResourceState{
+					"aws_instance.a": &ResourceState{
+						Type: "aws_instance",
+						Primary: &InstanceState{
+							ID: "a",
+						},
+					},
+				},
+			},
+			&ModuleState{
+				Path: []string{"root", "b_module"},
+				Resources: map[string]*ResourceState{
+					"aws_instance.b": &ResourceState{
+						Type: "aws_instance",
+						Primary: &InstanceState{
+							ID: "b",
+						},
+					},
+				},
+			},
+		},
+	}
+	ctx := testContext2(t, &ContextOpts{
+		Module: m,
+		Providers: map[string]ResourceProviderFactory{
+			"aws": testProviderFuncFixed(p),
+		},
+		State:   s,
+		Destroy: true,
+	})
+
+	plan, err := ctx.Plan()
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	actual := strings.TrimSpace(plan.String())
+	expected := strings.TrimSpace(testTerraformPlanModuleDestroyCycleStr)
+	if actual != expected {
+		t.Fatalf("bad:\n%s", actual)
+	}
+}
+
 func TestContext2Plan_moduleDestroyMultivar(t *testing.T) {
 	m := testModule(t, "plan-module-destroy-multivar")
 	p := testProvider("aws")

terraform/diff.go

@@ -80,7 +80,18 @@ func (d *Diff) Empty() bool {
 
 func (d *Diff) String() string {
 	var buf bytes.Buffer
+
+	keys := make([]string, 0, len(d.Modules))
+	lookup := make(map[string]*ModuleDiff)
 	for _, m := range d.Modules {
+		key := fmt.Sprintf("module.%s", strings.Join(m.Path[1:], "."))
+		keys = append(keys, key)
+		lookup[key] = m
+	}
+	sort.Strings(keys)
+
+	for _, key := range keys {
+		m := lookup[key]
 		mStr := m.String()
 
 		// If we're the root module, we just write the output directly.
@@ -89,7 +100,7 @@ func (d *Diff) String() string {
 			continue
 		}
 
-		buf.WriteString(fmt.Sprintf("module.%s:\n", strings.Join(m.Path[1:], ".")))
+		buf.WriteString(fmt.Sprintf("%s:\n", key))
 
 		s := bufio.NewScanner(strings.NewReader(mStr))
 		for s.Scan() {

terraform/graph_config_node_variable.go

@@ -55,6 +55,14 @@ func (n *GraphNodeConfigVariable) VariableName() string {
 	return n.Variable.Name
 }
 
+// GraphNodeDestroyEdgeInclude impl.
+func (n *GraphNodeConfigVariable) DestroyEdgeInclude() bool {
+	// Don't include variables as dependencies in destroy nodes.
+	// Destroy nodes don't interpolate anyways and this has a possibility
+	// to create cycles. See GH-1835
+	return false
+}
+
 // GraphNodeProxy impl.
 func (n *GraphNodeConfigVariable) Proxy() bool {
 	return true

terraform/terraform_test.go

@@ -996,6 +996,26 @@ module.child:
     ID = bar
 `
 
+const testTerraformPlanModuleDestroyCycleStr = `
+DIFF:
+
+module.a_module:
+  DESTROY MODULE
+  DESTROY: aws_instance.a
+module.b_module:
+  DESTROY MODULE
+  DESTROY: aws_instance.b
+
+STATE:
+
+module.a_module:
+  aws_instance.a:
+    ID = a
+module.b_module:
+  aws_instance.b:
+    ID = b
+`
+
 const testTerraformPlanModuleDestroyMultivarStr = `
 DIFF:
 

terraform/test-fixtures/plan-module-destroy-gh-1835/a/main.tf

@@ -0,0 +1,5 @@
+resource "aws_instance" "a" {}
+
+output "a_output" {
+  value = "${aws_instance.a.id}"
+}

terraform/test-fixtures/plan-module-destroy-gh-1835/b/main.tf

@@ -0,0 +1,5 @@
+variable "a_id" {}
+
+resource "aws_instance" "b" {
+  command = "echo ${var.a_id}"
+}

terraform/test-fixtures/plan-module-destroy-gh-1835/main.tf

@@ -0,0 +1,8 @@
+module "a_module" {
+  source = "./a"
+}
+
+module "b_module" {
+  source = "./b"
+  a_id = "${module.a_module.a_output}"
+}