Commit fe673b49 authored by Jarka Kadlecova's avatar Jarka Kadlecova Committed by Felipe Artur
Browse files

Ensure pippeline corresponds with the sha of an MR

parent 003a816a
......@@ -145,6 +145,14 @@ def self.reference_prefix
'!'
end
def head_pipeline
return unless head_pipeline_id
last_pipeline = Ci::Pipeline.find(head_pipeline_id)
last_pipeline.sha == diff_head_sha ? last_pipeline : nil
end
# Pattern used to extract `!123` merge request references from text
#
# This pattern supports cross-project references.
......
......@@ -29,7 +29,7 @@ def execute(source, ignore_skip_ci: false, save_on_errors: true, trigger_request
.new(pipeline, command, SEQUENCE)
sequence.build! do |pipeline, sequence|
update_merge_requests_head_pipeline if pipeline.persisted?
schedule_head_pipeline_update
if sequence.complete?
cancel_pending_pipelines if project.auto_cancel_pending_pipelines?
......@@ -38,15 +38,18 @@ def execute(source, ignore_skip_ci: false, save_on_errors: true, trigger_request
pipeline.process!
end
end
pipeline
end
private
def update_merge_requests_head_pipeline
return unless pipeline.latest?
def commit
@commit ||= project.commit(origin_sha || origin_ref)
end
MergeRequest.where(source_project: @pipeline.project, source_branch: @pipeline.ref)
.update_all(head_pipeline_id: @pipeline.id)
def sha
commit.try(:id)
end
def cancel_pending_pipelines
......@@ -69,5 +72,15 @@ def pipeline_created_counter
@pipeline_created_counter ||= Gitlab::Metrics
.counter(:pipelines_created_total, "Counter of pipelines created")
end
def schedule_head_pipeline_update
related_merge_requests.each do |merge_request|
UpdateHeadPipelineForMergeRequestWorker.perform_async(merge_request.id)
end
end
def related_merge_requests
MergeRequest.where(source_project: pipeline.project, source_branch: pipeline.ref)
end
end
end
......@@ -76,6 +76,7 @@ def reload_merge_requests
end
merge_request.mark_as_unchecked
UpdateHeadPipelineForMergeRequestWorker.perform_async(merge_request.id)
end
end
......
class UpdateHeadPipelineForMergeRequestWorker
include Sidekiq::Worker
sidekiq_options queue: 'pipeline_default'
def perform(merge_request_id)
merge_request = MergeRequest.find(merge_request_id)
pipeline = Ci::Pipeline.where(project: merge_request.source_project, ref: merge_request.source_branch).last
return unless pipeline && pipeline.latest?
raise ArgumentError, 'merge request sha does not equal pipeline sha' if merge_request.diff_head_sha != pipeline.sha
merge_request.update_attribute(:head_pipeline_id, pipeline.id)
end
end
---
title: Make sure head pippeline always corresponds with the head sha of an MR
merge_request:
author:
type: fixed
......@@ -20,10 +20,14 @@
end
before do
visit project_merge_request_path(project, merge_request)
merge_request.update_attribute(:head_pipeline_id, pipeline.id)
end
scenario 'user visits merge request pipelines tab' do
visit project_merge_request_path(project, merge_request)
expect(page.find('.ci-widget')).to have_content('pending')
page.within('.merge-request-tabs') do
click_link('Pipelines')
end
......@@ -31,6 +35,15 @@
expect(page).to have_selector('.stage-cell')
end
scenario 'pipeline sha does not equal last commit sha' do
pipeline.update_attribute(:sha, '19e2e9b4ef76b422ce1154af39a91323ccc57434')
visit project_merge_request_path(project, merge_request)
wait_for_requests
expect(page.find('.ci-widget')).to have_content(
'Could not connect to the CI server. Please check your settings and try again')
end
end
context 'without pipelines' do
......
......@@ -828,20 +828,28 @@ def set_compare(merge_request)
end
describe '#head_pipeline' do
describe 'when the source project exists' do
it 'returns the latest pipeline' do
pipeline = create(:ci_empty_pipeline, project: subject.source_project, ref: 'master', status: 'running', sha: "123abc", head_pipeline_of: subject)
before do
allow(subject).to receive(:diff_head_sha).and_return('lastsha')
end
expect(subject.head_pipeline).to eq(pipeline)
end
it 'returns nil for MR without head_pipeline_id' do
subject.update_attribute(:head_pipeline_id, nil)
expect(subject.head_pipeline).to be_nil
end
describe 'when the source project does not exist' do
it 'returns nil' do
allow(subject).to receive(:source_project).and_return(nil)
it 'returns nil for MR with old pipeline' do
pipeline = create(:ci_empty_pipeline, sha: 'notlatestsha')
subject.update_attribute(:head_pipeline_id, pipeline.id)
expect(subject.head_pipeline).to be_nil
end
expect(subject.head_pipeline).to be_nil
end
it 'returns the pipeline for MR with recent pipeline' do
pipeline = create(:ci_empty_pipeline, sha: 'lastsha')
subject.update_attribute(:head_pipeline_id, pipeline.id)
expect(subject.head_pipeline).to eq(pipeline)
end
end
......
......@@ -57,19 +57,39 @@ def execute_service(
end
context 'when merge requests already exist for this source branch' do
it 'updates head pipeline of each merge request' do
merge_request_1 = create(:merge_request, source_branch: 'master',
target_branch: "branch_1",
source_project: project)
let(:merge_request_1) do
create(:merge_request, source_branch: 'master', target_branch: "branch_1", source_project: project)
end
let(:merge_request_2) do
create(:merge_request, source_branch: 'master', target_branch: "branch_2", source_project: project)
end
merge_request_2 = create(:merge_request, source_branch: 'master',
target_branch: "branch_2",
source_project: project)
context 'when the head pipeline sha equals merge request sha' do
it 'updates head pipeline of each merge request' do
merge_request_1
merge_request_2
head_pipeline = execute_service
head_pipeline = execute_service
expect(merge_request_1.reload.head_pipeline).to eq(head_pipeline)
expect(merge_request_2.reload.head_pipeline).to eq(head_pipeline)
expect(merge_request_1.reload.head_pipeline).to eq(head_pipeline)
expect(merge_request_2.reload.head_pipeline).to eq(head_pipeline)
end
end
context 'when the head pipeline sha does not equal merge request sha' do
it 'raises the ArgumentError error from worker and does not update the head piepeline of MRs' do
merge_request_1
merge_request_2
allow_any_instance_of(Ci::Pipeline).to receive(:latest?).and_return(true)
expect { execute_service(after: 'ae73cb07c9eeaf35924a10f713b364d32b2dd34f') }.to raise_error(ArgumentError)
last_pipeline = Ci::Pipeline.last
expect(merge_request_1.reload.head_pipeline).not_to eq(last_pipeline)
expect(merge_request_2.reload.head_pipeline).not_to eq(last_pipeline)
end
end
context 'when there is no pipeline for source branch' do
......@@ -106,8 +126,7 @@ def execute_service(
target_branch: "branch_1",
source_project: project)
allow_any_instance_of(Ci::Pipeline)
.to receive(:latest?).and_return(false)
allow_any_instance_of(Ci::Pipeline).to receive(:latest?).and_return(false)
execute_service
......
......@@ -74,6 +74,20 @@
end
end
context 'when pipeline exists for the source branch' do
let!(:pipeline) { create(:ci_empty_pipeline, ref: @merge_request.source_branch, project: @project, sha: @commits.first.sha)}
subject { service.new(@project, @user).execute(@oldrev, @newrev, 'refs/heads/master') }
it 'updates the head_pipeline_id for @merge_request' do
expect { subject }.to change { @merge_request.reload.head_pipeline_id }.from(nil).to(pipeline.id)
end
it 'does not update the head_pipeline_id for @fork_merge_request' do
expect { subject }.not_to change { @fork_merge_request.reload.head_pipeline_id }
end
end
context 'push to origin repo source branch when an MR was reopened' do
let(:refresh_service) { service.new(@project, @user) }
......
require 'spec_helper'
describe UpdateHeadPipelineForMergeRequestWorker do
describe '#perform' do
let(:user) { create(:user) }
let(:project) { create(:project, :repository) }
let(:merge_request) { create(:merge_request, source_project: project) }
let(:latest_sha) { 'b83d6e391c22777fca1ed3012fce84f633d7fed0' }
context 'when pipeline exists for the source project and branch' do
before do
create(:ci_empty_pipeline, project: project, ref: merge_request.source_branch, sha: latest_sha)
end
it 'updates the head_pipeline_id of the merge_request' do
expect { subject.perform(merge_request.id) }.to change { merge_request.reload.head_pipeline_id }
end
context 'when merge request sha does not equal pipeline sha' do
before do
merge_request.merge_request_diff.update(head_commit_sha: 'different_sha')
end
it 'does not update head_pipeline_id' do
expect { subject.perform(merge_request.id) }.to raise_error(ArgumentError)
expect(merge_request.reload.head_pipeline_id).to eq(nil)
end
end
end
context 'when pipeline does not exist for the source project and branch' do
it 'does not update the head_pipeline_id of the merge_request' do
expect { subject.perform(merge_request.id) }.not_to change { merge_request.reload.head_pipeline_id }
end
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