From 2bbc69a6d2b519534e4f534618a9683130161139 Mon Sep 17 00:00:00 2001 From: Pedro Pombeiro Date: Fri, 30 Jul 2021 18:09:17 +0200 Subject: [PATCH 1/3] Add mutation to change namespace shared runners setting Changelog: added --- .../groups/update_shared_runners_setting.rb | 38 ++++++++++ app/graphql/types/mutation_type.rb | 1 + app/policies/group_policy.rb | 4 + doc/api/graphql/reference/index.md | 19 +++++ .../update_shared_runners_setting_spec.rb | 73 +++++++++++++++++++ 5 files changed, 135 insertions(+) create mode 100644 app/graphql/mutations/groups/update_shared_runners_setting.rb create mode 100644 spec/graphql/mutations/groups/update_shared_runners_setting_spec.rb diff --git a/app/graphql/mutations/groups/update_shared_runners_setting.rb b/app/graphql/mutations/groups/update_shared_runners_setting.rb new file mode 100644 index 00000000000000..21a1d65bd42000 --- /dev/null +++ b/app/graphql/mutations/groups/update_shared_runners_setting.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +module Mutations + module Groups + class UpdateSharedRunnersSetting < Mutations::BaseMutation + include Mutations::ResolvesGroup + + graphql_name 'GroupSharedRunnersSettingUpdate' + + authorize :update_group_shared_runners_setting + + argument :full_path, GraphQL::Types::ID, + required: true, + description: 'Full path of the group that will be updated.' + argument :shared_runners_setting, Types::Namespace::SharedRunnersSettingEnum, + required: true, + description: copy_field_description(Types::GroupType, :shared_runners_setting) + + def resolve(full_path:, **args) + group = authorized_find!(full_path: full_path) + + result = ::Groups::UpdateSharedRunnersService + .new(group, current_user, args) + .execute + + { + errors: result[:status] == :success ? [] : Array.wrap(result[:message]) + } + end + + private + + def find_object(full_path:) + resolve_group(full_path: full_path) + end + end + end +end diff --git a/app/graphql/types/mutation_type.rb b/app/graphql/types/mutation_type.rb index 812943e0a1e1d6..d5ffe1d74ec991 100644 --- a/app/graphql/types/mutation_type.rb +++ b/app/graphql/types/mutation_type.rb @@ -105,6 +105,7 @@ class MutationType < BaseObject mount_mutation Mutations::Ci::Runner::Delete, feature_flag: :runner_graphql_query mount_mutation Mutations::Ci::RunnersRegistrationToken::Reset, feature_flag: :runner_graphql_query mount_mutation Mutations::Namespace::PackageSettings::Update + mount_mutation Mutations::Groups::UpdateSharedRunnersSetting mount_mutation Mutations::UserCallouts::Create mount_mutation Mutations::Packages::Destroy mount_mutation Mutations::Packages::DestroyFile diff --git a/app/policies/group_policy.rb b/app/policies/group_policy.rb index fa7834c10df211..3a1cac38384e5b 100644 --- a/app/policies/group_policy.rb +++ b/app/policies/group_policy.rb @@ -226,6 +226,10 @@ class GroupPolicy < BasePolicy rule { developer & dependency_proxy_available } .enable :admin_dependency_proxy + rule { can?(:admin_group) }.policy do + enable :update_group_shared_runners_setting + end + rule { can?(:admin_group) & resource_access_token_feature_available }.policy do enable :read_resource_access_tokens enable :destroy_resource_access_tokens diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 99b1b023524c25..9d3543200567f8 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -2360,6 +2360,25 @@ Input type: `GitlabSubscriptionActivateInput` | `errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. | | `license` | [`CurrentLicense`](#currentlicense) | The current license. | +### `Mutation.groupSharedRunnersSettingUpdate` + +Input type: `GroupSharedRunnersSettingUpdateInput` + +#### Arguments + +| Name | Type | Description | +| ---- | ---- | ----------- | +| `clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | +| `fullPath` | [`ID!`](#id) | Full path of the group that will be updated. | +| `sharedRunnersSetting` | [`SharedRunnersSetting!`](#sharedrunnerssetting) | Shared runners availability for the namespace and its descendants. | + +#### Fields + +| Name | Type | Description | +| ---- | ---- | ----------- | +| `clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | +| `errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. | + ### `Mutation.httpIntegrationCreate` Input type: `HttpIntegrationCreateInput` diff --git a/spec/graphql/mutations/groups/update_shared_runners_setting_spec.rb b/spec/graphql/mutations/groups/update_shared_runners_setting_spec.rb new file mode 100644 index 00000000000000..d653fc0ce51c78 --- /dev/null +++ b/spec/graphql/mutations/groups/update_shared_runners_setting_spec.rb @@ -0,0 +1,73 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Mutations::Groups::UpdateSharedRunnersSetting do + using RSpec::Parameterized::TableSyntax + + let_it_be_with_reload(:group) { create(:group) } + let_it_be(:user) { create(:user) } + + let(:params) { { full_path: group.full_path } } + + specify { expect(described_class).to require_graphql_authorizations(:update_group_shared_runners_setting) } + + describe '#resolve' do + subject { described_class.new(object: group, context: { current_user: user }, field: nil).resolve(**params) } + + RSpec.shared_examples 'updating the group shared runners setting' do + it 'updates the group shared runners setting' do + expect { subject } + .to change { group.reload.shared_runners_setting }.from('enabled').to('disabled_and_unoverridable') + end + + it 'returns no errors' do + expect(subject).to eq(errors: []) + end + + context 'with invalid params' do + let_it_be(:params) { { full_path: group.full_path, shared_runners_setting: 'inexistent_setting' } } + + it 'doesn\'t update the shared_runners_setting' do + expect { subject } + .not_to change { group.reload.shared_runners_setting } + end + + it 'returns an error' do + expect(subject).to eq( + errors: ["state must be one of: #{::Namespace::SHARED_RUNNERS_SETTINGS.join(', ')}"] + ) + end + end + end + + RSpec.shared_examples 'denying access to group shared runners setting' do + it 'raises Gitlab::Graphql::Errors::ResourceNotAvailable' do + expect { subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable) + end + end + + context 'with existing group shared runners setting' do + let_it_be(:params) do + { full_path: group.full_path, + shared_runners_setting: 'disabled_and_unoverridable' } + end + + where(:user_role, :shared_examples_name) do + :owner | 'updating the group shared runners setting' + :developer | 'denying access to group shared runners setting' + :reporter | 'denying access to group shared runners setting' + :guest | 'denying access to group shared runners setting' + :anonymous | 'denying access to group shared runners setting' + end + + with_them do + before do + group.send("add_#{user_role}", user) unless user_role == :anonymous + end + + it_behaves_like params[:shared_examples_name] + end + end + end +end -- GitLab From 0fb493ee273dcde973c525ae3ffa4e67f41f6500 Mon Sep 17 00:00:00 2001 From: Pedro Pombeiro Date: Tue, 3 Aug 2021 13:58:07 +0200 Subject: [PATCH 2/3] Add GroupSharedRunnersSettingUpdate request spec --- .../update_shared_runners_setting_spec.rb | 66 +++++++++++++++++++ 1 file changed, 66 insertions(+) create mode 100644 spec/requests/api/graphql/mutations/groups/update_shared_runners_setting_spec.rb diff --git a/spec/requests/api/graphql/mutations/groups/update_shared_runners_setting_spec.rb b/spec/requests/api/graphql/mutations/groups/update_shared_runners_setting_spec.rb new file mode 100644 index 00000000000000..ecf3021bd73bd8 --- /dev/null +++ b/spec/requests/api/graphql/mutations/groups/update_shared_runners_setting_spec.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'GroupSharedRunnersSettingUpdate' do + include GraphqlHelpers + + let_it_be(:user) { create(:user) } + let_it_be_with_reload(:group) { create(:group) } + + let(:variables) do + { + full_path: group.full_path, + shared_runners_setting: 'DISABLED_WITH_OVERRIDE' + } + end + + let(:mutation) { graphql_mutation(:group_shared_runners_setting_update, variables) } + + context 'when unauthorized' do + shared_examples 'unauthorized' do + it 'returns an error' do + post_graphql_mutation(mutation, current_user: user) + + expect(graphql_errors).not_to be_empty + end + end + + context 'when not a group member' do + it_behaves_like 'unauthorized' + end + + context 'when a non-admin group member' do + before do + group.add_developer(user) + end + + it_behaves_like 'unauthorized' + end + end + + context 'when authorized' do + before do + group.add_owner(user) + end + + it 'updates shared runners settings' 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(variables[:shared_runners_setting].downcase) + end + + context 'when bad arguments are provided' do + let(:variables) { { full_path: '', shared_runners_setting: 'INVALID' } } + + it 'returns the errors' do + post_graphql_mutation(mutation, current_user: user) + + expect(graphql_errors).not_to be_empty + expect(group.reload.shared_runners_setting).to eq('enabled') + end + end + end +end -- GitLab From dd3c0c6decf07e77ff3ccc95fa881d4aba9e5755 Mon Sep 17 00:00:00 2001 From: Pedro Pombeiro Date: Thu, 5 Aug 2021 14:24:32 +0200 Subject: [PATCH 3/3] Convert mutation to GroupUpdate mutation --- ...te_shared_runners_setting.rb => update.rb} | 20 ++++++++++--------- app/graphql/types/mutation_type.rb | 2 +- app/policies/group_policy.rb | 4 ---- doc/api/graphql/reference/index.md | 15 +++++++------- ...runners_setting_spec.rb => update_spec.rb} | 11 +++++----- ...runners_setting_spec.rb => update_spec.rb} | 4 ++-- 6 files changed, 28 insertions(+), 28 deletions(-) rename app/graphql/mutations/groups/{update_shared_runners_setting.rb => update.rb} (62%) rename spec/graphql/mutations/groups/{update_shared_runners_setting_spec.rb => update_spec.rb} (87%) rename spec/requests/api/graphql/mutations/groups/{update_shared_runners_setting_spec.rb => update_spec.rb} (91%) diff --git a/app/graphql/mutations/groups/update_shared_runners_setting.rb b/app/graphql/mutations/groups/update.rb similarity index 62% rename from app/graphql/mutations/groups/update_shared_runners_setting.rb rename to app/graphql/mutations/groups/update.rb index 21a1d65bd42000..f4eb94f9f8422a 100644 --- a/app/graphql/mutations/groups/update_shared_runners_setting.rb +++ b/app/graphql/mutations/groups/update.rb @@ -2,12 +2,16 @@ module Mutations module Groups - class UpdateSharedRunnersSetting < Mutations::BaseMutation + class Update < Mutations::BaseMutation include Mutations::ResolvesGroup - graphql_name 'GroupSharedRunnersSettingUpdate' + graphql_name 'GroupUpdate' - authorize :update_group_shared_runners_setting + authorize :admin_group + + field :group, Types::GroupType, + null: true, + description: 'The group after update.' argument :full_path, GraphQL::Types::ID, required: true, @@ -19,13 +23,11 @@ class UpdateSharedRunnersSetting < Mutations::BaseMutation def resolve(full_path:, **args) group = authorized_find!(full_path: full_path) - result = ::Groups::UpdateSharedRunnersService - .new(group, current_user, args) - .execute + unless ::Groups::UpdateService.new(group, current_user, args).execute + return { group: nil, errors: group.errors.full_messages } + end - { - errors: result[:status] == :success ? [] : Array.wrap(result[:message]) - } + { group: group, errors: [] } end private diff --git a/app/graphql/types/mutation_type.rb b/app/graphql/types/mutation_type.rb index d5ffe1d74ec991..530298ba07a337 100644 --- a/app/graphql/types/mutation_type.rb +++ b/app/graphql/types/mutation_type.rb @@ -105,7 +105,7 @@ class MutationType < BaseObject mount_mutation Mutations::Ci::Runner::Delete, feature_flag: :runner_graphql_query mount_mutation Mutations::Ci::RunnersRegistrationToken::Reset, feature_flag: :runner_graphql_query mount_mutation Mutations::Namespace::PackageSettings::Update - mount_mutation Mutations::Groups::UpdateSharedRunnersSetting + mount_mutation Mutations::Groups::Update mount_mutation Mutations::UserCallouts::Create mount_mutation Mutations::Packages::Destroy mount_mutation Mutations::Packages::DestroyFile diff --git a/app/policies/group_policy.rb b/app/policies/group_policy.rb index 3a1cac38384e5b..fa7834c10df211 100644 --- a/app/policies/group_policy.rb +++ b/app/policies/group_policy.rb @@ -226,10 +226,6 @@ class GroupPolicy < BasePolicy rule { developer & dependency_proxy_available } .enable :admin_dependency_proxy - rule { can?(:admin_group) }.policy do - enable :update_group_shared_runners_setting - end - rule { can?(:admin_group) & resource_access_token_feature_available }.policy do enable :read_resource_access_tokens enable :destroy_resource_access_tokens diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 9d3543200567f8..b6b266ada81068 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -2360,24 +2360,25 @@ Input type: `GitlabSubscriptionActivateInput` | `errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. | | `license` | [`CurrentLicense`](#currentlicense) | The current license. | -### `Mutation.groupSharedRunnersSettingUpdate` +### `Mutation.groupUpdate` -Input type: `GroupSharedRunnersSettingUpdateInput` +Input type: `GroupUpdateInput` #### Arguments | Name | Type | Description | | ---- | ---- | ----------- | -| `clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | -| `fullPath` | [`ID!`](#id) | Full path of the group that will be updated. | -| `sharedRunnersSetting` | [`SharedRunnersSetting!`](#sharedrunnerssetting) | Shared runners availability for the namespace and its descendants. | +| `clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | +| `fullPath` | [`ID!`](#id) | Full path of the group that will be updated. | +| `sharedRunnersSetting` | [`SharedRunnersSetting!`](#sharedrunnerssetting) | Shared runners availability for the namespace and its descendants. | #### Fields | Name | Type | Description | | ---- | ---- | ----------- | -| `clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | -| `errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. | +| `clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | +| `errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. | +| `group` | [`Group`](#group) | The group after update. | ### `Mutation.httpIntegrationCreate` diff --git a/spec/graphql/mutations/groups/update_shared_runners_setting_spec.rb b/spec/graphql/mutations/groups/update_spec.rb similarity index 87% rename from spec/graphql/mutations/groups/update_shared_runners_setting_spec.rb rename to spec/graphql/mutations/groups/update_spec.rb index d653fc0ce51c78..2118134e8e6c83 100644 --- a/spec/graphql/mutations/groups/update_shared_runners_setting_spec.rb +++ b/spec/graphql/mutations/groups/update_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Mutations::Groups::UpdateSharedRunnersSetting do +RSpec.describe Mutations::Groups::Update do using RSpec::Parameterized::TableSyntax let_it_be_with_reload(:group) { create(:group) } @@ -10,7 +10,7 @@ let(:params) { { full_path: group.full_path } } - specify { expect(described_class).to require_graphql_authorizations(:update_group_shared_runners_setting) } + specify { expect(described_class).to require_graphql_authorizations(:admin_group) } describe '#resolve' do subject { described_class.new(object: group, context: { current_user: user }, field: nil).resolve(**params) } @@ -22,7 +22,7 @@ end it 'returns no errors' do - expect(subject).to eq(errors: []) + expect(subject).to eq(errors: [], group: group) end context 'with invalid params' do @@ -35,7 +35,8 @@ it 'returns an error' do expect(subject).to eq( - errors: ["state must be one of: #{::Namespace::SHARED_RUNNERS_SETTINGS.join(', ')}"] + group: nil, + errors: ["Update shared runners state must be one of: #{::Namespace::SHARED_RUNNERS_SETTINGS.join(', ')}"] ) end end @@ -47,7 +48,7 @@ end end - context 'with existing group shared runners setting' do + context 'changing shared runners setting' do let_it_be(:params) do { full_path: group.full_path, shared_runners_setting: 'disabled_and_unoverridable' } diff --git a/spec/requests/api/graphql/mutations/groups/update_shared_runners_setting_spec.rb b/spec/requests/api/graphql/mutations/groups/update_spec.rb similarity index 91% rename from spec/requests/api/graphql/mutations/groups/update_shared_runners_setting_spec.rb rename to spec/requests/api/graphql/mutations/groups/update_spec.rb index ecf3021bd73bd8..b9dfb8e37ab1e3 100644 --- a/spec/requests/api/graphql/mutations/groups/update_shared_runners_setting_spec.rb +++ b/spec/requests/api/graphql/mutations/groups/update_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'GroupSharedRunnersSettingUpdate' do +RSpec.describe 'GroupUpdate' do include GraphqlHelpers let_it_be(:user) { create(:user) } @@ -15,7 +15,7 @@ } end - let(:mutation) { graphql_mutation(:group_shared_runners_setting_update, variables) } + let(:mutation) { graphql_mutation(:group_update, variables) } context 'when unauthorized' do shared_examples 'unauthorized' do -- GitLab