core: Generate placeholder results for named values under partially-expanded module prefixes

internal/terraform/node_local.go

@@ -145,35 +145,9 @@ func (n *NodeLocal) References() []*addrs.Reference {
 // expression for a local value and writes it into a transient part of
 // the state.
 func (n *NodeLocal) Execute(ctx EvalContext, op walkOperation) (diags tfdiags.Diagnostics) {
-	expr := n.Config.Expr
-	addr := n.Addr.LocalValue
-
-	// We ignore diags here because any problems we might find will be found
-	// again in EvaluateExpr below.
-	refs, _ := lang.ReferencesInExpr(addrs.ParseRef, expr)
-	for _, ref := range refs {
-		if ref.Subject == addr {
-			diags = diags.Append(&hcl.Diagnostic{
-				Severity: hcl.DiagError,
-				Summary:  "Self-referencing local value",
-				Detail:   fmt.Sprintf("Local value %s cannot use its own result as part of its expression.", addr),
-				Subject:  ref.SourceRange.ToHCL().Ptr(),
-				Context:  expr.Range().Ptr(),
-			})
-		}
-	}
-	if diags.HasErrors() {
-		return diags
-	}
-
-	val, moreDiags := ctx.EvaluateExpr(expr, cty.DynamicPseudoType, nil)
-	diags = diags.Append(moreDiags)
-	if moreDiags.HasErrors() {
-		return diags
-	}
-
-	ctx.NamedValues().SetLocalValue(addr.Absolute(ctx.Path()), val)
-
+	namedVals := ctx.NamedValues()
+	val, diags := evaluateLocalValue(n.Config, n.Addr.LocalValue, n.Addr.String(), ctx)
+	namedVals.SetLocalValue(n.Addr, val)
 	return diags
 }
 
@@ -201,8 +175,71 @@ type nodeLocalInPartialModule struct {
 	Config *configs.Local
 }
 
+// Path implements [GraphNodePartialExpandedModule], meaning that the
+// Execute method receives an [EvalContext] that's set up for partial-expanded
+// evaluation instead of full evaluation.
 func (n *nodeLocalInPartialModule) Path() addrs.PartialExpandedModule {
 	return n.Addr.Module
 }
 
-// TODO: Implement nodeLocalUnexpandedPlaceholder.Execute
+func (n *nodeLocalInPartialModule) Execute(ctx EvalContext, op walkOperation) tfdiags.Diagnostics {
+	// Our job here is to make sure that the local value definition is
+	// valid for all instances of this local value across all of the possible
+	// module instances under our partially-expanded prefix, and to record
+	// a placeholder value that captures as precisely as possible what all
+	// of those results have in common. In the worst case where they have
+	// absolutely nothing in common cty.DynamicVal is the ultimate fallback,
+	// but we should try to do better when possible to give operators earlier
+	// feedback about any problems they would definitely encounter on a
+	// subsequent plan where the local values get evaluated concretely.
+
+	namedVals := ctx.NamedValues()
+	val, diags := evaluateLocalValue(n.Config, n.Addr.Local, n.Addr.String(), ctx)
+	namedVals.SetLocalValuePlaceholder(n.Addr, val)
+	return diags
+}
+
+// evaluateLocalValue is the common evaluation logic shared between
+// [NodeLocal] and [nodeLocalInPartialModule].
+//
+// The overall validation and evaluation process is the same for each, with
+// the differences encapsulated inside the given [EvalContext], which is
+// configured in a different way when doing partial-expanded evaluation.
+//
+// the addrStr argument should be the canonical string representation of the
+// anbsolute address of the object being evaluated, which should either be an
+// [addrs.AbsLocalValue] or an [addrs.InPartialEvaluatedModule[addrs.LocalValue]]
+// depending on which of the two callers are calling this function.
+//
+// localAddr should match the local portion of the address that was stringified
+// for addrStr, describing the local value relative to the module it's declared
+// inside.
+func evaluateLocalValue(config *configs.Local, localAddr addrs.LocalValue, addrStr string, ctx EvalContext) (cty.Value, tfdiags.Diagnostics) {
+	var diags tfdiags.Diagnostics
+	expr := config.Expr
+
+	// We ignore diags here because any problems we might find will be found
+	// again in EvaluateExpr below.
+	refs, _ := lang.ReferencesInExpr(addrs.ParseRef, expr)
+	for _, ref := range refs {
+		if ref.Subject == localAddr {
+			diags = diags.Append(&hcl.Diagnostic{
+				Severity: hcl.DiagError,
+				Summary:  "Self-referencing local value",
+				Detail:   fmt.Sprintf("Local value %s cannot use its own result as part of its expression.", addrStr),
+				Subject:  ref.SourceRange.ToHCL().Ptr(),
+				Context:  expr.Range().Ptr(),
+			})
+		}
+	}
+	if diags.HasErrors() {
+		return cty.DynamicVal, diags
+	}
+
+	val, moreDiags := ctx.EvaluateExpr(expr, cty.DynamicPseudoType, nil)
+	diags = diags.Append(moreDiags)
+	if val == cty.NilVal {
+		val = cty.DynamicVal
+	}
+	return val, diags
+}

internal/terraform/node_local_test.go

@@ -8,33 +8,33 @@ import (
 
 	"github.com/hashicorp/hcl/v2"
 	"github.com/hashicorp/hcl/v2/hclsyntax"
+	"github.com/zclconf/go-cty/cty"
 
 	"github.com/hashicorp/terraform/internal/addrs"
 	"github.com/hashicorp/terraform/internal/configs"
-	"github.com/hashicorp/terraform/internal/configs/hcl2shim"
 	"github.com/hashicorp/terraform/internal/namedvals"
 	"github.com/hashicorp/terraform/internal/states"
 )
 
 func TestNodeLocalExecute(t *testing.T) {
 	tests := []struct {
 		Value string
-		Want  interface{}
+		Want  cty.Value
 		Err   bool
 	}{
 		{
 			"hello!",
-			"hello!",
+			cty.StringVal("hello!"),
 			false,
 		},
 		{
 			"",
-			"",
+			cty.StringVal(""),
 			false,
 		},
 		{
 			"Hello, ${local.foo}",
-			nil,
+			cty.DynamicVal,
 			true, // self-referencing
 		},
 	}
@@ -57,7 +57,7 @@ func TestNodeLocalExecute(t *testing.T) {
 				StateState:       states.NewState().SyncWrapper(),
 				NamedValuesState: namedvals.NewState(),
 
-				EvaluateExprResult: hcl2shim.HCL2ValueFromConfigValue(test.Want),
+				EvaluateExprResult: test.Want,
 			}
 
 			err := n.Execute(ctx, walkApply)
@@ -69,19 +69,13 @@ func TestNodeLocalExecute(t *testing.T) {
 				}
 			}
 
-			if test.Err {
-				if ctx.NamedValues().HasLocalValue(localAddr) {
-					t.Errorf("have value for %s, but wanted none", localAddr)
-				}
-			} else {
-				if !ctx.NamedValues().HasLocalValue(localAddr) {
-					t.Fatalf("no value for %s", localAddr)
-				}
-				got := ctx.NamedValues().GetLocalValue(localAddr)
-				want := hcl2shim.HCL2ValueFromConfigValue(test.Want)
-				if !want.RawEquals(got) {
-					t.Errorf("wrong value for %s\ngot:  %#v\nwant: %#v", localAddr, got, want)
-				}
+			if !ctx.NamedValues().HasLocalValue(localAddr) {
+				t.Fatalf("no value for %s", localAddr)
+			}
+			got := ctx.NamedValues().GetLocalValue(localAddr)
+			want := test.Want
+			if !want.RawEquals(got) {
+				t.Errorf("wrong value for %s\ngot:  %#v\nwant: %#v", localAddr, got, want)
 			}
 		})
 	}

