Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow variable validation conditions to refer to other variables #25609

Open
ae-ou opened this issue Jul 19, 2020 · 26 comments · May be fixed by #34955
Open

Allow variable validation conditions to refer to other variables #25609

ae-ou opened this issue Jul 19, 2020 · 26 comments · May be fixed by #34955
Labels
custom-conditions Feedback about the "variable_validation" experiment enhancement

Comments

@ae-ou
Copy link

ae-ou commented Jul 19, 2020

Current Terraform Version

Terraform v0.13.0-beta3

Use-cases

I want to be able to have conditional validation on variables - the condition being the value of other variables.
Currently, I'm trying to implement a load balancer (resource "aws_lb") in AWS - the load balancer resource can either be of type "application" or "network". Depending on the load balancer type, there are a number of required and optional variables that you can pass in. Terraform throws an error if you fail to pass a required variable, but, if you pass in an optional variable to the wrong load balancer type, there isn't much warning.

For the sake of example, an application load balancer can optionally take a list of security groups, but a network load balancer can't. If a developer passes in a list of security groups to an NLB, they may have specified the wrong value for load_balancer_type, or they may be passing variables into the wrong load balancer resource - in either case, I'd like to give a warning to the developer to save them having to go debugging.

Obviously, this isn't a request that's strictly for AWS load balancers - the pattern of "value X is required if value Y is Z" seems to be fairly common across Terraform plugins.

Attempted Solutions

I tried adding a validation block here:

variable "security_groups" {
  description = "A list of security groups to attach to the load balancer. Only works with ALBs"
  type = list(string)

  validation {
    condition = var.lb_type == "application"
    error_message = "Security groups can only be used by Application Load Balancers."
  }
}

This is the error that I received:

Error: Invalid variable validation condition

  on variables.tf line 36, in variable "security_groups":
  36:     condition = var.lb_type == "application"

The condition for variable "security_groups" must refer to var.security_groups
in order to test incoming values.

Proposal

Just a few prefaces to my proposals:

  • I haven't created a Terraform plugin before.
  • I'm a huge fan of Terraform, and I apologise if it looks like I'm crapping over the addition of this feature.
  • As @alexjurkiewicz mentioned in Variable validation should allow checking other variables #24374, implementing such a feature could allow for cyclic dependencies between variables - although, I think that it's at the developer's own risk - like how package systems in programming languages facilitate cyclic dependencies (but we still all recognise the benefit of having them).
  • I think that the (bigger) potential issue is causing a significant growth in the dependency graph by creating a complex set of conditional validation rules - @apparentlymart even mentioned this in the comments on the validation block decoder.
  • I'm a huge fan of Terraform, and I apologise if it looks like I'm crapping over the addition of this feature.

I think there are two potential approaches to this:

  1. Remove or alter the statement that checks that validation only refers to itself.
  2. Rewrite schema.Schema so that it isn't a choice between "optional" and "required" being true - this would be a major release change because it would fundamentally alter the workings of plugins.

References

@dimisjim
Copy link

That would be awesome to have.

One could force a module user to only set the variables necessary for the module instantiation and prevent unecessary confusion and questions like:

"Should I set this var, provided that I have set another?"

@vide
Copy link

vide commented Apr 29, 2021

As another example of usefulness of this feature request: every resource that has min and max values, where you can quickly and locally enforce sanity checks on those values without ever hitting the API of $PROVIDER.

@nsballmann
Copy link

nsballmann commented May 27, 2021

Another example of usefulness of this feature request: module parameter lists/sets that need to be non overlapping.

For example:

variable "service_other_ports" {
  description = "Other ports on which the service serves content with the HTTP protocol."
  type        = set(number)
  default     = []
  validation {
    condition     = alltrue([for port in var.service_other_ports : 0 <= port && port <= 65535])
    error_message = "Other ports must have a valid port number."
  }
  validation {
    condition     = alltrue([for port in var.service_other_ports : port != 80])
    error_message = "Other ports can't be 80. Port 80 is reserved for the ELB."
  }
  validation {
    condition     = alltrue([for port in var.service_other_ports : port != 443])
    error_message = "Other ports can't be 443. Port 443 is reserved for the ELB."
  }
}

