diff --git a/.changes/v1.15/BUG FIXES-20260430-152314.yaml b/.changes/v1.15/BUG FIXES-20260430-152314.yaml new file mode 100644 index 0000000000..c69854def0 --- /dev/null +++ b/.changes/v1.15/BUG FIXES-20260430-152314.yaml @@ -0,0 +1,5 @@ +kind: BUG FIXES +body: 'Fixed "unknown provider function" errors occurring during init' +time: 2026-04-30T15:23:14.65033-04:00 +custom: + Issue: "38472" diff --git a/internal/command/init2_test.go b/internal/command/init2_test.go index 4f239bdc61..6f8e91188a 100644 --- a/internal/command/init2_test.go +++ b/internal/command/init2_test.go @@ -34,7 +34,7 @@ func TestInit2_dynamicSourceErrors(t *testing.T) { "non-const variable in module source": { fixture: "local-source-with-non-const-variable", args: []string{"-var", "module_name=example"}, - wantError: "Invalid module source", + wantError: "Unknown module source", }, "resource reference in module source": { fixture: "source-with-resource-reference", @@ -70,6 +70,32 @@ func TestInit2_dynamicSourceErrors(t *testing.T) { args: []string{"-get=false", "-var", "module_version=0.0.2"}, wantError: "Module version requirements have changed", }, + + "const variable with static validation check": { + fixture: "const-var-source-with-validation", + args: []string{"-var", "module_name=nonexistent"}, + wantError: "The module_name variable must be set to \"example\"", + }, + // This error is in addition to the expected validation error in the test case above and is + // a consequence of validation blocks having their own node in the graph, thus a validation + // error doesn't prevent the module install node from executing. + "const variable with static validation check - module install bug": { + fixture: "const-var-source-with-validation", + args: []string{"-var", "module_name=nonexistent"}, + wantError: "Unable to evaluate directory symlink: lstat modules/nonexistent", + }, + + // we can't determine if a provider function fails somewhere in the + // evaluation chain during init, so we have to return a generic error + // about unknown values. + "provider function in module source": { + fixture: "provider-function-in-source", + wantError: "Only literal values and const variables can be evaluated during init.", + }, + "provider function in module version": { + fixture: "provider-function-in-version", + wantError: "Only literal values and const variables can be evaluated during init.", + }, } for name, tc := range tests { @@ -78,6 +104,10 @@ func TestInit2_dynamicSourceErrors(t *testing.T) { testCopyDir(t, testFixturePath(filepath.Join("dynamic-module-sources", tc.fixture)), td) t.Chdir(td) + providerSource := newMockProviderSource(t, map[string][]string{ + "hashicorp/test": {"1.0.0"}, + }) + ui := new(cli.MockUi) view, done := testView(t) c := &InitCommand{ @@ -85,6 +115,7 @@ func TestInit2_dynamicSourceErrors(t *testing.T) { testingOverrides: metaOverridesForProvider(testProvider()), Ui: ui, View: view, + ProviderSource: providerSource, }, } @@ -131,6 +162,23 @@ func TestInit2_dynamicSourceSuccess(t *testing.T) { "path.module in module source": { fixture: "path-attr-in-module-source", }, + "const variable with static validation check": { + fixture: "const-var-source-with-validation", + args: []string{"-var", "module_name=example"}, + }, + "const variable with validation referencing non-const variable": { + fixture: "const-var-with-validation-and-reference", + // the non_const_var variable is not defined here since we are only testing init + // so the value will always be unknown regardless of input. + args: []string{"-var", "const_var=hello"}, + }, + + // The validation returns unknown during init, and must wait until + // a full validation is actually run. + "provider function in const variable validation check": { + fixture: "provider-function-in-const-validation", + args: []string{"-var", "module_name=example"}, + }, } for name, tc := range tests { @@ -139,6 +187,10 @@ func TestInit2_dynamicSourceSuccess(t *testing.T) { testCopyDir(t, testFixturePath(filepath.Join("dynamic-module-sources", tc.fixture)), td) t.Chdir(td) + providerSource := newMockProviderSource(t, map[string][]string{ + "hashicorp/test": {"1.0.0"}, + }) + ui := new(cli.MockUi) view, done := testView(t) c := &InitCommand{ @@ -146,6 +198,7 @@ func TestInit2_dynamicSourceSuccess(t *testing.T) { testingOverrides: metaOverridesForProvider(testProvider()), Ui: ui, View: view, + ProviderSource: providerSource, }, } @@ -265,9 +318,9 @@ func TestInit2_reinitWithDifferentVariable(t *testing.T) { } func TestInit2_fromModuleWithDynamicSource(t *testing.T) { - // TODO: -from-module currently panics when the copied configuration + // TODO: -from-module currently errors when the copied configuration // contains a dynamic module source (e.g. "./modules/${var.module_name}"). - t.Skip("skipping: -from-module panics on dynamic module sources (see TODO in from_module.go)") + t.Skip("skipping: -from-module errors on dynamic module sources (see TODO in from_module.go)") // Create an empty target directory for -from-module to copy into td := t.TempDir() @@ -287,7 +340,7 @@ func TestInit2_fromModuleWithDynamicSource(t *testing.T) { // into the empty working directory. This should copy the files but the // nested dynamic module won't be resolved by -from-module itself. srcDir := testFixturePath(filepath.Join("dynamic-module-sources", "from-module-with-dynamic-source", "source-module")) - args := []string{"-from-module=" + srcDir} + args := []string{"-from-module=" + srcDir, "-var", "module_name=test"} code := c.Run(args) testOutput := done(t) diff --git a/internal/command/test_test.go b/internal/command/test_test.go index d3dd717a73..5ece6a0712 100644 --- a/internal/command/test_test.go +++ b/internal/command/test_test.go @@ -449,7 +449,7 @@ func TestTest_Runs(t *testing.T) { }, "dynamic_source_non_const_var": { initCode: 1, - expectedErr: []string{"Invalid module source"}, + expectedErr: []string{"Unknown module source"}, code: 1, }, } diff --git a/internal/command/testdata/dynamic-module-sources/const-var-source-with-validation/main.tf b/internal/command/testdata/dynamic-module-sources/const-var-source-with-validation/main.tf new file mode 100644 index 0000000000..bb66122f91 --- /dev/null +++ b/internal/command/testdata/dynamic-module-sources/const-var-source-with-validation/main.tf @@ -0,0 +1,12 @@ +variable "module_name" { + type = string + const = true + validation { + condition = var.module_name == "example" + error_message = "The module_name variable must be set to \"example\"" + } +} + +module "example" { + source = "./modules/${var.module_name}" +} diff --git a/internal/command/testdata/dynamic-module-sources/const-var-source-with-validation/modules/example/empty.tf b/internal/command/testdata/dynamic-module-sources/const-var-source-with-validation/modules/example/empty.tf new file mode 100644 index 0000000000..8b13789179 --- /dev/null +++ b/internal/command/testdata/dynamic-module-sources/const-var-source-with-validation/modules/example/empty.tf @@ -0,0 +1 @@ + diff --git a/internal/command/testdata/dynamic-module-sources/const-var-with-validation-and-reference/main.tf b/internal/command/testdata/dynamic-module-sources/const-var-with-validation-and-reference/main.tf new file mode 100644 index 0000000000..9a01f63b59 --- /dev/null +++ b/internal/command/testdata/dynamic-module-sources/const-var-with-validation-and-reference/main.tf @@ -0,0 +1,19 @@ +variable "const_var" { + type = string + const = true + + # This validation block will be unknown during init because of the reference to var.non_const_var + validation { + condition = var.non_const_var == var.const_var + error_message = "The const_var variable must be equal the non_const_var variable" + } +} + +variable "non_const_var" { + type = string +} + +module "example" { + source = "./modules/example" + in = var.const_var +} diff --git a/internal/command/testdata/dynamic-module-sources/const-var-with-validation-and-reference/modules/example/main.tf b/internal/command/testdata/dynamic-module-sources/const-var-with-validation-and-reference/modules/example/main.tf new file mode 100644 index 0000000000..357ca34837 --- /dev/null +++ b/internal/command/testdata/dynamic-module-sources/const-var-with-validation-and-reference/modules/example/main.tf @@ -0,0 +1,4 @@ +variable "in" { + type = string + const = true +} diff --git a/internal/command/testdata/dynamic-module-sources/provider-function-in-const-validation/main.tf b/internal/command/testdata/dynamic-module-sources/provider-function-in-const-validation/main.tf new file mode 100644 index 0000000000..5fcf308b96 --- /dev/null +++ b/internal/command/testdata/dynamic-module-sources/provider-function-in-const-validation/main.tf @@ -0,0 +1,20 @@ +terraform { + required_providers { + test = { + source = "test" + } + } +} + +variable "module_name" { + type = string + const = true + validation { + condition = provider::test::is_true(var.module_name == "example") + error_message = "The module_name variable must be set to \"example\"" + } +} + +module "example" { + source = "./modules/${var.module_name}" +} diff --git a/internal/command/testdata/dynamic-module-sources/provider-function-in-const-validation/modules/example/empty.tf b/internal/command/testdata/dynamic-module-sources/provider-function-in-const-validation/modules/example/empty.tf new file mode 100644 index 0000000000..8b13789179 --- /dev/null +++ b/internal/command/testdata/dynamic-module-sources/provider-function-in-const-validation/modules/example/empty.tf @@ -0,0 +1 @@ + diff --git a/internal/command/testdata/dynamic-module-sources/provider-function-in-source/main.tf b/internal/command/testdata/dynamic-module-sources/provider-function-in-source/main.tf new file mode 100644 index 0000000000..005a7e9989 --- /dev/null +++ b/internal/command/testdata/dynamic-module-sources/provider-function-in-source/main.tf @@ -0,0 +1,11 @@ +terraform { + required_providers { + test = { + source = "test" + } + } +} + +module "example" { + source = provider::test::is_true(true) ? "./modules/example" : "" +} diff --git a/internal/command/testdata/dynamic-module-sources/provider-function-in-source/modules/example/empty.tf b/internal/command/testdata/dynamic-module-sources/provider-function-in-source/modules/example/empty.tf new file mode 100644 index 0000000000..8b13789179 --- /dev/null +++ b/internal/command/testdata/dynamic-module-sources/provider-function-in-source/modules/example/empty.tf @@ -0,0 +1 @@ + diff --git a/internal/command/testdata/dynamic-module-sources/provider-function-in-validation/main.tf b/internal/command/testdata/dynamic-module-sources/provider-function-in-validation/main.tf new file mode 100644 index 0000000000..b46f25f585 --- /dev/null +++ b/internal/command/testdata/dynamic-module-sources/provider-function-in-validation/main.tf @@ -0,0 +1,20 @@ +terraform { + required_providers { + test = { + source = "test" + } + } +} + +variable "module_input" { + type = string + validation { + condition = provider::test::is_true(var.module_input == "hello") + error_message = "The module_input variable must be set to \"hello\"" + } +} + +module "example" { + source = "./modules/example" + in = var.module_input +} diff --git a/internal/command/testdata/dynamic-module-sources/provider-function-in-validation/modules/example/main.tf b/internal/command/testdata/dynamic-module-sources/provider-function-in-validation/modules/example/main.tf new file mode 100644 index 0000000000..e0ca300755 --- /dev/null +++ b/internal/command/testdata/dynamic-module-sources/provider-function-in-validation/modules/example/main.tf @@ -0,0 +1,3 @@ +variable "in" { + type = string +} diff --git a/internal/command/testdata/dynamic-module-sources/provider-function-in-version/main.tf b/internal/command/testdata/dynamic-module-sources/provider-function-in-version/main.tf new file mode 100644 index 0000000000..f2122dedd4 --- /dev/null +++ b/internal/command/testdata/dynamic-module-sources/provider-function-in-version/main.tf @@ -0,0 +1,12 @@ +terraform { + required_providers { + test = { + source = "test" + } + } +} + +module "example" { + source = "./modules/example" + version = provider::test::is_true(true) ? "1.0.0" : "" +} diff --git a/internal/command/testdata/dynamic-module-sources/provider-function-in-version/modules/example/empty.tf b/internal/command/testdata/dynamic-module-sources/provider-function-in-version/modules/example/empty.tf new file mode 100644 index 0000000000..8b13789179 --- /dev/null +++ b/internal/command/testdata/dynamic-module-sources/provider-function-in-version/modules/example/empty.tf @@ -0,0 +1 @@ + diff --git a/internal/initwd/from_module.go b/internal/initwd/from_module.go index aeb6799664..01742a141f 100644 --- a/internal/initwd/from_module.go +++ b/internal/initwd/from_module.go @@ -219,6 +219,17 @@ func DirFromModule(ctx context.Context, loader *configload.Loader, rootDir, modu for _, mc := range mod.ModuleCalls { // TODO improve this sourceVal, _ := mc.SourceExpr.Value(nil) + + if !sourceVal.IsKnown() { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Unknown module source", + Detail: "Dynamic module sources cannot be used in conjunction with -from-module", + Subject: mc.SourceExpr.Range().Ptr(), + }) + return diags + } + sourceRaw := sourceVal.AsString() if pathTraversesUp(sourceRaw) { packageAddr, givenSubdir := moduleaddrs.SplitPackageSubdir(sourceAddrStr) diff --git a/internal/lang/eval.go b/internal/lang/eval.go index b1481a3641..249c93d370 100644 --- a/internal/lang/eval.go +++ b/internal/lang/eval.go @@ -6,6 +6,7 @@ package lang import ( "fmt" "log" + "maps" "strings" "github.com/hashicorp/hcl/v2" @@ -15,8 +16,6 @@ import ( "github.com/zclconf/go-cty/cty" "github.com/zclconf/go-cty/cty/convert" - "maps" - "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/configs/configschema" "github.com/hashicorp/terraform/internal/instances" @@ -75,7 +74,7 @@ func (s *Scope) EvalBlock(body hcl.Body, schema *configschema.Block) (cty.Value, body = blocktoattr.FixUpBlockAttrs(body, schema) val, evalDiags := hcldec.Decode(body, spec, ctx) - diags = diags.Append(checkForUnknownFunctionDiags(evalDiags)) + diags = diags.Append(CheckForUnknownFunctionDiags(evalDiags, s.IgnoreUnknownProviderFunctions)) return val, diags } @@ -153,7 +152,7 @@ func (s *Scope) EvalSelfBlock(body hcl.Body, self cty.Value, schema *configschem } val, decDiags := hcldec.Decode(body, schema.DecoderSpec(), ctx) - diags = diags.Append(checkForUnknownFunctionDiags(decDiags)) + diags = diags.Append(CheckForUnknownFunctionDiags(decDiags, s.IgnoreUnknownProviderFunctions)) return val, diags } @@ -179,7 +178,7 @@ func (s *Scope) EvalExpr(expr hcl.Expression, wantType cty.Type) (cty.Value, tfd } val, evalDiags := expr.Value(ctx) - diags = diags.Append(checkForUnknownFunctionDiags(evalDiags)) + diags = diags.Append(CheckForUnknownFunctionDiags(evalDiags, s.IgnoreUnknownProviderFunctions)) if wantType != cty.DynamicPseudoType { var convErr error @@ -517,23 +516,29 @@ func normalizeRefValue(val cty.Value, diags tfdiags.Diagnostics) (cty.Value, tfd return val, diags } -// checkForUnknownFunctionDiags inspects the diagnostics for errors from unknown +// CheckForUnknownFunctionDiags inspects the diagnostics for errors from unknown // function calls, and tailors the messages to better suit Terraform. We now // have multiple namespaces where functions may be declared, and it's up to the // user to have properly configured the module to populate the provider // namespace. The generic unknown function diagnostic from hcl does not direct // the user on how to remedy the situation in Terraform, and we can give more // useful information in a few Terraform specific cases here. -func checkForUnknownFunctionDiags(diags hcl.Diagnostics) hcl.Diagnostics { +func CheckForUnknownFunctionDiags(diags hcl.Diagnostics, ignoreUnknownProviderFunctions bool) hcl.Diagnostics { + var filteredDiags hcl.Diagnostics for _, d := range diags { extra, ok := hcl.DiagnosticExtra[hclsyntax.FunctionCallUnknownDiagExtra](d) if !ok { + filteredDiags = filteredDiags.Append(d) // we always want to include unrelated diags continue } + name := extra.CalledFunctionName() namespace := extra.CalledFunctionNamespace() namespaceParts := strings.Split(namespace, "::") if len(namespaceParts) < 2 { + // we always include diags for unknown regular function calls + filteredDiags = filteredDiags.Append(d) + // no namespace (namespace includes ::, so will have at least 2 // parts), but check if there is a matching name in a provider // namspace. @@ -550,6 +555,16 @@ func checkForUnknownFunctionDiags(diags hcl.Diagnostics) hcl.Diagnostics { continue } + // We might run into provider-defined functions during init. We can't filter out all + // places where they might be used, so we'll just filter out the diagnostics for the + // unknown function calls if we're configured to ignore them. This means that the user will + // just get an unknown value result for any provider function calls, which is fine because + // we won't have any provider functions available at this point anyway. + if ignoreUnknownProviderFunctions { + continue + } + filteredDiags = filteredDiags.Append(d) + // the diagnostic isn't really shared with anything, and copying would // still retain the internal pointers, so we're going to modify the // diagnostic in-place if we want to change the output. Log the original @@ -598,5 +613,5 @@ func checkForUnknownFunctionDiags(diags hcl.Diagnostics) hcl.Diagnostics { d.Detail = fmt.Sprintf(`There is no function named "%s%s". Ensure that provider name %q is declared in this module's required_providers block, and that this provider offers a function named %q.`, namespace, name, namespaceParts[1], name) } - return diags + return filteredDiags } diff --git a/internal/lang/scope.go b/internal/lang/scope.go index 4416e65f02..41334a51f3 100644 --- a/internal/lang/scope.go +++ b/internal/lang/scope.go @@ -84,6 +84,12 @@ type Scope struct { // marked as ephemeral. // FIXME: plan to officially deprecate this workaround. ForProvider bool + + // IgnoreUnknownProviderFunctions can be set to true to request that any unknown + // functions produce unknown value results rather than an error. This is + // important during a plan phase to avoid generating errors for functions that + // are only available in provider versions that have not yet been selected. + IgnoreUnknownProviderFunctions bool } // SetActiveExperiments allows a caller to declare that a set of experiments diff --git a/internal/terraform/context_init_test.go b/internal/terraform/context_init_test.go index eae41b8944..84484e0398 100644 --- a/internal/terraform/context_init_test.go +++ b/internal/terraform/context_init_test.go @@ -138,12 +138,12 @@ module "example" { // that this may be caused by a non-const variable used during init. return tfdiags.Diagnostics{}.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, - Summary: `Invalid module source`, - Detail: "The value of a reference in the module source is unknown." + constVariableDetail, + Summary: `Unknown module source`, + Detail: `Only literal values and const variables can be evaluated during init.`, Subject: &hcl.Range{ Filename: filepath.Join(m.SourceDir, "main.tf"), - Start: hcl.Pos{Line: 6, Column: 27, Byte: 82}, - End: hcl.Pos{Line: 6, Column: 35, Byte: 90}, + Start: hcl.Pos{Line: 6, Column: 14, Byte: 69}, + End: hcl.Pos{Line: 6, Column: 37, Byte: 92}, }, }) }, @@ -625,12 +625,12 @@ module "nested" { // that this may be caused by a non-const variable used during init. return tfdiags.Diagnostics{}.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, - Summary: `Invalid module source`, - Detail: "The value of a reference in the module source is unknown." + constVariableDetail, + Summary: `Unknown module source`, + Detail: `Only literal values and const variables can be evaluated during init.`, Subject: &hcl.Range{ Filename: filepath.Join(mc["./modules/example"].SourceDir, "main.tf"), - Start: hcl.Pos{Line: 7, Column: 27, Byte: 82}, - End: hcl.Pos{Line: 7, Column: 35, Byte: 90}, + Start: hcl.Pos{Line: 7, Column: 14, Byte: 69}, + End: hcl.Pos{Line: 7, Column: 37, Byte: 92}, }, }) }, diff --git a/internal/terraform/eval_variable.go b/internal/terraform/eval_variable.go index 814696392b..f300c2f044 100644 --- a/internal/terraform/eval_variable.go +++ b/internal/terraform/eval_variable.go @@ -16,6 +16,7 @@ import ( "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/checks" "github.com/hashicorp/terraform/internal/configs" + "github.com/hashicorp/terraform/internal/lang" "github.com/hashicorp/terraform/internal/lang/langrefs" "github.com/hashicorp/terraform/internal/lang/marks" "github.com/hashicorp/terraform/internal/tfdiags" @@ -228,7 +229,7 @@ func PrepareFinalInputVariableValue(addr addrs.AbsInputVariableInstance, raw *In // This must be used only after any side-effects that make the value of the // variable available for use in expression evaluation, such as // EvalModuleCallArgument for variables in descendant modules. -func evalVariableValidations(addr addrs.AbsInputVariableInstance, ctx EvalContext, rules []*configs.CheckRule, valueRng hcl.Range, validateWalk bool) (diags tfdiags.Diagnostics) { +func evalVariableValidations(addr addrs.AbsInputVariableInstance, ctx EvalContext, rules []*configs.CheckRule, valueRng hcl.Range, walkOp walkOperation) (diags tfdiags.Diagnostics) { if len(rules) == 0 { log.Printf("[TRACE] evalVariableValidations: no validation rules declared for %s, so skipping", addr) return nil @@ -283,7 +284,7 @@ func evalVariableValidations(addr addrs.AbsInputVariableInstance, ctx EvalContex // fake it here by overwriting the unknown value that scope.EvalContext // will have inserted during validate walks with a possibly-more-known value // using the same strategy our special code used to use. - if validateWalk { + if walkOp == walkValidate { ourVal := ctx.NamedValues().GetInputVariableValue(addr) if ourVal != cty.NilVal { // (it would be weird for ourVal to be nil here, but we'll tolerate it @@ -312,7 +313,7 @@ func evalVariableValidations(addr addrs.AbsInputVariableInstance, ctx EvalContex } for ix, validation := range rules { - result, ruleDiags := evalVariableValidation(validation, hclCtx, valueRng, addr, ix, validateWalk) + result, ruleDiags := evalVariableValidation(validation, hclCtx, valueRng, addr, ix, walkOp) diags = diags.Append(ruleDiags) log.Printf("[TRACE] evalVariableValidations: %s status is now %s", addr, result.Status) @@ -326,13 +327,13 @@ func evalVariableValidations(addr addrs.AbsInputVariableInstance, ctx EvalContex return diags } -func evalVariableValidation(validation *configs.CheckRule, hclCtx *hcl.EvalContext, valueRng hcl.Range, addr addrs.AbsInputVariableInstance, ix int, validateWalk bool) (checkResult, tfdiags.Diagnostics) { +func evalVariableValidation(validation *configs.CheckRule, hclCtx *hcl.EvalContext, valueRng hcl.Range, addr addrs.AbsInputVariableInstance, ix int, walkOp walkOperation) (checkResult, tfdiags.Diagnostics) { const errInvalidCondition = "Invalid variable validation result" const errInvalidValue = "Invalid value for variable" var diags tfdiags.Diagnostics result, moreDiags := validation.Condition.Value(hclCtx) - diags = diags.Append(moreDiags) + diags = diags.Append(lang.CheckForUnknownFunctionDiags(moreDiags, walkOp == walkInit)) errorValue, errorDiags := validation.ErrorMessage.Value(hclCtx) // The following error handling is a workaround to preserve backwards @@ -430,7 +431,7 @@ func evalVariableValidation(validation *configs.CheckRule, hclCtx *hcl.EvalConte } if !errorValue.IsKnown() { - if validateWalk { + if walkOp == walkValidate { log.Printf("[DEBUG] evalVariableValidations: %s rule %s error_message value is unknown, so skipping validation for now", addr, validation.DeclRange) return checkResult{Status: checks.StatusUnknown}, diags } diff --git a/internal/terraform/eval_variable_test.go b/internal/terraform/eval_variable_test.go index dda4cdcf36..854a6ac5f4 100644 --- a/internal/terraform/eval_variable_test.go +++ b/internal/terraform/eval_variable_test.go @@ -1188,7 +1188,7 @@ func TestEvalVariableValidations_jsonErrorMessageEdgeCase(t *testing.T) { ctx.ChecksState.ReportCheckableObjects(varAddr.ConfigCheckable(), addrs.MakeSet[addrs.Checkable](varAddr)) gotDiags := evalVariableValidations( - varAddr, ctx, varCfg.Validations, varCfg.DeclRange, false, + varAddr, ctx, varCfg.Validations, varCfg.DeclRange, walkPlan, ) if ctx.ChecksState.ObjectCheckStatus(varAddr) != test.status { @@ -1347,7 +1347,7 @@ variable "bar" { ctx.ChecksState.ReportCheckableObjects(varAddr.ConfigCheckable(), addrs.MakeSet[addrs.Checkable](varAddr)) gotDiags := evalVariableValidations( - varAddr, ctx, varCfg.Validations, varCfg.DeclRange, false, + varAddr, ctx, varCfg.Validations, varCfg.DeclRange, walkPlan, ) if ctx.ChecksState.ObjectCheckStatus(varAddr) != test.status { @@ -1390,7 +1390,7 @@ func TestEvalVariableValidation_unknownErrorMessage(t *testing.T) { } // this should not produce any error when validationWalk is true - result, diags := evalVariableValidation(rule, hclCtx, hcl.Range{}, varAddr, 0, true) + result, diags := evalVariableValidation(rule, hclCtx, hcl.Range{}, varAddr, 0, walkValidate) if got, want := result.Status, checks.StatusUnknown; got != want { t.Errorf("wrong result.Status\ngot: %s\nwant: %s", got, want) } @@ -1399,7 +1399,7 @@ func TestEvalVariableValidation_unknownErrorMessage(t *testing.T) { } // any other time this should result in an error - result, diags = evalVariableValidation(rule, hclCtx, hcl.Range{}, varAddr, 0, false) + result, diags = evalVariableValidation(rule, hclCtx, hcl.Range{}, varAddr, 0, walkPlan) if got, want := result.Status, checks.StatusError; got != want { t.Errorf("wrong result.Status\ngot: %s\nwant: %s", got, want) } diff --git a/internal/terraform/evaluate.go b/internal/terraform/evaluate.go index c69da96e32..52f5b45f35 100644 --- a/internal/terraform/evaluate.go +++ b/internal/terraform/evaluate.go @@ -93,15 +93,16 @@ type Evaluator struct { // address. func (e *Evaluator) Scope(data lang.Data, self addrs.Referenceable, source addrs.Referenceable, extFuncs lang.ExternalFuncs) *lang.Scope { return &lang.Scope{ - Data: data, - ParseRef: addrs.ParseRef, - SelfAddr: self, - SourceAddr: source, - PureOnly: e.Operation != walkApply && e.Operation != walkDestroy && e.Operation != walkEval, - BaseDir: ".", // Always current working directory for now. - PlanTimestamp: e.PlanTimestamp, - ExternalFuncs: extFuncs, - FunctionResults: e.FunctionResults, + Data: data, + ParseRef: addrs.ParseRef, + SelfAddr: self, + SourceAddr: source, + PureOnly: e.Operation != walkApply && e.Operation != walkDestroy && e.Operation != walkEval, + BaseDir: ".", // Always current working directory for now. + PlanTimestamp: e.PlanTimestamp, + ExternalFuncs: extFuncs, + FunctionResults: e.FunctionResults, + IgnoreUnknownProviderFunctions: e.Operation == walkInit, } } diff --git a/internal/terraform/graph_builder_apply.go b/internal/terraform/graph_builder_apply.go index fba4f8a16f..08828604f4 100644 --- a/internal/terraform/graph_builder_apply.go +++ b/internal/terraform/graph_builder_apply.go @@ -143,7 +143,9 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer { Config: b.Config, DestroyApply: b.Operation == walkDestroy, }, - &variableValidationTransformer{}, + &variableValidationTransformer{ + operation: b.Operation, + }, &LocalTransformer{Config: b.Config}, &OutputTransformer{ Config: b.Config, diff --git a/internal/terraform/graph_builder_eval.go b/internal/terraform/graph_builder_eval.go index 260e8a7c51..ee8f8bcd28 100644 --- a/internal/terraform/graph_builder_eval.go +++ b/internal/terraform/graph_builder_eval.go @@ -76,7 +76,9 @@ func (b *EvalGraphBuilder) Steps() []GraphTransformer { // Add dynamic values &RootVariableTransformer{Config: b.Config, RawValues: b.RootVariableValues, ValidateChecks: true}, &ModuleVariableTransformer{Config: b.Config, ValidateChecks: true}, - &variableValidationTransformer{}, + &variableValidationTransformer{ + operation: walkEval, + }, &LocalTransformer{Config: b.Config}, &OutputTransformer{ Config: b.Config, diff --git a/internal/terraform/graph_builder_init.go b/internal/terraform/graph_builder_init.go index bf22bec4c3..81c5389c70 100644 --- a/internal/terraform/graph_builder_init.go +++ b/internal/terraform/graph_builder_init.go @@ -76,7 +76,9 @@ func (b *InitGraphBuilder) Steps() []GraphTransformer { }, }, - &variableValidationTransformer{}, + &variableValidationTransformer{ + operation: walkInit, + }, &RootTransformer{}, diff --git a/internal/terraform/graph_builder_plan.go b/internal/terraform/graph_builder_plan.go index 19d2cc50eb..13d1c8e19e 100644 --- a/internal/terraform/graph_builder_plan.go +++ b/internal/terraform/graph_builder_plan.go @@ -205,7 +205,7 @@ func (b *PlanGraphBuilder) Steps() []GraphTransformer { DestroyApply: false, // always false for planning }, &variableValidationTransformer{ - validateWalk: b.Operation == walkValidate, + operation: b.Operation, }, &LocalTransformer{Config: b.Config}, &OutputTransformer{ diff --git a/internal/terraform/node_module_install.go b/internal/terraform/node_module_install.go index 102bd5e597..1797ac09c8 100644 --- a/internal/terraform/node_module_install.go +++ b/internal/terraform/node_module_install.go @@ -8,7 +8,6 @@ import ( "github.com/hashicorp/go-version" "github.com/hashicorp/hcl/v2" - "github.com/hashicorp/hcl/v2/hclsyntax" "github.com/zclconf/go-cty/cty" "github.com/hashicorp/terraform/internal/addrs" @@ -160,8 +159,6 @@ func (n *nodeInstallModule) DynamicExpand(ctx EvalContext) (*Graph, tfdiags.Diag return &g, nil } -const constVariableDetail = "\n\nOnly literal values and constant variables (with const = true) are allowed for this attribute, as well as values derived from these." - func evalSource(sourceExpr hcl.Expression, hasVersion bool, ctx EvalContext) (addrs.ModuleSource, string, tfdiags.Diagnostics) { var diags tfdiags.Diagnostics var addr addrs.ModuleSource @@ -195,46 +192,10 @@ func evalSource(sourceExpr hcl.Expression, hasVersion bool, ctx EvalContext) (ad } if !value.IsWhollyKnown() { - tExpr, ok := sourceExpr.(*hclsyntax.TemplateExpr) - if !ok { - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Invalid module source", - Detail: "The module source contains a reference that is unknown during init." + constVariableDetail, - Subject: sourceExpr.Range().Ptr(), - }) - return nil, "", diags - } - for _, part := range tExpr.Parts { - partVal, partDiags := ctx.EvaluateExpr(part, cty.DynamicPseudoType, nil) - diags = diags.Append(partDiags) - if diags.HasErrors() { - return nil, "", diags - } - - scope := ctx.EvaluationScope(nil, nil, EvalDataForNoInstanceKey) - hclCtx, evalDiags := scope.EvalContext(refs) - diags = diags.Append(evalDiags) - if diags.HasErrors() { - return nil, "", diags - } - if !partVal.IsKnown() { - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Invalid module source", - Detail: "The value of a reference in the module source is unknown." + constVariableDetail, - Subject: part.Range().Ptr(), - Expression: part, - EvalContext: hclCtx, - Extra: diagnosticCausedByUnknown(true), - }) - return nil, "", diags - } - } diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, - Summary: "Invalid module source", - Detail: "The module source contains a reference that is unknown." + constVariableDetail, + Summary: "Unknown module source", + Detail: `Only literal values and const variables can be evaluated during init.`, Subject: sourceExpr.Range().Ptr(), }) return nil, "", diags @@ -338,46 +299,10 @@ func evalVersionConstraint(versionExpr hcl.Expression, ctx EvalContext) (configs } if !value.IsWhollyKnown() { - tExpr, ok := versionExpr.(*hclsyntax.TemplateExpr) - if !ok { - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Invalid module version", - Detail: "The module version contains a reference that is unknown during init." + constVariableDetail, - Subject: versionExpr.Range().Ptr(), - }) - return ret, diags - } - for _, part := range tExpr.Parts { - partVal, partDiags := ctx.EvaluateExpr(part, cty.DynamicPseudoType, nil) - diags = diags.Append(partDiags) - if diags.HasErrors() { - return ret, diags - } - - scope := ctx.EvaluationScope(nil, nil, EvalDataForNoInstanceKey) - hclCtx, evalDiags := scope.EvalContext(refs) - diags = diags.Append(evalDiags) - if diags.HasErrors() { - return ret, diags - } - if !partVal.IsKnown() { - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Invalid module version", - Detail: "The value of a reference in the module version is unknown." + constVariableDetail, - Subject: part.Range().Ptr(), - Expression: part, - EvalContext: hclCtx, - Extra: diagnosticCausedByUnknown(true), - }) - return ret, diags - } - } diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, - Summary: "Invalid module version", - Detail: "The module version contains a reference that is unknown." + constVariableDetail, + Summary: "Unknown module version", + Detail: `Only literal values and const variables can be evaluated during init.`, Subject: versionExpr.Range().Ptr(), }) return ret, diags diff --git a/internal/terraform/node_module_variable.go b/internal/terraform/node_module_variable.go index 3aadece928..97a7dc258e 100644 --- a/internal/terraform/node_module_variable.go +++ b/internal/terraform/node_module_variable.go @@ -162,6 +162,10 @@ func (n *nodeExpandModuleVariable) variableValidationRules() (addrs.ConfigInputV return n.Addr.InModule(n.Module), rules, defnRange } +func (n *nodeExpandModuleVariable) isConst() bool { + return n.Config != nil && n.Config.Const +} + // nodeModuleVariable represents a module variable input during // the apply step. type nodeModuleVariable struct { diff --git a/internal/terraform/node_root_variable.go b/internal/terraform/node_root_variable.go index f1b390475f..ae1baf68b1 100644 --- a/internal/terraform/node_root_variable.go +++ b/internal/terraform/node_root_variable.go @@ -190,3 +190,7 @@ func (n *NodeRootVariable) variableValidationRules() (addrs.ConfigInputVariable, } return n.Addr.InModule(addrs.RootModule), rules, defnRange } + +func (n *NodeRootVariable) isConst() bool { + return n.Config != nil && n.Config.Const +} diff --git a/internal/terraform/node_variable_validation.go b/internal/terraform/node_variable_validation.go index e30eb7db90..edf7455378 100644 --- a/internal/terraform/node_variable_validation.go +++ b/internal/terraform/node_variable_validation.go @@ -128,7 +128,7 @@ func (n *nodeVariableValidation) Execute(globalCtx EvalContext, op walkOperation moduleCtx, n.rules, n.defnRange, - n.validateWalk, + op, )) } diff --git a/internal/terraform/transform_variable_validation.go b/internal/terraform/transform_variable_validation.go index 8ae8bc8aaa..00edec85e3 100644 --- a/internal/terraform/transform_variable_validation.go +++ b/internal/terraform/transform_variable_validation.go @@ -33,6 +33,9 @@ type graphNodeValidatableVariable interface { // for example, if the value came from an environment variable -- then // the location of the variable declaration is a plausible substitute. variableValidationRules() (configAddr addrs.ConfigInputVariable, rules []*configs.CheckRule, defnRange hcl.Range) + + // isConst returns the variable's const value. + isConst() bool } // Correct behavior requires both of the input variable node types to @@ -53,7 +56,7 @@ var _ graphNodeValidatableVariable = (*nodeExpandModuleVariable)(nil) // with the new [nodeVariableValidation] nodes to prevent downstream nodes // from relying on unvalidated values. type variableValidationTransformer struct { - validateWalk bool + operation walkOperation } var _ GraphTransformer = (*variableValidationTransformer)(nil) @@ -66,12 +69,18 @@ func (t *variableValidationTransformer) Transform(g *Graph) error { continue // irrelevant node } + // Variable validation nodes don't need to be added to the init graph for non-constant variables since they will always be unknown + if !v.isConst() && t.operation == walkInit { + continue + } + configAddr, rules, defnRange := v.variableValidationRules() + newV := &nodeVariableValidation{ configAddr: configAddr, rules: rules, defnRange: defnRange, - validateWalk: t.validateWalk, + validateWalk: t.operation == walkValidate, } if len(rules) != 0 { diff --git a/internal/terraform/transform_variable_validation_test.go b/internal/terraform/transform_variable_validation_test.go index c70f23297a..90c3ae0d27 100644 --- a/internal/terraform/transform_variable_validation_test.go +++ b/internal/terraform/transform_variable_validation_test.go @@ -71,7 +71,141 @@ func TestVariableValidationTransformer(t *testing.T) { g.Add(barNode) g.Add(bazNode) - transformer := &variableValidationTransformer{} + transformer := &variableValidationTransformer{ + operation: walkValidate, + } + err := transformer.Transform(g) + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + + gotStr := strings.TrimSpace(g.String()) + wantStr := strings.TrimSpace(` +var.bar (test fake) +var.bar (validation) + var.bar (test fake) +var.baz (test fake) +var.baz (validation) + var.baz (test fake) +var.foo (test fake) +var.foo (validation) + var.foo (test fake) +`) + if diff := cmp.Diff(wantStr, gotStr); diff != "" { + t.Errorf("wrong graph after transform\n%s", diff) + } + + // This transformer is not responsible for wiring up dependencies based + // on references -- that's ReferenceTransformer's job -- but we'll + // verify that the nodes that were added by this transformer do at least + // report the references we expect them to report, in the way that + // ReferenceTransformer would expect. + gotRefs := map[string]map[string]struct{}{} + for _, v := range g.Vertices() { + v, ok := v.(*nodeVariableValidation) // the type of all nodes that this transformer adds + if !ok { + continue + } + var _ GraphNodeReferencer = v // static assertion just to make sure we'll fail to compile if GraphNodeReferencer changes later + + refs := v.References() + gotRefs[v.Name()] = map[string]struct{}{} + for _, ref := range refs { + gotRefs[v.Name()][ref.Subject.String()] = struct{}{} + } + } + wantRefs := map[string]map[string]struct{}{ + "var.bar (validation)": { + "var.foo": struct{}{}, + }, + "var.baz (validation)": { + "var.foo": struct{}{}, + }, + "var.foo (validation)": {}, + } + if diff := cmp.Diff(wantRefs, gotRefs); diff != "" { + t.Errorf("wrong references for the added nodes\n%s", diff) + } +} + +func TestVariableValidationTransformer_init(t *testing.T) { + g := &Graph{} + fooNode := &nodeTestOnlyInputVariable{ + configAddr: addrs.InputVariable{Name: "foo"}.InModule(addrs.RootModule), + rules: []*configs.CheckRule{ + { + // The condition contains a self-reference, which is required + // for a realistic input variable validation because otherwise + // it wouldn't actually be checking the variable it's + // supposed to be validating. (This transformer is not the + // one responsible for validating that though, so it's + // okay for the examples below to not meet that requirement.) + Condition: hcltest.MockExprTraversalSrc("var.foo"), + ErrorMessage: hcltest.MockExprLiteral(cty.StringVal("wrong")), + }, + }, + config: &configs.Variable{ + Name: "foo", + Type: cty.String, + Const: true, + }, + } + barNode := &nodeTestOnlyInputVariable{ + configAddr: addrs.InputVariable{Name: "bar"}.InModule(addrs.RootModule), + rules: []*configs.CheckRule{ + { + // The condition of this one refers to var.foo + Condition: hcltest.MockExprTraversalSrc("var.foo"), + ErrorMessage: hcltest.MockExprLiteral(cty.StringVal("wrong")), + }, + }, + config: &configs.Variable{ + Name: "bar", + Type: cty.String, + Const: true, + }, + } + bazNode := &nodeTestOnlyInputVariable{ + configAddr: addrs.InputVariable{Name: "baz"}.InModule(addrs.RootModule), + rules: []*configs.CheckRule{ + { + // The error message of this one refers to var.foo + Condition: hcltest.MockExprLiteral(cty.False), + ErrorMessage: hcltest.MockExprTraversalSrc("var.foo"), + }, + }, + config: &configs.Variable{ + Name: "baz", + Type: cty.String, + Const: true, + }, + } + // This variable validation node will be skipped as: + // 1. It's not a constant variable + // 2. The graph walk for this test is init (which only needs constant variables) + quxNode := &nodeTestOnlyInputVariable{ + configAddr: addrs.InputVariable{Name: "qux"}.InModule(addrs.RootModule), + rules: []*configs.CheckRule{ + { + // The error message of this one refers to var.foo + Condition: hcltest.MockExprLiteral(cty.False), + ErrorMessage: hcltest.MockExprTraversalSrc("var.foo"), + }, + }, + config: &configs.Variable{ + Name: "qux", + Type: cty.String, + Const: false, + }, + } + g.Add(fooNode) + g.Add(barNode) + g.Add(bazNode) + g.Add(quxNode) + + transformer := &variableValidationTransformer{ + operation: walkInit, + } err := transformer.Transform(g) if err != nil { t.Fatalf("unexpected error: %s", err) @@ -88,6 +222,7 @@ var.baz (validation) var.foo (test fake) var.foo (validation) var.foo (test fake) +var.qux (test fake) `) if diff := cmp.Diff(wantStr, gotStr); diff != "" { t.Errorf("wrong graph after transform\n%s", diff) @@ -129,6 +264,7 @@ var.foo (validation) type nodeTestOnlyInputVariable struct { configAddr addrs.ConfigInputVariable rules []*configs.CheckRule + config *configs.Variable } var _ graphNodeValidatableVariable = (*nodeTestOnlyInputVariable)(nil) @@ -145,3 +281,7 @@ func (n *nodeTestOnlyInputVariable) variableValidationRules() (addrs.ConfigInput End: hcl.InitialPos, } } + +func (n *nodeTestOnlyInputVariable) isConst() bool { + return n.config != nil && n.config.Const +}