summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--app/models/ci/runner.rb2
-rw-r--r--app/models/user.rb16
-rw-r--r--lib/api/runners.rb1
-rw-r--r--spec/controllers/projects/settings/ci_cd_controller_spec.rb6
-rw-r--r--spec/models/ci/runner_spec.rb62
-rw-r--r--spec/models/user_spec.rb66
-rw-r--r--spec/requests/api/runners_spec.rb4
7 files changed, 87 insertions, 70 deletions
diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb
index bda69f85a78..e6f1ed519be 100644
--- a/app/models/ci/runner.rb
+++ b/app/models/ci/runner.rb
@@ -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
diff --git a/app/models/user.rb b/app/models/user.rb
index 173ab38e20c..2afe9ea77f9 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -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
diff --git a/lib/api/runners.rb b/lib/api/runners.rb
index 5f2a9567605..1b528a8490c 100644
--- a/lib/api/runners.rb
+++ b/lib/api/runners.rb
@@ -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)
diff --git a/spec/controllers/projects/settings/ci_cd_controller_spec.rb b/spec/controllers/projects/settings/ci_cd_controller_spec.rb
index a91c868cbaf..f1810763d2d 100644
--- a/spec/controllers/projects/settings/ci_cd_controller_spec.rb
+++ b/spec/controllers/projects/settings/ci_cd_controller_spec.rb
@@ -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
diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb
index e2b212f4f4c..0fbc934f669 100644
--- a/spec/models/ci/runner_spec.rb
+++ b/spec/models/ci/runner_spec.rb
@@ -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
- subject { described_class.assignable_for(project) }
-
- it { is_expected.to be_empty }
- end
+ context 'with already assigned project' do
+ subject { described_class.assignable_for(project) }
- context 'does give a specific runner' do
- subject { described_class.assignable_for(another_project) }
-
- it { is_expected.to contain_exactly(runner) }
- end
+ it { is_expected.to be_empty }
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) }
+ context 'with a different project' 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
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index bb5308221f0..52d25d20442 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -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)
+ 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_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)
diff --git a/spec/requests/api/runners_spec.rb b/spec/requests/api/runners_spec.rb
index 981ac768e3a..44eac4b2073 100644
--- a/spec/requests/api/runners_spec.rb
+++ b/spec/requests/api/runners_spec.rb
@@ -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