From bf0e7705b1c5dfa0b6270bbfdcc207caf992a7ba Mon Sep 17 00:00:00 2001
From: Paul Hinze <phinze@phinze.com>
Date: Sat, 11 Jun 2016 20:31:43 -0500
Subject: [PATCH] core: Fix destroy when module vars used in provider config

For `terraform destroy`, we currently build up the same graph we do for
`plan` and `apply` and we do a walk with a special Diff that says
"destroy everything".

We have fought the interpolation subsystem time and again through this
code path. Beginning in #2775 we gained a new feature to selectively
prune out problematic graph nodes. The past chain of destroy fixes I
have been involved with (#6557, #6599, #6753) have attempted to massage
the "noop" definitions to properly handle the edge cases reported.

"Variable is depended on by provider config" is another edge case we add
here and try to fix.

This dive only makes me more convinced that the whole `terraform
destroy` code path needs to be reworked.

For now, I went with a "surgical strike" approach to the problem
expressed in #7047. I found a couple of issues with the existing
Noop and DestroyEdgeInclude logic, especially with regards to
flattening, but I'm explicitly ignoring these for now so we can get this
particular bug fixed ahead of the 0.7 release. My hope is that we can
circle around with a fully specced initiative to refactor `terraform
destroy`'s graph to be more state-derived than config-derived.

Until then, this fixes #7407
---
 terraform/context_apply_test.go               | 39 +++++++++++++++++++
 terraform/graph_config_node_variable.go       | 21 ++++++++++
 .../child/child.tf                            |  7 ++++
 .../main.tf                                   |  4 ++
 4 files changed, 71 insertions(+)
 create mode 100644 terraform/test-fixtures/apply-destroy-mod-var-provider-config/child/child.tf
 create mode 100644 terraform/test-fixtures/apply-destroy-mod-var-provider-config/main.tf

diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go
index 99f2d805b..d64399159 100644
--- a/terraform/context_apply_test.go
+++ b/terraform/context_apply_test.go
@@ -443,6 +443,45 @@ func TestContext2Apply_destroySkipsCBD(t *testing.T) {
 	}
 }
 
+func TestContext2Apply_destroyModuleVarProviderConfig(t *testing.T) {
+	m := testModule(t, "apply-destroy-mod-var-provider-config")
+	p := testProvider("aws")
+	p.ApplyFn = testApplyFn
+	p.DiffFn = testDiffFn
+	state := &State{
+		Modules: []*ModuleState{
+			&ModuleState{
+				Path: []string{"root", "child"},
+				Resources: map[string]*ResourceState{
+					"aws_instance.foo": &ResourceState{
+						Type: "aws_instance",
+						Primary: &InstanceState{
+							ID: "foo",
+						},
+					},
+				},
+			},
+		},
+	}
+	ctx := testContext2(t, &ContextOpts{
+		Module: m,
+		Providers: map[string]ResourceProviderFactory{
+			"aws": testProviderFuncFixed(p),
+		},
+		State:   state,
+		Destroy: true,
+	})
+
+	if _, err := ctx.Plan(); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	_, err := ctx.Apply()
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+}
+
 // https://github.com/hashicorp/terraform/issues/2892
 func TestContext2Apply_destroyCrossProviders(t *testing.T) {
 	m := testModule(t, "apply-destroy-cross-providers")
diff --git a/terraform/graph_config_node_variable.go b/terraform/graph_config_node_variable.go
index 1f17471e8..0a86c4881 100644
--- a/terraform/graph_config_node_variable.go
+++ b/terraform/graph_config_node_variable.go
@@ -234,3 +234,24 @@ func (n *GraphNodeConfigVariableFlat) Path() []string {
 
 	return nil
 }
+
+func (n *GraphNodeConfigVariableFlat) Noop(opts *NoopOpts) bool {
+	// First look for provider nodes that depend on this variable downstream
+	modDiff := opts.Diff.ModuleByPath(n.ModulePath)
+	if modDiff != nil && modDiff.Destroy {
+		ds, err := opts.Graph.Descendents(n)
+		if err != nil {
+			log.Printf("[ERROR] Error looking up descendents of %s: %s", n.Name(), err)
+		} else {
+			for _, d := range ds.List() {
+				if _, ok := d.(GraphNodeProvider); ok {
+					log.Printf("[DEBUG] This variable is depended on by a provider, can't be a noop.")
+					return false
+				}
+			}
+		}
+	}
+
+	// Then fall back to existing impl
+	return n.GraphNodeConfigVariable.Noop(opts)
+}
diff --git a/terraform/test-fixtures/apply-destroy-mod-var-provider-config/child/child.tf b/terraform/test-fixtures/apply-destroy-mod-var-provider-config/child/child.tf
new file mode 100644
index 000000000..6544cf6cb
--- /dev/null
+++ b/terraform/test-fixtures/apply-destroy-mod-var-provider-config/child/child.tf
@@ -0,0 +1,7 @@
+variable "input" {}
+
+provider "aws" {
+  region = "us-east-${var.input}"
+}
+
+resource "aws_instance" "foo" { }
diff --git a/terraform/test-fixtures/apply-destroy-mod-var-provider-config/main.tf b/terraform/test-fixtures/apply-destroy-mod-var-provider-config/main.tf
new file mode 100644
index 000000000..1e2dfb352
--- /dev/null
+++ b/terraform/test-fixtures/apply-destroy-mod-var-provider-config/main.tf
@@ -0,0 +1,4 @@
+module "child" {
+  source = "./child"
+  input = "1"
+}