variable "service_other_tcp_ports" {
  description = "Other TCP ports on which the service serves content."
  type        = set(number)
  default     = []
  validation {
    condition     = alltrue([for port in var.service_other_tcp_ports : 0 <= port && port <= 65535])
    error_message = "Other TCP ports must have a valid port number."
  }
  validation {
    condition     = alltrue([for port in var.service_other_tcp_ports : port != 80])
    error_message = "Other TCP ports can't be 80. Port 80 is reserved for the ELB."
  }
  validation {
    condition     = alltrue([for port in var.service_other_tcp_ports : port != 443])
    error_message = "Other TCP ports can't be 443. Port 443 is reserved for the ELB."
  }
  validation {
    condition     = alltrue([for port in var.service_other_tcp_ports : !contains(var.service_other_ports, port)])
    error_message = "Other TCP ports can't be also in service_other_ports."
  }
}

@ystoneman
Copy link

This would also be useful for situations where a variable can only be true if another variable is true, like in AWS ElastiCache:

variable "dedicated_master_enabled" {
  type           = bool
  default        = false
}
variable "warm_enabled" {
  type           = bool
  default        = false
  description = "Whether AWS UltraWarm is enabled."
  validation {
    condition           = var.warm_enabled == var.dedicated_master_enabled
    error_message = "If warm nodes are enabled, dedicated master must also be enabled."
  }
}

On the other hand, the provider does out-of-the-box come back with Error: ValidationException: To use warm storage, your domain must have dedicated master nodes. when I make this mistake. But it might be nice to have the option of explicitly stating it via a validation error_message, so that the relationship between the variables in my variables.tf file is more explicit to the user of my module.

@apparentlymart
Copy link
Member

Hi all,

I totally agree that it would be great to support this. We did originally intend to make this work, but ran into an internal architectural problem that we need to resolve first and so what we currently have is the result of getting as much of the feature as was practical to implement at the time, as opposed to holding back the entire feature until we have time to make the architectural change.

For folks interested in the details, the architectural problem is that Terraform Core was originally built so that all of the references belonging to a particular object must all be resolved in a single module namespace. Input variables are a funny example of this: their references must always belong to the calling module, because prior to custom validation rules the only expressions that were associated with input variables were the ones in the module block that called the module.

I expect that the design change we will need here is to split Terraform Core's modelling of variables into two parts, so that there's one object representing the definition inside the calling module block ("outside") and another object representing the declaration inside the called module ("inside"). The validation rule expressions would then belong to the "inside" half and could have their references resolved in the module containing the variable block, and we'd internally make the "inside" half depend on the "outside" half to preserve the correct order of operations.

This will be a pretty invasive change to the Terraform language runtime, and so isn't something we'll be able to contend with in the very near future, because our current team size is such that we only really have the bandwidth to focus on one complex project at a time. However, we'd very much like to meet this use-case eventually, and hopefully we can staff up our team some more in the not-too-distant future to allow us to get through more of the larger feature requests like this one.

With that said, it isn't necessary to add new use-cases to this one: we already have a good enough understanding of the goals and it's now a matter of having the time to complete a more detailed design, get consensus on that design, and implement that design for a future release.

Thanks!

@ermik
Copy link

ermik commented Aug 25, 2021

Just another example:

variable "gcp_region" {
  description = "Default region for GCP resources."
  type        = string
  default     = "us-east1"
}

variable "gcp_zone" {
  description = "Default zone for GCP resources."
  type        = string
  default     = "us-east1-d"
  validation {
    condition     = substr(var.gcp_zone, 0, length(var.gcp_region)) != var.gcp_region
    error_message = "Resource zone specified for GCP must be a subset of the region. ${var.gcp_zone} is not part of ${var.gcp_region}"
  }
}

littlejo added a commit to littlejo/terraform-aws-prefix-list that referenced this issue Nov 24, 2021
@infogulch
Copy link

infogulch commented Dec 1, 2021

@ermik I think that's another good and valid use-case for cross-variable validations. If you're willing to massage the inputs a little that particular issue could be alleviated in the short-term, though I admit it's not as nice as the proposed validation:

variable "gcp_region" {
  description = "Default region for GCP resources."
  type        = string
  default     = "us-east1"
}

variable "gcp_zone_suffix" {
  description = "Default zone suffix for GCP resources. The zone will be constructed as `${var.gcp_region}-${var.gcp_zone_suffix}`"
  type        = string
  default     = "d"
}

