Commit b569f842 authored by Rémy Coutable's avatar Rémy Coutable

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
parents a647d557 03dcb690
...@@ -30,6 +30,10 @@ v 8.10.0 (unreleased) ...@@ -30,6 +30,10 @@ v 8.10.0 (unreleased)
- Allow [ci skip] to be in any case and allow [skip ci]. !4785 (simon_w) - Allow [ci skip] to be in any case and allow [skip ci]. !4785 (simon_w)
- Add basic system information like memory and disk usage to the admin panel - Add basic system information like memory and disk usage to the admin panel
v 8.9.5 (unreleased)
- Fix assigning shared runners as admins
- Show "locked" label for locked runners on runners admin
v 8.9.4 v 8.9.4
- Fix privilege escalation issue with OAuth external users. - Fix privilege escalation issue with OAuth external users.
- Ensure references to private repos aren't shown to logged-out users. - Ensure references to private repos aren't shown to logged-out users.
......
...@@ -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