From 620f1985a1555db3d35a77d87256a7040436c0d8 Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Tue, 3 Apr 2018 13:19:04 -0400
Subject: [PATCH] unused outputs in a destroy should be pruned

During a full destroy when outputs are removed, the
NodeDestroyableOutput was preventing it's sibling output from being
destroyed. Prune the output node if it only has its destroy node as a
dependent.

The destroy output test is simply run a second time with no state, which
would cause the output interpolation to fail if it remained in the
graph.
---
 terraform/context_apply_test.go  | 31 ++++++++++++++++++-------
 terraform/transform_reference.go | 39 ++++++++++++++++++++++++--------
 2 files changed, 52 insertions(+), 18 deletions(-)

diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go
index f53bae1d9d..0e73bd50ce 100644
--- a/terraform/context_apply_test.go
+++ b/terraform/context_apply_test.go
@@ -6582,13 +6582,11 @@ module.child.child2:
 
 func TestContext2Apply_destroyOutputs(t *testing.T) {
 	m := testModule(t, "apply-destroy-outputs")
-	h := new(HookRecordApplyOrder)
 	p := testProvider("aws")
 	p.ApplyFn = testApplyFn
 	p.DiffFn = testDiffFn
 	ctx := testContext2(t, &ContextOpts{
 		Module: m,
-		Hooks:  []Hook{h},
 		ProviderResolver: ResourceProviderResolverFixed(
 			map[string]ResourceProviderFactory{
 				"aws": testProviderFuncFixed(p),
@@ -6608,12 +6606,10 @@ func TestContext2Apply_destroyOutputs(t *testing.T) {
 	}
 
 	// Next, plan and apply a destroy operation
-	h.Active = true
 	ctx = testContext2(t, &ContextOpts{
 		Destroy: true,
 		State:   state,
 		Module:  m,
-		Hooks:   []Hook{h},
 		ProviderResolver: ResourceProviderResolverFixed(
 			map[string]ResourceProviderFactory{
 				"aws": testProviderFuncFixed(p),
@@ -6622,17 +6618,36 @@ func TestContext2Apply_destroyOutputs(t *testing.T) {
 	})
 
 	if _, err := ctx.Plan(); err != nil {
-		t.Fatalf("err: %s", err)
+		t.Fatal(err)
 	}
 
 	state, err = ctx.Apply()
 	if err != nil {
-		t.Fatalf("err: %s", err)
+		t.Fatal(err)
 	}
 
 	mod := state.RootModule()
 	if len(mod.Resources) > 0 {
-		t.Fatalf("bad: %#v", mod)
+		t.Fatalf("expected no resources, got: %#v", mod)
+	}
+
+	// destroying again should produce no errors
+	ctx = testContext2(t, &ContextOpts{
+		Destroy: true,
+		State:   state,
+		Module:  m,
+		ProviderResolver: ResourceProviderResolverFixed(
+			map[string]ResourceProviderFactory{
+				"aws": testProviderFuncFixed(p),
+			},
+		),
+	})
+	if _, err := ctx.Plan(); err != nil {
+		t.Fatal(err)
+	}
+
+	if _, err = ctx.Apply(); err != nil {
+		t.Fatal(err)
 	}
 }
 
@@ -7766,7 +7781,7 @@ func TestContext2Apply_destroyProvisionerWithOutput(t *testing.T) {
 		},
 		Destroy: true,
 
-		// targeting the source of the value used by all resources shoudl still
+		// targeting the source of the value used by all resources should still
 		// destroy them all.
 		Targets: []string{"module.mod.aws_instance.baz"},
 	})
diff --git a/terraform/transform_reference.go b/terraform/transform_reference.go
index 403b7e4246..be8c7f96c3 100644
--- a/terraform/transform_reference.go
+++ b/terraform/transform_reference.go
@@ -119,17 +119,36 @@ func (t *DestroyValueReferenceTransformer) Transform(g *Graph) error {
 type PruneUnusedValuesTransformer struct{}
 
 func (t *PruneUnusedValuesTransformer) Transform(g *Graph) error {
-	vs := g.Vertices()
-	for _, v := range vs {
-		switch v.(type) {
-		case *NodeApplyableOutput, *NodeLocal:
-			// OK
-		default:
-			continue
-		}
+	// this might need multiple runs in order to ensure that pruning a value
+	// doesn't effect a previously checked value.
+	for removed := 0; ; removed = 0 {
+		for _, v := range g.Vertices() {
+			switch v.(type) {
+			case *NodeApplyableOutput, *NodeLocal:
+				// OK
+			default:
+				continue
+			}
 
-		if len(g.EdgesTo(v)) == 0 {
-			g.Remove(v)
+			dependants := g.UpEdges(v)
+
+			switch dependants.Len() {
+			case 0:
+				// nothing at all depends on this
+				g.Remove(v)
+				removed++
+			case 1:
+				// because an output's destroy node always depends on the output,
+				// we need to check for the case of a single destroy node.
+				d := dependants.List()[0]
+				if _, ok := d.(*NodeDestroyableOutput); ok {
+					g.Remove(v)
+					removed++
+				}
+			}
+		}
+		if removed == 0 {
+			break
 		}
 	}