diff options
102 files changed, 3048 insertions, 2578 deletions
diff --git a/CHANGELOG b/CHANGELOG index 3d44e53e4a7..fb87415bf53 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,12 +1,15 @@ Please view this file on the master branch, on stable branches it's out of date. v 8.11.0 (unreleased) + - Remove the http_parser.rb dependency by removing the tinder gem. !5758 (tbalthazar) - Fix don't pass a local variable called `i` to a partial. !20510 (herminiotorres) - Fix rename `add_users_into_project` and `projects_ids`. !20512 (herminiotorres) - Fix the title of the toggle dropdown button. !5515 (herminiotorres) - Improve diff performance by eliminating redundant checks for text blobs + - Ensure that branch names containing escapable characters (e.g. %20) aren't unescaped indiscriminately. !5770 (ewiltshi) - Convert switch icon into icon font (ClemMakesApps) - API: Endpoints for enabling and disabling deploy keys + - API: List access requests, request access, approve, and deny access requests to a project or a group. !4833 - Use long options for curl examples in documentation !5703 (winniehell) - Remove magic comments (`# encoding: UTF-8`) from Ruby files. !5456 (winniehell) - Add support for relative links starting with ./ or / to RelativeLinkFilter (winniehell) @@ -17,6 +20,7 @@ v 8.11.0 (unreleased) - Cache the commit author in RequestStore to avoid extra lookups in PostReceive - Expand commit message width in repo view (ClemMakesApps) - Cache highlighted diff lines for merge requests + - Pre-create all builds for a Pipeline when the new Pipeline is created !5295 - Fix of 'Commits being passed to custom hooks are already reachable when using the UI' - Fix awardable button mutuality loading spinners (ClemMakesApps) - Add support for using RequestStore within Sidekiq tasks via SIDEKIQ_REQUEST_STORE env variable @@ -24,7 +28,9 @@ v 8.11.0 (unreleased) - Add "No one can push" as an option for protected branches. !5081 - Improve performance of AutolinkFilter#text_parse by using XPath - Add experimental Redis Sentinel support !1877 + - Fix branches page dropdown sort initial state (ClemMakesApps) - Environments have an url to link to + - Various redundant database indexes have been removed - Update `timeago` plugin to use multiple string/locale settings - Remove unused images (ClemMakesApps) - Limit git rev-list output count to one in forced push check @@ -38,6 +44,7 @@ v 8.11.0 (unreleased) - Retrieve rendered HTML from cache in one request - Fix renaming repository when name contains invalid chararacters under project settings - Upgrade Grape from 0.13.0 to 0.15.0. !4601 + - Trigram indexes for the "ci_runners" table have been removed to speed up UPDATE queries - Fix devise deprecation warnings. - Update version_sorter and use new interface for faster tag sorting - Optimize checking if a user has read access to a list of issues !5370 @@ -70,6 +77,7 @@ v 8.11.0 (unreleased) - Add GitLab Workhorse version to admin dashboard (Katarzyna Kobierska Ula Budziszewska) - Allow branch names ending with .json for graph and network page !5579 (winniehell) - Add the `sprockets-es6` gem + - Improve OAuth2 client documentation (muteor) - Multiple trigger variables show in separate lines (Katarzyna Kobierska Ula Budziszewska) - Profile requests when a header is passed - Avoid calculation of line_code and position for _line partial when showing diff notes on discussion tab. @@ -93,6 +101,7 @@ v 8.11.0 (unreleased) - Avoid to show the original password field when password is automatically set. !5712 (duduribeiro) - Fix importing GitLab projects with an invalid MR source project - Sort folders with submodules in Files view !5521 + - Each `File::exists?` replaced to `File::exist?` because of deprecate since ruby version 2.2.0 v 8.10.5 - Add a data migration to fix some missing timestamps in the members table. !5670 @@ -163,9 +163,6 @@ gem 'redis-rails', '~> 4.0.0' gem 'redis', '~> 3.2' gem 'connection_pool', '~> 2.0' -# Campfire integration -gem 'tinder', '~> 1.10.0' - # HipChat integration gem 'hipchat', '~> 1.5.0' diff --git a/Gemfile.lock b/Gemfile.lock index 43ed6081274..3f39bbd754d 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -335,7 +335,6 @@ GEM activesupport (>= 2) nokogiri (~> 1.4) htmlentities (4.3.4) - http_parser.rb (0.5.3) httparty (0.13.7) json (~> 1.8) multi_xml (>= 0.5.2) @@ -672,7 +671,6 @@ GEM redis-namespace (>= 1.5.2) rufus-scheduler (>= 2.0.24) sidekiq (>= 4.0.0) - simple_oauth (0.1.9) simplecov (0.12.0) docile (~> 1.1.0) json (>= 1.8, < 3) @@ -742,21 +740,8 @@ GEM tilt (2.0.5) timecop (0.8.1) timfel-krb5-auth (0.8.3) - tinder (1.10.1) - eventmachine (~> 1.0) - faraday (~> 0.9.0) - faraday_middleware (~> 0.9) - hashie (>= 1.0) - json (~> 1.8.0) - mime-types - multi_json (~> 1.7) - twitter-stream (~> 0.1) turbolinks (2.5.3) coffee-rails - twitter-stream (0.1.16) - eventmachine (>= 0.12.8) - http_parser.rb (~> 0.5.1) - simple_oauth (~> 0.1.4) tzinfo (1.2.2) thread_safe (~> 0.1) u2f (0.2.1) @@ -981,7 +966,6 @@ DEPENDENCIES teaspoon-jasmine (~> 2.2.0) test_after_commit (~> 0.4.2) thin (~> 1.7.0) - tinder (~> 1.10.0) turbolinks (~> 2.5.0) u2f (~> 0.2.1) uglifier (~> 2.7.2) diff --git a/app/controllers/projects/branches_controller.rb b/app/controllers/projects/branches_controller.rb index e926043f3eb..48fe81b0d74 100644 --- a/app/controllers/projects/branches_controller.rb +++ b/app/controllers/projects/branches_controller.rb @@ -1,12 +1,13 @@ class Projects::BranchesController < Projects::ApplicationController include ActionView::Helpers::SanitizeHelper + include SortingHelper # Authorize before_action :require_non_empty_project before_action :authorize_download_code! before_action :authorize_push_code!, only: [:new, :create, :destroy] def index - @sort = params[:sort].presence || 'name' + @sort = params[:sort].presence || sort_value_name @branches = BranchesFinder.new(@repository, params).execute @branches = Kaminari.paginate_array(@branches).page(params[:page]) diff --git a/app/controllers/projects/builds_controller.rb b/app/controllers/projects/builds_controller.rb index 553b62741a5..12195c3cbb8 100644 --- a/app/controllers/projects/builds_controller.rb +++ b/app/controllers/projects/builds_controller.rb @@ -6,7 +6,7 @@ class Projects::BuildsController < Projects::ApplicationController def index @scope = params[:scope] - @all_builds = project.builds + @all_builds = project.builds.relevant @builds = @all_builds.order('created_at DESC') @builds = case @scope diff --git a/app/controllers/projects/commit_controller.rb b/app/controllers/projects/commit_controller.rb index fdfe7c65b7b..f44e9bb3fd7 100644 --- a/app/controllers/projects/commit_controller.rb +++ b/app/controllers/projects/commit_controller.rb @@ -134,8 +134,8 @@ class Projects::CommitController < Projects::ApplicationController end def define_status_vars - @statuses = CommitStatus.where(pipeline: pipelines) - @builds = Ci::Build.where(pipeline: pipelines) + @statuses = CommitStatus.where(pipeline: pipelines).relevant + @builds = Ci::Build.where(pipeline: pipelines).relevant end def assign_change_commit_vars(mr_source_branch) diff --git a/app/controllers/projects/git_http_client_controller.rb b/app/controllers/projects/git_http_client_controller.rb new file mode 100644 index 00000000000..7c21bd181dc --- /dev/null +++ b/app/controllers/projects/git_http_client_controller.rb @@ -0,0 +1,110 @@ +# This file should be identical in GitLab Community Edition and Enterprise Edition + +class Projects::GitHttpClientController < Projects::ApplicationController + include ActionController::HttpAuthentication::Basic + include KerberosSpnegoHelper + + attr_reader :user + + # Git clients will not know what authenticity token to send along + skip_before_action :verify_authenticity_token + skip_before_action :repository + before_action :authenticate_user + before_action :ensure_project_found! + + private + + def authenticate_user + if project && project.public? && download_request? + return # Allow access + end + + if allow_basic_auth? && basic_auth_provided? + login, password = user_name_and_password(request) + auth_result = Gitlab::Auth.find_for_git_client(login, password, project: project, ip: request.ip) + + if auth_result.type == :ci && download_request? + @ci = true + elsif auth_result.type == :oauth && !download_request? + # Not allowed + else + @user = auth_result.user + end + + if ci? || user + return # Allow access + end + elsif allow_kerberos_spnego_auth? && spnego_provided? + @user = find_kerberos_user + + if user + send_final_spnego_response + return # Allow access + end + end + + send_challenges + render plain: "HTTP Basic: Access denied\n", status: 401 + end + + def basic_auth_provided? + has_basic_credentials?(request) + end + + def send_challenges + challenges = [] + challenges << 'Basic realm="GitLab"' if allow_basic_auth? + challenges << spnego_challenge if allow_kerberos_spnego_auth? + headers['Www-Authenticate'] = challenges.join("\n") if challenges.any? + end + + def ensure_project_found! + render_not_found if project.blank? + end + + def project + return @project if defined?(@project) + + project_id, _ = project_id_with_suffix + if project_id.blank? + @project = nil + else + @project = Project.find_with_namespace("#{params[:namespace_id]}/#{project_id}") + end + end + + # This method returns two values so that we can parse + # params[:project_id] (untrusted input!) in exactly one place. + def project_id_with_suffix + id = params[:project_id] || '' + + %w[.wiki.git .git].each do |suffix| + if id.end_with?(suffix) + # Be careful to only remove the suffix from the end of 'id'. + # Accidentally removing it from the middle is how security + # vulnerabilities happen! + return [id.slice(0, id.length - suffix.length), suffix] + end + end + + # Something is wrong with params[:project_id]; do not pass it on. + [nil, nil] + end + + def repository + _, suffix = project_id_with_suffix + if suffix == '.wiki.git' + project.wiki.repository + else + project.repository + end + end + + def render_not_found + render plain: 'Not Found', status: :not_found + end + + def ci? + @ci.present? + end +end diff --git a/app/controllers/projects/git_http_controller.rb b/app/controllers/projects/git_http_controller.rb index e2f93e239bd..b4373ef89ef 100644 --- a/app/controllers/projects/git_http_controller.rb +++ b/app/controllers/projects/git_http_controller.rb @@ -1,17 +1,6 @@ # This file should be identical in GitLab Community Edition and Enterprise Edition -class Projects::GitHttpController < Projects::ApplicationController - include ActionController::HttpAuthentication::Basic - include KerberosSpnegoHelper - - attr_reader :user - - # Git clients will not know what authenticity token to send along - skip_before_action :verify_authenticity_token - skip_before_action :repository - before_action :authenticate_user - before_action :ensure_project_found! - +class Projects::GitHttpController < Projects::GitHttpClientController # GET /foo/bar.git/info/refs?service=git-upload-pack (git pull) # GET /foo/bar.git/info/refs?service=git-receive-pack (git push) def info_refs @@ -46,81 +35,8 @@ class Projects::GitHttpController < Projects::ApplicationController private - def authenticate_user - if project && project.public? && upload_pack? - return # Allow access - end - - if allow_basic_auth? && basic_auth_provided? - login, password = user_name_and_password(request) - auth_result = Gitlab::Auth.find_for_git_client(login, password, project: project, ip: request.ip) - - if auth_result.type == :ci && upload_pack? - @ci = true - elsif auth_result.type == :oauth && !upload_pack? - # Not allowed - else - @user = auth_result.user - end - - if ci? || user - return # Allow access - end - elsif allow_kerberos_spnego_auth? && spnego_provided? - @user = find_kerberos_user - - if user - send_final_spnego_response - return # Allow access - end - end - - send_challenges - render plain: "HTTP Basic: Access denied\n", status: 401 - end - - def basic_auth_provided? - has_basic_credentials?(request) - end - - def send_challenges - challenges = [] - challenges << 'Basic realm="GitLab"' if allow_basic_auth? - challenges << spnego_challenge if allow_kerberos_spnego_auth? - headers['Www-Authenticate'] = challenges.join("\n") if challenges.any? - end - - def ensure_project_found! - render_not_found if project.blank? - end - - def project - return @project if defined?(@project) - - project_id, _ = project_id_with_suffix - if project_id.blank? - @project = nil - else - @project = Project.find_with_namespace("#{params[:namespace_id]}/#{project_id}") - end - end - - # This method returns two values so that we can parse - # params[:project_id] (untrusted input!) in exactly one place. - def project_id_with_suffix - id = params[:project_id] || '' - - %w[.wiki.git .git].each do |suffix| - if id.end_with?(suffix) - # Be careful to only remove the suffix from the end of 'id'. - # Accidentally removing it from the middle is how security - # vulnerabilities happen! - return [id.slice(0, id.length - suffix.length), suffix] - end - end - - # Something is wrong with params[:project_id]; do not pass it on. - [nil, nil] + def download_request? + upload_pack? end def upload_pack? @@ -143,19 +59,6 @@ class Projects::GitHttpController < Projects::ApplicationController render json: Gitlab::Workhorse.git_http_ok(repository, user) end - def repository - _, suffix = project_id_with_suffix - if suffix == '.wiki.git' - project.wiki.repository - else - project.repository - end - end - - def render_not_found - render plain: 'Not Found', status: :not_found - end - def render_http_not_allowed render plain: access_check.message, status: :forbidden end @@ -169,10 +72,6 @@ class Projects::GitHttpController < Projects::ApplicationController end end - def ci? - @ci.present? - end - def upload_pack_allowed? return false unless Gitlab.config.gitlab_shell.upload_pack diff --git a/app/controllers/projects/lfs_api_controller.rb b/app/controllers/projects/lfs_api_controller.rb new file mode 100644 index 00000000000..ece49dcd922 --- /dev/null +++ b/app/controllers/projects/lfs_api_controller.rb @@ -0,0 +1,94 @@ +class Projects::LfsApiController < Projects::GitHttpClientController + include LfsHelper + + before_action :require_lfs_enabled! + before_action :lfs_check_access!, except: [:deprecated] + + def batch + unless objects.present? + render_lfs_not_found + return + end + + if download_request? + render json: { objects: download_objects! } + elsif upload_request? + render json: { objects: upload_objects! } + else + raise "Never reached" + end + end + + def deprecated + render( + json: { + message: 'Server supports batch API only, please update your Git LFS client to version 1.0.1 and up.', + documentation_url: "#{Gitlab.config.gitlab.url}/help", + }, + status: 501 + ) + end + + private + + def objects + @objects ||= (params[:objects] || []).to_a + end + + def existing_oids + @existing_oids ||= begin + storage_project.lfs_objects.where(oid: objects.map { |o| o['oid'].to_s }).pluck(:oid) + end + end + + def download_objects! + objects.each do |object| + if existing_oids.include?(object[:oid]) + object[:actions] = download_actions(object) + else + object[:error] = { + code: 404, + message: "Object does not exist on the server or you don't have permissions to access it", + } + end + end + objects + end + + def upload_objects! + objects.each do |object| + object[:actions] = upload_actions(object) unless existing_oids.include?(object[:oid]) + end + objects + end + + def download_actions(object) + { + download: { + href: "#{project.http_url_to_repo}/gitlab-lfs/objects/#{object[:oid]}", + header: { + Authorization: request.headers['Authorization'] + }.compact + } + } + end + + def upload_actions(object) + { + upload: { + href: "#{project.http_url_to_repo}/gitlab-lfs/objects/#{object[:oid]}/#{object[:size]}", + header: { + Authorization: request.headers['Authorization'] + }.compact + } + } + end + + def download_request? + params[:operation] == 'download' + end + + def upload_request? + params[:operation] == 'upload' + end +end diff --git a/app/controllers/projects/lfs_storage_controller.rb b/app/controllers/projects/lfs_storage_controller.rb new file mode 100644 index 00000000000..69066cb40e6 --- /dev/null +++ b/app/controllers/projects/lfs_storage_controller.rb @@ -0,0 +1,92 @@ +class Projects::LfsStorageController < Projects::GitHttpClientController + include LfsHelper + + before_action :require_lfs_enabled! + before_action :lfs_check_access! + + def download + lfs_object = LfsObject.find_by_oid(oid) + unless lfs_object && lfs_object.file.exists? + render_lfs_not_found + return + end + + send_file lfs_object.file.path, content_type: "application/octet-stream" + end + + def upload_authorize + render( + json: { + StoreLFSPath: "#{Gitlab.config.lfs.storage_path}/tmp/upload", + LfsOid: oid, + LfsSize: size, + }, + content_type: 'application/json; charset=utf-8' + ) + end + + def upload_finalize + unless tmp_filename + render_lfs_forbidden + return + end + + if store_file(oid, size, tmp_filename) + head 200 + else + render plain: 'Unprocessable entity', status: 422 + end + end + + private + + def download_request? + action_name == 'download' + end + + def upload_request? + %w[upload_authorize upload_finalize].include? action_name + end + + def oid + params[:oid].to_s + end + + def size + params[:size].to_i + end + + def tmp_filename + name = request.headers['X-Gitlab-Lfs-Tmp'] + return if name.include?('/') + return unless oid.present? && name.start_with?(oid) + name + end + + def store_file(oid, size, tmp_file) + # Define tmp_file_path early because we use it in "ensure" + tmp_file_path = File.join("#{Gitlab.config.lfs.storage_path}/tmp/upload", tmp_file) + + object = LfsObject.find_or_create_by(oid: oid, size: size) + file_exists = object.file.exists? || move_tmp_file_to_storage(object, tmp_file_path) + file_exists && link_to_project(object) + ensure + FileUtils.rm_f(tmp_file_path) + end + + def move_tmp_file_to_storage(object, path) + File.open(path) do |f| + object.file = f + end + + object.file.store! + object.save + end + + def link_to_project(object) + if object && !object.projects.exists?(storage_project.id) + object.projects << storage_project + object.save + end + end +end diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 2cf6a2dd1b3..139680d2df9 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -160,7 +160,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController @diff_notes_disabled = true @pipeline = @merge_request.pipeline - @statuses = @pipeline.statuses if @pipeline + @statuses = @pipeline.statuses.relevant if @pipeline @note_counts = Note.where(commit_id: @commits.map(&:id)). group(:commit_id).count @@ -362,7 +362,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController @commits_count = @merge_request.commits.count @pipeline = @merge_request.pipeline - @statuses = @pipeline.statuses if @pipeline + @statuses = @pipeline.statuses.relevant if @pipeline if @merge_request.locked_long_ago? @merge_request.unlock_mr diff --git a/app/controllers/projects/pipelines_controller.rb b/app/controllers/projects/pipelines_controller.rb index 487963fdcd7..b0c72cfe4b4 100644 --- a/app/controllers/projects/pipelines_controller.rb +++ b/app/controllers/projects/pipelines_controller.rb @@ -19,7 +19,7 @@ class Projects::PipelinesController < Projects::ApplicationController end def create - @pipeline = Ci::CreatePipelineService.new(project, current_user, create_params).execute + @pipeline = Ci::CreatePipelineService.new(project, current_user, create_params).execute(ignore_skip_ci: true, save_on_errors: false) unless @pipeline.persisted? render 'new' return diff --git a/app/helpers/lfs_helper.rb b/app/helpers/lfs_helper.rb new file mode 100644 index 00000000000..eb651e3687e --- /dev/null +++ b/app/helpers/lfs_helper.rb @@ -0,0 +1,67 @@ +module LfsHelper + def require_lfs_enabled! + return if Gitlab.config.lfs.enabled + + render( + json: { + message: 'Git LFS is not enabled on this GitLab server, contact your admin.', + documentation_url: "#{Gitlab.config.gitlab.url}/help", + }, + status: 501 + ) + end + + def lfs_check_access! + return if download_request? && lfs_download_access? + return if upload_request? && lfs_upload_access? + + if project.public? || (user && user.can?(:read_project, project)) + render_lfs_forbidden + else + render_lfs_not_found + end + end + + def lfs_download_access? + project.public? || ci? || (user && user.can?(:download_code, project)) + end + + def lfs_upload_access? + user && user.can?(:push_code, project) + end + + def render_lfs_forbidden + render( + json: { + message: 'Access forbidden. Check your access level.', + documentation_url: "#{Gitlab.config.gitlab.url}/help", + }, + content_type: "application/vnd.git-lfs+json", + status: 403 + ) + end + + def render_lfs_not_found + render( + json: { + message: 'Not found.', + documentation_url: "#{Gitlab.config.gitlab.url}/help", + }, + content_type: "application/vnd.git-lfs+json", + status: 404 + ) + end + + def storage_project + @storage_project ||= begin + result = project + + loop do + break unless result.forked? + result = result.forked_from_project + end + + result + end + end +end diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index b919846af22..05b11f3b115 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -16,7 +16,7 @@ module Ci scope :with_artifacts_not_expired, ->() { with_artifacts.where('artifacts_expire_at IS NULL OR artifacts_expire_at > ?', Time.now) } scope :with_expired_artifacts, ->() { with_artifacts.where('artifacts_expire_at < ?', Time.now) } scope :last_month, ->() { where('created_at > ?', Date.today - 1.month) } - scope :manual_actions, ->() { where(when: :manual) } + scope :manual_actions, ->() { where(when: :manual).relevant } mount_uploader :artifacts_file, ArtifactUploader mount_uploader :artifacts_metadata, ArtifactUploader @@ -65,17 +65,11 @@ module Ci end end - state_machine :status, initial: :pending do + state_machine :status do after_transition pending: :running do |build| build.execute_hooks end - # We use around_transition to create builds for next stage as soon as possible, before the `after_*` is executed - around_transition any => [:success, :failed, :canceled] do |build, block| - block.call - build.pipeline.create_next_builds(build) if build.pipeline - end - after_transition any => [:success, :failed, :canceled] do |build| build.update_coverage build.execute_hooks diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index d6b75411022..9545a6e3ab9 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -13,11 +13,10 @@ module Ci has_many :trigger_requests, dependent: :destroy, class_name: 'Ci::TriggerRequest', foreign_key: :commit_id validates_presence_of :sha + validates_presence_of :ref validates_presence_of :status validate :valid_commit_sha - # Invalidate object and save if when touched - after_touch :update_state after_save :keep_around_commits # ref can't be HEAD or SHA, can only be branch/tag name @@ -90,12 +89,16 @@ module Ci def cancel_running builds.running_or_pending.each(&:cancel) + + reload_status! end def retry_failed(user) builds.latest.failed.select(&:retryable?).each do |build| Ci::Build.retry(build, user) end + + reload_status! end def latest? @@ -109,37 +112,6 @@ module Ci trigger_requests.any? end - def create_builds(user, trigger_request = nil) - ## - # We persist pipeline only if there are builds available - # - return unless config_processor - - build_builds_for_stages(config_processor.stages, user, - 'success', trigger_request) && save - end - - def create_next_builds(build) - return unless config_processor - - # don't create other builds if this one is retried - latest_builds = builds.latest - return unless latest_builds.exists?(build.id) - - # get list of stages after this build - next_stages = config_processor.stages.drop_while { |stage| stage != build.stage } - next_stages.delete(build.stage) - - # get status for all prior builds - prior_builds = latest_builds.where.not(stage: next_stages) - prior_status = prior_builds.status - - # build builds for next stage that has builds available - # and save pipeline if we have builds - build_builds_for_stages(next_stages, build.user, prior_status, - build.trigger_request) && save - end - def retried @retried ||= (statuses.order(id: :desc) - statuses.latest) end @@ -151,6 +123,14 @@ module Ci end end + def config_builds_attributes + return [] unless config_processor + + config_processor. + builds_for_ref(ref, tag?, trigger_requests.first). + sort_by { |build| build[:stage_idx] } + end + def has_warnings? builds.latest.ignored.any? end @@ -182,10 +162,6 @@ module Ci end end - def skip_ci? - git_commit_message =~ /\[(ci skip|skip ci)\]/i if git_commit_message - end - def environments builds.where.not(environment: nil).success.pluck(:environment).uniq end @@ -207,50 +183,34 @@ module Ci Note.for_commit_id(sha) end + def process! + Ci::ProcessPipelineService.new(project, user).execute(self) + reload_status! + end + def predefined_variables [ { key: 'CI_PIPELINE_ID', value: id.to_s, public: true } ] end - private - - def build_builds_for_stages(stages, user, status, trigger_request) - ## - # Note that `Array#any?` implements a short circuit evaluation, so we - # build builds only for the first stage that has builds available. - # - stages.any? do |stage| - CreateBuildsService.new(self). - execute(stage, user, status, trigger_request). - any?(&:active?) - end - end - - def update_state - last_status = status - - if update_state_from_commit_statuses - execute_hooks if last_status != status - true - else - false - end - end - - def update_state_from_commit_statuses + def reload_status! statuses.reload - self.status = if yaml_errors.blank? - statuses.latest.status || 'skipped' - else - 'failed' - end + self.status = + if yaml_errors.blank? + statuses.latest.status || 'skipped' + else + 'failed' + end self.started_at = statuses.started_at self.finished_at = statuses.finished_at self.duration = statuses.latest.duration save + execute_hooks if status_changed? end + private + def execute_hooks project.execute_hooks(pipeline_data, :pipeline_hooks) project.execute_services(pipeline_data, :pipeline_hooks) diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb index 2d185c28809..20713314a25 100644 --- a/app/models/commit_status.rb +++ b/app/models/commit_status.rb @@ -5,7 +5,7 @@ class CommitStatus < ActiveRecord::Base self.table_name = 'ci_builds' belongs_to :project, class_name: '::Project', foreign_key: :gl_project_id - belongs_to :pipeline, class_name: 'Ci::Pipeline', foreign_key: :commit_id, touch: true + belongs_to :pipeline, class_name: 'Ci::Pipeline', foreign_key: :commit_id belongs_to :user delegate :commit, to: :pipeline @@ -25,28 +25,36 @@ class CommitStatus < ActiveRecord::Base scope :ordered, -> { order(:name) } scope :ignored, -> { where(allow_failure: true, status: [:failed, :canceled]) } - state_machine :status, initial: :pending do + state_machine :status do event :queue do - transition skipped: :pending + transition [:created, :skipped] => :pending end event :run do transition pending: :running end + event :skip do + transition [:created, :pending] => :skipped + end + event :drop do - transition [:pending, :running] => :failed + transition [:created, :pending, :running] => :failed end event :success do - transition [:pending, :running] => :success + transition [:created, :pending, :running] => :success end event :cancel do - transition [:pending, :running] => :canceled + transition [:created, :pending, :running] => :canceled + end + + after_transition created: [:pending, :running] do |commit_status| + commit_status.update_attributes queued_at: Time.now end - after_transition pending: :running do |commit_status| + after_transition [:created, :pending] => :running do |commit_status| commit_status.update_attributes started_at: Time.now end @@ -54,13 +62,20 @@ class CommitStatus < ActiveRecord::Base commit_status.update_attributes finished_at: Time.now end - after_transition [:pending, :running] => :success do |commit_status| + after_transition [:created, :pending, :running] => :success do |commit_status| MergeRequests::MergeWhenBuildSucceedsService.new(commit_status.pipeline.project, nil).trigger(commit_status) end after_transition any => :failed do |commit_status| MergeRequests::AddTodoWhenBuildFailsService.new(commit_status.pipeline.project, nil).execute(commit_status) end + + # We use around_transition to process pipeline on next stages as soon as possible, before the `after_*` is executed + around_transition any => [:success, :failed, :canceled] do |commit_status, block| + block.call + + commit_status.pipeline.process! if commit_status.pipeline + end end delegate :sha, :short_sha, to: :pipeline diff --git a/app/models/concerns/statuseable.rb b/app/models/concerns/statuseable.rb index 44c6b30f278..5d4b0a86899 100644 --- a/app/models/concerns/statuseable.rb +++ b/app/models/concerns/statuseable.rb @@ -1,18 +1,22 @@ module Statuseable extend ActiveSupport::Concern - AVAILABLE_STATUSES = %w(pending running success failed canceled skipped) + AVAILABLE_STATUSES = %w[created pending running success failed canceled skipped] + STARTED_STATUSES = %w[running success failed skipped] + ACTIVE_STATUSES = %w[pending running] + COMPLETED_STATUSES = %w[success failed canceled] class_methods do def status_sql - builds = all.select('count(*)').to_sql - success = all.success.select('count(*)').to_sql - ignored = all.ignored.select('count(*)').to_sql if all.respond_to?(:ignored) + scope = all.relevant + builds = scope.select('count(*)').to_sql + success = scope.success.select('count(*)').to_sql + ignored = scope.ignored.select('count(*)').to_sql if scope.respond_to?(:ignored) ignored ||= '0' - pending = all.pending.select('count(*)').to_sql - running = all.running.select('count(*)').to_sql - canceled = all.canceled.select('count(*)').to_sql - skipped = all.skipped.select('count(*)').to_sql + pending = scope.pending.select('count(*)').to_sql + running = scope.running.select('count(*)').to_sql + canceled = scope.canceled.select('count(*)').to_sql + skipped = scope.skipped.select('count(*)').to_sql deduce_status = "(CASE WHEN (#{builds})=0 THEN NULL @@ -48,7 +52,8 @@ module Statuseable included do validates :status, inclusion: { in: AVAILABLE_STATUSES } - state_machine :status, initial: :pending do + state_machine :status, initial: :created do + state :created, value: 'created' state :pending, value: 'pending' state :running, value: 'running' state :failed, value: 'failed' @@ -57,6 +62,8 @@ module Statuseable state :skipped, value: 'skipped' end + scope :created, -> { where(status: 'created') } + scope :relevant, -> { where.not(status: 'created') } scope :running, -> { where(status: 'running') } scope :pending, -> { where(status: 'pending') } scope :success, -> { where(status: 'success') } @@ -68,14 +75,14 @@ module Statuseable end def started? - !pending? && !canceled? && started_at + STARTED_STATUSES.include?(status) && started_at end def active? - running? || pending? + ACTIVE_STATUSES.include?(status) end def complete? - canceled? || success? || failed? + COMPLETED_STATUSES.include?(status) end end diff --git a/app/models/members/project_member.rb b/app/models/members/project_member.rb index f176feddbad..18e97c969d7 100644 --- a/app/models/members/project_member.rb +++ b/app/models/members/project_member.rb @@ -8,6 +8,7 @@ class ProjectMember < Member # Make sure project member points only to project as it source default_value_for :source_type, SOURCE_TYPE validates_format_of :source_type, with: /\AProject\z/ + validates :access_level, inclusion: { in: Gitlab::Access.values } default_scope { where(source_type: SOURCE_TYPE) } scope :in_project, ->(project) { where(source_id: project.id) } diff --git a/app/models/project.rb b/app/models/project.rb index 3b1a53edc75..e0b28160937 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -999,6 +999,10 @@ class Project < ActiveRecord::Base project_members.find_by(user_id: user) end + def add_user(user, access_level, current_user = nil) + team.add_user(user, access_level, current_user) + end + def default_branch @default_branch ||= repository.root_ref if repository.exists? end diff --git a/app/models/project_services/campfire_service.rb b/app/models/project_services/campfire_service.rb index 511b2eac792..5af93860d09 100644 --- a/app/models/project_services/campfire_service.rb +++ b/app/models/project_services/campfire_service.rb @@ -1,4 +1,6 @@ class CampfireService < Service + include HTTParty + prop_accessor :token, :subdomain, :room validates :token, presence: true, if: :activated? @@ -29,18 +31,53 @@ class CampfireService < Service def execute(data) return unless supported_events.include?(data[:object_kind]) - room = gate.find_room_by_name(self.room) - return true unless room - + self.class.base_uri base_uri message = build_message(data) - - room.speak(message) + speak(self.room, message, auth) end private - def gate - @gate ||= Tinder::Campfire.new(subdomain, token: token) + def base_uri + @base_uri ||= "https://#{subdomain}.campfirenow.com" + end + + def auth + # use a dummy password, as explained in the Campfire API doc: + # https://github.com/basecamp/campfire-api#authentication + @auth ||= { + basic_auth: { + username: token, + password: 'X' + } + } + end + + # Post a message into a room, returns the message Hash in case of success. + # Returns nil otherwise. + # https://github.com/basecamp/campfire-api/blob/master/sections/messages.md#create-message + def speak(room_name, message, auth) + room = rooms(auth).find { |r| r["name"] == room_name } + return nil unless room + + path = "/room/#{room["id"]}/speak.json" + body = { + body: { + message: { + type: 'TextMessage', + body: message + } + } + } + res = self.class.post(path, auth.merge(body)) + res.code == 201 ? res : nil + end + + # Returns a list of rooms, or []. + # https://github.com/basecamp/campfire-api/blob/master/sections/rooms.md#get-rooms + def rooms(auth) + res = self.class.get("/rooms.json", auth) + res.code == 200 ? res["rooms"] : [] end def build_message(push) diff --git a/app/services/ci/create_builds_service.rb b/app/services/ci/create_builds_service.rb deleted file mode 100644 index 4946f7076fd..00000000000 --- a/app/services/ci/create_builds_service.rb +++ /dev/null @@ -1,62 +0,0 @@ -module Ci - class CreateBuildsService - def initialize(pipeline) - @pipeline = pipeline - @config = pipeline.config_processor - end - - def execute(stage, user, status, trigger_request = nil) - builds_attrs = @config.builds_for_stage_and_ref(stage, @pipeline.ref, @pipeline.tag, trigger_request) - - # check when to create next build - builds_attrs = builds_attrs.select do |build_attrs| - case build_attrs[:when] - when 'on_success' - status == 'success' - when 'on_failure' - status == 'failed' - when 'always', 'manual' - %w(success failed).include?(status) - end - end - - # don't create the same build twice - builds_attrs.reject! do |build_attrs| - @pipeline.builds.find_by(ref: @pipeline.ref, - tag: @pipeline.tag, - trigger_request: trigger_request, - name: build_attrs[:name]) - end - - builds_attrs.map do |build_attrs| - build_attrs.slice!(:name, - :commands, - :tag_list, - :options, - :allow_failure, - :stage, - :stage_idx, - :environment, - :when, - :yaml_variables) - - build_attrs.merge!(pipeline: @pipeline, - ref: @pipeline.ref, - tag: @pipeline.tag, - trigger_request: trigger_request, - user: user, - project: @pipeline.project) - - # TODO: The proper implementation for this is in - # https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5295 - build_attrs[:status] = 'skipped' if build_attrs[:when] == 'manual' - - ## - # We do not persist new builds here. - # Those will be persisted when @pipeline is saved. - # - @pipeline.builds.new(build_attrs) - end - end - end -end diff --git a/app/services/ci/create_pipeline_builds_service.rb b/app/services/ci/create_pipeline_builds_service.rb new file mode 100644 index 00000000000..005014fa1de --- /dev/null +++ b/app/services/ci/create_pipeline_builds_service.rb @@ -0,0 +1,42 @@ +module Ci + class CreatePipelineBuildsService < BaseService + attr_reader :pipeline + + def execute(pipeline) + @pipeline = pipeline + + new_builds.map do |build_attributes| + create_build(build_attributes) + end + end + + private + + def create_build(build_attributes) + build_attributes = build_attributes.merge( + pipeline: pipeline, + project: pipeline.project, + ref: pipeline.ref, + tag: pipeline.tag, + user: current_user, + trigger_request: trigger_request + ) + pipeline.builds.create(build_attributes) + end + + def new_builds + @new_builds ||= pipeline.config_builds_attributes. + reject { |build| existing_build_names.include?(build[:name]) } + end + + def existing_build_names + @existing_build_names ||= pipeline.builds.pluck(:name) + end + + def trigger_request + return @trigger_request if defined?(@trigger_request) + + @trigger_request ||= pipeline.trigger_requests.first + end + end +end diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb index b3772968ef3..7398fd8e10a 100644 --- a/app/services/ci/create_pipeline_service.rb +++ b/app/services/ci/create_pipeline_service.rb @@ -1,50 +1,100 @@ module Ci class CreatePipelineService < BaseService - def execute - pipeline = project.pipelines.new(params) - pipeline.user = current_user + attr_reader :pipeline - unless ref_names.include?(params[:ref]) - pipeline.errors.add(:base, 'Reference not found') - return pipeline + def execute(ignore_skip_ci: false, save_on_errors: true, trigger_request: nil) + @pipeline = Ci::Pipeline.new( + project: project, + ref: ref, + sha: sha, + before_sha: before_sha, + tag: tag?, + trigger_requests: Array(trigger_request), + user: current_user + ) + + unless project.builds_enabled? + return error('Pipeline is disabled') + end + + unless trigger_request || can?(current_user, :create_pipeline, project) + return error('Insufficient permissions to create a new pipeline') end - if commit - pipeline.sha = commit.id - else - pipeline.errors.add(:base, 'Commit not found') - return pipeline + unless branch? || tag? + return error('Reference not found') end - unless can?(current_user, :create_pipeline, project) - pipeline.errors.add(:base, 'Insufficient permissions to create a new pipeline') - return pipeline + unless commit + return error('Commit not found') end unless pipeline.config_processor - pipeline.errors.add(:base, pipeline.yaml_errors || 'Missing .gitlab-ci.yml file') - return pipeline + unless pipeline.ci_yaml_file + return error('Missing .gitlab-ci.yml file') + end + return error(pipeline.yaml_errors, save: save_on_errors) end - pipeline.save! + if !ignore_skip_ci && skip_ci? + return error('Creation of pipeline is skipped', save: save_on_errors) + end - unless pipeline.create_builds(current_user) - pipeline.errors.add(:base, 'No builds for this pipeline.') + unless pipeline.config_builds_attributes.present? + return error('No builds for this pipeline.') end pipeline.save - pipeline.touch + pipeline.process! pipeline end private - def ref_names - @ref_names ||= project.repository.ref_names + def skip_ci? + pipeline.git_commit_message =~ /\[(ci skip|skip ci)\]/i if pipeline.git_commit_message end def commit - @commit ||= project.commit(params[:ref]) + @commit ||= project.commit(origin_sha || origin_ref) + end + + def sha + commit.try(:id) + end + + def before_sha + params[:checkout_sha] || params[:before] || Gitlab::Git::BLANK_SHA + end + + def origin_sha + params[:checkout_sha] || params[:after] + end + + def origin_ref + params[:ref] + end + + def branch? + project.repository.ref_exists?(Gitlab::Git::BRANCH_REF_PREFIX + ref) + end + + def tag? + project.repository.ref_exists?(Gitlab::Git::TAG_REF_PREFIX + ref) + end + + def ref + Gitlab::Git.ref_name(origin_ref) + end + + def valid_sha? + origin_sha && origin_sha != Gitlab::Git::BLANK_SHA + end + + def error(message, save: false) + pipeline.errors.add(:base, message) + pipeline.reload_status! if save + pipeline end end end diff --git a/app/services/ci/create_trigger_request_service.rb b/app/services/ci/create_trigger_request_service.rb index 1e629cf119a..6af3c1ca5b1 100644 --- a/app/services/ci/create_trigger_request_service.rb +++ b/app/services/ci/create_trigger_request_service.rb @@ -1,20 +1,11 @@ module Ci class CreateTriggerRequestService def execute(project, trigger, ref, variables = nil) - commit = project.commit(ref) - return unless commit + trigger_request = trigger.trigger_requests.create(variables: variables) - # check if ref is tag - tag = project.repository.find_tag(ref).present? - - pipeline = project.pipelines.create(sha: commit.sha, ref: ref, tag: tag) - - trigger_request = trigger.trigger_requests.create!( - variables: variables, - pipeline: pipeline, - ) - - if pipeline.create_builds(nil, trigger_request) + pipeline = Ci::CreatePipelineService.new(project, nil, ref: ref). + execute(ignore_skip_ci: true, trigger_request: trigger_request) + if pipeline.persisted? trigger_request end end diff --git a/app/services/ci/process_pipeline_service.rb b/app/services/ci/process_pipeline_service.rb new file mode 100644 index 00000000000..86c4823d18a --- /dev/null +++ b/app/services/ci/process_pipeline_service.rb @@ -0,0 +1,77 @@ +module Ci + class ProcessPipelineService < BaseService + attr_reader :pipeline + + def execute(pipeline) + @pipeline = pipeline + + # This method will ensure that our pipeline does have all builds for all stages created + if created_builds.empty? + create_builds! + end + + new_builds = + stage_indexes_of_created_builds.map do |index| + process_stage(index) + end + + # Return a flag if a when builds got enqueued + new_builds.flatten.any? + end + + private + + def create_builds! + Ci::CreatePipelineBuildsService.new(project, current_user).execute(pipeline) + end + + def process_stage(index) + current_status = status_for_prior_stages(index) + + created_builds_in_stage(index).select do |build| + process_build(build, current_status) + end + end + + def process_build(build, current_status) + return false unless Statuseable::COMPLETED_STATUSES.include?(current_status) + + if valid_statuses_for_when(build.when).include?(current_status) + build.queue + true + else + build.skip + false + end + end + + def valid_statuses_for_when(value) + case value + when 'on_success' + %w[success] + when 'on_failure' + %w[failed] + when 'always' + %w[success failed] + else + [] + end + end + + def status_for_prior_stages(index) + pipeline.builds.where('stage_idx < ?', index).latest.status || 'success' + end + + def stage_indexes_of_created_builds + created_builds.order(:stage_idx).pluck('distinct stage_idx') + end + + def created_builds_in_stage(index) + created_builds.where(stage_idx: index) + end + + def created_builds + pipeline.builds.created + end + end +end diff --git a/app/services/create_commit_builds_service.rb b/app/services/create_commit_builds_service.rb deleted file mode 100644 index 0b66b854dea..00000000000 --- a/app/services/create_commit_builds_service.rb +++ /dev/null @@ -1,69 +0,0 @@ -class CreateCommitBuildsService - def execute(project, user, params) - return unless project.builds_enabled? - - before_sha = params[:checkout_sha] || params[:before] - sha = params[:checkout_sha] || params[:after] - origin_ref = params[:ref] - - ref = Gitlab::Git.ref_name(origin_ref) - tag = Gitlab::Git.tag_ref?(origin_ref) - - # Skip branch removal - if sha == Gitlab::Git::BLANK_SHA - return false - end - - @pipeline = Ci::Pipeline.new( - project: project, - sha: sha, - ref: ref, - before_sha: before_sha, - tag: tag, - user: user) - - ## - # Skip creating pipeline if no gitlab-ci.yml is found - # - unless @pipeline.ci_yaml_file - return false - end - - ## - # Skip creating builds for commits that have [ci skip] - # but save pipeline object - # - if @pipeline.skip_ci? - return save_pipeline! - end - - ## - # Skip creating builds when CI config is invalid - # but save pipeline object - # - unless @pipeline.config_processor - return save_pipeline! - end - - ## - # Skip creating pipeline object if there are no builds for it. - # - unless @pipeline.create_builds(user) - @pipeline.errors.add(:base, 'No builds created') - return false - end - - save_pipeline! - end - - private - - ## - # Create a new pipeline and touch object to calculate status - # - def save_pipeline! - @pipeline.save! - @pipeline.touch - @pipeline - end -end diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb index 473eb5d902f..b7c5cfb58b4 100644 --- a/app/services/git_push_service.rb +++ b/app/services/git_push_service.rb @@ -69,7 +69,7 @@ class GitPushService < BaseService SystemHooksService.new.execute_hooks(build_push_data_system_hook.dup, :push_hooks) @project.execute_hooks(build_push_data.dup, :push_hooks) @project.execute_services(build_push_data.dup, :push_hooks) - CreateCommitBuildsService.new.execute(@project, current_user, build_push_data) + Ci::CreatePipelineService.new(project, current_user, build_push_data).execute ProjectCacheWorker.perform_async(@project.id) end diff --git a/app/services/git_tag_push_service.rb b/app/services/git_tag_push_service.rb index 73bbbc36270..a578aaaa3b1 100644 --- a/app/services/git_tag_push_service.rb +++ b/app/services/git_tag_push_service.rb @@ -11,7 +11,7 @@ class GitTagPushService < BaseService SystemHooksService.new.execute_hooks(build_system_push_data.dup, :tag_push_hooks) project.execute_hooks(@push_data.dup, :tag_push_hooks) project.execute_services(@push_data.dup, :tag_push_hooks) - CreateCommitBuildsService.new.execute(project, current_user, @push_data) + Ci::CreatePipelineService.new(project, current_user, @push_data).execute ProjectCacheWorker.perform_async(project.id) true diff --git a/app/services/members/destroy_service.rb b/app/services/members/destroy_service.rb index 15358f80208..9e3f6af628d 100644 --- a/app/services/members/destroy_service.rb +++ b/app/services/members/destroy_service.rb @@ -2,8 +2,9 @@ module Members class DestroyService < BaseService attr_accessor :member, :current_user - def initialize(member, user) - @member, @current_user = member, user + def initialize(member, current_user) + @member = member + @current_user = current_user end def execute diff --git a/app/views/projects/ci/pipelines/_pipeline.html.haml b/app/views/projects/ci/pipelines/_pipeline.html.haml index 9a594877803..78709a92aed 100644 --- a/app/views/projects/ci/pipelines/_pipeline.html.haml +++ b/app/views/projects/ci/pipelines/_pipeline.html.haml @@ -33,7 +33,7 @@ Cant find HEAD commit for this branch - - stages_status = pipeline.statuses.latest.stages_status + - stages_status = pipeline.statuses.relevant.latest.stages_status - stages.each do |stage| %td.stage-cell - status = stages_status[stage] diff --git a/app/views/projects/commit/_pipeline.html.haml b/app/views/projects/commit/_pipeline.html.haml index 540689f4a61..640abdb993f 100644 --- a/app/views/projects/commit/_pipeline.html.haml +++ b/app/views/projects/commit/_pipeline.html.haml @@ -46,5 +46,5 @@ - if pipeline.project.build_coverage_enabled? %th Coverage %th - - pipeline.statuses.stages.each do |stage| - = render 'projects/commit/ci_stage', stage: stage, statuses: pipeline.statuses.where(stage: stage) + - pipeline.statuses.relevant.stages.each do |stage| + = render 'projects/commit/ci_stage', stage: stage, statuses: pipeline.statuses.relevant.where(stage: stage) diff --git a/config/initializers/5_backend.rb b/config/initializers/5_backend.rb index e026151a032..ed88c8ee1b8 100644 --- a/config/initializers/5_backend.rb +++ b/config/initializers/5_backend.rb @@ -1,6 +1,3 @@ -# GIT over HTTP -require_dependency Rails.root.join('lib/gitlab/backend/grack_auth') - # GIT over SSH require_dependency Rails.root.join('lib/gitlab/backend/shell') diff --git a/config/initializers/mime_types.rb b/config/initializers/mime_types.rb index 3e553120205..f498732feca 100644 --- a/config/initializers/mime_types.rb +++ b/config/initializers/mime_types.rb @@ -12,3 +12,10 @@ Mime::Type.register_alias "text/html", :md Mime::Type.register "video/mp4", :mp4, [], [:m4v, :mov] Mime::Type.register "video/webm", :webm Mime::Type.register "video/ogg", :ogv + +middlewares = Gitlab::Application.config.middleware +middlewares.swap(ActionDispatch::ParamsParser, ActionDispatch::ParamsParser, { + Mime::Type.lookup('application/vnd.git-lfs+json') => lambda do |body| + ActiveSupport::JSON.decode(body) + end +}) diff --git a/config/routes.rb b/config/routes.rb index 2f5f32d9e30..99a2cd884fe 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -84,9 +84,6 @@ Rails.application.routes.draw do # Health check get 'health_check(/:checks)' => 'health_check#index', as: :health_check - # Enable Grack support (for LFS only) - mount Grack::AuthSpawner, at: '/', constraints: lambda { |request| /[-\/\w\.]+\.git\/(info\/lfs|gitlab-lfs)/.match(request.path_info) }, via: [:get, :post, :put] - # Help get 'help' => 'help#index' get 'help/shortcuts' => 'help#shortcuts' @@ -482,11 +479,26 @@ Rails.application.routes.draw do end scope module: :projects do - # Git HTTP clients ('git clone' etc.) scope constraints: { id: /.+\.git/, format: nil } do + # Git HTTP clients ('git clone' etc.) get '/info/refs', to: 'git_http#info_refs' post '/git-upload-pack', to: 'git_http#git_upload_pack' post '/git-receive-pack', to: 'git_http#git_receive_pack' + + # Git LFS API (metadata) + post '/info/lfs/objects/batch', to: 'lfs_api#batch' + post '/info/lfs/objects', to: 'lfs_api#deprecated' + get '/info/lfs/objects/*oid', to: 'lfs_api#deprecated' + + # GitLab LFS object storage + scope constraints: { oid: /[a-f0-9]{64}/ } do + get '/gitlab-lfs/objects/*oid', to: 'lfs_storage#download' + + scope constraints: { size: /[0-9]+/ } do + put '/gitlab-lfs/objects/*oid/*size/authorize', to: 'lfs_storage#upload_authorize' + put '/gitlab-lfs/objects/*oid/*size', to: 'lfs_storage#upload_finalize' + end + end end # Allow /info/refs, /info/refs?service=git-upload-pack, and diff --git a/db/migrate/20160716115711_add_queued_at_to_ci_builds.rb b/db/migrate/20160716115711_add_queued_at_to_ci_builds.rb new file mode 100644 index 00000000000..756910a1fa0 --- /dev/null +++ b/db/migrate/20160716115711_add_queued_at_to_ci_builds.rb @@ -0,0 +1,9 @@ +class AddQueuedAtToCiBuilds < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def change + add_column :ci_builds, :queued_at, :timestamp + end +end diff --git a/db/migrate/20160810102349_remove_ci_runner_trigram_indexes.rb b/db/migrate/20160810102349_remove_ci_runner_trigram_indexes.rb new file mode 100644 index 00000000000..0cfb637804b --- /dev/null +++ b/db/migrate/20160810102349_remove_ci_runner_trigram_indexes.rb @@ -0,0 +1,27 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class RemoveCiRunnerTrigramIndexes < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + # Disabled for the "down" method so the indexes can be re-created concurrently. + disable_ddl_transaction! + + def up + return unless Gitlab::Database.postgresql? + + transaction do + execute 'DROP INDEX IF EXISTS index_ci_runners_on_token_trigram;' + execute 'DROP INDEX IF EXISTS index_ci_runners_on_description_trigram;' + end + end + + def down + return unless Gitlab::Database.postgresql? + + execute 'CREATE INDEX CONCURRENTLY index_ci_runners_on_token_trigram ON ci_runners USING gin(token gin_trgm_ops);' + execute 'CREATE INDEX CONCURRENTLY index_ci_runners_on_description_trigram ON ci_runners USING gin(description gin_trgm_ops);' + end +end diff --git a/db/migrate/20160810142633_remove_redundant_indexes.rb b/db/migrate/20160810142633_remove_redundant_indexes.rb new file mode 100644 index 00000000000..8641c6ffa8f --- /dev/null +++ b/db/migrate/20160810142633_remove_redundant_indexes.rb @@ -0,0 +1,112 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class RemoveRedundantIndexes < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + indexes = [ + [:ci_taggings, 'ci_taggings_idx'], + [:audit_events, 'index_audit_events_on_author_id'], + [:audit_events, 'index_audit_events_on_type'], + [:ci_builds, 'index_ci_builds_on_erased_by_id'], + [:ci_builds, 'index_ci_builds_on_project_id_and_commit_id'], + [:ci_builds, 'index_ci_builds_on_type'], + [:ci_commits, 'index_ci_commits_on_project_id'], + [:ci_commits, 'index_ci_commits_on_project_id_and_committed_at'], + [:ci_commits, 'index_ci_commits_on_project_id_and_committed_at_and_id'], + [:ci_commits, 'index_ci_commits_on_project_id_and_sha'], + [:ci_commits, 'index_ci_commits_on_sha'], + [:ci_events, 'index_ci_events_on_created_at'], + [:ci_events, 'index_ci_events_on_is_admin'], + [:ci_events, 'index_ci_events_on_project_id'], + [:ci_jobs, 'index_ci_jobs_on_deleted_at'], + [:ci_jobs, 'index_ci_jobs_on_project_id'], + [:ci_projects, 'index_ci_projects_on_gitlab_id'], + [:ci_projects, 'index_ci_projects_on_shared_runners_enabled'], + [:ci_services, 'index_ci_services_on_project_id'], + [:ci_sessions, 'index_ci_sessions_on_session_id'], + [:ci_sessions, 'index_ci_sessions_on_updated_at'], + [:ci_tags, 'index_ci_tags_on_name'], + [:ci_triggers, 'index_ci_triggers_on_deleted_at'], + [:identities, 'index_identities_on_created_at_and_id'], + [:issues, 'index_issues_on_title'], + [:keys, 'index_keys_on_created_at_and_id'], + [:members, 'index_members_on_created_at_and_id'], + [:members, 'index_members_on_type'], + [:milestones, 'index_milestones_on_created_at_and_id'], + [:namespaces, 'index_namespaces_on_visibility_level'], + [:projects, 'index_projects_on_builds_enabled_and_shared_runners_enabled'], + [:services, 'index_services_on_category'], + [:services, 'index_services_on_created_at_and_id'], + [:services, 'index_services_on_default'], + [:snippets, 'index_snippets_on_created_at'], + [:snippets, 'index_snippets_on_created_at_and_id'], + [:todos, 'index_todos_on_state'], + [:web_hooks, 'index_web_hooks_on_created_at_and_id'], + + # These indexes _may_ be used but they can be replaced by other existing + # indexes. + + # There's already a composite index on (project_id, iid) which means that + # a separate index for _just_ project_id is not needed. + [:issues, 'index_issues_on_project_id'], + + # These are all composite indexes for the columns (created_at, id). In all + # these cases there's already a standalone index for "created_at" which + # can be used instead. + # + # Because the "id" column of these composite indexes is never needed (due + # to "id" already being indexed as its a primary key) these composite + # indexes are useless. + [:issues, 'index_issues_on_created_at_and_id'], + [:merge_requests, 'index_merge_requests_on_created_at_and_id'], + [:namespaces, 'index_namespaces_on_created_at_and_id'], + [:notes, 'index_notes_on_created_at_and_id'], + [:projects, 'index_projects_on_created_at_and_id'], + [:users, 'index_users_on_created_at_and_id'], + ] + + transaction do + indexes.each do |(table, index)| + remove_index(table, name: index) if index_exists_by_name?(table, index) + end + end + + add_concurrent_index(:users, :created_at) + add_concurrent_index(:projects, :created_at) + add_concurrent_index(:namespaces, :created_at) + end + + def down + # We're only restoring the composite indexes that could be replaced with + # individual ones, just in case somebody would ever want to revert. + transaction do + remove_index(:users, :created_at) + remove_index(:projects, :created_at) + remove_index(:namespaces, :created_at) + end + + [:issues, :merge_requests, :namespaces, :notes, :projects, :users].each do |table| + add_concurrent_index(table, [:created_at, :id], + name: "index_#{table}_on_created_at_and_id") + end + end + + # Rails' index_exists? doesn't work when you only give it a table and index + # name. As such we have to use some extra code to check if an index exists for + # a given name. + def index_exists_by_name?(table, index) + indexes_for_table[table].include?(index) + end + + def indexes_for_table + @indexes_for_table ||= Hash.new do |hash, table_name| + hash[table_name] = indexes(table_name).map(&:name) + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 71980a6d51f..6c85e1e9dba 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: 20160804150737) do +ActiveRecord::Schema.define(version: 20160810142633) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -102,9 +102,7 @@ ActiveRecord::Schema.define(version: 20160804150737) do t.datetime "updated_at" end - add_index "audit_events", ["author_id"], name: "index_audit_events_on_author_id", using: :btree add_index "audit_events", ["entity_id", "entity_type"], name: "index_audit_events_on_entity_id_and_entity_type", using: :btree - add_index "audit_events", ["type"], name: "index_audit_events_on_type", using: :btree create_table "award_emoji", force: :cascade do |t| t.string "name" @@ -172,6 +170,7 @@ ActiveRecord::Schema.define(version: 20160804150737) do t.integer "artifacts_size" t.string "when" t.text "yaml_variables" + t.datetime "queued_at" end add_index "ci_builds", ["commit_id", "stage_idx", "created_at"], name: "index_ci_builds_on_commit_id_and_stage_idx_and_created_at", using: :btree @@ -179,13 +178,10 @@ ActiveRecord::Schema.define(version: 20160804150737) do add_index "ci_builds", ["commit_id", "type", "name", "ref"], name: "index_ci_builds_on_commit_id_and_type_and_name_and_ref", using: :btree add_index "ci_builds", ["commit_id", "type", "ref"], name: "index_ci_builds_on_commit_id_and_type_and_ref", using: :btree add_index "ci_builds", ["commit_id"], name: "index_ci_builds_on_commit_id", using: :btree - add_index "ci_builds", ["erased_by_id"], name: "index_ci_builds_on_erased_by_id", using: :btree add_index "ci_builds", ["gl_project_id"], name: "index_ci_builds_on_gl_project_id", using: :btree - add_index "ci_builds", ["project_id", "commit_id"], name: "index_ci_builds_on_project_id_and_commit_id", using: :btree add_index "ci_builds", ["project_id"], name: "index_ci_builds_on_project_id", using: :btree add_index "ci_builds", ["runner_id"], name: "index_ci_builds_on_runner_id", using: :btree add_index "ci_builds", ["status"], name: "index_ci_builds_on_status", using: :btree - add_index "ci_builds", ["type"], name: "index_ci_builds_on_type", using: :btree create_table "ci_commits", force: :cascade do |t| t.integer "project_id" @@ -209,11 +205,6 @@ ActiveRecord::Schema.define(version: 20160804150737) do add_index "ci_commits", ["gl_project_id", "sha"], name: "index_ci_commits_on_gl_project_id_and_sha", using: :btree add_index "ci_commits", ["gl_project_id", "status"], name: "index_ci_commits_on_gl_project_id_and_status", using: :btree add_index "ci_commits", ["gl_project_id"], name: "index_ci_commits_on_gl_project_id", using: :btree - add_index "ci_commits", ["project_id", "committed_at", "id"], name: "index_ci_commits_on_project_id_and_committed_at_and_id", using: :btree - add_index "ci_commits", ["project_id", "committed_at"], name: "index_ci_commits_on_project_id_and_committed_at", using: :btree - add_index "ci_commits", ["project_id", "sha"], name: "index_ci_commits_on_project_id_and_sha", using: :btree - add_index "ci_commits", ["project_id"], name: "index_ci_commits_on_project_id", using: :btree - add_index "ci_commits", ["sha"], name: "index_ci_commits_on_sha", using: :btree add_index "ci_commits", ["status"], name: "index_ci_commits_on_status", using: :btree add_index "ci_commits", ["user_id"], name: "index_ci_commits_on_user_id", using: :btree @@ -226,10 +217,6 @@ ActiveRecord::Schema.define(version: 20160804150737) do t.datetime "updated_at" end - add_index "ci_events", ["created_at"], name: "index_ci_events_on_created_at", using: :btree - add_index "ci_events", ["is_admin"], name: "index_ci_events_on_is_admin", using: :btree - add_index "ci_events", ["project_id"], name: "index_ci_events_on_project_id", using: :btree - create_table "ci_jobs", force: :cascade do |t| t.integer "project_id", null: false t.text "commands" @@ -244,9 +231,6 @@ ActiveRecord::Schema.define(version: 20160804150737) do t.datetime "deleted_at" end - add_index "ci_jobs", ["deleted_at"], name: "index_ci_jobs_on_deleted_at", using: :btree - add_index "ci_jobs", ["project_id"], name: "index_ci_jobs_on_project_id", using: :btree - create_table "ci_projects", force: :cascade do |t| t.string "name" t.integer "timeout", default: 3600, null: false @@ -270,9 +254,6 @@ ActiveRecord::Schema.define(version: 20160804150737) do t.text "generated_yaml_config" end - add_index "ci_projects", ["gitlab_id"], name: "index_ci_projects_on_gitlab_id", using: :btree - add_index "ci_projects", ["shared_runners_enabled"], name: "index_ci_projects_on_shared_runners_enabled", using: :btree - create_table "ci_runner_projects", force: :cascade do |t| t.integer "runner_id", null: false t.integer "project_id" @@ -301,10 +282,8 @@ ActiveRecord::Schema.define(version: 20160804150737) do 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"} create_table "ci_services", force: :cascade do |t| t.string "type" @@ -316,8 +295,6 @@ ActiveRecord::Schema.define(version: 20160804150737) do t.text "properties" end - add_index "ci_services", ["project_id"], name: "index_ci_services_on_project_id", using: :btree - create_table "ci_sessions", force: :cascade do |t| t.string "session_id", null: false t.text "data" @@ -325,9 +302,6 @@ ActiveRecord::Schema.define(version: 20160804150737) do t.datetime "updated_at" end - add_index "ci_sessions", ["session_id"], name: "index_ci_sessions_on_session_id", using: :btree - add_index "ci_sessions", ["updated_at"], name: "index_ci_sessions_on_updated_at", using: :btree - create_table "ci_taggings", force: :cascade do |t| t.integer "tag_id" t.integer "taggable_id" @@ -338,7 +312,6 @@ ActiveRecord::Schema.define(version: 20160804150737) do t.datetime "created_at" end - add_index "ci_taggings", ["tag_id", "taggable_id", "taggable_type", "context", "tagger_id", "tagger_type"], name: "ci_taggings_idx", unique: true, using: :btree add_index "ci_taggings", ["taggable_id", "taggable_type", "context"], name: "index_ci_taggings_on_taggable_id_and_taggable_type_and_context", using: :btree create_table "ci_tags", force: :cascade do |t| @@ -346,8 +319,6 @@ ActiveRecord::Schema.define(version: 20160804150737) do t.integer "taggings_count", default: 0 end - add_index "ci_tags", ["name"], name: "index_ci_tags_on_name", unique: true, using: :btree - create_table "ci_trigger_requests", force: :cascade do |t| t.integer "trigger_id", null: false t.text "variables" @@ -365,7 +336,6 @@ ActiveRecord::Schema.define(version: 20160804150737) do t.integer "gl_project_id" end - add_index "ci_triggers", ["deleted_at"], name: "index_ci_triggers_on_deleted_at", using: :btree add_index "ci_triggers", ["gl_project_id"], name: "index_ci_triggers_on_gl_project_id", using: :btree create_table "ci_variables", force: :cascade do |t| @@ -471,7 +441,6 @@ ActiveRecord::Schema.define(version: 20160804150737) do t.datetime "updated_at" end - add_index "identities", ["created_at", "id"], name: "index_identities_on_created_at_and_id", using: :btree add_index "identities", ["user_id"], name: "index_identities_on_user_id", using: :btree create_table "issues", force: :cascade do |t| @@ -497,16 +466,13 @@ ActiveRecord::Schema.define(version: 20160804150737) do add_index "issues", ["assignee_id"], name: "index_issues_on_assignee_id", using: :btree add_index "issues", ["author_id"], name: "index_issues_on_author_id", using: :btree add_index "issues", ["confidential"], name: "index_issues_on_confidential", using: :btree - add_index "issues", ["created_at", "id"], name: "index_issues_on_created_at_and_id", using: :btree add_index "issues", ["created_at"], name: "index_issues_on_created_at", using: :btree add_index "issues", ["deleted_at"], name: "index_issues_on_deleted_at", using: :btree add_index "issues", ["description"], name: "index_issues_on_description_trigram", using: :gin, opclasses: {"description"=>"gin_trgm_ops"} add_index "issues", ["due_date"], name: "index_issues_on_due_date", using: :btree add_index "issues", ["milestone_id"], name: "index_issues_on_milestone_id", using: :btree add_index "issues", ["project_id", "iid"], name: "index_issues_on_project_id_and_iid", unique: true, using: :btree - add_index "issues", ["project_id"], name: "index_issues_on_project_id", using: :btree add_index "issues", ["state"], name: "index_issues_on_state", using: :btree - add_index "issues", ["title"], name: "index_issues_on_title", using: :btree add_index "issues", ["title"], name: "index_issues_on_title_trigram", using: :gin, opclasses: {"title"=>"gin_trgm_ops"} create_table "keys", force: :cascade do |t| @@ -520,7 +486,6 @@ ActiveRecord::Schema.define(version: 20160804150737) do t.boolean "public", default: false, null: false end - add_index "keys", ["created_at", "id"], name: "index_keys_on_created_at_and_id", using: :btree add_index "keys", ["fingerprint"], name: "index_keys_on_fingerprint", unique: true, using: :btree add_index "keys", ["user_id"], name: "index_keys_on_user_id", using: :btree @@ -585,11 +550,9 @@ ActiveRecord::Schema.define(version: 20160804150737) do end add_index "members", ["access_level"], name: "index_members_on_access_level", using: :btree - add_index "members", ["created_at", "id"], name: "index_members_on_created_at_and_id", using: :btree add_index "members", ["invite_token"], name: "index_members_on_invite_token", unique: true, using: :btree add_index "members", ["requested_at"], name: "index_members_on_requested_at", using: :btree add_index "members", ["source_id", "source_type"], name: "index_members_on_source_id_and_source_type", using: :btree - add_index "members", ["type"], name: "index_members_on_type", using: :btree add_index "members", ["user_id"], name: "index_members_on_user_id", using: :btree create_table "merge_request_diffs", force: :cascade do |t| @@ -636,7 +599,6 @@ ActiveRecord::Schema.define(version: 20160804150737) do add_index "merge_requests", ["assignee_id"], name: "index_merge_requests_on_assignee_id", using: :btree add_index "merge_requests", ["author_id"], name: "index_merge_requests_on_author_id", using: :btree - add_index "merge_requests", ["created_at", "id"], name: "index_merge_requests_on_created_at_and_id", using: :btree add_index "merge_requests", ["created_at"], name: "index_merge_requests_on_created_at", using: :btree add_index "merge_requests", ["deleted_at"], name: "index_merge_requests_on_deleted_at", using: :btree add_index "merge_requests", ["description"], name: "index_merge_requests_on_description_trigram", using: :gin, opclasses: {"description"=>"gin_trgm_ops"} @@ -659,7 +621,6 @@ ActiveRecord::Schema.define(version: 20160804150737) do t.integer "iid" end - add_index "milestones", ["created_at", "id"], name: "index_milestones_on_created_at_and_id", using: :btree add_index "milestones", ["description"], name: "index_milestones_on_description_trigram", using: :gin, opclasses: {"description"=>"gin_trgm_ops"} add_index "milestones", ["due_date"], name: "index_milestones_on_due_date", using: :btree add_index "milestones", ["project_id", "iid"], name: "index_milestones_on_project_id_and_iid", unique: true, using: :btree @@ -681,14 +642,13 @@ ActiveRecord::Schema.define(version: 20160804150737) do t.boolean "request_access_enabled", default: true, null: false end - add_index "namespaces", ["created_at", "id"], name: "index_namespaces_on_created_at_and_id", using: :btree + add_index "namespaces", ["created_at"], name: "index_namespaces_on_created_at", using: :btree add_index "namespaces", ["name"], name: "index_namespaces_on_name", unique: true, using: :btree add_index "namespaces", ["name"], name: "index_namespaces_on_name_trigram", using: :gin, opclasses: {"name"=>"gin_trgm_ops"} add_index "namespaces", ["owner_id"], name: "index_namespaces_on_owner_id", using: :btree add_index "namespaces", ["path"], name: "index_namespaces_on_path", unique: true, using: :btree add_index "namespaces", ["path"], name: "index_namespaces_on_path_trigram", using: :gin, opclasses: {"path"=>"gin_trgm_ops"} add_index "namespaces", ["type"], name: "index_namespaces_on_type", using: :btree - add_index "namespaces", ["visibility_level"], name: "index_namespaces_on_visibility_level", using: :btree create_table "notes", force: :cascade do |t| t.text "note" @@ -711,7 +671,6 @@ ActiveRecord::Schema.define(version: 20160804150737) do add_index "notes", ["author_id"], name: "index_notes_on_author_id", using: :btree add_index "notes", ["commit_id"], name: "index_notes_on_commit_id", using: :btree - add_index "notes", ["created_at", "id"], name: "index_notes_on_created_at_and_id", using: :btree add_index "notes", ["created_at"], name: "index_notes_on_created_at", using: :btree add_index "notes", ["line_code"], name: "index_notes_on_line_code", using: :btree add_index "notes", ["note"], name: "index_notes_on_note_trigram", using: :gin, opclasses: {"note"=>"gin_trgm_ops"} @@ -851,9 +810,8 @@ ActiveRecord::Schema.define(version: 20160804150737) do t.boolean "request_access_enabled", default: true, null: false end - add_index "projects", ["builds_enabled", "shared_runners_enabled"], name: "index_projects_on_builds_enabled_and_shared_runners_enabled", using: :btree add_index "projects", ["ci_id"], name: "index_projects_on_ci_id", using: :btree - add_index "projects", ["created_at", "id"], name: "index_projects_on_created_at_and_id", using: :btree + add_index "projects", ["created_at"], name: "index_projects_on_created_at", using: :btree add_index "projects", ["creator_id"], name: "index_projects_on_creator_id", using: :btree add_index "projects", ["description"], name: "index_projects_on_description_trigram", using: :gin, opclasses: {"description"=>"gin_trgm_ops"} add_index "projects", ["last_activity_at"], name: "index_projects_on_last_activity_at", using: :btree @@ -939,9 +897,6 @@ ActiveRecord::Schema.define(version: 20160804150737) do t.boolean "wiki_page_events", default: true end - add_index "services", ["category"], name: "index_services_on_category", using: :btree - add_index "services", ["created_at", "id"], name: "index_services_on_created_at_and_id", using: :btree - add_index "services", ["default"], name: "index_services_on_default", using: :btree add_index "services", ["project_id"], name: "index_services_on_project_id", using: :btree add_index "services", ["template"], name: "index_services_on_template", using: :btree @@ -958,8 +913,6 @@ ActiveRecord::Schema.define(version: 20160804150737) do end add_index "snippets", ["author_id"], name: "index_snippets_on_author_id", using: :btree - add_index "snippets", ["created_at", "id"], name: "index_snippets_on_created_at_and_id", using: :btree - add_index "snippets", ["created_at"], name: "index_snippets_on_created_at", using: :btree add_index "snippets", ["file_name"], name: "index_snippets_on_file_name_trigram", using: :gin, opclasses: {"file_name"=>"gin_trgm_ops"} add_index "snippets", ["project_id"], name: "index_snippets_on_project_id", using: :btree add_index "snippets", ["title"], name: "index_snippets_on_title_trigram", using: :gin, opclasses: {"title"=>"gin_trgm_ops"} @@ -1028,7 +981,6 @@ ActiveRecord::Schema.define(version: 20160804150737) do add_index "todos", ["commit_id"], name: "index_todos_on_commit_id", using: :btree add_index "todos", ["note_id"], name: "index_todos_on_note_id", using: :btree add_index "todos", ["project_id"], name: "index_todos_on_project_id", using: :btree - add_index "todos", ["state"], name: "index_todos_on_state", using: :btree add_index "todos", ["target_type", "target_id"], name: "index_todos_on_target_type_and_target_id", using: :btree add_index "todos", ["user_id"], name: "index_todos_on_user_id", using: :btree @@ -1108,7 +1060,7 @@ ActiveRecord::Schema.define(version: 20160804150737) do add_index "users", ["admin"], name: "index_users_on_admin", using: :btree add_index "users", ["authentication_token"], name: "index_users_on_authentication_token", unique: true, using: :btree add_index "users", ["confirmation_token"], name: "index_users_on_confirmation_token", unique: true, using: :btree - add_index "users", ["created_at", "id"], name: "index_users_on_created_at_and_id", using: :btree + add_index "users", ["created_at"], name: "index_users_on_created_at", using: :btree add_index "users", ["current_sign_in_at"], name: "index_users_on_current_sign_in_at", using: :btree add_index "users", ["email"], name: "index_users_on_email", unique: true, using: :btree add_index "users", ["email"], name: "index_users_on_email_trigram", using: :gin, opclasses: {"email"=>"gin_trgm_ops"} @@ -1148,7 +1100,6 @@ ActiveRecord::Schema.define(version: 20160804150737) do t.string "token" end - add_index "web_hooks", ["created_at", "id"], name: "index_web_hooks_on_created_at_and_id", using: :btree add_index "web_hooks", ["project_id"], name: "index_web_hooks_on_project_id", using: :btree add_foreign_key "personal_access_tokens", "users" diff --git a/doc/api/README.md b/doc/api/README.md index a357af3831d..f3117815c7c 100644 --- a/doc/api/README.md +++ b/doc/api/README.md @@ -16,6 +16,8 @@ following locations: - [Commits](commits.md) - [Deploy Keys](deploy_keys.md) - [Groups](groups.md) +- [Group Access Requests](access_requests.md) +- [Group Members](members.md) - [Issues](issues.md) - [Keys](keys.md) - [Labels](labels.md) @@ -25,6 +27,8 @@ following locations: - [Namespaces](namespaces.md) - [Notes](notes.md) (comments) - [Projects](projects.md) including setting Webhooks +- [Project Access Requests](access_requests.md) +- [Project Members](members.md) - [Project Snippets](project_snippets.md) - [Repositories](repositories.md) - [Repository Files](repository_files.md) @@ -154,7 +158,7 @@ be returned with status code `403`: ```json { - "message": "403 Forbidden: Must be admin to use sudo" + "message": "403 Forbidden - Must be admin to use sudo" } ``` diff --git a/doc/api/access_requests.md b/doc/api/access_requests.md new file mode 100644 index 00000000000..ea308b54d62 --- /dev/null +++ b/doc/api/access_requests.md @@ -0,0 +1,147 @@ +# Group and project access requests + + >**Note:** This feature was introduced in GitLab 8.11 + + **Valid access levels** + + The access levels are defined in the `Gitlab::Access` module. Currently, these levels are recognized: + +``` +10 => Guest access +20 => Reporter access +30 => Developer access +40 => Master access +50 => Owner access # Only valid for groups +``` + +## List access requests for a group or project + +Gets a list of access requests viewable by the authenticated user. + +Returns `200` if the request succeeds. + +``` +GET /groups/:id/access_requests +GET /projects/:id/access_requests +``` + +| Attribute | Type | Required | Description | +| --------- | ---- | -------- | ----------- | +| `id` | integer/string | yes | The group/project ID or path | + +```bash +curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/groups/:id/access_requests +curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/:id/access_requests +``` + +Example response: + +```json +[ + { + "id": 1, + "username": "raymond_smith", + "name": "Raymond Smith", + "state": "active", + "created_at": "2012-10-22T14:13:35Z", + "requested_at": "2012-10-22T14:13:35Z" + }, + { + "id": 2, + "username": "john_doe", + "name": "John Doe", + "state": "active", + "created_at": "2012-10-22T14:13:35Z", + "requested_at": "2012-10-22T14:13:35Z" + } +] +``` + +## Request access to a group or project + +Requests access for the authenticated user to a group or project. + +Returns `201` if the request succeeds. + +``` +POST /groups/:id/access_requests +POST /projects/:id/access_requests +``` + +| Attribute | Type | Required | Description | +| --------- | ---- | -------- | ----------- | +| `id` | integer/string | yes | The group/project ID or path | + +```bash +curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/groups/:id/access_requests +curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/:id/access_requests +``` + +Example response: + +```json +{ + "id": 1, + "username": "raymond_smith", + "name": "Raymond Smith", + "state": "active", + "created_at": "2012-10-22T14:13:35Z", + "requested_at": "2012-10-22T14:13:35Z" +} +``` + +## Approve an access request + +Approves an access request for the given user. + +Returns `201` if the request succeeds. + +``` +PUT /groups/:id/access_requests/:user_id/approve +PUT /projects/:id/access_requests/:user_id/approve +``` + +| Attribute | Type | Required | Description | +| --------- | ---- | -------- | ----------- | +| `id` | integer/string | yes | The group/project ID or path | +| `user_id` | integer | yes | The user ID of the access requester | +| `access_level` | integer | no | A valid access level (defaults: `30`, developer access level) | + +```bash +curl --request PUT --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/groups/:id/access_requests/:user_id/approve?access_level=20 +curl --request PUT --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/:id/access_requests/:user_id/approve?access_level=20 +``` + +Example response: + +```json +{ + "id": 1, + "username": "raymond_smith", + "name": "Raymond Smith", + "state": "active", + "created_at": "2012-10-22T14:13:35Z", + "access_level": 20 +} +``` + +## Deny an access request + +Denies an access request for the given user. + +Returns `200` if the request succeeds. + +``` +DELETE /groups/:id/access_requests/:user_id +DELETE /projects/:id/access_requests/:user_id +``` + +| Attribute | Type | Required | Description | +| --------- | ---- | -------- | ----------- | +| `id` | integer/string | yes | The group/project ID or path | +| `user_id` | integer | yes | The user ID of the access requester | + +```bash +curl --request DELETE --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/groups/:id/access_requests/:user_id +curl --request DELETE --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/:id/access_requests/:user_id +``` diff --git a/doc/api/groups.md b/doc/api/groups.md index fd665e967a9..a898387eaa2 100644 --- a/doc/api/groups.md +++ b/doc/api/groups.md @@ -417,87 +417,7 @@ GET /groups?search=foobar ## Group members -**Group access levels** - -The group access levels are defined in the `Gitlab::Access` module. Currently, these levels are recognized: - -``` -GUEST = 10 -REPORTER = 20 -DEVELOPER = 30 -MASTER = 40 -OWNER = 50 -``` - -### List group members - -Get a list of group members viewable by the authenticated user. - -``` -GET /groups/:id/members -``` - -```json -[ - { - "id": 1, - "username": "raymond_smith", - "name": "Raymond Smith", - "state": "active", - "created_at": "2012-10-22T14:13:35Z", - "access_level": 30 - }, - { - "id": 2, - "username": "john_doe", - "name": "John Doe", - "state": "active", - "created_at": "2012-10-22T14:13:35Z", - "access_level": 30 - } -] -``` - -### Add group member - -Adds a user to the list of group members. - -``` -POST /groups/:id/members -``` - -Parameters: - -- `id` (required) - The ID or path of a group -- `user_id` (required) - The ID of a user to add -- `access_level` (required) - Project access level - -### Edit group team member - -Updates a group team member to a specified access level. - -``` -PUT /groups/:id/members/:user_id -``` - -Parameters: - -- `id` (required) - The ID of a group -- `user_id` (required) - The ID of a group member -- `access_level` (required) - Project access level - -### Remove user team member - -Removes user from user team. - -``` -DELETE /groups/:id/members/:user_id -``` - -Parameters: - -- `id` (required) - The ID or path of a user group -- `user_id` (required) - The ID of a group member +Please consult the [Group Members](members.md) documentation. ## Namespaces in groups diff --git a/doc/api/members.md b/doc/api/members.md new file mode 100644 index 00000000000..d002e6eaf89 --- /dev/null +++ b/doc/api/members.md @@ -0,0 +1,182 @@ +# Group and project members + +**Valid access levels** + +The access levels are defined in the `Gitlab::Access` module. Currently, these levels are recognized: + +``` +10 => Guest access +20 => Reporter access +30 => Developer access +40 => Master access +50 => Owner access # Only valid for groups +``` + +## List all members of a group or project + +Gets a list of group or project members viewable by the authenticated user. + +Returns `200` if the request succeeds. + +``` +GET /groups/:id/members +GET /projects/:id/members +``` + +| Attribute | Type | Required | Description | +| --------- | ---- | -------- | ----------- | +| `id` | integer/string | yes | The group/project ID or path | +| `query` | string | no | A query string to search for members | + +```bash +curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/groups/:id/members +curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/:id/members +``` + +Example response: + +```json +[ + { + "id": 1, + "username": "raymond_smith", + "name": "Raymond Smith", + "state": "active", + "created_at": "2012-10-22T14:13:35Z", + "access_level": 30 + }, + { + "id": 2, + "username": "john_doe", + "name": "John Doe", + "state": "active", + "created_at": "2012-10-22T14:13:35Z", + "access_level": 30 + } +] +``` + +## Get a member of a group or project + +Gets a member of a group or project. + +Returns `200` if the request succeeds. + +``` +GET /groups/:id/members/:user_id +GET /projects/:id/members/:user_id +``` + +| Attribute | Type | Required | Description | +| --------- | ---- | -------- | ----------- | +| `id` | integer/string | yes | The group/project ID or path | +| `user_id` | integer | yes | The user ID of the member | + +```bash +curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/groups/:id/members/:user_id +curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/:id/members/:user_id +``` + +Example response: + +```json +{ + "id": 1, + "username": "raymond_smith", + "name": "Raymond Smith", + "state": "active", + "created_at": "2012-10-22T14:13:35Z", + "access_level": 30 +} +``` + +## Add a member to a group or project + +Adds a member to a group or project. + +Returns `201` if the request succeeds. + +``` +POST /groups/:id/members +POST /projects/:id/members +``` + +| Attribute | Type | Required | Description | +| --------- | ---- | -------- | ----------- | +| `id` | integer/string | yes | The group/project ID or path | +| `user_id` | integer | yes | The user ID of the new member | +| `access_level` | integer | yes | A valid access level | + +```bash +curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/groups/:id/members/:user_id?access_level=30 +curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/:id/members/:user_id?access_level=30 +``` + +Example response: + +```json +{ + "id": 1, + "username": "raymond_smith", + "name": "Raymond Smith", + "state": "active", + "created_at": "2012-10-22T14:13:35Z", + "access_level": 30 +} +``` + +## Edit a member of a group or project + +Updates a member of a group or project. + +Returns `200` if the request succeeds. + +``` +PUT /groups/:id/members/:user_id +PUT /projects/:id/members/:user_id +``` + +| Attribute | Type | Required | Description | +| --------- | ---- | -------- | ----------- | +| `id` | integer/string | yes | The group/project ID or path | +| `user_id` | integer | yes | The user ID of the member | +| `access_level` | integer | yes | A valid access level | + +```bash +curl --request PUT --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/groups/:id/members/:user_id?access_level=40 +curl --request PUT --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/:id/members/:user_id?access_level=40 +``` + +Example response: + +```json +{ + "id": 1, + "username": "raymond_smith", + "name": "Raymond Smith", + "state": "active", + "created_at": "2012-10-22T14:13:35Z", + "access_level": 40 +} +``` + +## Remove a member from a group or project + +Removes a user from a group or project. + +Returns `200` if the request succeeds. + +``` +DELETE /groups/:id/members/:user_id +DELETE /projects/:id/members/:user_id +``` + +| Attribute | Type | Required | Description | +| --------- | ---- | -------- | ----------- | +| `id` | integer/string | yes | The group/project ID or path | +| `user_id` | integer | yes | The user ID of the member | + +```bash +curl --request DELETE --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/groups/:id/members/:user_id +curl --request DELETE --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/:id/members/:user_id +``` diff --git a/doc/api/oauth2.md b/doc/api/oauth2.md index 2e6542281d2..16ef79617c0 100644 --- a/doc/api/oauth2.md +++ b/doc/api/oauth2.md @@ -1,38 +1,59 @@ # GitLab as an OAuth2 client -This document is about using other OAuth authentication service providers to sign into GitLab. -If you want GitLab to be an OAuth authentication service provider to sign into other services please see the [Oauth2 provider documentation](../integration/oauth_provider.md). +This document covers using the OAuth2 protocol to access GitLab. -OAuth2 is a protocol that enables us to authenticate a user without requiring them to give their password. +If you want GitLab to be an OAuth authentication service provider to sign into other services please see the [Oauth2 provider documentation](../integration/oauth_provider.md). -Before using the OAuth2 you should create an application in user's account. Each application gets a unique App ID and App Secret parameters. You should not share these. +OAuth2 is a protocol that enables us to authenticate a user without requiring them to give their password to a third-party. This functionality is based on [doorkeeper gem](https://github.com/doorkeeper-gem/doorkeeper) ## Web Application Flow -This flow is using for authentication from third-party web sites and is probably used the most. -It basically consists of an exchange of an authorization token for an access token. For more detailed info, check out the [RFC spec here](http://tools.ietf.org/html/rfc6749#section-4.1) +This is the most common type of flow and is used by server-side clients that wish to access GitLab on a user's behalf. + +>**Note:** +This flow **should not** be used for client-side clients as you would leak your `client_secret`. Client-side clients should use the Implicit Grant (which is currently unsupported). -This flow consists from 3 steps. +For more detailed information, check out the [RFC spec](http://tools.ietf.org/html/rfc6749#section-4.1) + +In the following sections you will be introduced to the three steps needed for this flow. ### 1. Registering the client -Create an application in user's account profile. +First, you should create an application (`/profile/applications`) in your user's account. +Each application gets a unique App ID and App Secret parameters. + +>**Note:** +**You should not share/leak your App ID or App Secret.** ### 2. Requesting authorization -To request the authorization token, you should visit the `/oauth/authorize` endpoint. You can do that by visiting manually the URL: +To request the authorization code, you should redirect the user to the `/oauth/authorize` endpoint: + +``` +https://gitlab.example.com/oauth/authorize?client_id=APP_ID&redirect_uri=REDIRECT_URI&response_type=code&state=your_unique_state_hash +``` + +This will ask the user to approve the applications access to their account and then redirect back to the `REDIRECT_URI` you provided. + +The redirect will include the GET `code` parameter, for example: ``` -http://localhost:3000/oauth/authorize?client_id=APP_ID&redirect_uri=REDIRECT_URI&response_type=code +http://myapp.com/oauth/redirect?code=1234567890&state=your_unique_state_hash ``` -Where REDIRECT_URI is the URL in your app where users will be sent after authorization. +You should then use the `code` to request an access token. + +>**Important:** +It is highly recommended that you send a `state` value with the request to `/oauth/authorize` and +validate that value is returned and matches in the redirect request. +This is important to prevent [CSFR attacks](http://www.oauthsecurity.com/#user-content-authorization-code-flow), +`state` really should have been a requirement in the standard! ### 3. Requesting the access token -To request the access token, you should use the returned code and exchange it for an access token. To do that you can use any HTTP client. In this case, I used rest-client: +Once you have the authorization code you can request an `access_token` using the code, to do that you can use any HTTP client. In the following example, we are using Ruby's `rest-client`: ``` parameters = 'client_id=APP_ID&client_secret=APP_SECRET&code=RETURNED_CODE&grant_type=authorization_code&redirect_uri=REDIRECT_URI' @@ -46,6 +67,8 @@ RestClient.post 'http://localhost:3000/oauth/token', parameters "refresh_token": "8257e65c97202ed1726cf9571600918f3bffb2544b26e00a61df9897668c33a1" } ``` +>**Note:** +The `redirect_uri` must match the `redirect_uri` used in the original authorization request. You can now make requests to the API with the access token returned. @@ -77,6 +100,9 @@ The credentials should only be used when there is a high degree of trust between client is part of the device operating system or a highly privileged application), and when other authorization grant types are not available (such as an authorization code). +>**Important:** +Never store the users credentials and only use this grant type when your client is deployed to a trusted environment, in 99% of cases [personal access tokens] are a better choice. + Even though this grant type requires direct client access to the resource owner credentials, the resource owner credentials are used for a single request and are exchanged for an access token. This grant type can eliminate the need for the client to store the resource owner credentials for future use, by exchanging the credentials with a long-lived access token or refresh token. diff --git a/doc/api/projects.md b/doc/api/projects.md index 727cb44f335..37d97b2db44 100644 --- a/doc/api/projects.md +++ b/doc/api/projects.md @@ -858,95 +858,9 @@ Parameters: In Markdown contexts, the link is automatically expanded when the format in `markdown` is used. -## Team members +## Project members -### List project team members - -Get a list of a project's team members. - -``` -GET /projects/:id/members -``` - -Parameters: - -- `id` (required) - The ID or NAMESPACE/PROJECT_NAME of a project -- `query` (optional) - Query string to search for members - -### Get project team member - -Gets a project team member. - -``` -GET /projects/:id/members/:user_id -``` - -Parameters: - -- `id` (required) - The ID or NAMESPACE/PROJECT_NAME of a project -- `user_id` (required) - The ID of a user - -```json -{ - "id": 1, - "username": "john_smith", - "email": "john@example.com", - "name": "John Smith", - "state": "active", - "created_at": "2012-05-23T08:00:58Z", - "access_level": 40 -} -``` - -### Add project team member - -Adds a user to a project team. This is an idempotent method and can be called multiple times -with the same parameters. Adding team membership to a user that is already a member does not -affect the existing membership. - -``` -POST /projects/:id/members -``` - -Parameters: - -- `id` (required) - The ID or NAMESPACE/PROJECT_NAME of a project -- `user_id` (required) - The ID of a user to add -- `access_level` (required) - Project access level - -### Edit project team member - -Updates a project team member to a specified access level. - -``` -PUT /projects/:id/members/:user_id -``` - -Parameters: - -- `id` (required) - The ID or NAMESPACE/PROJECT_NAME of a project -- `user_id` (required) - The ID of a team member -- `access_level` (required) - Project access level - -### Remove project team member - -Removes a user from a project team. - -``` -DELETE /projects/:id/members/:user_id -``` - -Parameters: - -- `id` (required) - The ID or NAMESPACE/PROJECT_NAME of a project -- `user_id` (required) - The ID of a team member - -This method removes the project member if the user has the proper access rights to do so. -It returns a status code 403 if the member does not have the proper rights to perform this action. -In all other cases this method is idempotent and revoking team membership for a user who is not -currently a team member is considered success. -Please note that the returned JSON currently differs slightly. Thus you should not -rely on the returned JSON structure. +Please consult the [Project Members](members.md) documentation. ### Share project with group diff --git a/features/steps/shared/builds.rb b/features/steps/shared/builds.rb index 4d6b258f577..c7f61da05fa 100644 --- a/features/steps/shared/builds.rb +++ b/features/steps/shared/builds.rb @@ -10,20 +10,22 @@ module SharedBuilds end step 'project has a recent build' do - @pipeline = create(:ci_pipeline, project: @project, sha: @project.commit.sha, ref: 'master') + @pipeline = create(:ci_empty_pipeline, project: @project, sha: @project.commit.sha, ref: 'master') @build = create(:ci_build_with_coverage, pipeline: @pipeline) + @pipeline.reload_status! end step 'recent build is successful' do - @build.update(status: 'success') + @build.success end step 'recent build failed' do - @build.update(status: 'failed') + @build.drop end step 'project has another build that is running' do create(:ci_build, pipeline: @pipeline, name: 'second build', status: 'running') + @pipeline.reload_status! end step 'I visit recent build details page' do diff --git a/lib/api/access_requests.rb b/lib/api/access_requests.rb new file mode 100644 index 00000000000..d02b469dac8 --- /dev/null +++ b/lib/api/access_requests.rb @@ -0,0 +1,90 @@ +module API + class AccessRequests < Grape::API + before { authenticate! } + + helpers ::API::Helpers::MembersHelpers + + %w[group project].each do |source_type| + resource source_type.pluralize do + # Get a list of group/project access requests viewable by the authenticated user. + # + # Parameters: + # id (required) - The group/project ID + # + # Example Request: + # GET /groups/:id/access_requests + # GET /projects/:id/access_requests + get ":id/access_requests" do + source = find_source(source_type, params[:id]) + authorize_admin_source!(source_type, source) + + access_requesters = paginate(source.requesters.includes(:user)) + + present access_requesters.map(&:user), with: Entities::AccessRequester, access_requesters: access_requesters + end + + # Request access to the group/project + # + # Parameters: + # id (required) - The group/project ID + # + # Example Request: + # POST /groups/:id/access_requests + # POST /projects/:id/access_requests + post ":id/access_requests" do + source = find_source(source_type, params[:id]) + access_requester = source.request_access(current_user) + + if access_requester.persisted? + present access_requester.user, with: Entities::AccessRequester, access_requester: access_requester + else + render_validation_error!(access_requester) + end + end + + # Approve a group/project access request + # + # Parameters: + # id (required) - The group/project ID + # user_id (required) - The user ID of the access requester + # access_level (optional) - Access level + # + # Example Request: + # PUT /groups/:id/access_requests/:user_id/approve + # PUT /projects/:id/access_requests/:user_id/approve + put ':id/access_requests/:user_id/approve' do + required_attributes! [:user_id] + source = find_source(source_type, params[:id]) + authorize_admin_source!(source_type, source) + + member = source.requesters.find_by!(user_id: params[:user_id]) + if params[:access_level] + member.update(access_level: params[:access_level]) + end + member.accept_request + + status :created + present member.user, with: Entities::Member, member: member + end + + # Deny a group/project access request + # + # Parameters: + # id (required) - The group/project ID + # user_id (required) - The user ID of the access requester + # + # Example Request: + # DELETE /groups/:id/access_requests/:user_id + # DELETE /projects/:id/access_requests/:user_id + delete ":id/access_requests/:user_id" do + required_attributes! [:user_id] + source = find_source(source_type, params[:id]) + + access_requester = source.requesters.find_by!(user_id: params[:user_id]) + + ::Members::DestroyService.new(access_requester, current_user).execute + end + end + end + end +end diff --git a/lib/api/api.rb b/lib/api/api.rb index 6cd4a853dbe..d43af3f24e9 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -3,6 +3,10 @@ module API include APIGuard version 'v3', using: :path + rescue_from Gitlab::Access::AccessDeniedError do + rack_response({ 'message' => '403 Forbidden' }.to_json, 403) + end + rescue_from ActiveRecord::RecordNotFound do rack_response({ 'message' => '404 Not found' }.to_json, 404) end @@ -32,6 +36,7 @@ module API # Ensure the namespace is right, otherwise we might load Grape::API::Helpers helpers ::API::Helpers + mount ::API::AccessRequests mount ::API::AwardEmoji mount ::API::Branches mount ::API::Builds @@ -40,19 +45,18 @@ module API mount ::API::DeployKeys mount ::API::Environments mount ::API::Files - mount ::API::GroupMembers mount ::API::Groups mount ::API::Internal mount ::API::Issues mount ::API::Keys mount ::API::Labels mount ::API::LicenseTemplates + mount ::API::Members mount ::API::MergeRequests mount ::API::Milestones mount ::API::Namespaces mount ::API::Notes mount ::API::ProjectHooks - mount ::API::ProjectMembers mount ::API::ProjectSnippets mount ::API::Projects mount ::API::Repositories diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 0178bbe7d1d..e8a3a6a9df0 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -92,9 +92,17 @@ module API end end - class ProjectMember < UserBasic + class Member < UserBasic expose :access_level do |user, options| - options[:project].project_members.find_by(user_id: user.id).access_level + member = options[:member] || options[:members].find { |m| m.user_id == user.id } + member.access_level + end + end + + class AccessRequester < UserBasic + expose :requested_at do |user, options| + access_requester = options[:access_requester] || options[:access_requesters].find { |m| m.user_id == user.id } + access_requester.requested_at end end @@ -109,12 +117,6 @@ module API expose :shared_projects, using: Entities::Project end - class GroupMember < UserBasic - expose :access_level do |user, options| - options[:group].group_members.find_by(user_id: user.id).access_level - end - end - class RepoBranch < Grape::Entity expose :name @@ -326,7 +328,7 @@ module API expose :id, :path, :kind end - class Member < Grape::Entity + class MemberAccess < Grape::Entity expose :access_level expose :notification_level do |member, options| if member.notification_setting @@ -335,10 +337,10 @@ module API end end - class ProjectAccess < Member + class ProjectAccess < MemberAccess end - class GroupAccess < Member + class GroupAccess < MemberAccess end class ProjectService < Grape::Entity diff --git a/lib/api/group_members.rb b/lib/api/group_members.rb deleted file mode 100644 index dbe5bb08d3f..00000000000 --- a/lib/api/group_members.rb +++ /dev/null @@ -1,87 +0,0 @@ -module API - class GroupMembers < Grape::API - before { authenticate! } - - resource :groups do - # Get a list of group members viewable by the authenticated user. - # - # Example Request: - # GET /groups/:id/members - get ":id/members" do - group = find_group(params[:id]) - users = group.users - present users, with: Entities::GroupMember, group: group - end - - # Add a user to the list of group members - # - # Parameters: - # id (required) - group id - # user_id (required) - the users id - # access_level (required) - Project access level - # Example Request: - # POST /groups/:id/members - post ":id/members" do - group = find_group(params[:id]) - authorize! :admin_group, group - required_attributes! [:user_id, :access_level] - - unless validate_access_level?(params[:access_level]) - render_api_error!("Wrong access level", 422) - end - - if group.group_members.find_by(user_id: params[:user_id]) - render_api_error!("Already exists", 409) - end - - group.add_users([params[:user_id]], params[:access_level], current_user) - member = group.group_members.find_by(user_id: params[:user_id]) - present member.user, with: Entities::GroupMember, group: group - end - - # Update group member - # - # Parameters: - # id (required) - The ID of a group - # user_id (required) - The ID of a group member - # access_level (required) - Project access level - # Example Request: - # PUT /groups/:id/members/:user_id - put ':id/members/:user_id' do - group = find_group(params[:id]) - authorize! :admin_group, group - required_attributes! [:access_level] - - group_member = group.group_members.find_by(user_id: params[:user_id]) - not_found!('User can not be found') if group_member.nil? - - if group_member.update_attributes(access_level: params[:access_level]) - @member = group_member.user - present @member, with: Entities::GroupMember, group: group - else - handle_member_errors group_member.errors - end - end - - # Remove member. - # - # Parameters: - # id (required) - group id - # user_id (required) - the users id - # - # Example Request: - # DELETE /groups/:id/members/:user_id - delete ":id/members/:user_id" do - group = find_group(params[:id]) - authorize! :admin_group, group - member = group.group_members.find_by(user_id: params[:user_id]) - - if member.nil? - render_api_error!("404 Not Found - user_id:#{params[:user_id]} not a member of group #{group.name}", 404) - else - member.destroy - end - end - end - end -end diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 130509cdad6..d0469d6602d 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -28,7 +28,7 @@ module API # If the sudo is the current user do nothing if identifier && !(@current_user.id == identifier || @current_user.username == identifier) - render_api_error!('403 Forbidden: Must be admin to use sudo', 403) unless @current_user.is_admin? + forbidden!('Must be admin to use sudo') unless @current_user.is_admin? @current_user = User.by_username_or_id(identifier) not_found!("No user id or username for: #{identifier}") if @current_user.nil? end @@ -49,16 +49,15 @@ module API def user_project @project ||= find_project(params[:id]) - @project || not_found!("Project") end def find_project(id) project = Project.find_with_namespace(id) || Project.find_by(id: id) - if project && can?(current_user, :read_project, project) + if can?(current_user, :read_project, project) project else - nil + not_found!('Project') end end @@ -89,11 +88,7 @@ module API end def find_group(id) - begin - group = Group.find(id) - rescue ActiveRecord::RecordNotFound - group = Group.find_by!(path: id) - end + group = Group.find_by(path: id) || Group.find_by(id: id) if can?(current_user, :read_group, group) group @@ -135,7 +130,7 @@ module API end def authorize!(action, subject) - forbidden! unless abilities.allowed?(current_user, action, subject) + forbidden! unless can?(current_user, action, subject) end def authorize_push_project @@ -197,10 +192,6 @@ module API errors end - def validate_access_level?(level) - Gitlab::Access.options_with_owner.values.include? level.to_i - end - # Checks the occurrences of datetime attributes, each attribute if present in the params hash must be in ISO 8601 # format (YYYY-MM-DDTHH:MM:SSZ) or a Bad Request error is invoked. # @@ -411,11 +402,6 @@ module API File.read(Gitlab.config.gitlab_shell.secret_file).chomp end - def handle_member_errors(errors) - error!(errors[:access_level], 422) if errors[:access_level].any? - not_found!(errors) - end - def send_git_blob(repository, blob) env['api.format'] = :txt content_type 'text/plain' diff --git a/lib/api/helpers/members_helpers.rb b/lib/api/helpers/members_helpers.rb new file mode 100644 index 00000000000..90114f6f667 --- /dev/null +++ b/lib/api/helpers/members_helpers.rb @@ -0,0 +1,13 @@ +module API + module Helpers + module MembersHelpers + def find_source(source_type, id) + public_send("find_#{source_type}", id) + end + + def authorize_admin_source!(source_type, source) + authorize! :"admin_#{source_type}", source + end + end + end +end diff --git a/lib/api/members.rb b/lib/api/members.rb new file mode 100644 index 00000000000..2fae83f60b2 --- /dev/null +++ b/lib/api/members.rb @@ -0,0 +1,155 @@ +module API + class Members < Grape::API + before { authenticate! } + + helpers ::API::Helpers::MembersHelpers + + %w[group project].each do |source_type| + resource source_type.pluralize do + # Get a list of group/project members viewable by the authenticated user. + # + # Parameters: + # id (required) - The group/project ID + # query - Query string + # + # Example Request: + # GET /groups/:id/members + # GET /projects/:id/members + get ":id/members" do + source = find_source(source_type, params[:id]) + + members = source.members.includes(:user) + members = members.joins(:user).merge(User.search(params[:query])) if params[:query] + members = paginate(members) + + present members.map(&:user), with: Entities::Member, members: members + end + + # Get a group/project member + # + # Parameters: + # id (required) - The group/project ID + # user_id (required) - The user ID of the member + # + # Example Request: + # GET /groups/:id/members/:user_id + # GET /projects/:id/members/:user_id + get ":id/members/:user_id" do + source = find_source(source_type, params[:id]) + + members = source.members + member = members.find_by!(user_id: params[:user_id]) + + present member.user, with: Entities::Member, member: member + end + + # Add a new group/project member + # + # Parameters: + # id (required) - The group/project ID + # user_id (required) - The user ID of the new member + # access_level (required) - A valid access level + # + # Example Request: + # POST /groups/:id/members + # POST /projects/:id/members + post ":id/members" do + source = find_source(source_type, params[:id]) + authorize_admin_source!(source_type, source) + required_attributes! [:user_id, :access_level] + + access_requester = source.requesters.find_by(user_id: params[:user_id]) + if access_requester + # We pass current_user = access_requester so that the requester doesn't + # receive a "access denied" email + ::Members::DestroyService.new(access_requester, access_requester.user).execute + end + + member = source.members.find_by(user_id: params[:user_id]) + + # This is to ensure back-compatibility but 409 behavior should be used + # for both project and group members in 9.0! + conflict!('Member already exists') if source_type == 'group' && member + + unless member + source.add_user(params[:user_id], params[:access_level], current_user) + member = source.members.find_by(user_id: params[:user_id]) + end + + if member + present member.user, with: Entities::Member, member: member + else + # Since `source.add_user` doesn't return a member object, we have to + # build a new one and populate its errors in order to render them. + member = source.members.build(attributes_for_keys([:user_id, :access_level])) + member.valid? # populate the errors + + # 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 + + # Update a group/project member + # + # Parameters: + # id (required) - The group/project ID + # user_id (required) - The user ID of the member + # access_level (required) - A valid access level + # + # Example Request: + # PUT /groups/:id/members/:user_id + # PUT /projects/:id/members/:user_id + put ":id/members/:user_id" do + source = find_source(source_type, params[:id]) + authorize_admin_source!(source_type, source) + required_attributes! [:user_id, :access_level] + + member = source.members.find_by!(user_id: params[:user_id]) + + if member.update_attributes(access_level: params[:access_level]) + 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 + + # Remove a group/project member + # + # Parameters: + # id (required) - The group/project ID + # user_id (required) - The user ID of the member + # + # Example Request: + # DELETE /groups/:id/members/:user_id + # DELETE /projects/:id/members/:user_id + delete ":id/members/:user_id" do + source = find_source(source_type, params[:id]) + required_attributes! [:user_id] + + # This is to ensure back-compatibility but find_by! should be used + # in that casse in 9.0! + member = source.members.find_by(user_id: params[:user_id]) + + # This is to ensure back-compatibility but this should be removed in + # favor of find_by! in 9.0! + not_found!("Member: user_id:#{params[:user_id]}") if source_type == 'group' && member.nil? + + # This is to ensure back-compatibility but 204 behavior should be used + # for all DELETE endpoints in 9.0! + if member.nil? + { message: "Access revoked", id: params[:user_id].to_i } + else + ::Members::DestroyService.new(member, current_user).execute + + present member.user, with: Entities::Member, member: member + end + end + end + end + end +end diff --git a/lib/api/project_members.rb b/lib/api/project_members.rb deleted file mode 100644 index 6a0b3e7d134..00000000000 --- a/lib/api/project_members.rb +++ /dev/null @@ -1,110 +0,0 @@ -module API - # Projects members API - class ProjectMembers < Grape::API - before { authenticate! } - - resource :projects do - # Get a project team members - # - # Parameters: - # id (required) - The ID of a project - # query - Query string - # Example Request: - # GET /projects/:id/members - get ":id/members" do - if params[:query].present? - @members = paginate user_project.users.where("username LIKE ?", "%#{params[:query]}%") - else - @members = paginate user_project.users - end - present @members, with: Entities::ProjectMember, project: user_project - end - - # Get a project team members - # - # Parameters: - # id (required) - The ID of a project - # user_id (required) - The ID of a user - # Example Request: - # GET /projects/:id/members/:user_id - get ":id/members/:user_id" do - @member = user_project.users.find params[:user_id] - present @member, with: Entities::ProjectMember, project: user_project - end - - # Add a new project team member - # - # Parameters: - # id (required) - The ID of a project - # user_id (required) - The ID of a user - # access_level (required) - Project access level - # Example Request: - # POST /projects/:id/members - post ":id/members" do - authorize! :admin_project, user_project - required_attributes! [:user_id, :access_level] - - # either the user is already a team member or a new one - project_member = user_project.project_member(params[:user_id]) - if project_member.nil? - project_member = user_project.project_members.new( - user_id: params[:user_id], - access_level: params[:access_level] - ) - end - - if project_member.save - @member = project_member.user - present @member, with: Entities::ProjectMember, project: user_project - else - handle_member_errors project_member.errors - end - end - - # Update project team member - # - # Parameters: - # id (required) - The ID of a project - # user_id (required) - The ID of a team member - # access_level (required) - Project access level - # Example Request: - # PUT /projects/:id/members/:user_id - put ":id/members/:user_id" do - authorize! :admin_project, user_project - required_attributes! [:access_level] - - project_member = user_project.project_members.find_by(user_id: params[:user_id]) - not_found!("User can not be found") if project_member.nil? - - if project_member.update_attributes(access_level: params[:access_level]) - @member = project_member.user - present @member, with: Entities::ProjectMember, project: user_project - else - handle_member_errors project_member.errors - end - end - - # Remove a team member from project - # - # Parameters: - # id (required) - The ID of a project - # user_id (required) - The ID of a team member - # Example Request: - # DELETE /projects/:id/members/:user_id - delete ":id/members/:user_id" do - project_member = user_project.project_members.find_by(user_id: params[:user_id]) - - unless current_user.can?(:admin_project, user_project) || - current_user.can?(:destroy_project_member, project_member) - forbidden! - end - - if project_member.nil? - { message: "Access revoked", id: params[:user_id].to_i } - else - project_member.destroy - end - end - end - end -end diff --git a/lib/backup/files.rb b/lib/backup/files.rb index 654b4d1c896..cedbb289f6a 100644 --- a/lib/backup/files.rb +++ b/lib/backup/files.rb @@ -27,7 +27,7 @@ module Backup def backup_existing_files_dir timestamped_files_path = File.join(files_parent_dir, "#{name}.#{Time.now.to_i}") - if File.exists?(app_files_dir) + if File.exist?(app_files_dir) FileUtils.mv(app_files_dir, File.expand_path(timestamped_files_path)) end end diff --git a/lib/backup/manager.rb b/lib/backup/manager.rb index 2ff3e3bdfb0..0dfffaf0bc6 100644 --- a/lib/backup/manager.rb +++ b/lib/backup/manager.rb @@ -114,7 +114,7 @@ module Backup tar_file = ENV["BACKUP"].nil? ? File.join("#{file_list.first}_gitlab_backup.tar") : File.join(ENV["BACKUP"] + "_gitlab_backup.tar") - unless File.exists?(tar_file) + unless File.exist?(tar_file) puts "The specified backup doesn't exist!" exit 1 end diff --git a/lib/backup/repository.rb b/lib/backup/repository.rb index 1f5917b8127..f117fc3d37d 100644 --- a/lib/backup/repository.rb +++ b/lib/backup/repository.rb @@ -28,7 +28,7 @@ module Backup wiki = ProjectWiki.new(project) - if File.exists?(path_to_repo(wiki)) + if File.exist?(path_to_repo(wiki)) $progress.print " * #{wiki.path_with_namespace} ... " if wiki.repository.empty? $progress.puts " [SKIPPED]".color(:cyan) @@ -49,7 +49,7 @@ module Backup def restore Gitlab.config.repositories.storages.each do |name, path| - next unless File.exists?(path) + next unless File.exist?(path) # Move repos dir to 'repositories.old' dir bk_repos_path = File.join(path, '..', 'repositories.old.' + Time.now.to_i.to_s) @@ -63,7 +63,7 @@ module Backup project.ensure_dir_exist - if File.exists?(path_to_bundle(project)) + if File.exist?(path_to_bundle(project)) FileUtils.mkdir_p(path_to_repo(project)) cmd = %W(tar -xf #{path_to_bundle(project)} -C #{path_to_repo(project)}) else @@ -80,7 +80,7 @@ module Backup wiki = ProjectWiki.new(project) - if File.exists?(path_to_bundle(wiki)) + if File.exist?(path_to_bundle(wiki)) $progress.print " * #{wiki.path_with_namespace} ... " # If a wiki bundle exists, first remove the empty repo diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb index a2e8bd22a52..47efd5bd9f2 100644 --- a/lib/ci/gitlab_ci_yaml_processor.rb +++ b/lib/ci/gitlab_ci_yaml_processor.rb @@ -62,7 +62,7 @@ module Ci # - before script should be a concatenated command commands: [job[:before_script] || @before_script, job[:script]].flatten.compact.join("\n"), tag_list: job[:tags] || [], - name: job[:name], + name: job[:name].to_s, allow_failure: job[:allow_failure] || false, when: job[:when] || 'on_success', environment: job[:environment], diff --git a/lib/extracts_path.rb b/lib/extracts_path.rb index 51e46da82cc..84688f6646e 100644 --- a/lib/extracts_path.rb +++ b/lib/extracts_path.rb @@ -94,7 +94,7 @@ module ExtractsPath @options = params.select {|key, value| allowed_options.include?(key) && !value.blank? } @options = HashWithIndifferentAccess.new(@options) - @id = Addressable::URI.unescape(get_id) + @id = Addressable::URI.normalize_component(get_id) @ref, @path = extract_ref(@id) @repo = @project.repository if @options[:extended_sha1].blank? diff --git a/lib/gitlab/backend/grack_auth.rb b/lib/gitlab/backend/grack_auth.rb deleted file mode 100644 index ab94abeda77..00000000000 --- a/lib/gitlab/backend/grack_auth.rb +++ /dev/null @@ -1,163 +0,0 @@ -module Grack - class AuthSpawner - def self.call(env) - # Avoid issues with instance variables in Grack::Auth persisting across - # requests by creating a new instance for each request. - Auth.new({}).call(env) - end - end - - class Auth < Rack::Auth::Basic - attr_accessor :user, :project, :env - - def call(env) - @env = env - @request = Rack::Request.new(env) - @auth = Request.new(env) - - @ci = false - - # Need this patch due to the rails mount - # Need this if under RELATIVE_URL_ROOT - unless Gitlab.config.gitlab.relative_url_root.empty? - # If website is mounted using relative_url_root need to remove it first - @env['PATH_INFO'] = @request.path.sub(Gitlab.config.gitlab.relative_url_root, '') - else - @env['PATH_INFO'] = @request.path - end - - @env['SCRIPT_NAME'] = "" - - auth! - - lfs_response = Gitlab::Lfs::Router.new(project, @user, @ci, @request).try_call - return lfs_response unless lfs_response.nil? - - if @user.nil? && !@ci - unauthorized - else - render_not_found - end - end - - private - - def auth! - return unless @auth.provided? - - return bad_request unless @auth.basic? - - # Authentication with username and password - login, password = @auth.credentials - - # Allow authentication for GitLab CI service - # if valid token passed - if ci_request?(login, password) - @ci = true - return - end - - @user = authenticate_user(login, password) - end - - def ci_request?(login, password) - matched_login = /(?<s>^[a-zA-Z]*-ci)-token$/.match(login) - - if project && matched_login.present? - underscored_service = matched_login['s'].underscore - - if underscored_service == 'gitlab_ci' - return project && project.valid_build_token?(password) - elsif Service.available_services_names.include?(underscored_service) - service_method = "#{underscored_service}_service" - service = project.send(service_method) - - return service && service.activated? && service.valid_token?(password) - end - end - - false - end - - def oauth_access_token_check(login, password) - if login == "oauth2" && git_cmd == 'git-upload-pack' && password.present? - token = Doorkeeper::AccessToken.by_token(password) - token && token.accessible? && User.find_by(id: token.resource_owner_id) - end - end - - def authenticate_user(login, password) - user = Gitlab::Auth.find_with_user_password(login, password) - - unless user - user = oauth_access_token_check(login, password) - end - - # If the user authenticated successfully, we reset the auth failure count - # from Rack::Attack for that IP. A client may attempt to authenticate - # with a username and blank password first, and only after it receives - # a 401 error does it present a password. Resetting the count prevents - # false positives from occurring. - # - # Otherwise, we let Rack::Attack know there was a failed authentication - # attempt from this IP. This information is stored in the Rails cache - # (Redis) and will be used by the Rack::Attack middleware to decide - # whether to block requests from this IP. - config = Gitlab.config.rack_attack.git_basic_auth - - if config.enabled - if user - # A successful login will reset the auth failure count from this IP - Rack::Attack::Allow2Ban.reset(@request.ip, config) - else - banned = Rack::Attack::Allow2Ban.filter(@request.ip, config) do - # Unless the IP is whitelisted, return true so that Allow2Ban - # increments the counter (stored in Rails.cache) for the IP - if config.ip_whitelist.include?(@request.ip) - false - else - true - end - end - - if banned - Rails.logger.info "IP #{@request.ip} failed to login " \ - "as #{login} but has been temporarily banned from Git auth" - end - end - end - - user - end - - def git_cmd - if @request.get? - @request.params['service'] - elsif @request.post? - File.basename(@request.path) - else - nil - end - end - - def project - return @project if defined?(@project) - - @project = project_by_path(@request.path_info) - end - - def project_by_path(path) - if m = /^([\w\.\/-]+)\.git/.match(path).to_a - path_with_namespace = m.last - path_with_namespace.gsub!(/\.wiki$/, '') - - path_with_namespace[0] = '' if path_with_namespace.start_with?('/') - Project.find_with_namespace(path_with_namespace) - end - end - - def render_not_found - [404, { "Content-Type" => "text/plain" }, ["Not Found"]] - end - end -end diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb index 69943e22353..5bc70f530d2 100644 --- a/lib/gitlab/git_access.rb +++ b/lib/gitlab/git_access.rb @@ -134,7 +134,7 @@ module Gitlab end def build_status_object(status, message = '') - GitAccessStatus.new(status, message) + Gitlab::GitAccessStatus.new(status, message) end end end diff --git a/lib/gitlab/lfs/response.rb b/lib/gitlab/lfs/response.rb deleted file mode 100644 index a1ee1aa81ff..00000000000 --- a/lib/gitlab/lfs/response.rb +++ /dev/null @@ -1,329 +0,0 @@ -module Gitlab - module Lfs - class Response - def initialize(project, user, ci, request) - @origin_project = project - @project = storage_project(project) - @user = user - @ci = ci - @env = request.env - @request = request - end - - def render_download_object_response(oid) - render_response_to_download do - if check_download_sendfile_header? - render_lfs_sendfile(oid) - else - render_not_found - end - end - end - - def render_batch_operation_response - request_body = JSON.parse(@request.body.read) - case request_body["operation"] - when "download" - render_batch_download(request_body) - when "upload" - render_batch_upload(request_body) - else - render_not_found - end - end - - def render_storage_upload_authorize_response(oid, size) - render_response_to_push do - [ - 200, - { "Content-Type" => "application/json; charset=utf-8" }, - [JSON.dump({ - 'StoreLFSPath' => "#{Gitlab.config.lfs.storage_path}/tmp/upload", - 'LfsOid' => oid, - 'LfsSize' => size - })] - ] - end - end - - def render_storage_upload_store_response(oid, size, tmp_file_name) - return render_forbidden unless tmp_file_name - - render_response_to_push do - render_lfs_upload_ok(oid, size, tmp_file_name) - end - end - - def render_unsupported_deprecated_api - [ - 501, - { "Content-Type" => "application/json; charset=utf-8" }, - [JSON.dump({ - 'message' => 'Server supports batch API only, please update your Git LFS client to version 1.0.1 and up.', - 'documentation_url' => "#{Gitlab.config.gitlab.url}/help", - })] - ] - end - - private - - def render_not_enabled - [ - 501, - { - "Content-Type" => "application/json; charset=utf-8", - }, - [JSON.dump({ - 'message' => 'Git LFS is not enabled on this GitLab server, contact your admin.', - 'documentation_url' => "#{Gitlab.config.gitlab.url}/help", - })] - ] - end - - def render_unauthorized - [ - 401, - { - 'Content-Type' => 'text/plain' - }, - ['Unauthorized'] - ] - end - - def render_not_found - [ - 404, - { - "Content-Type" => "application/vnd.git-lfs+json" - }, - [JSON.dump({ - 'message' => 'Not found.', - 'documentation_url' => "#{Gitlab.config.gitlab.url}/help", - })] - ] - end - - def render_forbidden - [ - 403, - { - "Content-Type" => "application/vnd.git-lfs+json" - }, - [JSON.dump({ - 'message' => 'Access forbidden. Check your access level.', - 'documentation_url' => "#{Gitlab.config.gitlab.url}/help", - })] - ] - end - - def render_lfs_sendfile(oid) - return render_not_found unless oid.present? - - lfs_object = object_for_download(oid) - - if lfs_object && lfs_object.file.exists? - [ - 200, - { - # GitLab-workhorse will forward Content-Type header - "Content-Type" => "application/octet-stream", - "X-Sendfile" => lfs_object.file.path - }, - [] - ] - else - render_not_found - end - end - - def render_batch_upload(body) - return render_not_found if body.empty? || body['objects'].nil? - - render_response_to_push do - response = build_upload_batch_response(body['objects']) - [ - 200, - { - "Content-Type" => "application/json; charset=utf-8", - "Cache-Control" => "private", - }, - [JSON.dump(response)] - ] - end - end - - def render_batch_download(body) - return render_not_found if body.empty? || body['objects'].nil? - - render_response_to_download do - response = build_download_batch_response(body['objects']) - [ - 200, - { - "Content-Type" => "application/json; charset=utf-8", - "Cache-Control" => "private", - }, - [JSON.dump(response)] - ] - end - end - - def render_lfs_upload_ok(oid, size, tmp_file) - if store_file(oid, size, tmp_file) - [ - 200, - { - 'Content-Type' => 'text/plain', - 'Content-Length' => 0 - }, - [] - ] - else - [ - 422, - { 'Content-Type' => 'text/plain' }, - ["Unprocessable entity"] - ] - end - end - - def render_response_to_download - return render_not_enabled unless Gitlab.config.lfs.enabled - - unless @project.public? - return render_unauthorized unless @user || @ci - return render_forbidden unless user_can_fetch? - end - - yield - end - - def render_response_to_push - return render_not_enabled unless Gitlab.config.lfs.enabled - return render_unauthorized unless @user - return render_forbidden unless user_can_push? - - yield - end - - def check_download_sendfile_header? - @env['HTTP_X_SENDFILE_TYPE'].to_s == "X-Sendfile" - end - - def user_can_fetch? - # Check user access against the project they used to initiate the pull - @ci || @user.can?(:download_code, @origin_project) - end - - def user_can_push? - # Check user access against the project they used to initiate the push - @user.can?(:push_code, @origin_project) - end - - def storage_project(project) - if project.forked? - storage_project(project.forked_from_project) - else - project - end - end - - def store_file(oid, size, tmp_file) - tmp_file_path = File.join("#{Gitlab.config.lfs.storage_path}/tmp/upload", tmp_file) - - object = LfsObject.find_or_create_by(oid: oid, size: size) - if object.file.exists? - success = true - else - success = move_tmp_file_to_storage(object, tmp_file_path) - end - - if success - success = link_to_project(object) - end - - success - ensure - # Ensure that the tmp file is removed - FileUtils.rm_f(tmp_file_path) - end - - def object_for_download(oid) - @project.lfs_objects.find_by(oid: oid) - end - - def move_tmp_file_to_storage(object, path) - File.open(path) do |f| - object.file = f - end - - object.file.store! - object.save - end - - def link_to_project(object) - if object && !object.projects.exists?(@project.id) - object.projects << @project - object.save - end - end - - def select_existing_objects(objects) - objects_oids = objects.map { |o| o['oid'] } - @project.lfs_objects.where(oid: objects_oids).pluck(:oid).to_set - end - - def build_upload_batch_response(objects) - selected_objects = select_existing_objects(objects) - - upload_hypermedia_links(objects, selected_objects) - end - - def build_download_batch_response(objects) - selected_objects = select_existing_objects(objects) - - download_hypermedia_links(objects, selected_objects) - end - - def download_hypermedia_links(all_objects, existing_objects) - all_objects.each do |object| - if existing_objects.include?(object['oid']) - object['actions'] = { - 'download' => { - 'href' => "#{@origin_project.http_url_to_repo}/gitlab-lfs/objects/#{object['oid']}", - 'header' => { - 'Authorization' => @env['HTTP_AUTHORIZATION'] - }.compact - } - } - else - object['error'] = { - 'code' => 404, - 'message' => "Object does not exist on the server or you don't have permissions to access it", - } - end - end - - { 'objects' => all_objects } - end - - def upload_hypermedia_links(all_objects, existing_objects) - all_objects.each do |object| - # generate actions only for non-existing objects - next if existing_objects.include?(object['oid']) - - object['actions'] = { - 'upload' => { - 'href' => "#{@origin_project.http_url_to_repo}/gitlab-lfs/objects/#{object['oid']}/#{object['size']}", - 'header' => { - 'Authorization' => @env['HTTP_AUTHORIZATION'] - }.compact - } - } - end - - { 'objects' => all_objects } - end - end - end -end diff --git a/lib/gitlab/lfs/router.rb b/lib/gitlab/lfs/router.rb deleted file mode 100644 index f2a76a56b8f..00000000000 --- a/lib/gitlab/lfs/router.rb +++ /dev/null @@ -1,98 +0,0 @@ -module Gitlab - module Lfs - class Router - attr_reader :project, :user, :ci, :request - - def initialize(project, user, ci, request) - @project = project - @user = user - @ci = ci - @env = request.env - @request = request - end - - def try_call - return unless @request && @request.path.present? - - case @request.request_method - when 'GET' - get_response - when 'POST' - post_response - when 'PUT' - put_response - else - nil - end - end - - private - - def get_response - path_match = @request.path.match(/\/(info\/lfs|gitlab-lfs)\/objects\/([0-9a-f]{64})$/) - return nil unless path_match - - oid = path_match[2] - return nil unless oid - - case path_match[1] - when "info/lfs" - lfs.render_unsupported_deprecated_api - when "gitlab-lfs" - lfs.render_download_object_response(oid) - else - nil - end - end - - def post_response - post_path = @request.path.match(/\/info\/lfs\/objects(\/batch)?$/) - return nil unless post_path - - # Check for Batch API - if post_path[0].ends_with?("/info/lfs/objects/batch") - lfs.render_batch_operation_response - elsif post_path[0].ends_with?("/info/lfs/objects") - lfs.render_unsupported_deprecated_api - else - nil - end - end - - def put_response - object_match = @request.path.match(/\/gitlab-lfs\/objects\/([0-9a-f]{64})\/([0-9]+)(|\/authorize){1}$/) - return nil if object_match.nil? - - oid = object_match[1] - size = object_match[2].try(:to_i) - return nil if oid.nil? || size.nil? - - # GitLab-workhorse requests - # 1. Try to authorize the request - # 2. send a request with a header containing the name of the temporary file - if object_match[3] && object_match[3] == '/authorize' - lfs.render_storage_upload_authorize_response(oid, size) - else - tmp_file_name = sanitize_tmp_filename(@request.env['HTTP_X_GITLAB_LFS_TMP']) - lfs.render_storage_upload_store_response(oid, size, tmp_file_name) - end - end - - def lfs - return unless @project - - Gitlab::Lfs::Response.new(@project, @user, @ci, @request) - end - - def sanitize_tmp_filename(name) - if name.present? - name.gsub!(/^.*(\\|\/)/, '') - name = name.match(/[0-9a-f]{73}/) - name[0] if name - else - nil - end - end - end - end -end diff --git a/lib/tasks/gitlab/check.rake b/lib/tasks/gitlab/check.rake index 0894994200f..5f4a6bbfa35 100644 --- a/lib/tasks/gitlab/check.rake +++ b/lib/tasks/gitlab/check.rake @@ -64,7 +64,7 @@ namespace :gitlab do for_more_information( see_installation_guide_section "GitLab" ) - end + end end end @@ -73,7 +73,7 @@ namespace :gitlab do database_config_file = Rails.root.join("config", "database.yml") - if File.exists?(database_config_file) + if File.exist?(database_config_file) puts "yes".color(:green) else puts "no".color(:red) @@ -94,7 +94,7 @@ namespace :gitlab do gitlab_config_file = Rails.root.join("config", "gitlab.yml") - if File.exists?(gitlab_config_file) + if File.exist?(gitlab_config_file) puts "yes".color(:green) else puts "no".color(:red) @@ -113,7 +113,7 @@ namespace :gitlab do print "GitLab config outdated? ... " gitlab_config_file = Rails.root.join("config", "gitlab.yml") - unless File.exists?(gitlab_config_file) + unless File.exist?(gitlab_config_file) puts "can't check because of previous errors".color(:magenta) end @@ -144,7 +144,7 @@ namespace :gitlab do script_path = "/etc/init.d/gitlab" - if File.exists?(script_path) + if File.exist?(script_path) puts "yes".color(:green) else puts "no".color(:red) @@ -169,7 +169,7 @@ namespace :gitlab do recipe_path = Rails.root.join("lib/support/init.d/", "gitlab") script_path = "/etc/init.d/gitlab" - unless File.exists?(script_path) + unless File.exist?(script_path) puts "can't check because of previous errors".color(:magenta) return end @@ -361,7 +361,7 @@ namespace :gitlab do Gitlab.config.repositories.storages.each do |name, repo_base_path| print "#{name}... " - if File.exists?(repo_base_path) + if File.exist?(repo_base_path) puts "yes".color(:green) else puts "no".color(:red) @@ -385,7 +385,7 @@ namespace :gitlab do Gitlab.config.repositories.storages.each do |name, repo_base_path| print "#{name}... " - unless File.exists?(repo_base_path) + unless File.exist?(repo_base_path) puts "can't check because of previous errors".color(:magenta) return end @@ -408,7 +408,7 @@ namespace :gitlab do Gitlab.config.repositories.storages.each do |name, repo_base_path| print "#{name}... " - unless File.exists?(repo_base_path) + unless File.exist?(repo_base_path) puts "can't check because of previous errors".color(:magenta) return end @@ -438,7 +438,7 @@ namespace :gitlab do Gitlab.config.repositories.storages.each do |name, repo_base_path| print "#{name}... " - unless File.exists?(repo_base_path) + unless File.exist?(repo_base_path) puts "can't check because of previous errors".color(:magenta) return end diff --git a/lib/tasks/gitlab/shell.rake b/lib/tasks/gitlab/shell.rake index ba93945bd03..bb7eb852f1b 100644 --- a/lib/tasks/gitlab/shell.rake +++ b/lib/tasks/gitlab/shell.rake @@ -90,7 +90,7 @@ namespace :gitlab do task build_missing_projects: :environment do Project.find_each(batch_size: 1000) do |project| path_to_repo = project.repository.path_to_repo - if File.exists?(path_to_repo) + if File.exist?(path_to_repo) print '-' else if Gitlab::Shell.new.add_repository(project.repository_storage_path, diff --git a/lib/tasks/spinach.rake b/lib/tasks/spinach.rake index c0f860a82d2..8dbfa7751dc 100644 --- a/lib/tasks/spinach.rake +++ b/lib/tasks/spinach.rake @@ -46,7 +46,7 @@ def run_spinach_tests(tags) success = run_spinach_command(%W(--tags #{tags})) 3.times do |_| break if success - break unless File.exists?('tmp/spinach-rerun.txt') + break unless File.exist?('tmp/spinach-rerun.txt') tests = File.foreach('tmp/spinach-rerun.txt').map(&:chomp) puts '' diff --git a/spec/factories/ci/builds.rb b/spec/factories/ci/builds.rb index 1b32d560b16..0c93bbdfe26 100644 --- a/spec/factories/ci/builds.rb +++ b/spec/factories/ci/builds.rb @@ -7,6 +7,7 @@ FactoryGirl.define do stage_idx 0 ref 'master' tag false + status 'pending' created_at 'Di 29. Okt 09:50:00 CET 2013' started_at 'Di 29. Okt 09:51:28 CET 2013' finished_at 'Di 29. Okt 09:53:28 CET 2013' @@ -45,6 +46,10 @@ FactoryGirl.define do status 'pending' end + trait :created do + status 'created' + end + trait :manual do status 'skipped' self.when 'manual' diff --git a/spec/factories/ci/pipelines.rb b/spec/factories/ci/pipelines.rb index a039bef6f3c..04d66020c87 100644 --- a/spec/factories/ci/pipelines.rb +++ b/spec/factories/ci/pipelines.rb @@ -18,7 +18,9 @@ FactoryGirl.define do factory :ci_empty_pipeline, class: Ci::Pipeline do + ref 'master' sha '97de212e80737a608d939f648d959671fb0a0142' + status 'pending' project factory: :empty_project diff --git a/spec/factories/commit_statuses.rb b/spec/factories/commit_statuses.rb index 1e5c479616c..995f2080f10 100644 --- a/spec/factories/commit_statuses.rb +++ b/spec/factories/commit_statuses.rb @@ -7,6 +7,30 @@ FactoryGirl.define do started_at 'Tue, 26 Jan 2016 08:21:42 +0100' finished_at 'Tue, 26 Jan 2016 08:23:42 +0100' + trait :success do + status 'success' + end + + trait :failed do + status 'failed' + end + + trait :canceled do + status 'canceled' + end + + trait :running do + status 'running' + end + + trait :pending do + status 'pending' + end + + trait :created do + status 'created' + end + after(:build) do |build, evaluator| build.project = build.pipeline.project end diff --git a/spec/features/merge_requests/created_from_fork_spec.rb b/spec/features/merge_requests/created_from_fork_spec.rb index f676200ecf3..4d5d4aa121a 100644 --- a/spec/features/merge_requests/created_from_fork_spec.rb +++ b/spec/features/merge_requests/created_from_fork_spec.rb @@ -29,12 +29,16 @@ feature 'Merge request created from fork' do include WaitForAjax given(:pipeline) do - create(:ci_pipeline_with_two_job, project: fork_project, - sha: merge_request.diff_head_sha, - ref: merge_request.source_branch) + create(:ci_pipeline, + project: fork_project, + sha: merge_request.diff_head_sha, + ref: merge_request.source_branch) end - background { pipeline.create_builds(user) } + background do + create(:ci_build, pipeline: pipeline, name: 'rspec') + create(:ci_build, pipeline: pipeline, name: 'spinach') + end scenario 'user visits a pipelines page', js: true do visit_merge_request(merge_request) diff --git a/spec/features/pipelines_spec.rb b/spec/features/pipelines_spec.rb index eace76c370f..f88b8f8e60b 100644 --- a/spec/features/pipelines_spec.rb +++ b/spec/features/pipelines_spec.rb @@ -33,7 +33,10 @@ describe "Pipelines" do context 'cancelable pipeline' do let!(:running) { create(:ci_build, :running, pipeline: pipeline, stage: 'test', commands: 'test') } - before { visit namespace_project_pipelines_path(project.namespace, project) } + before do + pipeline.reload_status! + visit namespace_project_pipelines_path(project.namespace, project) + end it { expect(page).to have_link('Cancel') } it { expect(page).to have_selector('.ci-running') } @@ -49,7 +52,10 @@ describe "Pipelines" do context 'retryable pipelines' do let!(:failed) { create(:ci_build, :failed, pipeline: pipeline, stage: 'test', commands: 'test') } - before { visit namespace_project_pipelines_path(project.namespace, project) } + before do + pipeline.reload_status! + visit namespace_project_pipelines_path(project.namespace, project) + end it { expect(page).to have_link('Retry') } it { expect(page).to have_selector('.ci-failed') } @@ -80,7 +86,10 @@ describe "Pipelines" do context 'when running' do let!(:running) { create(:generic_commit_status, status: 'running', pipeline: pipeline, stage: 'test') } - before { visit namespace_project_pipelines_path(project.namespace, project) } + before do + pipeline.reload_status! + visit namespace_project_pipelines_path(project.namespace, project) + end it 'is not cancelable' do expect(page).not_to have_link('Cancel') @@ -92,9 +101,12 @@ describe "Pipelines" do end context 'when failed' do - let!(:running) { create(:generic_commit_status, status: 'failed', pipeline: pipeline, stage: 'test') } + let!(:failed) { create(:generic_commit_status, status: 'failed', pipeline: pipeline, stage: 'test') } - before { visit namespace_project_pipelines_path(project.namespace, project) } + before do + pipeline.reload_status! + visit namespace_project_pipelines_path(project.namespace, project) + end it 'is not retryable' do expect(page).not_to have_link('Retry') @@ -211,7 +223,7 @@ describe "Pipelines" do context 'for invalid commit' do before do - fill_in('Create for', with: 'invalid reference') + fill_in('Create for', with: 'invalid-reference') click_on 'Create pipeline' end diff --git a/spec/fixtures/project_services/campfire/rooms.json b/spec/fixtures/project_services/campfire/rooms.json new file mode 100644 index 00000000000..71e9645c955 --- /dev/null +++ b/spec/fixtures/project_services/campfire/rooms.json @@ -0,0 +1,22 @@ +{ + "rooms": [ + { + "name": "test-room", + "locked": false, + "created_at": "2009/01/07 20:43:11 +0000", + "updated_at": "2009/03/18 14:31:39 +0000", + "topic": "The room topic\n", + "id": 123, + "membership_limit": 4 + }, + { + "name": "another room", + "locked": true, + "created_at": "2009/03/18 14:30:42 +0000", + "updated_at": "2013/01/27 14:14:27 +0000", + "topic": "Comment, ideas, GitHub notifications for eCommittee App", + "id": 456, + "membership_limit": 4 + } + ] +} diff --git a/spec/fixtures/project_services/campfire/rooms2.json b/spec/fixtures/project_services/campfire/rooms2.json new file mode 100644 index 00000000000..3d5f635d8b3 --- /dev/null +++ b/spec/fixtures/project_services/campfire/rooms2.json @@ -0,0 +1,22 @@ +{ + "rooms": [ + { + "name": "test-room-not-found", + "locked": false, + "created_at": "2009/01/07 20:43:11 +0000", + "updated_at": "2009/03/18 14:31:39 +0000", + "topic": "The room topic\n", + "id": 123, + "membership_limit": 4 + }, + { + "name": "another room", + "locked": true, + "created_at": "2009/03/18 14:30:42 +0000", + "updated_at": "2013/01/27 14:14:27 +0000", + "topic": "Comment, ideas, GitHub notifications for eCommittee App", + "id": 456, + "membership_limit": 4 + } + ] +} diff --git a/spec/lib/ci/charts_spec.rb b/spec/lib/ci/charts_spec.rb index 034ea098193..2cd6b00dad6 100644 --- a/spec/lib/ci/charts_spec.rb +++ b/spec/lib/ci/charts_spec.rb @@ -5,6 +5,7 @@ describe Ci::Charts, lib: true do before do @pipeline = FactoryGirl.create(:ci_pipeline) FactoryGirl.create(:ci_build, pipeline: @pipeline) + @pipeline.reload_status! end it 'returns build times in minutes' do diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb index 85374b8761d..be51d942af7 100644 --- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb +++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb @@ -19,7 +19,7 @@ module Ci expect(config_processor.builds_for_stage_and_ref(type, "master").first).to eq({ stage: "test", stage_idx: 1, - name: :rspec, + name: "rspec", commands: "pwd\nrspec", tag_list: [], options: {}, @@ -433,7 +433,7 @@ module Ci expect(config_processor.builds_for_stage_and_ref("test", "master").first).to eq({ stage: "test", stage_idx: 1, - name: :rspec, + name: "rspec", commands: "pwd\nrspec", tag_list: [], options: { @@ -461,7 +461,7 @@ module Ci expect(config_processor.builds_for_stage_and_ref("test", "master").first).to eq({ stage: "test", stage_idx: 1, - name: :rspec, + name: "rspec", commands: "pwd\nrspec", tag_list: [], options: { @@ -700,7 +700,7 @@ module Ci expect(config_processor.builds_for_stage_and_ref("test", "master").first).to eq({ stage: "test", stage_idx: 1, - name: :rspec, + name: "rspec", commands: "pwd\nrspec", tag_list: [], options: { @@ -837,7 +837,7 @@ module Ci expect(subject.first).to eq({ stage: "test", stage_idx: 1, - name: :normal_job, + name: "normal_job", commands: "test", tag_list: [], options: {}, @@ -882,7 +882,7 @@ module Ci expect(subject.first).to eq({ stage: "build", stage_idx: 0, - name: :job1, + name: "job1", commands: "execute-script-for-job", tag_list: [], options: {}, @@ -894,7 +894,7 @@ module Ci expect(subject.second).to eq({ stage: "build", stage_idx: 0, - name: :job2, + name: "job2", commands: "execute-script-for-job", tag_list: [], options: {}, diff --git a/spec/lib/extracts_path_spec.rb b/spec/lib/extracts_path_spec.rb index b12a7b98d4d..36c77206a3f 100644 --- a/spec/lib/extracts_path_spec.rb +++ b/spec/lib/extracts_path_spec.rb @@ -30,15 +30,28 @@ describe ExtractsPath, lib: true do expect(@logs_path).to eq("/#{@project.path_with_namespace}/refs/#{ref}/logs_tree/files/ruby/popen.rb") end - context 'escaped sequences in ref' do - let(:ref) { "improve%2Fawesome" } + context 'escaped slash character in ref' do + let(:ref) { 'improve%2Fawesome' } - it "id has no escape sequences" do + it 'has no escape sequences in @ref or @logs_path' do assign_ref_vars + expect(@ref).to eq('improve/awesome') expect(@logs_path).to eq("/#{@project.path_with_namespace}/refs/#{ref}/logs_tree/files/ruby/popen.rb") end end + + context 'ref contains %20' do + let(:ref) { 'foo%20bar' } + + it 'is not converted to a space in @id' do + @project.repository.add_branch(@project.owner, 'foo%20bar', 'master') + + assign_ref_vars + + expect(@id).to start_with('foo%20bar/') + end + end end describe '#extract_ref' do diff --git a/spec/lib/gitlab/badge/build_spec.rb b/spec/lib/gitlab/badge/build_spec.rb index ef9d9e7fef4..bb8144d5122 100644 --- a/spec/lib/gitlab/badge/build_spec.rb +++ b/spec/lib/gitlab/badge/build_spec.rb @@ -96,9 +96,10 @@ describe Gitlab::Badge::Build do end def create_build(project, sha, branch) - pipeline = create(:ci_pipeline, project: project, - sha: sha, - ref: branch) + pipeline = create(:ci_empty_pipeline, + project: project, + sha: sha, + ref: branch) create(:ci_build, pipeline: pipeline, stage: 'notify') end diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb index ba0242b071e..395316a8766 100644 --- a/spec/models/build_spec.rb +++ b/spec/models/build_spec.rb @@ -765,6 +765,53 @@ describe Ci::Build, models: true do end end + describe '#when' do + subject { build.when } + + context 'if is undefined' do + before do + build.when = nil + end + + context 'use from gitlab-ci.yml' do + before do + stub_ci_pipeline_yaml_file(config) + end + + context 'if config is not found' do + let(:config) { nil } + + it { is_expected.to eq('on_success') } + end + + context 'if config does not have a questioned job' do + let(:config) do + YAML.dump({ + test_other: { + script: 'Hello World' + } + }) + end + + it { is_expected.to eq('on_success') } + end + + context 'if config has when' do + let(:config) do + YAML.dump({ + test: { + script: 'Hello World', + when: 'always' + } + }) + end + + it { is_expected.to eq('always') } + end + end + end + end + describe '#retryable?' do context 'when build is running' do before do diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index a5c26cbe146..ceae2e60f2f 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -38,9 +38,6 @@ describe Ci::Pipeline, models: true do it { expect(pipeline.sha).to start_with(subject) } end - describe '#create_next_builds' do - end - describe '#retried' do subject { pipeline.retried } @@ -54,304 +51,20 @@ describe Ci::Pipeline, models: true do end end - describe '#create_builds' do - let!(:pipeline) { FactoryGirl.create :ci_pipeline, project: project, ref: 'master', tag: false } - - def create_builds(trigger_request = nil) - pipeline.create_builds(nil, trigger_request) - end - - def create_next_builds - pipeline.create_next_builds(pipeline.builds.order(:id).last) - end - - it 'creates builds' do - expect(create_builds).to be_truthy - pipeline.builds.update_all(status: "success") - expect(pipeline.builds.count(:all)).to eq(2) - - expect(create_next_builds).to be_truthy - pipeline.builds.update_all(status: "success") - expect(pipeline.builds.count(:all)).to eq(4) - - expect(create_next_builds).to be_truthy - pipeline.builds.update_all(status: "success") - expect(pipeline.builds.count(:all)).to eq(5) - - expect(create_next_builds).to be_falsey - end - - context 'custom stage with first job allowed to fail' do - let(:yaml) do - { - stages: ['clean', 'test'], - clean_job: { - stage: 'clean', - allow_failure: true, - script: 'BUILD', - }, - test_job: { - stage: 'test', - script: 'TEST', - }, - } - end - - before do - stub_ci_pipeline_yaml_file(YAML.dump(yaml)) - create_builds - end - - it 'properly schedules builds' do - expect(pipeline.builds.pluck(:status)).to contain_exactly('pending') - pipeline.builds.running_or_pending.each(&:drop) - expect(pipeline.builds.pluck(:status)).to contain_exactly('pending', 'failed') - end - end - - context 'properly creates builds when "when" is defined' do - let(:yaml) do - { - stages: ["build", "test", "test_failure", "deploy", "cleanup"], - build: { - stage: "build", - script: "BUILD", - }, - test: { - stage: "test", - script: "TEST", - }, - test_failure: { - stage: "test_failure", - script: "ON test failure", - when: "on_failure", - }, - deploy: { - stage: "deploy", - script: "PUBLISH", - }, - cleanup: { - stage: "cleanup", - script: "TIDY UP", - when: "always", - } - } - end - - before do - stub_ci_pipeline_yaml_file(YAML.dump(yaml)) - end - - context 'when builds are successful' do - it 'properly creates builds' do - expect(create_builds).to be_truthy - expect(pipeline.builds.pluck(:name)).to contain_exactly('build') - expect(pipeline.builds.pluck(:status)).to contain_exactly('pending') - pipeline.builds.running_or_pending.each(&:success) - - expect(pipeline.builds.pluck(:name)).to contain_exactly('build', 'test') - expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'pending') - pipeline.builds.running_or_pending.each(&:success) - - expect(pipeline.builds.pluck(:name)).to contain_exactly('build', 'test', 'deploy') - expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'success', 'pending') - pipeline.builds.running_or_pending.each(&:success) - - expect(pipeline.builds.pluck(:name)).to contain_exactly('build', 'test', 'deploy', 'cleanup') - expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'success', 'success', 'pending') - pipeline.builds.running_or_pending.each(&:success) - - expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'success', 'success', 'success') - pipeline.reload - expect(pipeline.status).to eq('success') - end - end - - context 'when test job fails' do - it 'properly creates builds' do - expect(create_builds).to be_truthy - expect(pipeline.builds.pluck(:name)).to contain_exactly('build') - expect(pipeline.builds.pluck(:status)).to contain_exactly('pending') - pipeline.builds.running_or_pending.each(&:success) - - expect(pipeline.builds.pluck(:name)).to contain_exactly('build', 'test') - expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'pending') - pipeline.builds.running_or_pending.each(&:drop) - - expect(pipeline.builds.pluck(:name)).to contain_exactly('build', 'test', 'test_failure') - expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'failed', 'pending') - pipeline.builds.running_or_pending.each(&:success) - - expect(pipeline.builds.pluck(:name)).to contain_exactly('build', 'test', 'test_failure', 'cleanup') - expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'failed', 'success', 'pending') - pipeline.builds.running_or_pending.each(&:success) - - expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'failed', 'success', 'success') - pipeline.reload - expect(pipeline.status).to eq('failed') - end - end - - context 'when test and test_failure jobs fail' do - it 'properly creates builds' do - expect(create_builds).to be_truthy - expect(pipeline.builds.pluck(:name)).to contain_exactly('build') - expect(pipeline.builds.pluck(:status)).to contain_exactly('pending') - pipeline.builds.running_or_pending.each(&:success) - - expect(pipeline.builds.pluck(:name)).to contain_exactly('build', 'test') - expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'pending') - pipeline.builds.running_or_pending.each(&:drop) - - expect(pipeline.builds.pluck(:name)).to contain_exactly('build', 'test', 'test_failure') - expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'failed', 'pending') - pipeline.builds.running_or_pending.each(&:drop) - - expect(pipeline.builds.pluck(:name)).to contain_exactly('build', 'test', 'test_failure', 'cleanup') - expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'failed', 'failed', 'pending') - pipeline.builds.running_or_pending.each(&:success) - - expect(pipeline.builds.pluck(:name)).to contain_exactly('build', 'test', 'test_failure', 'cleanup') - expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'failed', 'failed', 'success') - pipeline.reload - expect(pipeline.status).to eq('failed') - end - end - - context 'when deploy job fails' do - it 'properly creates builds' do - expect(create_builds).to be_truthy - expect(pipeline.builds.pluck(:name)).to contain_exactly('build') - expect(pipeline.builds.pluck(:status)).to contain_exactly('pending') - pipeline.builds.running_or_pending.each(&:success) - - expect(pipeline.builds.pluck(:name)).to contain_exactly('build', 'test') - expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'pending') - pipeline.builds.running_or_pending.each(&:success) - - expect(pipeline.builds.pluck(:name)).to contain_exactly('build', 'test', 'deploy') - expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'success', 'pending') - pipeline.builds.running_or_pending.each(&:drop) - - expect(pipeline.builds.pluck(:name)).to contain_exactly('build', 'test', 'deploy', 'cleanup') - expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'success', 'failed', 'pending') - pipeline.builds.running_or_pending.each(&:success) - - expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'success', 'failed', 'success') - pipeline.reload - expect(pipeline.status).to eq('failed') - end - end - - context 'when build is canceled in the second stage' do - it 'does not schedule builds after build has been canceled' do - expect(create_builds).to be_truthy - expect(pipeline.builds.pluck(:name)).to contain_exactly('build') - expect(pipeline.builds.pluck(:status)).to contain_exactly('pending') - pipeline.builds.running_or_pending.each(&:success) - - expect(pipeline.builds.running_or_pending).not_to be_empty - - expect(pipeline.builds.pluck(:name)).to contain_exactly('build', 'test') - expect(pipeline.builds.pluck(:status)).to contain_exactly('success', 'pending') - pipeline.builds.running_or_pending.each(&:cancel) - - expect(pipeline.builds.running_or_pending).to be_empty - expect(pipeline.reload.status).to eq('canceled') - end - end - - context 'when listing manual actions' do - let(:yaml) do - { - stages: ["build", "test", "staging", "production", "cleanup"], - build: { - stage: "build", - script: "BUILD", - }, - test: { - stage: "test", - script: "TEST", - }, - staging: { - stage: "staging", - script: "PUBLISH", - }, - production: { - stage: "production", - script: "PUBLISH", - when: "manual", - }, - cleanup: { - stage: "cleanup", - script: "TIDY UP", - when: "always", - }, - clear_cache: { - stage: "cleanup", - script: "CLEAR CACHE", - when: "manual", - } - } - end - - it 'returns only for skipped builds' do - # currently all builds are created - expect(create_builds).to be_truthy - expect(manual_actions).to be_empty - - # succeed stage build - pipeline.builds.running_or_pending.each(&:success) - expect(manual_actions).to be_empty - - # succeed stage test - pipeline.builds.running_or_pending.each(&:success) - expect(manual_actions).to be_empty - - # succeed stage staging and skip stage production - pipeline.builds.running_or_pending.each(&:success) - expect(manual_actions).to be_many # production and clear cache - - # succeed stage cleanup - pipeline.builds.running_or_pending.each(&:success) - - # after processing a pipeline we should have 6 builds, 5 succeeded - expect(pipeline.builds.count).to eq(6) - expect(pipeline.builds.success.count).to eq(4) - end - - def manual_actions - pipeline.manual_actions - end - end - end - - context 'when no builds created' do - let(:pipeline) { build(:ci_pipeline) } - - before do - stub_ci_pipeline_yaml_file(YAML.dump(before_script: ['ls'])) - end - - it 'returns false' do - expect(pipeline.create_builds(nil)).to be_falsey - expect(pipeline).not_to be_persisted - end - end - end - describe "#finished_at" do let(:pipeline) { FactoryGirl.create :ci_pipeline } it "returns finished_at of latest build" do build = FactoryGirl.create :ci_build, pipeline: pipeline, finished_at: Time.now - 60 FactoryGirl.create :ci_build, pipeline: pipeline, finished_at: Time.now - 120 + pipeline.reload_status! expect(pipeline.finished_at.to_i).to eq(build.finished_at.to_i) end it "returns nil if there is no finished build" do FactoryGirl.create :ci_not_started_build, pipeline: pipeline + pipeline.reload_status! expect(pipeline.finished_at).to be_nil end @@ -359,7 +72,7 @@ describe Ci::Pipeline, models: true do describe "coverage" do let(:project) { FactoryGirl.create :empty_project, build_coverage_regex: "/.*/" } - let(:pipeline) { FactoryGirl.create :ci_pipeline, project: project } + let(:pipeline) { FactoryGirl.create :ci_empty_pipeline, project: project } it "calculates average when there are two builds with coverage" do FactoryGirl.create :ci_build, name: "rspec", coverage: 30, pipeline: pipeline @@ -426,31 +139,30 @@ describe Ci::Pipeline, models: true do end end - describe '#update_state' do - it 'executes update_state after touching object' do - expect(pipeline).to receive(:update_state).and_return(true) - pipeline.touch - end + describe '#reload_status!' do + let(:pipeline) { create :ci_empty_pipeline, project: project } context 'dependent objects' do - let(:commit_status) { build :commit_status, pipeline: pipeline } + let(:commit_status) { create :commit_status, :pending, pipeline: pipeline } + + it 'executes reload_status! after succeeding dependent object' do + expect(pipeline).to receive(:reload_status!).and_return(true) - it 'executes update_state after saving dependent object' do - expect(pipeline).to receive(:update_state).and_return(true) - commit_status.save + commit_status.success end end - context 'update state' do + context 'updates' do let(:current) { Time.now.change(usec: 0) } - let(:build) { FactoryGirl.create :ci_build, :success, pipeline: pipeline, started_at: current - 120, finished_at: current - 60 } + let(:build) { FactoryGirl.create :ci_build, pipeline: pipeline, started_at: current - 120, finished_at: current - 60 } before do build + pipeline.reload_status! end [:status, :started_at, :finished_at, :duration].each do |param| - it "update #{param}" do + it "#{param}" do expect(pipeline.send(param)).to eq(build.send(param)) end end diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index 44cd3c08718..2277f4e13bf 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -10,7 +10,7 @@ describe Member, models: true do it { is_expected.to validate_presence_of(:user) } it { is_expected.to validate_presence_of(:source) } - it { is_expected.to validate_inclusion_of(:access_level).in_array(Gitlab::Access.values) } + it { is_expected.to validate_inclusion_of(:access_level).in_array(Gitlab::Access.all_values) } it_behaves_like 'an object with email-formated attributes', :invite_email do subject { build(:project_member) } diff --git a/spec/models/members/project_member_spec.rb b/spec/models/members/project_member_spec.rb index 28673de3189..913d74645a7 100644 --- a/spec/models/members/project_member_spec.rb +++ b/spec/models/members/project_member_spec.rb @@ -27,6 +27,7 @@ describe ProjectMember, models: true do describe 'validations' do it { is_expected.to allow_value('Project').for(:source_type) } it { is_expected.not_to allow_value('project').for(:source_type) } + it { is_expected.to validate_inclusion_of(:access_level).in_array(Gitlab::Access.values) } end describe 'modules' do @@ -40,7 +41,7 @@ describe ProjectMember, models: true do end describe "#destroy" do - let(:owner) { create(:project_member, access_level: ProjectMember::OWNER) } + let(:owner) { create(:project_member, access_level: ProjectMember::MASTER) } let(:project) { owner.project } let(:master) { create(:project_member, project: project) } diff --git a/spec/models/project_services/campfire_service_spec.rb b/spec/models/project_services/campfire_service_spec.rb index 3e6da42803b..1adf93258f3 100644 --- a/spec/models/project_services/campfire_service_spec.rb +++ b/spec/models/project_services/campfire_service_spec.rb @@ -39,4 +39,62 @@ describe CampfireService, models: true do it { is_expected.not_to validate_presence_of(:token) } end end + + describe "#execute" do + let(:user) { create(:user) } + let(:project) { create(:project) } + + before do + @campfire_service = CampfireService.new + allow(@campfire_service).to receive_messages( + project_id: project.id, + project: project, + service_hook: true, + token: 'verySecret', + subdomain: 'project-name', + room: 'test-room' + ) + @sample_data = Gitlab::PushDataBuilder.build_sample(project, user) + @rooms_url = 'https://verySecret:X@project-name.campfirenow.com/rooms.json' + @headers = { 'Content-Type' => 'application/json; charset=utf-8' } + end + + it "calls Campfire API to get a list of rooms and speak in a room" do + # make sure a valid list of rooms is returned + body = File.read(Rails.root + 'spec/fixtures/project_services/campfire/rooms.json') + WebMock.stub_request(:get, @rooms_url).to_return( + body: body, + status: 200, + headers: @headers + ) + # stub the speak request with the room id found in the previous request's response + speak_url = 'https://verySecret:X@project-name.campfirenow.com/room/123/speak.json' + WebMock.stub_request(:post, speak_url) + + @campfire_service.execute(@sample_data) + + expect(WebMock).to have_requested(:get, @rooms_url).once + expect(WebMock).to have_requested(:post, speak_url).with( + body: /#{project.path}.*#{@sample_data[:before]}.*#{@sample_data[:after]}/ + ).once + end + + it "calls Campfire API to get a list of rooms but shouldn't speak in a room" do + # return a list of rooms that do not contain a room named 'test-room' + body = File.read(Rails.root + 'spec/fixtures/project_services/campfire/rooms2.json') + WebMock.stub_request(:get, @rooms_url).to_return( + body: body, + status: 200, + headers: @headers + ) + # we want to make sure no request is sent to the /speak endpoint, here is a basic + # regexp that matches this endpoint + speak_url = 'https://verySecret:X@project-name.campfirenow.com/room/.*/speak.json' + + @campfire_service.execute(@sample_data) + + expect(WebMock).to have_requested(:get, @rooms_url).once + expect(WebMock).not_to have_requested(:post, /#{speak_url}/) + end + end end diff --git a/spec/models/project_services/hipchat_service_spec.rb b/spec/models/project_services/hipchat_service_spec.rb index 878afa13f1f..bb6c75d6050 100644 --- a/spec/models/project_services/hipchat_service_spec.rb +++ b/spec/models/project_services/hipchat_service_spec.rb @@ -301,7 +301,8 @@ describe HipchatService, models: true do end context 'build events' do - let(:build) { create(:ci_build) } + let(:pipeline) { create(:ci_empty_pipeline) } + let(:build) { create(:ci_build, pipeline: pipeline) } let(:data) { Gitlab::DataBuilder::BuildDataBuilder.build(build) } context 'for failed' do diff --git a/spec/requests/api/access_requests_spec.rb b/spec/requests/api/access_requests_spec.rb new file mode 100644 index 00000000000..d78494b76fa --- /dev/null +++ b/spec/requests/api/access_requests_spec.rb @@ -0,0 +1,246 @@ +require 'spec_helper' + +describe API::AccessRequests, api: true do + include ApiHelpers + + let(:master) { create(:user) } + let(:developer) { create(:user) } + let(:access_requester) { create(:user) } + let(:stranger) { create(:user) } + + let(:project) do + project = create(:project, :public, creator_id: master.id, namespace: master.namespace) + project.team << [developer, :developer] + project.team << [master, :master] + project.request_access(access_requester) + project + end + + let(:group) do + group = create(:group, :public) + group.add_developer(developer) + group.add_owner(master) + group.request_access(access_requester) + group + end + + shared_examples 'GET /:sources/:id/access_requests' do |source_type| + context "with :sources == #{source_type.pluralize}" do + it_behaves_like 'a 404 response when source is private' do + let(:route) { get api("/#{source_type.pluralize}/#{source.id}/access_requests", stranger) } + end + + context 'when authenticated as a non-master/owner' do + %i[developer access_requester stranger].each do |type| + context "as a #{type}" do + it 'returns 403' do + user = public_send(type) + get api("/#{source_type.pluralize}/#{source.id}/access_requests", user) + + expect(response).to have_http_status(403) + end + end + end + end + + context 'when authenticated as a master/owner' do + it 'returns access requesters' do + get api("/#{source_type.pluralize}/#{source.id}/access_requests", master) + + expect(response).to have_http_status(200) + expect(json_response).to be_an Array + expect(json_response.size).to eq(1) + end + end + end + end + + shared_examples 'POST /:sources/:id/access_requests' do |source_type| + context "with :sources == #{source_type.pluralize}" do + it_behaves_like 'a 404 response when source is private' do + let(:route) { post api("/#{source_type.pluralize}/#{source.id}/access_requests", stranger) } + end + + context 'when authenticated as a member' do + %i[developer master].each do |type| + context "as a #{type}" do + it 'returns 400' do + expect do + user = public_send(type) + post api("/#{source_type.pluralize}/#{source.id}/access_requests", user) + + expect(response).to have_http_status(400) + end.not_to change { source.requesters.count } + end + end + end + end + + context 'when authenticated as an access requester' do + it 'returns 400' do + expect do + post api("/#{source_type.pluralize}/#{source.id}/access_requests", access_requester) + + expect(response).to have_http_status(400) + end.not_to change { source.requesters.count } + end + end + + context 'when authenticated as a stranger' do + it 'returns 201' do + expect do + post api("/#{source_type.pluralize}/#{source.id}/access_requests", stranger) + + expect(response).to have_http_status(201) + end.to change { source.requesters.count }.by(1) + + # User attributes + expect(json_response['id']).to eq(stranger.id) + expect(json_response['name']).to eq(stranger.name) + expect(json_response['username']).to eq(stranger.username) + expect(json_response['state']).to eq(stranger.state) + expect(json_response['avatar_url']).to eq(stranger.avatar_url) + expect(json_response['web_url']).to eq(Gitlab::Routing.url_helpers.user_url(stranger)) + + # Member attributes + expect(json_response['requested_at']).to be_present + end + end + end + end + + shared_examples 'PUT /:sources/:id/access_requests/:user_id/approve' do |source_type| + context "with :sources == #{source_type.pluralize}" do + it_behaves_like 'a 404 response when source is private' do + let(:route) { put api("/#{source_type.pluralize}/#{source.id}/access_requests/#{access_requester.id}/approve", stranger) } + end + + context 'when authenticated as a non-master/owner' do + %i[developer access_requester stranger].each do |type| + context "as a #{type}" do + it 'returns 403' do + user = public_send(type) + put api("/#{source_type.pluralize}/#{source.id}/access_requests/#{access_requester.id}/approve", user) + + expect(response).to have_http_status(403) + end + end + end + end + + context 'when authenticated as a master/owner' do + it 'returns 201' do + expect do + put api("/#{source_type.pluralize}/#{source.id}/access_requests/#{access_requester.id}/approve", master), + access_level: Member::MASTER + + expect(response).to have_http_status(201) + end.to change { source.members.count }.by(1) + # User attributes + expect(json_response['id']).to eq(access_requester.id) + expect(json_response['name']).to eq(access_requester.name) + expect(json_response['username']).to eq(access_requester.username) + expect(json_response['state']).to eq(access_requester.state) + expect(json_response['avatar_url']).to eq(access_requester.avatar_url) + expect(json_response['web_url']).to eq(Gitlab::Routing.url_helpers.user_url(access_requester)) + + # Member attributes + expect(json_response['access_level']).to eq(Member::MASTER) + end + + context 'user_id does not match an existing access requester' do + it 'returns 404' do + expect do + put api("/#{source_type.pluralize}/#{source.id}/access_requests/#{stranger.id}/approve", master) + + expect(response).to have_http_status(404) + end.not_to change { source.members.count } + end + end + end + end + end + + shared_examples 'DELETE /:sources/:id/access_requests/:user_id' do |source_type| + context "with :sources == #{source_type.pluralize}" do + it_behaves_like 'a 404 response when source is private' do + let(:route) { delete api("/#{source_type.pluralize}/#{source.id}/access_requests/#{access_requester.id}", stranger) } + end + + context 'when authenticated as a non-master/owner' do + %i[developer stranger].each do |type| + context "as a #{type}" do + it 'returns 403' do + user = public_send(type) + delete api("/#{source_type.pluralize}/#{source.id}/access_requests/#{access_requester.id}", user) + + expect(response).to have_http_status(403) + end + end + end + end + + context 'when authenticated as the access requester' do + it 'returns 200' do + expect do + delete api("/#{source_type.pluralize}/#{source.id}/access_requests/#{access_requester.id}", access_requester) + + expect(response).to have_http_status(200) + end.to change { source.requesters.count }.by(-1) + end + end + + context 'when authenticated as a master/owner' do + it 'returns 200' do + expect do + delete api("/#{source_type.pluralize}/#{source.id}/access_requests/#{access_requester.id}", master) + + expect(response).to have_http_status(200) + end.to change { source.requesters.count }.by(-1) + end + + context 'user_id does not match an existing access requester' do + it 'returns 404' do + expect do + delete api("/#{source_type.pluralize}/#{source.id}/access_requests/#{stranger.id}", master) + + expect(response).to have_http_status(404) + end.not_to change { source.requesters.count } + end + end + end + end + end + + it_behaves_like 'GET /:sources/:id/access_requests', 'project' do + let(:source) { project } + end + + it_behaves_like 'GET /:sources/:id/access_requests', 'group' do + let(:source) { group } + end + + it_behaves_like 'POST /:sources/:id/access_requests', 'project' do + let(:source) { project } + end + + it_behaves_like 'POST /:sources/:id/access_requests', 'group' do + let(:source) { group } + end + + it_behaves_like 'PUT /:sources/:id/access_requests/:user_id/approve', 'project' do + let(:source) { project } + end + + it_behaves_like 'PUT /:sources/:id/access_requests/:user_id/approve', 'group' do + let(:source) { group } + end + + it_behaves_like 'DELETE /:sources/:id/access_requests/:user_id', 'project' do + let(:source) { project } + end + + it_behaves_like 'DELETE /:sources/:id/access_requests/:user_id', 'group' do + let(:source) { group } + end +end diff --git a/spec/requests/api/builds_spec.rb b/spec/requests/api/builds_spec.rb index 966d302dfd3..a4cdd8f3140 100644 --- a/spec/requests/api/builds_spec.rb +++ b/spec/requests/api/builds_spec.rb @@ -238,6 +238,10 @@ describe API::API, api: true do it { expect(response.headers).to include(download_headers) } end + before do + pipeline.reload_status! + end + context 'with regular branch' do before do pipeline.update(ref: 'master', diff --git a/spec/requests/api/commits_spec.rb b/spec/requests/api/commits_spec.rb index 4379fcb3c1e..7ca75d77673 100644 --- a/spec/requests/api/commits_spec.rb +++ b/spec/requests/api/commits_spec.rb @@ -89,16 +89,29 @@ describe API::API, api: true do it "returns nil for commit without CI" do get api("/projects/#{project.id}/repository/commits/#{project.repository.commit.id}", user) + expect(response).to have_http_status(200) expect(json_response['status']).to be_nil end it "returns status for CI" do pipeline = project.ensure_pipeline(project.repository.commit.sha, 'master') + pipeline.update(status: 'success') + get api("/projects/#{project.id}/repository/commits/#{project.repository.commit.id}", user) + expect(response).to have_http_status(200) expect(json_response['status']).to eq(pipeline.status) end + + it "returns status for CI when pipeline is created" do + project.ensure_pipeline(project.repository.commit.sha, 'master') + + get api("/projects/#{project.id}/repository/commits/#{project.repository.commit.id}", user) + + expect(response).to have_http_status(200) + expect(json_response['status']).to be_nil + end end context "unauthorized user" do diff --git a/spec/requests/api/group_members_spec.rb b/spec/requests/api/group_members_spec.rb deleted file mode 100644 index 8bd6a8062ae..00000000000 --- a/spec/requests/api/group_members_spec.rb +++ /dev/null @@ -1,199 +0,0 @@ -require 'spec_helper' - -describe API::API, api: true do - include ApiHelpers - - let(:owner) { create(:user) } - let(:reporter) { create(:user) } - let(:developer) { create(:user) } - let(:master) { create(:user) } - let(:guest) { create(:user) } - let(:stranger) { create(:user) } - - let!(:group_with_members) do - group = create(:group, :private) - group.add_users([reporter.id], GroupMember::REPORTER) - group.add_users([developer.id], GroupMember::DEVELOPER) - group.add_users([master.id], GroupMember::MASTER) - group.add_users([guest.id], GroupMember::GUEST) - group - end - - let!(:group_no_members) { create(:group) } - - before do - group_with_members.add_owner owner - group_no_members.add_owner owner - end - - describe "GET /groups/:id/members" do - context "when authenticated as user that is part or the group" do - it "each user: returns an array of members groups of group3" do - [owner, master, developer, reporter, guest].each do |user| - get api("/groups/#{group_with_members.id}/members", user) - expect(response).to have_http_status(200) - expect(json_response).to be_an Array - expect(json_response.size).to eq(5) - expect(json_response.find { |e| e['id'] == owner.id }['access_level']).to eq(GroupMember::OWNER) - expect(json_response.find { |e| e['id'] == reporter.id }['access_level']).to eq(GroupMember::REPORTER) - expect(json_response.find { |e| e['id'] == developer.id }['access_level']).to eq(GroupMember::DEVELOPER) - expect(json_response.find { |e| e['id'] == master.id }['access_level']).to eq(GroupMember::MASTER) - expect(json_response.find { |e| e['id'] == guest.id }['access_level']).to eq(GroupMember::GUEST) - end - end - - it 'users not part of the group should get access error' do - get api("/groups/#{group_with_members.id}/members", stranger) - - expect(response).to have_http_status(404) - end - end - end - - describe "POST /groups/:id/members" do - context "when not a member of the group" do - it "does not add guest as member of group_no_members when adding being done by person outside the group" do - post api("/groups/#{group_no_members.id}/members", reporter), user_id: guest.id, access_level: GroupMember::MASTER - expect(response).to have_http_status(403) - end - end - - context "when a member of the group" do - it "returns ok and add new member" do - new_user = create(:user) - - expect do - post api("/groups/#{group_no_members.id}/members", owner), user_id: new_user.id, access_level: GroupMember::MASTER - end.to change { group_no_members.members.count }.by(1) - - expect(response).to have_http_status(201) - expect(json_response['name']).to eq(new_user.name) - expect(json_response['access_level']).to eq(GroupMember::MASTER) - end - - it "does not allow guest to modify group members" do - new_user = create(:user) - - expect do - post api("/groups/#{group_with_members.id}/members", guest), user_id: new_user.id, access_level: GroupMember::MASTER - end.not_to change { group_with_members.members.count } - - expect(response).to have_http_status(403) - end - - it "returns error if member already exists" do - post api("/groups/#{group_with_members.id}/members", owner), user_id: master.id, access_level: GroupMember::MASTER - expect(response).to have_http_status(409) - end - - it "returns a 400 error when user id is not given" do - post api("/groups/#{group_no_members.id}/members", owner), access_level: GroupMember::MASTER - expect(response).to have_http_status(400) - end - - it "returns a 400 error when access level is not given" do - post api("/groups/#{group_no_members.id}/members", owner), user_id: master.id - expect(response).to have_http_status(400) - end - - it "returns a 422 error when access level is not known" do - post api("/groups/#{group_no_members.id}/members", owner), user_id: master.id, access_level: 1234 - expect(response).to have_http_status(422) - end - end - end - - describe 'PUT /groups/:id/members/:user_id' do - context 'when not a member of the group' do - it 'returns a 409 error if the user is not a group member' do - put( - api("/groups/#{group_no_members.id}/members/#{developer.id}", - owner), access_level: GroupMember::MASTER - ) - expect(response).to have_http_status(404) - end - end - - context 'when a member of the group' do - it 'returns ok and update member access level' do - put( - api("/groups/#{group_with_members.id}/members/#{reporter.id}", - owner), - access_level: GroupMember::MASTER - ) - - expect(response).to have_http_status(200) - - get api("/groups/#{group_with_members.id}/members", owner) - json_reporter = json_response.find do |e| - e['id'] == reporter.id - end - - expect(json_reporter['access_level']).to eq(GroupMember::MASTER) - end - - it 'does not allow guest to modify group members' do - put( - api("/groups/#{group_with_members.id}/members/#{developer.id}", - guest), - access_level: GroupMember::MASTER - ) - - expect(response).to have_http_status(403) - - get api("/groups/#{group_with_members.id}/members", owner) - json_developer = json_response.find do |e| - e['id'] == developer.id - end - - expect(json_developer['access_level']).to eq(GroupMember::DEVELOPER) - end - - it 'returns a 400 error when access level is not given' do - put( - api("/groups/#{group_with_members.id}/members/#{master.id}", owner) - ) - expect(response).to have_http_status(400) - end - - it 'returns a 422 error when access level is not known' do - put( - api("/groups/#{group_with_members.id}/members/#{master.id}", owner), - access_level: 1234 - ) - expect(response).to have_http_status(422) - end - end - end - - describe 'DELETE /groups/:id/members/:user_id' do - context 'when not a member of the group' do - it "does not delete guest's membership of group_with_members" do - random_user = create(:user) - delete api("/groups/#{group_with_members.id}/members/#{owner.id}", random_user) - - expect(response).to have_http_status(404) - end - end - - context "when a member of the group" do - it "deletes guest's membership of group" do - expect do - delete api("/groups/#{group_with_members.id}/members/#{guest.id}", owner) - end.to change { group_with_members.members.count }.by(-1) - - expect(response).to have_http_status(200) - end - - it "returns a 404 error when user id is not known" do - delete api("/groups/#{group_with_members.id}/members/1328", owner) - expect(response).to have_http_status(404) - end - - it "does not allow guest to modify group members" do - delete api("/groups/#{group_with_members.id}/members/#{master.id}", guest) - expect(response).to have_http_status(403) - end - end - end -end diff --git a/spec/requests/api/members_spec.rb b/spec/requests/api/members_spec.rb new file mode 100644 index 00000000000..a56ee30f7b1 --- /dev/null +++ b/spec/requests/api/members_spec.rb @@ -0,0 +1,312 @@ +require 'spec_helper' + +describe API::Members, api: true do + include ApiHelpers + + let(:master) { create(:user) } + let(:developer) { create(:user) } + let(:access_requester) { create(:user) } + let(:stranger) { create(:user) } + + let(:project) do + project = create(:project, :public, creator_id: master.id, namespace: master.namespace) + project.team << [developer, :developer] + project.team << [master, :master] + project.request_access(access_requester) + project + end + + let!(:group) do + group = create(:group, :public) + group.add_developer(developer) + group.add_owner(master) + group.request_access(access_requester) + group + end + + shared_examples 'GET /:sources/:id/members' do |source_type| + context "with :sources == #{source_type.pluralize}" do + it_behaves_like 'a 404 response when source is private' do + let(:route) { get api("/#{source_type.pluralize}/#{source.id}/members", stranger) } + end + + context 'when authenticated as a non-member' do + %i[access_requester stranger].each do |type| + context "as a #{type}" do + it 'returns 200' do + user = public_send(type) + get api("/#{source_type.pluralize}/#{source.id}/members", user) + + expect(response).to have_http_status(200) + expect(json_response.size).to eq(2) + end + end + end + end + + it 'finds members with query string' do + get api("/#{source_type.pluralize}/#{source.id}/members", developer), query: master.username + + expect(response).to have_http_status(200) + expect(json_response.count).to eq(1) + expect(json_response.first['username']).to eq(master.username) + end + end + end + + shared_examples 'GET /:sources/:id/members/:user_id' do |source_type| + context "with :sources == #{source_type.pluralize}" do + it_behaves_like 'a 404 response when source is private' do + let(:route) { get api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", stranger) } + end + + context 'when authenticated as a non-member' do + %i[access_requester stranger].each do |type| + context "as a #{type}" do + it 'returns 200' do + user = public_send(type) + get api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", user) + + expect(response).to have_http_status(200) + # User attributes + expect(json_response['id']).to eq(developer.id) + expect(json_response['name']).to eq(developer.name) + expect(json_response['username']).to eq(developer.username) + expect(json_response['state']).to eq(developer.state) + expect(json_response['avatar_url']).to eq(developer.avatar_url) + expect(json_response['web_url']).to eq(Gitlab::Routing.url_helpers.user_url(developer)) + + # Member attributes + expect(json_response['access_level']).to eq(Member::DEVELOPER) + end + end + end + end + end + end + + shared_examples 'POST /:sources/:id/members' do |source_type| + context "with :sources == #{source_type.pluralize}" do + it_behaves_like 'a 404 response when source is private' do + let(:route) { post api("/#{source_type.pluralize}/#{source.id}/members", stranger) } + end + + context 'when authenticated as a non-member or member with insufficient rights' do + %i[access_requester stranger developer].each do |type| + context "as a #{type}" do + it 'returns 403' do + user = public_send(type) + post api("/#{source_type.pluralize}/#{source.id}/members", user) + + expect(response).to have_http_status(403) + end + end + end + end + + context 'when authenticated as a master/owner' do + context 'and new member is already a requester' do + it 'transforms the requester into a proper member' do + expect do + post api("/#{source_type.pluralize}/#{source.id}/members", master), + user_id: access_requester.id, access_level: Member::MASTER + + expect(response).to have_http_status(201) + end.to change { source.members.count }.by(1) + expect(source.requesters.count).to eq(0) + expect(json_response['id']).to eq(access_requester.id) + expect(json_response['access_level']).to eq(Member::MASTER) + end + end + + it 'creates a new member' do + expect do + post api("/#{source_type.pluralize}/#{source.id}/members", master), + user_id: stranger.id, access_level: Member::DEVELOPER + + expect(response).to have_http_status(201) + end.to change { source.members.count }.by(1) + expect(json_response['id']).to eq(stranger.id) + expect(json_response['access_level']).to eq(Member::DEVELOPER) + end + end + + it "returns #{source_type == 'project' ? 201 : 409} if member already exists" do + post api("/#{source_type.pluralize}/#{source.id}/members", master), + user_id: master.id, access_level: Member::MASTER + + expect(response).to have_http_status(source_type == 'project' ? 201 : 409) + end + + it 'returns 400 when user_id is not given' do + post api("/#{source_type.pluralize}/#{source.id}/members", master), + access_level: Member::MASTER + + expect(response).to have_http_status(400) + end + + it 'returns 400 when access_level is not given' do + post api("/#{source_type.pluralize}/#{source.id}/members", master), + user_id: stranger.id + + expect(response).to have_http_status(400) + end + + it 'returns 422 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) + end + end + end + + shared_examples 'PUT /:sources/:id/members/:user_id' do |source_type| + context "with :sources == #{source_type.pluralize}" do + it_behaves_like 'a 404 response when source is private' do + let(:route) { put api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", stranger) } + end + + context 'when authenticated as a non-member or member with insufficient rights' do + %i[access_requester stranger developer].each do |type| + context "as a #{type}" do + it 'returns 403' do + user = public_send(type) + put api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", user) + + expect(response).to have_http_status(403) + end + end + end + end + + context 'when authenticated as a master/owner' do + it 'updates the member' do + put api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", master), + access_level: Member::MASTER + + expect(response).to have_http_status(200) + expect(json_response['id']).to eq(developer.id) + expect(json_response['access_level']).to eq(Member::MASTER) + end + end + + it 'returns 409 if member does not exist' do + put api("/#{source_type.pluralize}/#{source.id}/members/123", master), + access_level: Member::MASTER + + expect(response).to have_http_status(404) + end + + it 'returns 400 when access_level is not given' do + put api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", master) + + expect(response).to have_http_status(400) + end + + it 'returns 422 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) + end + end + end + + shared_examples 'DELETE /:sources/:id/members/:user_id' do |source_type| + context "with :sources == #{source_type.pluralize}" do + it_behaves_like 'a 404 response when source is private' do + let(:route) { delete api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", stranger) } + end + + context 'when authenticated as a non-member or member with insufficient rights' do + %i[access_requester stranger].each do |type| + context "as a #{type}" do + it 'returns 403' do + user = public_send(type) + delete api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", user) + + expect(response).to have_http_status(403) + end + end + end + end + + context 'when authenticated as a member and deleting themself' do + it 'deletes the member' do + expect do + delete api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", developer) + + expect(response).to have_http_status(200) + end.to change { source.members.count }.by(-1) + end + end + + context 'when authenticated as a master/owner' do + context 'and member is a requester' do + it "returns #{source_type == 'project' ? 200 : 404}" do + expect do + delete api("/#{source_type.pluralize}/#{source.id}/members/#{access_requester.id}", master) + + expect(response).to have_http_status(source_type == 'project' ? 200 : 404) + end.not_to change { source.requesters.count } + end + end + + it 'deletes the member' do + expect do + delete api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", master) + + expect(response).to have_http_status(200) + end.to change { source.members.count }.by(-1) + end + end + + it "returns #{source_type == 'project' ? 200 : 404} if member does not exist" do + delete api("/#{source_type.pluralize}/#{source.id}/members/123", master) + + expect(response).to have_http_status(source_type == 'project' ? 200 : 404) + end + end + end + + it_behaves_like 'GET /:sources/:id/members', 'project' do + let(:source) { project } + end + + it_behaves_like 'GET /:sources/:id/members', 'group' do + let(:source) { group } + end + + it_behaves_like 'GET /:sources/:id/members/:user_id', 'project' do + let(:source) { project } + end + + it_behaves_like 'GET /:sources/:id/members/:user_id', 'group' do + let(:source) { group } + end + + it_behaves_like 'POST /:sources/:id/members', 'project' do + let(:source) { project } + end + + it_behaves_like 'POST /:sources/:id/members', 'group' do + let(:source) { group } + end + + it_behaves_like 'PUT /:sources/:id/members/:user_id', 'project' do + let(:source) { project } + end + + it_behaves_like 'PUT /:sources/:id/members/:user_id', 'group' do + let(:source) { group } + end + + it_behaves_like 'DELETE /:sources/:id/members/:user_id', 'project' do + let(:source) { project } + end + + it_behaves_like 'DELETE /:sources/:id/members/:user_id', 'group' do + let(:source) { group } + end +end diff --git a/spec/requests/api/project_members_spec.rb b/spec/requests/api/project_members_spec.rb deleted file mode 100644 index 13cc0d81ac8..00000000000 --- a/spec/requests/api/project_members_spec.rb +++ /dev/null @@ -1,166 +0,0 @@ -require 'spec_helper' - -describe API::API, api: true do - include ApiHelpers - let(:user) { create(:user) } - let(:user2) { create(:user) } - let(:user3) { create(:user) } - let(:project) { create(:project, creator_id: user.id, namespace: user.namespace) } - let(:project_member) { create(:project_member, :master, user: user, project: project) } - let(:project_member2) { create(:project_member, :developer, user: user3, project: project) } - - describe "GET /projects/:id/members" do - before { project_member } - before { project_member2 } - - it "returns project team members" do - get api("/projects/#{project.id}/members", user) - expect(response).to have_http_status(200) - expect(json_response).to be_an Array - expect(json_response.count).to eq(2) - expect(json_response.map { |u| u['username'] }).to include user.username - end - - it "finds team members with query string" do - get api("/projects/#{project.id}/members", user), query: user.username - expect(response).to have_http_status(200) - expect(json_response).to be_an Array - expect(json_response.count).to eq(1) - expect(json_response.first['username']).to eq(user.username) - end - - it "returns a 404 error if id not found" do - get api("/projects/9999/members", user) - expect(response).to have_http_status(404) - end - end - - describe "GET /projects/:id/members/:user_id" do - before { project_member } - - it "returns project team member" do - get api("/projects/#{project.id}/members/#{user.id}", user) - expect(response).to have_http_status(200) - expect(json_response['username']).to eq(user.username) - expect(json_response['access_level']).to eq(ProjectMember::MASTER) - end - - it "returns a 404 error if user id not found" do - get api("/projects/#{project.id}/members/1234", user) - expect(response).to have_http_status(404) - end - end - - describe "POST /projects/:id/members" do - it "adds user to project team" do - expect do - post api("/projects/#{project.id}/members", user), user_id: user2.id, access_level: ProjectMember::DEVELOPER - end.to change { ProjectMember.count }.by(1) - - expect(response).to have_http_status(201) - expect(json_response['username']).to eq(user2.username) - expect(json_response['access_level']).to eq(ProjectMember::DEVELOPER) - end - - it "returns a 201 status if user is already project member" do - post api("/projects/#{project.id}/members", user), - user_id: user2.id, - access_level: ProjectMember::DEVELOPER - expect do - post api("/projects/#{project.id}/members", user), user_id: user2.id, access_level: ProjectMember::DEVELOPER - end.not_to change { ProjectMember.count } - - expect(response).to have_http_status(201) - expect(json_response['username']).to eq(user2.username) - expect(json_response['access_level']).to eq(ProjectMember::DEVELOPER) - end - - it "returns a 400 error when user id is not given" do - post api("/projects/#{project.id}/members", user), access_level: ProjectMember::MASTER - expect(response).to have_http_status(400) - end - - it "returns a 400 error when access level is not given" do - post api("/projects/#{project.id}/members", user), user_id: user2.id - expect(response).to have_http_status(400) - end - - it "returns a 422 error when access level is not known" do - post api("/projects/#{project.id}/members", user), user_id: user2.id, access_level: 1234 - expect(response).to have_http_status(422) - end - end - - describe "PUT /projects/:id/members/:user_id" do - before { project_member2 } - - it "updates project team member" do - put api("/projects/#{project.id}/members/#{user3.id}", user), access_level: ProjectMember::MASTER - expect(response).to have_http_status(200) - expect(json_response['username']).to eq(user3.username) - expect(json_response['access_level']).to eq(ProjectMember::MASTER) - end - - it "returns a 404 error if user_id is not found" do - put api("/projects/#{project.id}/members/1234", user), access_level: ProjectMember::MASTER - expect(response).to have_http_status(404) - end - - it "returns a 400 error when access level is not given" do - put api("/projects/#{project.id}/members/#{user3.id}", user) - expect(response).to have_http_status(400) - end - - it "returns a 422 error when access level is not known" do - put api("/projects/#{project.id}/members/#{user3.id}", user), access_level: 123 - expect(response).to have_http_status(422) - end - end - - describe "DELETE /projects/:id/members/:user_id" do - before do - project_member - project_member2 - end - - it "removes user from project team" do - expect do - delete api("/projects/#{project.id}/members/#{user3.id}", user) - end.to change { ProjectMember.count }.by(-1) - end - - it "returns 200 if team member is not part of a project" do - delete api("/projects/#{project.id}/members/#{user3.id}", user) - expect do - delete api("/projects/#{project.id}/members/#{user3.id}", user) - end.not_to change { ProjectMember.count } - expect(response).to have_http_status(200) - end - - it "returns 200 if team member already removed" do - delete api("/projects/#{project.id}/members/#{user3.id}", user) - delete api("/projects/#{project.id}/members/#{user3.id}", user) - expect(response).to have_http_status(200) - end - - it "returns 200 OK when the user was not member" do - expect do - delete api("/projects/#{project.id}/members/1000000", user) - end.to change { ProjectMember.count }.by(0) - expect(response).to have_http_status(200) - expect(json_response['id']).to eq(1000000) - expect(json_response['message']).to eq('Access revoked') - end - - context 'when the user is not an admin or owner' do - it 'can leave the project' do - expect do - delete api("/projects/#{project.id}/members/#{user3.id}", user3) - end.to change { ProjectMember.count }.by(-1) - - expect(response).to have_http_status(200) - expect(json_response['id']).to eq(project_member2.id) - end - end - end -end diff --git a/spec/requests/api/triggers_spec.rb b/spec/requests/api/triggers_spec.rb index 5702682fc7d..82bba1ce8a4 100644 --- a/spec/requests/api/triggers_spec.rb +++ b/spec/requests/api/triggers_spec.rb @@ -50,7 +50,8 @@ describe API::API do post api("/projects/#{project.id}/trigger/builds"), options.merge(ref: 'master') expect(response).to have_http_status(201) pipeline.builds.reload - expect(pipeline.builds.size).to eq(2) + expect(pipeline.builds.pending.size).to eq(2) + expect(pipeline.builds.size).to eq(5) end it 'returns bad request with no builds created if there\'s no commit for that ref' do diff --git a/spec/requests/ci/api/builds_spec.rb b/spec/requests/ci/api/builds_spec.rb index 05b309096cb..ca7932dc5da 100644 --- a/spec/requests/ci/api/builds_spec.rb +++ b/spec/requests/ci/api/builds_spec.rb @@ -6,112 +6,102 @@ describe Ci::API::API do let(:runner) { FactoryGirl.create(:ci_runner, tag_list: ["mysql", "ruby"]) } let(:project) { FactoryGirl.create(:empty_project) } - before do - stub_ci_pipeline_to_return_yaml_file - end - describe "Builds API for runners" do - let(:shared_runner) { FactoryGirl.create(:ci_runner, token: "SharedRunner") } - let(:shared_project) { FactoryGirl.create(:empty_project, name: "SharedProject") } + let(:pipeline) { create(:ci_pipeline_without_jobs, project: project, ref: 'master') } before do - FactoryGirl.create :ci_runner_project, project: project, runner: runner + project.runners << runner end describe "POST /builds/register" do - it "starts a build" do - pipeline = FactoryGirl.create(:ci_pipeline, project: project, ref: 'master') - pipeline.create_builds(nil) - build = pipeline.builds.first + let!(:build) { create(:ci_build, pipeline: pipeline, name: 'spinach', stage: 'test', stage_idx: 0) } - post ci_api("/builds/register"), token: runner.token, info: { platform: :darwin } + it "starts a build" do + register_builds info: { platform: :darwin } expect(response).to have_http_status(201) expect(json_response['sha']).to eq(build.sha) expect(runner.reload.platform).to eq("darwin") + expect(json_response["options"]).to eq({ "image" => "ruby:2.1", "services" => ["postgres"] }) + expect(json_response["variables"]).to include( + { "key" => "CI_BUILD_NAME", "value" => "spinach", "public" => true }, + { "key" => "CI_BUILD_STAGE", "value" => "test", "public" => true }, + { "key" => "DB_NAME", "value" => "postgres", "public" => true } + ) end - it "returns 404 error if no pending build found" do - post ci_api("/builds/register"), token: runner.token - - expect(response).to have_http_status(404) - end - - it "returns 404 error if no builds for specific runner" do - pipeline = FactoryGirl.create(:ci_pipeline, project: shared_project) - FactoryGirl.create(:ci_build, pipeline: pipeline, status: 'pending') + context 'when builds are finished' do + before do + build.success + end - post ci_api("/builds/register"), token: runner.token + it "returns 404 error if no builds for specific runner" do + register_builds - expect(response).to have_http_status(404) + expect(response).to have_http_status(404) + end end - it "returns 404 error if no builds for shared runner" do - pipeline = FactoryGirl.create(:ci_pipeline, project: project) - FactoryGirl.create(:ci_build, pipeline: pipeline, status: 'pending') + context 'for other project with builds' do + before do + build.success + create(:ci_build, :pending) + end - post ci_api("/builds/register"), token: shared_runner.token + it "returns 404 error if no builds for shared runner" do + register_builds - expect(response).to have_http_status(404) + expect(response).to have_http_status(404) + end end - it "returns options" do - pipeline = FactoryGirl.create(:ci_pipeline, project: project, ref: 'master') - pipeline.create_builds(nil) + context 'for shared runner' do + let(:shared_runner) { create(:ci_runner, token: "SharedRunner") } - post ci_api("/builds/register"), token: runner.token, info: { platform: :darwin } + it "should return 404 error if no builds for shared runner" do + register_builds shared_runner.token - expect(response).to have_http_status(201) - expect(json_response["options"]).to eq({ "image" => "ruby:2.1", "services" => ["postgres"] }) + expect(response).to have_http_status(404) + end end - it "returns variables" do - pipeline = FactoryGirl.create(:ci_pipeline, project: project, ref: 'master') - pipeline.create_builds(nil) - project.variables << Ci::Variable.new(key: "SECRET_KEY", value: "secret_value") - - post ci_api("/builds/register"), token: runner.token, info: { platform: :darwin } + context 'for triggered build' do + before do + trigger = create(:ci_trigger, project: project) + create(:ci_trigger_request_with_variables, pipeline: pipeline, builds: [build], trigger: trigger) + project.variables << Ci::Variable.new(key: "SECRET_KEY", value: "secret_value") + end - expect(response).to have_http_status(201) - expect(json_response["variables"]).to include( - { "key" => "CI_BUILD_NAME", "value" => "spinach", "public" => true }, - { "key" => "CI_BUILD_STAGE", "value" => "test", "public" => true }, - { "key" => "DB_NAME", "value" => "postgres", "public" => true }, - { "key" => "SECRET_KEY", "value" => "secret_value", "public" => false } - ) + it "returns variables for triggers" do + register_builds info: { platform: :darwin } + + expect(response).to have_http_status(201) + expect(json_response["variables"]).to include( + { "key" => "CI_BUILD_NAME", "value" => "spinach", "public" => true }, + { "key" => "CI_BUILD_STAGE", "value" => "test", "public" => true }, + { "key" => "CI_BUILD_TRIGGERED", "value" => "true", "public" => true }, + { "key" => "DB_NAME", "value" => "postgres", "public" => true }, + { "key" => "SECRET_KEY", "value" => "secret_value", "public" => false }, + { "key" => "TRIGGER_KEY_1", "value" => "TRIGGER_VALUE_1", "public" => false }, + ) + end end - it "returns variables for triggers" do - trigger = FactoryGirl.create(:ci_trigger, project: project) - pipeline = FactoryGirl.create(:ci_pipeline, project: project, ref: 'master') - - trigger_request = FactoryGirl.create(:ci_trigger_request_with_variables, pipeline: pipeline, trigger: trigger) - pipeline.create_builds(nil, trigger_request) - project.variables << Ci::Variable.new(key: "SECRET_KEY", value: "secret_value") - - post ci_api("/builds/register"), token: runner.token, info: { platform: :darwin } - - expect(response).to have_http_status(201) - expect(json_response["variables"]).to include( - { "key" => "CI_BUILD_NAME", "value" => "spinach", "public" => true }, - { "key" => "CI_BUILD_STAGE", "value" => "test", "public" => true }, - { "key" => "CI_BUILD_TRIGGERED", "value" => "true", "public" => true }, - { "key" => "DB_NAME", "value" => "postgres", "public" => true }, - { "key" => "SECRET_KEY", "value" => "secret_value", "public" => false }, - { "key" => "TRIGGER_KEY_1", "value" => "TRIGGER_VALUE_1", "public" => false } - ) - end + context 'with multiple builds' do + before do + build.success + end - it "returns dependent builds" do - pipeline = FactoryGirl.create(:ci_pipeline, project: project, ref: 'master') - pipeline.create_builds(nil, nil) - pipeline.builds.where(stage: 'test').each(&:success) + let!(:test_build) { create(:ci_build, pipeline: pipeline, name: 'deploy', stage: 'deploy', stage_idx: 1) } - post ci_api("/builds/register"), token: runner.token, info: { platform: :darwin } + it "returns dependent builds" do + register_builds info: { platform: :darwin } - expect(response).to have_http_status(201) - expect(json_response["depends_on_builds"].count).to eq(2) - expect(json_response["depends_on_builds"][0]["name"]).to eq("rspec") + expect(response).to have_http_status(201) + expect(json_response["id"]).to eq(test_build.id) + expect(json_response["depends_on_builds"].count).to eq(1) + expect(json_response["depends_on_builds"][0]).to include('id' => build.id, 'name' => 'spinach') + end end %w(name version revision platform architecture).each do |param| @@ -121,8 +111,9 @@ describe Ci::API::API do subject { runner.read_attribute(param.to_sym) } it do - post ci_api("/builds/register"), token: runner.token, info: { param => value } - expect(response).to have_http_status(404) + register_builds info: { param => value } + + expect(response).to have_http_status(201) runner.reload is_expected.to eq(value) end @@ -131,8 +122,7 @@ describe Ci::API::API do context 'when build has no tags' do before do - pipeline = create(:ci_pipeline, project: project) - create(:ci_build, pipeline: pipeline, tags: []) + build.update(tags: []) end context 'when runner is allowed to pick untagged builds' do @@ -154,17 +144,15 @@ describe Ci::API::API do expect(response).to have_http_status 404 end end + end - def register_builds - post ci_api("/builds/register"), token: runner.token, - info: { platform: :darwin } - end + def register_builds(token = runner.token, **params) + post ci_api("/builds/register"), params.merge(token: token) end end describe "PUT /builds/:id" do - let(:pipeline) {create(:ci_pipeline, project: project)} - let(:build) { create(:ci_build, :trace, pipeline: pipeline, runner_id: runner.id) } + let(:build) { create(:ci_build, :pending, :trace, pipeline: pipeline, runner_id: runner.id) } before do build.run! @@ -189,7 +177,7 @@ describe Ci::API::API do end describe 'PATCH /builds/:id/trace.txt' do - let(:build) { create(:ci_build, :trace, runner_id: runner.id) } + let(:build) { create(:ci_build, :pending, :trace, runner_id: runner.id) } let(:headers) { { Ci::API::Helpers::BUILD_TOKEN_HEADER => build.token, 'Content-Type' => 'text/plain' } } let(:headers_with_range) { headers.merge({ 'Content-Range' => '11-20' }) } @@ -237,8 +225,7 @@ describe Ci::API::API do context "Artifacts" do let(:file_upload) { fixture_file_upload(Rails.root + 'spec/fixtures/banana_sample.gif', 'image/gif') } let(:file_upload2) { fixture_file_upload(Rails.root + 'spec/fixtures/dk.png', 'image/gif') } - let(:pipeline) { create(:ci_pipeline, project: project) } - let(:build) { create(:ci_build, pipeline: pipeline, runner_id: runner.id) } + let(:build) { create(:ci_build, :pending, pipeline: pipeline, runner_id: runner.id) } let(:authorize_url) { ci_api("/builds/#{build.id}/artifacts/authorize") } let(:post_url) { ci_api("/builds/#{build.id}/artifacts") } let(:delete_url) { ci_api("/builds/#{build.id}/artifacts") } diff --git a/spec/requests/ci/api/triggers_spec.rb b/spec/requests/ci/api/triggers_spec.rb index 3312bd11669..0a0f979f57d 100644 --- a/spec/requests/ci/api/triggers_spec.rb +++ b/spec/requests/ci/api/triggers_spec.rb @@ -42,7 +42,8 @@ describe Ci::API::API do post ci_api("/projects/#{project.ci_id}/refs/master/trigger"), options expect(response).to have_http_status(201) pipeline.builds.reload - expect(pipeline.builds.size).to eq(2) + expect(pipeline.builds.pending.size).to eq(2) + expect(pipeline.builds.size).to eq(5) end it 'returns bad request with no builds created if there\'s no commit for that ref' do diff --git a/spec/requests/lfs_http_spec.rb b/spec/requests/lfs_http_spec.rb index 93d2bc160cc..4c9b4a8ba42 100644 --- a/spec/requests/lfs_http_spec.rb +++ b/spec/requests/lfs_http_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Gitlab::Lfs::Router do +describe 'Git LFS API and storage' do let(:user) { create(:user) } let!(:lfs_object) { create(:lfs_object, :with_file) } @@ -31,10 +31,11 @@ describe Gitlab::Lfs::Router do 'operation' => 'upload' } end + let(:authorization) { authorize_user } before do allow(Gitlab.config.lfs).to receive(:enabled).and_return(false) - post_json "#{project.http_url_to_repo}/info/lfs/objects/batch", body, headers + post_lfs_json "#{project.http_url_to_repo}/info/lfs/objects/batch", body, headers end it 'responds with 501' do @@ -71,8 +72,9 @@ describe Gitlab::Lfs::Router do end context 'when handling lfs request using deprecated API' do + let(:authorization) { authorize_user } before do - post_json "#{project.http_url_to_repo}/info/lfs/objects", nil, headers + post_lfs_json "#{project.http_url_to_repo}/info/lfs/objects", nil, headers end it_behaves_like 'a deprecated' @@ -118,8 +120,8 @@ describe Gitlab::Lfs::Router do project.lfs_objects << lfs_object end - it 'responds with status 403' do - expect(response).to have_http_status(403) + it 'responds with status 404' do + expect(response).to have_http_status(404) end end @@ -147,8 +149,8 @@ describe Gitlab::Lfs::Router do context 'without required headers' do let(:authorization) { authorize_user } - it 'responds with status 403' do - expect(response).to have_http_status(403) + it 'responds with status 404' do + expect(response).to have_http_status(404) end end end @@ -162,7 +164,7 @@ describe Gitlab::Lfs::Router do enable_lfs update_lfs_permissions update_user_permissions - post_json "#{project.http_url_to_repo}/info/lfs/objects/batch", body, headers + post_lfs_json "#{project.http_url_to_repo}/info/lfs/objects/batch", body, headers end describe 'download' do @@ -304,10 +306,10 @@ describe Gitlab::Lfs::Router do end context 'when user does is not member of the project' do - let(:role) { :guest } + let(:update_user_permissions) { nil } - it 'responds with 403' do - expect(response).to have_http_status(403) + it 'responds with 404' do + expect(response).to have_http_status(404) end end @@ -510,6 +512,7 @@ describe Gitlab::Lfs::Router do describe 'unsupported' do let(:project) { create(:empty_project) } + let(:authorization) { authorize_user } let(:body) do { 'operation' => 'other', 'objects' => [ @@ -553,11 +556,11 @@ describe Gitlab::Lfs::Router do context 'and request is sent with a malformed headers' do before do - put_finalize('cat /etc/passwd') + put_finalize('/etc/passwd') end it 'does not recognize it as a valid lfs command' do - expect(response).to have_http_status(403) + expect(response).to have_http_status(401) end end end @@ -582,6 +585,16 @@ describe Gitlab::Lfs::Router do expect(response).to have_http_status(403) end end + + context 'and request is sent with a malformed headers' do + before do + put_finalize('/etc/passwd') + end + + it 'does not recognize it as a valid lfs command' do + expect(response).to have_http_status(403) + end + end end describe 'to one project' do @@ -624,9 +637,25 @@ describe Gitlab::Lfs::Router do expect(lfs_object.projects.pluck(:id)).to include(project.id) end end + + context 'invalid tempfiles' do + it 'rejects slashes in the tempfile name (path traversal' do + put_finalize('foo/bar') + expect(response).to have_http_status(403) + end + + it 'rejects tempfile names that do not start with the oid' do + put_finalize("foo#{sample_oid}") + expect(response).to have_http_status(403) + end + end end describe 'and user does not have push access' do + before do + project.team << [user, :reporter] + end + it_behaves_like 'forbidden' end end @@ -758,8 +787,8 @@ describe Gitlab::Lfs::Router do Projects::ForkService.new(project, user, {}).execute end - def post_json(url, body = nil, headers = nil) - post(url, body.try(:to_json), (headers || {}).merge('Content-Type' => 'application/json')) + def post_lfs_json(url, body = nil, headers = nil) + post(url, body.try(:to_json), (headers || {}).merge('Content-Type' => 'application/vnd.git-lfs+json')) end def json_response diff --git a/spec/services/ci/create_builds_service_spec.rb b/spec/services/ci/create_builds_service_spec.rb deleted file mode 100644 index 8b0becd83d3..00000000000 --- a/spec/services/ci/create_builds_service_spec.rb +++ /dev/null @@ -1,32 +0,0 @@ -require 'spec_helper' - -describe Ci::CreateBuildsService, services: true do - let(:pipeline) { create(:ci_pipeline, ref: 'master') } - let(:user) { create(:user) } - - describe '#execute' do - # Using stubbed .gitlab-ci.yml created in commit factory - # - - subject do - described_class.new(pipeline).execute('test', user, status, nil) - end - - context 'next builds available' do - let(:status) { 'success' } - - it { is_expected.to be_an_instance_of Array } - it { is_expected.to all(be_an_instance_of Ci::Build) } - - it 'does not persist created builds' do - expect(subject.first).not_to be_persisted - end - end - - context 'builds skipped' do - let(:status) { 'skipped' } - - it { is_expected.to be_empty } - end - end -end diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb new file mode 100644 index 00000000000..4aadd009f3e --- /dev/null +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -0,0 +1,214 @@ +require 'spec_helper' + +describe Ci::CreatePipelineService, services: true do + let(:project) { FactoryGirl.create(:project) } + let(:user) { create(:admin) } + + before do + stub_ci_pipeline_to_return_yaml_file + end + + describe '#execute' do + def execute(params) + described_class.new(project, user, params).execute + end + + context 'valid params' do + let(:pipeline) do + execute(ref: 'refs/heads/master', + before: '00000000', + after: project.commit.id, + commits: [{ message: "Message" }]) + end + + it { expect(pipeline).to be_kind_of(Ci::Pipeline) } + it { expect(pipeline).to be_valid } + it { expect(pipeline).to be_persisted } + it { expect(pipeline).to eq(project.pipelines.last) } + it { expect(pipeline).to have_attributes(user: user) } + it { expect(pipeline.builds.first).to be_kind_of(Ci::Build) } + end + + context "skip tag if there is no build for it" do + it "creates commit if there is appropriate job" do + result = execute(ref: 'refs/heads/master', + before: '00000000', + after: project.commit.id, + commits: [{ message: "Message" }]) + expect(result).to be_persisted + end + + it "creates commit if there is no appropriate job but deploy job has right ref setting" do + config = YAML.dump({ deploy: { script: "ls", only: ["master"] } }) + stub_ci_pipeline_yaml_file(config) + result = execute(ref: 'refs/heads/master', + before: '00000000', + after: project.commit.id, + commits: [{ message: "Message" }]) + + expect(result).to be_persisted + end + end + + it 'skips creating pipeline for refs without .gitlab-ci.yml' do + stub_ci_pipeline_yaml_file(nil) + result = execute(ref: 'refs/heads/master', + before: '00000000', + after: project.commit.id, + commits: [{ message: 'Message' }]) + + expect(result).not_to be_persisted + expect(Ci::Pipeline.count).to eq(0) + end + + it 'fails commits if yaml is invalid' do + message = 'message' + allow_any_instance_of(Ci::Pipeline).to receive(:git_commit_message) { message } + stub_ci_pipeline_yaml_file('invalid: file: file') + commits = [{ message: message }] + pipeline = execute(ref: 'refs/heads/master', + before: '00000000', + after: project.commit.id, + commits: commits) + + expect(pipeline).to be_persisted + expect(pipeline.builds.any?).to be false + expect(pipeline.status).to eq('failed') + expect(pipeline.yaml_errors).not_to be_nil + end + + context 'when commit contains a [ci skip] directive' do + let(:message) { "some message[ci skip]" } + let(:messageFlip) { "some message[skip ci]" } + let(:capMessage) { "some message[CI SKIP]" } + let(:capMessageFlip) { "some message[SKIP CI]" } + + before do + allow_any_instance_of(Ci::Pipeline).to receive(:git_commit_message) { message } + end + + it "skips builds creation if there is [ci skip] tag in commit message" do + commits = [{ message: message }] + pipeline = execute(ref: 'refs/heads/master', + before: '00000000', + after: project.commit.id, + commits: commits) + + expect(pipeline).to be_persisted + expect(pipeline.builds.any?).to be false + expect(pipeline.status).to eq("skipped") + end + + it "skips builds creation if there is [skip ci] tag in commit message" do + commits = [{ message: messageFlip }] + pipeline = execute(ref: 'refs/heads/master', + before: '00000000', + after: project.commit.id, + commits: commits) + + expect(pipeline).to be_persisted + expect(pipeline.builds.any?).to be false + expect(pipeline.status).to eq("skipped") + end + + it "skips builds creation if there is [CI SKIP] tag in commit message" do + commits = [{ message: capMessage }] + pipeline = execute(ref: 'refs/heads/master', + before: '00000000', + after: project.commit.id, + commits: commits) + + expect(pipeline).to be_persisted + expect(pipeline.builds.any?).to be false + expect(pipeline.status).to eq("skipped") + end + + it "skips builds creation if there is [SKIP CI] tag in commit message" do + commits = [{ message: capMessageFlip }] + pipeline = execute(ref: 'refs/heads/master', + before: '00000000', + after: project.commit.id, + commits: commits) + + expect(pipeline).to be_persisted + expect(pipeline.builds.any?).to be false + expect(pipeline.status).to eq("skipped") + end + + it "does not skips builds creation if there is no [ci skip] or [skip ci] tag in commit message" do + allow_any_instance_of(Ci::Pipeline).to receive(:git_commit_message) { "some message" } + + commits = [{ message: "some message" }] + pipeline = execute(ref: 'refs/heads/master', + before: '00000000', + after: project.commit.id, + commits: commits) + + expect(pipeline).to be_persisted + expect(pipeline.builds.first.name).to eq("rspec") + end + + it "fails builds creation if there is [ci skip] tag in commit message and yaml is invalid" do + stub_ci_pipeline_yaml_file('invalid: file: fiile') + commits = [{ message: message }] + pipeline = execute(ref: 'refs/heads/master', + before: '00000000', + after: project.commit.id, + commits: commits) + + expect(pipeline).to be_persisted + expect(pipeline.builds.any?).to be false + expect(pipeline.status).to eq("failed") + expect(pipeline.yaml_errors).not_to be_nil + end + end + + it "creates commit with failed status if yaml is invalid" do + stub_ci_pipeline_yaml_file('invalid: file') + commits = [{ message: "some message" }] + pipeline = execute(ref: 'refs/heads/master', + before: '00000000', + after: project.commit.id, + commits: commits) + + expect(pipeline).to be_persisted + expect(pipeline.status).to eq("failed") + expect(pipeline.builds.any?).to be false + end + + context 'when there are no jobs for this pipeline' do + before do + config = YAML.dump({ test: { script: 'ls', only: ['feature'] } }) + stub_ci_pipeline_yaml_file(config) + end + + it 'does not create a new pipeline' do + result = execute(ref: 'refs/heads/master', + before: '00000000', + after: project.commit.id, + commits: [{ message: 'some msg' }]) + + expect(result).not_to be_persisted + expect(Ci::Build.all).to be_empty + expect(Ci::Pipeline.count).to eq(0) + end + end + + context 'with manual actions' do + before do + config = YAML.dump({ deploy: { script: 'ls', when: 'manual' } }) + stub_ci_pipeline_yaml_file(config) + end + + it 'does not create a new pipeline' do + result = execute(ref: 'refs/heads/master', + before: '00000000', + after: project.commit.id, + commits: [{ message: 'some msg' }]) + + expect(result).to be_persisted + expect(result.manual_actions).not_to be_empty + end + end + end +end diff --git a/spec/services/ci/create_trigger_request_service_spec.rb b/spec/services/ci/create_trigger_request_service_spec.rb index b72e0bd3dbe..d8c443d29d5 100644 --- a/spec/services/ci/create_trigger_request_service_spec.rb +++ b/spec/services/ci/create_trigger_request_service_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Ci::CreateTriggerRequestService, services: true do - let(:service) { Ci::CreateTriggerRequestService.new } + let(:service) { described_class.new } let(:project) { create(:project) } let(:trigger) { create(:ci_trigger, project: project) } @@ -27,8 +27,7 @@ describe Ci::CreateTriggerRequestService, services: true do subject { service.execute(project, trigger, 'master') } before do - stub_ci_pipeline_yaml_file('{}') - FactoryGirl.create :ci_pipeline, project: project + stub_ci_pipeline_yaml_file('script: { only: [develop], script: hello World }') end it { expect(subject).to be_nil } diff --git a/spec/services/ci/image_for_build_service_spec.rb b/spec/services/ci/image_for_build_service_spec.rb index 3a3e3efe709..259062406c7 100644 --- a/spec/services/ci/image_for_build_service_spec.rb +++ b/spec/services/ci/image_for_build_service_spec.rb @@ -5,8 +5,8 @@ module Ci let(:service) { ImageForBuildService.new } let(:project) { FactoryGirl.create(:empty_project) } let(:commit_sha) { '01234567890123456789' } - let(:commit) { project.ensure_pipeline(commit_sha, 'master') } - let(:build) { FactoryGirl.create(:ci_build, pipeline: commit) } + let(:pipeline) { project.ensure_pipeline(commit_sha, 'master') } + let(:build) { FactoryGirl.create(:ci_build, pipeline: pipeline) } describe '#execute' do before { build } @@ -14,6 +14,7 @@ module Ci context 'branch name' do before { allow(project).to receive(:commit).and_return(OpenStruct.new(sha: commit_sha)) } before { build.run! } + before { pipeline.reload_status! } let(:image) { service.execute(project, ref: 'master') } it { expect(image).to be_kind_of(OpenStruct) } @@ -31,6 +32,7 @@ module Ci context 'commit sha' do before { build.run! } + before { pipeline.reload_status! } let(:image) { service.execute(project, sha: build.sha) } it { expect(image).to be_kind_of(OpenStruct) } diff --git a/spec/services/ci/process_pipeline_service_spec.rb b/spec/services/ci/process_pipeline_service_spec.rb new file mode 100644 index 00000000000..ad8c2485888 --- /dev/null +++ b/spec/services/ci/process_pipeline_service_spec.rb @@ -0,0 +1,288 @@ +require 'spec_helper' + +describe Ci::ProcessPipelineService, services: true do + let(:pipeline) { create(:ci_pipeline, ref: 'master') } + let(:user) { create(:user) } + let(:all_builds) { pipeline.builds } + let(:builds) { all_builds.where.not(status: [:created, :skipped]) } + let(:config) { nil } + + before do + allow(pipeline).to receive(:ci_yaml_file).and_return(config) + end + + describe '#execute' do + def create_builds + described_class.new(pipeline.project, user).execute(pipeline) + end + + def succeed_pending + builds.pending.update_all(status: 'success') + end + + context 'start queuing next builds' do + before do + create(:ci_build, :created, pipeline: pipeline, name: 'linux', stage_idx: 0) + create(:ci_build, :created, pipeline: pipeline, name: 'mac', stage_idx: 0) + create(:ci_build, :created, pipeline: pipeline, name: 'rspec', stage_idx: 1) + create(:ci_build, :created, pipeline: pipeline, name: 'rubocop', stage_idx: 1) + create(:ci_build, :created, pipeline: pipeline, name: 'deploy', stage_idx: 2) + end + + it 'processes a pipeline' do + expect(create_builds).to be_truthy + succeed_pending + expect(builds.success.count).to eq(2) + + expect(create_builds).to be_truthy + succeed_pending + expect(builds.success.count).to eq(4) + + expect(create_builds).to be_truthy + succeed_pending + expect(builds.success.count).to eq(5) + + expect(create_builds).to be_falsey + end + + it 'does not process pipeline if existing stage is running' do + expect(create_builds).to be_truthy + expect(builds.pending.count).to eq(2) + + expect(create_builds).to be_falsey + expect(builds.pending.count).to eq(2) + end + end + + context 'custom stage with first job allowed to fail' do + before do + create(:ci_build, :created, pipeline: pipeline, name: 'clean_job', stage_idx: 0, allow_failure: true) + create(:ci_build, :created, pipeline: pipeline, name: 'test_job', stage_idx: 1, allow_failure: true) + end + + it 'automatically triggers a next stage when build finishes' do + expect(create_builds).to be_truthy + expect(builds.pluck(:status)).to contain_exactly('pending') + + pipeline.builds.running_or_pending.each(&:drop) + expect(builds.pluck(:status)).to contain_exactly('failed', 'pending') + end + end + + context 'properly creates builds when "when" is defined' do + before do + create(:ci_build, :created, pipeline: pipeline, name: 'build', stage_idx: 0) + create(:ci_build, :created, pipeline: pipeline, name: 'test', stage_idx: 1) + create(:ci_build, :created, pipeline: pipeline, name: 'test_failure', stage_idx: 2, when: 'on_failure') + create(:ci_build, :created, pipeline: pipeline, name: 'deploy', stage_idx: 3) + create(:ci_build, :created, pipeline: pipeline, name: 'production', stage_idx: 3, when: 'manual') + create(:ci_build, :created, pipeline: pipeline, name: 'cleanup', stage_idx: 4, when: 'always') + create(:ci_build, :created, pipeline: pipeline, name: 'clear cache', stage_idx: 4, when: 'manual') + end + + context 'when builds are successful' do + it 'properly creates builds' do + expect(create_builds).to be_truthy + expect(builds.pluck(:name)).to contain_exactly('build') + expect(builds.pluck(:status)).to contain_exactly('pending') + pipeline.builds.running_or_pending.each(&:success) + + expect(builds.pluck(:name)).to contain_exactly('build', 'test') + expect(builds.pluck(:status)).to contain_exactly('success', 'pending') + pipeline.builds.running_or_pending.each(&:success) + + expect(builds.pluck(:name)).to contain_exactly('build', 'test', 'deploy') + expect(builds.pluck(:status)).to contain_exactly('success', 'success', 'pending') + pipeline.builds.running_or_pending.each(&:success) + + expect(builds.pluck(:name)).to contain_exactly('build', 'test', 'deploy', 'cleanup') + expect(builds.pluck(:status)).to contain_exactly('success', 'success', 'success', 'pending') + pipeline.builds.running_or_pending.each(&:success) + + expect(builds.pluck(:status)).to contain_exactly('success', 'success', 'success', 'success') + pipeline.reload + expect(pipeline.status).to eq('success') + end + end + + context 'when test job fails' do + it 'properly creates builds' do + expect(create_builds).to be_truthy + expect(builds.pluck(:name)).to contain_exactly('build') + expect(builds.pluck(:status)).to contain_exactly('pending') + pipeline.builds.running_or_pending.each(&:success) + + expect(builds.pluck(:name)).to contain_exactly('build', 'test') + expect(builds.pluck(:status)).to contain_exactly('success', 'pending') + pipeline.builds.running_or_pending.each(&:drop) + + expect(builds.pluck(:name)).to contain_exactly('build', 'test', 'test_failure') + expect(builds.pluck(:status)).to contain_exactly('success', 'failed', 'pending') + pipeline.builds.running_or_pending.each(&:success) + + expect(builds.pluck(:name)).to contain_exactly('build', 'test', 'test_failure', 'cleanup') + expect(builds.pluck(:status)).to contain_exactly('success', 'failed', 'success', 'pending') + pipeline.builds.running_or_pending.each(&:success) + + expect(builds.pluck(:status)).to contain_exactly('success', 'failed', 'success', 'success') + pipeline.reload + expect(pipeline.status).to eq('failed') + end + end + + context 'when test and test_failure jobs fail' do + it 'properly creates builds' do + expect(create_builds).to be_truthy + expect(builds.pluck(:name)).to contain_exactly('build') + expect(builds.pluck(:status)).to contain_exactly('pending') + pipeline.builds.running_or_pending.each(&:success) + + expect(builds.pluck(:name)).to contain_exactly('build', 'test') + expect(builds.pluck(:status)).to contain_exactly('success', 'pending') + pipeline.builds.running_or_pending.each(&:drop) + + expect(builds.pluck(:name)).to contain_exactly('build', 'test', 'test_failure') + expect(builds.pluck(:status)).to contain_exactly('success', 'failed', 'pending') + pipeline.builds.running_or_pending.each(&:drop) + + expect(builds.pluck(:name)).to contain_exactly('build', 'test', 'test_failure', 'cleanup') + expect(builds.pluck(:status)).to contain_exactly('success', 'failed', 'failed', 'pending') + pipeline.builds.running_or_pending.each(&:success) + + expect(builds.pluck(:name)).to contain_exactly('build', 'test', 'test_failure', 'cleanup') + expect(builds.pluck(:status)).to contain_exactly('success', 'failed', 'failed', 'success') + pipeline.reload + expect(pipeline.status).to eq('failed') + end + end + + context 'when deploy job fails' do + it 'properly creates builds' do + expect(create_builds).to be_truthy + expect(builds.pluck(:name)).to contain_exactly('build') + expect(builds.pluck(:status)).to contain_exactly('pending') + pipeline.builds.running_or_pending.each(&:success) + + expect(builds.pluck(:name)).to contain_exactly('build', 'test') + expect(builds.pluck(:status)).to contain_exactly('success', 'pending') + pipeline.builds.running_or_pending.each(&:success) + + expect(builds.pluck(:name)).to contain_exactly('build', 'test', 'deploy') + expect(builds.pluck(:status)).to contain_exactly('success', 'success', 'pending') + pipeline.builds.running_or_pending.each(&:drop) + + expect(builds.pluck(:name)).to contain_exactly('build', 'test', 'deploy', 'cleanup') + expect(builds.pluck(:status)).to contain_exactly('success', 'success', 'failed', 'pending') + pipeline.builds.running_or_pending.each(&:success) + + expect(builds.pluck(:status)).to contain_exactly('success', 'success', 'failed', 'success') + pipeline.reload + expect(pipeline.status).to eq('failed') + end + end + + context 'when build is canceled in the second stage' do + it 'does not schedule builds after build has been canceled' do + expect(create_builds).to be_truthy + expect(builds.pluck(:name)).to contain_exactly('build') + expect(builds.pluck(:status)).to contain_exactly('pending') + pipeline.builds.running_or_pending.each(&:success) + + expect(builds.running_or_pending).not_to be_empty + + expect(builds.pluck(:name)).to contain_exactly('build', 'test') + expect(builds.pluck(:status)).to contain_exactly('success', 'pending') + pipeline.builds.running_or_pending.each(&:cancel) + + expect(builds.running_or_pending).to be_empty + expect(pipeline.reload.status).to eq('canceled') + end + end + + context 'when listing manual actions' do + it 'returns only for skipped builds' do + # currently all builds are created + expect(create_builds).to be_truthy + expect(manual_actions).to be_empty + + # succeed stage build + pipeline.builds.running_or_pending.each(&:success) + expect(manual_actions).to be_empty + + # succeed stage test + pipeline.builds.running_or_pending.each(&:success) + expect(manual_actions).to be_one # production + + # succeed stage deploy + pipeline.builds.running_or_pending.each(&:success) + expect(manual_actions).to be_many # production and clear cache + end + + def manual_actions + pipeline.manual_actions + end + end + end + + context 'creates a builds from .gitlab-ci.yml' do + let(:config) do + YAML.dump({ + rspec: { + stage: 'test', + script: 'rspec' + }, + rubocop: { + stage: 'test', + script: 'rubocop' + }, + deploy: { + stage: 'deploy', + script: 'deploy' + } + }) + end + + # Using stubbed .gitlab-ci.yml created in commit factory + # + + before do + stub_ci_pipeline_yaml_file(config) + create(:ci_build, :created, pipeline: pipeline, name: 'linux', stage: 'build', stage_idx: 0) + create(:ci_build, :created, pipeline: pipeline, name: 'mac', stage: 'build', stage_idx: 0) + end + + it 'when processing a pipeline' do + # Currently we have two builds with state created + expect(builds.count).to eq(0) + expect(all_builds.count).to eq(2) + + # Create builds will mark the created as pending + expect(create_builds).to be_truthy + expect(builds.count).to eq(2) + expect(all_builds.count).to eq(2) + + # When we builds succeed we will create a rest of pipeline from .gitlab-ci.yml + # We will have 2 succeeded, 2 pending (from stage test), total 5 (one more build from deploy) + succeed_pending + expect(create_builds).to be_truthy + expect(builds.success.count).to eq(2) + expect(builds.pending.count).to eq(2) + expect(all_builds.count).to eq(5) + + # When we succeed the 2 pending from stage test, + # We will queue a deploy stage, no new builds will be created + succeed_pending + expect(create_builds).to be_truthy + expect(builds.pending.count).to eq(1) + expect(builds.success.count).to eq(4) + expect(all_builds.count).to eq(5) + + # When we succeed last pending build, we will have a total of 5 succeeded builds, no new builds will be created + succeed_pending + expect(create_builds).to be_falsey + expect(builds.success.count).to eq(5) + expect(all_builds.count).to eq(5) + end + end + end +end diff --git a/spec/services/create_commit_builds_service_spec.rb b/spec/services/create_commit_builds_service_spec.rb deleted file mode 100644 index d4c5e584421..00000000000 --- a/spec/services/create_commit_builds_service_spec.rb +++ /dev/null @@ -1,241 +0,0 @@ -require 'spec_helper' - -describe CreateCommitBuildsService, services: true do - let(:service) { CreateCommitBuildsService.new } - let(:project) { FactoryGirl.create(:empty_project) } - let(:user) { create(:user) } - - before do - stub_ci_pipeline_to_return_yaml_file - end - - describe '#execute' do - context 'valid params' do - let(:pipeline) do - service.execute(project, user, - ref: 'refs/heads/master', - before: '00000000', - after: '31das312', - commits: [{ message: "Message" }] - ) - end - - it { expect(pipeline).to be_kind_of(Ci::Pipeline) } - it { expect(pipeline).to be_valid } - it { expect(pipeline).to be_persisted } - it { expect(pipeline).to eq(project.pipelines.last) } - it { expect(pipeline).to have_attributes(user: user) } - it { expect(pipeline.builds.first).to be_kind_of(Ci::Build) } - end - - context "skip tag if there is no build for it" do - it "creates commit if there is appropriate job" do - result = service.execute(project, user, - ref: 'refs/tags/0_1', - before: '00000000', - after: '31das312', - commits: [{ message: "Message" }] - ) - expect(result).to be_persisted - end - - it "creates commit if there is no appropriate job but deploy job has right ref setting" do - config = YAML.dump({ deploy: { script: "ls", only: ["0_1"] } }) - stub_ci_pipeline_yaml_file(config) - - result = service.execute(project, user, - ref: 'refs/heads/0_1', - before: '00000000', - after: '31das312', - commits: [{ message: "Message" }] - ) - expect(result).to be_persisted - end - end - - it 'skips creating pipeline for refs without .gitlab-ci.yml' do - stub_ci_pipeline_yaml_file(nil) - result = service.execute(project, user, - ref: 'refs/heads/0_1', - before: '00000000', - after: '31das312', - commits: [{ message: 'Message' }] - ) - expect(result).to be_falsey - expect(Ci::Pipeline.count).to eq(0) - end - - it 'fails commits if yaml is invalid' do - message = 'message' - allow_any_instance_of(Ci::Pipeline).to receive(:git_commit_message) { message } - stub_ci_pipeline_yaml_file('invalid: file: file') - commits = [{ message: message }] - pipeline = service.execute(project, user, - ref: 'refs/tags/0_1', - before: '00000000', - after: '31das312', - commits: commits - ) - expect(pipeline).to be_persisted - expect(pipeline.builds.any?).to be false - expect(pipeline.status).to eq('failed') - expect(pipeline.yaml_errors).not_to be_nil - end - - context 'when commit contains a [ci skip] directive' do - let(:message) { "some message[ci skip]" } - let(:messageFlip) { "some message[skip ci]" } - let(:capMessage) { "some message[CI SKIP]" } - let(:capMessageFlip) { "some message[SKIP CI]" } - - before do - allow_any_instance_of(Ci::Pipeline).to receive(:git_commit_message) { message } - end - - it "skips builds creation if there is [ci skip] tag in commit message" do - commits = [{ message: message }] - pipeline = service.execute(project, user, - ref: 'refs/tags/0_1', - before: '00000000', - after: '31das312', - commits: commits - ) - - expect(pipeline).to be_persisted - expect(pipeline.builds.any?).to be false - expect(pipeline.status).to eq("skipped") - end - - it "skips builds creation if there is [skip ci] tag in commit message" do - commits = [{ message: messageFlip }] - pipeline = service.execute(project, user, - ref: 'refs/tags/0_1', - before: '00000000', - after: '31das312', - commits: commits - ) - - expect(pipeline).to be_persisted - expect(pipeline.builds.any?).to be false - expect(pipeline.status).to eq("skipped") - end - - it "skips builds creation if there is [CI SKIP] tag in commit message" do - commits = [{ message: capMessage }] - pipeline = service.execute(project, user, - ref: 'refs/tags/0_1', - before: '00000000', - after: '31das312', - commits: commits - ) - - expect(pipeline).to be_persisted - expect(pipeline.builds.any?).to be false - expect(pipeline.status).to eq("skipped") - end - - it "skips builds creation if there is [SKIP CI] tag in commit message" do - commits = [{ message: capMessageFlip }] - pipeline = service.execute(project, user, - ref: 'refs/tags/0_1', - before: '00000000', - after: '31das312', - commits: commits - ) - - expect(pipeline).to be_persisted - expect(pipeline.builds.any?).to be false - expect(pipeline.status).to eq("skipped") - end - - it "does not skips builds creation if there is no [ci skip] or [skip ci] tag in commit message" do - allow_any_instance_of(Ci::Pipeline).to receive(:git_commit_message) { "some message" } - - commits = [{ message: "some message" }] - pipeline = service.execute(project, user, - ref: 'refs/tags/0_1', - before: '00000000', - after: '31das312', - commits: commits - ) - - expect(pipeline).to be_persisted - expect(pipeline.builds.first.name).to eq("staging") - end - - it "skips builds creation if there is [ci skip] tag in commit message and yaml is invalid" do - stub_ci_pipeline_yaml_file('invalid: file: fiile') - commits = [{ message: message }] - pipeline = service.execute(project, user, - ref: 'refs/tags/0_1', - before: '00000000', - after: '31das312', - commits: commits - ) - expect(pipeline).to be_persisted - expect(pipeline.builds.any?).to be false - expect(pipeline.status).to eq("skipped") - expect(pipeline.yaml_errors).to be_nil - end - end - - it "skips build creation if there are already builds" do - allow_any_instance_of(Ci::Pipeline).to receive(:ci_yaml_file) { gitlab_ci_yaml } - - commits = [{ message: "message" }] - pipeline = service.execute(project, user, - ref: 'refs/heads/master', - before: '00000000', - after: '31das312', - commits: commits - ) - expect(pipeline).to be_persisted - expect(pipeline.builds.count(:all)).to eq(2) - - pipeline = service.execute(project, user, - ref: 'refs/heads/master', - before: '00000000', - after: '31das312', - commits: commits - ) - expect(pipeline).to be_persisted - expect(pipeline.builds.count(:all)).to eq(2) - end - - it "creates commit with failed status if yaml is invalid" do - stub_ci_pipeline_yaml_file('invalid: file') - - commits = [{ message: "some message" }] - - pipeline = service.execute(project, user, - ref: 'refs/tags/0_1', - before: '00000000', - after: '31das312', - commits: commits - ) - - expect(pipeline).to be_persisted - expect(pipeline.status).to eq("failed") - expect(pipeline.builds.any?).to be false - end - - context 'when there are no jobs for this pipeline' do - before do - config = YAML.dump({ test: { script: 'ls', only: ['feature'] } }) - stub_ci_pipeline_yaml_file(config) - end - - it 'does not create a new pipeline' do - result = service.execute(project, user, - ref: 'refs/heads/master', - before: '00000000', - after: '31das312', - commits: [{ message: 'some msg' }]) - - expect(result).to be_falsey - expect(Ci::Build.all).to be_empty - expect(Ci::Pipeline.count).to eq(0) - end - end - end -end diff --git a/spec/services/merge_requests/merge_when_build_succeeds_service_spec.rb b/spec/services/merge_requests/merge_when_build_succeeds_service_spec.rb index 4da8146e3d6..520e906b21f 100644 --- a/spec/services/merge_requests/merge_when_build_succeeds_service_spec.rb +++ b/spec/services/merge_requests/merge_when_build_succeeds_service_spec.rb @@ -110,19 +110,15 @@ describe MergeRequests::MergeWhenBuildSucceedsService do context 'properly handles multiple stages' do let(:ref) { mr_merge_if_green_enabled.source_branch } - let(:build) { create(:ci_build, pipeline: pipeline, ref: ref, name: 'build', stage: 'build') } - let(:test) { create(:ci_build, pipeline: pipeline, ref: ref, name: 'test', stage: 'test') } + let!(:build) { create(:ci_build, :created, pipeline: pipeline, ref: ref, name: 'build', stage: 'build') } + let!(:test) { create(:ci_build, :created, pipeline: pipeline, ref: ref, name: 'test', stage: 'test') } + let(:pipeline) { create(:ci_empty_pipeline, ref: mr_merge_if_green_enabled.source_branch, project: project) } before do # This behavior of MergeRequest: we instantiate a new object allow_any_instance_of(MergeRequest).to receive(:pipeline).and_wrap_original do Ci::Pipeline.find(pipeline.id) end - - # We create test after the build - allow(pipeline).to receive(:create_next_builds).and_wrap_original do - test - end end it "doesn't merge if some stages failed" do diff --git a/spec/support/api/members_shared_examples.rb b/spec/support/api/members_shared_examples.rb new file mode 100644 index 00000000000..dab71a35a55 --- /dev/null +++ b/spec/support/api/members_shared_examples.rb @@ -0,0 +1,11 @@ +shared_examples 'a 404 response when source is private' do + before do + source.update_column(:visibility_level, Gitlab::VisibilityLevel::PRIVATE) + end + + it 'returns 404' do + route + + expect(response).to have_http_status(404) + end +end diff --git a/spec/tasks/gitlab/backup_rake_spec.rb b/spec/tasks/gitlab/backup_rake_spec.rb index baf78208ec5..548e7780c36 100644 --- a/spec/tasks/gitlab/backup_rake_spec.rb +++ b/spec/tasks/gitlab/backup_rake_spec.rb @@ -42,7 +42,7 @@ describe 'gitlab:app namespace rake task' do before do allow(Dir).to receive(:glob).and_return([]) allow(Dir).to receive(:chdir) - allow(File).to receive(:exists?).and_return(true) + allow(File).to receive(:exist?).and_return(true) allow(Kernel).to receive(:system).and_return(true) allow(FileUtils).to receive(:cp_r).and_return(true) allow(FileUtils).to receive(:mv).and_return(true) diff --git a/spec/workers/post_receive_spec.rb b/spec/workers/post_receive_spec.rb index 7f803a06902..1d2cf7acddd 100644 --- a/spec/workers/post_receive_spec.rb +++ b/spec/workers/post_receive_spec.rb @@ -53,7 +53,13 @@ describe PostReceive do subject { PostReceive.new.perform(pwd(project), key_id, base64_changes) } context "creates a Ci::Pipeline for every change" do - before { stub_ci_pipeline_to_return_yaml_file } + before do + allow_any_instance_of(Ci::CreatePipelineService).to receive(:commit) do + OpenStruct.new(id: '123456') + end + allow_any_instance_of(Ci::CreatePipelineService).to receive(:branch?).and_return(true) + stub_ci_pipeline_to_return_yaml_file + end it { expect{ subject }.to change{ Ci::Pipeline.count }.by(2) } end |