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/internal.rb | 36 +++++++++++++++++++++++++++++------- 1 file changed, 29 insertions(+), 7 deletions(-) (limited to 'lib/api/internal.rb') 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 -- 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 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib/api/internal.rb') 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 -- 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/internal.rb | 20 ++------------------ 1 file changed, 2 insertions(+), 18 deletions(-) (limited to 'lib/api/internal.rb') 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/api/internal.rb') 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