Commit f65eda80 authored by Aufar Gilbran's avatar Aufar Gilbran

Make commit status created for any pipelines by default

Resolves https://gitlab.com/gitlab-org/gitlab-ee/issues/14064.

On commit status creation, the controller tries to find pipeline that
matches the specified parameters. If 'ref' not is given as parameters,
'ref' will be assigned to branch name that contains 'sha' and pipeline
for 'ref' will be searched. If there is an existing pipeline for 'ref',
commit status stage is created and appended at the end of the pipeline.
Otherwise, it will create a new pipeline for 'ref'.

The above behavior is not desirable when the only pipelines available
are not for branch. One example of other 'ref' is merge request ref.
For these pipelines, the expected value of 'ref' is supposed to be the
merge request ref name. For example, merge request with ID 42 will have
/refs/merge-requests/42/head as its ref name. Thus, if there are only
pipelines for merge requests, unless we pass 'ref' explicitly, it will
create a new pipeline for the branch that contains the 'sha' instead of
adding the created stage to the existing pipeline for merge requests.

This commit change the logic so commit status can be created on any
pipelines by default. The goal can be achieved by changing the logic
when existing pipeline is searched using the given parameters. First,
if 'pipeline_id' is specified, search for pipeline that has the same id
as 'pipeline_id'. When 'pipeline_id' is not given, but 'ref' is given,
search for pipeline that has the same 'ref' and 'sha' value. Note that
'sha' will always be given. Otherwise, search the latest pipeline that
has the same 'sha', which will only returns nil if there is no pipeline
created for that particular 'sha'.
parent 753fa91a
......@@ -211,6 +211,8 @@ module Ci
scope :for_sha, -> (sha) { where(sha: sha) }
scope :for_source_sha, -> (source_sha) { where(source_sha: source_sha) }
scope :for_sha_or_source_sha, -> (sha) { for_sha(sha).or(for_source_sha(sha)) }
scope :for_ref, -> (ref) { where(ref: ref) }
scope :for_id, -> (id) { where(id: id) }
scope :created_after, -> (time) { where('ci_pipelines.created_at > ?', time) }
scope :triggered_by_merge_request, -> (merge_request) do
......
---
title: Make commit status created for any pipelines
merge_request: 17524
author: Aufar Gilbran
type: changed
......@@ -58,7 +58,6 @@ module API
post ':id/statuses/:sha' do
authorize! :create_commit_status, user_project
commit = @project.commit(params[:sha])
not_found! 'Commit' unless commit
# Since the CommitStatus is attached to Ci::Pipeline (in the future Pipeline)
......@@ -68,14 +67,15 @@ module API
# If we don't receive it, we will attach the CommitStatus to
# the first found branch on that commit
pipeline = all_matching_pipelines.first
ref = params[:ref]
ref ||= pipeline&.ref
ref ||= @project.repository.branch_names_contains(commit.sha).first
not_found! 'References for commit' unless ref
name = params[:name] || params[:context] || 'default'
pipeline = @project.pipeline_for(ref, commit.sha, params[:pipeline_id])
unless pipeline
pipeline = @project.ci_pipelines.create!(
source: :external,
......@@ -126,6 +126,20 @@ module API
end
end
# rubocop: enable CodeReuse/ActiveRecord
helpers do
def commit
strong_memoize(:commit) do
user_project.commit(params[:sha])
end
end
def all_matching_pipelines
pipelines = user_project.ci_pipelines.newest_first(sha: commit.sha)
pipelines = pipelines.for_ref(params[:ref]) if params[:ref]
pipelines = pipelines.for_id(params[:pipeline_id]) if params[:pipeline_id]
pipelines
end
end
end
end
end
......@@ -125,25 +125,55 @@ describe API::CommitStatuses do
let(:post_url) { "/projects/#{project.id}/statuses/#{sha}" }
context 'developer user' do
%w[pending running success failed canceled].each do |status|
context "for #{status}" do
context 'uses only required parameters' do
it 'creates commit status' do
post api(post_url, developer), params: { state: status }
context 'uses only required parameters' do
%w[pending running success failed canceled].each do |status|
context "for #{status}" do
context 'when pipeline for sha does not exists' do
it 'creates commit status' do
post api(post_url, developer), params: { state: status }
expect(response).to have_gitlab_http_status(201)
expect(json_response['sha']).to eq(commit.id)
expect(json_response['status']).to eq(status)
expect(json_response['name']).to eq('default')
expect(json_response['ref']).not_to be_empty
expect(json_response['target_url']).to be_nil
expect(json_response['description']).to be_nil
if status == 'failed'
expect(CommitStatus.find(json_response['id'])).to be_api_failure
end
end
end
end
end
context 'when pipeline already exists for the specified sha' do
let!(:pipeline) { create(:ci_pipeline, project: project, sha: sha, ref: 'ref') }
let(:params) { { state: 'pending' } }
shared_examples_for 'creates a commit status for the existing pipeline' do
it do
expect do
post api(post_url, developer), params: params
end.not_to change { Ci::Pipeline.count }
job = pipeline.statuses.find_by_name(json_response['name'])
expect(response).to have_gitlab_http_status(201)
expect(json_response['sha']).to eq(commit.id)
expect(json_response['status']).to eq(status)
expect(json_response['name']).to eq('default')
expect(json_response['ref']).not_to be_empty
expect(json_response['target_url']).to be_nil
expect(json_response['description']).to be_nil
if status == 'failed'
expect(CommitStatus.find(json_response['id'])).to be_api_failure
end
expect(job.status).to eq('pending')
end
end
it_behaves_like 'creates a commit status for the existing pipeline'
context 'with pipeline for merge request' do
let!(:merge_request) { create(:merge_request, :with_detached_merge_request_pipeline, source_project: project) }
let!(:pipeline) { merge_request.all_pipelines.last }
let(:sha) { pipeline.sha }
it_behaves_like 'creates a commit status for the existing pipeline'
end
end
end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment