ResiLien
/
terraform
2
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

core: Check rule error message expressions 

Error messages for preconditions, postconditions, and custom variable
validations have until now been string literals. This commit changes
this to treat the field as an HCL expression, which must evaluate to a
string. Most commonly this will either be a string literal or a template
expression.

When the check rule condition is evaluated, we also evaluate the error
message. This means that the error message should always evaluate to a
string value, even if the condition passes. If it does not, this will
result in an error diagnostic.

If the condition fails, and the error message also fails to evaluate, we
fall back to a default error message. This means that the check rule
failure will still be reported, alongside diagnostics explaining why the
custom error message failed to render.

As part of this change, we also necessarily remove the heuristic about
the error message format. This guidance can be readded in future as part
of a configuration hint system.
This commit is contained in:
Alisdair McDiarmid 2022-02-03 14:14:21 -05:00
parent 979ac3da44
commit b06fe04621
11 changed files with 157 additions and 142 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

82
internal/configs/checks.go
View File

@ -2,10 +2,8 @@ package configs
import (
"fmt"
"unicode"
"github.com/hashicorp/hcl/v2"
"github.com/hashicorp/hcl/v2/gohcl"
"github.com/hashicorp/terraform/internal/addrs"
"github.com/hashicorp/terraform/internal/lang"
)
@ -24,12 +22,15 @@ type CheckRule struct {
// input variables can only refer to the variable that is being validated.
Condition hcl.Expression
// ErrorMessage is one or more full sentences, which would need to be in
// ErrorMessage should be one or more full sentences, which should 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
// can in practice be in any language. 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.
//
// The error message expression has the same variables available for
// interpolation as the corresponding condition.
ErrorMessage hcl.Expression
DeclRange hcl.Range
}
@ -111,77 +112,12 @@ func decodeCheckRuleBlock(block *hcl.Block, override bool) (*CheckRule, hcl.Diag
}
if attr, exists := content.Attributes["error_message"]; exists {
moreDiags := gohcl.DecodeExpression(attr.Expr, nil, &cr.ErrorMessage)
diags = append(diags, moreDiags...)
if !moreDiags.HasErrors() {
const errSummary = "Invalid validation error message"
switch {
case cr.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(cr.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: "The validation error message must be at least one full sentence starting with an uppercase letter and ending with a period or question mark.\n\nYour given message will be included as part of a larger Terraform error message, written as English prose. For broadly-shared modules we suggest using a similar writing style so that the overall result will be consistent.",
Subject: attr.Expr.Range().Ptr(),
})
}
}
cr.ErrorMessage = attr.Expr
}
return cr, 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(runes)-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 == '!'
}
var checkRuleBlockSchema = &hcl.BodySchema{
Attributes: []hcl.AttributeSchema{
{

25
internal/configs/named_values.go
View File

@ -345,6 +345,31 @@ func decodeVariableValidationBlock(varName string, block *hcl.Block, override bo
}
}
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 vv, diags
}

33
internal/configs/named_values_test.go
View File

@ -1,33 +0,0 @@
package configs
import (
"testing"
)
func Test_looksLikeSentences(t *testing.T) {
tests := map[string]struct {
args string
want bool
}{
"empty sentence": {
args: "",
want: false,
},
"valid sentence": {
args: "A valid sentence.",
want: true,
},
"valid sentence with an accent": {
args: `A Valid sentence with an accent "é".`,
want: true,
},
}
for name, tt := range tests {
t.Run(name, func(t *testing.T) {
if got := looksLikeSentences(tt.args); got != tt.want {
t.Errorf("looksLikeSentences() = %v, want %v", got, tt.want)
}
})
}
}

6
internal/configs/testdata/invalid-files/variable-validation-bad-msg.tf vendored
View File

@ -1,6 +0,0 @@
variable "validation" {
validation {
condition = var.validation != 4
error_message = "not four" # ERROR: Invalid validation error message
}
}

7
internal/configs/testdata/invalid-files/variable-validation-condition-badref.tf vendored
View File

@ -9,3 +9,10 @@ variable "validation" {
error_message = "Must be five."
}
}
variable "validation_error_expression" {
validation {
condition = var.validation_error_expression != 1
error_message = "Cannot equal ${local.foo}." # ERROR: Invalid reference in variable validation
}
}

