summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKerri Miller <kerrizor@kerrizor.com>2019-05-07 16:06:53 +0000
committerJames Lopez <james@gitlab.com>2019-05-07 16:06:53 +0000
commit29f9d92642539eaa04bdb485425d6f1cd3b459fd (patch)
treec23954af03947e8af936e2c6428340c28dbf3aab
parentf0ff33d8bf3c0fd2dae4f6c762f161b127aa88d7 (diff)
downloadgitlab-ce-29f9d92642539eaa04bdb485425d6f1cd3b459fd.tar.gz
Confirm existence of head_pipeline if pipeline success required
Pipelines are created by an async worker, so a rapid sequence of API calls can trigger a state where the pipeline, whose existence is part of determining if we wait for the pipeline to successfully complete before merging, can trigger the MR to be immediately merged instead. Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/55127
-rw-r--r--changelogs/unreleased/55127-add-delay-after-mr-creation-for-async-tasks-to-complete.yml5
-rw-r--r--lib/api/merge_requests.rb4
-rw-r--r--spec/requests/api/merge_requests_spec.rb27
3 files changed, 36 insertions, 0 deletions
diff --git a/changelogs/unreleased/55127-add-delay-after-mr-creation-for-async-tasks-to-complete.yml b/changelogs/unreleased/55127-add-delay-after-mr-creation-for-async-tasks-to-complete.yml
new file mode 100644
index 00000000000..ac3bb596842
--- /dev/null
+++ b/changelogs/unreleased/55127-add-delay-after-mr-creation-for-async-tasks-to-complete.yml
@@ -0,0 +1,5 @@
+---
+title: Wait for pipeline creation to complete before accepting a MR via API
+merge_request: 27978
+author: kerrizor
+type: fixed
diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb
index ce85772e4ed..daa98c22e5e 100644
--- a/lib/api/merge_requests.rb
+++ b/lib/api/merge_requests.rb
@@ -367,6 +367,10 @@ module API
merge_request = find_project_merge_request(params[:merge_request_iid])
merge_when_pipeline_succeeds = to_boolean(params[:merge_when_pipeline_succeeds])
+ if merge_when_pipeline_succeeds || merge_request.merge_when_pipeline_succeeds
+ render_api_error!('Not allowed: pipeline does not exist', 405) unless merge_request.head_pipeline
+ end
+
# Merge request can not be merged
# because user dont have permissions to push into target branch
unauthorized! unless merge_request.can_be_merged_by?(current_user)
diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb
index 5c94a87529b..007f3517e64 100644
--- a/spec/requests/api/merge_requests_spec.rb
+++ b/spec/requests/api/merge_requests_spec.rb
@@ -1495,6 +1495,33 @@ describe API::MergeRequests do
expect(json_response['merge_when_pipeline_succeeds']).to eq(true)
end
+ context 'when the MR requires pipeline success' do
+ it 'returns 405 if the pipeline is missing' do
+ allow_any_instance_of(MergeRequest)
+ .to receive(:merge_when_pipeline_succeeds).and_return(true)
+ allow_any_instance_of(MergeRequest).to receive(:head_pipeline).and_return(nil)
+
+ put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user)
+
+ expect(response).to have_gitlab_http_status(405)
+ expect(json_response['message']).to eq('Not allowed: pipeline does not exist')
+ end
+ end
+
+ context 'when the request requires pipeline success' do
+ it 'returns 405 if the pipeline is missing' do
+ allow_any_instance_of(MergeRequest)
+ .to receive(:merge_when_pipeline_succeeds).and_return(true)
+ allow_any_instance_of(MergeRequest).to receive(:head_pipeline).and_return(nil)
+
+ put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user),
+ params: { merge_when_pipeline_succeeds: true }
+
+ expect(response).to have_gitlab_http_status(405)
+ expect(json_response['message']).to eq('Not allowed: pipeline does not exist')
+ end
+ end
+
it "returns 404 for an invalid merge request IID" do
put api("/projects/#{project.id}/merge_requests/12345/merge", user)