core: Don't re-register checkable outputs during the apply step

internal/terraform/context_apply2_test.go

@@ -14,11 +14,13 @@ import (
 	"github.com/zclconf/go-cty/cty"
 
 	"github.com/hashicorp/terraform/internal/addrs"
+	"github.com/hashicorp/terraform/internal/checks"
 	"github.com/hashicorp/terraform/internal/configs/configschema"
 	"github.com/hashicorp/terraform/internal/lang/marks"
 	"github.com/hashicorp/terraform/internal/plans"
 	"github.com/hashicorp/terraform/internal/providers"
 	"github.com/hashicorp/terraform/internal/states"
+	"github.com/hashicorp/terraform/internal/tfdiags"
 )
 
 // Test that the PreApply hook is called with the correct deposed key
@@ -917,6 +919,119 @@ resource "test_resource" "c" {
 	})
 }
 
+func TestContext2Apply_outputValuePrecondition(t *testing.T) {
+	m := testModuleInline(t, map[string]string{
+		"main.tf": `
+			variable "input" {
+				type = string
+			}
+
+			module "child" {
+				source = "./child"
+
+				input = var.input
+			}
+
+			output "result" {
+				value = module.child.result
+
+				precondition {
+					condition     = var.input != ""
+					error_message = "Input must not be empty."
+				}
+			}
+		`,
+		"child/main.tf": `
+			variable "input" {
+				type = string
+			}
+
+			output "result" {
+				value = var.input
+
+				precondition {
+					condition     = var.input != ""
+					error_message = "Input must not be empty."
+				}
+			}
+		`,
+	})
+
+	checkableObjects := []addrs.Checkable{
+		addrs.OutputValue{Name: "result"}.Absolute(addrs.RootModuleInstance),
+		addrs.OutputValue{Name: "result"}.Absolute(addrs.RootModuleInstance.Child("child", addrs.NoKey)),
+	}
+
+	t.Run("pass", func(t *testing.T) {
+		ctx := testContext2(t, &ContextOpts{})
+		plan, diags := ctx.Plan(m, states.NewState(), &PlanOpts{
+			Mode: plans.NormalMode,
+			SetVariables: InputValues{
+				"input": &InputValue{
+					Value:      cty.StringVal("beep"),
+					SourceType: ValueFromCLIArg,
+				},
+			},
+		})
+		assertNoDiagnostics(t, diags)
+
+		for _, addr := range checkableObjects {
+			result := plan.Checks.GetObjectResult(addr)
+			if result == nil {
+				t.Fatalf("no check result for %s in the plan", addr)
+			}
+			if got, want := result.Status, checks.StatusPass; got != want {
+				t.Fatalf("wrong check status for %s during planning\ngot:  %s\nwant: %s", addr, got, want)
+			}
+		}
+
+		state, diags := ctx.Apply(plan, m)
+		assertNoDiagnostics(t, diags)
+		for _, addr := range checkableObjects {
+			result := state.CheckResults.GetObjectResult(addr)
+			if result == nil {
+				t.Fatalf("no check result for %s in the final state", addr)
+			}
+			if got, want := result.Status, checks.StatusPass; got != want {
+				t.Errorf("wrong check status for %s after apply\ngot:  %s\nwant: %s", addr, got, want)
+			}
+		}
+	})
+
+	t.Run("fail", func(t *testing.T) {
+		// NOTE: This test actually catches a failure during planning and so
+		// cannot proceed to apply, so it's really more of a plan test
+		// than an apply test but better to keep all of these
+		// thematically-related test cases together.
+		ctx := testContext2(t, &ContextOpts{})
+		_, diags := ctx.Plan(m, states.NewState(), &PlanOpts{
+			Mode: plans.NormalMode,
+			SetVariables: InputValues{
+				"input": &InputValue{
+					Value:      cty.StringVal(""),
+					SourceType: ValueFromCLIArg,
+				},
+			},
+		})
+		if !diags.HasErrors() {
+			t.Fatalf("succeeded; want error")
+		}
+
+		const wantSummary = "Module output value precondition failed"
+		found := false
+		for _, diag := range diags {
+			if diag.Severity() == tfdiags.Error && diag.Description().Summary == wantSummary {
+				found = true
+				break
+			}
+		}
+
+		if !found {
+			t.Fatalf("missing expected error\nwant summary: %s\ngot: %s", wantSummary, diags.Err().Error())
+		}
+	})
+}
+
 func TestContext2Apply_resourceConditionApplyTimeFail(t *testing.T) {
 	// This tests the less common situation where a condition fails due to
 	// a change in a resource other than the one the condition is attached to,

internal/terraform/graph_builder_plan.go

@@ -113,6 +113,13 @@ func (b *PlanGraphBuilder) Steps() []GraphTransformer {
 			Config:            b.Config,
 			RefreshOnly:       b.skipPlanChanges,
 			removeRootOutputs: b.Operation == walkPlanDestroy,
+
+			// NOTE: We currently treat anything built with the plan graph
+			// builder as "planning" for our purposes here, because we share
+			// the same graph node implementation between all of the walk
+			// types and so the pre-planning walks still think they are
+			// producing a plan even though we immediately discard it.
+			Planning: true,
 		},
 
 		// Add orphan resources

internal/terraform/node_output.go

@@ -25,6 +25,14 @@ type nodeExpandOutput struct {
 	Config      *configs.Output
 	Destroy     bool
 	RefreshOnly bool
+
+	// Planning is set to true when this node is in a graph that was produced
+	// by the plan graph builder, as opposed to the apply graph builder.
+	// This quirk is just because we share the same node type between both
+	// phases but in practice there are a few small differences in the actions
+	// we need to take between plan and apply. See method DynamicExpand for
+	// details.
+	Planning bool
 }
 
 var (
@@ -59,9 +67,18 @@ func (n *nodeExpandOutput) DynamicExpand(ctx EvalContext) (*Graph, error) {
 	// wants to know the addresses of the checkable objects so that it can
 	// treat them as unknown status if we encounter an error before actually
 	// visiting the checks.
+	//
+	// We must do this only during planning, because the apply phase will start
+	// with all of the same checkable objects that were registered during the
+	// planning phase. Consumers of our JSON plan and state formats expect
+	// that the set of checkable objects will be consistent between the plan
+	// and any state snapshots created during apply, and that only the statuses
+	// of those objects will have changed.
 	var checkableAddrs addrs.Set[addrs.Checkable]
-	if checkState := ctx.Checks(); checkState.ConfigHasChecks(n.Addr.InModule(n.Module)) {
-		checkableAddrs = addrs.MakeSet[addrs.Checkable]()
+	if n.Planning {
+		if checkState := ctx.Checks(); checkState.ConfigHasChecks(n.Addr.InModule(n.Module)) {
+			checkableAddrs = addrs.MakeSet[addrs.Checkable]()
+		}
 	}
 
 	var g Graph

internal/terraform/transform_output.go

@@ -26,6 +26,10 @@ type OutputTransformer struct {
 	// Refresh-only mode means that any failing output preconditions are
 	// reported as warnings rather than errors
 	RefreshOnly bool
+
+	// Planning must be set to true only when we're building a planning graph.
+	// It must be set to false whenever we're building an apply graph.
+	Planning bool
 }
 
 func (t *OutputTransformer) Transform(g *Graph) error {
@@ -89,6 +93,7 @@ func (t *OutputTransformer) transform(g *Graph, c *configs.Config) error {
 				Config:      o,
 				Destroy:     t.removeRootOutputs,
 				RefreshOnly: t.RefreshOnly,
+				Planning:    t.Planning,
 			}
 		}