Terraform 0.12 support #731

brikis98 · 2019-06-06T17:20:20Z

This PR upgrades Terragrunt to work with Terraform 0.12. Key changes:

We are migrating from having the Terragrunt configuration in terraform.tfvars to terragrunt.hcl. This is necessary because Terraform 0.12 is more strict about what can be in terraform.tfvars files, and the terragrunt = { ... } configuration block we used to work is no longer allowed.
The configuration in terragrunt.hcl is largely the same as what we used to have in terraform.tfvars, except (a) we no longer need the terragrunt = { ... } wrapper, (b) we now use HCL2 syntax, (c) any input variables that need to be passed to the Terraform module need to be moved into an inputs = { ... } block.
HCL2 brings two major benefits: (a) first-class expressions, so not everything has to be wrapped in ${...} and (b) a better parser, so we can now use Terragrunt's built-in functions anywhere in terragrunt.hcl.

PR progress:

Note, this PR looks huge, but the vast majority of it is renaming all the terraform.tfvars files to terragrunt.hcl and updating to 0.12 syntax with first-class expressions.

Fixes #466.

yorinasub17

I reviewed roughly half of this so far and ran out of stream. Left a few nits, but so far looks great! You can see the marker on how far I got.

yorinasub17 · 2019-06-06T23:57:15Z

README.md

+TF_VAR_instance_count=10 \
+TF_VAR_tags='{"Name":"example-app"}' \
+terraform apply
+```


What happens if you already have TF_VAR_instance_type="t2.large" set, with the intention of overriding what is in the inputs?

Oh, good point! I've updated the code to explicitly not override any env vars the user has already set, and added tests for the same:

fdce37c
993e195

_docs/migration_guides/upgrading_to_terragrunt_0.19.x.md

yorinasub17 · 2019-06-07T00:02:38Z

_docs/migration_guides/upgrading_to_terragrunt_0.19.x.md

+}
+
+# This now works with Terragrunt 0.19.x and newer!
+foo = get_env("FOO", "default")


Should this be wrapped in inputs?

Yes, fixed! 47391c6

yorinasub17 · 2019-06-07T00:06:25Z

cli/cli_app.go

+
+	for key, value := range asEnvVars {
+		terragruntOptions.Env[key] = value
+	}


So this means we can't set the env var TF_VAR_xxx outside of terragrunt to override the inputs?

Thanks for the great catch. Fixed and added tests:

fdce37c
993e195

config/config.go

yorinasub17 · 2019-06-07T00:16:42Z

config/config_helpers_test.go

@@ -560,120 +313,57 @@ func TestResolveEnvInterpolationConfigString(t *testing.T) {
 		include           *IncludeConfig
 		terragruntOptions *options.TerragruntOptions
 		expectedOut       string
-		expectedErr       error
+		expectedErr       string


Just curious: Why did you switch to checking the string?

Before, the error was returned directly from my code, so I could check it exactly. Now, the error is wrapped by the HCL2 parser, so doing an exactly equality check is harder. Therefore, I'm just doing a string contains check for the error name.

yorinasub17 · 2019-06-07T00:17:27Z

config/config_helpers_test.go

-			assert.Nil(t, actualErr, "For string '%s' include %v and options %v, unexpected error: %v", testCase.str, testCase.include, testCase.terragruntOptions, actualErr)
-			assert.Equal(t, testCase.expectedOut, actualOut, "For string '%s' include %v and options %v", testCase.str, testCase.include, testCase.terragruntOptions)
-		}
+		testCase := testCase


NIT: We usually add the boilerplate comment about this being done to bring the var into the scope of the forloop.

Fixed: 9f19fd9

lorengordon · 2019-06-07T10:30:30Z

_docs/migration_guides/upgrading_to_terragrunt_0.19.x.md

+}
+
+# Input variables to set for your Terraform module
+inputs = {


The block vs attr distinction is a bit confusing, but either hcl2 or terraform 0.12 impose some validation on this distinction. Just wanting to clarify here that terraform and remote_state truly are blocks, where inputs truly is an attr. Meaning this would not work (with the = denoting the attr notation):

terraform = { # ... } remote_state = { # ... }

and notionally, because those two are actually blocks, you could have several of them in the same config, somehow?

I found the HCL2 parser was a bit weird with these. To be able to define the attributes within these terraform and remote_state, I had to make them blocks... They really should only appear once, so I wanted to make them attributes, but I got confusing errors when I did that. Maybe attributes can't have well-defined properties within? Not sure.

Yeah, I'm frustrated with this change in hcl2 at the moment. Not sure if you've seen it already, but there is a property you can set on the schema to treat a block as an attr. The many, many PRs linked to this issue might point to a way forward to use attrs?

Here is an example: https://github.com/terraform-providers/terraform-provider-aws/pull/8228/files

Also just found the behavior better described here, https://www.terraform.io/docs/extend/terraform-0.12-compatibility.html#computed-resource-attributes

lorengordon · 2019-06-07T10:35:13Z

_docs/migration_guides/upgrading_to_terragrunt_0.19.x.md

+```hcl
+# terragrunt.hcl
+
+# terraform is a block, so make sure NOT to include an equals sign


