Merge pull request #23717 from hashicorp/jbardin/destroy-plan-values

Always prune unused values

terraform/context_apply_test.go

@@ -10526,7 +10526,6 @@ func TestContext2Apply_plannedDestroyInterpolatedCount(t *testing.T) {
 	}
 
 	ctxOpts.ProviderResolver = providerResolver
-	ctxOpts.Destroy = true
 	ctx, diags = NewContext(ctxOpts)
 	if diags.HasErrors() {
 		t.Fatalf("err: %s", diags.Err())

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,

terraform/terraform_test.go

@@ -406,9 +406,7 @@ module.child:
 <no state>
 Outputs:
 
-aws_access_key = YYYYY
 aws_route53_zone_id = XXXX
-aws_secret_key = ZZZZ
 `
 
 const testTerraformApplyDependsCreateBeforeStr = `
@@ -693,11 +691,6 @@ foo = bar
 
 const testTerraformApplyOutputOrphanModuleStr = `
 <no state>
-module.child:
-  <no state>
-  Outputs:
-
-  foo = bar
 `
 
 const testTerraformApplyProvisionerStr = `

terraform/testdata/plan-destroy-interpolated-count/main.tf

@@ -3,9 +3,18 @@ variable "list" {
 }
 
 resource "aws_instance" "a" {
-  count = "${length(var.list)}"
+  count = length(var.list)
+}
+
+locals {
+  ids = aws_instance.a[*].id
+}
+
+module "empty" {
+  source = "./mod"
+  input = zipmap(var.list, local.ids)
 }
 
 output "out" {
-  value = "${aws_instance.a.*.id}"
+  value = aws_instance.a[*].id
 }

terraform/testdata/plan-destroy-interpolated-count/mod/main.tf

@@ -0,0 +1,2 @@
+variable "input" {
+}

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,13 +238,15 @@ 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:
 				// 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 {
+					log.Printf("[TRACE] PruneUnusedValuesTransformer: removing unused value %s", dag.VertexName(v))
 					g.Remove(v)
 					removed++
 				}