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

Upgrades to work with Terraform v0.12.20 #12

Closed
wants to merge 13 commits into from

Conversation

Gowiem
Copy link
Member

@Gowiem Gowiem commented Feb 27, 2020

Info

Following up on #10, this PR uses that fork and then applies the most recent requirement: Updates the type for parameter_write variable so terraform can run without errors.

joshmyers and others added 2 commits August 15, 2019 11:31
Run `terraform 0.12upgrade` against codebase. Note that due to Terraform
0.12 now using sets, we call `tolist` to workaround [1]

[1] hashicorp/terraform-provider-aws#7522
@maximmi maximmi mentioned this pull request Feb 28, 2020
Copy link
Contributor

@maximmi maximmi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @Gowiem for contrubution!
This repo is in our queue to convert to TF 0.12. It is not enough to just replace syntax but we have requirements to cover module with tests. If you want to contribute it in a proper way (see example) you are most welcome! Give it a try, then ping me, so I could review and run tests. This will help us and speed up the process. Otherwise we all have to wait until it will be done as well as many other modules in queue

@Gowiem
Copy link
Member Author

Gowiem commented Mar 14, 2020

@maximmi I'm giving this a shot as I'd like to give back to you folks for these awesome modules (even if this one is one of the simplest out there), but I didn't get too far. Honestly, I think my lack of Make + Go knowledge make this an uphill battle, but I'm happy to bang at it if you or @osterman are willing to provide some guidance. 😅

Current status, is that the example works when running locally, but I'm unable to get very far due to the following:

  1. I installed bats since that seemed like an unmentioned prereq. I then ran cd tests/ && make which gave me a number of "not ok" values:

Running tests in  ../
1..10
ok 1 check if terraform is installed
not ok 2 check if terraform code needs formatting
ok 3 check if terraform modules are valid
ok 4 check if terraform modules are properly pinned
ok 5 check if terraform plugins are valid

Test: check if terraform providers are properly pinned
File: provider-pinning.bats
---------------------------------
* provider.aws: version = "~> 2.53"
---------------------------------

not ok 6 check if terraform providers are properly pinned
# (in test file test/.test-harness/test/terraform/provider-pinning.bats, line 17)
#   `[ $status -ne 0 ]' failed
not ok 7 check if terraform code is valid
# (in test file test/.test-harness/test/terraform/validate.bats, line 19)
#   `[ $status -eq 0 ]' failed
#
# Initializing provider plugins...
# - Checking for available provider plugins...
# - Downloading plugin for provider "aws" (hashicorp/aws) 2.53.0...
#
# The following providers do not have any version constraints in configuration,
# so the latest version was installed.
#
# To prevent automatic upgrades to new major versions that may contain breaking
# changes, it is recommended to add version = "..." constraints to the
# corresponding provider blocks in configuration, with the constraint strings
# suggested below.
#
# * provider.aws: version = "~> 2.53"
#
# Terraform has been successfully initialized!
#
# You may now begin working with Terraform. Try running "terraform plan" to see
# any changes that are required for your infrastructure. All Terraform commands
# should now work.
#
# If you ever set or change modules or backend configuration for Terraform,
# rerun this command to reinitialize your working directory. If you forget, other
# commands will detect it and remind you to do so if necessary.
not ok 8 check if terraform-docs is installed
# (in test file test/.test-harness/test/terraform/terraform-docs.bats, line 5)
#   `which terraform-docs' failed
not ok 9 check if terraform inputs have descriptions
not ok 10 check if terraform outputs have descriptions
make: *** [module] Error 1

The above led me to running the following and I got the following errors:

  • make terraform/validate:
Error: Error parsing /Users/gowiem/Workspace/Cloned/terraform-aws-ssm-parameter-store/main.tf: At 2:11: Unknown token: 2:11 IDENT var.enabled
  • make terraform/lint:
xargs: illegal option -- -
usage: xargs [-0opt] [-E eofstr] [-I replstr [-R replacements]] [-J replstr]
             [-L number] [-n number [-x]] [-P maxprocs] [-s size]
             [utility [argument ...]]
Error running fmt: In examples/complete/.terraform/modules/kms_key/examples/complete/main.tf: At 2:12: Unknown token: 2:12 IDENT var.region

Stopped there as something seems off. Is it possibly trying to lint / validate using v0.11 Terraform instead of v0.12?

  1. I also attempted the following and am getting the corresponding error:

cd tests/src && make
Error:

make: *** No rule to make target `/usr/bin/go', needed by `/Users/gowiem/Workspace/Cloned/terraform-aws-ssm-parameter-store/test/src/.gopath/src/terraform-aws-ssm-parameter-store'.  Stop.

Anyway, I'm stopping here as I'm a little out of my depth on your folks setup, but if it seems I'm just missing some key steps then please point me in a direction and I'll work to give it another shot. Thanks folks!

@deadlysyn
Copy link

@Gowiem this is your PR/hard work so i'm just going to share some learnings...

bats

aside from ensuring bats was installed, i:

  • installed terraform-docs
  • ran terraform fmt
  • updated main.cf with (pin providers):
