From b346ba32d16e970259ba8e380b2fa71f9ed191d6 Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <mitchell.hashimoto@gmail.com>
Date: Sat, 10 Dec 2016 20:22:12 -0500
Subject: [PATCH] terraform: dependent provider resources are destroyed first
 in modules

This extends the prior commit to also verify (and fix) that resources of
dependent providers are destroyed first even when they're within
modules.
---
 terraform/context_apply_test.go               | 103 ++++++++++++++++++
 .../child/main.tf                             |   3 +
 .../main.tf                                   |   9 ++
 terraform/transform_destroy_edge.go           |   2 +
 4 files changed, 117 insertions(+)
 create mode 100644 terraform/test-fixtures/apply-multi-provider-destroy-child/child/main.tf
 create mode 100644 terraform/test-fixtures/apply-multi-provider-destroy-child/main.tf

diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go
index 3988c58e6..7f14e98c1 100644
--- a/terraform/context_apply_test.go
+++ b/terraform/context_apply_test.go
@@ -2843,6 +2843,109 @@ func TestContext2Apply_multiProviderDestroy(t *testing.T) {
 	checkStateString(t, state, `<no state>`)
 }
 
+// This is like the multiProviderDestroy test except it tests that
+// dependent resources within a child module that inherit provider
+// configuration are still destroyed first.
+func TestContext2Apply_multiProviderDestroyChild(t *testing.T) {
+	m := testModule(t, "apply-multi-provider-destroy-child")
+	p := testProvider("aws")
+	p.ApplyFn = testApplyFn
+	p.DiffFn = testDiffFn
+
+	p2 := testProvider("do")
+	p2.ApplyFn = testApplyFn
+	p2.DiffFn = testDiffFn
+
+	var state *State
+
+	// First, create the instances
+	{
+		ctx := testContext2(t, &ContextOpts{
+			Module: m,
+			Providers: map[string]ResourceProviderFactory{
+				"aws":   testProviderFuncFixed(p),
+				"vault": testProviderFuncFixed(p2),
+			},
+		})
+
+		if _, err := ctx.Plan(); err != nil {
+			t.Fatalf("err: %s", err)
+		}
+
+		s, err := ctx.Apply()
+		if err != nil {
+			t.Fatalf("err: %s", err)
+		}
+
+		state = s
+	}
+
+	// Destroy them
+	{
+		// Verify that aws_instance.bar is destroyed first
+		var checked bool
+		var called int32
+		var lock sync.Mutex
+		applyFn := func(
+			info *InstanceInfo,
+			is *InstanceState,
+			id *InstanceDiff) (*InstanceState, error) {
+			lock.Lock()
+			defer lock.Unlock()
+
+			if info.HumanId() == "module.child.aws_instance.bar" {
+				checked = true
+
+				// Sleep to allow parallel execution
+				time.Sleep(50 * time.Millisecond)
+
+				// Verify that called is 0 (dep not called)
+				if atomic.LoadInt32(&called) != 0 {
+					return nil, fmt.Errorf("nothing else should be called")
+				}
+			}
+
+			atomic.AddInt32(&called, 1)
+			return testApplyFn(info, is, id)
+		}
+
+		// Set the apply functions
+		p.ApplyFn = applyFn
+		p2.ApplyFn = applyFn
+
+		ctx := testContext2(t, &ContextOpts{
+			Destroy: true,
+			State:   state,
+			Module:  m,
+			Providers: map[string]ResourceProviderFactory{
+				"aws":   testProviderFuncFixed(p),
+				"vault": testProviderFuncFixed(p2),
+			},
+		})
+
+		if _, err := ctx.Plan(); err != nil {
+			t.Fatalf("err: %s", err)
+		}
+
+		s, err := ctx.Apply()
+		if err != nil {
+			t.Fatalf("err: %s", err)
+		}
+
+		if !checked {
+			t.Fatal("should be checked")
+		}
+
+		state = s
+	}
+
+	checkStateString(t, state, `
+<no state>
+module.child:
+  <no state>
+`)
+}
+
 func TestContext2Apply_multiVar(t *testing.T) {
 	m := testModule(t, "apply-multi-var")
 	p := testProvider("aws")
diff --git a/terraform/test-fixtures/apply-multi-provider-destroy-child/child/main.tf b/terraform/test-fixtures/apply-multi-provider-destroy-child/child/main.tf
new file mode 100644
index 000000000..ae1bc8ee4
--- /dev/null
+++ b/terraform/test-fixtures/apply-multi-provider-destroy-child/child/main.tf
@@ -0,0 +1,3 @@
+resource "aws_instance" "bar" {
+    foo = "bar"
+}
diff --git a/terraform/test-fixtures/apply-multi-provider-destroy-child/main.tf b/terraform/test-fixtures/apply-multi-provider-destroy-child/main.tf
new file mode 100644
index 000000000..61e69e816
--- /dev/null
+++ b/terraform/test-fixtures/apply-multi-provider-destroy-child/main.tf
@@ -0,0 +1,9 @@
+resource "vault_instance" "foo" {}
+
+provider "aws" {
+  addr = "${vault_instance.foo.id}"
+}
+
+module "child" {
+  source = "./child"
+}
diff --git a/terraform/transform_destroy_edge.go b/terraform/transform_destroy_edge.go
index 76e3fd659..8c067892c 100644
--- a/terraform/transform_destroy_edge.go
+++ b/terraform/transform_destroy_edge.go
@@ -127,6 +127,8 @@ func (t *DestroyEdgeTransformer) Transform(g *Graph) error {
 		// Add providers since they can affect destroy order as well
 		&MissingProviderTransformer{AllowAny: true, Concrete: providerFn},
 		&ProviderTransformer{},
+		&DisableProviderTransformer{},
+		&ParentProviderTransformer{},
 		&AttachProviderConfigTransformer{Module: t.Module},
 
 		&ReferenceTransformer{},