From bb83af8ab5aa61900662c3695f82d8cb5e891e96 Mon Sep 17 00:00:00 2001 From: bcreddy-gcp <123543489+bcreddy-gcp@users.noreply.github.com> Date: Mon, 3 Feb 2025 08:28:17 -0800 Subject: [PATCH] workbench: Fix a bug with instance metadata being removed not working as expected (#12884) Co-authored-by: Stephen Lewis (Burrows) --- .../constants/workbench_instance.go.tmpl | 26 +++--- .../pre_update/workbench_instance.go.tmpl | 14 ++- .../resource_workbench_instance_test.go.tmpl | 88 +++++++++++++++++++ 3 files changed, 114 insertions(+), 14 deletions(-) diff --git a/mmv1/templates/terraform/constants/workbench_instance.go.tmpl b/mmv1/templates/terraform/constants/workbench_instance.go.tmpl index b6cc9afc7db5..046ce1e2cea4 100644 --- a/mmv1/templates/terraform/constants/workbench_instance.go.tmpl +++ b/mmv1/templates/terraform/constants/workbench_instance.go.tmpl @@ -243,23 +243,23 @@ func resizeWorkbenchInstanceDisk(config *transport_tpg.Config, d *schema.Resourc return nil } -// mergeLabels takes two maps of labels and returns a new map with the labels merged. -// If a key exists in old_labels but not in new_labels, it is added to the new map with an empty value. -func mergeLabels(oldLabels, newLabels map[string]interface{}) map[string]string { - modifiedLabels := make(map[string]string) +// mergeMaps takes two maps and returns a new map with the merged results. +// If a key exists in oldMap but not in newMap, it is added to the new map with an empty value. +func mergeMaps(oldMap, newMap map[string]interface{}) map[string]string { + modifiedMap := make(map[string]string) - // Add all labels from newLabels to modifiedLabels - for k, v := range newLabels { - modifiedLabels[k] = v.(string) + // Add all key-values from newMap to modifiedMap + for k, v := range newMap { + modifiedMap[k] = v.(string) } - // Add any keys from oldLabels that are not in newLabels with an empty value - for k := range oldLabels { - if _, ok := newLabels[k]; !ok { - modifiedLabels[k] = "" + // Add any keys from oldMap that are not in newMap with an empty value + for k := range oldMap { + if _, ok := newMap[k]; !ok { + modifiedMap[k] = "" } } - return modifiedLabels + return modifiedMap } -{{- end }} \ No newline at end of file +{{- end }} diff --git a/mmv1/templates/terraform/pre_update/workbench_instance.go.tmpl b/mmv1/templates/terraform/pre_update/workbench_instance.go.tmpl index c74166b07abe..4209b13b9853 100644 --- a/mmv1/templates/terraform/pre_update/workbench_instance.go.tmpl +++ b/mmv1/templates/terraform/pre_update/workbench_instance.go.tmpl @@ -42,7 +42,19 @@ if d.HasChange("effective_labels") { old_labels_interface, new_labels_interface := d.GetChange("effective_labels") old_labels := old_labels_interface.(map[string]interface{}) new_labels := new_labels_interface.(map[string]interface{}) - obj["labels"] = mergeLabels(old_labels,new_labels) + obj["labels"] = mergeMaps(old_labels,new_labels) +} + +if d.HasChange("gce_setup.0.metadata") { + old_metadata_interface, new_metadata_interface := d.GetChange("gce_setup.0.metadata") + old_metadata := old_metadata_interface.(map[string]interface{}) + new_metadata := new_metadata_interface.(map[string]interface{}) + + gceSetup, exists := obj["gceSetup"] + if !exists { + return fmt.Errorf("gceSetup cannot be empty") + } + gceSetup.(map[string]interface{})["metadata"] = mergeMaps(old_metadata,new_metadata) } name := d.Get("name").(string) diff --git a/mmv1/third_party/terraform/services/workbench/resource_workbench_instance_test.go.tmpl b/mmv1/third_party/terraform/services/workbench/resource_workbench_instance_test.go.tmpl index f031aa885e59..c40f3c141c0c 100644 --- a/mmv1/third_party/terraform/services/workbench/resource_workbench_instance_test.go.tmpl +++ b/mmv1/third_party/terraform/services/workbench/resource_workbench_instance_test.go.tmpl @@ -290,6 +290,73 @@ func TestAccWorkbenchInstance_updateMetadata(t *testing.T) { Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr( "google_workbench_instance.instance", "state", "ACTIVE"), + ), + }, + { + ResourceName: "google_workbench_instance.instance", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"name", "instance_owners", "location", "instance_id", "request_id", "labels", "terraform_labels"}, + }, + { + Config: testAccWorkbenchInstance_basic(context), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr( + "google_workbench_instance.instance", "state", "ACTIVE"), + ), + }, + { + ResourceName: "google_workbench_instance.instance", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"name", "instance_owners", "location", "instance_id", "request_id", "labels", "terraform_labels"}, + }, + }, + }) +} + +func TestAccWorkbenchInstance_updateMetadataKey(t *testing.T) { + t.Parallel() + + context := map[string]interface{}{ + "random_suffix": acctest.RandString(t, 10), + } + + acctest.VcrTest(t, resource.TestCase{ + PreCheck: func() { acctest.AccTestPreCheck(t) }, + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t), + Steps: []resource.TestStep{ + { + Config: testAccWorkbenchInstance_updateMetadata(context), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr( + "google_workbench_instance.instance", "state", "ACTIVE"), + ), + }, + { + ResourceName: "google_workbench_instance.instance", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"name", "instance_owners", "location", "instance_id", "request_id", "labels", "terraform_labels"}, + }, + { + Config: testAccWorkbenchInstance_updateMetadataKey(context), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr( + "google_workbench_instance.instance", "state", "ACTIVE"), + ), + }, + { + ResourceName: "google_workbench_instance.instance", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"name", "instance_owners", "location", "instance_id", "request_id", "labels", "terraform_labels"}, + }, + { + Config: testAccWorkbenchInstance_updateMetadata(context), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr( + "google_workbench_instance.instance", "state", "ACTIVE"), ), }, { @@ -322,6 +389,27 @@ resource "google_workbench_instance" "instance" { `, context) } +func testAccWorkbenchInstance_updateMetadataKey(context map[string]interface{}) string { + return acctest.Nprintf(` +resource "google_workbench_instance" "instance" { + name = "tf-test-workbench-instance%{random_suffix}" + location = "us-central1-a" + + gce_setup { + metadata = { + terraform = "true", + "idle-timeout-seconds" = "10800", + } + } + + labels = { + k = "val" + } + +} +`, context) +} + func TestAccWorkbenchInstance_updateState(t *testing.T) { t.Parallel()