Evaluate any_merge_request rules for applicable branch

With this change, approval rules for any_merge_request policies
are correctly applied based on the merge request's target branch
and no violation is created for branches not covered by policy.

ee/app/services/ee/merge_requests/update_service.rb

@@ -36,6 +36,7 @@ def after_update(merge_request, old_associations)
       override :delete_approvals_on_target_branch_change
       def delete_approvals_on_target_branch_change(merge_request)
         delete_approvals(merge_request) if reset_approvals?(merge_request, nil)
+        sync_any_merge_request_approval_rules(merge_request)
       end
 
       def reset_approval_rules(merge_request)
@@ -43,6 +44,13 @@ def reset_approval_rules(merge_request)
 
         merge_request.approval_rules.regular_or_any_approver.delete_all
       end
+
+      def sync_any_merge_request_approval_rules(merge_request)
+        return if ::Feature.disabled?(:scan_result_any_merge_request, merge_request.project)
+        return unless merge_request.approval_rules.any_merge_request.any?
+
+        ::Security::ScanResultPolicies::SyncAnyMergeRequestApprovalRulesWorker.perform_async(merge_request.id)
+      end
     end
   end
 end

ee/app/services/security/scan_result_policies/sync_any_merge_request_rules_service.rb

@@ -25,34 +25,47 @@ def remove_required_approvals
         related_policies = merge_request.project.scan_result_policy_reads.targeting_commits
         return if related_policies.empty?
 
-        violated_policies, unviolated_policies = partition_policies(related_policies)
+        violated_policies_ids, unviolated_policies_ids = evaluate_policy_violations(related_policies)
+
+        violated_rules, unviolated_rules = rules_for_violated_policies(violated_policies_ids)
+        violated_rules, unviolated_rules = update_required_approvals(violated_rules, unviolated_rules)
 
-        violated_rules, _unviolated_rules = update_required_approvals(violated_policies, unviolated_policies)
         generate_policy_bot_comment(merge_request, violated_rules, :any_merge_request)
         log_violated_rules(violated_rules)
-        violations.add(violated_policies.pluck(:id), unviolated_policies.pluck(:id)) # rubocop:disable CodeReuse/ActiveRecord
+        # rubocop:disable CodeReuse/ActiveRecord
+        violations.add(
+          violated_policies_ids - unviolated_rules.pluck(:scan_result_policy_id),
+          unviolated_policies_ids + unviolated_rules.pluck(:scan_result_policy_id)
+        )
+        # rubocop:enable CodeReuse/ActiveRecord
       end
 
-      def partition_policies(scan_result_policy_reads)
+      def evaluate_policy_violations(scan_result_policy_reads)
         has_unsigned_commits = !merge_request.commits(load_from_gitaly: true).all?(&:has_signature?)
-        scan_result_policy_reads.partition do |scan_result_policy_read|
+        violated, unviolated = scan_result_policy_reads.partition do |scan_result_policy_read|
           scan_result_policy_read.commits_any? ||
             (scan_result_policy_read.commits_unsigned? && has_unsigned_commits)
         end
+        [violated.pluck(:id), unviolated.pluck(:id)] # rubocop:disable CodeReuse/ActiveRecord
       end
 
-      def update_required_approvals(violated_policies, unviolated_policies)
+      def rules_for_violated_policies(violated_policies_ids)
         approval_rules = merge_request.approval_rules.any_merge_request
-        violated_rules = approval_rules_for_policies(approval_rules, violated_policies)
-        unviolated_rules = approval_rules_for_policies(approval_rules, unviolated_policies)
+        approval_rules_for_target_branch = approval_rules.applicable_to_branch(merge_request.target_branch)
+
+        violated_rules = approval_rules_for_policies(approval_rules_for_target_branch, violated_policies_ids)
+        unviolated_rules = approval_rules - violated_rules
+
+        [violated_rules, unviolated_rules]
+      end
 
+      def update_required_approvals(violated_rules, unviolated_rules)
         updated_violated_rules = merge_request.reset_required_approvals(violated_rules)
         ApprovalMergeRequestRule.remove_required_approved(unviolated_rules) if unviolated_rules.any?
         [updated_violated_rules, unviolated_rules]
       end
 
-      def approval_rules_for_policies(approval_rules, policies)
-        policies_ids = policies.pluck(:id) # rubocop:disable CodeReuse/ActiveRecord
+      def approval_rules_for_policies(approval_rules, policies_ids)
         approval_rules.select { |rule| policies_ids.include? rule.scan_result_policy_id }
       end
 

ee/app/services/security/security_orchestration_policies/update_violations_service.rb

@@ -9,13 +9,13 @@ class UpdateViolationsService
 
       def initialize(merge_request)
         @merge_request = merge_request
-        @violated_policy_ids = []
-        @unviolated_policy_ids = []
+        @violated_policy_ids = Set.new
+        @unviolated_policy_ids = Set.new
       end
 
       def add(violated_ids, unviolated_ids)
-        violated_policy_ids.concat(violated_ids)
-        unviolated_policy_ids.concat(unviolated_ids)
+        violated_policy_ids.merge(violated_ids)
+        unviolated_policy_ids.merge(unviolated_ids)
       end
 
       def execute

ee/spec/services/ee/merge_requests/update_service_spec.rb

@@ -383,5 +383,54 @@ def update_merge_request(opts)
         it_behaves_like 'not capturing suggested_reviewer_ids'
       end
     end
