Commit 872ed408 authored by Mark Chao's avatar Mark Chao

Merge branch 'change-transfer-update-create-services' into 'master'

Change transfer, update and create services for groups and projects

See merge request gitlab-org/gitlab!36080
parents b6436fe6 05d505d8
...@@ -50,6 +50,10 @@ class Projects::RunnersController < Projects::ApplicationController ...@@ -50,6 +50,10 @@ class Projects::RunnersController < Projects::ApplicationController
end end
def toggle_shared_runners def toggle_shared_runners
if Feature.enabled?(:disable_shared_runners_on_group, default_enabled: true) && !project.shared_runners_enabled && project.group && project.group.shared_runners_setting == 'disabled_and_unoverridable'
return redirect_to project_runners_path(@project), alert: _("Cannot enable shared runners because parent group does not allow it")
end
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? case state
when 'disabled_and_unoverridable' then disable_shared_runners! # also disallows override
when 'disabled_with_override' then disable_shared_runners_and_allow_override!
when 'enabled' then enable_shared_runners! # set both to true
end 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
return if group_ids.empty?
Group.by_id(group_ids).update_all(allow_descendants_override_disabled_shared_runners: false)
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,52 @@ class Namespace < ApplicationRecord ...@@ -378,6 +382,52 @@ class Namespace < ApplicationRecord
actual_plan.name actual_plan.name
end end
def changing_shared_runners_enabled_is_allowed
return unless Feature.enabled?(:disable_shared_runners_on_group, default_enabled: true)
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 Feature.enabled?(:disable_shared_runners_on_group, default_enabled: true)
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,15 @@ class Project < ApplicationRecord ...@@ -1189,6 +1190,15 @@ class Project < ApplicationRecord
end end
end end
def changing_shared_runners_enabled_is_allowed
return unless Feature.enabled?(:disable_shared_runners_on_group, default_enabled: true)
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
---
name: disable_shared_runners_on_group
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/36080
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/258991
type: development
group: group::runner
default_enabled: true
...@@ -4551,6 +4551,9 @@ msgstr "" ...@@ -4551,6 +4551,9 @@ msgstr ""
msgid "Cannot create the abuse report. This user has been blocked." msgid "Cannot create the abuse report. This user has been blocked."
msgstr "" msgstr ""
msgid "Cannot enable shared runners because parent group does not allow it"
msgstr ""
msgid "Cannot have multiple Jira imports running at the same time" msgid "Cannot have multiple Jira imports running at the same time"
msgstr "" msgstr ""
...@@ -29957,6 +29960,15 @@ msgstr "" ...@@ -29957,6 +29960,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,45 @@ RSpec.describe Projects::RunnersController do ...@@ -73,4 +73,45 @@ 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 when the group allows shared runners' 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 'toggles shared_runners_enabled when the group disallows shared runners but allows overrides' do
group.update!(shared_runners_enabled: false, allow_descendants_override_disabled_shared_runners: true)
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(true)
end
it 'does not enable if the group disallows shared runners' do
group.update!(shared_runners_enabled: false, allow_descendants_override_disabled_shared_runners: 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("Cannot enable shared runners because parent group does not allow it")
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
...@@ -1344,83 +1344,14 @@ RSpec.describe Group do ...@@ -1344,83 +1344,14 @@ RSpec.describe Group do
end end
end end
describe '#shared_runners_allowed?' do def subject_and_reload(*models)
using RSpec::Parameterized::TableSyntax subject
models.map(&:reload)
where(:shared_runners_enabled, :allow_descendants_override, :expected_shared_runners_allowed) do
true | false | true
true | true | true
false | false | false
false | true | true
end
with_them do
let!(:group) { create(:group, shared_runners_enabled: shared_runners_enabled, allow_descendants_override_disabled_shared_runners: allow_descendants_override) }
it 'returns the expected result' do
expect(group.shared_runners_allowed?).to eq(expected_shared_runners_allowed)
end
end
end end
describe '#parent_allows_shared_runners?' do describe '#update_shared_runners_setting!' do
context 'when parent group is present' do context 'enabled' do
using RSpec::Parameterized::TableSyntax subject { group.update_shared_runners_setting!('enabled') }
where(:shared_runners_enabled, :allow_descendants_override, :expected_shared_runners_allowed) do
true | false | true
true | true | true
false | false | false
false | true | true
end
with_them do
let!(:parent_group) { create(:group, shared_runners_enabled: shared_runners_enabled, allow_descendants_override_disabled_shared_runners: allow_descendants_override) }
let!(:group) { create(:group, parent: parent_group) }
it 'returns the expected result' do
expect(group.parent_allows_shared_runners?).to eq(expected_shared_runners_allowed)
end
end
end
context 'when parent group is missing' do
let!(:group) { create(:group) }
it 'returns true' do
expect(group.parent_allows_shared_runners?).to be_truthy
end
end
end
describe '#parent_enabled_shared_runners?' do
subject { group.parent_enabled_shared_runners? }
context 'when parent group is present' do
context 'When shared Runners are disabled' do
let!(:parent_group) { create(:group, :shared_runners_disabled) }
let!(:group) { create(:group, parent: parent_group) }
it { is_expected.to be_falsy }
end
context 'When shared Runners are enabled' do
let!(:parent_group) { create(:group) }
let!(:group) { create(:group, parent: parent_group) }
it { is_expected.to be_truthy }
end
end
context 'when parent group is missing' do
let!(:group) { create(:group) }
it { is_expected.to be_truthy }
end
end
describe '#enable_shared_runners!' do
subject { group.enable_shared_runners! }
context 'group that its ancestors have shared runners disabled' do context 'group that its ancestors have shared runners disabled' do
let_it_be(:parent) { create(:group, :shared_runners_disabled) } let_it_be(:parent) { create(:group, :shared_runners_disabled) }
...@@ -1428,11 +1359,11 @@ RSpec.describe Group do ...@@ -1428,11 +1359,11 @@ RSpec.describe Group do
let_it_be(:project) { create(:project, shared_runners_enabled: false, group: group) } let_it_be(:project) { create(:project, shared_runners_enabled: false, group: group) }
it 'raises error and does not enable shared Runners' do it 'raises error and does not enable shared Runners' do
expect { subject } expect { subject_and_reload(parent, group, project) }
.to raise_error(described_class::UpdateSharedRunnersError, 'Shared Runners disabled for the parent group') .to raise_error(ActiveRecord::RecordInvalid, 'Validation failed: Shared runners enabled cannot be enabled because parent group has shared Runners disabled')
.and not_change { parent.reload.shared_runners_enabled } .and not_change { parent.shared_runners_enabled }
.and not_change { group.reload.shared_runners_enabled } .and not_change { group.shared_runners_enabled }
.and not_change { project.reload.shared_runners_enabled } .and not_change { project.shared_runners_enabled }
end end
end end
...@@ -1442,38 +1373,48 @@ RSpec.describe Group do ...@@ -1442,38 +1373,48 @@ RSpec.describe Group do
let_it_be(:project) { create(:project, shared_runners_enabled: false, group: sub_group) } let_it_be(:project) { create(:project, shared_runners_enabled: false, group: sub_group) }
it 'enables shared Runners only for itself' do it 'enables shared Runners only for itself' do
expect { subject } expect { subject_and_reload(group, sub_group, project) }
.to change { group.reload.shared_runners_enabled }.from(false).to(true) .to change { group.shared_runners_enabled }.from(false).to(true)
.and not_change { sub_group.reload.shared_runners_enabled } .and not_change { sub_group.shared_runners_enabled }
.and not_change { project.reload.shared_runners_enabled } .and not_change { project.shared_runners_enabled }
end end
end end
end end
describe '#disable_shared_runners!' do context 'disabled_and_unoverridable' do
let_it_be(:group) { create(:group) } let_it_be(:group) { create(:group) }
let_it_be(:sub_group) { create(:group, :shared_runners_disabled, :allow_descendants_override_disabled_shared_runners, parent: group) } let_it_be(:sub_group) { create(:group, :shared_runners_disabled, :allow_descendants_override_disabled_shared_runners, parent: group) }
let_it_be(:sub_group_2) { create(:group, parent: group) } let_it_be(:sub_group_2) { create(:group, parent: group) }
let_it_be(:project) { create(:project, group: group, shared_runners_enabled: true) } let_it_be(:project) { create(:project, group: group, shared_runners_enabled: true) }
let_it_be(:project_2) { create(:project, group: sub_group_2, shared_runners_enabled: true) } let_it_be(:project_2) { create(:project, group: sub_group_2, shared_runners_enabled: true) }
subject { group.disable_shared_runners! } subject { group.update_shared_runners_setting!('disabled_and_unoverridable') }
it 'disables shared Runners for all descendant groups and projects' do it 'disables shared Runners for all descendant groups and projects' do
expect { subject } expect { subject_and_reload(group, sub_group, sub_group_2, project, project_2) }
.to change { group.reload.shared_runners_enabled }.from(true).to(false) .to change { group.shared_runners_enabled }.from(true).to(false)
.and not_change { group.reload.allow_descendants_override_disabled_shared_runners } .and not_change { group.allow_descendants_override_disabled_shared_runners }
.and not_change { sub_group.reload.shared_runners_enabled } .and not_change { sub_group.shared_runners_enabled }
.and not_change { sub_group.reload.allow_descendants_override_disabled_shared_runners } .and change { sub_group.allow_descendants_override_disabled_shared_runners }.from(true).to(false)
.and change { sub_group_2.reload.shared_runners_enabled }.from(true).to(false) .and change { sub_group_2.shared_runners_enabled }.from(true).to(false)
.and not_change { sub_group_2.reload.allow_descendants_override_disabled_shared_runners } .and not_change { sub_group_2.allow_descendants_override_disabled_shared_runners }
.and change { project.reload.shared_runners_enabled }.from(true).to(false) .and change { project.shared_runners_enabled }.from(true).to(false)
.and change { project_2.reload.shared_runners_enabled }.from(true).to(false) .and change { project_2.shared_runners_enabled }.from(true).to(false)
end
context 'with override on self' do
let_it_be(:group) { create(:group, :shared_runners_disabled, :allow_descendants_override_disabled_shared_runners) }
it 'disables it' do
expect { subject_and_reload(group) }
.to not_change { group.shared_runners_enabled }
.and change { group.allow_descendants_override_disabled_shared_runners }.from(true).to(false)
end
end end
end end
describe '#allow_descendants_override_disabled_shared_runners!' do context 'disabled_with_override' do
subject { group.allow_descendants_override_disabled_shared_runners! } subject { group.update_shared_runners_setting!('disabled_with_override') }
context 'top level group' do context 'top level group' do
let_it_be(:group) { create(:group, :shared_runners_disabled) } let_it_be(:group) { create(:group, :shared_runners_disabled) }
...@@ -1481,12 +1422,12 @@ RSpec.describe Group do ...@@ -1481,12 +1422,12 @@ RSpec.describe Group do
let_it_be(:project) { create(:project, shared_runners_enabled: false, group: sub_group) } let_it_be(:project) { create(:project, shared_runners_enabled: false, group: sub_group) }
it 'enables allow descendants to override only for itself' do it 'enables allow descendants to override only for itself' do
expect { subject } expect { subject_and_reload(group, sub_group, project) }
.to change { group.reload.allow_descendants_override_disabled_shared_runners }.from(false).to(true) .to change { group.allow_descendants_override_disabled_shared_runners }.from(false).to(true)
.and not_change { group.reload.shared_runners_enabled } .and not_change { group.shared_runners_enabled }
.and not_change { sub_group.reload.allow_descendants_override_disabled_shared_runners } .and not_change { sub_group.allow_descendants_override_disabled_shared_runners }
.and not_change { sub_group.reload.shared_runners_enabled } .and not_change { sub_group.shared_runners_enabled }
.and not_change { project.reload.shared_runners_enabled } .and not_change { project.shared_runners_enabled }
end end
end end
...@@ -1496,12 +1437,12 @@ RSpec.describe Group do ...@@ -1496,12 +1437,12 @@ RSpec.describe Group do
let_it_be(:project) { create(:project, shared_runners_enabled: false, group: group) } let_it_be(:project) { create(:project, shared_runners_enabled: false, group: group) }
it 'enables allow descendants to override' do it 'enables allow descendants to override' do
expect { subject } expect { subject_and_reload(parent, group, project) }
.to not_change { parent.reload.allow_descendants_override_disabled_shared_runners } .to not_change { parent.allow_descendants_override_disabled_shared_runners }
.and not_change { parent.reload.shared_runners_enabled } .and not_change { parent.shared_runners_enabled }
.and change { group.reload.allow_descendants_override_disabled_shared_runners }.from(false).to(true) .and change { group.allow_descendants_override_disabled_shared_runners }.from(false).to(true)
.and not_change { group.reload.shared_runners_enabled } .and not_change { group.shared_runners_enabled }
.and not_change { project.reload.shared_runners_enabled } .and not_change { project.shared_runners_enabled }
end end
end end
...@@ -1510,64 +1451,28 @@ RSpec.describe Group do ...@@ -1510,64 +1451,28 @@ RSpec.describe Group do
let_it_be(:group) { create(:group, :shared_runners_disabled, allow_descendants_override_disabled_shared_runners: false, parent: parent) } let_it_be(:group) { create(:group, :shared_runners_disabled, allow_descendants_override_disabled_shared_runners: false, parent: parent) }
it 'raises error and does not allow descendants to override' do it 'raises error and does not allow descendants to override' do
expect { subject } expect { subject_and_reload(parent, group) }
.to raise_error(described_class::UpdateSharedRunnersError, 'Group level shared Runners not allowed') .to raise_error(ActiveRecord::RecordInvalid, 'Validation failed: Allow descendants override disabled shared runners cannot be enabled because parent group does not allow it')
.and not_change { parent.reload.allow_descendants_override_disabled_shared_runners } .and not_change { parent.allow_descendants_override_disabled_shared_runners }
.and not_change { parent.reload.shared_runners_enabled } .and not_change { parent.shared_runners_enabled }
.and not_change { group.reload.allow_descendants_override_disabled_shared_runners } .and not_change { group.allow_descendants_override_disabled_shared_runners }
.and not_change { group.reload.shared_runners_enabled } .and not_change { group.shared_runners_enabled }
end end
end end
context 'top level group that has shared Runners enabled' do context 'top level group that has shared Runners enabled' do
let_it_be(:group) { create(:group, shared_runners_enabled: true) } let_it_be(:group) { create(:group, shared_runners_enabled: true) }
let_it_be(:sub_group) { create(:group, :shared_runners_disabled, parent: group) } let_it_be(:sub_group) { create(:group, shared_runners_enabled: true, parent: group) }
let_it_be(:project) { create(:project, shared_runners_enabled: false, group: sub_group) }
it 'raises error and does not change config' do
expect { subject }
.to raise_error(described_class::UpdateSharedRunnersError, 'Shared Runners enabled')
.and not_change { group.reload.allow_descendants_override_disabled_shared_runners }
.and not_change { group.reload.shared_runners_enabled }
.and not_change { sub_group.reload.allow_descendants_override_disabled_shared_runners }
.and not_change { sub_group.reload.shared_runners_enabled }
.and not_change { project.reload.shared_runners_enabled }
end
end
end
describe '#disallow_descendants_override_disabled_shared_runners!' do
subject { group.disallow_descendants_override_disabled_shared_runners! }
context 'top level group' do
let_it_be(:group) { create(:group, :shared_runners_disabled, :allow_descendants_override_disabled_shared_runners ) }
let_it_be(:sub_group) { create(:group, :shared_runners_disabled, :allow_descendants_override_disabled_shared_runners, parent: group) }
let_it_be(:project) { create(:project, shared_runners_enabled: true, group: sub_group) } let_it_be(:project) { create(:project, shared_runners_enabled: true, group: sub_group) }
it 'disables allow project to override for descendants and disables project shared Runners' do it 'enables allow descendants to override & disables shared runners everywhere' do
expect { subject } expect { subject_and_reload(group, sub_group, project) }
.to not_change { group.reload.shared_runners_enabled } .to change { group.shared_runners_enabled }.from(true).to(false)
.and change { group.reload.allow_descendants_override_disabled_shared_runners }.from(true).to(false) .and change { group.allow_descendants_override_disabled_shared_runners }.from(false).to(true)
.and not_change { sub_group.reload.shared_runners_enabled } .and change { sub_group.shared_runners_enabled }.from(true).to(false)
.and change { sub_group.reload.allow_descendants_override_disabled_shared_runners }.from(true).to(false) .and change { project.shared_runners_enabled }.from(true).to(false)
.and change { project.reload.shared_runners_enabled }.from(true).to(false)
end end
end end
context 'top level group that has shared Runners enabled' do
let_it_be(:group) { create(:group, shared_runners_enabled: true) }
let_it_be(:sub_group) { create(:group, :shared_runners_disabled, parent: group) }
let_it_be(:project) { create(:project, shared_runners_enabled: false, group: sub_group) }
it 'results error and does not change config' do
expect { subject }
.to raise_error(described_class::UpdateSharedRunnersError, 'Shared Runners enabled')
.and not_change { group.reload.allow_descendants_override_disabled_shared_runners }
.and not_change { group.reload.shared_runners_enabled }
.and not_change { sub_group.reload.allow_descendants_override_disabled_shared_runners }
.and not_change { sub_group.reload.shared_runners_enabled }
.and not_change { project.reload.shared_runners_enabled }
end
end end
end end
......
...@@ -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
......
...@@ -13,17 +13,14 @@ RSpec.describe Groups::UpdateSharedRunnersService do ...@@ -13,17 +13,14 @@ RSpec.describe Groups::UpdateSharedRunnersService do
context 'when current_user is not the group owner' do context 'when current_user is not the group owner' do
let_it_be(:group) { create(:group) } let_it_be(:group) { create(:group) }
let(:params) { { shared_runners_enabled: '0' } } let(:params) { { shared_runners_setting: 'enabled' } }
before do before do
group.add_maintainer(user) group.add_maintainer(user)
end end
it 'results error and does not call any method' do it 'results error and does not call any method' do
expect(group).not_to receive(:enable_shared_runners!) expect(group).not_to receive(:update_shared_runners_setting!)
expect(group).not_to receive(:disable_shared_runners!)
expect(group).not_to receive(:allow_descendants_override_disabled_shared_runners!)
expect(group).not_to receive(:disallow_descendants_override_disabled_shared_runners!)
expect(subject[:status]).to eq(:error) expect(subject[:status]).to eq(:error)
expect(subject[:message]).to eq('Operation not allowed') expect(subject[:message]).to eq('Operation not allowed')
...@@ -37,12 +34,7 @@ RSpec.describe Groups::UpdateSharedRunnersService do ...@@ -37,12 +34,7 @@ RSpec.describe Groups::UpdateSharedRunnersService do
end end
context 'enable shared Runners' do context 'enable shared Runners' do
where(:desired_params) do let(:params) { { shared_runners_setting: 'enabled' } }
['1', true]
end
with_them do
let(:params) { { shared_runners_enabled: desired_params } }
context 'group that its ancestors have shared runners disabled' do context 'group that its ancestors have shared runners disabled' do
let_it_be(:parent) { create(:group, :shared_runners_disabled) } let_it_be(:parent) { create(:group, :shared_runners_disabled) }
...@@ -50,7 +42,7 @@ RSpec.describe Groups::UpdateSharedRunnersService do ...@@ -50,7 +42,7 @@ RSpec.describe Groups::UpdateSharedRunnersService do
it 'results error' do it 'results error' do
expect(subject[:status]).to eq(:error) expect(subject[:status]).to eq(:error)
expect(subject[:message]).to eq('Shared Runners disabled for the parent group') expect(subject[:message]).to eq('Validation failed: Shared runners enabled cannot be enabled because parent group has shared Runners disabled')
end end
end end
...@@ -58,54 +50,32 @@ RSpec.describe Groups::UpdateSharedRunnersService do ...@@ -58,54 +50,32 @@ RSpec.describe Groups::UpdateSharedRunnersService do
let_it_be(:group) { create(:group, :shared_runners_disabled) } let_it_be(:group) { create(:group, :shared_runners_disabled) }
it 'receives correct method and succeeds' do it 'receives correct method and succeeds' do
expect(group).to receive(:enable_shared_runners!) expect(group).to receive(:update_shared_runners_setting!).with('enabled')
expect(group).not_to receive(:disable_shared_runners!)
expect(group).not_to receive(:allow_descendants_override_disabled_shared_runners!)
expect(group).not_to receive(:disallow_descendants_override_disabled_shared_runners!)
expect(subject[:status]).to eq(:success) expect(subject[:status]).to eq(:success)
end end
end end
end end
end
context 'disable shared Runners' do context 'disable shared Runners' do
let_it_be(:group) { create(:group) } let_it_be(:group) { create(:group) }
let(:params) { { shared_runners_setting: 'disabled_and_unoverridable' } }
where(:desired_params) do
['0', false]
end
with_them do
let(:params) { { shared_runners_enabled: desired_params } }
it 'receives correct method and succeeds' do it 'receives correct method and succeeds' do
expect(group).to receive(:disable_shared_runners!) expect(group).to receive(:update_shared_runners_setting!).with('disabled_and_unoverridable')
expect(group).not_to receive(:enable_shared_runners!)
expect(group).not_to receive(:allow_descendants_override_disabled_shared_runners!)
expect(group).not_to receive(:disallow_descendants_override_disabled_shared_runners!)
expect(subject[:status]).to eq(:success) expect(subject[:status]).to eq(:success)
end end
end end
end
context 'allow descendants to override' do context 'allow descendants to override' do
where(:desired_params) do let(:params) { { shared_runners_setting: 'disabled_with_override' } }
['1', true]
end
with_them do
let(:params) { { allow_descendants_override_disabled_shared_runners: desired_params } }
context 'top level group' do context 'top level group' do
let_it_be(:group) { create(:group, :shared_runners_disabled) } let_it_be(:group) { create(:group, :shared_runners_disabled) }
it 'receives correct method and succeeds' do it 'receives correct method and succeeds' do
expect(group).to receive(:allow_descendants_override_disabled_shared_runners!) expect(group).to receive(:update_shared_runners_setting!).with('disabled_with_override')
expect(group).not_to receive(:disallow_descendants_override_disabled_shared_runners!)
expect(group).not_to receive(:enable_shared_runners!)
expect(group).not_to receive(:disable_shared_runners!)
expect(subject[:status]).to eq(:success) expect(subject[:status]).to eq(:success)
end end
...@@ -117,111 +87,7 @@ RSpec.describe Groups::UpdateSharedRunnersService do ...@@ -117,111 +87,7 @@ RSpec.describe Groups::UpdateSharedRunnersService do
it 'results error' do it 'results error' do
expect(subject[:status]).to eq(:error) expect(subject[:status]).to eq(:error)
expect(subject[:message]).to eq('Group level shared Runners not allowed') expect(subject[:message]).to eq('Validation failed: Allow descendants override disabled shared runners cannot be enabled because parent group does not allow it')
end
end
end
end
context 'disallow descendants to override' do
where(:desired_params) do
['0', false]
end
with_them do
let(:params) { { allow_descendants_override_disabled_shared_runners: desired_params } }
context 'top level group' do
let_it_be(:group) { create(:group, :shared_runners_disabled, :allow_descendants_override_disabled_shared_runners ) }
it 'receives correct method and succeeds' do
expect(group).to receive(:disallow_descendants_override_disabled_shared_runners!)
expect(group).not_to receive(:allow_descendants_override_disabled_shared_runners!)
expect(group).not_to receive(:enable_shared_runners!)
expect(group).not_to receive(:disable_shared_runners!)
expect(subject[:status]).to eq(:success)
end
end
context 'top level group that has shared Runners enabled' do
let_it_be(:group) { create(:group, shared_runners_enabled: true) }
it 'results error' do
expect(subject[:status]).to eq(:error)
expect(subject[:message]).to eq('Shared Runners enabled')
end
end
end
end
context 'both params are present' do
context 'shared_runners_enabled: 1 and allow_descendants_override_disabled_shared_runners' do
let_it_be(:group) { create(:group, :shared_runners_disabled) }
let_it_be(:sub_group) { create(:group, :shared_runners_disabled, parent: group) }
let_it_be(:project) { create(:project, shared_runners_enabled: false, group: sub_group) }
where(:allow_descendants_override) do
['1', true, '0', false]
end
with_them do
let(:params) { { shared_runners_enabled: '1', allow_descendants_override_disabled_shared_runners: allow_descendants_override } }
it 'results in an error because shared Runners are enabled' do
expect { subject }
.to not_change { group.reload.shared_runners_enabled }
.and not_change { sub_group.reload.shared_runners_enabled }
.and not_change { project.reload.shared_runners_enabled }
.and not_change { group.reload.allow_descendants_override_disabled_shared_runners }
.and not_change { sub_group.reload.allow_descendants_override_disabled_shared_runners }
expect(subject[:status]).to eq(:error)
expect(subject[:message]).to eq('Cannot set shared_runners_enabled to true and allow_descendants_override_disabled_shared_runners')
end
end
end
context 'shared_runners_enabled: 0 and allow_descendants_override_disabled_shared_runners: 0' do
let_it_be(:group) { create(:group, :allow_descendants_override_disabled_shared_runners) }
let_it_be(:sub_group) { create(:group, :shared_runners_disabled, :allow_descendants_override_disabled_shared_runners, parent: group) }
let_it_be(:sub_group_2) { create(:group, parent: group) }
let_it_be(:project) { create(:project, group: group, shared_runners_enabled: true) }
let_it_be(:project_2) { create(:project, group: sub_group_2, shared_runners_enabled: true) }
let(:params) { { shared_runners_enabled: '0', allow_descendants_override_disabled_shared_runners: '0' } }
it 'disables shared Runners and disable allow_descendants_override_disabled_shared_runners' do
expect { subject }
.to change { group.reload.shared_runners_enabled }.from(true).to(false)
.and change { group.reload.allow_descendants_override_disabled_shared_runners }.from(true).to(false)
.and not_change { sub_group.reload.shared_runners_enabled }
.and change { sub_group.reload.allow_descendants_override_disabled_shared_runners }.from(true).to(false)
.and change { sub_group_2.reload.shared_runners_enabled }.from(true).to(false)
.and not_change { sub_group_2.reload.allow_descendants_override_disabled_shared_runners }
.and change { project.reload.shared_runners_enabled }.from(true).to(false)
.and change { project_2.reload.shared_runners_enabled }.from(true).to(false)
end
end
context 'shared_runners_enabled: 0 and allow_descendants_override_disabled_shared_runners: 1' do
let_it_be(:group) { create(:group) }
let_it_be(:sub_group) { create(:group, :shared_runners_disabled, parent: group) }
let_it_be(:sub_group_2) { create(:group, parent: group) }
let_it_be(:project) { create(:project, group: group, shared_runners_enabled: true) }
let_it_be(:project_2) { create(:project, group: sub_group_2, shared_runners_enabled: true) }
let(:params) { { shared_runners_enabled: '0', allow_descendants_override_disabled_shared_runners: '1' } }
it 'disables shared Runners and enable allow_descendants_override_disabled_shared_runners only for itself' do
expect { subject }
.to change { group.reload.shared_runners_enabled }.from(true).to(false)
.and change { group.reload.allow_descendants_override_disabled_shared_runners }.from(false).to(true)
.and not_change { sub_group.reload.shared_runners_enabled }
.and not_change { sub_group.reload.allow_descendants_override_disabled_shared_runners }
.and change { sub_group_2.reload.shared_runners_enabled }.from(true).to(false)
.and not_change { sub_group_2.reload.allow_descendants_override_disabled_shared_runners }
.and change { project.reload.shared_runners_enabled }.from(true).to(false)
.and change { project_2.reload.shared_runners_enabled }.from(true).to(false)
end end
end end
end end
......
...@@ -782,4 +782,100 @@ RSpec.describe Projects::CreateService, '#execute' do ...@@ -782,4 +782,100 @@ 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
using RSpec::Parameterized::TableSyntax
let_it_be(:user) { create :user }
context 'when parent group is present' do
let_it_be(:group) do
create(:group) do |group|
group.add_owner(user)
end
end
before do
allow_next_found_instance_of(Group) do |group|
allow(group).to receive(:shared_runners_setting).and_return(shared_runners_setting)
end
user.refresh_authorized_projects # Ensure cache is warm
end
context 'default value based on parent group setting' do
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
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
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
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
where(:shared_runners_setting, :desired_config_for_new_project) do
'disabled_and_unoverridable' | true
end
with_them do
it 'does not create project' do
params = opts.merge(namespace_id: group.id, shared_runners_enabled: desired_config_for_new_project)
project = create_project(user, params)
expect(project.persisted?).to eq(false)
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
end
context 'parent group is not present' do
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