From 7f40caa92c82ba1eec6262980772984a2c4ab61b Mon Sep 17 00:00:00 2001 From: Modular Magician Date: Mon, 17 Jun 2024 15:40:08 +0000 Subject: [PATCH] `google_storage_bucket`: fix `custom_placement_config` values not normalized (#10936) [upstream:4c0b1d9ce480bb5a8f56c0643da68e1415c147cf] Signed-off-by: Modular Magician --- .changelog/10936.txt | 3 + .../storage/resource_storage_bucket.go | 11 ++- .../storage/resource_storage_bucket_test.go | 88 +++++++++++++++++++ 3 files changed, 101 insertions(+), 1 deletion(-) create mode 100644 .changelog/10936.txt diff --git a/.changelog/10936.txt b/.changelog/10936.txt new file mode 100644 index 00000000000..a37df7ef672 --- /dev/null +++ b/.changelog/10936.txt @@ -0,0 +1,3 @@ +```release-note:bug +storage: fixed `custom_placement_config` values not normalized in `google_storage_bucket` +``` \ No newline at end of file diff --git a/google/services/storage/resource_storage_bucket.go b/google/services/storage/resource_storage_bucket.go index d07c23651c5..1ac1a847b75 100644 --- a/google/services/storage/resource_storage_bucket.go +++ b/google/services/storage/resource_storage_bucket.go @@ -497,6 +497,9 @@ func ResourceStorageBucket() *schema.Resource { MinItems: 2, Elem: &schema.Schema{ Type: schema.TypeString, + StateFunc: func(s interface{}) string { + return strings.ToUpper(s.(string)) + }, }, Description: `The list of individual regions that comprise a dual-region bucket. See the docs for a list of acceptable regions. Note: If any of the data_locations changes, it will recreate the bucket.`, }, @@ -1170,9 +1173,15 @@ func flattenBucketCustomPlacementConfig(cfc *storage.BucketCustomPlacementConfig func expandBucketDataLocations(configured interface{}) []string { l := configured.(*schema.Set).List() + // Since we only want uppercase values to prevent unnecessary diffs, we can do a comparison + // to determine whether or not to include the value as part of the request. + + // This extra check comes from the limitations of both DiffStateFunc and StateFunc towards types of Sets,Lists, and Maps. req := make([]string, 0, len(l)) for _, raw := range l { - req = append(req, raw.(string)) + if raw.(string) == strings.ToUpper(raw.(string)) { + req = append(req, raw.(string)) + } } return req } diff --git a/google/services/storage/resource_storage_bucket_test.go b/google/services/storage/resource_storage_bucket_test.go index 7d78ec69466..4ff047d3992 100644 --- a/google/services/storage/resource_storage_bucket_test.go +++ b/google/services/storage/resource_storage_bucket_test.go @@ -250,6 +250,81 @@ func TestAccStorageBucket_dualLocation(t *testing.T) { }) } +func TestAccStorageBucket_dualLocation_lowercase(t *testing.T) { + t.Parallel() + + bucketName := acctest.TestBucketName(t) + + acctest.VcrTest(t, resource.TestCase{ + PreCheck: func() { acctest.AccTestPreCheck(t) }, + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t), + CheckDestroy: testAccStorageBucketDestroyProducer(t), + Steps: []resource.TestStep{ + { + Config: testAccStorageBucket_dualLocation_lowercase(bucketName), + }, + { + ResourceName: "google_storage_bucket.bucket", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"force_destroy"}, + }, + }, + }) +} + +func TestAccStorageBucket_dualLocation_versionChange(t *testing.T) { + // Test is not parallel because ENVs are set. + // Need to skip VCR as this test downloads providers from the Terraform Registry + acctest.SkipIfVcr(t) + + creds := envvar.GetTestCredsFromEnv() + project := envvar.GetTestProjectFromEnv() + t.Setenv("GOOGLE_CREDENTIALS", creds) + t.Setenv("GOOGLE_PROJECT", project) + + bucketName := acctest.TestBucketName(t) + + acctest.VcrTest(t, resource.TestCase{ + PreCheck: func() { acctest.AccTestPreCheck(t) }, + CheckDestroy: testAccStorageBucketDestroyProducer(t), + Steps: []resource.TestStep{ + { + Config: testAccStorageBucket_dualLocation(bucketName), + ExternalProviders: map[string]resource.ExternalProvider{ + "google": { + VersionConstraint: "5.30.0", + Source: "hashicorp/google", + }, + }, + }, + { + ResourceName: "google_storage_bucket.bucket", + ExternalProviders: map[string]resource.ExternalProvider{ + "google": { + VersionConstraint: "5.30.0", + Source: "hashicorp/google", + }, + }, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"force_destroy"}, + }, + { + Config: testAccStorageBucket_dualLocation(bucketName), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t), + }, + { + ResourceName: "google_storage_bucket.bucket", + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t), + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"force_destroy"}, + }, + }, + }) +} + func TestAccStorageBucket_dualLocation_rpo(t *testing.T) { t.Parallel() bucketName := acctest.TestBucketName(t) @@ -1715,6 +1790,19 @@ resource "google_storage_bucket" "bucket" { `, bucketName) } +func testAccStorageBucket_dualLocation_lowercase(bucketName string) string { + return fmt.Sprintf(` +resource "google_storage_bucket" "bucket" { + name = "%s" + location = "ASIA" + force_destroy = true + custom_placement_config { + data_locations = ["asia-east1", "asia-southeast1"] + } +} +`, bucketName) +} + func testAccStorageBucket_dualLocation_rpo(bucketName string, rpo string) string { return fmt.Sprintf(` resource "google_storage_bucket" "bucket" {