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

Cleanup show_report_validation_warnings flag

Changelog: other
parent 964c59c0
---
name: show_report_validation_warnings
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/80930
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/353125
milestone: '14.9'
type: development
group: group::threat insights
default_enabled: true
......@@ -43,26 +43,25 @@ module Gitlab
attr_reader :json_data, :report, :validate
def valid?
if Feature.enabled?(:show_report_validation_warnings, default_enabled: :yaml)
# We want validation to happen regardless of VALIDATE_SCHEMA CI variable
schema_validation_passed = schema_validator.valid?
if validate
schema_validator.errors.each { |error| report.add_error('Schema', error) } unless schema_validation_passed
schema_validation_passed
else
# We treat all schema validation errors as warnings
schema_validator.errors.each { |error| report.add_warning('Schema', error) }
true
end
# We want validation to happen regardless of VALIDATE_SCHEMA
# CI variable.
#
# Previously it controlled BOTH validation and enforcement of
# schema validation result.
#
# After 15.0 we will enforce schema validation by default
# See: https://gitlab.com/groups/gitlab-org/-/epics/6968
schema_validation_passed = schema_validator.valid?
if validate
schema_validator.errors.each { |error| report.add_error('Schema', error) } unless schema_validation_passed
schema_validation_passed
else
return true if !validate || schema_validator.valid?
schema_validator.errors.each { |error| report.add_error('Schema', error) }
# We treat all schema validation errors as warnings
schema_validator.errors.each { |error| report.add_warning('Schema', error) }
false
true
end
end
......
......@@ -38,172 +38,102 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Common do
allow(validator_class).to receive(:new).and_call_original
end
context 'when show_report_validation_warnings is enabled' do
before do
stub_feature_flags(show_report_validation_warnings: true)
end
context 'when the validate flag is set to `false`' do
let(:validate) { false }
let(:valid?) { false }
let(:errors) { ['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(errors)
end
allow(parser).to receive_messages(create_scanner: true, create_scan: true)
end
it 'instantiates the validator with correct params' do
parse_report
expect(validator_class).to have_received(:new).with(report.type, {}, report.version)
end
context 'when the report data is not valid according to the schema' do
it 'adds warnings to the report' do
expect { parse_report }.to change { report.warnings }.from([]).to([{ message: 'foo', type: 'Schema' }])
end
context 'when the validate flag is set to `false`' do
let(:validate) { false }
let(:valid?) { false }
let(:errors) { ['foo'] }
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
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(errors)
end
context 'when the report data is valid according to the schema' do
let(:valid?) { true }
let(:errors) { [] }
it 'does not add warnings to the report' do
expect { parse_report }.not_to change { report.errors }
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 validate flag is set to `true`' do
let(:validate) { true }
let(:valid?) { false }
let(:errors) { ['foo'] }
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(errors)
end
expect(validator_class).to have_received(:new).with(report.type, {}, report.version)
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
it 'adds warnings to the report' do
expect { parse_report }.to change { report.warnings }.from([]).to([{ message: 'foo', type: 'Schema' }])
end
it 'instantiates the validator with correct params' do
it 'keeps the execution flow as normal' do
parse_report
expect(validator_class).to have_received(:new).with(report.type, {}, report.version)
expect(parser).to have_received(:create_scanner)
expect(parser).to have_received(:create_scan)
end
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
context 'when the report data is valid according to the schema' do
let(:valid?) { true }
let(:errors) { [] }
expect(parser).not_to have_received(:create_scanner)
expect(parser).not_to have_received(:create_scan)
end
it 'does not add warnings to the report' do
expect { parse_report }.not_to change { report.errors }
end
context 'when the report data is valid according to the schema' do
let(:valid?) { true }
let(:errors) { [] }
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
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
expect(parser).to have_received(:create_scanner)
expect(parser).to have_received(:create_scan)
end
end
end
context 'when show_report_validation_warnings is disabled' do
before do
stub_feature_flags(show_report_validation_warnings: false)
end
context 'when the validate flag is set as `false`' do
let(:validate) { false }
context 'when the validate flag is set to `true`' do
let(:validate) { true }
let(:valid?) { false }
let(:errors) { ['foo'] }
it 'does not run the validation logic' do
parse_report
expect(validator_class).not_to have_received(:new)
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(errors)
end
allow(parser).to receive_messages(create_scanner: true, create_scan: true)
end
context 'when the validate flag is set as `true`' do
let(:validate) { true }
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, {}, report.version)
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
it 'adds errors to the report' do
expect { parse_report }.to change { report.errors }.from([]).to([{ message: 'foo', type: 'Schema' }])
end
it 'instantiates the validator with correct params' do
it 'does not try to create report entities' do
parse_report
expect(validator_class).to have_received(:new).with(report.type, {}, report.version)
expect(parser).not_to have_received(:create_scanner)
expect(parser).not_to have_received(:create_scan)
end
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
context 'when the report data is valid according to the schema' do
let(:valid?) { true }
let(:errors) { [] }
expect(parser).not_to have_received(:create_scanner)
expect(parser).not_to have_received(:create_scan)
end
it 'does not add errors to the report' do
expect { parse_report }.not_to change { report.errors }.from([])
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
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
expect(parser).to have_received(:create_scanner)
expect(parser).to have_received(:create_scan)
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