core: Whole-module evaluation must consider planned output values

Just as when we resolve single output values we must check to see if there
is a planned new value for an output before using the value in state,
because the planned new value might contain unknowns that can't be
represented directly in the state (and would thus be incorrectly returned
as null).

terraform/context_plan_test.go

@@ -5580,3 +5580,70 @@ func TestContext2Plan_requiredModuleOutput(t *testing.T) {
 		})
 	}
 }
+
+func TestContext2Plan_requiredModuleObject(t *testing.T) {
+	m := testModule(t, "plan-required-whole-mod")
+	p := testProvider("test")
+	p.GetSchemaReturn = &ProviderSchema{
+		ResourceTypes: map[string]*configschema.Block{
+			"test_resource": {
+				Attributes: map[string]*configschema.Attribute{
+					"id":       {Type: cty.String, Computed: true},
+					"required": {Type: cty.String, Required: true},
+				},
+			},
+		},
+	}
+	p.DiffFn = testDiffFn
+
+	ctx := testContext2(t, &ContextOpts{
+		Config: m,
+		ProviderResolver: providers.ResolverFixed(
+			map[string]providers.Factory{
+				"test": testProviderFuncFixed(p),
+			},
+		),
+	})
+
+	plan, diags := ctx.Plan()
+	if diags.HasErrors() {
+		t.Fatalf("unexpected errors: %s", diags.Err())
+	}
+
+	schema := p.GetSchemaReturn.ResourceTypes["test_resource"]
+	ty := schema.ImpliedType()
+
+	if len(plan.Changes.Resources) != 2 {
+		t.Fatal("expected 2 changes, got", len(plan.Changes.Resources))
+	}
+
+	for _, res := range plan.Changes.Resources {
+		t.Run(fmt.Sprintf("%s %s", res.Action, res.Addr), func(t *testing.T) {
+			if res.Action != plans.Create {
+				t.Fatalf("expected resource creation, got %s", res.Action)
+			}
+			ric, err := res.Decode(ty)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			var expected cty.Value
+			switch i := ric.Addr.String(); i {
+			case "test_resource.root":
+				expected = objectVal(t, schema, map[string]cty.Value{
+					"id":       cty.UnknownVal(cty.String),
+					"required": cty.UnknownVal(cty.String),
+				})
+			case "module.mod.test_resource.for_output":
+				expected = objectVal(t, schema, map[string]cty.Value{
+					"id":       cty.UnknownVal(cty.String),
+					"required": cty.StringVal("val"),
+				})
+			default:
+				t.Fatal("unknown instance:", i)
+			}
+
+			checkVals(t, expected, ric.After)
+		})
+	}
+}

terraform/evaluate.go

@@ -305,14 +305,31 @@ func (d *evaluationStateData) GetModuleInstance(addr addrs.ModuleCallInstance, r
 	vals := map[string]cty.Value{}
 	for n := range outputConfigs {
 		addr := addrs.OutputValue{Name: n}.Absolute(moduleAddr)
-		os := d.Evaluator.State.OutputValue(addr)
-		if os == nil {
-			// Not evaluated yet?
-			vals[n] = cty.DynamicVal
-			continue
-		}
 
-		vals[n] = os.Value
+		// If a pending change is present in our current changeset then its value
+		// takes priority over what's in state. (It will usually be the same but
+		// will differ if the new value is unknown during planning.)
+		if changeSrc := d.Evaluator.Changes.GetOutputChange(addr); changeSrc != nil {
+			change, err := changeSrc.Decode()
+			if err != nil {
+				// This should happen only if someone has tampered with a plan
+				// file, so we won't bother with a pretty error for it.
+				diags = diags.Append(fmt.Errorf("planned change for %s could not be decoded: %s", addr, err))
+				vals[n] = cty.DynamicVal
+				continue
+			}
+			// We care only about the "after" value, which is the value this output
+			// will take on after the plan is applied.
+			vals[n] = change.After
+		} else {
+			os := d.Evaluator.State.OutputValue(addr)
+			if os == nil {
+				// Not evaluated yet?
+				vals[n] = cty.DynamicVal
+				continue
+			}
+			vals[n] = os.Value
+		}
 	}
 	return cty.ObjectVal(vals), diags
 }

terraform/test-fixtures/plan-required-whole-mod/main.tf

@@ -0,0 +1,17 @@
+resource "test_resource" "root" {
+  required = local.object.id
+}
+
+locals {
+  # This indirection is here to force the evaluator to produce the whole
+  # module object here rather than just fetching the single "object" output.
+  # This makes this fixture different than plan-required-output, which just
+  # accesses module.mod.object.id directly and thus visits a different
+  # codepath in the evaluator.
+  mod    = module.mod
+  object = local.mod.object
+}
+
+module "mod" {
+  source = "./mod"
+}

terraform/test-fixtures/plan-required-whole-mod/mod/main.tf

@@ -0,0 +1,7 @@
+resource "test_resource" "for_output" {
+  required = "val"
+}
+
+output "object" {
+  value = test_resource.for_output
+}