Merge branch 'add_vulnerability_amount_into_sync_rules_for_approval' into 'master'

Add vulnerabilities_allowed into the sync worker

See merge request gitlab-org/gitlab!66679

ee/app/models/approval_project_rule.rb

@@ -25,6 +25,9 @@ class ApprovalProjectRule < ApplicationRecord
   validates :scanners, inclusion: { in: ::Ci::JobArtifact::SECURITY_REPORT_FILE_TYPES }
   default_value_for :scanners, allows_nil: false, value: ::Ci::JobArtifact::SECURITY_REPORT_FILE_TYPES
 
+  validates :vulnerabilities_allowed, numericality: { only_integer: true }
+  default_value_for :vulnerabilities_allowed, allows_nil: false, value: 0
+
   def applies_to_branch?(branch)
     return true if protected_branches.empty?
 

ee/app/models/ee/project.rb

@@ -804,6 +804,10 @@ module EE
         namespace.created_at >= PUBLIC_COST_FACTOR_RELEASE_DAY
     end
 
+    def vulnerability_report_rule
+      approval_rules.vulnerability_reports.first
+    end
+
     private
 
     def github_integration_enabled?

ee/app/services/ci/sync_reports_to_approval_rules_service.rb

@@ -22,6 +22,10 @@ module Ci
         backtrace: error.backtrace
       )
       error("Failed to update approval rules")
+    ensure
+      [:project_rule_vulnerabilities_allowed, :project_vulnerability_rule, :reports].each do |memoization|
+        clear_memoization(memoization)
+      end
     end
 
     private
@@ -55,13 +59,19 @@ module Ci
 
     def reports
       strong_memoize(:reports) do
-        project_vulnerability_rules ? pipeline.security_reports(report_types: project_vulnerability_rules) : []
+        project_vulnerability_rule ? pipeline.security_reports(report_types: project_vulnerability_rule) : []
+      end
+    end
+
+    def project_vulnerability_rule
+      strong_memoize(:project_vulnerability_rule) do
+        pipeline.project.vulnerability_report_rule&.scanners
       end
     end
 
-    def project_vulnerability_rules
-      strong_memoize(:project_vulnerability_rules) do
-        pipeline.project.approval_rules.vulnerability_reports.first&.scanners
+    def project_rule_vulnerabilities_allowed
+      strong_memoize(:project_rule_vulnerabilities_allowed) do
+        pipeline.project.vulnerability_report_rule&.vulnerabilities_allowed
       end
     end
 
@@ -76,7 +86,7 @@ module Ci
 
     def merge_requests_approved_security_reports
       pipeline.merge_requests_as_head_pipeline.reject do |merge_request|
-        reports.present? && reports.violates_default_policy_against?(merge_request.base_pipeline&.security_reports)
+        reports.present? && reports.violates_default_policy_against?(merge_request.base_pipeline&.security_reports, project_rule_vulnerabilities_allowed)
       end
     end
 

ee/lib/gitlab/ci/reports/security/reports.rb

@@ -22,8 +22,8 @@ module Gitlab
             reports.values.flat_map(&:findings)
           end
 
-          def violates_default_policy_against?(target_reports)
-            findings_diff(target_reports).any?(&:unsafe?)
+          def violates_default_policy_against?(target_reports, vulnerabilities_allowed)
+            unsafe_findings_count(target_reports) > vulnerabilities_allowed
           end
 
           private
@@ -31,6 +31,10 @@ module Gitlab
           def findings_diff(target_reports)
             findings - target_reports&.findings.to_a
           end
+
+          def unsafe_findings_count(target_reports)
+            findings_diff(target_reports).count(&:unsafe?)
+          end
         end
       end
     end

ee/spec/lib/gitlab/ci/reports/security/reports_spec.rb

@@ -56,8 +56,9 @@ RSpec.describe Gitlab::Ci::Reports::Security::Reports do
   describe "#violates_default_policy_against?" do
     let(:low_severity_sast) { build(:ci_reports_security_finding, severity: 'low', report_type: :sast) }
     let(:high_severity_dast) { build(:ci_reports_security_finding, severity: 'high', report_type: :dast) }
+    let(:vulnerabilities_allowed) { 0 }
 
