Merge branch '343332_remove_security_report_ingestion_framework_ff' into 'master'

Remove `security_report_ingestion_framework` feature flag

See merge request gitlab-org/gitlab!81021

config/feature_flags/development/security_report_ingestion_framework.yml

----
-name: security_report_ingestion_framework
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/66735
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/343332
-milestone: '14.4'
-type: development
-group: group::threat insights
-default_enabled: true

ee/app/services/security/store_reports_service.rb

-# frozen_string_literal: true
-
-module Security
-  # Service for storing security reports into the database.
-  #
-  class StoreReportsService < ::BaseService
-    def initialize(pipeline)
-      @pipeline = pipeline
-      @errors = []
-    end
-
-    def execute
-      set_latest_pipeline!
-      mark_project_as_vulnerable!
-      store_reports
-
-      errors.any? ? error(full_errors) : success
-    end
-
-    private
-
-    attr_reader :pipeline, :errors
-
-    delegate :project, to: :pipeline, private: true
-
-    def store_reports
-      pipeline.security_reports.reports.each do |report_type, report|
-        result = StoreReportService.new(pipeline, report).execute
-        errors << result[:message] if result[:status] == :error
-      end
-    end
-
-    def mark_project_as_vulnerable!
-      project.project_setting.update!(has_vulnerabilities: true)
-    end
-
-    def set_latest_pipeline!
-      Vulnerabilities::Statistic.set_latest_pipeline_with(pipeline)
-    end
-
-    def full_errors
-      errors.join(", ")
-    end
-  end
-end

ee/app/workers/store_security_reports_worker.rb

@@ -18,11 +18,7 @@ class StoreSecurityReportsWorker # rubocop:disable Scalability/IdempotentWorker
     Ci::Pipeline.find(pipeline_id).try do |pipeline|
       break unless pipeline.project.can_store_security_reports?
 
-      if Feature.enabled?(:security_report_ingestion_framework, pipeline.project, default_enabled: :yaml)
-        ::Security::Ingestion::IngestReportsService.execute(pipeline)
-      else
-        ::Security::StoreReportsService.new(pipeline).execute
-      end
+      ::Security::Ingestion::IngestReportsService.execute(pipeline)
 
       if revoke_secret_detection_token?(pipeline)
         logger.info "StoreSecurityReportsWorker: token revocation started for pipeline: #{pipeline.id}"

