core: Add fallback for JSON syntax error messages

Custom variable validations specified using JSON syntax would always
parse error messages as string literals, even if they included template
expressions. We need to be as backwards compatible with this behaviour
as possible, which results in this complex fallback logic. More detail
about this in the extensive code comments.

internal/terraform/eval_variable.go

@@ -6,6 +6,7 @@ import (
 	"strings"
 
 	"github.com/hashicorp/hcl/v2"
+	"github.com/hashicorp/hcl/v2/gohcl"
 	"github.com/hashicorp/terraform/internal/addrs"
 	"github.com/hashicorp/terraform/internal/configs"
 	"github.com/hashicorp/terraform/internal/tfdiags"
@@ -211,12 +212,62 @@ func evalVariableValidations(addr addrs.AbsInputVariableInstance, config *config
 		result, moreDiags := validation.Condition.Value(hclCtx)
 		ruleDiags = ruleDiags.Append(moreDiags)
 		errorValue, errorDiags := validation.ErrorMessage.Value(hclCtx)
+
+		// The following error handling is a workaround to preserve backwards
+		// compatibility. Due to an implementation quirk, all prior versions of
+		// Terraform would treat error messages specified using JSON
+		// configuration syntax (.tf.json) as string literals, even if they
+		// contained the "${" template expression operator. This behaviour did
+		// not match that of HCL configuration syntax, where a template
+		// expression would result in a validation error.
+		//
+		// As a result, users writing or generating JSON configuration syntax
+		// may have specified error messages which are invalid template
+		// expressions. As we add support for error message expressions, we are
+		// unable to perfectly distinguish between these two cases.
+		//
+		// To ensure that we don't break backwards compatibility, we have the
+		// below fallback logic if the error message fails to evaluate. This
+		// should only have any effect for JSON configurations. The gohcl
+		// DecodeExpression function behaves differently when the source of the
+		// expression is a JSON configuration file and a nil context is passed.
+		if errorDiags.HasErrors() {
+			// Attempt to decode the expression as a string literal. Passing
+			// nil as the context forces a JSON syntax string value to be
+			// interpreted as a string literal.
+			var errorString string
+			moreErrorDiags := gohcl.DecodeExpression(validation.ErrorMessage, nil, &errorString)
+			if !moreErrorDiags.HasErrors() {
+				// Decoding succeeded, meaning that this is a JSON syntax
+				// string value. We rewrap that as a cty value to allow later
+				// decoding to succeed.
+				errorValue = cty.StringVal(errorString)
+
+				// This warning diagnostic explains this odd behaviour, while
+				// giving us an escape hatch to change this to a hard failure
+				// in some future Terraform 1.x version.
+				errorDiags = hcl.Diagnostics{
+					&hcl.Diagnostic{
+						Severity:    hcl.DiagWarning,
+						Summary:     "Validation error message expression is invalid",
+						Detail:      fmt.Sprintf("The error message provided could not be evaluated as an expression, so Terraform is interpreting it as a string literal.\n\nIn future versions of Terraform, this will be considered an error. Please file a GitHub issue if this would break your workflow.\n\n%s", errorDiags.Error()),
+						Subject:     validation.ErrorMessage.Range().Ptr(),
+						Context:     validation.DeclRange.Ptr(),
+						Expression:  validation.ErrorMessage,
+						EvalContext: hclCtx,
+					},
+				}
+			}
+
+			// We want to either report the original diagnostics if the
+			// fallback failed, or the warning generated above if it succeeded.
 			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())
+			log.Printf("[TRACE] evalVariableValidations: %s rule %s check rule evaluation 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)

internal/terraform/eval_variable_test.go

@@ -2,12 +2,14 @@ package terraform
 
 import (
 	"fmt"
+	"strings"
 	"testing"
 
 	"github.com/hashicorp/hcl/v2"
 	"github.com/zclconf/go-cty/cty"
 
 	"github.com/hashicorp/terraform/internal/addrs"
+	"github.com/hashicorp/terraform/internal/lang"
 	"github.com/hashicorp/terraform/internal/tfdiags"
 )
 
@@ -561,3 +563,155 @@ func TestPrepareFinalInputVariableValue(t *testing.T) {
 		}
 	})
 }
