Merge branch 'eb-code-coverage-approvals' into 'master'

Add backend support for Coverage-Check rule

See merge request gitlab-org/gitlab!63079

app/models/ci/build.rb

@@ -205,6 +205,8 @@ module Ci
     end
 
     scope :with_coverage, -> { where.not(coverage: nil) }
+    scope :without_coverage, -> { where(coverage: nil) }
+    scope :with_coverage_regex, -> { where.not(coverage_regex: nil) }
 
     scope :for_project, -> (project_id) { where(project_id: project_id) }
 

app/models/ci/pipeline.rb

@@ -644,6 +644,10 @@ module Ci
       end
     end
 
+    def update_builds_coverage
+      builds.with_coverage_regex.without_coverage.each(&:update_coverage)
+    end
+
     def batch_lookup_report_artifact_for_file_type(file_type)
       latest_report_artifacts
         .values_at(*::Ci::JobArtifact.associated_file_types_for(file_type.to_s))

db/migrate/20210526155257_rename_sync_security_report_approval_rules_sidekiq_queue.rb

+# frozen_string_literal: true
+
+class RenameSyncSecurityReportApprovalRulesSidekiqQueue < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def up
+    sidekiq_queue_migrate 'sync_security_reports_to_report_approval_rules', to: 'ci_sync_reports_to_report_approval_rules'
+  end
+
+  def down
+    sidekiq_queue_migrate 'ci_sync_reports_to_report_approval_rules', to: 'sync_security_reports_to_report_approval_rules'
+  end
+end

db/schema_migrations/20210526155257

+ec4cd687062118b30e516ed7c36677dda056f25c4d96c6ee0b503e457b5a18d4
\ No newline at end of file

ee/app/models/approval_merge_request_rule.rb

@@ -56,11 +56,13 @@ class ApprovalMergeRequestRule < ApplicationRecord
 
   enum report_type: {
     vulnerability: 1,
-    license_scanning: 2
+    license_scanning: 2,
+    code_coverage: 3
   }
 
   scope :vulnerability_report, -> { report_approver.vulnerability }
   scope :license_compliance, -> { report_approver.license_scanning }
+  scope :coverage, -> { report_approver.code_coverage }
   scope :with_head_pipeline, -> { includes(merge_request: [:head_pipeline]) }
   scope :open_merge_requests, -> { merge(MergeRequest.opened) }
   scope :for_checks_that_can_be_refreshed, -> { license_compliance.open_merge_requests.with_head_pipeline }

ee/app/models/concerns/approval_rule_like.rb

@@ -6,9 +6,11 @@ module ApprovalRuleLike
   DEFAULT_NAME = 'Default'
   DEFAULT_NAME_FOR_LICENSE_REPORT = 'License-Check'
   DEFAULT_NAME_FOR_VULNERABILITY_REPORT = 'Vulnerability-Check'
+  DEFAULT_NAME_FOR_COVERAGE = 'Coverage-Check'
   REPORT_TYPES_BY_DEFAULT_NAME = {
     DEFAULT_NAME_FOR_LICENSE_REPORT => :license_scanning,
-    DEFAULT_NAME_FOR_VULNERABILITY_REPORT => :vulnerability
+    DEFAULT_NAME_FOR_VULNERABILITY_REPORT => :vulnerability,
+    DEFAULT_NAME_FOR_COVERAGE => :code_coverage
   }.freeze
   APPROVALS_REQUIRED_MAX = 100
   ALL_MEMBERS = 'All Members'

ee/app/models/ee/ci/pipeline.rb

@@ -65,7 +65,12 @@ module EE
             pipeline.run_after_commit do
               StoreSecurityReportsWorker.perform_async(pipeline.id) if pipeline.default_branch?
               ::Security::StoreScansWorker.perform_async(pipeline.id)
-              SyncSecurityReportsToReportApprovalRulesWorker.perform_async(pipeline.id)
+            end
+          end
+
+          after_transition any => ::Ci::Pipeline.completed_statuses do |pipeline|
+            pipeline.run_after_commit do
+              ::Ci::SyncReportsToReportApprovalRulesWorker.perform_async(pipeline.id)
             end
           end
 

ee/app/services/security/sync_reports_to_approval_rules_service.rb → ee/app/services/ci/sync_reports_to_approval_rules_service.rb

 # frozen_string_literal: true
 
-module Security
-  # Service for syncing security reports results to report_approver approval rules
-  #
+module Ci
   class SyncReportsToApprovalRulesService < ::BaseService
     def initialize(pipeline)
       @pipeline = pipeline
@@ -11,6 +9,7 @@ module Security
     def execute
       sync_license_scanning_rules
       sync_vulnerability_rules