-    subject { security_reports.violates_default_policy_against?(target_reports) }
+    subject { security_reports.violates_default_policy_against?(target_reports, vulnerabilities_allowed) }
 
     context 'when the target_reports is `nil`' do
       let(:target_reports) { nil }
@@ -90,6 +91,12 @@ RSpec.describe Gitlab::Ci::Reports::Security::Reports do
         end
 
         it { is_expected.to be(true) }
+
+        context 'with vulnerabilities_allowed higher than the number of new vulnerabilities' do
+          let(:vulnerabilities_allowed) { 10000 }
+
+          it { is_expected.to be(false) }
+        end
       end
 
       context "when none of the reports have a new unsafe vulnerability" do

ee/spec/models/approval_project_rule_spec.rb

@@ -161,17 +161,19 @@ RSpec.describe ApprovalProjectRule do
       context "with a `Vulnerability-Check` rule" do
         using RSpec::Parameterized::TableSyntax
 
-        where(:is_valid, :scanners) do
-          true  | []
-          true  | %w(dast)
-          true  | %w(dast sast)
-          true  | %w(dast dast)
-          false | %w(dast unknown_scanner)
-          false | %w(unknown_scanner)
+        where(:is_valid, :scanners, :vulnerabilities_allowed) do
+          true  | []                       | 0
+          true  | %w(dast)                 | 1
+          true  | %w(dast sast)            | 10
+          true  | %w(dast dast)            | 100
+          false | %w(dast unknown_scanner) | 100
+          false | %w(unknown_scanner)      | 100
+          false | %w(dast sast)            | 1.1
+          false | %w(dast sast)            | 'one'
         end
 
         with_them do
-          let(:vulnerability_check_rule) { build(:approval_project_rule, :vulnerability, scanners: scanners) }
+          let(:vulnerability_check_rule) { build(:approval_project_rule, :vulnerability, scanners: scanners, vulnerabilities_allowed: vulnerabilities_allowed) }
 
           specify { expect(vulnerability_check_rule.valid?).to be(is_valid) }
         end

ee/spec/models/project_spec.rb

@@ -2921,4 +2921,27 @@ RSpec.describe Project do
       end
     end
   end
+
+  describe '#vulnerability_report_rule' do
+    subject { project.vulnerability_report_rule }
+
+    context 'with vulnerability report rule' do
+      let!(:rule) { create(:approval_project_rule, :vulnerability_report, project: project) }
+
+      it { is_expected.to eql(rule) }
+    end
+
+    context 'without vulnerability report rule' do
+      let!(:rule) { create(:approval_project_rule, project: project) }
+
+      it { is_expected.to be_nil }
+    end
+
+    context 'with multiple rules' do
+      let!(:rule) { create(:approval_project_rule, project: project) }
+      let!(:vuln_rule) { create(:approval_project_rule, :vulnerability_report, project: project) }
+
+      it { is_expected.to eql(vuln_rule) }
+    end
+  end
 end

ee/spec/services/ci/sync_reports_to_approval_rules_service_spec.rb

@@ -20,9 +20,10 @@ RSpec.describe Ci::SyncReportsToApprovalRulesService, '#execute' do
   context 'with security rules' do
     let(:report_approver_rule) { create(:report_approver_rule, merge_request: merge_request, approvals_required: 2) }
     let(:scanners) { %w[dependency_scanning] }
+    let(:vulnerabilities_allowed) { 0 }
 
     before do
-      create(:approval_project_rule, :vulnerability, project: project, approvals_required: 2, scanners: scanners)
+      create(:approval_project_rule, :vulnerability, project: project, approvals_required: 2, scanners: scanners, vulnerabilities_allowed: vulnerabilities_allowed)
     end
 
     context 'when there are security reports' do
@@ -58,6 +59,15 @@ RSpec.describe Ci::SyncReportsToApprovalRulesService, '#execute' do
                 .to change { report_approver_rule.reload.approvals_required }.from(2).to(0)
             end
           end
+
+          context 'with the minimum number of vulnerabilities allowed greater than the amount from the security reports' do
+            let(:vulnerabilities_allowed) { 10000 }
+
+            it 'lowers approvals_required count to zero' do
+              expect { subject }
+                .to change { report_approver_rule.reload.approvals_required }.from(2).to(0)
+            end
+          end
         end
 
         context 'when only low-severity vulnerabilities are present' do