From 559f017ebb7649d4db425f9f2412c1463a36f97d Mon Sep 17 00:00:00 2001
From: Paul Hinze <phinze@phinze.com>
Date: Tue, 10 May 2016 15:25:58 -0500
Subject: [PATCH] terraform: Correct fix for destroy interp errors

The fix that landed in #6557 was unfortunately the wrong subset of the
work I had been doing locally, and users of the attached bugs are still
reporting problems with Terraform v0.6.16.

At the very last step, I attempted to scope down both the failing test
and the implementation to their bare essentials, but ended up with a
test that did not exercise the root of the problem and a subset of the
implementation that was insufficient for a full bugfix.

The key thing I removed from the test was a _referencing output_ for the
module, which is what breaks down the #6557 solution.

I've re-tested the examples in #5440 and #3268 to verify this solution
does indeed solve the problem.
---
 terraform/context_apply_test.go                   | 15 ++++++---------
 terraform/graph_config_node_variable.go           | 10 ++++++++++
 .../apply-destroy-module-with-attrs/child/main.tf |  4 ++++
 .../apply-destroy-module-with-attrs/main.tf       |  6 +++++-
 4 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go
index fe030b2c6..ca172ca1f 100644
--- a/terraform/context_apply_test.go
+++ b/terraform/context_apply_test.go
@@ -2549,19 +2549,16 @@ func TestContext2Apply_destroyModuleWithAttrsReferencingResource(t *testing.T) {
 			Providers: map[string]ResourceProviderFactory{
 				"aws": testProviderFuncFixed(p),
 			},
-			Variables: map[string]string{
-				"key_name": "foobarkey",
-			},
 		})
 
 		// First plan and apply a create operation
 		if _, err := ctx.Plan(); err != nil {
-			t.Fatalf("err: %s", err)
+			t.Fatalf("plan err: %s", err)
 		}
 
 		state, err = ctx.Apply()
 		if err != nil {
-			t.Fatalf("err: %s", err)
+			t.Fatalf("apply err: %s", err)
 		}
 	}
 
@@ -2585,17 +2582,17 @@ func TestContext2Apply_destroyModuleWithAttrsReferencingResource(t *testing.T) {
 		// First plan and apply a create operation
 		plan, err := ctx.Plan()
 		if err != nil {
-			t.Fatalf("err: %s", err)
+			t.Fatalf("destroy plan err: %s", err)
 		}
 
 		var buf bytes.Buffer
 		if err := WritePlan(plan, &buf); err != nil {
-			t.Fatalf("err: %s", err)
+			t.Fatalf("plan write err: %s", err)
 		}
 
 		planFromFile, err := ReadPlan(&buf)
 		if err != nil {
-			t.Fatalf("err: %s", err)
+			t.Fatalf("plan read err: %s", err)
 		}
 
 		ctx, err = planFromFile.Context(&ContextOpts{
@@ -2609,7 +2606,7 @@ func TestContext2Apply_destroyModuleWithAttrsReferencingResource(t *testing.T) {
 
 		state, err = ctx.Apply()
 		if err != nil {
-			t.Fatalf("err: %s", err)
+			t.Fatalf("destroy apply err: %s", err)
 		}
 	}
 
diff --git a/terraform/graph_config_node_variable.go b/terraform/graph_config_node_variable.go
index 41aedff9e..f08520532 100644
--- a/terraform/graph_config_node_variable.go
+++ b/terraform/graph_config_node_variable.go
@@ -90,6 +90,16 @@ func (n *GraphNodeConfigVariable) Noop(opts *NoopOpts) bool {
 		return false
 	}
 
+	// We have to find our our module diff since we do funky things with
+	// the flat node's implementation of Path() below.
+	modDiff := opts.Diff.ModuleByPath(n.ModulePath)
+
+	// If we're destroying, we have no need of variables.
+	if modDiff != nil && modDiff.Destroy {
+		log.Printf("[DEBUG] Destroy diff, treating variable as a noop")
+		return true
+	}
+
 	for _, v := range opts.Graph.UpEdges(opts.Vertex).List() {
 		// This is terrible, but I can't think of a better way to do this.
 		if dag.VertexName(v) == rootNodeName {
diff --git a/terraform/test-fixtures/apply-destroy-module-with-attrs/child/main.tf b/terraform/test-fixtures/apply-destroy-module-with-attrs/child/main.tf
index 2aa1e1045..95c701268 100644
--- a/terraform/test-fixtures/apply-destroy-module-with-attrs/child/main.tf
+++ b/terraform/test-fixtures/apply-destroy-module-with-attrs/child/main.tf
@@ -3,3 +3,7 @@ variable "vpc_id" {}
 resource "aws_instance" "child" {
   vpc_id = "${var.vpc_id}"
 }
+
+output "modout" {
+  value = "${aws_instance.child.id}"
+}
diff --git a/terraform/test-fixtures/apply-destroy-module-with-attrs/main.tf b/terraform/test-fixtures/apply-destroy-module-with-attrs/main.tf
index 0f26089ff..d369873bf 100644
--- a/terraform/test-fixtures/apply-destroy-module-with-attrs/main.tf
+++ b/terraform/test-fixtures/apply-destroy-module-with-attrs/main.tf
@@ -1,6 +1,10 @@
-resource "aws_instance" "vpc"   { }
+resource "aws_instance" "vpc" { }
 
 module "child" {
   source = "./child"
   vpc_id = "${aws_instance.vpc.id}"
 }
+
+output "out" {
+  value = "${module.child.modout}"
+}