diff --git a/app/controllers/groups/settings/ci_cd_controller.rb b/app/controllers/groups/settings/ci_cd_controller.rb
index 715555fdd8c5568787e11eb7b272b670e4867869..9e00fa69f94ee04c660982aa3576c06e42e6b57f 100644
--- a/app/controllers/groups/settings/ci_cd_controller.rb
+++ b/app/controllers/groups/settings/ci_cd_controller.rb
@@ -27,9 +27,9 @@ def show
def update
if update_group_service.execute
- flash[:notice] = s_('GroupSettings|Pipeline settings was updated for the group')
+ flash[:notice] = s_('GroupSettings|Group CI/CD settings were successfully updated.')
else
- flash[:alert] = format(s_("GroupSettings|There was a problem updating the pipeline settings: %{error_messages}."), error_messages: group.errors.full_messages)
+ flash[:alert] = format(s_("GroupSettings|There was a problem updating the group CI/CD settings: %{error_messages}."), error_messages: group.errors.full_messages)
end
redirect_to group_settings_ci_cd_path
@@ -59,11 +59,13 @@ def define_ci_variables
end
def authorize_admin_group!
- return render_404 unless can?(current_user, :admin_group, group)
+ render_404 unless can?(current_user, :admin_group, group)
end
def authorize_update_max_artifacts_size!
- return render_404 unless can?(current_user, :update_max_artifacts_size, group)
+ if update_group_params.has_key?(:max_artifacts_size) && !can?(current_user, :update_max_artifacts_size, group)
+ render_404
+ end
end
def auto_devops_params
diff --git a/app/services/groups/update_service.rb b/app/services/groups/update_service.rb
index 2aa5f88aadf16f491ddcd114521b8c7b8ab3a1f8..4a79a9af9d2392732a50bf6c99d076e8be6dd3ef 100644
--- a/app/services/groups/update_service.rb
+++ b/app/services/groups/update_service.rb
@@ -138,6 +138,7 @@ def reject_parent_id!
# overridden in EE
def remove_unallowed_params
params.delete(:emails_enabled) unless can?(current_user, :set_emails_disabled, group)
+ params.delete(:max_artifacts_size) unless can?(current_user, :update_max_artifacts_size, group)
unless can?(current_user, :update_default_branch_protection, group)
params.delete(:default_branch_protection)
@@ -147,6 +148,7 @@ def remove_unallowed_params
unless can?(current_user, :admin_namespace, group)
params.delete(:math_rendering_limits_enabled)
params.delete(:lock_math_rendering_limits_enabled)
+ params.delete(:allow_runner_registration_token)
end
end
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index cb280e5ce8a85149ed7bc2437eff35c1278256d9..9b2db03f7cd5fd616a017d2a6bbc150d9e648ec9 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -25001,6 +25001,9 @@ msgstr ""
msgid "GroupSettings|Git abuse rate limit"
msgstr ""
+msgid "GroupSettings|Group CI/CD settings were successfully updated."
+msgstr ""
+
msgid "GroupSettings|Group members are not notified if the group is mentioned."
msgstr ""
@@ -25037,9 +25040,6 @@ msgstr ""
msgid "GroupSettings|Organizations and contacts can be created and associated with issues."
msgstr ""
-msgid "GroupSettings|Pipeline settings was updated for the group"
-msgstr ""
-
msgid "GroupSettings|Please choose a group URL with no special characters or spaces."
msgstr ""
@@ -25094,7 +25094,7 @@ msgstr ""
msgid "GroupSettings|There was a problem updating Auto DevOps pipeline: %{error_messages}."
msgstr ""
-msgid "GroupSettings|There was a problem updating the pipeline settings: %{error_messages}."
+msgid "GroupSettings|There was a problem updating the group CI/CD settings: %{error_messages}."
msgstr ""
msgid "GroupSettings|These features are being developed and might be unstable."
diff --git a/spec/controllers/groups/settings/ci_cd_controller_spec.rb b/spec/controllers/groups/settings/ci_cd_controller_spec.rb
index 9aa97c37add7495d0ab0a8d0fb8db1ef2c78c589..1ce61025dfc88730226582216da20ac0fbf4dcfe 100644
--- a/spec/controllers/groups/settings/ci_cd_controller_spec.rb
+++ b/spec/controllers/groups/settings/ci_cd_controller_spec.rb
@@ -2,7 +2,7 @@
require 'spec_helper'
-RSpec.describe Groups::Settings::CiCdController do
+RSpec.describe Groups::Settings::CiCdController, feature_category: :continuous_integration do
include ExternalAuthorizationServiceHelpers
let_it_be(:group) { create(:group) }
@@ -126,33 +126,84 @@
end
describe 'PATCH #update' do
- subject do
- patch :update, params: {
- group_id: group,
- group: { max_artifacts_size: 10 }
- }
+ subject(:response) { perform_request }
+
+ def perform_request
+ patch :update, params: params
end
- context 'when user is not an admin' do
- before do
+ context 'when user is a group owner' do
+ before_all do
group.add_owner(user)
end
- it { is_expected.to have_gitlab_http_status(:not_found) }
+ context 'when updating max_artifacts_size' do
+ let(:params) { { group_id: group, group: { max_artifacts_size: 10 } } }
+
+ it 'cannot update max_artifacts_size' do
+ expect { perform_request }.not_to change { group.reload.max_artifacts_size }
+
+ expect(response).to have_gitlab_http_status(:not_found)
+ end
+ end
+
+ context 'when updating allow_runner_registration_token' do
+ let(:params) { { group_id: group, group: { allow_runner_registration_token: false } } }
+
+ it 'can update allow_runner_registration_token' do
+ expect { perform_request }.to change { group.reload.allow_runner_registration_token? }.from(true).to(false)
+ end
+
+ context 'when user is not a group owner' do
+ before_all do
+ group.add_maintainer(user)
+ end
+
+ it 'cannot update allow_runner_registration_token?' do
+ expect { perform_request }.not_to change { group.reload.allow_runner_registration_token? }
+
+ expect(response).to have_gitlab_http_status(:not_found)
+ end
+ end
+ end
end
- context 'when user is an admin' do
- let(:user) { create(:admin) }
+ context 'when user is a group maintainer' do
+ let_it_be(:user) { create(:user).tap { |user| group.add_maintainer(user) } }
- before do
- group.add_owner(user)
+ context 'when updating allow_runner_registration_token' do
+ let(:params) { { group_id: group, group: { allow_runner_registration_token: false } } }
+
+ it 'cannot update allow_runner_registration_token?' do
+ expect { perform_request }.not_to change { group.reload.allow_runner_registration_token? }
+
+ expect(response).to have_gitlab_http_status(:not_found)
+ end
end
+ end
+
+ context 'when user is an admin' do
+ let_it_be(:user) { create(:admin).tap { |user| group.add_owner(user) } }
context 'when admin mode is disabled' do
- it { is_expected.to have_gitlab_http_status(:not_found) }
+ context 'when updating max_artifacts_size' do
+ let(:params) { { group_id: group, group: { max_artifacts_size: 10 } } }
+
+ it { is_expected.to have_gitlab_http_status(:not_found) }
+ end
+
+ context 'when updating allow_runner_registration_token' do
+ let(:params) { { group_id: group, group: { allow_runner_registration_token: false } } }
+
+ it 'can update allow_runner_registration_token' do
+ expect { perform_request }.to change { group.reload.allow_runner_registration_token? }.from(true).to(false)
+ end
+ end
end
context 'when admin mode is enabled', :enable_admin_mode do
+ let(:params) { { group_id: group, group: { max_artifacts_size: 10 } } }
+
it { is_expected.to redirect_to(group_settings_ci_cd_path) }
context 'when service execution went wrong' do
@@ -164,21 +215,21 @@
allow_any_instance_of(Group).to receive_message_chain(:errors, :full_messages)
.and_return(['Error 1'])
- subject
+ response
end
it 'returns a flash alert' do
expect(controller).to set_flash[:alert]
- .to eq("There was a problem updating the pipeline settings: [\"Error 1\"].")
+ .to eq("There was a problem updating the group CI/CD settings: [\"Error 1\"].")
end
end
context 'when service execution was successful' do
it 'returns a flash notice' do
- subject
+ response
expect(controller).to set_flash[:notice]
- .to eq('Pipeline settings was updated for the group')
+ .to eq('Group CI/CD settings were successfully updated.')
end
end
end
diff --git a/spec/services/groups/update_service_spec.rb b/spec/services/groups/update_service_spec.rb
index e1dd574dcf12a045ce99053a2931af4cf1a23a93..43774b619c1a2eb7e7ac6d49fd4a975198dd6d1a 100644
--- a/spec/services/groups/update_service_spec.rb
+++ b/spec/services/groups/update_service_spec.rb
@@ -440,6 +440,56 @@
end
end
+ context 'when updating #max_artifacts_size' do
+ let(:params) { { max_artifacts_size: 10 } }
+
+ let(:service) do
+ described_class.new(internal_group, user, **params)
+ end
+
+ before do
+ internal_group.add_owner(user)
+ end
+
+ context 'for users who have the ability to update max_artifacts_size', :enable_admin_mode do
+ let(:user) { create(:admin) }
+
+ it 'updates max_artifacts_size' do
+ expect { service.execute }.to change { internal_group.max_artifacts_size }.from(nil).to(10)
+ end
+ end
+
+ context 'for users who do not have the ability to update max_artifacts_size' do
+ it 'does not update max_artifacts_size' do
+ expect { service.execute }.not_to change { internal_group.max_artifacts_size }
+ end
+ end
+ end
+
+ context 'when updating #allow_runner_registration_token' do
+ let(:params) { { allow_runner_registration_token: false } }
+
+ let(:service) do
+ described_class.new(internal_group, user, **params)
+ end
+
+ context 'for users who have the ability to update allow_runner_registration_token' do
+ before do
+ internal_group.add_owner(user)
+ end
+
+ it 'updates allow_runner_registration_token' do
+ expect { service.execute }.to change { internal_group.allow_runner_registration_token }.from(true).to(false)
+ end
+ end
+
+ context 'for users who do not have the ability to update allow_runner_registration_token' do
+ it 'does not update allow_runner_registration_token' do
+ expect { service.execute }.not_to change { internal_group.allow_runner_registration_token }
+ end
+ end
+ end
+
context 'when updating #math_rendering_limits_enabled' do
let(:service) { described_class.new(internal_group, user, math_rendering_limits_enabled: false) }