Merge branch 'expose-fields-matching-with-fe' into 'master'

Expose fields matching with FE

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

ee/app/finders/security/pipeline_vulnerabilities_finder.rb

@@ -13,6 +13,7 @@
 module Security
   class PipelineVulnerabilitiesFinder
     include Gitlab::Utils::StrongMemoize
+    ParseError = Class.new(Gitlab::Ci::Parsers::ParserError)
 
     attr_accessor :params
     attr_reader :pipeline
@@ -31,6 +32,10 @@ module Security
 
         occurrences.concat(filtered_occurrences)
       end
+
+    # Created follow-up issue to better handle exception case - https://gitlab.com/gitlab-org/gitlab-ee/issues/14007
+    rescue NoMethodError => _ # propagate error for CompareReportsBaseService
+      raise ParseError, 'JSON parsing failed'
     end
 
     private

ee/app/models/vulnerabilities/occurrence.rb

@@ -182,5 +182,24 @@ module Vulnerabilities
     def remediations
       metadata.dig('remediations')
     end
+
+    alias_method :==, :eql? # eql? is necessary in some cases like array intersection
+
+    def eql?(other)
+      other.report_type == report_type &&
+        other.location == location &&
+        other.first_fingerprint == first_fingerprint
+    end
+
+    # Array.difference (-) method uses hash and eql? methods to do comparison
+    def hash
+      report_type.hash ^ location.hash ^ first_fingerprint.hash
+    end
+
+    protected
+
+    def first_fingerprint
+      identifiers.first&.fingerprint
+    end
   end
 end

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
+  expose :added, using: Vulnerabilities::OccurrenceEntity
+  expose :fixed, using: Vulnerabilities::OccurrenceEntity
+  expose :existing, using: Vulnerabilities::OccurrenceEntity
 end

ee/app/serializers/vulnerabilities/occurrence_entity.rb

@@ -36,5 +36,8 @@ class Vulnerabilities::OccurrenceEntity < Grape::Entity
   end
 
   alias_method :occurrence, :object
-  delegate :current_user, to: :request
+
+  def current_user
+    return request.current_user if request.respond_to?(:current_user)
+  end
 end

ee/app/services/ci/compare_container_scanning_reports_service.rb

@@ -11,11 +11,7 @@ module Ci
     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
+      Security::PipelineVulnerabilitiesFinder.new(pipeline: pipeline, params: { report_type: %w[container_scanning] }).execute
     end
   end
 end

ee/app/services/ci/compare_dependency_scanning_reports_service.rb

@@ -11,11 +11,7 @@ module Ci
     end
 
     def get_report(pipeline)
-      report = pipeline&.security_reports&.get_report('dependency_scanning')
-
-      raise report.error if report&.errored? # propagate error to base class's execute method
-
-      report
+      Security::PipelineVulnerabilitiesFinder.new(pipeline: pipeline, params: { report_type: %w[dependency_scanning] }).execute
     end
   end
 end

ee/app/services/ci/compare_sast_reports_service.rb

@@ -11,11 +11,7 @@ module Ci
     end
 
     def get_report(pipeline)
-      report = pipeline&.security_reports&.get_report('sast')
-
-      raise report.error if report&.errored? # propagate error to base class's execute method
-
-      report
+      Security::PipelineVulnerabilitiesFinder.new(pipeline: pipeline, params: { report_type: %w[sast] }).execute
     end
   end
 end

ee/changelogs/unreleased/expose-fields-matching-with-fe.yml

+---
+title: Change payload for comparing security reports in MR widget
+merge_request: 15531
+author:
+type: fixed

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

@@ -10,25 +10,25 @@ module Gitlab
           attr_reader :base_report, :head_report
 
           def initialize(base_report, head_report)
-            @base_report = base_report || ::Gitlab::Ci::Reports::Security::Report.new(head_report&.type, '')
+            @base_report = base_report || []
             @head_report = head_report
           end
 
           def added
             strong_memoize(:added) do
-              head_report.occurrences - base_report.occurrences
+              head_report - base_report
             end
           end
 
           def fixed
             strong_memoize(:fixed) do
-              base_report.occurrences - head_report.occurrences
+              base_report - head_report
             end
           end
 
           def existing
             strong_memoize(:existing) do
-              base_report.occurrences & head_report.occurrences
+              base_report & head_report
             end
           end
         end

ee/spec/serializers/vulnerabilities/occurrence_reports_comparer_entity_spec.rb

@@ -4,12 +4,16 @@ require 'spec_helper'
 
 describe Vulnerabilities::OccurrenceReportsComparerEntity do
   describe 'container scanning report comparison' 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')}
+    set(:user) { create(:user) }
+
+    let(:base_report) { create_list(:vulnerabilities_occurrence, 2) }
+    let(:head_report) { create_list(:vulnerabilities_occurrence, 1) }
+
     let(:comparer) { Gitlab::Ci::Reports::Security::VulnerabilityReportsComparer.new(base_report, head_report) }
