google_storage_bucket: fix custom_placement_config values not normalized (#10936) (#7551)

[upstream:4c0b1d9ce480bb5a8f56c0643da68e1415c147cf]

Signed-off-by: Modular Magician <[email protected]>

Author: modular-magician

.changelog/10936.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+storage: fixed `custom_placement_config` values not normalized in `google_storage_bucket`
+```

google-beta/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
 }

google-beta/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" {