OR operator evals the right part even if the left is true

Terraform
1

Hello!

I don’t know if it is the expected behavior, but in the condition of a validation of a variable, the OR operator evaluates both the left and right parts even if the left part is true. That can lead to unexpected failure, especially when playing with null values.

Here is an example:

variable "test" {
  type = string
  default = null
  validation {
    condition = var.test == null || contains(["a", "b", "c"], var.test)
    error_message = "test must be one of a, b, c or null"
  }
}

Terraform fails because it executes the contains function with var.test set to null. But the expression in the right (var.test == null) is true, so normally Terraform shouldn’t execute the left part? I feel most of the programming languages behave like that?

I found a dirty hack for the condition, but I feel it isn’t very clear and not readable:

variable "test" {
  type = string
  default = null
  validation {
    condition = contains(["a", "b", "c"], var.test == null ? "a" : var.test)
    error_message = "test must be one of a, b, c or null"
  }
}

Does anyone have a better version for the condition?

Thank you!

Cheers,
Antoine

2

Hi @Erouan50,

Personally I’d tend to write this the other way around to how you did, with the condition on the outside and the contains call on the inside. I also tend to wrap conditionals with non-trivial result arms over multiple lines so it’s easier for a sighted reader to scan for the different parts when reading:

  validation {
    condition = (
      var.test != null ?
      contains(["a", "b", "c"], var.test) :
      true
    )
    error_message = "test must be one of a, b, c or null"
  }

This is of course essentially the same as what you wrote, so this is just a matter of taste. If you prefer the ordering you used, then my only suggestion would be to wrap the contains arguments over two separate lines so that it’s easier to visually scan for the conditional expression inside:

    condition = contains(
      ["a", "b", "c"],
      var.test == null ? "a" : var.test,
    )

Terraform does currently evaluate all operands of a boolean operator regardless of the outcome. That might change in future, but there is some debate about what subset of evaluation should be done nonetheless for the “unvisited” operands; Terraform should still report certain problems for unvisited operands, such as type mismatch errors, in order to give good feedback when an expression has been written incorrectly rather than with the intention of ignoring a particular problem.

For now, a conditional expression the way to do this sort of thing in the Terraform language. It’s clearer how Terraform should treat each operand of the conditional operator because they each serve a distinct purpose, whereas short-circuiting operators overload a single expression to represent both a result value and control flow, making the design considerably more subtle.

3

After posting that I thought of another possibility, although I don’t know that I would recommend it, and am just sharing it for completeness:

variable "test" {
  type = string
  default = null
  validation {
    condition = try(contains(["a", "b", "c"], var.test, true))
    error_message = "test must be one of a, b, c or null"
  }
}

This uses try to intercept the error caused by trying to use a null value as the second argument to contains, making the expression return true instead in that case.

This is concise, but I don’t personally like it because it’s not very clear about exactly what situation the try call is trying to handle. Normally I’d use try only in situations like attribute access – e.g. try(foo.bar.baz, "") when either foo or foo.baz might be null – whereas other expression types are potentially more troublesome in try because there are multiple different ways they could dynamically fail, and try doesn’t allow to distinguish between them.

Therefore I think I would still prefer my conditional expression formulation, but I’ll leave the final decision up to you of course. :grinning:

4

Hi @apparentlymart,

Thank you very much for your very detailed answer!

I do prefer your first suggestion, it easy to read and matches the error message.

Cheers,
Antoine