diff --git a/ee/app/services/security/store_findings_service.rb b/ee/app/services/security/store_findings_service.rb
index fcd5aa334edfaf9da34eba96a737b704e36deb3f..82fe9b5fcaf1f966be3412eb5cfc247d5d09127b 100644
--- a/ee/app/services/security/store_findings_service.rb
+++ b/ee/app/services/security/store_findings_service.rb
@@ -7,12 +7,13 @@ class StoreFindingsService < ::BaseService
attr_reader :security_scan, :report, :deduplicated_finding_uuids
- def self.execute(security_scan, report, deduplicated_finding_uuids)
- new(security_scan, report, deduplicated_finding_uuids).execute
+ def self.execute(security_scan, scanner, report, deduplicated_finding_uuids)
+ new(security_scan, scanner, report, deduplicated_finding_uuids).execute
end
- def initialize(security_scan, report, deduplicated_finding_uuids)
+ def initialize(security_scan, scanner, report, deduplicated_finding_uuids)
@security_scan = security_scan
+ @scanner = scanner
@report = report
@deduplicated_finding_uuids = deduplicated_finding_uuids
end
@@ -26,6 +27,8 @@ def execute
private
+ attr_reader :scanner
+
delegate :project, to: :security_scan
def already_stored?
@@ -63,7 +66,7 @@ def finding_data(report_finding)
uuid: report_finding.uuid,
overridden_uuid: report_finding.overridden_uuid,
project_fingerprint: report_finding.project_fingerprint,
- scanner_id: persisted_scanner_for(report_finding.scanner).id,
+ scanner_id: scanner.id,
deduplicated: deduplicated?(report_finding),
finding_data: finding_data_for(report_finding)
}
@@ -89,24 +92,5 @@ def finding_data_for(report_finding)
raw_source_code_extract: report_finding.raw_source_code_extract
}
end
-
- def persisted_scanner_for(report_scanner)
- existing_scanners[report_scanner.key] ||= create_scanner!(report_scanner)
- end
-
- def existing_scanners
- @existing_scanners ||= project.vulnerability_scanners
- .with_external_id(scanner_external_ids)
- .group_by(&:external_id)
- .transform_values(&:first)
- end
-
- def scanner_external_ids
- report.scanners.values.map(&:external_id)
- end
-
- def create_scanner!(report_scanner)
- project.vulnerability_scanners.create!(report_scanner.to_hash)
- end
end
end
diff --git a/ee/app/services/security/store_scan_service.rb b/ee/app/services/security/store_scan_service.rb
index 7968ded1fe2368350c6c7663d12fbb25fc470b92..2170480b94864607c1dbf59735726b10f40f9322 100644
--- a/ee/app/services/security/store_scan_service.rb
+++ b/ee/app/services/security/store_scan_service.rb
@@ -68,7 +68,7 @@ def initial_scan_status
end
def store_findings
- StoreFindingsService.execute(security_scan, security_report, register_finding_keys).then do |result|
+ StoreFindingsService.execute(security_scan, vulnerability_scanner, security_report, register_finding_keys).then do |result|
# If `StoreFindingsService` returns error, it means the findings
# have already been stored before so we may re-run the deduplication logic.
update_deduplicated_findings if result[:status] == :error && deduplicate_findings?
@@ -123,5 +123,11 @@ def mark_scan_as_failed!
security_scan.add_processing_error!(SCAN_INGESTION_ERROR)
end
+
+ def vulnerability_scanner
+ project.vulnerability_scanners.safe_find_or_create_by!(external_id: security_report.primary_scanner.external_id) do |scanner|
+ scanner.assign_attributes(security_report.primary_scanner.to_hash)
+ end
+ end
end
end
diff --git a/ee/spec/services/security/store_findings_service_spec.rb b/ee/spec/services/security/store_findings_service_spec.rb
index d90c734cd704b5909d74377bf7f8eaffb0de4f58..842d810c72d53d9568e5d79c67ab8a6da2da4b2c 100644
--- a/ee/spec/services/security/store_findings_service_spec.rb
+++ b/ee/spec/services/security/store_findings_service_spec.rb
@@ -6,6 +6,7 @@
let_it_be(:findings_partition_number) { Security::Finding.active_partition_number }
let_it_be(:security_scan) { create(:security_scan, findings_partition_number: findings_partition_number) }
let_it_be(:project) { security_scan.project }
+ let_it_be(:scanner) { create(:vulnerabilities_scanner, project: project) }
let_it_be(:security_finding_1) { build(:ci_reports_security_finding) }
let_it_be(:security_finding_2) { build(:ci_reports_security_finding) }
let_it_be(:security_finding_3) { build(:ci_reports_security_finding) }
@@ -21,7 +22,7 @@
end
describe '#execute' do
- let(:service_object) { described_class.new(security_scan, report, deduplicated_finding_uuids) }
+ let(:service_object) { described_class.new(security_scan, scanner, report, deduplicated_finding_uuids) }
subject(:store_findings) { service_object.execute }
@@ -83,22 +84,6 @@
)
)
end
-
- context 'when the scanners already exist in the database' do
- before do
- create(:vulnerabilities_scanner, project: project, external_id: security_scanner.key)
- end
-
- it 'does not create new scanner entries in the database' do
- expect { store_findings }.not_to change(Vulnerabilities::Scanner, :count)
- end
- end
-
- context 'when the scanner does not exist in the database' do
- it 'creates new scanner entry in the database' do
- expect { store_findings }.to change { project.vulnerability_scanners.count }.by(1)
- end
- end
end
end
end
diff --git a/ee/spec/services/security/store_scan_service_spec.rb b/ee/spec/services/security/store_scan_service_spec.rb
index 8b4b625d50e36cd680a32fe9847298be095650d0..6d8f94a1f5e83322bb3164670c93dfcbd7f705fe 100644
--- a/ee/spec/services/security/store_scan_service_spec.rb
+++ b/ee/spec/services/security/store_scan_service_spec.rb
@@ -185,10 +185,41 @@
artifact.security_report.errors.clear
end
- it 'calls the `Security::StoreFindingsService` to store findings' do
- store_scan
+ describe 'executing `StoreFindingsService`' do
+ let_it_be(:project) { artifact.project }
+ let_it_be(:security_scanner) { artifact.security_report.primary_scanner }
+
+ context 'when there is already a vulnerability scanner' do
+ let_it_be(:scanner) do
+ create(:vulnerabilities_scanner, project: project, external_id: security_scanner.external_id)
+ end
+
+ it 'calls the `Security::StoreFindingsService` to store findings' do
+ store_scan
- expect(Security::StoreFindingsService).to have_received(:execute)
+ expect(Security::StoreFindingsService).to have_received(:execute).with(
+ an_instance_of(Security::Scan), scanner, artifact.security_report, an_instance_of(Array))
+ end
+
+ it 'does not create a new scanner' do
+ expect { store_scan }.not_to change { Vulnerabilities::Scanner.count }
+ end
+ end
+
+ context 'when there is no vulnerability scanner' do
+ it 'calls the `Security::StoreFindingsService` to store findings with the recently created scanner' do
+ store_scan
+
+ created_scanner = project.vulnerability_scanners.find_by(external_id: security_scanner.external_id)
+
+ expect(Security::StoreFindingsService).to have_received(:execute).with(
+ an_instance_of(Security::Scan), created_scanner, artifact.security_report, an_instance_of(Array))
+ end
+
+ it 'creates a new scanner' do
+ expect { store_scan }.to change { Vulnerabilities::Scanner.count }.by(1)
+ end
+ end
end
context 'when the report has no warnings' do