Merge branch '12004-move-cs-reports-to-mr-widget-backend' into 'master'

Present container scanning methods in merge request widget

See merge request gitlab-org/gitlab-ee!14898

ee/app/controllers/ee/projects/merge_requests_controller.rb

@@ -42,6 +42,10 @@ module EE
         reports_response(merge_request.compare_license_management_reports)
       end
 
+      def container_scanning_reports
+        reports_response(merge_request.compare_container_scanning_reports)
+      end
+
       def metrics_reports
         reports_response(merge_request.compare_metrics_reports)
       end

ee/app/models/ee/ci/job_artifact.rb

@@ -15,6 +15,7 @@ module EE
       LICENSE_MANAGEMENT_REPORT_FILE_TYPES = %w[license_management].freeze
       DEPENDENCY_LIST_REPORT_FILE_TYPES = %w[dependency_scanning].freeze
       METRICS_REPORT_FILE_TYPES = %w[metrics].freeze
+      CONTAINER_SCANNING_REPORT_TYPES = %w[container_scanning].freeze
 
       scope :not_expired, -> { where('expire_at IS NULL OR expire_at > ?', Time.current) }
       scope :project_id_in, ->(ids) { joins(:project).merge(::Project.id_in(ids)) }
@@ -33,6 +34,10 @@ module EE
         with_file_types(DEPENDENCY_LIST_REPORT_FILE_TYPES)
       end
 
+      scope :container_scanning_reports, -> do
+        with_file_types(CONTAINER_SCANNING_REPORT_TYPES)
+      end
+
       scope :metrics_reports, -> do
         with_file_types(METRICS_REPORT_FILE_TYPES)
       end

ee/app/models/ee/merge_request.rb

@@ -116,6 +116,18 @@ module EE
       actual_head_pipeline&.has_reports?(::Ci::JobArtifact.license_management_reports)
     end
 
+    def has_container_scanning_reports?
+      actual_head_pipeline&.has_reports?(::Ci::JobArtifact.container_scanning_reports)
+    end
+
+    def compare_container_scanning_reports
+      unless has_container_scanning_reports?
+        return { status: :error, status_reason: 'This merge request does not have container scanning reports' }
+      end
+
+      compare_reports(::Ci::CompareContainerScanningReportsService)
+    end
+
     def compare_license_management_reports
       unless has_license_management_reports?
         return { status: :error, status_reason: 'This merge request does not have license management reports' }

ee/app/serializers/vulnerabilities/occurence_reports_comparer_entity.rb

+# frozen_string_literal: true
+
+class Vulnerabilities::OccurrenceReportsComparerEntity < Grape::Entity
+  expose :added, using: Vulnerabilities::OccurrenceReportEntity
+  expose :fixed, using: Vulnerabilities::OccurrenceReportEntity
+  expose :existing, using: Vulnerabilities::OccurrenceReportEntity
+end

ee/app/serializers/vulnerabilities/occurrence_diff_serializer.rb

+# frozen_string_literal: true
+
+class Vulnerabilities::OccurrenceDiffSerializer < BaseSerializer
+  include WithPagination
+
+  entity Vulnerabilities::OccurrenceReportsComparerEntity
+end

ee/app/serializers/vulnerabilities/occurrence_report_entity.rb

+# frozen_string_literal: true
+
+class Vulnerabilities::OccurrenceReportEntity < Grape::Entity
+  expose :report_type, :name, :severity, :confidence, :compare_key, :identifiers, :scanner, :project_fingerprint, :uuid, :metadata_version, :location
+end

ee/app/services/ci/compare_container_scanning_reports_service.rb

