From 47fed6d31eeadbf6d44b8cee844edff7d537612f Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Thu, 26 Jan 2023 10:48:49 -0500
Subject: [PATCH] save null module outputs in state

Although they are not serialized to the final stored state, all module
outputs must be saved in the state for evaluation. There is no defined
schema which is used to identify the overall type of module outputs, so
all outputs must exist in the state to build the correct type for proper
evaluation.
---
 internal/terraform/context_apply2_test.go | 55 +++++++++++++++++++++++
 internal/terraform/node_output.go         | 45 ++++++++++---------
 2 files changed, 78 insertions(+), 22 deletions(-)

diff --git a/internal/terraform/context_apply2_test.go b/internal/terraform/context_apply2_test.go
index 5875fbf6ce63..4ce61ff2a5dc 100644
--- a/internal/terraform/context_apply2_test.go
+++ b/internal/terraform/context_apply2_test.go
@@ -1832,3 +1832,58 @@ output "a" {
 	_, diags = ctx.Apply(plan, m)
 	assertNoErrors(t, diags)
 }
+
+func TestContext2Apply_destroyNullModuleOutput(t *testing.T) {
+	p := testProvider("test")
+	ctx := testContext2(t, &ContextOpts{
+		Providers: map[addrs.Provider]providers.Factory{
+			addrs.NewDefaultProvider("test"): testProviderFuncFixed(p),
+		},
+	})
+
+	m := testModuleInline(t, map[string]string{
+		"main.tf": `
+module "null_module" {
+  source = "./mod"
+}
+
+locals {
+  module_output = module.null_module.null_module_test
+}
+
+output "test_root" {
+  value = module.null_module.test_output
+}
+
+output "root_module" {
+  value = local.module_output #fails
+}
+`,
+
+		"mod/main.tf": `
+output "test_output" {
+  value = "test"
+}
+
+output "null_module_test" {
+  value = null
+}
+`,
+	})
+
+	// verify plan and apply
+	plan, diags := ctx.Plan(m, states.NewState(), &PlanOpts{
+		Mode: plans.NormalMode,
+	})
+	assertNoErrors(t, diags)
+	state, diags := ctx.Apply(plan, m)
+	assertNoErrors(t, diags)
+
+	// now destroy
+	plan, diags = ctx.Plan(m, state, &PlanOpts{
+		Mode: plans.DestroyMode,
+	})
+	assertNoErrors(t, diags)
+	_, diags = ctx.Apply(plan, m)
+	assertNoErrors(t, diags)
+}
diff --git a/internal/terraform/node_output.go b/internal/terraform/node_output.go
index 2d5abead1748..af60c95691be 100644
--- a/internal/terraform/node_output.go
+++ b/internal/terraform/node_output.go
@@ -592,30 +592,31 @@ func (n *NodeApplyableOutput) setValue(state *states.SyncState, changes *plans.C
 		changes.RemoveOutputChange(n.Addr)
 	}
 
-	if val.IsKnown() && !val.IsNull() {
-		// The state itself doesn't represent unknown values, so we null them
-		// out here and then we'll save the real unknown value in the planned
-		// changeset below, if we have one on this graph walk.
-		log.Printf("[TRACE] setValue: Saving value for %s in state", n.Addr)
-
-		sensitive := n.Config.Sensitive
-		unmarkedVal, valueMarks := val.UnmarkDeep()
-
-		// If the evaluate value contains sensitive marks, the output has no
-		// choice but to declare itself as "sensitive".
-		for mark := range valueMarks {
-			if mark == marks.Sensitive {
-				sensitive = true
-				break
-			}
-		}
-
-		stateVal := cty.UnknownAsNull(unmarkedVal)
-		state.SetOutputValue(n.Addr, stateVal, sensitive)
-
-	} else {
+	// Null outputs must be saved for modules so that they can still be
+	// evaluated. Null root outputs are removed entirely, which is always fine
+	// because they can't be referenced by anything else in the configuration.
+	if n.Addr.Module.IsRoot() && val.IsNull() {
 		log.Printf("[TRACE] setValue: Removing %s from state (it is now null)", n.Addr)
 		state.RemoveOutputValue(n.Addr)
+		return
+	}
+
+	// The state itself doesn't represent unknown values, so we null them
+	// out here and then we'll save the real unknown value in the planned
+	// changeset, if we have one on this graph walk.
+	log.Printf("[TRACE] setValue: Saving value for %s in state", n.Addr)
+	sensitive := n.Config.Sensitive
+	unmarkedVal, valueMarks := val.UnmarkDeep()
+
+	// If the evaluated value contains sensitive marks, the output has no
+	// choice but to declare itself as "sensitive".
+	for mark := range valueMarks {
+		if mark == marks.Sensitive {
+			sensitive = true
+			break
+		}
 	}
 
+	stateVal := cty.UnknownAsNull(unmarkedVal)
+	state.SetOutputValue(n.Addr, stateVal, sensitive)
 }