fix: string based ordering for google_compute_region_security_policy causes recreate after apply (#14093) (#10105)

[upstream:f55da64d5c5a549bb0da32e383922adb75d79fe7]

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

Author: modular-magician

.changelog/14093.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+compute: fixed an issue where rules ordering in `google_compute_region_security_policy` caused a diff after apply
+```

google-beta/services/compute/resource_compute_region_security_policy.go

@@ -35,6 +35,35 @@ import (
 	"github.com/hashicorp/terraform-provider-google-beta/google-beta/verify"
 )
 
+func resourceComputeRegionSecurityPolicySpecRulesDiffSuppress(k, o, n string, d *schema.ResourceData) bool {
+	oldCount, newCount := d.GetChange("rules.#")
+	var count int
+	// There could be duplicates - worth continuing even if the counts are unequal.
+	if oldCount.(int) < newCount.(int) {
+		count = newCount.(int)
+	} else {
+		count = oldCount.(int)
+	}
+
+	old := make([]interface{}, 0, count)
+	new := make([]interface{}, 0, count)
+	for i := 0; i < count; i++ {
+		o, n := d.GetChange(fmt.Sprintf("rules.%d", i))
+
+		if o != nil {
+			old = append(old, o)
+		}
+		if n != nil {
+			new = append(new, n)
+		}
+	}
+
+	oldSet := schema.NewSet(schema.HashResource(ResourceComputeRegionSecurityPolicy().Schema["rules"].Elem.(*schema.Resource)), old)
+	newSet := schema.NewSet(schema.HashResource(ResourceComputeRegionSecurityPolicy().Schema["rules"].Elem.(*schema.Resource)), new)
+
+	return oldSet.Equal(newSet)
+}
+
 func ResourceComputeRegionSecurityPolicy() *schema.Resource {
 	return &schema.Resource{
 		Create: resourceComputeRegionSecurityPolicyCreate,
@@ -98,10 +127,11 @@ Specifically, the name must be 1-63 characters long and match the regular expres
 If it is not provided, the provider region is used.`,
 			},
 			"rules": {
-				Type:        schema.TypeList,
-				Computed:    true,
-				Optional:    true,
-				Description: `The set of rules that belong to this policy. There must always be a default rule (rule with priority 2147483647 and match "*"). If no rules are provided when creating a security policy, a default rule with action "allow" will be added.`,
+				Type:             schema.TypeList,
+				Computed:         true,
+				Optional:         true,
+				DiffSuppressFunc: resourceComputeRegionSecurityPolicySpecRulesDiffSuppress,
+				Description:      `The set of rules that belong to this policy. There must always be a default rule (rule with priority 2147483647 and match "*"). If no rules are provided when creating a security policy, a default rule with action "allow" will be added.`,
 				Elem: &schema.Resource{
 					Schema: map[string]*schema.Schema{
 						"action": {

google-beta/services/compute/resource_compute_region_security_policy_test.go

@@ -735,6 +735,118 @@ func testAccComputeRegionSecurityPolicy_withMultipleEnforceOnKeyConfigs_update(c
 		}
 	`, context)
 }
+
+func TestAccComputeRegionSecurityPolicy_regionSecurityPolicyRuleOrderingWithMultipleRules(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),
+		CheckDestroy:             testAccCheckComputeRegionSecurityPolicyDestroyProducer(t),
+		Steps: []resource.TestStep{
+			{
+				Config: testAccComputeRegionSecurityPolicy_ruleOrderingWithMultipleRules_create(context),
+			},
+			{
+				ResourceName:      "google_compute_region_security_policy.policy",
+				ImportState:       true,
+				ImportStateVerify: true,
+			},
+			{
+				Config: testAccComputeRegionSecurityPolicy_ruleOrderingWithMultipleRules_update(context),
+			},
+			{
+				ResourceName:      "google_compute_region_security_policy.policy",
+				ImportState:       true,
+				ImportStateVerify: true,
+			},
+		},
+	})
+}
+
+func testAccComputeRegionSecurityPolicy_ruleOrderingWithMultipleRules_create(context map[string]interface{}) string {
+	return acctest.Nprintf(`
+
+resource "google_compute_region_security_policy" "policy" {
+	name	    = "tf-test-ordering%{random_suffix}"
+	description = "basic region security policy with multiple rules"
+	type        = "CLOUD_ARMOR"
+	region      = "us-central1"
+
+	rules {
+		action   = "deny"
+		priority = "3000"
+		match {
+			expr {
+			expression = "request.path.matches(\"/login.html\") && token.recaptcha_session.score < 0.2"
+			}
+		}
+	}
+
+	rules {
+		action   = "deny"
+		priority = "2147483647"
+		match {
+			versioned_expr = "SRC_IPS_V1"
+			config {
+				src_ip_ranges = ["*"]
+			}
+		}
+		description = "default rule"
+	}
+}
+
+	`, context)
+}
+
+func testAccComputeRegionSecurityPolicy_ruleOrderingWithMultipleRules_update(context map[string]interface{}) string {
+	return acctest.Nprintf(`
+
+resource "google_compute_region_security_policy" "policy" {
+	name	    = "tf-test-ordering%{random_suffix}"
+	description = "basic region security policy with multiple rules, updated"
+	type        = "CLOUD_ARMOR"
+	region      = "us-central1"
+
+	rules {
+		action   = "allow"
+		priority = "4000"
+		match {
+			expr {
+				expression = "request.path.matches(\"/login.html\") && token.recaptcha_session.score < 0.2"
+			}
+		}
+	}
+
+	rules {
+		action   = "allow"
+		priority = "5000"
+		match {
+			expr {
+				expression = "request.path.matches(\"/404.html\") && token.recaptcha_session.score > 0.4"
+			}
+		}
+		description = "new rule"
+	}
+
+	rules {
+		action   = "deny"
+		priority = "2147483647"
+		match {
+			versioned_expr = "SRC_IPS_V1"
+			config {
+				src_ip_ranges = ["*"]
+			}
+		}
+		description = "default rule"
+	}
+}
+	`, context)
+}
 func TestAccComputeRegionSecurityPolicy_regionSecurityPolicyWithRulesNetworkMatch(t *testing.T) {
 	t.Parallel()