16
internal/configs/testdata/valid-files/variable_validation.tf vendored
View File

@ -4,3 +4,19 @@ variable "validation" {
error_message = "Must be five."
}
}
variable "validation_function" {
type = list(string)
validation {
condition = length(var.validation_function) > 0
error_message = "Must not be empty."
}
}
variable "validation_error_expression" {
type = list(string)
validation {
condition = length(var.validation_error_expression) < 10
error_message = "Too long (${length(var.validation_error_expression)} is greater than 10)."
}
}

2
internal/terraform/context_plan2_test.go
View File

@ -2561,7 +2561,7 @@ func TestContext2Plan_preconditionErrors(t *testing.T) {
{
"test_resource.c.value",
"Invalid condition result",
"Invalid validation condition result value: a bool is required",
"Invalid condition result value: a bool is required",
},
}

39
internal/terraform/eval_conditions.go
View File

@ -3,6 +3,7 @@ package terraform
import (
"fmt"
"log"
"strings"
"github.com/hashicorp/hcl/v2"
"github.com/zclconf/go-cty/cty"
@ -59,12 +60,20 @@ func evalCheckRules(typ checkType, rules []*configs.CheckRule, ctx EvalContext,
refs, moreDiags := lang.ReferencesInExpr(rule.Condition)
ruleDiags = ruleDiags.Append(moreDiags)
moreRefs, moreDiags := lang.ReferencesInExpr(rule.ErrorMessage)
ruleDiags = ruleDiags.Append(moreDiags)
refs = append(refs, moreRefs...)
scope := ctx.EvaluationScope(self, keyData)
hclCtx, moreDiags := scope.EvalContext(refs)
ruleDiags = ruleDiags.Append(moreDiags)
result, hclDiags := rule.Condition.Value(hclCtx)
ruleDiags = ruleDiags.Append(hclDiags)
errorValue, errorDiags := rule.ErrorMessage.Value(hclCtx)
ruleDiags = ruleDiags.Append(errorDiags)
diags = diags.Append(ruleDiags)
if ruleDiags.HasErrors() {
@ -91,7 +100,7 @@ func evalCheckRules(typ checkType, rules []*configs.CheckRule, ctx EvalContext,
diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: errInvalidCondition,
Detail: fmt.Sprintf("Invalid validation condition result value: %s.", tfdiags.FormatError(err)),
Detail: fmt.Sprintf("Invalid condition result value: %s.", tfdiags.FormatError(err)),
Subject: rule.Condition.Range().Ptr(),
Expression: rule.Condition,
EvalContext: hclCtx,
@ -99,17 +108,39 @@ func evalCheckRules(typ checkType, rules []*configs.CheckRule, ctx EvalContext,
continue
}
if result.False() {
if result.True() {
continue
}
var errorMessage string
if !errorDiags.HasErrors() && errorValue.IsKnown() && !errorValue.IsNull() {
var err error
errorValue, err = convert.Convert(errorValue, cty.String)
if err != nil {
diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Invalid error message",
Detail: fmt.Sprintf("Unsuitable value for error message: %s.", tfdiags.FormatError(err)),
Subject: rule.ErrorMessage.Range().Ptr(),
Expression: rule.ErrorMessage,
EvalContext: hclCtx,
})
} else {
errorMessage = strings.TrimSpace(errorValue.AsString())
}
}
if errorMessage == "" {
errorMessage = "Failed to evaluate condition error message."
}
diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: typ.FailureSummary(),
Detail: rule.ErrorMessage,
Detail: errorMessage,
Subject: rule.Condition.Range().Ptr(),
Expression: rule.Condition,
EvalContext: hclCtx,
})
}
}
return diags
}

48
internal/terraform/eval_variable.go
View File