locals {
  gcp_zone = "${var.gcp_region}-${var.gcp_zone_suffix}"
}

# use var.gcp_region & local.gcp_zone

@iNoahNothing
Copy link

Thanks for the great discussion on this thread. While this will not solve all of the use cases identified here, I thought I would share a workaround I discovered for those who had similar requirements to me.


I needed to ensure that, if one variable (foo) was set to a particular value (BAR), the user of the module was also setting another variable (depends_on_foo_bar) and not just leaving it as the null default which is valid and preferred (but not required) if foo was left as its default.

If variable validation could reference other variables my variables.tf file would have looked like this

variable "foo" {
  type    = string
  default = "BAZ"

  validation {
    condition = (
      var.foo == "BAZ" || var.foo == "BAR"
    )
    error_message = "The foo variable must be set to either BAZ or BAR."
  }
}

variable "depends_on_foo_bar" {
  type    = number
  default = null

  validation {
    condition = (
      (var.foo == "BAR" && var.depends_on_foo_bar != null) ||
      var.foo == "BAZ"
    )
    error_message = "The depends_on_foo_bar variable must be set if foo == BAR."
  }
}

Because I cannot check the value of var.foo in the validation condtion of var.depends_on_foo_bar, this fails.

The workaround for this is to consolidate foo and depends_on_foo_bar into a single variable of type = object and run validatio on the values of the attributes of that object variable.

variable "foo" {
  type = object(
    {
      value              = string
      depends_on_foo_bar = optional(number)
    }
  )
  default = {
    value = "BAZ"
  }
  validation {
    condition = (
      (var.foo == "BAR" && var.depends_on_foo_bar != null) ||
      var.foo == "BAZ"
    )
    error_message = "The foo.value must be set to either BAR or BAZ. If foo.value is set to BAR, then foo.depends_on_foo_bar must also be set.
  }
}

There are some use cases on this thread that this solution will not work for but, in the case where it is logical to consolidate the variables into one, this ended up being a pretty elegant solution to the problem.

@nsballmann
Copy link

I have a solution like this also in my modules here and there, but only in clean cases. It can't be used everywhere. Sometimes it violates separation of concerns from the modules user perspective. From the modules perspective they have clearly a dependency, but this doesn't necessarily mean they should be communicated to the user like that.

@joeaawad
Copy link

joeaawad commented Mar 3, 2022

Here's another workaround that people can do until the work Martin described occurs or something like Terraform allowing users to raise an error happens.

variable "versioning" {
  default = true
}

variable "lifecycle_rules" {
  default = null
}

locals {
  # tflint-ignore: terraform_unused_declarations
  validate_versioning = (var.versioning && var.lifecycle_rules == null) ? tobool("Please pass in lifecycle_rules or change versioning to false.") : true
}

This results in the following error, which while a bit confusing, does provide the necessary info

