Merge branch 'support-only-changes-on-mr-pipelines' into 'master'

Support `only: changes:` on MR pipelines

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

6 files changed, 112 insertions, 10 deletions

diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index acef5d2e643..372f6d678f6 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -687,9 +687,18 @@ module Ci
       end
     end
 
+    # Returns the modified paths.
+    #
+    # The returned value is
+    # * Array: List of modified paths that should be evaluated
+    # * nil: Modified path can not be evaluated
     def modified_paths
       strong_memoize(:modified_paths) do
-        push_details.modified_paths
+        if merge_request?
+          merge_request.modified_paths
+        elsif branch_updated?
+          push_details.modified_paths
+        end
       end
     end
 
diff --git a/changelogs/unreleased/support-only-changes-on-mr-pipelines.yml b/changelogs/unreleased/support-only-changes-on-mr-pipelines.yml
new file mode 100644
index 00000000000..fbab898b799
--- /dev/null
+++ b/changelogs/unreleased/support-only-changes-on-mr-pipelines.yml
@@ -0,0 +1,5 @@
+---
+title: 'Support `only: changes:` on MR pipelines'
+merge_request: 24490
+author: Hiroyuki Sato
+type: added
diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md
index 984878b6c9b..8c8a31e9323 100644
--- a/doc/ci/yaml/README.md
+++ b/doc/ci/yaml/README.md
@@ -423,10 +423,28 @@ connected with merge requests yet, and because GitLab is creating pipelines
 before an user can create a merge request we don't know a target branch at
 this point.
 
-Without a target branch, it is not possible to know what the common ancestor is,
-thus we always create a job in that case. This feature works best for stable
-branches like `master` because in that case GitLab uses the previous commit
-that is present in a branch to compare against the latest SHA that was pushed.
+#### Using `changes` with `merge_requests`
+
+With [pipelines for merge requests](../merge_request_pipelines/index.md),
+make it possible to define if a job should be created base on files modified
+in a merge request.
+
+For example:
+
+```
+docker build service one:
+  script: docker build -t my-service-one-image:$CI_COMMIT_REF_SLUG .
+  only:
+    refs:
+      - merge_requests
+    changes:
+      - Dockerfile
+      - service-one/**/*
+```
+
+In the scenario above, if you create or update a merge request that changes
+either files in `service-one` folder or `Dockerfile`, GitLab creates and triggers
+the `docker build service one` job.
 
 ## `tags`
 
diff --git a/lib/gitlab/ci/build/policy/changes.rb b/lib/gitlab/ci/build/policy/changes.rb
index 1663c875426..9c705a1cd3e 100644
--- a/lib/gitlab/ci/build/policy/changes.rb
+++ b/lib/gitlab/ci/build/policy/changes.rb
@@ -10,7 +10,7 @@ module Gitlab
           end
 
           def satisfied_by?(pipeline, seed)
-            return true unless pipeline.branch_updated?
+            return true if pipeline.modified_paths.nil?
 
             pipeline.modified_paths.any? do |path|
               @globs.any? do |glob|
diff --git a/spec/lib/gitlab/ci/build/policy/changes_spec.rb b/spec/lib/gitlab/ci/build/policy/changes_spec.rb
index 5fee37bb43e..dc3329061d1 100644
--- a/spec/lib/gitlab/ci/build/policy/changes_spec.rb
+++ b/spec/lib/gitlab/ci/build/policy/changes_spec.rb
@@ -73,9 +73,9 @@ describe Gitlab::Ci::Build::Policy::Changes do
         expect(policy).not_to be_satisfied_by(pipeline, seed)
       end
 
-      context 'when pipelines does not run for a branch update' do
+      context 'when modified paths can not be evaluated' do
         before do
-          pipeline.before_sha = Gitlab::Git::BLANK_SHA
+          allow(pipeline).to receive(:modified_paths) { nil }
         end
 
         it 'is always satisfied' do
@@ -115,5 +115,57 @@ describe Gitlab::Ci::Build::Policy::Changes do
         expect(policy).not_to be_satisfied_by(pipeline, seed)
       end
     end
+
+    context 'when branch is created' do
+      let(:pipeline) do
+        create(:ci_empty_pipeline, project: project,
+                                   ref: 'feature',
+                                   source: source,
+                                   sha: '0b4bc9a4',
+                                   before_sha: Gitlab::Git::BLANK_SHA,
+                                   merge_request: merge_request)
+      end
+
+      let(:ci_build) do
+        build(:ci_build, pipeline: pipeline, project: project, ref: 'feature')
+      end
+
+      let(:seed) { double('build seed', to_resource: ci_build) }
+
+      context 'when source is merge request' do
+        let(:source) { :merge_request }
+
+        let(:merge_request) do
+          create(:merge_request,
+                 source_project: project,
+                 source_branch: 'feature',
+                 target_project: project,
+                 target_branch: 'master')
+        end
+
+        it 'is satified by changes in the merge request' do
+          policy = described_class.new(%w[files/ruby/feature.rb])
+
+          expect(policy).to be_satisfied_by(pipeline, seed)
+        end
+
+        it 'is not satified by changes not in the merge request' do
+          policy = described_class.new(%w[foo.rb])
+
+          expect(policy).not_to be_satisfied_by(pipeline, seed)
+        end
+      end
+
+      context 'when source is push' do
+        let(:source) { :push }
+        let(:merge_request) { nil }
+
+        it 'is always satified' do
+          policy = described_class.new(%w[foo.rb])
+
+          expect(policy).to be_satisfied_by(pipeline, seed)
+        end
+      end
+    end
   end
 end
diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index 72a0df96a80..460b5c8cd31 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -1172,8 +1172,26 @@ describe Ci::Pipeline, :mailer do
         pipeline.update_column(:before_sha, Gitlab::Git::BLANK_SHA)
       end
 
-      it 'raises an error' do
-        expect { pipeline.modified_paths }.to raise_error(ArgumentError)
+      it 'returns nil' do
+        expect(pipeline.modified_paths).to be_nil
+      end
+    end
+
+    context 'when source is merge request' do
+      let(:pipeline) do
+        create(:ci_pipeline, source: :merge_request, merge_request: merge_request)
+      end
+
+      let(:merge_request) do
+        create(:merge_request,
+               source_project: project,
+               source_branch: 'feature',
+               target_project: project,
+               target_branch: 'master')
+      end
+
+      it 'returns merge request modified paths' do
+        expect(pipeline.modified_paths).to match(merge_request.modified_paths)
       end
     end
   end