Sync orchestration_policy_idx when a policy is reordered

What does this MR do and why?

Compliance-framework-scoped approval policies can go silently missing from the MR approval widget on projects that were already enrolled in the framework, after later edits to the security policy project's policy.yml. Newly enrolled projects always get the full, correct policy set.

Root cause: repositioning a policy within policy.yml (which happens on almost any add/remove/reorder of entries in the array) updates that policy's own policy_index, but PersistPolicyService never diffs or publishes an event for a pure position change. So Security::SyncPolicyWorker never runs for it, and projects already linked to that policy never get their derived orchestration_policy_idx (stored on Security::ScanResultPolicyRead and ApprovalProjectRule) refreshed to match.

That stale index becomes a hard failure later: those two tables carry a DB-level unique index keyed in part by orchestration_policy_idx. When a genuinely new policy is added and lands on the now-freed index, creating its ScanResultPolicyRead collides with the stale row and raises ActiveRecord::RecordInvalid. The existing Sidekiq handling for that error retries once via a resync that only touches rows already owned by the new policy's own rules, then discards — the new policy's approval rule for that project is dropped, silently, permanently.

Fix, two parts:

  1. PersistPolicyService#update_rearranged_policies now propagates the new index to already-linked projects' derived rows, keyed by approval_policy_rule_id (the real identity), stopping new drift at the source.
  2. CreateService#create_scan_result_policy now repairs a stale colliding row (keyed the same way) instead of giving up on RecordInvalid, so drift from any other cause self-heals the next time that slot is touched, rather than requiring the documented resyncSecurityPolicies workaround.

References

Resolves #607745

How to set up and validate locally

  1. Create a security policy project with two approval_policy entries (e.g. one scan_finding, one license_finding) and link a project to it via a compliance framework.
  2. Reorder the two policies in policy.yml and merge to the security policy project's default branch (no content change, position only).
  3. Add a third approval_policy entry to policy.yml and merge again.
  4. Before this fix: the third policy's approval rule silently fails to create for the already-linked project (see ee/spec/services/security/scan_result_policies/approval_rules/create_service_spec.rb, context "when a stale read from another policy occupies the same slot", and ee/spec/services/security/security_orchestration_policies/persist_policy_service_spec.rb, context "when a project is already linked to the reordered policies").

Database review

This MR adds three update_all bulk-update queries, all keyed on the already-indexed approval_policy_rule_id column and touching at most a handful of rows (a policy's own rules, and only the projects already linked to it):

1. ee/app/services/security/security_orchestration_policies/persist_policy_service.rb#L156

Security::ScanResultPolicyRead.for_approval_policy_rules(rules).update_all(orchestration_policy_idx: new_index)
UPDATE "scan_result_policies" SET "orchestration_policy_idx" = $1
WHERE "scan_result_policies"."approval_policy_rule_id" IN
  (SELECT "approval_policy_rules"."id" FROM "approval_policy_rules" WHERE "approval_policy_rules"."security_policy_id" = $2)

Local dev-DB EXPLAIN (small table, 29 rows - included for query shape only, not scale):

Hash Join  (cost=5.27..7.64 rows=1 width=362)
  Hash Cond: (scan_result_policies.approval_policy_rule_id = approval_policy_rules.id)
  ->  Seq Scan on scan_result_policies  (cost=0.00..2.29 rows=29 width=362)
  ->  Hash  (cost=5.22..5.22 rows=4 width=8)
        ->  Index Only Scan using idx_approval_policy_rules_security_policy_id_id on approval_policy_rules  (cost=0.15..5.22 rows=4 width=8)
              Index Cond: (security_policy_id = 1)

At production scale scan_result_policies is queried via index_scan_result_policies_on_approval_policy_rule_id (already exists, unrelated to this change) rather than a seq scan; the planner only chose seq scan here because the dev table has 29 rows. Requesting a Database Lab run against production-scale data to confirm.

2. Same file, #L157

ApprovalProjectRule.for_approval_policy_rules(rules).update_all(orchestration_policy_idx: new_index)
UPDATE "approval_project_rules" SET "orchestration_policy_idx" = $1
WHERE "approval_project_rules"."approval_policy_rule_id" IN
  (SELECT "approval_policy_rules"."id" FROM "approval_policy_rules" WHERE "approval_policy_rules"."security_policy_id" = $2)

Local dev-DB EXPLAIN:

Nested Loop  (cost=0.30..14.03 rows=2 width=205)
  ->  Index Only Scan using idx_approval_policy_rules_security_policy_id_id on approval_policy_rules  (cost=0.15..5.22 rows=4 width=8)
        Index Cond: (security_policy_id = 1)
  ->  Index Scan using index_approval_project_rules_on_approval_policy_rule_id on approval_project_rules  (cost=0.15..2.18 rows=2 width=205)
        Index Cond: (approval_policy_rule_id = approval_policy_rules.id)

3. ee/app/services/security/scan_result_policies/approval_rules/create_service.rb#L101

ApprovalProjectRule.for_project(project.id).for_approval_policy_rules(stale_rule).update_all(orchestration_policy_idx: correct_index)
UPDATE "approval_project_rules" SET "orchestration_policy_idx" = $1
WHERE "approval_project_rules"."project_id" = $2 AND "approval_project_rules"."approval_policy_rule_id" = $3

Single-row lookup on index_approval_project_rules_on_approval_policy_rule_id (or the project_id-leading index if more selective); no plan captured separately, shape is the same pattern as query 2 with an extra equality filter.

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Alan (Maciej) Paruszewski

Merge request reports

Loading
Loading