From 0f5b735685476ff51b26031e9b74fa8256354868 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Wed, 3 Apr 2019 22:00:29 -0700 Subject: Fix stage index migration failing in PostgreSQL 10 As discussed in https://www.postgresql.org/message-id/9922.1353433645%40sss.pgh.pa.us, the PostgreSQL window function last_value may not consider the right rows: Note that first_value, last_value, and nth_value consider only the rows within the "window frame", which by default contains the rows from the start of the partition through the last peer of the current row. This is likely to give unhelpful results for last_value and sometimes also nth_value. You can redefine the frame by adding a suitable frame specification (RANGE or ROWS) to the OVER clause. See Section 4.2.8 for more information about frame specifications. This query could be fixed by adding `RANGE BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING`, but that's quite verbose. It's simpler just to use the first_value function. Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/59985 --- lib/gitlab/background_migration/migrate_stage_index.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/background_migration/migrate_stage_index.rb b/lib/gitlab/background_migration/migrate_stage_index.rb index f90f35a913d..f921233460d 100644 --- a/lib/gitlab/background_migration/migrate_stage_index.rb +++ b/lib/gitlab/background_migration/migrate_stage_index.rb @@ -21,8 +21,8 @@ module Gitlab AND stage_idx IS NOT NULL GROUP BY stage_id, stage_idx ), indexes AS ( - SELECT DISTINCT stage_id, last_value(stage_idx) - OVER (PARTITION BY stage_id ORDER BY freq ASC) AS index + SELECT DISTINCT stage_id, first_value(stage_idx) + OVER (PARTITION BY stage_id ORDER BY freq DESC) AS index FROM freqs ) -- cgit v1.2.1 From f3ad51f8a57df96bcc69b0821355ef29c3df2ac8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Trzci=C5=84ski?= Date: Mon, 8 Apr 2019 14:41:05 +0200 Subject: Improve performance of PR import This removes unneeded `.reload` call which makes AR to load ALL objects, and create its in-memory representation. --- lib/gitlab/import/merge_request_helpers.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/gitlab/import/merge_request_helpers.rb b/lib/gitlab/import/merge_request_helpers.rb index b3fe1fc0685..4bc39868389 100644 --- a/lib/gitlab/import/merge_request_helpers.rb +++ b/lib/gitlab/import/merge_request_helpers.rb @@ -22,7 +22,7 @@ module Gitlab # additional work that is strictly necessary. merge_request_id = insert_and_return_id(attributes, project.merge_requests) - merge_request = project.merge_requests.reload.find(merge_request_id) + merge_request = project.merge_requests.reset.find(merge_request_id) [merge_request, false] end -- cgit v1.2.1 From 4317a2a3a2e39e4c2594b0b28abf7a8cc694eeab Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Mon, 8 Apr 2019 15:33:30 +0000 Subject: Fix `updated_at` doesn't apply to `state_event` updates of issues via API --- lib/api/issues.rb | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) (limited to 'lib') diff --git a/lib/api/issues.rb b/lib/api/issues.rb index 999a9cb5a82..000c00ea9f9 100644 --- a/lib/api/issues.rb +++ b/lib/api/issues.rb @@ -230,9 +230,13 @@ module API issue = user_project.issues.find_by!(iid: params.delete(:issue_iid)) authorize! :update_issue, issue - # Setting created_at time only allowed for admins and project/group owners - unless current_user.admin? || user_project.owner == current_user || current_user.owned_groups.include?(user_project.owner) - params.delete(:updated_at) + # Setting updated_at only allowed for admins and owners as well + if params[:updated_at].present? + if current_user.admin? || user_project.owner == current_user || current_user.owned_groups.include?(user_project.owner) + issue.system_note_timestamp = params[:updated_at] + else + params.delete(:updated_at) + end end update_params = declared_params(include_missing: false).merge(request: request, api: true) -- cgit v1.2.1 From 26fdcf7b6103aa47943271a5f6358d9779d5a9b3 Mon Sep 17 00:00:00 2001 From: Daniel Wyatt Date: Mon, 8 Apr 2019 10:12:39 -0400 Subject: Fix GitHub project import visibility --- lib/gitlab/legacy_github_import/project_creator.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/gitlab/legacy_github_import/project_creator.rb b/lib/gitlab/legacy_github_import/project_creator.rb index ca1a1b8e9bd..b484b69c932 100644 --- a/lib/gitlab/legacy_github_import/project_creator.rb +++ b/lib/gitlab/legacy_github_import/project_creator.rb @@ -37,7 +37,7 @@ module Gitlab end def visibility_level - visibility_level = repo.private ? Gitlab::VisibilityLevel::PRIVATE : Gitlab::VisibilityLevel::PUBLIC + visibility_level = repo.private ? Gitlab::VisibilityLevel::PRIVATE : @namespace.visibility_level visibility_level = Gitlab::CurrentSettings.default_project_visibility if Gitlab::CurrentSettings.restricted_visibility_levels.include?(visibility_level) visibility_level -- cgit v1.2.1 From aa352a95df665ded5178c1b26d4492433e47714e Mon Sep 17 00:00:00 2001 From: Luke Duncalfe Date: Fri, 29 Mar 2019 14:07:03 +1300 Subject: Support merge request create with push options To create a new merge request: git push -u origin -o merge_request.create To create a new merge request setting target branch: git push -u origin -o merge_request.create \ -o merge_request.target=123 To update an existing merge request with a new target branch: git push -u origin -o merge_request.target=123 A new Gitlab::PushOptions class handles parsing and validating the push options array. This can be the start of the standard of GitLab accepting push options that follow namespacing rules. Rules are discussed in issue https://gitlab.com/gitlab-org/gitlab-ce/issues/43263. E.g. these push options: -o merge_request.create -o merge_request.target=123 Become parsed as: { merge_request: { create: true, target: '123', } } And are fetched with the class via: push_options.get(:merge_request) push_options.get(:merge_request, :create) push_options.get(:merge_request, :target) A new MergeRequests::PushOptionsHandlerService takes the `merge_request` namespaced push options and handles creating and updating merge requests. Any errors encountered are passed to the existing `output` Hash in Api::Internal's `post_receive` endpoint, and passed to gitlab-shell where they're output to the user. Issue https://gitlab.com/gitlab-org/gitlab-ce/issues/43263 --- lib/api/helpers/internal_helpers.rb | 5 +++ lib/api/internal.rb | 36 ++++++++++++++++---- lib/gitlab/push_options.rb | 65 +++++++++++++++++++++++++++++++++++++ 3 files changed, 99 insertions(+), 7 deletions(-) create mode 100644 lib/gitlab/push_options.rb (limited to 'lib') diff --git a/lib/api/helpers/internal_helpers.rb b/lib/api/helpers/internal_helpers.rb index 3fd824877ae..5014ba51b94 100644 --- a/lib/api/helpers/internal_helpers.rb +++ b/lib/api/helpers/internal_helpers.rb @@ -43,6 +43,11 @@ module API ::MergeRequests::GetUrlsService.new(project).execute(params[:changes]) end + def push_options_warning(warning) + options = Array.wrap(params[:push_options]).map { |p| "'#{p}'" }.join(' ') + "Error encountered with push options #{options}: #{warning}" + end + def redis_ping result = Gitlab::Redis::SharedState.with { |redis| redis.ping } diff --git a/lib/api/internal.rb b/lib/api/internal.rb index 9c7b9146c8f..75202fa953c 100644 --- a/lib/api/internal.rb +++ b/lib/api/internal.rb @@ -256,19 +256,41 @@ module API post '/post_receive' do status 200 + output = {} # Messages to gitlab-shell + user = identify(params[:identifier]) + project = Gitlab::GlRepository.parse(params[:gl_repository]).first + push_options = Gitlab::PushOptions.new(params[:push_options]) + PostReceive.perform_async(params[:gl_repository], params[:identifier], params[:changes], params[:push_options].to_a) + + if (mr_options = push_options.get(:merge_request)) + begin + service = ::MergeRequests::PushOptionsHandlerService.new( + project, + user, + params[:changes], + mr_options + ).execute + + if service.errors.present? + output[:warnings] = push_options_warning(service.errors.join("\n\n")) + end + rescue ::MergeRequests::PushOptionsHandlerService::Error => e + output[:warnings] = push_options_warning(e.message) + rescue Gitlab::Access::AccessDeniedError + output[:warnings] = push_options_warning('User access was denied') + end + end + broadcast_message = BroadcastMessage.current&.last&.message reference_counter_decreased = Gitlab::ReferenceCounter.new(params[:gl_repository]).decrease - output = { - merge_request_urls: merge_request_urls, + output.merge!( broadcast_message: broadcast_message, - reference_counter_decreased: reference_counter_decreased - } - - project = Gitlab::GlRepository.parse(params[:gl_repository]).first - user = identify(params[:identifier]) + reference_counter_decreased: reference_counter_decreased, + merge_request_urls: merge_request_urls + ) # A user is not guaranteed to be returned; an orphaned write deploy # key could be used diff --git a/lib/gitlab/push_options.rb b/lib/gitlab/push_options.rb new file mode 100644 index 00000000000..923aa09527d --- /dev/null +++ b/lib/gitlab/push_options.rb @@ -0,0 +1,65 @@ +# frozen_string_literal: true + +module Gitlab + class PushOptions + VALID_OPTIONS = HashWithIndifferentAccess.new({ + merge_request: { + keys: [:create, :target] + }, + ci: { + keys: [:skip] + } + }).freeze + + NAMESPACE_ALIASES = HashWithIndifferentAccess.new({ + mr: :merge_request + }).freeze + + OPTION_MATCHER = /(?[^\.]+)\.(?[^=]+)=?(?.*)/ + + attr_reader :options + + def initialize(options = []) + @options = parse_options(options) + end + + def get(*args) + options.dig(*args) + end + + private + + def parse_options(raw_options) + options = HashWithIndifferentAccess.new + + Array.wrap(raw_options).each do |option| + namespace, key, value = parse_option(option) + + next if [namespace, key].any?(&:nil?) + + options[namespace] ||= HashWithIndifferentAccess.new + options[namespace][key] = value + end + + options + end + + def parse_option(option) + parts = OPTION_MATCHER.match(option) + return unless parts + + namespace, key, value = parts.values_at(:namespace, :key, :value).map(&:strip) + namespace = NAMESPACE_ALIASES[namespace] if NAMESPACE_ALIASES[namespace] + value = value.presence || true + + return unless valid_option?(namespace, key) + + [namespace, key, value] + end + + def valid_option?(namespace, key) + keys = VALID_OPTIONS.dig(namespace, :keys) + keys && keys.include?(key.to_sym) + end + end +end -- cgit v1.2.1 From ca884980ee8e6fe1269f5abdb803519d51aa09c0 Mon Sep 17 00:00:00 2001 From: Oswaldo Ferreira Date: Sun, 7 Apr 2019 15:35:16 -0300 Subject: [CE] Support multiple assignees for merge requests Backports https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/10161 (code out of ee/ folder). --- lib/api/entities.rb | 6 +++++- lib/api/merge_requests.rb | 4 ++++ lib/banzai/reference_parser/merge_request_parser.rb | 2 +- lib/gitlab/hook_data/issuable_builder.rb | 6 +----- lib/gitlab/hook_data/merge_request_builder.rb | 5 +++-- 5 files changed, 14 insertions(+), 9 deletions(-) (limited to 'lib') diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 2dd3120d3fc..8e0623887a8 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -663,7 +663,11 @@ module API expose(:user_notes_count) { |merge_request, options| issuable_metadata(merge_request, options, :user_notes_count) } expose(:upvotes) { |merge_request, options| issuable_metadata(merge_request, options, :upvotes) } expose(:downvotes) { |merge_request, options| issuable_metadata(merge_request, options, :downvotes) } - expose :author, :assignee, using: Entities::UserBasic + expose :assignee, using: ::API::Entities::UserBasic do |merge_request| + merge_request.assignee + end + expose :author, :assignees, using: Entities::UserBasic + expose :source_project_id, :target_project_id expose :labels do |merge_request| # Avoids an N+1 query since labels are preloaded diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index e4b21b7d1c4..1cc0ecc6df8 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -20,6 +20,7 @@ module API def self.update_params_at_least_one_of %i[ assignee_id + assignee_ids description labels milestone_id @@ -184,6 +185,7 @@ module API params :optional_params do optional :description, type: String, desc: 'The description of the merge request' optional :assignee_id, type: Integer, desc: 'The ID of a user to assign the merge request' + optional :assignee_ids, type: Array[Integer], desc: 'The array of user IDs to assign issue' optional :milestone_id, type: Integer, desc: 'The ID of a milestone to assign the merge request' optional :labels, type: Array[String], coerce_with: Validations::Types::LabelsList.coerce, desc: 'Comma-separated list of label names' optional :remove_source_branch, type: Boolean, desc: 'Remove source branch when merging' @@ -231,6 +233,7 @@ module API mr_params = declared_params(include_missing: false) mr_params[:force_remove_source_branch] = mr_params.delete(:remove_source_branch) + mr_params = convert_parameters_from_legacy_format(mr_params) merge_request = ::MergeRequests::CreateService.new(user_project, current_user, mr_params).execute @@ -334,6 +337,7 @@ module API mr_params = declared_params(include_missing: false) mr_params[:force_remove_source_branch] = mr_params.delete(:remove_source_branch) if mr_params[:remove_source_branch].present? + mr_params = convert_parameters_from_legacy_format(mr_params) merge_request = ::MergeRequests::UpdateService.new(user_project, current_user, mr_params).execute(merge_request) diff --git a/lib/banzai/reference_parser/merge_request_parser.rb b/lib/banzai/reference_parser/merge_request_parser.rb index e8147ac591a..d7bf450465e 100644 --- a/lib/banzai/reference_parser/merge_request_parser.rb +++ b/lib/banzai/reference_parser/merge_request_parser.rb @@ -10,7 +10,7 @@ module Banzai nodes, MergeRequest.includes( :author, - :assignee, + :assignees, { # These associations are primarily used for checking permissions. # Eager loading these ensures we don't end up running dozens of diff --git a/lib/gitlab/hook_data/issuable_builder.rb b/lib/gitlab/hook_data/issuable_builder.rb index 0803df65632..b8da6731081 100644 --- a/lib/gitlab/hook_data/issuable_builder.rb +++ b/lib/gitlab/hook_data/issuable_builder.rb @@ -20,11 +20,7 @@ module Gitlab repository: issuable.project.hook_attrs.slice(:name, :url, :description, :homepage) } - if issuable.is_a?(Issue) - hook_data[:assignees] = issuable.assignees.map(&:hook_attrs) if issuable.assignees.any? - else - hook_data[:assignee] = issuable.assignee.hook_attrs if issuable.assignee - end + hook_data[:assignees] = issuable.assignees.map(&:hook_attrs) if issuable.assignees.any? hook_data end diff --git a/lib/gitlab/hook_data/merge_request_builder.rb b/lib/gitlab/hook_data/merge_request_builder.rb index d77b1d04644..a8e993e087e 100644 --- a/lib/gitlab/hook_data/merge_request_builder.rb +++ b/lib/gitlab/hook_data/merge_request_builder.rb @@ -34,7 +34,6 @@ module Gitlab end SAFE_HOOK_RELATIONS = %i[ - assignee labels total_time_spent ].freeze @@ -51,7 +50,9 @@ module Gitlab work_in_progress: merge_request.work_in_progress?, total_time_spent: merge_request.total_time_spent, human_total_time_spent: merge_request.human_total_time_spent, - human_time_estimate: merge_request.human_time_estimate + human_time_estimate: merge_request.human_time_estimate, + assignee_ids: merge_request.assignee_ids, + assignee_id: merge_request.assignee_ids.first # This key is deprecated } merge_request.attributes.with_indifferent_access.slice(*self.class.safe_hook_attributes) -- cgit v1.2.1 From 1883e320eafa02b332a16eec658f65c4a28def83 Mon Sep 17 00:00:00 2001 From: Luke Duncalfe Date: Fri, 5 Apr 2019 17:19:30 +1300 Subject: Use Gitlab::PushOptions for `ci.skip` push option Previously the raw push option Array was sent to Pipeline::Chain::Skip. This commit updates this class (and the chain of classes that pass the push option parameters from the API internal `post_receive` endpoint to that class) to treat push options as a Hash of options parsed by GitLab::PushOptions. The GitLab::PushOptions class takes options like this: -o ci.skip -o merge_request.create -o merge_request.target=branch and turns them into a Hash like this: { ci: { skip: true }, merge_request: { create: true, target: 'branch' } } This now how Pipeline::Chain::Skip is determining if the `ci.skip` push option was used. --- lib/api/internal.rb | 2 +- lib/gitlab/ci/pipeline/chain/skip.rb | 4 ++-- lib/gitlab/data_builder/push.rb | 9 +++------ lib/gitlab/git_post_receive.rb | 2 +- lib/gitlab/push_options.rb | 5 +++++ 5 files changed, 12 insertions(+), 10 deletions(-) (limited to 'lib') diff --git a/lib/api/internal.rb b/lib/api/internal.rb index 75202fa953c..ee1fb465608 100644 --- a/lib/api/internal.rb +++ b/lib/api/internal.rb @@ -262,7 +262,7 @@ module API push_options = Gitlab::PushOptions.new(params[:push_options]) PostReceive.perform_async(params[:gl_repository], params[:identifier], - params[:changes], params[:push_options].to_a) + params[:changes], push_options.as_json) if (mr_options = push_options.get(:merge_request)) begin diff --git a/lib/gitlab/ci/pipeline/chain/skip.rb b/lib/gitlab/ci/pipeline/chain/skip.rb index 79bbcc1ed1e..7d6e0704d4a 100644 --- a/lib/gitlab/ci/pipeline/chain/skip.rb +++ b/lib/gitlab/ci/pipeline/chain/skip.rb @@ -8,7 +8,6 @@ module Gitlab include ::Gitlab::Utils::StrongMemoize SKIP_PATTERN = /\[(ci[ _-]skip|skip[ _-]ci)\]/i - SKIP_PUSH_OPTION = 'ci.skip' def perform! if skipped? @@ -35,7 +34,8 @@ module Gitlab end def push_option_skips_ci? - !!(@command.push_options&.include?(SKIP_PUSH_OPTION)) + @command.push_options.present? && + @command.push_options.deep_symbolize_keys.dig(:ci, :skip).present? end end end diff --git a/lib/gitlab/data_builder/push.rb b/lib/gitlab/data_builder/push.rb index ea08b5f7eae..af385d7d4ca 100644 --- a/lib/gitlab/data_builder/push.rb +++ b/lib/gitlab/data_builder/push.rb @@ -32,10 +32,7 @@ module Gitlab } ], total_commits_count: 1, - push_options: [ - "ci.skip", - "custom option" - ] + push_options: { ci: { skip: true } } }.freeze # Produce a hash of post-receive data @@ -57,11 +54,11 @@ module Gitlab # }, # commits: Array, # total_commits_count: Fixnum, - # push_options: Array + # push_options: Hash # } # # rubocop:disable Metrics/ParameterLists - def build(project, user, oldrev, newrev, ref, commits = [], message = nil, commits_count: nil, push_options: []) + def build(project, user, oldrev, newrev, ref, commits = [], message = nil, commits_count: nil, push_options: {}) commits = Array(commits) # Total commits count diff --git a/lib/gitlab/git_post_receive.rb b/lib/gitlab/git_post_receive.rb index 426436c2164..d98b85fecc4 100644 --- a/lib/gitlab/git_post_receive.rb +++ b/lib/gitlab/git_post_receive.rb @@ -5,7 +5,7 @@ module Gitlab include Gitlab::Identifier attr_reader :project, :identifier, :changes, :push_options - def initialize(project, identifier, changes, push_options) + def initialize(project, identifier, changes, push_options = {}) @project = project @identifier = identifier @changes = deserialize_changes(changes) diff --git a/lib/gitlab/push_options.rb b/lib/gitlab/push_options.rb index 923aa09527d..7c655c12995 100644 --- a/lib/gitlab/push_options.rb +++ b/lib/gitlab/push_options.rb @@ -27,6 +27,11 @@ module Gitlab options.dig(*args) end + # Allow #to_json serialization + def as_json(*_args) + options + end + private def parse_options(raw_options) -- cgit v1.2.1 From 68f189ad23d7a384f40caa152d263fdf1465b30a Mon Sep 17 00:00:00 2001 From: Luke Duncalfe Date: Fri, 5 Apr 2019 13:22:58 +0000 Subject: Support merge on pipeline success w/ push options MergeRequests::PushOptionsHandlerService has been updated to allow creating and updating merge requests with the `merge_when_pipeline_succeeds` set using git push options. To create a new merge request and set it to merge when the pipeline succeeds: git push -u origin -o merge_request.create \ -o merge_request.merge_when_pipeline_succeeds To update an existing merge request and set it to merge when the pipeline succeeds: git push -u origin -o merge_request.merge_when_pipeline_succeeds Issue https://gitlab.com/gitlab-org/gitlab-ce/issues/53198 --- lib/gitlab/push_options.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/gitlab/push_options.rb b/lib/gitlab/push_options.rb index 7c655c12995..810aba436cc 100644 --- a/lib/gitlab/push_options.rb +++ b/lib/gitlab/push_options.rb @@ -4,7 +4,7 @@ module Gitlab class PushOptions VALID_OPTIONS = HashWithIndifferentAccess.new({ merge_request: { - keys: [:create, :target] + keys: [:create, :merge_when_pipeline_succeeds, :target] }, ci: { keys: [:skip] -- cgit v1.2.1 From 285fcb4744ed5464aab7d2ff248e7824dd77315c Mon Sep 17 00:00:00 2001 From: Oswaldo Ferreira Date: Mon, 8 Apr 2019 19:47:23 -0300 Subject: Add methods to check dead and retrying jobs It adds two methods for checking if a background job (for a given class) has dead or retrying jobs. --- lib/gitlab/background_migration.rb | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/background_migration.rb b/lib/gitlab/background_migration.rb index 5251e0fadf9..2e3a4f3b869 100644 --- a/lib/gitlab/background_migration.rb +++ b/lib/gitlab/background_migration.rb @@ -58,11 +58,31 @@ module Gitlab migration_class_for(class_name).new.perform(*arguments) end - def self.exists?(migration_class) + def self.exists?(migration_class, additional_queues = []) enqueued = Sidekiq::Queue.new(self.queue) scheduled = Sidekiq::ScheduledSet.new - [enqueued, scheduled].each do |queue| + enqueued_job?([enqueued, scheduled], migration_class) + end + + def self.dead_jobs?(migration_class) + dead_set = Sidekiq::DeadSet.new + + enqueued_job?([dead_set], migration_class) + end + + def self.retrying_jobs?(migration_class) + retry_set = Sidekiq::RetrySet.new + + enqueued_job?([retry_set], migration_class) + end + + def self.migration_class_for(class_name) + const_get(class_name) + end + + def self.enqueued_job?(queues, migration_class) + queues.each do |queue| queue.each do |job| return true if job.queue == self.queue && job.args.first == migration_class end @@ -70,9 +90,5 @@ module Gitlab false end - - def self.migration_class_for(class_name) - const_get(class_name) - end end end -- cgit v1.2.1 From e73f537cb5097e85849110bafe184cb89e3bbc22 Mon Sep 17 00:00:00 2001 From: Luke Duncalfe Date: Mon, 8 Apr 2019 17:07:53 +1200 Subject: Refactor PushOptionsHandlerService from review Exceptions are no longer raised, instead all errors encountered are added to the errors property. MergeRequests::BuildService is used to generate attributes of a new merge request. Code moved from Api::Internal to Api::Helpers::InternalHelpers. --- lib/api/helpers/internal_helpers.rb | 17 +++++++++++++++++ lib/api/internal.rb | 20 ++------------------ 2 files changed, 19 insertions(+), 18 deletions(-) (limited to 'lib') diff --git a/lib/api/helpers/internal_helpers.rb b/lib/api/helpers/internal_helpers.rb index 5014ba51b94..71c30ec99a5 100644 --- a/lib/api/helpers/internal_helpers.rb +++ b/lib/api/helpers/internal_helpers.rb @@ -43,6 +43,23 @@ module API ::MergeRequests::GetUrlsService.new(project).execute(params[:changes]) end + def process_mr_push_options(push_options, project, user, changes) + output = {} + + service = ::MergeRequests::PushOptionsHandlerService.new( + project, + user, + changes, + push_options + ).execute + + if service.errors.present? + output[:warnings] = push_options_warning(service.errors.join("\n\n")) + end + + output + end + def push_options_warning(warning) options = Array.wrap(params[:push_options]).map { |p| "'#{p}'" }.join(' ') "Error encountered with push options #{options}: #{warning}" diff --git a/lib/api/internal.rb b/lib/api/internal.rb index ee1fb465608..224aaaaf006 100644 --- a/lib/api/internal.rb +++ b/lib/api/internal.rb @@ -264,24 +264,8 @@ module API PostReceive.perform_async(params[:gl_repository], params[:identifier], params[:changes], push_options.as_json) - if (mr_options = push_options.get(:merge_request)) - begin - service = ::MergeRequests::PushOptionsHandlerService.new( - project, - user, - params[:changes], - mr_options - ).execute - - if service.errors.present? - output[:warnings] = push_options_warning(service.errors.join("\n\n")) - end - rescue ::MergeRequests::PushOptionsHandlerService::Error => e - output[:warnings] = push_options_warning(e.message) - rescue Gitlab::Access::AccessDeniedError - output[:warnings] = push_options_warning('User access was denied') - end - end + mr_options = push_options.get(:merge_request) + output.merge!(process_mr_push_options(mr_options, project, user, params[:changes])) if mr_options.present? broadcast_message = BroadcastMessage.current&.last&.message reference_counter_decreased = Gitlab::ReferenceCounter.new(params[:gl_repository]).decrease -- cgit v1.2.1 From 3c40c98e263328ceb11a008dbec108362e727dbc Mon Sep 17 00:00:00 2001 From: Luke Duncalfe Date: Mon, 8 Apr 2019 21:59:12 +1200 Subject: Feature flag for merge requestion push options https://gitlab.com/gitlab-org/gitlab-ce/issues/43263 https://gitlab.com/gitlab-org/gitlab-ce/issues/53198 --- lib/api/internal.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/api/internal.rb b/lib/api/internal.rb index 224aaaaf006..00f0bbab231 100644 --- a/lib/api/internal.rb +++ b/lib/api/internal.rb @@ -264,8 +264,10 @@ module API PostReceive.perform_async(params[:gl_repository], params[:identifier], params[:changes], push_options.as_json) - mr_options = push_options.get(:merge_request) - output.merge!(process_mr_push_options(mr_options, project, user, params[:changes])) if mr_options.present? + if Feature.enabled?(:mr_push_options, default_enabled: true) + mr_options = push_options.get(:merge_request) + output.merge!(process_mr_push_options(mr_options, project, user, params[:changes])) if mr_options.present? + end broadcast_message = BroadcastMessage.current&.last&.message reference_counter_decreased = Gitlab::ReferenceCounter.new(params[:gl_repository]).decrease -- cgit v1.2.1 From 5b7003282b6b3ce1bfc313b3271bd6827a230c34 Mon Sep 17 00:00:00 2001 From: Jason Goodman Date: Tue, 9 Apr 2019 06:52:15 +0000 Subject: Set release name when adding release notes to an existing tag Also set the release sha and author --- lib/gitlab/legacy_github_import/release_formatter.rb | 1 + 1 file changed, 1 insertion(+) (limited to 'lib') diff --git a/lib/gitlab/legacy_github_import/release_formatter.rb b/lib/gitlab/legacy_github_import/release_formatter.rb index 8c0c17780ca..746786b5a66 100644 --- a/lib/gitlab/legacy_github_import/release_formatter.rb +++ b/lib/gitlab/legacy_github_import/release_formatter.rb @@ -7,6 +7,7 @@ module Gitlab { project: project, tag: raw_data.tag_name, + name: raw_data.name, description: raw_data.body, created_at: raw_data.created_at, updated_at: raw_data.created_at -- cgit v1.2.1 From 724f19ba0a051bbe8e9dd89f208261abe0f8133a Mon Sep 17 00:00:00 2001 From: Krasimir Angelov Date: Tue, 9 Apr 2019 09:16:57 +0000 Subject: Add new API endpoint to expose single environment This is resolving https://gitlab.com/gitlab-org/gitlab-ce/issues/30157. Implement new API endpoint `/projects/:id/environments/:environment_id` to expose single environment. Include information for environment's last deployment if there is one. --- lib/api/entities.rb | 9 +++++---- lib/api/environments.rb | 15 +++++++++++++++ 2 files changed, 20 insertions(+), 4 deletions(-) (limited to 'lib') diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 2dd3120d3fc..f9773086f69 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -1294,10 +1294,6 @@ module API expose :id, :name, :slug, :external_url end - class Environment < EnvironmentBasic - expose :project, using: Entities::BasicProjectDetails - end - class Deployment < Grape::Entity expose :id, :iid, :ref, :sha, :created_at expose :user, using: Entities::UserBasic @@ -1305,6 +1301,11 @@ module API expose :deployable, using: Entities::Job end + class Environment < EnvironmentBasic + expose :project, using: Entities::BasicProjectDetails + expose :last_deployment, using: Entities::Deployment, if: { last_deployment: true } + end + class LicenseBasic < Grape::Entity expose :key, :name, :nickname expose :url, as: :html_url diff --git a/lib/api/environments.rb b/lib/api/environments.rb index 5b0f3b914cb..6cd43923559 100644 --- a/lib/api/environments.rb +++ b/lib/api/environments.rb @@ -101,6 +101,21 @@ module API status 200 present environment, with: Entities::Environment, current_user: current_user end + + desc 'Get a single environment' do + success Entities::Environment + end + params do + requires :environment_id, type: Integer, desc: 'The environment ID' + end + get ':id/environments/:environment_id' do + authorize! :read_environment, user_project + + environment = user_project.environments.find(params[:environment_id]) + present environment, with: Entities::Environment, current_user: current_user, + except: [:project, { last_deployment: [:environment] }], + last_deployment: true + end end end end -- cgit v1.2.1 From 0a4f44de83fac88f39d9f4507c2fa4d935703e1b Mon Sep 17 00:00:00 2001 From: Wolphin Date: Tue, 9 Apr 2019 09:28:55 +0000 Subject: Add environment url validation --- lib/gitlab/ci/config/entry/environment.rb | 2 ++ 1 file changed, 2 insertions(+) (limited to 'lib') diff --git a/lib/gitlab/ci/config/entry/environment.rb b/lib/gitlab/ci/config/entry/environment.rb index 69a3a1aedef..5a13fd18504 100644 --- a/lib/gitlab/ci/config/entry/environment.rb +++ b/lib/gitlab/ci/config/entry/environment.rb @@ -36,10 +36,12 @@ module Gitlab validates :config, allowed_keys: ALLOWED_KEYS validates :url, + type: String, length: { maximum: 255 }, allow_nil: true validates :action, + type: String, inclusion: { in: %w[start stop], message: 'should be start or stop' }, allow_nil: true -- cgit v1.2.1 From 20093f9de0b34da88a8b01ca94ee773685b16308 Mon Sep 17 00:00:00 2001 From: Agustin Henze Date: Tue, 9 Apr 2019 14:53:44 +0000 Subject: Add new permission model `read-pipeline-variable` Used to get the variables via the API endpoint `/projects/:id/pipelines/:pipeline_id/variables` Signed-off-by: Agustin Henze --- lib/api/pipelines.rb | 13 +++++++++++++ 1 file changed, 13 insertions(+) (limited to 'lib') diff --git a/lib/api/pipelines.rb b/lib/api/pipelines.rb index ac8fe98e55e..f29a18e94cf 100644 --- a/lib/api/pipelines.rb +++ b/lib/api/pipelines.rb @@ -81,6 +81,19 @@ module API present pipeline, with: Entities::Pipeline end + desc 'Gets the variables for a given pipeline' do + detail 'This feature was introduced in GitLab 11.11' + success Entities::Variable + end + params do + requires :pipeline_id, type: Integer, desc: 'The pipeline ID' + end + get ':id/pipelines/:pipeline_id/variables' do + authorize! :read_pipeline_variable, pipeline + + present pipeline.variables, with: Entities::Variable + end + desc 'Deletes a pipeline' do detail 'This feature was introduced in GitLab 11.6' http_codes [[204, 'Pipeline was deleted'], [403, 'Forbidden']] -- cgit v1.2.1 From 469844c4f926c408cda4cb5b2a08dfd14a2c3997 Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Tue, 9 Apr 2019 17:21:16 +0200 Subject: Update comments about N + 1 Gitaly calls To make sure all known issues are linked to the correct epic, I've gone through the code base, and updated the comments where required. --- lib/banzai/filter/relative_link_filter.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/banzai/filter/relative_link_filter.rb b/lib/banzai/filter/relative_link_filter.rb index 2745905c5ff..199b3533cf4 100644 --- a/lib/banzai/filter/relative_link_filter.rb +++ b/lib/banzai/filter/relative_link_filter.rb @@ -150,7 +150,7 @@ module Banzai end def uri_type(path) - # https://gitlab.com/gitlab-org/gitlab-ce/issues/58011 + # https://gitlab.com/gitlab-org/gitlab-ce/issues/58657 Gitlab::GitalyClient.allow_n_plus_1_calls do @uri_types[path] ||= current_commit.uri_type(path) end -- cgit v1.2.1 From 9bc5ed14fe97fe63cd5be30c013c6af978715621 Mon Sep 17 00:00:00 2001 From: Imre Farkas Date: Tue, 9 Apr 2019 15:38:58 +0000 Subject: Move Contribution Analytics related spec in spec/features/groups/group_page_with_external_authorization_service_spec to EE --- lib/api/entities.rb | 3 ++ lib/api/helpers/projects_helpers.rb | 5 ++- lib/api/settings.rb | 4 +- lib/gitlab/external_authorization.rb | 40 +++++++++++++++++ lib/gitlab/external_authorization/access.rb | 55 +++++++++++++++++++++++ lib/gitlab/external_authorization/cache.rb | 62 ++++++++++++++++++++++++++ lib/gitlab/external_authorization/client.rb | 63 +++++++++++++++++++++++++++ lib/gitlab/external_authorization/config.rb | 47 ++++++++++++++++++++ lib/gitlab/external_authorization/logger.rb | 21 +++++++++ lib/gitlab/external_authorization/response.rb | 38 ++++++++++++++++ 10 files changed, 335 insertions(+), 3 deletions(-) create mode 100644 lib/gitlab/external_authorization.rb create mode 100644 lib/gitlab/external_authorization/access.rb create mode 100644 lib/gitlab/external_authorization/cache.rb create mode 100644 lib/gitlab/external_authorization/client.rb create mode 100644 lib/gitlab/external_authorization/config.rb create mode 100644 lib/gitlab/external_authorization/logger.rb create mode 100644 lib/gitlab/external_authorization/response.rb (limited to 'lib') diff --git a/lib/api/entities.rb b/lib/api/entities.rb index b45484945fd..4bdac278add 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -277,6 +277,7 @@ module API expose :statistics, using: 'API::Entities::ProjectStatistics', if: -> (project, options) { options[:statistics] && Ability.allowed?(options[:current_user], :read_statistics, project) } + expose :external_authorization_classification_label # rubocop: disable CodeReuse/ActiveRecord def self.preload_relation(projects_relation, options = {}) @@ -1120,6 +1121,8 @@ module API expose(:default_snippet_visibility) { |setting, _options| Gitlab::VisibilityLevel.string_level(setting.default_snippet_visibility) } expose(:default_group_visibility) { |setting, _options| Gitlab::VisibilityLevel.string_level(setting.default_group_visibility) } + expose(*::ApplicationSettingsHelper.external_authorization_service_attributes) + # support legacy names, can be removed in v5 expose :password_authentication_enabled_for_web, as: :password_authentication_enabled expose :password_authentication_enabled_for_web, as: :signin_enabled diff --git a/lib/api/helpers/projects_helpers.rb b/lib/api/helpers/projects_helpers.rb index 7b858dc2e72..aaf32dafca4 100644 --- a/lib/api/helpers/projects_helpers.rb +++ b/lib/api/helpers/projects_helpers.rb @@ -29,13 +29,13 @@ module API optional :printing_merge_request_link_enabled, type: Boolean, desc: 'Show link to create/view merge request when pushing from the command line' optional :merge_method, type: String, values: %w(ff rebase_merge merge), desc: 'The merge method used when merging merge requests' optional :initialize_with_readme, type: Boolean, desc: "Initialize a project with a README.md" + optional :external_authorization_classification_label, type: String, desc: 'The classification label for the project' end if Gitlab.ee? params :optional_project_params_ee do optional :repository_storage, type: String, desc: 'Which storage shard the repository is on. Available only to admins' optional :approvals_before_merge, type: Integer, desc: 'How many approvers should approve merge request by default' - optional :external_authorization_classification_label, type: String, desc: 'The classification label for the project' optional :mirror, type: Boolean, desc: 'Enables pull mirroring in a project' optional :mirror_trigger_builds, type: Boolean, desc: 'Pull mirroring triggers builds' end @@ -72,7 +72,8 @@ module API :tag_list, :visibility, :wiki_enabled, - :avatar + :avatar, + :external_authorization_classification_label ] end end diff --git a/lib/api/settings.rb b/lib/api/settings.rb index d96cdc31212..b064747e5fc 100644 --- a/lib/api/settings.rb +++ b/lib/api/settings.rb @@ -168,7 +168,9 @@ module API optional :usage_ping_enabled, type: Boolean, desc: 'Every week GitLab will report license usage back to GitLab, Inc.' end - optional_attributes = ::ApplicationSettingsHelper.visible_attributes << :performance_bar_allowed_group_id + optional_attributes = [*::ApplicationSettingsHelper.visible_attributes, + *::ApplicationSettingsHelper.external_authorization_service_attributes, + :performance_bar_allowed_group_id] if Gitlab.ee? optional_attributes += EE::ApplicationSettingsHelper.possible_licensed_attributes diff --git a/lib/gitlab/external_authorization.rb b/lib/gitlab/external_authorization.rb new file mode 100644 index 00000000000..25f8b7b3628 --- /dev/null +++ b/lib/gitlab/external_authorization.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +module Gitlab + module ExternalAuthorization + extend ExternalAuthorization::Config + + RequestFailed = Class.new(StandardError) + + def self.access_allowed?(user, label, project_path = nil) + return true unless perform_check? + return false unless user + + access_for_user_to_label(user, label, project_path).has_access? + end + + def self.rejection_reason(user, label) + return unless enabled? + return unless user + + access_for_user_to_label(user, label, nil).reason + end + + def self.access_for_user_to_label(user, label, project_path) + if RequestStore.active? + RequestStore.fetch("external_authorisation:user-#{user.id}:label-#{label}") do + load_access(user, label, project_path) + end + else + load_access(user, label, project_path) + end + end + + def self.load_access(user, label, project_path) + access = ::Gitlab::ExternalAuthorization::Access.new(user, label).load! + ::Gitlab::ExternalAuthorization::Logger.log_access(access, project_path) + + access + end + end +end diff --git a/lib/gitlab/external_authorization/access.rb b/lib/gitlab/external_authorization/access.rb new file mode 100644 index 00000000000..e111c41fcc2 --- /dev/null +++ b/lib/gitlab/external_authorization/access.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +module Gitlab + module ExternalAuthorization + class Access + attr_reader :user, + :reason, + :loaded_at, + :label, + :load_type + + def initialize(user, label) + @user, @label = user, label + end + + def loaded? + loaded_at && (loaded_at > ExternalAuthorization::Cache::VALIDITY_TIME.ago) + end + + def has_access? + @access + end + + def load! + load_from_cache + load_from_service unless loaded? + self + end + + private + + def load_from_cache + @load_type = :cache + @access, @reason, @loaded_at = cache.load + end + + def load_from_service + @load_type = :request + response = Client.new(@user, @label).request_access + @access = response.successful? + @reason = response.reason + @loaded_at = Time.now + cache.store(@access, @reason, @loaded_at) if response.valid? + rescue ::Gitlab::ExternalAuthorization::RequestFailed => e + @access = false + @reason = e.message + @loaded_at = Time.now + end + + def cache + @cache ||= ExternalAuthorization::Cache.new(@user, @label) + end + end + end +end diff --git a/lib/gitlab/external_authorization/cache.rb b/lib/gitlab/external_authorization/cache.rb new file mode 100644 index 00000000000..acdc028b4dc --- /dev/null +++ b/lib/gitlab/external_authorization/cache.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +module Gitlab + module ExternalAuthorization + class Cache + VALIDITY_TIME = 6.hours + + def initialize(user, label) + @user, @label = user, label + end + + def load + @access, @reason, @refreshed_at = ::Gitlab::Redis::Cache.with do |redis| + redis.hmget(cache_key, :access, :reason, :refreshed_at) + end + + [access, reason, refreshed_at] + end + + def store(new_access, new_reason, new_refreshed_at) + ::Gitlab::Redis::Cache.with do |redis| + redis.pipelined do + redis.mapped_hmset( + cache_key, + { + access: new_access.to_s, + reason: new_reason.to_s, + refreshed_at: new_refreshed_at.to_s + } + ) + + redis.expire(cache_key, VALIDITY_TIME) + end + end + end + + private + + def access + ::Gitlab::Utils.to_boolean(@access) + end + + def reason + # `nil` if the cached value was an empty string + return unless @reason.present? + + @reason + end + + def refreshed_at + # Don't try to parse a time if there was no cache + return unless @refreshed_at.present? + + Time.parse(@refreshed_at) + end + + def cache_key + "external_authorization:user-#{@user.id}:label-#{@label}" + end + end + end +end diff --git a/lib/gitlab/external_authorization/client.rb b/lib/gitlab/external_authorization/client.rb new file mode 100644 index 00000000000..60aab2e7044 --- /dev/null +++ b/lib/gitlab/external_authorization/client.rb @@ -0,0 +1,63 @@ +# frozen_string_literal: true + +Excon.defaults[:ssl_verify_peer] = false + +module Gitlab + module ExternalAuthorization + class Client + include ExternalAuthorization::Config + + REQUEST_HEADERS = { + 'Content-Type' => 'application/json', + 'Accept' => 'application/json' + }.freeze + + def initialize(user, label) + @user, @label = user, label + end + + def request_access + response = Excon.post( + service_url, + post_params + ) + ::Gitlab::ExternalAuthorization::Response.new(response) + rescue Excon::Error => e + raise ::Gitlab::ExternalAuthorization::RequestFailed.new(e) + end + + private + + def post_params + params = { headers: REQUEST_HEADERS, + body: body.to_json, + connect_timeout: timeout, + read_timeout: timeout, + write_timeout: timeout } + + if has_tls? + params[:client_cert_data] = client_cert + params[:client_key_data] = client_key + params[:client_key_pass] = client_key_pass + end + + params + end + + def body + @body ||= begin + body = { + user_identifier: @user.email, + project_classification_label: @label + } + + if @user.ldap_identity + body[:user_ldap_dn] = @user.ldap_identity.extern_uid + end + + body + end + end + end + end +end diff --git a/lib/gitlab/external_authorization/config.rb b/lib/gitlab/external_authorization/config.rb new file mode 100644 index 00000000000..8654a8c1e2e --- /dev/null +++ b/lib/gitlab/external_authorization/config.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +module Gitlab + module ExternalAuthorization + module Config + extend self + + def timeout + application_settings.external_authorization_service_timeout + end + + def service_url + application_settings.external_authorization_service_url + end + + def enabled? + application_settings.external_authorization_service_enabled + end + + def perform_check? + enabled? && service_url.present? + end + + def client_cert + application_settings.external_auth_client_cert + end + + def client_key + application_settings.external_auth_client_key + end + + def client_key_pass + application_settings.external_auth_client_key_pass + end + + def has_tls? + client_cert.present? && client_key.present? + end + + private + + def application_settings + ::Gitlab::CurrentSettings.current_application_settings + end + end + end +end diff --git a/lib/gitlab/external_authorization/logger.rb b/lib/gitlab/external_authorization/logger.rb new file mode 100644 index 00000000000..61246cd870e --- /dev/null +++ b/lib/gitlab/external_authorization/logger.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +module Gitlab + module ExternalAuthorization + class Logger < ::Gitlab::Logger + def self.log_access(access, project_path) + status = access.has_access? ? "GRANTED" : "DENIED" + message = ["#{status} #{access.user.email} access to '#{access.label}'"] + + message << "(#{project_path})" if project_path.present? + message << "- #{access.load_type} #{access.loaded_at}" if access.load_type == :cache + + info(message.join(' ')) + end + + def self.file_name_noext + 'external-policy-access-control' + end + end + end +end diff --git a/lib/gitlab/external_authorization/response.rb b/lib/gitlab/external_authorization/response.rb new file mode 100644 index 00000000000..4f3fe5882db --- /dev/null +++ b/lib/gitlab/external_authorization/response.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +module Gitlab + module ExternalAuthorization + class Response + include ::Gitlab::Utils::StrongMemoize + + def initialize(excon_response) + @excon_response = excon_response + end + + def valid? + @excon_response && [200, 401, 403].include?(@excon_response.status) + end + + def successful? + valid? && @excon_response.status == 200 + end + + def reason + parsed_response['reason'] if parsed_response + end + + private + + def parsed_response + strong_memoize(:parsed_response) { parse_response! } + end + + def parse_response! + JSON.parse(@excon_response.body) + rescue JSON::JSONError + # The JSON response is optional, so don't fail when it's missing + nil + end + end + end +end -- cgit v1.2.1 From c239bfcb1750794ec1bf8172dfa380dea64fe4c1 Mon Sep 17 00:00:00 2001 From: Dylan Griffith Date: Wed, 10 Apr 2019 06:38:27 +0000 Subject: Add more info logging to cluster apps Log events so that it's easy to see when different requests are starting. --- lib/gitlab/kubernetes/namespace.rb | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) (limited to 'lib') diff --git a/lib/gitlab/kubernetes/namespace.rb b/lib/gitlab/kubernetes/namespace.rb index 919f19c86d7..8a3bea95a04 100644 --- a/lib/gitlab/kubernetes/namespace.rb +++ b/lib/gitlab/kubernetes/namespace.rb @@ -19,11 +19,40 @@ module Gitlab def create! resource = ::Kubeclient::Resource.new(metadata: { name: name }) + log_event(:begin_create) @client.create_namespace(resource) end def ensure_exists! exists? || create! + rescue ::Kubeclient::HttpError => error + log_create_failed(error) + raise + end + + private + + def log_create_failed(error) + logger.error({ + exception: error.class.name, + status_code: error.error_code, + namespace: name, + class_name: self.class.name, + event: :failed_to_create_namespace, + message: error.message + }) + end + + def log_event(event) + logger.info( + namespace: name, + class_name: self.class.name, + event: event + ) + end + + def logger + @logger ||= Gitlab::Kubernetes::Logger.build end end end -- cgit v1.2.1