mirror of https://github.com/hashicorp/terraform
So far we've handled custom validation rules as a part of the same graph
node that is responsible for deciding the final value for an input
variable, but that's bothersome for two reasons:
- There are two different pieces of code both trying to implement the
same rules: the root variable node and the module variable node. Those
have accidentally diverged in the past.
- Each graph node can evaluate expressions only in one module's scope,
and so the module variable node is set up to evaluate in the parent
module's scope because its purpose is to explicitly pass parent module
data into the child module. That is why we haven't yet allowed input
variable validation rules to refer to objects declared in the module
where the variable is declared.
This commit does not actually yet change any real behavior, because this
new graph node has no "Execute" implementation. The focus here is just to
make sure the graph nodes are inserted into the graph with the needed
edges so that a future commit can move the validation logic into here,
and hopefully eventually also allow the validation rules to refer to
objects declared in the child module.
pull/34957/head
Martin Atkins
2 years ago
parent
ea67f2093f
commit
9b8d25b9e9
@ -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
|
||||
}
|
||||
@ -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
|
||||
}
|
||||
@ -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
|
||||
}
|
||||
Loading…
Reference in new issue