diff --git a/internal/terraform/context_apply.go b/internal/terraform/context_apply.go index e94449b7261d..d4167e2a1158 100644 --- a/internal/terraform/context_apply.go +++ b/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 { diff --git a/internal/terraform/context_eval.go b/internal/terraform/context_eval.go index efc24767c205..c6af77635fc3 100644 --- a/internal/terraform/context_eval.go +++ b/internal/terraform/context_eval.go @@ -60,9 +60,10 @@ 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() { @@ -70,9 +71,8 @@ func (c *Context) Eval(config *configs.Config, state *states.State, moduleAddr a } walkOpts := &graphWalkOpts{ - InputState: state, - Config: config, - RootVariableValues: variables, + InputState: state, + Config: config, } walker, moreDiags = c.walk(graph, walkEval, walkOpts) diff --git a/internal/terraform/context_import.go b/internal/terraform/context_import.go index af17cbd62dc8..b5417405fbb0 100644 --- a/internal/terraform/context_import.go +++ b/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() { diff --git a/internal/terraform/context_plan.go b/internal/terraform/context_plan.go index 3f860ef1bd1f..06fe83afdcd2 100644 --- a/internal/terraform/context_plan.go +++ b/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: diff --git a/internal/terraform/context_validate.go b/internal/terraform/context_validate.go index fb54be420221..4fb02f767164 100644 --- a/internal/terraform/context_validate.go +++ b/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) diff --git a/internal/terraform/context_walk.go b/internal/terraform/context_walk.go index 166341513cce..e8b506314ede 100644 --- a/internal/terraform/context_walk.go +++ b/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, } } diff --git a/internal/terraform/eval_context.go b/internal/terraform/eval_context.go index 4b5a3a5c2cac..d43244e379d9 100644 --- a/internal/terraform/eval_context.go +++ b/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 diff --git a/internal/terraform/eval_context_builtin.go b/internal/terraform/eval_context_builtin.go index ecbac446e7cc..f228d245f1b2 100644 --- a/internal/terraform/eval_context_builtin.go +++ b/internal/terraform/eval_context_builtin.go @@ -313,7 +313,22 @@ 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() @@ -321,18 +336,16 @@ func (ctx *BuiltinEvalContext) SetModuleCallArguments(n addrs.ModuleCallInstance 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 { diff --git a/internal/terraform/eval_context_mock.go b/internal/terraform/eval_context_mock.go index edcdaac6b62c..ffc3462a51a7 100644 --- a/internal/terraform/eval_context_mock.go +++ b/internal/terraform/eval_context_mock.go @@ -111,9 +111,14 @@ type MockEvalContext struct { PathCalled bool PathPath addrs.ModuleInstance - SetModuleCallArgumentsCalled bool - SetModuleCallArgumentsModule addrs.ModuleCallInstance - SetModuleCallArgumentsValues map[string]cty.Value + SetRootModuleArgumentCalled bool + SetRootModuleArgumentAddr addrs.InputVariable + SetRootModuleArgumentValue cty.Value + + SetModuleCallArgumentCalled bool + SetModuleCallArgumentModuleCall addrs.ModuleCallInstance + SetModuleCallArgumentVariable addrs.InputVariable + SetModuleCallArgumentValue cty.Value GetVariableValueCalled bool GetVariableValueAddr addrs.AbsInputVariableInstance @@ -321,10 +326,17 @@ func (c *MockEvalContext) Path() addrs.ModuleInstance { return c.PathPath } -func (c *MockEvalContext) SetModuleCallArguments(n addrs.ModuleCallInstance, values map[string]cty.Value) { - c.SetModuleCallArgumentsCalled = true - c.SetModuleCallArgumentsModule = n - c.SetModuleCallArgumentsValues = values +func (c *MockEvalContext) SetRootModuleArgument(addr addrs.InputVariable, v cty.Value) { + c.SetRootModuleArgumentCalled = true + c.SetRootModuleArgumentAddr = addr + c.SetRootModuleArgumentValue = v +} + +func (c *MockEvalContext) SetModuleCallArgument(callAddr addrs.ModuleCallInstance, varAddr addrs.InputVariable, v cty.Value) { + c.SetModuleCallArgumentCalled = true + c.SetModuleCallArgumentModuleCall = callAddr + c.SetModuleCallArgumentVariable = varAddr + c.SetModuleCallArgumentValue = v } func (c *MockEvalContext) GetVariableValue(addr addrs.AbsInputVariableInstance) cty.Value { diff --git a/internal/terraform/evaluate.go b/internal/terraform/evaluate.go index 8fd05d548192..b0f9b9353691 100644 --- a/internal/terraform/evaluate.go +++ b/internal/terraform/evaluate.go @@ -10,7 +10,6 @@ import ( "github.com/agext/levenshtein" "github.com/hashicorp/hcl/v2" "github.com/zclconf/go-cty/cty" - "github.com/zclconf/go-cty/cty/convert" "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/configs" @@ -248,7 +247,7 @@ func (d *evaluationStateData) GetInputVariable(addr addrs.InputVariable, rng tfd // This is important because otherwise the validation walk will tend to be // overly strict, requiring expressions throughout the configuration to // be complicated to accommodate all possible inputs, whereas returning - // known here allows for simpler patterns like using input values as + // unknown here allows for simpler patterns like using input values as // guards to broadly enable/disable resources, avoid processing things // that are disabled, etc. Terraform's static validation leans towards // being liberal in what it accepts because the subsequent plan walk has @@ -267,28 +266,27 @@ func (d *evaluationStateData) GetInputVariable(addr addrs.InputVariable, rng tfd return cty.UnknownVal(config.Type), diags } - val, isSet := vals[addr.Name] - switch { - case !isSet: - // The config loader will ensure there is a default if the value is not - // set at all. - val = config.Default - - case val.IsNull() && !config.Nullable && config.Default != cty.NilVal: - // If nullable=false a null value will use the configured default. - val = config.Default - } + // d.Evaluator.VariableValues should always contain valid "final values" + // for variables, which is to say that they have already had type + // conversions, validations, and default value handling applied to them. + // Those are the responsibility of the graph notes representing the + // variable declarations. Therefore here we just trust that we already + // have a correct value. - var err error - val, err = convert.Convert(val, config.ConstraintType) - if err != nil { - // We should never get here because this problem should've been caught - // during earlier validation, but we'll do something reasonable anyway. + val, isSet := vals[addr.Name] + if !isSet { + // We should not be able to get here without having a valid value + // for every variable, so this always indicates a bug in either + // the graph builder (not including all the needed nodes) or in + // the graph nodes representing variables. diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, - Summary: `Incorrect variable type`, - Detail: fmt.Sprintf(`The resolved value of variable %q is not appropriate: %s.`, addr.Name, err), - Subject: &config.DeclRange, + Summary: `Reference to unresolved input variable`, + Detail: fmt.Sprintf( + `The final value for %s is missing in Terraform's evaluation context. This is a bug in Terraform; please report it!`, + addr.Absolute(d.ModulePath), + ), + Subject: rng.ToHCL().Ptr(), }) val = cty.UnknownVal(config.Type) } diff --git a/internal/terraform/graph_builder_apply.go b/internal/terraform/graph_builder_apply.go index 75f9d3d4ad25..86d825560276 100644 --- a/internal/terraform/graph_builder_apply.go +++ b/internal/terraform/graph_builder_apply.go @@ -26,6 +26,11 @@ type ApplyGraphBuilder struct { // State is the current state State *states.State + // RootVariableValues are the root module input variables captured as + // part of the plan object, which we must reproduce in the apply step + // to get a consistent result. + RootVariableValues InputValues + // Plugins is a library of the plug-in components (providers and // provisioners) available for use. Plugins *contextPlugins @@ -88,7 +93,7 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer { }, // Add dynamic values - &RootVariableTransformer{Config: b.Config}, + &RootVariableTransformer{Config: b.Config, RawValues: b.RootVariableValues}, &ModuleVariableTransformer{Config: b.Config}, &LocalTransformer{Config: b.Config}, &OutputTransformer{Config: b.Config, Changes: b.Changes}, diff --git a/internal/terraform/graph_builder_destroy_plan.go b/internal/terraform/graph_builder_destroy_plan.go index 0bac6305e08f..def1aa3739aa 100644 --- a/internal/terraform/graph_builder_destroy_plan.go +++ b/internal/terraform/graph_builder_destroy_plan.go @@ -23,6 +23,11 @@ type DestroyPlanGraphBuilder struct { // State is the current state State *states.State + // RootVariableValues are the raw input values for root input variables + // given by the caller, which we'll resolve into final values as part + // of the plan walk. + RootVariableValues InputValues + // Plugins is a library of plug-in components (providers and // provisioners) available for use. Plugins *contextPlugins diff --git a/internal/terraform/graph_builder_eval.go b/internal/terraform/graph_builder_eval.go index ee9d6b8e83de..78031e21f9f3 100644 --- a/internal/terraform/graph_builder_eval.go +++ b/internal/terraform/graph_builder_eval.go @@ -30,6 +30,11 @@ type EvalGraphBuilder struct { // State is the current state State *states.State + // RootVariableValues are the raw input values for root input variables + // given by the caller, which we'll resolve into final values as part + // of the plan walk. + RootVariableValues InputValues + // Plugins is a library of plug-in components (providers and // provisioners) available for use. Plugins *contextPlugins @@ -60,7 +65,7 @@ func (b *EvalGraphBuilder) Steps() []GraphTransformer { }, // Add dynamic values - &RootVariableTransformer{Config: b.Config}, + &RootVariableTransformer{Config: b.Config, RawValues: b.RootVariableValues}, &ModuleVariableTransformer{Config: b.Config}, &LocalTransformer{Config: b.Config}, &OutputTransformer{Config: b.Config}, diff --git a/internal/terraform/graph_builder_import.go b/internal/terraform/graph_builder_import.go index 9910354cf5f9..d8d609ebaa85 100644 --- a/internal/terraform/graph_builder_import.go +++ b/internal/terraform/graph_builder_import.go @@ -17,6 +17,11 @@ type ImportGraphBuilder struct { // Module is a configuration to build the graph from. See ImportOpts.Config. Config *configs.Config + // RootVariableValues are the raw input values for root input variables + // given by the caller, which we'll resolve into final values as part + // of the plan walk. + RootVariableValues InputValues + // Plugins is a library of plug-in components (providers and // provisioners) available for use. Plugins *contextPlugins @@ -53,7 +58,7 @@ func (b *ImportGraphBuilder) Steps() []GraphTransformer { &ConfigTransformer{Config: config}, // Add dynamic values - &RootVariableTransformer{Config: b.Config}, + &RootVariableTransformer{Config: b.Config, RawValues: b.RootVariableValues}, &ModuleVariableTransformer{Config: b.Config}, &LocalTransformer{Config: b.Config}, &OutputTransformer{Config: b.Config}, diff --git a/internal/terraform/graph_builder_plan.go b/internal/terraform/graph_builder_plan.go index 709b917b6733..1b8ce58338a9 100644 --- a/internal/terraform/graph_builder_plan.go +++ b/internal/terraform/graph_builder_plan.go @@ -28,6 +28,11 @@ type PlanGraphBuilder struct { // State is the current state State *states.State + // RootVariableValues are the raw input values for root input variables + // given by the caller, which we'll resolve into final values as part + // of the plan walk. + RootVariableValues InputValues + // Plugins is a library of plug-in components (providers and // provisioners) available for use. Plugins *contextPlugins @@ -95,7 +100,7 @@ func (b *PlanGraphBuilder) Steps() []GraphTransformer { }, // Add dynamic values - &RootVariableTransformer{Config: b.Config}, + &RootVariableTransformer{Config: b.Config, RawValues: b.RootVariableValues}, &ModuleVariableTransformer{Config: b.Config}, &LocalTransformer{Config: b.Config}, &OutputTransformer{Config: b.Config}, diff --git a/internal/terraform/node_module_variable.go b/internal/terraform/node_module_variable.go index 9f15587eca1b..321cd874845f 100644 --- a/internal/terraform/node_module_variable.go +++ b/internal/terraform/node_module_variable.go @@ -12,7 +12,6 @@ import ( "github.com/hashicorp/terraform/internal/lang" "github.com/hashicorp/terraform/internal/tfdiags" "github.com/zclconf/go-cty/cty" - "github.com/zclconf/go-cty/cty/convert" ) // nodeExpandModuleVariable is the placeholder for an variable that has not yet had @@ -143,35 +142,27 @@ func (n *nodeModuleVariable) ModulePath() addrs.Module { // GraphNodeExecutable func (n *nodeModuleVariable) Execute(ctx EvalContext, op walkOperation) (diags tfdiags.Diagnostics) { - // If we have no value, do nothing - if n.Expr == nil { - return nil - } + log.Printf("[TRACE] nodeModuleVariable: evaluating %s", n.Addr) - // Otherwise, interpolate the value of this variable and set it - // within the variables mapping. - var vals map[string]cty.Value + var val cty.Value var err error switch op { case walkValidate: - vals, err = n.evalModuleCallArgument(ctx, true) + val, err = n.evalModuleCallArgument(ctx, true) diags = diags.Append(err) - if diags.HasErrors() { - return diags - } default: - vals, err = n.evalModuleCallArgument(ctx, false) + val, err = n.evalModuleCallArgument(ctx, false) diags = diags.Append(err) - if diags.HasErrors() { - return diags - } + } + if diags.HasErrors() { + return diags } // Set values for arguments of a child module call, for later retrieval // during expression evaluation. _, call := n.Addr.Module.CallInstance() - ctx.SetModuleCallArguments(call, vals) + ctx.SetModuleCallArgument(call, n.Addr.Variable, val) return evalVariableValidations(n.Addr, n.Config, n.Expr, ctx) } @@ -199,77 +190,45 @@ func (n *nodeModuleVariable) DotNode(name string, opts *dag.DotOpts) *dag.DotNod // validateOnly indicates that this evaluation is only for config // validation, and we will not have any expansion module instance // repetition data. -func (n *nodeModuleVariable) evalModuleCallArgument(ctx EvalContext, validateOnly bool) (map[string]cty.Value, error) { - name := n.Addr.Variable.Name - expr := n.Expr - - if expr == nil { - // Should never happen, but we'll bail out early here rather than - // crash in case it does. We set no value at all in this case, - // making a subsequent call to EvalContext.SetModuleCallArguments - // a no-op. - log.Printf("[ERROR] attempt to evaluate %s with nil expression", n.Addr.String()) - return nil, nil - } - - var moduleInstanceRepetitionData instances.RepetitionData - - switch { - 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, +func (n *nodeModuleVariable) evalModuleCallArgument(ctx EvalContext, validateOnly bool) (cty.Value, error) { + var diags tfdiags.Diagnostics + var givenVal cty.Value + var errSourceRange tfdiags.SourceRange + if expr := n.Expr; expr != nil { + var moduleInstanceRepetitionData instances.RepetitionData + + switch { + 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, + } + + default: + // Get the repetition data for this module instance, + // so we can create the appropriate scope for evaluating our expression + moduleInstanceRepetitionData = ctx.InstanceExpander().GetModuleInstanceRepetitionData(n.ModuleInstance) } - default: - // Get the repetition data for this module instance, - // so we can create the appropriate scope for evaluating our expression - moduleInstanceRepetitionData = ctx.InstanceExpander().GetModuleInstanceRepetitionData(n.ModuleInstance) - } - - scope := ctx.EvaluationScope(nil, moduleInstanceRepetitionData) - val, diags := scope.EvalExpr(expr, cty.DynamicPseudoType) - - // We intentionally passed DynamicPseudoType to EvalExpr above because - // now we can do our own local type conversion and produce an error message - // with better context if it fails. - var convErr error - val, convErr = convert.Convert(val, n.Config.ConstraintType) - if convErr != nil { - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Invalid value for module argument", - Detail: fmt.Sprintf( - "The given value is not suitable for child module variable %q defined at %s: %s.", - name, n.Config.DeclRange.String(), convErr, - ), - Subject: expr.Range().Ptr(), - }) - // We'll return a placeholder unknown value to avoid producing - // redundant downstream errors. - val = cty.UnknownVal(n.Config.Type) - } - - // If there is no default, we have to ensure that a null value is allowed - // for this variable. - if n.Config.Default == cty.NilVal && !n.Config.Nullable && val.IsNull() { - // The value cannot be null, and there is no configured default. - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: `Invalid variable value`, - Detail: fmt.Sprintf(`The variable %q is required, but the given value is null.`, n.Addr), - Subject: &n.Config.DeclRange, - }) - // Stub out our return value so that the semantic checker doesn't - // produce redundant downstream errors. - val = cty.UnknownVal(n.Config.Type) + scope := ctx.EvaluationScope(nil, moduleInstanceRepetitionData) + val, moreDiags := scope.EvalExpr(expr, cty.DynamicPseudoType) + diags = diags.Append(moreDiags) + if moreDiags.HasErrors() { + return cty.DynamicVal, diags.ErrWithWarnings() + } + givenVal = val + errSourceRange = tfdiags.SourceRangeFromHCL(expr.Range()) + } else { + // We'll use cty.NilVal to represent the variable not being set at all. + givenVal = cty.NilVal + errSourceRange = tfdiags.SourceRangeFromHCL(n.Config.DeclRange) // we use the declaration range as a fallback for an undefined variable } - vals := make(map[string]cty.Value) - vals[name] = val + finalVal, moreDiags := prepareFinalInputVariableValue(n.Addr, givenVal, errSourceRange, n.Config) + diags = diags.Append(moreDiags) - return vals, diags.ErrWithWarnings() + return finalVal, diags.ErrWithWarnings() } diff --git a/internal/terraform/node_root_variable.go b/internal/terraform/node_root_variable.go index 56ee5149a571..d023be3500ba 100644 --- a/internal/terraform/node_root_variable.go +++ b/internal/terraform/node_root_variable.go @@ -1,16 +1,26 @@ package terraform import ( + "log" + "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/configs" "github.com/hashicorp/terraform/internal/dag" "github.com/hashicorp/terraform/internal/tfdiags" + "github.com/zclconf/go-cty/cty" ) // NodeRootVariable represents a root variable input. type NodeRootVariable struct { Addr addrs.InputVariable Config *configs.Variable + + // RawValue is the value for the variable set from outside Terraform + // Core, such as on the command line, or from an environment variable, + // or similar. This is the raw value that was provided, not yet + // converted or validated, and can be nil for a variable that isn't + // set at all. + RawValue *InputValue } var ( @@ -38,21 +48,56 @@ func (n *NodeRootVariable) ReferenceableAddrs() []addrs.Referenceable { // GraphNodeExecutable func (n *NodeRootVariable) Execute(ctx EvalContext, op walkOperation) tfdiags.Diagnostics { - // We don't actually need to _evaluate_ a root module variable, because - // its value is always constant and already stashed away in our EvalContext. - // However, we might need to run some user-defined validation rules against - // the value. + // Root module variables are special in that they are provided directly + // by the caller (usually, the CLI layer) and so we don't really need to + // evaluate them in the usual sense, but we do need to process the raw + // values given by the caller to match what the module is expecting, and + // make sure the values are valid. + var diags tfdiags.Diagnostics + + addr := addrs.RootModuleInstance.InputVariable(n.Addr.Name) + log.Printf("[TRACE] NodeRootVariable: evaluating %s", addr) + + if n.Config == nil { + // Because we build NodeRootVariable from configuration in the normal + // case it's strange to get here, but we tolerate it to allow for + // tests that might not populate the inputs fully. + return nil + } - if n.Config == nil || len(n.Config.Validations) == 0 { - return nil // nothing to do + var givenVal cty.Value + if n.RawValue != nil { + givenVal = n.RawValue.Value + } else { + // We'll use cty.NilVal to represent the variable not being set at + // all, which for historical reasons is unfortunately different than + // explicitly setting it to null in some cases. + givenVal = cty.NilVal } - return evalVariableValidations( + finalVal, moreDiags := prepareFinalInputVariableValue( + addr, + givenVal, + tfdiags.SourceRangeFromHCL(n.Config.DeclRange), + n.Config, + ) + diags = diags.Append(moreDiags) + if moreDiags.HasErrors() { + // No point in proceeding to validations then, because they'll + // probably fail trying to work with a value of the wrong type. + return diags + } + + ctx.SetRootModuleArgument(addr.Variable, finalVal) + + moreDiags = evalVariableValidations( addrs.RootModuleInstance.InputVariable(n.Addr.Name), n.Config, nil, // not set for root module variables ctx, ) + diags = diags.Append(moreDiags) + return diags } // dag.GraphNodeDotter impl. diff --git a/internal/terraform/transform_variable.go b/internal/terraform/transform_variable.go index 86bd6a981aff..4262ea3d6db0 100644 --- a/internal/terraform/transform_variable.go +++ b/internal/terraform/transform_variable.go @@ -13,6 +13,8 @@ import ( // reach them. type RootVariableTransformer struct { Config *configs.Config + + RawValues InputValues } func (t *RootVariableTransformer) Transform(g *Graph) error { @@ -31,7 +33,8 @@ func (t *RootVariableTransformer) Transform(g *Graph) error { Addr: addrs.InputVariable{ Name: v.Name, }, - Config: v, + Config: v, + RawValue: t.RawValues[v.Name], } g.Add(node) } diff --git a/internal/terraform/variables.go b/internal/terraform/variables.go index 7a6ace0eee5c..b086d19ce00b 100644 --- a/internal/terraform/variables.go +++ b/internal/terraform/variables.go @@ -2,11 +2,13 @@ package terraform import ( "fmt" + "log" "github.com/hashicorp/hcl/v2" "github.com/zclconf/go-cty/cty" "github.com/zclconf/go-cty/cty/convert" + "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/configs" "github.com/hashicorp/terraform/internal/tfdiags" ) @@ -322,3 +324,99 @@ func checkInputVariables(vcs map[string]*configs.Variable, vs InputValues) tfdia return diags } + +func prepareFinalInputVariableValue(addr addrs.AbsInputVariableInstance, given cty.Value, valRange tfdiags.SourceRange, cfg *configs.Variable) (cty.Value, tfdiags.Diagnostics) { + var diags tfdiags.Diagnostics + + convertTy := cfg.ConstraintType + log.Printf("[TRACE] prepareFinalInputVariableValue: preparing %s", addr) + + var defaultVal cty.Value + if cfg.Default != cty.NilVal { + log.Printf("[TRACE] prepareFinalInputVariableValue: %s has a default value", addr) + var err error + defaultVal, err = convert.Convert(cfg.Default, convertTy) + if err != nil { + // Validation of the declaration should typically catch this, + // but we'll check it here too to be robust. + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid default value for module argument", + Detail: fmt.Sprintf( + "The default value for variable %q is incompatible with its type constraint: %s.", + cfg.Name, err, + ), + Subject: &cfg.DeclRange, + }) + // We'll return a placeholder unknown value to avoid producing + // redundant downstream errors. + return cty.UnknownVal(cfg.Type), diags + } + } + + if given == cty.NilVal { // The variable wasn't set at all (even to null) + log.Printf("[TRACE] prepareFinalInputVariableValue: %s has no defined value", addr) + if cfg.Required() { + // NOTE: The CLI layer typically checks for itself whether all of + // the required _root_ module variables are not set, which would + // mask this error. We can get here for child module variables, + // though. + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: `Required variable not set`, + Detail: fmt.Sprintf(`The variable %q is required, but is not set.`, addr.Variable.Name), + Subject: valRange.ToHCL().Ptr(), + }) + // We'll return a placeholder unknown value to avoid producing + // redundant downstream errors. + return cty.UnknownVal(cfg.Type), diags + } + + given = defaultVal // must be set, because we checked above that the variable isn't required + } + + val, err := convert.Convert(given, convertTy) + if err != nil { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid value for module argument", + Detail: fmt.Sprintf( + "The given value is not suitable for child module variable %q defined at %s: %s.", + cfg.Name, cfg.DeclRange.String(), err, + ), + Subject: valRange.ToHCL().Ptr(), + }) + // We'll return a placeholder unknown value to avoid producing + // redundant downstream errors. + return cty.UnknownVal(cfg.Type), diags + } + + // By the time we get here, we know: + // - val matches the variable's type constraint + // - val is definitely not cty.NilVal, but might be a null value if the given was already null. + // + // That means we just need to handle the case where the value is null, + // which might mean we need to use the default value, or produce an error. + // + // For historical reasons we do this only for a "non-nullable" variable. + // Nullable variables just appear as null if they were set to null, + // regardless of any default value. + if val.IsNull() && !cfg.Nullable { + log.Printf("[TRACE] prepareFinalInputVariableValue: %s is defined as null", addr) + if defaultVal != cty.NilVal { + val = defaultVal + } else { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: `Required variable not set`, + Detail: fmt.Sprintf(`The variable %q is required, but the given value is null.`, addr.Variable.Name), + Subject: valRange.ToHCL().Ptr(), + }) + // Stub out our return value so that the semantic checker doesn't + // produce redundant downstream errors. + val = cty.UnknownVal(cfg.Type) + } + } + + return val, diags +} diff --git a/internal/terraform/variables_test.go b/internal/terraform/variables_test.go index 41decbae2a38..0c02fad16d02 100644 --- a/internal/terraform/variables_test.go +++ b/internal/terraform/variables_test.go @@ -1,9 +1,11 @@ package terraform import ( + "fmt" "testing" "github.com/davecgh/go-spew/spew" + "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/tfdiags" "github.com/go-test/deep" @@ -280,3 +282,418 @@ func TestCheckInputVariables(t *testing.T) { } }) } + +func TestPrepareFinalInputVariableValue(t *testing.T) { + // This is just a concise way to define a bunch of *configs.Variable + // objects to use in our tests below. We're only going to decode this + // config, not fully evaluate it. + cfgSrc := ` + variable "nullable_required" { + } + variable "nullable_optional_default_string" { + default = "hello" + } + variable "nullable_optional_default_null" { + default = null + } + variable "constrained_string_nullable_required" { + type = string + } + variable "constrained_string_nullable_optional_default_string" { + type = string + default = "hello" + } + variable "constrained_string_nullable_optional_default_bool" { + type = string + default = true + } + variable "constrained_string_nullable_optional_default_null" { + type = string + default = null + } + variable "required" { + nullable = false + } + variable "optional_default_string" { + nullable = false + default = "hello" + } + variable "constrained_string_required" { + nullable = false + type = string + } + variable "constrained_string_optional_default_string" { + nullable = false + type = string + default = "hello" + } + variable "constrained_string_optional_default_bool" { + nullable = false + type = string + default = true + } + ` + cfg := testModuleInline(t, map[string]string{ + "main.tf": cfgSrc, + }) + variableConfigs := cfg.Module.Variables + + tests := []struct { + varName string + given cty.Value + want cty.Value + wantErr string + }{ + // nullable_required + { + "nullable_required", + cty.NilVal, + cty.UnknownVal(cty.DynamicPseudoType), + `Required variable not set: The variable "nullable_required" is required, but is not set.`, + }, + { + "nullable_required", + cty.NullVal(cty.DynamicPseudoType), + cty.NullVal(cty.DynamicPseudoType), + ``, // "required" for a nullable variable means only that it must be set, even if it's set to null + }, + { + "nullable_required", + cty.StringVal("ahoy"), + cty.StringVal("ahoy"), + ``, + }, + { + "nullable_required", + cty.UnknownVal(cty.String), + cty.UnknownVal(cty.String), + ``, + }, + + // nullable_optional_default_string + { + "nullable_optional_default_string", + cty.NilVal, + cty.StringVal("hello"), // the declared default value + ``, + }, + { + "nullable_optional_default_string", + cty.NullVal(cty.DynamicPseudoType), + cty.NullVal(cty.DynamicPseudoType), // nullable variables can be really set to null, masking the default + ``, + }, + { + "nullable_optional_default_string", + cty.StringVal("ahoy"), + cty.StringVal("ahoy"), + ``, + }, + { + "nullable_optional_default_string", + cty.UnknownVal(cty.String), + cty.UnknownVal(cty.String), + ``, + }, + + // nullable_optional_default_null + { + "nullable_optional_default_null", + cty.NilVal, + cty.NullVal(cty.DynamicPseudoType), // the declared default value + ``, + }, + { + "nullable_optional_default_null", + cty.NullVal(cty.String), + cty.NullVal(cty.String), // nullable variables can be really set to null, masking the default + ``, + }, + { + "nullable_optional_default_null", + cty.StringVal("ahoy"), + cty.StringVal("ahoy"), + ``, + }, + { + "nullable_optional_default_null", + cty.UnknownVal(cty.String), + cty.UnknownVal(cty.String), + ``, + }, + + // constrained_string_nullable_required + { + "constrained_string_nullable_required", + cty.NilVal, + cty.UnknownVal(cty.String), + `Required variable not set: The variable "constrained_string_nullable_required" is required, but is not set.`, + }, + { + "constrained_string_nullable_required", + cty.NullVal(cty.DynamicPseudoType), + cty.NullVal(cty.String), // the null value still gets converted to match the type constraint + ``, // "required" for a nullable variable means only that it must be set, even if it's set to null + }, + { + "constrained_string_nullable_required", + cty.StringVal("ahoy"), + cty.StringVal("ahoy"), + ``, + }, + { + "constrained_string_nullable_required", + cty.UnknownVal(cty.String), + cty.UnknownVal(cty.String), + ``, + }, + + // constrained_string_nullable_optional_default_string + { + "constrained_string_nullable_optional_default_string", + cty.NilVal, + cty.StringVal("hello"), // the declared default value + ``, + }, + { + "constrained_string_nullable_optional_default_string", + cty.NullVal(cty.DynamicPseudoType), + cty.NullVal(cty.String), // nullable variables can be really set to null, masking the default + ``, + }, + { + "constrained_string_nullable_optional_default_string", + cty.StringVal("ahoy"), + cty.StringVal("ahoy"), + ``, + }, + { + "constrained_string_nullable_optional_default_string", + cty.UnknownVal(cty.String), + cty.UnknownVal(cty.String), + ``, + }, + + // constrained_string_nullable_optional_default_bool + { + "constrained_string_nullable_optional_default_bool", + cty.NilVal, + cty.StringVal("true"), // the declared default value, automatically converted to match type constraint + ``, + }, + { + "constrained_string_nullable_optional_default_bool", + cty.NullVal(cty.DynamicPseudoType), + cty.NullVal(cty.String), // nullable variables can be really set to null, masking the default + ``, + }, + { + "constrained_string_nullable_optional_default_bool", + cty.StringVal("ahoy"), + cty.StringVal("ahoy"), + ``, + }, + { + "constrained_string_nullable_optional_default_bool", + cty.UnknownVal(cty.String), + cty.UnknownVal(cty.String), + ``, + }, + + // constrained_string_nullable_optional_default_null + { + "constrained_string_nullable_optional_default_null", + cty.NilVal, + cty.NullVal(cty.String), + ``, + }, + { + "constrained_string_nullable_optional_default_null", + cty.NullVal(cty.DynamicPseudoType), + cty.NullVal(cty.String), + ``, + }, + { + "constrained_string_nullable_optional_default_null", + cty.StringVal("ahoy"), + cty.StringVal("ahoy"), + ``, + }, + { + "constrained_string_nullable_optional_default_null", + cty.UnknownVal(cty.String), + cty.UnknownVal(cty.String), + ``, + }, + + // required + { + "required", + cty.NilVal, + cty.UnknownVal(cty.DynamicPseudoType), + `Required variable not set: The variable "required" is required, but is not set.`, + }, + { + "required", + cty.NullVal(cty.DynamicPseudoType), + cty.UnknownVal(cty.DynamicPseudoType), + `Required variable not set: The variable "required" is required, but the given value is null.`, + }, + { + "required", + cty.StringVal("ahoy"), + cty.StringVal("ahoy"), + ``, + }, + { + "required", + cty.UnknownVal(cty.String), + cty.UnknownVal(cty.String), + ``, + }, + + // optional_default_string + { + "optional_default_string", + cty.NilVal, + cty.StringVal("hello"), // the declared default value + ``, + }, + { + "optional_default_string", + cty.NullVal(cty.DynamicPseudoType), + cty.StringVal("hello"), // the declared default value + ``, + }, + { + "optional_default_string", + cty.StringVal("ahoy"), + cty.StringVal("ahoy"), + ``, + }, + { + "optional_default_string", + cty.UnknownVal(cty.String), + cty.UnknownVal(cty.String), + ``, + }, + + // constrained_string_required + { + "constrained_string_required", + cty.NilVal, + cty.UnknownVal(cty.String), + `Required variable not set: The variable "constrained_string_required" is required, but is not set.`, + }, + { + "constrained_string_required", + cty.NullVal(cty.DynamicPseudoType), + cty.UnknownVal(cty.String), + `Required variable not set: The variable "constrained_string_required" is required, but the given value is null.`, + }, + { + "constrained_string_required", + cty.StringVal("ahoy"), + cty.StringVal("ahoy"), + ``, + }, + { + "constrained_string_required", + cty.UnknownVal(cty.String), + cty.UnknownVal(cty.String), + ``, + }, + + // constrained_string_optional_default_string + { + "constrained_string_optional_default_string", + cty.NilVal, + cty.StringVal("hello"), // the declared default value + ``, + }, + { + "constrained_string_optional_default_string", + cty.NullVal(cty.DynamicPseudoType), + cty.StringVal("hello"), // the declared default value + ``, + }, + { + "constrained_string_optional_default_string", + cty.StringVal("ahoy"), + cty.StringVal("ahoy"), + ``, + }, + { + "constrained_string_optional_default_string", + cty.UnknownVal(cty.String), + cty.UnknownVal(cty.String), + ``, + }, + + // constrained_string_optional_default_bool + { + "constrained_string_optional_default_bool", + cty.NilVal, + cty.StringVal("true"), // the declared default value, automatically converted to match type constraint + ``, + }, + { + "constrained_string_optional_default_bool", + cty.NullVal(cty.DynamicPseudoType), + cty.StringVal("true"), // the declared default value, automatically converted to match type constraint + ``, + }, + { + "constrained_string_optional_default_bool", + cty.StringVal("ahoy"), + cty.StringVal("ahoy"), + ``, + }, + { + "constrained_string_optional_default_bool", + cty.UnknownVal(cty.String), + cty.UnknownVal(cty.String), + ``, + }, + } + + for _, test := range tests { + t.Run(fmt.Sprintf("%s %#v", test.varName, test.given), func(t *testing.T) { + varAddr := addrs.InputVariable{Name: test.varName}.Absolute(addrs.RootModuleInstance) + varCfg := variableConfigs[test.varName] + if varCfg == nil { + t.Fatalf("invalid variable name %q", test.varName) + } + + t.Logf( + "test case\nvariable: %s\nconstraint: %#v\ndefault: %#v\nnullable: %#v\ngiven value: %#v", + varAddr, + varCfg.Type, + varCfg.Default, + varCfg.Nullable, + test.given, + ) + + got, diags := prepareFinalInputVariableValue( + varAddr, test.given, tfdiags.SourceRangeFromHCL(varCfg.DeclRange), varCfg, + ) + + if test.wantErr != "" { + if !diags.HasErrors() { + t.Errorf("unexpected success\nwant error: %s", test.wantErr) + } else if got, want := diags.Err().Error(), test.wantErr; got != want { + t.Errorf("wrong error\ngot: %s\nwant: %s", got, want) + } + } else { + if diags.HasErrors() { + t.Errorf("unexpected error\ngot: %s", diags.Err().Error()) + } + } + + // NOTE: should still have returned some reasonable value even if there was an error + if !test.want.RawEquals(got) { + t.Fatalf("wrong result\ngot: %#v\nwant: %#v", got, test.want) + } + }) + } +}