modules/gcp - GCP support #141

robmorgan · 2018-08-09T10:09:19Z

Hi Guys,

I'm contributing a module with initial support for GCP in Terratest.

Cheers,

Rob

New Features & Changes

Packer Module

Extend Packer template packer-basic-example to build both AWS & GCP resources.
Add BuildArtifact & BuildArtifactE
Modify Regex to work with GCE Packer logs: extractAMIID -> extractArtifactID.

The change to the Packer module is potentially controversial. I have deprecated the BuildAmi and BuildAmiE methods and added BuildArtifact and BuildArtifactE. I thought this makes more sense when working with multiple Packer builders. Thoughts?

Test Structure Module

SaveArtifactID: A possible replacement for SaveAmiId.
LoadArtifactID: A possible replacement for LoadAmiId.

GCP Module

Misc

Find a better way to work with the ENV vars/credentials.
GCP refers to Public IPs as External IPs. Should we change the methods accordingly?

compute.go

provider.go

GetGoogleCredentialsFromEnvVar
GetGoogleProjectIDFromEnvVar
GetGoogleRegionFromEnvVar
multiEnvSearch

region.go

region_test.go

storage.go

storage_test.go

Add a test that reads and writes an objects to a storage bucket
Look at removing env vars: GOOGLE_STORAGE_PROJECT_ID
TestCreateAndDestroyStorageBucket
TestAssertStorageBucketExistsNoFalseNegative
TestAssertStorageBucketExistsNoFalsePositive

Examples

terraform-gcp-example

Format TF code better and add comment blocks
Create compute resources

packer-basic-example

Add an additional builder for GCP

Tests

test/packer_basic_example_test.go should only build the AWS AMI.
test/packer_gcp_basic_example_test.go should only build the GCP Image.
test/packer_gcp_basic_example_test.go should bake using a random zone.
test/terraform_gcp_example_test.go should create and validate Compute resources.

…Google Storage.

…dules/gcp

brikis98

Wonderful, thanks Rob! The tests look great 👍

brikis98 · 2018-08-09T12:09:37Z

examples/terraform-gcp-example/main.tf

+  name     = "${var.bucket_name}"
+  location = "${var.location}"
+  project  = "${var.project}"
+}


The README mentions a google compute instance, but this just seems to create a storage bucket?

Also, do you need any configuration for the google provider?

Yeah, that's a good point. I think this example was originally added by @Dahs81. I'll either cleanup the README or even better should we actually add compute resources like terraform-aws-example, then add tests for those?

And yes we should definitely add a provider block just for clarity.

Right now I'm only setting the following ENV vars locally to test the GCP module:

GOOGLE_APPLICATION_CREDENTIALS

GOOGLE_CLOUD_PROJECT_ID

GOOGLE_STORAGE_PROJECT_ID

We might be able to consolidate that a bit better.

brikis98 · 2018-08-09T12:09:53Z

examples/terraform-gcp-example/variables.tf

+  default = "US"
+}
+
+variable "project" {}


Please add a description to all variables

Also, I noticed you are using a project variable in the Terraform code, but a GOOGLE_CLOUD_PROJECT_ID env var in the test code, which seems a bit redundant. Is there a convention around the best way to do this with GCP and Terraform?

brikis98 · 2018-08-09T12:12:43Z

modules/gcp/storage.go

+	it := bucket.Objects(ctx, nil)
+	if _, err := it.Next(); err == storage.ErrBucketNotExist {
+		return err
+	}


