Commit fef17598 authored by Michał Zając's avatar Michał Zając

Mark report as :valid_schema or :invalid_schema

Simplest approach to get the ball rolling. Will require some refinement.
parent d8a469b0
---
name: enforce_security_report_validation
introduced_by_url:
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/351000
milestone: '14.8'
type: development
group: group::threat insights
default_enabled: false
...@@ -42,11 +42,22 @@ module Gitlab ...@@ -42,11 +42,22 @@ module Gitlab
attr_reader :json_data, :report, :validate attr_reader :json_data, :report, :validate
def valid? 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 end
def schema_validator def schema_validator
......
...@@ -6,7 +6,7 @@ module Gitlab ...@@ -6,7 +6,7 @@ module Gitlab
module Security module Security
class Report class Report
attr_reader :created_at, :type, :pipeline, :findings, :scanners, :identifiers 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 delegate :project_id, to: :pipeline
......
...@@ -40,60 +40,142 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Common do ...@@ -40,60 +40,142 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Common do
allow(validator_class).to receive(:new).and_call_original allow(validator_class).to receive(:new).and_call_original
end end
context 'when the validate flag is set as `false`' do context 'when enforce_security_report_validation is enabled' do
let(:validate) { false } before do
stub_feature_flags(enforce_security_report_validation: true)
end
it 'does not run the validation logic' do context 'when the validate flag is set as `true`' do
parse_report let(:validate) { true }
expect(validator_class).not_to have_received(:new) it 'instantiates the validator with correct params' do
end parse_report
end
context 'when the validate flag is set as `true`' do expect(validator_class).to have_received(:new).with(report.type, {})
let(:validate) { true } end
let(:valid?) { false }
before do context 'when the report data is valid according to the schema' do
allow_next_instance_of(validator_class) do |instance| let(:valid?) { true }
allow(instance).to receive(:valid?).and_return(valid?)
allow(instance).to receive(:errors).and_return(['foo']) 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 end
allow(parser).to receive_messages(create_scanner: true, create_scan: true) context 'when the report data is not valid according to the schema' do
end let(:valid?) { false }
it 'instantiates the validator with correct params' do before do
parse_report 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, {}) allow(parser).to receive_messages(create_scanner: true, create_scan: true)
end 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 expect(report.schema_validation_status).to eq(:invalid_schema)
it 'adds errors to the report' do end
expect { parse_report }.to change { report.errors }.from([]).to([{ message: 'foo', type: 'Schema' }])
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
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 parse_report
expect(parser).not_to have_received(:create_scanner) expect(validator_class).not_to have_received(:new)
expect(parser).not_to have_received(:create_scan)
end end
end end
context 'when the report data is valid according to the schema' do context 'when the validate flag is set as `true`' do
let(:valid?) { true } let(:validate) { true }
let(:valid?) { false }
it 'does not add errors to the report' do before do
expect { parse_report }.not_to change { report.errors }.from([]) 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 end
it 'keeps the execution flow as normal' do it 'instantiates the validator with correct params' do
parse_report parse_report
expect(parser).to have_received(:create_scanner) expect(validator_class).to have_received(:new).with(report.type, {})
expect(parser).to have_received(:create_scan) 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 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