From fe3edb8e46f8f8677277e3fd8a2a5466dbcd16aa Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Wed, 18 Dec 2019 14:37:28 -0500
Subject: [PATCH] more aggressively prune unused values

Since a planned destroy can no longer indicate it is a full destroy,
unused values were being left in the apply graph for evaluation. If
these values contains interpolations that can fail, (for example, a
zipmap with mismatched list sizes), it will cause the apply to abort.

The PrunUnusedValuesTransformer was only previously run during destroy,
more out of conservatism than for any other particular reason. Adapt it
to always remove unused values from the graph, with the exception being
the root module outputs, which must be retained when we don't have a
clear indication that a full destroy is being executed.
---
 terraform/graph_builder_apply.go |  9 ++++++---
 terraform/transform_reference.go | 29 ++++++++++++++++++++---------
 2 files changed, 26 insertions(+), 12 deletions(-)

diff --git a/terraform/graph_builder_apply.go b/terraform/graph_builder_apply.go
index 615731328..00ddcf5c4 100644
--- a/terraform/graph_builder_apply.go
+++ b/terraform/graph_builder_apply.go
@@ -175,17 +175,20 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer {
 		// Reverse the edges from outputs and locals, so that
 		// interpolations don't fail during destroy.
 		// Create a destroy node for outputs to remove them from the state.
-		// Prune unreferenced values, which may have interpolations that can't
-		// be resolved.
 		GraphTransformIf(
 			func() bool { return b.Destroy },
 			GraphTransformMulti(
 				&DestroyValueReferenceTransformer{},
 				&DestroyOutputTransformer{},
-				&PruneUnusedValuesTransformer{},
 			),
 		),
 
+		// Prune unreferenced values, which may have interpolations that can't
+		// be resolved.
+		&PruneUnusedValuesTransformer{
+			Destroy: b.Destroy,
+		},
+
 		// Add the node to fix the state count boundaries
 		&CountBoundaryTransformer{
 			Config: b.Config,
diff --git a/terraform/transform_reference.go b/terraform/transform_reference.go
index 25e544996..df34a3cea 100644
--- a/terraform/transform_reference.go
+++ b/terraform/transform_reference.go
@@ -206,21 +206,30 @@ func (t *DestroyValueReferenceTransformer) Transform(g *Graph) error {
 	return nil
 }
 
-// PruneUnusedValuesTransformer is s GraphTransformer that removes local and
-// output values which are not referenced in the graph. Since outputs and
-// locals always need to be evaluated, if they reference a resource that is not
-// available in the state the interpolation could fail.
-type PruneUnusedValuesTransformer struct{}
+// PruneUnusedValuesTransformer is a GraphTransformer that removes local,
+// variable, and output values which are not referenced in the graph. If these
+// values reference a resource that is no longer in the state the interpolation
+// could fail.
+type PruneUnusedValuesTransformer struct {
+	Destroy bool
+}
 
 func (t *PruneUnusedValuesTransformer) Transform(g *Graph) error {
-	// this might need multiple runs in order to ensure that pruning a value
-	// doesn't effect a previously checked value.
+	// Pruning a value can effect previously checked edges, so loop until there
+	// are no more changes.
 	for removed := 0; ; removed = 0 {
 		for _, v := range g.Vertices() {
-			switch v.(type) {
-			case *NodeApplyableOutput, *NodeLocal:
+			switch v := v.(type) {
+			case *NodeApplyableOutput:
+				// If we're not certain this is a full destroy, we need to keep any
+				// root module outputs
+				if v.Addr.Module.IsRoot() && !t.Destroy {
+					continue
+				}
+			case *NodeLocal, *NodeApplyableModuleVariable:
 				// OK
 			default:
+				// We're only concerned with variables, locals and outputs
 				continue
 			}
 
@@ -229,6 +238,7 @@ func (t *PruneUnusedValuesTransformer) Transform(g *Graph) error {
 			switch dependants.Len() {
 			case 0:
 				// nothing at all depends on this
+				log.Printf("[TRACE] PruneUnusedValuesTransformer: removing unused value %s", dag.VertexName(v))
 				g.Remove(v)
 				removed++
 			case 1:
@@ -236,6 +246,7 @@ func (t *PruneUnusedValuesTransformer) Transform(g *Graph) error {
 				// we need to check for the case of a single destroy node.
 				d := dependants.List()[0]
 				if _, ok := d.(*NodeDestroyableOutput); ok {
+					log.Printf("[TRACE] PruneUnusedValuesTransformer: removing unused value %s", dag.VertexName(v))
 					g.Remove(v)
 					removed++
 				}