+# frozen_string_literal: true
+
+module Ci
+  class CompareContainerScanningReportsService < ::Ci::CompareReportsBaseService
+    def comparer_class
+      Gitlab::Ci::Reports::Security::ContainerScanningReportsComparer
+    end
+
+    def serializer_class
+      Vulnerabilities::OccurrenceDiffSerializer
+    end
+
+    def get_report(pipeline)
+      report = pipeline&.security_reports&.get_report('container_scanning')
+
+      raise report.error if report&.errored? # propagate error to base class's execute method
+
+      report
+    end
+  end
+end

ee/app/views/projects/merge_requests/show.html.haml

@@ -17,3 +17,4 @@
     window.gl.mrWidgetData.approvals_help_path = '#{help_page_path("user/project/merge_requests/merge_request_approvals")}';
     window.gl.mrWidgetData.visual_review_app_available = '#{@project.feature_available?(:visual_review_app)}' === 'true';
     window.gl.mrWidgetData.license_management_comparsion_path = '#{license_management_reports_project_merge_request_path(@project, @merge_request) if @project.feature_available?(:license_management)}'
+    window.gl.mrWidgetData.container_scanning_comparsion_path = '#{container_scanning_reports_project_merge_request_path(@project, @merge_request) if @project.feature_available?(:container_scanning)}'

ee/changelogs/unreleased/12004-move-cs-reports-to-mr-widget-backend.yml

+---
+title: Present container scanning report comparison via API
+merge_request: 14898
+author:
+type: changed

ee/config/routes/project.rb

@@ -69,6 +69,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
         member do
           get :metrics_reports
           get :license_management_reports
+          get :container_scanning_reports
         end
       end
 

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

+# frozen_string_literal: true
+
+module Gitlab
+  module Ci
+    module Reports
+      module Security
+        class ContainerScanningReportsComparer
+          include Gitlab::Utils::StrongMemoize
+
+          attr_reader :base_report, :head_report
+
+          def initialize(base_report, head_report)
+            @base_report = base_report || ::Gitlab::Ci::Reports::Security::Report.new('container_scanning', '')
+            @head_report = head_report
+          end
+
+          def added
+            strong_memoize(:added) do
+              head_report.occurrences - base_report.occurrences
+            end
+          end
+
+          def fixed
+            strong_memoize(:fixed) do
+              base_report.occurrences - head_report.occurrences
+            end
+          end
+
+          def existing
+            strong_memoize(:existing) do
+              base_report.occurrences & head_report.occurrences
+            end
+          end
+        end
+      end
+    end
+  end
+end

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

@@ -70,6 +70,12 @@ module Gitlab
               other.location == location &&
               other.primary_identifier == primary_identifier
           end
+
+          # Array.difference (-) method uses hash and eq? methods to do comparison
+          def hash
+            compare_key.hash
+          end
+
           alias_method :eql?, :== # eql? is necessary in some cases like array intersection
 
           private

ee/spec/controllers/projects/merge_requests_controller_spec.rb

@@ -393,6 +393,91 @@ describe Projects::MergeRequestsController do
     end
   end
 
