Spike: Cells - Investigate and separate imported software licenses from custom licenses
Time-box: 3 days
Why are we doing this work
In the scope of this Spike, we would like to know what we need to do to move forward with separating software_licenses table to support Cell architecture and prepare changes that will allow us to work on this incrementally.
As an expected result of this Spike, we would like to get the following:
- MRs prepared for review with changes to add new tables, migrations, etc.
- in case more work is needed to support these scenarios new implementation issues created with the Implementation Plan added to (size: M to L) Cells - Workflows: Security Poli... (&12709 - closed),
Implementation plan
MR 1: !151445 (merged)
- Add a new Cell-local table custom_software_licenses
# frozen_string_literal: true
class CreateCustomSoftwareLicensesTable < Gitlab::Database::Migration[2.2]
  enable_lock_retries!
  milestone '16.10'
  def up
    create_table :custom_software_licenses do |t|
      t.references :project, index: true, foreign_key: { on_delete: :cascade }, null: false
      t.text :name, null: false, limit: 255
      t.index [:project_id, :name], unique: true
    end
  end
  def down
    drop_table :custom_software_licenses
  end
end- Add a new model for the custom_software_licensestable
# frozen_string_literal: true
module Security
  class CustomSoftwareLicense < ApplicationRecord
    self.table_name = 'custom_software_licenses'
    belongs_to :project
    validates :name, presence: true, uniqueness: { scope: :project_id }, length: { maximum: 255 }
  end
endMR 2: !152054 (merged)
- Update the software_license_policiestable
1.1  Remove the NOT NULL constraint on the software_license_id column.
# frozen_string_literal: true
class RemoveNotNullConstraintFromSoftwareLicensePoliciesSoftwareLicenceIdColumn < Gitlab::Database::Migration[2.2]
  milestone '16.10'
  disable_ddl_transaction!
  def up
    change_column_null :software_license_policies, :software_license_id, true
  end
  def down
    change_column_null :software_license_policies, :software_license_id, false
  end
end1.2 Add the custom_software_licenses_id column
# frozen_string_literal: true
class AddCustomSoftwareLicensesIdToSoftwareLicensePolicies < Gitlab::Database::Migration[2.2]
  milestone '16.10'
  def up
    add_column :software_license_policies,
      :custom_software_license_id,
      :bigint,
      null: true,
      if_not_exists: true
  end
  def down
    remove_column :software_license_policies, :custom_software_license_id, if_exists: true
  end
end1.3 Add a constraint to check the existence of one of the custom_software_licenses_id or software_licenses_id
# frozen_string_literal: true
class AddSoftwareLicenseExistenceConstraintToSoftwareLicensePolicies < Gitlab::Database::Migration[2.2]
  milestone '16.10'
  disable_ddl_transaction!
  CHECK_LICENSE_CONSTRAINT = 'check_software_license_policies_license_or_custom_license_existence'
  def up
    add_check_constraint(:software_license_policies,
      '(((software_license_id IS NULL) <> (custom_software_license_id IS NULL)))',
      CHECK_LICENSE_CONSTRAINT)
  end
  def down
    remove_check_constraint :software_license_policies, CHECK_LICENSE_CONSTRAINT
  end
end- Update the SoftwareLicensePolicymodel
2.1 Add belongs_to to custom_software_license
belongs_to :custom_software_license, class_name: 'Security::CustomSoftwareLicense'2.2 Update the validations related to software_license
Remove
# Software license is mandatory, it contains the license informations.
  validates_associated :software_licenseAdd
validates :software_license, presence: true, unless: :custom_software_license
validates :custom_software_license, presence: true, unless: :software_license
validates :custom_software_license, uniqueness: { scope: [:project_id, :scan_result_policy_id] }2.4 Add a scope that includes custom licenses
scope :including_custom_license, -> { includes(:custom_software_license) }- Add the custom_software_licensetoProject
ee/app/models/ee/project.rb
has_many :custom_software_licenses, through: :software_license_policiesMR 3: !153461 (closed)
- Update the SoftwareLicensePolicies::CreateServiceto save the custom licenses on the new table behind a feature flag
def create_software_license_policy
      insert_software_license_policy(true)
    end
    def create_for_scan_result_policy
      insert_software_license_policy(false)
    end
    def insert_software_license_policy(subtransactions_allowed)
      software_license = SoftwareLicense.find_by_name(params[:name])
      if software_license
        create_software_license_policies_with_software_license(software_license)
      else
        create_software_license_policies_with_custom_software_license(subtransactions_allowed)
      end
    end
    def create_software_license_policies_with_software_license(software_license)
      @project.software_license_policies.create!(
        classification: params[:classification],
        software_license: software_license,
        scan_result_policy_read: params[:scan_result_policy_read]
      )
    end
    def create_software_license_policies_with_custom_software_license(subtransactions_allowed)
      @project.software_license_policies.create!(
        classification: params[:classification],
        custom_software_license: custom_software_license(subtransactions_allowed),
        scan_result_policy_read: params[:scan_result_policy_read]
      )
    end
    def custom_software_license(subtransactions_allowed)
      if subtransactions_allowed
        CustomSoftwareLicense.safe_find_or_create_by!(name: params[:name],  project: project)
      else
        CustomSoftwareLicense.find_or_create_by!(name: name, project: project)
      end
    end2 Update Security::ScanResultPolicies::LicenseViolationChecker to load all licenses if the Feature Flag is enabled
software_license_policies method
def software_license_policies(scan_result_policy_read)
        if Feature.enabled?(:custom_software_license, project)
          project
            .software_license_policies
            .including_license
            .including_custom_license
            .for_scan_result_policy_read(scan_result_policy_read.id)
        else
...- Update SoftwareLicensePolicyto return the names of custom license
3.1 Remove the delegate name to software_license
delegate :spdx_identifier, to: :software_license3.2 Add a name method to also consider the custom_software_license name
def name
    if Feature.enabled?(:custom_software_license, project)
      software_license&.name || custom_software_license&.name
    else
      software_license&.name
    end
  end- Update SoftwareLicensePolicies::BulkCreateScanResultPolicyServiceto create CustomSoftwareLicenses
Cleanup after enabling the feature flag
- Add a data migration to fill the new table with the license nameandproject_id
# frozen_string_literal: true
class CopyCustomLicenses < Gitlab::Database::Migration[2.2]
  milestone '16.10'
  def up
    sql = <<-SQL
      INSERT INTO custom_software_licenses (name, project_id)
      SELECT
          name,
          project_id
      FROM
          software_licenses
          JOIN software_license_policies ON (software_licenses.id = software_license_id)
      WHERE
          spdx_identifier IS NULL
      GROUP BY
          name,
          project_id;
    SQL
    execute(sql)
  end
  def down
    # no-op
  end
endWe are expecting duplicated licenses among projects.
| License | project | 
|---|---|
| custom | 432 | 
| custom | 165 | 
| custom | 137 | 
- Remove the method unclassified_licenses_foronSoftwareLicense
2.1 Remove the test cases related to this method from ee/spec/models/software_license_spec.rb
- Add a migration to delete the software_licenses without spdx_identifier
DELETE FROM software_licenses WHERE spdx_identifier IS NULL;- Add a migration to require the spdx_identifieronsoftware_licensesto prevent the creation ofcustom_licenseson thesoftware_licensesmain_clusterwidetable
5 Delete the methods that creates software_license_policies in software_license
Draft MR: !145341 (closed)