diff --git a/app/validators/json_schemas/scan_result_policy_violation_data.json b/app/validators/json_schemas/scan_result_policy_violation_data.json
new file mode 100644
index 0000000000000000000000000000000000000000..555f9bebcae81cff793a6f16ba16f075db9daf03
--- /dev/null
+++ b/app/validators/json_schemas/scan_result_policy_violation_data.json
@@ -0,0 +1,22 @@
+{
+ "$schema": "http://json-schema.org/draft-07/schema#",
+ "description": "Scan result policy violation data",
+ "type": "object",
+ "properties": {
+ "context": {
+ "type": "object"
+ },
+ "violations": {
+ "type": "object"
+ },
+ "errors": {
+ "type": "array",
+ "items": {
+ "type": "object",
+ "required": [
+ "error"
+ ]
+ }
+ }
+ }
+}
diff --git a/db/migrate/20240212155716_add_violation_data_to_scan_result_policy_violations.rb b/db/migrate/20240212155716_add_violation_data_to_scan_result_policy_violations.rb
new file mode 100644
index 0000000000000000000000000000000000000000..fa334c206c98b5a13d07291ad9bfefc20c2c1c31
--- /dev/null
+++ b/db/migrate/20240212155716_add_violation_data_to_scan_result_policy_violations.rb
@@ -0,0 +1,10 @@
+# frozen_string_literal: true
+
+class AddViolationDataToScanResultPolicyViolations < Gitlab::Database::Migration[2.2]
+ enable_lock_retries!
+ milestone '16.10'
+
+ def change
+ add_column :scan_result_policy_violations, :violation_data, :jsonb, null: true
+ end
+end
diff --git a/db/schema_migrations/20240212155716 b/db/schema_migrations/20240212155716
new file mode 100644
index 0000000000000000000000000000000000000000..826b83b8f6fdd95efbebe17b953c7a4b6f1c9edd
--- /dev/null
+++ b/db/schema_migrations/20240212155716
@@ -0,0 +1 @@
+5f9073fc752c552134286eca244bae41e71a92f62b20328a7601f8ea8ef887dd
\ No newline at end of file
diff --git a/db/structure.sql b/db/structure.sql
index 10ad3f81c3e4522fc59326e99be2a971fb8d6a4e..fd9027155d6a8ae5c3c1479e20cec644579f294a 100644
--- a/db/structure.sql
+++ b/db/structure.sql
@@ -15262,7 +15262,8 @@ CREATE TABLE scan_result_policy_violations (
merge_request_id bigint NOT NULL,
project_id bigint NOT NULL,
created_at timestamp with time zone NOT NULL,
- updated_at timestamp with time zone NOT NULL
+ updated_at timestamp with time zone NOT NULL,
+ violation_data jsonb
);
CREATE SEQUENCE scan_result_policy_violations_id_seq
diff --git a/ee/app/models/security/scan_result_policy_violation.rb b/ee/app/models/security/scan_result_policy_violation.rb
index 2ae82c74c74e64472bd4ed6d7c54b10a93db0c6c..dd1c64316aafd1615c28d518121aeddc696b3c50 100644
--- a/ee/app/models/security/scan_result_policy_violation.rb
+++ b/ee/app/models/security/scan_result_policy_violation.rb
@@ -13,6 +13,7 @@ class ScanResultPolicyViolation < ApplicationRecord
belongs_to :merge_request, inverse_of: :scan_result_policy_violations
validates :scan_result_policy_id, uniqueness: { scope: %i[merge_request_id] }
+ validates :violation_data, json_schema: { filename: 'scan_result_policy_violation_data' }, allow_blank: true
scope :including_scan_result_policy_reads, -> { includes(:scan_result_policy_read) }
diff --git a/ee/app/services/security/security_orchestration_policies/update_violations_service.rb b/ee/app/services/security/security_orchestration_policies/update_violations_service.rb
index 9034e86bc75973c9723c5910ceb600c3dc9bbb92..4fa9383cf7908ba52f41e72b9981af95268eafc0 100644
--- a/ee/app/services/security/security_orchestration_policies/update_violations_service.rb
+++ b/ee/app/services/security/security_orchestration_policies/update_violations_service.rb
@@ -3,14 +3,13 @@
module Security
module SecurityOrchestrationPolicies
class UpdateViolationsService
- attr_reader :merge_request,
- :violated_policy_ids,
- :unviolated_policy_ids
+ attr_reader :merge_request, :violated_policy_ids, :unviolated_policy_ids, :violation_data
def initialize(merge_request)
@merge_request = merge_request
@violated_policy_ids = Set.new
@unviolated_policy_ids = Set.new
+ @violation_data = {}
end
def add(violated_ids, unviolated_ids)
@@ -18,6 +17,10 @@ def add(violated_ids, unviolated_ids)
unviolated_policy_ids.merge(unviolated_ids)
end
+ def set_violation_data(policy_id, data)
+ @violation_data[policy_id] = data
+ end
+
def execute
Security::ScanResultPolicyViolation.transaction do
delete_violations if unviolated_policy_ids.any?
@@ -37,10 +40,12 @@ def delete_violations
def create_violations
attrs = violated_policy_ids.map do |id|
- { scan_result_policy_id: id, merge_request_id: merge_request.id, project_id: merge_request.project_id }
+ { scan_result_policy_id: id, merge_request_id: merge_request.id, project_id: merge_request.project_id,
+ violation_data: violation_data[id] }
end
+ return unless attrs.any?
- Security::ScanResultPolicyViolation.insert_all(attrs) if attrs.any?
+ Security::ScanResultPolicyViolation.upsert_all(attrs, unique_by: %w[scan_result_policy_id merge_request_id])
end
end
end
diff --git a/ee/spec/models/security/scan_result_policy_violation_spec.rb b/ee/spec/models/security/scan_result_policy_violation_spec.rb
index 83dee3d1a51a612e05b09dd3e20967f4ab3fe3e2..7a876d7179dcf8d749755c0283706ca89bbc65d3 100644
--- a/ee/spec/models/security/scan_result_policy_violation_spec.rb
+++ b/ee/spec/models/security/scan_result_policy_violation_spec.rb
@@ -15,6 +15,26 @@
subject { violation }
it { is_expected.to(validate_uniqueness_of(:scan_result_policy_id).scoped_to(%i[merge_request_id])) }
+
+ describe 'violation_data' do
+ it { is_expected.not_to allow_value('string').for(:violation_data) }
+ it { is_expected.to allow_value({}).for(:violation_data) }
+
+ it do
+ is_expected.to allow_value(
+ { violations: { uuids: { newly_detected: ['123'], previously_existing: ['456'] }, licenses: ['MIT'] },
+ context: { pipeline_ids: [123], target_pipeline_ids: [456] } }
+ ).for(:violation_data)
+ end
+
+ it do
+ is_expected.to allow_value(
+ { errors: [{ error: 'SCAN_REMOVED', missing_scans: ['sast'] }] }
+ ).for(:violation_data)
+ end
+
+ it { is_expected.not_to allow_value({ errors: [{}] }).for(:violation_data) }
+ end
end
describe '.for_approval_rules' do
diff --git a/ee/spec/services/security/security_orchestration_policies/update_violations_service_spec.rb b/ee/spec/services/security/security_orchestration_policies/update_violations_service_spec.rb
index 730964ad53cbf83c950fff1218abf5c00db2e010..6605b4f8df01b213cdf27a2f2efe9affdbe4939e 100644
--- a/ee/spec/services/security/security_orchestration_policies/update_violations_service_spec.rb
+++ b/ee/spec/services/security/security_orchestration_policies/update_violations_service_spec.rb
@@ -11,8 +11,9 @@
let_it_be(:policy_a) { create(:scan_result_policy_read, project: project) }
let_it_be(:policy_b) { create(:scan_result_policy_read, project: project) }
+ let(:violated_policies) { violations.map(&:scan_result_policy_read) }
- subject(:violated_policies) { merge_request.scan_result_policy_violations.map(&:scan_result_policy_read) }
+ subject(:violations) { merge_request.scan_result_policy_violations }
describe 'attributes' do
subject(:attrs) { project.scan_result_policy_violations.last.attributes }
@@ -31,17 +32,28 @@
end
context 'without pre-existing violations' do
- it 'creates violations' do
+ before do
service.add([policy_b.id], [])
+ end
+
+ it 'creates violations' do
service.execute
expect(violated_policies).to contain_exactly(policy_b)
end
+
+ it 'can persist violation data' do
+ service.set_violation_data(policy_b.id, { violations: { uuid: { newly_detected: [123] } } })
+ service.execute
+
+ expect(violations.last.violation_data).to eq({ "violations" => { "uuid" => { "newly_detected" => [123] } } })
+ end
end
context 'with pre-existing violations' do
before do
service.add([policy_a.id], [])
+ service.set_violation_data(policy_a.id, { violations: { uuid: { newly_detected: [123] } } })
service.execute
end
@@ -52,11 +64,21 @@
expect(violated_policies).to contain_exactly(policy_b)
end
+ it 'updates existing violation data' do
+ service.add([policy_a.id], [])
+ service.set_violation_data(policy_a.id, { errors: [{ error: 'SCAN_REMOVED', missing_scans: ['sast'] }] })
+
+ expect { service.execute }
+ .to change { violations.last.violation_data }.to(
+ { "errors" => [{ "error" => "SCAN_REMOVED", "missing_scans" => ["sast"] }] }
+ )
+ end
+
context 'with identical state' do
it 'does not clear violations' do
service.add([policy_a.id], [])
- service.execute
+ expect { service.execute }.not_to change { violations.last.violation_data }
expect(violated_policies).to contain_exactly(policy_a)
end
end
@@ -74,7 +96,7 @@
it 'removes only violations provided in unviolated ids' do
service.execute
- expect(merge_request.scan_result_policy_violations).to contain_exactly(unrelated_violation)
+ expect(violations).to contain_exactly(unrelated_violation)
end
end
@@ -82,7 +104,7 @@
it 'clears all violations' do
service.execute
- expect(violated_policies).to be_empty
+ expect(violations).to be_empty
end
end
end