From f80762d3d539eeaf4c8894c3edd0d5f33aeefe60 Mon Sep 17 00:00:00 2001 From: Liam Cervante Date: Tue, 3 Jun 2025 09:47:12 +0200 Subject: [PATCH] terraform test: add variable definitions to test files (#37195) * terraform test: add variable definitions to test files Currently, `terraform test` attempts to work out the type of any external variables by delaying evaluation until each run block executes so it can use the definitions within the run blocks's module. This means that the values of variables can technically change between run blocks which isn't ideal. This commit is the first in a chain which will move the evaluation of variables into the terraform test graph. We need to give the users the option of specifying the type for external variables within the file as these variables are now going to be assessed outside of the context of a run block. To do this, we introduce the optional variable blocks that are added by this commit. * update comment --- .../v1.13/ENHANCEMENTS-20250602-152211.yaml | 5 + .../v1.13/UPGRADE NOTES-20250602-152009.yaml | 5 + internal/backend/local/test.go | 30 ++- internal/command/test_test.go | 5 + .../test/with-default-variables/main.tf | 12 + .../with-default-variables/main.tftest.hcl | 17 ++ internal/configs/test_file.go | 51 +++- internal/moduletest/graph/eval_context.go | 17 +- internal/moduletest/graph/transform_config.go | 4 +- internal/moduletest/graph/variables.go | 56 ++-- internal/moduletest/hcl/provider.go | 4 +- internal/moduletest/hcl/provider_test.go | 10 +- internal/moduletest/hcl/variable_cache.go | 254 ++++++++++-------- .../moduletest/hcl/variable_cache_test.go | 179 ++++-------- internal/terraform/eval_variable.go | 16 +- internal/terraform/eval_variable_test.go | 8 +- internal/terraform/node_module_variable.go | 2 +- internal/terraform/node_root_variable.go | 2 +- 18 files changed, 357 insertions(+), 320 deletions(-) create mode 100644 .changes/v1.13/ENHANCEMENTS-20250602-152211.yaml create mode 100644 .changes/v1.13/UPGRADE NOTES-20250602-152009.yaml create mode 100644 internal/command/testdata/test/with-default-variables/main.tf create mode 100644 internal/command/testdata/test/with-default-variables/main.tftest.hcl 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,