Skip to content

G
gitlab-ce
Commit 1aeb6283 authored by Pedro Pombeiro's avatar Pedro Pombeiro
Browse files
Options

Remove group and project params from runner install 

- RunnerInstructions no longer makes use of project nor group arguments,
so we can remove all references;
- Deprecate groupId and projectId from GraphQL endpoint.
parent 5d2c154f
Hide whitespace changes
Inline Side-by-side
Showing with 28 additions and 219 deletions
app/controllers/concerns/runner_setup_scripts.rb
View file @ 1aeb6283
......@@ -5,8 +5,8 @@ module RunnerSetupScripts
private
def private_runner_setup_scripts(**kwargs)
instructions = Gitlab::Ci::RunnerInstructions.new(current_user: current_user, os: script_params[:os], arch: script_params[:arch], **kwargs)
def private_runner_setup_scripts
instructions = Gitlab::Ci::RunnerInstructions.new(os: script_params[:os], arch: script_params[:arch])
output = {
install: instructions.install_script,
register: instructions.register_command
......
app/controllers/groups/settings/ci_cd_controller.rb
View file @ 1aeb6283
......@@ -52,7 +52,7 @@ module Groups
end
def runner_setup_scripts
private_runner_setup_scripts(group: group)
private_runner_setup_scripts
end
private
......
app/controllers/projects/settings/ci_cd_controller.rb
View file @ 1aeb6283
......@@ -62,7 +62,7 @@ module Projects
end
def runner_setup_scripts
private_runner_setup_scripts(project: @project)
private_runner_setup_scripts
end
private
......
app/graphql/resolvers/ci/runner_setup_resolver.rb
View file @ 1aeb6283
......@@ -21,19 +21,19 @@ module Resolvers
argument :project_id,
type: ::Types::GlobalIDType[::Project],
required: false,
deprecated: { reason: 'No longer used', milestone: '13.11' },
description: 'Project to register the runner for.'
argument :group_id,
type: ::Types::GlobalIDType[::Group],
required: false,
deprecated: { reason: 'No longer used', milestone: '13.11' },
description: 'Group to register the runner for.'
def resolve(platform:, architecture:, **args)
instructions = Gitlab::Ci::RunnerInstructions.new(
current_user: current_user,
os: platform,
arch: architecture,
**target_param(args)
arch: architecture
)
{
......
changelogs/unreleased/326102-deprecate-runner-install-arguments.yml 0 → 100644
View file @ 1aeb6283
---
title: Remove groupId and projectId arguments to Runner install instructions
merge_request: 57720
author:
type: changed
doc/api/graphql/reference/index.md
View file @ 1aeb6283
......@@ -281,9 +281,9 @@ Returns [`RunnerSetup`](#runnersetup).
| Name | Type | Description |
| ---- | ---- | ----------- |
| `architecture` | [`String!`](#string) | Architecture to generate the instructions for. |
| `groupId` | [`GroupID`](#groupid) | Group to register the runner for. |
| `groupId` | [`GroupID`](#groupid) | Group to register the runner for. Deprecated in 13.11: No longer used. |
| `platform` | [`String!`](#string) | Platform to generate the instructions for. |
| `projectId` | [`ProjectID`](#projectid) | Project to register the runner for. |
| `projectId` | [`ProjectID`](#projectid) | Project to register the runner for. Deprecated in 13.11: No longer used. |
### `snippets`
......
lib/gitlab/ci/runner_instructions.rb
View file @ 1aeb6283
......@@ -51,10 +51,7 @@ module Gitlab
attr_reader :errors
def initialize(current_user:, group: nil, project: nil, os:, arch:)
@current_user = current_user
@group = group
@project = project
def initialize(os:, arch:)
@os = os
@arch = arch
@errors = []
......@@ -77,7 +74,7 @@ module Gitlab
server_url = Gitlab::Routing.url_helpers.root_url(only_path: false)
runner_executable = environment[:runner_executable]
"#{runner_executable} register --url #{server_url} --registration-token #{registration_token}"
"#{runner_executable} register --url #{server_url} --registration-token $REGISTRATION_TOKEN"
end
end
......@@ -108,30 +105,6 @@ module Gitlab
def get_file(path)
File.read(Rails.root.join(path).to_s)
end
def registration_token
project_token || group_token || instance_token
end
def project_token
return unless @project
raise Gitlab::Access::AccessDeniedError unless can?(@current_user, :admin_pipeline, @project)
'$REGISTRATION_TOKEN'
end
def group_token
return unless @group
raise Gitlab::Access::AccessDeniedError unless can?(@current_user, :admin_group, @group)
'$REGISTRATION_TOKEN'
end
def instance_token
raise Gitlab::Access::AccessDeniedError unless @current_user&.admin?
'$REGISTRATION_TOKEN'
end
end
end
end
spec/graphql/resolvers/ci/runner_setup_resolver_spec.rb
View file @ 1aeb6283
......@@ -8,12 +8,11 @@ RSpec.describe Resolvers::Ci::RunnerSetupResolver do
describe '#resolve' do
let(:user) { create(:user) }
subject(:resolve_subject) { resolve(described_class, ctx: { current_user: user }, args: { platform: platform, architecture: 'amd64' }.merge(target_param)) }
subject(:resolve_subject) { resolve(described_class, ctx: { current_user: user }, args: { platform: platform, architecture: 'amd64' }) }
context 'with container platforms' do
let(:platform) { 'docker' }
let(:project) { create(:project) }
let(:target_param) { { project_id: project.to_global_id } }
it 'returns install instructions' do
expect(resolve_subject[:install_instructions]).not_to eq(nil)
......@@ -27,77 +26,9 @@ RSpec.describe Resolvers::Ci::RunnerSetupResolver do
context 'with regular platforms' do
let(:platform) { 'linux' }
context 'without target parameter' do
let(:target_param) { {} }
context 'when user is not admin' do
it 'returns access error' do
expect { resolve_subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable)
end
end
context 'when user is admin' do
before do
user.update!(admin: true)
end
it 'returns install and register instructions' do
expect(resolve_subject.keys).to contain_exactly(:install_instructions, :register_instructions)
expect(resolve_subject.values).not_to include(nil)
end
end
end
context 'with project target parameter' do
let(:project) { create(:project) }
let(:target_param) { { project_id: project.to_global_id } }
context 'when user has access to admin builds on project' do
before do
project.add_maintainer(user)
end
it 'returns install and register instructions' do
expect(resolve_subject.keys).to contain_exactly(:install_instructions, :register_instructions)
expect(resolve_subject.values).not_to include(nil)
end
end
context 'when user does not have access to admin builds on project' do
before do
project.add_developer(user)
end
it 'returns access error' do
expect { resolve_subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable)
end
end
end
context 'with group target parameter' do
let(:group) { create(:group) }
let(:target_param) { { group_id: group.to_global_id } }
context 'when user has access to admin builds on group' do
before do
group.add_owner(user)
end
it 'returns install and register instructions' do
expect(resolve_subject.keys).to contain_exactly(:install_instructions, :register_instructions)
expect(resolve_subject.values).not_to include(nil)
end
end
context 'when user does not have access to admin builds on group' do
before do
group.add_developer(user)
end
it 'returns access error' do
expect { resolve_subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable)
end
end
it 'returns install and register instructions' do
expect(resolve_subject.keys).to contain_exactly(:install_instructions, :register_instructions)
expect(resolve_subject.values).not_to include(nil)
end
end
end
......
spec/lib/gitlab/ci/runner_instructions_spec.rb
View file @ 1aeb6283
......@@ -6,7 +6,6 @@ RSpec.describe Gitlab::Ci::RunnerInstructions do
using RSpec::Parameterized::TableSyntax
let(:params) { {} }
let(:user) { create(:user) }
describe 'OS' do
Gitlab::Ci::RunnerInstructions::OS.each do |name, subject|
......@@ -37,7 +36,7 @@ RSpec.describe Gitlab::Ci::RunnerInstructions do
end
describe '#install_script' do
subject { described_class.new(current_user: user, **params) }
subject { described_class.new(**params) }
context 'invalid params' do
where(:current_params, :expected_error_message) do
......@@ -106,117 +105,18 @@ RSpec.describe Gitlab::Ci::RunnerInstructions do
end
end
context 'group' do
let(:group) { create(:group) }
subject { described_class.new(current_user: user, group: group, **params) }
context 'user is owner' do
before do
group.add_owner(user)
end
with_them do
let(:params) { { os: commands.each_key.first, arch: 'foo' } }
it 'have correct configurations' do
result = subject.register_command
expect(result).to include("#{commands[commands.each_key.first]} register")
expect(result).to include("--registration-token $REGISTRATION_TOKEN")
expect(result).to include("--url #{Gitlab::Routing.url_helpers.root_url(only_path: false)}")
end
end
end
context 'user is not owner' do
where(:user_permission) do
[:maintainer, :developer, :reporter, :guest]
end
with_them do
before do
create(:group_member, user_permission, group: group, user: user)
end
it 'raises error' do
result = subject.register_command
expect(result).to be_nil
expect(subject.errors).to include("Gitlab::Access::AccessDeniedError")
end
end
end
end
context 'project' do
let(:project) { create(:project) }
subject { described_class.new(current_user: user, project: project, **params) }
context 'user is maintainer' do
before do
project.add_maintainer(user)
end
with_them do
let(:params) { { os: commands.each_key.first, arch: 'foo' } }
it 'have correct configurations' do
result = subject.register_command
expect(result).to include("#{commands[commands.each_key.first]} register")
expect(result).to include("--registration-token $REGISTRATION_TOKEN")
expect(result).to include("--url #{Gitlab::Routing.url_helpers.root_url(only_path: false)}")
end
end
end
context 'user is not maintainer' do
where(:user_permission) do
[:developer, :reporter, :guest]
end
with_them do
before do
create(:project_member, user_permission, project: project, user: user)
end
it 'raises error' do
result = subject.register_command
expect(result).to be_nil
expect(subject.errors).to include("Gitlab::Access::AccessDeniedError")
end
end
end
end
context 'instance' do
subject { described_class.new(current_user: user, **params) }
context 'user is admin' do
let(:user) { create(:user, :admin) }
with_them do
let(:params) { { os: commands.each_key.first, arch: 'foo' } }
subject { described_class.new(**params) }
it 'have correct configurations' do
result = subject.register_command
expect(result).to include("#{commands[commands.each_key.first]} register")
expect(result).to include("--registration-token $REGISTRATION_TOKEN")
expect(result).to include("--url #{Gitlab::Routing.url_helpers.root_url(only_path: false)}")
end
end
end
with_them do
let(:params) { { os: commands.each_key.first, arch: 'foo' } }
context 'user is not admin' do
it 'raises error' do
it 'have correct configurations' do
result = subject.register_command
expect(result).to be_nil
expect(subject.errors).to include("Gitlab::Access::AccessDeniedError")
expect(result).to include("#{commands[commands.each_key.first]} register")
expect(result).to include("--registration-token $REGISTRATION_TOKEN")
expect(result).to include("--url #{Gitlab::Routing.url_helpers.root_url(only_path: false)}")
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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7