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

Fold "extra_arguments" into terragrunt when using "include" to retrieve remote TF configurations #147

Closed
rkr-kununu opened this issue Feb 24, 2017 · 15 comments
Labels
enhancement New feature or request

Comments

@rkr-kununu
Copy link

So, I've got the following setup:

  • /terraform.tfvars
    • /preview/terraform.tfvars

The contents of /terraform.tfvars is:

terragrunt = {
  lock { ... }
  remote_state { ... }
  terraform {
    extra_arguments "global" {
      arguments = [ "-var", "account_id=12345" ]
    }
  }
}

The contents of /preview/terraform.tfvars is:

terragrunt = {
  include {
    path = "${find_in_parent_folders()}"
  }
  terraform {
    source = "git::git@github.com:foo/bar.git//app?ref=v0.0.1"
  }
}

The idea being that in app I would have access to var.account_id. This is of course not possible (currently) because the terraform source-block will override the terraform extra_arguments-block.

The solution described in #143 could be applied (put my configuration into a separate tfvars), but it would mean I'd need to cut-and-paste the extra_arguments blocks into /preview/terraform.tfvars (and future staging environments).

@brikis98 brikis98 added enhancement New feature or request help wanted labels Feb 24, 2017
@brikis98
Copy link
Member

Ah, yea, that's not ideal.

I think the way we are merging keys from include is fairly coarse grained, where we simply override top-level keys: lock, remote_state, terraform. We could merge the values together, but that will leave other holes, as then there would be no way to unset a value...

I'm open to ideas here.

One thing I've been considering is a new interpolation function:

read_var_from_file(VAR, FILE)

The idea is that it returns the value of a variable VAR read from .tfvars file FILE. It could replace most use cases for include, as you could just explicitly pull in the values you want, but also allow much finer grained control:

terragrunt = {
  lock = "${read_var_from_file("terragrunt.lock", find_in_parent_folders())}"
  remote_state = "${read_var_from_file("terragrunt.remote_state", find_in_parent_folders())}"
  terraform {
    source = "git::git@github.com:foo/bar.git//app?ref=v0.0.1"
    extra_arguments {
      global = "${read_var_from_file("terragrunt.terraform.extra_arguments", find_in_parent_folders())}"
    }
  }
}

This may also solve #132, as it will allow you to reuse values of arbitrary variables (not just Terragrunt ones) from your .tfvars files:

aws_region = "us-east-1"

terragrunt = {
  remote_state {
    backend = "s3"
    config {
      encrypt = "true"
      bucket = "my-bucket"
      key = "terraform.tfstate"
      region = "${read_var_from_file("aws_region", self())}"
    }    
  }
}

@andrewrynhard
Copy link

This is something I would like to see as well. If we could come to an agreement on a solution, I could put some work into making a PR.

@brikis98
Copy link
Member

@andrewrynhard My best guess for a solution is the one in the comment above yours: a read_var_from_file helper. On a related note, in order for path lookups to work well, we should also add support for a path.module style helper as described here.

I want to work on both of these, but have been utterly buried recently, and just haven't been able to find the time, so I'd be grateful for PRs!

@andrewrynhard
Copy link

Great, I will take a look and see what I can get done this week.

@shaharmor
Copy link

This sounds like a very needed feature for reducing duplication. Any update on this?

@brikis98
Copy link
Member

Nothing on my end. I submitted a flurry of PRs recently to update Terragrunt to support Terraform 0.9 and add a few other critical features, but have not had time for this one. @andrewrynhard, have you had a chance to take a look?

@andrewrynhard
Copy link

I have not had a chance yet. Hopefully soon.

@askainet
Copy link

A deep merge of the terraform key would be really nice to solve this!

@jocgir
Copy link
Contributor

jocgir commented May 25, 2017

On our fork, we made a lot of enhancements including:

  • Support of ${var.varname}
  • Deep merge of terragrunt/terraform config
  • Support for nested configurations (more than one level of include)
  • Add a criteria to determine uniqueness of a configuration by using separate temporary folder for each configuration (in our case, it is based on region/env)
  • Include of remote configuration to easy share our configuration between our multiple teams
  • Importing files from external source in the current temporary folder
  • Pre and Post hooks that are executed during the terragrunt process
  • Auto-discovery of configuration through some magic with tags and security groups
  • More flexible logging (using go-logging)
  • Possibility to ignore some folder in -all operations
  • Clean output on -all operations

Ultimately, our team only have to include that config file in their terraform folder and the rest (discovery of environment, region, providers, S3 backend, keys, extra_args for variables, locking, etc.) is magic:

project = "projectname"

terragrunt = {
    include {
        source = "${discover("bootstrap", var.env, "default")}"
    }
}

But having a new feature is one thing. Make a good PR from it with complete documentation and tests is another thing. We are trying to find time to package our changes in complete PRs to make them available to the root project.

@brikis98
Copy link
Member

@jocgir Wow, that's fantastic 👍

We'd love to hear more about each of these and get many of them into terragrunt itself. Please let me know if there's anything we can do to make that easier! Of course, we should discuss each item to make sure it's a good fit for the general Terragrunt community before you do the work to submit a PR, but I suspect a lot of these will be useful for a lot of people!

@philsttr
Copy link
Contributor

I like the read_var_from_file interpolation function as a solution for the problem originally posted by @rkr-kununu

I would use it in the parent tfvars file's terragrunt.terraform.source to look up a source variable from the child tfvars file. This would allow me to put everything in the parent tfvars file.

I also see lots of other use cases for a read_var_from_file interpolation function.
Any chance this will be implemented soon?

I also like the ideas done/proposed by @jocgir , but I would hate to hold up implementation of a simple function while waiting on that much larger work.

philsttr added a commit to philsttr/terragrunt that referenced this issue Jun 16, 2017
…rguments

Copied @jocgir's base implementation from https://github.com/coveo/terragrunt

Added documentation and unit tests.
@philsttr
Copy link
Contributor

After investigating adding the read_var_from_file option, I found that it was going to be pretty complex since the interpolation syntax and associated regexes were going to need modifying to support functions within functions (in order to support things like ${read_var_from_file("terragrunt.lock", find_in_parent_folders())}).

I then looked at @jocgir's deep merging approach for extra_arguments, and found it to be quite simple and elegant. (Nice work @jocgir!) So I have created PR #235 by isolating that change, and providing documentation and unit tests.

BTW, you can "unset" an extra_arguments block by creating an empty extra_arguments block in the child with the same name as a parent. Fairly intuitive, I think.

@philsttr
Copy link
Contributor

I believe PR #235 fixes @rkr-kununu's original request. So this can probably be closed.

Separate individual issues could be opened for the items implemented by @jocgir

@brikis98
Copy link
Member

@philsttr Good point. New issues for the other items discussed in this thread are more than welcome.

@geekifier
Copy link

@jocgir are there any plans to work towards some PRs from your fork?

I found that your logging system is a major improvement, however, diff-ing your codebase against the current master produces an overwhelming set of changes.

Are there pieces that could be incorporated with relative ease given some community involvement?

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

8 participants