terraform-google-modules / terraform-example-foundation

Shows how the CFT modules can be composed to build a secure cloud foundation
https://cloud.google.com/architecture/security-foundations
Apache License 2.0
1.2k stars 706 forks source link

In 4-projects CMEK GCS bucket name length exceeds the statutory 63 characters #1169

Closed mromascanu123 closed 3 months ago

mromascanu123 commented 5 months ago

TL;DR

When using non-US regions, deployment of 4-Projects fails due to invalid (too long) bucket name

Expected behavior

The bucket name should always be max 63 characters regardless of the length of a region's name.

Observed behavior

... ./tf-wrapper.sh apply production

Error: error: bucket name validation failed bkt-prj-p-bu1sample-base-vc6x-northamerica-northeast2-cmek-encrypted-3bcbb

with module.env.module.gcs_buckets.google_storage_bucket.bucket, on ../../../terraform-google-modules/cloud-storage/google/modules/simple_bucket/main.tf line 17, in resource "google_storage_bucket" "bucket": 17: resource "google_storage_bucket" "bucket" {

Terraform Configuration

common.auto.tfvars
remote_state_bucket = "REMOTE_STATE_BUCKET"

{development,non-production,production}.auto.tfvars
location_kms = "northamerica-northeast2"
location_gcs = "northamerica-northeast2"

shared.auto.tfvars
default_region = "northamerica-northeast2"

Terraform Version

Terraform v1.6.0
on linux_amd64
+ provider registry.terraform.io/hashicorp/google v5.20.0
+ provider registry.terraform.io/hashicorp/google-beta v5.20.0
+ provider registry.terraform.io/hashicorp/null v3.2.2
+ provider registry.terraform.io/hashicorp/random v3.6.0
+ provider registry.terraform.io/hashicorp/time v0.11.1

Your version of Terraform is out of date! The latest version
is 1.7.5. You can update by downloading from https://www.terraform.io/downloads.html

Additional information

Easy fix in 4-projects/modules/base_env/example_storage_cmek.tf locals { cmek_bucket_suffix = "${module.base_shared_vpc_project.project_id}-${lower(var.location_gcs)}-${random_string.bucket_name.result}" cmek_bucket_prefix = "${var.gcs_bucket_prefix}-cmek-encrypted" } ... module "gcs_buckets" { ...

//name = "${var.gcs_bucket_prefix}-${module.base_shared_vpc_project.project_id}-${lower(var.location_gcs)}-cmek-encrypted-${random_string.bucket_name.result}" name = "${local.cmek_bucket_prefix}-${md5(local.cmek_bucket_suffix)}"

nbugden commented 4 months ago

In addition to fixing the name, terraform should also fail to plan a bucket with an invalid name.

Related issue in terraform-google-cloud-storage module: https://github.com/terraform-google-modules/terraform-google-cloud-storage/issues/307

Fix proposed for terraform-google-cloud-storage module: https://github.com/terraform-google-modules/terraform-google-cloud-storage/pull/308

fmichaelobrien commented 4 months ago

Nathan, perfect - I'll pull in the PR in a 2nd org to verify https://github.com/terraform-google-modules/terraform-google-cloud-storage/pull/308

fmichaelobrien commented 4 months ago

Normally a minimum fix on the bucket length may require our own validation check on the size of any tfvars defined variables like the business unit prefix/postfix along with the with the additional 5 char random postfix. However in this case the 9+ char overage is more due to the long form region name var.location_gcs Yes, either truncate the project id, or use the short form of the region - as it is less important

bkt-prj-p-bu1sample-base-vc6x-northamerica-northeast2-cmek-encrypted-3bcbb
so
bkt-prj-p-bu1sample-base-vc6x-cmek-encrypted-3bcbb
or
bkt-prj-p-bu1sample-base-vc6x-nane2-cmek-encrypted-3bcbb

would work
mromascanu123 commented 4 months ago

Or even simpler use the same technique as in Azure - randomize the name by hashing all the original string representing the way-toooo-long name ending-up with a fixed-length string, random enough for avoiding global-scope name colisions. Then cut/replace the irrelevant tail of the name with the hash or substring of it bkt-prj-p-bu1sample-base-vc6x-northamerica-northeast2-cmek-encrypted-3bcbb => bkt-prj-p-bu1sample-base-<32-hex-chars-hash> => 57 chars in all Even better can base64 (6bits/char) the binary md5 hash and end up with 25+22 ( for the randomized name tail)=47 chars

fmichaelobrien commented 4 months ago

stale bot timer restart - https://github.com/terraform-google-modules/terraform-example-foundation/blob/master/.github/workflows/stale.yml#L21

nbugden commented 4 months ago

Upstream changes to the provider to validate the bucket name:

eeaton commented 3 months ago

Thanks for the contributions on the upstream issues to validate GCS names on plan.

There is also https://github.com/terraform-google-modules/terraform-example-foundation/issues/1166 to to track v5 backlog of how we'll address the issue of naming conventions exceeding character limit more broadly, so I'll mark this issue duplicate and close it for now.