+  describe 'GET #container_scanning_reports' do
+    let(:merge_request) { create(:ee_merge_request, :with_container_scanning_reports, source_project: project, author: create(:user)) }
+    let(:params) do
+      {
+        namespace_id: project.namespace.to_param,
+        project_id: project,
+        id: merge_request.iid
+      }
+    end
+
+    subject { get :container_scanning_reports, params: params, format: :json }
+
+    before do
+      allow_any_instance_of(::MergeRequest).to receive(:compare_reports)
+        .with(::Ci::CompareContainerScanningReportsService).and_return(comparison_status)
+    end
+
+    context 'when comparison is being processed' do
+      let(:comparison_status) { { status: :parsing } }
+
+      it 'sends polling interval' do
+        expect(::Gitlab::PollingInterval).to receive(:set_header)
+
+        subject
+      end
+
+      it 'returns 204 HTTP status' do
+        subject
+
+        expect(response).to have_gitlab_http_status(:no_content)
+      end
+    end
+
+    context 'when comparison is done' do
+      let(:comparison_status) { { status: :parsed, data: { added: [], fixed: [], existing: [] } } }
+
+      it 'does not send polling interval' do
+        expect(::Gitlab::PollingInterval).not_to receive(:set_header)
+
+        subject
+      end
+
+      it 'returns 200 HTTP status' do
+        subject
+
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(json_response).to eq({ "added" => [], "fixed" => [], "existing" => [] })
+      end
+    end
+
+    context 'when user created corrupted vulnerability reports' do
+      let(:comparison_status) { { status: :error, status_reason: 'Failed to parse container scanning reports' } }
+
+      it 'does not send polling interval' do
+        expect(::Gitlab::PollingInterval).not_to receive(:set_header)
+
+        subject
+      end
+
+      it 'returns 400 HTTP status' do
+        subject
+
+        expect(response).to have_gitlab_http_status(:bad_request)
+        expect(json_response).to eq({ 'status_reason' => 'Failed to parse container scanning reports' })
+      end
+    end
+
+    context 'when something went wrong on our system' do
+      let(:comparison_status) { {} }
+
+      it 'does not send polling interval' do
+        expect(::Gitlab::PollingInterval).not_to receive(:set_header)
+
+        subject
+      end
+
+      it 'returns 500 HTTP status' do
+        subject
+
+        expect(response).to have_gitlab_http_status(:internal_server_error)
+        expect(json_response).to eq({ 'status_reason' => 'Unknown error' })
+      end
+    end
+  end
+
   describe 'GET #license_management_reports' do
     let(:merge_request) { create(:ee_merge_request, :with_license_management_reports, source_project: project, author: create(:user)) }
     let(:params) do

ee/spec/factories/ci/builds.rb

@@ -48,6 +48,18 @@ FactoryBot.define do
       end
     end
 
+    trait :container_scanning_feature_branch do
+      after(:build) do |build|
+        build.job_artifacts << create(:ee_ci_job_artifact, :container_scanning_feature_branch, job: build)
+      end
+    end
+
+    trait :corrupted_container_scanning_report do
+      after(:build) do |build|
+        build.job_artifacts << create(:ee_ci_job_artifact, :corrupted_container_scanning_report, job: build)
+      end
+    end
+
     trait :license_management_feature_branch do
       after(:build) do |build|
         build.job_artifacts << create(:ee_ci_job_artifact, :license_management_feature_branch, job: build)

ee/spec/factories/ci/job_artifacts.rb

@@ -122,6 +122,26 @@ FactoryBot.define do
       end
     end
 
+    trait :container_scanning_feature_branch do
+      file_format :raw
+      file_type :container_scanning
+
+      after(:build) do |artifact, _|
+        artifact.file = fixture_file_upload(
+          Rails.root.join('spec/fixtures/security-reports/feature-branch/gl-container-scanning-report.json'), 'application/json')
+      end
+    end
+
+    trait :corrupted_container_scanning_report do
+      file_format :raw
+      file_type :container_scanning
+
+      after(:build) do |artifact, evaluator|
+        artifact.file = fixture_file_upload(
+          Rails.root.join('spec/fixtures/trace/sample_trace'), 'application/json')
+      end
+    end
+
     trait :dast do
       file_format :raw
       file_type :dast

ee/spec/factories/ci/pipelines.rb

@@ -7,7 +7,7 @@ FactoryBot.define do
       config_source :webide_source
     end
 
-    %i[license_management dependency_list dependency_scanning sast].each do |report_type|
+    %i[license_management dependency_list dependency_scanning sast container_scanning].each do |report_type|
       trait "with_#{report_type}_report".to_sym do
         status :success
 
@@ -17,6 +17,22 @@ FactoryBot.define do
       end
     end
 
