terraform-google-modules / terraform-google-vault

Deploys Vault on Compute Engine
https://registry.terraform.io/modules/terraform-google-modules/vault/google
Apache License 2.0
192 stars 127 forks source link

fix(TPG >= 4.53)!: set bgp keepalive_interval to default 20 #169

Closed dlouvier closed 1 year ago

dlouvier commented 1 year ago

issue: https://github.com/terraform-google-modules/terraform-google-vault/issues/171

keepalive_interval is not set and this value needs to be between 20 and 60.

If set, this value must be between 20 and 60. The default is 20.

Provider documentation

As this is not set, every time we run terraform plan, the plan outputs this change and it is quite noisy.

image

google-cla[bot] commented 1 year ago

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

dlouvier commented 1 year ago

Hello @cft-admins could you run /gcbrun ?

Thanks

apeabody commented 1 year ago

/gcbrun

apeabody commented 1 year ago

Looks like keepalive_interval was added in TPG 4.53.0, would need to also bump https://github.com/terraform-google-modules/terraform-google-vault/blob/master/versions.tf#L23

apeabody commented 1 year ago

Thanks @dlouvier! - Some relevant output from the LINT tests:

terraform_validate . 
╷
│ Error: Unsupported argument
│ 
│   on network.tf line 56, in resource "google_compute_router" "vault-router":
│   56:     bgp = 20
│ 
│ An argument named "bgp" is not expected here.
╵
terraform_validate ./examples/shared_vpc_internal 
Success! The configuration is valid.

terraform_validate ./examples/vault-on-gce 
╷
│ Error: Unsupported argument
│ 
│   on ../../network.tf line 56, in resource "google_compute_router" "vault-router":
│   56:     bgp = 20
│ 
│ An argument named "bgp" is not expected here.
╵
terraform_validate ./modules/cluster 
Success! The configuration is valid.

terraform_validate ./test/fixtures/simple_external 
╷
│ Error: Unsupported argument
│ 
│   on ../../../network.tf line 56, in resource "google_compute_router" "vault-router":
│   56:     bgp = 20
│ 
│ An argument named "bgp" is not expected here.
╵
terraform_validate ./test/setup 
dlouvier commented 1 year ago

Sure, thanks for your review @apeabody

apeabody commented 1 year ago

/gcbrun

apeabody commented 1 year ago

Hi @dlouvier - Looks like just needs a terraform fmt:

Running terraform fmt
network.tf
--- old/network.tf
+++ new/network.tf
@@ -52,7 +52,7 @@
   network = local.network

   bgp {
-    asn = 64514
+    asn                = 64514
     keepalive_interval = 20
   }

Error: terraform fmt failed with exit code 3
Check the output for diffs and correct using terraform fmt <dir>
dlouvier commented 1 year ago

@apeabody hey sorry, I did abandoned this. Fixed the format (I was editing from the GitHub UI and yep... it seemed straight forward but it wasn't).

Can you run tests again ? Should be good now.

apeabody commented 1 year ago

/gcbrun

dlouvier commented 1 year ago

🚀

dlouvier commented 1 year ago

Any update folks,

@apeabody @bharathkkb Can you run again /gcbrun ?

I've rebased the PR as there was a commit in the main branch.

apeabody commented 1 year ago

/gcbrun

dlouvier commented 1 year ago

you welcome, see ya around!