│ Error: Invalid function argument
│ 
│   on [main.tf](http://main.tf/) line 11, in locals:
│   11:   validate_versioning = (var.versioning && var.lifecycle_rules == null) ? tobool("Please pass in lifecycle_rules or change versioning to false.") : true
│ 
│ Invalid value for "v" parameter: cannot convert "Please pass in
│ lifecycle_rules or change versioning to false." to bool; only the strings
│ "true" or "false" are allowed.

@joe-a-t
Copy link

joe-a-t commented May 24, 2022

FYI 1.2.0 included https://www.terraform.io/language/expressions/custom-conditions#preconditions-and-postconditions so that is the best current workaround for this request if you are able to do your validation at the resource level.

I also just opened #31122 to extend this precondition functionality to module calls, but that issue would really be better solved by addressing this Issue per Martin's comment above.

@gallowaystorm
Copy link

Is this going to be supported?

@bocian-marek-bcg
Copy link

Is this going to be supported?

I bump the question. :3

@crw
Copy link
Collaborator

crw commented Aug 11, 2022

A large amount of re-architecting would be needed to implement this feature request. You can read #25609 (comment) for more info on this. We are leaving this issue open for future consideration, but it is not currently prioritized.

In the meantime the workaround described in #25609 (comment) is the working solution.

edited to say: thanks for the feedback on this issue! We do appreciate it and it is considered.

@gdsotirov
Copy link

Another possible workaround for this issue (as hinted previously) and possible since Terraform 1.2.0, which introduced precondition blocks for output values as custom condition checks, is the following:

variable "versioning" {
  default = true
}

variable "lifecycle_rules" {
  default = null
}

output "validate_versioning" {
  value = null

  precondition {
    condition     = (var.versioning && var.lifecycle_rules == null)
    error_message = "Please pass in lifecycle_rules or change versioning to false."
}

The output value constantly evaluates to null, so it is no printed in 'Changes to Outputs' and the validation dos not have to be at the resource level. I find it better than the workaround with locals, because it is actual validation with error. The only drawback is that you get the error only after the plan is done with wrong inputs, but I take it for now.

I really hope that this is solved in one way or another, because cross variables validation is really necessary to ensure correct inputs to modules.

@bew
Copy link

bew commented Oct 25, 2023

With the new terraform test subsystem we can check variables and expect variable validation failure which is a great way to unit test our validations.
However this only works with the validation block in variable, and does NOT work with local check, check blocks, etc..

👉 I can't unit test module input validations using this system :/


I think it's also important to be able to refer to values (local., path., ..) that are known in advance (constant or loading already-existing content from a file).

Example of code I want to work
locals {
  regions_data = yamldecode(file("${path.module}/../../data-referential/supported_regions.yml"))
}
variable "doesnt_work" {
  type = string

  # Only allow supported regions
  validation {
    condition = contains(local.regions_data.all_regions, var.location)
    error_message = "The location must be one of the supported regions"
  }
}
# => complains about `local.regions_data`
# if I put everything in the condition it complains about `path.module` (which is definitely known beforehand!)

# In the meantime the following seems to work okay, but it's not great..
variable "works_but_meh" {
  type = string

  # Only allow supported regions
  validation {
    condition = contains(yamldecode(file("../../data-referential/supported_regions.yml")).all_regions, var.location)
    error_message = "The location must be one of the supported regions"
  }
}

@apparentlymart
Copy link
Member

The test language's concept of "expected failures" is supposed to work with anything that is a "checkable object" by Terraform's definition, which includes check blocks and any objects that have preconditions and postconditions.

If that isn't working for you then that suggests a bug in the test language. Please report that as a separate issue.

@omarismail
Copy link
Contributor

👋 Hey folks, the terraform team is doing research into this area. I'd love to chat with folks and understand their workflows and use cases. Please reach out to me oismail@hashicorp.com and we can schedule a time!

@gdsotirov
Copy link

gdsotirov commented Feb 23, 2024

@omarismail You could see some examples into this issue. Generally speaking, the limitation to refer only the variable that is defined in its validation blocks’ condition does not allow for any more complex validation rules to be implemented unless with the workaround I showcased before. The simplest of such validations would be to check variable's value against a list of values (e.g. cloud regions or zones defined in local variables as they may be used elsewhere), but there are many more use cases. Personally, I would be very glad if you remove the limitation because this would allow me to rewrite all precondition blocks into true variable validation blocks, because this is where these validations actually belong.

@fabian-ro
Copy link

fabian-ro commented Feb 23, 2024

A simple use case would be an autoscaling Kubernetes node pool and you want to define the minimum and maximum node count. If we could refer the other variable, it would be possible to validate if the max count is actually higher than the min as it is supposed to be:

variable "node_pool_max_count" {
  type    = number
  default = 5

  validation {
    condition     = var.node_pool_max_count > var.node_pool_min_count
    error_message = "The maximum node count must be higher than the minimum node count."
  }
}

variable "node_pool_min_count" {
  type    = number
  default = 1
}

Although this might not be the perfect example because we could merge both variables into one variable of type object, I still think it showcases this issue pretty well.

Another use case is to make sure a certain variable (B) is set if another variable (A) is set. So basically: If A != null, B must also be != null.

@vamsinm
Copy link

vamsinm commented Feb 23, 2024

similar to what @fabian-ro posted here is work around example what have to use:

variable scaling {
    type = object({
        min_replicas        = number
        max_replicas        = number
        autoscaling_enabled = bool
        multi_az            = bool
    })
    default = {
        min_replicas        = 2
        max_replicas        = 2
        autoscaling_enabled = true
        multi_az            = false
    }
    validation {
        condition = (((var.scaling.multi_az == true && var.scaling.min_replicas % 3 == 0 && var.scaling.min_replicas >= 3 && var.scaling.max_replicas % 3 == 0 && var.scaling.max_replicas >= 3) || (var.scaling.multi_az == false && var.scaling.min_replicas >= 2 && var.scaling.max_replicas >= 2)) && (var.scaling.max_replicas >= var.scaling.min_replicas))
        error_message = "For a multi-AZ Cluster minimum/maximum replicas must be greater than or equal to 3 and must be multiples of 3. For a non-multi-AZ Cluster minimum/maximum replicas must be greater than or equal to 2. Maximum replicas must be greater than or equal to Minimum replicas."
    }
}

instead of

variable min_replicas {
   type = number
    default = 2
}

variable max_replicas {
   type = number
    default = 2
    validation {
       condition = var.max_replicas >= var.min_replicas
       error_message = "minimum replicas must be greater than or equal to maximum replicas"
   }
}

variable multiaz {
  type = bool
  validation {
    condition = (var.multiaz == true && var.min_replicas % 3 == 0 &&  && var.max_replicas %3 == 0)
    error_message = "minimum and maximum replicas must be multiples of three for multiaz"
  }
}

@RoseSecurity
Copy link

Are there any plans to get this on the road map as a feature?

@crw
Copy link
Collaborator

crw commented Mar 1, 2024

@RoseSecurity I'd recommend reading this comment (#25609 (comment)) and reaching out if you are interested in providing direct feedback. Thanks!

@TRiggAtGM
Copy link

I'd like to provide another example. Let's say there's a validation that we want to perform only for higher environments, but not for lowers in relationship to our scaling and availability policy. In lower environments, it's fine for databases to have a lower number of instances or instance sizes in a cluster, but in a production environment, those instances must have a minimum count. I would like to add it to a module that if environment is either the performance testing environment, or the production environment, there must be at least 3 instances present in the environment. Ideally (I think) I would write this as

variable "instances" {
  description = "Map of cluster instances and any specific/overriding attributes to be created"
  type        = any
  default     = {}

  validation {
    condition     = var.env == "prod" ? length(3 <= var.instances) : true
    error_message = "Production environment must have at least three instances"
  }
}

Currently, it feels like the lack of this form of validation forces checks higher up the testing pyramid than they should be. I could technically rewrite ALL of my variables into one big object that allows me to read object attributes. The problem with this is that at a certain point, we lose all the benefits of IDE/LSP discovery and good variable descriptions, making the API we interact with the module from much less… Good.

@joe-a-t
Copy link

joe-a-t commented May 2, 2024

@apparentlymart I just saw

If we do run into problems with this, I would then like to try for a compromise where we allow referring to other input variables but not to anything else.

from #34947 and wanted to call out that the two biggest use cases I see are 1) referencing other variables and 2) referencing a local (for example DRYing the allowed values list in both the condition and the error message). Hopefully the general approach will work, but if you do need to limit it, if there is a way to keep locals as possible values (at least as long as they do not reference any resources), that would be much appreciated and have a big impact for my company.

@apparentlymart
Copy link
Member

apparentlymart commented May 2, 2024

Hi! @joe-a-t stole my thunder here a little by pre-reacting to the thing I was about to announce, but anyway... 😀

Yesterday's v1.9.0 alpha release of Terraform CLI included an experimental solution to this feature request, and so it'd be very helpful if some of the folks who were interested in this issue would give it a try and share information about what happened.

For experiments we prefer to gather feedback in community forum topics rather than GitHub because GitHub doesn't really work well for multiple threads of discussion, so if you'd like to participate please take a look at this topic instead of posting new comments here:

Experiment Feedback: Input Variable Validation can cross-reference other objects

For experiments like this it's helpful to see both successful and unsuccessful attempts to use it, so if you try it out and it works as you expected I'd still encourage you to leave a comment describing what you tried. Positive feedback on this will increase our confidence in stabilizing the current implementation as-is, as opposed to iterating on it further.

I'm going to lock this issue temporarily just to encourage keeping the experiment feedback all contained in the linked community forum topic, since it'll be harder to keep track of feedback in two different places. I'll unlock this again once our focus shifts away from feedback on this specific release and we're ready to discuss what might happen next.

Thanks!

@hashicorp hashicorp locked and limited conversation to collaborators May 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
custom-conditions Feedback about the "variable_validation" experiment enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.