-    let(:entity) { described_class.new(comparer) }
+
+    let(:request) { double('request') }
+
+    let(:entity) { described_class.new(comparer, request: request) }
 
     before do
       stub_licensed_features(container_scanning: true)
@@ -18,24 +22,11 @@ describe Vulnerabilities::OccurrenceReportsComparerEntity do
     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])
+      before do
+        allow(request).to receive(:current_user).and_return(user)
       end
-    end
-  end
-
-  describe 'sast report comparison' do
-    let!(:base_pipeline) { create(:ee_ci_pipeline, :with_sast_report) }
-    let!(:head_pipeline) { create(:ee_ci_pipeline, :with_sast_feature_branch) }
-    let(:base_report) { base_pipeline.security_reports.get_report('sast')}
-    let(:head_report) { head_pipeline.security_reports.get_report('sast')}
-    let(:comparer) { Gitlab::Ci::Reports::Security::VulnerabilityReportsComparer.new(base_report, head_report) }
-    let(:entity) { described_class.new(comparer) }
 
-    describe '#as_json' do
-      subject { entity.as_json }
-
-      it 'contains the added existing and fixed vulnerabilities for sast' do
+      it 'contains the added existing and fixed vulnerabilities for container scanning' do
         expect(subject.keys).to match_array([:added, :existing, :fixed])
       end
     end

ee/spec/services/ci/compare_container_scanning_reports_service_spec.rb

@@ -33,7 +33,7 @@ describe Ci::CompareContainerScanningReportsService do
 
       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'))
+        expect(subject[:data]['added'].first['identifiers']).to include(a_hash_including('external_id' => 'CVE-2017-15650'))
       end
 
       it 'reports existing container vulnerabilities' do
@@ -42,7 +42,7 @@ describe Ci::CompareContainerScanningReportsService do
 
       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'] }
+        compare_keys = subject[:data]['fixed'].map { |t| t['identifiers'].first['external_id'] }
         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

ee/spec/services/ci/compare_dependency_scanning_reports_service_spec.rb

@@ -35,7 +35,7 @@ describe Ci::CompareDependencyScanningReportsService do
 
       it 'reports fixed vulnerability' do
         expect(subject[:data]['added'].count).to eq(1)
-        expect(subject[:data]['added']).to include(a_hash_including('compare_key' => 'Gemfile.lock:rubyzip:cve:CVE-2017-5946'))
+        expect(subject[:data]['added'].first['identifiers']).to include(a_hash_including('external_id' => 'CVE-2017-5946'))
       end
 
       it 'reports existing dependency vulenerabilities' do
@@ -44,8 +44,8 @@ describe Ci::CompareDependencyScanningReportsService do
 
       it 'reports fixed dependency scanning vulnerabilities' do
         expect(subject[:data]['fixed'].count).to eq(1)
-        compare_keys = subject[:data]['fixed'].map { |t| t['compare_key'] }
-        expected_keys = %w(rails/Gemfile.lock:nokogiri@1.8.0:USN-3424-1)
+        compare_keys = subject[:data]['fixed'].map { |t| t['identifiers'].first['external_id'] }
+        expected_keys = %w(06565b64-486d-4326-b906-890d9915804d)
         expect(compare_keys - expected_keys).to eq([])
       end
     end

ee/spec/services/ci/compare_sast_reports_service_spec.rb

@@ -36,7 +36,7 @@ describe Ci::CompareSastReportsService do
 
       it 'reports new vulnerability' do
         expect(subject[:data]['added'].count).to eq(1)
-        expect(subject[:data]['added']).to include(a_hash_including('compare_key' => 'c/subdir/utils.c:b466873101951fe96e1332f6728eb7010acbbd5dfc3b65d7d53571d091a06d9e:CWE-119!/CWE-120'))
+        expect(subject[:data]['added'].first['identifiers']).to include(a_hash_including('name' => 'CWE-120'))
       end
 
       it 'reports existing sast vulnerabilities' do
@@ -45,8 +45,8 @@ describe Ci::CompareSastReportsService do
 
       it 'reports fixed sast vulnerabilities' do
         expect(subject[:data]['fixed'].count).to eq(4)
-        compare_keys = subject[:data]['fixed'].map { |t| t['compare_key'] }
-        expected_keys = ['c/subdir/utils.c:b466873101951fe96e1332f6728eb7010acbbd5dfc3b65d7d53571d091a06d9e:CWE-119!/CWE-120', 'c/subdir/utils.c:bab681140fcc8fc3085b6bba74081b44ea145c1c98b5e70cf19ace2417d30770:CWE-362', 'cplusplus/src/hello.cpp:c8c6dd0afdae6814194cf0930b719f757ab7b379cf8f261e7f4f9f2f323a818a:CWE-119!/CWE-120', 'cplusplus/src/hello.cpp:331c04062c4fe0c7c486f66f59e82ad146ab33cdd76ae757ca41f392d568cbd0:CWE-120']
+        compare_keys = subject[:data]['fixed'].map { |t| t['identifiers'].first['external_id'] }
+        expected_keys = %w(char fopen strcpy char)
         expect(compare_keys - expected_keys).to eq([])
       end
     end