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

Variable validation block is not parsed #73

Open
Lahiri opened this issue Dec 1, 2021 · 11 comments
Open

Variable validation block is not parsed #73

Lahiri opened this issue Dec 1, 2021 · 11 comments

Comments

@Lahiri
Copy link

Lahiri commented Dec 1, 2021

The validation block in variables is not parsed.

For example:

variable "location" {
  type        = string
  default     = "westeurope"
  validation {
    condition = contains(["westeurope", "northeurope", "global"], var.location)
    error_message = "Unsupported location."
  }
}

Is parsed as

 "location": {
      "name": "location",
      "type": "string",
      "default": "westeurope",
      "required": false,
      "pos": {
        "filename": "<file-path>",
        "line": 1
      }
    }
@apparentlymart
Copy link
Contributor

Hi @Lahiri! Thanks for this enhancement request.

Can you say something about what you intend to do with this information if we were to add support for it? The answer to that seems likely to influence how the information ought to be represented in the result, and also help us understand whether your use-case is within the intended scope of this library.

Thanks again!

@tbaumann-sf
Copy link

Hi @apparentlymart,

This is also a feature that my group would be interested in having this information available. We are in the process of writing a CLI that puts some scaffolding around terraform and being able to parse the validation rules at the time we construct the module block to validate the value that the users would be putting in would be invaluable.

If you need to know any more information please let me know and I would be happy to add some more details where I can.

@apparentlymart
Copy link
Contributor

Hi @tbaumann-sf!

If your goal is to evaluate the validation rule then I expect you'd need access to the raw expression in the condition argument, rather than just metadata about it. So far this library has never directly exposed the underlying implementation details, and I'd be nervous about doing so here because it would make it hard to evolve this part of the language in future, such as by allowing variable validation to refer to other items in the same module.

I wonder if your use-case could be better met by something in Terraform CLI itself, which could in theory take a set of -var and -var-file options in the same way that terraform plan would and then "validate" those variables against the root module validation rules in order to determine if those variable values are valid without taking any other action against the module. Does that sound like it would be helpful for your situation? If so, I suggest opening an enhancement request in the main Terraform repository about it, since that'll be a better place to discuss the implications of offering such a feature and potentially to discuss the tradeoffs between a few different ways to achieve it (which could, in the end, be via a change to this library, but we'll see.)

@tbaumann-sf
Copy link

@apparentlymart
I think I need to take a step back and provide a little more context on this.

We have some custom built terraform modules which take terraform variables as inputs and drops them in a module block. To determine the terraform inputs for this we are using terraform-docs to generate the json output and parsing the json to see which variables are needed or not needed. With this comes the need to in some cases have validation on what are acceptable values. The hope is here we do our validation before we even get to the step where we run terraform plan.

Does that help ?

@apparentlymart
Copy link
Contributor

Hi @tbaumann-sf,

It would be better to share this use-case in more detail in an issue in the main Terraform repository. We can then determine if it is something that would be better to solve by something in Terraform CLI (which contains logic to actually check validation rules) or something in this library (which could only possibly export the source code of the expression, and not actually evaluate it).

@rquadling
Copy link

rquadling commented Mar 3, 2023

I've just come here via terraform-docs as I would like to include the validation error messages in the module output. So, along with the name, type, default, and description of an input variable, including the potential error messages in that documentation means I don't have to double up the actual message in the description so that they get presented to the module consumer via the README (or wherever terraform-docs has been instructed to save the content).

Advertising the known restrictions is nicer that waiting until you fall foul of them.

Whilst I'm not a go expert, this would be one of those nicely scoped features to learn from. I use Terraform every day and have written a few tiny PRs for it. This enhancement would be something that I think would be worthwhile.

@rquadling
Copy link

Would it be possible to have a Hashicorp maintained fork of this for accepting PRs?

So, http://github.com/hashicorp/terraform-config-inspect-ext (-ext is often used to mean 'extended' in some way).

That way, enhancements COULD be made with the oversight of Hashicorp.

This would be "nicer" than having some random owner (me for example) of the enhanced package.

Another option would be to have the main branch as the official branch and an community branch that is maintained alongside the main branch.

Both branches can have releases. This is even "nicer" as 1 repo with all the work in.

Admittedly, workflow for this may not be consistent with other Hashicorp projects, but the community branch could have its own workflows on top of the main branch's.

@scr-oath
Copy link

scr-oath commented May 6, 2023

I also have a need for this - for a tool that helps maintain tf modules - I would like to be able to parse a module and output its variables and outputs, but I do it with the existing library, the validation block is not retrievable.

I'm curious about the resistance to this and whether there's a more appropriate library to parse a module that can actually parse the entire language - certainly SOMETHING can parse the entire language for the terraform tool to use - why is this not made available to clients to read it for any purpose they come up with - in other words, why should this even be a feature request - shouldn't parsing the language in its entirety be the goal?

@rquadling
Copy link

@scr-oath I don't have info as to why the PRs are not accepted (ignoring things like CLAs/NDAs/any sort of dev doc signing). It may be that HCL is a far more limited language and that Terraform has added a complete sidepath that isn't backward compatible. If there was a way to build a workflow and project that could take the PRs and maintain accurate releases, then tools like terraform-docs could switch to the new package, but the route back to the official base library is maintained and acknowledged.

Obviously, anyone can make a public project, but keeping it uptodate ... well, we come, we go ... which is why I'd like to get @apparentlymart's response as to what can we do that will provide the enhancements that we currently see as needed and support an isolated release process that's acknowledged as safe and secure.

@crw
Copy link
Contributor

crw commented May 19, 2023

[...] shouldn't parsing the language in its entirety be the goal?

I asked this question specifically. The answer is that this repository was created to be used by other internal HashiCorp products to inspect terraform config. As such, features ported in on an as-needed basis.

Probably not satisfying, but that is the answer. With regards to HashiCorp managing a community-led extension, that feels pretty unlikely and would lead to the same support and maintenance concerns as just accepting these community pull requests in the first place. However, please don't take that as the definitive or "forever" answer -- it is just my gut feeling. I'll leave the question open for future respondents. :)

@rquadling
Copy link

As the PR is not going to be accepted as is (https://discuss.hashicorp.com/t/improving-the-terraform-config-inspect-library/58474/2 has a good explanation as to why), I'm deleting the PR, but keeping the fork. And hopefully, enough traction will exist to add new features to the fork.

If anyone is reading this and wants their PR merged to the fork, then take a visit to https://github.com/rquadling/terraform-config-inspect and let's see how far we can get!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants