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

new interpolation function to get values from var files #603

Closed
marionettist opened this issue Nov 9, 2018 · 10 comments
Closed

new interpolation function to get values from var files #603

marionettist opened this issue Nov 9, 2018 · 10 comments
Labels
enhancement New feature or request

Comments

@marionettist
Copy link

I would like to propose the following enhancement to terragrunt.
My team and I have been using terragrunt for about a year now and we like
the value it adds on top of terraform :)

The issue I have is that when writing generic terraform modules that are re-used
by several terragrunt configurations, the names in the module are and should be generic.

However, we also like to define some variables such as account ids in a single place.
That's why we put them in a file at the top level of the tree (in vcs), but at the moment
I believe that there is no way of getting hold of those variables' values and they
have to be included the terraform.tfvars file invoked by terragrunt (or included in a file generated by another tool - which is what we are doing).

So let's assume that I have defined the following in global_vars.json:

{
  "service_account_id": "1234567890"
  "business_unit_1_account_id": "2345678901",
  "business_unit_2_account_id": "3456789012"
}

Le'st also assume we have a terraform module that requires 2 variables to be set:
source_account_id and target_account_id.

I would like a way for terragrunt to invoke the module and set the value of
source_account_id and target_account_id to the values of business_unit_1_account_id
and service_account_id for example.

So I propose adding an interpolation function get_tfvar_from_var_files(var_name) that walks
the files specifed in the -var-file argument, in the order they're specified
looking for a variable called var_name and returning its value.

If several variables with the same name are encountered, the value of the last variable is used.

If the variable is not found, an error is raised.

The terragrunt config would look like something like:

terragrunt = {

  include {
    path = "${find_in_parent_folders()}"
  }

  terraform = {
    source = "git::ssh://git@gitserver/my_cross_account_module.git"

    extra_arguments "custom_vars" {
      commands = ["${get_terraform_commands_that_need_vars()}"]

      arguments = [
        "-var-file=${get_tfvars_dir()}/../../../global_vars.json",
        "-var-file=${get_tfvars_dir()}/../../level2.json",
        "-var-file=${get_tfvars_dir()}/../level3.json",
        "-var-file=terraform.tfvars",
        "-var", "source_account_id=${get_tfvar_from_var_files("business_unit_1_account_id")}",
        "-var", "target_account_id=${get_tfvar_from_var_files("service_account_id")}",
      ]
    }
  }
}

another_var = "my_value"

To set variable source_account_id, terragrunt would walk through files ../../../global_vars.json,
../../level2.json and ../level3.json looking for variable business_unit_1_account_id.
It would find it in ../../../global_vars.json and set its value to 2345678901

This would allow for another configuration based on that module to be deployed easily,
by amending the following lines:

        "-var", "source_account_id=${get_tfvar_from_var_files("business_unit_2_account_id")}",
        "-var", "target_account_id=${get_tfvar_from_var_files("service_account_id")}",

If the ids for the account change, they can be changed in a single place and the stacks
reapplied.

@brikis98 brikis98 added the enhancement New feature or request label Nov 11, 2018
@brikis98
Copy link
Member

Could you not create a business_unit_1.tfvars and business_unit_2.tfvars and include the corresponding one with extra_arguments and required_var_files?

@marionettist
Copy link
Author

@brikis98 I don't think that having business_unit_1.tfvars and business_unit_2.tfvars files would quite achieve the level of DRYness for the config I'm aiming for.

In particular, if the terraform modules don't follow the same naming conventions (for example, if they are open source modules written by different owners), e.g. if a set of modules expects a variable called service_account and another set needs a variable representing the same entity but is called svc_accnt, both variables would have to be defined with the same value. With the approach I'm proposing, a variable with an assigned value would be defined once and mapped locally in the -var stanza.

An other example of where I can't see what you're proposing achieving what I'm aiming for is the case of a layered architecture. For example, a module might have the concept of a client and service account id. And another module might have the same concept. But the actual config for the client id in the 2nd module is the same account as the service account id for the first module. client_account_id and service_account_id are in my opinion local names (within a module) that are not specific enough to be included in a more global tfvars file. Without a way of mapping variables, making variables names specific in the global vars file would force the modules being written to use those specific names, which would make them not very generic at best and would rule out using open source modules where variable names are already defined.

If you think I'm missing somethig in the solution you're proposing, then by all means please clarify. I'd love to find a better way than what I'm currently doing, which is to achieve this mapping in a little python script and including the generated tfvars file in the terragrunt config. I think that if it's somehting that other users would like to use, it would be best handled by terragrunt itself.

If you're happy it's worthy enhancement, I might be able to submit a PR for this.

@brikis98
Copy link
Member

Thanks for explaining the use case more! It makes sense, and I agree it can't be handled via extra_arguments and required_var_files alone. A PR for reading .tfvars files and parsing values out of them would be very welcome!

@marionettist
Copy link
Author

@brikis98 I've created a test repo with terraform files that I think describe the use cases we want this new interpolation function to cover.
I think that use case 1: get_tfvar_from_var_files is what we discussed.

use case 2: get_tfvar_from_var_files_with_include_1 and use case 3: get_tfvar_from_var_files_with_include_2 we didn't explicitly talk about, but I think it feels natural to read mapped values from parent terraform.tfvars files.

Finally, use case 4: get_tfvar_from_var_files_with_include_3 is similar to use case 1, but I wanted to double-check that you would expect that var files defined with -var-file in parent terraform.tfvars files would also be parsed.

Please cast your eyes on those use cases to make sure we're on the same page. Thanks!

@brikis98
Copy link
Member

I think that use case 1: get_tfvar_from_var_files is what we discussed.

This example contains:

arguments = [
	"-var-file=${get_tfvars_dir()}/../env1.tfvars",
	"-var", "foo=${get_tfvar_from_var_files("bar")}"  # should set foo to "bar_value"
]

How does get_tfvar_from_var_files("bar") know which file bar is defined in? I think this is the central part of the behavior that needs to be defined and what you're asking about. Some of the options include:

  1. Searching all .tfvar files passed in via -var-file.
  2. Searching the terraform.tfvars file and any included parent terraform.tfvars.
  3. Searching every .tfvars file in the current and parent dirs.
  4. Explicitly specifying the file when calling the function: get_tfvar_from_var_files("../foo.tfvars", "bar")

use case 3: get_tfvar_from_var_files_with_include_2

Terragrunt currently only supports one level of includes... This seems to be doing two?

use case 3: get_tfvar_from_var_files_with_include_2

Not sure I follow this one.

@marionettist
Copy link
Author

First of all regarding use case 3 and several levels of include, it isn't something I did before (I guess obviously now, since it isn't supported). I thought it was supported so was trying to be comprehensive regarding the use cases. So let's forget about that one.

So back to use case 1 and the options you have identified...

  1. If you mean searching the dirs for .tfvars file without regards to whether or not they are included in the configuration with -var-file I don't think that would work: I think there will be edge cases where the wrong .tfvars file would provide the value for the variable, e.g. if there are .tfvars files which are region specific and the files are just loaded in disk order

  2. I personally don't like that option much. I think it is not flexible enough. It probably just about solves the problem I have, but not in an elegant way by the caller having to explicitly understand where the variable is going to be defined

  1. is what I had initially in mind. But as I started thinking about the implementation, more questions arose. For example:
    a) should only the var files defined by -var-file=... in the extra_arguments block where get_tfvar_from_var_files is called be parsed? That actually led to the following question:
    b) should the function also parse the -var-file stanzas included in the other extra_arguments blocks? Now thinking about it, I think it should. As I was creating some terraform code for that use case, I realised that for the simple example I was putting together, what I had produced was something that was not the most natural/intuitive code that could be written. That lead me to another question:
    c) should the function also parse the terraform.tfvars file that is included from a child file? I think it should

So, to summarize, I think that what we should go for is a blend of option 1 and 2 where both -var-filess and parent terraform.tfvars file are parsed. All .tfvars files specified with -var-fileshould be parsed, whether specified in the current extra_arguments block (where get_tfvar_from_var_file is called from) or in any parent extra_arguments block.

Having had a look at the code, I was planning on achieving this by doing a two-pass interpolation of the terragrunt configuration.
The first pass would basically do what the current interpolation does and is required to resolve all the -var-file interpolations and the location of the parent terraform.tfvars file.
The first pass would produce an (ordered) list of .tfvars file.
The second pass would use that list as the list of files to parse for the interpolation.
I think doing it that way would make it easy to debug mapping issue (the issue of the type "where did this value come from?") because the terragrunt output shows the list of .tfvars files included when calling terraform and the interpolation function would parse the files in the same order.

@brikis98
Copy link
Member

brikis98 commented Nov 17, 2018

Hm, having thought about it, it seems a bit weird to parse these values from .tfvars files specified via extra_arguments... For example, if all you wanted to do was read a single variable from some .tfvars file, you now have to:

  1. Add a -var-file argument that pulls in the entire .tfvars file, including variables that you might not want from it.
  2. Add the get_tfvar_from_var_files("foo") call to read the one variable you do want.

Moreover, later, when maintaining this code, you won't be able to tell where a variable comes from simply by looking at the get_tfvar_from_var_files("foo") call; you'll have to manually search every .tfvars file for the variable foo.

It seems more straightforward to be explicit about this and just require the user to tell us what .tfvars file to read: get_tfvar_from_var_file("../foo.tfvars", "foo").

@marionettist
Copy link
Author

You might be right. I quite liked (and still do) the thought of being able to specify the var files in one place (a parent terraform.tfvars file) instead of having to specify locations of .tfvars files in the various calls to get_tfvar_from_var_file, but I'm also concerned about how terraform 0.12 is going to react to variables defined with a value in a .tfvars file when there is no corresponding variable declaration in a .tf file. I have a feeling the new version of terraform will throw an error...

So ok, let's go for your solution of providing a get_tfvar_from_var_file function. Regarding the parameters order, I find ("../foo.tfvars", "foo") a bit counter-intuitive and would prefer the parameters the other way round, unless the order you specified is more consistent with the other functions supported by terragrunt.

@bernardoVale
Copy link
Contributor

@marionettist, this is not what you want, but it might help you to implement what you want in a different way: #670

@brikis98
Copy link
Member

You can now use read_terragrunt_config.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants