Update `match_on_inclusion` to `match_on_inclusion_license` in %17.0
Description
Based on discussion in &10203 (comment 1545826313)+, we plan to update the yaml definition for more clarity. This will be a breaking change and will need to be introduced in %17.0.
Implementation plan
Draft MR: !141931 (merged)
Update the match_on_inclusion
attribute in ScanResultPolicyRead
===================================================================
diff --git a/ee/app/models/security/scan_result_policy_read.rb b/ee/app/models/security/scan_result_policy_read.rb
--- a/ee/app/models/security/scan_result_policy_read.rb (revision 31e66c8e63cdc50ad0f8ad264e9abac8522c3d4e)
+++ b/ee/app/models/security/scan_result_policy_read.rb (date 1705432027272)
@@ -6,6 +6,8 @@
self.table_name = 'scan_result_policies'
+ alias_attribute :match_on_inclusion_license, :match_on_inclusion
+
enum age_operator: { greater_than: 0, less_than: 1 }
enum age_interval: { day: 0, week: 1, month: 2, year: 3 }
enum commits: { any: 0, unsigned: 1 }, _prefix: true
@@ -17,7 +19,7 @@
has_many :violations, foreign_key: 'scan_result_policy_id', class_name: 'Security::ScanResultPolicyViolation',
inverse_of: :scan_result_policy_read
- validates :match_on_inclusion, inclusion: { in: [true, false], message: 'must be a boolean value' }
+ validates :match_on_inclusion_license, inclusion: { in: [true, false], message: 'must be a boolean value' }
validates :role_approvers, inclusion: { in: Gitlab::Access.all_values }
validates :age_value, numericality: { only_integer: true, greater_than_or_equal_to: 0 }, allow_nil: true
validates :vulnerability_attributes, json_schema: { filename: 'scan_result_policy_vulnerability_attributes' },
Rename match_on_inclusion
to match_on_inclusion_license
in
- security_orchestration_policy.json JSON schema
===================================================================
diff --git a/ee/app/validators/json_schemas/security_orchestration_policy.json b/ee/app/validators/json_schemas/security_orchestration_policy.json
--- a/ee/app/validators/json_schemas/security_orchestration_policy.json (revision 31e66c8e63cdc50ad0f8ad264e9abac8522c3d4e)
+++ b/ee/app/validators/json_schemas/security_orchestration_policy.json (date 1705430241845)
@@ -634,7 +634,7 @@
],
"additionalProperties": false
},
- "match_on_inclusion": {
+ "match_on_inclusion_license": {
"type": "boolean",
"description": "Specifies whether to match licenses on inclusion or exclusion."
},
@@ -714,7 +714,7 @@
"then": {
"required": [
"type",
- "match_on_inclusion",
+ "match_on_inclusion_license",
"license_types",
"license_states"
]
- ScanResultPolicyRead spec ee/spec/models/security/scan_result_policy_read_spec.rb
===================================================================
diff --git a/ee/spec/models/security/scan_result_policy_read_spec.rb b/ee/spec/models/security/scan_result_policy_read_spec.rb
--- a/ee/spec/models/security/scan_result_policy_read_spec.rb (revision 31e66c8e63cdc50ad0f8ad264e9abac8522c3d4e)
+++ b/ee/spec/models/security/scan_result_policy_read_spec.rb (date 1705434159598)
@@ -12,7 +12,7 @@
subject { scan_result_policy_read }
- it { is_expected.to validate_inclusion_of(:match_on_inclusion).in_array([true, false]) }
+ it { is_expected.to validate_inclusion_of(:match_on_inclusion_license).in_array([true, false]) }
it { is_expected.not_to allow_value(nil).for(:role_approvers) }
it { is_expected.to(validate_inclusion_of(:role_approvers).in_array(Gitlab::Access.values)) }
- SyncLicenseScanningRulesService ee/app/services/security/sync_license_scanning_rules_service.rb
===================================================================
diff --git a/ee/app/services/security/sync_license_scanning_rules_service.rb b/ee/app/services/security/sync_license_scanning_rules_service.rb
--- a/ee/app/services/security/sync_license_scanning_rules_service.rb (revision 31e66c8e63cdc50ad0f8ad264e9abac8522c3d4e)
+++ b/ee/app/services/security/sync_license_scanning_rules_service.rb (date 1705432315277)
@@ -64,11 +64,11 @@
## Checks if a policy rule violates the following conditions:
## - If license_states has `newly_detected`, check for newly detected dependency
## with license type violating the policy.
- ## - If match_on_inclusion is false, any detected licenses that does not match
+ ## - If match_on_inclusion_license is false, any detected licenses that does not match
## the licenses from `license_types` should require approval
def violates_policy?(merge_request, rule)
scan_result_policy_read = rule.scan_result_policy_read
- check_denied_licenses = scan_result_policy_read.match_on_inclusion
+ check_denied_licenses = scan_result_policy_read.match_on_inclusion_license
target_branch_report = target_branch_report(merge_request)
license_ids, license_names = licenses_to_check(target_branch_report, scan_result_policy_read)
@@ -82,7 +82,7 @@
denied_licenses = license_names_from_policy
violates_license_policy = report.violates_for_licenses?(license_policies, license_ids, license_names)
else
- # when match_on_inclusion is false, only allowed licenses are mentioned in policy
+ # when match_on_inclusion_license is false, only allowed licenses are mentioned in policy
denied_licenses = (license_names_from_report - license_names_from_policy).uniq
license_names_from_ids = license_names_from_ids(license_ids, license_names)
violates_license_policy = (license_names_from_ids - license_names_from_policy).present?
- SyncLicenseScanningRulesService spec
===================================================================
diff --git a/ee/spec/services/security/sync_license_scanning_rules_service_spec.rb b/ee/spec/services/security/sync_license_scanning_rules_service_spec.rb
--- a/ee/spec/services/security/sync_license_scanning_rules_service_spec.rb (revision 31e66c8e63cdc50ad0f8ad264e9abac8522c3d4e)
+++ b/ee/spec/services/security/sync_license_scanning_rules_service_spec.rb (date 1705432584847)
@@ -62,11 +62,11 @@
context 'with license_finding security policy' do
let(:license_states) { ['newly_detected'] }
- let(:match_on_inclusion) { true }
+ let(:match_on_inclusion_license) { true }
let(:approvals_required) { 1 }
let(:scan_result_policy_read) do
- create(:scan_result_policy_read, license_states: license_states, match_on_inclusion: match_on_inclusion)
+ create(:scan_result_policy_read, license_states: license_states, match_on_inclusion_license: match_on_inclusion_license)
end
let!(:license_finding_rule) do
@@ -211,7 +211,7 @@
end
with_them do
- let(:match_on_inclusion) { policy_state == :denied }
+ let(:match_on_inclusion_license) { policy_state == :denied }
let(:target_branch_report) { create(:ci_reports_license_scanning_report) }
let(:pipeline_report) { create(:ci_reports_license_scanning_report) }
let(:license_states) { states }
- Update SoftwareLicensePolicy
===================================================================
diff --git a/ee/app/models/software_license_policy.rb b/ee/app/models/software_license_policy.rb
--- a/ee/app/models/software_license_policy.rb (revision 31e66c8e63cdc50ad0f8ad264e9abac8522c3d4e)
+++ b/ee/app/models/software_license_policy.rb (date 1705433075921)
@@ -46,7 +46,7 @@
scope :exclusion_allowed, -> do
joins(:scan_result_policy_read)
- .where(scan_result_policy_read: { match_on_inclusion: false })
+ .where(scan_result_policy_read: { match_on_inclusion_license: false })
end
scope :with_license_by_name, -> (license_name) do
- Update SoftwareLicensePolicy spec
===================================================================
diff --git a/ee/spec/models/software_license_policy_spec.rb b/ee/spec/models/software_license_policy_spec.rb
--- a/ee/spec/models/software_license_policy_spec.rb (revision 31e66c8e63cdc50ad0f8ad264e9abac8522c3d4e)
+++ b/ee/spec/models/software_license_policy_spec.rb (date 1705433181498)
@@ -55,8 +55,8 @@
describe '.exclusion_allowed' do
let_it_be(:mit) { create(:software_license, :mit) }
- let_it_be(:scan_result_policy_read_with_inclusion) { create(:scan_result_policy_read, match_on_inclusion: true) }
- let_it_be(:scan_result_policy_read_without_inclusion) { create(:scan_result_policy_read, match_on_inclusion: false) }
+ let_it_be(:scan_result_policy_read_with_inclusion) { create(:scan_result_policy_read, match_on_inclusion_license: true) }
+ let_it_be(:scan_result_policy_read_without_inclusion) { create(:scan_result_policy_read, match_on_inclusion_license: false) }
let!(:mit_policy) { create(:software_license_policy, software_license: mit) }
let!(:mit_policy_with_inclusion) { create(:software_license_policy, software_license: mit, scan_result_policy_read: scan_result_policy_read_with_inclusion) }
let!(:mit_policy_without_inclusion) { create(:software_license_policy, software_license: mit, scan_result_policy_read: scan_result_policy_read_without_inclusion) }
- Update Security::SecurityOrchestrationPolicies::ProcessScanResultPolicyService
===================================================================
diff --git a/ee/app/services/security/security_orchestration_policies/process_scan_result_policy_service.rb b/ee/app/services/security/security_orchestration_policies/process_scan_result_policy_service.rb
--- a/ee/app/services/security/security_orchestration_policies/process_scan_result_policy_service.rb (revision 31e66c8e63cdc50ad0f8ad264e9abac8522c3d4e)
+++ b/ee/app/services/security/security_orchestration_policies/process_scan_result_policy_service.rb (date 1705432794053)
@@ -81,7 +81,7 @@
rule[:license_types].map do |license_type|
{
name: license_type,
- approval_status: rule[:match_on_inclusion] ? 'denied' : 'allowed',
+ approval_status: rule[:match_on_inclusion_license] ? 'denied' : 'allowed',
scan_result_policy_read: scan_result_policy_read
}
end
@@ -92,7 +92,7 @@
orchestration_policy_idx: policy_index,
rule_idx: rule_index,
license_states: rule[:license_states],
- match_on_inclusion: rule[:match_on_inclusion] || false,
+ match_on_inclusion_license: rule[:match_on_inclusion_license] || false,
role_approvers: role_access_levels(action_info&.dig(:role_approvers)),
vulnerability_attributes: rule[:vulnerability_attributes],
project_id: project.id,
- Security::SecurityOrchestrationPolicies::ProcessScanResultPolicyService spec
===================================================================
diff --git a/ee/spec/services/security/security_orchestration_policies/process_scan_result_policy_service_spec.rb b/ee/spec/services/security/security_orchestration_policies/process_scan_result_policy_service_spec.rb
--- a/ee/spec/services/security/security_orchestration_policies/process_scan_result_policy_service_spec.rb (revision 31e66c8e63cdc50ad0f8ad264e9abac8522c3d4e)
+++ b/ee/spec/services/security/security_orchestration_policies/process_scan_result_policy_service_spec.rb (date 1705432946029)
@@ -548,7 +548,7 @@
scan_result_policy_read = project.approval_rules.first.scan_result_policy_read
expect(scan_result_policy_read).to eq(Security::ScanResultPolicyRead.first)
- expect(scan_result_policy_read.match_on_inclusion).to be_truthy
+ expect(scan_result_policy_read.match_on_inclusion_license).to be_truthy
expect(scan_result_policy_read.license_states).to match_array(%w[newly_detected detected])
expect(scan_result_policy_read.rule_idx).to be(0)
end
- ee/spec/factories/security/policies.rb
===================================================================
diff --git a/ee/spec/factories/security/policies.rb b/ee/spec/factories/security/policies.rb
--- a/ee/spec/factories/security/policies.rb (revision 31e66c8e63cdc50ad0f8ad264e9abac8522c3d4e)
+++ b/ee/spec/factories/security/policies.rb (date 1705433408858)
@@ -105,7 +105,7 @@
{
type: 'license_finding',
branches: branches,
- match_on_inclusion: true,
+ match_on_inclusion_license: true,
license_types: %w[BSD MIT],
license_states: %w[newly_detected detected]
}
- ee/spec/factories/security/scan_result_policy_reads.rb
===================================================================
diff --git a/ee/app/models/security/scan_result_policy_read.rb b/ee/app/models/security/scan_result_policy_read.rb
--- a/ee/app/models/security/scan_result_policy_read.rb (revision 31e66c8e63cdc50ad0f8ad264e9abac8522c3d4e)
+++ b/ee/app/models/security/scan_result_policy_read.rb (date 1705432027272)
@@ -6,6 +6,8 @@
self.table_name = 'scan_result_policies'
+ alias_attribute :match_on_inclusion_license, :match_on_inclusion
+
enum age_operator: { greater_than: 0, less_than: 1 }
enum age_interval: { day: 0, week: 1, month: 2, year: 3 }
enum commits: { any: 0, unsigned: 1 }, _prefix: true
@@ -17,7 +19,7 @@
has_many :violations, foreign_key: 'scan_result_policy_id', class_name: 'Security::ScanResultPolicyViolation',
inverse_of: :scan_result_policy_read
- validates :match_on_inclusion, inclusion: { in: [true, false], message: 'must be a boolean value' }
+ validates :match_on_inclusion_license, inclusion: { in: [true, false], message: 'must be a boolean value' }
validates :role_approvers, inclusion: { in: Gitlab::Access.all_values }
validates :age_value, numericality: { only_integer: true, greater_than_or_equal_to: 0 }, allow_nil: true
validates :vulnerability_attributes, json_schema: { filename: 'scan_result_policy_vulnerability_attributes' },
- LicenseCompliance
===================================================================
diff --git a/ee/app/models/sca/license_compliance.rb b/ee/app/models/sca/license_compliance.rb
--- a/ee/app/models/sca/license_compliance.rb (revision 31e66c8e63cdc50ad0f8ad264e9abac8522c3d4e)
+++ b/ee/app/models/sca/license_compliance.rb (date 1705430241844)
@@ -92,7 +92,7 @@
end
end
- # Constructs license to policy map for policy with `match_on_inclusion` as false
+ # Constructs license to policy map for policy with `match_on_inclusion_license` as false
# by setting the `approval_status` as denied for all licenses from report except
# for the one mentioned in the policy.
def denied_license_policies
- ee/spec/models/sca/license_compliance_spec.rb
===================================================================
diff --git a/ee/spec/models/sca/license_compliance_spec.rb b/ee/spec/models/sca/license_compliance_spec.rb
--- a/ee/spec/models/sca/license_compliance_spec.rb (revision 31e66c8e63cdc50ad0f8ad264e9abac8522c3d4e)
+++ b/ee/spec/models/sca/license_compliance_spec.rb (date 1705433523200)
@@ -96,7 +96,7 @@
allow(license_compliance).to receive(:license_scanning_report).and_return(report)
input.each do |policy|
- scan_result_policy_read = policy[:scan_result_policy] ? create(:scan_result_policy_read, match_on_inclusion: policy[:classification] == 'denied') : nil
+ scan_result_policy_read = policy[:scan_result_policy] ? create(:scan_result_policy_read, match_on_inclusion_license: policy[:classification] == 'denied') : nil
create(:software_license_policy, policy[:classification],
project: project,
software_license: license_map[policy[:id]],
@@ -447,8 +447,8 @@
let(:base_report) { create(:ci_reports_license_scanning_report) }
let(:report) { create(:ci_reports_license_scanning_report) }
- let(:scan_result_policy_read_with_inclusion) { create(:scan_result_policy_read, match_on_inclusion: true) }
- let(:scan_result_policy_read_without_inclusion) { create(:scan_result_policy_read, match_on_inclusion: false) }
+ let(:scan_result_policy_read_with_inclusion) { create(:scan_result_policy_read, match_on_inclusion_license: true) }
+ let(:scan_result_policy_read_without_inclusion) { create(:scan_result_policy_read, match_on_inclusion_license: false) }
context 'when base_report has new denied licenses' do
before do
@@ -563,7 +563,7 @@
end
let(:scan_result_policy_read) do
- create(:scan_result_policy_read, license_states: ['newly_detected'], match_on_inclusion: true,
+ create(:scan_result_policy_read, license_states: ['newly_detected'], match_on_inclusion_license: true,
role_approvers: [Gitlab::Access::MAINTAINER], project: project)
end
- ee/spec/models/security/orchestration_policy_configuration_spec.rb
===================================================================
diff --git a/ee/spec/models/security/orchestration_policy_configuration_spec.rb b/ee/spec/models/security/orchestration_policy_configuration_spec.rb
--- a/ee/spec/models/security/orchestration_policy_configuration_spec.rb (revision 31e66c8e63cdc50ad0f8ad264e9abac8522c3d4e)
+++ b/ee/spec/models/security/orchestration_policy_configuration_spec.rb (date 1705433977549)
@@ -1192,7 +1192,7 @@
{
type: "license_finding",
branches: %w[master],
- match_on_inclusion: true,
+ match_on_inclusion_license: true,
license_types: %w[BSD MIT],
license_states: %w[newly_detected detected]
}
@@ -1200,7 +1200,7 @@
specify { expect(errors).to be_empty }
- it_behaves_like "scan result policy", %i[match_on_inclusion license_types license_states]
+ it_behaves_like "scan result policy", %i[match_on_inclusion_license license_types license_states]
it_behaves_like 'rule has branches or branch_type'
describe "license_types" do
- ee/spec/support/helpers/features/security_policy_helpers.rb
===================================================================
diff --git a/ee/spec/support/helpers/features/security_policy_helpers.rb b/ee/spec/support/helpers/features/security_policy_helpers.rb
--- a/ee/spec/support/helpers/features/security_policy_helpers.rb (revision 31e66c8e63cdc50ad0f8ad264e9abac8522c3d4e)
+++ b/ee/spec/support/helpers/features/security_policy_helpers.rb (date 1705434253874)
@@ -38,7 +38,7 @@
{
type: 'license_finding',
branches: %w[master],
- match_on_inclusion: true,
+ match_on_inclusion_license: true,
license_types: [license_type],
license_states: %w[newly_detected]
}
Edited by Marcos Rocha