Add a custom diff function for org_policy rules to fix permadiff (#13068) (#9611)

[upstream:733d7d1b8ff4a1dea7630caa225f62f45f7f92fd]

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

Author: modular-magician

.changelog/13068.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+orgpolicy: fix permadiff in `google_org_policy_policy`  when multiple rules are present
+```

google-beta/services/orgpolicy/resource_org_policy_policy.go

@@ -42,6 +42,63 @@ func resourceOrgpolicyPolicyRulesConditionExpressionDiffSuppress(_, old, new str
 	newReplaced := strings.ReplaceAll(strings.ReplaceAll(new, "Labels", "TagId"), "label", "tag")
 	return oldReplaced == newReplaced
 }
+func resourceOrgPolicyPolicySpecRulesDiffSuppress(k, o, n string, d *schema.ResourceData) bool {
+	oldCount, newCount := d.GetChange("spec.0.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("spec.0.rules.%d", i))
+
+		if o != nil {
+			old = append(old, o)
+		}
+		if n != nil {
+			new = append(new, n)
+		}
+	}
+
+	oldSet := schema.NewSet(schema.HashResource(ResourceOrgPolicyPolicy().Schema["spec"].Elem.(*schema.Resource).Schema["rules"].Elem.(*schema.Resource)), old)
+	newSet := schema.NewSet(schema.HashResource(ResourceOrgPolicyPolicy().Schema["spec"].Elem.(*schema.Resource).Schema["rules"].Elem.(*schema.Resource)), new)
+
+	return oldSet.Equal(newSet)
+}
+
+func resourceOrgPolicyPolicyDryRunSpecRulesDiffSuppress(k, o, n string, d *schema.ResourceData) bool {
+	oldCount, newCount := d.GetChange("dry_run_spec.0.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("dry_run_spec.0.rules.%d", i))
+
+		if o != nil {
+			old = append(old, o)
+		}
+		if n != nil {
+			new = append(new, n)
+		}
+	}
+
+	oldSet := schema.NewSet(schema.HashResource(ResourceOrgPolicyPolicy().Schema["dry_run_spec"].Elem.(*schema.Resource).Schema["rules"].Elem.(*schema.Resource)), old)
+	newSet := schema.NewSet(schema.HashResource(ResourceOrgPolicyPolicy().Schema["dry_run_spec"].Elem.(*schema.Resource).Schema["rules"].Elem.(*schema.Resource)), new)
+
+	return oldSet.Equal(newSet)
+}
 
 func ResourceOrgPolicyPolicy() *schema.Resource {
 	return &schema.Resource{
@@ -93,9 +150,10 @@ func ResourceOrgPolicyPolicy() *schema.Resource {
 							Description: `Ignores policies set above this resource and restores the 'constraint_default' enforcement behavior of the specific constraint at this resource. This field can be set in policies for either list or boolean constraints. If set, 'rules' must be empty and 'inherit_from_parent' must be set to false.`,
 						},
 						"rules": {
-							Type:        schema.TypeList,
-							Optional:    true,
-							Description: `In policies for boolean constraints, the following requirements apply: - There must be one and only one policy rule where condition is unset. - Boolean policy rules with conditions must set 'enforced' to the opposite of the policy rule without a condition. - During policy evaluation, policy rules with conditions that are true for a target resource take precedence.`,
+							Type:             schema.TypeList,
+							Optional:         true,
+							DiffSuppressFunc: resourceOrgPolicyPolicyDryRunSpecRulesDiffSuppress,
+							Description:      `In policies for boolean constraints, the following requirements apply: - There must be one and only one policy rule where condition is unset. - Boolean policy rules with conditions must set 'enforced' to the opposite of the policy rule without a condition. - During policy evaluation, policy rules with conditions that are true for a target resource take precedence.`,
 							Elem: &schema.Resource{
 								Schema: map[string]*schema.Schema{
 									"allow_all": {
@@ -211,9 +269,10 @@ func ResourceOrgPolicyPolicy() *schema.Resource {
 							Description: `Ignores policies set above this resource and restores the 'constraint_default' enforcement behavior of the specific 'Constraint' at this resource. This field can be set in policies for either list or boolean constraints. If set, 'rules' must be empty and 'inherit_from_parent' must be set to false.`,
 						},
 						"rules": {
-							Type:        schema.TypeList,
-							Optional:    true,
-							Description: `In Policies for boolean constraints, the following requirements apply: - There must be one and only one PolicyRule where condition is unset. - BooleanPolicyRules with conditions must set 'enforced' to the opposite of the PolicyRule without a condition. - During policy evaluation, PolicyRules with conditions that are true for a target resource take precedence.`,
+							Type:             schema.TypeList,
+							Optional:         true,
+							DiffSuppressFunc: resourceOrgPolicyPolicySpecRulesDiffSuppress,
+							Description:      `In Policies for boolean constraints, the following requirements apply: - There must be one and only one PolicyRule where condition is unset. - BooleanPolicyRules with conditions must set 'enforced' to the opposite of the PolicyRule without a condition. - During policy evaluation, PolicyRules with conditions that are true for a target resource take precedence.`,
 							Elem: &schema.Resource{
 								Schema: map[string]*schema.Schema{
 									"allow_all": {

google-beta/services/orgpolicy/resource_org_policy_policy_test.go

@@ -140,6 +140,15 @@ func TestAccOrgPolicyPolicy_ProjectPolicy(t *testing.T) {
 				ImportStateVerify:       true,
 				ImportStateVerifyIgnore: []string{"name", "spec.0.rules.0.condition.0.expression"},
 			},
+			{
+				Config: testAccOrgPolicyPolicy_ProjectPolicyUpdate1(context),
+			},
+			{
+				ResourceName:            "google_org_policy_policy.primary",
+				ImportState:             true,
+				ImportStateVerify:       true,
+				ImportStateVerifyIgnore: []string{"name", "spec.0.rules.0.condition.0.expression"},
+			},
 		},
 	})
 }
@@ -383,6 +392,39 @@ resource "google_project" "basic" {
 }
 
 
+`, context)
+}
+
+func testAccOrgPolicyPolicy_ProjectPolicyUpdate1(context map[string]interface{}) string {
+	return acctest.Nprintf(`
+resource "google_org_policy_policy" "primary" {
+  name   = "projects/${google_project.basic.name}/policies/gcp.resourceLocations"
+  parent = "projects/${google_project.basic.name}"
+
+  spec {
+	rules {
+	  deny_all = "TRUE"
+	}
+    rules {
+      allow_all = "TRUE"
+      condition {
+        description = "A sample condition for the policy"
+        expression  = "resource.matchTagId('tagKeys/123', 'tagValues/345')"
+        location    = "sample-location.log"
+        title       = "sample-condition"
+      }
+    }
+  }
+}
+
+resource "google_project" "basic" {
+  project_id = "tf-test-id%{random_suffix}"
+  name       = "tf-test-id%{random_suffix}"
+  org_id     = "%{org_id}"
+  deletion_policy = "DELETE"
+}
+
+
 `, context)
 }