Reuse orphaned scan result policy reads on approval rule sync

Sashi Kumar Kumaresanrequested to merge
625031-reuse-orphaned-scan-result-policy-read into master

What does this MR do and why?

Security::ScanResultPolicies::ApprovalRules::CreateService#create_rule creates a Security::ScanResultPolicyRead row per approval policy rule and action, then an approval_project_rules row pointing at it.

Before this change, the service returned early whenever a read already existed for the same approval_policy_rule_id and action_idx, without checking whether the approval rule itself existed. That guard was added in !192033 (merged) for #538759 (closed), to protect against a race with the legacy Security::ProcessScanResultPolicyWorker. That worker is gone, and this service is now the only creator of reads.

A read can outlive its approval rule when Security::Policy#delete_approval_policy_rules_for_project is interrupted between its deletion passes (see #625031 (closed)). With the early return in place, every later sync skipped that rule permanently: the approval rule was never recreated and nothing was logged. The self-healing added in !252506 (closed) only covers reads whose approval_policy_rule_id differs or is nil, not this same-rule-id case.

create_rule now calls a new private method, find_or_create_scan_result_policy. If a read exists for the rule and action index, it is refreshed via update! with the current params and reused; otherwise a new read is created as before. Refreshing matters because an orphaned read can carry stale policy content, for example an old fallback_behavior. The rest of the flow is unchanged: create_approval_rule? still decides whether an approval_project_rules row gets created (not for any_merge_request rules without approval actions), license policies are synced, and ::ApprovalRules::CreateService creates the rule against the read. The early return for a fully synced rule (read and approval rule both present) is unchanged.

This complements !253938 (merged), which stops new orphans from being produced by the delete path. This MR repairs orphans that already exist, or that arise from other interruptions.

Known limitation, out of scope: two Security::SyncProjectPolicyWorker jobs for the same project and policy can run concurrently, since delayed enqueues bypass the worker's deduplication. If both pass the approval-rule check before either creates the rule, the second ::ApprovalRules::CreateService call fails the ApprovalProjectRule name uniqueness validation and gets logged by log_service_failure, same as today. Serializing those jobs is a follow-up.

This touches create_scan_result_policy in the same area as !252506 (closed); whichever lands second needs a small rebase.

References

Screenshots or screen recordings

Not applicable. This is a backend-only change with no UI impact.

How to set up and validate locally

  1. In a Rails console, for a project linked to an approval policy, find the project's Security::ScanResultPolicyRead for one rule and delete its approval rule: ApprovalProjectRule.where(scan_result_policy_id: read.id).delete_all.
  2. Run Security::ScanResultPolicies::ApprovalRules::CreateService.new(project:, security_policy:, approval_policy_rules: security_policy.approval_policy_rules.undeleted, author: security_policy.security_orchestration_policy_configuration.policy_last_updated_by).execute.
  3. Confirm a new approval_project_rules row exists with scan_result_policy_id == read.id, and that the read count is unchanged (the existing read was reused, not duplicated).

Automated coverage: ee/spec/services/security/scan_result_policies/approval_rules/create_service_spec.rb. The context "when scan_result_policy_read was already created" now covers three cases: the approval rule also exists (nothing created or changed), the approval rule is missing (read reused, approval rule created against it, stale fallback_behavior refreshed from the policy), and an any_merge_request rule with no approval actions (read reused, no approval rule created).

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.

Merge request reports