diff --git a/app/services/ci/runners/bulk_delete_runners_service.rb b/app/services/ci/runners/bulk_delete_runners_service.rb index d22123e318334dadd43fa6f437f2c1e790ea727e..5eff2ec0f02fa316cdf2ef29a05b1229931eba2a 100644 --- a/app/services/ci/runners/bulk_delete_runners_service.rb +++ b/app/services/ci/runners/bulk_delete_runners_service.rb @@ -3,7 +3,7 @@ module Ci module Runners class BulkDeleteRunnersService - attr_reader :runners + attr_reader :current_user, :runners RUNNER_LIMIT = 50 @@ -15,18 +15,18 @@ def initialize(runners:, current_user:) end def execute - if @runners + if runners # Delete a few runners immediately return delete_runners end - ServiceResponse.success(payload: { deleted_count: 0, deleted_ids: [], errors: [] }) + ServiceResponse.success(payload: { deleted_count: 0, deleted_ids: [], deleted_models: [], errors: [] }) end private def delete_runners - runner_count = @runners.limit(RUNNER_LIMIT + 1).count + runner_count = runners.limit(RUNNER_LIMIT + 1).count authorized_runners_ids, unauthorized_runners_ids = compute_authorized_runners # rubocop:disable CodeReuse/ActiveRecord runners_to_be_deleted = @@ -34,21 +34,26 @@ def delete_runners .id_in(authorized_runners_ids) .preload([:taggings, :runner_namespaces, :runner_projects]) # rubocop:enable CodeReuse/ActiveRecord - deleted_ids = runners_to_be_deleted.destroy_all.map(&:id) # rubocop:disable Cop/DestroyAll + # rubocop:disable Cop/DestroyAll -- loading objects into memory to run callbacks and return objects + deleted_models = runners_to_be_deleted.destroy_all + # rubocop:enable Cop/DestroyAll + deleted_ids = deleted_models.map(&:id) ServiceResponse.success( payload: { deleted_count: deleted_ids.count, + deleted_models: deleted_models, deleted_ids: deleted_ids, errors: error_messages(runner_count, authorized_runners_ids, unauthorized_runners_ids) }) end def compute_authorized_runners - @current_user.ci_owned_runners.load # preload the owned runners to avoid an N+1 + current_user.ci_owned_runners.load # preload the owned runners to avoid an N+1 + authorized_runners, unauthorized_runners = - @runners.limit(RUNNER_LIMIT) - .partition { |runner| Ability.allowed?(@current_user, :delete_runner, runner) } + runners.limit(RUNNER_LIMIT) + .partition { |runner| Ability.allowed?(current_user, :delete_runner, runner) } [authorized_runners.map(&:id), unauthorized_runners.map(&:id)] end @@ -71,3 +76,5 @@ def error_messages(runner_count, authorized_runners_ids, unauthorized_runners_id end end end + +Ci::Runners::BulkDeleteRunnersService.prepend_mod diff --git a/doc/user/compliance/audit_event_types.md b/doc/user/compliance/audit_event_types.md index 687fb18e3f8aa3b94aff5e1b571536dd756704ed..6594d2762b654bf179c45458adc1284939b4ac3c 100644 --- a/doc/user/compliance/audit_event_types.md +++ b/doc/user/compliance/audit_event_types.md @@ -310,6 +310,7 @@ Audit event types belong to the following product categories. | Name | Description | Saved to database | Introduced in | Scope | |:------------|:------------|:------------------|:---------|:--------------|:--------------| | [`ci_runner_usage_export`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/139578) | Triggered when a runner usage report is generated | **{check-circle}** Yes | GitLab [16.8](https://gitlab.com/gitlab-org/gitlab/-/issues/426560) | Instance | +| [`ci_runners_bulk_deleted`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/173886) | Triggered when runners are deleted in bulk | **{check-circle}** Yes | GitLab [17.7](https://gitlab.com/gitlab-org/gitlab/-/issues/503315) | User | ### Fuzz testing diff --git a/ee/app/services/audit_events/bulk_delete_runners_audit_event_service.rb b/ee/app/services/audit_events/bulk_delete_runners_audit_event_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..9c52f3f4925f5a2a42d5390ba0cc31362819ad89 --- /dev/null +++ b/ee/app/services/audit_events/bulk_delete_runners_audit_event_service.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +module AuditEvents + class BulkDeleteRunnersAuditEventService < ::AuditEventService + # Logs an audit event related to a bulk runner delete event + # + # @param [Array[Ci::Runner]] runners + # @param [User] current_user: the User initiating the operation + def initialize(runners, current_user) + @runners = runners.to_a + + details = { + custom_message: message, + errors: @runners.filter_map { |runner| runner.errors.full_messages.presence }.join(', ').presence, + runner_ids: @runners.map(&:id), + runner_short_shas: @runners.map(&:short_sha) + }.compact + + super(current_user, current_user, details) + end + + def track_event + security_event + end + + private + + def message + runner_short_shas = @runners.map(&:short_sha) + + "Deleted CI runners in bulk. Runner tokens: [#{runner_short_shas.join(', ')}]" + end + end +end diff --git a/ee/app/services/ee/ci/runners/bulk_delete_runners_service.rb b/ee/app/services/ee/ci/runners/bulk_delete_runners_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..052b1618dc38af460df329d5097f9d6fed1ffc48 --- /dev/null +++ b/ee/app/services/ee/ci/runners/bulk_delete_runners_service.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +module EE + module Ci + module Runners + # Unregisters CI Runners in bulk and logs an audit event + # + module BulkDeleteRunnersService + extend ::Gitlab::Utils::Override + + override :execute + def execute + super.tap do |result| + next unless runners && result.success? + + audit_event(result.payload[:deleted_models]) + end + end + + private + + def audit_event(runners) + ::AuditEvents::BulkDeleteRunnersAuditEventService.new(runners, current_user).track_event + end + end + end + end +end diff --git a/ee/config/audit_events/types/ci_runners_bulk_deleted.yml b/ee/config/audit_events/types/ci_runners_bulk_deleted.yml new file mode 100644 index 0000000000000000000000000000000000000000..4ccddeef3292845570dc6ed627a8672f4651b61e --- /dev/null +++ b/ee/config/audit_events/types/ci_runners_bulk_deleted.yml @@ -0,0 +1,10 @@ +--- +name: ci_runners_bulk_deleted +description: Triggered when runners are deleted in bulk +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/503315 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/173886 +feature_category: fleet_visibility +milestone: '17.7' +saved_to_database: true +streamed: true +scope: [User] diff --git a/ee/spec/services/audit_events/bulk_delete_runners_audit_event_service_spec.rb b/ee/spec/services/audit_events/bulk_delete_runners_audit_event_service_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..ee06b3b9432c759c1095ee6ec3509f05c3ec7d5e --- /dev/null +++ b/ee/spec/services/audit_events/bulk_delete_runners_audit_event_service_spec.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe AuditEvents::BulkDeleteRunnersAuditEventService, feature_category: :fleet_visibility do + let_it_be(:user) { create(:user) } + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, group: group) } + + let(:service) { described_class.new(runners, user) } + + describe '#track_event' do + before do + stub_licensed_features(extended_audit_events: true, admin_audit_log: true) + end + + subject(:track_event) { service.track_event } + + let_it_be(:runners) do + [ + create(:ci_runner), + create(:ci_runner, :group, groups: [group]), + create(:ci_runner, :project, projects: [project]) + ] + end + + let(:common_attrs) do + { + created_at: timestamp, + ip_address: nil, + target_details: nil, + target_id: nil, + target_type: nil, + details: { + ip_address: nil + } + } + end + + let(:short_shas) { runners.map(&:short_sha).join(', ') } + let(:timestamp) { Time.zone.local(2021, 12, 28) } + let(:attrs) do + common_attrs.deep_merge( + author_id: user.id, + author_name: user.name, + entity_id: user.id, + entity_type: user.class.name, + entity_path: user.username, + details: { + author_name: user.name, + custom_message: "Deleted CI runners in bulk. Runner tokens: [#{short_shas}]", + entity_path: user.username, + runner_ids: runners.map(&:id), + runner_short_shas: runners.map(&:short_sha) + } + ) + end + + it 'returns audit event attributes' do + travel_to(timestamp) do + expect(track_event.attributes).to eq( + **attrs.stringify_keys, + "id" => track_event.id + ) + end + end + end +end diff --git a/ee/spec/services/ci/runners/bulk_delete_runners_service_spec.rb b/ee/spec/services/ci/runners/bulk_delete_runners_service_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..223706947fe88374b32820ed1d5775f00aecbd93 --- /dev/null +++ b/ee/spec/services/ci/runners/bulk_delete_runners_service_spec.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ::Ci::Runners::BulkDeleteRunnersService, '#execute', feature_category: :fleet_visibility do + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, group: group) } + + let(:audit_service) { instance_double(::AuditEvents::BulkDeleteRunnersAuditEventService) } + let(:current_user) { create(:admin) } + + subject(:execute) { described_class.new(runners: runners, current_user: current_user).execute } + + shared_examples 'a service that logs an audit event' do + let!(:expected_runners) { runners.to_a } + + it 'logs an audit event with the scope of the current user' do + expect(audit_service).to receive(:track_event).once.and_return('track_event_return_value') + expect(::AuditEvents::BulkDeleteRunnersAuditEventService).to receive(:new) + .with(expected_runners, current_user) + .once.and_return(audit_service) + + execute + end + end + + context 'when user is allowed to delete runners', :enable_admin_mode do + context 'on an instance runner' do + let!(:instance_runners) { create_list(:ci_runner, 3) } + let(:runners) { Ci::Runner.id_in(instance_runners.map(&:id).take(2)) } + + it_behaves_like 'a service that logs an audit event' + end + + context 'on a group runner' do + let!(:group_runners) { create_list(:ci_runner, 2, :group, groups: [group]) } + let(:runners) { Ci::Runner.group_type } + + it_behaves_like 'a service that logs an audit event' + end + + context 'on a project runner' do + let!(:project_runners) { create_list(:ci_runner, 2, :project, projects: [project]) } + let(:runners) { Ci::Runner.project_type } + + it_behaves_like 'a service that logs an audit event' + end + end +end diff --git a/spec/services/ci/runners/bulk_delete_runners_service_spec.rb b/spec/services/ci/runners/bulk_delete_runners_service_spec.rb index cc820744370babd4ee1d9f74b88bc6d5c368a66a..7f6b8f434b958e7b378c470dadeaf53508119561 100644 --- a/spec/services/ci/runners/bulk_delete_runners_service_spec.rb +++ b/spec/services/ci/runners/bulk_delete_runners_service_spec.rb @@ -27,11 +27,11 @@ expect(execute).to be_success expect(execute.payload).to eq( - { - deleted_count: expected_deleted_ids.count, - deleted_ids: expected_deleted_ids, - errors: [] - }) + deleted_count: expected_deleted_ids.count, + deleted_ids: expected_deleted_ids, + deleted_models: expected_deleted_runners, + errors: [] + ) expect { project_runner.runner_projects.first.reload }.to raise_error(ActiveRecord::RecordNotFound) expected_deleted_runners.each do |deleted_runner| expect(deleted_runner[:errors]).to be_nil @@ -52,9 +52,11 @@ { deleted_count: 1, deleted_ids: a_collection_containing_exactly(an_instance_of(Integer)), + deleted_models: a_collection_containing_exactly(an_instance_of(::Ci::Runner)), errors: ["Can only delete up to 1 runners per call. Ignored the remaining runner(s)."] }) expect(expected_deleted_ids).to include(execute.payload[:deleted_ids].first) + expect(expected_deleted_runners).to include(execute.payload[:deleted_models].first) end end end @@ -163,11 +165,11 @@ expect(execute).to be_success expect(execute.payload).to eq( - { - deleted_count: 2, - deleted_ids: [group_runner.id, project_runner.id], - errors: ["User does not have permission to delete runner(s) ##{instance_runner.id}"] - }) + deleted_count: 2, + deleted_ids: [group_runner.id, project_runner.id], + deleted_models: [group_runner, project_runner], + errors: ["User does not have permission to delete runner(s) ##{instance_runner.id}"] + ) end end end @@ -179,7 +181,7 @@ it 'returns 0 deleted runners' do expect(execute).to be_success - expect(execute.payload).to eq({ deleted_count: 0, deleted_ids: [], errors: [] }) + expect(execute.payload).to eq(deleted_count: 0, deleted_ids: [], deleted_models: [], errors: []) end end end