Do not silently ignore invalid findings on finding creation

If there is a problem with a scaner, it's better if we catch this
instead of ignoring the invalid data.

ee/app/services/security/store_report_service.rb

@@ -42,7 +42,7 @@ module Security
     end
 
     def create_vulnerability_finding(finding)
-      return unless finding.valid?
+      return put_warning_for(finding) && nil unless finding.valid?
 
       vulnerability_params = finding.to_hash.except(:compare_key, :identifiers, :location, :scanner)
       vulnerability_finding = create_or_find_vulnerability_finding(finding, vulnerability_params)
@@ -172,5 +172,9 @@ module Security
         end.to_h
       end
     end
+
+    def put_warning_for(finding)
+      Gitlab::AppLogger.warn(message: "Invalid vulnerability finding record found", finding: finding.to_hash)
+    end
   end
 end

ee/spec/factories/ci/job_artifacts.rb

@@ -179,22 +179,6 @@ FactoryBot.define do
       end
     end
 
-    trait :sast_with_missing_identifiers do
-      file_type { :sast }
-      file_format { :raw }
-
-      after(:build) do |artifact, _|
-        file = fixture_file_upload(
-          Rails.root.join('ee/spec/fixtures/security_reports/master/gl-sast-report.json'), 'application/json')
-        data = Gitlab::Json.parse(file.tempfile.read)['vulnerabilities'].each { |v| v.delete('identifiers') }.to_json
-        output = Tempfile.new("gl-sast-missing-identifiers")
-        output.write(data)
-        artifact.file = fixture_file_upload(output.path, 'application/json')
-        output.close
-        output.unlink
-      end
-    end
-
     trait :license_management do
       to_create { |instance| instance.save!(validate: false) }
 

ee/spec/services/security/store_report_service_spec.rb

@@ -181,20 +181,14 @@ RSpec.describe Security::StoreReportService, '#execute' do
       end
     end
 
-    context 'when the finding does not include a scanner' do
-      let(:bad_pipeline) { create(:ci_pipeline, project: project) }
-      let(:bad_build) { create(:ci_build, pipeline: bad_pipeline) }
-      let!(:bad_artifact) { create(:ee_ci_job_artifact, :sast_with_missing_scanner, job: bad_build) }
-      let(:bad_report) { bad_pipeline.security_reports.get_report(report_type.to_s, bad_artifact) }
-      let(:report_type) { :sast }
-
+    context 'when the finding is not valid' do
       before do
-        project.add_developer(user)
-        allow(bad_pipeline).to receive(:user).and_return(user)
+        allow(Gitlab::AppLogger).to receive(:warn)
+        allow_next_instance_of(::Gitlab::Ci::Reports::Security::Finding) do |finding|
+          allow(finding).to receive(:valid?).and_return(false)
+        end
       end
 
-      subject { described_class.new(bad_pipeline, bad_report).execute }
-
       it 'does not create a new finding' do
         expect { subject }.not_to change { Vulnerabilities::Finding.count }
       end
@@ -202,28 +196,11 @@ RSpec.describe Security::StoreReportService, '#execute' do
       it 'does not raise an error' do
         expect { subject }.not_to raise_error
       end
-    end
-
-    context 'when the finding does not include a primary identifier' do
-      let(:bad_project) { bad_artifact.project }
-      let(:bad_pipeline) { bad_artifact.job.pipeline }
-      let!(:bad_artifact) { create(:ee_ci_job_artifact, :sast_with_missing_identifiers) }
-      let(:bad_report) { bad_pipeline.security_reports.get_report(report_type.to_s, bad_artifact) }
-      let(:report_type) { :sast }
-
-      before do
-        bad_project.add_developer(user)
-        allow(bad_pipeline).to receive(:user).and_return(user)
-      end
 
-      subject { described_class.new(bad_pipeline, bad_report).execute }
+      it 'puts a warning log' do
+        subject
 
-      it 'does not create a new finding' do
-        expect { subject }.not_to change { Vulnerabilities::Finding.count }
-      end
-
-      it 'does not raise an error' do
-        expect { subject }.not_to raise_error
+        expect(Gitlab::AppLogger).to have_received(:warn).exactly(new_report.findings.length).times
       end
     end
   end