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

Upgrade to tf 12 syntax; add terratest plumbing #34

Merged
merged 2 commits into from Nov 1, 2019

Conversation

@jsclarridge
Copy link
Contributor

jsclarridge commented Nov 1, 2019

Per https://www.pivotaltracker.com/story/show/169407344, update the module to use Terraform 12 syntax and introduce Terratest into the module. The PR also includes:

  • Makefile - For running pre-commit and Terratest
  • golangci-lint - Go linting
  • dependabot - To keep go modules up to date
  • README - Updates for 0.12 versioning and how to run tests

The Terratest example lives in examples/simple, which instantiates this module with the default settings. Running against our CI AWS account returns

--- PASS: TestTerraformAwsLogs (41.80s)
PASS
ok github.com/trussworks/terraform-aws-logs/test 41.815s

@@ -1 +0,0 @@
0.11.14

This comment has been minimized.

Copy link
@chrisgilmerproj

chrisgilmerproj Nov 1, 2019

Contributor

Would it be better to update this to 0.12.12 or 0.12 or is the thinking that we shouldn't preference a version for tfenv in our repos?

This comment has been minimized.

Copy link
@jsclarridge

jsclarridge Nov 1, 2019

Author Contributor

As we've migrated some of the other modules to TF 12, we've added a versions.tf file (also in this PR) which constrains the TF version. I think that makes sense as a more tool-agnostic approach. tfenv and specific .terraform-version files can still be managed at the project level.

This comment has been minimized.

Copy link
@chrisgilmerproj

chrisgilmerproj Nov 1, 2019

Contributor

That's fair. I've only found it useful when using terraform 0.12upgrade in this repo. But generally I do work from other repos. So I guess its up to you.


Terraform 0.12. Pin module version to ~> 4.x Submit pull-requests to master branch.

Terraform 0.11. Pin module version to ~> 3.5.0. Submit pull-requests to terraform011 branch.

This comment has been minimized.

Copy link
@chrisgilmerproj

chrisgilmerproj Nov 1, 2019

Contributor

For some reason I thought 3.5.0 was already tf12. But I'm ok with this.

This comment has been minimized.

Copy link
@jsclarridge

jsclarridge Nov 1, 2019

Author Contributor

The module currently supports both. v.3.4.0 added support for tf 11 again.

Copy link
Contributor

chrisgilmerproj left a comment

🚀 - LGTM but I only reviewed the TF stuff, not the terratest.

region = var.region
bucket_arn = format("arn:aws:s3:::%s", var.s3_bucket_name)
alb_principal = data.aws_elb_service_account.main.arn
alb_effect = var.default_allow || var.allow_alb ? "Allow" : "Deny"

This comment has been minimized.

Copy link
@dynamike

dynamike Nov 1, 2019

Contributor

Fuck me this is complicated. But the formatting is better :)

@jsclarridge jsclarridge merged commit b590bc2 into master Nov 1, 2019
1 check passed
1 check passed
ci/circleci: validate Your tests passed on CircleCI!
Details
@jsclarridge jsclarridge deleted the 169407344-tf12-upgrade branch Nov 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants
You can’t perform that action at this time.