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

Cross Variable Validation When Using Nested Modules. #34117

Closed
nuryupin-kr opened this issue Oct 19, 2023 · 7 comments
Closed

Cross Variable Validation When Using Nested Modules. #34117

nuryupin-kr opened this issue Oct 19, 2023 · 7 comments
Labels
enhancement new new issue not yet triaged

Comments

@nuryupin-kr
Copy link

nuryupin-kr commented Oct 19, 2023

Terraform Version

Terraform v1.5.4
on windows_amd64

As of today, not aware of such feature being available in latest versions 1.6.x either.

Use Cases

When having terraform module that has one or more child(nested) modules, we would like to do a cross variable validation on a parent level, before passing data down to the child module. Validation should fail our configuration if the conditions aren't met.

As of today, we have the following features available:

  1. Check blocks - doesn't fail configuration when assertion fails.
  2. Input variable validation - can't do cross variable validation.
  3. pre/post conditions - works only on resource and outputs.

None of these work for us.

Attempted Solutions

  1. Instead of having separate variables, create a single variable of type object that contains all of the values, so we can cross-value validate. Almost always this is not ideal approach, because our variables are already quite complex objects and merging them together would render them unreadable and wouldn't make sense from a domain perspective.
  2. Create a child module that combines desired variables from a parent module into a single object and use it for validation as explained in the solution 1. This approach is clunky and leads to more repetitive code.
  3. To avoid combining variables, use the assert_test provider to write cross variable validation. So far this is the best solution, but it's not native.

Proposal

Options:

  • Make check block optionally fail so that way one can choose to have a warning or error.
  • Add pre/post conditions to module blocks.
  • Make cross variable validation possible.

References

No response

@nuryupin-kr nuryupin-kr added enhancement new new issue not yet triaged labels Oct 19, 2023
@apparentlymart
Copy link
Member

Thanks for this feedback, @nuryupin-kr!

One of your proposed solutions is to support conditions in modules. By that, did you mean conditions inside a module block?

I ask this because the rest of what you described made me assume you were talking about having the module check that its caller passed appropriate values, but adding conditions in a module block would presumably mean the caller is checking its own inputs to the child module, which seems like a different requirement than the other way.

I think both are reasonable but I want to make sure we understand which of the two you want to have.

@nuryupin-kr
Copy link
Author

nuryupin-kr commented Oct 19, 2023

Sorry for the confusion.

Yes, something like this in the module block:

module "my_child_module_for_legacy_stuff" {
  source = "./child-module_legacy"

  for_each = var.configuration.legacy_values
  config    = each.value
  naming  = var.naming

  lifecycle {
    precondition {
      condition        = (length(var.configuration.legacy_values) > 0 
                                   ? var.naming.legacy_suffix == null || var.naming.legacy_suffix == "" ? false  :  true
                                   : true
                                 )
      error_message = "When configuration contains legacy values, the legacy suffix must be supplied."
    }
  }
}

OR having a check block support optional boolean flag to fail configuration like this:

check "validate_legacy_inputs" {
  fail = true
  assert {
    condition         = (length(var.configuration.legacy_values) > 0 
                                   ? var.naming.legacy_suffix == null || var.naming.legacy_suffix == "" ? false  :  true
                                   : true
                                 )
    error_message = "When configuration contains legacy values, the legacy suffix must be supplied."
  }
}

The option with check block is probably preferable when validation applies to all child modules.

@nuryupin-kr
Copy link
Author

Having cross variable validation enabled could be a solution as well :)

@apparentlymart
Copy link
Member

apparentlymart commented Oct 19, 2023

An important detail about check blocks is that they get checked only after everything is already done; I don't think that can solve your problem as I understand it, because you seem to need the check to block evaluating the objects inside the module. precondition is the most natural primitive for that situation, because then it's clear to Terraform (based on the dependency graph) when the condition should be checked in relation to other side-effects.

However, I'm still noticing that you are suggesting both checks in the caller and checks in the called module. Those feel like separate requirements to me: a module checking that the caller is passing suitable values is different to the caller checking that its own inputs to the module meet some condition that is presumably specific to that particular calling module.

@nuryupin-kr
Copy link
Author

nuryupin-kr commented Oct 20, 2023

In that case if check blocks only execute after the plan is done, then it definitely won't work for us.