+      sync_coverage_rules
       success
     rescue StandardError => error
       log_error(
@@ -42,14 +41,30 @@ module Security
       # If we don't have reports, then we should wait until pipeline stops.
       return if reports.empty? && !pipeline.complete?
 
-      remove_required_approvals_for(ApprovalMergeRequestRule.vulnerability_report, sync_required_merge_requests)
+      remove_required_approvals_for(ApprovalMergeRequestRule.vulnerability_report, merge_requests_approved_security_reports)
+    end
+
+    def sync_coverage_rules
+      return unless pipeline.complete?
+
+      pipeline.update_builds_coverage
+      remove_required_approvals_for(ApprovalMergeRequestRule.code_coverage, merge_requests_approved_coverage)
     end
 
     def reports
       @reports ||= pipeline.security_reports
     end
 
-    def sync_required_merge_requests
+    def merge_requests_approved_coverage
+      pipeline.merge_requests_as_head_pipeline.reject do |merge_request|
+        base_pipeline = merge_request.base_pipeline
+
+        # if base pipeline is missing we just default to not require approval.
+        pipeline.coverage < base_pipeline.coverage if base_pipeline.present?
+      end
+    end
+
+    def merge_requests_approved_security_reports
       pipeline.merge_requests_as_head_pipeline.reject do |merge_request|
         reports.violates_default_policy_against?(merge_request.base_pipeline&.security_reports)
       end

ee/app/services/ee/merge_requests/after_create_service.rb

@@ -15,7 +15,7 @@ module EE
       private
 
       def schedule_sync_for(pipeline_id)
-        ::SyncSecurityReportsToReportApprovalRulesWorker.perform_async(pipeline_id) if pipeline_id
+        ::Ci::SyncReportsToReportApprovalRulesWorker.perform_async(pipeline_id) if pipeline_id
       end
     end
   end

ee/app/workers/all_queues.yml

@@ -773,6 +773,15 @@
   :weight: 1
   :idempotent:
   :tags: []
+- :name: pipeline_background:ci_sync_reports_to_report_approval_rules
+  :worker_name: Ci::SyncReportsToReportApprovalRulesWorker
+  :feature_category: :continuous_integration
+  :has_external_dependencies:
+  :urgency: :high
+  :resource_boundary: :cpu
+  :weight: 1
+  :idempotent:
+  :tags: []
 - :name: pipeline_default:ci_trigger_downstream_subscriptions
   :worker_name: Ci::TriggerDownstreamSubscriptionsWorker
   :feature_category: :continuous_integration
@@ -819,15 +828,6 @@
   :weight: 2
   :idempotent:
   :tags: []
-- :name: security_scans:sync_security_reports_to_report_approval_rules
-  :worker_name: SyncSecurityReportsToReportApprovalRulesWorker
-  :feature_category: :vulnerability_management
-  :has_external_dependencies:
-  :urgency: :high
-  :resource_boundary: :cpu
-  :weight: 2
-  :idempotent:
-  :tags: []
 - :name: todos_destroyer:todos_destroyer_confidential_epic
   :worker_name: TodosDestroyer::ConfidentialEpicWorker
   :feature_category: :epics

ee/app/workers/ci/sync_reports_to_report_approval_rules_worker.rb

+# frozen_string_literal: true
+
+# Worker for syncing report_type approval_rules approvals_required
+module Ci
+  class SyncReportsToReportApprovalRulesWorker # rubocop:disable Scalability/IdempotentWorker
+    include ApplicationWorker
+
+    sidekiq_options retry: 3
+    include PipelineBackgroundQueue
+
+    urgency :high
+    worker_resource_boundary :cpu
+
+    def perform(pipeline_id)
+      pipeline = Ci::Pipeline.find_by_id(pipeline_id)
+      return unless pipeline
+
+      ::Ci::SyncReportsToApprovalRulesService.new(pipeline).execute
+    end
+  end
+end

ee/app/workers/sync_security_reports_to_report_approval_rules_worker.rb

-# frozen_string_literal: true
-
-# Worker for syncing report_type approval_rules approvals_required
-#
-class SyncSecurityReportsToReportApprovalRulesWorker # rubocop:disable Scalability/IdempotentWorker
-  include ApplicationWorker
-
-  sidekiq_options retry: 3
-  include SecurityScansQueue
-
-  urgency :high
-  worker_resource_boundary :cpu
-
-  def perform(pipeline_id)
-    pipeline = Ci::Pipeline.find_by_id(pipeline_id)
-    return unless pipeline
-
-    ::Security::SyncReportsToApprovalRulesService.new(pipeline).execute
-  end
-end

ee/spec/factories/approval_rules.rb

@@ -28,10 +28,20 @@ FactoryBot.define do
       approvals_required { rand(1..ApprovalProjectRule::APPROVALS_REQUIRED_MAX) }
     end
 
+    trait :vulnerability do
+      name { ApprovalRuleLike::DEFAULT_NAME_FOR_VULNERABILITY_REPORT }
+      report_type { :vulnerability }
+    end
+
     trait :license_scanning do
       name { ApprovalRuleLike::DEFAULT_NAME_FOR_LICENSE_REPORT }
       report_type { :license_scanning }
     end
+
+    trait :code_coverage do
+      name { ApprovalRuleLike::DEFAULT_NAME_FOR_COVERAGE }
+      report_type { :code_coverage }
+    end
   end
 
   factory :any_approver_rule, parent: :approval_merge_request_rule do
@@ -61,5 +71,10 @@ FactoryBot.define do
       name { ApprovalRuleLike::DEFAULT_NAME_FOR_LICENSE_REPORT }
       rule_type { :report_approver }
     end
+
+    trait :code_coverage do
+      name { ApprovalRuleLike::DEFAULT_NAME_FOR_COVERAGE }
+      rule_type { :report_approver }
+    end
   end
 end

ee/spec/models/approval_merge_request_rule_spec.rb

@@ -115,6 +115,8 @@ RSpec.describe ApprovalMergeRequestRule, factory_default: :keep do
     let!(:css_rule) { create(:code_owner_rule, name: '*.css') }
     let!(:approval_rule) { create(:approval_merge_request_rule) }
     let!(:report_approver_rule) { create(:report_approver_rule) }
+    let!(:coverage_rule) { create(:report_approver_rule, :code_coverage) }
+    let!(:license_rule) { create(:report_approver_rule, :license_scanning) }
 
     describe '.not_matching_pattern' do
       it 'returns the correct rules' do
@@ -143,6 +145,20 @@ RSpec.describe ApprovalMergeRequestRule, factory_default: :keep do
           .to contain_exactly(report_approver_rule)
       end
     end
+
+    describe '.license_compliance' do
+      it 'returns the correct rules' do
+        expect(described_class.license_compliance)
+          .to contain_exactly(license_rule)
+      end
+    end
+
+    describe '.coverage' do
+      it 'returns the correct rules' do
+        expect(described_class.coverage)
+          .to contain_exactly(coverage_rule)
+      end
+    end
   end
 
   describe '.find_or_create_code_owner_rule' do

ee/spec/models/ci/pipeline_spec.rb

@@ -353,6 +353,22 @@ RSpec.describe Ci::Pipeline do
   end
 
   describe 'state machine transitions' do
+    context 'on pipeline complete' do
+      let(:pipeline) { create(:ci_empty_pipeline, status: from_status) }
+
+      Ci::HasStatus::ACTIVE_STATUSES.each do |status|
+        context "from #{status}" do
+          let(:from_status) { status }
+
+          it 'schedules Ci::SyncReportsToReportApprovalRulesWorker' do
+            expect(Ci::SyncReportsToReportApprovalRulesWorker).to receive(:perform_async).with(pipeline.id)
+
+            pipeline.succeed
+          end
+        end
+      end
+    end
+
     context 'when pipeline has downstream bridges' do
       before do
         pipeline.downstream_bridges << create(:ci_bridge)

ee/spec/services/ci/sync_reports_to_approval_rules_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Ci::SyncReportsToApprovalRulesService, '#execute' do
+  let_it_be(:project) { create(:project, :public, :repository) }
+
+  let(:merge_request) { create(:merge_request, source_project: project) }
+  let(:pipeline) { create(:ee_ci_pipeline, :success, project: project, merge_requests_as_head_pipeline: [merge_request]) }
+  let(:base_pipeline) { create(:ee_ci_pipeline, :success, project: project, ref: merge_request.target_branch, sha: merge_request.diff_base_sha) }
+
+  subject(:sync_rules) { described_class.new(pipeline).execute }
+
+  before do
+    allow(Ci::Pipeline).to receive(:find).with(pipeline.id) { pipeline }
+
+    stub_licensed_features(dependency_scanning: true, dast: true, license_scanning: true)
+  end
+
+  context 'with security rules' do
+    let(:report_approver_rule) { create(:report_approver_rule, merge_request: merge_request, approvals_required: 2) }
+
+    context 'when there are security reports' do
+      context 'when pipeline passes' do
+        context 'when high-severity vulnerabilities are present' do
+          before do
+            create(:ee_ci_build, :success, :dependency_scanning, name: 'ds_job', pipeline: pipeline, project: project)
+          end
+
+          context 'when high-severity vulnerabilities already present in target branch pipeline' do
+            before do
+              create(:ee_ci_build, :success, :dependency_scanning, name: 'ds_job', pipeline: base_pipeline, project: project)
+            end
+
+            it 'lowers approvals_required count to zero' do
+              expect { subject }
+                .to change { report_approver_rule.reload.approvals_required }.from(2).to(0)
+            end
+          end
+
+          context 'when high-severity vulnerabilities do not present in target branch pipeline' do
+            it "won't change approvals_required count" do
+              expect { subject }
+                .not_to change { report_approver_rule.reload.approvals_required }
+            end
+          end
+        end
+
+        context 'when only low-severity vulnerabilities are present' do
+          before do
+            create(:ee_ci_build, :success, :low_severity_dast_report, name: 'dast_job', pipeline: pipeline, project: project)
+          end
+
+          it 'lowers approvals_required count to zero' do
+            expect { subject }
+              .to change { report_approver_rule.reload.approvals_required }.from(2).to(0)
+          end
+        end
+
+        context 'when merge_requests are merged' do
+          let!(:merge_request) { create(:merge_request, :merged, source_project: project) }
+
+          before do
+            create(:ee_ci_build, :success, :dast, name: 'dast_job', pipeline: pipeline, project: project)
+          end
+
+          it "won't change approvals_required count" do
+            expect { subject }
+              .not_to change { report_approver_rule.reload.approvals_required }
+          end
+        end
+
+        context "license compliance policy" do
+          let!(:license_compliance_rule) { create(:report_approver_rule, :license_scanning, merge_request: merge_request, approvals_required: 1) }
+
+          before do
+            stub_feature_flags(drop_license_management_artifact: false)
+          end
+
+          context "when a license violates the license compliance policy" do
+            let!(:software_license_policy) { create(:software_license_policy, :denied, project: project, software_license: denied_license) }
+            let(:denied_license) { create(:software_license, name: license_name) }
+            let(:license_name) { ci_build.pipeline.license_scanning_report.license_names[0] }
+
+            context 'with a new report' do
+              let!(:ci_build) { create(:ee_ci_build, :success, :license_scanning, pipeline: pipeline, project: project) }
+
+              specify { expect { subject }.not_to change { license_compliance_rule.reload.approvals_required } }
+              specify { expect(subject[:status]).to be(:success) }
+            end
+
+            context 'with an old report' do
+              let!(:ci_build) { create(:ee_ci_build, :success, :license_management, pipeline: pipeline, project: project) }
+
+              specify { expect { subject }.not_to change { license_compliance_rule.reload.approvals_required } }
+              specify { expect(subject[:status]).to be(:success) }
+            end
+          end
+
+          context "when no licenses violate the license compliance policy" do
+            context 'with a new report' do
+              let!(:ci_build) { create(:ee_ci_build, :success, :license_scanning, pipeline: pipeline, project: project) }
+
+              specify { expect { subject }.to change { license_compliance_rule.reload.approvals_required }.from(1).to(0) }
+              specify { expect(subject[:status]).to be(:success) }
+            end
+
+            context 'with an old report' do
+              let!(:ci_build) { create(:ee_ci_build, :success, :license_management, pipeline: pipeline, project: project) }
+
+              before do
+                stub_feature_flags(drop_license_management_artifact: false)
+              end
+
+              specify { expect { subject }.to change { license_compliance_rule.reload.approvals_required }.from(1).to(0) }
+              specify { expect(subject[:status]).to be(:success) }
+            end
+          end
+
+          context "when an unexpected error occurs" do
+            before do
+              allow_next_instance_of(Gitlab::Ci::Reports::LicenseScanning::Report) do |instance|
+                allow(instance).to receive(:violates?).and_raise('heck')
+              end
+            end
+
+            specify { expect(subject[:status]).to be(:error) }
+            specify { expect(subject[:message]).to eql("Failed to update approval rules") }
+          end
+        end
+      end
+
+      context 'when pipeline fails' do
+        before do
+          pipeline.update!(status: :failed)
+        end
+
+        context 'when high-severity vulnerabilities are present' do
+          before do
+            create(:ee_ci_build, :success, :dependency_scanning, name: 'ds_job', pipeline: pipeline, project: project)
+          end
+
+          context 'when high-severity vulnerabilities already present in target branch pipeline' do
+            before do
+              create(:ee_ci_build, :success, :dependency_scanning, name: 'ds_job', pipeline: base_pipeline, project: project)
+            end
+
+            it 'lowers approvals_required count to zero' do
+              expect { subject }
+                .to change { report_approver_rule.reload.approvals_required }.from(2).to(0)
+            end
+          end
+
+          context 'when high-severity vulnerabilities do not present in target branch pipeline' do
+            it "won't change approvals_required count" do
+              expect { subject }
+                .not_to change { report_approver_rule.reload.approvals_required }
+            end
+          end
+        end
+
+        context 'when only low-severity vulnerabilities are present' do
+          before do
+            create(:ee_ci_build, :success, :low_severity_dast_report, name: 'dast_job', pipeline: pipeline, project: project)
+          end
+
+          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
+    end
+
+    context 'without security reports' do
+      let(:pipeline) { create(:ci_pipeline, :running, project: project, merge_requests_as_head_pipeline: [merge_request]) }
+
+      it "won't change approvals_required count" do
+        expect { subject }
+          .not_to change { report_approver_rule.reload.approvals_required }
+      end
+
+      context "license compliance policy" do
+        let!(:software_license_policy) { create(:software_license_policy, :denied, project: project, software_license: denied_license) }
+        let!(:license_compliance_rule) { create(:report_approver_rule, :license_scanning, merge_request: merge_request, approvals_required: 1) }
+        let!(:denied_license) { create(:software_license) }
+
+        specify { expect { subject }.not_to change { license_compliance_rule.reload.approvals_required } }
+        specify { expect(subject[:status]).to be(:success) }
+      end
+    end
+  end
+
+  context 'with code coverage rules' do
+    let!(:head_pipeline_builds) do
+      [
+        create(:ci_build, :success, :trace_with_coverage, trace_coverage: 60.0, pipeline: pipeline),
+        create(:ci_build, :success, :trace_with_coverage, trace_coverage: 80.0, pipeline: pipeline),
+        create(:ci_build, :success, coverage: nil, pipeline: pipeline),
+        create(:ci_build, :success, coverage: 40.0, pipeline: pipeline)
+      ]
+    end
+
+    let!(:report_approver_rule) { create(:report_approver_rule, :code_coverage, merge_request: merge_request, approvals_required: 2) }
+
+    context 'when pipeline is complete' do
+      before do
+        allow(pipeline).to receive(:complete?).and_return(true)
+      end
+
+      context 'and head pipeline coverage is lower than base pipeline coverage' do
+        let!(:base_pipeline_builds) do
+          [
+            create(:ci_build, :success, coverage: 90.0, pipeline: base_pipeline),
+            create(:ci_build, :success, coverage: 100.0, pipeline: base_pipeline)
+          ]
+        end
+
+        it "won't lower approvals_required count" do
+          expect { sync_rules }
+            .not_to change { report_approver_rule.reload.approvals_required }
+        end
+      end
+
+      context 'and head pipeline coverage is higher than base pipeline coverage' do
+        let!(:base_pipeline_builds) do
+          [
+            create(:ci_build, :success, coverage: 60.0, pipeline: base_pipeline),
+            create(:ci_build, :success, coverage: 80.0, pipeline: base_pipeline),
+            create(:ci_build, :success, coverage: 30.0, pipeline: base_pipeline)
+          ]
+        end
+
+        it "lowers approvals_required count" do
+          expect { sync_rules }
+            .to change { report_approver_rule.reload.approvals_required }.from(2).to(0)
+        end
+
+        context 'when MR is merged' do
+          let!(:merge_request) { create(:merge_request, :merged, source_project: project) }
+
+          it "won't change approvals_required count" do
+            expect { subject }
+              .not_to change { report_approver_rule.reload.approvals_required }
+          end
+        end
+      end
+
+      context 'and head pipeline coverage is the same as base pipeline coverage' do
+        let!(:base_pipeline_builds) do
+          [
+            create(:ci_build, :success, coverage: 60.0, pipeline: base_pipeline),
+            create(:ci_build, :success, coverage: 80.0, pipeline: base_pipeline),
+            create(:ci_build, :success, coverage: 40.0, pipeline: base_pipeline)
+          ]
+        end
+
+        it "lowers approvals_required count" do
+          expect { sync_rules }
+            .to change { report_approver_rule.reload.approvals_required }.from(2).to(0)
+        end
+      end
+    end
+
+    context 'when pipeline is incomplete' do
+      let!(:base_pipeline_builds) do
+        [
+          create(:ci_build, :success, coverage: 40.0, pipeline: base_pipeline),
+          create(:ci_build, :success, coverage: 30.0, pipeline: base_pipeline)
+        ]
+      end
+
+      before do
+        allow(pipeline).to receive(:complete?).and_return(false)
+      end
+
+      it "won't lower approvals_required count" do
+        expect { sync_rules }
+          .not_to change { report_approver_rule.reload.approvals_required }
+      end
+    end
+
+    context 'when base pipeline is missing' do
+      before do
+        allow(pipeline).to receive(:complete?).and_return(true)
+      end
+
+      it "lowers approvals_required count" do
+        expect { sync_rules }
+          .to change { report_approver_rule.reload.approvals_required }.from(2).to(0)
+      end
+    end
+  end
+end

ee/spec/services/ee/merge_requests/after_create_service_spec.rb

@@ -11,7 +11,7 @@ RSpec.describe MergeRequests::AfterCreateService do
     subject(:execute) { service_object.execute(merge_request) }
 
     before do
-      allow(SyncSecurityReportsToReportApprovalRulesWorker).to receive(:perform_async)
+      allow(Ci::SyncReportsToReportApprovalRulesWorker).to receive(:perform_async)
     end
 
     context 'when the merge request has actual_head_pipeline' do
@@ -26,7 +26,7 @@ RSpec.describe MergeRequests::AfterCreateService do
         execute
 
         expect(merge_request).to have_received(:update_head_pipeline).ordered
-        expect(SyncSecurityReportsToReportApprovalRulesWorker).to have_received(:perform_async).ordered.with(pipeline_id)
+        expect(Ci::SyncReportsToReportApprovalRulesWorker).to have_received(:perform_async).ordered.with(pipeline_id)
       end
     end
 
@@ -34,7 +34,7 @@ RSpec.describe MergeRequests::AfterCreateService do
       it 'does not schedule a background job to sync security reports to approval rules' do
         execute
 
-        expect(SyncSecurityReportsToReportApprovalRulesWorker).not_to have_received(:perform_async)
+        expect(Ci::SyncReportsToReportApprovalRulesWorker).not_to have_received(:perform_async)
       end
     end
   end

ee/spec/services/merge_requests/sync_report_approver_approval_rules_spec.rb

@@ -12,87 +12,64 @@ RSpec.describe MergeRequests::SyncReportApproverApprovalRules do
       stub_licensed_features(report_approver_rules: true)
     end
 
-    context "when a project has a single `#{ApprovalProjectRule::DEFAULT_NAME_FOR_VULNERABILITY_REPORT}` approval rule" do
-      let!(:vulnerability_approval_project_rule) { create(:approval_project_rule, :vulnerability_report, project: merge_request.target_project, approvals_required: 2) }
-
-      context 'when report_approver_rules are enabled' do
+    ApprovalRuleLike::REPORT_TYPES_BY_DEFAULT_NAME.keys.each do |default_name|
+      context "when a project has a single `#{default_name}` approval rule" do
+        let(:report_type) { ApprovalRuleLike::REPORT_TYPES_BY_DEFAULT_NAME[default_name] }
+        let!(:report_approval_project_rule) { create(:approval_project_rule, report_type, project: merge_request.target_project, approvals_required: 2) }
         let!(:regular_approval_project_rule) { create(:approval_project_rule, project: merge_request.target_project) }
 
+        context 'when report_approver_rules are enabled' do
           it 'creates rule for report approvers' do
             expect { service.execute }
-            .to change { merge_request.approval_rules.vulnerability_report.count }.from(0).to(1)
+              .to change { merge_request.approval_rules.where(name: default_name).count }.from(0).to(1)
 
-          rule = merge_request.approval_rules.vulnerability_report.first
+            rule = merge_request.approval_rules.find_by(name: default_name)
 
             expect(rule).to be_report_approver
-          expect(rule.report_type).to eq 'vulnerability'
-          expect(rule.name).to eq(vulnerability_approval_project_rule.name)
-          expect(rule.approval_project_rule).to eq(vulnerability_approval_project_rule)
+            expect(rule.report_type).to eq(report_type.to_s)
+            expect(rule.name).to eq(report_approval_project_rule.name)
+            expect(rule.approvals_required).to eq(report_approval_project_rule.approvals_required)
+            expect(rule.approval_project_rule).to eq(report_approval_project_rule)
           end
 
-        it 'updates previous rules if defined' do
-          mr_rule = create(:report_approver_rule, merge_request: merge_request, approvals_required: 0)
+          it 'updates previous report approval rule if defined' do
+            previous_rule = create(:report_approver_rule, report_type, merge_request: merge_request, approvals_required: 0)
 
             expect { service.execute }
-            .not_to change { merge_request.approval_rules.vulnerability_report.count }
+              .not_to change { merge_request.approval_rules.where(name: default_name).count }
 
-          expect(mr_rule.reload).to be_report_approver
-          expect(mr_rule.report_type).to eq 'vulnerability'
-          expect(mr_rule.name).to eq(vulnerability_approval_project_rule.name)
-          expect(mr_rule.approvals_required).to eq vulnerability_approval_project_rule.approvals_required
-          expect(mr_rule.approval_project_rule).to eq(vulnerability_approval_project_rule)
-        end
-      end
+            expect(previous_rule.reload).to be_report_approver
+            expect(previous_rule.report_type).to eq(report_type.to_s)
+            expect(previous_rule.name).to eq(report_approval_project_rule.name)
+            expect(previous_rule.approvals_required).to eq(report_approval_project_rule.approvals_required)
+            expect(previous_rule.approval_project_rule).to eq(report_approval_project_rule)
           end
-
-    context "when a project has a single `#{ApprovalProjectRule::DEFAULT_NAME_FOR_LICENSE_REPORT}` approval rule" do
-      let!(:project_rule) { create(:approval_project_rule, :license_scanning, project: merge_request.target_project) }
-
-      context "when the rule has not been synchronized to the merge request yet" do
-        let(:result) { merge_request.reload.approval_rules.last }
-
-        before do
-          service.execute
         end
-
-        specify { expect(merge_request.reload.approval_rules.count).to be(1) }
-        specify { expect(result).to be_report_approver }
-        specify { expect(result.report_type).to eq('license_scanning') }
-        specify { expect(result.name).to eq(project_rule.name) }
-        specify { expect(result.approval_project_rule).to eq(project_rule) }
-        specify { expect(result.approvals_required).to eql(project_rule.approvals_required) }
-      end
-
-      context "when the rule had previously been synchronized" do
-        let!(:previous_rule) { create(:report_approver_rule, :license_scanning, merge_request: merge_request) }
-
-        before do
-          service.execute
-        end
-
-        specify { expect(merge_request.reload.approval_rules.count).to be(1) }
-        specify { expect(merge_request.reload.approval_rules[0]).to eql(previous_rule) }
       end
     end
 
     context "when a project has multiple report approval rules" do
       let!(:vulnerability_project_rule) { create(:approval_project_rule, :vulnerability_report, project: merge_request.target_project) }
       let!(:license_compliance_project_rule) { create(:approval_project_rule, :license_scanning, project: merge_request.target_project) }
+      let!(:coverage_project_rule) { create(:approval_project_rule, :code_coverage, project: merge_request.target_project) }
 
       context "when none of the rules have been synchronized to the merge request yet" do
         let(:vulnerability_check_rule) { merge_request.reload.approval_rules.vulnerability_report.last }
-        let(:license_check_rule) { merge_request.reload.approval_rules.find_by(name: ApprovalProjectRule::DEFAULT_NAME_FOR_LICENSE_REPORT) }
+        let(:license_check_rule) { merge_request.reload.approval_rules.license_compliance.last }
+        let(:coverage_check_rule) { merge_request.reload.approval_rules.coverage.last }
 
         before do
           vulnerability_project_rule.users << create(:user)
           vulnerability_project_rule.groups << create(:group)
           license_compliance_project_rule.users << create(:user)
           license_compliance_project_rule.groups << create(:group)
+          coverage_project_rule.users << create(:user)
+          coverage_project_rule.groups << create(:group)
 
           service.execute
         end
 
-        specify { expect(merge_request.reload.approval_rules.count).to be(2) }
+        specify { expect(merge_request.reload.approval_rules.count).to be(3) }
         specify { expect(vulnerability_check_rule).to be_report_approver }
         specify { expect(vulnerability_check_rule.approvals_required).to eql(vulnerability_project_rule.approvals_required) }
         specify { expect(vulnerability_check_rule).to be_vulnerability }
@@ -103,6 +80,11 @@ RSpec.describe MergeRequests::SyncReportApproverApprovalRules do
         specify { expect(license_check_rule).to be_license_scanning }
         specify { expect(license_check_rule.name).to eq(license_compliance_project_rule.name) }
         specify { expect(license_check_rule.approval_project_rule).to eq(license_compliance_project_rule) }
+        specify { expect(coverage_check_rule).to be_report_approver }
+        specify { expect(coverage_check_rule.approvals_required).to eql(coverage_project_rule.approvals_required) }
+        specify { expect(coverage_check_rule).to be_code_coverage }
+        specify { expect(coverage_check_rule.name).to eq(coverage_project_rule.name) }
+        specify { expect(coverage_check_rule.approval_project_rule).to eq(coverage_project_rule) }
       end
 
       context "when some of the rules have been synchronized to the merge request" do
@@ -112,9 +94,10 @@ RSpec.describe MergeRequests::SyncReportApproverApprovalRules do
           service.execute
         end
 
-        specify { expect(merge_request.reload.approval_rules.count).to be(2) }
+        specify { expect(merge_request.reload.approval_rules.count).to be(3) }
         specify { expect(merge_request.reload.approval_rules.vulnerability_report.count).to be(1) }
-        specify { expect(merge_request.reload.approval_rules.where(report_type: :license_scanning)).to match_array([previous_rule]) }
+        specify { expect(merge_request.reload.approval_rules.coverage.count).to be(1) }
+        specify { expect(merge_request.reload.approval_rules.license_compliance).to match_array([previous_rule]) }
       end
     end
 

ee/spec/services/security/sync_reports_to_approval_rules_service_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe Security::SyncReportsToApprovalRulesService, '#execute' do
-  let(:merge_request) { create(:merge_request) }
-  let(:project) { merge_request.project }
-  let(:pipeline) { create(:ee_ci_pipeline, :success, project: project, merge_requests_as_head_pipeline: [merge_request]) }
-  let(:report_approver_rule) { create(:report_approver_rule, merge_request: merge_request, approvals_required: 2) }
-  let(:base_pipeline) { create(:ee_ci_pipeline, :success, project: project, ref: merge_request.target_branch, sha: merge_request.diff_base_sha) }
-
-  subject { described_class.new(pipeline).execute }
-
-  before do
-    allow(Ci::Pipeline).to receive(:find).with(pipeline.id) { pipeline }
-
-    stub_licensed_features(dependency_scanning: true, dast: true, license_scanning: true)
-  end
-
-  context 'when there are reports' do
-    context 'when pipeline passes' do
-      context 'when high-severity vulnerabilities are present' do
-        before do
-          create(:ee_ci_build, :success, :dependency_scanning, name: 'ds_job', pipeline: pipeline, project: project)
-        end
-
-        context 'when high-severity vulnerabilities already present in target branch pipeline' do
-          before do
-            create(:ee_ci_build, :success, :dependency_scanning, name: 'ds_job', pipeline: base_pipeline, project: project)
-          end
-
-          it 'lowers approvals_required count to zero' do
-            expect { subject }
-              .to change { report_approver_rule.reload.approvals_required }.from(2).to(0)
-          end
-        end
-
-        context 'when high-severity vulnerabilities do not present in target branch pipeline' do
-          it "won't change approvals_required count" do
-            expect { subject }
-              .not_to change { report_approver_rule.reload.approvals_required }
-          end
-        end
-      end
-
-      context 'when only low-severity vulnerabilities are present' do
-        before do
-          create(:ee_ci_build, :success, :low_severity_dast_report, name: 'dast_job', pipeline: pipeline, project: project)
-        end
-
-        it 'lowers approvals_required count to zero' do
-          expect { subject }
-            .to change { report_approver_rule.reload.approvals_required }.from(2).to(0)
-        end
-      end
-
-      context 'when merge_requests are merged' do
-        let!(:merge_request) { create(:merge_request, :merged) }
-
-        before do
-          create(:ee_ci_build, :success, :dast, name: 'dast_job', pipeline: pipeline, project: project)
-        end
-
-        it "won't change approvals_required count" do
-          expect { subject }
-            .not_to change { report_approver_rule.reload.approvals_required }
-        end
-      end
-
-      context "license compliance policy" do
-        let!(:license_compliance_rule) { create(:report_approver_rule, :license_scanning, merge_request: merge_request, approvals_required: 1) }
-
-        before do
-          stub_feature_flags(drop_license_management_artifact: false)
-        end
-
-        context "when a license violates the license compliance policy" do
-          let!(:software_license_policy) { create(:software_license_policy, :denied, project: project, software_license: denied_license) }
-          let(:denied_license) { create(:software_license, name: license_name) }
-          let(:license_name) { ci_build.pipeline.license_scanning_report.license_names[0] }
-
-          context 'with a new report' do
-            let!(:ci_build) { create(:ee_ci_build, :success, :license_scanning, pipeline: pipeline, project: project) }
-
-            specify { expect { subject }.not_to change { license_compliance_rule.reload.approvals_required } }
-            specify { expect(subject[:status]).to be(:success) }
-          end
-
-          context 'with an old report' do
-            let!(:ci_build) { create(:ee_ci_build, :success, :license_management, pipeline: pipeline, project: project) }
-
-            specify { expect { subject }.not_to change { license_compliance_rule.reload.approvals_required } }
-            specify { expect(subject[:status]).to be(:success) }
-          end
-        end
-
-        context "when no licenses violate the license compliance policy" do
-          context 'with a new report' do
-            let!(:ci_build) { create(:ee_ci_build, :success, :license_scanning, pipeline: pipeline, project: project) }
-
-            specify { expect { subject }.to change { license_compliance_rule.reload.approvals_required }.from(1).to(0) }
-            specify { expect(subject[:status]).to be(:success) }
-          end
-
-          context 'with an old report' do
-            let!(:ci_build) { create(:ee_ci_build, :success, :license_management, pipeline: pipeline, project: project) }
-
-            before do
-              stub_feature_flags(drop_license_management_artifact: false)
-            end
-
-            specify { expect { subject }.to change { license_compliance_rule.reload.approvals_required }.from(1).to(0) }
-            specify { expect(subject[:status]).to be(:success) }
-          end
-        end
-
-        context "when an unexpected error occurs" do
-          before do
-            allow_next_instance_of(Gitlab::Ci::Reports::LicenseScanning::Report) do |instance|
-              allow(instance).to receive(:violates?).and_raise('heck')
-            end
-          end
-
-          specify { expect(subject[:status]).to be(:error) }
-          specify { expect(subject[:message]).to eql("Failed to update approval rules") }
-        end
-      end
-    end
-
-    context 'when pipeline fails' do
-      before do
-        pipeline.update!(status: :failed)
-      end
-
-      context 'when high-severity vulnerabilities are present' do
-        before do
-          create(:ee_ci_build, :success, :dependency_scanning, name: 'ds_job', pipeline: pipeline, project: project)
-        end
-
-        context 'when high-severity vulnerabilities already present in target branch pipeline' do
-          before do
-            create(:ee_ci_build, :success, :dependency_scanning, name: 'ds_job', pipeline: base_pipeline, project: project)
-          end
-
-          it 'lowers approvals_required count to zero' do
-            expect { subject }
-              .to change { report_approver_rule.reload.approvals_required }.from(2).to(0)
-          end
-        end
-
-        context 'when high-severity vulnerabilities do not present in target branch pipeline' do
-          it "won't change approvals_required count" do
-            expect { subject }
-              .not_to change { report_approver_rule.reload.approvals_required }
-          end
-        end
-      end
-
-      context 'when only low-severity vulnerabilities are present' do
-        before do
-          create(:ee_ci_build, :success, :low_severity_dast_report, name: 'dast_job', pipeline: pipeline, project: project)
-        end
-
-        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
-  end
-
-  context 'without reports' do
-    let(:pipeline) { create(:ci_pipeline, :running, project: project, merge_requests_as_head_pipeline: [merge_request]) }
-
-    it "won't change approvals_required count" do
-      expect { subject }
-        .not_to change { report_approver_rule.reload.approvals_required }
-    end
-
-    context "license compliance policy" do
-      let!(:software_license_policy) { create(:software_license_policy, :denied, project: project, software_license: denied_license) }
-      let!(:license_compliance_rule) { create(:report_approver_rule, :license_scanning, merge_request: merge_request, approvals_required: 1) }
-      let!(:denied_license) { create(:software_license) }
-
-      specify { expect { subject }.not_to change { license_compliance_rule.reload.approvals_required } }
-      specify { expect(subject[:status]).to be(:success) }
-    end
-  end
-end

ee/spec/workers/sync_security_reports_to_report_approval_rules_worker_spec.rb → ee/spec/workers/ci/sync_reports_to_report_approval_rules_worker_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe SyncSecurityReportsToReportApprovalRulesWorker do
+RSpec.describe Ci::SyncReportsToReportApprovalRulesWorker do
   describe '#perform' do
     let(:pipeline) { double(:pipeline, id: 42) }
     let(:sync_service) { double(:service, execute: true) }
@@ -13,7 +13,7 @@ RSpec.describe SyncSecurityReportsToReportApprovalRulesWorker do
       end
 
       it "executes SyncReportsToApprovalRulesService for given pipeline" do
-        expect(Security::SyncReportsToApprovalRulesService).to receive(:new)
+        expect(Ci::SyncReportsToApprovalRulesService).to receive(:new)
           .with(pipeline).once.and_return(sync_service)
 
         described_class.new.perform(pipeline.id)
@@ -22,7 +22,7 @@ RSpec.describe SyncSecurityReportsToReportApprovalRulesWorker do
 
     context 'when pipeline is missing' do
       it 'does not execute SyncReportsToApprovalRulesService' do
-        expect(Security::SyncReportsToApprovalRulesService).not_to receive(:new)
+        expect(Ci::SyncReportsToApprovalRulesService).not_to receive(:new)
 
         described_class.new.perform(pipeline.id)
       end

spec/factories/ci/builds.rb

@@ -238,6 +238,20 @@ FactoryBot.define do
       coverage_regex { '/(d+)/' }
     end
 
+    trait :trace_with_coverage do
+      coverage { nil }
+      coverage_regex { '(\d+\.\d+)%' }
+
+      transient do
+        trace_coverage { 60.0 }
+      end
+
+      after(:create) do |build, evaluator|
+        build.trace.set("Coverage #{evaluator.trace_coverage}%")
+        build.trace.archive! if build.complete?
+      end
+    end
+
     trait :trace_live do
       after(:create) do |build, evaluator|
         build.trace.set('BUILD TRACE')

spec/models/ci/build_spec.rb

@@ -5239,4 +5239,20 @@ RSpec.describe Ci::Build do
       end
     end
   end
+
+  describe '.without_coverage' do
+    let!(:build_with_coverage) { create(:ci_build, pipeline: pipeline, coverage: 100.0) }
+
+    it 'returns builds without coverage values' do
+      expect(described_class.without_coverage).to eq([build])
+    end
+  end
+
+  describe '.with_coverage_regex' do
+    let!(:build_with_coverage_regex) { create(:ci_build, pipeline: pipeline, coverage_regex: '\d') }
+
+    it 'returns builds with coverage regex values' do
+      expect(described_class.with_coverage_regex).to eq([build_with_coverage_regex])
+    end
+  end
 end

spec/models/ci/pipeline_spec.rb

@@ -744,6 +744,42 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do
     end
   end
 
+  describe '#update_builds_coverage' do
+    let_it_be(:pipeline) { create(:ci_empty_pipeline) }
+
+    context 'builds with coverage_regex defined' do
+      let!(:build_1) { create(:ci_build, :success, :trace_with_coverage, trace_coverage: 60.0, pipeline: pipeline) }
+      let!(:build_2) { create(:ci_build, :success, :trace_with_coverage, trace_coverage: 80.0, pipeline: pipeline) }
+
+      it 'updates the coverage value of each build from the trace' do
+        pipeline.update_builds_coverage
+
+        expect(build_1.reload.coverage).to eq(60.0)
+        expect(build_2.reload.coverage).to eq(80.0)
+      end
+    end
+
+    context 'builds without coverage_regex defined' do
+      let!(:build) { create(:ci_build, :success, :trace_with_coverage, coverage_regex: nil, trace_coverage: 60.0, pipeline: pipeline) }
+
+      it 'does not update the coverage value of each build from the trace' do
+        pipeline.update_builds_coverage
+
+        expect(build.reload.coverage).to eq(nil)
+      end
+    end
+
+    context 'builds with coverage values already present' do
+      let!(:build) { create(:ci_build, :success, :trace_with_coverage, trace_coverage: 60.0, coverage: 10.0, pipeline: pipeline) }
+
+      it 'does not update the coverage value of each build from the trace' do
+        pipeline.update_builds_coverage
+
+        expect(build.reload.coverage).to eq(10.0)
+      end
+    end
+  end
+
   describe '#retryable?' do
     subject { pipeline.retryable? }
 

spec/workers/every_sidekiq_worker_spec.rb

@@ -165,6 +165,7 @@ RSpec.describe 'Every Sidekiq worker' do
         'Ci::ResourceGroups::AssignResourceFromResourceGroupWorker' => 3,
         'Ci::TestFailureHistoryWorker' => 3,
         'Ci::TriggerDownstreamSubscriptionsWorker' => 3,
+        'Ci::SyncReportsToReportApprovalRulesWorker' => 3,
         'CleanupContainerRepositoryWorker' => 3,
         'ClusterConfigureIstioWorker' => 3,
         'ClusterInstallAppWorker' => 3,
@@ -428,7 +429,6 @@ RSpec.describe 'Every Sidekiq worker' do
         'StoreSecurityScansWorker' => 3,
         'SyncSeatLinkRequestWorker' => 20,
         'SyncSeatLinkWorker' => 12,
-        'SyncSecurityReportsToReportApprovalRulesWorker' => 3,
         'SystemHookPushWorker' => 3,
         'TodosDestroyer::ConfidentialEpicWorker' => 3,
         'TodosDestroyer::ConfidentialIssueWorker' => 3,