Skip to content

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

backend

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

  1. 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"
                     ]
  1. 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)) }
  1. 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?
  1. 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 }
  1. 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
  1. 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) }
  1. 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,
  1. 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
  1. 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]
           }
  1. 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' },
  1. 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
  1. 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
 
  1. 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
  1. 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