WIP refactoring input variable processing

internal/terraform/context_apply.go

@@ -31,30 +31,11 @@ func (c *Context) Apply(plan *plans.Plan, config *configs.Config) (*states.State
 		return nil, diags
 	}
 
-	variables := InputValues{}
-	for name, dyVal := range plan.VariableValues {
-		val, err := dyVal.Decode(cty.DynamicPseudoType)
-		if err != nil {
-			diags = diags.Append(tfdiags.Sourceless(
-				tfdiags.Error,
-				"Invalid variable value in plan",
-				fmt.Sprintf("Invalid value for variable %q recorded in plan file: %s.", name, err),
-			))
-			continue
-		}
-
-		variables[name] = &InputValue{
-			Value:      val,
-			SourceType: ValueFromPlan,
-		}
-	}
-
 	workingState := plan.PriorState.DeepCopy()
 	walker, walkDiags := c.walk(graph, operation, &graphWalkOpts{
-		Config:             config,
-		InputState:         workingState,
-		Changes:            plan.Changes,
-		RootVariableValues: variables,
+		Config:     config,
+		InputState: workingState,
+		Changes:    plan.Changes,
 	})
 	diags = diags.Append(walker.NonFatalDiagnostics)
 	diags = diags.Append(walkDiags)
@@ -84,15 +65,43 @@ Note that the -target option is not suitable for routine use, and is provided on
 }
 
 func (c *Context) applyGraph(plan *plans.Plan, config *configs.Config, validate bool) (*Graph, walkOperation, tfdiags.Diagnostics) {
-	graph, diags := (&ApplyGraphBuilder{
-		Config:       config,
-		Changes:      plan.Changes,
-		State:        plan.PriorState,
-		Plugins:      c.plugins,
-		Targets:      plan.TargetAddrs,
-		ForceReplace: plan.ForceReplaceAddrs,
-		Validate:     validate,
+	var diags tfdiags.Diagnostics
+
+	variables := InputValues{}
+	for name, dyVal := range plan.VariableValues {
+		val, err := dyVal.Decode(cty.DynamicPseudoType)
+		if err != nil {
+			diags = diags.Append(tfdiags.Sourceless(
+				tfdiags.Error,
+				"Invalid variable value in plan",
+				fmt.Sprintf("Invalid value for variable %q recorded in plan file: %s.", name, err),
+			))
+			continue
+		}
+
+		variables[name] = &InputValue{
+			Value:      val,
+			SourceType: ValueFromPlan,
+		}
+	}
+	if diags.HasErrors() {
+		return nil, walkApply, diags
+	}
+
+	graph, moreDiags := (&ApplyGraphBuilder{
+		Config:             config,
+		Changes:            plan.Changes,
+		State:              plan.PriorState,
+		RootVariableValues: variables,
+		Plugins:            c.plugins,
+		Targets:            plan.TargetAddrs,
+		ForceReplace:       plan.ForceReplaceAddrs,
+		Validate:           validate,
 	}).Build(addrs.RootModuleInstance)
+	diags = diags.Append(moreDiags)
+	if moreDiags.HasErrors() {
+		return nil, walkApply, diags
+	}
 
 	operation := walkApply
 	if plan.UIMode == plans.DestroyMode {

internal/terraform/context_eval.go

@@ -60,19 +60,19 @@ func (c *Context) Eval(config *configs.Config, state *states.State, moduleAddr a
 	log.Printf("[DEBUG] Building and walking 'eval' graph")
 
 	graph, moreDiags := (&EvalGraphBuilder{
-		Config:  config,
-		State:   state,
-		Plugins: c.plugins,
+		Config:             config,
+		State:              state,
+		RootVariableValues: variables,
+		Plugins:            c.plugins,
 	}).Build(addrs.RootModuleInstance)
 	diags = diags.Append(moreDiags)
 	if moreDiags.HasErrors() {
 		return nil, diags
 	}
 
 	walkOpts := &graphWalkOpts{
-		InputState:         state,
-		Config:             config,
-		RootVariableValues: variables,
+		InputState: state,
+		Config:     config,
 	}
 
 	walker, moreDiags = c.walk(graph, walkEval, walkOpts)

internal/terraform/context_import.go

@@ -53,11 +53,14 @@ func (c *Context) Import(config *configs.Config, prevRunState *states.State, opt
 
 	log.Printf("[DEBUG] Building and walking import graph")
 
+	variables := mergeDefaultInputVariableValues(opts.SetVariables, config.Module.Variables)
+
 	// Initialize our graph builder
 	builder := &ImportGraphBuilder{
-		ImportTargets: opts.Targets,
-		Config:        config,
-		Plugins:       c.plugins,
+		ImportTargets:      opts.Targets,
+		Config:             config,
+		RootVariableValues: variables,
+		Plugins:            c.plugins,
 	}
 
 	// Build the graph
@@ -67,13 +70,10 @@ func (c *Context) Import(config *configs.Config, prevRunState *states.State, opt
 		return state, diags
 	}
 
-	variables := mergeDefaultInputVariableValues(opts.SetVariables, config.Module.Variables)
-
 	// Walk it
 	walker, walkDiags := c.walk(graph, walkImport, &graphWalkOpts{
-		Config:             config,
-		InputState:         state,
-		RootVariableValues: variables,
+		Config:     config,
+		InputState: state,
 	})
 	diags = diags.Append(walkDiags)
 	if walkDiags.HasErrors() {

internal/terraform/context_plan.go

@@ -419,11 +419,10 @@ func (c *Context) planWalk(config *configs.Config, prevRunState *states.State, r
 	// we can now walk.
 	changes := plans.NewChanges()
 	walker, walkDiags := c.walk(graph, walkOp, &graphWalkOpts{
-		Config:             config,
-		InputState:         prevRunState,
-		Changes:            changes,
-		MoveResults:        moveResults,
-		RootVariableValues: rootVariables,
+		Config:      config,
+		InputState:  prevRunState,
+		Changes:     changes,
+		MoveResults: moveResults,
 	})
 	diags = diags.Append(walker.NonFatalDiagnostics)
 	diags = diags.Append(walkDiags)
@@ -469,34 +468,37 @@ func (c *Context) planGraph(config *configs.Config, prevRunState *states.State,
 	switch mode := opts.Mode; mode {
 	case plans.NormalMode:
 		graph, diags := (&PlanGraphBuilder{
-			Config:       config,
-			State:        prevRunState,
-			Plugins:      c.plugins,
-			Targets:      opts.Targets,
-			ForceReplace: opts.ForceReplace,
-			Validate:     validate,
-			skipRefresh:  opts.SkipRefresh,
+			Config:             config,
+			State:              prevRunState,
+			RootVariableValues: opts.SetVariables,
+			Plugins:            c.plugins,
+			Targets:            opts.Targets,
+			ForceReplace:       opts.ForceReplace,
+			Validate:           validate,
+			skipRefresh:        opts.SkipRefresh,
 		}).Build(addrs.RootModuleInstance)
 		return graph, walkPlan, diags
 	case plans.RefreshOnlyMode:
 		graph, diags := (&PlanGraphBuilder{
-			Config:          config,
-			State:           prevRunState,
-			Plugins:         c.plugins,
-			Targets:         opts.Targets,
-			Validate:        validate,
-			skipRefresh:     opts.SkipRefresh,
-			skipPlanChanges: true, // this activates "refresh only" mode.
+			Config:             config,
+			State:              prevRunState,
+			RootVariableValues: opts.SetVariables,
+			Plugins:            c.plugins,
+			Targets:            opts.Targets,
+			Validate:           validate,
+			skipRefresh:        opts.SkipRefresh,
+			skipPlanChanges:    true, // this activates "refresh only" mode.
 		}).Build(addrs.RootModuleInstance)
 		return graph, walkPlan, diags
 	case plans.DestroyMode:
 		graph, diags := (&DestroyPlanGraphBuilder{
-			Config:      config,
-			State:       prevRunState,
-			Plugins:     c.plugins,
-			Targets:     opts.Targets,
-			Validate:    validate,
-			skipRefresh: opts.SkipRefresh,
+			Config:             config,
+			State:              prevRunState,
+			RootVariableValues: opts.SetVariables,
+			Plugins:            c.plugins,
+			Targets:            opts.Targets,
+			Validate:           validate,
+			skipRefresh:        opts.SkipRefresh,
 		}).Build(addrs.RootModuleInstance)
 		return graph, walkPlanDestroy, diags
 	default:

internal/terraform/context_validate.go

@@ -37,17 +37,6 @@ func (c *Context) Validate(config *configs.Config) tfdiags.Diagnostics {
 
 	log.Printf("[DEBUG] Building and walking validate graph")
 
-	graph, moreDiags := ValidateGraphBuilder(&PlanGraphBuilder{
-		Config:   config,
-		Plugins:  c.plugins,
-		Validate: true,
-		State:    states.NewState(),
-	}).Build(addrs.RootModuleInstance)
-	diags = diags.Append(moreDiags)
-	if moreDiags.HasErrors() {
-		return diags
-	}
-
 	// Validate is to check if the given module is valid regardless of
 	// input values, current state, etc. Therefore we populate all of the
 	// input values with unknown values of the expected type, allowing us
@@ -66,9 +55,20 @@ func (c *Context) Validate(config *configs.Config) tfdiags.Diagnostics {
 		}
 	}
 
-	walker, walkDiags := c.walk(graph, walkValidate, &graphWalkOpts{
+	graph, moreDiags := ValidateGraphBuilder(&PlanGraphBuilder{
 		Config:             config,
+		Plugins:            c.plugins,
+		Validate:           true,
+		State:              states.NewState(),
 		RootVariableValues: varValues,
+	}).Build(addrs.RootModuleInstance)
+	diags = diags.Append(moreDiags)
+	if moreDiags.HasErrors() {
+		return diags
+	}
+
+	walker, walkDiags := c.walk(graph, walkValidate, &graphWalkOpts{
+		Config: config,
 	})
 	diags = diags.Append(walker.NonFatalDiagnostics)
 	diags = diags.Append(walkDiags)

internal/terraform/context_walk.go

@@ -23,8 +23,7 @@ type graphWalkOpts struct {
 	Changes    *plans.Changes
 	Config     *configs.Config
 
-	RootVariableValues InputValues
-	MoveResults        refactoring.MoveResults
+	MoveResults refactoring.MoveResults
 }
 
 func (c *Context) walk(graph *Graph, operation walkOperation, opts *graphWalkOpts) (*ContextGraphWalker, tfdiags.Diagnostics) {
@@ -98,16 +97,15 @@ func (c *Context) graphWalker(operation walkOperation, opts *graphWalkOpts) *Con
 	}
 
 	return &ContextGraphWalker{
-		Context:            c,
-		State:              state,
-		Config:             opts.Config,
-		RefreshState:       refreshState,
-		PrevRunState:       prevRunState,
-		Changes:            changes.SyncWrapper(),
-		InstanceExpander:   instances.NewExpander(),
-		MoveResults:        opts.MoveResults,
-		Operation:          operation,
-		StopContext:        c.runContext,
-		RootVariableValues: opts.RootVariableValues,
+		Context:          c,
+		State:            state,
+		Config:           opts.Config,
+		RefreshState:     refreshState,
+		PrevRunState:     prevRunState,
+		Changes:          changes.SyncWrapper(),
+		InstanceExpander: instances.NewExpander(),
+		MoveResults:      opts.MoveResults,
+		Operation:        operation,
+		StopContext:      c.runContext,
 	}
 }

internal/terraform/eval_context.go

@@ -121,12 +121,24 @@ type EvalContext interface {
 	// addresses in this context.
 	EvaluationScope(self addrs.Referenceable, keyData InstanceKeyEvalData) *lang.Scope
 
-	// SetModuleCallArguments defines values for the variables of a particular
-	// child module call.
+	// SetRootModuleArgument defines the value for one variable of the root
+	// module. The caller must ensure that given value is a suitable
+	// "final value" for the variable, which means that it's already converted
+	// and validated to match any configured constraints and validation rules.
 	//
-	// Calling this function multiple times has merging behavior, keeping any
-	// previously-set keys that are not present in the new map.
-	SetModuleCallArguments(addrs.ModuleCallInstance, map[string]cty.Value)
+	// Calling this function multiple times with the same variable address
+	// will silently overwrite the value provided by a previous call.
+	SetRootModuleArgument(addrs.InputVariable, cty.Value)
+
+	// SetModuleCallArgument the values for one input variable of a particular
+	// child module call. The caller must ensure that the given value is a
+	// suitable "final value" for the variable, which means that it's already
+	// converted and validated to match any configured constraints and
+	// validation rules.
+	//
+	// Calling this function multiple times with the same variable address
+	// will silently overwrite the value provided by a previous call.
+	SetModuleCallArgument(addrs.ModuleCallInstance, addrs.InputVariable, cty.Value)
 
 	// GetVariableValue returns the value provided for the input variable with
 	// the given address, or cty.DynamicVal if the variable hasn't been assigned

internal/terraform/eval_context_builtin.go

@@ -313,26 +313,39 @@ func (ctx *BuiltinEvalContext) Path() addrs.ModuleInstance {
 	return ctx.PathValue
 }
 
-func (ctx *BuiltinEvalContext) SetModuleCallArguments(n addrs.ModuleCallInstance, vals map[string]cty.Value) {
+func (ctx *BuiltinEvalContext) SetRootModuleArgument(addr addrs.InputVariable, v cty.Value) {
+	ctx.VariableValuesLock.Lock()
+	defer ctx.VariableValuesLock.Unlock()
+
+	log.Printf("[TRACE] BuiltinEvalContext: Storing final value for variable %s", addr.Absolute(addrs.RootModuleInstance))
+	key := addrs.RootModuleInstance.String()
+	args := ctx.VariableValues[key]
+	if args == nil {
+		args = make(map[string]cty.Value)
+		ctx.VariableValues[key] = args
+		return
+	}
+	args[addr.Name] = v
+}
+
+func (ctx *BuiltinEvalContext) SetModuleCallArgument(callAddr addrs.ModuleCallInstance, varAddr addrs.InputVariable, v cty.Value) {
 	ctx.VariableValuesLock.Lock()
 	defer ctx.VariableValuesLock.Unlock()
 
 	if !ctx.pathSet {
 		panic("context path not set")
 	}
 
-	childPath := n.ModuleInstance(ctx.PathValue)
+	childPath := callAddr.ModuleInstance(ctx.PathValue)
+	log.Printf("[TRACE] BuiltinEvalContext: Storing final value for variable %s", varAddr.Absolute(childPath))
 	key := childPath.String()
-
 	args := ctx.VariableValues[key]
 	if args == nil {
-		ctx.VariableValues[key] = vals
+		args = make(map[string]cty.Value)
+		ctx.VariableValues[key] = args
 		return
 	}
-
-	for k, v := range vals {
-		args[k] = v
-	}
+	args[varAddr.Name] = v
 }
 
 func (ctx *BuiltinEvalContext) GetVariableValue(addr addrs.AbsInputVariableInstance) cty.Value {