diff --git a/app/graphql/types/namespace/shared_runners_setting_enum.rb b/app/graphql/types/namespace/shared_runners_setting_enum.rb index 4773e414aebb7aec675a0e1d1902136882bb0a86..fd067c9d803cd7fb952ea391fbeb120821e6a970 100644 --- a/app/graphql/types/namespace/shared_runners_setting_enum.rb +++ b/app/graphql/types/namespace/shared_runners_setting_enum.rb @@ -4,10 +4,21 @@ module Types class Namespace::SharedRunnersSettingEnum < BaseEnum graphql_name 'SharedRunnersSetting' - ::Namespace::SHARED_RUNNERS_SETTINGS.each do |type| + DEPRECATED_SETTINGS = [::Namespace::SR_DISABLED_WITH_OVERRIDE].freeze + + ::Namespace::SHARED_RUNNERS_SETTINGS.excluding(DEPRECATED_SETTINGS).each do |type| value type.upcase, description: "Sharing of runners is #{type.tr('_', ' ')}.", value: type end + + value ::Namespace::SR_DISABLED_WITH_OVERRIDE.upcase, + description: "Sharing of runners is disabled and overridable.", + value: ::Namespace::SR_DISABLED_WITH_OVERRIDE, + deprecated: { + reason: :renamed, + replacement: ::Namespace::SR_DISABLED_AND_OVERRIDABLE, + milestone: "17.0" + } end end diff --git a/app/helpers/ci/runners_helper.rb b/app/helpers/ci/runners_helper.rb index 8df30ee1f0de1e443188664b5e86489ce6aa6cd1..ac36c867bafce4bb51694c0b59506c7b813345b3 100644 --- a/app/helpers/ci/runners_helper.rb +++ b/app/helpers/ci/runners_helper.rb @@ -78,7 +78,7 @@ def group_shared_runners_settings_data(group) parent_shared_runners_setting: group.parent&.shared_runners_setting, runner_enabled_value: Namespace::SR_ENABLED, runner_disabled_value: Namespace::SR_DISABLED_AND_UNOVERRIDABLE, - runner_allow_override_value: Namespace::SR_DISABLED_WITH_OVERRIDE + runner_allow_override_value: Namespace::SR_DISABLED_AND_OVERRIDABLE } end diff --git a/app/models/group.rb b/app/models/group.rb index 0cdd7dd8596333a500db695561058834e0d5ee3a..96911e1024ae47e4a21880701ca5c1935c721e50 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -815,7 +815,7 @@ def update_shared_runners_setting!(state) case state when SR_DISABLED_AND_UNOVERRIDABLE then disable_shared_runners! # also disallows override - when SR_DISABLED_WITH_OVERRIDE then disable_shared_runners_and_allow_override! + when SR_DISABLED_WITH_OVERRIDE, SR_DISABLED_AND_OVERRIDABLE then disable_shared_runners_and_allow_override! when SR_ENABLED then enable_shared_runners! # set both to true end end @@ -1055,7 +1055,7 @@ def disable_shared_runners! end def disable_shared_runners_and_allow_override! - # enabled -> disabled_with_override + # enabled -> disabled_and_overridable if shared_runners_enabled? update!( shared_runners_enabled: false, @@ -1068,7 +1068,7 @@ def disable_shared_runners_and_allow_override! all_projects.update_all(shared_runners_enabled: false) - # disabled_and_unoverridable -> disabled_with_override + # disabled_and_unoverridable -> disabled_and_overridable else update!(allow_descendants_override_disabled_shared_runners: true) end diff --git a/app/models/namespace.rb b/app/models/namespace.rb index d7d539566565c41019a01ce65e9d07b5c63fc72a..5dcda413ae5480ccd679c283f70f4f7910442b53 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -33,9 +33,11 @@ class Namespace < ApplicationRecord NUMBER_OF_ANCESTORS_ALLOWED = 20 SR_DISABLED_AND_UNOVERRIDABLE = 'disabled_and_unoverridable' + # DISABLED_WITH_OVERRIDE is deprecated in favour of DISABLED_AND_OVERRIDABLE. SR_DISABLED_WITH_OVERRIDE = 'disabled_with_override' + SR_DISABLED_AND_OVERRIDABLE = 'disabled_and_overridable' SR_ENABLED = 'enabled' - SHARED_RUNNERS_SETTINGS = [SR_DISABLED_AND_UNOVERRIDABLE, SR_DISABLED_WITH_OVERRIDE, SR_ENABLED].freeze + SHARED_RUNNERS_SETTINGS = [SR_DISABLED_AND_UNOVERRIDABLE, SR_DISABLED_WITH_OVERRIDE, SR_DISABLED_AND_OVERRIDABLE, SR_ENABLED].freeze URL_MAX_LENGTH = 255 PATH_TRAILING_VIOLATIONS = %w[.git .atom .].freeze @@ -556,7 +558,7 @@ def shared_runners_setting if shared_runners_enabled SR_ENABLED elsif allow_descendants_override_disabled_shared_runners - SR_DISABLED_WITH_OVERRIDE + SR_DISABLED_AND_OVERRIDABLE else SR_DISABLED_AND_UNOVERRIDABLE end @@ -566,10 +568,10 @@ def shared_runners_setting_higher_than?(other_setting) case other_setting when SR_ENABLED false - when SR_DISABLED_WITH_OVERRIDE + when SR_DISABLED_WITH_OVERRIDE, SR_DISABLED_AND_OVERRIDABLE shared_runners_setting == SR_ENABLED when SR_DISABLED_AND_UNOVERRIDABLE - shared_runners_setting == SR_ENABLED || shared_runners_setting == SR_DISABLED_WITH_OVERRIDE + shared_runners_setting == SR_ENABLED || shared_runners_setting == SR_DISABLED_AND_OVERRIDABLE || shared_runners_setting == SR_DISABLED_WITH_OVERRIDE else raise ArgumentError end diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index f71aced27ef065f376ae36a98aca5b6ddc51e304..b95f18b417f1adcfe03f475ee76663dc274de0f9 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -22777,8 +22777,9 @@ How to format SHA strings. | Value | Description | | ----- | ----------- | +| <a id="sharedrunnerssettingdisabled_and_overridable"></a>`DISABLED_AND_OVERRIDABLE` | Sharing of runners is disabled and overridable. | | <a id="sharedrunnerssettingdisabled_and_unoverridable"></a>`DISABLED_AND_UNOVERRIDABLE` | Sharing of runners is disabled and unoverridable. | -| <a id="sharedrunnerssettingdisabled_with_override"></a>`DISABLED_WITH_OVERRIDE` | Sharing of runners is disabled with override. | +| <a id="sharedrunnerssettingdisabled_with_override"></a>`DISABLED_WITH_OVERRIDE` **{warning-solid}** | **Deprecated** in 17.0. This was renamed. Use: `disabled_and_overridable`. | | <a id="sharedrunnerssettingenabled"></a>`ENABLED` | Sharing of runners is enabled. | ### `SnippetBlobActionEnum` diff --git a/doc/api/groups.md b/doc/api/groups.md index d017876b9c2e0588bf7c3944eee88ccdbbdcd784..d547515a550e1031f461eef9cf0ff86fa223b7ad 100644 --- a/doc/api/groups.md +++ b/doc/api/groups.md @@ -1099,8 +1099,9 @@ The `shared_runners_setting` attribute determines whether shared runners are ena | Value | Description | |-------|-------------------------------------------------------------------------------------------------------------| | `enabled` | Enables shared runners for all projects and subgroups in this group. | -| `disabled_with_override` | Disables shared runners for all projects and subgroups in this group, but allows subgroups to override this setting. | +| `disabled_and_overridable` | Disables shared runners for all projects and subgroups in this group, but allows subgroups to override this setting. | | `disabled_and_unoverridable` | Disables shared runners for all projects and subgroups in this group, and prevents subgroups from overriding this setting. | +| `disabled_with_override` | (Deprecated. Use `disabled_and_overridable`) Disables shared runners for all projects and subgroups in this group, but allows subgroups to override this setting. | ### Upload a group avatar diff --git a/spec/factories/groups.rb b/spec/factories/groups.rb index f4d47b9ff8cc450975a5fcab0de3d84da0b10b13..5b4839df2d31cd7093acf7b0a3370867870e3719 100644 --- a/spec/factories/groups.rb +++ b/spec/factories/groups.rb @@ -74,7 +74,7 @@ allow_descendants_override_disabled_shared_runners { false } end - trait :disabled_with_override do + trait :disabled_and_overridable do shared_runners_disabled allow_descendants_override_disabled_shared_runners end diff --git a/spec/frontend/group_settings/components/shared_runners_form_spec.js b/spec/frontend/group_settings/components/shared_runners_form_spec.js index 5282c0ed8396edb6c4583156ae620b0e143eb022..85475c749b0170024511bf7a3d65e89621c1c6e5 100644 --- a/spec/frontend/group_settings/components/shared_runners_form_spec.js +++ b/spec/frontend/group_settings/components/shared_runners_form_spec.js @@ -10,7 +10,7 @@ jest.mock('~/api/groups_api'); const GROUP_ID = '99'; const RUNNER_ENABLED_VALUE = 'enabled'; const RUNNER_DISABLED_VALUE = 'disabled_and_unoverridable'; -const RUNNER_ALLOW_OVERRIDE_VALUE = 'disabled_with_override'; +const RUNNER_ALLOW_OVERRIDE_VALUE = 'disabled_and_overridable'; describe('group_settings/components/shared_runners_form', () => { let wrapper; diff --git a/spec/helpers/ci/runners_helper_spec.rb b/spec/helpers/ci/runners_helper_spec.rb index 1b1edde8faf0e1d469ac785d58a3c47d3ec0c1fe..6d14abd6574be2b607bcdb076bb1d3eea3bf76d7 100644 --- a/spec/helpers/ci/runners_helper_spec.rb +++ b/spec/helpers/ci/runners_helper_spec.rb @@ -103,7 +103,7 @@ { runner_enabled_value: Namespace::SR_ENABLED, runner_disabled_value: Namespace::SR_DISABLED_AND_UNOVERRIDABLE, - runner_allow_override_value: Namespace::SR_DISABLED_WITH_OVERRIDE + runner_allow_override_value: Namespace::SR_DISABLED_AND_OVERRIDABLE } end @@ -197,7 +197,7 @@ where(:shared_runners_setting, :is_disabled_and_unoverridable) do :shared_runners_enabled | "false" - :disabled_with_override | "false" + :disabled_and_overridable | "false" :disabled_and_unoverridable | "true" end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index dfba0470d35128d762ba1429a3644df7856e3575..f243f639acf375915f228ce25f5aeb6074d8e9ca 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -2648,7 +2648,81 @@ def define_cache_expectations(cache_key) end end - context 'disabled_with_override' do + context 'disabled_and_overridable' do + subject { group.update_shared_runners_setting!(Namespace::SR_DISABLED_AND_OVERRIDABLE) } + + context 'top level group' 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) } + + it 'enables allow descendants to override only for itself' do + expect { subject_and_reload(group, sub_group, project) } + .to change { group.allow_descendants_override_disabled_shared_runners }.from(false).to(true) + .and not_change { group.shared_runners_enabled } + .and not_change { sub_group.allow_descendants_override_disabled_shared_runners } + .and not_change { sub_group.shared_runners_enabled } + .and not_change { project.shared_runners_enabled } + end + end + + context 'group that its ancestors have shared Runners disabled but allows to override' do + let_it_be(:parent) { create(:group, :shared_runners_disabled, :allow_descendants_override_disabled_shared_runners) } + let_it_be(:group) { create(:group, :shared_runners_disabled, parent: parent) } + let_it_be(:project) { create(:project, shared_runners_enabled: false, group: group) } + + it 'enables allow descendants to override' do + expect { subject_and_reload(parent, group, project) } + .to not_change { parent.allow_descendants_override_disabled_shared_runners } + .and not_change { parent.shared_runners_enabled } + .and change { group.allow_descendants_override_disabled_shared_runners }.from(false).to(true) + .and not_change { group.shared_runners_enabled } + .and not_change { project.shared_runners_enabled } + end + end + + context 'when parent does not allow' do + let_it_be(:parent, reload: true) { create(:group, :shared_runners_disabled, allow_descendants_override_disabled_shared_runners: false) } + let_it_be(:group, reload: true) { create(:group, :shared_runners_disabled, allow_descendants_override_disabled_shared_runners: false, parent: parent) } + + it 'raises exception' do + expect { subject } + .to raise_error(ActiveRecord::RecordInvalid, 'Validation failed: Allow descendants override disabled shared runners cannot be enabled because parent group does not allow it') + end + + it 'does not allow descendants to override' do + expect do + begin + subject + rescue StandardError + nil + end + + parent.reload + group.reload + end.to not_change { parent.allow_descendants_override_disabled_shared_runners } + .and not_change { parent.shared_runners_enabled } + .and not_change { group.allow_descendants_override_disabled_shared_runners } + .and not_change { group.shared_runners_enabled } + 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_enabled: true, parent: group) } + let_it_be(:project) { create(:project, shared_runners_enabled: true, group: sub_group) } + + it 'enables allow descendants to override & disables shared runners everywhere' do + expect { subject_and_reload(group, sub_group, project) } + .to change { group.shared_runners_enabled }.from(true).to(false) + .and change { group.allow_descendants_override_disabled_shared_runners }.from(false).to(true) + .and change { sub_group.shared_runners_enabled }.from(true).to(false) + .and change { project.shared_runners_enabled }.from(true).to(false) + end + end + end + + context 'disabled_with_override (deprecated)' do subject { group.update_shared_runners_setting!(Namespace::SR_DISABLED_WITH_OVERRIDE) } context 'top level group' do diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 80721e110498f5575696a34560c589c83edda5c7..706b73cb60718007c61c1ef8f619d070dac74d4e 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -2114,7 +2114,7 @@ def expect_project_directories_at(namespace_path, with_pages: true) where(:shared_runners_enabled, :allow_descendants_override_disabled_shared_runners, :shared_runners_setting) do true | true | Namespace::SR_ENABLED true | false | Namespace::SR_ENABLED - false | true | Namespace::SR_DISABLED_WITH_OVERRIDE + false | true | Namespace::SR_DISABLED_AND_OVERRIDABLE false | false | Namespace::SR_DISABLED_AND_UNOVERRIDABLE end @@ -2133,12 +2133,15 @@ def expect_project_directories_at(namespace_path, with_pages: true) where(:shared_runners_enabled, :allow_descendants_override_disabled_shared_runners, :other_setting, :result) do true | true | Namespace::SR_ENABLED | false true | true | Namespace::SR_DISABLED_WITH_OVERRIDE | true + true | true | Namespace::SR_DISABLED_AND_OVERRIDABLE | true true | true | Namespace::SR_DISABLED_AND_UNOVERRIDABLE | true false | true | Namespace::SR_ENABLED | false false | true | Namespace::SR_DISABLED_WITH_OVERRIDE | false + false | true | Namespace::SR_DISABLED_AND_OVERRIDABLE | false false | true | Namespace::SR_DISABLED_AND_UNOVERRIDABLE | true false | false | Namespace::SR_ENABLED | false false | false | Namespace::SR_DISABLED_WITH_OVERRIDE | false + false | false | Namespace::SR_DISABLED_AND_OVERRIDABLE | false false | false | Namespace::SR_DISABLED_AND_UNOVERRIDABLE | false end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index b9aa575224344f3a06b69fbfc00cf42769b1b006..0c513078783205ab3f8a2eee1480ea0f33adc638 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -6637,8 +6637,8 @@ def has_external_wiki where(:shared_runners_setting, :project_shared_runners_enabled, :valid_record) do :shared_runners_enabled | true | true :shared_runners_enabled | false | true - :disabled_with_override | true | true - :disabled_with_override | false | true + :disabled_and_overridable | true | true + :disabled_and_overridable | false | true :disabled_and_unoverridable | true | false :disabled_and_unoverridable | false | true end diff --git a/spec/requests/api/graphql/mutations/groups/update_spec.rb b/spec/requests/api/graphql/mutations/groups/update_spec.rb index ea3d42a446371feef5e22b59ce32df1d0b7ad3eb..a9acc593229cbdee5517787cac4b7b3e4f69f511 100644 --- a/spec/requests/api/graphql/mutations/groups/update_spec.rb +++ b/spec/requests/api/graphql/mutations/groups/update_spec.rb @@ -11,7 +11,7 @@ let(:variables) do { full_path: group.full_path, - shared_runners_setting: 'DISABLED_WITH_OVERRIDE' + shared_runners_setting: 'DISABLED_AND_OVERRIDABLE' } end @@ -52,6 +52,23 @@ expect(group.reload.shared_runners_setting).to eq(variables[:shared_runners_setting].downcase) end + context 'when using DISABLED_WITH_OVERRIDE (deprecated)' do + let(:variables) do + { + full_path: group.full_path, + shared_runners_setting: 'DISABLED_WITH_OVERRIDE' + } + end + + it 'updates shared runners settings with disabled_and_overridable' do + post_graphql_mutation(mutation, current_user: user) + + expect(response).to have_gitlab_http_status(:success) + expect(graphql_errors).to be_nil + expect(group.reload.shared_runners_setting).to eq('disabled_and_overridable') + end + end + context 'when bad arguments are provided' do let(:variables) { { full_path: '', shared_runners_setting: 'INVALID' } } diff --git a/spec/services/groups/transfer_service_spec.rb b/spec/services/groups/transfer_service_spec.rb index 3cf2c8753416601a7c8d5c01721510bc177cab77..75a1374a248f139eda76a9be2a8ec2d89d9e586a 100644 --- a/spec/services/groups/transfer_service_spec.rb +++ b/spec/services/groups/transfer_service_spec.rb @@ -364,7 +364,7 @@ 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: Namespace::SR_DISABLED_WITH_OVERRIDE }).and_call_original + expect(Groups::UpdateSharedRunnersService).to receive(:new).with(group, user, { shared_runners_setting: Namespace::SR_DISABLED_AND_OVERRIDABLE }).and_call_original transfer_service.execute(new_parent_group) end diff --git a/spec/services/groups/update_shared_runners_service_spec.rb b/spec/services/groups/update_shared_runners_service_spec.rb index 98eccedeaceaa3db2bcd63b7ba051f572201d63e..a29f73a71c2d6737163f74ae7fb3428ccc688f55 100644 --- a/spec/services/groups/update_shared_runners_service_spec.rb +++ b/spec/services/groups/update_shared_runners_service_spec.rb @@ -114,13 +114,13 @@ end context 'allow descendants to override' do - let(:params) { { shared_runners_setting: Namespace::SR_DISABLED_WITH_OVERRIDE } } + let(:params) { { shared_runners_setting: Namespace::SR_DISABLED_AND_OVERRIDABLE } } context 'top level group' do let_it_be(:group) { create(:group, :shared_runners_disabled) } it 'receives correct method and succeeds' do - expect(group).to receive(:update_shared_runners_setting!).with(Namespace::SR_DISABLED_WITH_OVERRIDE) + expect(group).to receive(:update_shared_runners_setting!).with(Namespace::SR_DISABLED_AND_OVERRIDABLE) expect(subject[:status]).to eq(:success) end @@ -135,6 +135,30 @@ 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 + + context 'when using DISABLED_WITH_OVERRIDE (deprecated)' do + let(:params) { { shared_runners_setting: Namespace::SR_DISABLED_WITH_OVERRIDE } } + + context 'top level group' do + let_it_be(:group) { create(:group, :shared_runners_disabled) } + + it 'receives correct method and succeeds' do + expect(group).to receive(:update_shared_runners_setting!).with(Namespace::SR_DISABLED_WITH_OVERRIDE) + + expect(subject[:status]).to eq(:success) + end + end + + context 'when parent does not allow' do + let_it_be(:parent) { create(:group, :shared_runners_disabled, allow_descendants_override_disabled_shared_runners: false) } + let_it_be(:group) { create(:group, :shared_runners_disabled, allow_descendants_override_disabled_shared_runners: false, parent: parent) } + + it 'results error' do + expect(subject[:status]).to eq(:error) + 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 end end diff --git a/spec/services/projects/create_service_spec.rb b/spec/services/projects/create_service_spec.rb index f42ab198a042686f6f64f60b9b60fbdff9c4e525..608e9e08f87ba64a1c7a6e7488967de7f5f51c5c 100644 --- a/spec/services/projects/create_service_spec.rb +++ b/spec/services/projects/create_service_spec.rb @@ -995,6 +995,7 @@ def create_project(user, opts) where(:shared_runners_setting, :desired_config_for_new_project, :expected_result_for_project) do Namespace::SR_ENABLED | nil | true Namespace::SR_DISABLED_WITH_OVERRIDE | nil | false + Namespace::SR_DISABLED_AND_OVERRIDABLE | nil | false Namespace::SR_DISABLED_AND_UNOVERRIDABLE | nil | false end @@ -1017,6 +1018,8 @@ def create_project(user, opts) Namespace::SR_ENABLED | false | false Namespace::SR_DISABLED_WITH_OVERRIDE | false | false Namespace::SR_DISABLED_WITH_OVERRIDE | true | true + Namespace::SR_DISABLED_AND_OVERRIDABLE | false | false + Namespace::SR_DISABLED_AND_OVERRIDABLE | true | true Namespace::SR_DISABLED_AND_UNOVERRIDABLE | false | false end diff --git a/spec/services/projects/transfer_service_spec.rb b/spec/services/projects/transfer_service_spec.rb index 4d75786a4c3e21c2aa393d5c4fe0ee723ddcbca4..5171836f91787c00c5f2bffdcf1b0335b944ca47 100644 --- a/spec/services/projects/transfer_service_spec.rb +++ b/spec/services/projects/transfer_service_spec.rb @@ -535,8 +535,8 @@ def attempt_project_transfer(&block) 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 | :disabled_and_overridable | true + false | :disabled_and_overridable | false true | :shared_runners_enabled | true false | :shared_runners_enabled | false end