Commit 2a7ea341 authored by Rémy Coutable's avatar Rémy Coutable Committed by Robert Speicher

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 b569f842)
parent 71212ece
Please view this file on the master branch, on stable branches it's out of date. Please view this file on the master branch, on stable branches it's out of date.
v 8.9.5 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) - Update RedCloth to 4.3.2 for CVE-2012-6684. !4929 (Takuya Noguchi)
- Improve the request / withdraw access button. !4860 - Improve the request / withdraw access button. !4860
......
...@@ -4,8 +4,6 @@ class Admin::RunnerProjectsController < Admin::ApplicationController ...@@ -4,8 +4,6 @@ class Admin::RunnerProjectsController < Admin::ApplicationController
def create def create
@runner = Ci::Runner.find(params[:runner_project][:runner_id]) @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) runner_project = @runner.assign_to(@project, current_user)
if runner_project.persisted? if runner_project.persisted?
......
...@@ -6,8 +6,7 @@ class Projects::RunnerProjectsController < Projects::ApplicationController ...@@ -6,8 +6,7 @@ class Projects::RunnerProjectsController < Projects::ApplicationController
def create def create
@runner = Ci::Runner.find(params[:runner_project][:runner_id]) @runner = Ci::Runner.find(params[:runner_project][:runner_id])
return head(403) if @runner.is_shared? || @runner.locked? return head(403) unless can?(current_user, :assign_runner, @runner)
return head(403) unless current_user.ci_authorized_runners.include?(@runner)
path = runners_path(project) path = runners_path(project)
runner_project = @runner.assign_to(project, current_user) runner_project = @runner.assign_to(project, current_user)
......
class Ability class Ability
class << self class << self
# rubocop: disable Metrics/CyclomaticComplexity
def allowed(user, subject) def allowed(user, subject)
return anonymous_abilities(user, subject) if user.nil? return anonymous_abilities(user, subject) if user.nil?
return [] unless user.is_a?(User) return [] unless user.is_a?(User)
...@@ -19,6 +20,7 @@ class Ability ...@@ -19,6 +20,7 @@ class Ability
when ProjectMember then project_member_abilities(user, subject) when ProjectMember then project_member_abilities(user, subject)
when User then user_abilities when User then user_abilities
when ExternalIssue, Deployment, Environment then project_abilities(user, subject.project) when ExternalIssue, Deployment, Environment then project_abilities(user, subject.project)
when Ci::Runner then runner_abilities(user, subject)
else [] else []
end.concat(global_abilities(user)) end.concat(global_abilities(user))
end end
...@@ -512,6 +514,18 @@ class Ability ...@@ -512,6 +514,18 @@ class Ability
rules rules
end 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 def user_abilities
[:read_user] [:read_user]
end end
......
...@@ -4,6 +4,8 @@ ...@@ -4,6 +4,8 @@
%span.label.label-success shared %span.label.label-success shared
- else - else
%span.label.label-info specific %span.label.label-info specific
- if runner.locked?
%span.label.label-warning locked
- unless runner.active? - unless runner.active?
%span.label.label-danger paused %span.label.label-danger paused
......
...@@ -39,6 +39,9 @@ ...@@ -39,6 +39,9 @@
%li %li
%span.label.label-info specific %span.label.label-info specific
\- run builds from assigned projects \- run builds from assigned projects
%li
%span.label.label-warning locked
\- runner cannot be assigned to other projects
%li %li
%span.label.label-danger paused %span.label.label-danger paused
\- runner will not receive any new builds \- runner will not receive any new builds
......
...@@ -62,19 +62,45 @@ describe "Admin Runners" do ...@@ -62,19 +62,45 @@ describe "Admin Runners" do
end end
describe 'enable/create' do describe 'enable/create' do
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
context 'with specific runner' do
before do before do
@project1.runners << runner @project1.runners << runner
visit admin_runner_path(runner) visit admin_runner_path(runner)
end end
it 'enables specific runner for project' do it_behaves_like 'assignable runner'
within '.unassigned-projects' do
click_on 'Enable'
end end
assigned_project = page.find('.assigned-projects') context 'with locked runner' do
before do
runner.update(locked: true)
@project1.runners << runner
visit admin_runner_path(runner)
end
expect(assigned_project).to have_content(@project2.path) 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
it_behaves_like 'assignable runner'
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment