diff --git a/internal/configs/experiments.go b/internal/configs/experiments.go index 8f0433a083b8..163e75e6d154 100644 --- a/internal/configs/experiments.go +++ b/internal/configs/experiments.go @@ -208,19 +208,5 @@ func checkModuleExperiments(m *Module) hcl.Diagnostics { } */ - if !m.ActiveExperiments.Has(experiments.VariableValidationCrossRef) { - // Without this experiment, validation rules are subject to the old - // rule that they can only refer to the variable whose value they - // are checking. This experiment removes that constraint, and makes - // the modules runtime responsible for validating and evaluating - // the conditions and error messages, just as we'd do for any other - // dynamic expression. - for varName, vc := range m.Variables { - for _, vv := range vc.Validations { - diags = append(diags, checkVariableValidationBlock(varName, vv)...) - } - } - } - return diags } diff --git a/internal/configs/named_values.go b/internal/configs/named_values.go index 2c733d959385..286c507a1187 100644 --- a/internal/configs/named_values.go +++ b/internal/configs/named_values.go @@ -178,7 +178,7 @@ func decodeVariableBlock(block *hcl.Block, override bool) (*Variable, hcl.Diagno switch block.Type { case "validation": - vv, moreDiags := decodeVariableValidationBlock(block, override) + vv, moreDiags := decodeCheckRuleBlock(block, override) diags = append(diags, moreDiags...) v.Validations = append(v.Validations, vv) @@ -324,77 +324,6 @@ func (m VariableParsingMode) Parse(name, value string) (cty.Value, hcl.Diagnosti } } -// decodeVariableValidationBlock is a wrapper around decodeCheckRuleBlock -// that imposes the additional rule that the condition expression can refer -// only to an input variable of the given name. -func decodeVariableValidationBlock(block *hcl.Block, override bool) (*CheckRule, hcl.Diagnostics) { - return decodeCheckRuleBlock(block, override) -} - -func checkVariableValidationBlock(varName string, vv *CheckRule) hcl.Diagnostics { - var diags hcl.Diagnostics - - if vv.Condition != nil { - // The validation condition can only refer to the variable itself, - // to ensure that the variable declaration can't create additional - // edges in the dependency graph. - goodRefs := 0 - for _, traversal := range vv.Condition.Variables() { - ref, moreDiags := addrs.ParseRef(traversal) - if !moreDiags.HasErrors() { - if addr, ok := ref.Subject.(addrs.InputVariable); ok { - if addr.Name == varName { - goodRefs++ - continue // Reference is valid - } - } - } - // If we fall out here then the reference is invalid. - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Invalid reference in variable validation", - Detail: fmt.Sprintf("The condition for variable %q can only refer to the variable itself, using var.%s.", varName, varName), - Subject: traversal.SourceRange().Ptr(), - }) - } - if goodRefs < 1 { - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Invalid variable validation condition", - Detail: fmt.Sprintf("The condition for variable %q must refer to var.%s in order to test incoming values.", varName, varName), - Subject: vv.Condition.Range().Ptr(), - }) - } - } - - if vv.ErrorMessage != nil { - // The same applies to the validation error message, except that - // references are not required. A string literal is a valid error - // message. - goodRefs := 0 - for _, traversal := range vv.ErrorMessage.Variables() { - ref, moreDiags := addrs.ParseRef(traversal) - if !moreDiags.HasErrors() { - if addr, ok := ref.Subject.(addrs.InputVariable); ok { - if addr.Name == varName { - goodRefs++ - continue // Reference is valid - } - } - } - // If we fall out here then the reference is invalid. - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Invalid reference in variable validation", - Detail: fmt.Sprintf("The error message for variable %q can only refer to the variable itself, using var.%s.", varName, varName), - Subject: traversal.SourceRange().Ptr(), - }) - } - } - - return diags -} - // Output represents an "output" block in a module or file. type Output struct { Name string diff --git a/internal/configs/testdata/invalid-modules/variable-validation-condition-badref/variable-validation-condition-badref.tf b/internal/configs/testdata/valid-files/variable-validation-condition-crossref.tf similarity index 53% rename from internal/configs/testdata/invalid-modules/variable-validation-condition-badref/variable-validation-condition-badref.tf rename to internal/configs/testdata/valid-files/variable-validation-condition-crossref.tf index 9b9e935767af..adfdd7c31efe 100644 --- a/internal/configs/testdata/invalid-modules/variable-validation-condition-badref/variable-validation-condition-badref.tf +++ b/internal/configs/testdata/valid-files/variable-validation-condition-crossref.tf @@ -5,7 +5,7 @@ locals { variable "validation" { validation { - condition = local.foo == var.validation # ERROR: Invalid reference in variable validation + condition = local.foo == var.validation error_message = "Must be five." } } @@ -13,6 +13,6 @@ variable "validation" { variable "validation_error_expression" { validation { condition = var.validation_error_expression != 1 - error_message = "Cannot equal ${local.foo}." # ERROR: Invalid reference in variable validation + error_message = "Cannot equal ${local.foo}." } } diff --git a/internal/experiments/experiment.go b/internal/experiments/experiment.go index 7fa4b902f0e6..8a7b770962ac 100644 --- a/internal/experiments/experiment.go +++ b/internal/experiments/experiment.go @@ -31,7 +31,7 @@ func init() { // a current or a concluded experiment. registerConcludedExperiment(UnknownInstances, "Unknown instances are being rolled into a larger feature for deferring unready resources and modules.") registerConcludedExperiment(VariableValidation, "Custom variable validation can now be used by default, without enabling an experiment.") - registerCurrentExperiment(VariableValidationCrossRef) + registerConcludedExperiment(VariableValidationCrossRef, "Input variable validation rules may now refer to other objects in the same module without enabling any experiment.") registerConcludedExperiment(SuppressProviderSensitiveAttrs, "Provider-defined sensitive attributes are now redacted by default, without enabling an experiment.") registerCurrentExperiment(TemplateStringFunc) registerConcludedExperiment(ConfigDrivenMove, "Declarations of moved resource instances using \"moved\" blocks can now be used by default, without enabling an experiment.") diff --git a/internal/terraform/context_plan_test.go b/internal/terraform/context_plan_test.go index 8c4ff6509923..d0b8220d0c68 100644 --- a/internal/terraform/context_plan_test.go +++ b/internal/terraform/context_plan_test.go @@ -6921,8 +6921,6 @@ resource "test_resource" "foo" { } func TestContext2Plan_variableCustomValidationsSimple(t *testing.T) { - // This test is dealing with validation rules that refer to other objects - // in the same module. m := testModuleInline(t, map[string]string{ "main.tf": ` variable "a" { @@ -6978,11 +6976,6 @@ func TestContext2Plan_variableCustomValidationsCrossRef(t *testing.T) { // in the same module. m := testModuleInline(t, map[string]string{ "main.tf": ` - # Validation cross-references are currently experimental - terraform { - experiments = [variable_validation_crossref] - } - variable "a" { type = string } diff --git a/internal/terraform/eval_variable.go b/internal/terraform/eval_variable.go index 8c65256aca7e..fc34598a92a9 100644 --- a/internal/terraform/eval_variable.go +++ b/internal/terraform/eval_variable.go @@ -215,67 +215,6 @@ func evalVariableValidations(addr addrs.AbsInputVariableInstance, ctx EvalContex return diags } - // Validation expressions are statically validated (during configuration - // loading) to refer only to the variable being validated, so we can - // bypass our usual evaluation machinery here and just produce a minimal - // evaluation context containing just the required value. - val := ctx.NamedValues().GetInputVariableValue(addr) - if val == cty.NilVal { - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "No final value for variable", - Detail: fmt.Sprintf("Terraform doesn't have a final value for %s during validation. This is a bug in Terraform; please report it!", addr), - }) - return diags - } - hclCtx := &hcl.EvalContext{ - Variables: map[string]cty.Value{ - "var": cty.ObjectVal(map[string]cty.Value{ - addr.Variable.Name: val, - }), - }, - Functions: ctx.EvaluationScope(nil, nil, EvalDataForNoInstanceKey).Functions(), - } - - for ix, validation := range rules { - result, ruleDiags := evalVariableValidation(validation, hclCtx, valueRng, addr, ix) - diags = diags.Append(ruleDiags) - - log.Printf("[TRACE] evalVariableValidations: %s status is now %s", addr, result.Status) - if result.Status == checks.StatusFail { - checkState.ReportCheckFailure(addr, addrs.InputValidation, ix, result.FailureMessage) - } else { - checkState.ReportCheckResult(addr, addrs.InputValidation, ix, result.Status) - } - } - - return diags -} - -// evalVariableValidationsCrossRef is an experimental variant of -// [evalVariableValidations] that allows arbitrary references to any object -// declared in the same module as the variable. -// -// If the experiment is successful, this function should replace -// [evalVariableValidations], but it's currently written separately to minimize -// the risk of the experiment impacting non-opted modules. -func evalVariableValidationsCrossRef(addr addrs.AbsInputVariableInstance, ctx EvalContext, rules []*configs.CheckRule, valueRng hcl.Range) (diags tfdiags.Diagnostics) { - if len(rules) == 0 { - log.Printf("[TRACE] evalVariableValidations: no validation rules declared for %s, so skipping", addr) - return nil - } - log.Printf("[TRACE] evalVariableValidations: validating %s", addr) - - checkState := ctx.Checks() - if !checkState.ConfigHasChecks(addr.ConfigCheckable()) { - // We have nothing to do if this object doesn't have any checks, - // but the "rules" slice should agree that we don't. - if ct := len(rules); ct != 0 { - panic(fmt.Sprintf("check state says that %s should have no rules, but it has %d", addr, ct)) - } - return diags - } - // We'll build just one evaluation context covering the data needed by // all of the rules together, since that'll minimize lock contention // on the state, plan, etc. @@ -300,6 +239,46 @@ func evalVariableValidationsCrossRef(addr addrs.AbsInputVariableInstance, ctx Ev return diags } + // HACK: Historically we manually built a very constrained hcl.EvalContext + // here, which only included the value of the one specific input variable + // we're validating, since we didn't yet support referring to anything + // else. That accidentally bypassed our rule that input variables are + // always unknown during the validate walk, and thus accidentally created + // a useful behavior of actually checking constant-only values against + // their validation rules just during "terraform validate", rather than + // having to run "terraform plan". + // + // Although that behavior was accidental, it makes simple validation rules + // more useful and is protected by compatibility promises, and so we'll + // fake it here by overwriting the unknown value that scope.EvalContext + // will have inserted with a possibly-more-known value using the same + // strategy our special code used to use. + ourVal := ctx.NamedValues().GetInputVariableValue(addr) + if ourVal != cty.NilVal { + // (it would be weird for ourVal to be nil here, but we'll tolerate it + // because it was scope.EvalContext's responsibility to check for the + // absent final value, and even if it didn't we'll just get an + // evaluation error when evaluating the expressions below anyway.) + + // Our goal here is to make sure that a reference to the variable + // we're checking will evaluate to ourVal, regardless of what else + // scope.EvalContext might have put in the variables table. + if hclCtx.Variables == nil { + hclCtx.Variables = make(map[string]cty.Value) + } + if varsVal, ok := hclCtx.Variables["var"]; ok { + // Unfortunately we need to unpack and repack the object here, + // because cty values are immutable. + attrs := varsVal.AsValueMap() + attrs[addr.Variable.Name] = ourVal + hclCtx.Variables["var"] = cty.ObjectVal(attrs) + } else { + hclCtx.Variables["var"] = cty.ObjectVal(map[string]cty.Value{ + addr.Variable.Name: ourVal, + }) + } + } + for ix, validation := range rules { result, ruleDiags := evalVariableValidation(validation, hclCtx, valueRng, addr, ix) diags = diags.Append(ruleDiags) diff --git a/internal/terraform/eval_variable_test.go b/internal/terraform/eval_variable_test.go index 3a6738e626ea..7781b353817c 100644 --- a/internal/terraform/eval_variable_test.go +++ b/internal/terraform/eval_variable_test.go @@ -1173,7 +1173,13 @@ func TestEvalVariableValidations_jsonErrorMessageEdgeCase(t *testing.T) { // We need a minimal scope to allow basic functions to be passed to // the HCL scope - ctx.EvaluationScopeScope = &lang.Scope{} + ctx.EvaluationScopeScope = &lang.Scope{ + Data: &fakeEvaluationData{ + inputVariables: map[addrs.InputVariable]cty.Value{ + varAddr.Variable: test.given, + }, + }, + } ctx.NamedValuesState = namedvals.NewState() ctx.NamedValuesState.SetInputVariableValue(varAddr, test.given) ctx.ChecksState = checks.NewState(cfg) @@ -1322,13 +1328,19 @@ variable "bar" { // We need a minimal scope to allow basic functions to be passed to // the HCL scope - ctx.EvaluationScopeScope = &lang.Scope{} - ctx.NamedValuesState = namedvals.NewState() + varVal := test.given if varCfg.Sensitive { - ctx.NamedValuesState.SetInputVariableValue(varAddr, test.given.Mark(marks.Sensitive)) - } else { - ctx.NamedValuesState.SetInputVariableValue(varAddr, test.given) + varVal = varVal.Mark(marks.Sensitive) } + ctx.EvaluationScopeScope = &lang.Scope{ + Data: &fakeEvaluationData{ + inputVariables: map[addrs.InputVariable]cty.Value{ + varAddr.Variable: varVal, + }, + }, + } + ctx.NamedValuesState = namedvals.NewState() + ctx.NamedValuesState.SetInputVariableValue(varAddr, varVal) ctx.ChecksState = checks.NewState(cfg) ctx.ChecksState.ReportCheckableObjects(varAddr.ConfigCheckable(), addrs.MakeSet[addrs.Checkable](varAddr)) diff --git a/internal/terraform/evaluate_test.go b/internal/terraform/evaluate_test.go index 7dbabb467207..1a497db007d8 100644 --- a/internal/terraform/evaluate_test.go +++ b/internal/terraform/evaluate_test.go @@ -4,6 +4,7 @@ package terraform import ( + "fmt" "testing" "github.com/davecgh/go-spew/spew" @@ -617,3 +618,102 @@ func evaluatorForModule(stateSync *states.SyncState, changesSync *plans.ChangesS NamedValues: namedvals.NewState(), } } + +// fakeEvaluationData is an implementation of [lang.Data] that answers most +// questions just by returning data directly from the maps stored inside it. +type fakeEvaluationData struct { + checkBlocks map[addrs.Check]cty.Value + countAttrs map[addrs.CountAttr]cty.Value + forEachAttrs map[addrs.ForEachAttr]cty.Value + inputVariables map[addrs.InputVariable]cty.Value + localValues map[addrs.LocalValue]cty.Value + modules map[addrs.ModuleCall]cty.Value + outputValues map[addrs.OutputValue]cty.Value + pathAttrs map[addrs.PathAttr]cty.Value + resources map[addrs.Resource]cty.Value + runBlocks map[addrs.Run]cty.Value + terraformAttrs map[addrs.TerraformAttr]cty.Value + + // staticValidateRefs optionally implements [lang.Data.StaticValidateReferences], + // but can be left as nil to just skip static validation altogether. + staticValidateRefs func(refs []*addrs.Reference, self addrs.Referenceable, source addrs.Referenceable) tfdiags.Diagnostics +} + +var _ lang.Data = (*fakeEvaluationData)(nil) + +func fakeEvaluationDataLookup[Addr interface { + comparable + addrs.Referenceable +}](addr Addr, _ tfdiags.SourceRange, table map[Addr]cty.Value) (cty.Value, tfdiags.Diagnostics) { + ret, ok := table[addr] + if !ok { + var diags tfdiags.Diagnostics + diags = diags.Append(fmt.Errorf("fakeEvaluationData does not know about %s", addr)) + return cty.DynamicVal, diags + } + return ret, nil +} + +// GetCheckBlock implements lang.Data. +func (d *fakeEvaluationData) GetCheckBlock(addr addrs.Check, rng tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) { + return fakeEvaluationDataLookup(addr, rng, d.checkBlocks) +} + +// GetCountAttr implements lang.Data. +func (d *fakeEvaluationData) GetCountAttr(addr addrs.CountAttr, rng tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) { + return fakeEvaluationDataLookup(addr, rng, d.countAttrs) +} + +// GetForEachAttr implements lang.Data. +func (d *fakeEvaluationData) GetForEachAttr(addr addrs.ForEachAttr, rng tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) { + return fakeEvaluationDataLookup(addr, rng, d.forEachAttrs) +} + +// GetInputVariable implements lang.Data. +func (d *fakeEvaluationData) GetInputVariable(addr addrs.InputVariable, rng tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) { + return fakeEvaluationDataLookup(addr, rng, d.inputVariables) +} + +// GetLocalValue implements lang.Data. +func (d *fakeEvaluationData) GetLocalValue(addr addrs.LocalValue, rng tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) { + return fakeEvaluationDataLookup(addr, rng, d.localValues) +} + +// GetModule implements lang.Data. +func (d *fakeEvaluationData) GetModule(addr addrs.ModuleCall, rng tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) { + return fakeEvaluationDataLookup(addr, rng, d.modules) +} + +// GetOutput implements lang.Data. +func (d *fakeEvaluationData) GetOutput(addr addrs.OutputValue, rng tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) { + return fakeEvaluationDataLookup(addr, rng, d.outputValues) +} + +// GetPathAttr implements lang.Data. +func (d *fakeEvaluationData) GetPathAttr(addr addrs.PathAttr, rng tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) { + return fakeEvaluationDataLookup(addr, rng, d.pathAttrs) +} + +// GetResource implements lang.Data. +func (d *fakeEvaluationData) GetResource(addr addrs.Resource, rng tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) { + return fakeEvaluationDataLookup(addr, rng, d.resources) +} + +// GetRunBlock implements lang.Data. +func (d *fakeEvaluationData) GetRunBlock(addr addrs.Run, rng tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) { + return fakeEvaluationDataLookup(addr, rng, d.runBlocks) +} + +// GetTerraformAttr implements lang.Data. +func (d *fakeEvaluationData) GetTerraformAttr(addr addrs.TerraformAttr, rng tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) { + return fakeEvaluationDataLookup(addr, rng, d.terraformAttrs) +} + +// StaticValidateReferences implements lang.Data. +func (d *fakeEvaluationData) StaticValidateReferences(refs []*addrs.Reference, self addrs.Referenceable, source addrs.Referenceable) tfdiags.Diagnostics { + if d.staticValidateRefs == nil { + // By default we just skip static validation + return nil + } + return d.staticValidateRefs(refs, self, source) +} diff --git a/internal/terraform/graph_builder_apply.go b/internal/terraform/graph_builder_apply.go index fa337a229000..3289d0dc7c6b 100644 --- a/internal/terraform/graph_builder_apply.go +++ b/internal/terraform/graph_builder_apply.go @@ -125,9 +125,7 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer { Config: b.Config, DestroyApply: b.Operation == walkDestroy, }, - &variableValidationTransformer{ - config: b.Config, - }, + &variableValidationTransformer{}, &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 1a35ded13f56..697b66fee03c 100644 --- a/internal/terraform/graph_builder_eval.go +++ b/internal/terraform/graph_builder_eval.go @@ -76,9 +76,7 @@ func (b *EvalGraphBuilder) Steps() []GraphTransformer { // Add dynamic values &RootVariableTransformer{Config: b.Config, RawValues: b.RootVariableValues, Planning: true}, &ModuleVariableTransformer{Config: b.Config, Planning: true}, - &variableValidationTransformer{ - config: b.Config, - }, + &variableValidationTransformer{}, &LocalTransformer{Config: b.Config}, &OutputTransformer{ Config: b.Config, diff --git a/internal/terraform/graph_builder_plan.go b/internal/terraform/graph_builder_plan.go index 23455b5f15c3..ba7bc7507cd6 100644 --- a/internal/terraform/graph_builder_plan.go +++ b/internal/terraform/graph_builder_plan.go @@ -159,9 +159,7 @@ func (b *PlanGraphBuilder) Steps() []GraphTransformer { Planning: true, DestroyApply: false, // always false for planning }, - &variableValidationTransformer{ - config: b.Config, - }, + &variableValidationTransformer{}, &LocalTransformer{Config: b.Config}, &OutputTransformer{ Config: b.Config, diff --git a/internal/terraform/node_root_variable_test.go b/internal/terraform/node_root_variable_test.go index e894eae32079..d22d15cc8ddd 100644 --- a/internal/terraform/node_root_variable_test.go +++ b/internal/terraform/node_root_variable_test.go @@ -62,39 +62,56 @@ func TestNodeRootVariableExecute(t *testing.T) { ctx.NamedValuesState = namedvals.NewState() - // The variable validation function gets called with Terraform's - // built-in functions available, so we need a minimal scope just for - // it to get the functions from. - ctx.EvaluationScopeScope = &lang.Scope{} + // We need a minimal scope that knows just enough to complete evaluation + // of this input variable. + varAddr := addrs.InputVariable{Name: "foo"} + varValue := cty.StringVal("5") + ctx.EvaluationScopeScope = &lang.Scope{ + Data: &fakeEvaluationData{ + inputVariables: map[addrs.InputVariable]cty.Value{ + // Nothing here to start, since in realistic use it + // would be NodeRootVariable that decides the final + // value to populate in here. + }, + }, + } n := &NodeRootVariable{ - Addr: addrs.InputVariable{Name: "foo"}, + Addr: varAddr, Config: &configs.Variable{ - Name: "foo", + Name: varAddr.Name, Type: cty.Number, ConstraintType: cty.Number, Validations: []*configs.CheckRule{ { - Condition: fakeHCLExpressionFunc(func(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnostics) { - // This returns true only if the given variable value - // is exactly cty.Number, which allows us to verify - // that we were given the value _after_ type - // conversion. - // This had previously not been handled correctly, - // as reported in: - // https://github.com/hashicorp/terraform/issues/29899 - vars := ctx.Variables["var"] - if vars == cty.NilVal || !vars.Type().IsObjectType() || !vars.Type().HasAttribute("foo") { - t.Logf("var.foo isn't available") - return cty.False, nil - } - val := vars.GetAttr("foo") - if val == cty.NilVal || val.Type() != cty.Number { - t.Logf("var.foo is %#v; want a number", val) - return cty.False, nil - } - return cty.True, nil - }), + Condition: fakeHCLExpression( + []hcl.Traversal{ + { + hcl.TraverseRoot{Name: "var"}, + hcl.TraverseAttr{Name: varAddr.Name}, + }, + }, + func(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnostics) { + // This returns true only if the given variable value + // is exactly cty.Number, which allows us to verify + // that we were given the value _after_ type + // conversion. + // This had previously not been handled correctly, + // as reported in: + // https://github.com/hashicorp/terraform/issues/29899 + vars := ctx.Variables["var"] + if vars == cty.NilVal || !vars.Type().IsObjectType() || !vars.Type().HasAttribute(varAddr.Name) { + t.Logf("%s isn't available", varAddr) + return cty.False, nil + } + val := vars.GetAttr(varAddr.Name) + if val == cty.NilVal || val.Type() != cty.Number { + t.Logf("%s is %#v; want a number", varAddr, val) + return cty.False, nil + } + return cty.True, nil + }, + ), ErrorMessage: hcltest.MockExprLiteral(cty.StringVal("Must be a number.")), }, }, @@ -102,7 +119,7 @@ func TestNodeRootVariableExecute(t *testing.T) { RawValue: &InputValue{ // Note: This is a string, but the variable's type constraint // is number so it should be converted before use. - Value: cty.StringVal("5"), + Value: varValue, SourceType: ValueFromUnknown, }, Planning: true, @@ -117,7 +134,7 @@ func TestNodeRootVariableExecute(t *testing.T) { ctx.ChecksState = checks.NewState(&configs.Config{ Module: &configs.Module{ Variables: map[string]*configs.Variable{ - "foo": n.Config, + varAddr.Name: n.Config, }, }, }) @@ -126,12 +143,9 @@ func TestNodeRootVariableExecute(t *testing.T) { if diags.HasErrors() { t.Fatalf("unexpected error from NodeRootVariable: %s", diags.Err()) } - diags = validateN.Execute(ctx, walkApply) - if diags.HasErrors() { - t.Fatalf("unexpected error from nodeVariableValidation: %s", diags.Err()) - } - absAddr := addrs.RootModuleInstance.InputVariable(n.Addr.Name) + // We should now have a final value for the variable, pending validation. + absAddr := varAddr.Absolute(addrs.RootModuleInstance) if !ctx.NamedValues().HasInputVariableValue(absAddr) { t.Fatalf("no result value for input variable") } @@ -139,8 +153,23 @@ func TestNodeRootVariableExecute(t *testing.T) { // NOTE: The given value was cty.Bool but the type constraint was // cty.String, so it was NodeRootVariable's responsibility to convert // as part of preparing the "final value". - t.Errorf("wrong value for ctx.SetRootModuleArgument\ngot: %#v\nwant: %#v", got, want) + t.Fatalf("wrong value for ctx.SetRootModuleArgument\ngot: %#v\nwant: %#v", got, want) + } else { + // Our evaluation scope would now, if using the _real_ + // evaluationStateData implementation, include that final value. + // + // There are also integration tests covering the fully-integrated + // form of this test, using the real evaluation data implementation: + // TestContext2Plan_variableCustomValidationsSimple + // TestContext2Plan_variableCustomValidationsCrossRef + ctx.EvaluationScopeScope.Data.(*fakeEvaluationData).inputVariables[varAddr] = got } + + diags = validateN.Execute(ctx, walkApply) + if diags.HasErrors() { + t.Fatalf("unexpected error from nodeVariableValidation: %s", diags.Err()) + } + if status := ctx.Checks().ObjectCheckStatus(n.Addr.Absolute(addrs.RootModuleInstance)); status != checks.StatusPass { t.Errorf("expected checks to pass but go %s instead", status) } @@ -178,3 +207,32 @@ func (f fakeHCLExpressionFunc) Range() hcl.Range { func (f fakeHCLExpressionFunc) StartRange() hcl.Range { return f.Range() } + +// fakeHCLExpressionFuncWithTraversals extends [fakeHCLExpressionFunc] with +// a set of traversals that it reports from the [hcl.Expression.Variables] +// method, thereby allowing the expression to also ask Terraform to include +// specific data in the evaluation context that'll eventually be passed +// to the callback function. +type fakeHCLExpressionFuncWithTraversals struct { + fakeHCLExpressionFunc + traversals []hcl.Traversal +} + +// fakeHCLExpression returns a [fakeHCLExpressionFuncWithTraversals] that +// announces that it requires the traversals given in required, and then +// calls the eval callback when asked to evaluate itself. +// +// If the evaluation callback expects to find any variables in the given +// HCL evaluation context then the corresponding traversals MUST be given +// in "required", because Terraform typically populates the context only +// with the minimum required data for a given expression. +func fakeHCLExpression(required []hcl.Traversal, eval fakeHCLExpressionFunc) fakeHCLExpressionFuncWithTraversals { + return fakeHCLExpressionFuncWithTraversals{ + fakeHCLExpressionFunc: eval, + traversals: required, + } +} + +func (f fakeHCLExpressionFuncWithTraversals) Variables() []hcl.Traversal { + return f.traversals +} diff --git a/internal/terraform/node_variable_validation.go b/internal/terraform/node_variable_validation.go index b2c7aede2733..4322e35902aa 100644 --- a/internal/terraform/node_variable_validation.go +++ b/internal/terraform/node_variable_validation.go @@ -34,13 +34,6 @@ type nodeVariableValidation struct { // set from a non-configuration location like an environment variable -- // it's acceptable to use the declaration location instead. defnRange hcl.Range - - // allowGeneralReference is set for nodes that are associated with input - // variables that belong to modules participating in the - // "variable_validation_crossref" language experiment, which allows - // validation rules to refer to other objects declared in the same - // module as the variable. - allowGeneralReferences bool } var _ GraphNodeModulePath = (*nodeVariableValidation)(nil) @@ -114,25 +107,12 @@ func (n *nodeVariableValidation) Execute(globalCtx EvalContext, op walkOperation for _, modInst := range expander.ExpandModule(n.configAddr.Module, false) { addr := n.configAddr.Variable.Absolute(modInst) moduleCtx := globalCtx.withScope(evalContextModuleInstance{Addr: addr.Module}) - if n.allowGeneralReferences { - // This is a more general form that's currently available only - // as an opt-in language experiment. Hopefully eventually this - // evalVariableValidationsCrossRef function replaces the - // old evalVariableValidations and we remove the experiment. - diags = diags.Append(evalVariableValidationsCrossRef( - addr, - moduleCtx, - n.rules, - n.defnRange, - )) - } else { - diags = diags.Append(evalVariableValidations( - addr, - moduleCtx, - n.rules, - n.defnRange, - )) - } + diags = diags.Append(evalVariableValidations( + addr, + moduleCtx, + n.rules, + n.defnRange, + )) } return diags diff --git a/internal/terraform/transform_variable_validation.go b/internal/terraform/transform_variable_validation.go index ad57c3b4115b..7c6ae820d489 100644 --- a/internal/terraform/transform_variable_validation.go +++ b/internal/terraform/transform_variable_validation.go @@ -11,7 +11,6 @@ import ( "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/configs" "github.com/hashicorp/terraform/internal/dag" - "github.com/hashicorp/terraform/internal/experiments" ) // graphNodeValidatableVariable is implemented by nodes that represent @@ -54,7 +53,6 @@ var _ graphNodeValidatableVariable = (*nodeExpandModuleVariable)(nil) // with the new [nodeVariableValidation] nodes to prevent downstream nodes // from relying on unvalidated values. type variableValidationTransformer struct { - config *configs.Config } var _ GraphTransformer = (*variableValidationTransformer)(nil) @@ -67,19 +65,11 @@ func (t *variableValidationTransformer) Transform(g *Graph) error { continue // irrelevant node } - crossRefAllowed := false configAddr, rules, defnRange := v.variableValidationRules() - if moduleConfig := t.config.Descendent(configAddr.Module); moduleConfig != nil { - if moduleConfig.Module.ActiveExperiments.Has(experiments.VariableValidationCrossRef) { - crossRefAllowed = true - } - } - newV := &nodeVariableValidation{ - configAddr: configAddr, - rules: rules, - defnRange: defnRange, - allowGeneralReferences: crossRefAllowed, + configAddr: configAddr, + rules: rules, + defnRange: defnRange, } if len(rules) != 0 { diff --git a/website/docs/language/expressions/custom-conditions.mdx b/website/docs/language/expressions/custom-conditions.mdx index d68107664e2a..6f658ad36ba4 100644 --- a/website/docs/language/expressions/custom-conditions.mdx +++ b/website/docs/language/expressions/custom-conditions.mdx @@ -29,9 +29,9 @@ For more information on when to use certain custom conditions, see [Choosing Bet ## Input Variable Validation --> **Note:** Input variable validation is available in Terraform v0.13.0 and later. +-> **Note:** Input variable validation is available in Terraform v0.13.0 and later. Before Terraform v1.9.0, validation rules can refer only to the variable being validated, and not to any other variables. -Add one or more `validation` blocks within the `variable` block to specify custom conditions. Each validation requires a [`condition` argument](#condition-expressions), an expression that must use the value of the variable to return `true` if the value is valid, or `false` if it is invalid. The expression can refer only to the containing variable and must not produce errors. +Add one or more `validation` blocks within the `variable` block to specify custom conditions. Each validation requires a [`condition` argument](#condition-expressions), an expression that must use the value of the variable to return `true` if the value is valid, or `false` if it is invalid. The expression must not cause errors directly itself. If the condition evaluates to `false`, Terraform produces an [error message](#error-messages) that includes the result of the `error_message` expression. If you declare multiple validations, Terraform returns error messages for all failed conditions.