Merge branch 'create_project_rules_on_scan_result_policies' into 'master'

Create approval project rules

See merge request gitlab-org/gitlab!70632

ee/app/models/approval_project_rule.rb

@@ -20,6 +20,8 @@ class ApprovalProjectRule < ApplicationRecord
     any_approver: 3
   }
 
+  scope :report_approver_without_scan_finding, -> { report_approver.where.not(report_type: :scan_finding) }
+
   alias_method :code_owner, :code_owner?
   validate :validate_default_license_report_name, on: :update, if: :report_approver?
 
@@ -50,11 +52,8 @@ class ApprovalProjectRule < ApplicationRecord
   end
 
   def apply_report_approver_rules_to(merge_request)
-    rule = merge_request
-      .approval_rules
-      .report_approver
-      .find_or_initialize_by(report_type: report_type)
-    rule.update!(attributes_to_apply_for(report_type))
+    rule = merge_request_report_approver_rule(merge_request)
+    rule.update!(report_approver_attributes)
     rule
   end
 
@@ -68,7 +67,7 @@ class ApprovalProjectRule < ApplicationRecord
 
   private
 
-  def attributes_to_apply_for(report_type)
+  def report_approver_attributes
     attributes
       .slice('approvals_required', 'name')
       .merge(
@@ -86,4 +85,20 @@ class ApprovalProjectRule < ApplicationRecord
 
     errors.add(:name, _("cannot be modified"))
   end
+
+  def merge_request_report_approver_rule(merge_request)
+    if scan_finding?
+      merge_request
+        .approval_rules
+        .report_approver
+        .joins(:approval_merge_request_rule_source)
+        .where(approval_merge_request_rule_source: { approval_project_rule_id: self.id })
+        .first_or_initialize
+    else
+      merge_request
+        .approval_rules
+        .report_approver
+        .find_or_initialize_by(report_type: report_type)
+    end
+  end
 end

ee/app/models/ee/project.rb

@@ -510,7 +510,7 @@ module EE
     def visible_approval_rules(target_branch: nil)
       rules = strong_memoize(:visible_approval_rules) do
         Hash.new do |h, key|
-          h[key] = visible_user_defined_rules(branch: key) + approval_rules.report_approver
+          h[key] = visible_user_defined_rules(branch: key) + approval_rules.report_approver_without_scan_finding
         end
       end
 

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

@@ -12,7 +12,6 @@ module Security
       def execute
         policy_configuration.delete_all_schedules
         create_new_schedule_rules
-        policy_configuration.update!(configured_at: Time.current)
       end
 
       private

ee/app/workers/security/create_orchestration_policy_worker.rb

@@ -25,6 +25,17 @@ module Security
               .new(policy_configuration: configuration, policy_index: policy_index, policy: policy)
               .execute
           end
+
+          configuration.transaction do
+            configuration.approval_rules.scan_finding.delete_all
+            configuration.active_scan_result_policies.each do |policy|
+              Security::SecurityOrchestrationPolicies::ProcessScanResultPolicyService
+                .new(policy_configuration: configuration, policy: policy)
+                .execute
+            end
+          end
+
+          configuration.update!(configured_at: Time.current)
         end
       end
     end

ee/spec/factories/approval_rules.rb

@@ -79,5 +79,11 @@ FactoryBot.define do
       rule_type { :report_approver }
       report_type { :code_coverage }
     end
+
+    trait :scan_finding do
+      sequence(:name) { |n| "Scan finding #{n}" }
+      rule_type { :report_approver }
+      report_type { :scan_finding }
+    end
   end
 end

ee/spec/models/approval_project_rule_spec.rb

@@ -125,9 +125,10 @@ RSpec.describe ApprovalProjectRule do
     end
 
     where(:default_name, :report_type) do
-      'Vulnerability-Check' | :vulnerability
-      'License-Check'       | :license_scanning
-      'Coverage-Check'      | :code_coverage
+      'Vulnerability-Check'  | :vulnerability
+      'License-Check'        | :license_scanning
+      'Coverage-Check'       | :code_coverage
+      'Scan finding example' | :scan_finding
     end
 
     context "when there is a project rule for each report type" do

ee/spec/models/project_spec.rb

@@ -3313,4 +3313,12 @@ RSpec.describe Project do
       it { is_expected.to eql(vuln_rule) }
     end
   end
+
+  describe '#visible_approval_rules' do
+    let(:scan_finding_rule) { create(:approval_project_rule, project: project, report_type: :scan_finding) }
+
+    subject { project.visible_approval_rules }
+
+    it { is_expected.not_to include(scan_finding_rule) }
+  end
 end

ee/spec/services/security/security_orchestration_policies/process_rule_service_spec.rb

@@ -32,7 +32,6 @@ RSpec.describe Security::SecurityOrchestrationPolicies::ProcessRuleService do
         service.execute
 
         new_schedule = Security::OrchestrationPolicyRuleSchedule.first
-        expect(policy_configuration.configured_at).not_to be_nil
         expect(Security::OrchestrationPolicyRuleSchedule.count).to eq(1)
         expect(new_schedule.id).not_to eq(schedule.id)
         expect(new_schedule.rule_index).to eq(1)
@@ -45,7 +44,6 @@ RSpec.describe Security::SecurityOrchestrationPolicies::ProcessRuleService do
 
       it 'deletes schedules' do
         expect { service.execute }.to change(Security::OrchestrationPolicyRuleSchedule, :count).by(-1)
-        expect(policy_configuration.configured_at).not_to be_nil
       end
     end
   end

ee/spec/workers/security/create_orchestration_policy_worker_spec.rb

@@ -4,12 +4,13 @@ require 'spec_helper'
 
 RSpec.describe Security::CreateOrchestrationPolicyWorker do
   describe '#perform' do
-    let_it_be(:configuration) { create(:security_orchestration_policy_configuration) }
+    let_it_be(:configuration) { create(:security_orchestration_policy_configuration, configured_at: nil) }
     let_it_be(:schedule) { create(:security_orchestration_policy_rule_schedule, security_orchestration_policy_configuration: configuration) }
 
     before do
       allow_next_instance_of(Repository) do |repository|
-        allow(repository).to receive(:blob_data_at).and_return({ scan_execution_policy: active_policies }.to_yaml)
+        allow(repository).to receive(:blob_data_at).and_return(active_policies.to_yaml)
+        allow(repository).to receive(:last_commit_for_path)
       end
     end
 
@@ -17,54 +18,117 @@ RSpec.describe Security::CreateOrchestrationPolicyWorker do
 
     context 'when policy is valid' do
       let(:active_policies) do
-        [
-          {
-            name: 'Scheduled DAST 1',
-            description: 'This policy runs DAST for every 20 mins',
-            enabled: true,
-            rules: [{ type: 'schedule', branches: %w[production], cadence: '*/20 * * * *' }],
-            actions: [
-              { scan: 'dast', site_profile: 'Site Profile', scanner_profile: 'Scanner Profile' }
-            ]
-          },
-          {
-            name: 'Scheduled DAST 2',
-            description: 'This policy runs DAST for every 20 mins',
-            enabled: true,
-            rules: [{ type: 'schedule', branches: %w[production], cadence: '*/20 * * * *' }],
-            actions: [
-              { scan: 'dast', site_profile: 'Site Profile', scanner_profile: 'Scanner Profile' }
-            ]
-          }
-        ]
+        {
+          scan_execution_policy:
+          [
+            {
+              name: 'Scheduled DAST 1',
+              description: 'This policy runs DAST for every 20 mins',
+              enabled: true,
+              rules: [{ type: 'schedule', branches: %w[production], cadence: '*/20 * * * *' }],
+              actions: [
+                { scan: 'dast', site_profile: 'Site Profile', scanner_profile: 'Scanner Profile' }
+              ]
+            },
+            {
+              name: 'Scheduled DAST 2',
+              description: 'This policy runs DAST for every 20 mins',
+              enabled: true,
+              rules: [{ type: 'schedule', branches: %w[production], cadence: '*/20 * * * *' }],
+              actions: [
+                { scan: 'dast', site_profile: 'Site Profile', scanner_profile: 'Scanner Profile' }
+              ]
+            }
+          ],
+          scan_result_policy:
+          [
+            {
+              name: 'CS critical policy',
+              description: 'This policy with CS for critical policy',
+              enabled: true,
+              rules: [{ type: 'scan_finding', branches: %w[production], vulnerabilities_allowed: 0, severity_levels: %w[critical], scanners: %w[container_scanning] }],
+              actions: [
+                { type: 'require_approval', approvals_required: 1, approvers: %w[admin] }
+              ]
+            }
+          ]
+        }
       end
 
-      it 'executes the process rule service' do
-        active_policies.each_with_index do |policy, policy_index|
+      it 'executes process services for all policies' do
+        active_policies[:scan_execution_policy].each_with_index do |policy, policy_index|
           expect_next_instance_of(Security::SecurityOrchestrationPolicies::ProcessRuleService,
                                   policy_configuration: configuration, policy_index: policy_index, policy: policy) do |service|
             expect(service).to receive(:execute)
           end
         end
 
+        active_policies[:scan_result_policy].each do |policy|
+          expect_next_instance_of(Security::SecurityOrchestrationPolicies::ProcessScanResultPolicyService,
+                                  policy_configuration: configuration, policy: policy) do |service|
+            expect(service).to receive(:execute)
+          end
+        end
+
+        expect(configuration.configured_at).to be_nil
         expect { worker.perform }.not_to change(Security::OrchestrationPolicyRuleSchedule, :count)
+        expect(configuration.reload.configured_at).not_to be_nil
+      end
+
+      context 'with existing project approval rules' do
+        let!(:approval_rule) { create(:approval_project_rule, :scan_finding, project: configuration.project )}
+
+        before do
+          allow_next_instance_of(Security::SecurityOrchestrationPolicies::ProcessRuleService) do |rule_service|
+            allow(rule_service).to receive(:execute)
+          end
+          allow_next_instance_of(Security::SecurityOrchestrationPolicies::ProcessScanResultPolicyService) do |rule_service|
+            allow(rule_service).to receive(:execute)
+          end
+        end
+
+        it 'deletes all approval_rules' do
+          expect { worker.perform }.to change(configuration.approval_rules, :count).by(-1)
+        end
       end
     end
 
     context 'when policy is invalid' do
       let(:active_policies) do
-        [
-          {
-            key: 'invalid',
-            label: 'invalid'
-          }
-        ]
+        {
+          scan_execution_policy:
+          [
+            {
+              key: 'invalid',
+              label: 'invalid'
+            }
+          ]
+        }
       end
 
-      it 'does not execute process rule service' do
+      it 'does not execute process for any policy' do
         expect(Security::SecurityOrchestrationPolicies::ProcessRuleService).not_to receive(:new)
+        expect(Security::SecurityOrchestrationPolicies::ProcessScanResultPolicyService).not_to receive(:new)
 
         expect { worker.perform }.to change(Security::OrchestrationPolicyRuleSchedule, :count).by(-1)
+        expect(configuration.reload.configured_at).to be_nil
+      end
+
+      context 'with existing project approval rules' do
+        let!(:approval_rule) { create(:approval_project_rule, :scan_finding, project: configuration.project )}
+
+        before do
+          allow_next_instance_of(Security::SecurityOrchestrationPolicies::ProcessRuleService) do |rule_service|
+            allow(rule_service).to receive(:execute)
+          end
+          allow_next_instance_of(Security::SecurityOrchestrationPolicies::ProcessScanResultPolicyService) do |rule_service|
+            allow(rule_service).to receive(:execute)
+          end
+        end
+
+        it 'does not delete the existing approval_rules' do
+          expect { worker.perform }.not_to change(configuration.approval_rules, :count)
+        end
       end
     end
   end