Merge branch 'persist-source-sha-and-target-sha-for-pipelines' into 'master'

Persist source sha and target sha for merge pipelines

See merge request gitlab-org/gitlab-ce!25417

app/models/ci/pipeline.rb

@@ -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

app/models/concerns/sha_attribute.rb

@@ -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

app/services/ci/create_pipeline_service.rb

@@ -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,

changelogs/unreleased/persist-source-sha-and-target-sha-for-pipelines.yml

+---
+title: Persist source sha and target sha for merge pipelines
+merge_request: 25417
+author:
+type: added

db/migrate/20190220150130_add_extra_shas_to_ci_pipelines.rb

+# 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

db/schema.rb

@@ -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

doc/ci/variables/README.md

@@ -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 |

lib/gitlab/ci/pipeline/chain/build.rb

@@ -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,

lib/gitlab/ci/pipeline/chain/command.rb

@@ -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,

spec/lib/gitlab/ci/pipeline/chain/build_spec.rb

@@ -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

spec/lib/gitlab/ci/pipeline/chain/command_spec.rb

@@ -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') }
 

spec/lib/gitlab/import_export/safe_model_attributes.yml

@@ -235,6 +235,8 @@ Ci::Pipeline:
 - ref
 - sha
 - before_sha
+- source_sha
+- target_sha
 - push_data
 - created_at
 - updated_at

spec/models/ci/pipeline_spec.rb

@@ -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

spec/services/ci/create_pipeline_service_spec.rb

@@ -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' }