Commit d52ccb0e authored by Reuben Pereira's avatar Reuben Pereira Committed by Reuben Pereira

Add background migration to copy container_registry_enabled

The background migration copies projects.container_registry_enabled
values to project_features.container_registry_access_level.
parent df76f97d
...@@ -95,6 +95,9 @@ class Project < ApplicationRecord ...@@ -95,6 +95,9 @@ class Project < ApplicationRecord
before_save :ensure_runners_token before_save :ensure_runners_token
# https://api.rubyonrails.org/v6.0.3.4/classes/ActiveRecord/AttributeMethods/Dirty.html#method-i-will_save_change_to_attribute-3F
before_update :set_container_registry_access_level, if: :will_save_change_to_container_registry_enabled?
after_save :update_project_statistics, if: :saved_change_to_namespace_id? after_save :update_project_statistics, if: :saved_change_to_namespace_id?
after_save :create_import_state, if: ->(project) { project.import? && project.import_state.nil? } after_save :create_import_state, if: ->(project) { project.import? && project.import_state.nil? }
...@@ -2555,6 +2558,20 @@ class Project < ApplicationRecord ...@@ -2555,6 +2558,20 @@ class Project < ApplicationRecord
private private
def set_container_registry_access_level
# changes_to_save = { 'container_registry_enabled' => [value_before_update, value_after_update] }
value = changes_to_save['container_registry_enabled'][1]
access_level =
if value
ProjectFeature::ENABLED
else
ProjectFeature::DISABLED
end
project_feature.update!(container_registry_access_level: access_level)
end
def find_service(services, name) def find_service(services, name)
services.find { |service| service.to_param == name } services.find { |service| service.to_param == name }
end end
......
...@@ -43,6 +43,8 @@ class ProjectFeature < ApplicationRecord ...@@ -43,6 +43,8 @@ class ProjectFeature < ApplicationRecord
end end
end end
before_create :set_container_registry_access_level
# Default scopes force us to unscope here since a service may need to check # Default scopes force us to unscope here since a service may need to check
# permissions for a project in pending_delete # permissions for a project in pending_delete
# http://stackoverflow.com/questions/1540645/how-to-disable-default-scope-for-a-belongs-to # http://stackoverflow.com/questions/1540645/how-to-disable-default-scope-for-a-belongs-to
...@@ -87,6 +89,15 @@ class ProjectFeature < ApplicationRecord ...@@ -87,6 +89,15 @@ class ProjectFeature < ApplicationRecord
private private
def set_container_registry_access_level
self.container_registry_access_level =
if project&.container_registry_enabled
ENABLED
else
DISABLED
end
end
# Validates builds and merge requests access level # Validates builds and merge requests access level
# which cannot be higher than repository access level # which cannot be higher than repository access level
def repository_children_level def repository_children_level
......
---
title: Add a background migration to copy projects.container_registry_enabled to project_features.container_registry_access_level
merge_request: 55327
author:
type: added
# frozen_string_literal: true
class MoveContainerRegistryEnabledToProjectFeatures < ActiveRecord::Migration[6.0]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
BATCH_SIZE = 50_000
MIGRATION = 'MoveContainerRegistryEnabledToProjectFeature'
disable_ddl_transaction!
class Project < ActiveRecord::Base
include EachBatch
self.table_name = 'projects'
end
def up
queue_background_migration_jobs_by_range_at_intervals(Project, MIGRATION, 2.minutes, batch_size: BATCH_SIZE)
end
def down
# no-op
end
end
ced4e314f2653ff56a946d334b4cb12085825b8d21ceea42cb686bef688b714c
\ No newline at end of file
# frozen_string_literal: true
module Gitlab
module BackgroundMigration
# This migration moves projects.container_registry_enabled values to
# project_features.container_registry_access_level for the projects within
# the given range of ids.
class MoveContainerRegistryEnabledToProjectFeature
MAX_BATCH_SIZE = 1_000
module Migratable
# Migration model namespace isolated from application code.
class ProjectFeature < ActiveRecord::Base
ENABLED = 20
DISABLED = 0
end
end
def perform(from_id, to_id)
(from_id..to_id).each_slice(MAX_BATCH_SIZE) do |batch|
process_batch(batch.first, batch.last)
end
end
private
def process_batch(from_id, to_id)
ActiveRecord::Base.connection.execute(update_sql(from_id, to_id))
logger.info(message: "#{self.class}: Copied container_registry_enabled values for projects with IDs between #{from_id}..#{to_id}")
end
# For projects that have a project_feature:
# Set project_features.container_registry_access_level to ENABLED (20) or DISABLED (0)
# depending if container_registry_enabled is true or false.
def update_sql(from_id, to_id)
<<~SQL
UPDATE project_features
SET container_registry_access_level = (CASE p.container_registry_enabled
WHEN true THEN #{ProjectFeature::ENABLED}
WHEN false THEN #{ProjectFeature::DISABLED}
ELSE #{ProjectFeature::DISABLED}
END)
FROM projects p
WHERE project_id = p.id AND
project_id BETWEEN #{from_id} AND #{to_id}
SQL
end
def logger
@logger ||= Gitlab::BackgroundMigration::Logger.build
end
end
end
end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Gitlab::BackgroundMigration::MoveContainerRegistryEnabledToProjectFeature, :migration, schema: 2021_02_26_120851 do
let(:enabled) { 20 }
let(:disabled) { 0 }
let(:namespaces) { table(:namespaces) }
let(:project_features) { table(:project_features) }
let(:projects) { table(:projects) }
let(:namespace) { namespaces.create!(name: 'user', path: 'user') }
let!(:project1) { projects.create!(namespace_id: namespace.id) }
let!(:project2) { projects.create!(namespace_id: namespace.id) }
let!(:project3) { projects.create!(namespace_id: namespace.id) }
let!(:project4) { projects.create!(namespace_id: namespace.id) }
# pages_access_level cannot be null.
let(:non_null_project_features) { { pages_access_level: enabled } }
let!(:project_feature1) { project_features.create!(project_id: project1.id, **non_null_project_features) }
let!(:project_feature2) { project_features.create!(project_id: project2.id, **non_null_project_features) }
let!(:project_feature3) { project_features.create!(project_id: project3.id, **non_null_project_features) }
describe '#perform' do
before do
project1.update!(container_registry_enabled: true)
project2.update!(container_registry_enabled: false)
project3.update!(container_registry_enabled: nil)
project4.update!(container_registry_enabled: true)
end
it 'copies values to project_features' do
expect(project1.container_registry_enabled).to eq(true)
expect(project2.container_registry_enabled).to eq(false)
expect(project3.container_registry_enabled).to eq(nil)
expect(project4.container_registry_enabled).to eq(true)
expect(project_feature1.container_registry_access_level).to eq(disabled)
expect(project_feature2.container_registry_access_level).to eq(disabled)
expect(project_feature3.container_registry_access_level).to eq(disabled)
expect_next_instance_of(Gitlab::BackgroundMigration::Logger) do |logger|
expect(logger).to receive(:info)
.with(message: "#{described_class}: Copied container_registry_enabled values for projects with IDs between #{project1.id}..#{project4.id}")
expect(logger).not_to receive(:info)
end
subject.perform(project1.id, project4.id)
expect(project1.reload.container_registry_enabled).to eq(true)
expect(project2.reload.container_registry_enabled).to eq(false)
expect(project3.reload.container_registry_enabled).to eq(nil)
expect(project4.container_registry_enabled).to eq(true)
expect(project_feature1.reload.container_registry_access_level).to eq(enabled)
expect(project_feature2.reload.container_registry_access_level).to eq(disabled)
expect(project_feature3.reload.container_registry_access_level).to eq(disabled)
end
context 'when no projects exist in range' do
it 'does not fail' do
expect(project1.container_registry_enabled).to eq(true)
expect(project_feature1.container_registry_access_level).to eq(disabled)
expect { subject.perform(-1, -2) }.not_to raise_error
expect(project1.container_registry_enabled).to eq(true)
expect(project_feature1.container_registry_access_level).to eq(disabled)
end
end
context 'when projects in range all have nil container_registry_enabled' do
it 'does not fail' do
expect(project3.container_registry_enabled).to eq(nil)
expect(project_feature3.container_registry_access_level).to eq(disabled)
expect { subject.perform(project3.id, project3.id) }.not_to raise_error
expect(project3.container_registry_enabled).to eq(nil)
expect(project_feature3.container_registry_access_level).to eq(disabled)
end
end
end
end
# frozen_string_literal: true
require 'spec_helper'
require Rails.root.join('db', 'post_migrate', '20210226120851_move_container_registry_enabled_to_project_features.rb')
RSpec.describe MoveContainerRegistryEnabledToProjectFeatures, :migration do
let(:namespace) { table(:namespaces).create!(name: 'gitlab', path: 'gitlab-org') }
let!(:projects) do
[
table(:projects).create!(namespace_id: namespace.id, name: 'project 1'),
table(:projects).create!(namespace_id: namespace.id, name: 'project 2'),
table(:projects).create!(namespace_id: namespace.id, name: 'project 3'),
table(:projects).create!(namespace_id: namespace.id, name: 'project 4')
]
end
before do
stub_const("#{described_class.name}::BATCH_SIZE", 3)
end
around do |example|
Sidekiq::Testing.fake! do
freeze_time do
example.call
end
end
end
it 'schedules jobs for ranges of projects' do
migrate!
expect(described_class::MIGRATION)
.to be_scheduled_delayed_migration(2.minutes, projects[0].id, projects[2].id)
expect(described_class::MIGRATION)
.to be_scheduled_delayed_migration(4.minutes, projects[3].id, projects[3].id)
end
it 'schedules jobs according to the configured batch size' do
expect { migrate! }.to change { BackgroundMigrationWorker.jobs.size }.by(2)
end
end
...@@ -187,4 +187,30 @@ RSpec.describe ProjectFeature do ...@@ -187,4 +187,30 @@ RSpec.describe ProjectFeature do
expect(described_class.required_minimum_access_level_for_private_project(:issues)).to eq(Gitlab::Access::GUEST) expect(described_class.required_minimum_access_level_for_private_project(:issues)).to eq(Gitlab::Access::GUEST)
end end
end end
describe 'container_registry_access_level' do
context 'when the project is created with container_registry_enabled false' do
it 'creates project with DISABLED container_registry_access_level' do
project = create(:project, container_registry_enabled: false)
expect(project.project_feature.container_registry_access_level).to eq(described_class::DISABLED)
end
end
context 'when the project is created with container_registry_enabled true' do
it 'creates project with ENABLED container_registry_access_level' do
project = create(:project, container_registry_enabled: true)
expect(project.project_feature.container_registry_access_level).to eq(described_class::ENABLED)
end
end
context 'when the project is created with container_registry_enabled nil' do
it 'creates project with DISABLED container_registry_access_level' do
project = create(:project, container_registry_enabled: nil)
expect(project.project_feature.container_registry_access_level).to eq(described_class::DISABLED)
end
end
end
end end
...@@ -2202,6 +2202,44 @@ RSpec.describe Project, factory_default: :keep do ...@@ -2202,6 +2202,44 @@ RSpec.describe Project, factory_default: :keep do
end end
end end
describe '#set_container_registry_access_level' do
let_it_be_with_reload(:project) { create(:project) }
it 'updates project_feature', :aggregate_failures do
# Simulate an existing project that has container_registry enabled
project.update_column(:container_registry_enabled, true)
project.project_feature.update_column(:container_registry_access_level, ProjectFeature::DISABLED)
expect(project.container_registry_enabled).to eq(true)
expect(project.project_feature.container_registry_access_level).to eq(ProjectFeature::DISABLED)
project.update!(container_registry_enabled: false)
expect(project.container_registry_enabled).to eq(false)
expect(project.project_feature.container_registry_access_level).to eq(ProjectFeature::DISABLED)
project.update!(container_registry_enabled: true)
expect(project.container_registry_enabled).to eq(true)
expect(project.project_feature.container_registry_access_level).to eq(ProjectFeature::ENABLED)
end
it 'rollsback both projects and project_features row in case of error', :aggregate_failures do
project.update_column(:container_registry_enabled, true)
project.project_feature.update_column(:container_registry_access_level, ProjectFeature::DISABLED)
expect(project.container_registry_enabled).to eq(true)
expect(project.project_feature.container_registry_access_level).to eq(ProjectFeature::DISABLED)
allow(project).to receive(:valid?).and_return(false)
expect { project.update!(container_registry_enabled: false) }.to raise_error(ActiveRecord::RecordInvalid)
expect(project.reload.container_registry_enabled).to eq(true)
expect(project.project_feature.reload.container_registry_access_level).to eq(ProjectFeature::DISABLED)
end
end
describe '#has_container_registry_tags?' do describe '#has_container_registry_tags?' do
let(:project) { build(:project) } let(:project) { build(:project) }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment