Remove deprecate_scan_result_policies from approval enforcement

What does this MR do and why?

This is part 1 of a 3-MR stack splitting !251778 (closed), which Danger flagged as too large. This MR targets master and merges first; MR 2 targets this MR's branch, and MR 3 (the closer that deletes the flag definition) targets MR 2's branch.

It collapses Feature.enabled?(:deprecate_scan_result_policies, ...) branches in the approval-rule enforcement read paths to the enabled path, so evaluation prefers Security::ApprovalPolicyRule in these call sites. Production still has approval rules with scan_result_policy_id set but approval_policy_rule_id NULL (roughly 20k approval_merge_request_rules and 4k approval_project_rules rows, see !252700 (comment 3772878675)), so the scan_result_policy_read fallbacks for those rows are kept unchanged. Only the flag checks are removed; the flag definition itself is not removed here.

Changes:

  • MergeRequest#policy_approval_settings now always reads approval settings from Security::Policy content via violations; deletes the private policy_approval_settings_from_scan_result_policy_reads method.
  • ApprovalWrappedRule#policy_violation looks up the violation by approval_policy_rule_id.
  • ApprovalProjectRule#delete_approver_rules_and_violations deletes violations by approval_policy_rule_id, falling back to scan_result_policy_id when the rule has no approval_policy_rule_id.
  • Security::ApprovalPolicySource drops use_approval_policy_rule? and resolves through approval_policy_rule || scan_result_policy_read, which is what the flag-enabled path already did. The read remains the fallback source for rules without an approval_policy_rule_id until #617802.
  • MergeRequests::Mergeability::CheckSecurityPolicyViolationsService collapsed to the enabled path, which still checks scan_result_policy_reads_through_approval_rules so the merge gate stays active for legacy-only rules.
  • Removes the FF-disabled spec contexts in 5 spec files, and converts read-backed fixtures to approval_policy_rule-backed ones in denied_licenses_checker_spec and approval_wrapped_rule_spec.

References

Zero-downtime deployment safety

The flag is explicitly enabled on GitLab.com, so old-version nodes still evaluating Feature.enabled? take the same enabled path during a rolling deploy. Approval rules with scan_result_policy_id set but approval_policy_rule_id NULL still exist in production, so the scan_result_policy_read fallbacks are kept unchanged in Security::ApprovalPolicySource, ApprovalProjectRule#delete_approver_rules_and_violations, and CheckSecurityPolicyViolationsService. Old and new nodes therefore behave identically for those rows as well. No migrations or schema changes are involved.

MR acceptance checklist

This checklist encourages us to confirm any changes were analyzed for conformity with our guidelines. Please evaluate this MR against the MR acceptance checklist.

Edited by Sashi Kumar Kumaresan

Merge request reports

Loading
Loading