Merge branch '238156_short_term_solution_to_load_pipeline_security_reports' into 'master'

Load only the requested report artifacts into memory

See merge request gitlab-org/gitlab!39749

ee/app/finders/security/pipeline_vulnerabilities_finder.rb

@@ -24,8 +24,6 @@ module Security
     end
 
     def execute
-      requested_reports = pipeline_reports.select { |report_type| requested_type?(report_type) }
-
       findings = requested_reports.each_with_object([]) do |(type, report), findings|
         raise ParseError, 'JSON parsing failed' if report.error.is_a?(Gitlab::Ci::Parsers::Security::Common::SecurityReportParserError)
 
@@ -54,8 +52,8 @@ module Security
       Gitlab::Utils.stable_sort_by(findings) { |x| [-x.severity_value, -x.confidence_value] }
     end
 
-    def pipeline_reports
-      pipeline&.security_reports&.reports || {}
+    def requested_reports
+      @requested_reports ||= pipeline&.security_reports(report_types: report_types)&.reports || {}
     end
 
     def vulnerabilities_by_finding_fingerprint(report_type, report)
@@ -105,10 +103,6 @@ module Security
       end
     end
 
-    def requested_type?(type)
-      report_types.include?(type)
-    end
-
     def include_dismissed?
       params[:scope] == 'all'
     end

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

@@ -26,8 +26,10 @@ module EE
       scope :project_id_in, ->(ids) { where(project_id: ids) }
       scope :with_files_stored_remotely, -> { where(file_store: ::JobArtifactUploader::Store::REMOTE) }
 
-      scope :security_reports, -> do
-        with_file_types(SECURITY_REPORT_FILE_TYPES)
+      scope :security_reports, -> (file_types: SECURITY_REPORT_FILE_TYPES) do
+        requested_file_types = *file_types
+
+        with_file_types(requested_file_types & SECURITY_REPORT_FILE_TYPES)
       end
 
       scope :license_scanning_reports, -> do

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

@@ -101,9 +101,11 @@ module EE
         batch_lookup_report_artifact_for_file_type(:license_scanning).present?
       end
 
-      def security_reports
+      def security_reports(report_types: [])
+        reports_scope = report_types.empty? ? ::Ci::JobArtifact.security_reports : ::Ci::JobArtifact.security_reports(file_types: report_types)
+
         ::Gitlab::Ci::Reports::Security::Reports.new(self).tap do |security_reports|
-          builds.latest.with_reports(::Ci::JobArtifact.security_reports).each do |build|
+          builds.latest.with_reports(reports_scope).each do |build|
             build.collect_security_reports!(security_reports)
           end
         end

ee/changelogs/unreleased/238156_short_term_solution_to_load_pipeline_security_reports.yml

+---
+title: Load only the requested report artifacts into the memory for vulnerability_findings
+  endpoint
+merge_request: 39749
+author:
+type: performance

ee/spec/models/ci/pipeline_spec.rb

@@ -169,6 +169,14 @@ RSpec.describe Ci::Pipeline do
           expect(subject.get_report('container_scanning', cs1_artifact).findings.size).to eq(8)
         end
       end
+
+      context 'when the `report_types` parameter is provided' do
+        subject(:filtered_report_types) { pipeline.security_reports(report_types: %w(sast)).reports.values.map(&:type).uniq }
+
+        it 'returns only the reports which are requested' do
+          expect(filtered_report_types).to eq(%w(sast))
+        end
+      end
     end
 
     context 'when pipeline does not have any builds with security reports' do

ee/spec/models/ee/ci/job_artifact_spec.rb

@@ -54,19 +54,48 @@ RSpec.describe Ci::JobArtifact do
   end
 
   describe '.security_reports' do
-    subject { Ci::JobArtifact.security_reports }
-
-    context 'when there is a security report' do
+    context 'when the `file_types` parameter is provided' do
       let!(:sast_artifact) { create(:ee_ci_job_artifact, :sast) }
-      let!(:secret_detection_artifact) { create(:ee_ci_job_artifact, :secret_detection) }
 
-      it { is_expected.to eq([sast_artifact, secret_detection_artifact]) }
+      subject { Ci::JobArtifact.security_reports(file_types: file_types) }
+
+      context 'when the provided file_types is array' do
+        let(:file_types) { %w(secret_detection) }
+
+        context 'when there is a security report with the given value' do
+          let!(:secret_detection_artifact) { create(:ee_ci_job_artifact, :secret_detection) }
+
+          it { is_expected.to eq([secret_detection_artifact]) }
+        end
+
+        context 'when there are no security reports with the given value' do
+          it { is_expected.to be_empty }
+        end
+      end
+
+      context 'when the provided file_types is string' do
+        let(:file_types) { 'secret_detection' }
+        let!(:secret_detection_artifact) { create(:ee_ci_job_artifact, :secret_detection) }
+
+        it { is_expected.to eq([secret_detection_artifact]) }
+      end
     end
 
-    context 'when there are no security reports' do
-      let!(:artifact) { create(:ci_job_artifact, :archive) }
+    context 'when the file_types parameter is not provided' do
+      subject { Ci::JobArtifact.security_reports }
 
-      it { is_expected.to be_empty }
+      context 'when there is a security report' do
+        let!(:sast_artifact) { create(:ee_ci_job_artifact, :sast) }
+        let!(:secret_detection_artifact) { create(:ee_ci_job_artifact, :secret_detection) }
+
+        it { is_expected.to match_array([sast_artifact, secret_detection_artifact]) }
+      end
+
+      context 'when there are no security reports' do
+        let!(:artifact) { create(:ci_job_artifact, :archive) }
+
+        it { is_expected.to be_empty }
+      end
     end
   end