+
+    describe '#sync_any_merge_request_approval_rules' do
+      let(:opts) { { target_branch: 'feature-2' } }
+      let!(:any_merge_request_approval_rule) do
+        create(:report_approver_rule, :any_merge_request, merge_request: merge_request)
+      end
+
+      subject(:execute) { update_merge_request(opts) }
+
+      it 'enqueues SyncAnyMergeRequestApprovalRulesWorker' do
+        expect(Security::ScanResultPolicies::SyncAnyMergeRequestApprovalRulesWorker).to(
+          receive(:perform_async).with(merge_request.id)
+        )
+
+        execute
+      end
+
+      context 'when target_branch is not changing' do
+        let(:opts) { {} }
+
+        it 'does not enqueue SyncAnyMergeRequestApprovalRulesWorker' do
+          expect(Security::ScanResultPolicies::SyncAnyMergeRequestApprovalRulesWorker).not_to receive(:perform_async)
+
+          execute
+        end
+      end
+
+      context 'without any_merge_request rule' do
+        let!(:any_merge_request_approval_rule) { nil }
+
+        it 'does not enqueue SyncAnyMergeRequestApprovalRulesWorker' do
+          expect(Security::ScanResultPolicies::SyncAnyMergeRequestApprovalRulesWorker).not_to receive(:perform_async)
+
+          execute
+        end
+      end
+
+      context 'when feature flag "scan_result_any_merge_request" is disabled' do
+        before do
+          stub_feature_flags(scan_result_any_merge_request: false)
+        end
+
+        it 'does not enqueue SyncAnyMergeRequestApprovalRulesWorker' do
+          expect(Security::ScanResultPolicies::SyncAnyMergeRequestApprovalRulesWorker).not_to receive(:perform_async)
+
+          execute
+        end
+      end
+    end
   end
 end

ee/spec/services/security/scan_result_policies/sync_any_merge_request_rules_service_spec.rb

@@ -19,12 +19,17 @@
     let(:approvals_required) { 1 }
     let(:signed_commit) { instance_double(Commit, has_signature?: true) }
     let(:unsigned_commit) { instance_double(Commit, has_signature?: false) }
+    let_it_be(:protected_branch) do
+      create(:protected_branch, name: merge_request.target_branch, project: project)
+    end
+
     let_it_be(:scan_result_policy_read, reload: true) do
       create(:scan_result_policy_read, project: project)
     end
 
     let!(:approval_project_rule) do
       create(:approval_project_rule, :any_merge_request, project: project, approvals_required: approvals_required,
+        applies_to_all_protected_branches: true, protected_branches: [protected_branch],
         scan_result_policy_read: scan_result_policy_read)
     end
 
@@ -46,6 +51,12 @@
       end
     end
 
+    shared_examples_for 'creates no violation records' do
+      it 'creates no violation records' do
+        expect { execute }.not_to change { merge_request.scan_result_policy_violations.count }
+      end
+    end
+
     context 'when merge_request is merged' do
       before do
         merge_request.update!(state: 'merged')
@@ -65,10 +76,7 @@
 
           it_behaves_like 'sets approvals_required to 0'
           it_behaves_like 'triggers policy bot comment', :any_merge_request, false
-
-          it 'creates no violation records' do
-            expect { execute }.not_to change { merge_request.scan_result_policy_violations.count }
-          end
+          it_behaves_like 'creates no violation records'
 
           it 'does not create a log' do
             expect(Gitlab::AppJsonLogger).not_to receive(:info)
@@ -76,6 +84,28 @@
             execute
           end
         end
+
+        context 'when target branch is not protected' do
+          before do
+            scan_result_policy_read.update!(commits: :any)
+            merge_request.update!(target_branch: 'non-protected')
+          end
+
+          it_behaves_like 'sets approvals_required to 0'
+          it_behaves_like 'triggers policy bot comment', :any_merge_request, false
+          it_behaves_like 'creates no violation records'
+
+          context 'with previous violation record' do
+            let!(:unrelated_violation) do
+              create(:scan_result_policy_violation, scan_result_policy_read: scan_result_policy_read,
+                merge_request: merge_request)
+            end
+
+            it 'removes the violation record' do
+              expect { execute }.to change { merge_request.scan_result_policy_violations.count }.by(-1)
+            end
+          end
+        end
       end
 
       context 'with violations' do
@@ -156,7 +186,7 @@
 
         context 'when policies target commits' do
           let_it_be(:scan_result_policy_read_with_commits, reload: true) do
-            create(:scan_result_policy_read, project: project, commits: :any)
+            create(:scan_result_policy_read, project: project, commits: :unsigned)
           end
 
           it 'creates violations for policies that have no approval rules' do
@@ -166,6 +196,21 @@
             )
           end
 
+          context 'with previous violation for policy that is now unviolated' do
+            let!(:unrelated_violation) do
+              create(:scan_result_policy_violation, scan_result_policy_read: scan_result_policy_read_with_commits,
+                merge_request: merge_request)
+            end
+
+            before do
+              allow(merge_request).to receive(:commits).and_return([signed_commit])
+            end
+
+            it 'removes the violation record' do
+              expect { execute }.to change { merge_request.scan_result_policy_violations.count }.by(-1)
+            end
+          end
+
           context 'when there are other approval rules' do
             let_it_be(:scan_finding_project_rule) do
               create(:approval_project_rule, :scan_finding, project: project,