Skip to content

Commit

Permalink
config: Allow module authors to specify validation rules for variables
Browse files Browse the repository at this point in the history
The existing "type" argument allows specifying a type constraint that
allows for some basic validation, but often there are more constraints on
a variable value than just its type.

This new feature (requiring an experiment opt-in for now, while we refine
it) allows specifying arbitrary validation rules for any variable which
can then cause custom error messages to be returned when a caller provides
an inappropriate value.

    variable "example" {
      validation {
        condition = var.example != "nope"
        error_message = "Example value must not be \"nope\"."
      }
    }

The core parts of this are designed to do as little new work as possible
when no validations are specified, and thus the main new checking codepath
here can therefore only run when the experiment is enabled in order to
permit having validations.
  • Loading branch information
apparentlymart committed Jan 10, 2020
1 parent cef4c3b commit 830a065
Show file tree
Hide file tree
Showing 21 changed files with 641 additions and 14 deletions.
13 changes: 13 additions & 0 deletions configs/experiments.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,5 +139,18 @@ func checkModuleExperiments(m *Module) hcl.Diagnostics {
}
*/

if !m.ActiveExperiments.Has(experiments.VariableValidation) {
for _, vc := range m.Variables {
if len(vc.Validations) != 0 {
diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Custom variable validation is experimental",
Detail: "This feature is currently an opt-in experiment, subject to change in future releases based on feedback.\n\nActivate the feature for this module by adding variable_validation to the list of active experiments.",
Subject: vc.Validations[0].DeclRange.Ptr(),
})
}
}
}

return diags
}
3 changes: 2 additions & 1 deletion configs/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,8 @@ func NewModule(primaryFiles, overrideFiles []*File) (*Module, hcl.Diagnostics) {
diags = append(diags, fileDiags...)
}

diags = append(diags, checkModuleExperiments(mod)...)
moreDiags := checkModuleExperiments(mod)
diags = append(diags, moreDiags...)

return mod, diags
}
Expand Down
186 changes: 186 additions & 0 deletions configs/named_values.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package configs

import (
"fmt"
"unicode"

"github.com/hashicorp/hcl/v2"
"github.com/hashicorp/hcl/v2/ext/typeexpr"
Expand All @@ -23,6 +24,7 @@ type Variable struct {
Default cty.Value
Type cty.Type
ParsingMode VariableParsingMode
Validations []*VariableValidation

DescriptionSet bool

Expand Down Expand Up @@ -119,6 +121,21 @@ func decodeVariableBlock(block *hcl.Block, override bool) (*Variable, hcl.Diagno
v.Default = val
}

for _, block := range content.Blocks {
switch block.Type {

case "validation":
vv, moreDiags := decodeVariableValidationBlock(v.Name, block, override)
diags = append(diags, moreDiags...)
v.Validations = append(v.Validations, vv)

default:
// The above cases should be exhaustive for all block types
// defined in variableBlockSchema
panic(fmt.Sprintf("unhandled block type %q", block.Type))
}
}

return v, diags
}

Expand Down Expand Up @@ -250,6 +267,157 @@ func (m VariableParsingMode) Parse(name, value string) (cty.Value, hcl.Diagnosti
}
}

// VariableValidation represents a configuration-defined validation rule
// for a particular input variable, given as a "validation" block inside
// a "variable" block.
type VariableValidation struct {
// Condition is an expression that refers to the variable being tested
// and contains no other references. The expression must return true
// to indicate that the value is valid or false to indicate that it is
// invalid. If the expression produces an error, that's considered a bug
// in the module defining the validation rule, not an error in the caller.
Condition hcl.Expression

// ErrorMessage is one or more full sentences, which would need to be in
// English for consistency with the rest of the error message output but
// can in practice be in any language as long as it ends with a period.
// The message should describe what is required for the condition to return
// true in a way that would make sense to a caller of the module.
ErrorMessage string

DeclRange hcl.Range
}

func decodeVariableValidationBlock(varName string, block *hcl.Block, override bool) (*VariableValidation, hcl.Diagnostics) {
var diags hcl.Diagnostics
vv := &VariableValidation{
DeclRange: block.DefRange,
}

if override {
// For now we'll just forbid overriding validation blocks, to simplify
// the initial design. If we can find a clear use-case for overriding
// validations in override files and there's a way to define it that
// isn't confusing then we could relax this.
diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Can't override variable validation rules",
Detail: "Variable \"validation\" blocks cannot be used in override files.",
Subject: vv.DeclRange.Ptr(),
})
return vv, diags
}

content, moreDiags := block.Body.Content(variableValidationBlockSchema)
diags = append(diags, moreDiags...)

if attr, exists := content.Attributes["condition"]; exists {
vv.Condition = attr.Expr

// 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: attr.Expr.Range().Ptr(),
})
}
}