I haven't used GCP's SDK, but this looks like a really strange API for checking if a bucket exists! What does the bucket.Attrs call above check? Will bucket.Objects give you an error on empty buckets (as opposed to those that don't exist at all)?

Hi @brikis98, I spent a while digging through their Go & Java SDK, but it didn't turn up much luck. It appears some calls e.g: client.Bucket(name) create an object but don't actually do any activity across the network.

I stumbled on their own integration tests and adapted a bit of the code from there.

You might want to take a look: https://github.com/GoogleCloudPlatform/google-cloud-go/blob/de879f7be552d57556875b8aaa383bce9396cc8c/storage/integration_test.go#L1231

I thought it might be a tad easier and theres still the possibilty I'm on the wrong track.

I had the same reaction as @brikis98, though I agree the API here is confusing. Let's add a comment clarifying the uncertainty and linking back to the URL you posted above. If this passes tests, I think it's good enough for now.

@josh-padnick ok sure

brikis98 · 2018-08-09T12:14:28Z

modules/gcp/storage_test.go

+)
+
+var (
+	projectId = os.Getenv("GOOGLE_STORAGE_PROJECT_ID")


Is this env var a convention used with GCP? If so, you may want to add a helper function in Terratest called something like getGoogleProjectIdFromEnvVar and use it throughout these tests.

Not that I'm aware of.

I have a feeling @Dahs81 may have adapted it from the Google examples, see: https://cloud.google.com/storage/docs/reference/libraries#client-libraries-usage-go.

What would be the best way for us to handle PROJECT_ID for GCP in Terratest? I guess the AWS GO SDK just automatically reads ENV vars right?

One option is that you could create something similar to the ~/.aws directory, but for terratest. Maybe have a ~/.terratest/config.toml that looks something like this:

[google] project_id="somegoogleprojectidhere"

Then maybe have a terratest config command that configures it. Then check if that file exists. I'd probably use env variable as a failback though.

I'm not sure if that is too much for this feature. Just an idea.

@josh-padnick Could you chime on with thoughts on how to handle GCP env vars?

So far I've just thrown a few functions in a provider.go file: https://github.com/gruntwork-io/terratest/pull/141/files#diff-86e2fd38705590c638b44a550afeaab3

brikis98 · 2018-08-09T12:16:23Z

modules/gcp/storage_test.go

+
+	gsBucketName := "gruntwork-terratest-" + strings.ToLower(id)
+
+	CreateStorageBucket(t, projectId, gsBucketName, nil)


Perhaps write something to the bucket and read it back as a sanity check it actually exists? Otherwise, even if CreateStorageBucket and DeleteStorageBucket were no-ops, this test would still pass.

ok good point!

…ordingly

Dahs81 · 2018-08-09T20:36:08Z

Thanks, @robmorgan for taking over. I have been busy doing a bunch of k8s stuff and haven't had time to work on this. I appreciate your willingness to take over. Sorry for not getting too far with this.

robmorgan · 2018-08-10T10:25:42Z

@Dahs81 yeah sure, not a problem!

robmorgan · 2018-08-20T10:29:44Z

@josh-padnick Hi Josh,

Yes definitely, Jim mentioned this to me last week. I was going to try and break everything down into smaller PRs, but it looks like you've reviewed it all now.. so thanks!

No, I've replied to all of your feedback. Just work through the remaining issues and then merge it as it stands. I'm happy to implement new functionality in subsequent PRs.

josh-padnick · 2018-08-22T02:32:02Z

Alright, I think we're ready to merge! There were one or two open comments, so I'll wait for your ok to officially merge. Thanks again for this awesome work.

robmorgan · 2018-08-22T09:54:38Z

Hi @josh-padnick, I've made another two micro changes based on your feedback, but I'm happy with how it stands now. Ok to merge! 😎 👍

josh-padnick · 2018-08-23T06:14:14Z

'cc @infosecgithub on this significant update to Terratest which will be directly leveraged to add tests for the Consul and Vault modules for GCP.

brikis98 · 2018-08-28T01:08:32Z

@robmorgan The original merge of this PR failed because we hadn't yet set up our GCP creds in our CircleCI account. I've now done that and all the GCP tests pass except one:

--- FAIL: TestTerraformGcpExample (204.21s)
	terraform_gcp_example_test.go:68: 
			Error Trace:	terraform_gcp_example_test.go:68
			Error:      	Should be true
			Test:       	TestTerraformGcpExample
	terraform_gcp_example_test.go:69: 
			Error Trace:	terraform_gcp_example_test.go:69
			Error:      	Not equal: 
			            	expected: "testing-tag-value2"
			            	actual  : ""
			            	
			            	Diff:
			            	--- Expected
			            	+++ Actual
			            	@@ -1 +1 @@
			            	-testing-tag-value2
			            	+
			Test:       	TestTerraformGcpExample

Any idea why?

robmorgan · 2018-08-29T15:58:23Z

@brikis98 Hi Jim,

Just scanning through some of your commits - I believe you may need to create a new project under your Gruntwork GCP account and then add GOOGLE_CLOUD_PROJECT=foo-123 to the CircleCI build environment.

Here's another test that failed:

TestGetRandomRegionExcludesForbiddenRegions 2018-08-27T19:13:37Z region.go:66: Using region us-east4
--- FAIL: TestGetAllGcpRegions (0.17s)
TestGetRandomRegionExcludesForbiddenRegions 2018-08-27T19:13:37Z region.go:66: Using region asia-east1
TestGetRandomRegionExcludesForbiddenRegions 2018-08-27T19:13:37Z region.go:66: Using region europe-west3
TestGetRandomRegionExcludesForbiddenRegions 2018-08-27T19:13:37Z region.go:66: Using region europe-north1
	region.go:112: googleapi: got HTTP response code 404 with body: Not Found
--- FAIL: TestGetRandomZone (0.23s)
	region.go:76: googleapi: got HTTP response code 404 with body: Not Found
TestGetRandomRegionExcludesForbiddenRegions 2018-08-27T19:13:37Z region.go:66: Using region southamerica-east1
--- FAIL: TestGetAllGcpZones (0.24s)
	region.go:153: googleapi: got HTTP response code 404 with body: Not Found
--- PASS: TestAssertStorageBucketExistsNoFalsePositive (0.21s)
--- FAIL: TestAssertStorageBucketExistsNoFalseNegative (0.23s)
	storage.go:18: googleapi: Error 400: Unknown project id: , invalid
TestGetRandomRegionExcludesForbiddenRegions 2018-08-27T19:13:37Z region.go:66: Using region europe-west3
TestGetRandomRegionExcludesForbiddenRegions 2018-08-27T19:13:37Z region.go:66: Using region us-east1
--- FAIL: TestCreateAndDestroyStorageBucket (0.23s)
	storage.go:18: googleapi: Error 400: Unknown project id: , invalid
TestGetRandomRegionExcludesForbiddenRegions 2018-08-27T19:13:37Z region.go:66: Using region europe-north1
--- FAIL: TestGetRandomRegion (0.24s)
	region.go:38: googleapi: got HTTP response code 404 with body: Not Found

I can see it complaining about a project id. We could extend provider.go to throw a fatal error?

Your commit:

5d7c405

brikis98 · 2018-08-29T17:45:40Z

@robmorgan Yea, I fixed all of those by adding the necessary env vars. In the most recent build, all of those issues are fixed. The only remaining one is TestTerraformGcpExample.

robmorgan · 2018-08-29T20:25:13Z

@brikis98 hmmm looks like a good one this one. I haven't been able to reproduce locally yet:

TestTerraformGcpExample 2018-08-29T21:15:09+02:00 storage.go:190: Finding bucket terratest-gcp-example-qgs4z9
TestTerraformGcpExample 2018-08-29T21:15:11+02:00 compute.go:114: Adding labels to instance terratest-gcp-example-0orxlk in zone us-east1-b
TestTerraformGcpExample 2018-08-29T21:15:12+02:00 compute.go:61: Getting Labels for Compute Instance terratest-gcp-example-0orxlk

let me keep digging. I'll try and run it on my own Circle/GCP accounts tomorrow.

brikis98 · 2018-08-29T21:49:24Z

Could it be an eventual consistency thing? The tags are applied after a few seconds (sometimes), so it's timing dependent? If so, you may just need to use retry.DoWithRetry.

robmorgan · 2018-08-30T11:13:15Z

@brikis98 yeah I believe it probably is. From memory I found during development there was a lag for this operation, I'm going to go down the retry path and see if it makes a different. Should I essentially move my asserts to the retry func?

brikis98 · 2018-08-30T11:17:41Z

Yea. In the example automated tests, anything that is eventually consistent should be done in a retry loop.

Dahs81 and others added 7 commits June 9, 2018 00:27

Initial work on GCP module. Currently only has create and delete for …

263eb86

…Google Storage.

Merge branch 'develop' of https://github.com/Dahs81/terratest into mo…

aa75662

…dules/gcp

update doc

898402a

rework storage functions a bit. add new test methods

c5f85c2

run terraform fmt

6954181

remove blank line

19a498c

expand test to verify storage bucket exists

771b5f4

brikis98 reviewed Aug 9, 2018

View reviewed changes

robmorgan added 10 commits August 9, 2018 16:43

update dep constraints

39eef0d

add initial compute code. needs tests

bfaaf41

rework packer module to better support both aws & gcp

d42f026

only build the AWS AMI

b1b0b37

remove commented code

44b5232

improve docs

f0d4da5

add region code

3bd1aa9

add methods for dealing with instance labels

7d7d301

expand terraform-gcp-example to create compute instances and test acc…

f422807

…ordingly

fix typo in terraform-aws-example

6c3b34b

robmorgan added 6 commits August 10, 2018 11:24

add gcp provider stub

eb7310e

use scope url in sdk

cfeeeaa

add region and zone code

9993fe8

use correct zone format

bb1ec6b

add a provider for creds. can change later

1c3c996

use a random zone and the credentials provider

0e2f9c7

robmorgan added 4 commits August 10, 2018 12:48

fix default zone

53a561c

add more methods to work with cloud storage

2b3bfab

update comment

c6ef961

remove dead code

f966503

robmorgan added 11 commits August 20, 2018 11:03

update deprecation notices

af8fbbc

Use correct provider ;)

