Fix enforce field in google_securityposture_posture always being sent…

[steffencircle]

Fix enforce field in google_securityposture_posture always being sent as false

Problem

The enforce attribute within policy_sets[*].policies[*].constraint[*].org_policy_constraint[*].policy_rules[*] is optional and only valid for boolean constraints. For list constraints (which use values, allow_all, or deny_all instead), sending enforce: false to the API causes an error.

The field was previously marked with send_empty_value: true ( added by myself in #16796 ) to allow users to explicitly set enforce = false in their Terraform config (since false is the Go zero value and would otherwise be dropped by the IsEmptyValue guard). However, this introduced a regression: send_empty_value: true works correctly for top-level properties, where the template generates:

} else if v, ok := d.GetOkExists("enforce"); ok || !reflect.DeepEqual(v, enforceProp) {

For nested properties inside arrays, the template instead generates an unconditional bare else:

} else {
    transformed["enforce"] = transformedEnforce
}

At that point in the expand call stack, the value has already been deserialized from Terraform state as a plain Go map[string]interface{}, where an unset TypeBool is indistinguishable from false. The result was that enforce: false was always included in every API request, regardless of whether the user had set it.

Solution

Replace send_empty_value: true with a custom_expand on the orgPolicyConstraint NestedObject. The custom template is fully self-contained (inlining all nested field expansion) because placing custom_expand on a NestedObject prevents the MMv1 template engine from recursively generating child expand functions.

For the enforce field specifically, the custom expander calls d.GetRawConfig() which returns the unprocessed cty value tree from the Terraform configuration. In the cty type system, an omitted optional field is cty.NullVal(cty.Bool), while an explicitly configured enforce = false is cty.False — these are distinguishable. The helper function securitypostureGetEnforceIsSetForOrgPolicyConstraint traverses the raw config tree, locates the matching org_policy_constraint block by canned_constraint_id, and returns a per-rule boolean slice indicating whether enforce was explicitly set. Only rules where it was explicitly set include enforce in the API request body.

Behaviour after this change

User config	API request
enforce omitted	enforce not sent
enforce = true	enforce: true sent
enforce = false	enforce: false sent

Files changed

• Posture.yaml — removed send_empty_value: true from enforce under orgPolicyConstraint.policyRules; added custom_expand reference to the orgPolicyConstraint property
• securityposture_posture_org_policy_constraint.go.tmpl (new file) — custom expand template implementing the above logic; fully self-contained with all nested field expansion inlined
• resource_securityposture_posture_test.go — added a second policy_sets block to both the full and update test configs using the list constraint constraints/gcp.resourceLocations with allowed_values = ["in:us-locations"] and enforce intentionally omitted, directly exercising the regression scenario
• resource_securityposture_posture_deployment_test.go — same list constraint policy_sets block added to both the basic and update configs for the same reason

Release Note Template for Downstream PRs (will be copied)

See Write release notes for guidance.

securityposture_posture: Fix enforce field in google_securityposture_posture always being sent as false 

[github-actions]

Googlers: For automatic test runs see go/terraform-auto-test-runs.

@c2thorn, a repository maintainer, has been assigned to review your changes. If you have not received review feedback within 2 business days, please leave a comment on this PR asking them to take a look.

You can help make sure that review is quick by doing a self-review and by running impacted tests locally.

[github-actions]

@c2thorn This PR has been waiting for review for 3 weekdays. Please take a look! Use the label disable-review-reminders to disable these notifications.

[github-actions]

@GoogleCloudPlatform/terraform-team @c2thorn This PR has been waiting for review for 1 week. Please take a look! Use the label disable-review-reminders to disable these notifications.

[c2thorn]

Given the complexity of the expander, can we add unit testing for securitypostureGetEnforceIsSetForOrgPolicyConstraint?
We can use schema.TestResourceDataRaw to mock the state and verify that the function correctly identifies when enforce is set or omitted, without needing to run full acceptance tests.

[c2thorn]

Should we identify by policy_id instead of canned_constraint_id to handle cases where the same constraint might be used multiple times in a posture?

[c2thorn]

Since there is one constraint per policy?

[c2thorn]

running the build

[modular-magician]

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 3 files changed, 136 insertions(+), 179 deletions(-))
google-beta provider: Diff ( 3 files changed, 136 insertions(+), 179 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 121 insertions(+), 155 deletions(-))

[modular-magician]

Tests analytics

Total tests: 3
Passed tests: 1
Skipped tests: 0
Affected tests: 2

Click here to see the affected service packages

• securityposture

Action taken

Found 2 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests

• TestAccSecurityPosturePostureDeployment_securityposturePostureDeployment_update
• TestAccSecurityposturePosture_securityposturePosture_update

Get to know how VCR tests work

[modular-magician]

🟢 Tests passed during RECORDING mode:
TestAccSecurityPosturePostureDeployment_securityposturePostureDeployment_update [Debug log]
TestAccSecurityposturePosture_securityposturePosture_update [Debug log]

🟢 No issues found for passed tests after REPLAYING rerun.

---

🟢 All tests passed!

View the build log or the debug log for each test

[github-actions]

@steffencircle, this PR is waiting for action from you. If no action is taken, this PR will be closed in 28 days.

Please address any comments or change requests, or re-request review from a core reviewer if no action is required.

This notification can be disabled with the disable-automatic-closure label.

[github-actions]

@steffencircle, this PR is waiting for action from you. If no action is taken, this PR will be closed in 14 days.

Please address any comments or change requests, or re-request review from a core reviewer if no action is required.

This notification can be disabled with the disable-automatic-closure label.

[github-actions]

@steffencircle, this PR is waiting for action from you. If no action is taken, this PR will be closed in 2 weekdays.