ee/spec/services/security/store_reports_service_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe Security::StoreReportsService do
-  let_it_be(:user) { create(:user) }
-  let_it_be(:group) { create(:group) }
-  let_it_be(:project) { create(:project, :public, namespace: group) }
-  let_it_be(:pipeline) { create(:ci_pipeline, project: project) }
-
-  describe '#execute' do
-    subject(:execute_service_object) { described_class.new(pipeline).execute }
-
-    context 'when there are reports' do
-      before do
-        stub_licensed_features(sast: true, dependency_scanning: true, container_scanning: true, security_dashboard: true)
-        create(:ee_ci_build, :sast, pipeline: pipeline)
-        create(:ee_ci_build, :dependency_scanning, pipeline: pipeline)
-        create(:ee_ci_build, :container_scanning, pipeline: pipeline)
-        project.add_developer(user)
-        allow(pipeline).to receive(:user).and_return(user)
-      end
-
-      it 'initializes and execute a StoreReportService for each report' do
-        expect(Security::StoreReportService).to receive(:new)
-          .exactly(3).times.with(pipeline, instance_of(::Gitlab::Ci::Reports::Security::Report))
-          .and_wrap_original do |method, *original_args|
-            method.call(*original_args).tap do |store_service|
-              expect(store_service).to receive(:execute).once.and_call_original
-            end
-          end
-
-        execute_service_object
-      end
-
-      it 'marks the project as vulnerable' do
-        expect { execute_service_object }.to change { project.reload.project_setting.has_vulnerabilities }.from(false).to(true)
-      end
-
-      it 'updates the `latest_pipeline_id` attribute of the associated `vulnerability_statistic` record' do
-        expect { execute_service_object }.to change { project.reload.vulnerability_statistic&.latest_pipeline_id }.from(nil).to(pipeline.id)
-      end
-
-      context 'when the StoreReportService raises an error' do
-        let(:error) { RuntimeError.new('foo') }
-
-        before do
-          allow_next_instance_of(Security::StoreReportService) do |service_object|
-            allow(service_object).to receive(:execute).and_raise(error)
-          end
-        end
-
-        it 'marks the project as vulnerable' do
-          expect { execute_service_object }.to raise_error(error)
-                                           .and change { project.reload.project_setting.has_vulnerabilities }.from(false).to(true)
-        end
-
-        it 'updates the `latest_pipeline_id` attribute of the associated `vulnerability_statistic` record' do
-          expect { execute_service_object }.to raise_error(error)
-                                           .and change { project.reload.vulnerability_statistic&.latest_pipeline_id }.from(nil).to(pipeline.id)
-        end
-      end
-
-      context 'when StoreReportService returns an error for a report' do
-        let(:reports) { Gitlab::Ci::Reports::Security::Reports.new(pipeline) }
-        let(:sast_report) { reports.get_report('sast', sast_artifact) }
-        let(:dast_report) { reports.get_report('dast', dast_artifact) }
-        let(:success) { { status: :success } }
-        let(:error) { { status: :error, message: "something went wrong" } }
-        let(:sast_artifact) { create(:ee_ci_job_artifact, :sast) }
-        let(:dast_artifact) { create(:ee_ci_job_artifact, :dast) }
-
-        before do
-          allow(pipeline).to receive(:security_reports).and_return(reports)
-        end
-
-        it 'returns the errors after having processed all reports' do
-          expect_next_instance_of(Security::StoreReportService, pipeline, sast_report) do |store_service|
-            expect(store_service).to receive(:execute).and_return(error)
-          end
-          expect_next_instance_of(Security::StoreReportService, pipeline, dast_report) do |store_service|
-            expect(store_service).to receive(:execute).and_return(success)
-          end
-
-          is_expected.to eq(error)
-        end
-      end
-    end
-  end
-end

ee/spec/workers/store_security_reports_worker_spec.rb

@@ -70,29 +70,10 @@ RSpec.describe StoreSecurityReportsWorker do
           described_class.new.perform(pipeline.id)
         end
 
-        context 'when the `security_report_ingestion_framework` feature is enabled' do
-          before do
-            stub_feature_flags(security_report_ingestion_framework: project)
-          end
+        it 'executes IngestReportsService for given pipeline' do
+          expect(::Security::Ingestion::IngestReportsService).to receive(:execute).with(pipeline)
 
-          it 'executes IngestReportsService for given pipeline' do
-            expect(::Security::Ingestion::IngestReportsService).to receive(:execute).with(pipeline)
-
-            described_class.new.perform(pipeline.id)
-          end
-        end
-
-        context 'when the `security_report_ingestion_framework` feature is disabled' do
-          before do
-            stub_feature_flags(security_report_ingestion_framework: false)
-          end
-
-          it 'executes StoreReportsService for given pipeline' do
-            expect(Security::StoreReportsService).to receive(:new)
-              .with(pipeline).once.and_call_original
-
-            described_class.new.perform(pipeline.id)
-          end
+          described_class.new.perform(pipeline.id)
         end
       end
     end
@@ -100,8 +81,8 @@ RSpec.describe StoreSecurityReportsWorker do
     context "when security reports feature is not available" do
       let(:default_branch) { pipeline.ref }
 
-      it 'does not execute StoreReportsService' do
-        expect(Security::StoreReportsService).not_to receive(:new)
+      it 'does not execute IngestReportsService' do
+        expect(::Security::Ingestion::IngestReportsService).not_to receive(:execute)
 
         described_class.new.perform(pipeline.id)
       end