+
+// These tests cover the JSON syntax configuration edge case handling,
+// the background of which is described in detail in comments in the
+// evalVariableValidations function. Future versions of Terraform may
+// be able to remove this behaviour altogether.
+func TestEvalVariableValidations_jsonErrorMessageEdgeCase(t *testing.T) {
+	cfgSrc := `{
+  "variable": {
+    "valid": {
+      "type": "string",
+      "validation": {
+        "condition": "${var.valid != \"bar\"}",
+        "error_message": "Valid template string ${var.valid}"
+      }
+    },
+    "invalid": {
+      "type": "string",
+      "validation": {
+        "condition": "${var.invalid != \"bar\"}",
+        "error_message": "Invalid template string ${"
+      }
+    }
+  }
+}
+`
+	cfg := testModuleInline(t, map[string]string{
+		"main.tf.json": cfgSrc,
+	})
+	variableConfigs := cfg.Module.Variables
+
+	// Because we loaded our pseudo-module from a temporary file, the
+	// declaration source ranges will have unpredictable filenames. We'll
+	// fix that here just to make things easier below.
+	for _, vc := range variableConfigs {
+		vc.DeclRange.Filename = "main.tf.json"
+		for _, v := range vc.Validations {
+			v.DeclRange.Filename = "main.tf.json"
+		}
+	}
+
+	tests := []struct {
+		varName  string
+		given    cty.Value
+		wantErr  []string
+		wantWarn []string
+	}{
+		// Valid variable validation declaration, assigned value which passes
+		// the condition generates no diagnostics.
+		{
+			varName: "valid",
+			given:   cty.StringVal("foo"),
+		},
+		// Assigning a value which fails the condition generates an error
+		// message with the expression successfully evaluated.
+		{
+			varName: "valid",
+			given:   cty.StringVal("bar"),
+			wantErr: []string{
+				"Invalid value for variable",
+				"Valid template string bar",
+			},
+		},
+		// Invalid variable validation declaration due to an unparseable
+		// template string. Assigning a value which passes the condition
+		// results in a warning about the error message.
+		{
+			varName: "invalid",
+			given:   cty.StringVal("foo"),
+			wantWarn: []string{
+				"Validation error message expression is invalid",
+				"Missing expression; Expected the start of an expression, but found the end of the file.",
+			},
+		},
+		// Assigning a value which fails the condition generates an error
+		// message including the configured string interpreted as a literal
+		// value, and the same warning diagnostic as above.
+		{
+			varName: "invalid",
+			given:   cty.StringVal("bar"),
+			wantErr: []string{
+				"Invalid value for variable",
+				"Invalid template string ${",
+			},
+			wantWarn: []string{
+				"Validation error message expression is invalid",
+				"Missing expression; Expected the start of an expression, but found the end of the file.",
+			},
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(fmt.Sprintf("%s %#v", test.varName, test.given), func(t *testing.T) {
+			varAddr := addrs.InputVariable{Name: test.varName}.Absolute(addrs.RootModuleInstance)
+			varCfg := variableConfigs[test.varName]
+			if varCfg == nil {
+				t.Fatalf("invalid variable name %q", test.varName)
+			}
+
+			// Build a mock context to allow the function under test to
+			// retrieve the variable value and evaluate the expressions
+			ctx := &MockEvalContext{}
+
+			// We need a minimal scope to allow basic functions to be passed to
+			// the HCL scope
+			ctx.EvaluationScopeScope = &lang.Scope{}
+			ctx.GetVariableValueFunc = func(addr addrs.AbsInputVariableInstance) cty.Value {
+				if got, want := addr.String(), varAddr.String(); got != want {
+					t.Errorf("incorrect argument to GetVariableValue: got %s, want %s", got, want)
+				}
+				return test.given
+			}
+
+			gotDiags := evalVariableValidations(
+				varAddr, varCfg, nil, ctx,
+			)
+
+			if len(test.wantErr) == 0 && len(test.wantWarn) == 0 {
+				if len(gotDiags) > 0 {
+					t.Errorf("no diags expected, got %s", gotDiags.Err().Error())
+				}
+			} else {
+			wantErrs:
+				for _, want := range test.wantErr {
+					for _, diag := range gotDiags {
+						if diag.Severity() != tfdiags.Error {
+							continue
+						}
+						desc := diag.Description()
+						if strings.Contains(desc.Summary, want) || strings.Contains(desc.Detail, want) {
+							continue wantErrs
+						}
+					}
+					t.Errorf("no error diagnostics found containing %q\ngot: %s", want, gotDiags.Err().Error())
+				}
+
+			wantWarns:
+				for _, want := range test.wantWarn {
+					for _, diag := range gotDiags {
+						if diag.Severity() != tfdiags.Warning {
+							continue
+						}
+						desc := diag.Description()
+						if strings.Contains(desc.Summary, want) || strings.Contains(desc.Detail, want) {
+							continue wantWarns
+						}
+					}
+					t.Errorf("no warning diagnostics found containing %q\ngot: %s", want, gotDiags.Err().Error())
+				}
+			}
+		})
+	}
+}