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 <[email protected]>

* address comments

---------

Co-authored-by: kmoe <[email protected]>

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,21 +239,44 @@ 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,
 		},
 		"global_var_ref_in_suite_var": {
 			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) {
 			if tc.skip {
 				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 {

internal/command/testdata/test/env-vars-in-module/main.tf

@@ -0,0 +1 @@
+resource "test_resource" "resource" {}

internal/command/testdata/test/env-vars-in-module/main.tftest.hcl

@@ -0,0 +1,7 @@
+run "module" {
+  module {
+    source = "./mod"
+  }
+}
+
+run "test" {}

internal/command/testdata/test/env-vars-in-module/mod/main.tf

@@ -0,0 +1,5 @@
+variable "input" {}
+
+output "value" {
+  value = var.input
+}

internal/command/testdata/test/env-vars/main.tf

@@ -0,0 +1,5 @@
+variable "input" {}
+
+resource "test_resource" "resource" {
+  value = var.input
+}

internal/command/testdata/test/env-vars/main.tftest.hcl

@@ -0,0 +1 @@
+run "test" {}

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,

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() {

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

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,