Commit e9cee7d3 authored by Matthias Käppler's avatar Matthias Käppler

Merge branch '355943-add-logging-for-deprecated-and-unsupported-reports' into 'master'

Log deprecated/unsupported report schema versions

See merge request gitlab-org/gitlab!84117
parents 0d9adc4a 5caa90d0
......@@ -48,7 +48,7 @@ module Security
def security_scan
@security_scan ||= Security::Scan.safe_find_or_create_by!(build: job, scan_type: artifact.file_type) do |scan|
scan.processing_errors = security_report.errors.map(&:stringify_keys) if security_report.errored?
scan.processing_warnings = security_report.warnings.map(&:stringify_keys)
scan.processing_warnings = security_report.warnings.map(&:stringify_keys) if security_report.warnings?
scan.status = initial_scan_status
end
end
......
......@@ -3,6 +3,8 @@
require 'spec_helper'
RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator do
let_it_be(:project) { create(:project) }
using RSpec::Parameterized::TableSyntax
where(:report_type, :expected_errors, :expected_warnings, :valid_data) do
......@@ -15,7 +17,7 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator do
end
with_them do
let(:validator) { described_class.new(report_type, report_data, valid_data['version']) }
let(:validator) { described_class.new(report_type, report_data, valid_data['version'], project: project) }
describe '#valid?' do
subject { validator.valid? }
......@@ -33,20 +35,64 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator do
end
end
describe '#deprecation_warnings' do
subject { validator.deprecation_warnings }
let(:supported_versions) { described_class::SUPPORTED_VERSIONS[report_type].join(", ") }
context 'when report uses a deprecated version' do
let(:report_data) { valid_data }
let(:expected_deprecation_warnings_array) do
[
"Version 10.0.0 for report type #{report_type} has been deprecated, supported versions for this report type are: #{supported_versions}"
]
end
it { is_expected.to eq(expected_deprecation_warnings_array) }
end
context 'when report uses a supported version' do
let(:supported_version) { described_class::SUPPORTED_VERSIONS[report_type].first }
let(:report_data) do
valid_data['version'] = supported_version
valid_data
end
it { is_expected.to eq([]) }
end
end
describe '#warnings' do
subject { validator.warnings }
context 'when given data is valid according to the schema' do
let(:report_data) { valid_data }
let(:supported_version) { described_class::SUPPORTED_VERSIONS[report_type].join(", ") }
let(:expected_warnings_array) do
[
"Version 10.0.0 for report type #{report_type} has been deprecated, supported versions for this report type are: #{supported_version}"
]
end
let(:expected_warnings_array) { [] }
it { is_expected.to eq(expected_warnings) }
end
context 'when given data is invalid according to the schema' do
let(:report_data) { { 'version' => '10.0.0' } }
context 'and enforce_security_report_validation is enabled' do
before do
stub_feature_flags(enforce_security_report_validation: project)
end
it { is_expected.to be_empty }
end
context 'and enforce_security_report_validation is disabled' do
before do
stub_feature_flags(enforce_security_report_validation: false)
end
it { is_expected.to eq(expected_errors) }
end
end
end
describe '#errors' do
......@@ -54,7 +100,21 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator do
subject { validator.errors }
context 'when enforce_security_report_validation is enabled' do
before do
stub_feature_flags(enforce_security_report_validation: project)
end
it { is_expected.to eq(expected_errors) }
end
context 'when enforce_security_report_validation is disabled' do
before do
stub_feature_flags(enforce_security_report_validation: false)
end
it { is_expected.to be_empty }
end
end
end
end
......@@ -158,6 +158,20 @@ RSpec.describe Security::StoreScanService do
expect(Security::StoreFindingsMetadataService).to have_received(:execute)
end
context 'when the report has no warnings' do
before do
artifact.security_report.warnings = []
end
let(:security_scan) { Security::Scan.last }
it 'does not store an empty array' do
store_scan
expect(security_scan.info).to eq({})
end
end
context 'when the report has some warnings' do
before do
artifact.security_report.warnings << { 'type' => 'foo', 'message' => 'bar' }
......
......@@ -14,6 +14,7 @@ module Gitlab
def initialize(json_data, report, vulnerability_finding_signatures_enabled = false, validate: false)
@json_data = json_data
@report = report
@project = report.project
@validate = validate
@vulnerability_finding_signatures_enabled = vulnerability_finding_signatures_enabled
end
......@@ -51,22 +52,27 @@ module Gitlab
#
# 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?
schema_validator.deprecation_warnings.each { |deprecation_warning| report.add_warning('Schema', deprecation_warning) }
if validate
schema_validator.errors.each { |error| report.add_error('Schema', error) } unless schema_validation_passed
schema_validation_passed = schema_validator.valid?
# Validation warnings are errors
schema_validator.errors.each { |error| report.add_error('Schema', error) }
schema_validator.warnings.each { |warning| report.add_error('Schema', warning) }
schema_validation_passed
else
# We treat all schema validation errors as warnings
# Validation warnings are warnings
schema_validator.errors.each { |error| report.add_warning('Schema', error) }
schema_validator.warnings.each { |warning| report.add_warning('Schema', warning) }
true
end
end
def schema_validator
@schema_validator ||= ::Gitlab::Ci::Parsers::Security::Validators::SchemaValidator.new(report.type, report_data, report.version)
@schema_validator ||= ::Gitlab::Ci::Parsers::Security::Validators::SchemaValidator.new(report.type, report_data, report.version, project: @project)
end
def report_data
......@@ -136,7 +142,7 @@ module Gitlab
metadata_version: report_version,
details: data['details'] || {},
signatures: signatures,
project_id: report.project_id,
project_id: @project.id,
vulnerability_finding_signatures_enabled: @vulnerability_finding_signatures_enabled))
end
......@@ -279,7 +285,7 @@ module Gitlab
report_type: report.type,
primary_identifier_fingerprint: primary_identifier&.fingerprint,
location_fingerprint: location_fingerprint,
project_id: report.project_id
project_id: @project.id
}
if uuid_v5_name_components.values.any?(&:nil?)
......
......@@ -26,19 +26,19 @@ module Gitlab
8.0.0-rc1 8.0.1-rc1 8.1.0-rc1 9.0.0-rc1].freeze
# These come from https://app.periscopedata.com/app/gitlab/895813/Secure-Scan-metrics?widget=12248944&udv=1385516
KNOWN_VERSIONS_TO_DEPRECATE = %w[0.1 1.0 1.0.0 1.2 1.3 10.0.0 12.1.0 13.1.0 2.0 2.1 2.1.0 2.3 2.3.0 2.4 3.0 3.0.0 3.0.6 3.13.2 V2.7.0].freeze
KNOWN_VERSIONS_TO_REMOVE = %w[0.1 1.0 1.0.0 1.2 1.3 10.0.0 12.1.0 13.1.0 2.0 2.1 2.1.0 2.3 2.3.0 2.4 3.0 3.0.0 3.0.6 3.13.2 V2.7.0].freeze
VERSIONS_TO_DEPRECATE_IN_15_0 = (PREVIOUS_RELEASES + KNOWN_VERSIONS_TO_DEPRECATE).freeze
VERSIONS_TO_REMOVE_IN_15_0 = (PREVIOUS_RELEASES + KNOWN_VERSIONS_TO_REMOVE).freeze
DEPRECATED_VERSIONS = {
cluster_image_scanning: VERSIONS_TO_DEPRECATE_IN_15_0,
container_scanning: VERSIONS_TO_DEPRECATE_IN_15_0,
coverage_fuzzing: VERSIONS_TO_DEPRECATE_IN_15_0,
dast: VERSIONS_TO_DEPRECATE_IN_15_0,
api_fuzzing: VERSIONS_TO_DEPRECATE_IN_15_0,
dependency_scanning: VERSIONS_TO_DEPRECATE_IN_15_0,
sast: VERSIONS_TO_DEPRECATE_IN_15_0,
secret_detection: VERSIONS_TO_DEPRECATE_IN_15_0
cluster_image_scanning: VERSIONS_TO_REMOVE_IN_15_0,
container_scanning: VERSIONS_TO_REMOVE_IN_15_0,
coverage_fuzzing: VERSIONS_TO_REMOVE_IN_15_0,
dast: VERSIONS_TO_REMOVE_IN_15_0,
api_fuzzing: VERSIONS_TO_REMOVE_IN_15_0,
dependency_scanning: VERSIONS_TO_REMOVE_IN_15_0,
sast: VERSIONS_TO_REMOVE_IN_15_0,
secret_detection: VERSIONS_TO_REMOVE_IN_15_0
}.freeze
class Schema
......@@ -86,15 +86,18 @@ module Gitlab
end
end
def initialize(report_type, report_data, report_version = nil)
def initialize(report_type, report_data, report_version = nil, project: nil)
@report_type = report_type&.to_sym
@report_data = report_data
@report_version = report_version
@project = project
@errors = []
@warnings = []
@deprecation_warnings = []
populate_errors
populate_warnings
populate_deprecation_warnings
end
def valid?
......@@ -102,25 +105,46 @@ module Gitlab
end
def populate_errors
if Feature.enabled?(:enforce_security_report_validation)
@errors += schema.validate(report_data).map { |error| JSONSchemer::Errors.pretty(error) }
schema_validation_errors = schema.validate(report_data).map { |error| JSONSchemer::Errors.pretty(error) }
log_warnings(problem_type: 'schema_validation_fails') unless schema_validation_errors.empty?
if Feature.enabled?(:enforce_security_report_validation, @project)
@errors += schema_validation_errors
else
@warnings += schema.validate(report_data).map { |error| JSONSchemer::Errors.pretty(error) }
@warnings += schema_validation_errors
end
end
def populate_warnings
add_deprecated_report_version_message if report_uses_deprecated_schema_version?
add_unsupported_report_version_message if !report_uses_supported_schema_version? && !report_uses_deprecated_schema_version?
end
def populate_deprecation_warnings
add_deprecated_report_version_message if report_uses_deprecated_schema_version?
end
def add_deprecated_report_version_message
log_warnings(problem_type: 'using_deprecated_schema_version')
message = "Version #{report_version} for report type #{report_type} has been deprecated, supported versions for this report type are: #{supported_schema_versions}"
add_message_as(level: :warning, message: message)
add_message_as(level: :deprecation_warning, message: message)
end
def log_warnings(problem_type:)
Gitlab::AppLogger.info(
message: 'security report schema validation problem',
security_report_type: report_type,
security_report_version: report_version,
project_id: @project.id,
security_report_failure: problem_type
)
end
def add_unsupported_report_version_message
if Feature.enabled?(:enforce_security_report_validation)
log_warnings(problem_type: 'using_unsupported_schema_version')
if Feature.enabled?(:enforce_security_report_validation, @project)
handle_unsupported_report_version(treat_as: :error)
else
handle_unsupported_report_version(treat_as: :warning)
......@@ -152,6 +176,8 @@ module Gitlab
def add_message_as(level:, message:)
case level
when :deprecation_warning
@deprecation_warnings << message
when :error
@errors << message
when :warning
......@@ -159,7 +185,7 @@ module Gitlab
end
end
attr_reader :errors, :warnings
attr_reader :errors, :warnings, :deprecation_warnings
private
......
......@@ -9,6 +9,7 @@ module Gitlab
attr_accessor :scan, :scanned_resources, :errors, :analyzer, :version, :schema_validation_status, :warnings
delegate :project_id, to: :pipeline
delegate :project, to: :pipeline
def initialize(type, pipeline, created_at)
@type = type
......@@ -38,6 +39,10 @@ module Gitlab
errors.present?
end
def warnings?
warnings.present?
end
def add_scanner(scanner)
scanners[scanner.key] ||= scanner
end
......
......@@ -42,11 +42,13 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Common do
let(:validate) { false }
let(:valid?) { false }
let(:errors) { ['foo'] }
let(:warnings) { ['bar'] }
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)
allow(instance).to receive(:warnings).and_return(warnings)
end
allow(parser).to receive_messages(create_scanner: true, create_scan: true)
......@@ -55,12 +57,17 @@ 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)
expect(validator_class).to have_received(:new).with(report.type, {}, report.version, project: pipeline.project)
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' }])
expect { parse_report }.to change { report.warnings }.from([]).to(
[
{ message: 'foo', type: 'Schema' },
{ message: 'bar', type: 'Schema' }
]
)
end
it 'keeps the execution flow as normal' do
......@@ -74,11 +81,16 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Common do
context 'when the report data is valid according to the schema' do
let(:valid?) { true }
let(:errors) { [] }
let(:warnings) { [] }
it 'does not add warnings to the report' do
it 'does not add errors to the report' do
expect { parse_report }.not_to change { report.errors }
end
it 'does not add warnings to the report' do
expect { parse_report }.not_to change { report.warnings }
end
it 'keeps the execution flow as normal' do
parse_report
......@@ -92,11 +104,13 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Common do
let(:validate) { true }
let(:valid?) { false }
let(:errors) { ['foo'] }
let(:warnings) { ['bar'] }
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)
allow(instance).to receive(:warnings).and_return(warnings)
end
allow(parser).to receive_messages(create_scanner: true, create_scan: true)
......@@ -105,12 +119,17 @@ 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)
expect(validator_class).to have_received(:new).with(report.type, {}, report.version, project: pipeline.project)
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' }])
expect { parse_report }.to change { report.errors }.from([]).to(
[
{ message: 'foo', type: 'Schema' },
{ message: 'bar', type: 'Schema' }
]
)
end
it 'does not try to create report entities' do
......@@ -124,11 +143,16 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Common do
context 'when the report data is valid according to the schema' do
let(:valid?) { true }
let(:errors) { [] }
let(:warnings) { [] }
it 'does not add errors to the report' do
expect { parse_report }.not_to change { report.errors }.from([])
end
it 'does not add warnings to the report' do
expect { parse_report }.not_to change { report.warnings }.from([])
end
it 'keeps the execution flow as normal' do
parse_report
......
......@@ -3,7 +3,9 @@
require 'spec_helper'
RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator do
let(:validator) { described_class.new(report_type, report_data, report_version) }
let_it_be(:project) { create(:project) }
let(:validator) { described_class.new(report_type, report_data, report_version, project: project) }
describe 'SUPPORTED_VERSIONS' do
schema_path = Rails.root.join("lib", "gitlab", "ci", "parsers", "security", "validators", "schemas")
......@@ -75,6 +77,18 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator do
end
it { is_expected.to be_falsey }
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'
)
subject
end
end
end
......@@ -91,6 +105,18 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator do
end
it { is_expected.to be_truthy }
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: 'using_deprecated_schema_version'
)
subject
end
end
context 'and the report does not pass schema validation' do
......@@ -142,6 +168,18 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator do
end
it { is_expected.to be_falsey }
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: 'using_unsupported_schema_version'
)
subject
end
end
context 'and the report is invalid' do
......@@ -211,6 +249,11 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator do
}
end
context 'if enforce_security_report_validation is enabled' do
before do
stub_feature_flags(enforce_security_report_validation: project)
end
let(:expected_errors) do
[
'root is missing required keys: vulnerabilities'
......@@ -219,6 +262,17 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator do
it { is_expected.to match_array(expected_errors) }
end
context 'if enforce_security_report_validation is disabled' do
before do
stub_feature_flags(enforce_security_report_validation: false)
end
let(:expected_errors) { [] }
it { is_expected.to match_array(expected_errors) }
end
end
end
context 'when given a deprecated schema version' do
......@@ -355,6 +409,83 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator do
end
end
describe '#deprecation_warnings' do
subject { validator.deprecation_warnings }
context 'when given a supported schema version' do
let(:report_type) { :dast }
let(:report_version) { described_class::SUPPORTED_VERSIONS[report_type].last }
let(:expected_deprecation_warnings) { [] }
context 'and the report is valid' do
let(:report_data) do
{
'version' => report_version,
'vulnerabilities' => []
}
end
it { is_expected.to match_array(expected_deprecation_warnings) }
end
context 'and the report is invalid' do
let(:report_data) do
{
'version' => report_version
}
end
it { is_expected.to match_array(expected_deprecation_warnings) }
end
end
context 'when given a deprecated schema version' do
let(:report_type) { :dast }
let(:report_version) { described_class::DEPRECATED_VERSIONS[report_type].last }
let(:expected_deprecation_warnings) do
[
"Version V2.7.0 for report type dast has been deprecated, supported versions for this report type are: 14.0.0, 14.0.1, 14.0.2, 14.0.3, 14.0.4, 14.0.5, 14.0.6, 14.1.0"
]
end
context 'and the report passes schema validation' do
let(:report_data) do
{
'version' => report_version,
'vulnerabilities' => []
}
end
it { is_expected.to match_array(expected_deprecation_warnings) }
end
context 'and the report does not pass schema validation' do
let(:report_data) do
{
'version' => 'V2.7.0'
}
end
it { is_expected.to match_array(expected_deprecation_warnings) }
end
end
context 'when given an unsupported schema version' do
let(:report_type) { :dast }
let(:report_version) { "21.37.0" }
let(:expected_deprecation_warnings) { [] }
let(:report_data) do
{
'version' => report_version,
'vulnerabilities' => []
}
end
it { is_expected.to match_array(expected_deprecation_warnings) }
end
end
describe '#warnings' do
subject { validator.warnings }
......@@ -382,10 +513,30 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator do
}
end
context 'if enforce_security_report_validation is enabled' do
before do
stub_feature_flags(enforce_security_report_validation: project)
end
let(:expected_warnings) { [] }
it { is_expected.to match_array(expected_warnings) }
end
context 'if enforce_security_report_validation is disabled' do
before do
stub_feature_flags(enforce_security_report_validation: false)
end
let(:expected_warnings) do
[
'root is missing required keys: vulnerabilities'
]
end
it { is_expected.to match_array(expected_warnings) }
end
end
end
context 'when given a deprecated schema version' do
......@@ -399,33 +550,25 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator do
}
end
let(:expected_warnings) do
[
"Version V2.7.0 for report type dast has been deprecated, supported versions for this report type are: 14.0.0, 14.0.1, 14.0.2, 14.0.3, 14.0.4, 14.0.5, 14.0.6, 14.1.0"
]
end
let(:expected_warnings) { [] }
it { is_expected.to match_array(expected_warnings) }
end
context 'and the report does not pass schema validation' do
context 'and enforce_security_report_validation is enabled' do
before do
stub_feature_flags(enforce_security_report_validation: true)
end
let(:report_data) do
{
'version' => 'V2.7.0'
}
end
let(:expected_warnings) do
[
"Version V2.7.0 for report type dast has been deprecated, supported versions for this report type are: 14.0.0, 14.0.1, 14.0.2, 14.0.3, 14.0.4, 14.0.5, 14.0.6, 14.1.0"
]
context 'and enforce_security_report_validation is enabled' do
before do
stub_feature_flags(enforce_security_report_validation: true)
end
let(:expected_warnings) { [] }
it { is_expected.to match_array(expected_warnings) }
end
......@@ -434,15 +577,8 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator do
stub_feature_flags(enforce_security_report_validation: false)
end
let(:report_data) do
{
'version' => 'V2.7.0'
}
end
let(:expected_warnings) do
[
"Version V2.7.0 for report type dast has been deprecated, supported versions for this report type are: 14.0.0, 14.0.1, 14.0.2, 14.0.3, 14.0.4, 14.0.5, 14.0.6, 14.1.0",
"property '/version' does not match pattern: ^[0-9]+\\.[0-9]+\\.[0-9]+$",
"root is missing required keys: vulnerabilities"
]
......
......@@ -184,6 +184,22 @@ RSpec.describe Gitlab::Ci::Reports::Security::Report do
end
end
describe 'warnings?' do
subject { report.warnings? }
context 'when the report does not have any errors' do
it { is_expected.to be_falsey }
end
context 'when the report has warnings' do
before do
report.add_warning('foo', 'bar')
end
it { is_expected.to be_truthy }
end
end
describe '#primary_scanner_order_to' do
let(:scanner_1) { build(:ci_reports_security_scanner) }
let(:scanner_2) { build(:ci_reports_security_scanner) }
......
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