Commit 314062fe authored by Shinya Maeda's avatar Shinya Maeda

Persist source sha and target sha for merge pipelines

source_sha and target_sha are used for merge request pipelines
parent aa77c5bd
......@@ -12,6 +12,10 @@ module Ci
include AtomicInternalId
include EnumWithNil
include HasRef
include ShaAttribute
sha_attribute :source_sha
sha_attribute :target_sha
belongs_to :project, inverse_of: :all_pipelines
belongs_to :user
......@@ -191,6 +195,22 @@ module Ci
.sort_by_merge_request_pipelines
end
scope :triggered_by_merge_request, -> (merge_request) do
where(source: :merge_request, merge_request: merge_request)
end
scope :detached_merge_request_pipelines, -> (merge_request) do
triggered_by_merge_request(merge_request).where(target_sha: nil)
end
scope :merge_request_pipelines, -> (merge_request) do
triggered_by_merge_request(merge_request).where.not(target_sha: nil)
end
scope :mergeable_merge_request_pipelines, -> (merge_request) do
triggered_by_merge_request(merge_request).where(target_sha: merge_request.target_branch_sha)
end
# Returns the pipelines in descending order (= newest first), optionally
# limited to a number of references.
#
......@@ -624,6 +644,8 @@ module Ci
variables.append(key: 'CI_COMMIT_DESCRIPTION', value: git_commit_description.to_s)
if merge_request? && merge_request
variables.append(key: 'CI_MERGE_REQUEST_SOURCE_BRANCH_SHA', value: source_sha.to_s)
variables.append(key: 'CI_MERGE_REQUEST_TARGET_BRANCH_SHA', value: target_sha.to_s)
variables.concat(merge_request.predefined_variables)
end
end
......@@ -708,6 +730,22 @@ module Ci
ref == project.default_branch
end
def triggered_by_merge_request?
merge_request? && merge_request_id.present?
end
def detached_merge_request_pipeline?
triggered_by_merge_request? && target_sha.nil?
end
def merge_request_pipeline?
triggered_by_merge_request? && target_sha.present?
end
def mergeable_merge_request_pipeline?
triggered_by_merge_request? && target_sha == merge_request.target_branch_sha
end
private
def ci_yaml_from_repo
......
......@@ -16,6 +16,8 @@ module ShaAttribute
# the column is the correct type. In production it should behave like any other attribute.
# See https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/5502 for more discussion
def validate_binary_column_exists!(name)
return unless database_exists?
unless table_exists?
warn "WARNING: sha_attribute #{name.inspect} is invalid since the table doesn't exist - you may need to run database migrations"
return
......@@ -35,5 +37,13 @@ module ShaAttribute
Gitlab::AppLogger.error "ShaAttribute initialization: #{error.message}"
raise
end
def database_exists?
ActiveRecord::Base.connection
true
rescue
false
end
end
end
......@@ -25,7 +25,9 @@ module Ci
origin_ref: params[:ref],
checkout_sha: params[:checkout_sha],
after_sha: params[:after],
before_sha: params[:before],
before_sha: params[:before], # The base SHA of the source branch (i.e merge_request.diff_base_sha).
source_sha: params[:source_sha], # The HEAD SHA of the source branch (i.e merge_request.diff_head_sha).
target_sha: params[:target_sha], # The HEAD SHA of the target branch.
trigger_request: trigger_request,
schedule: schedule,
merge_request: merge_request,
......
---
title: Persist source sha and target sha for merge pipelines
merge_request: 25417
author:
type: added
# frozen_string_literal: true
class AddExtraShasToCiPipelines < ActiveRecord::Migration[5.0]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
def change
add_column :ci_pipelines, :source_sha, :binary
add_column :ci_pipelines, :target_sha, :binary
end
end
......@@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema.define(version: 20190204115450) do
ActiveRecord::Schema.define(version: 20190220150130) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
......@@ -497,6 +497,8 @@ ActiveRecord::Schema.define(version: 20190204115450) do
t.integer "failure_reason"
t.integer "iid"
t.integer "merge_request_id"
t.binary "source_sha"
t.binary "target_sha"
t.index ["auto_canceled_by_id"], name: "index_ci_pipelines_on_auto_canceled_by_id", using: :btree
t.index ["merge_request_id"], name: "index_ci_pipelines_on_merge_request_id", where: "(merge_request_id IS NOT NULL)", using: :btree
t.index ["pipeline_schedule_id"], name: "index_ci_pipelines_on_pipeline_schedule_id", using: :btree
......
......@@ -86,10 +86,12 @@ future GitLab releases.**
| **CI_MERGE_REQUEST_PROJECT_URL** | 11.6 | all | The URL of the project of the merge request if it's [pipelines for merge requests](../merge_request_pipelines/index.md) (e.g. `http://192.168.10.15:3000/namespace/awesome-project`) |
| **CI_MERGE_REQUEST_REF_PATH** | 11.6 | all | The ref path of the merge request if it's [pipelines for merge requests](../merge_request_pipelines/index.md). (e.g. `refs/merge-requests/1/head`) |
| **CI_MERGE_REQUEST_SOURCE_BRANCH_NAME** | 11.6 | all | The source branch name of the merge request if it's [pipelines for merge requests](../merge_request_pipelines/index.md) |
| **CI_MERGE_REQUEST_SOURCE_BRANCH_SHA** | 11.9 | all | The HEAD sha of the source branch of the merge request if it's [pipelines for merge requests](../merge_request_pipelines/index.md) |
| **CI_MERGE_REQUEST_SOURCE_PROJECT_ID** | 11.6 | all | The ID of the source project of the merge request if it's [pipelines for merge requests](../merge_request_pipelines/index.md) |
| **CI_MERGE_REQUEST_SOURCE_PROJECT_PATH** | 11.6 | all | The path of the source project of the merge request if it's [pipelines for merge requests](../merge_request_pipelines/index.md) |
| **CI_MERGE_REQUEST_SOURCE_PROJECT_URL** | 11.6 | all | The URL of the source project of the merge request if it's [pipelines for merge requests](../merge_request_pipelines/index.md) |
| **CI_MERGE_REQUEST_TARGET_BRANCH_NAME** | 11.6 | all | The target branch name of the merge request if it's [pipelines for merge requests](../merge_request_pipelines/index.md) |
| **CI_MERGE_REQUEST_TARGET_BRANCH_SHA** | 11.9 | all | The HEAD sha of the target branch of the merge request if it's [pipelines for merge requests](../merge_request_pipelines/index.md) |
| **CI_NODE_INDEX** | 11.5 | all | Index of the job in the job set. If the job is not parallelized, this variable is not set. |
| **CI_NODE_TOTAL** | 11.5 | all | Total number of instances of this job running in parallel. If the job is not parallelized, this variable is set to `1`. |
| **CI_API_V4_URL** | 11.7 | all | The GitLab API v4 root URL |
......
......@@ -12,6 +12,8 @@ module Gitlab
ref: @command.ref,
sha: @command.sha,
before_sha: @command.before_sha,
source_sha: @command.source_sha,
target_sha: @command.target_sha,
tag: @command.tag_exists?,
trigger_requests: Array(@command.trigger_request),
user: @command.current_user,
......
......@@ -7,7 +7,7 @@ module Gitlab
module Chain
Command = Struct.new(
:source, :project, :current_user,
:origin_ref, :checkout_sha, :after_sha, :before_sha,
:origin_ref, :checkout_sha, :after_sha, :before_sha, :source_sha, :target_sha,
:trigger_request, :schedule, :merge_request,
:ignore_skip_ci, :save_incompleted,
:seeds_block, :variables_attributes, :push_options,
......
......@@ -101,6 +101,8 @@ describe Gitlab::Ci::Pipeline::Chain::Build do
checkout_sha: project.commit.id,
after_sha: nil,
before_sha: nil,
source_sha: merge_request.diff_head_sha,
target_sha: merge_request.target_branch_sha,
trigger_request: nil,
schedule: nil,
merge_request: merge_request,
......@@ -118,5 +120,10 @@ describe Gitlab::Ci::Pipeline::Chain::Build do
expect(pipeline).to be_merge_request
expect(pipeline.merge_request).to eq(merge_request)
end
it 'correctly sets souce sha and target sha to pipeline' do
expect(pipeline.source_sha).to eq(merge_request.diff_head_sha)
expect(pipeline.target_sha).to eq(merge_request.target_branch_sha)
end
end
end
......@@ -161,6 +161,54 @@ describe Gitlab::Ci::Pipeline::Chain::Command do
end
end
describe '#source_sha' do
subject { command.source_sha }
let(:command) do
described_class.new(project: project,
source_sha: source_sha,
merge_request: merge_request)
end
let(:merge_request) do
create(:merge_request, target_project: project, source_project: project)
end
let(:source_sha) { nil }
context 'when source_sha is specified' do
let(:source_sha) { 'abc' }
it 'returns the specified value' do
is_expected.to eq('abc')
end
end
end
describe '#target_sha' do
subject { command.target_sha }
let(:command) do
described_class.new(project: project,
target_sha: target_sha,
merge_request: merge_request)
end
let(:merge_request) do
create(:merge_request, target_project: project, source_project: project)
end
let(:target_sha) { nil }
context 'when target_sha is specified' do
let(:target_sha) { 'abc' }
it 'returns the specified value' do
is_expected.to eq('abc')
end
end
end
describe '#protected_ref?' do
let(:command) { described_class.new(project: project, origin_ref: 'my-branch') }
......
......@@ -235,6 +235,8 @@ Ci::Pipeline:
- ref
- sha
- before_sha
- source_sha
- target_sha
- push_data
- created_at
- updated_at
......
......@@ -130,6 +130,132 @@ describe Ci::Pipeline, :mailer do
end
end
describe '.detached_merge_request_pipelines' do
subject { described_class.detached_merge_request_pipelines(merge_request) }
let!(:pipeline) do
create(:ci_pipeline, source: :merge_request, merge_request: merge_request, target_sha: target_sha)
end
let(:merge_request) { create(:merge_request) }
let(:target_sha) { nil }
it 'returns detached merge request pipelines' do
is_expected.to eq([pipeline])
end
context 'when target sha exists' do
let(:target_sha) { merge_request.target_branch_sha }
it 'returns empty array' do
is_expected.to be_empty
end
end
end
describe '#detached_merge_request_pipeline?' do
subject { pipeline.detached_merge_request_pipeline? }
let!(:pipeline) do
create(:ci_pipeline, source: :merge_request, merge_request: merge_request, target_sha: target_sha)
end
let(:merge_request) { create(:merge_request) }
let(:target_sha) { nil }
it { is_expected.to be_truthy }
context 'when target sha exists' do
let(:target_sha) { merge_request.target_branch_sha }
it { is_expected.to be_falsy }
end
end
describe '.merge_request_pipelines' do
subject { described_class.merge_request_pipelines(merge_request) }
let!(:pipeline) do
create(:ci_pipeline, source: :merge_request, merge_request: merge_request, target_sha: target_sha)
end
let(:merge_request) { create(:merge_request) }
let(:target_sha) { merge_request.target_branch_sha }
it 'returns merge pipelines' do
is_expected.to eq([pipeline])
end
context 'when target sha is empty' do
let(:target_sha) { nil }
it 'returns empty array' do
is_expected.to be_empty
end
end
end
describe '#merge_request_pipeline?' do
subject { pipeline.merge_request_pipeline? }
let!(:pipeline) do
create(:ci_pipeline, source: :merge_request, merge_request: merge_request, target_sha: target_sha)
end
let(:merge_request) { create(:merge_request) }
let(:target_sha) { merge_request.target_branch_sha }
it { is_expected.to be_truthy }
context 'when target sha is empty' do
let(:target_sha) { nil }
it { is_expected.to be_falsy }
end
end
describe '.mergeable_merge_request_pipelines' do
subject { described_class.mergeable_merge_request_pipelines(merge_request) }
let!(:pipeline) do
create(:ci_pipeline, source: :merge_request, merge_request: merge_request, target_sha: target_sha)
end
let(:merge_request) { create(:merge_request) }
let(:target_sha) { merge_request.target_branch_sha }
it 'returns mergeable merge pipelines' do
is_expected.to eq([pipeline])
end
context 'when target sha does not point the head of the target branch' do
let(:target_sha) { merge_request.diff_head_sha }
it 'returns empty array' do
is_expected.to be_empty
end
end
end
describe '#mergeable_merge_request_pipeline?' do
subject { pipeline.mergeable_merge_request_pipeline? }
let!(:pipeline) do
create(:ci_pipeline, source: :merge_request, merge_request: merge_request, target_sha: target_sha)
end
let(:merge_request) { create(:merge_request) }
let(:target_sha) { merge_request.target_branch_sha }
it { is_expected.to be_truthy }
context 'when target sha does not point the head of the target branch' do
let(:target_sha) { merge_request.diff_head_sha }
it { is_expected.to be_falsy }
end
end
describe '.merge_request' do
subject { described_class.merge_request }
......@@ -400,10 +526,12 @@ describe Ci::Pipeline, :mailer do
'CI_MERGE_REQUEST_PROJECT_PATH' => merge_request.project.full_path,
'CI_MERGE_REQUEST_PROJECT_URL' => merge_request.project.web_url,
'CI_MERGE_REQUEST_TARGET_BRANCH_NAME' => merge_request.target_branch.to_s,
'CI_MERGE_REQUEST_TARGET_BRANCH_SHA' => pipeline.target_sha.to_s,
'CI_MERGE_REQUEST_SOURCE_PROJECT_ID' => merge_request.source_project.id.to_s,
'CI_MERGE_REQUEST_SOURCE_PROJECT_PATH' => merge_request.source_project.full_path,
'CI_MERGE_REQUEST_SOURCE_PROJECT_URL' => merge_request.source_project.web_url,
'CI_MERGE_REQUEST_SOURCE_BRANCH_NAME' => merge_request.source_branch.to_s)
'CI_MERGE_REQUEST_SOURCE_BRANCH_NAME' => merge_request.source_branch.to_s,
'CI_MERGE_REQUEST_SOURCE_BRANCH_SHA' => pipeline.source_sha.to_s)
end
context 'when source project does not exist' do
......
......@@ -12,6 +12,7 @@ describe Ci::CreatePipelineService do
end
describe '#execute' do
# rubocop:disable Metrics/ParameterLists
def execute_service(
source: :push,
after: project.commit.id,
......@@ -20,17 +21,22 @@ describe Ci::CreatePipelineService do
trigger_request: nil,
variables_attributes: nil,
merge_request: nil,
push_options: nil)
push_options: nil,
source_sha: nil,
target_sha: nil)
params = { ref: ref,
before: '00000000',
after: after,
commits: [{ message: message }],
variables_attributes: variables_attributes,
push_options: push_options }
push_options: push_options,
source_sha: source_sha,
target_sha: target_sha }
described_class.new(project, user, params).execute(
source, trigger_request: trigger_request, merge_request: merge_request)
end
# rubocop:enable Metrics/ParameterLists
context 'valid params' do
let(:pipeline) { execute_service }
......@@ -679,7 +685,11 @@ describe Ci::CreatePipelineService do
describe 'Merge request pipelines' do
let(:pipeline) do
execute_service(source: source, merge_request: merge_request, ref: ref_name)
execute_service(source: source,
merge_request: merge_request,
ref: ref_name,
source_sha: source_sha,
target_sha: target_sha)
end
before do
......@@ -687,6 +697,8 @@ describe Ci::CreatePipelineService do
end
let(:ref_name) { 'refs/heads/feature' }
let(:source_sha) { project.commit(ref_name).id }
let(:target_sha) { nil }
context 'when source is merge request' do
let(:source) { :merge_request }
......@@ -727,6 +739,22 @@ describe Ci::CreatePipelineService do
expect(pipeline.builds.order(:stage_id).map(&:name)).to eq(%w[test])
end
it 'persists the specified source sha' do
expect(pipeline.source_sha).to eq(source_sha)
end
it 'does not persist target sha for detached merge request pipeline' do
expect(pipeline.target_sha).to be_nil
end
context 'when target sha is specified' do
let(:target_sha) { merge_request.target_branch_sha }
it 'persists the target sha' do
expect(pipeline.target_sha).to eq(target_sha)
end
end
context 'when ref is tag' do
let(:ref_name) { 'refs/tags/v1.1.0' }
......
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