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)

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
@@ -40,6 +40,9 @@
           %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