Merge branch...

Merge branch '351615-ensure-gitlab-ci-reports-security-report-indicates-schema-validation-status' into 'master'

Mark report as :valid_schema or :invalid_schema

See merge request gitlab-org/gitlab!79798

config/feature_flags/development/enforce_security_report_validation.yml

+---
+name: enforce_security_report_validation
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/79798
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/351000
+milestone: '14.8'
+type: development
+group: group::threat insights
+default_enabled: false

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

@@ -42,11 +42,22 @@ module Gitlab
           attr_reader :json_data, :report, :validate
 
           def valid?
-            return true if !validate || schema_validator.valid?
+            if Feature.enabled?(:enforce_security_report_validation)
+              if !validate || schema_validator.valid?
+                report.schema_validation_status = :valid_schema
+                true
+              else
+                report.schema_validation_status = :invalid_schema
+                schema_validator.errors.each { |error| report.add_error('Schema', error) }
+                false
+              end
+            else
+              return true if !validate || schema_validator.valid?
 
-            schema_validator.errors.each { |error| report.add_error('Schema', error) }
+              schema_validator.errors.each { |error| report.add_error('Schema', error) }
 
-            false
+              false
+            end
           end
 
           def schema_validator

lib/gitlab/ci/parsers/security/validators/schemas/dependency-scanning-report-format.json

+14.0.0/dependency-scanning-report-format.json
\ No newline at end of file

lib/gitlab/ci/reports/security/report.rb

@@ -6,7 +6,7 @@ module Gitlab
       module Security
         class Report
           attr_reader :created_at, :type, :pipeline, :findings, :scanners, :identifiers
-          attr_accessor :scan, :scanned_resources, :errors, :analyzer, :version
+          attr_accessor :scan, :scanned_resources, :errors, :analyzer, :version, :schema_validation_status
 
           delegate :project_id, to: :pipeline
 

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

@@ -40,60 +40,142 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Common do
           allow(validator_class).to receive(:new).and_call_original
         end
 
-        context 'when the validate flag is set as `false`' do
-          let(:validate) { false }
+        context 'when enforce_security_report_validation is enabled' do
+          before do
+            stub_feature_flags(enforce_security_report_validation: true)
+          end
 
-          it 'does not run the validation logic' do
-            parse_report
+          context 'when the validate flag is set as `true`' do
+            let(:validate) { true }
 
-            expect(validator_class).not_to have_received(:new)
-          end
-        end
+            it 'instantiates the validator with correct params' do
+              parse_report
 
-        context 'when the validate flag is set as `true`' do
-          let(:validate) { true }
-          let(:valid?) { false }
+              expect(validator_class).to have_received(:new).with(report.type, {})
+            end
 
-          before do
-            allow_next_instance_of(validator_class) do |instance|
-              allow(instance).to receive(:valid?).and_return(valid?)
-              allow(instance).to receive(:errors).and_return(['foo'])
+            context 'when the report data is valid according to the schema' do
+              let(:valid?) { true }
+
+              before do
+                allow_next_instance_of(validator_class) do |instance|
+                  allow(instance).to receive(:valid?).and_return(valid?)
+                  allow(instance).to receive(:errors).and_return([])
+                end
+
+                allow(parser).to receive_messages(create_scanner: true, create_scan: true)
+              end
+
+              it 'does not add errors to the report' do
+                expect { parse_report }.not_to change { report.errors }.from([])
+              end
+
+              it 'adds the schema validation status to the report' do
+                parse_report
+
+                expect(report.schema_validation_status).to eq(:valid_schema)
+              end
+
+              it 'keeps the execution flow as normal' do
+                parse_report
+
+                expect(parser).to have_received(:create_scanner)
+                expect(parser).to have_received(:create_scan)
+              end
             end
 
-            allow(parser).to receive_messages(create_scanner: true, create_scan: true)
-          end
+            context 'when the report data is not valid according to the schema' do
+              let(:valid?) { false }
 
-          it 'instantiates the validator with correct params' do
-            parse_report
+              before do
+                allow_next_instance_of(validator_class) do |instance|
+                  allow(instance).to receive(:valid?).and_return(valid?)
+                  allow(instance).to receive(:errors).and_return(['foo'])
+                end
 
-            expect(validator_class).to have_received(:new).with(report.type, {})
-          end
+                allow(parser).to receive_messages(create_scanner: true, create_scan: true)
+              end
+
+              it 'adds errors to the report' do
+                expect { parse_report }.to change { report.errors }.from([]).to([{ message: 'foo', type: 'Schema' }])
+              end
+
+              it 'adds the schema validation status to the report' do
+                parse_report
 
-          context 'when the report data is not valid according to the schema' do
-            it 'adds errors to the report' do
-              expect { parse_report }.to change { report.errors }.from([]).to([{ message: 'foo', type: 'Schema' }])
+                expect(report.schema_validation_status).to eq(:invalid_schema)
+              end
+
+              it 'does not try to create report entities' do
+                parse_report
+
+                expect(parser).not_to have_received(:create_scanner)
+                expect(parser).not_to have_received(:create_scan)
+              end
             end
+          end
+        end
+
+        context 'when enforce_security_report_validation is disabled' do
+          before do
+            stub_feature_flags(enforce_security_report_validation: false)
+          end
+
+          context 'when the validate flag is set as `false`' do
+            let(:validate) { false }
 
-            it 'does not try to create report entities' do
+            it 'does not run the validation logic' do
               parse_report
 
-              expect(parser).not_to have_received(:create_scanner)
-              expect(parser).not_to have_received(:create_scan)
+              expect(validator_class).not_to have_received(:new)
             end
           end
 
-          context 'when the report data is valid according to the schema' do
-            let(:valid?) { true }
+          context 'when the validate flag is set as `true`' do
+            let(:validate) { true }
+            let(:valid?) { false }
 
-            it 'does not add errors to the report' do
-              expect { parse_report }.not_to change { report.errors }.from([])
+            before do
+              allow_next_instance_of(validator_class) do |instance|
+                allow(instance).to receive(:valid?).and_return(valid?)
+                allow(instance).to receive(:errors).and_return(['foo'])
+              end
+
+              allow(parser).to receive_messages(create_scanner: true, create_scan: true)
             end
 
-            it 'keeps the execution flow as normal' do
+            it 'instantiates the validator with correct params' do
               parse_report
 
-              expect(parser).to have_received(:create_scanner)
-              expect(parser).to have_received(:create_scan)
+              expect(validator_class).to have_received(:new).with(report.type, {})
+            end
+
+            context 'when the report data is not valid according to the schema' do
+              it 'adds errors to the report' do
+                expect { parse_report }.to change { report.errors }.from([]).to([{ message: 'foo', type: 'Schema' }])
+              end
+
+              it 'does not try to create report entities' do
+                parse_report
+
+                expect(parser).not_to have_received(:create_scanner)
+                expect(parser).not_to have_received(:create_scan)
+              end
+            end
+
+            context 'when the report data is valid according to the schema' do
+              let(:valid?) { true }
+
+              it 'does not add errors to the report' do
+                expect { parse_report }.not_to change { report.errors }.from([])
+              end
+
+              it 'keeps the execution flow as normal' do
+                parse_report
+
+                expect(parser).to have_received(:create_scanner)
+                expect(parser).to have_received(:create_scan)
+              end
             end
           end
         end