Please address any comments or change requests, or re-request review from a core reviewer if no action is required.

This notification can be disabled with the disable-automatic-closure label.

[steffencircle]

Changes addressing reviewer feedback

1. Disambiguate by policy_id instead of canned_constraint_id

File: securityposture_posture_org_policy_constraint.go.tmpl

Added a new helper securitypostureGetPolicyIdForCannedConstraint that resolves the owning policy_id from the raw cty config before looking up the per-rule enforce state. The existing securitypostureGetEnforceIsSetForOrgPolicyConstraint now takes (d, policyId, cannedConstraintId) and filters by policy_id first when it is available, falling back to canned_constraint_id-only matching when it cannot be resolved.

This correctly handles the case where the same canned constraint (e.g. constraints/iam.allowedPolicyMemberDomains) is reused across multiple policies in a posture — previously the first matching constraint would have been returned for all of them.

---

2. Unit tests for the helper functions

File: resource_securityposture_posture_unit_test.go (new)

Added package securityposture (internal) unit tests for both helper functions. Rather than using schema.TestResourceDataRaw (which does not populate the raw config), the tests use a minimal rawConfigMock that satisfies tpgresource.TerraformResourceData and returns a hand-crafted cty.Value tree from GetRawConfig(). This accurately reflects the runtime behaviour since enforce omitted in config is cty.NullVal(cty.Bool), while enforce = false is cty.False — the distinction the helpers rely on.

15 sub-tests across two table-driven functions:

TestSecuritypostureGetPolicyIdForCannedConstraint:

• Matching policy found
• No matching constraint returns ""
• Second of two policies matches
• Null org_policy_constraint is skipped
• Null raw config returns ""
• Duplicate canned_constraint_id returns first match

TestSecuritypostureGetEnforceIsSetForOrgPolicyConstraint:

• enforce = true explicitly set
• enforce = false explicitly set (must still be sent to the API)
• enforce omitted (null) — list constraint, must not be sent
• Multiple rules with mixed enforce settings
• Same canned constraint in two policies — correct policy selected by policy_id (regression case for the original bug)
• Same constraint, other policy selected
• No matching policy returns nil
• Null raw config returns nil
• Empty policy_id falls back to canned_constraint_id-only matching

All 15 tests pass locally.

[github-actions]

@c2thorn This PR has been waiting for review for 3 weekdays. Please take a look! Use the label disable-review-reminders to disable these notifications.

[github-actions]

@GoogleCloudPlatform/terraform-team @c2thorn This PR has been waiting for review for 1 week. Please take a look! Use the label disable-review-reminders to disable these notifications.

[github-actions]

@GoogleCloudPlatform/terraform-team @c2thorn This PR has been waiting for review for 2 weeks. Please take a look! Use the label disable-review-reminders to disable these notifications.

[c2thorn]

Thanks for the back and forth on this issue, @steffencircle. Before you put more into the custom_expand version, I took another look at the problem, and I think there's a simpler approach that avoids the duplicate-constraint bug instead of having to handle it.

With custom_expand on the orgPolicyConstraint NestedObject, the expander runs once per constraint with no positional context: the block it gets carries only canned_constraint_id, not the policy_id or policy_set it came from. So it has to map back to the raw config by constraint id, and that mapping can't be made correct.

securitypostureGetPolicyIdForCannedConstraint returns the first policy matching a constraint id (lines 159-161), so when the same canned_constraint_id appears in two policies, both expander calls resolve to the same policyId, and the second policy inherits the first's enforce state (42-43 → 196-201). The policy_id argument isn't disambiguating anything: it produces the same result as the policyId == "" fallback, so it adds code without fixing the duplicate case. The unit tests only pass because they pass policyId in directly, which the expander can't do.

The other cost is maintenance. custom_expand on a NestedObject turns off the generated child expanders, so the template has to inline all of them. Any field added to policyRules, values, or condition upstream later gets silently dropped from requests, with nothing to flag it.

"Don't send enforce for list constraints" is easier on the outgoing request than during expansion, because an encoder sees the finished body in place and doesn't need to recover user intent. Keep send_empty_value: true so explicit enforce = true/false on boolean constraints still works (#16796), and drop enforce from any rule that's a list constraint:
Using encoder

// rough sketch
func resourceSecurityposturePostureEncoder(d *schema.ResourceData, meta interface{}, obj map[string]interface{}) (map[string]interface{}, error) {
    for _, ps := range obj["policySets"].([]interface{}) {
        for _, p := range ps.(map[string]interface{})["policies"].([]interface{}) {
            constraint, _ := p.(map[string]interface{})["constraint"].(map[string]interface{})
            opc, _ := constraint["orgPolicyConstraint"].(map[string]interface{})
            rules, _ := opc["policyRules"].([]interface{})
            for _, r := range rules {
                rule := r.(map[string]interface{})
                _, isList := rule["values"]
                _, hasAllow := rule["allowAll"]
                _, hasDeny := rule["denyAll"]
                if isList || hasAllow || hasDeny {
                    delete(rule, "enforce") // enforce is only valid for boolean constraints
                }
            }
        }
    }
    return obj, nil
}

No GetRawConfig, no inlined expanders, no per-instance correlation, and it stays correct as new fields get added since the generated expanders are still used.

Boolean constraint with enforce omitted will still send enforce: false, same as the released #16796 behavior, rather than dropping it. false is a valid default there, so I think that's fine.

Let me know what you think.

[c2thorn]

see #17147 (comment)

I think we need to switch to a smaller encoder vs this large expander

[averbuks]

securityposture_posture: Fix enforce field in google_securityposture_posture always being sent as false

Hi @c2thorn, I did implement the change you proposed in a separate PR