internal/terraform/node_module_variable.go

@@ -160,6 +160,11 @@ type nodeModuleVariable struct {
 	// ModuleCallArguments, ex. so count.index and each.key can resolve
 	ModuleInstance addrs.ModuleInstance
 
+	// ModuleCallConfig is the module call that the expression in field Expr
+	// came from, which helps decide what [instances.RepetitionData] we should
+	// use when evaluating Expr.
+	ModuleCallConfig *configs.ModuleCall
+
 	// DestroyApply must be set to true when applying a destroy operation and
 	// false otherwise.
 	DestroyApply bool
@@ -241,12 +246,6 @@ func (n *nodeModuleVariable) DotNode(name string, opts *dag.DotOpts) *dag.DotNod
 // evalModuleVariable produces the value for a particular variable as will
 // be used by a child module instance.
 //
-// The result is written into a map, with its key set to the local name of the
-// variable, disregarding the module instance address. A map is returned instead
-// of a single value as a result of trying to be convenient for use with
-// EvalContext.SetModuleCallArguments, which expects a map to merge in with any
-// existing arguments.
-//
 // validateOnly indicates that this evaluation is only for config
 // validation, and we will not have any expansion module instance
 // repetition data.
@@ -261,11 +260,10 @@ func (n *nodeModuleVariable) evalModuleVariable(ctx EvalContext, validateOnly bo
 		case validateOnly:
 			// the instance expander does not track unknown expansion values, so we
 			// have to assume all RepetitionData is unknown.
-			moduleInstanceRepetitionData = instances.RepetitionData{
-				CountIndex: cty.UnknownVal(cty.Number),
-				EachKey:    cty.UnknownVal(cty.String),
-				EachValue:  cty.DynamicVal,
-			}
+			// TODO: Ideally we should vary the placeholder we use here based
+			// on how the module call repetition was configured, but we don't
+			// have enough information here to decide that.
+			moduleInstanceRepetitionData = instances.TotallyUnknownRepetitionData
 
 		default:
 			// Get the repetition data for this module instance,
@@ -327,4 +325,33 @@ func (n *nodeModuleVariableInPartialModule) Path() addrs.PartialExpandedModule {
 	return n.Addr.Module
 }
 
-// TODO: Implement nodeModuleVariableInPartialModule.Execute
+func (n *nodeModuleVariableInPartialModule) Execute(ctx EvalContext, op walkOperation) tfdiags.Diagnostics {
+	// Our job here is to make sure that the input variable definition is
+	// valid for all instances of this input variable across all of the possible
+	// module instances under our partially-expanded prefix, and to record
+	// a placeholder value that captures as precisely as possible what all
+	// of those results have in common. In the worst case where they have
+	// absolutely nothing in common cty.DynamicVal is the ultimate fallback,
+	// but we should try to do better when possible to give operators earlier
+	// feedback about any problems they would definitely encounter on a
+	// subsequent plan where the input variables get evaluated concretely.
+
+	namedVals := ctx.NamedValues()
+
+	// TODO: Ideally we should vary the placeholder we use here based
+	// on how the module call repetition was configured, but we don't
+	// have enough information here to decide that.
+	moduleInstanceRepetitionData := instances.TotallyUnknownRepetitionData
+
+	// NOTE WELL: Input variables are a little strange in that they announce
+	// themselves as belonging to the caller of the module they are declared
+	// in, because that's where their definition expressions get evaluated.
+	// Therefore this [EvalContext] is in the scope of the parent module,
+	// while n.Addr describes an object in the child module (where the
+	// variable declaration appeared).
+	scope := ctx.EvaluationScope(nil, nil, moduleInstanceRepetitionData)
+	val, diags := scope.EvalExpr(n.Expr, cty.DynamicPseudoType)
+
+	namedVals.SetInputVariablePlaceholder(n.Addr, val)
+	return diags
+}

internal/terraform/node_output.go

@@ -132,6 +132,10 @@ func (n *nodeExpandOutput) DynamicExpand(ctx EvalContext) (*Graph, tfdiags.Diagn
 				Config:      n.Config,
 				RefreshOnly: n.RefreshOnly,
 			}
+			// We don't need to handle the module.IsRoot() && n.Destroying case
+			// seen in the fully-expanded case above, because the root module
+			// instance is always "fully expanded" (it's always a singleton)
+			// and so we can't get here for output values in the root module.
 			log.Printf("[TRACE] Expanding output: adding placeholder for all %s as %T", absAddr.String(), node)
 			g.Add(node)
 		},
@@ -464,11 +468,43 @@ type nodeOutputInPartialModule struct {
 	RefreshOnly bool
 }
 
+// Path implements [GraphNodePartialExpandedModule], meaning that the
+// Execute method receives an [EvalContext] that's set up for partial-expanded
+// evaluation instead of full evaluation.
 func (n *nodeOutputInPartialModule) Path() addrs.PartialExpandedModule {
 	return n.Addr.Module
 }
 
-// TODO: Implement nodeOutputInPartialModule.Execute
+func (n *nodeOutputInPartialModule) Execute(ctx EvalContext, op walkOperation) tfdiags.Diagnostics {
+	// Our job here is to make sure that the output value definition is
+	// valid for all instances of this output value across all of the possible
+	// module instances under our partially-expanded prefix, and to record
+	// a placeholder value that captures as precisely as possible what all
+	// of those results have in common. In the worst case where they have
+	// absolutely nothing in common cty.DynamicVal is the ultimate fallback,
+	// but we should try to do better when possible to give operators earlier
+	// feedback about any problems they would definitely encounter on a
+	// subsequent plan where the output values get evaluated concretely.
+
+	namedVals := ctx.NamedValues()
+
+	// this "ctx" is preconfigured to evaluate in terms of other placeholder
+	// values generated in the same unexpanded module prefix, rather than
+	// from the active state/plan, so this result is likely to be derived
+	// from unknown value placeholders itself.
+	val, diags := ctx.EvaluateExpr(n.Config.Expr, cty.DynamicPseudoType, nil)
+	if val == cty.NilVal {
+		val = cty.DynamicVal
+	}
+
+	// We'll also check that the depends_on argument is valid, since that's
+	// a static concern anyway and so cannot vary between instances of the
+	// same module.
+	diags = diags.Append(validateDependsOn(ctx, n.Config.DependsOn))
+
+	namedVals.SetOutputValuePlaceholder(n.Addr, val)
+	return diags
+}
 
 // NodeDestroyableOutput represents an output that is "destroyable":
 // its application will remove the output from the state.