From 25f9eb27389dcec34f8b1b2b2003eee6c15b07d9 Mon Sep 17 00:00:00 2001 From: Pedro Pombeiro Date: Thu, 24 Feb 2022 17:07:14 +0100 Subject: [PATCH 1/2] Add audit logs when unassigning CI runner from a project Changelog: added EE: true --- .../ci/runners/unassign_runner_service.rb | 6 +++ .../ee/ci/runners/unassign_runner_service.rb | 31 +++++++++++++ .../runners/unassign_runner_service_spec.rb | 43 +++++++++++++++++++ .../runners/unassign_runner_service_spec.rb | 4 ++ 4 files changed, 84 insertions(+) create mode 100644 ee/app/services/ee/ci/runners/unassign_runner_service.rb create mode 100644 ee/spec/services/ci/runners/unassign_runner_service_spec.rb diff --git a/app/services/ci/runners/unassign_runner_service.rb b/app/services/ci/runners/unassign_runner_service.rb index a38a41960213f7..3832e66aca31c3 100644 --- a/app/services/ci/runners/unassign_runner_service.rb +++ b/app/services/ci/runners/unassign_runner_service.rb @@ -16,6 +16,12 @@ def execute @runner_project.destroy end + + private + + attr_reader :runner_project, :user end end end + +Ci::Runners::UnassignRunnerService.prepend_mod diff --git a/ee/app/services/ee/ci/runners/unassign_runner_service.rb b/ee/app/services/ee/ci/runners/unassign_runner_service.rb new file mode 100644 index 00000000000000..8be5f20e9b3077 --- /dev/null +++ b/ee/app/services/ee/ci/runners/unassign_runner_service.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +module EE + module Ci + module Runners + module UnassignRunnerService + extend ::Gitlab::Utils::Override + include ::Audit::Changes + + override :execute + def execute + runner = runner_project.runner + project = runner_project.project + result = super + + audit_log_event(runner, project) if result + + result + end + + private + + AUDIT_MESSAGE = 'Unassigned CI runner from project' + + def audit_log_event(runner, project) + ::AuditEvents::RunnerCustomAuditEventService.new(runner, user, project, AUDIT_MESSAGE).track_event + end + end + end + end +end diff --git a/ee/spec/services/ci/runners/unassign_runner_service_spec.rb b/ee/spec/services/ci/runners/unassign_runner_service_spec.rb new file mode 100644 index 00000000000000..920a1ecf8995bd --- /dev/null +++ b/ee/spec/services/ci/runners/unassign_runner_service_spec.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ::Ci::Runners::UnassignRunnerService, '#execute' do + let_it_be(:owner_project) { create(:project) } + let_it_be(:other_project) { create(:project) } + let_it_be(:project_runner) { create(:ci_runner, :project, projects: [owner_project, other_project]) } + + let(:runner_project) { project_runner.runner_projects.last } + + let(:audit_service) { instance_double(::AuditEvents::RunnerCustomAuditEventService) } + + subject { described_class.new(runner_project, user).execute } + + context 'with unauthorized user' do + let(:user) { build(:user) } + + it 'does not call assign_to on runner and returns false', :aggregate_failures do + expect(project_runner).not_to receive(:assign_to) + expect(::AuditEvents::RunnerCustomAuditEventService).not_to receive(:new) + + is_expected.to be_falsey + end + end + + context 'with admin user', :enable_admin_mode do + let(:user) { create_default(:admin) } + + before do + expect(audit_service).to receive(:track_event).once.and_return('track_event_return_value') + end + + it 'calls track_event on RunnerCustomAuditEventService and returns the runner_project', :aggregate_failures do + expect(runner_project).to receive(:destroy).once.and_call_original + expect(::AuditEvents::RunnerCustomAuditEventService).to receive(:new) + .with(project_runner, user, other_project, 'Unassigned CI runner from project') + .once.and_return(audit_service) + + is_expected.to eq(runner_project) + end + end +end diff --git a/spec/services/ci/runners/unassign_runner_service_spec.rb b/spec/services/ci/runners/unassign_runner_service_spec.rb index 3fb6925f4bd66f..f7f8a5b1aa93d6 100644 --- a/spec/services/ci/runners/unassign_runner_service_spec.rb +++ b/spec/services/ci/runners/unassign_runner_service_spec.rb @@ -10,6 +10,10 @@ let(:runner_project) { runner.runner_projects.last } + before do + allow(runner_project).to receive(:project).and_return(project) + end + context 'without user' do let(:user) { nil } -- GitLab From 6362129ae4604fbde0c13f635848af8e82c4d44e Mon Sep 17 00:00:00 2001 From: Pedro Pombeiro Date: Thu, 10 Mar 2022 17:27:07 +0100 Subject: [PATCH 2/2] Apply MR review suggestions --- app/services/ci/runners/unassign_runner_service.rb | 5 +++-- ee/app/services/ee/ci/runners/unassign_runner_service.rb | 6 ++---- spec/services/ci/runners/unassign_runner_service_spec.rb | 4 ---- 3 files changed, 5 insertions(+), 10 deletions(-) diff --git a/app/services/ci/runners/unassign_runner_service.rb b/app/services/ci/runners/unassign_runner_service.rb index 3832e66aca31c3..1e46cf6add85e7 100644 --- a/app/services/ci/runners/unassign_runner_service.rb +++ b/app/services/ci/runners/unassign_runner_service.rb @@ -6,8 +6,9 @@ class UnassignRunnerService # @param [Ci::RunnerProject] runner_project the runner/project association to destroy # @param [User] user the user performing the operation def initialize(runner_project, user) - @runner = runner_project.runner @runner_project = runner_project + @runner = runner_project.runner + @project = runner_project.project @user = user end @@ -19,7 +20,7 @@ def execute private - attr_reader :runner_project, :user + attr_reader :runner, :project, :user end end end diff --git a/ee/app/services/ee/ci/runners/unassign_runner_service.rb b/ee/app/services/ee/ci/runners/unassign_runner_service.rb index 8be5f20e9b3077..e7a0440bea6a75 100644 --- a/ee/app/services/ee/ci/runners/unassign_runner_service.rb +++ b/ee/app/services/ee/ci/runners/unassign_runner_service.rb @@ -9,11 +9,9 @@ module UnassignRunnerService override :execute def execute - runner = runner_project.runner - project = runner_project.project result = super - audit_log_event(runner, project) if result + audit_log_event if result result end @@ -22,7 +20,7 @@ def execute AUDIT_MESSAGE = 'Unassigned CI runner from project' - def audit_log_event(runner, project) + def audit_log_event ::AuditEvents::RunnerCustomAuditEventService.new(runner, user, project, AUDIT_MESSAGE).track_event end end diff --git a/spec/services/ci/runners/unassign_runner_service_spec.rb b/spec/services/ci/runners/unassign_runner_service_spec.rb index f7f8a5b1aa93d6..3fb6925f4bd66f 100644 --- a/spec/services/ci/runners/unassign_runner_service_spec.rb +++ b/spec/services/ci/runners/unassign_runner_service_spec.rb @@ -10,10 +10,6 @@ let(:runner_project) { runner.runner_projects.last } - before do - allow(runner_project).to receive(:project).and_return(project) - end - context 'without user' do let(:user) { nil } -- GitLab