Introduce a new worker to create security scans and findings

Once we deploy this new worker, security scan entries and findings will
be saved by the new worker class.
We will stop using the old worker but we can't remove it completely as
there might be some jobs enqueued for the worker.

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

@@ -60,6 +60,7 @@ 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
@@ -169,8 +170,16 @@ module EE
         builds.latest.with_reports(::Ci::JobArtifact.license_scanning_reports).exists?
       end
 
+      def can_store_security_reports?
+        project.can_store_security_reports? && has_security_reports?
+      end
+
       private
 
+      def has_security_reports?
+        has_reports?(::Ci::JobArtifact.security_reports.or(::Ci::JobArtifact.license_scanning_reports))
+      end
+
       def project_has_subscriptions?
         project.beta_feature_available?(:ci_project_subscriptions) &&
           project.downstream_projects.any?

ee/app/workers/all_queues.yml

@@ -515,6 +515,14 @@
   :weight: 3
   :idempotent:
   :tags: []
+- :name: security_scans:security_store_scans
+  :feature_category: :static_application_security_testing
+  :has_external_dependencies:
+  :urgency: :low
+  :resource_boundary: :unknown
+  :weight: 2
+  :idempotent:
+  :tags: []
 - :name: security_scans:store_security_reports
   :feature_category: :static_application_security_testing
   :has_external_dependencies:

ee/app/workers/ee/build_finished_worker.rb

@@ -8,7 +8,6 @@ module EE
       # and `Namespace#namespace_statistics` will return stale data.
       ::Ci::Minutes::EmailNotificationService.new(build.project.reset).execute if ::Gitlab.com?
 
-      StoreSecurityScansWorker.perform_async(build.id)
       RequirementsManagement::ProcessRequirementsReportsWorker.perform_async(build.id)
 
       super

ee/app/workers/security/store_scans_worker.rb

+# frozen_string_literal: true
+
+module Security
+  class StoreScansWorker # rubocop:disable Scalability/IdempotentWorker
+    include ApplicationWorker
+    include SecurityScansQueue
+
+    # rubocop: disable CodeReuse/ActiveRecord
+    def perform(pipeline_id)
+      ::Ci::Pipeline.find_by(id: pipeline_id).try do |pipeline|
+        break unless pipeline.can_store_security_reports?
+
+        Security::StoreScansService.execute(pipeline)
+      end
+    end
+  end
+end

ee/app/workers/store_security_scans_worker.rb

@@ -5,11 +5,11 @@ class StoreSecurityScansWorker # rubocop:disable Scalability/IdempotentWorker
   include SecurityScansQueue
 
   # rubocop: disable CodeReuse/ActiveRecord
-  def perform(build_id)
-    ::Ci::Build.find_by(id: build_id).try do |build|
-      break if build.job_artifacts.security_reports.empty?
-
-      Security::StoreScansService.new(build).execute
-    end
+  def perform(*)
+    # no-op
+    # This worker has been deprecated and will be removed with next release.
+    # New worker to do the same job is, `Security::StoreScansWorker`,
+    # We will save all the security scans and findings here
+    # as well as solve the deduplication thingy.
   end
 end

ee/spec/models/ci/pipeline_spec.rb

@@ -544,4 +544,48 @@ RSpec.describe Ci::Pipeline do
       specify { expect(subject).to eq(expected_status) }
     end
   end
+
+  describe '#can_store_security_reports?' do
+    subject { pipeline.can_store_security_reports? }
+
+    before do
+      pipeline.succeed!
+    end
+
+    context 'when the security reports can not be stored for the project' do
+      before do
+        allow(project).to receive(:can_store_security_reports?).and_return(false)
+      end
+
+      context 'when the pipeline does not have security reports' do
+        it { is_expected.to be_falsy }
+      end
+
+      context 'when the pipeline has security reports' do
+        before do
+          create(:ee_ci_build, :sast, pipeline: pipeline, project: project)
+        end
+
+        it { is_expected.to be_falsy }
+      end
+    end
+
+    context 'when the security reports can be stored for the project' do
+      before do
+        allow(project).to receive(:can_store_security_reports?).and_return(true)
+      end
+
+      context 'when the pipeline does not have security reports' do
+        it { is_expected.to be_falsy }
+      end
+
+      context 'when the pipeline has security reports' do
+        before do
+          create(:ee_ci_build, :sast, pipeline: pipeline, project: project)
+        end
+
+        it { is_expected.to be_truthy }
+      end
+    end
+  end
 end

ee/spec/workers/build_finished_worker_spec.rb

@@ -43,12 +43,6 @@ RSpec.describe BuildFinishedWorker do
 
         subject
       end
-
-      it 'stores security scans' do
-        expect(StoreSecurityScansWorker).to receive(:perform_async)
-
-        subject
-      end
     end
 
     context 'when not on .com' do

ee/spec/workers/security/store_scans_worker_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Security::StoreScansWorker do
+  let_it_be(:pipeline) { create(:ci_pipeline) }
+
+  describe '#perform' do
+    subject(:run_worker) { described_class.new.perform(pipeline.id) }
+
+    before do
+      allow(Security::StoreScansService).to receive(:execute)
+      allow_next_found_instance_of(Ci::Pipeline) do |record|
+        allow(record).to receive(:can_store_security_reports?).and_return(can_store_security_reports)
+      end
+    end
+
+    context 'when security reports can not be stored for the pipeline' do
+      let(:can_store_security_reports) { false }
+
+      it 'does not call `Security::StoreScansService`' do
+        run_worker
+
+        expect(Security::StoreScansService).not_to have_received(:execute)
+      end
+    end
+
+    context 'when security reports can be stored for the pipeline' do
+      let(:can_store_security_reports) { true }
+
+      it 'calls `Security::StoreScansService`' do
+        run_worker
+
+        expect(Security::StoreScansService).to have_received(:execute)
+      end
+    end
+  end
+end

ee/spec/workers/store_security_scans_worker_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe StoreSecurityScansWorker do
-  describe '#perform' do
-    context 'build has security reports' do
-      let(:build) { create(:ci_build, :dast) }
-
-      before do
-        create(:ee_ci_job_artifact, :dast, job: build)
-      end
-
-      it 'stores security scans' do
-        expect(Security::StoreScansService).to receive(:new).with(build).and_call_original
-
-        StoreSecurityScansWorker.new.perform(build.id)
-      end
-    end
-
-    context 'build does not have security reports' do
-      let(:build) { create(:ci_build) }
-
-      it 'does not store security scans' do
-        expect(Security::StoreScansService).not_to receive(:new)
-
-        StoreSecurityScansWorker.new.perform(build.id)
-      end
-    end
-
-    context 'build does not exist' do
-      it 'does not store security scans' do
-        expect(Security::StoreScansService).not_to receive(:new)
-
-        StoreSecurityScansWorker.new.perform(666)
-      end
-    end
-  end
-end