Groan. I see you address this here.

brikis98 · 2019-06-08T14:49:34Z

Just added a bunch of fixes:

Switch to using go-getter directly instead of terraform init -from-module. The latter seems to be poorly maintained, as it had some unfortunate bugs in 0.11 (terraform init -from-module does not work with -get=false hashicorp/terraform#18460) and now in 0.12, it flat out crashes/panics.
Since I'm no longer running terraform init twice, I had to backport the hooks functionality so you could still execute hooks before/after the usage of go-getter.
Terraform 0.12 stores state differently in .tfstate files (all possible keys are now stored, unset ones defaulting to null), so I had to fix the state equality check too.

brikis98 · 2019-06-08T14:58:18Z

Oh, one more fix:

Looks like with the --terragrunt-non-interactive, we were assuming "yes" for all prompts, including the question you get for xxx-all commands of whether the xxx command should be applied to "external dependencies" (those outside the working directory). So if you ran destroy-all in folder foo, it not only destroyed everything in folder foo, but also all external dependencies of anything in foo. That is not a safe default, so I have reversed that behavior. It was exposed by a test failure that, I guess, 0.11 was somehow covering up?

lorengordon · 2019-06-08T15:03:34Z

cli/download_source.go

+	terragruntOptions.Logger.Printf("Downloading Terraform configurations from %s into %s", terraformSource.CanonicalSourceURL, terraformSource.DownloadDir)
+
+	// go-getter will not download into folders that already exist, so initially, we download into a temp folder
+	tmpDownloadDir, err := ioutil.TempDir("", "terragrunt-download-temp")


Would it make sense to respect TERRAGRUNT_DOWNLOAD_DIR here? I'm thinking about issues on Windows with long filepaths, and a workaround of exporting something like TERRAGRUNT_DOWNLOAD_DIR = "C:\.terragrunt-cache". If we first download instead to TEMP, that makes the workaround ineffective...

Ah, that's a good point. Fixed in 65741db.

yorinasub17 · 2019-06-09T06:00:39Z

cli/download_source.go

+	// Clean up the tmp folder
+	if err := os.RemoveAll(tmpDownloadDir); err != nil {
+		return errors.WithStackTrace(err)
+	}


Why not use defer here?

Done: e239808

brikis98 · 2019-06-09T21:20:23Z

Alright, this PR is driving me bonkers with an endless yak shave. First the bugs with terraform init -from-module, which I worked around by moving to go-getter; then bugs with copying folder contents and concurrency; I worked around those, and now TestTerragruntStackCommands is failing with the error:

NoSuchBucket: The specified bucket does not exist

Digging into it, I think this is happening in the part of the code that automatically creates the S3 bucket. What's bewildering is that, as far as I can tell from the logs, the WaitUntilS3BucketExists is returning successfully, which means the S3 HeadBucket operation is saying that the bucket does exist. However, when the code moves on to enable versioning on the bucket, I get back the error the bucket doesn't exist. I even added a 5 second sleep between creation and enabling versioning, in case it was an eventual consistency issue, and it didn't help. WTF? 😕

lorengordon · 2019-06-10T00:11:12Z

Any chance it might be a credential issue? Maybe temporary keys expiring at an inopportune moment? Or perhaps credential in combination with region, where the bucket is created in one region but then the versioning logic is somehow referencing another region?

brikis98 · 2019-06-10T10:11:51Z

Any chance it might be a credential issue? Maybe temporary keys expiring at an inopportune moment? Or perhaps credential in combination with region, where the bucket is created in one region but then the versioning logic is somehow referencing another region?

No, I don't think it's a credentials issue, as there are a bunch of tests running in parallel, all with the same credentials, and while all the tests before/during/after TestTerragruntStackCommands pass, this one test fails consistently.

brikis98 · 2019-06-10T10:51:29Z

And of course TestTerragruntStackCommands passes without issues on my own computer... It only fails in CI...

brikis98 · 2019-06-10T11:48:18Z

OK, I've put in a workaround that does the S3 bucket creation/configuration in a retry loop with some sleep between retries. Tests seem to be passing consistently now. Finally merging this!

brikis98 · 2019-06-10T11:52:22Z

This code is now available in: https://github.com/gruntwork-io/terragrunt/releases/tag/v0.19.0

barryib · 2019-06-11T14:02:46Z

@brikis98 awesome. Thanks for your great work. I tried this release, but I was wondering how do we have to handle var files (required and optional) in Terragrunt, because Terraform is now rising and deprecation warning for unknown variables. I opened the following issue #737 to track this

brikis98 added 13 commits June 6, 2019 16:13

Switch to HCL2 parser

42fa65c

Update all the test fixtures

6466cd5

Update all the tests

5222aab

Fix compile error and bool check

b42fd86

Fix configstack test

0e3ddc0

Fix dependencies syntax

2d35949

Fix a few more configstack tests

73bb229

Fix terraform block syntax

1bfa50f

Attr should be called inputs

77ce597

Remove missing references to terraform.tfvars

50cfb11

Fix handling of tags

ab495c9

Fix include syntax

e88e106

Set inputs as env vars

b2b7f4b

brikis98 requested review from autero1 and eak12913 as code owners June 6, 2019 17:20

brikis98 added 6 commits June 6, 2019 22:27

Run make fmt

0611d5a

Fix string handling. Add integration test case for input handling.

477ac27

Update docs

08402c1

Document inputs

1faf766

Rename get_tfvars_dir and get_parent_tfvars_dir

9fe446c

Add migration guide

722b6f2

brikis98 changed the title ~~WIP: Terraform 0.12 support~~ Terraform 0.12 support Jun 6, 2019

brikis98 added 4 commits June 7, 2019 00:45

Merge branch 'master' into tf12

ae9e4b1

Remove TF 0.12 note

02b0eb2

Update Terraform version check

78cb92f

Update to new circle image with Terraform 0.12 on it

9857bbb

yorinasub17 reviewed Jun 7, 2019

View reviewed changes

yorinasub17 mentioned this pull request Jun 7, 2019

Is there no support for explicitly defining access_key and secret_key in terragrunt backend configuration? #732

Open

lorengordon reviewed Jun 7, 2019

View reviewed changes

Fix configstack tests

2d79cde

lorengordon reviewed Jun 8, 2019

View reviewed changes

brikis98 added 3 commits June 9, 2019 00:05

Skip external dependencies test

8d8dcc7

Put tmp dir into configured download dir

65741db

Fix tmp dir location

b0ca9e4

yorinasub17 reviewed Jun 9, 2019

View reviewed changes

brikis98 added 9 commits June 9, 2019 13:34

Try CopyFolderContents implementation based on filepath.Glob

cc241e8

Exztract UniqueId helper

bb61c8d

Use UniqueId in temp folder creation. Defer temp folder deletion.

e239808

Try to add sleep on S3 bucket creation

58556e0

Increase sleep

68f6a1b

Replace filepath.Walk in tests

71b72e1

Fix dest dir in copyfoldercontents

f9c08ed

Create parent dir before copying

31fa15b

Remove deprecated S3 settings

866e689

Slightly clearer logging. Remove sleep as it doesn't help.

a531167

domcleal mentioned this pull request Jun 10, 2019

Terraform 0.15 support dependabot/dependabot-core#1176

Closed

brikis98 added 2 commits June 10, 2019 12:17

Do S3 bucket creation in retry loop

2aa4573

Skip symlinks

264812d

brikis98 merged commit 2fe4b90 into master Jun 10, 2019

brikis98 deleted the tf12 branch June 10, 2019 11:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Terraform 0.12 support #731

Terraform 0.12 support #731

brikis98 commented Jun 6, 2019 •

edited

Loading

yorinasub17 left a comment

yorinasub17 Jun 6, 2019

brikis98 Jun 7, 2019

yorinasub17 Jun 7, 2019

brikis98 Jun 7, 2019

yorinasub17 Jun 7, 2019

brikis98 Jun 7, 2019

yorinasub17 Jun 7, 2019

brikis98 Jun 7, 2019

yorinasub17 Jun 7, 2019

brikis98 Jun 7, 2019

lorengordon Jun 7, 2019 •

edited

Loading

brikis98 Jun 7, 2019

lorengordon Jun 8, 2019

lorengordon Jun 8, 2019

lorengordon Jun 8, 2019

lorengordon Jun 7, 2019

brikis98 commented Jun 8, 2019

brikis98 commented Jun 8, 2019

lorengordon Jun 8, 2019

brikis98 Jun 8, 2019

yorinasub17 Jun 9, 2019

brikis98 Jun 9, 2019

brikis98 commented Jun 9, 2019

lorengordon commented Jun 10, 2019

brikis98 commented Jun 10, 2019

brikis98 commented Jun 10, 2019

brikis98 commented Jun 10, 2019

brikis98 commented Jun 10, 2019

barryib commented Jun 11, 2019

Terraform 0.12 support #731

Terraform 0.12 support #731

Conversation

brikis98 commented Jun 6, 2019 • edited Loading

yorinasub17 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lorengordon Jun 7, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brikis98 commented Jun 8, 2019

brikis98 commented Jun 8, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brikis98 commented Jun 9, 2019

lorengordon commented Jun 10, 2019

brikis98 commented Jun 10, 2019

brikis98 commented Jun 10, 2019

brikis98 commented Jun 10, 2019

brikis98 commented Jun 10, 2019

barryib commented Jun 11, 2019

brikis98 commented Jun 6, 2019 •

edited

Loading

lorengordon Jun 7, 2019 •

edited

Loading