Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changelog/13068.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
orgpolicy: fix permadiff in `google_org_policy_policy` when multiple rules are present
```
71 changes: 65 additions & 6 deletions google-beta/services/orgpolicy/resource_org_policy_policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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": {
Expand Down Expand Up @@ -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": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
},
},
})
}
Expand Down Expand Up @@ -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)
}

Expand Down
Loading