Commit 846f73b5 authored by Dylan Griffith's avatar Dylan Griffith

Allow group runners to be viewed/edited in API

parent 4790e726
......@@ -52,7 +52,7 @@ module Ci
# Without that, placeholders would miss one and couldn't match.
where(locked: false)
.where.not("ci_runners.id IN (#{project.runners.select(:id).to_sql})")
.specific
.project_type
end
validate :tag_constraints
......
......@@ -1001,10 +1001,17 @@ class User < ActiveRecord::Base
def ci_authorized_runners
@ci_authorized_runners ||= begin
runner_ids = Ci::RunnerProject
project_runner_ids = Ci::RunnerProject
.where(project: authorized_projects(Gitlab::Access::MASTER))
.select(:runner_id)
Ci::Runner.specific.where(id: runner_ids)
group_runner_ids = Ci::RunnerNamespace
.where(namespace_id: owned_or_masters_groups.select(:id))
.select(:runner_id)
union = Gitlab::SQL::Union.new([project_runner_ids, group_runner_ids])
Ci::Runner.specific.where("ci_runners.id IN (#{union.to_sql})") # rubocop:disable GitlabSecurity/SqlInjection
end
end
......@@ -1205,6 +1212,11 @@ class User < ActiveRecord::Base
!terms_accepted?
end
def owned_or_masters_groups
union = Gitlab::SQL::Union.new([owned_groups, masters_groups])
Group.from("(#{union.to_sql}) namespaces")
end
protected
# override, from Devise::Validatable
......
......@@ -205,6 +205,7 @@ module API
def authenticate_enable_runner!(runner)
forbidden!("Runner is shared") if runner.is_shared?
forbidden!("Runner is locked") if runner.locked?
forbidden!("Runner is a group runner") if runner.group_type?
return if current_user.admin?
forbidden!("No access granted") unless user_can_access_runner?(runner)
......
......@@ -19,11 +19,11 @@ describe Projects::Settings::CiCdController do
end
context 'with group runners' do
let(:group_runner) { create(:ci_runner) }
let(:group_runner) { create(:ci_runner, runner_type: :group_type) }
let(:parent_group) { create(:group) }
let(:group) { create(:group, runners: [group_runner], parent: parent_group) }
let(:other_project) { create(:project, group: group) }
let!(:project_runner) { create(:ci_runner, projects: [other_project]) }
let!(:project_runner) { create(:ci_runner, projects: [other_project], runner_type: :project_type) }
let!(:shared_runner) { create(:ci_runner, :shared) }
it 'sets assignable project runners only' do
......@@ -31,7 +31,7 @@ describe Projects::Settings::CiCdController do
get :show, namespace_id: project.namespace, project_id: project
expect(assigns(:assignable_runners)).to eq [project_runner]
expect(assigns(:assignable_runners)).to contain_exactly(project_runner)
end
end
end
......
......@@ -626,62 +626,26 @@ describe Ci::Runner do
end
describe '.assignable_for' do
let(:runner) { create(:ci_runner) }
let!(:unlocked_project_runner) { create(:ci_runner, runner_type: :project_type, projects: [project]) }
let!(:locked_project_runner) { create(:ci_runner, runner_type: :project_type, locked: true, projects: [project]) }
let!(:group_runner) { create(:ci_runner, runner_type: :group_type) }
let!(:instance_runner) { create(:ci_runner, :shared) }
let(:project) { create(:project) }
let(:another_project) { create(:project) }
before do
project.runners << runner
end
context 'with shared runners' do
before do
runner.update(is_shared: true)
end
context 'does not give owned runner' do
subject { described_class.assignable_for(project) }
it { is_expected.to be_empty }
end
context 'does not give shared runner' do
subject { described_class.assignable_for(another_project) }
it { is_expected.to be_empty }
end
end
context 'with unlocked runner' do
context 'does not give owned runner' do
context 'with already assigned project' do
subject { described_class.assignable_for(project) }
it { is_expected.to be_empty }
end
context 'does give a specific runner' do
context 'with a different project' do
subject { described_class.assignable_for(another_project) }
it { is_expected.to contain_exactly(runner) }
end
end
context 'with locked runner' do
before do
runner.update(locked: true)
end
context 'does not give owned runner' do
subject { described_class.assignable_for(project) }
it { is_expected.to be_empty }
end
context 'does not give a locked runner' do
subject { described_class.assignable_for(another_project) }
it { is_expected.to be_empty }
end
it { is_expected.to include(unlocked_project_runner) }
it { is_expected.not_to include(group_runner) }
it { is_expected.not_to include(locked_project_runner) }
it { is_expected.not_to include(instance_runner) }
end
end
......
......@@ -1788,14 +1788,12 @@ describe User do
describe '#ci_authorized_runners' do
let(:user) { create(:user) }
let(:runner) { create(:ci_runner) }
let(:runner_1) { create(:ci_runner) }
let(:runner_2) { create(:ci_runner) }
before do
project.runners << runner
end
context 'without any projects' do
let(:project) { create(:project) }
context 'without any projects nor groups' do
let!(:project) { create(:project, runners: [runner_1]) }
let!(:group) { create(:group) }
it 'does not load' do
expect(user.ci_authorized_runners).to be_empty
......@@ -1804,10 +1802,38 @@ describe User do
context 'with personal projects runners' do
let(:namespace) { create(:namespace, owner: user) }
let(:project) { create(:project, namespace: namespace) }
let!(:project) { create(:project, namespace: namespace, runners: [runner_1]) }
it 'loads' do
expect(user.ci_authorized_runners).to contain_exactly(runner_1)
end
end
context 'with personal group runner' do
let!(:project) { create(:project, runners: [runner_1]) }
let!(:group) do
create(:group, runners: [runner_2]).tap do |group|
group.add_owner(user)
end
end
it 'loads' do
expect(user.ci_authorized_runners).to contain_exactly(runner_2)
end
end
context 'with personal project and group runner' do
let(:namespace) { create(:namespace, owner: user) }
let!(:project) { create(:project, namespace: namespace, runners: [runner_1]) }
let!(:group) do
create(:group, runners: [runner_2]).tap do |group|
group.add_owner(user)
end
end
it 'loads' do
expect(user.ci_authorized_runners).to contain_exactly(runner)
expect(user.ci_authorized_runners).to contain_exactly(runner_1, runner_2)
end
end
......@@ -1818,7 +1844,7 @@ describe User do
end
it 'loads' do
expect(user.ci_authorized_runners).to contain_exactly(runner)
expect(user.ci_authorized_runners).to contain_exactly(runner_1)
end
end
......@@ -1835,7 +1861,21 @@ describe User do
context 'with groups projects runners' do
let(:group) { create(:group) }
let(:project) { create(:project, group: group) }
let!(:project) { create(:project, group: group, runners: [runner_1]) }
def add_user(access)
group.add_user(user, access)
end
it_behaves_like :member
end
context 'with groups runners' do
let!(:group) do
create(:group, runners: [runner_1]).tap do |group|
group.add_owner(user)
end
end
def add_user(access)
group.add_user(user, access)
......@@ -1845,7 +1885,7 @@ describe User do
end
context 'with other projects runners' do
let(:project) { create(:project) }
let!(:project) { create(:project, runners: [runner_1]) }
def add_user(access)
project.add_role(user, access)
......@@ -1858,7 +1898,7 @@ describe User do
let(:group) { create(:group) }
let(:another_user) { create(:user) }
let(:subgroup) { create(:group, parent: group) }
let(:project) { create(:project, group: subgroup) }
let!(:project) { create(:project, group: subgroup, runners: [runner_1]) }
def add_user(access)
group.add_user(user, access)
......
......@@ -27,7 +27,7 @@ describe API::Runners do
end
end
let!(:group_runner) { create(:ci_runner, description: 'Group runner', groups: [group]) }
let!(:group_runner) { create(:ci_runner, description: 'Group runner', groups: [group], runner_type: :group_type) }
before do
# Set project access for users
......@@ -48,7 +48,7 @@ describe API::Runners do
expect(json_response).to be_an Array
expect(json_response[0]).to have_key('ip_address')
expect(descriptions).to contain_exactly(
'Project runner', 'Two projects runner'
'Project runner', 'Two projects runner', 'Group runner'
)
expect(shared).to be_falsey
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