fix: create random suffix resource on demand #185

Tensho · 2022-09-15T09:43:38Z

There is no need to create random_id.bucket_suffix resource if user doesn't enable bucket suffix via randomize_suffix input variable.

Before

randomize_suffix is disabled.

$ cd examples/multiple_buckets
$ terraform plan
...
# random_string.prefix will be created
# module.cloud_storage.google_storage_bucket.buckets["one"] will be created
# module.cloud_storage.random_id.bucket_suffix will be created
...
Plan: 3 to add, 0 to change, 0 to destroy.

randomize_suffix is enabled.

$ cd examples/multiple_buckets
$ terraform plan
...
# random_string.prefix will be created
# module.cloud_storage.google_storage_bucket.buckets["one"] will be created
# module.cloud_storage.random_id.bucket_suffix will be created
...
Plan: 3 to add, 0 to change, 0 to destroy.

After

randomize_suffix is disabled.

$ cd examples/multiple_buckets
$ terraform plan
...
# random_string.prefix will be created
# module.cloud_storage.google_storage_bucket.buckets["one"] will be created
...
Plan: 2 to add, 0 to change, 0 to destroy.

randomize_suffix is enabled.

$ cd examples/multiple_buckets
$ terraform plan
...
# random_string.prefix will be created
# module.cloud_storage.google_storage_bucket.buckets["one"] will be created
# module.cloud_storage.random_id.bucket_suffix[0] will be created
...
Plan: 3 to add, 0 to change, 0 to destroy.

Note

Resource moves to new location implicitly by Terraform:

# random_id.bucket_suffix moved to random_id.bucket_suffix[0]

google-cla · 2022-09-15T09:43:42Z

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.

comment-bot-dev · 2022-09-15T09:48:46Z

@Tensho
Thanks for the PR! 🚀
✅ Lint checks have passed.

Tensho · 2022-11-03T23:20:54Z

Could we have this merged please?

tpolekhin · 2022-11-22T13:20:04Z

@Tensho thank you for this PR - I'm looking forward to use it after the merge.
I would also add that sometimes you want to create buckets conditionally, so var.names could be an empty list []
Could you please also add this as a condition for the random_id.bucket_suffix resource?
I think it might look something like this:

resource "random_id" "bucket_suffix" {
   count       = alltrue([length(var.names) > 0, var.randomize_suffix]) ? 1 : 0
   byte_length = 2
 }

Tensho · 2022-11-28T16:34:02Z

@tpolekhin I'm glad someone else is interested in this small change. I'm not sure that's necessary – random_id has its own toggler. If the names input variable is set to an empty list, the bucket list would be also an empty list, ergo no resources are created. The randomized_suffix input variable represents opt-in behaviour, ergo if it's not set/enabled there is no random_id resource is created. Would you mind elaborating on your use case more, please?

tpolekhin · 2022-11-29T12:45:56Z

@Tensho yes, sorry for not being more clear on that.
Here is an example:

module "gcs" {
  source  = "terraform-google-modules/cloud-storage/google"
  version = "3.3.0"

  names            = local.create_gcs_bucket ? ["testbucket"] : []
  randomize_suffix = true
}

I want to create bucket conditionally and I want it to have a randomised suffix if we do create it
But while I was writing this response I realised that I can use the same condition for the randomize_suffix as well

module "gcs" {
  source  = "terraform-google-modules/cloud-storage/google"
  version = "3.3.0"

  names            = local.create_gcs_bucket ? ["testbucket"] : []
  randomize_suffix = local.create_gcs_bucket
}

This will prevent the creation of the random_id if we're not creating any buckets 👍
Thanks for your help! 😄

bharathkkb

Thanks for the PR @Tensho and apologies for the late reply. Just a question if there is any regression with this change.

bharathkkb · 2022-12-06T01:02:17Z

examples/multiple_buckets/main.tf

