From 277c957c85b93b983bac5cbd8668ce2b2aa97f49 Mon Sep 17 00:00:00 2001 From: Pedro Pombeiro Date: Mon, 4 Jul 2022 15:05:35 +0200 Subject: [PATCH 1/3] GraphQL: Add bulkRunnerDelete mutation Changelog: added --- .../mutations/ci/runner/bulk_delete.rb | 64 +++++++++++++ app/graphql/types/mutation_type.rb | 1 + doc/api/graphql/reference/index.md | 24 +++++ .../mutations/ci/runner/bulk_delete_spec.rb | 91 +++++++++++++++++++ .../mutations/ci/runner/update_spec.rb | 3 +- 5 files changed, 181 insertions(+), 2 deletions(-) create mode 100644 app/graphql/mutations/ci/runner/bulk_delete.rb create mode 100644 spec/graphql/mutations/ci/runner/bulk_delete_spec.rb diff --git a/app/graphql/mutations/ci/runner/bulk_delete.rb b/app/graphql/mutations/ci/runner/bulk_delete.rb new file mode 100644 index 00000000000000..9381678a472837 --- /dev/null +++ b/app/graphql/mutations/ci/runner/bulk_delete.rb @@ -0,0 +1,64 @@ +# frozen_string_literal: true + +module Mutations + module Ci + module Runner + class BulkDelete < BaseMutation + graphql_name 'BulkRunnerDelete' + + RunnerID = ::Types::GlobalIDType[::Ci::Runner] + + argument :ids, [RunnerID], + required: false, + description: 'IDs of the runners to delete.' + + field :deleted_count, + ::GraphQL::Types::Int, + null: true, + description: 'Number of records effectively deleted. ' \ + 'Only present if operation was performed synchronously.' + + field :deleted_ids, + [RunnerID], + null: true, + description: 'IDs of records effectively deleted. ' \ + 'Only present if operation was performed synchronously.' + + def resolve(**runner_attrs) + if ids = runner_attrs[:ids] + raise_resource_not_available_error! unless Ability.allowed?(current_user, :delete_runners) + + runners = find_all_by_current_user(model_ids_of(ids)) + + result = ::Ci::Runners::BulkDeleteRunnersService.new(runners: runners).execute + result.slice(:deleted_count, :deleted_ids).merge(errors: []) + else + { errors: [] } + end + end + + private + + def raise_too_many_runners_requested_error + raise Gitlab::Graphql::Errors::ArgumentError, 'Too many runners requested for deletion.' + end + + def check_delete_amount_limit!(ids) + raise_too_many_runners_requested_error if ids.size > ::Ci::Runners::BulkDeleteRunnersService::RUNNER_LIMIT + end + + def model_ids_of(ids) + ids.map do |gid| + gid.model_id.to_i + end.compact + end + + def find_all_by_current_user(ids) + return ::Ci::Runner.none if ids.blank? || current_user.nil? + + ::Ci::Runner.id_in(ids) + end + end + end + end +end diff --git a/app/graphql/types/mutation_type.rb b/app/graphql/types/mutation_type.rb index dc9eb369dc8909..ba595a1191133c 100644 --- a/app/graphql/types/mutation_type.rb +++ b/app/graphql/types/mutation_type.rb @@ -130,6 +130,7 @@ class MutationType < BaseObject mount_mutation Mutations::Ci::JobTokenScope::RemoveProject mount_mutation Mutations::Ci::Runner::Update mount_mutation Mutations::Ci::Runner::Delete + mount_mutation Mutations::Ci::Runner::BulkDelete, alpha: { milestone: '15.3' } mount_mutation Mutations::Ci::RunnersRegistrationToken::Reset mount_mutation Mutations::Namespace::PackageSettings::Update mount_mutation Mutations::Groups::Update diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 461938b061dd36..42749dd5774420 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -951,6 +951,30 @@ Input type: `BulkEnableDevopsAdoptionNamespacesInput` | `enabledNamespaces` | [`[DevopsAdoptionEnabledNamespace!]`](#devopsadoptionenablednamespace) | Enabled namespaces after mutation. | | `errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. | +### `Mutation.bulkRunnerDelete` + +WARNING: +**Introduced** in 15.3. +This feature is in Alpha. It can be changed or removed at any time. + +Input type: `BulkRunnerDeleteInput` + +#### Arguments + +| Name | Type | Description | +| ---- | ---- | ----------- | +| `clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | +| `ids` | [`[CiRunnerID!]`](#cirunnerid) | IDs of the runners to delete. | + +#### Fields + +| Name | Type | Description | +| ---- | ---- | ----------- | +| `clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | +| `deletedCount` | [`Int`](#int) | Number of records effectively deleted. Only present if operation was performed synchronously. | +| `deletedIds` | [`[CiRunnerID!]`](#cirunnerid) | IDs of records effectively deleted. Only present if operation was performed synchronously. | +| `errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. | + ### `Mutation.ciCdSettingsUpdate` WARNING: diff --git a/spec/graphql/mutations/ci/runner/bulk_delete_spec.rb b/spec/graphql/mutations/ci/runner/bulk_delete_spec.rb new file mode 100644 index 00000000000000..f47f1b9869e4aa --- /dev/null +++ b/spec/graphql/mutations/ci/runner/bulk_delete_spec.rb @@ -0,0 +1,91 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Mutations::Ci::Runner::BulkDelete do + include GraphqlHelpers + + let_it_be(:admin_user) { create(:user, :admin) } + let_it_be(:user) { create(:user) } + + let(:current_ctx) { { current_user: user } } + + let(:mutation_params) do + {} + end + + describe '#resolve' do + subject(:response) do + sync(resolve(described_class, args: mutation_params, ctx: current_ctx)) + end + + context 'when the user cannot admin the runner' do + let(:runner) { create(:ci_runner) } + let(:mutation_params) do + { ids: [runner.to_global_id] } + end + + it 'generates an error' do + expect_graphql_error_to_be_created(Gitlab::Graphql::Errors::ResourceNotAvailable) { response } + end + end + + context 'when user can delete runners' do + let(:user) { admin_user } + let!(:runners) do + create_list(:ci_runner, 2, :instance) + end + + context 'when required arguments are missing' do + let(:mutation_params) { {} } + + context 'when admin mode is enabled', :enable_admin_mode do + it 'does not return an error' do + is_expected.to match a_hash_including(errors: []) + end + end + end + + context 'with runners specified by id' do + let(:mutation_params) do + { ids: runners.map(&:to_global_id) } + end + + context 'when admin mode is enabled', :enable_admin_mode do + it 'deletes runners', :aggregate_failures do + expect_next_instance_of( + ::Ci::Runners::BulkDeleteRunnersService, { runners: runners } + ) do |service| + expect(service).to receive(:execute).once.and_call_original + end + + expect { response }.to change { Ci::Runner.count }.by(-2) + expect(response[:errors]).to be_empty + end + + context 'when runner list is is above limit' do + before do + stub_const('::Ci::Runners::BulkDeleteRunnersService::RUNNER_LIMIT', 1) + end + + it 'only deletes up to the defined limit', :aggregate_failures do + expect { response }.to change { Ci::Runner.count } + .by(-::Ci::Runners::BulkDeleteRunnersService::RUNNER_LIMIT) + expect(response[:errors]).to be_empty + end + end + end + + context 'when admin mode is disabled', :aggregate_failures do + it 'returns error', :aggregate_failures do + expect do + expect_graphql_error_to_be_created(Gitlab::Graphql::Errors::ResourceNotAvailable) do + response + end + end.not_to change { Ci::Runner.count } + end + end + end + end + end +end diff --git a/spec/graphql/mutations/ci/runner/update_spec.rb b/spec/graphql/mutations/ci/runner/update_spec.rb index ffaa6e93d1bbed..b8efd4213fa9e5 100644 --- a/spec/graphql/mutations/ci/runner/update_spec.rb +++ b/spec/graphql/mutations/ci/runner/update_spec.rb @@ -2,12 +2,11 @@ require 'spec_helper' -RSpec.describe 'Mutations::Ci::Runner::Update' do +RSpec.describe Mutations::Ci::Runner::Update do include GraphqlHelpers let_it_be(:user) { create(:user) } let_it_be(:runner) { create(:ci_runner, active: true, locked: false, run_untagged: true) } - let_it_be(:described_class) { Mutations::Ci::Runner::Update } let(:current_ctx) { { current_user: user } } let(:mutated_runner) { subject[:runner] } -- GitLab From a87856cd757e3d18bd71698def043bd3a11040ec Mon Sep 17 00:00:00 2001 From: Pedro Pombeiro Date: Tue, 2 Aug 2022 10:11:43 +0200 Subject: [PATCH 2/3] Address MR review comments --- app/graphql/mutations/ci/runner/bulk_delete.rb | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/app/graphql/mutations/ci/runner/bulk_delete.rb b/app/graphql/mutations/ci/runner/bulk_delete.rb index 9381678a472837..b6be65ffc2790d 100644 --- a/app/graphql/mutations/ci/runner/bulk_delete.rb +++ b/app/graphql/mutations/ci/runner/bulk_delete.rb @@ -25,9 +25,9 @@ class BulkDelete < BaseMutation 'Only present if operation was performed synchronously.' def resolve(**runner_attrs) - if ids = runner_attrs[:ids] - raise_resource_not_available_error! unless Ability.allowed?(current_user, :delete_runners) + raise_resource_not_available_error! unless Ability.allowed?(current_user, :delete_runners) + if ids = runner_attrs[:ids] runners = find_all_by_current_user(model_ids_of(ids)) result = ::Ci::Runners::BulkDeleteRunnersService.new(runners: runners).execute @@ -39,14 +39,6 @@ def resolve(**runner_attrs) private - def raise_too_many_runners_requested_error - raise Gitlab::Graphql::Errors::ArgumentError, 'Too many runners requested for deletion.' - end - - def check_delete_amount_limit!(ids) - raise_too_many_runners_requested_error if ids.size > ::Ci::Runners::BulkDeleteRunnersService::RUNNER_LIMIT - end - def model_ids_of(ids) ids.map do |gid| gid.model_id.to_i @@ -54,7 +46,7 @@ def model_ids_of(ids) end def find_all_by_current_user(ids) - return ::Ci::Runner.none if ids.blank? || current_user.nil? + return ::Ci::Runner.none if ids.blank? ::Ci::Runner.id_in(ids) end -- GitLab From c983c112843297ebf4021c42bf5faa0295043d46 Mon Sep 17 00:00:00 2001 From: Pedro Pombeiro Date: Thu, 4 Aug 2022 08:08:46 +0200 Subject: [PATCH 3/3] Address MR review comment --- app/graphql/mutations/ci/runner/bulk_delete.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/graphql/mutations/ci/runner/bulk_delete.rb b/app/graphql/mutations/ci/runner/bulk_delete.rb index b6be65ffc2790d..09aeee37317937 100644 --- a/app/graphql/mutations/ci/runner/bulk_delete.rb +++ b/app/graphql/mutations/ci/runner/bulk_delete.rb @@ -28,7 +28,7 @@ def resolve(**runner_attrs) raise_resource_not_available_error! unless Ability.allowed?(current_user, :delete_runners) if ids = runner_attrs[:ids] - runners = find_all_by_current_user(model_ids_of(ids)) + runners = find_all_runners_by_ids(model_ids_of(ids)) result = ::Ci::Runners::BulkDeleteRunnersService.new(runners: runners).execute result.slice(:deleted_count, :deleted_ids).merge(errors: []) @@ -45,7 +45,7 @@ def model_ids_of(ids) end.compact end - def find_all_by_current_user(ids) + def find_all_runners_by_ids(ids) return ::Ci::Runner.none if ids.blank? ::Ci::Runner.id_in(ids) -- GitLab