diff --git a/terraform/context_test.go b/terraform/context_test.go index f72e602c4..15bb62778 100644 --- a/terraform/context_test.go +++ b/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") diff --git a/terraform/diff.go b/terraform/diff.go index 96b8c654a..7cd907422 100644 --- a/terraform/diff.go +++ b/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() { diff --git a/terraform/graph_config_node_variable.go b/terraform/graph_config_node_variable.go index 54c1c8343..a3a7e77c8 100644 --- a/terraform/graph_config_node_variable.go +++ b/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 diff --git a/terraform/terraform_test.go b/terraform/terraform_test.go index 878078668..e30d27a13 100644 --- a/terraform/terraform_test.go +++ b/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: diff --git a/terraform/test-fixtures/plan-module-destroy-gh-1835/a/main.tf b/terraform/test-fixtures/plan-module-destroy-gh-1835/a/main.tf new file mode 100644 index 000000000..ca44c757d --- /dev/null +++ b/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}" +} diff --git a/terraform/test-fixtures/plan-module-destroy-gh-1835/b/main.tf b/terraform/test-fixtures/plan-module-destroy-gh-1835/b/main.tf new file mode 100644 index 000000000..c3b0270b0 --- /dev/null +++ b/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}" +} diff --git a/terraform/test-fixtures/plan-module-destroy-gh-1835/main.tf b/terraform/test-fixtures/plan-module-destroy-gh-1835/main.tf new file mode 100644 index 000000000..c2f72c45e --- /dev/null +++ b/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}" +}