-
Notifications
You must be signed in to change notification settings - Fork 545
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
Add support for Terraform v0.12 #237
Conversation
@aaron-lane Could you kindly rebase this against |
7504354
to
d4cf4a8
Compare
Co-Authored-By: Aaron Lane <[email protected]>
Co-Authored-By: Aaron Lane <[email protected]>
In ./test/make.sh line 133: cd $path && echo "Working in ${path} ..." ^---^ SC2086: Double quote to prevent globbing and word splitting.
Without this patch CI fails with Terraform 0.12 syntax errors becasue Terraform 0.11 is used. Error: Error parsing /tmp/build/f541ec31/terraform-google-project-factory/test/fixtures/shared_vpc_no_subnets/main.tf: At 26:29: Unknown token: 26:29 IDENT file This patch upgrades to Terraform 0.12.3 using the kitchen-terraform 2.2.0 docker image. See GoogleCloudPlatform/cloud-foundation-toolkit#216
Without this patch make check_terraform fails, causing the lint test pipeline to fail. This patch updates the task to work with Terraform 0.12. In 0.12 terraform validate requires terraform init to have run. This behavior is implemented in the helper script, helpers/terraform_validate Example output: make check_terraform Running terraform fmt terraform fmt -diff -check=true -write=false . terraform fmt -diff -check=true -write=false ./examples/app_engine examples/app_engine/main.tf --- old/examples/app_engine/main.tf +++ new/examples/app_engine/main.tf @@ -22,15 +22,15 @@ Provider configuration *****************************************/ provider "gsuite" { - credentials = file(local.credentials_file_path) - version = "~> 0.1.9" + credentials = file(local.credentials_file_path) + version = "~> 0.1.9" } module "app-engine" { - source = "../../modules/app_engine" - location_id = var.location_id - auth_domain = var.auth_domain - serving_status = var.serving_status + source = "../../modules/app_engine" + location_id = var.location_id + auth_domain = var.auth_domain + serving_status = var.serving_status feature_settings = [{ enabled = true }] - project_id = "example-project" + project_id = "example-project" } terraform fmt -diff -check=true -write=false ./examples/gke_shared_vpc Warning: compat_xargs -t -n1 terraform fmt -diff -check=true -write=false failed with exit code 1 Error: terraform fmt failed with exit code 1 Check the output for diffs and correct using terraform fmt <dir> make: *** [check_terraform] Error 1 This patch also uses a temporary version of the network module to correct the error: Error: Unsupported block type on .terraform/modules/vpc/terraform-google-modules-terraform-google-network-d1b6646/main.tf line 94, in resource "null_resource" "delete_default_internet_gateway_routes": 94: triggers { Blocks of type "triggers" are not expected here. Did you mean to define argument "triggers"? If so, use the equals sign to assign it a value.
This patch fixes: make check_python Running flake8 ./modules/core_project_factory/scripts/preconditions/preconditions.py:405:19: E201 whitespace after '[' Warning: compat_xargs -0 flake8 failed with exit code 1
Without this patch lint checks fail with: make check_headers Checking file headers 1 files have incorrect boilerplate headers: examples/app_engine/outputs.tf Warning: compat_xargs -0 python test/verify_boilerplate.py failed with exit code 1 make: *** [check_headers] Error 1
Without this patch `make check` fails lint checks with: Checking for trailing whitespace ./.kitchen/minimal-local.yml:2:last_error: ./.kitchen/logs/minimal-local.log:76:I, [2019-07-08T22:44:40.720556 #252] INFO -- minimal-local: ./.kitchen/logs/minimal-local.log:78:I, [2019-07-08T22:44:40.721441 #252] INFO -- minimal-local: group_email = ./.kitchen/logs/minimal-local.log:87:I, [2019-07-08T22:44:40.730335 #252] INFO -- minimal-local: shared_vpc = ./.kitchen/logs/minimal-local.log:88:I, [2019-07-08T22:44:40.730870 #252] INFO -- minimal-local: usage_bucket_name = ./.kitchen/logs/kitchen.log:4:I, [2019-07-08T22:34:45.979975 #252] INFO -- Kitchen: ./.kitchen/logs/kitchen.log:9:I, [2019-07-08T22:39:51.765208 #252] INFO -- Kitchen: ./.kitchen/logs/kitchen.log:14:I, [2019-07-08T22:44:40.886354 #252] INFO -- Kitchen: ./.kitchen/logs/shared-vpc-no-subnets-local.log:77:I, [2019-07-08T22:49:02.628309 #252] INFO -- shared-vpc-no-subnets-local: ./.kitchen/logs/full-local.log:95:I, [2019-07-08T22:39:51.630494 #252] INFO -- full-local: make: *** [check_trailing_whitespace] Error 1
Without this patch the following test fails: × project-factory-app-engine: Project Factory App Engine configuration (1 failed) ✔ Command: `gcloud app describe --project pf-ci-test-full-id-mvuis --format=json` should include {:authDomain => "phoogle.net"} × Command: `gcloud app describe --project pf-ci-test-full-id-mvuis --format=json` should include {:featureSettings => {:splitHealthChecks => true}} expected {:authDomain => "phoogle.net", :codeBucket => "staging.pf-ci-test-full-id-mvuis.appspot.com", :defaultBuc...-mvuis", :locationId => "us-east4", :name => "apps/pf-ci-test-full-id-mvuis", :servingStatus => "SERVING"} to include {:featureSettings => {:splitHealthChecks => true}} Diff: @@ -1,2 +1,11 @@ -:featureSettings => {:splitHealthChecks=>true}, +:authDomain => "phoogle.net", +:codeBucket => "staging.pf-ci-test-full-id-mvuis.appspot.com", +:defaultBucket => "pf-ci-test-full-id-mvuis.appspot.com", +:defaultHostname => "pf-ci-test-full-id-mvuis.appspot.com", +:featureSettings => {:splitHealthChecks=>true, :useContainerOptimizedOs=>true}, +:gcrDomain => "us.gcr.io", +:id => "pf-ci-test-full-id-mvuis", +:locationId => "us-east4", +:name => "apps/pf-ci-test-full-id-mvuis", +:servingStatus => "SERVING",
6004b31
to
5d17bd7
Compare
b54d2fc
to
f90b827
Compare
LGTM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple nit-picks, but largely LGTM otherwise!
impersonated_user_email = "${var.admin_email}" | ||
version = "~> 0.1.9" | ||
credentials = file(local.credentials_file_path) | ||
version = "~> 0.1.9" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given the addition to TROUBLESHOOTING.md, do we want to change this version-pin to > 0.1.12
?
@@ -52,22 +52,22 @@ locals { | |||
} | |||
|
|||
module "vpc" { | |||
source = "terraform-google-modules/network/google" | |||
version = "~> 0.4.0" | |||
source = "git::https://github.com/terraform-google-modules/terraform-google-network.git?ref=master" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we pin this to a specific version? 0.12.x compatibility was introduced in terraform-google-modules/terraform-google-network@7ba1b23
# | ||
# source = "terraform-google-modules/network/google" | ||
# version = "0.8.0" | ||
source = "git::https://github.com/terraform-google-modules/terraform-google-network.git?ref=master" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we pin this to a specific version? 0.12.x compatibility was introduced in terraform-google-modules/terraform-google-network@7ba1b23
Sorry, race condition where this was merged while I was reviewing it. My suggestions are nit-picky -- would suggest making them follow-up issues for future addressing. |
No description provided.