if attr, exists := content.Attributes["error_message"]; exists {
moreDiags := gohcl.DecodeExpression(attr.Expr, nil, &vv.ErrorMessage)
diags = append(diags, moreDiags...)
if !moreDiags.HasErrors() {
const errSummary = "Invalid validation error message"
switch {
case vv.ErrorMessage == "":
diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: errSummary,
Detail: "An empty string is not a valid nor useful error message.",
Subject: attr.Expr.Range().Ptr(),
})
case !looksLikeSentences(vv.ErrorMessage):
// Because we're going to include this string verbatim as part
// of a bigger error message written in our usual style in
// English, we'll require the given error message to conform
// to that. We might relax this in future if e.g. we start
// presenting these error messages in a different way, or if
// Terraform starts supporting producing error messages in
// other human languages, etc.
// For pragmatism we also allow sentences ending with
// exclamation points, but we don't mention it explicitly here
// because that's not really consistent with the Terraform UI
// writing style.
diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: errSummary,
Detail: "Validation error message must be at least one full English sentence starting with an uppercase letter and ending with a period or question mark.",
Subject: attr.Expr.Range().Ptr(),
})
}
}
}

return vv, diags
}

// looksLikeSentence is a simple heuristic that encourages writing error
// messages that will be presentable when included as part of a larger
// Terraform error diagnostic whose other text is written in the Terraform
// UI writing style.
//
// This is intentionally not a very strong validation since we're assuming
// that module authors want to write good messages and might just need a nudge
// about Terraform's specific style, rather than that they are going to try
// to work around these rules to write a lower-quality message.
func looksLikeSentences(s string) bool {
if len(s) < 1 {
return false
}
runes := []rune(s) // HCL guarantees that all strings are valid UTF-8
first := runes[0]
last := runes[len(s)-1]

// If the first rune is a letter then it must be an uppercase letter.
// (This will only see the first rune in a multi-rune combining sequence,
// but the first rune is generally the letter if any are, and if not then
// we'll just ignore it because we're primarily expecting English messages
// right now anyway, for consistency with all of Terraform's other output.)
if unicode.IsLetter(first) && !unicode.IsUpper(first) {
return false
}

// The string must be at least one full sentence, which implies having
// sentence-ending punctuation.
// (This assumes that if a sentence ends with quotes then the period
// will be outside the quotes, which is consistent with Terraform's UI
// writing style.)
return last == '.' || last == '?' || last == '!'
}

// Output represents an "output" block in a module or file.
type Output struct {
Name string
Expand Down Expand Up @@ -367,6 +535,24 @@ var variableBlockSchema = &hcl.BodySchema{
Name: "type",
},
},
Blocks: []hcl.BlockHeaderSchema{
{
Type: "validation",
},
},
}

var variableValidationBlockSchema = &hcl.BodySchema{
Attributes: []hcl.AttributeSchema{
{
Name: "condition",
Required: true,
},
{
Name: "error_message",
Required: true,
},
},
}

var outputBlockSchema = &hcl.BodySchema{
Expand Down
11 changes: 11 additions & 0 deletions configs/testdata/invalid-files/variable-validation-bad-msg.tf
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@

terraform {
experiments = [variable_validation]
}

variable "validation" {
validation {
condition = var.validation != 4
error_message = "not four" # ERROR: Invalid validation error message
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@

terraform {
experiments = [variable_validation]
}

locals {
foo = 1
}

variable "validation" {
validation {
condition = local.foo == var.validation # ERROR: Invalid reference in variable validation
error_message = "Must be five."
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@

terraform {
experiments = [variable_validation]
}

variable "validation" {
validation {
condition = true # ERROR: Invalid variable validation condition
error_message = "Must be true."
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@

variable "validation_without_optin" {
validation { # ERROR: Custom variable validation is experimental
condition = var.validation_without_optin != 4
error_message = "Must not be four."
}
}
10 changes: 10 additions & 0 deletions configs/testdata/warning-files/variable_validation_experiment.tf
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
terraform {
experiments = [variable_validation] # WARNING: Experimental feature "variable_validation" is active
}

variable "validation" {
validation {
condition = var.validation == 5
error_message = "Must be five."
}
}
4 changes: 2 additions & 2 deletions experiments/experiment.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,13 @@ type Experiment string
// Each experiment is represented by a string that must be a valid HCL
// identifier so that it can be specified in configuration.
const (
// Example = Experiment("example")
VariableValidation = Experiment("variable_validation")
)

func init() {
// Each experiment constant defined above must be registered here as either
// a current or a concluded experiment.
// registerCurrentExperiment(Example)
registerCurrentExperiment(VariableValidation)
}

// GetCurrent takes an experiment name and returns the experiment value
Expand Down
Loading

0 comments on commit 830a065

Please sign in to comment.