From 2a7ea3417310d450cc21a07f7d61b822d3da9011 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Fri, 1 Jul 2016 10:10:46 +0000 Subject: Merge branch 'enable-shared-runners-with-admins' into 'master' Admin should be able to turn shared runners into specific ones: ## What does this MR do? Make sure admins could turn shared runners into specific runners. ## Are there points in the code the reviewer needs to double check? Is this the desired behaviour? ## Why was this MR needed? Closes #19039 Closes #19272 ![Screen_Shot_2016-06-30_at_9.30.05_PM](/uploads/97eb3b4923fd4e498b1f8ca70b1345c8/Screen_Shot_2016-06-30_at_9.30.05_PM.png) See merge request !4961 (cherry picked from commit b569f842b37434efd0cbae4b0c197391e0610b12) --- CHANGELOG | 1 + .../admin/runner_projects_controller.rb | 2 -- .../projects/runner_projects_controller.rb | 3 +- app/models/ability.rb | 14 ++++++++ app/views/admin/runners/_runner.html.haml | 2 ++ app/views/admin/runners/index.html.haml | 3 ++ spec/features/admin/admin_runners_spec.rb | 42 +++++++++++++++++----- 7 files changed, 55 insertions(+), 12 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index ae4419c233d..bb78fcc7374 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,7 @@ Please view this file on the master branch, on stable branches it's out of date. v 8.9.5 + - Admin should be able to turn shared runners into specific ones. !4961 - Update RedCloth to 4.3.2 for CVE-2012-6684. !4929 (Takuya Noguchi) - Improve the request / withdraw access button. !4860 diff --git a/app/controllers/admin/runner_projects_controller.rb b/app/controllers/admin/runner_projects_controller.rb index bf20c5305a7..bc65dcc33d3 100644 --- a/app/controllers/admin/runner_projects_controller.rb +++ b/app/controllers/admin/runner_projects_controller.rb @@ -4,8 +4,6 @@ class Admin::RunnerProjectsController < Admin::ApplicationController def create @runner = Ci::Runner.find(params[:runner_project][:runner_id]) - return head(403) if @runner.is_shared? || @runner.locked? - runner_project = @runner.assign_to(@project, current_user) if runner_project.persisted? diff --git a/app/controllers/projects/runner_projects_controller.rb b/app/controllers/projects/runner_projects_controller.rb index dc1a18f8d42..8267b14941d 100644 --- a/app/controllers/projects/runner_projects_controller.rb +++ b/app/controllers/projects/runner_projects_controller.rb @@ -6,8 +6,7 @@ class Projects::RunnerProjectsController < Projects::ApplicationController def create @runner = Ci::Runner.find(params[:runner_project][:runner_id]) - return head(403) if @runner.is_shared? || @runner.locked? - return head(403) unless current_user.ci_authorized_runners.include?(@runner) + return head(403) unless can?(current_user, :assign_runner, @runner) path = runners_path(project) runner_project = @runner.assign_to(project, current_user) diff --git a/app/models/ability.rb b/app/models/ability.rb index f5950879ccb..ba1f2ae4075 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -1,5 +1,6 @@ class Ability class << self + # rubocop: disable Metrics/CyclomaticComplexity def allowed(user, subject) return anonymous_abilities(user, subject) if user.nil? return [] unless user.is_a?(User) @@ -19,6 +20,7 @@ class Ability when ProjectMember then project_member_abilities(user, subject) when User then user_abilities when ExternalIssue, Deployment, Environment then project_abilities(user, subject.project) + when Ci::Runner then runner_abilities(user, subject) else [] end.concat(global_abilities(user)) end @@ -512,6 +514,18 @@ class Ability rules end + def runner_abilities(user, runner) + if user.is_admin? + [:assign_runner] + elsif runner.is_shared? || runner.locked? + [] + elsif user.ci_authorized_runners.include?(runner) + [:assign_runner] + else + [] + end + end + def user_abilities [:read_user] end diff --git a/app/views/admin/runners/_runner.html.haml b/app/views/admin/runners/_runner.html.haml index 36b21eefdee..64893b38c58 100644 --- a/app/views/admin/runners/_runner.html.haml +++ b/app/views/admin/runners/_runner.html.haml @@ -4,6 +4,8 @@ %span.label.label-success shared - else %span.label.label-info specific + - if runner.locked? + %span.label.label-warning locked - unless runner.active? %span.label.label-danger paused diff --git a/app/views/admin/runners/index.html.haml b/app/views/admin/runners/index.html.haml index 5eff77aff2d..b5c6b5effe3 100644 --- a/app/views/admin/runners/index.html.haml +++ b/app/views/admin/runners/index.html.haml @@ -39,6 +39,9 @@ %li %span.label.label-info specific \- run builds from assigned projects + %li + %span.label.label-warning locked + \- runner cannot be assigned to other projects %li %span.label.label-danger paused \- runner will not receive any new builds diff --git a/spec/features/admin/admin_runners_spec.rb b/spec/features/admin/admin_runners_spec.rb index 2d297776cb0..2f82fafc13a 100644 --- a/spec/features/admin/admin_runners_spec.rb +++ b/spec/features/admin/admin_runners_spec.rb @@ -62,19 +62,45 @@ describe "Admin Runners" do end describe 'enable/create' do - before do - @project1.runners << runner - visit admin_runner_path(runner) + shared_examples 'assignable runner' do + it 'enables a runner for a project' do + within '.unassigned-projects' do + click_on 'Enable' + end + + assigned_project = page.find('.assigned-projects') + + expect(assigned_project).to have_content(@project2.path) + end end - it 'enables specific runner for project' do - within '.unassigned-projects' do - click_on 'Enable' + context 'with specific runner' do + before do + @project1.runners << runner + visit admin_runner_path(runner) end - assigned_project = page.find('.assigned-projects') + it_behaves_like 'assignable runner' + end + + context 'with locked runner' do + before do + runner.update(locked: true) + @project1.runners << runner + visit admin_runner_path(runner) + end + + it_behaves_like 'assignable runner' + end + + context 'with shared runner' do + before do + @project1.destroy + runner.update(is_shared: true) + visit admin_runner_path(runner) + end - expect(assigned_project).to have_content(@project2.path) + it_behaves_like 'assignable runner' end end -- cgit v1.2.1