# main.tf
terraform {
  required_version = "~> 0.12"

  required_providers {
    aws = "~> 2.58"
  }
}

provider "aws" {
  region = var.region
}
  • added region var as part of that:
# variables.tf
variable "region" {
  type        = string
  description = "AWS region to target"
  default     = ""
}
  • rm /tmp/terraform-modules-XXXXXXXXXXX.txt (module-pinning.bats leaves it behind)

that got bats happy:

make all
Running tests in  ../
1..10
ok 1 check if terraform is installed
ok 2 check if terraform code needs formatting
ok 3 check if terraform modules are valid
ok 4 check if terraform modules are properly pinned
ok 5 check if terraform plugins are valid
ok 6 check if terraform providers are properly pinned
ok 7 check if terraform code is valid
ok 8 check if terraform-docs is installed
ok 9 check if terraform inputs have descriptions
ok 10 check if terraform outputs have descriptions
Running tests in  ../examples/complete
1..5
ok 1 check if terraform is installed
ok 2 check if terraform code needs formatting
ok 3 check if terraform modules are valid
ok 4 check if terraform plugins are valid
ok 5 check if terraform code is valid

terratest

from the looks of the Makefile it's intended to run in an Alpine Dockerfile in a CI/CD pipeline,
so my hacks were likely better solved another way...but to get it running i:

  • made sure dep was installed
  • linked it into GOBIN
  • specified GOEXE when running the test (GOEXE=/usr/local/bin/go make)
  • updated example to pass in region since module has a var for it now:
# examples/complete/main.tf
module "store" {
  source = "../../"

  region = "us-east-1" # added
  • made sure all my AWS env vars were set for test to find credentials
  • changed region in examples/complete/provider.tf to us-east-1 (got a region error as-is, should this be a var somehow?)
  • updated examples/complete/maini.tf kms_key.alias (i already had one, should that be a random string in tests?)

then it was all go test related, you gave me a good start. here's what i ended up with:

package test

import (
	"strings"
	"testing"

	"github.com/gruntwork-io/terratest/modules/terraform"
	"github.com/stretchr/testify/assert"
)

// Test the Terraform module in examples/complete using Terratest.
func TestExamplesComplete(t *testing.T) {
	t.Parallel()

	terraformOptions := &terraform.Options{
		// The path to where our Terraform code is located
		TerraformDir: "../../examples/complete",
		Upgrade:      true,
	}

	// At the end of the test, run `terraform destroy` to clean up any resources that were created
	defer terraform.Destroy(t, terraformOptions)

	// This will run `terraform init` and `terraform apply` and fail the test if there are any errors
	terraform.InitAndApply(t, terraformOptions)

	// Run `terraform output` to get the value of an output variable
	output := terraform.Output(t, terraformOptions, "map")
	result := strings.Contains(output, "Amazon")
	// Verify we're getting back the outputs we expect
	assert.Equal(t, true, result)
}

that got terratest happy:

--- PASS: TestExamplesComplete (57.94s)
PASS
ok      terraform-aws-ssm-parameter-store       59.188s

observation

this was a fun learning experience, and i'm not sure how far we should stray from the intent of the PR with this update... i do have some concern over putting passwords/secrets (even examples) in source control. it might be worth another PR to re-work this (and anything like it) to inject secrets via the environment (12F/HashiCorp best practice).

hth, let me know if you have any questions!

@jamengual
Copy link

well if you guys are doing all this, then I guess I should try to add the KMS + s3 options too, if you are ok with that @Gowiem

@Gowiem
Copy link
Member Author

Gowiem commented Apr 24, 2020

@maximmi This should be ready for 👀 . I got everything running locally and all bats + terratests pass.

@deadlysyn Thanks for the pointers! Some of them definitely helped, but I also had some weird environment issues going on which were holding me back. Primarily:

  1. I installed the original bats initially, which is massively out-of-date. This led me astray a bit.
  2. I also was using Mac OSX's default Bash which is v3.2.57 and that was causing havoc once I finally did get onto bats-core. Upgrading to bats v5 got me fixed there.

As part of trying to get this working I ran into some other issues with the CP build-harness and test-harness. Put up PRs for those issues:

  1. Fixes #207, supplying AWS_DEFAULT_REGION fixes v0.12 tf validate build-harness#208
  2. Fixes #149, tf/lint on mac xargs does not support --no-run-if-empty build-harness#206
  3. Fix terraform validate test test-harness#11

Haha it has been a fun few hours 😅

@jamengual, not sure what you're referring to regarding KMS + S3 option (would love a link), but go for it!

@Gowiem Gowiem requested a review from maximmi April 24, 2020 21:19
@maximmi
Copy link
Contributor

maximmi commented Apr 24, 2020

@Gowiem thank you for your contribution! Great job! I will take a look on this closely next week

@maximmi maximmi self-assigned this Apr 24, 2020
@aknysh
Copy link
Member

aknysh commented May 7, 2020

/codefresh run test

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

Successfully merging this pull request may close these issues.

6 participants