diff --git a/.changes/v1.13/ENHANCEMENTS-20250602-152211.yaml b/.changes/v1.13/ENHANCEMENTS-20250602-152211.yaml new file mode 100644 index 0000000000..4bc92af913 --- /dev/null +++ b/.changes/v1.13/ENHANCEMENTS-20250602-152211.yaml @@ -0,0 +1,5 @@ +kind: ENHANCEMENTS +body: '`terraform test`: Test authors can now specify definitions for external variables that are referenced within test files directly within the test file itself.' +time: 2025-06-02T15:22:11.453413+02:00 +custom: + Issue: "37195" diff --git a/.changes/v1.13/UPGRADE NOTES-20250602-152009.yaml b/.changes/v1.13/UPGRADE NOTES-20250602-152009.yaml new file mode 100644 index 0000000000..f159eb77cc --- /dev/null +++ b/.changes/v1.13/UPGRADE NOTES-20250602-152009.yaml @@ -0,0 +1,5 @@ +kind: UPGRADE NOTES +body: '`terraform test`: External variables referenced within test files should now be accompanied by a `variable` definition block within the test file. This is optional, but users with complex external variables may see error diagnostics without the additional variable definition.' +time: 2025-06-02T15:20:09.188388+02:00 +custom: + Issue: "37195" diff --git a/internal/backend/local/test.go b/internal/backend/local/test.go index fda60140d0..fe4cfda1ac 100644 --- a/internal/backend/local/test.go +++ b/internal/backend/local/test.go @@ -110,15 +110,6 @@ func (runner *TestSuiteRunner) Test() (moduletest.Status, tfdiags.Diagnostics) { } file := suite.Files[name] - evalCtx := graph.NewEvalContext(graph.EvalContextOpts{ - CancelCtx: runner.CancelledCtx, - StopCtx: runner.StoppedCtx, - Verbose: runner.Verbose, - Render: runner.View, - }) - - // TODO(liamcervante): Do the variables in the EvalContextTransformer - // as well as the run blocks. currentGlobalVariables := runner.GlobalVariables if filepath.Dir(file.Name) == runner.TestingDirectory { @@ -126,10 +117,23 @@ func (runner *TestSuiteRunner) Test() (moduletest.Status, tfdiags.Diagnostics) { // global variables and the global test variables. currentGlobalVariables = testDirectoryGlobalVariables } - evalCtx.VariableCaches = hcltest.NewVariableCaches(func(vc *hcltest.VariableCaches) { - maps.Copy(vc.GlobalVariables, currentGlobalVariables) - vc.FileVariables = file.Config.Variables + + evalCtx := graph.NewEvalContext(graph.EvalContextOpts{ + CancelCtx: runner.CancelledCtx, + StopCtx: runner.StoppedCtx, + Verbose: runner.Verbose, + Render: runner.View, + VariableCache: &hcltest.VariableCache{ + + // TODO(liamcervante): Do the variables in the EvalContextTransformer + // as well as the run blocks. + + ExternalVariableValues: currentGlobalVariables, + TestFileVariableDefinitions: file.Config.VariableDefinitions, + TestFileVariableExpressions: file.Config.Variables, + }, }) + fileRunner := &TestFileRunner{ Suite: runner, EvalContext: evalCtx, @@ -248,7 +252,7 @@ func (runner *TestFileRunner) Test(file *moduletest.File) { // Build the graph for the file. b := graph.TestGraphBuilder{ File: file, - GlobalVars: runner.EvalContext.VariableCaches.GlobalVariables, + GlobalVars: runner.EvalContext.VariableCache.ExternalVariableValues, ContextOpts: runner.Suite.Opts, } g, diags := b.Build() diff --git a/internal/command/test_test.go b/internal/command/test_test.go index d3ec9f0552..a70e611f06 100644 --- a/internal/command/test_test.go +++ b/internal/command/test_test.go @@ -334,6 +334,11 @@ func TestTest_Runs(t *testing.T) { expectedOut: []string{"1 passed, 0 failed."}, code: 0, }, + "with-default-variables": { + args: []string{"-var=input_two=universe"}, + expectedOut: []string{"1 passed, 0 failed."}, + code: 0, + }, } for name, tc := range tcs { t.Run(name, func(t *testing.T) { diff --git a/internal/command/testdata/test/with-default-variables/main.tf b/internal/command/testdata/test/with-default-variables/main.tf new file mode 100644 index 0000000000..0ada3b9031 --- /dev/null +++ b/internal/command/testdata/test/with-default-variables/main.tf @@ -0,0 +1,12 @@ + +variable "input_one" { + type = string +} + +variable "input_two" { + type = string +} + +resource "test_resource" "resource" { + value = "${var.input_one} - ${var.input_two}" +} diff --git a/internal/command/testdata/test/with-default-variables/main.tftest.hcl b/internal/command/testdata/test/with-default-variables/main.tftest.hcl new file mode 100644 index 0000000000..8a4cbd35f5 --- /dev/null +++ b/internal/command/testdata/test/with-default-variables/main.tftest.hcl @@ -0,0 +1,17 @@ + +variable "input_one" { + type = string + default = "hello" +} + +variable "input_two" { + type = string + default = "world" // we will override this an external value +} + +run "test" { + assert { + condition = test_resource.resource.value == "hello - universe" + error_message = "bad concatenation" + } +} diff --git a/internal/configs/test_file.go b/internal/configs/test_file.go index fb2cdce6c6..46c8e9268a 100644 --- a/internal/configs/test_file.go +++ b/internal/configs/test_file.go @@ -45,6 +45,14 @@ const ( // A test file is made up of a sequential list of run blocks, each designating // a command to execute and a series of validations to check after the command. type TestFile struct { + + // VariableDefinitions allows users to specify variables that should be + // provided externally (eg. from the command line or external files). + // + // This conflicts with the Variables block. Variables specified in the + // VariableDefinitions cannot also be specified within the Variables block. + VariableDefinitions map[string]*Variable + // Variables defines a set of global variable definitions that should be set // for every run block within the test file. Variables map[string]hcl.Expression @@ -327,8 +335,9 @@ type TestRunOptions struct { func loadTestFile(body hcl.Body) (*TestFile, hcl.Diagnostics) { var diags hcl.Diagnostics tf := &TestFile{ - Providers: make(map[string]*Provider), - Overrides: addrs.MakeMap[addrs.Targetable, *Override](), + VariableDefinitions: make(map[string]*Variable), + Providers: make(map[string]*Provider), + Overrides: addrs.MakeMap[addrs.Targetable, *Override](), } // we need to retrieve the file config block first, because the run blocks @@ -370,6 +379,31 @@ func loadTestFile(body hcl.Body) (*TestFile, hcl.Diagnostics) { } runBlockNames[run.Name] = run.DeclRange + case "variable": + variable, variableDiags := decodeVariableBlock(block, false) + diags = append(diags, variableDiags...) + if !variableDiags.HasErrors() { + if existing, exists := tf.VariableDefinitions[variable.Name]; exists { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Duplicate \"variable\" block names", + Detail: fmt.Sprintf("This test file already has a variable named %s defined at %s.", variable.Name, existing.DeclRange), + Subject: variable.DeclRange.Ptr(), + }) + continue + } + tf.VariableDefinitions[variable.Name] = variable + + if existing, exists := tf.Variables[variable.Name]; exists { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Duplicate \"variable\" block names", + Detail: fmt.Sprintf("This test file already has a variable named %s defined at %s.", variable.Name, existing.Range()), + Subject: variable.DeclRange.Ptr(), + }) + } + } + case "variables": if tf.Variables != nil { diags = append(diags, &hcl.Diagnostic{ @@ -388,6 +422,15 @@ func loadTestFile(body hcl.Body) (*TestFile, hcl.Diagnostics) { diags = append(diags, varsDiags...) for _, v := range vars { tf.Variables[v.Name] = v.Expr + + if existing, exists := tf.VariableDefinitions[v.Name]; exists { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Duplicate \"variable\" block names", + Detail: fmt.Sprintf("This test file already has a variable named %s defined at %s.", v.Name, v.Range), + Subject: existing.DeclRange.Ptr(), + }) + } } case "provider": provider, providerDiags := decodeProviderBlock(block, true) @@ -888,6 +931,10 @@ var testFileSchema = &hcl.BodySchema{ Type: "mock_provider", LabelNames: []string{"name"}, }, + { + Type: "variable", + LabelNames: []string{"name"}, + }, { Type: "variables", }, diff --git a/internal/moduletest/graph/eval_context.go b/internal/moduletest/graph/eval_context.go index 57ebf9cabc..8f0093c7ed 100644 --- a/internal/moduletest/graph/eval_context.go +++ b/internal/moduletest/graph/eval_context.go @@ -42,7 +42,7 @@ type TestFileState struct { // within the suite. // The struct provides concurrency-safe access to the various maps it contains. type EvalContext struct { - VariableCaches *hcltest.VariableCaches + VariableCache *hcltest.VariableCache // runOutputs is a mapping from run addresses to cty object values // representing the collected output values from the module under test. @@ -84,10 +84,11 @@ type EvalContext struct { } type EvalContextOpts struct { - Verbose bool - Render views.Test - CancelCtx context.Context - StopCtx context.Context + Verbose bool + Render views.Test + CancelCtx context.Context + StopCtx context.Context + VariableCache *hcltest.VariableCache } // NewEvalContext constructs a new graph evaluation context for use in @@ -104,7 +105,7 @@ func NewEvalContext(opts EvalContextOpts) *EvalContext { providersLock: sync.Mutex{}, FileStates: make(map[string]*TestFileState), stateLock: sync.Mutex{}, - VariableCaches: hcltest.NewVariableCaches(), + VariableCache: opts.VariableCache, cancelContext: cancelCtx, cancelFunc: cancel, stopContext: stopCtx, @@ -326,10 +327,6 @@ func (ec *EvalContext) GetOutputs() map[addrs.Run]cty.Value { return outputCopy } -func (ec *EvalContext) GetCache(run *moduletest.Run) *hcltest.VariableCache { - return ec.VariableCaches.GetCache(run.Name, run.ModuleConfig) -} - // ProviderExists returns true if the provider exists for the run inside the context. func (ec *EvalContext) ProviderExists(run *moduletest.Run, key string) bool { ec.providersLock.Lock() diff --git a/internal/moduletest/graph/transform_config.go b/internal/moduletest/graph/transform_config.go index db8a8055b1..f24ee8a6ba 100644 --- a/internal/moduletest/graph/transform_config.go +++ b/internal/moduletest/graph/transform_config.go @@ -88,7 +88,7 @@ func TransformConfigForRun(ctx *EvalContext, run *moduletest.Run, file *modulete AliasRange: ref.InChild.AliasRange, Config: &hcltest.ProviderConfig{ Original: testProvider.Config, - VariableCache: ctx.GetCache(run), + VariableCache: ctx.VariableCache, AvailableRunOutputs: runOutputs, }, Mock: testProvider.Mock, @@ -114,7 +114,7 @@ func TransformConfigForRun(ctx *EvalContext, run *moduletest.Run, file *modulete AliasRange: provider.AliasRange, Config: &hcltest.ProviderConfig{ Original: provider.Config, - VariableCache: ctx.GetCache(run), + VariableCache: ctx.VariableCache, AvailableRunOutputs: runOutputs, }, Mock: provider.Mock, diff --git a/internal/moduletest/graph/variables.go b/internal/moduletest/graph/variables.go index e48d5ca41c..c796ed84d7 100644 --- a/internal/moduletest/graph/variables.go +++ b/internal/moduletest/graph/variables.go @@ -59,19 +59,8 @@ func (n *NodeTestRun) GetVariables(ctx *EvalContext, includeWarnings bool) (terr refs, refDiags := langrefs.ReferencesInExpr(addrs.ParseRefFromTestingScope, expr) for _, ref := range refs { if addr, ok := ref.Subject.(addrs.InputVariable); ok { - cache := ctx.GetCache(run) - - value, valueDiags := cache.GetFileVariable(addr.Name) - diags = diags.Append(valueDiags) - if value != nil { - requiredValues[addr.Name] = value.Value - continue - } - - // Otherwise, it might be a global variable. - value, valueDiags = cache.GetGlobalVariable(addr.Name) - diags = diags.Append(valueDiags) - if value != nil { + if value, valueDiags := ctx.VariableCache.GetVariableValue(addr.Name); value != nil { + diags = diags.Append(valueDiags) requiredValues[addr.Name] = value.Value continue } @@ -118,18 +107,14 @@ func (n *NodeTestRun) GetVariables(ctx *EvalContext, includeWarnings bool) (terr continue } - // Otherwise, we'll get it from the cache as a file-level or global - // variable. - cache := ctx.GetCache(run) - - value, valueDiags := cache.GetFileVariable(variable) - diags = diags.Append(valueDiags) - if value != nil { - values[variable] = value + if _, exists := run.ModuleConfig.Module.Variables[variable]; exists { + // We'll deal with this later. continue } - value, valueDiags = cache.GetGlobalVariable(variable) + // Otherwise, we'll get it from the cache as a file-level or global + // variable. + value, valueDiags := ctx.VariableCache.GetVariableValue(variable) diags = diags.Append(valueDiags) if value != nil { values[variable] = value @@ -143,13 +128,32 @@ func (n *NodeTestRun) GetVariables(ctx *EvalContext, includeWarnings bool) (terr for name, variable := range run.ModuleConfig.Module.Variables { if _, exists := values[name]; exists { - // Then we've provided a variable for this. It's all good. + // Then we've provided a variable for this explicitly. It's all + // good. continue } - // Otherwise, we're going to give these variables a value. They'll be - // processed by the Terraform graph and provided a default value later - // if they have one. + // The user might have provided a value for this externally or at the + // file level, so we can also just pass it through. + + if ctx.VariableCache.HasVariableDefinition(variable.Name) { + if value, valueDiags := ctx.VariableCache.GetVariableValue(variable.Name); value != nil { + diags = diags.Append(valueDiags) + values[name] = value + continue + } + } else { + if value, valueDiags := ctx.VariableCache.EvaluateExternalVariable(name, variable); value != nil { + diags = diags.Append(valueDiags) + values[name] = value + continue + } + } + + // If all else fails, these variables may have default values set within + // the to-be-executed Terraform config. We'll put in placeholder values + // if that is the case, otherwise add a diagnostic early to avoid + // executing something we know will fail. if variable.Required() { diags = diags.Append(&hcl.Diagnostic{ diff --git a/internal/moduletest/hcl/provider.go b/internal/moduletest/hcl/provider.go index c5bce33de7..905076226e 100644 --- a/internal/moduletest/hcl/provider.go +++ b/internal/moduletest/hcl/provider.go @@ -81,7 +81,7 @@ func (p *ProviderConfig) transformAttributes(originals hcl.Attributes) (hcl.Attr refs, _ := langrefs.ReferencesInExpr(addrs.ParseRefFromTestingScope, original.Expr) for _, ref := range refs { if addr, ok := ref.Subject.(addrs.InputVariable); ok { - value, valueDiags := p.VariableCache.GetFileVariable(addr.Name) + value, valueDiags := p.VariableCache.GetVariableValue(addr.Name) diags = append(diags, valueDiags.ToHCL()...) if value != nil { availableVariables[addr.Name] = value.Value @@ -89,7 +89,7 @@ func (p *ProviderConfig) transformAttributes(originals hcl.Attributes) (hcl.Attr } // If the variable wasn't a file variable, it might be a global. - value, valueDiags = p.VariableCache.GetGlobalVariable(addr.Name) + value, valueDiags = p.VariableCache.GetVariableValue(addr.Name) diags = append(diags, valueDiags.ToHCL()...) if value != nil { availableVariables[addr.Name] = value.Value diff --git a/internal/moduletest/hcl/provider_test.go b/internal/moduletest/hcl/provider_test.go index 9e0f66fb36..979db06c30 100644 --- a/internal/moduletest/hcl/provider_test.go +++ b/internal/moduletest/hcl/provider_test.go @@ -184,19 +184,19 @@ func TestProviderConfig(t *testing.T) { outputs[addr] = cty.ObjectVal(attrs) } - variableCaches := NewVariableCaches(func(vc *VariableCaches) { - vc.FileVariables = func() map[string]hcl.Expression { + variableCache := &VariableCache{ + TestFileVariableExpressions: func() map[string]hcl.Expression { variables := make(map[string]hcl.Expression) for name, value := range tc.variables { variables[name] = hcl.StaticExpr(value, hcl.Range{}) } return variables - }() - }) + }(), + } config := ProviderConfig{ Original: file.Body, - VariableCache: variableCaches.GetCache("test", nil), + VariableCache: variableCache, AvailableRunOutputs: outputs, } diff --git a/internal/moduletest/hcl/variable_cache.go b/internal/moduletest/hcl/variable_cache.go index cc1298b16c..a03ccbb5a4 100644 --- a/internal/moduletest/hcl/variable_cache.go +++ b/internal/moduletest/hcl/variable_cache.go @@ -4,6 +4,7 @@ package hcl import ( + "fmt" "sync" "github.com/hashicorp/hcl/v2" @@ -17,125 +18,118 @@ import ( "github.com/hashicorp/terraform/internal/tfdiags" ) -// VariableCaches contains a mapping between test run blocks and evaluated -// variables. This is used to cache the results of evaluating variables so that -// they are only evaluated once per run. -// -// Each run block has its own configuration and therefore its own set of -// evaluated variables. -type VariableCaches struct { - GlobalVariables map[string]backendrun.UnparsedVariableValue - FileVariables map[string]hcl.Expression - - caches map[string]*VariableCache - cacheLock sync.Mutex +type VariableCache struct { + mutex sync.Mutex + + // ExternalVariableValues contains the raw values provided by the user + // via either the CLI, environment variables, or a variable file. + ExternalVariableValues map[string]backendrun.UnparsedVariableValue + + // TestFileVariableDefinitions contains the definitions for variables + // defined within the test file in `variable` blocks. + TestFileVariableDefinitions map[string]*configs.Variable + + // TestFileVariableExpressions contains the concrete variable expressions + // defined within the test file `variables` block. + TestFileVariableExpressions map[string]hcl.Expression + + // fileVariableValues contains the set of available file level + fileVariableValues map[string]*terraform.InputValue } -func NewVariableCaches(opts ...func(*VariableCaches)) *VariableCaches { - ret := &VariableCaches{ - GlobalVariables: make(map[string]backendrun.UnparsedVariableValue), - FileVariables: make(map[string]hcl.Expression), - caches: make(map[string]*VariableCache), - cacheLock: sync.Mutex{}, +func (cache *VariableCache) EvaluateExternalVariable(name string, config *configs.Variable) (*terraform.InputValue, tfdiags.Diagnostics) { + variable, exists := cache.ExternalVariableValues[name] + if !exists { + return nil, nil } - for _, opt := range opts { - opt(ret) + if config != nil { + + // If we have a configuration, then we'll using the parsing mode from + // that. + + value, diags := variable.ParseVariableValue(config.ParsingMode) + if diags.HasErrors() { + value = &terraform.InputValue{ + Value: cty.DynamicVal, + } + } + return value, diags } - return ret -} + // 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 + // global variables so we do need to keep supporting this use case. -// VariableCache contains the cache for a single run block. This cache contains -// the evaluated values for global and file-level variables. -type VariableCache struct { - config *configs.Config + // Otherwise, we have no configuration so we're going to try both parsing + // modes. - globals terraform.InputValues - files terraform.InputValues + value, diags := variable.ParseVariableValue(configs.VariableParseHCL) + if !diags.HasErrors() { + // then good! we can just return these values directly. + return value, diags + } - values *VariableCaches // back reference so we can access the stored values -} + // otherwise, we'll try the other one. -// GetCache returns the cache for the named run. If the cache does not exist, it -// is created and returned. -func (caches *VariableCaches) GetCache(name string, config *configs.Config) *VariableCache { - caches.cacheLock.Lock() - defer caches.cacheLock.Unlock() - cache, exists := caches.caches[name] - if !exists { - cache = &VariableCache{ - config: config, - globals: make(terraform.InputValues), - files: make(terraform.InputValues), - values: caches, + 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))) + + // as usual make sure we still provide something for this value. + + value = &terraform.InputValue{ + Value: cty.DynamicVal, } - caches.caches[name] = cache } - return cache + return value, diags } -// GetGlobalVariable returns a value for the named global variable evaluated -// against the current run. -// -// This function caches the result of evaluating the variable so that it is -// only evaluated once per run. -// -// This function will return a valid input value if parsing fails for any reason -// so the caller can continue processing the configuration. The diagnostics -// returned will contain the error message that occurred during parsing and as -// such should be shown to the user. -func (cache *VariableCache) GetGlobalVariable(name string) (*terraform.InputValue, tfdiags.Diagnostics) { - val, exists := cache.globals[name] - if exists { - return val, nil - } - - variable, exists := cache.values.GlobalVariables[name] +func (cache *VariableCache) evaluateVariableDefinition(name string) (*terraform.InputValue, tfdiags.Diagnostics) { + definition, exists := cache.TestFileVariableDefinitions[name] if !exists { + // no definition for this variable return nil, nil } - // TODO: We should also introduce a way to specify the mode in the test - // file itself. Suggestion, optional variable blocks. - parsingMode := configs.VariableParseHCL - - if cfg, exists := cache.config.Module.Variables[name]; exists { - parsingMode = cfg.ParsingMode - } + var diags tfdiags.Diagnostics - value, diags := variable.ParseVariableValue(parsingMode) - if diags.HasErrors() { - // In this case, the variable exists but we couldn't parse it. We'll - // return a usable value so that we don't compound errors later by - // claiming a variable doesn't exist when it does. We also return the - // diagnostics explaining the error which will be shown to the user. - value = &terraform.InputValue{ - Value: cty.DynamicVal, + var input *terraform.InputValue + if _, exists := cache.ExternalVariableValues[name]; exists { + parsed, moreDiags := cache.EvaluateExternalVariable(name, definition) + diags = diags.Append(moreDiags) + input = parsed + } else { + input = &terraform.InputValue{ + Value: cty.NilVal, } } - cache.globals[name] = value - return value, diags + value, moreDiags := terraform.PrepareFinalInputVariableValue(addrs.AbsInputVariableInstance{ + Module: addrs.RootModuleInstance, + Variable: addrs.InputVariable{ + Name: name, + }, + }, input, definition) + diags = diags.Append(moreDiags) + + return &terraform.InputValue{ + Value: value, + SourceType: terraform.ValueFromConfig, + SourceRange: tfdiags.SourceRangeFromHCL(definition.DeclRange), + }, diags } -// GetFileVariable returns a value for the named file-level variable evaluated -// against the current run. -// -// This function caches the result of evaluating the variable so that it is -// only evaluated once per run. -// -// This function will return a valid input value if parsing fails for any reason -// so the caller can continue processing the configuration. The diagnostics -// returned will contain the error message that occurred during parsing and as -// such should be shown to the user. -func (cache *VariableCache) GetFileVariable(name string) (*terraform.InputValue, tfdiags.Diagnostics) { - val, exists := cache.files[name] - if exists { - return val, nil - } - - expr, exists := cache.values.FileVariables[name] +func (cache *VariableCache) evaluateFileVariable(name string) (*terraform.InputValue, tfdiags.Diagnostics) { + expr, exists := cache.TestFileVariableExpressions[name] if !exists { return nil, nil } @@ -146,9 +140,12 @@ func (cache *VariableCache) GetFileVariable(name string) (*terraform.InputValue, refs, refDiags := langrefs.ReferencesInExpr(addrs.ParseRefFromTestingScope, expr) for _, ref := range refs { if input, ok := ref.Subject.(addrs.InputVariable); ok { - variable, variableDiags := cache.GetGlobalVariable(input.Name) - diags = diags.Append(variableDiags) + variable, variableDiags := cache.evaluateVariableDefinition(input.Name) if variable != nil { + diags = diags.Append(variableDiags) + availableVariables[input.Name] = variable.Value + } else if variable, variableDiags := cache.EvaluateExternalVariable(input.Name, nil); variable != nil { + diags = diags.Append(variableDiags) availableVariables[input.Name] = variable.Value } } @@ -156,28 +153,17 @@ func (cache *VariableCache) GetFileVariable(name string) (*terraform.InputValue, diags = diags.Append(refDiags) if diags.HasErrors() { - // There's no point trying to evaluate the variable as we know it will - // fail. We'll just return a usable value so that we don't compound - // errors later by claiming a variable doesn't exist when it does. We - // also return the diagnostics explaining the error which will be shown - // to the user. - cache.files[name] = &terraform.InputValue{ + return &terraform.InputValue{ Value: cty.DynamicVal, - } - return cache.files[name], diags + }, diags } ctx, ctxDiags := EvalContext(TargetFileVariable, map[string]hcl.Expression{name: expr}, availableVariables, nil) diags = diags.Append(ctxDiags) - if ctxDiags.HasErrors() { - // If we couldn't build the context, we won't actually process these - // variables. Instead, we'll fill them with an empty value but still - // make a note that the user did provide them. - cache.files[name] = &terraform.InputValue{ + return &terraform.InputValue{ Value: cty.DynamicVal, - } - return cache.files[name], diags + }, diags } value, valueDiags := expr.Value(ctx) @@ -190,10 +176,50 @@ func (cache *VariableCache) GetFileVariable(name string) (*terraform.InputValue, value = cty.DynamicVal } - cache.files[name] = &terraform.InputValue{ + return &terraform.InputValue{ Value: value, SourceType: terraform.ValueFromConfig, SourceRange: tfdiags.SourceRangeFromHCL(expr.Range()), + }, diags +} + +func (cache *VariableCache) GetVariableValue(name string) (*terraform.InputValue, tfdiags.Diagnostics) { + cache.mutex.Lock() + defer cache.mutex.Unlock() + + if cache.fileVariableValues == nil { + cache.fileVariableValues = make(map[string]*terraform.InputValue) + } + + if value, exists := cache.fileVariableValues[name]; exists { + return value, nil + } + + if value, valueDiags := cache.evaluateFileVariable(name); value != nil { + cache.fileVariableValues[name] = value + return value, valueDiags + } + + if value, valueDiags := cache.evaluateVariableDefinition(name); value != nil { + cache.fileVariableValues[name] = value + return value, valueDiags + } + + if value, valueDiags := cache.EvaluateExternalVariable(name, nil); value != nil { + cache.fileVariableValues[name] = value + return value, valueDiags + } + + return nil, nil +} + +func (cache *VariableCache) HasVariableDefinition(name string) bool { + if _, exists := cache.TestFileVariableExpressions[name]; exists { + return true + } + + if _, exists := cache.TestFileVariableDefinitions[name]; exists { + return true } - return cache.files[name], diags + return false } diff --git a/internal/moduletest/hcl/variable_cache_test.go b/internal/moduletest/hcl/variable_cache_test.go index d67b97c8de..d6e11f3927 100644 --- a/internal/moduletest/hcl/variable_cache_test.go +++ b/internal/moduletest/hcl/variable_cache_test.go @@ -21,16 +21,16 @@ import ( func TestFileVariables(t *testing.T) { tcs := map[string]struct { - Values map[string]string - GlobalValues map[string]string - Variables map[string]configs.VariableParsingMode - Want map[string]cty.Value + TestFileVariableExpressions map[string]string + ExternalVariableValues map[string]string + TestFileVariableDefinitions map[string]*configs.Variable + Want map[string]cty.Value }{ "no_variables": { Want: make(map[string]cty.Value), }, "string": { - Values: map[string]string{ + TestFileVariableExpressions: map[string]string{ "foo": `"bar"`, }, Want: map[string]cty.Value{ @@ -38,7 +38,7 @@ func TestFileVariables(t *testing.T) { }, }, "boolean": { - Values: map[string]string{ + TestFileVariableExpressions: map[string]string{ "foo": "true", }, Want: map[string]cty.Value{ @@ -46,123 +46,46 @@ func TestFileVariables(t *testing.T) { }, }, "reference": { - Values: map[string]string{ + TestFileVariableExpressions: map[string]string{ "foo": "var.bar", }, - GlobalValues: map[string]string{ + ExternalVariableValues: map[string]string{ "bar": `"baz"`, }, - Variables: map[string]configs.VariableParsingMode{ - "foo": configs.VariableParseLiteral, + TestFileVariableDefinitions: map[string]*configs.Variable{ + "bar": { + ParsingMode: configs.VariableParseHCL, + ConstraintType: cty.String, + }, }, Want: map[string]cty.Value{ "foo": cty.StringVal("baz"), }, }, - } - - for name, tc := range tcs { - t.Run(name, func(t *testing.T) { - - caches := NewVariableCaches(func(vc *VariableCaches) { - vc.FileVariables = func() map[string]hcl.Expression { - vars := make(map[string]hcl.Expression) - for name, value := range tc.Values { - expr, diags := hclsyntax.ParseExpression([]byte(value), "test.tf", hcl.Pos{Line: 0, Column: 0, Byte: 0}) - if len(diags) > 0 { - t.Fatalf("unexpected errors: %v", diags) - } - vars[name] = expr - } - return vars - }() - vc.GlobalVariables = func() map[string]backendrun.UnparsedVariableValue { - vars := make(map[string]backendrun.UnparsedVariableValue) - for name, value := range tc.GlobalValues { - vars[name] = &variable{name, value} - } - return vars - }() - }) - config := makeConfigWithVariables(tc.Variables) - - cache := caches.GetCache("test", config) - got := make(map[string]cty.Value) - for name := range tc.Want { - value, diags := cache.GetFileVariable(name) - if diags.HasErrors() { - t.Fatalf("unexpected errors: %v", diags) - } - got[name] = value.Value - } - - if diff := cmp.Diff(tc.Want, got, ctydebug.CmpOptions); len(diff) > 0 { - t.Fatalf("unexpected result\n%s", diff) - } - }) - } -} - -func TestGlobalVariables(t *testing.T) { - - tcs := map[string]struct { - Values map[string]string - Variables map[string]configs.VariableParsingMode - Want map[string]cty.Value - }{ - "no_variables": { - Want: make(map[string]cty.Value), - }, - "string": { - Values: map[string]string{ - "foo": "bar", - }, - Variables: map[string]configs.VariableParsingMode{ - "foo": configs.VariableParseLiteral, - }, - Want: map[string]cty.Value{ - "foo": cty.StringVal("bar"), - }, - }, - "boolean_string": { - Values: map[string]string{ - "foo": "true", - }, - Variables: map[string]configs.VariableParsingMode{ - "foo": configs.VariableParseLiteral, - }, - Want: map[string]cty.Value{ - "foo": cty.StringVal("true"), - }, - }, - "boolean": { - Values: map[string]string{ - "foo": "true", + "reference to missing external": { + TestFileVariableExpressions: map[string]string{ + "foo": "var.bar", }, - Variables: map[string]configs.VariableParsingMode{ - "foo": configs.VariableParseHCL, + ExternalVariableValues: map[string]string{ + "bar": `"baz"`, }, Want: map[string]cty.Value{ - "foo": cty.BoolVal(true), + "foo": cty.StringVal("baz"), }, }, - "string_hcl": { - Values: map[string]string{ - "foo": `"bar"`, - }, - Variables: map[string]configs.VariableParsingMode{ - "foo": configs.VariableParseHCL, - }, - Want: map[string]cty.Value{ - "foo": cty.StringVal("bar"), + "reference with default": { + TestFileVariableExpressions: map[string]string{ + "foo": "var.bar", }, - }, - "missing_config": { - Values: map[string]string{ - "foo": `"bar"`, + TestFileVariableDefinitions: map[string]*configs.Variable{ + "bar": { + ParsingMode: configs.VariableParseLiteral, + ConstraintType: cty.String, + Default: cty.StringVal("baz"), + }, }, Want: map[string]cty.Value{ - "foo": cty.StringVal("bar"), + "foo": cty.StringVal("baz"), }, }, } @@ -170,22 +93,31 @@ func TestGlobalVariables(t *testing.T) { for name, tc := range tcs { t.Run(name, func(t *testing.T) { - caches := NewVariableCaches(func(vc *VariableCaches) { - vc.GlobalVariables = func() map[string]backendrun.UnparsedVariableValue { + cache := &VariableCache{ + TestFileVariableExpressions: func() map[string]hcl.Expression { + vars := make(map[string]hcl.Expression) + for name, value := range tc.TestFileVariableExpressions { + expr, diags := hclsyntax.ParseExpression([]byte(value), "test.tf", hcl.Pos{Line: 0, Column: 0, Byte: 0}) + if len(diags) > 0 { + t.Fatalf("unexpected errors: %v", diags) + } + vars[name] = expr + } + return vars + }(), + ExternalVariableValues: func() map[string]backendrun.UnparsedVariableValue { vars := make(map[string]backendrun.UnparsedVariableValue) - for name, value := range tc.Values { + for name, value := range tc.ExternalVariableValues { vars[name] = &variable{name, value} } return vars - }() - }) - - config := makeConfigWithVariables(tc.Variables) + }(), + TestFileVariableDefinitions: tc.TestFileVariableDefinitions, + } - cache := caches.GetCache("test", config) got := make(map[string]cty.Value) for name := range tc.Want { - value, diags := cache.GetGlobalVariable(name) + value, diags := cache.GetVariableValue(name) if diags.HasErrors() { t.Fatalf("unexpected errors: %v", diags) } @@ -197,23 +129,6 @@ func TestGlobalVariables(t *testing.T) { } }) } - -} - -func makeConfigWithVariables(modes map[string]configs.VariableParsingMode) *configs.Config { - return &configs.Config{ - Module: &configs.Module{ - Variables: func() map[string]*configs.Variable { - vars := make(map[string]*configs.Variable) - for name, mode := range modes { - vars[name] = &configs.Variable{ - ParsingMode: mode, - } - } - return vars - }(), - }, - } } var _ backendrun.UnparsedVariableValue = (*variable)(nil) diff --git a/internal/terraform/eval_variable.go b/internal/terraform/eval_variable.go index 2bcbc11ec7..6e70cf6e03 100644 --- a/internal/terraform/eval_variable.go +++ b/internal/terraform/eval_variable.go @@ -21,15 +21,15 @@ import ( "github.com/hashicorp/terraform/internal/tfdiags" ) -func prepareFinalInputVariableValue(addr addrs.AbsInputVariableInstance, raw *InputValue, cfg *configs.Variable) (cty.Value, tfdiags.Diagnostics) { +func PrepareFinalInputVariableValue(addr addrs.AbsInputVariableInstance, raw *InputValue, cfg *configs.Variable) (cty.Value, tfdiags.Diagnostics) { var diags tfdiags.Diagnostics convertTy := cfg.ConstraintType - log.Printf("[TRACE] prepareFinalInputVariableValue: preparing %s", addr) + 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) + log.Printf("[TRACE] PrepareFinalInputVariableValue: %s has a default value", addr) var err error defaultVal, err = convert.Convert(cfg.Default, convertTy) if err != nil { @@ -74,14 +74,14 @@ func prepareFinalInputVariableValue(addr addrs.AbsInputVariableInstance, raw *In given := raw.Value if given == cty.NilVal { // The variable wasn't set at all (even to null) - log.Printf("[TRACE] prepareFinalInputVariableValue: %s has no defined value", addr) + 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 set, which would // mask this error with a more specific one that refers to the // CLI features for setting such variables. We can get here for // child module variables, though. - log.Printf("[ERROR] prepareFinalInputVariableValue: %s is required but is not set", addr) + log.Printf("[ERROR] PrepareFinalInputVariableValue: %s is required but is not set", addr) diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, Summary: `Required variable not set`, @@ -106,7 +106,7 @@ func prepareFinalInputVariableValue(addr addrs.AbsInputVariableInstance, raw *In val, err := convert.Convert(given, convertTy) if err != nil { - log.Printf("[ERROR] prepareFinalInputVariableValue: %s has unsuitable type\n got: %s\n want: %s", addr, given.Type(), convertTy) + log.Printf("[ERROR] PrepareFinalInputVariableValue: %s has unsuitable type\n got: %s\n want: %s", addr, given.Type(), convertTy) var detail string var subject *hcl.Range if nonFileSource != "" { @@ -157,11 +157,11 @@ func prepareFinalInputVariableValue(addr addrs.AbsInputVariableInstance, raw *In // 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) + log.Printf("[TRACE] PrepareFinalInputVariableValue: %s is defined as null", addr) if defaultVal != cty.NilVal { val = defaultVal } else { - log.Printf("[ERROR] prepareFinalInputVariableValue: %s is non-nullable but set to null, and is required", addr) + log.Printf("[ERROR] PrepareFinalInputVariableValue: %s is non-nullable but set to null, and is required", addr) if nonFileSource != "" { diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, diff --git a/internal/terraform/eval_variable_test.go b/internal/terraform/eval_variable_test.go index 910bbd4626..0ff59bf584 100644 --- a/internal/terraform/eval_variable_test.go +++ b/internal/terraform/eval_variable_test.go @@ -862,7 +862,7 @@ func TestPrepareFinalInputVariableValue(t *testing.T) { SourceType: ValueFromCaller, } - got, diags := prepareFinalInputVariableValue( + got, diags := PrepareFinalInputVariableValue( varAddr, rawVal, varCfg, ) @@ -976,7 +976,7 @@ func TestPrepareFinalInputVariableValue(t *testing.T) { SourceRange: test.SourceRange, } - _, diags := prepareFinalInputVariableValue( + _, diags := PrepareFinalInputVariableValue( varAddr, rawVal, varCfg, ) if !diags.HasErrors() { @@ -994,7 +994,7 @@ func TestPrepareFinalInputVariableValue(t *testing.T) { SourceRange: test.SourceRange, } - _, diags := prepareFinalInputVariableValue( + _, diags := PrepareFinalInputVariableValue( varAddr, rawVal, varCfg, ) if !diags.HasErrors() { @@ -1045,7 +1045,7 @@ func TestPrepareFinalInputVariableValue(t *testing.T) { SourceRange: test.SourceRange, } - _, diags := prepareFinalInputVariableValue( + _, diags := PrepareFinalInputVariableValue( varAddr, rawVal, varCfg, ) if !diags.HasErrors() { diff --git a/internal/terraform/node_module_variable.go b/internal/terraform/node_module_variable.go index d8f9830cfe..005edc4ebc 100644 --- a/internal/terraform/node_module_variable.go +++ b/internal/terraform/node_module_variable.go @@ -303,7 +303,7 @@ func (n *nodeModuleVariable) evalModuleVariable(ctx EvalContext, validateOnly bo SourceRange: errSourceRange, } - finalVal, moreDiags := prepareFinalInputVariableValue(n.Addr, rawVal, n.Config) + finalVal, moreDiags := PrepareFinalInputVariableValue(n.Addr, rawVal, n.Config) diags = diags.Append(moreDiags) return finalVal, diags.ErrWithWarnings() diff --git a/internal/terraform/node_root_variable.go b/internal/terraform/node_root_variable.go index 3255cd211c..ccf5791fd7 100644 --- a/internal/terraform/node_root_variable.go +++ b/internal/terraform/node_root_variable.go @@ -100,7 +100,7 @@ func (n *NodeRootVariable) Execute(ctx EvalContext, op walkOperation) tfdiags.Di } } - finalVal, moreDiags := prepareFinalInputVariableValue( + finalVal, moreDiags := PrepareFinalInputVariableValue( addr, givenVal, n.Config,