Change variable validation error message type from string to expression

internal/configs/named_values.go

@@ -2,7 +2,6 @@ package configs
 
 import (
 	"fmt"
-	"unicode"
 
 	"github.com/hashicorp/hcl/v2"
 	"github.com/hashicorp/hcl/v2/gohcl"
@@ -290,8 +289,10 @@ type VariableValidation struct {
 	// 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
+	// true in a way that would make sense to a caller of the module. It is an
+	// expression so that you can include the invalid variable in the error
+	// message.
+	ErrorMessage hcl.Expression
 
 	DeclRange hcl.Range
 }
@@ -355,77 +356,29 @@ func decodeVariableValidationBlock(varName string, block *hcl.Block, override bo
 	}
 
 	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:   "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(),
-				})
+		vv.ErrorMessage = attr.Expr
+		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 {
+						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 validation error message",
+				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
 }
 
-// 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 == '!'
-}
-
 // Output represents an "output" block in a module or file.
 type Output struct {
 	Name        string

internal/configs/testdata/invalid-files/variable-validation-bad-msg.tf

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

internal/configs/testdata/invalid-files/variable-validation-error-msg-badref.tf

@@ -0,0 +1,12 @@
+
+locals {
+  foo = 1
+}
+
+variable "validation" {
+  default = 1
+  validation {
+    condition     = var.validation == 1
+    error_message = "Must be ${local.foo}." # ERROR: Invalid reference in variable validation
+  }
+}

internal/terraform/eval_variable.go

@@ -3,6 +3,7 @@ package terraform
 import (
 	"fmt"
 	"log"
+	"unicode"
 
 	"github.com/hashicorp/hcl/v2"
 	"github.com/hashicorp/terraform/internal/addrs"
@@ -25,7 +26,7 @@ func evalVariableValidations(addr addrs.AbsInputVariableInstance, config *config
 	}
 
 	// Variable nodes evaluate in the parent module to where they were declared
-	// because the value expression (n.Expr, if set) comes from the calling
+	// because the value expression (expr, if set) comes from the calling
 	// "module" block in the parent module.
 	//
 	// Validation expressions are statically validated (during configuration
@@ -46,6 +47,7 @@ func evalVariableValidations(addr addrs.AbsInputVariableInstance, config *config
 	for _, validation := range config.Validations {
 		const errInvalidCondition = "Invalid variable validation result"
 		const errInvalidValue = "Invalid value for variable"
+		const errInvalidErrorMessage = "Invalid validation error message"
 
 		result, moreDiags := validation.Condition.Value(hclCtx)
 		diags = diags.Append(moreDiags)
@@ -81,6 +83,40 @@ func evalVariableValidations(addr addrs.AbsInputVariableInstance, config *config
 			continue
 		}
 
+		errorMessage, moreDiags := validation.ErrorMessage.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())
+		}
+		if !errorMessage.IsKnown() {
+			log.Printf("[TRACE] evalVariableValidations: %s rule %s condition value is unknown, so skipping validation for now", addr, validation.DeclRange)
+			continue // We'll wait until we've learned more, then.
+		}
+		if errorMessage.IsNull() {
+			diags = diags.Append(&hcl.Diagnostic{
+				Severity:    hcl.DiagError,
+				Summary:     errInvalidCondition,
+				Detail:      "Validation error message expression must return a string, not null.",
+				Subject:     validation.ErrorMessage.Range().Ptr(),
+				Expression:  validation.ErrorMessage,
+				EvalContext: hclCtx,
+			})
+			continue
+		}
+
+		errorMessage, err = convert.Convert(errorMessage, cty.String)
+		if err != nil {
+			diags = diags.Append(&hcl.Diagnostic{
+				Severity:    hcl.DiagError,
+				Summary:     errInvalidErrorMessage,
+				Detail:      fmt.Sprintf("Invalid validation error message result value: %s.", tfdiags.FormatError(err)),
+				Subject:     validation.ErrorMessage.Range().Ptr(),
+				Expression:  validation.ErrorMessage,
+				EvalContext: hclCtx,
+			})
+			continue
+		}
+
 		// Validation condition may be marked if the input variable is bound to
 		// a sensitive value. This is irrelevant to the validation process, so
 		// we discard the marks now.
@@ -91,7 +127,7 @@ 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.AsString(), validation.DeclRange.String()),
 					Subject:  expr.Range().Ptr(),
 				})
 			} else {
@@ -101,12 +137,95 @@ 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.AsString(), validation.DeclRange.String()),
+					Subject:  config.DeclRange.Ptr(),
+				})
+			}
+		}
+
+		if errorMessage.Type() != cty.String {
+			if expr != nil {
+				diags = diags.Append(&hcl.Diagnostic{
+					Severity: hcl.DiagError,
+					Summary:  errInvalidErrorMessage,
+					Detail:   fmt.Sprintf("%s\n\nThis was checked by the validation rule at %s.", errorMessage.AsString(), validation.DeclRange.String()),
+					Subject:  expr.Range().Ptr(),
+				})
+			} else {
+				// Since we don't have a source expression for a root module
+				// variable, we'll just report the error from the perspective
+				// of the variable declaration itself.
+				diags = diags.Append(&hcl.Diagnostic{
+					Severity: hcl.DiagError,
+					Summary:  errInvalidErrorMessage,
+					Detail:   fmt.Sprintf("%s\n\nThis was checked by the validation rule at %s.", errorMessage.AsString(), validation.DeclRange.String()),
 					Subject:  config.DeclRange.Ptr(),
 				})
 			}
 		}
+
+		switch {
+		case errorMessage.AsString() == "":
+			diags = diags.Append(&hcl.Diagnostic{
+				Severity: hcl.DiagError,
+				Summary:  errInvalidErrorMessage,
+				Detail:   "An empty string is not a valid nor useful error message.",
+				Subject:  config.DeclRange.Ptr(),
+			})
+		case !looksLikeSentences(errorMessage.AsString()):
+			// 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:  errInvalidErrorMessage,
+				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:  config.DeclRange.Ptr(),
+			})
+		}
 	}
 
 	return 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 == '!'
+}