-      matches_storage_class = "MULTI_REGIONAL,STANDARD,DURABLE_REDUCED_AVAILABILITY"
-    }
-  }]
+  # FIXME: This doesn't work:


I am not sure why this would happen with the proposed code change in the module? Did this happen for you locally?

I just wanted to flag the current example didn't work for me, hence I changed it. If you're fine with the proposed example I'm happy to drop the comment.

Hi @Tensho - The INT tests also include assertions for the example which need to match: https://github.com/terraform-google-modules/terraform-google-cloud-storage/blob/master/test/integration/multiple_buckets/multiple_buckets_test.go#L56

Fair enough, I'll make adjustments.

@apeabody The test passes now:

[root@bb37621fcfea workspace]# cft test run all ok github.com/terraform-google-modules/terraform-google-cloud-storage/test/integration 65.892s ok github.com/terraform-google-modules/terraform-google-cloud-storage/test/integration/multiple_buckets 92.755s

Tensho · 2022-12-08T20:28:49Z

@bharathkkb There is no regression.

github-actions · 2023-02-06T23:19:39Z

This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 7 days

apeabody · 2023-03-21T16:30:31Z

Thanks for the contribution @Tensho! Can you please resolve the conflicts so I can trigger the INT tests.

Tensho · 2023-03-21T21:38:25Z

@apeabody Done

apeabody · 2023-03-21T23:19:11Z

/gcbrun

apeabody · 2023-04-10T22:30:00Z

/gcbrun

apeabody · 2023-04-10T22:35:36Z

Thanks @Tensho - From the LINT:

Checking for tflint
Working in . ...
2 issue(s) found:

Warning: List items should be accessed using square brackets (terraform_deprecated_index)

  on main.tf line 26:
  26:   suffix       = var.randomize_suffix ? random_id.bucket_suffix.0.hex : ""

Reference: https://github.com/terraform-linters/tflint-ruleset-terraform/blob/v0.1.1/docs/rules/terraform_deprecated_index.md

Warning: List items should be accessed using square brackets (terraform_deprecated_index)

  on main.tf line 26:
  26:   suffix       = var.randomize_suffix ? random_id.bucket_suffix.0.hex : ""

Reference: https://github.com/terraform-linters/tflint-ruleset-terraform/blob/v0.1.1/docs/rules/terraform_deprecated_index.md

tflint failed .

Tensho · 2023-04-10T22:47:13Z

@apeabody Fixed.

apeabody · 2023-04-10T22:48:22Z

/gcbrun

Error: Invalid index

apeabody · 2023-05-01T15:19:44Z

/gcbrun

github-actions · 2023-06-30T23:19:07Z

This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 7 days

Tensho · 2023-07-01T11:20:33Z

@apeabody Hey 👋 Could we have some traction here please? 🙂

apeabody · 2023-07-05T15:23:42Z

/gcbrun

apeabody · 2023-07-05T15:32:42Z

Pre-Req: #257

apeabody · 2023-07-05T18:18:31Z

/gcbrun

apeabody · 2023-07-05T19:58:28Z

/gcbrun

apeabody

Thanks for the contribution @Tensho!

It looks like this will be a breaking change for existing users, can you include how to migrate (details in the note below) called docs/upgrading_to_v4.0.md similar to https://github.com/terraform-google-modules/terraform-google-cloud-storage/blob/master/docs/upgrading_to_v4.0.md

Alternatively the minimum terraform version could be bumped (v1.1) to use https://developer.hashicorp.com/terraform/tutorials/configuration-language/move-config#move-your-resources-with-the-moved-configuration-block

apeabody · 2023-07-05T22:21:57Z