@ -3,6 +3,7 @@ package terraform
import (
"fmt"
"log"
"strings"
"github.com/hashicorp/hcl/v2"
"github.com/hashicorp/terraform/internal/addrs"
@ -205,11 +206,17 @@ func evalVariableValidations(addr addrs.AbsInputVariableInstance, config *config
for _, validation := range config.Validations {
const errInvalidCondition = "Invalid variable validation result"
const errInvalidValue = "Invalid value for variable"
var ruleDiags tfdiags.Diagnostics
result, moreDiags := validation.Condition.Value(hclCtx)
diags = diags.Append(moreDiags)
if moreDiags.HasErrors() {
log.Printf("[TRACE] evalVariableValidations: %s rule %s condition expression failed: %s", addr, validation.DeclRange, diags.Err().Error())
ruleDiags = ruleDiags.Append(moreDiags)
errorValue, errorDiags := validation.ErrorMessage.Value(hclCtx)
ruleDiags = ruleDiags.Append(errorDiags)
diags = diags.Append(ruleDiags)
if ruleDiags.HasErrors() {
log.Printf("[TRACE] evalVariableValidations: %s rule %s condition expression failed: %s", addr, validation.DeclRange, ruleDiags.Err().Error())
}
if !result.IsKnown() {
log.Printf("[TRACE] evalVariableValidations: %s rule %s condition value is unknown, so skipping validation for now", addr, validation.DeclRange)
@ -245,13 +252,39 @@ func evalVariableValidations(addr addrs.AbsInputVariableInstance, config *config
// we discard the marks now.
result, _ = result.Unmark()
if result.False() {
if result.True() {
continue
}
var errorMessage string
if !errorDiags.HasErrors() && errorValue.IsKnown() && !errorValue.IsNull() {
var err error
errorValue, err = convert.Convert(errorValue, cty.String)
if err != nil {
diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Invalid error message",
Detail: fmt.Sprintf("Unsuitable value for error message: %s.", tfdiags.FormatError(err)),
Subject: validation.ErrorMessage.Range().Ptr(),
Expression: validation.ErrorMessage,
EvalContext: hclCtx,
})
} else {
errorMessage = strings.TrimSpace(errorValue.AsString())
}
}
if errorMessage == "" {
errorMessage = "Failed to evaluate condition error message."
}
if expr != nil {
diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: errInvalidValue,
Detail: fmt.Sprintf("%s\n\nThis was checked by the validation rule at %s.", validation.ErrorMessage, validation.DeclRange.String()),
Detail: fmt.Sprintf("%s\n\nThis was checked by the validation rule at %s.", errorMessage, validation.DeclRange.String()),
Subject: expr.Range().Ptr(),
Expression: validation.Condition,
EvalContext: hclCtx,
})
} else {
// Since we don't have a source expression for a root module
@ -260,12 +293,13 @@ func evalVariableValidations(addr addrs.AbsInputVariableInstance, config *config
diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: errInvalidValue,
Detail: fmt.Sprintf("%s\n\nThis was checked by the validation rule at %s.", validation.ErrorMessage, validation.DeclRange.String()),
Detail: fmt.Sprintf("%s\n\nThis was checked by the validation rule at %s.", errorMessage, validation.DeclRange.String()),
Subject: config.DeclRange.Ptr(),
Expression: validation.Condition,
EvalContext: hclCtx,
})
}
}
}
return diags
}

4
internal/terraform/node_resource_abstract.go
View File

@ -176,10 +176,14 @@ func (n *NodeAbstractResource) References() []*addrs.Reference {
for _, check := range c.Preconditions {
refs, _ := lang.ReferencesInExpr(check.Condition)
result = append(result, refs...)
refs, _ = lang.ReferencesInExpr(check.ErrorMessage)
result = append(result, refs...)
}
for _, check := range c.Postconditions {
refs, _ := lang.ReferencesInExpr(check.Condition)
result = append(result, refs...)
refs, _ = lang.ReferencesInExpr(check.ErrorMessage)
result = append(result, refs...)
}
return result

3
internal/terraform/node_root_variable_test.go
View File

@ -4,6 +4,7 @@ import (
"testing"
"github.com/hashicorp/hcl/v2"
"github.com/hashicorp/hcl/v2/hcltest"
"github.com/zclconf/go-cty/cty"
"github.com/hashicorp/terraform/internal/addrs"
@ -101,7 +102,7 @@ func TestNodeRootVariableExecute(t *testing.T) {
}
return cty.True, nil
}),
ErrorMessage: "Must be a number.",
ErrorMessage: hcltest.MockExprLiteral(cty.StringVal("Must be a number.")),
},
},
},