+    trait :with_container_scanning_feature_branch do
+      status :success
+
+      after(:build) do |pipeline, evaluator|
+        pipeline.builds << build(:ee_ci_build, :container_scanning_feature_branch, pipeline: pipeline, project: pipeline.project)
+      end
+    end
+
+    trait :with_corrupted_container_scanning_report do
+      status :success
+
+      after(:build) do |pipeline, evaluator|
+        pipeline.builds << build(:ee_ci_build, :corrupted_container_scanning_report, pipeline: pipeline, project: pipeline.project)
+      end
+    end
+
     trait :with_license_management_feature_branch do
       status :success
 

ee/spec/factories/merge_requests.rb

@@ -61,6 +61,18 @@ FactoryBot.define do
       end
     end
 
+    trait :with_container_scanning_reports do
+      after(:build) do |merge_request|
+        merge_request.head_pipeline = build(
+          :ee_ci_pipeline,
+          :success,
+          :with_container_scanning_report,
+          project: merge_request.source_project,
+          ref: merge_request.source_branch,
+          sha: merge_request.diff_head_sha)
+      end
+    end
+
     trait :with_metrics_reports do
       after(:build) do |merge_request|
         merge_request.head_pipeline = build(

ee/spec/models/merge_request_spec.rb

@@ -139,6 +139,27 @@ describe MergeRequest do
     end
   end
 
+  describe '#has_container_scanning_reports?' do
+    subject { merge_request.has_container_scanning_reports? }
+    let(:project) { create(:project, :repository) }
+
+    before do
+      stub_licensed_features(container_scanning: true)
+    end
+
+    context 'when head pipeline has container scannning reports' do
+      let(:merge_request) { create(:ee_merge_request, :with_container_scanning_reports, source_project: project) }
+
+      it { is_expected.to be_truthy }
+    end
+
+    context 'when head pipeline does not have container scanning reports' do
+      let(:merge_request) { create(:ee_merge_request, source_project: project) }
+
+      it { is_expected.to be_falsey }
+    end
+  end
+
   describe '#has_metrics_reports?' do
     subject { merge_request.has_metrics_reports? }
     let(:project) { create(:project, :repository) }
@@ -160,6 +181,65 @@ describe MergeRequest do
     end
   end
 
+  describe '#compare_container_scanning_reports' do
+    subject { merge_request.compare_container_scanning_reports }
+
+    let(:project) { create(:project, :repository) }
+    let(:merge_request) { create(:merge_request, source_project: project) }
+
+    let!(:base_pipeline) do
+      create(:ee_ci_pipeline,
+             :with_container_scanning_report,
+             project: project,
+             ref: merge_request.target_branch,
+             sha: merge_request.diff_base_sha)
+    end
+
+    before do
+      merge_request.update!(head_pipeline_id: head_pipeline.id)
+    end
+
+    context 'when head pipeline has container scanning reports' do
+      let!(:head_pipeline) do
+        create(:ee_ci_pipeline,
+               :with_container_scanning_report,
+               project: project,
+               ref: merge_request.source_branch,
+               sha: merge_request.diff_head_sha)
+      end
+
+      context 'when reactive cache worker is parsing asynchronously' do
+        it 'returns status' do
+          expect(subject[:status]).to eq(:parsing)
+        end
+      end
+
+      context 'when reactive cache worker is inline' do
+        before do
+          synchronous_reactive_cache(merge_request)
+        end
+
+        it 'returns status and data' do
+          expect_any_instance_of(Ci::CompareContainerScanningReportsService)
+              .to receive(:execute).with(base_pipeline, head_pipeline).and_call_original
+
+          subject
+        end
+
+        context 'when cached results is not latest' do
+          before do
+            allow_any_instance_of(Ci::CompareContainerScanningReportsService)
+                .to receive(:latest?).and_return(false)
+          end
+
+          it 'raises and InvalidateReactiveCache error' do
+            expect { subject }.to raise_error(ReactiveCaching::InvalidateReactiveCache)
+          end
+        end
+      end
+    end
+  end
+
   describe '#compare_license_management_reports' do
     subject { merge_request.compare_license_management_reports }
 

ee/spec/serializers/vulnerabilities/occurrence_reports_comparer_entity_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Vulnerabilities::OccurrenceReportsComparerEntity do
+  let!(:base_pipeline) { create(:ee_ci_pipeline, :with_container_scanning_report) }
+  let!(:head_pipeline) { create(:ee_ci_pipeline, :with_container_scanning_feature_branch) }
+  let(:base_report) { base_pipeline.security_reports.get_report('container_scanning')}
+  let(:head_report) { head_pipeline.security_reports.get_report('container_scanning')}
+  let(:comparer) { Gitlab::Ci::Reports::Security::ContainerScanningReportsComparer.new(base_report, head_report) }
+  let(:entity) { described_class.new(comparer) }
+
+  before do
+    stub_licensed_features(container_scanning: true)
+  end
+
+  describe '#as_json' do
+    subject { entity.as_json }
+
+    it 'contains the added existing and fixed vulnerabilities for container scanning' do
+      expect(subject.keys).to match_array([:added, :existing, :fixed])
+    end
+  end
+end

ee/spec/services/ci/compare_container_scanning_reports_service_spec.rb

+require 'spec_helper'
+
+describe Ci::CompareContainerScanningReportsService do
+  let(:service) { described_class.new(project) }
+  let(:project) { create(:project, :repository) }
+
+  before do
+    stub_licensed_features(container_scanning: true)
+  end
+
+  describe '#execute' do
+    subject { service.execute(base_pipeline, head_pipeline) }
+
+    context 'when head pipeline has container scanning reports' do
+      let!(:base_pipeline) { create(:ee_ci_pipeline) }
+      let!(:head_pipeline) { create(:ee_ci_pipeline, :with_container_scanning_report, project: project) }
+
+      it 'reports new licenses' do
+        expect(subject[:status]).to eq(:parsed)
+        expect(subject[:data]['added'].count).to eq(8)
+        expect(subject[:data]['existing'].count).to eq(0)
+        expect(subject[:data]['fixed'].count).to eq(0)
+      end
+    end
+
+    context 'when base and head pipelines have container scanning reports' do
+      let!(:base_pipeline) { create(:ee_ci_pipeline, :with_container_scanning_report, project: project) }
+      let!(:head_pipeline) { create(:ee_ci_pipeline, :with_container_scanning_feature_branch, project: project) }
+
+      it 'reports status as parsed' do
+        expect(subject[:status]).to eq(:parsed)
+      end
+
+      it 'reports new vulnerability' do
+        expect(subject[:data]['added'].count).to eq(1)
+        expect(subject[:data]['added']).to include(a_hash_including('compare_key' => 'CVE-2017-15650'))
+      end
+
+      it 'reports existing container vulenerabilities' do
+        expect(subject[:data]['existing'].count).to eq(0)
+      end
+
+      it 'reports fixed container scanning vulnerabilities' do
+        expect(subject[:data]['fixed'].count).to eq(8)
+        compare_keys = subject[:data]['fixed'].map { |t| t['compare_key'] }
+        expected_keys = %w(CVE-2017-16997 CVE-2017-18269 CVE-2018-1000001 CVE-2016-10228 CVE-2010-4052 CVE-2018-18520 CVE-2018-16869 CVE-2018-18311)
+        expect(compare_keys - expected_keys).to eq([])
+      end
+    end
+
+    context 'when head pipeline has corrupted container scanning vulnerability reports' do
+      let!(:base_pipeline) { nil }
+      let!(:head_pipeline) { create(:ee_ci_pipeline, :with_corrupted_container_scanning_report, project: project) }
+
+      it 'returns status and error message' do
+        expect(subject[:status]).to eq(:error)
+        expect(subject[:status_reason]).to include('JSON parsing failed')
+      end
+    end
+  end
+end