main.tf

  byte_length = 2
 }

 locals {
-  suffix       = var.randomize_suffix ? random_id.bucket_suffix.hex : ""
+  suffix       = var.randomize_suffix ? random_id.bucket_suffix[0].hex : ""


FYI: This will be a breaking change for existing users. Probably can use something like terraform state mv 'random_id.bucket_suffix.hex' 'random_id.bucket_suffix[0].hex' to migrate the existing suffix to the new resource location.

I specified it in the PR description "Note" section – it won't be a breaking change. Terraform implicitly modifies the state. Try to run the following experiment:

Draft a simple configuration without count and apply it:

resource "random_id" "x" { byte_length = 1 }

$ terraform apply -auto-approve Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols: + create Terraform will perform the following actions: # random_id.x will be created + resource "random_id" "x" { + b64_std = (known after apply) + b64_url = (known after apply) + byte_length = 1 + dec = (known after apply) + hex = (known after apply) + id = (known after apply) } Plan: 1 to add, 0 to change, 0 to destroy. random_id.x: Creating... random_id.x: Creation complete after 0s [id=wA] Apply complete! Resources: 1 added, 0 changed, 0 destroyed.

Modify resource with count and apply it:

resource "random_id" "x" { count = 1 byte_length = 1 }

$ terraform apply -auto-approve random_id.x[0]: Refreshing state... [id=wA] Terraform will perform the following actions: # random_id.x has moved to random_id.x[0] resource "random_id" "x" { id = "wA" # (5 unchanged attributes hidden) } Plan: 0 to add, 0 to change, 0 to destroy. Apply complete! Resources: 0 added, 0 changed, 0 destroyed.

Pay attention resource has been moved, no replacement happened.

Thanks for explanation @Tensho!

I just confirmed this worked on TF 0.13 (https://github.com/terraform-google-modules/terraform-google-cloud-storage/blob/master/versions.tf#L18), although the messaging is not as clear:

./terraform apply random_id.x[0]: Refreshing state... [id=9w] Apply complete! Resources: 0 added, 0 changed, 0 destroyed.

apeabody · 2023-07-05T23:22:06Z

/gcbrun

Tensho changed the title ~~Create random suffix on demand~~ fix: create random suffix resource on demand Sep 15, 2022

bharathkkb reviewed Dec 6, 2022

View reviewed changes

github-actions bot added the Stale label Feb 6, 2023

Tensho requested a review from bharathkkb February 7, 2023 09:21

github-actions bot removed the Stale label Feb 7, 2023

Tensho force-pushed the conditional-random-suffix branch from d11d7aa to 89a7ac7 Compare March 21, 2023 21:35

Tensho requested a review from a team as a code owner March 21, 2023 21:35

apeabody mentioned this pull request Mar 21, 2023

chore(master): release 4.0.0 #214

Merged

Tensho force-pushed the conditional-random-suffix branch from abf5d93 to 150cbb7 Compare April 10, 2023 22:46

Tensho added 6 commits April 28, 2023 20:19

fix: create random id resource for bucket name suffix on demand

43be8ea

chore: format multiple buckets example terraform

d78d864

comment: folders don't work in multiple buckets example

cf75d6e

Error: Invalid index

ci: gratify lint

d966f83

style: gratify terraform linter

92096f8

fix multiple buckets with randomized suffix test

ae55809

Tensho force-pushed the conditional-random-suffix branch from 150cbb7 to ae55809 Compare April 28, 2023 20:54

return 2 folders back

abdc9f9

Tensho requested a review from apeabody May 1, 2023 13:08

github-actions bot added the Stale label Jun 30, 2023

github-actions bot removed the Stale label Jul 1, 2023

Merge branch 'master' into conditional-random-suffix

2603909

Merge branch 'master' into conditional-random-suffix

37f5626

Merge branch 'master' into conditional-random-suffix

26f72f9

apeabody changed the title ~~fix: create random suffix resource on demand~~ fix!: create random suffix resource on demand Jul 5, 2023

apeabody requested changes Jul 5, 2023

View reviewed changes

Merge branch 'master' into conditional-random-suffix

f71b8fe

apeabody approved these changes Jul 5, 2023

View reviewed changes

apeabody changed the title ~~fix!: create random suffix resource on demand~~ fix: create random suffix resource on demand Jul 5, 2023

apeabody merged commit 5050c91 into terraform-google-modules:master Jul 5, 2023

release-please bot mentioned this pull request Jul 5, 2023

chore(master): release 4.0.1 #258

Merged

Tensho deleted the conditional-random-suffix branch July 6, 2023 08:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: create random suffix resource on demand #185

fix: create random suffix resource on demand #185

Tensho commented Sep 15, 2022 •

edited

Loading

google-cla bot commented Sep 15, 2022

comment-bot-dev commented Sep 15, 2022

Tensho commented Nov 3, 2022

tpolekhin commented Nov 22, 2022

Tensho commented Nov 28, 2022

tpolekhin commented Nov 29, 2022

bharathkkb left a comment

bharathkkb Dec 6, 2022

Tensho Dec 6, 2022

apeabody Apr 10, 2023

Tensho Apr 10, 2023

Tensho May 1, 2023

Tensho commented Dec 8, 2022

github-actions bot commented Feb 6, 2023

apeabody commented Mar 21, 2023

Tensho commented Mar 21, 2023

apeabody commented Mar 21, 2023

apeabody commented Apr 10, 2023

apeabody commented Apr 10, 2023

Tensho commented Apr 10, 2023

apeabody commented Apr 10, 2023

apeabody commented May 1, 2023

github-actions bot commented Jun 30, 2023

Tensho commented Jul 1, 2023

apeabody commented Jul 5, 2023

apeabody commented Jul 5, 2023

apeabody commented Jul 5, 2023

apeabody commented Jul 5, 2023

apeabody left a comment •

edited

Loading

apeabody Jul 5, 2023

Tensho Jul 5, 2023 •

edited

Loading

apeabody Jul 5, 2023

apeabody commented Jul 5, 2023

fix: create random suffix resource on demand #185

fix: create random suffix resource on demand #185

Conversation

Tensho commented Sep 15, 2022 • edited Loading

Before

After

Note

google-cla bot commented Sep 15, 2022

comment-bot-dev commented Sep 15, 2022

Tensho commented Nov 3, 2022

tpolekhin commented Nov 22, 2022

Tensho commented Nov 28, 2022

tpolekhin commented Nov 29, 2022

bharathkkb left a comment

Choose a reason for hiding this comment

bharathkkb Dec 6, 2022

Choose a reason for hiding this comment

Tensho Dec 6, 2022

Choose a reason for hiding this comment

apeabody Apr 10, 2023

Choose a reason for hiding this comment

Tensho Apr 10, 2023

Choose a reason for hiding this comment

Tensho May 1, 2023

Choose a reason for hiding this comment

Tensho commented Dec 8, 2022

github-actions bot commented Feb 6, 2023

apeabody commented Mar 21, 2023

Tensho commented Mar 21, 2023

apeabody commented Mar 21, 2023

apeabody commented Apr 10, 2023

apeabody commented Apr 10, 2023

Tensho commented Apr 10, 2023

apeabody commented Apr 10, 2023

apeabody commented May 1, 2023

github-actions bot commented Jun 30, 2023

Tensho commented Jul 1, 2023

apeabody commented Jul 5, 2023

apeabody commented Jul 5, 2023

apeabody commented Jul 5, 2023

apeabody commented Jul 5, 2023

apeabody left a comment • edited Loading

Choose a reason for hiding this comment

apeabody Jul 5, 2023

Choose a reason for hiding this comment

Tensho Jul 5, 2023 • edited Loading

Choose a reason for hiding this comment

apeabody Jul 5, 2023

Choose a reason for hiding this comment

apeabody commented Jul 5, 2023

Tensho commented Sep 15, 2022 •

edited

Loading

apeabody left a comment •

edited

Loading

Tensho Jul 5, 2023 •

edited

Loading