Commit f26389a0 authored by Robert Speicher's avatar Robert Speicher

Merge branch 'feature/runner-config-untagged-jobs' into 'master'

Add config for CI Runner that prevents it from picking untagged jobs

Closes #3456

See merge request !4039
parents f73def90 52c8b9da
......@@ -6,6 +6,7 @@ v 8.8.0 (unreleased)
- Assign labels and milestone to target project when moving issue. !3934 (Long Nguyen)
- Use a case-insensitive comparison in sanitizing URI schemes
- Toggle sign-up confirmation emails in application settings
- Make it possible to prevent tagged runner from picking untagged jobs
- Project#open_branches has been cleaned up and no longer loads entire records into memory.
- Escape HTML in commit titles in system note messages
- Fix creation of Ci::Commit object which can lead to pending, failed in some scenarios
......
......@@ -9,23 +9,18 @@ def index
end
def show
@builds = @runner.builds.order('id DESC').first(30)
@projects =
if params[:search].present?
::Project.search(params[:search])
else
Project.all
end
@projects = @projects.where.not(id: @runner.projects.select(:id)) if @runner.projects.any?
@projects = @projects.page(params[:page]).per(30)
assign_builds_and_projects
end
def update
@runner.update_attributes(runner_params)
respond_to do |format|
format.js
format.html { redirect_to admin_runner_path(@runner) }
if @runner.update_attributes(runner_params)
respond_to do |format|
format.js
format.html { redirect_to admin_runner_path(@runner) }
end
else
assign_builds_and_projects
render 'show'
end
end
......@@ -60,4 +55,16 @@ def runner
def runner_params
params.require(:runner).permit(Ci::Runner::FORM_EDITABLE)
end
def assign_builds_and_projects
@builds = runner.builds.order('id DESC').first(30)
@projects =
if params[:search].present?
::Project.search(params[:search])
else
Project.all
end
@projects = @projects.where.not(id: runner.projects.select(:id)) if runner.projects.any?
@projects = @projects.page(params[:page]).per(30)
end
end
......@@ -20,7 +20,7 @@ def update
if @runner.update_attributes(runner_params)
redirect_to runner_path(@runner), notice: 'Runner was successfully updated.'
else
redirect_to runner_path(@runner), alert: 'Runner was not updated.'
render 'edit'
end
end
......
......@@ -291,9 +291,15 @@ def valid_token? token
end
def can_be_served?(runner)
return false unless has_tags? || runner.run_untagged?
(tag_list - runner.tag_list).empty?
end
def has_tags?
tag_list.any?
end
def any_runners_online?
project.any_runners? { |runner| runner.active? && runner.online? && can_be_served?(runner) }
end
......
......@@ -4,7 +4,7 @@ class Runner < ActiveRecord::Base
LAST_CONTACT_TIME = 5.minutes.ago
AVAILABLE_SCOPES = %w[specific shared active paused online]
FORM_EDITABLE = %i[description tag_list active]
FORM_EDITABLE = %i[description tag_list active run_untagged]
has_many :builds, class_name: 'Ci::Build'
has_many :runner_projects, dependent: :destroy, class_name: 'Ci::RunnerProject'
......@@ -26,6 +26,8 @@ class Runner < ActiveRecord::Base
.where("ci_runner_projects.gl_project_id = :project_id OR ci_runners.is_shared = true", project_id: project_id)
end
validate :tag_constraints
acts_as_taggable
# Searches for runners matching the given query.
......@@ -96,5 +98,18 @@ def only_for?(project)
def short_sha
token[0...8] if token
end
def has_tags?
tag_list.any?
end
private
def tag_constraints
unless has_tags? || run_untagged?
errors.add(:tags_list,
'can not be empty when runner is not allowed to pick untagged jobs')
end
end
end
end
......@@ -9,8 +9,6 @@
%span.runner-state.runner-state-specific
Specific
- if @runner.shared?
.bs-callout.bs-callout-success
%h4 This runner will process builds from ALL UNASSIGNED projects
......
= form_for runner, url: runner_form_url, html: { class: 'form-horizontal' } do |f|
= form_errors(runner)
.form-group
= label :active, "Active", class: 'control-label'
.col-sm-10
.checkbox
= f.check_box :active
%span.light Paused runners don't accept new builds
.form-group
= label :run_untagged, 'Run untagged jobs', class: 'control-label'
.col-sm-10
.checkbox
= f.check_box :run_untagged
%span.light Indicates whether this runner can pick jobs without tags
.form-group
= label_tag :token, class: 'control-label' do
Token
......
......@@ -5,7 +5,7 @@
- if @runners.include?(runner)
= link_to runner.short_sha, runner_path(runner)
%small
=link_to edit_namespace_project_runner_path(@project.namespace, @project, runner) do
= link_to edit_namespace_project_runner_path(@project.namespace, @project, runner) do
%i.fa.fa-edit.btn
- else
= runner.short_sha
......
- page_title "Edit", "#{@runner.description} ##{@runner.id}", "Runners"
%h4 Runner ##{@runner.id}
%hr
= render 'form', runner: @runner, runner_form_url: runner_path(@runner)
......@@ -17,50 +17,39 @@
%th Property Name
%th Value
%tr
%td
Tags
%td Active
%td= @runner.active? ? 'Yes' : 'No'
%tr
%td Can run untagged jobs
%td= @runner.run_untagged? ? 'Yes' : 'No'
%tr
%td Tags
%td
- @runner.tag_list.each do |tag|
%span.label.label-primary
= tag
%tr
%td
Name
%td
= @runner.name
%td Name
%td= @runner.name
%tr
%td
Version
%td
= @runner.version
%td Version
%td= @runner.version
%tr
%td
Revision
%td
= @runner.revision
%td Revision
%td= @runner.revision
%tr
%td
Platform
%td
= @runner.platform
%td Platform
%td= @runner.platform
%tr
%td
Architecture
%td
= @runner.architecture
%td Architecture
%td= @runner.architecture
%tr
%td
Description
%td
= @runner.description
%td Description
%td= @runner.description
%tr
%td
Last contact
%td Last contact
%td
- if @runner.contacted_at
#{time_ago_in_words(@runner.contacted_at)} ago
- else
Never
class AddRunUntaggedToCiRunner < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
disable_ddl_transaction!
def up
add_column_with_default(:ci_runners, :run_untagged, :boolean,
default: true, allow_null: false)
end
def down
remove_column(:ci_runners, :run_untagged)
end
end
......@@ -269,6 +269,7 @@
t.string "revision"
t.string "platform"
t.string "architecture"
t.boolean "run_untagged", default: true, null: false
end
add_index "ci_runners", ["description"], name: "index_ci_runners_on_description_trigram", using: :gin, opclasses: {"description"=>"gin_trgm_ops"}
......
......@@ -125,7 +125,13 @@ shared runners will only run the jobs they are equipped to run.
For instance, at GitLab we have runners tagged with "rails" if they contain
the appropriate dependencies to run Rails test suites.
### Be Careful with Sensitive Information
### Prevent runner with tags from picking jobs without tags
You can configure a runner to prevent it from picking jobs with tags when
the runnner does not have tags assigned. This setting is available on each
runner in *Project Settings* > *Runners*.
### Be careful with sensitive information
If you can run a build on a runner, you can get access to any code it runs
and get the token of the runner. With shared runners, this means that anyone
......
......@@ -408,6 +408,7 @@ class Runner < Grape::Entity
class RunnerDetails < Runner
expose :tag_list
expose :run_untagged
expose :version, :revision, :platform, :architecture
expose :contacted_at
expose :token, if: lambda { |runner, options| options[:current_user].is_admin? || !runner.is_shared? }
......
......@@ -49,7 +49,7 @@ class Runners < Grape::API
runner = get_runner(params[:id])
authenticate_update_runner!(runner)
attrs = attributes_for_keys [:description, :active, :tag_list]
attrs = attributes_for_keys [:description, :active, :tag_list, :run_untagged]
if runner.update(attrs)
present runner, with: Entities::RunnerDetails, current_user: current_user
else
......
......@@ -28,20 +28,20 @@ class Runners < Grape::API
post "register" do
required_attributes! [:token]
attributes = { description: params[:description],
tag_list: params[:tag_list] }
unless params[:run_untagged].nil?
attributes[:run_untagged] = params[:run_untagged]
end
runner =
if runner_registration_token_valid?
# Create shared runner. Requires admin access
Ci::Runner.create(
description: params[:description],
tag_list: params[:tag_list],
is_shared: true
)
Ci::Runner.create(attributes.merge(is_shared: true))
elsif project = Project.find_by(runners_token: params[:token])
# Create a specific runner for project.
project.runners.create(
description: params[:description],
tag_list: params[:tag_list]
)
project.runners.create(attributes)
end
return forbidden! unless runner
......
......@@ -110,4 +110,37 @@
expect(page).to have_content(@specific_runner.platform)
end
end
feature 'configuring runners ability to picking untagged jobs' do
given(:project) { create(:empty_project) }
given(:runner) { create(:ci_runner) }
background do
project.team << [user, :master]
project.runners << runner
end
scenario 'user checks default configuration' do
visit namespace_project_runner_path(project.namespace, project, runner)
expect(page).to have_content 'Can run untagged jobs Yes'
end
context 'when runner has tags' do
before { runner.update_attribute(:tag_list, ['tag']) }
scenario 'user wants to prevent runner from running untagged job' do
visit runners_path(project)
page.within('.activated-specific-runners') do
first('small > a').click
end
uncheck 'runner_run_untagged'
click_button 'Save changes'
expect(page).to have_content 'Can run untagged jobs No'
expect(runner.reload.run_untagged?).to eq false
end
end
end
end
......@@ -259,11 +259,11 @@
end
describe '#can_be_served?' do
let(:runner) { FactoryGirl.create :ci_runner }
let(:runner) { create(:ci_runner) }
before { build.project.runners << runner }
context 'runner without tags' do
context 'when runner does not have tags' do
it 'can handle builds without tags' do
expect(build.can_be_served?(runner)).to be_truthy
end
......@@ -274,25 +274,53 @@
end
end
context 'runner with tags' do
context 'when runner has tags' do
before { runner.tag_list = ['bb', 'cc'] }
it 'can handle builds without tags' do
expect(build.can_be_served?(runner)).to be_truthy
shared_examples 'tagged build picker' do
it 'can handle build with matching tags' do
build.tag_list = ['bb']
expect(build.can_be_served?(runner)).to be_truthy
end
it 'cannot handle build without matching tags' do
build.tag_list = ['aa']
expect(build.can_be_served?(runner)).to be_falsey
end
end
it 'can handle build with matching tags' do
build.tag_list = ['bb']
expect(build.can_be_served?(runner)).to be_truthy
context 'when runner can pick untagged jobs' do
it 'can handle builds without tags' do
expect(build.can_be_served?(runner)).to be_truthy
end
it_behaves_like 'tagged build picker'
end
it 'cannot handle build with not matching tags' do
build.tag_list = ['aa']
expect(build.can_be_served?(runner)).to be_falsey
context 'when runner can not pick untagged jobs' do
before { runner.run_untagged = false }
it 'can not handle builds without tags' do
expect(build.can_be_served?(runner)).to be_falsey
end
it_behaves_like 'tagged build picker'
end
end
end
describe '#has_tags?' do
context 'when build has tags' do
subject { create(:ci_build, tag_list: ['tag']) }
it { is_expected.to have_tags }
end
context 'when build does not have tags' do
subject { create(:ci_build, tag_list: []) }
it { is_expected.to_not have_tags }
end
end
describe '#any_runners_online?' do
subject { build.any_runners_online? }
......
require 'spec_helper'
describe Ci::Runner, models: true do
describe 'validation' do
context 'when runner is not allowed to pick untagged jobs' do
context 'when runner does not have tags' do
it 'is not valid' do
runner = build(:ci_runner, tag_list: [], run_untagged: false)
expect(runner).to be_invalid
end
end
context 'when runner has tags' do
it 'is valid' do
runner = build(:ci_runner, tag_list: ['tag'], run_untagged: false)
expect(runner).to be_valid
end
end
end
end
describe '#display_name' do
it 'should return the description if it has a value' do
runner = FactoryGirl.build(:ci_runner, description: 'Linux/Ruby-1.9.3-p448')
......@@ -114,7 +132,19 @@
end
end
describe '#search' do
describe '#has_tags?' do
context 'when runner has tags' do
subject { create(:ci_runner, tag_list: ['tag']) }
it { is_expected.to have_tags }
end
context 'when runner does not have tags' do
subject { create(:ci_runner, tag_list: []) }
it { is_expected.to_not have_tags }
end
end
describe '.search' do
let(:runner) { create(:ci_runner, token: '123abc') }
it 'returns runners with a matching token' do
......
......@@ -184,21 +184,24 @@
description = shared_runner.description
active = shared_runner.active
put api("/runners/#{shared_runner.id}", admin), description: "#{description}_updated", active: !active,
tag_list: ['ruby2.1', 'pgsql', 'mysql']
update_runner(shared_runner.id, admin, description: "#{description}_updated",
active: !active,
tag_list: ['ruby2.1', 'pgsql', 'mysql'],
run_untagged: 'false')
shared_runner.reload
expect(response.status).to eq(200)
expect(shared_runner.description).to eq("#{description}_updated")
expect(shared_runner.active).to eq(!active)
expect(shared_runner.tag_list).to include('ruby2.1', 'pgsql', 'mysql')
expect(shared_runner.run_untagged?).to be false
end
end
context 'when runner is not shared' do
it 'should update runner' do
description = specific_runner.description
put api("/runners/#{specific_runner.id}", admin), description: 'test'
update_runner(specific_runner.id, admin, description: 'test')
specific_runner.reload
expect(response.status).to eq(200)
......@@ -208,10 +211,14 @@
end
it 'should return 404 if runner does not exists' do
put api('/runners/9999', admin), description: 'test'
update_runner(9999, admin, description: 'test')
expect(response.status).to eq(404)
end
def update_runner(id, user, args)
put api("/runners/#{id}", user), args
end
end
context 'authorized user' do
......
......@@ -128,6 +128,38 @@
end
end
end
context 'when build has no tags' do
before do
commit = create(:ci_commit, project: project)
create(:ci_build, commit: commit, tags: [])
end
context 'when runner is allowed to pick untagged builds' do
before { runner.update_column(:run_untagged, true) }
it 'picks build' do
register_builds
expect(response).to have_http_status 201
end
end
context 'when runner is not allowed to pick untagged builds' do
before { runner.update_column(:run_untagged, false) }
it 'does not pick build' do
register_builds
expect(response).to have_http_status 404
end
end
def register_builds
post ci_api("/builds/register"), token: runner.token,
info: { platform: :darwin }
end
end
end
describe "PUT /builds/:id" do
......
......@@ -12,44 +12,85 @@
end
describe "POST /runners/register" do
describe "should create a runner if token provided" do
context 'when runner token is provided' do
before { post ci_api("/runners/register"), token: registration_token }
it { expect(response.status).to eq(201) }
it 'creates runner with default values' do
expect(response).to have_http_status 201
expect(Ci::Runner.first.run_untagged).to be true
end
end
describe "should create a runner with description" do
before { post ci_api("/runners/register"), token: registration_token, description: "server.hostname" }
context 'when runner description is provided' do
before do
post ci_api("/runners/register"), token: registration_token,
description: "server.hostname"
end
it { expect(response.status).to eq(201) }
it { expect(Ci::Runner.first.description).to eq("server.hostname") }
it 'creates runner' do
expect(response).to have_http_status 201
expect(Ci::Runner.first.description).to eq("server.hostname")
end
end
describe "should create a runner with tags" do
before { post ci_api("/runners/register"), token: registration_token, tag_list: "tag1, tag2" }
context 'when runner tags are provided' do
before do
post ci_api("/runners/register"), token: registration_token,
tag_list: "tag1, tag2"
end
it { expect(response.status).to eq(201) }
it { expect(Ci::Runner.first.tag_list.sort).to eq(["tag1", "tag2"]) }
it 'creates runner' do
expect(response).to have_http_status 201
expect(Ci::Runner.first.tag_list.sort).to eq(["tag1", "tag2"])
end
end
describe "should create a runner if project token provided" do
context 'when option for running untagged jobs is provided' do
context 'when tags are provided' do
it 'creates runner' do
post ci_api("/runners/register"), token: registration_token,
run_untagged: false,
tag_list: ['tag']
expect(response).to have_http_status 201
expect(Ci::Runner.first.run_untagged).to be false
end
end
context 'when tags are not provided' do