Merge branch '358020-impove-logging-for-deprecated-and-unsupported-reports' into 'master'

Impove logging for deprecated and unsupported reports

See merge request gitlab-org/gitlab!84522

lib/gitlab/ci/parsers/security/common.rb

@@ -72,7 +72,13 @@ module Gitlab
           end
 
           def schema_validator
-            @schema_validator ||= ::Gitlab::Ci::Parsers::Security::Validators::SchemaValidator.new(report.type, report_data, report.version, project: @project)
+            @schema_validator ||= ::Gitlab::Ci::Parsers::Security::Validators::SchemaValidator.new(
+              report.type,
+              report_data,
+              report.version,
+              project: @project,
+              scanner: top_level_scanner
+            )
           end
 
           def report_data

lib/gitlab/ci/parsers/security/validators/schema_validator.rb

@@ -86,11 +86,12 @@ module Gitlab
               end
             end
 
-            def initialize(report_type, report_data, report_version = nil, project: nil)
+            def initialize(report_type, report_data, report_version = nil, project: nil, scanner: nil)
               @report_type = report_type&.to_sym
               @report_data = report_data
               @report_version = report_version
               @project = project
+              @scanner = scanner
               @errors = []
               @warnings = []
               @deprecation_warnings = []
@@ -137,7 +138,9 @@ module Gitlab
                 security_report_type: report_type,
                 security_report_version: report_version,
                 project_id: @project.id,
-                security_report_failure: problem_type
+                security_report_failure: problem_type,
+                security_report_scanner_id: @scanner&.dig('id'),
+                security_report_scanner_version: @scanner&.dig('version')
               )
             end
 

spec/lib/gitlab/ci/parsers/security/common_spec.rb

@@ -4,6 +4,18 @@ require 'spec_helper'
 
 RSpec.describe Gitlab::Ci::Parsers::Security::Common do
   describe '#parse!' do
+    let_it_be(:scanner_data) do
+      {
+        scan: {
+          scanner: {
+              id: "gemnasium",
+              name: "Gemnasium",
+              version: "2.1.0"
+          }
+        }
+      }
+    end
+
     where(vulnerability_finding_signatures_enabled: [true, false])
     with_them do
       let_it_be(:pipeline) { create(:ci_pipeline) }
@@ -30,7 +42,9 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Common do
 
       describe 'schema validation' do
         let(:validator_class) { Gitlab::Ci::Parsers::Security::Validators::SchemaValidator }
-        let(:parser) { described_class.new('{}', report, vulnerability_finding_signatures_enabled, validate: validate) }
+        let(:data) { {}.merge(scanner_data) }
+        let(:json_data) { data.to_json }
+        let(:parser) { described_class.new(json_data, report, vulnerability_finding_signatures_enabled, validate: validate) }
 
         subject(:parse_report) { parser.parse! }
 
@@ -57,7 +71,13 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Common do
           it 'instantiates the validator with correct params' do
             parse_report
 
-            expect(validator_class).to have_received(:new).with(report.type, {}, report.version, project: pipeline.project)
+            expect(validator_class).to have_received(:new).with(
+              report.type,
+              data.deep_stringify_keys,
+              report.version,
+              project: pipeline.project,
+              scanner: data.dig(:scan, :scanner).deep_stringify_keys
+            )
           end
 
           context 'when the report data is not valid according to the schema' do
@@ -119,7 +139,13 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Common do
           it 'instantiates the validator with correct params' do
             parse_report
 
-            expect(validator_class).to have_received(:new).with(report.type, {}, report.version, project: pipeline.project)
+            expect(validator_class).to have_received(:new).with(
+              report.type,
+              data.deep_stringify_keys,
+              report.version,
+              project: pipeline.project,
+              scanner: data.dig(:scan, :scanner).deep_stringify_keys
+            )
           end
 
           context 'when the report data is not valid according to the schema' do

spec/lib/gitlab/ci/parsers/security/validators/schema_validator_spec.rb

@@ -5,7 +5,15 @@ require 'spec_helper'
 RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator do
   let_it_be(:project) { create(:project) }
 
-  let(:validator) { described_class.new(report_type, report_data, report_version, project: project) }
+  let(:scanner) do
+    {
+      'id' => 'gemnasium',
+      'name' => 'Gemnasium',
+      'version' => '2.1.0'
+    }
+  end
+
+  let(:validator) { described_class.new(report_type, report_data, report_version, project: project, scanner: scanner) }
 
   describe 'SUPPORTED_VERSIONS' do
     schema_path = Rails.root.join("lib", "gitlab", "ci", "parsers", "security", "validators", "schemas")
@@ -84,7 +92,9 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator do
             security_report_type: report_type,
             security_report_version: report_version,
             project_id: project.id,
-            security_report_failure: 'schema_validation_fails'
+            security_report_failure: 'schema_validation_fails',
+            security_report_scanner_id: 'gemnasium',
+            security_report_scanner_version: '2.1.0'
           )
 
           subject
@@ -112,7 +122,9 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator do
             security_report_type: report_type,
             security_report_version: report_version,
             project_id: project.id,
-            security_report_failure: 'using_deprecated_schema_version'
+            security_report_failure: 'using_deprecated_schema_version',
+            security_report_scanner_id: 'gemnasium',
+            security_report_scanner_version: '2.1.0'
           )
 
           subject
@@ -175,7 +187,9 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator do
               security_report_type: report_type,
               security_report_version: report_version,
               project_id: project.id,
-              security_report_failure: 'using_unsupported_schema_version'
+              security_report_failure: 'using_unsupported_schema_version',
+              security_report_scanner_id: 'gemnasium',
+              security_report_scanner_version: '2.1.0'
             )
 
             subject
@@ -189,6 +203,34 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator do
             }
           end
 
+          context 'and scanner information is empty' do
+            let(:scanner) { {} }
+
+            it 'logs related information' do
+              expect(Gitlab::AppLogger).to receive(:info).with(
+                message: "security report schema validation problem",
+                security_report_type: report_type,
+                security_report_version: report_version,
+                project_id: project.id,
+                security_report_failure: 'schema_validation_fails',
+                security_report_scanner_id: nil,
+                security_report_scanner_version: nil
+              )
+
+              expect(Gitlab::AppLogger).to receive(:info).with(
+                message: "security report schema validation problem",
+                security_report_type: report_type,
+                security_report_version: report_version,
+                project_id: project.id,
+                security_report_failure: 'using_unsupported_schema_version',
+                security_report_scanner_id: nil,
+                security_report_scanner_version: nil
+              )
+
+              subject
+            end
+          end
+
           it { is_expected.to be_falsey }
         end
       end