Change variable validation error message type from string to expression

[cgetzen]

Resolves #24407.

This PR changes configs.VariableValidation.ErrorMessage from a string to an expression.

One consequence of this is that some of the validations on the error message (nilstring and lookslikesentence) are moved from the configs package to the terraform package and checked during Eval.

Tests

Template

This file is used as a template for the table below

variable "x" {
  type    = string
  default = "four"
  validation {
    condition     = length(var.x) > 5
    error_message = "not a sentence"
  }
}

terraform {
  experiments = [variable_validation]
}

Results

error_message	output
[4, 3]	Error: Unsuitable value type on main.tf line 6, in variable "x": 6: error_message = [4, 3] Unsuitable value: string required
"Your string ${var.x} isn't long enough."	Error: Invalid value for variable on main.tf line 1: 1: variable "x" { Your string four isn't long enough. This was checked by the validation rule at main.tf:4,3-13.
"not a sentence"	Error: Invalid value for variable on main.tf line 1: 1: variable "x" { not a sentence This was checked by the validation rule at main.tf:4,3-13. Error: Invalid validation error message on main.tf line 1: 1: variable "x" { Validation error message must be at least one full English sentence starting with an uppercase letter and ending with a period or question mark.

[codecov]

Codecov Report

Merging #25028 into master will increase coverage by 0.00%.
The diff coverage is 62.65%.

Impacted Files	Coverage Δ
terraform/eval_variable.go	60.77% <60.00%> (+3.25%)	⬆️
configs/named_values.go	66.80% <76.92%> (-0.40%)	⬇️
terraform/node_resource_plan.go	91.80% <0.00%> (-1.64%)	⬇️
dag/marshal.go	52.27% <0.00%> (-1.14%)	⬇️

[cgetzen]

Hi @jbardin, I seem to be battling merge conflicts like #25144. I'm wondering if you have the bandwidth to give feedback on (or merge) this PR since you've merged close-by code.

[apparentlymart]

Hi @cgetzen,

We're not intending to make any enhancements to the custom variable validation functionality before it's stabilized in the 0.13.0 release, because we want to release exactly what folks have already tested in that release (bugs notwithstanding), so we'll hold on looking at this deeply until at least after the 0.13.0 release has settled down (including any follow-up patch releases we might issue based on bug reports on the initial release), but this does seem to me like a good enhancement.

I don't want you to spend time constantly rebasing this so I'd suggest at least to hold until 0.13.0 seems stable (which might also mean after one or two patch releases if there are any critical bugs in 0.13.0) and then we can deal with any rebase conflicts all at once. (If you have your branch set to allow reviewers here to push to it then we'd be happy to deal with merge conflicts that have an "obvious" resolution on your behalf as part of review+merge.)

[cgetzen]

👋 Hoping to give this one last bump @apparentlymart @jbardin

[joe-a-t]

Is there any chance of getting this in? It looks like it would resolve a ton of validation issues that I have to help debug, particularly when the call into the variable validation is from a loop. Right now, the error message looks like

Error: Invalid value for variable

  on .terraform/modules/gcp/bucket-iam/main.tf line 77, in module "iam_binding":
  77:   member         = each.key

Invalid member, must match this regex '^(domain|group|serviceAccount|user):'.

This was checked by the validation rule at
.terraform/modules/gcp/bucket-iam/variables.tf:14,3-13.

and is really hard to trace back and figure out what string from each.key or each.value is invalid. Being able to have the error message be "Invalid member '${var.member}', must match this regex '^(domain|group|serviceAccount|user):'. would be a HUGE improvement

[joe-a-t]

FYI @cgetzen if you want to close this PR too #28044 (comment)

[crw]

@alisdair per #28044 (comment) shall we close this PR as well?

[alisdair]

Thanks! See #30613 for the in-progress work which should achieve the root goal here.

[github-actions]

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.