diff options
35 files changed, 473 insertions, 49 deletions
@@ -20,7 +20,7 @@ gem 'rugged', '~> 0.24.0' # Authentication libraries gem 'devise', '~> 4.2' gem 'doorkeeper', '~> 4.2.0' -gem 'omniauth', '~> 1.3.2' +gem 'omniauth', '~> 1.4.2' gem 'omniauth-auth0', '~> 1.4.1' gem 'omniauth-azure-oauth2', '~> 0.0.6' gem 'omniauth-cas3', '~> 1.1.2' diff --git a/Gemfile.lock b/Gemfile.lock index b46b09c7544..5ff18442c4f 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -328,7 +328,7 @@ GEM temple (~> 0.7.6) thor tilt - hashie (3.4.4) + hashie (3.5.5) health_check (2.2.1) rails (>= 4.0) hipchat (1.5.2) @@ -441,7 +441,7 @@ GEM octokit (4.6.2) sawyer (~> 0.8.0, >= 0.5.3) oj (2.17.4) - omniauth (1.3.2) + omniauth (1.4.2) hashie (>= 1.2, < 4) rack (>= 1.0, < 3) omniauth-auth0 (1.4.1) @@ -920,7 +920,7 @@ DEPENDENCIES oauth2 (~> 1.2.0) octokit (~> 4.6.2) oj (~> 2.17.4) - omniauth (~> 1.3.2) + omniauth (~> 1.4.2) omniauth-auth0 (~> 1.4.1) omniauth-authentiq (~> 0.3.0) omniauth-azure-oauth2 (~> 0.0.6) diff --git a/app/assets/javascripts/lib/utils/common_utils.js.es6 b/app/assets/javascripts/lib/utils/common_utils.js.es6 index dbf40ec7fcf..0242350f718 100644 --- a/app/assets/javascripts/lib/utils/common_utils.js.es6 +++ b/app/assets/javascripts/lib/utils/common_utils.js.es6 @@ -329,17 +329,18 @@ * ``` */ w.gl.utils.backOff = (fn, timeout = 60000) => { + const maxInterval = 32000; let nextInterval = 2000; - const startTime = (+new Date()); + const startTime = Date.now(); return new Promise((resolve, reject) => { const stop = arg => ((arg instanceof Error) ? reject(arg) : resolve(arg)); const next = () => { - if (new Date().getTime() - startTime < timeout) { + if (Date.now() - startTime < timeout) { setTimeout(fn.bind(null, next, stop), nextInterval); - nextInterval *= 2; + nextInterval = Math.min(nextInterval + nextInterval, maxInterval); } else { reject(new Error('BACKOFF_TIMEOUT')); } diff --git a/app/assets/javascripts/lib/utils/http_status.js b/app/assets/javascripts/lib/utils/http_status.js new file mode 100644 index 00000000000..bc109a69c20 --- /dev/null +++ b/app/assets/javascripts/lib/utils/http_status.js @@ -0,0 +1,10 @@ +/** + * exports HTTP status codes + */ + +const statusCodes = { + NO_CONTENT: 204, + OK: 200, +}; + +module.exports = statusCodes; diff --git a/app/assets/javascripts/task_list.js b/app/assets/javascripts/task_list.js index dfe24d1fb33..b1402c0a880 100644 --- a/app/assets/javascripts/task_list.js +++ b/app/assets/javascripts/task_list.js @@ -1,3 +1,4 @@ +/* global Flash */ require('vendor/task_list'); class TaskList { @@ -6,6 +7,16 @@ class TaskList { this.dataType = options.dataType; this.fieldName = options.fieldName; this.onSuccess = options.onSuccess || (() => {}); + this.onError = function showFlash(response) { + let errorMessages = ''; + + if (response.responseJSON) { + errorMessages = response.responseJSON.errors.join(' '); + } + + return new Flash(errorMessages || 'Update failed', 'alert'); + }; + this.init(); } @@ -32,6 +43,7 @@ class TaskList { url: $target.data('update-url') || $('form.js-issuable-update').attr('action'), data: patchData, success: this.onSuccess, + error: this.onError, }); } } diff --git a/app/controllers/concerns/issuable_actions.rb b/app/controllers/concerns/issuable_actions.rb index 0821974aa93..3ccf2a9ce33 100644 --- a/app/controllers/concerns/issuable_actions.rb +++ b/app/controllers/concerns/issuable_actions.rb @@ -26,6 +26,23 @@ module IssuableActions private + def render_conflict_response + respond_to do |format| + format.html do + @conflict = true + render :edit + end + + format.json do + render json: { + errors: [ + "Someone edited this #{issuable.human_class_name} at the same time you did. Please refresh your browser and make sure your changes will not unintentionally remove theirs." + ] + }, status: 409 + end + end + end + def labels @labels ||= LabelsFinder.new(current_user, project_id: @project.id).execute end diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index ca5e81100da..1151555b8fa 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -134,8 +134,7 @@ class Projects::IssuesController < Projects::ApplicationController end rescue ActiveRecord::StaleObjectError - @conflict = true - render :edit + render_conflict_response end def referenced_merge_requests diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index d122c7fdcb2..53f30a24312 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -296,22 +296,21 @@ class Projects::MergeRequestsController < Projects::ApplicationController def update @merge_request = MergeRequests::UpdateService.new(project, current_user, merge_request_params).execute(@merge_request) - if @merge_request.valid? - respond_to do |format| - format.html do - redirect_to([@merge_request.target_project.namespace.becomes(Namespace), - @merge_request.target_project, @merge_request]) - end - format.json do - render json: @merge_request.to_json(include: { milestone: {}, assignee: { methods: :avatar_url }, labels: { methods: :text_color } }, methods: [:task_status, :task_status_short]) + respond_to do |format| + format.html do + if @merge_request.valid? + redirect_to([@merge_request.target_project.namespace.becomes(Namespace), @merge_request.target_project, @merge_request]) + else + render :edit end end - else - render "edit" + + format.json do + render json: @merge_request.to_json(include: { milestone: {}, assignee: { methods: :avatar_url }, labels: { methods: :text_color } }, methods: [:task_status, :task_status_short]) + end end rescue ActiveRecord::StaleObjectError - @conflict = true - render :edit + render_conflict_response end def remove_wip diff --git a/app/helpers/button_helper.rb b/app/helpers/button_helper.rb index 4c7c16d694c..195094730aa 100644 --- a/app/helpers/button_helper.rb +++ b/app/helpers/button_helper.rb @@ -34,7 +34,7 @@ module ButtonHelper content_tag (append_link ? :a : :span), protocol, class: klass, - href: (project.http_url_to_repo if append_link), + href: (project.http_url_to_repo(current_user) if append_link), data: { html: true, placement: placement, diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index 735a355c25a..4befeacc135 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -241,7 +241,7 @@ module ProjectsHelper when 'ssh' project.ssh_url_to_repo else - project.http_url_to_repo + project.http_url_to_repo(current_user) end end diff --git a/app/models/event.rb b/app/models/event.rb index 4b8eac9accf..d7ca8e3c599 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -36,7 +36,7 @@ class Event < ActiveRecord::Base scope :code_push, -> { where(action: PUSHED) } scope :in_projects, ->(projects) do - where(project_id: projects).recent + where(project_id: projects.pluck(:id)).recent end scope :with_associations, -> { includes(:author, :project, project: :namespace).preload(:target) } diff --git a/app/models/project.rb b/app/models/project.rb index aedd5bedcb9..e06fc30dc8a 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -869,8 +869,14 @@ class Project < ActiveRecord::Base url_to_repo end - def http_url_to_repo - "#{web_url}.git" + def http_url_to_repo(user = nil) + url = web_url + + if user + url.sub!(%r{\Ahttps?://}) { |protocol| "#{protocol}#{user.username}@" } + end + + "#{url}.git" end # Check if current branch name is marked as protected in the system diff --git a/changelogs/unreleased/1937-https-clone-url-username.yml b/changelogs/unreleased/1937-https-clone-url-username.yml new file mode 100644 index 00000000000..fa89d94e0f3 --- /dev/null +++ b/changelogs/unreleased/1937-https-clone-url-username.yml @@ -0,0 +1,4 @@ +--- +title: Add the Username to the HTTP(S) clone URL of a Repository +merge_request: 9347 +author: Jan Christophersen diff --git a/changelogs/unreleased/27267-events-project-query-performance-regression.yml b/changelogs/unreleased/27267-events-project-query-performance-regression.yml new file mode 100644 index 00000000000..a1697b57eac --- /dev/null +++ b/changelogs/unreleased/27267-events-project-query-performance-regression.yml @@ -0,0 +1,4 @@ +--- +title: 'Add performance query regression fix for !9088 affecting #27267' +merge_request: +author: diff --git a/changelogs/unreleased/feature-runners-registration-deletion-v4-api.yml b/changelogs/unreleased/feature-runners-registration-deletion-v4-api.yml new file mode 100644 index 00000000000..e646a6a17b7 --- /dev/null +++ b/changelogs/unreleased/feature-runners-registration-deletion-v4-api.yml @@ -0,0 +1,4 @@ +--- +title: Add Runner's registration/deletion v4 API +merge_request: 9246 +author: diff --git a/changelogs/unreleased/issue_24815.yml b/changelogs/unreleased/issue_24815.yml new file mode 100644 index 00000000000..916e47d36a9 --- /dev/null +++ b/changelogs/unreleased/issue_24815.yml @@ -0,0 +1,4 @@ +--- +title: Fix issuable stale object error handler for js when updating tasklists +merge_request: +author: diff --git a/changelogs/unreleased/sh-bump-hashie-to-3-5-5.yml b/changelogs/unreleased/sh-bump-hashie-to-3-5-5.yml new file mode 100644 index 00000000000..57f1474093a --- /dev/null +++ b/changelogs/unreleased/sh-bump-hashie-to-3-5-5.yml @@ -0,0 +1,4 @@ +--- +title: Bump Hashie to 3.5.5 and omniauth to 1.4.2 to eliminate warning noise +merge_request: +author: diff --git a/changelogs/unreleased/unified-member-api-response.yml b/changelogs/unreleased/unified-member-api-response.yml new file mode 100644 index 00000000000..0a60b4d46a3 --- /dev/null +++ b/changelogs/unreleased/unified-member-api-response.yml @@ -0,0 +1,4 @@ +--- +title: 'API: Return 400 for all validation erros in the mebers API' +merge_request: 9523 +author: Robert Schilling diff --git a/doc/administration/container_registry.md b/doc/administration/container_registry.md index a6300e18dc0..28e413ef447 100644 --- a/doc/administration/container_registry.md +++ b/doc/administration/container_registry.md @@ -466,6 +466,46 @@ If Registry is enabled in your GitLab instance, but you don't need it for your project, you can disable it from your project's settings. Read the user guide on how to achieve that. +## Disable Container Registry but use GitLab as an auth endpoint + +You can disable the embedded Container Registry to use an external one, but +still use GitLab as an auth endpoint. + +**Omnibus GitLab** +1. Open `/etc/gitlab/gitlab.rb` and set necessary configurations: + + ```ruby + registry['enable'] = false + gitlab_rails['registry_enabled'] = true + gitlab_rails['registry_host'] = "registry.gitlab.example.com" + gitlab_rails['registry_port'] = "5005" + gitlab_rails['registry_api_url'] = "http://localhost:5000" + gitlab_rails['registry_key_path'] = "/var/opt/gitlab/gitlab-rails/certificate.key" + gitlab_rails['registry_path'] = "/var/opt/gitlab/gitlab-rails/shared/registry" + gitlab_rails['registry_issuer'] = "omnibus-gitlab-issuer" + ``` + +1. Save the file and [reconfigure GitLab][] for the changes to take effect. + +**Installations from source** + +1. Open `/home/git/gitlab/config/gitlab.yml`, and edit the configuration settings under `registry`: + + ``` + ## Container Registry + + registry: + enabled: true + host: "registry.gitlab.example.com" + port: "5005" + api_url: "http://localhost:5000" + path: /var/opt/gitlab/gitlab-rails/shared/registry + key: /var/opt/gitlab/gitlab-rails/certificate.key + issuer: omnibus-gitlab-issuer + ``` + +1. Save the file and [restart GitLab][] for the changes to take effect. + ## Storage limitations Currently, there is no storage limitation, which means a user can upload an diff --git a/doc/api/v3_to_v4.md b/doc/api/v3_to_v4.md index 8af041be234..c178e224cc5 100644 --- a/doc/api/v3_to_v4.md +++ b/doc/api/v3_to_v4.md @@ -41,5 +41,6 @@ changes are in V4: - Renamed `branch_name` to `branch` on DELETE `id/repository/branches/:branch` response [!8936](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/8936) - Remove `public` param from create and edit actions of projects [!8736](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/8736) - Notes do not return deprecated field `upvote` and `downvote` [!9384](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/9384) +- Return HTTP status code `400` for all validation errors when creating or updating a member instead of sometimes `422` error. [!9523](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/9523) - Remove `GET /groups/owned`. Use `GET /groups?owned=true` instead [!9505](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/9505) -- Return 202 with JSON body on async removals on V4 API (DELETE `/projects/:id/repository/merged_branches` and DELETE `/projects/:id`) [!9449](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/9449)
\ No newline at end of file +- Return 202 with JSON body on async removals on V4 API (DELETE `/projects/:id/repository/merged_branches` and DELETE `/projects/:id`) [!9449](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/9449) diff --git a/doc/workflow/lfs/manage_large_binaries_with_git_lfs.md b/doc/workflow/lfs/manage_large_binaries_with_git_lfs.md index 8c5020bee37..9cc45065eb2 100644 --- a/doc/workflow/lfs/manage_large_binaries_with_git_lfs.md +++ b/doc/workflow/lfs/manage_large_binaries_with_git_lfs.md @@ -63,6 +63,12 @@ git commit -am "Added Debian iso" # commit the file meta data git push origin master # sync the git repo and large file to the GitLab server ``` +>**Note**: Make sure that `.gitattributes` is tracked by git. Otherwise Git + LFS will not be working properly for people cloning the project. + ```bash + git add .gitattributes + ``` + Cloning the repository works the same as before. Git automatically detects the LFS-tracked files and clones them via HTTP. If you performed the git clone command with a SSH URL, you have to enter your GitLab credentials for HTTP diff --git a/features/steps/explore/projects.rb b/features/steps/explore/projects.rb index 2b4a5ab0864..7dc33ab5683 100644 --- a/features/steps/explore/projects.rb +++ b/features/steps/explore/projects.rb @@ -49,7 +49,7 @@ class Spinach::Features::ExploreProjects < Spinach::FeatureSteps step 'I should see an http link to the repository' do project = Project.find_by(name: 'Community') - expect(page).to have_field('project_clone', with: project.http_url_to_repo) + expect(page).to have_field('project_clone', with: project.http_url_to_repo(@user)) end step 'I should see an ssh link to the repository' do diff --git a/lib/api/api.rb b/lib/api/api.rb index dc732012a33..7aa95a4a3c1 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -91,6 +91,7 @@ module API mount ::API::Projects mount ::API::ProjectSnippets mount ::API::Repositories + mount ::API::Runner mount ::API::Runners mount ::API::Services mount ::API::Session diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 0e37f40a887..a99d9cadc8a 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -618,6 +618,10 @@ module API end end + class RunnerRegistrationDetails < Grape::Entity + expose :id, :token + end + class BuildArtifactFile < Grape::Entity expose :filename, :size end diff --git a/lib/api/helpers/runner.rb b/lib/api/helpers/runner.rb new file mode 100644 index 00000000000..119ca81b883 --- /dev/null +++ b/lib/api/helpers/runner.rb @@ -0,0 +1,23 @@ +module API + module Helpers + module Runner + def runner_registration_token_valid? + ActiveSupport::SecurityUtils.variable_size_secure_compare(params[:token], + current_application_settings.runners_registration_token) + end + + def get_runner_version_from_params + return unless params['info'].present? + attributes_for_keys(%w(name version revision platform architecture), params['info']) + end + + def authenticate_runner! + forbidden! unless current_runner + end + + def current_runner + @runner ||= ::Ci::Runner.find_by_token(params[:token].to_s) + end + end + end +end diff --git a/lib/api/members.rb b/lib/api/members.rb index 8360c007005..5f6913d1a27 100644 --- a/lib/api/members.rb +++ b/lib/api/members.rb @@ -55,7 +55,6 @@ module API authorize_admin_source!(source_type, source) member = source.members.find_by(user_id: params[:user_id]) - conflict!('Member already exists') if member member = source.add_user(params[:user_id], params[:access_level], current_user: current_user, expires_at: params[:expires_at]) @@ -63,9 +62,6 @@ module API if member.persisted? && member.valid? present member.user, with: Entities::Member, member: member else - # This is to ensure back-compatibility but 400 behavior should be used - # for all validation errors in 9.0! - render_api_error!('Access level is not known', 422) if member.errors.key?(:access_level) render_validation_error!(member) end end @@ -87,9 +83,6 @@ module API if member.update_attributes(declared_params(include_missing: false)) present member.user, with: Entities::Member, member: member else - # This is to ensure back-compatibility but 400 behavior should be used - # for all validation errors in 9.0! - render_api_error!('Access level is not known', 422) if member.errors.key?(:access_level) render_validation_error!(member) end end diff --git a/lib/api/runner.rb b/lib/api/runner.rb new file mode 100644 index 00000000000..804b27d40a7 --- /dev/null +++ b/lib/api/runner.rb @@ -0,0 +1,52 @@ +module API + class Runner < Grape::API + helpers ::API::Helpers::Runner + + resource :runners do + desc 'Registers a new Runner' do + success Entities::RunnerRegistrationDetails + http_codes [[201, 'Runner was created'], [403, 'Forbidden']] + end + params do + requires :token, type: String, desc: 'Registration token' + optional :description, type: String, desc: %q(Runner's description) + optional :info, type: Hash, desc: %q(Runner's metadata) + optional :locked, type: Boolean, desc: 'Should Runner be locked for current project' + optional :run_untagged, type: Boolean, desc: 'Should Runner handle untagged jobs' + optional :tag_list, type: Array[String], desc: %q(List of Runner's tags) + end + post '/' do + attributes = attributes_for_keys [:description, :locked, :run_untagged, :tag_list] + + runner = + if runner_registration_token_valid? + # Create shared runner. Requires admin access + 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(attributes) + end + + return forbidden! unless runner + + if runner.id + runner.update(get_runner_version_from_params) + present runner, with: Entities::RunnerRegistrationDetails + else + not_found! + end + end + + desc 'Deletes a registered Runner' do + http_codes [[200, 'Runner was deleted'], [403, 'Forbidden']] + end + params do + requires :token, type: String, desc: %q(Runner's authentication token) + end + delete '/' do + authenticate_runner! + Ci::Runner.find_by_token(params[:token]).destroy + end + end + end +end diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index e493b9396f6..46c758b4654 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -125,14 +125,16 @@ describe Projects::IssuesController do end describe 'PUT #update' do + before do + sign_in(user) + project.team << [user, :developer] + end + + it_behaves_like 'update invalid issuable', Issue + context 'when moving issue to another private project' do let(:another_project) { create(:empty_project, :private) } - before do - sign_in(user) - project.team << [user, :developer] - end - context 'when user has access to move issue' do before { another_project.team << [user, :reporter] } diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index d9cb429132f..1ced666bb36 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -255,6 +255,8 @@ describe Projects::MergeRequestsController do expect { merge_request.reload.target_branch }.not_to change { merge_request.target_branch } end + + it_behaves_like 'update invalid issuable', MergeRequest end end diff --git a/spec/features/admin/admin_disables_git_access_protocol_spec.rb b/spec/features/admin/admin_disables_git_access_protocol_spec.rb index e8e080ce3e2..273cacd82cd 100644 --- a/spec/features/admin/admin_disables_git_access_protocol_spec.rb +++ b/spec/features/admin/admin_disables_git_access_protocol_spec.rb @@ -32,7 +32,7 @@ feature 'Admin disables Git access protocol', feature: true do scenario 'shows only HTTP url' do visit_project - expect(page).to have_content("git clone #{project.http_url_to_repo}") + expect(page).to have_content("git clone #{project.http_url_to_repo(admin)}") expect(page).not_to have_selector('#clone-dropdown') end end diff --git a/spec/features/projects/developer_views_empty_project_instructions_spec.rb b/spec/features/projects/developer_views_empty_project_instructions_spec.rb index 0c51fe72ca4..2352329d58c 100644 --- a/spec/features/projects/developer_views_empty_project_instructions_spec.rb +++ b/spec/features/projects/developer_views_empty_project_instructions_spec.rb @@ -56,8 +56,14 @@ feature 'Developer views empty project instructions', feature: true do end def expect_instructions_for(protocol) - msg = :"#{protocol.downcase}_url_to_repo" - - expect(page).to have_content("git clone #{project.send(msg)}") + url = + case protocol + when 'ssh' + project.ssh_url_to_repo + when 'http' + project.http_url_to_repo(developer) + end + + expect(page).to have_content("git clone #{url}") end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 5232c531635..ee4f4092062 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1894,4 +1894,25 @@ describe Project, models: true do end end end + + describe '#http_url_to_repo' do + let(:project) { create :empty_project } + + context 'when no user is given' do + it 'returns the url to the repo without a username' do + url = project.http_url_to_repo + + expect(url).to eq(project.http_url_to_repo) + expect(url).not_to include('@') + end + end + + context 'when user is given' do + it 'returns the url to the repo with the username' do + user = build_stubbed(:user) + + expect(project.http_url_to_repo(user)).to match(%r{https?:\/\/#{user.username}@}) + end + end + end end diff --git a/spec/requests/api/members_spec.rb b/spec/requests/api/members_spec.rb index 31166b50033..127498ed109 100644 --- a/spec/requests/api/members_spec.rb +++ b/spec/requests/api/members_spec.rb @@ -173,11 +173,11 @@ describe API::Members, api: true do expect(response).to have_http_status(400) end - it 'returns 422 when access_level is not valid' do + it 'returns 400 when access_level is not valid' do post api("/#{source_type.pluralize}/#{source.id}/members", master), user_id: stranger.id, access_level: 1234 - expect(response).to have_http_status(422) + expect(response).to have_http_status(400) end end end @@ -230,11 +230,11 @@ describe API::Members, api: true do expect(response).to have_http_status(400) end - it 'returns 422 when access level is not valid' do + it 'returns 400 when access level is not valid' do put api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", master), access_level: 1234 - expect(response).to have_http_status(422) + expect(response).to have_http_status(400) end end end @@ -342,7 +342,7 @@ describe API::Members, api: true do post api("/projects/#{project.id}/members", master), user_id: stranger.id, access_level: Member::OWNER - expect(response).to have_http_status(422) + expect(response).to have_http_status(400) end.to change { project.members.count }.by(0) end end diff --git a/spec/requests/api/runner_spec.rb b/spec/requests/api/runner_spec.rb new file mode 100644 index 00000000000..73e82647ca0 --- /dev/null +++ b/spec/requests/api/runner_spec.rb @@ -0,0 +1,148 @@ +require 'spec_helper' + +describe API::Runner do + include ApiHelpers + include StubGitlabCalls + + let(:registration_token) { 'abcdefg123456' } + + before do + stub_gitlab_calls + stub_application_setting(runners_registration_token: registration_token) + end + + describe '/api/v4/runners' do + describe 'POST /api/v4/runners' do + context 'when no token is provided' do + it 'returns 400 error' do + post api('/runners') + expect(response).to have_http_status 400 + end + end + + context 'when invalid token is provided' do + it 'returns 403 error' do + post api('/runners'), token: 'invalid' + expect(response).to have_http_status 403 + end + end + + context 'when valid token is provided' do + it 'creates runner with default values' do + post api('/runners'), token: registration_token + + runner = Ci::Runner.first + + expect(response).to have_http_status 201 + expect(json_response['id']).to eq(runner.id) + expect(json_response['token']).to eq(runner.token) + expect(runner.run_untagged).to be true + end + + context 'when project token is used' do + let(:project) { create(:empty_project) } + + it 'creates runner' do + post api('/runners'), token: project.runners_token + + expect(response).to have_http_status 201 + expect(project.runners.size).to eq(1) + end + end + end + + context 'when runner description is provided' do + it 'creates runner' do + post api('/runners'), token: registration_token, + description: 'server.hostname' + + expect(response).to have_http_status 201 + expect(Ci::Runner.first.description).to eq('server.hostname') + end + end + + context 'when runner tags are provided' do + it 'creates runner' do + post api('/runners'), token: registration_token, + tag_list: 'tag1, tag2' + + expect(response).to have_http_status 201 + expect(Ci::Runner.first.tag_list.sort).to eq(%w(tag1 tag2)) + end + end + + context 'when option for running untagged jobs is provided' do + context 'when tags are provided' do + it 'creates runner' do + post api('/runners'), 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 + expect(Ci::Runner.first.tag_list.sort).to eq(['tag']) + end + end + + context 'when tags are not provided' do + it 'returns 404 error' do + post api('/runners'), token: registration_token, + run_untagged: false + + expect(response).to have_http_status 404 + end + end + end + + context 'when option for locking Runner is provided' do + it 'creates runner' do + post api('/runners'), token: registration_token, + locked: true + + expect(response).to have_http_status 201 + expect(Ci::Runner.first.locked).to be true + end + end + + %w(name version revision platform architecture).each do |param| + context "when info parameter '#{param}' info is present" do + let(:value) { "#{param}_value" } + + it %q(updates provided Runner's parameter) do + post api('/runners'), token: registration_token, + info: { param => value } + + expect(response).to have_http_status 201 + expect(Ci::Runner.first.read_attribute(param.to_sym)).to eq(value) + end + end + end + end + + describe 'DELETE /api/v4/runners' do + context 'when no token is provided' do + it 'returns 400 error' do + delete api('/runners') + expect(response).to have_http_status 400 + end + end + + context 'when invalid token is provided' do + it 'returns 403 error' do + delete api('/runners'), token: 'invalid' + expect(response).to have_http_status 403 + end + end + + context 'when valid token is provided' do + let(:runner) { create(:ci_runner) } + + it 'deletes Runner' do + delete api('/runners'), token: runner.token + expect(response).to have_http_status 200 + expect(Ci::Runner.count).to eq(0) + end + end + end + end +end diff --git a/spec/support/update_invalid_issuable.rb b/spec/support/update_invalid_issuable.rb new file mode 100644 index 00000000000..f984ac7bfa7 --- /dev/null +++ b/spec/support/update_invalid_issuable.rb @@ -0,0 +1,57 @@ +shared_examples 'update invalid issuable' do |klass| + let(:params) do + { + namespace_id: project.namespace.path, + project_id: project.path, + id: issuable.iid + } + end + + let(:issuable) do + klass == Issue ? issue : merge_request + end + + before do + if klass == Issue + params.merge!(issue: { title: "any" }) + else + params.merge!(merge_request: { title: "any" }) + end + end + + context 'when updating causes conflicts' do + before do + allow_any_instance_of(issuable.class).to receive(:save). + and_raise(ActiveRecord::StaleObjectError.new(issuable, :save)) + end + + it 'renders edit when format is html' do + put :update, params + + expect(response).to render_template(:edit) + expect(assigns[:conflict]).to be_truthy + end + + it 'renders json error message when format is json' do + params.merge!(format: "json") + + put :update, params + + expect(response.status).to eq(409) + expect(JSON.parse(response.body)).to have_key('errors') + end + end + + context 'when updating an invalid issuable' do + before do + key = klass == Issue ? :issue : :merge_request + params[key][:title] = "" + end + + it 'renders edit when merge request is invalid' do + put :update, params + + expect(response).to render_template(:edit) + end + end +end |