From 448775102c3a0af25247a392ccb99d7c04ea7018 Mon Sep 17 00:00:00 2001 From: Liam Cervante Date: Tue, 23 Apr 2024 13:10:41 +0200 Subject: [PATCH] terraform test: Push evaluation of variables to as late as possible (#35014) * terraform test: Push evaluation of variables to as late as possible * Update internal/moduletest/hcl/variable_cache.go Co-authored-by: kmoe <5575356+kmoe@users.noreply.github.com> * address comments --------- Co-authored-by: kmoe <5575356+kmoe@users.noreply.github.com> --- internal/backend/local/test.go | 321 ++++++------------ internal/command/test_test.go | 40 ++- .../testdata/test/env-vars-in-module/main.tf | 1 + .../test/env-vars-in-module/main.tftest.hcl | 7 + .../test/env-vars-in-module/mod/main.tf | 5 + .../command/testdata/test/env-vars/main.tf | 5 + .../testdata/test/env-vars/main.tftest.hcl | 1 + internal/moduletest/config/config.go | 7 +- internal/moduletest/config/config_test.go | 9 +- internal/moduletest/hcl/context.go | 41 ++- internal/moduletest/hcl/provider.go | 32 +- internal/moduletest/hcl/provider_test.go | 72 ++-- internal/moduletest/hcl/variable_cache.go | 183 ++++++++++ .../moduletest/hcl/variable_cache_test.go | 235 +++++++++++++ 14 files changed, 661 insertions(+), 298 deletions(-) create mode 100644 internal/command/testdata/test/env-vars-in-module/main.tf create mode 100644 internal/command/testdata/test/env-vars-in-module/main.tftest.hcl create mode 100644 internal/command/testdata/test/env-vars-in-module/mod/main.tf create mode 100644 internal/command/testdata/test/env-vars/main.tf create mode 100644 internal/command/testdata/test/env-vars/main.tftest.hcl create mode 100644 internal/moduletest/hcl/variable_cache.go create mode 100644 internal/moduletest/hcl/variable_cache_test.go diff --git a/internal/backend/local/test.go b/internal/backend/local/test.go index 31d3a359d36e..2adba1ecdab1 100644 --- a/internal/backend/local/test.go +++ b/internal/backend/local/test.go @@ -20,6 +20,7 @@ import ( "github.com/hashicorp/terraform/internal/command/views" "github.com/hashicorp/terraform/internal/configs" "github.com/hashicorp/terraform/internal/lang" + "github.com/hashicorp/terraform/internal/lang/langrefs" "github.com/hashicorp/terraform/internal/logging" "github.com/hashicorp/terraform/internal/moduletest" configtest "github.com/hashicorp/terraform/internal/moduletest/config" @@ -107,6 +108,20 @@ func (runner *TestSuiteRunner) Test() (moduletest.Status, tfdiags.Diagnostics) { } sort.Strings(files) // execute the files in alphabetical order + // We have two sets of variables that are available to different test files. + // Test files in the root directory have access to the GlobalVariables only, + // while test files in the test directory have access to the union of + // GlobalVariables and GlobalTestVariables. + testDirectoryGlobalVariables := make(map[string]backendrun.UnparsedVariableValue) + for name, value := range runner.GlobalVariables { + testDirectoryGlobalVariables[name] = value + } + for name, value := range runner.GlobalTestVariables { + // We're okay to overwrite the global variables in case of name + // collisions, as the test directory variables should take precedence. + testDirectoryGlobalVariables[name] = value + } + suite.Status = moduletest.Pass for _, name := range files { if runner.Cancelled { @@ -124,6 +139,13 @@ func (runner *TestSuiteRunner) Test() (moduletest.Status, tfdiags.Diagnostics) { priorOutputs[run.Addr()] = cty.NilVal } + currentGlobalVariables := runner.GlobalVariables + if filepath.Dir(file.Name) == runner.TestingDirectory { + // If the file is in the test directory, we'll use the union of the + // global variables and the global test variables. + currentGlobalVariables = testDirectoryGlobalVariables + } + fileRunner := &TestFileRunner{ Suite: runner, RelevantStates: map[string]*TestFileState{ @@ -133,6 +155,10 @@ func (runner *TestSuiteRunner) Test() (moduletest.Status, tfdiags.Diagnostics) { }, }, PriorOutputs: priorOutputs, + VariableCaches: &hcltest.VariableCaches{ + GlobalVariables: currentGlobalVariables, + FileVariables: file.Config.Variables, + }, } runner.View.File(file, moduletest.Starting) @@ -243,15 +269,7 @@ type TestFileRunner struct { // variables within run blocks. PriorOutputs map[addrs.Run]cty.Value - // globalVariables are globally defined variables, e.g. through tfvars or CLI flags - globalVariables terraform.InputValues - // fileVariables are defined in the variables section of a test file - fileVariables terraform.InputValues - // fileVariableExpressions are the hcl expressions for the fileVariables - fileVariableExpressions map[string]hcl.Expression - // globalAndFileVariables is a combination of globalVariables and fileVariables - // created for convenience - globalAndFileVariables terraform.InputValues + VariableCaches *hcltest.VariableCaches } // TestFileState is a helper struct that just maps a run block to the state that @@ -264,17 +282,6 @@ type TestFileState struct { func (runner *TestFileRunner) Test(file *moduletest.File) { log.Printf("[TRACE] TestFileRunner: executing test file %s", file.Name) - // First thing, initialise the global variables for the file - runner.initVariables(file) - - vars := make(terraform.InputValues) - for name, value := range runner.globalVariables { - vars[name] = value - } - for name, value := range runner.fileVariables { - vars[name] = value - } - // The file validation only returns warnings so we'll just add them without // checking anything about them. file.Diagnostics = file.Diagnostics.Append(file.Config.Validate(runner.Suite.Config)) @@ -401,7 +408,7 @@ func (runner *TestFileRunner) run(run *moduletest.Run, file *moduletest.File, st } runner.gatherProviders(key, config) - resetConfig, configDiags := configtest.TransformConfigForTest(config, run, file, runner.globalAndFileVariables, runner.PriorOutputs, runner.Suite.configProviders[key]) + resetConfig, configDiags := configtest.TransformConfigForTest(config, run, file, runner.VariableCaches, runner.PriorOutputs, runner.Suite.configProviders[key]) defer resetConfig() run.Diagnostics = run.Diagnostics.Append(configDiags) @@ -966,7 +973,7 @@ func (runner *TestFileRunner) cleanup(file *moduletest.File) { key = state.Run.Config.Module.Source.String() } - reset, configDiags := configtest.TransformConfigForTest(config, state.Run, file, runner.globalAndFileVariables, runner.PriorOutputs, runner.Suite.configProviders[key]) + reset, configDiags := configtest.TransformConfigForTest(config, state.Run, file, runner.VariableCaches, runner.PriorOutputs, runner.Suite.configProviders[key]) diags = diags.Append(configDiags) updated := state.State @@ -1001,9 +1008,9 @@ func (runner *TestFileRunner) GetVariables(config *configs.Config, run *modulete var diags tfdiags.Diagnostics // relevantVariables contains the variables that are of interest to this - // run block. We can have variables defined at the global level and at the - // file level that this run block doesn't need so we're going to make a - // quick list of the variables that are actually relevant. + // 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) // First, we'll check to see which variables the run block assertions @@ -1014,90 +1021,96 @@ func (runner *TestFileRunner) GetVariables(config *configs.Config, run *modulete } } - getRelevantVariables := func(src map[string]hcl.Expression) tfdiags.Diagnostics { - var getVarsDiags tfdiags.Diagnostics - for _, expr := range src { - for _, variable := range expr.Variables() { - reference, referenceDiags := addrs.ParseRefFromTestingScope(variable) - getVarsDiags = getVarsDiags.Append(referenceDiags) - if reference != nil { - if addr, ok := reference.Subject.(addrs.InputVariable); ok { - relevantVariables[addr.Name] = true - } - } - } - } - return getVarsDiags - } - - // Second, we'll check to see which variables the file variables - // themselves reference. - diags = diags.Append(getRelevantVariables(runner.fileVariableExpressions)) - - // Third, we'll check to see which variables the run block variables - // themselves reference. We might be processing variables just for the file - // so the run block itself could be nil. - diags = diags.Append(getRelevantVariables(run.Config.Variables)) - - // Finally, we'll check to see which variables are actually defined within - // the configuration. + // And check to see which variables the run block configuration references. for name := range config.Module.Variables { relevantVariables[name] = true } - // Now we know which variables are actually needed by this run block. - - // We're going to run over all the sets of variables we have access to: - // - Global variables, from the CLI / env vars / .tfvars files. - // - File variables, defined within the `variables` block in the file. - // - Run variables, defined within the `variables` block in this run. - // - ConfigVariables variables, defined directly within the config. + // We'll put the parsed values into this map. values := make(terraform.InputValues) - // First, let's look at the global variables. - for name, value := range runner.globalVariables { - if !relevantVariables[name] { - // Then this run block doesn't need this value. - continue - } + // First, let's step through the expressions within the run block and work + // them out. + for name, expr := range run.Config.Variables { + requiredValues := make(map[string]cty.Value) + + refs, refDiags := langrefs.ReferencesInExpr(addrs.ParseRefFromTestingScope, expr) + for _, ref := range refs { + if addr, ok := ref.Subject.(addrs.InputVariable); ok { + cache := runner.VariableCaches.GetCache(run.Name, config) + + value, valueDiags := cache.GetFileVariable(addr.Name) + diags = diags.Append(valueDiags) + if value != nil { + requiredValues[addr.Name] = value.Value + continue + } - values[name] = value - } + // Otherwise, it might be a global variable. + value, valueDiags = cache.GetGlobalVariable(addr.Name) + diags = diags.Append(valueDiags) + if value != nil { + requiredValues[addr.Name] = value.Value + continue + } + } + } + diags = diags.Append(refDiags) - // We don't care if the file level variables are relevant or not - ignoreRelevance := func(name string, expr hcl.Expression) (diags tfdiags.Diagnostics) { - return diags - } + ctx, ctxDiags := hcltest.EvalContext(hcltest.TargetRunBlock, map[string]hcl.Expression{name: expr}, requiredValues, runner.PriorOutputs) + diags = diags.Append(ctxDiags) - // Second, we'll check the file level variables - // This is a bit more complicated, as the file and run level variables can reference - // previously defined variables. - fileValues, fileDiags := runner.getVariablesFromConfiguration(values, ignoreRelevance, runner.fileVariableExpressions) - diags = diags.Append(fileDiags) - for name, value := range fileValues { - values[name] = value - } + value := cty.DynamicVal + if !ctxDiags.HasErrors() { + var valueDiags hcl.Diagnostics + value, valueDiags = expr.Value(ctx) + diags = diags.Append(valueDiags) + } - // We want to make sure every variable declared in the run block is actually relevant. - validateRelevance := func(name string, expr hcl.Expression) (diags tfdiags.Diagnostics) { - if !relevantVariables[name] { - // We'll add a warning for this. Since we're right in the run block - // users shouldn't be defining variables that are not relevant. + // We do this late on so we still validate whatever it was that the user + // wrote in the variable expression. But, we don't want to actually use + // it if it's not actually relevant. + if _, exists := relevantVariables[name]; !exists { diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagWarning, Summary: "Value for undeclared variable", Detail: fmt.Sprintf("The module under test does not declare a variable named %q, but it is declared in run block %q.", name, run.Name), Subject: expr.Range().Ptr(), }) + + continue // Don't add it to our final set of variables. + } + + values[name] = &terraform.InputValue{ + Value: value, + SourceType: terraform.ValueFromConfig, + SourceRange: tfdiags.SourceRangeFromHCL(expr.Range()), } - return diags } - // Third, we'll check the run level variables. - runValues, runDiags := runner.getVariablesFromConfiguration(values, validateRelevance, run.Config.Variables) - diags = diags.Append(runDiags) - for name, value := range runValues { - values[name] = value + for variable := range relevantVariables { + if _, exists := values[variable]; exists { + // Then we've already got a value for this variable. + continue + } + + // Otherwise, we'll get it from the cache as a file-level or global + // variable. + cache := runner.VariableCaches.GetCache(run.Name, config) + + value, valueDiags := cache.GetFileVariable(variable) + diags = diags.Append(valueDiags) + if value != nil { + values[variable] = value + continue + } + + value, valueDiags = cache.GetGlobalVariable(variable) + diags = diags.Append(valueDiags) + if value != nil { + values[variable] = value + continue + } } // Finally, we check the configuration again. This is where we'll discover @@ -1140,89 +1153,6 @@ func (runner *TestFileRunner) GetVariables(config *configs.Config, run *modulete return values, diags } -func (runner *TestFileRunner) getGlobalVariable(name string, variable backendrun.UnparsedVariableValue, config *configs.Config) *terraform.InputValue { - // By default, we parse global variables as HCL inputs. - parsingMode := configs.VariableParseHCL - - cfg, exists := config.Module.Variables[name] - - if exists { - // Unless we have some configuration that can actually tell us - // what parsing mode to use. - parsingMode = cfg.ParsingMode - } - - value, diags := variable.ParseVariableValue(parsingMode) - if diags.HasErrors() { - // We still add a value for this variable even though we couldn't - // parse it as we don't want to compound errors later. For example, - // the system would report this variable didn't have a value which - // would confuse the user because it does have a value, it's just - // not a valid value. We have added the diagnostics so the user - // will be informed about the error, and the test won't run. We'll - // just report only the relevant errors. - return &terraform.InputValue{ - Value: cty.NilVal, - } - } - return value -} - -// getVariablesFromConfiguration will process the variables from the configuration -// and return a map of the variables and their values. -func (runner *TestFileRunner) getVariablesFromConfiguration(knownVariables terraform.InputValues, validateRelevance func(string, hcl.Expression) tfdiags.Diagnostics, variableConfig map[string]hcl.Expression) (terraform.InputValues, tfdiags.Diagnostics) { - var exprs []hcl.Expression - var diags tfdiags.Diagnostics - variableValues := make(terraform.InputValues) - - // Preload the available expressions, we're going to validate them when we - // build the context. - for _, expr := range variableConfig { - exprs = append(exprs, expr) - } - - // Preformat the variables we've processed already - these will be made - // available to the eval context. - variables := make(map[string]cty.Value) - for name, value := range knownVariables { - variables[name] = value.Value - } - - ctx, ctxDiags := hcltest.EvalContext(hcltest.TargetRunBlock, exprs, variables, runner.PriorOutputs) - diags = diags.Append(ctxDiags) - - var failedContext bool - 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. - failedContext = true - } - - for name, expr := range variableConfig { - relevanceDiags := validateRelevance(name, expr) - diags = diags.Append(relevanceDiags) - if len(relevanceDiags) > 0 { - continue - } - - value := cty.NilVal - if !failedContext { - var valueDiags hcl.Diagnostics - value, valueDiags = expr.Value(ctx) - diags = diags.Append(valueDiags) - } - - variableValues[name] = &terraform.InputValue{ - Value: value, - SourceType: terraform.ValueFromConfig, - SourceRange: tfdiags.SourceRangeFromHCL(expr.Range()), - } - } - - return variableValues, diags -} - // FilterVariablesToModule splits the provided values into two disjoint maps: // moduleVars contains the ones that correspond with declarations in the root // module of the given configuration, while testOnlyVars contains any others @@ -1289,51 +1219,6 @@ func (runner *TestFileRunner) AddVariablesToConfig(config *configs.Config, varia } } -// initVariables initialises the globalVariables within the test runner by -// merging the global variables from the test suite into the variables from -// the file. -func (runner *TestFileRunner) initVariables(file *moduletest.File) { - // First, we get the global variables from the suite and test suite - runner.globalVariables = make(terraform.InputValues) - for name, value := range runner.Suite.GlobalVariables { - runner.globalVariables[name] = runner.getGlobalVariable(name, value, runner.Suite.Config) - } - if filepath.Dir(file.Name) == runner.Suite.TestingDirectory { - // If the file is in the testing directory, then also include any - // variables that are defined within the default variable file also in - // the test directory. - for name, value := range runner.Suite.GlobalTestVariables { - runner.globalVariables[name] = runner.getGlobalVariable(name, value, runner.Suite.Config) - } - } - - // Second, we collect the variable expressions so they can later be used to - // check for references to variables that are also relevant - runner.fileVariableExpressions = make(map[string]hcl.Expression) - for name, expr := range file.Config.Variables { - runner.fileVariableExpressions[name] = expr - } - - // Third, we get the variables from the file - runner.fileVariables = make(terraform.InputValues) - fileValues, fileDiags := runner.getVariablesFromConfiguration(runner.globalVariables, func(s string, e hcl.Expression) tfdiags.Diagnostics { return tfdiags.Diagnostics{} }, runner.fileVariableExpressions) - - for name, value := range fileValues { - runner.fileVariables[name] = value - } - file.Diagnostics = file.Diagnostics.Append(fileDiags) - - // Finally, we merge the global and file variables together to get all - // available variables outside the run specific ones - runner.globalAndFileVariables = make(terraform.InputValues) - for name, value := range runner.globalVariables { - runner.globalAndFileVariables[name] = value - } - for name, value := range runner.fileVariables { - runner.globalAndFileVariables[name] = value - } -} - func (runner *TestFileRunner) gatherProviders(key string, config *configs.Config) { if _, exists := runner.Suite.configProviders[key]; exists { // Then we've processed this key before, so skip it. diff --git a/internal/command/test_test.go b/internal/command/test_test.go index cff14768e2bf..7ac4f9655e39 100644 --- a/internal/command/test_test.go +++ b/internal/command/test_test.go @@ -6,6 +6,7 @@ package command import ( "encoding/json" "fmt" + "os" "path" "strings" "testing" @@ -25,6 +26,7 @@ func TestTest_Runs(t *testing.T) { tcs := map[string]struct { override string args []string + envVars map[string]string expectedOut string expectedErr []string expectedResourceCount int @@ -237,7 +239,7 @@ func TestTest_Runs(t *testing.T) { code: 0, }, "global_var_refs": { - expectedOut: "2 failed, 1 skipped.", + expectedOut: "1 passed, 2 failed.", expectedErr: []string{"The input variable \"env_var_input\" is not available to the current context", "The input variable \"setup\" is not available to the current context"}, code: 1, }, @@ -245,6 +247,20 @@ func TestTest_Runs(t *testing.T) { expectedOut: "1 passed, 0 failed.", code: 0, }, + "env-vars": { + expectedOut: "1 passed, 0 failed.", + envVars: map[string]string{ + "TF_VAR_input": "foo", + }, + code: 0, + }, + "env-vars-in-module": { + expectedOut: "2 passed, 0 failed.", + envVars: map[string]string{ + "TF_VAR_input": "foo", + }, + code: 0, + }, } for name, tc := range tcs { t.Run(name, func(t *testing.T) { @@ -252,6 +268,15 @@ func TestTest_Runs(t *testing.T) { t.Skip() } + for k, v := range tc.envVars { + os.Setenv(k, v) + } + defer func() { + for k := range tc.envVars { + os.Unsetenv(k) + } + }() + file := name if len(tc.override) > 0 { file = tc.override @@ -1300,10 +1325,8 @@ Error: Reference to unavailable variable on main.tftest.hcl line 15, in run "test": 15: input_one = var.notreal -The input variable "notreal" is not available to the current context. Within -the variables block of a run block you can only reference variables defined -at the file or global levels; within the variables block of a suite you can -only reference variables defined at the global levels. +The input variable "notreal" is not available to the current run block. You +can only reference variables defined at the file or global levels. Error: Reference to unavailable run block @@ -1328,10 +1351,9 @@ Error: Reference to unavailable variable on providers.tftest.hcl line 3, in provider "test": 3: resource_prefix = var.default -The input variable "default" is not available to the current context. Within -the variables block of a run block you can only reference variables defined -at the file or global levels; within the variables block of a suite you can -only reference variables defined at the global levels. +The input variable "default" is not available to the current provider +configuration. You can only reference variables defined at the file or global +levels. ` actualErr := output.Stderr() if diff := cmp.Diff(actualErr, expectedErr); len(diff) > 0 { diff --git a/internal/command/testdata/test/env-vars-in-module/main.tf b/internal/command/testdata/test/env-vars-in-module/main.tf new file mode 100644 index 000000000000..13d283df9216 --- /dev/null +++ b/internal/command/testdata/test/env-vars-in-module/main.tf @@ -0,0 +1 @@ +resource "test_resource" "resource" {} diff --git a/internal/command/testdata/test/env-vars-in-module/main.tftest.hcl b/internal/command/testdata/test/env-vars-in-module/main.tftest.hcl new file mode 100644 index 000000000000..8b33650bb2cd --- /dev/null +++ b/internal/command/testdata/test/env-vars-in-module/main.tftest.hcl @@ -0,0 +1,7 @@ +run "module" { + module { + source = "./mod" + } +} + +run "test" {} diff --git a/internal/command/testdata/test/env-vars-in-module/mod/main.tf b/internal/command/testdata/test/env-vars-in-module/mod/main.tf new file mode 100644 index 000000000000..4d5816c339a1 --- /dev/null +++ b/internal/command/testdata/test/env-vars-in-module/mod/main.tf @@ -0,0 +1,5 @@ +variable "input" {} + +output "value" { + value = var.input +} diff --git a/internal/command/testdata/test/env-vars/main.tf b/internal/command/testdata/test/env-vars/main.tf new file mode 100644 index 000000000000..97c6896727a3 --- /dev/null +++ b/internal/command/testdata/test/env-vars/main.tf @@ -0,0 +1,5 @@ +variable "input" {} + +resource "test_resource" "resource" { + value = var.input +} diff --git a/internal/command/testdata/test/env-vars/main.tftest.hcl b/internal/command/testdata/test/env-vars/main.tftest.hcl new file mode 100644 index 000000000000..eb3013a8779a --- /dev/null +++ b/internal/command/testdata/test/env-vars/main.tftest.hcl @@ -0,0 +1 @@ +run "test" {} diff --git a/internal/moduletest/config/config.go b/internal/moduletest/config/config.go index a1ea88b2891f..71c6527d46e9 100644 --- a/internal/moduletest/config/config.go +++ b/internal/moduletest/config/config.go @@ -13,7 +13,6 @@ import ( "github.com/hashicorp/terraform/internal/configs" "github.com/hashicorp/terraform/internal/moduletest" hcltest "github.com/hashicorp/terraform/internal/moduletest/hcl" - "github.com/hashicorp/terraform/internal/terraform" ) // TransformConfigForTest transforms the provided configuration ready for the @@ -27,7 +26,7 @@ import ( // We also return a reset function that should be called to return the // configuration to it's original state before the next run block or test file // needs to use it. -func TransformConfigForTest(config *configs.Config, run *moduletest.Run, file *moduletest.File, availableVariables terraform.InputValues, availableRunOutputs map[addrs.Run]cty.Value, requiredProviders map[string]bool) (func(), hcl.Diagnostics) { +func TransformConfigForTest(config *configs.Config, run *moduletest.Run, file *moduletest.File, variableCaches *hcltest.VariableCaches, availableRunOutputs map[addrs.Run]cty.Value, requiredProviders map[string]bool) (func(), hcl.Diagnostics) { var diags hcl.Diagnostics // Currently, we only need to override the provider settings. @@ -91,7 +90,7 @@ func TransformConfigForTest(config *configs.Config, run *moduletest.Run, file *m Version: testProvider.Version, Config: &hcltest.ProviderConfig{ Original: testProvider.Config, - AvailableVariables: availableVariables, + VariableCache: variableCaches.GetCache(run.Name, config), AvailableRunOutputs: availableRunOutputs, }, Mock: testProvider.Mock, @@ -118,7 +117,7 @@ func TransformConfigForTest(config *configs.Config, run *moduletest.Run, file *m Version: provider.Version, Config: &hcltest.ProviderConfig{ Original: provider.Config, - AvailableVariables: availableVariables, + VariableCache: variableCaches.GetCache(run.Name, config), AvailableRunOutputs: availableRunOutputs, }, Mock: provider.Mock, diff --git a/internal/moduletest/config/config_test.go b/internal/moduletest/config/config_test.go index b572bac5081b..2775efcbe14a 100644 --- a/internal/moduletest/config/config_test.go +++ b/internal/moduletest/config/config_test.go @@ -14,8 +14,10 @@ import ( "github.com/hashicorp/hcl/v2" "github.com/hashicorp/hcl/v2/hclparse" + "github.com/hashicorp/terraform/internal/backend/backendrun" "github.com/hashicorp/terraform/internal/configs" "github.com/hashicorp/terraform/internal/moduletest" + hcltest "github.com/hashicorp/terraform/internal/moduletest/hcl" ) func TestTransformForTest(t *testing.T) { @@ -244,7 +246,12 @@ func TestTransformForTest(t *testing.T) { availableProviders[provider] = true } - reset, diags := TransformConfigForTest(config, run, file, nil, nil, availableProviders) + variableCaches := &hcltest.VariableCaches{ + GlobalVariables: make(map[string]backendrun.UnparsedVariableValue), + FileVariables: make(map[string]hcl.Expression), + } + + reset, diags := TransformConfigForTest(config, run, file, variableCaches, nil, availableProviders) var actualErrs []string for _, err := range diags.Errs() { diff --git a/internal/moduletest/hcl/context.go b/internal/moduletest/hcl/context.go index 46e1842f5563..24ae4fd3fe8b 100644 --- a/internal/moduletest/hcl/context.go +++ b/internal/moduletest/hcl/context.go @@ -18,8 +18,9 @@ import ( type EvalContextTarget string const ( - TargetRunBlock EvalContextTarget = "run" - TargetProvider EvalContextTarget = "provider" + TargetRunBlock EvalContextTarget = "run" + TargetProvider EvalContextTarget = "provider" + TargetFileVariable EvalContextTarget = "file" ) // EvalContext builds hcl.EvalContext objects for use directly within the @@ -49,7 +50,7 @@ const ( // will be used to evaluate. This is just so we can provide some better error // messages and diagnostics. The expressions argument could be empty without // affecting the returned context. -func EvalContext(target EvalContextTarget, expressions []hcl.Expression, availableVariables map[string]cty.Value, availableRunOutputs map[addrs.Run]cty.Value) (*hcl.EvalContext, tfdiags.Diagnostics) { +func EvalContext(target EvalContextTarget, expressions map[string]hcl.Expression, availableVariables map[string]cty.Value, availableRunOutputs map[addrs.Run]cty.Value) (*hcl.EvalContext, tfdiags.Diagnostics) { var diags tfdiags.Diagnostics runs := make(map[string]cty.Value, len(availableRunOutputs)) @@ -63,6 +64,18 @@ func EvalContext(target EvalContextTarget, expressions []hcl.Expression, availab for _, ref := range refs { if addr, ok := ref.Subject.(addrs.Run); ok { + if target == TargetFileVariable { + // You can't reference run blocks from within the file + // variables block. + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid reference", + Detail: "You can not reference run blocks from within the file variables block.", + Subject: ref.SourceRange.ToHCL().Ptr(), + }) + continue + } + objVal, exists := availableRunOutputs[addr] var diagPrefix string @@ -133,9 +146,14 @@ func EvalContext(target EvalContextTarget, expressions []hcl.Expression, availab if _, exists := availableVariables[addr.Name]; !exists { // This variable reference doesn't exist. - detail := fmt.Sprintf("The input variable %q is not available to the current context. Within the variables block of a run block you can only reference variables defined at the file or global levels; within the variables block of a suite you can only reference variables defined at the global levels.", addr.Name) - if availableRunOutputs == nil { - detail = fmt.Sprintf("The input variable %q is not available to the current provider configuration. You can only reference variables defined at the file or global levels within provider configurations.", addr.Name) + var detail string + switch target { + case TargetRunBlock: + detail = fmt.Sprintf("The input variable %q is not available to the current run block. You can only reference variables defined at the file or global levels.", addr.Name) + case TargetProvider: + detail = fmt.Sprintf("The input variable %q is not available to the current provider configuration. You can only reference variables defined at the file or global levels.", addr.Name) + case TargetFileVariable: + detail = fmt.Sprintf("The input variable %q is not available to the current context. You can only reference global variables.", addr.Name) } diags = diags.Append(&hcl.Diagnostic{ @@ -152,9 +170,14 @@ func EvalContext(target EvalContextTarget, expressions []hcl.Expression, availab continue } - detail := "You can only reference earlier run blocks, file level, and global variables while defining variables from inside a run block." - if availableRunOutputs == nil { - detail = "You can only reference file level and global variables from inside provider configurations within test files." + var detail string + switch target { + case TargetRunBlock: + detail = "You can only reference earlier run blocks, file level, and global variables while defining variables from inside a run block." + case TargetProvider: + detail = "You can only reference run blocks, file level, and global variables while defining variables from inside provider configurations." + case TargetFileVariable: + detail = "You can only reference global variables within the test file variables block." } // You can only reference run blocks and variables from the run diff --git a/internal/moduletest/hcl/provider.go b/internal/moduletest/hcl/provider.go index ba35bb948472..c5bce33de7c6 100644 --- a/internal/moduletest/hcl/provider.go +++ b/internal/moduletest/hcl/provider.go @@ -9,7 +9,6 @@ import ( "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/lang/langrefs" - "github.com/hashicorp/terraform/internal/terraform" ) var _ hcl.Body = (*ProviderConfig)(nil) @@ -22,10 +21,13 @@ var _ hcl.Body = (*ProviderConfig)(nil) // framework, so they should only use variables available to the test framework // but are instead initialised within the Terraform graph so we have to delay // evaluation of their attributes until the schemas are retrieved. +// +// We don't parse the attributes until they are requested, so we can only use +// unparsed values and hcl.Expressions within the struct itself. type ProviderConfig struct { Original hcl.Body - AvailableVariables terraform.InputValues + VariableCache *VariableCache AvailableRunOutputs map[addrs.Run]cty.Value } @@ -50,7 +52,7 @@ func (p *ProviderConfig) PartialContent(schema *hcl.BodySchema) (*hcl.BodyConten Attributes: attrs, Blocks: p.transformBlocks(content.Blocks), MissingItemRange: content.MissingItemRange, - }, &ProviderConfig{rest, p.AvailableVariables, p.AvailableRunOutputs}, diags + }, &ProviderConfig{rest, p.VariableCache, p.AvailableRunOutputs}, diags } func (p *ProviderConfig) JustAttributes() (hcl.Attributes, hcl.Diagnostics) { @@ -67,10 +69,10 @@ func (p *ProviderConfig) transformAttributes(originals hcl.Attributes) (hcl.Attr var diags hcl.Diagnostics availableVariables := make(map[string]cty.Value) - var exprs []hcl.Expression + exprs := make(map[string]hcl.Expression, len(originals)) for _, original := range originals { - exprs = append(exprs, original.Expr) + exprs[original.Name] = original.Expr // We also need to parse the variables we're going to use, so we extract // the references from this expression now and see if they reference any @@ -79,17 +81,19 @@ 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 { - if _, exists := availableVariables[addr.Name]; exists { - // Then we've processed this variable before. This just - // means it's referenced twice in this provider config - - // which is fine, we just don't need to do it again. + value, valueDiags := p.VariableCache.GetFileVariable(addr.Name) + diags = append(diags, valueDiags.ToHCL()...) + if value != nil { + availableVariables[addr.Name] = value.Value continue } - if value, exists := p.AvailableVariables[addr.Name]; exists { - if value != nil { - availableVariables[addr.Name] = value.Value - } + // If the variable wasn't a file variable, it might be a global. + value, valueDiags = p.VariableCache.GetGlobalVariable(addr.Name) + diags = append(diags, valueDiags.ToHCL()...) + if value != nil { + availableVariables[addr.Name] = value.Value + continue } } } @@ -125,7 +129,7 @@ func (p *ProviderConfig) transformBlocks(originals hcl.Blocks) hcl.Blocks { blocks[name] = &hcl.Block{ Type: block.Type, Labels: block.Labels, - Body: &ProviderConfig{block.Body, p.AvailableVariables, p.AvailableRunOutputs}, + Body: &ProviderConfig{block.Body, p.VariableCache, p.AvailableRunOutputs}, DefRange: block.DefRange, TypeRange: block.TypeRange, LabelRanges: block.LabelRanges, diff --git a/internal/moduletest/hcl/provider_test.go b/internal/moduletest/hcl/provider_test.go index f122e918c073..726c26a49fc6 100644 --- a/internal/moduletest/hcl/provider_test.go +++ b/internal/moduletest/hcl/provider_test.go @@ -14,9 +14,6 @@ import ( "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/backend/backendrun" - "github.com/hashicorp/terraform/internal/configs" - "github.com/hashicorp/terraform/internal/terraform" - "github.com/hashicorp/terraform/internal/tfdiags" ) func TestProviderConfig(t *testing.T) { @@ -71,7 +68,7 @@ func TestProviderConfig(t *testing.T) { "input": cty.StringVal("string"), }, expectedErrors: []string{ - "The input variable \"missing\" is not available to the current context. Within the variables block of a run block you can only reference variables defined at the file or global levels; within the variables block of a suite you can only reference variables defined at the global levels.", + "The input variable \"missing\" is not available to the current provider configuration. You can only reference variables defined at the file or global levels.", }, validate: func(t *testing.T, content *hcl.BodyContent) { if len(content.Attributes) > 0 { @@ -154,7 +151,7 @@ func TestProviderConfig(t *testing.T) { "setup": nil, }, expectedErrors: []string{ - "You can only reference earlier run blocks, file level, and global variables while defining variables from inside a run block.", + "You can only reference run blocks, file level, and global variables while defining variables from inside provider configurations.", }, validate: func(t *testing.T, content *hcl.BodyContent) { if len(content.Attributes) > 0 { @@ -172,35 +169,37 @@ func TestProviderConfig(t *testing.T) { t.Fatalf("failed to parse hcl: %s", diags.Error()) } - config := ProviderConfig{ - Original: file.Body, - AvailableVariables: func() terraform.InputValues { - variables := make(terraform.InputValues) + outputs := make(map[addrs.Run]cty.Value) + for name, values := range tc.runBlockOutputs { + addr := addrs.Run{Name: name} + if values == nil { + outputs[addr] = cty.NilVal + continue + } + + attrs := make(map[string]cty.Value) + for name, value := range values { + attrs[name] = value + } + + outputs[addr] = cty.ObjectVal(attrs) + } + + variableCaches := &VariableCaches{ + GlobalVariables: make(map[string]backendrun.UnparsedVariableValue), + FileVariables: func() map[string]hcl.Expression { + variables := make(map[string]hcl.Expression) for name, value := range tc.variables { - variables[name] = &terraform.InputValue{ - Value: value, - } + variables[name] = hcl.StaticExpr(value, hcl.Range{}) } return variables }(), - AvailableRunOutputs: func() map[addrs.Run]cty.Value { - outputs := make(map[addrs.Run]cty.Value) - for name, values := range tc.runBlockOutputs { - addr := addrs.Run{Name: name} - if values == nil { - outputs[addr] = cty.NilVal - continue - } - - attrs := make(map[string]cty.Value) - for name, value := range values { - attrs[name] = value - } - - outputs[addr] = cty.ObjectVal(attrs) - } - return outputs - }(), + } + + config := ProviderConfig{ + Original: file.Body, + VariableCache: variableCaches.GetCache("test", nil), + AvailableRunOutputs: outputs, } content, diags := config.Content(tc.schema) @@ -227,16 +226,3 @@ func equals(t *testing.T, content *hcl.BodyContent, attribute string, expected c t.Errorf("expected:\n%s\nbut got:\n%s", expected.GoString(), value.GoString()) } } - -var _ backendrun.UnparsedVariableValue = (*variable)(nil) - -type variable struct { - value cty.Value -} - -func (v *variable) ParseVariableValue(mode configs.VariableParsingMode) (*terraform.InputValue, tfdiags.Diagnostics) { - return &terraform.InputValue{ - Value: v.value, - SourceType: terraform.ValueFromUnknown, - }, nil -} diff --git a/internal/moduletest/hcl/variable_cache.go b/internal/moduletest/hcl/variable_cache.go new file mode 100644 index 000000000000..dfe1ad21ad45 --- /dev/null +++ b/internal/moduletest/hcl/variable_cache.go @@ -0,0 +1,183 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +package hcl + +import ( + "github.com/hashicorp/hcl/v2" + "github.com/zclconf/go-cty/cty" + + "github.com/hashicorp/terraform/internal/addrs" + "github.com/hashicorp/terraform/internal/backend/backendrun" + "github.com/hashicorp/terraform/internal/configs" + "github.com/hashicorp/terraform/internal/lang/langrefs" + "github.com/hashicorp/terraform/internal/terraform" + "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 +} + +// 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 + + globals terraform.InputValues + files terraform.InputValues + + values *VariableCaches // back reference so we can access the stored values +} + +// 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 { + if caches.caches == nil { + caches.caches = make(map[string]*VariableCache) + } + + cache, exists := caches.caches[name] + if !exists { + cache = &VariableCache{ + config: config, + globals: make(terraform.InputValues), + files: make(terraform.InputValues), + values: caches, + } + caches.caches[name] = cache + } + return cache +} + +// 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] + if !exists { + 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 + } + + 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, + } + } + + cache.globals[name] = value + return value, 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] + if !exists { + return nil, nil + } + + var diags tfdiags.Diagnostics + + availableVariables := make(map[string]cty.Value) + 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) + if variable != nil { + availableVariables[input.Name] = variable.Value + } + } + } + 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{ + Value: cty.DynamicVal, + } + return cache.files[name], 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{ + Value: cty.DynamicVal, + } + return cache.files[name], diags + } + + value, valueDiags := expr.Value(ctx) + diags = diags.Append(valueDiags) + 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 = cty.DynamicVal + } + + cache.files[name] = &terraform.InputValue{ + Value: value, + SourceType: terraform.ValueFromConfig, + SourceRange: tfdiags.SourceRangeFromHCL(expr.Range()), + } + return cache.files[name], diags +} diff --git a/internal/moduletest/hcl/variable_cache_test.go b/internal/moduletest/hcl/variable_cache_test.go new file mode 100644 index 000000000000..ed5e4c39ce78 --- /dev/null +++ b/internal/moduletest/hcl/variable_cache_test.go @@ -0,0 +1,235 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +package hcl + +import ( + "github.com/google/go-cmp/cmp" + "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/hcl/v2/hclsyntax" + "github.com/zclconf/go-cty-debug/ctydebug" + "github.com/zclconf/go-cty/cty" + + "github.com/hashicorp/terraform/internal/backend/backendrun" + "github.com/hashicorp/terraform/internal/configs" + "github.com/hashicorp/terraform/internal/terraform" + "github.com/hashicorp/terraform/internal/tfdiags" + + "testing" +) + +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 + }{ + "no_variables": { + Want: make(map[string]cty.Value), + }, + "string": { + Values: map[string]string{ + "foo": `"bar"`, + }, + Want: map[string]cty.Value{ + "foo": cty.StringVal("bar"), + }, + }, + "boolean": { + Values: map[string]string{ + "foo": "true", + }, + Want: map[string]cty.Value{ + "foo": cty.BoolVal(true), + }, + }, + "reference": { + Values: map[string]string{ + "foo": "var.bar", + }, + GlobalValues: map[string]string{ + "bar": `"baz"`, + }, + Variables: map[string]configs.VariableParsingMode{ + "foo": configs.VariableParseLiteral, + }, + Want: map[string]cty.Value{ + "foo": cty.StringVal("baz"), + }, + }, + } + + for name, tc := range tcs { + t.Run(name, func(t *testing.T) { + + caches := &VariableCaches{ + 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{0, 0, 0}) + if len(diags) > 0 { + t.Fatalf("unexpected errors: %v", diags) + } + vars[name] = expr + } + return vars + }(), + 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", + }, + Variables: map[string]configs.VariableParsingMode{ + "foo": configs.VariableParseHCL, + }, + Want: map[string]cty.Value{ + "foo": cty.BoolVal(true), + }, + }, + "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"), + }, + }, + "missing_config": { + Values: map[string]string{ + "foo": `"bar"`, + }, + Want: map[string]cty.Value{ + "foo": cty.StringVal("bar"), + }, + }, + } + + for name, tc := range tcs { + t.Run(name, func(t *testing.T) { + + caches := &VariableCaches{ + GlobalVariables: func() map[string]backendrun.UnparsedVariableValue { + vars := make(map[string]backendrun.UnparsedVariableValue) + for name, value := range tc.Values { + 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.GetGlobalVariable(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 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) + +type variable struct { + name string + value string +} + +func (v *variable) ParseVariableValue(mode configs.VariableParsingMode) (*terraform.InputValue, tfdiags.Diagnostics) { + var diags tfdiags.Diagnostics + value, valueDiags := mode.Parse(v.name, v.value) + diags = diags.Append(valueDiags) + return &terraform.InputValue{ + Value: value, + SourceType: terraform.ValueFromUnknown, + }, diags +}