diff --git a/internal/terraform/graph_builder_apply.go b/internal/terraform/graph_builder_apply.go index 807674377e..f402657a54 100644 --- a/internal/terraform/graph_builder_apply.go +++ b/internal/terraform/graph_builder_apply.go @@ -116,6 +116,7 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer { Config: b.Config, DestroyApply: b.Operation == walkDestroy, }, + &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 386d0c73ab..697b66fee0 100644 --- a/internal/terraform/graph_builder_eval.go +++ b/internal/terraform/graph_builder_eval.go @@ -76,6 +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{}, &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 22d8522fdf..07104baa3b 100644 --- a/internal/terraform/graph_builder_plan.go +++ b/internal/terraform/graph_builder_plan.go @@ -154,6 +154,7 @@ func (b *PlanGraphBuilder) Steps() []GraphTransformer { Planning: true, DestroyApply: false, // always false for planning }, + &variableValidationTransformer{}, &LocalTransformer{Config: b.Config}, &OutputTransformer{ Config: b.Config, diff --git a/internal/terraform/graph_builder_plan_test.go b/internal/terraform/graph_builder_plan_test.go index bc7018f2d5..d880a7f377 100644 --- a/internal/terraform/graph_builder_plan_test.go +++ b/internal/terraform/graph_builder_plan_test.go @@ -262,7 +262,7 @@ func TestPlanGraphBuilder_forEach(t *testing.T) { const testPlanGraphBuilderStr = ` aws_instance.web (expand) aws_security_group.firewall (expand) - var.foo + var.foo (validation) aws_load_balancer.weblb (expand) aws_instance.web (expand) aws_security_group.firewall (expand) @@ -285,6 +285,8 @@ root provider["registry.terraform.io/hashicorp/aws"] (close) provider["registry.terraform.io/hashicorp/openstack"] (close) var.foo +var.foo (validation) + var.foo ` const testPlanGraphBuilderForEachStr = ` aws_instance.bar (expand) diff --git a/internal/terraform/node_module_variable.go b/internal/terraform/node_module_variable.go index c52ecca144..d08009db9e 100644 --- a/internal/terraform/node_module_variable.go +++ b/internal/terraform/node_module_variable.go @@ -150,6 +150,15 @@ func (n *nodeExpandModuleVariable) ReferenceableAddrs() []addrs.Referenceable { return []addrs.Referenceable{n.Addr} } +// variableValidationRules implements [graphNodeValidatableVariable]. +func (n *nodeExpandModuleVariable) variableValidationRules() (addrs.ConfigInputVariable, []*configs.CheckRule) { + var rules []*configs.CheckRule + if n.Config != nil { // always in normal code, but sometimes not in unit tests + rules = n.Config.Validations + } + return n.Addr.InModule(n.Module), rules +} + // 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 29cc4b1aa2..c3919af696 100644 --- a/internal/terraform/node_root_variable.go +++ b/internal/terraform/node_root_variable.go @@ -135,3 +135,12 @@ func (n *NodeRootVariable) DotNode(name string, opts *dag.DotOpts) *dag.DotNode }, } } + +// variableValidationRules implements [graphNodeValidatableVariable]. +func (n *NodeRootVariable) variableValidationRules() (addrs.ConfigInputVariable, []*configs.CheckRule) { + var rules []*configs.CheckRule + if n.Config != nil { // always in normal code, but sometimes not in unit tests + rules = n.Config.Validations + } + return n.Addr.InModule(addrs.RootModule), rules +} diff --git a/internal/terraform/node_variable_validation.go b/internal/terraform/node_variable_validation.go new file mode 100644 index 0000000000..b8c2b79834 --- /dev/null +++ b/internal/terraform/node_variable_validation.go @@ -0,0 +1,86 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +package terraform + +import ( + "fmt" + "slices" + + "github.com/hashicorp/terraform/internal/addrs" + "github.com/hashicorp/terraform/internal/configs" + "github.com/hashicorp/terraform/internal/lang/langrefs" +) + +// nodeVariableValidation checks the author-specified validation rules against +// the final value of all expanded instances of a given input variable. +// +// A node of this type should always depend on another node that's responsible +// for deciding the final values for the nominated variable and registering +// them in the current "named values" state. [variableValidationTransformer] +// is the one responsible for inserting nodes of this type and ensuring that +// they each depend on the node that will register the final variable value. +type nodeVariableValidation struct { + configAddr addrs.ConfigInputVariable + rules []*configs.CheckRule +} + +var _ GraphNodeModulePath = (*nodeVariableValidation)(nil) +var _ GraphNodeReferenceable = (*nodeVariableValidation)(nil) +var _ GraphNodeReferencer = (*nodeVariableValidation)(nil) + +func (n *nodeVariableValidation) Name() string { + return fmt.Sprintf("%s (validation)", n.configAddr.String()) +} + +// ModulePath implements [GraphNodeModulePath]. +func (n *nodeVariableValidation) ModulePath() addrs.Module { + return n.configAddr.Module +} + +// ReferenceableAddrs implements [GraphNodeReferenceable], announcing that +// this node contributes to the value for the input variable that it's +// validating, and must therefore run before any nodes that refer to it. +func (n *nodeVariableValidation) ReferenceableAddrs() []addrs.Referenceable { + return []addrs.Referenceable{n.configAddr.Variable} +} + +// References implements [GraphNodeReferencer], announcing anything that +// the check rules refer to, other than the variable that's being validated +// (which gets its dependency connected by [variableValidationTransformer] +// instead). +func (n *nodeVariableValidation) References() []*addrs.Reference { + var ret []*addrs.Reference + for _, rule := range n.rules { + // We ignore all diagnostics here because if an expression contains + // invalid references then we'll catch them once we visit the + // node (method Execute). + condRefs, _ := langrefs.ReferencesInExpr(addrs.ParseRef, rule.Condition) + msgRefs, _ := langrefs.ReferencesInExpr(addrs.ParseRef, rule.ErrorMessage) + ret = n.appendRefsFilterSelf(ret, condRefs...) + ret = n.appendRefsFilterSelf(ret, msgRefs...) + } + return ret +} + +// appendRefsFilterSelf is a specialized version of builtin [append] that +// ignores any new references to the input variable represented by the +// reciever. +func (n *nodeVariableValidation) appendRefsFilterSelf(to []*addrs.Reference, new ...*addrs.Reference) []*addrs.Reference { + // We need to filter out any self-references, because those would + // make the resulting graph invalid and we don't need them because + // variableValidationTransformer should've arranged for us to + // already depend on whatever node provides the final value for + // this variable. + ret := slices.Grow(to, len(new)) + ourAddr := n.configAddr.Variable + for _, ref := range new { + if refAddr, ok := ref.Subject.(addrs.InputVariable); ok { + if refAddr == ourAddr { + continue + } + } + ret = append(ret, ref) + } + return ret +} diff --git a/internal/terraform/transform_variable_validation.go b/internal/terraform/transform_variable_validation.go new file mode 100644 index 0000000000..d7c0200c75 --- /dev/null +++ b/internal/terraform/transform_variable_validation.go @@ -0,0 +1,60 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +package terraform + +import ( + "github.com/hashicorp/terraform/internal/addrs" + "github.com/hashicorp/terraform/internal/configs" + "github.com/hashicorp/terraform/internal/dag" +) + +// graphNodeValidatableVariable is implemented by nodes that represent +// input variables, and which must therefore have variable validation +// nodes created alongside them to verify that the final value matches +// the author's validation rules. +type graphNodeValidatableVariable interface { + variableValidationRules() (addrs.ConfigInputVariable, []*configs.CheckRule) +} + +// Correct behavior requires both of the input variable node types to +// announce themselves as producing final input variable values that need +// to be validated. +var _ graphNodeValidatableVariable = (*NodeRootVariable)(nil) +var _ graphNodeValidatableVariable = (*nodeExpandModuleVariable)(nil) + +// variableValidationTransformer searches the given graph for any nodes +// that implement [graphNodeValidatableVariable]. For each one found, it +// inserts a new [nodeVariableValidation] and makes it depend on the original +// node, to cause the validation action to happen only after the variable's +// final value has been registered. +// +// This transformer should run after any transformer that might insert a +// node that implements [graphNodeValidatableVariable], and before the +// [ReferenceTransformer] because references like "var.foo" must be connected +// with the new [nodeVariableValidation] nodes to prevent downstream nodes +// from relying on unvalidated values. +type variableValidationTransformer struct { +} + +var _ GraphTransformer = (*variableValidationTransformer)(nil) + +func (t *variableValidationTransformer) Transform(g *Graph) error { + for _, v := range g.Vertices() { + v, ok := v.(graphNodeValidatableVariable) + if !ok { + continue // irrelevant node + } + + configAddr, rules := v.variableValidationRules() + + newV := &nodeVariableValidation{ + configAddr: configAddr, + rules: rules, + } + + g.Add(newV) + g.Connect(dag.BasicEdge(newV, v)) + } + return nil +} diff --git a/internal/terraform/transform_variable_validation_test.go b/internal/terraform/transform_variable_validation_test.go new file mode 100644 index 0000000000..4052625883 --- /dev/null +++ b/internal/terraform/transform_variable_validation_test.go @@ -0,0 +1,140 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +package terraform + +import ( + "fmt" + "strings" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/hashicorp/hcl/v2/hcltest" + "github.com/zclconf/go-cty/cty" + + "github.com/hashicorp/terraform/internal/addrs" + "github.com/hashicorp/terraform/internal/configs" +) + +func TestVariableValidationTransformer(t *testing.T) { + // This is a unit test focused on just the validation transformer's + // behavior, which assumes that the caller correctly arranges for + // its invariants to be met: + // 1. all input variable evaluation nodes must already be present + // in the graph before running the transformer. + // 2. the reference transformer must run after this transformer. + // + // To avoid this depending on transformers other than the one we're + // testing we'll arrange for those to be met in a rather artificial + // way. Our integration tests complement this by verifying that + // the variable validation feature as a whole is working. For + // example: [TestContext2Plan_variableValidation]. + + 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")), + }, + }, + } + 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")), + }, + }, + } + 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"), + }, + }, + } + g.Add(fooNode) + g.Add(barNode) + g.Add(bazNode) + + transformer := &variableValidationTransformer{} + 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) + } +} + +type nodeTestOnlyInputVariable struct { + configAddr addrs.ConfigInputVariable + rules []*configs.CheckRule +} + +func (n *nodeTestOnlyInputVariable) Name() string { + return fmt.Sprintf("%s (test fake)", n.configAddr) +} + +// variableValidationRules implements [graphNodeValidatableVariable]. +func (n *nodeTestOnlyInputVariable) variableValidationRules() (addrs.ConfigInputVariable, []*configs.CheckRule) { + return n.configAddr, n.rules +}