ccb466e

clarify code

a13a81e

remove duplicate config block

f60625c

remove the need for the project param

313d46d

Prefer GOOGLE_CLOUD_PROJECT instead.

f0cb5e2

clarify require env vars better

1c95d9a

remove unimplemented code

53b4791

update regex

c9d3ba6

check for the presence of an AccessConfig

95022ea

update README

0379efc

robmorgan mentioned this pull request Aug 20, 2018

Initial work on GCP module. #100

Closed

Merge branch 'master' into modules/gcp

acf70d8

robmorgan added 2 commits August 22, 2018 11:46

clarify constraint

6d6ede8

change var name

536fd95

josh-padnick merged commit d800338 into gruntwork-io:master Aug 23, 2018

robmorgan deleted the modules/gcp branch February 4, 2019 08:09

robmorgan mentioned this pull request May 14, 2019

Question: How do I add a terratest module for alicloud resurces used in terraform #296

Closed

ptux mentioned this pull request Aug 6, 2019

terratest - alicloud support ptux/mashtun#3

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

modules/gcp - GCP support #141

modules/gcp - GCP support #141

robmorgan commented Aug 9, 2018 •

edited

Loading

brikis98 left a comment

brikis98 Aug 9, 2018

robmorgan Aug 9, 2018

brikis98 Aug 9, 2018