Looks like the only two options:

  • pre/post conditions on module block. While this would be great for running pre-condition logic for the input data passed to a single specific child module it wouldn't really be a good solution when applying same input data validation logic across all child modules on the same hierarchy level, the logic would have to be duplicated across all child module blocks.

  • Ability for cross variable validation. This probably would work best for a scenario when global validation is needed, involving multiple variables, before any child module is called.

I could definitely find a scenario where it would be useful to have pre-conditions applied to the input data of a single child module. But more often than not we would want to apply validation globally. I guess the difference would be if validation is module block specific, Terraform could generate the rest of the configuration plan and only show error for that specific module input data that is being validated (given there is no dependencies between child modules on the same hierarchy level). On the other hand, when we use global validation, it would fail before attempting to plan anything at all.

The other approach would be adding standalone validation blocks that would look just like check blocks but would run before the plan execution.

I'm trying to be as specific as I can with wording so hopefully it all makes sense. :)

@apparentlymart
Copy link
Member

While reading what you wrote I chuckled a bit because in my original proposal for what became check blocks I had presented them as a sort of "whole-module global postcondition", but during product discovery work our PM found that people preferred the word check for that concept even though it was ambiguous about when the checks run and what they check. One of the ideas you've proposed here is a sort of global precondition, which would perhaps be easier to imagine if we'd already named the current checks "postconditions" as I had originally imagined. But anyway...

Some subtext I'm reading into your answers is that you don't actually really care where the conditions live in your case, I assume because you're maintaining both the caller and the callee modules together and they are tightly coupled enough that the question of which module should be responsible for the checks is a totally irrelevant question.

However, I think when we try to imagine a feature that would be useful for more than just your situation we do need to consider the situation where the calling module and the module being called are maintained separately by different teams, which raises two separate use-cases:

  1. As the author of a shared module, I want to describe what constitutes a valid total set of input values -- not just each input value being valid in isolation -- so that invalid usage will appear as an error in the calling module block rather than an error in the implementation details of my module (which the author of the calling module might not be familiar with).
  2. As the user of a shared module, I want to describe some assumptions I'm making about the values I'm passing in to that shared module in a way that Terraform can raise an error if I get them wrong, so that I can get better feedback if future maintenance accidentally violates those assumptions. If I'm using terraform test to test my module, I'd also want the tests to fail if I violate those assumptions.

For me, this feels like two features that I think have both been requested previously and already have open issues:

  1. Either Standalone variable validations that can address multiple variables #32792 or Allow variable validation conditions to refer to other variables #25609, which are two different takes on being able to validate the full set of inputs together rather than only each variable separately.

    (I personally slightly prefer the second one because it allows the module author to decide which specific argument in the calling module block ought to be "blamed" if the condition doesn't hold, whereas the first would presumably require Terraform just report the entire module block as invalid and would thus give vaguer feedback, but that detail is not super relevant to what we're discussing here.)

  2. Support precondition and postcondition blocks in module calls #31122, which would allow the caller of a module to describe both what they are intending to guarantee about their inputs to the module and what assumptions they are making about the outputs from the module.

I think both of those are reasonable, and if some or all of those changes would be acceptable for your situation then I'd suggest we close this issue and have you add a 👍 upvote to one or both of them to record your interest in it, which we'll then consider as part of deciding what to work on next. (Keeping this separate issue open would just "split the vote", making it harder to use the emoji reactions as a signal.)

If you disagree with me that these are describing valid solutions to the problem you have then of course we can continue discussing to try to refine what makes this issue different from all of those.


If the precondition part of option 2 seems like it would address your problem then I'd note that you can already write something that's functionally equivalent to that in today's Terraform, like this:

resource "terraform_data" "checks" {
  lifecycle {
    precondition {
      # (write in here whatever you would've written in the precondition
      # block of the module, had that been allowed.)
    }
  }
}

module "example" {
  # ...

  depends_on = [terraform_data.checks]
}

By declaring that module "example" depends on resource "terraform_data" "checks", we effectively make any preconditions of that resource serve as indirect preconditions of the module call, since the module call cannot be evaluated until its dependencies are evaluated and valid.

Support for precondition inside a module block would be a more compact way to describe the same behavior. Either way, it would imply that everything inside the module can be evaluated only if the precondition holds.

Copy link

github-actions bot commented Dec 7, 2023

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

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement new new issue not yet triaged
Projects
None yet
Development

No branches or pull requests

2 participants