Don't send update request if updatmask is empty when resource has ter…

…raform_labels
GoogleCloudPlatform · Oct 3, 2023 · ea7e7b5 · ea7e7b5
1 parent 847abda
commit ea7e7b5
Show file tree

Hide file tree

Showing 4 changed files with 54 additions and 7 deletions.
diff --git a/mmv1/api/resource.rb b/mmv1/api/resource.rb
@@ -556,6 +556,14 @@ def get_labels_field_note(title)
 Please refer to the field `effective_#{title}` for all of the #{title} present on the resource."
     end
 
+    def terraform_labels?
+      terraform_label = root_properties.select do |p|
+        p.is_a?(Api::Type::KeyValueTerraformLabels)
+      end
+
+      terraform_label.length.positive?
+    end
+
     # ====================
     # Version-related methods
     # ====================

diff --git a/mmv1/templates/terraform/post_update/network_security_address_group_operation.go.erb b/mmv1/templates/terraform/post_update/network_security_address_group_operation.go.erb
@@ -1,7 +1,9 @@
-err = NetworkSecurityAddressGroupOperationWaitTime(
-    config, res, "Updating AddressGroup", userAgent,
-    d.Timeout(schema.TimeoutUpdate))
+if len(updateMask) > 0 {
+    err = NetworkSecurityAddressGroupOperationWaitTime(
+        config, res, "Updating AddressGroup", userAgent,
+        d.Timeout(schema.TimeoutUpdate))
 
-if err != nil {
-    return err
+    if err != nil {
+        return err
+    }
 }
diff --git a/mmv1/templates/terraform/resource.erb b/mmv1/templates/terraform/resource.erb
@@ -792,7 +792,7 @@ func resource<%= resource_name -%>Update(d *schema.ResourceData, meta interface{
       billingProject = bp
     }
 
-<%      if object.update_mask && field_specific_update_methods(object.root_properties) -%>
+<%      if object.update_mask && (field_specific_update_methods(object.root_properties) || object.terraform_labels?) -%>
 // if updateMask is empty we are not updating anything so skip the post
 if len(updateMask) > 0 {
 <%      end -%>
@@ -838,7 +838,7 @@ if len(updateMask) > 0 {
     }
 <%        end -%>
 <%      end -%>
-<%      if object.update_mask && field_specific_update_methods(object.root_properties) -%>
+<%      if object.update_mask && (field_specific_update_methods(object.root_properties) || object.terraform_labels?) -%>
   }
 <%      end -%>
 <%    end # if !object.immutable -%>

diff --git a/mmv1/third_party/terraform/services/pubsub/resource_pubsub_topic_test.go b/mmv1/third_party/terraform/services/pubsub/resource_pubsub_topic_test.go
@@ -103,6 +103,43 @@ func TestAccPubsubTopic_schema(t *testing.T) {
 	})
 }
 
+func TestAccPubsubTopic_migration(t *testing.T) {
+	acctest.SkipIfVcr(t)
+	t.Parallel()
+
+	topic := fmt.Sprintf("tf-test-topic-%s", acctest.RandString(t, 10))
+
+	oldVersion := map[string]resource.ExternalProvider{
+		"google": {
+			VersionConstraint: "4.84.0", // a version that doesn't separate user defined labels and system labels
+			Source:            "registry.terraform.io/hashicorp/google",
+		},
+	}
+
+	acctest.VcrTest(t, resource.TestCase{
+		PreCheck:     func() { acctest.AccTestPreCheck(t) },
+		CheckDestroy: testAccCheckPubsubTopicDestroyProducer(t),
+		Steps: []resource.TestStep{
+			{
+				Config:            testAccPubsubTopic_update(topic, "foo", "bar"),
+				ExternalProviders: oldVersion,
+			},
+			{
+				Config:                   testAccPubsubTopic_update(topic, "foo", "bar"),
+				ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t),
+			},
+			{
+				ResourceName:             "google_pubsub_topic.foo",
+				ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t),
+				ImportStateId:            topic,
+				ImportState:              true,
+				ImportStateVerify:        true,
+				ImportStateVerifyIgnore:  []string{"labels", "terraform_labels"},
+			},
+		},
+	})
+}
+
 func testAccPubsubTopic_update(topic, key, value string) string {
 	return fmt.Sprintf(`
 resource "google_pubsub_topic" "foo" {