From 5e67657325669772497bd78e6c66d6c78353c084 Mon Sep 17 00:00:00 2001
From: Paul Hinze <phinze@phinze.com>
Date: Wed, 15 Apr 2015 13:53:32 -0500
Subject: [PATCH] core: fix targeting in destroy w/ provisioners

The `TargetTransform` was dropping provisioner nodes, which caused graph
validation to fail with messages about uninitialized provisioners when a
`terraform destroy` was attempted.

This was because `destroy` flops the dependency calculation to try and
address any nodes in the graph that "depend on" the target node. But we
still need to keep the provisioner node in the graph.

Here we switch the strategy for filtering nodes to only drop
addressable, non-targeted nodes. This should prevent us from having to
whitelist nodes to keep in the future.

closes #1541
---
 terraform/context_test.go                     | 42 ++++++++++++++++++
 .../test-fixtures/validate-targeted/main.tf   | 13 ++++++
 terraform/transform_targets.go                | 43 +++++++++----------
 3 files changed, 76 insertions(+), 22 deletions(-)
 create mode 100644 terraform/test-fixtures/validate-targeted/main.tf

diff --git a/terraform/context_test.go b/terraform/context_test.go
index 6ff63d813..e9781918b 100644
--- a/terraform/context_test.go
+++ b/terraform/context_test.go
@@ -2688,6 +2688,48 @@ func TestContext2Validate_tainted(t *testing.T) {
 	}
 }
 
+func TestContext2Validate_targetedDestroy(t *testing.T) {
+	m := testModule(t, "validate-targeted")
+	p := testProvider("aws")
+	pr := testProvisioner()
+	p.ApplyFn = testApplyFn
+	p.DiffFn = testDiffFn
+	ctx := testContext2(t, &ContextOpts{
+		Module: m,
+		Providers: map[string]ResourceProviderFactory{
+			"aws": testProviderFuncFixed(p),
+		},
+		Provisioners: map[string]ResourceProvisionerFactory{
+			"shell": testProvisionerFuncFixed(pr),
+		},
+		State: &State{
+			Modules: []*ModuleState{
+				&ModuleState{
+					Path: rootModulePath,
+					Resources: map[string]*ResourceState{
+						"aws_instance.foo": resourceState("aws_instance", "i-bcd345"),
+						"aws_instance.bar": resourceState("aws_instance", "i-abc123"),
+					},
+				},
+			},
+		},
+		Targets: []string{"aws_instance.foo"},
+		Destroy: true,
+	})
+
+	w, e := ctx.Validate()
+	if len(w) > 0 {
+		warnStr := ""
+		for _, v := range w {
+			warnStr = warnStr + " " + v
+		}
+		t.Fatalf("bad: %s", warnStr)
+	}
+	if len(e) > 0 {
+		t.Fatalf("bad: %s", e)
+	}
+}
+
 func TestContext2Validate_varRef(t *testing.T) {
 	m := testModule(t, "validate-variable-ref")
 	p := testProvider("aws")
diff --git a/terraform/test-fixtures/validate-targeted/main.tf b/terraform/test-fixtures/validate-targeted/main.tf
new file mode 100644
index 000000000..b40b126fa
--- /dev/null
+++ b/terraform/test-fixtures/validate-targeted/main.tf
@@ -0,0 +1,13 @@
+resource "aws_instance" "foo" {
+    num = "2"
+    provisioner "shell" {
+       command = "echo hi"
+    }
+}
+
+resource "aws_instance" "bar" {
+    foo = "bar"
+    provisioner "shell" {
+       command = "echo hi"
+    }
+}
diff --git a/terraform/transform_targets.go b/terraform/transform_targets.go
index 29a6d53c6..1ddb7f9c4 100644
--- a/terraform/transform_targets.go
+++ b/terraform/transform_targets.go
@@ -28,9 +28,10 @@ func (t *TargetsTransformer) Transform(g *Graph) error {
 		}
 
 		for _, v := range g.Vertices() {
-			if targetedNodes.Include(v) {
-			} else {
-				g.Remove(v)
+			if _, ok := v.(GraphNodeAddressable); ok {
+				if !targetedNodes.Include(v) {
+					g.Remove(v)
+				}
 			}
 		}
 	}
@@ -49,35 +50,29 @@ func (t *TargetsTransformer) parseTargetAddresses() ([]ResourceAddress, error) {
 	return addrs, nil
 }
 
+// Returns the list of targeted nodes. A targeted node is either addressed
+// directly, or is an Ancestor of a targeted node. Destroy mode keeps
+// Descendents instead of Ancestors.
 func (t *TargetsTransformer) selectTargetedNodes(
 	g *Graph, addrs []ResourceAddress) (*dag.Set, error) {
 	targetedNodes := new(dag.Set)
 	for _, v := range g.Vertices() {
-		// Keep all providers; they'll be pruned later if necessary
-		if r, ok := v.(GraphNodeProvider); ok {
-			targetedNodes.Add(r)
-			continue
-		}
+		if t.nodeIsTarget(v, addrs) {
+			targetedNodes.Add(v)
 
-		// For the remaining filter, we only care about addressable nodes
-		r, ok := v.(GraphNodeAddressable)
-		if !ok {
-			continue
-		}
-
-		if t.nodeIsTarget(r, addrs) {
-			targetedNodes.Add(r)
-			// If the node would like to know about targets, tell it.
-			if n, ok := r.(GraphNodeTargetable); ok {
-				n.SetTargets(addrs)
+			// We inform nodes that ask about the list of targets - helps for nodes
+			// that need to dynamically expand. Note that this only occurs for nodes
+			// that are already directly targeted.
+			if tn, ok := v.(GraphNodeTargetable); ok {
+				tn.SetTargets(addrs)
 			}
 
 			var deps *dag.Set
 			var err error
 			if t.Destroy {
-				deps, err = g.Descendents(r)
+				deps, err = g.Descendents(v)
 			} else {
-				deps, err = g.Ancestors(r)
+				deps, err = g.Ancestors(v)
 			}
 			if err != nil {
 				return nil, err
@@ -92,7 +87,11 @@ func (t *TargetsTransformer) selectTargetedNodes(
 }
 
 func (t *TargetsTransformer) nodeIsTarget(
-	r GraphNodeAddressable, addrs []ResourceAddress) bool {
+	v dag.Vertex, addrs []ResourceAddress) bool {
+	r, ok := v.(GraphNodeAddressable)
+	if !ok {
+		return false
+	}
 	addr := r.ResourceAddress()
 	for _, targetAddr := range addrs {
 		if targetAddr.Equals(addr) {