Commit 5a510613 authored by Adrien Kohlbecker's avatar Adrien Kohlbecker

Change transfer, update and create services for groups and projects

Change code and add validations according to code review
Regactor helper methods
parent b49acd51
...@@ -50,6 +50,8 @@ class Projects::RunnersController < Projects::ApplicationController ...@@ -50,6 +50,8 @@ class Projects::RunnersController < Projects::ApplicationController
end end
def toggle_shared_runners def toggle_shared_runners
return redirect_to project_runners_path(@project), alert: _("Can't update due to restriction on group level") if project.group && project.group.shared_runners_setting != 'enabled'
project.toggle!(:shared_runners_enabled) project.toggle!(:shared_runners_enabled)
redirect_to project_settings_ci_cd_path(@project, anchor: 'js-runners-settings') redirect_to project_settings_ci_cd_path(@project, anchor: 'js-runners-settings')
......
...@@ -19,8 +19,6 @@ class Group < Namespace ...@@ -19,8 +19,6 @@ class Group < Namespace
ACCESS_REQUEST_APPROVERS_TO_BE_NOTIFIED_LIMIT = 10 ACCESS_REQUEST_APPROVERS_TO_BE_NOTIFIED_LIMIT = 10
UpdateSharedRunnersError = Class.new(StandardError)
has_many :all_group_members, -> { where(requested_at: nil) }, dependent: :destroy, as: :source, class_name: 'GroupMember' # rubocop:disable Cop/ActiveRecordDependent has_many :all_group_members, -> { where(requested_at: nil) }, dependent: :destroy, as: :source, class_name: 'GroupMember' # rubocop:disable Cop/ActiveRecordDependent
has_many :group_members, -> { where(requested_at: nil).where.not(members: { access_level: Gitlab::Access::MINIMAL_ACCESS }) }, dependent: :destroy, as: :source # rubocop:disable Cop/ActiveRecordDependent has_many :group_members, -> { where(requested_at: nil).where.not(members: { access_level: Gitlab::Access::MINIMAL_ACCESS }) }, dependent: :destroy, as: :source # rubocop:disable Cop/ActiveRecordDependent
alias_method :members, :group_members alias_method :members, :group_members
...@@ -538,53 +536,14 @@ class Group < Namespace ...@@ -538,53 +536,14 @@ class Group < Namespace
preloader.preload(self, shared_with_group_links: [shared_with_group: :route]) preloader.preload(self, shared_with_group_links: [shared_with_group: :route])
end end
def shared_runners_allowed? def update_shared_runners_setting!(state)
shared_runners_enabled? || allow_descendants_override_disabled_shared_runners? raise ArgumentError unless SHARED_RUNNERS_SETTINGS.include?(state)
end
def parent_allows_shared_runners?
return true unless has_parent?
parent.shared_runners_allowed?
end
def parent_enabled_shared_runners?
return true unless has_parent?
parent.shared_runners_enabled?
end
def enable_shared_runners!
raise UpdateSharedRunnersError, 'Shared Runners disabled for the parent group' unless parent_enabled_shared_runners?
update_column(:shared_runners_enabled, true)
end
def disable_shared_runners!
group_ids = self_and_descendants
return if group_ids.empty?
Group.by_id(group_ids).update_all(shared_runners_enabled: false)
all_projects.update_all(shared_runners_enabled: false)
end
def allow_descendants_override_disabled_shared_runners!
raise UpdateSharedRunnersError, 'Shared Runners enabled' if shared_runners_enabled?
raise UpdateSharedRunnersError, 'Group level shared Runners not allowed' unless parent_allows_shared_runners?
update_column(:allow_descendants_override_disabled_shared_runners, true)
end
def disallow_descendants_override_disabled_shared_runners!
raise UpdateSharedRunnersError, 'Shared Runners enabled' if shared_runners_enabled?
group_ids = self_and_descendants case state
return if group_ids.empty? when 'disabled_and_unoverridable' then disable_shared_runners! # also disallows override
when 'disabled_with_override' then disable_shared_runners_and_allow_override!
Group.by_id(group_ids).update_all(allow_descendants_override_disabled_shared_runners: false) when 'enabled' then enable_shared_runners! # set both to true
end
all_projects.update_all(shared_runners_enabled: false)
end end
def default_owner def default_owner
...@@ -668,6 +627,45 @@ class Group < Namespace ...@@ -668,6 +627,45 @@ class Group < Namespace
.new(Group.where(id: group_ids)) .new(Group.where(id: group_ids))
.base_and_descendants .base_and_descendants
end end
def disable_shared_runners!
update!(
shared_runners_enabled: false,
allow_descendants_override_disabled_shared_runners: false)
group_ids = descendants
unless group_ids.empty?
Group.by_id(group_ids).update_all(
shared_runners_enabled: false,
allow_descendants_override_disabled_shared_runners: false)
end
all_projects.update_all(shared_runners_enabled: false)
end
def disable_shared_runners_and_allow_override!
# enabled -> disabled_with_override
if shared_runners_enabled?
update!(
shared_runners_enabled: false,
allow_descendants_override_disabled_shared_runners: true)
group_ids = descendants
unless group_ids.empty?
Group.by_id(group_ids).update_all(shared_runners_enabled: false)
end
all_projects.update_all(shared_runners_enabled: false)
# disabled_and_unoverridable -> disabled_with_override
else
update!(allow_descendants_override_disabled_shared_runners: true)
end
end
def enable_shared_runners!
update!(shared_runners_enabled: true)
end
end end
Group.prepend_if_ee('EE::Group') Group.prepend_if_ee('EE::Group')
...@@ -18,6 +18,8 @@ class Namespace < ApplicationRecord ...@@ -18,6 +18,8 @@ class Namespace < ApplicationRecord
# Android repo (15) + some extra backup. # Android repo (15) + some extra backup.
NUMBER_OF_ANCESTORS_ALLOWED = 20 NUMBER_OF_ANCESTORS_ALLOWED = 20
SHARED_RUNNERS_SETTINGS = %w[disabled_and_unoverridable disabled_with_override enabled].freeze
cache_markdown_field :description, pipeline: :description cache_markdown_field :description, pipeline: :description
has_many :projects, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent has_many :projects, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
...@@ -59,6 +61,8 @@ class Namespace < ApplicationRecord ...@@ -59,6 +61,8 @@ class Namespace < ApplicationRecord
validates :max_artifacts_size, numericality: { only_integer: true, greater_than: 0, allow_nil: true } validates :max_artifacts_size, numericality: { only_integer: true, greater_than: 0, allow_nil: true }
validate :nesting_level_allowed validate :nesting_level_allowed
validate :changing_shared_runners_enabled_is_allowed
validate :changing_allow_descendants_override_disabled_shared_runners_is_allowed
validates_associated :runners validates_associated :runners
...@@ -378,6 +382,50 @@ class Namespace < ApplicationRecord ...@@ -378,6 +382,50 @@ class Namespace < ApplicationRecord
actual_plan.name actual_plan.name
end end
def changing_shared_runners_enabled_is_allowed
return unless new_record? || changes.has_key?(:shared_runners_enabled)
if shared_runners_enabled && has_parent? && parent.shared_runners_setting == 'disabled_and_unoverridable'
errors.add(:shared_runners_enabled, _('cannot be enabled because parent group has shared Runners disabled'))
end
end
def changing_allow_descendants_override_disabled_shared_runners_is_allowed
return unless new_record? || changes.has_key?(:allow_descendants_override_disabled_shared_runners)
if shared_runners_enabled && !new_record?
errors.add(:allow_descendants_override_disabled_shared_runners, _('cannot be changed if shared runners are enabled'))
end
if allow_descendants_override_disabled_shared_runners && has_parent? && parent.shared_runners_setting == 'disabled_and_unoverridable'
errors.add(:allow_descendants_override_disabled_shared_runners, _('cannot be enabled because parent group does not allow it'))
end
end
def shared_runners_setting
if shared_runners_enabled
'enabled'
else
if allow_descendants_override_disabled_shared_runners
'disabled_with_override'
else
'disabled_and_unoverridable'
end
end
end
def shared_runners_setting_higher_than?(other_setting)
if other_setting == 'enabled'
false
elsif other_setting == 'disabled_with_override'
shared_runners_setting == 'enabled'
elsif other_setting == 'disabled_and_unoverridable'
shared_runners_setting == 'enabled' || shared_runners_setting == 'disabled_with_override'
else
raise ArgumentError
end
end
private private
def all_projects_with_pages def all_projects_with_pages
......
...@@ -435,6 +435,7 @@ class Project < ApplicationRecord ...@@ -435,6 +435,7 @@ class Project < ApplicationRecord
validate :visibility_level_allowed_by_group, if: :should_validate_visibility_level? validate :visibility_level_allowed_by_group, if: :should_validate_visibility_level?
validate :visibility_level_allowed_as_fork, if: :should_validate_visibility_level? validate :visibility_level_allowed_as_fork, if: :should_validate_visibility_level?
validate :validate_pages_https_only, if: -> { changes.has_key?(:pages_https_only) } validate :validate_pages_https_only, if: -> { changes.has_key?(:pages_https_only) }
validate :changing_shared_runners_enabled_is_allowed
validates :repository_storage, validates :repository_storage,
presence: true, presence: true,
inclusion: { in: ->(_object) { Gitlab.config.repositories.storages.keys } } inclusion: { in: ->(_object) { Gitlab.config.repositories.storages.keys } }
...@@ -1189,6 +1190,14 @@ class Project < ApplicationRecord ...@@ -1189,6 +1190,14 @@ class Project < ApplicationRecord
end end
end end
def changing_shared_runners_enabled_is_allowed
return unless new_record? || changes.has_key?(:shared_runners_enabled)
if shared_runners_enabled && group && group.shared_runners_setting == 'disabled_and_unoverridable'
errors.add(:shared_runners_enabled, _('cannot be enabled because parent group does not allow it'))
end
end
def to_param def to_param
if persisted? && errors.include?(:path) if persisted? && errors.include?(:path)
path_was path_was
......
...@@ -15,6 +15,8 @@ module Groups ...@@ -15,6 +15,8 @@ module Groups
after_build_hook(@group, params) after_build_hook(@group, params)
inherit_group_shared_runners_settings
unless can_use_visibility_level? && can_create_group? unless can_use_visibility_level? && can_create_group?
return @group return @group
end end
...@@ -86,6 +88,13 @@ module Groups ...@@ -86,6 +88,13 @@ module Groups
params[:visibility_level] = Gitlab::CurrentSettings.current_application_settings.default_group_visibility params[:visibility_level] = Gitlab::CurrentSettings.current_application_settings.default_group_visibility
end end
def inherit_group_shared_runners_settings
return unless @group.parent
@group.shared_runners_enabled = @group.parent.shared_runners_enabled
@group.allow_descendants_override_disabled_shared_runners = @group.parent.allow_descendants_override_disabled_shared_runners
end
end end
end end
......
...@@ -103,6 +103,9 @@ module Groups ...@@ -103,6 +103,9 @@ module Groups
@group.parent = @new_parent_group @group.parent = @new_parent_group
@group.clear_memoization(:self_and_ancestors_ids) @group.clear_memoization(:self_and_ancestors_ids)
inherit_group_shared_runners_settings
@group.save! @group.save!
end end
...@@ -161,6 +164,17 @@ module Groups ...@@ -161,6 +164,17 @@ module Groups
group_contains_npm_packages: s_('TransferGroup|Group contains projects with NPM packages.') group_contains_npm_packages: s_('TransferGroup|Group contains projects with NPM packages.')
}.freeze }.freeze
end end
def inherit_group_shared_runners_settings
parent_setting = @group.parent&.shared_runners_setting
return unless parent_setting
if @group.shared_runners_setting_higher_than?(parent_setting)
result = Groups::UpdateSharedRunnersService.new(@group, current_user, shared_runners_setting: parent_setting).execute
raise TransferError, result[:message] unless result[:status] == :success
end
end
end end
end end
......
...@@ -19,6 +19,8 @@ module Groups ...@@ -19,6 +19,8 @@ module Groups
return false unless valid_path_change_with_npm_packages? return false unless valid_path_change_with_npm_packages?
return false unless update_shared_runners
before_assignment_hook(group, params) before_assignment_hook(group, params)
group.assign_attributes(params) group.assign_attributes(params)
...@@ -98,6 +100,17 @@ module Groups ...@@ -98,6 +100,17 @@ module Groups
params[:share_with_group_lock] != group.share_with_group_lock params[:share_with_group_lock] != group.share_with_group_lock
end end
def update_shared_runners
return true if params[:shared_runners_setting].nil?
result = Groups::UpdateSharedRunnersService.new(group, current_user, shared_runners_setting: params.delete(:shared_runners_setting)).execute
return true if result[:status] == :success
group.errors.add(:update_shared_runners, result[:message])
false
end
end end
end end
......
...@@ -7,44 +7,24 @@ module Groups ...@@ -7,44 +7,24 @@ module Groups
validate_params validate_params
enable_or_disable_shared_runners! update_shared_runners
allow_or_disallow_descendants_override_disabled_shared_runners!
success success
rescue Group::UpdateSharedRunnersError => error rescue ActiveRecord::RecordInvalid, ArgumentError => error
error(error.message) error(error.message)
end end
private private
def validate_params def validate_params
if Gitlab::Utils.to_boolean(params[:shared_runners_enabled]) && !params[:allow_descendants_override_disabled_shared_runners].nil? unless Namespace::SHARED_RUNNERS_SETTINGS.include?(params[:shared_runners_setting])
raise Group::UpdateSharedRunnersError, 'Cannot set shared_runners_enabled to true and allow_descendants_override_disabled_shared_runners' raise ArgumentError, "state must be one of: #{Namespace::SHARED_RUNNERS_SETTINGS.join(', ')}"
end end
end end
def enable_or_disable_shared_runners! def update_shared_runners
return if params[:shared_runners_enabled].nil? group.update_shared_runners_setting!(params[:shared_runners_setting])
if Gitlab::Utils.to_boolean(params[:shared_runners_enabled])
group.enable_shared_runners!
else
group.disable_shared_runners!
end
end
def allow_or_disallow_descendants_override_disabled_shared_runners!
return if params[:allow_descendants_override_disabled_shared_runners].nil?
# Needs to reset group because if both params are present could result in error
group.reset
if Gitlab::Utils.to_boolean(params[:allow_descendants_override_disabled_shared_runners])
group.allow_descendants_override_disabled_shared_runners!
else
group.disallow_descendants_override_disabled_shared_runners!
end
end end
end end
end end
...@@ -19,6 +19,10 @@ module Projects ...@@ -19,6 +19,10 @@ module Projects
@project = Project.new(params) @project = Project.new(params)
# If a project is newly created it should have shared runners settings
# based on its group having it enabled. This is like the "default value"
@project.shared_runners_enabled = false if !params.key?(:shared_runners_enabled) && @project.group && @project.group.shared_runners_setting != 'enabled'
# Make sure that the user is allowed to use the specified visibility level # Make sure that the user is allowed to use the specified visibility level
if project_visibility.restricted? if project_visibility.restricted?
deny_visibility_level(@project, project_visibility.visibility_level) deny_visibility_level(@project, project_visibility.visibility_level)
......
...@@ -88,6 +88,10 @@ module Projects ...@@ -88,6 +88,10 @@ module Projects
# Move uploads # Move uploads
move_project_uploads(project) move_project_uploads(project)
# If a project is being transferred to another group it means it can already
# have shared runners enabled but we need to check whether the new group allows that.
project.shared_runners_enabled = false if project.group && project.group.shared_runners_setting == 'disabled_and_unoverridable'
project.old_path_with_namespace = @old_path project.old_path_with_namespace = @old_path
update_repository_configuration(@new_path) update_repository_configuration(@new_path)
......
---
title: Change transfer, update and create services for groups and projects to take in consideration shared runners settings
merge_request: 36080
author: Arthur de Lapertosa Lisboa
type: added
...@@ -4509,6 +4509,9 @@ msgstr "" ...@@ -4509,6 +4509,9 @@ msgstr ""
msgid "Can't scan the code?" msgid "Can't scan the code?"
msgstr "" msgstr ""
msgid "Can't update due to restriction on group level"
msgstr ""
msgid "Can't update snippet: %{err}" msgid "Can't update snippet: %{err}"
msgstr "" msgstr ""
...@@ -29951,6 +29954,15 @@ msgstr "" ...@@ -29951,6 +29954,15 @@ msgstr ""
msgid "cannot be changed if a personal project has container registry tags." msgid "cannot be changed if a personal project has container registry tags."
msgstr "" msgstr ""
msgid "cannot be changed if shared runners are enabled"
msgstr ""
msgid "cannot be enabled because parent group does not allow it"
msgstr ""
msgid "cannot be enabled because parent group has shared Runners disabled"
msgstr ""
msgid "cannot be enabled unless all domains have TLS certificates" msgid "cannot be enabled unless all domains have TLS certificates"
msgstr "" msgstr ""
......
...@@ -73,4 +73,33 @@ RSpec.describe Projects::RunnersController do ...@@ -73,4 +73,33 @@ RSpec.describe Projects::RunnersController do
expect(runner.active).to eq(false) expect(runner.active).to eq(false)
end end
end end
describe '#toggle_shared_runners' do
let(:group) { create(:group) }
let(:project) { create(:project, group: group) }
it 'toggles shared_runners_enabled' do
project.update!(shared_runners_enabled: true)
post :toggle_shared_runners, params: params
project.reload
expect(response).to have_gitlab_http_status(:found)
expect(project.shared_runners_enabled).to eq(false)
end
it 'does not enable if the group disallows shared runners' do
group.update!(shared_runners_enabled: false)
project.update!(shared_runners_enabled: false)
post :toggle_shared_runners, params: params
project.reload
expect(response).to have_gitlab_http_status(:found)
expect(project.shared_runners_enabled).to eq(false)
expect(flash[:alert]).to eq("Can't update due to restriction on group level")
end
end
end end
...@@ -63,5 +63,13 @@ FactoryBot.define do ...@@ -63,5 +63,13 @@ FactoryBot.define do
) )
end end
end end
trait :shared_runners_disabled do
shared_runners_enabled { false }
end
trait :allow_descendants_override_disabled_shared_runners do
allow_descendants_override_disabled_shared_runners { true }
end
end end
end end
This diff is collapsed.
...@@ -1320,4 +1320,140 @@ RSpec.describe Namespace do ...@@ -1320,4 +1320,140 @@ RSpec.describe Namespace do
end end
end end
end end
describe '#shared_runners_setting' do
using RSpec::Parameterized::TableSyntax
where(:shared_runners_enabled, :allow_descendants_override_disabled_shared_runners, :shared_runners_setting) do
true | true | 'enabled'
true | false | 'enabled'
false | true | 'disabled_with_override'
false | false | 'disabled_and_unoverridable'
end
with_them do
let(:namespace) { build(:namespace, shared_runners_enabled: shared_runners_enabled, allow_descendants_override_disabled_shared_runners: allow_descendants_override_disabled_shared_runners)}
it 'returns the result' do
expect(namespace.shared_runners_setting).to eq(shared_runners_setting)
end
end
end
describe '#shared_runners_setting_higher_than?' do
using RSpec::Parameterized::TableSyntax
where(:shared_runners_enabled, :allow_descendants_override_disabled_shared_runners, :other_setting, :result) do
true | true | 'enabled' | false
true | true | 'disabled_with_override' | true
true | true | 'disabled_and_unoverridable' | true
false | true | 'enabled' | false
false | true | 'disabled_with_override' | false
false | true | 'disabled_and_unoverridable' | true
false | false | 'enabled' | false
false | false | 'disabled_with_override' | false
false | false | 'disabled_and_unoverridable' | false
end
with_them do
let(:namespace) { build(:namespace, shared_runners_enabled: shared_runners_enabled, allow_descendants_override_disabled_shared_runners: allow_descendants_override_disabled_shared_runners)}
it 'returns the result' do
expect(namespace.shared_runners_setting_higher_than?(other_setting)).to eq(result)
end
end
end
describe 'validation #changing_shared_runners_enabled_is_allowed' do
context 'without a parent' do
let(:namespace) { build(:namespace, shared_runners_enabled: true) }
it 'is valid' do
expect(namespace).to be_valid
end
end
context 'with a parent' do
context 'when parent has shared runners disabled' do
let(:parent) { create(:namespace, :shared_runners_disabled) }
let(:sub_namespace) { build(:namespace, shared_runners_enabled: true, parent_id: parent.id) }
it 'is invalid' do
expect(sub_namespace).to be_invalid
expect(sub_namespace.errors[:shared_runners_enabled]).to include('cannot be enabled because parent group has shared Runners disabled')
end
end
context 'when parent has shared runners disabled but allows override' do
let(:parent) { create(:namespace, :shared_runners_disabled, :allow_descendants_override_disabled_shared_runners) }
let(:sub_namespace) { build(:namespace, shared_runners_enabled: true, parent_id: parent.id) }
it 'is valid' do
expect(sub_namespace).to be_valid
end
end
context 'when parent has shared runners enabled' do
let(:parent) { create(:namespace, shared_runners_enabled: true) }
let(:sub_namespace) { build(:namespace, shared_runners_enabled: true, parent_id: parent.id) }
it 'is valid' do
expect(sub_namespace).to be_valid
end
end
end
end
describe 'validation #changing_allow_descendants_override_disabled_shared_runners_is_allowed' do
context 'without a parent' do
context 'with shared runners disabled' do
let(:namespace) { build(:namespace, :allow_descendants_override_disabled_shared_runners, :shared_runners_disabled) }
it 'is valid' do
expect(namespace).to be_valid
end
end
context 'with shared runners enabled' do
let(:namespace) { create(:namespace) }
it 'is invalid' do
namespace.allow_descendants_override_disabled_shared_runners = true
expect(namespace).to be_invalid
expect(namespace.errors[:allow_descendants_override_disabled_shared_runners]).to include('cannot be changed if shared runners are enabled')
end
end
end
context 'with a parent' do
context 'when parent does not allow shared runners' do
let(:parent) { create(:namespace, :shared_runners_disabled) }
let(:sub_namespace) { build(:namespace, :shared_runners_disabled, :allow_descendants_override_disabled_shared_runners, parent_id: parent.id) }
it 'is invalid' do
expect(sub_namespace).to be_invalid
expect(sub_namespace.errors[:allow_descendants_override_disabled_shared_runners]).to include('cannot be enabled because parent group does not allow it')
end
end
context 'when parent allows shared runners and setting to true' do
let(:parent) { create(:namespace, shared_runners_enabled: true) }
let(:sub_namespace) { build(:namespace, :shared_runners_disabled, :allow_descendants_override_disabled_shared_runners, parent_id: parent.id) }
it 'is valid' do
expect(sub_namespace).to be_valid
end
end
context 'when parent allows shared runners and setting to false' do
let(:parent) { create(:namespace, shared_runners_enabled: true) }
let(:sub_namespace) { build(:namespace, :shared_runners_disabled, allow_descendants_override_disabled_shared_runners: false, parent_id: parent.id) }
it 'is valid' do
expect(sub_namespace).to be_valid
end
end
end
end
end end
...@@ -5813,6 +5813,38 @@ RSpec.describe Project do ...@@ -5813,6 +5813,38 @@ RSpec.describe Project do
end end
end end
describe 'validation #changing_shared_runners_enabled_is_allowed' do
using RSpec::Parameterized::TableSyntax
where(:shared_runners_setting, :project_shared_runners_enabled, :valid_record) do
'enabled' | true | true
'enabled' | false | true
'disabled_with_override' | true | true
'disabled_with_override' | false | true
'disabled_and_unoverridable' | true | false
'disabled_and_unoverridable' | false | true
end
with_them do
let(:group) { create(:group) }
let(:project) { build(:project, namespace: group, shared_runners_enabled: project_shared_runners_enabled) }
before do
allow_next_found_instance_of(Group) do |group|
allow(group).to receive(:shared_runners_setting).and_return(shared_runners_setting)
end
end
it 'validates the configuration' do
expect(project.valid?).to eq(valid_record)
unless valid_record
expect(project.errors[:shared_runners_enabled]).to contain_exactly('cannot be enabled because parent group does not allow it')
end
end
end
end
describe '#mark_pages_as_deployed' do describe '#mark_pages_as_deployed' do
let(:project) { create(:project) } let(:project) { create(:project) }
let(:artifacts_archive) { create(:ci_job_artifact, project: project) } let(:artifacts_archive) { create(:ci_job_artifact, project: project) }
......
...@@ -185,4 +185,44 @@ RSpec.describe Groups::CreateService, '#execute' do ...@@ -185,4 +185,44 @@ RSpec.describe Groups::CreateService, '#execute' do
end end
end end
end end
context 'shared runners configuration' do
context 'parent group present' do
using RSpec::Parameterized::TableSyntax
where(:shared_runners_config, :descendants_override_disabled_shared_runners_config) do
true | false
false | false
# true | true # invalid at the group level, leaving as comment to make explicit
false | true
end
with_them do
let!(:group) { create(:group, shared_runners_enabled: shared_runners_config, allow_descendants_override_disabled_shared_runners: descendants_override_disabled_shared_runners_config) }
let!(:service) { described_class.new(user, group_params.merge(parent_id: group.id)) }
before do
group.add_owner(user)
end
it 'creates group following the parent config' do
new_group = service.execute
expect(new_group.shared_runners_enabled).to eq(shared_runners_config)
expect(new_group.allow_descendants_override_disabled_shared_runners).to eq(descendants_override_disabled_shared_runners_config)
end
end
end
context 'root group' do
let!(:service) { described_class.new(user) }
it 'follows default config' do
new_group = service.execute
expect(new_group.shared_runners_enabled).to eq(true)
expect(new_group.allow_descendants_override_disabled_shared_runners).to eq(false)
end
end
end
end end
...@@ -285,6 +285,44 @@ RSpec.describe Groups::TransferService do ...@@ -285,6 +285,44 @@ RSpec.describe Groups::TransferService do
end end
end end
context 'shared runners configuration' do
before do
create(:group_member, :owner, group: new_parent_group, user: user)
end
context 'if parent group has disabled shared runners but allows overrides' do
let(:new_parent_group) { create(:group, shared_runners_enabled: false, allow_descendants_override_disabled_shared_runners: true) }
it 'calls update service ' do
expect(Groups::UpdateSharedRunnersService).to receive(:new).with(group, user, { shared_runners_setting: 'disabled_with_override' }).and_call_original
transfer_service.execute(new_parent_group)
end
end
context 'if parent group does not allow shared runners' do
let(:new_parent_group) { create(:group, shared_runners_enabled: false, allow_descendants_override_disabled_shared_runners: false) }
it 'calls update service ' do
expect(Groups::UpdateSharedRunnersService).to receive(:new).with(group, user, { shared_runners_setting: 'disabled_and_unoverridable' }).and_call_original
transfer_service.execute(new_parent_group)
end
end
context 'if parent group allows shared runners' do
let(:group) { create(:group, :public, :nested, shared_runners_enabled: false) }
let(:new_parent_group) { create(:group, shared_runners_enabled: true) }
it 'does not call update service and keeps them disabled on the group' do
expect(Groups::UpdateSharedRunnersService).not_to receive(:new)
transfer_service.execute(new_parent_group)
expect(group.reload.shared_runners_enabled).to be_falsy
end
end
end
context 'when a group is transferred to its subgroup' do context 'when a group is transferred to its subgroup' do
let(:new_parent_group) { create(:group, parent: group) } let(:new_parent_group) { create(:group, parent: group) }
......
...@@ -283,6 +283,31 @@ RSpec.describe Groups::UpdateService do ...@@ -283,6 +283,31 @@ RSpec.describe Groups::UpdateService do
end end
end end
context 'change shared Runners config' do
let(:group) { create(:group) }
let(:project) { create(:project, shared_runners_enabled: true, group: group) }
subject { described_class.new(group, user, shared_runners_setting: 'disabled_and_unoverridable').execute }
before do
group.add_owner(user)
end
it 'calls the shared runners update service' do
expect_any_instance_of(::Groups::UpdateSharedRunnersService).to receive(:execute).and_return({ status: :success })
expect(subject).to be_truthy
end
it 'handles errors in the shared runners update service' do
expect_any_instance_of(::Groups::UpdateSharedRunnersService).to receive(:execute).and_return({ status: :error, message: 'something happened' })
expect(subject).to be_falsy
expect(group.errors[:update_shared_runners].first).to eq('something happened')
end
end
def update_group(group, user, opts) def update_group(group, user, opts)
Groups::UpdateService.new(group, user, opts).execute Groups::UpdateService.new(group, user, opts).execute
end end
......
...@@ -782,4 +782,120 @@ RSpec.describe Projects::CreateService, '#execute' do ...@@ -782,4 +782,120 @@ RSpec.describe Projects::CreateService, '#execute' do
def create_project(user, opts) def create_project(user, opts)
Projects::CreateService.new(user, opts).execute Projects::CreateService.new(user, opts).execute
end end
context 'shared Runners config' do
context 'default value based on parent group setting' do
using RSpec::Parameterized::TableSyntax
where(:shared_runners_setting, :desired_config_for_new_project, :expected_result_for_project) do
'enabled' | nil | true
'disabled_with_override' | nil | false
'disabled_and_unoverridable' | nil | false
end
with_them do
let(:group) { create(:group) }
before do
allow_next_found_instance_of(Group) do |group|
allow(group).to receive(:shared_runners_setting).and_return(shared_runners_setting)
end
group.add_owner(user)
user.refresh_authorized_projects # Ensure cache is warm
end
it 'creates project following the parent config' do
params = opts.merge(namespace_id: group.id)
params = params.merge(shared_runners_enabled: desired_config_for_new_project) unless desired_config_for_new_project.nil?
project = create_project(user, params)
expect(project).to be_valid
expect(project.shared_runners_enabled).to eq(expected_result_for_project)
end
end
end
context 'parent group is present and allows desired config' do
using RSpec::Parameterized::TableSyntax
where(:shared_runners_setting, :desired_config_for_new_project, :expected_result_for_project) do
'enabled' | true | true
'enabled' | false | false
'disabled_with_override' | false | false
'disabled_with_override' | true | true
'disabled_and_unoverridable' | false | false
end
with_them do
let(:group) { create(:group) }
before do
allow_next_found_instance_of(Group) do |group|
allow(group).to receive(:shared_runners_setting).and_return(shared_runners_setting)
end
group.add_owner(user)
user.refresh_authorized_projects # Ensure cache is warm
end
it 'creates project following the parent config' do
params = opts.merge(namespace_id: group.id, shared_runners_enabled: desired_config_for_new_project)
project = create_project(user, params)
expect(project).to be_valid
expect(project.shared_runners_enabled).to eq(expected_result_for_project)
end
end
end
context 'parent group is present and disallows desired config' do
using RSpec::Parameterized::TableSyntax
where(:shared_runners_setting, :desired_config_for_new_project, :expected_result_for_project) do
'disabled_and_unoverridable' | true | false
end
with_them do
let(:group) { create(:group) }
before do
allow_next_found_instance_of(Group) do |group|
allow(group).to receive(:shared_runners_setting).and_return(shared_runners_setting)
end
group.add_owner(user)
user.refresh_authorized_projects # Ensure cache is warm
end
it 'created project is invalid' do
params = opts.merge(namespace_id: group.id, shared_runners_enabled: desired_config_for_new_project)
project = create_project(user, params)
expect(project).to be_invalid
expect(project.errors[:shared_runners_enabled]).to include('cannot be enabled because parent group does not allow it')
end
end
end
context 'parent group is not present' do
using RSpec::Parameterized::TableSyntax
where(:desired_config, :expected_result) do
true | true
false | false
nil | true
end
with_them do
it 'follows desired config' do
opts[:shared_runners_enabled] = desired_config unless desired_config.nil?
project = create_project(user, opts)
expect(project).to be_valid
expect(project.shared_runners_enabled).to eq(expected_result)
end
end
end
end
end end
...@@ -314,6 +314,37 @@ RSpec.describe Projects::TransferService do ...@@ -314,6 +314,37 @@ RSpec.describe Projects::TransferService do
end end
end end
context 'shared Runners group level configurations' do
using RSpec::Parameterized::TableSyntax
where(:project_shared_runners_enabled, :shared_runners_setting, :expected_shared_runners_enabled) do
true | 'disabled_and_unoverridable' | false
false | 'disabled_and_unoverridable' | false
true | 'disabled_with_override' | true
false | 'disabled_with_override' | false
true | 'enabled' | true
false | 'enabled' | false
end
with_them do
let(:project) { create(:project, :public, :repository, namespace: user.namespace, shared_runners_enabled: project_shared_runners_enabled) }
let(:group) { create(:group) }
before do
group.add_owner(user)
expect_next_found_instance_of(Group) do |group|
expect(group).to receive(:shared_runners_setting).and_return(shared_runners_setting)
end
execute_transfer
end
it 'updates shared runners based on the parent group' do
expect(project.shared_runners_enabled).to eq(expected_shared_runners_enabled)
end
end
end
context 'missing group labels applied to issues or merge requests' do context 'missing group labels applied to issues or merge requests' do
it 'delegates transfer to Labels::TransferService' do it 'delegates transfer to Labels::TransferService' do
group.add_owner(user) group.add_owner(user)
......
...@@ -151,6 +151,32 @@ RSpec.describe Projects::UpdateService do ...@@ -151,6 +151,32 @@ RSpec.describe Projects::UpdateService do
expect(project.reload).to be_internal expect(project.reload).to be_internal
end end
end end
context 'when updating shared runners' do
context 'can enable shared runners' do
let(:group) { create(:group, shared_runners_enabled: true) }
let(:project) { create(:project, namespace: group, shared_runners_enabled: false) }
it 'enables shared runners' do
result = update_project(project, user, shared_runners_enabled: true)
expect(result).to eq({ status: :success })
expect(project.reload.shared_runners_enabled).to be_truthy
end
end
context 'cannot enable shared runners' do
let(:group) { create(:group, :shared_runners_disabled) }
let(:project) { create(:project, namespace: group, shared_runners_enabled: false) }
it 'does not enable shared runners' do
result = update_project(project, user, shared_runners_enabled: true)
expect(result).to eq({ status: :error, message: 'Shared runners enabled cannot be enabled because parent group does not allow it' })
expect(project.reload.shared_runners_enabled).to be_falsey
end
end
end
end end
describe 'when updating project that has forks' do describe 'when updating project that has forks' do
......
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