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
end
MR 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
end
1.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
end
1.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_license
Add
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_policies
MR 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
end
2 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_license
3.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
end
We 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)