diff options
46 files changed, 531 insertions, 171 deletions
diff --git a/CHANGELOG b/CHANGELOG index f26512c87ba..2f42aa34990 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -71,6 +71,7 @@ v 8.9.0 (unreleased) - Todos will display target state if issuable target is 'Closed' or 'Merged' - Validate only and except regexp - Fix bug when sorting issues by milestone due date and filtering by two or more labels + - POST to API /projects/:id/runners/:runner_id would give 409 if the runner was already enabled for this project - Add support for using Yubikeys (U2F) for two-factor authentication - Link to blank group icon doesn't throw a 404 anymore - Remove 'main language' feature @@ -82,9 +83,11 @@ v 8.9.0 (unreleased) - Projects pending deletion will render a 404 page - Measure queue duration between gitlab-workhorse and Rails - Added Gfm autocomplete for labels + - Added edit note 'up' shortcut documentation to the help panel and docs screenshot #18114 - Make Omniauth providers specs to not modify global configuration - Remove unused JiraIssue class and replace references with ExternalIssue. !4659 (Ilan Shamir) - Make authentication service for Container Registry to be compatible with < Docker 1.11 + - Make it possible to lock a runner from being enabled for other projects - Add Application Setting to configure Container Registry token expire delay (default 5min) - Cache assigned issue and merge request counts in sidebar nav - Use Knapsack only in CI environment @@ -131,9 +134,11 @@ v 8.9.0 (unreleased) - Various associations are now eager loaded when parsing issue references to reduce the number of queries executed - Set inverse_of for Project/Service association to reduce the number of queries - Update tanuki logo highlight/loading colors + - Remove explicit Gitlab::Metrics.action assignments, are already automatic. - Use Git cached counters for branches and tags on project page - Filter parameters for request_uri value on instrumented transactions. - Remove duplicated keys add UNIQUE index to keys fingerprint column + - ExtractsPath get ref_names from repository cache, if not there access git. - Cache user todo counts from TodoService - Ensure Todos counters doesn't count Todos for projects pending delete diff --git a/app/assets/javascripts/notes.js.coffee b/app/assets/javascripts/notes.js.coffee index e2d3241437b..17f7e180127 100644 --- a/app/assets/javascripts/notes.js.coffee +++ b/app/assets/javascripts/notes.js.coffee @@ -102,12 +102,15 @@ class @Notes keydownNoteText: (e) -> $this = $(this) - if $this.val() is '' and e.which is 38 #aka the up key + if $this.val() is '' and e.which is 38 and not isMetaKey e myLastNote = $("li.note[data-author-id='#{gon.current_user_id}'][data-editable]:last") if myLastNote.length myLastNoteEditBtn = myLastNote.find('.js-note-edit') myLastNoteEditBtn.trigger('click', [true, myLastNote]) + isMetaKey = (e) -> + (e.metaKey or e.ctrlKey or e.altKey or e.shiftKey) + initRefresh: -> clearInterval(Notes.interval) Notes.interval = setInterval => diff --git a/app/assets/stylesheets/framework/markdown_area.scss b/app/assets/stylesheets/framework/markdown_area.scss index 25f9c50258e..fd8eaa8a691 100644 --- a/app/assets/stylesheets/framework/markdown_area.scss +++ b/app/assets/stylesheets/framework/markdown_area.scss @@ -102,6 +102,8 @@ white-space: pre-wrap; word-break: keep-all; } + + @include bulleted-list; } } diff --git a/app/assets/stylesheets/framework/mixins.scss b/app/assets/stylesheets/framework/mixins.scss index 828e7224231..5ec5a96a597 100644 --- a/app/assets/stylesheets/framework/mixins.scss +++ b/app/assets/stylesheets/framework/mixins.scss @@ -110,3 +110,17 @@ font-size: 16px; line-height: 24px; } + +@mixin bulleted-list { + > ul { + list-style-type: disc; + + ul { + list-style-type: circle; + + ul { + list-style-type: square; + } + } + } +}
\ No newline at end of file diff --git a/app/assets/stylesheets/framework/nav.scss b/app/assets/stylesheets/framework/nav.scss index a55918f8711..5c68f90e343 100644 --- a/app/assets/stylesheets/framework/nav.scss +++ b/app/assets/stylesheets/framework/nav.scss @@ -136,7 +136,7 @@ } /* Small devices (phones, tablets, 768px and lower) */ - @media (max-width: $screen-sm-max) { + @media (max-width: $screen-xs-max) { width: 100%; } } @@ -220,6 +220,7 @@ form { display: block; height: auto; + margin-bottom: 14px; input { width: 100%; diff --git a/app/assets/stylesheets/pages/diff.scss b/app/assets/stylesheets/pages/diff.scss index 1a7d5f9666e..5286b73cc50 100644 --- a/app/assets/stylesheets/pages/diff.scss +++ b/app/assets/stylesheets/pages/diff.scss @@ -4,6 +4,11 @@ margin-bottom: $gl-padding; border-radius: 3px; + .commit-short-id { + font-family: $regular_font; + font-weight: 400; + } + .diff-header { position: relative; background: $background-color; diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss index e67271adfb1..aca82f7f7bf 100644 --- a/app/assets/stylesheets/pages/merge_requests.scss +++ b/app/assets/stylesheets/pages/merge_requests.scss @@ -119,7 +119,12 @@ margin-bottom: 0; } - @media (max-width: $screen-sm-max) { + .btn-grouped { + margin-left: 0; + margin-right: 7px; + } + + @media (max-width: $screen-xs-max) { h4 { font-size: 15px; } @@ -131,10 +136,14 @@ .btn, .btn-group, .accept-action { - width: 100%; margin-bottom: 4px; } + .accept-action { + width: 100%; + text-align: center; + } + .accept-control { width: 100%; text-align: center; @@ -284,7 +293,7 @@ margin-bottom: 0; } - @media (min-width: $screen-sm-min) { + @media (min-width: $screen-xs-min) { float: left; width: 50%; margin-bottom: 0; diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index df0c241bbf1..ffba3dc5bc6 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -84,24 +84,14 @@ ul.notes { word-wrap: break-word; @include md-typography; + // Reset ul style types since we're nested inside a ul already + @include bulleted-list; + // On diffs code should wrap nicely and not overflow code { white-space: pre-wrap; } - // Reset ul style types since we're nested inside a ul already - & > ul { - list-style-type: disc; - - ul { - list-style-type: circle; - - ul { - list-style-type: square; - } - } - } - ul.task-list { ul:not(.task-list) { padding-left: 1.3em; diff --git a/app/assets/stylesheets/pages/projects.scss b/app/assets/stylesheets/pages/projects.scss index 093d5e18516..346badf6d86 100644 --- a/app/assets/stylesheets/pages/projects.scss +++ b/app/assets/stylesheets/pages/projects.scss @@ -373,7 +373,7 @@ a.deploy-project-label { .project-stats { margin-top: $gl-padding; margin-bottom: 0; - padding: 16px 0; + padding: 0; background-color: $white-light; font-size: 0; @@ -382,13 +382,14 @@ a.deploy-project-label { } .nav li { - display: inline; + display: inline-block; + margin: 16px 0; + margin-right: 16px; } .nav > li > a { background-color: transparent; - margin-right: 12px; - padding: 0 10px; + padding: 5px 10px; font-size: 15px; color: $notes-light-color; } @@ -402,12 +403,17 @@ a.deploy-project-label { font-size: 17px; } - li.missing a { - color: #5a6069; - border: 1px dashed #dce0e5; + li.missing { + border: 1px dashed $border-gray-light; + border-radius: $border-radius-default; + + a { + color: $notes-light-color; + display: block; + } &:hover { - background-color: #f0f2f5; + background-color: $gray-normal; } } diff --git a/app/controllers/admin/runner_projects_controller.rb b/app/controllers/admin/runner_projects_controller.rb index d25619d94e0..bf20c5305a7 100644 --- a/app/controllers/admin/runner_projects_controller.rb +++ b/app/controllers/admin/runner_projects_controller.rb @@ -1,15 +1,14 @@ class Admin::RunnerProjectsController < Admin::ApplicationController before_action :project, only: [:create] - def index - @runner_projects = project.runner_projects.all - @runner_project = project.runner_projects.new - end - def create @runner = Ci::Runner.find(params[:runner_project][:runner_id]) - if @runner.assign_to(@project, current_user) + return head(403) if @runner.is_shared? || @runner.locked? + + runner_project = @runner.assign_to(@project, current_user) + + if runner_project.persisted? redirect_to admin_runner_path(@runner) else redirect_to admin_runner_path(@runner), alert: 'Failed adding runner to project' diff --git a/app/controllers/projects/application_controller.rb b/app/controllers/projects/application_controller.rb index 776ba92c9ab..996909a28c6 100644 --- a/app/controllers/projects/application_controller.rb +++ b/app/controllers/projects/application_controller.rb @@ -74,7 +74,7 @@ class Projects::ApplicationController < ApplicationController end def require_branch_head - unless @repository.branch_names.include?(@ref) + unless @repository.branch_exists?(@ref) redirect_to( namespace_project_tree_path(@project.namespace, @project, @ref), notice: "This action is not allowed unless you are on a branch" diff --git a/app/controllers/projects/runner_projects_controller.rb b/app/controllers/projects/runner_projects_controller.rb index bedeb4a295c..dc1a18f8d42 100644 --- a/app/controllers/projects/runner_projects_controller.rb +++ b/app/controllers/projects/runner_projects_controller.rb @@ -6,11 +6,13 @@ class Projects::RunnerProjectsController < Projects::ApplicationController def create @runner = Ci::Runner.find(params[:runner_project][:runner_id]) + return head(403) if @runner.is_shared? || @runner.locked? return head(403) unless current_user.ci_authorized_runners.include?(@runner) path = runners_path(project) + runner_project = @runner.assign_to(project, current_user) - if @runner.assign_to(project, current_user) + if runner_project.persisted? redirect_to path else redirect_to path, alert: 'Failed adding runner to project' diff --git a/app/controllers/projects/runners_controller.rb b/app/controllers/projects/runners_controller.rb index 0b4fa572501..53c36635efe 100644 --- a/app/controllers/projects/runners_controller.rb +++ b/app/controllers/projects/runners_controller.rb @@ -5,10 +5,9 @@ class Projects::RunnersController < Projects::ApplicationController layout 'project_settings' def index - @runners = project.runners.ordered - @specific_runners = current_user.ci_authorized_runners. - where.not(id: project.runners). - ordered.page(params[:page]).per(20) + @project_runners = project.runners.ordered + @assignable_runners = current_user.ci_authorized_runners. + assignable_for(project).ordered.page(params[:page]).per(20) @shared_runners = Ci::Runner.shared.active @shared_runners_count = @shared_runners.count(:all) end diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 78ceaf3237f..2b1f50fd01e 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -303,8 +303,14 @@ class ProjectsController < Projects::ApplicationController project.repository_exists? && !project.empty_repo? end - # Override get_id from ExtractsPath, which returns the branch and file path + # Override extract_ref from ExtractsPath, which returns the branch and file path # for the blob/tree, which in this case is just the root of the default branch. + # This way we avoid to access the repository.ref_names. + def extract_ref(_id) + [get_id, ''] + end + + # Override get_id from ExtractsPath in this case is just the root of the default branch. def get_id project.repository.root_ref end diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 82421d74de9..41859841834 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -116,7 +116,7 @@ module ApplicationHelper return false if project.merge_requests.where(source_branch: event.branch_name).opened.any? # Skip if user removed branch right after that - return false unless project.repository.branch_names.include?(event.branch_name) + return false unless project.repository.branch_exists?(event.branch_name) true end diff --git a/app/helpers/branches_helper.rb b/app/helpers/branches_helper.rb index 3ee3fc74f0c..c533659b600 100644 --- a/app/helpers/branches_helper.rb +++ b/app/helpers/branches_helper.rb @@ -10,7 +10,7 @@ module BranchesHelper end def can_push_branch?(project, branch_name) - return false unless project.repository.branch_names.include?(branch_name) + return false unless project.repository.branch_exists?(branch_name) ::Gitlab::GitAccess.new(current_user, project).can_push_to_branch?(branch_name) end diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index d618c84e983..2b0bec33131 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -300,18 +300,12 @@ module Ci project.valid_runners_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) } + project.any_runners? { |runner| runner.active? && runner.online? && runner.can_pick?(self) } end def stuck? diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb index adb65292208..b64ec79ec2b 100644 --- a/app/models/ci/runner.rb +++ b/app/models/ci/runner.rb @@ -4,7 +4,7 @@ module Ci LAST_CONTACT_TIME = 5.minutes.ago AVAILABLE_SCOPES = %w[specific shared active paused online] - FORM_EDITABLE = %i[description tag_list active run_untagged] + FORM_EDITABLE = %i[description tag_list active run_untagged locked] has_many :builds, class_name: 'Ci::Build' has_many :runner_projects, dependent: :destroy, class_name: 'Ci::RunnerProject' @@ -26,6 +26,13 @@ module Ci .where("ci_runner_projects.gl_project_id = :project_id OR ci_runners.is_shared = true", project_id: project_id) end + scope :assignable_for, ->(project) do + # FIXME: That `to_sql` is needed to workaround a weird Rails bug. + # Without that, placeholders would miss one and couldn't match. + where(locked: false). + where.not("id IN (#{project.runners.select(:id).to_sql})").specific + end + validate :tag_constraints acts_as_taggable @@ -56,7 +63,7 @@ module Ci def assign_to(project, current_user = nil) self.is_shared = false if shared? self.save - project.runner_projects.create!(runner_id: self.id) + project.runner_projects.create(runner_id: self.id) end def display_name @@ -91,6 +98,10 @@ module Ci !shared? end + def can_pick?(build) + assignable_for?(build.project) && accepting_tags?(build) + end + def only_for?(project) projects == [project] end @@ -111,5 +122,13 @@ module Ci 'can not be empty when runner is not allowed to pick untagged jobs') end end + + def assignable_for?(project) + !locked? || projects.exists?(id: project.id) + end + + def accepting_tags?(build) + (run_untagged? || build.has_tags?) && (build.tag_list - tag_list).empty? + end end end diff --git a/app/models/repository.rb b/app/models/repository.rb index bbd7682d8e7..221c87164ca 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -191,8 +191,12 @@ class Repository end end + def ref_names + branch_names + tag_names + end + def branch_names - cache.fetch(:branch_names) { branches.map(&:name) } + @branch_names ||= cache.fetch(:branch_names) { branches.map(&:name) } end def branch_exists?(branch_name) @@ -267,6 +271,7 @@ class Repository def expire_branches_cache cache.expire(:branch_names) + @branch_names = nil @local_branches = nil end @@ -332,10 +337,6 @@ class Repository @lookup_cache ||= {} end - def expire_branch_names - cache.expire(:branch_names) - end - def expire_avatar_cache(branch_name = nil, revision = nil) # Avatars are pulled from the default branch, thus if somebody pushes to a # different branch there's no need to expire anything. diff --git a/app/models/user.rb b/app/models/user.rb index 2e458329cb9..876ccc69d8d 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -487,9 +487,8 @@ class User < ActiveRecord::Base events.recent.find do |event| project = Project.find_by_id(event.project_id) next unless project - repo = project.repository - if repo.branch_names.include?(event.branch_name) + if project.repository.branch_exists?(event.branch_name) merge_requests = MergeRequest.where("created_at >= ?", event.created_at). where(source_project_id: project.id, source_branch: event.branch_name) diff --git a/app/services/ci/register_build_service.rb b/app/services/ci/register_build_service.rb index f0ed09a629a..9a187f5d694 100644 --- a/app/services/ci/register_build_service.rb +++ b/app/services/ci/register_build_service.rb @@ -21,7 +21,7 @@ module Ci end build = builds.find do |build| - build.can_be_served?(current_runner) + current_runner.can_pick?(build) end if build diff --git a/app/views/admin/runners/show.html.haml b/app/views/admin/runners/show.html.haml index e049b40bfab..61abfc6ecbe 100644 --- a/app/views/admin/runners/show.html.haml +++ b/app/views/admin/runners/show.html.haml @@ -28,7 +28,7 @@ .col-md-6 %h4 Restrict projects for this runner - if @runner.projects.any? - %table.table + %table.table.assigned-projects %thead %tr %th Assigned projects @@ -44,7 +44,7 @@ .pull-right = link_to 'Disable', [:admin, project.namespace.becomes(Namespace), project, runner_project], method: :delete, class: 'btn btn-danger btn-xs' - %table.table + %table.table.unassigned-projects %thead %tr %th Project diff --git a/app/views/help/_shortcuts.html.haml b/app/views/help/_shortcuts.html.haml index 01648047ce2..8cc0b59edeb 100644 --- a/app/views/help/_shortcuts.html.haml +++ b/app/views/help/_shortcuts.html.haml @@ -28,8 +28,12 @@ .key ⌘ shift p - else .key ctrl shift p - %td Toggle Markdown preview + %tr + %td.shortcut + .key + %i.fa.fa-arrow-up + %td Edit last comment (when focused on an empty textarea) %tbody %tr %th diff --git a/app/views/projects/runners/_form.html.haml b/app/views/projects/runners/_form.html.haml index d62f5c8f131..c45a9d4f81f 100644 --- a/app/views/projects/runners/_form.html.haml +++ b/app/views/projects/runners/_form.html.haml @@ -13,6 +13,12 @@ = f.check_box :run_untagged %span.light Indicates whether this runner can pick jobs without tags .form-group + = label :locked, 'Lock to current projects', class: 'control-label' + .col-sm-10 + .checkbox + = f.check_box :locked + %span.light When a runner is locked, it cannot be assigned to other projects + .form-group = label_tag :token, class: 'control-label' do Token .col-sm-10 diff --git a/app/views/projects/runners/_runner.html.haml b/app/views/projects/runners/_runner.html.haml index 96e2aac451f..85225857758 100644 --- a/app/views/projects/runners/_runner.html.haml +++ b/app/views/projects/runners/_runner.html.haml @@ -2,8 +2,10 @@ %h4 = runner_status_icon(runner) %span.monospace - - if @runners.include?(runner) + - if @project_runners.include?(runner) = link_to runner.short_sha, runner_path(runner) + - if runner.locked? + = icon('lock', class: 'has-tooltip', title: 'Locked to current projects') %small = link_to edit_namespace_project_runner_path(@project.namespace, @project, runner) do %i.fa.fa-edit.btn @@ -11,7 +13,7 @@ = runner.short_sha .pull-right - - if @runners.include?(runner) + - if @project_runners.include?(runner) - if runner.belongs_to_one_project? = link_to 'Remove runner', runner_path(runner), data: { confirm: "Are you sure?" }, method: :delete, class: 'btn btn-danger btn-sm' - else diff --git a/app/views/projects/runners/_specific_runners.html.haml b/app/views/projects/runners/_specific_runners.html.haml index 8ae9f0d95f7..d469dda5b81 100644 --- a/app/views/projects/runners/_specific_runners.html.haml +++ b/app/views/projects/runners/_specific_runners.html.haml @@ -17,13 +17,13 @@ Start runner! -- if @runners.any? +- if @project_runners.any? %h4.underlined-title Runners activated for this project %ul.bordered-list.activated-specific-runners - = render partial: 'runner', collection: @runners, as: :runner + = render partial: 'runner', collection: @project_runners, as: :runner -- if @specific_runners.any? +- if @assignable_runners.any? %h4.underlined-title Available specific runners %ul.bordered-list.available-specific-runners - = render partial: 'runner', collection: @specific_runners, as: :runner - = paginate @specific_runners + = render partial: 'runner', collection: @assignable_runners, as: :runner + = paginate @assignable_runners diff --git a/app/views/projects/runners/show.html.haml b/app/views/projects/runners/show.html.haml index f24e1b9144e..61b99f35d74 100644 --- a/app/views/projects/runners/show.html.haml +++ b/app/views/projects/runners/show.html.haml @@ -23,6 +23,9 @@ %td Can run untagged jobs %td= @runner.run_untagged? ? 'Yes' : 'No' %tr + %td Locked to this project + %td= @runner.locked? ? 'Yes' : 'No' + %tr %td Tags %td - @runner.tag_list.each do |tag| diff --git a/config/routes.rb b/config/routes.rb index a22559ebabc..e45293cdf7f 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -295,7 +295,7 @@ Rails.application.routes.draw do post :repository_check end - resources :runner_projects + resources :runner_projects, only: [:create, :destroy] end end diff --git a/db/migrate/20160509091049_add_locked_to_ci_runner.rb b/db/migrate/20160509091049_add_locked_to_ci_runner.rb new file mode 100644 index 00000000000..3fbaef3b7f0 --- /dev/null +++ b/db/migrate/20160509091049_add_locked_to_ci_runner.rb @@ -0,0 +1,13 @@ +class AddLockedToCiRunner < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + disable_ddl_transaction! + + def up + add_column_with_default(:ci_runners, :locked, :boolean, + default: false, allow_null: false) + end + + def down + remove_column(:ci_runners, :locked) + end +end diff --git a/db/migrate/20160620115026_add_index_on_runners_locked.rb b/db/migrate/20160620115026_add_index_on_runners_locked.rb new file mode 100644 index 00000000000..dfa5110dea4 --- /dev/null +++ b/db/migrate/20160620115026_add_index_on_runners_locked.rb @@ -0,0 +1,12 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddIndexOnRunnersLocked < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + disable_ddl_transaction! + + def change + add_concurrent_index :ci_runners, :locked + end +end diff --git a/db/schema.rb b/db/schema.rb index 29f90c47848..7a8377f687c 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20160617301627) do +ActiveRecord::Schema.define(version: 20160620115026) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -287,9 +287,11 @@ ActiveRecord::Schema.define(version: 20160617301627) do t.string "platform" t.string "architecture" t.boolean "run_untagged", default: true, null: false + t.boolean "locked", default: false, null: false end add_index "ci_runners", ["description"], name: "index_ci_runners_on_description_trigram", using: :gin, opclasses: {"description"=>"gin_trgm_ops"} + add_index "ci_runners", ["locked"], name: "index_ci_runners_on_locked", using: :btree add_index "ci_runners", ["token"], name: "index_ci_runners_on_token", using: :btree add_index "ci_runners", ["token"], name: "index_ci_runners_on_token_trigram", using: :gin, opclasses: {"token"=>"gin_trgm_ops"} diff --git a/doc/ci/runners/README.md b/doc/ci/runners/README.md index 400784da617..ddebd987650 100644 --- a/doc/ci/runners/README.md +++ b/doc/ci/runners/README.md @@ -96,6 +96,12 @@ To register the runner, run the command below and follow instructions: sudo gitlab-ci-multi-runner register ``` +### Lock a specific runner from being enabled for other projects + +You can configure a runner to assign it exclusively to a project. When a +runner is locked this way, it can no longer be enabled for other projects. +This setting is available on each runner in *Project Settings* > *Runners*. + ### Making an existing Shared Runner Specific If you are an admin on your GitLab instance, @@ -128,7 +134,7 @@ the appropriate dependencies to run Rails test suites. ### 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 +the runner does not have tags assigned. This setting is available on each runner in *Project Settings* > *Runners*. ### Be careful with sensitive information diff --git a/doc/project_services/emails_on_push.md b/doc/project_services/emails_on_push.md new file mode 100644 index 00000000000..2f9f36f962e --- /dev/null +++ b/doc/project_services/emails_on_push.md @@ -0,0 +1,17 @@ +## Enabling emails on push + +To receive email notifications for every change that is pushed to the project, visit +your project's **Settings > Services > Emails on push** and activate the service. + +In the _Recipients_ area, provide a list of emails separated by commas. + +You can configure any of the following settings depending on your preference. + ++ **Push events** - Email will be triggered when a push event is recieved ++ **Tag push events** - Email will be triggered when a tag is created and pushed ++ **Send from committer** - Send notifications from the committer's email address if the domain is part of the domain GitLab is running on (e.g. `user@gitlab.com`). ++ **Disable code diffs** - Don't include possibly sensitive code diffs in notification body. + +--- + + diff --git a/doc/project_services/img/emails_on_push_service.png b/doc/project_services/img/emails_on_push_service.png Binary files differnew file mode 100644 index 00000000000..cd6f79ad1eb --- /dev/null +++ b/doc/project_services/img/emails_on_push_service.png diff --git a/doc/project_services/project_services.md b/doc/project_services/project_services.md index a5af620d9be..f81a035f70b 100644 --- a/doc/project_services/project_services.md +++ b/doc/project_services/project_services.md @@ -33,7 +33,7 @@ further configuration instructions and details. Contributions are welcome. | Campfire | Simple web-based real-time group chat | | Custom Issue Tracker | Custom issue tracker | | Drone CI | Continuous Integration platform built on Docker, written in Go | -| Emails on push | Email the commits and diff of each push to a list of recipients | +| [Emails on push](emails_on_push.md) | Email the commits and diff of each push to a list of recipients | | External Wiki | Replaces the link to the internal wiki with a link to an external wiki | | Flowdock | Flowdock is a collaboration web app for technical teams | | Gemnasium | Gemnasium monitors your project dependencies and alerts you about updates and security vulnerabilities | diff --git a/doc/workflow/shortcuts.png b/doc/workflow/shortcuts.png Binary files differindex beb6c53ec77..16be0413b64 100644 --- a/doc/workflow/shortcuts.png +++ b/doc/workflow/shortcuts.png diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 0c9fc5604fd..0ee96d4c67b 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -423,6 +423,7 @@ module API class RunnerDetails < Runner expose :tag_list expose :run_untagged + expose :locked expose :version, :revision, :platform, :architecture expose :contacted_at expose :token, if: lambda { |runner, options| options[:current_user].is_admin? || !runner.is_shared? } diff --git a/lib/api/internal.rb b/lib/api/internal.rb index 3ac7b50c4ce..1d361569d59 100644 --- a/lib/api/internal.rb +++ b/lib/api/internal.rb @@ -23,8 +23,6 @@ module API end post "/allowed" do - Gitlab::Metrics.action = 'Grape#/internal/allowed' - status 200 actor = diff --git a/lib/api/runners.rb b/lib/api/runners.rb index 4faba9dc87b..ecc8f2fc5a2 100644 --- a/lib/api/runners.rb +++ b/lib/api/runners.rb @@ -49,7 +49,7 @@ module API runner = get_runner(params[:id]) authenticate_update_runner!(runner) - attrs = attributes_for_keys [:description, :active, :tag_list, :run_untagged] + attrs = attributes_for_keys [:description, :active, :tag_list, :run_untagged, :locked] if runner.update(attrs) present runner, with: Entities::RunnerDetails, current_user: current_user else @@ -96,9 +96,14 @@ module API runner = get_runner(params[:runner_id]) authenticate_enable_runner!(runner) - Ci::RunnerProject.create(runner: runner, project: user_project) - present runner, with: Entities::Runner + runner_project = runner.assign_to(user_project) + + if runner_project.persisted? + present runner, with: Entities::Runner + else + conflict!("Runner was already enabled for this project") + end end # Disable project's runner @@ -163,6 +168,7 @@ module API def authenticate_enable_runner!(runner) forbidden!("Runner is shared") if runner.is_shared? + forbidden!("Runner is locked") if runner.locked? return if current_user.is_admin? forbidden!("No access granted") unless user_can_access_runner?(runner) end diff --git a/lib/ci/api/runners.rb b/lib/ci/api/runners.rb index 0c41f22c7c5..bcc82969eb3 100644 --- a/lib/ci/api/runners.rb +++ b/lib/ci/api/runners.rb @@ -28,12 +28,9 @@ module Ci 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 + attributes = attributes_for_keys( + [:description, :tag_list, :run_untagged, :locked] + ) runner = if runner_registration_token_valid? diff --git a/spec/factories/ci/commits.rb b/spec/factories/ci/pipelines.rb index a039bef6f3c..a039bef6f3c 100644 --- a/spec/factories/ci/commits.rb +++ b/spec/factories/ci/pipelines.rb diff --git a/spec/features/admin/admin_runners_spec.rb b/spec/features/admin/admin_runners_spec.rb index 9499cd4e025..2d297776cb0 100644 --- a/spec/features/admin/admin_runners_spec.rb +++ b/spec/features/admin/admin_runners_spec.rb @@ -60,6 +60,40 @@ describe "Admin Runners" do it { expect(page).to have_content(@project1.name_with_namespace) } it { expect(page).not_to have_content(@project2.name_with_namespace) } end + + describe 'enable/create' do + before do + @project1.runners << runner + visit admin_runner_path(runner) + end + + it 'enables specific runner for 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 + + describe 'disable/destroy' do + before do + @project1.runners << runner + visit admin_runner_path(runner) + end + + it 'enables specific runner for project' do + within '.assigned-projects' do + click_on 'Disable' + end + + new_runner_project = page.find('.unassigned-projects') + + expect(new_runner_project).to have_content(@project1.path) + end + end end describe 'runners registration token' do diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb index 5d1fa8226e5..479b7309579 100644 --- a/spec/models/build_spec.rb +++ b/spec/models/build_spec.rb @@ -36,32 +36,44 @@ describe Ci::Build, models: true do subject { build.ignored? } context 'if build is not allowed to fail' do - before { build.allow_failure = false } + before do + build.allow_failure = false + end context 'and build.status is success' do - before { build.status = 'success' } + before do + build.status = 'success' + end it { is_expected.to be_falsey } end context 'and build.status is failed' do - before { build.status = 'failed' } + before do + build.status = 'failed' + end it { is_expected.to be_falsey } end end context 'if build is allowed to fail' do - before { build.allow_failure = true } + before do + build.allow_failure = true + end context 'and build.status is success' do - before { build.status = 'success' } + before do + build.status = 'success' + end it { is_expected.to be_falsey } end context 'and build.status is failed' do - before { build.status = 'failed' } + before do + build.status = 'failed' + end it { is_expected.to be_truthy } end @@ -75,7 +87,9 @@ describe Ci::Build, models: true do context 'if build.trace contains text' do let(:text) { 'example output' } - before { build.trace = text } + before do + build.trace = text + end it { is_expected.to include(text) } it { expect(subject.length).to be >= text.length } @@ -188,7 +202,9 @@ describe Ci::Build, models: true do ] end - before { build.update_attributes(stage: 'stage') } + before do + build.update_attributes(stage: 'stage') + end it { is_expected.to eq(predefined_variables + yaml_variables) } @@ -199,7 +215,9 @@ describe Ci::Build, models: true do ] end - before { build.update_attributes(tag: true) } + before do + build.update_attributes(tag: true) + end it { is_expected.to eq(tag_variable + predefined_variables + yaml_variables) } end @@ -257,57 +275,6 @@ describe Ci::Build, models: true do end end - describe '#can_be_served?' do - let(:runner) { create(:ci_runner) } - - before { build.project.runners << runner } - - 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 - - it 'cannot handle build with tags' do - build.tag_list = ['aa'] - expect(build.can_be_served?(runner)).to be_falsey - end - end - - context 'when runner has tags' do - before { runner.tag_list = ['bb', 'cc'] } - - 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 - - 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 - - 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']) } @@ -348,7 +315,7 @@ describe Ci::Build, models: true do end it 'that cannot handle build' do - expect_any_instance_of(Ci::Build).to receive(:can_be_served?).and_return(false) + expect_any_instance_of(Ci::Runner).to receive(:can_pick?).and_return(false) is_expected.to be_falsey end @@ -360,7 +327,9 @@ describe Ci::Build, models: true do %w(pending).each do |state| context "if commit_status.status is #{state}" do - before { build.status = state } + before do + build.status = state + end it { is_expected.to be_truthy } @@ -379,7 +348,9 @@ describe Ci::Build, models: true do %w(success failed canceled running).each do |state| context "if commit_status.status is #{state}" do - before { build.status = state } + before do + build.status = state + end it { is_expected.to be_falsey } end @@ -390,7 +361,10 @@ describe Ci::Build, models: true do subject { build.artifacts? } context 'artifacts archive does not exist' do - before { build.update_attributes(artifacts_file: nil) } + before do + build.update_attributes(artifacts_file: nil) + end + it { is_expected.to be_falsy } end @@ -623,7 +597,9 @@ describe Ci::Build, models: true do let!(:build) { create(:ci_build, :trace, :success, :artifacts) } describe '#erase' do - before { build.erase(erased_by: user) } + before do + build.erase(erased_by: user) + end context 'erased by user' do let!(:user) { create(:user, username: 'eraser') } @@ -660,7 +636,9 @@ describe Ci::Build, models: true do end context 'build has been erased' do - before { build.erase } + before do + build.erase + end it { is_expected.to be true } end @@ -668,7 +646,9 @@ describe Ci::Build, models: true do context 'metadata and build trace are not available' do let!(:build) { create(:ci_build, :success, :artifacts) } - before { build.remove_artifacts_metadata! } + before do + build.remove_artifacts_metadata! + end describe '#erase' do it 'should not raise error' do diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index 5d04d8ffcff..ef65eb99328 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -20,34 +20,36 @@ describe Ci::Runner, models: true do end describe '#display_name' do - it 'should return the description if it has a value' do + it 'returns the description if it has a value' do runner = FactoryGirl.build(:ci_runner, description: 'Linux/Ruby-1.9.3-p448') expect(runner.display_name).to eq 'Linux/Ruby-1.9.3-p448' end - it 'should return the token if it does not have a description' do + it 'returns the token if it does not have a description' do runner = FactoryGirl.create(:ci_runner) expect(runner.display_name).to eq runner.description end - it 'should return the token if the description is an empty string' do + it 'returns the token if the description is an empty string' do runner = FactoryGirl.build(:ci_runner, description: '', token: 'token') expect(runner.display_name).to eq runner.token end end - describe :assign_to do + describe '#assign_to' do let!(:project) { FactoryGirl.create :empty_project } let!(:shared_runner) { FactoryGirl.create(:ci_runner, :shared) } - before { shared_runner.assign_to(project) } + before do + shared_runner.assign_to(project) + end it { expect(shared_runner).to be_specific } it { expect(shared_runner.projects).to eq([project]) } it { expect(shared_runner.only_for?(project)).to be_truthy } end - describe :online do + describe '.online' do subject { Ci::Runner.online } before do @@ -58,60 +60,269 @@ describe Ci::Runner, models: true do it { is_expected.to eq([@runner2])} end - describe :online? do + describe '#online?' do let(:runner) { FactoryGirl.create(:ci_runner, :shared) } subject { runner.online? } context 'never contacted' do - before { runner.contacted_at = nil } + before do + runner.contacted_at = nil + end it { is_expected.to be_falsey } end context 'contacted long time ago time' do - before { runner.contacted_at = 1.year.ago } + before do + runner.contacted_at = 1.year.ago + end it { is_expected.to be_falsey } end context 'contacted 1s ago' do - before { runner.contacted_at = 1.second.ago } + before do + runner.contacted_at = 1.second.ago + end it { is_expected.to be_truthy } end end - describe :status do + describe '#can_pick?' do + let(:project) { create(:project) } + let(:pipeline) { create(:ci_pipeline, project: project) } + let(:build) { create(:ci_build, pipeline: pipeline) } + let(:runner) { create(:ci_runner) } + + before do + build.project.runners << runner + end + + context 'when runner does not have tags' do + it 'can handle builds without tags' do + expect(runner.can_pick?(build)).to be_truthy + end + + it 'cannot handle build with tags' do + build.tag_list = ['aa'] + + expect(runner.can_pick?(build)).to be_falsey + end + end + + context 'when runner has tags' do + before do + runner.tag_list = ['bb', 'cc'] + end + + shared_examples 'tagged build picker' do + it 'can handle build with matching tags' do + build.tag_list = ['bb'] + + expect(runner.can_pick?(build)).to be_truthy + end + + it 'cannot handle build without matching tags' do + build.tag_list = ['aa'] + + expect(runner.can_pick?(build)).to be_falsey + end + end + + context 'when runner can pick untagged jobs' do + it 'can handle builds without tags' do + expect(runner.can_pick?(build)).to be_truthy + end + + it_behaves_like 'tagged build picker' + end + + context 'when runner cannot pick untagged jobs' do + before do + runner.run_untagged = false + end + + it 'cannot handle builds without tags' do + expect(runner.can_pick?(build)).to be_falsey + end + + it_behaves_like 'tagged build picker' + end + end + + context 'when runner is locked' do + before do + runner.locked = true + end + + shared_examples 'locked build picker' do + context 'when runner cannot pick untagged jobs' do + before do + runner.run_untagged = false + end + + it 'cannot handle builds without tags' do + expect(runner.can_pick?(build)).to be_falsey + end + end + + context 'when having runner tags' do + before do + runner.tag_list = ['bb', 'cc'] + end + + it 'cannot handle it for builds without matching tags' do + build.tag_list = ['aa'] + + expect(runner.can_pick?(build)).to be_falsey + end + end + end + + context 'when serving the same project' do + it 'can handle it' do + expect(runner.can_pick?(build)).to be_truthy + end + + it_behaves_like 'locked build picker' + + context 'when having runner tags' do + before do + runner.tag_list = ['bb', 'cc'] + build.tag_list = ['bb'] + end + + it 'can handle it for matching tags' do + expect(runner.can_pick?(build)).to be_truthy + end + end + end + + context 'serving a different project' do + before do + runner.runner_projects.destroy_all + end + + it 'cannot handle it' do + expect(runner.can_pick?(build)).to be_falsey + end + + it_behaves_like 'locked build picker' + + context 'when having runner tags' do + before do + runner.tag_list = ['bb', 'cc'] + build.tag_list = ['bb'] + end + + it 'cannot handle it for matching tags' do + expect(runner.can_pick?(build)).to be_falsey + end + end + end + end + end + + describe '#status' do let(:runner) { FactoryGirl.create(:ci_runner, :shared, contacted_at: 1.second.ago) } subject { runner.status } context 'never connected' do - before { runner.contacted_at = nil } + before do + runner.contacted_at = nil + end it { is_expected.to eq(:not_connected) } end context 'contacted 1s ago' do - before { runner.contacted_at = 1.second.ago } + before do + runner.contacted_at = 1.second.ago + end it { is_expected.to eq(:online) } end context 'contacted long time ago' do - before { runner.contacted_at = 1.year.ago } + before do + runner.contacted_at = 1.year.ago + end it { is_expected.to eq(:offline) } end context 'inactive' do - before { runner.active = false } + before do + runner.active = false + end it { is_expected.to eq(:paused) } end end + describe '.assignable_for' do + let(:runner) { create(:ci_runner) } + 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 { Ci::Runner.assignable_for(project) } + + it { is_expected.to be_empty } + end + + context 'does not give shared runner' do + subject { Ci::Runner.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 { Ci::Runner.assignable_for(project) } + + it { is_expected.to be_empty } + end + + context 'does give a specific runner' do + subject { Ci::Runner.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 { Ci::Runner.assignable_for(project) } + + it { is_expected.to be_empty } + end + + context 'does not give a locked runner' do + subject { Ci::Runner.assignable_for(another_project) } + + it { is_expected.to be_empty } + end + end + end + describe "belongs_to_one_project?" do it "returns false if there are two projects runner assigned to" do runner = FactoryGirl.create(:ci_runner) diff --git a/spec/requests/api/runners_spec.rb b/spec/requests/api/runners_spec.rb index 73ae8ef631c..b4c826522a5 100644 --- a/spec/requests/api/runners_spec.rb +++ b/spec/requests/api/runners_spec.rb @@ -187,14 +187,16 @@ describe API::Runners, api: true do update_runner(shared_runner.id, admin, description: "#{description}_updated", active: !active, tag_list: ['ruby2.1', 'pgsql', 'mysql'], - run_untagged: 'false') + run_untagged: 'false', + locked: 'true') 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 + expect(shared_runner.run_untagged?).to be(false) + expect(shared_runner.locked?).to be(true) end end @@ -360,11 +362,13 @@ describe API::Runners, api: true do describe 'POST /projects/:id/runners' do context 'authorized user' do - it 'should enable specific runner' do - specific_runner2 = create(:ci_runner).tap do |runner| + let(:specific_runner2) do + create(:ci_runner).tap do |runner| create(:ci_runner_project, runner: runner, project: project2) end + end + it 'should enable specific runner' do expect do post api("/projects/#{project.id}/runners", user), runner_id: specific_runner2.id end.to change{ project.runners.count }.by(+1) @@ -375,7 +379,17 @@ describe API::Runners, api: true do expect do post api("/projects/#{project.id}/runners", user), runner_id: specific_runner.id end.to change{ project.runners.count }.by(0) - expect(response.status).to eq(201) + expect(response.status).to eq(409) + end + + it 'should not enable locked runner' do + specific_runner2.update(locked: true) + + expect do + post api("/projects/#{project.id}/runners", user), runner_id: specific_runner2.id + end.to change{ project.runners.count }.by(0) + + expect(response.status).to eq(403) end it 'should not enable shared runner' do diff --git a/spec/workers/merge_worker_spec.rb b/spec/workers/merge_worker_spec.rb index 1abd87d7d33..b5e1fdb8ded 100644 --- a/spec/workers/merge_worker_spec.rb +++ b/spec/workers/merge_worker_spec.rb @@ -9,7 +9,7 @@ describe MergeWorker do before do source_project.team << [author, :master] - source_project.repository.expire_branch_names + source_project.repository.expire_branches_cache end it 'clears cache of source repo after removing source branch' do |