brikis98 Aug 9, 2018

brikis98 Aug 9, 2018

robmorgan Aug 9, 2018

josh-padnick Aug 19, 2018

robmorgan Aug 20, 2018

brikis98 Aug 9, 2018

robmorgan Aug 9, 2018

Dahs81 Aug 9, 2018 •

edited

Loading

brikis98 Aug 9, 2018

robmorgan Aug 10, 2018

brikis98 Aug 9, 2018

robmorgan Aug 9, 2018

Dahs81 commented Aug 9, 2018

robmorgan commented Aug 10, 2018

robmorgan commented Aug 20, 2018

josh-padnick commented Aug 22, 2018

robmorgan commented Aug 22, 2018

josh-padnick commented Aug 23, 2018

brikis98 commented Aug 28, 2018

robmorgan commented Aug 29, 2018

brikis98 commented Aug 29, 2018

robmorgan commented Aug 29, 2018 •

edited

Loading

brikis98 commented Aug 29, 2018

robmorgan commented Aug 30, 2018

brikis98 commented Aug 30, 2018


		gsBucketName := "gruntwork-terratest-" + strings.ToLower(id)

		CreateStorageBucket(t, projectId, gsBucketName, nil)

modules/gcp - GCP support #141

modules/gcp - GCP support #141

Conversation

robmorgan commented Aug 9, 2018 • edited Loading

New Features & Changes

Packer Module

Test Structure Module

GCP Module

Misc

compute.go

provider.go

region.go

region_test.go

storage.go

storage_test.go

Examples

terraform-gcp-example

packer-basic-example

Tests

brikis98 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

Dahs81 Aug 9, 2018 • 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

Dahs81 commented Aug 9, 2018

robmorgan commented Aug 10, 2018

robmorgan commented Aug 20, 2018

josh-padnick commented Aug 22, 2018

robmorgan commented Aug 22, 2018

josh-padnick commented Aug 23, 2018

brikis98 commented Aug 28, 2018

robmorgan commented Aug 29, 2018

brikis98 commented Aug 29, 2018

robmorgan commented Aug 29, 2018 • edited Loading

brikis98 commented Aug 29, 2018

robmorgan commented Aug 30, 2018

brikis98 commented Aug 30, 2018

robmorgan commented Aug 9, 2018 •

edited

Loading

Dahs81 Aug 9, 2018 •

edited

Loading

robmorgan commented Aug 29, 2018 •

edited

Loading