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:
PersistPolicyService#update_rearranged_policiesnow propagates the new index to already-linked projects' derived rows, keyed byapproval_policy_rule_id(the real identity), stopping new drift at the source.CreateService#create_scan_result_policynow repairs a stale colliding row (keyed the same way) instead of giving up onRecordInvalid, so drift from any other cause self-heals the next time that slot is touched, rather than requiring the documentedresyncSecurityPoliciesworkaround.
References
Resolves #607745
How to set up and validate locally
- Create a security policy project with two
approval_policyentries (e.g. onescan_finding, onelicense_finding) and link a project to it via a compliance framework. - Reorder the two policies in
policy.ymland merge to the security policy project's default branch (no content change, position only). - Add a third
approval_policyentry topolicy.ymland merge again. - 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", andee/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" = $3Single-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.