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
This commit is contained in:
parent
f0ff33d8bf
commit
29f9d92642
|
@ -0,0 +1,5 @@
|
||||||
|
---
|
||||||
|
title: Wait for pipeline creation to complete before accepting a MR via API
|
||||||
|
merge_request: 27978
|
||||||
|
author: kerrizor
|
||||||
|
type: fixed
|
|
@ -367,6 +367,10 @@ module API
|
||||||
merge_request = find_project_merge_request(params[:merge_request_iid])
|
merge_request = find_project_merge_request(params[:merge_request_iid])
|
||||||
merge_when_pipeline_succeeds = to_boolean(params[:merge_when_pipeline_succeeds])
|
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
|
# Merge request can not be merged
|
||||||
# because user dont have permissions to push into target branch
|
# because user dont have permissions to push into target branch
|
||||||
unauthorized! unless merge_request.can_be_merged_by?(current_user)
|
unauthorized! unless merge_request.can_be_merged_by?(current_user)
|
||||||
|
|
|
@ -1495,6 +1495,33 @@ describe API::MergeRequests do
|
||||||
expect(json_response['merge_when_pipeline_succeeds']).to eq(true)
|
expect(json_response['merge_when_pipeline_succeeds']).to eq(true)
|
||||||
end
|
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
|
it "returns 404 for an invalid merge request IID" do
|
||||||
put api("/projects/#{project.id}/merge_requests/12345/merge", user)
|
put api("/projects/#{project.id}/merge_requests/12345/merge", user)
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue