diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb index 60a9eade5a5416b5b3fd09e97f5ba3a274d5f56d..28d9edcc13574654293d11335a995264314041f1 100644 --- a/app/models/ci/runner.rb +++ b/app/models/ci/runner.rb @@ -348,6 +348,12 @@ def deprecated_rest_status end end + def owner_project + return unless project_type? + + runner_projects.order(:id).first.project + end + def belongs_to_one_project? runner_projects.count == 1 end diff --git a/app/services/ci/runners/set_runner_associated_projects_service.rb b/app/services/ci/runners/set_runner_associated_projects_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..759bd2ffb160e516585c4f3aa001976a6160b035 --- /dev/null +++ b/app/services/ci/runners/set_runner_associated_projects_service.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true + +module Ci + module Runners + class SetRunnerAssociatedProjectsService + # @param [Ci::Runner] runner: the project runner to assign/unassign projects from + # @param [User] current_user: the user performing the operation + # @param [Array] project_ids: the IDs of the associated projects to assign the runner to + def initialize(runner:, current_user:, project_ids:) + @runner = runner + @current_user = current_user + @project_ids = project_ids + end + + def execute + unless current_user&.can?(:assign_runner, runner) + return ServiceResponse.error(message: 'user not allowed to assign runner', http_status: :forbidden) + end + + set_associated_projects + end + + private + + def set_associated_projects + new_project_ids = [runner.owner_project.id] + project_ids + + response = ServiceResponse.success + runner.transaction do + # rubocop:disable CodeReuse/ActiveRecord + current_project_ids = runner.projects.ids + # rubocop:enable CodeReuse/ActiveRecord + + unless associate_new_projects(new_project_ids, current_project_ids) + response = ServiceResponse.error(message: 'failed to assign projects to runner') + raise ActiveRecord::Rollback, response.errors + end + + unless disassociate_old_projects(new_project_ids, current_project_ids) + response = ServiceResponse.error(message: 'failed to destroy runner project') + raise ActiveRecord::Rollback, response.errors + end + end + + response + end + + def associate_new_projects(new_project_ids, current_project_ids) + missing_projects = Project.id_in(new_project_ids - current_project_ids) + missing_projects.each do |project| + return false unless runner.assign_to(project, current_user) + end + + true + end + + def disassociate_old_projects(new_project_ids, current_project_ids) + Ci::RunnerProject + .destroy_by(project_id: current_project_ids - new_project_ids) + .all?(&:destroyed?) + end + + attr_reader :runner, :current_user, :project_ids + end + end +end + +Ci::Runners::SetRunnerAssociatedProjectsService.prepend_mod diff --git a/ee/app/services/ee/ci/runners/set_runner_associated_projects_service.rb b/ee/app/services/ee/ci/runners/set_runner_associated_projects_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..0d48c569b550f39390316509620ae22362137da7 --- /dev/null +++ b/ee/app/services/ee/ci/runners/set_runner_associated_projects_service.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +module EE + module Ci + module Runners + module SetRunnerAssociatedProjectsService + extend ::Gitlab::Utils::Override + + override :execute + def execute + super.tap do |result| + audit_event_service(result) + end + end + + private + + def audit_event_service(result) + return if result.error? + + audit_context = { + name: 'set_runner_associated_projects', + author: current_user, + scope: current_user, + target: runner, + target_details: runner_path, + message: 'Changed CI runner project assignments', + additional_details: { + action: :custom + } + } + ::Gitlab::Audit::Auditor.audit(audit_context) + end + + def runner_path + url_helpers = ::Gitlab::Routing.url_helpers + + url_helpers.project_runner_path(runner.owner_project, runner) + end + end + end + end +end diff --git a/ee/config/audit_events/types/set_runner_associated_projects.yaml b/ee/config/audit_events/types/set_runner_associated_projects.yaml new file mode 100644 index 0000000000000000000000000000000000000000..8d889d95e06b9930d6a33bdfa7494cd9f37e3441 --- /dev/null +++ b/ee/config/audit_events/types/set_runner_associated_projects.yaml @@ -0,0 +1,8 @@ +name: set_runner_associated_projects +description: Event triggered on successful assignment of associated projects to a CI runner +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/359958 +introduced_by_mr: +group: "group::runner" +milestone: 15.4 +saved_to_database: false +streamed: true diff --git a/ee/spec/services/ci/runners/set_runner_associated_projects_service_spec.rb b/ee/spec/services/ci/runners/set_runner_associated_projects_service_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..d529a7406f5dbe69665ebdfdd6b49068649ab8aa --- /dev/null +++ b/ee/spec/services/ci/runners/set_runner_associated_projects_service_spec.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ::Ci::Runners::SetRunnerAssociatedProjectsService, '#execute' do + subject(:execute) do + described_class.new(runner: project_runner, current_user: user, project_ids: [new_project.id]).execute + end + + let_it_be(:owner_project) { create(:project) } + let_it_be(:new_project) { create(:project) } + let_it_be(:project_runner) { create(:ci_runner, :project, projects: [owner_project]) } + + context 'with unauthorized user' do + let(:user) { build(:user) } + + it 'does not call assign_to on runner and returns error response', :aggregate_failures do + expect(project_runner).not_to receive(:assign_to) + expect(::Gitlab::Audit::Auditor).not_to receive(:audit) + + expect(execute).to be_error + expect(execute.http_status).to eq :forbidden + end + end + + context 'with admin user', :enable_admin_mode do + let(:user) { create_default(:user, :admin) } + + context 'with assign_to returning true' do + it 'calls audit on Auditor and returns success response', :aggregate_failures do + expect(project_runner).to receive(:assign_to).with(new_project, user).once.and_return(true) + expect(::Gitlab::Audit::Auditor).to receive(:audit).with( + a_hash_including( + name: 'set_runner_associated_projects', + author: user, + scope: user, + target: project_runner + )) + + expect(execute).to be_success + end + end + + context 'with assign_to returning false' do + it 'does not call audit on Auditor and returns error response', :aggregate_failures do + expect(project_runner).to receive(:assign_to).with(new_project, user).once.and_return(false) + expect(::Gitlab::Audit::Auditor).not_to receive(:audit) + + expect(execute).to be_error + end + end + end +end diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index cb5c43a52707e8d4a92c9cf8ec3d5b79692f14d9..7226c9db15d3cbe69494c028cfb660fb2d42fe07 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -1186,6 +1186,25 @@ def does_db_update end end + describe '#owner_project' do + let_it_be(:project1) { create(:project) } + let_it_be(:project2) { create(:project) } + + subject(:owner_project) { project_runner.owner_project } + + context 'with project1 as first project associated with runner' do + let_it_be(:project_runner) { create(:ci_runner, :project, projects: [project1, project2]) } + + it { is_expected.to eq project1 } + end + + context 'with project2 as first project associated with runner' do + let_it_be(:project_runner) { create(:ci_runner, :project, projects: [project2, project1]) } + + it { is_expected.to eq project2 } + end + end + describe "belongs_to_one_project?" do it "returns false if there are two projects runner assigned to" do project1 = create(:project) diff --git a/spec/services/ci/runners/set_runner_associated_projects_service_spec.rb b/spec/services/ci/runners/set_runner_associated_projects_service_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..b3fee24e09d77859c599ca73f4625c672d466744 --- /dev/null +++ b/spec/services/ci/runners/set_runner_associated_projects_service_spec.rb @@ -0,0 +1,70 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ::Ci::Runners::SetRunnerAssociatedProjectsService, '#execute' do + subject(:execute) { described_class.new(runner: runner, current_user: user, project_ids: project_ids).execute } + + let_it_be(:owner_project) { create(:project) } + let_it_be(:project2) { create(:project) } + let_it_be(:original_projects) { [owner_project, project2] } + let_it_be(:runner) { create(:ci_runner, :project, projects: original_projects) } + + context 'without user' do + let(:user) { nil } + let(:project_ids) { [project2.id] } + + it 'does not call assign_to on runner and returns error response', :aggregate_failures do + expect(runner).not_to receive(:assign_to) + + expect(execute).to be_error + expect(execute.message).to eq('user not allowed to assign runner') + end + end + + context 'with unauthorized user' do + let(:user) { build(:user) } + let(:project_ids) { [project2.id] } + + it 'does not call assign_to on runner and returns error message' do + expect(runner).not_to receive(:assign_to) + + expect(execute).to be_error + expect(execute.message).to eq('user not allowed to assign runner') + end + end + + context 'with admin user', :enable_admin_mode do + let(:user) { create_default(:user, :admin) } + let(:project_ids) { [project3.id, project4.id] } + let(:project3) { create(:project) } + let(:project4) { create(:project) } + + context 'with successful requests' do + it 'calls assign_to on runner and returns success response' do + expect(execute).to be_success + expect(runner.reload.projects.ids).to match_array([owner_project.id] + project_ids) + end + end + + context 'with failing assign_to requests' do + it 'returns error response and rolls back transaction' do + expect(runner).to receive(:assign_to).with(project4, user).once.and_return(false) + + expect(execute).to be_error + expect(runner.reload.projects).to match_array(original_projects) + end + end + + context 'with failing destroy calls' do + it 'returns error response and rolls back transaction' do + allow_next_found_instance_of(Ci::RunnerProject) do |runner_project| + allow(runner_project).to receive(:destroy).and_return(false) + end + + expect(execute).to be_error + expect(runner.reload.projects).to match_array(original_projects) + end + end + end +end