Commit 70a6ae5e authored by Mehmet Emin INAC's avatar Mehmet Emin INAC Committed by Thong Kuah

Do not crash the whole background job if one type of report fails

Currently there is a problem with the container scanning reports on
staging environment which breaks all the other type of reports from
being ingested into the system.

With this change, we will at least ingest the findings for other report
types.
parent 3d606eb2
---
title: Do not crash the ingestion of all security reports if there is an invalid report
artifact
merge_request: 49181
author:
type: fixed
...@@ -22,6 +22,8 @@ module Security ...@@ -22,6 +22,8 @@ module Security
store_scan_for(artifact, deduplicate) store_scan_for(artifact, deduplicate)
end end
end end
rescue Gitlab::Ci::Parsers::ParserError => error
Gitlab::ErrorTracking.track_exception(error)
end end
private private
......
...@@ -36,46 +36,67 @@ RSpec.describe Security::StoreGroupedScansService do ...@@ -36,46 +36,67 @@ RSpec.describe Security::StoreGroupedScansService do
subject(:store_scan_group) { service_object.execute } subject(:store_scan_group) { service_object.execute }
before do context 'when there is a parsing error' do
allow(Security::StoreScanService).to receive(:execute).and_return(true) let(:expected_error) { Gitlab::Ci::Parsers::ParserError.new('Foo') }
end
before do
allow(Security::StoreScanService).to receive(:execute).and_raise(expected_error)
allow(Gitlab::ErrorTracking).to receive(:track_exception)
end
context 'when the artifacts are not dependency_scanning' do it 'does not propagate the error to the caller' do
it 'calls the Security::StoreScanService with ordered artifacts' do expect { store_scan_group }.not_to raise_error
end
it 'tracks the error' do
store_scan_group store_scan_group
expect(Security::StoreScanService).to have_received(:execute).with(artifact_1, empty_set, false).ordered expect(Gitlab::ErrorTracking).to have_received(:track_exception).with(expected_error)
expect(Security::StoreScanService).to have_received(:execute).with(artifact_3, empty_set, true).ordered
expect(Security::StoreScanService).to have_received(:execute).with(artifact_2, empty_set, true).ordered
end end
end end
context 'when the artifacts are dependency_scanning' do context 'when there is no error' do
let(:report_type) { :dependency_scanning }
let(:build_4) { create(:ee_ci_build, name: 'Report 0') }
let(:artifact_4) { create(:ee_ci_job_artifact, report_type, job: build_4) }
let(:scanner_1) { instance_double(::Gitlab::Ci::Reports::Security::Scanner, external_id: 'this is an unknown id') }
let(:scanner_2) { instance_double(::Gitlab::Ci::Reports::Security::Scanner, external_id: 'bundler_audit') }
let(:scanner_3) { instance_double(::Gitlab::Ci::Reports::Security::Scanner, external_id: 'retire.js') }
let(:mock_report_1) { instance_double(::Gitlab::Ci::Reports::Security::Report, primary_scanner: scanner_1) }
let(:mock_report_2) { instance_double(::Gitlab::Ci::Reports::Security::Report, primary_scanner: scanner_2) }
let(:mock_report_3) { instance_double(::Gitlab::Ci::Reports::Security::Report, primary_scanner: scanner_3) }
let(:artifacts) { [artifact_1, artifact_2, artifact_3, artifact_4] }
before do before do
allow(artifact_1).to receive(:security_report).and_return(mock_report_1) allow(Security::StoreScanService).to receive(:execute).and_return(true)
allow(artifact_2).to receive(:security_report).and_return(mock_report_2)
allow(artifact_3).to receive(:security_report).and_return(mock_report_3)
allow(artifact_4).to receive(:security_report).and_return(mock_report_2)
end end
it 'calls the Security::StoreScanService with ordered artifacts' do context 'when the artifacts are not dependency_scanning' do
store_scan_group it 'calls the Security::StoreScanService with ordered artifacts' do
store_scan_group
expect(Security::StoreScanService).to have_received(:execute).with(artifact_1, empty_set, false).ordered
expect(Security::StoreScanService).to have_received(:execute).with(artifact_3, empty_set, true).ordered
expect(Security::StoreScanService).to have_received(:execute).with(artifact_2, empty_set, true).ordered
end
end
expect(Security::StoreScanService).to have_received(:execute).with(artifact_4, empty_set, false).ordered context 'when the artifacts are dependency_scanning' do
expect(Security::StoreScanService).to have_received(:execute).with(artifact_2, empty_set, true).ordered let(:report_type) { :dependency_scanning }
expect(Security::StoreScanService).to have_received(:execute).with(artifact_3, empty_set, true).ordered let(:build_4) { create(:ee_ci_build, name: 'Report 0') }
expect(Security::StoreScanService).to have_received(:execute).with(artifact_1, empty_set, true).ordered let(:artifact_4) { create(:ee_ci_job_artifact, report_type, job: build_4) }
let(:scanner_1) { instance_double(::Gitlab::Ci::Reports::Security::Scanner, external_id: 'this is an unknown id') }
let(:scanner_2) { instance_double(::Gitlab::Ci::Reports::Security::Scanner, external_id: 'bundler_audit') }
let(:scanner_3) { instance_double(::Gitlab::Ci::Reports::Security::Scanner, external_id: 'retire.js') }
let(:mock_report_1) { instance_double(::Gitlab::Ci::Reports::Security::Report, primary_scanner: scanner_1) }
let(:mock_report_2) { instance_double(::Gitlab::Ci::Reports::Security::Report, primary_scanner: scanner_2) }
let(:mock_report_3) { instance_double(::Gitlab::Ci::Reports::Security::Report, primary_scanner: scanner_3) }
let(:artifacts) { [artifact_1, artifact_2, artifact_3, artifact_4] }
before do
allow(artifact_1).to receive(:security_report).and_return(mock_report_1)
allow(artifact_2).to receive(:security_report).and_return(mock_report_2)
allow(artifact_3).to receive(:security_report).and_return(mock_report_3)
allow(artifact_4).to receive(:security_report).and_return(mock_report_2)
end
it 'calls the Security::StoreScanService with ordered artifacts' do
store_scan_group
expect(Security::StoreScanService).to have_received(:execute).with(artifact_4, empty_set, false).ordered
expect(Security::StoreScanService).to have_received(:execute).with(artifact_2, empty_set, true).ordered
expect(Security::StoreScanService).to have_received(:execute).with(artifact_3, empty_set, true).ordered
expect(Security::StoreScanService).to have_received(:execute).with(artifact_1, empty_set, true).ordered
end
end end
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment