From b4a3e56a19d09c6fd5e9b72ee6fe43f944a03021 Mon Sep 17 00:00:00 2001 From: Liam Cervante Date: Fri, 6 Jun 2025 17:02:33 +0200 Subject: [PATCH] terraform test: deprecate ambigious variable references in test files (#37218) * terraform test: deprecate ambigious variable references in test files * fix panic caused by merge --- internal/moduletest/graph/eval_context.go | 65 +++++++++++++--------- internal/moduletest/graph/node_provider.go | 2 +- internal/moduletest/graph/node_variable.go | 8 +-- internal/moduletest/graph/variables.go | 25 +++++++-- 4 files changed, 63 insertions(+), 37 deletions(-) diff --git a/internal/moduletest/graph/eval_context.go b/internal/moduletest/graph/eval_context.go index f3906f05c0..3b93f6f3d5 100644 --- a/internal/moduletest/graph/eval_context.go +++ b/internal/moduletest/graph/eval_context.go @@ -151,7 +151,7 @@ func (ec *EvalContext) Verbose() bool { return ec.verbose } -func (ec *EvalContext) HclContext(references []*addrs.Reference, variableConfigs map[string]*configs.Variable) (*hcl.EvalContext, tfdiags.Diagnostics) { +func (ec *EvalContext) HclContext(references []*addrs.Reference) (*hcl.EvalContext, tfdiags.Diagnostics) { var diags tfdiags.Diagnostics runs := make(map[string]cty.Value) @@ -205,7 +205,7 @@ func (ec *EvalContext) HclContext(references []*addrs.Reference, variableConfigs continue } - if variable, moreDiags := ec.EvaluateUnparsedVariable(subject.Name, variableConfigs[subject.Name]); variable != nil { + if variable, moreDiags := ec.EvaluateUnparsedVariableDeprecated(subject.Name, reference); variable != nil { diags = diags.Append(moreDiags) vars[subject.Name] = variable.Value continue @@ -404,26 +404,47 @@ func (ec *EvalContext) EvaluateRun(run *moduletest.Run, resultScope *lang.Scope, return status, cty.ObjectVal(outputVals), diags } +// EvaluateUnparsedVariable accepts a variable name and a variable definition +// and checks if we have external unparsed variables that match the given +// configuration. If no variable was provided, we'll return a nil +// input value. func (ec *EvalContext) EvaluateUnparsedVariable(name string, config *configs.Variable) (*terraform.InputValue, tfdiags.Diagnostics) { variable, exists := ec.unparsedVariables[name] if !exists { return nil, nil } - if config != nil { + value, diags := variable.ParseVariableValue(config.ParsingMode) + if diags.HasErrors() { + value = &terraform.InputValue{ + Value: cty.DynamicVal, + } + } - // If we have a configuration, then we'll using the parsing mode from - // that. + return value, diags +} - value, diags := variable.ParseVariableValue(config.ParsingMode) - if diags.HasErrors() { - value = &terraform.InputValue{ - Value: cty.DynamicVal, - } - } - return value, diags +// EvaluateUnparsedVariableDeprecated accepts a variable name without a variable +// definition and attempts to parse it. +// +// This function represents deprecated functionality within the testing +// framework. It is no longer valid to reference external variables without a +// definition, but we do our best here and provide a warning that this will +// become completely unsupported in the future. +func (ec *EvalContext) EvaluateUnparsedVariableDeprecated(name string, ref *addrs.Reference) (*terraform.InputValue, tfdiags.Diagnostics) { + variable, exists := ec.unparsedVariables[name] + if !exists { + return nil, nil } + var diags tfdiags.Diagnostics + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagWarning, + Summary: "Variable referenced without definition", + Detail: fmt.Sprintf("Variable %q was referenced without providing a definition. Referencing undefined variables within Terraform Test files is deprecated, please add a `variable` block into the relevant test file to provide a definition for the variable. This will become required in future versions of Terraform.", name), + Subject: ref.SourceRange.ToHCL().Ptr(), + }) + // For backwards-compatibility reasons we do also have to support trying // to parse the global variables without a configuration. We introduced the // file-level variable definitions later, and users were already using @@ -432,27 +453,19 @@ func (ec *EvalContext) EvaluateUnparsedVariable(name string, config *configs.Var // Otherwise, we have no configuration so we're going to try both parsing // modes. - value, diags := variable.ParseVariableValue(configs.VariableParseHCL) - if !diags.HasErrors() { + value, moreDiags := variable.ParseVariableValue(configs.VariableParseHCL) + diags = diags.Append(moreDiags) + if !moreDiags.HasErrors() { // then good! we can just return these values directly. return value, diags } // otherwise, we'll try the other one. - value, diags = variable.ParseVariableValue(configs.VariableParseLiteral) - if diags.HasErrors() { - - // we'll add a warning diagnostic here, just telling the users they - // can avoid this by adding a variable definition. - - diags = diags.Append(tfdiags.Sourceless( - tfdiags.Warning, - "Missing variable definition", - fmt.Sprintf("The variable %q could not be parsed. Terraform had no definition block for this variable, this error could be avoided in future by including a definition block for this variable within the Terraform test file.", name))) - + value, moreDiags = variable.ParseVariableValue(configs.VariableParseLiteral) + diags = diags.Append(moreDiags) + if moreDiags.HasErrors() { // as usual make sure we still provide something for this value. - value = &terraform.InputValue{ Value: cty.DynamicVal, } diff --git a/internal/moduletest/graph/node_provider.go b/internal/moduletest/graph/node_provider.go index 1292b38026..ae4e9cbc9d 100644 --- a/internal/moduletest/graph/node_provider.go +++ b/internal/moduletest/graph/node_provider.go @@ -71,7 +71,7 @@ func (n *NodeProviderConfigure) Execute(ctx *EvalContext) { return } - hclContext, moreDiags := ctx.HclContext(references, nil) + hclContext, moreDiags := ctx.HclContext(references) n.File.AppendDiagnostics(moreDiags) if moreDiags.HasErrors() { ctx.SetProviderStatus(n.Addr, moduletest.Error) diff --git a/internal/moduletest/graph/node_variable.go b/internal/moduletest/graph/node_variable.go index 168f7a9b5c..990cfa2b1a 100644 --- a/internal/moduletest/graph/node_variable.go +++ b/internal/moduletest/graph/node_variable.go @@ -58,14 +58,14 @@ func (n *NodeVariableDefinition) Execute(ctx *EvalContext) { } } - value, moreDiags := terraform.PrepareFinalInputVariableValue(addrs.AbsInputVariableInstance{ + value, diags := terraform.PrepareFinalInputVariableValue(addrs.AbsInputVariableInstance{ Module: addrs.RootModuleInstance, Variable: addrs.InputVariable{ Name: n.Address, }, }, input, n.Config) - n.File.AppendDiagnostics(moreDiags) - if moreDiags.HasErrors() { + n.File.AppendDiagnostics(diags) + if diags.HasErrors() { ctx.SetVariableStatus(n.Address, moduletest.Error) return } @@ -125,7 +125,7 @@ func (n *NodeVariableExpression) Execute(ctx *EvalContext) { return } - evalContext, moreDiags := ctx.HclContext(refs, nil) + evalContext, moreDiags := ctx.HclContext(refs) n.File.AppendDiagnostics(moreDiags) if moreDiags.HasErrors() { ctx.SetVariableStatus(n.Address, moduletest.Error) diff --git a/internal/moduletest/graph/variables.go b/internal/moduletest/graph/variables.go index 0e0652eec5..6e3b8282bd 100644 --- a/internal/moduletest/graph/variables.go +++ b/internal/moduletest/graph/variables.go @@ -32,19 +32,19 @@ func (n *NodeTestRun) GetVariables(ctx *EvalContext, includeWarnings bool) (terr // run block. This is a combination of the variables declared within the // configuration for this run block, and the variables referenced by the // run block assertions. - relevantVariables := make(map[string]bool) + relevantVariables := make(map[string]*addrs.Reference) // First, we'll check to see which variables the run block assertions // reference. for _, reference := range n.References() { if addr, ok := reference.Subject.(addrs.InputVariable); ok { - relevantVariables[addr.Name] = true + relevantVariables[addr.Name] = reference } } // And check to see which variables the run block configuration references. for name := range run.ModuleConfig.Module.Variables { - relevantVariables[name] = true + relevantVariables[name] = nil } // We'll put the parsed values into this map. @@ -60,7 +60,7 @@ func (n *NodeTestRun) GetVariables(ctx *EvalContext, includeWarnings bool) (terr continue } - ctx, ctxDiags := ctx.HclContext(refs, run.ModuleConfig.Module.Variables) + ctx, ctxDiags := ctx.HclContext(refs) diags = diags.Append(ctxDiags) value := cty.DynamicVal @@ -151,7 +151,7 @@ func (n *NodeTestRun) GetVariables(ctx *EvalContext, includeWarnings bool) (terr // just referenced by parts of this run block. These must have been defined // elsewhere, but we need to include them. - for variable := range relevantVariables { + for variable, reference := range relevantVariables { if _, exists := values[variable]; exists { // Then we've already got a value for this variable. continue @@ -164,7 +164,20 @@ func (n *NodeTestRun) GetVariables(ctx *EvalContext, includeWarnings bool) (terr values[variable] = value continue } - if value, valueDiags := ctx.EvaluateUnparsedVariable(variable, nil); value != nil { + + if reference == nil { + // this shouldn't happen, we only put nil references into the + // relevantVariables map for values derived from the configuration + // and all of these should have been set in previous for loop. + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Missing reference", + Detail: fmt.Sprintf("The variable %q had no point of reference, which should not be possible. This is a bug in Terraform; please report it!", variable), + }) + continue + } + + if value, valueDiags := ctx.EvaluateUnparsedVariableDeprecated(variable, reference); value != nil { values[variable] = value diags = diags.Append(valueDiags) }