Skip to content

Commit

Permalink
workbench: Fix a bug with instance metadata being removed not working…
Browse files Browse the repository at this point in the history
… as expected (#12884)

Co-authored-by: Stephen Lewis (Burrows) <stephen.r.burrows@gmail.com>
  • Loading branch information
bcreddy-gcp and melinath authored Feb 3, 2025
1 parent 27fd884 commit bb83af8
Show file tree
Hide file tree
Showing 3 changed files with 114 additions and 14 deletions.
26 changes: 13 additions & 13 deletions mmv1/templates/terraform/constants/workbench_instance.go.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
{{- end }}
14 changes: 13 additions & 1 deletion mmv1/templates/terraform/pre_update/workbench_instance.go.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
),
},
{
Expand Down Expand Up @@ -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()

Expand Down

0 comments on commit bb83af8

Please sign in to comment.