Commit 7bdbacb4 authored by Kamil Trzciński's avatar Kamil Trzciński

Merge branch 'feature/gb/pipeline-only-except-with-modified-paths' into 'master'

Pipeline only/except for modified paths

See merge request gitlab-org/gitlab-ce!21981
parents ae014e18 33cf6171
......@@ -627,6 +627,18 @@ module Ci
end
end
def branch_updated?
strong_memoize(:branch_updated) do
push_details.branch_updated?
end
end
def modified_paths
strong_memoize(:modified_paths) do
push_details.modified_paths
end
end
def default_branch?
ref == project.default_branch
end
......@@ -654,6 +666,22 @@ module Ci
Gitlab::DataBuilder::Pipeline.build(self)
end
def push_details
strong_memoize(:push_details) do
Gitlab::Git::Push.new(project, before_sha, sha, push_ref)
end
end
def push_ref
if branch?
Gitlab::Git::BRANCH_REF_PREFIX + ref.to_s
elsif tag?
Gitlab::Git::TAG_REF_PREFIX + ref.to_s
else
raise ArgumentError, 'Invalid pipeline type!'
end
end
def latest_builds_status
return 'failed' unless yaml_errors.blank?
......
......@@ -57,10 +57,10 @@ module MergeRequests
# Returns all origin and fork merge requests from `@project` satisfying passed arguments.
# rubocop: disable CodeReuse/ActiveRecord
def merge_requests_for(source_branch, mr_states: [:opened])
MergeRequest
@project.source_of_merge_requests
.with_state(mr_states)
.where(source_branch: source_branch, source_project_id: @project.id)
.preload(:source_project) # we don't need a #includes since we're just preloading for the #select
.where(source_branch: source_branch)
.preload(:source_project) # we don't need #includes since we're just preloading for the #select
.select(&:source_project)
end
# rubocop: enable CodeReuse/ActiveRecord
......
......@@ -3,17 +3,16 @@
module MergeRequests
class RefreshService < MergeRequests::BaseService
def execute(oldrev, newrev, ref)
return true unless Gitlab::Git.branch_ref?(ref)
@push = Gitlab::Git::Push.new(@project, oldrev, newrev, ref)
do_execute(oldrev, newrev, ref)
return true unless @push.branch_push?
refresh_merge_requests!
end
private
def do_execute(oldrev, newrev, ref)
@oldrev, @newrev = oldrev, newrev
@branch_name = Gitlab::Git.ref_name(ref)
def refresh_merge_requests!
Gitlab::GitalyClient.allow_n_plus_1_calls(&method(:find_new_commits))
# Be sure to close outstanding MRs before reloading them to avoid generating an
# empty diff during a manual merge
......@@ -25,7 +24,7 @@ module MergeRequests
cache_merge_requests_closing_issues
# Leave a system note if a branch was deleted/added
if branch_added? || branch_removed?
if @push.branch_added? || @push.branch_removed?
comment_mr_branch_presence_changed
end
......@@ -54,8 +53,10 @@ module MergeRequests
# rubocop: disable CodeReuse/ActiveRecord
def post_merge_manually_merged
commit_ids = @commits.map(&:id)
merge_requests = @project.merge_requests.preload(:latest_merge_request_diff).opened.where(target_branch: @branch_name).to_a
merge_requests = merge_requests.select(&:diff_head_commit)
merge_requests = @project.merge_requests.opened
.preload(:latest_merge_request_diff)
.where(target_branch: @push.branch_name).to_a
.select(&:diff_head_commit)
merge_requests = merge_requests.select do |merge_request|
commit_ids.include?(merge_request.diff_head_sha) &&
......@@ -70,24 +71,20 @@ module MergeRequests
end
# rubocop: enable CodeReuse/ActiveRecord
def force_push?
Gitlab::Checks::ForcePush.force_push?(@project, @oldrev, @newrev)
end
# Refresh merge request diff if we push to source or target branch of merge request
# Note: we should update merge requests from forks too
# rubocop: disable CodeReuse/ActiveRecord
def reload_merge_requests
merge_requests = @project.merge_requests.opened
.by_source_or_target_branch(@branch_name).to_a
.by_source_or_target_branch(@push.branch_name).to_a
# Fork merge requests
merge_requests += MergeRequest.opened
.where(source_branch: @branch_name, source_project: @project)
.where(source_branch: @push.branch_name, source_project: @project)
.where.not(target_project: @project).to_a
filter_merge_requests(merge_requests).each do |merge_request|
if merge_request.source_branch == @branch_name || force_push?
if merge_request.source_branch == @push.branch_name || @push.force_push?
merge_request.reload_diff(current_user)
else
mr_commit_ids = merge_request.commit_shas
......@@ -117,7 +114,7 @@ module MergeRequests
end
def find_new_commits
if branch_added?
if @push.branch_added?
@commits = []
merge_request = merge_requests_for_source_branch.first
......@@ -126,28 +123,28 @@ module MergeRequests
begin
# Since any number of commits could have been made to the restored branch,
# find the common root to see what has been added.
common_ref = @project.repository.merge_base(merge_request.diff_head_sha, @newrev)
common_ref = @project.repository.merge_base(merge_request.diff_head_sha, @push.newrev)
# If the a commit no longer exists in this repo, gitlab_git throws
# a Rugged::OdbError. This is fixed in https://gitlab.com/gitlab-org/gitlab_git/merge_requests/52
@commits = @project.repository.commits_between(common_ref, @newrev) if common_ref
@commits = @project.repository.commits_between(common_ref, @push.newrev) if common_ref
rescue
end
elsif branch_removed?
elsif @push.branch_removed?
# No commits for a deleted branch.
@commits = []
else
@commits = @project.repository.commits_between(@oldrev, @newrev)
@commits = @project.repository.commits_between(@push.oldrev, @push.newrev)
end
end
# Add comment about branches being deleted or added to merge requests
def comment_mr_branch_presence_changed
presence = branch_added? ? :add : :delete
presence = @push.branch_added? ? :add : :delete
merge_requests_for_source_branch.each do |merge_request|
SystemNoteService.change_branch_presence(
merge_request, merge_request.project, @current_user,
:source, @branch_name, presence)
:source, @push.branch_name, presence)
end
end
......@@ -164,7 +161,7 @@ module MergeRequests
SystemNoteService.add_commits(merge_request, merge_request.project,
@current_user, new_commits,
existing_commits, @oldrev)
existing_commits, @push.oldrev)
notification_service.push_to_merge_request(merge_request, @current_user, new_commits: new_commits, existing_commits: existing_commits)
end
......@@ -195,7 +192,7 @@ module MergeRequests
# Call merge request webhook with update branches
def execute_mr_web_hooks
merge_requests_for_source_branch.each do |merge_request|
execute_hooks(merge_request, 'update', old_rev: @oldrev)
execute_hooks(merge_request, 'update', old_rev: @push.oldrev)
end
end
......@@ -203,7 +200,7 @@ module MergeRequests
# `MergeRequestsClosingIssues` model (as a performance optimization).
# rubocop: disable CodeReuse/ActiveRecord
def cache_merge_requests_closing_issues
@project.merge_requests.where(source_branch: @branch_name).each do |merge_request|
@project.merge_requests.where(source_branch: @push.branch_name).each do |merge_request|
merge_request.cache_merge_request_closes_issues!(@current_user)
end
end
......@@ -215,15 +212,7 @@ module MergeRequests
def merge_requests_for_source_branch(reload: false)
@source_merge_requests = nil if reload
@source_merge_requests ||= merge_requests_for(@branch_name)
end
def branch_added?
Gitlab::Git.blank_ref?(@oldrev)
end
def branch_removed?
Gitlab::Git.blank_ref?(@newrev)
@source_merge_requests ||= merge_requests_for(@push.branch_name)
end
end
end
---
title: Add support for pipeline only/except policy for modified paths
merge_request: 21981
author:
type: added
......@@ -387,6 +387,8 @@ except master.
> `refs` and `kubernetes` policies introduced in GitLab 10.0
>
> `variables` policy introduced in 10.7
>
> `changes` policy [introduced](https://gitlab.com/gitlab-org/gitlab-ce/issues/19232) in 11.4
CAUTION: **Warning:**
This an _alpha_ feature, and it it subject to change at any time without
......@@ -398,10 +400,15 @@ policy configuration.
GitLab now supports both, simple and complex strategies, so it is possible to
use an array and a hash configuration scheme.
Three keys are now available: `refs`, `kubernetes` and `variables`.
Four keys are now available: `refs`, `kubernetes` and `variables` and `changes`.
### `refs` and `kubernetes`
Refs strategy equals to simplified only/except configuration, whereas
kubernetes strategy accepts only `active` keyword.
### `variables`
`variables` keyword is used to define variables expressions. In other words
you can use predefined variables / project / group or
environment-scoped variables to define an expression GitLab is going to
......@@ -445,6 +452,46 @@ end-to-end:
Learn more about variables expressions on [a separate page][variables-expressions].
### `changes`
Using `changes` keyword with `only` or `except` makes it possible to define if
a job should be created based on files modified by a git push event.
For example:
```yaml
docker build:
script: docker build -t my-image:$CI_COMMIT_REF_SLUG .
only:
changes:
- Dockerfile
- docker/scripts/*
```
In the scenario above, if you are pushing multiple commits to GitLab to an
existing branch, GitLab creates and triggers `docker build` job, provided that
one of the commits contains changes to either:
- The `Dockerfile` file.
- Any of the files inside `docker/scripts/` directory.
CAUTION: **Warning:**
There are some caveats when using this feature with new branches and tags. See
the section below.
#### Using `changes` with new branches and tags
If you are pushing a **new** branch or a **new** tag to GitLab, the policy
always evaluates to true and GitLab will create a job. This feature is not
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.
## `tags`
`tags` is used to select specific Runners from the list of all Runners that are
......
# frozen_string_literal: true
module Gitlab
module Ci
module Build
module Policy
class Changes < Policy::Specification
def initialize(globs)
@globs = Array(globs)
end
def satisfied_by?(pipeline, seed)
return true unless pipeline.branch_updated?
pipeline.modified_paths.any? do |path|
@globs.any? do |glob|
File.fnmatch?(glob, path, File::FNM_PATHNAME | File::FNM_DOTMATCH)
end
end
end
end
end
end
end
end
......@@ -25,17 +25,19 @@ module Gitlab
include Entry::Validatable
include Entry::Attributable
attributes :refs, :kubernetes, :variables
ALLOWED_KEYS = %i[refs kubernetes variables changes].freeze
attributes :refs, :kubernetes, :variables, :changes
validations do
validates :config, presence: true
validates :config, allowed_keys: %i[refs kubernetes variables]
validates :config, allowed_keys: ALLOWED_KEYS
validate :variables_expressions_syntax
with_options allow_nil: true do
validates :refs, array_of_strings_or_regexps: true
validates :kubernetes, allowed_values: %w[active]
validates :variables, array_of_strings: true
validates :changes, array_of_strings: true
end
def variables_expressions_syntax
......
......@@ -18,6 +18,10 @@ module Gitlab
indexed_by_path[path]
end
def paths
@collection.map(&:path)
end
private
def indexed_by_path
......
# frozen_string_literal: true
module Gitlab
module Git
class Push
include Gitlab::Utils::StrongMemoize
attr_reader :oldrev, :newrev
def initialize(project, oldrev, newrev, ref)
@project = project
@oldrev = oldrev.presence || Gitlab::Git::BLANK_SHA
@newrev = newrev.presence || Gitlab::Git::BLANK_SHA
@ref = ref
end
def branch_name
strong_memoize(:branch_name) do
Gitlab::Git.branch_name(@ref)
end
end
def branch_added?
Gitlab::Git.blank_ref?(@oldrev)
end
def branch_removed?
Gitlab::Git.blank_ref?(@newrev)
end
def branch_updated?
branch_push? && !branch_added? && !branch_removed?
end
def force_push?
Gitlab::Checks::ForcePush.force_push?(@project, @oldrev, @newrev)
end
def branch_push?
strong_memoize(:branch_push) do
Gitlab::Git.branch_ref?(@ref)
end
end
def modified_paths
unless branch_updated?
raise ArgumentError, 'Unable to calculate modified paths!'
end
strong_memoize(:modified_paths) do
@project.repository.diff_stats(@oldrev, @newrev).paths
end
end
end
end
end
require 'spec_helper'
describe Gitlab::Ci::Build::Policy::Changes do
set(:project) { create(:project) }
describe '#satisfied_by?' do
describe 'paths matching matching' do
let(:pipeline) do
build(:ci_empty_pipeline, project: project,
ref: 'master',
source: :push,
sha: '1234abcd',
before_sha: '0123aabb')
end
let(:ci_build) do
build(:ci_build, pipeline: pipeline, project: project, ref: 'master')
end
let(:seed) { double('build seed', to_resource: ci_build) }
before do
allow(pipeline).to receive(:modified_paths) do
%w[some/modified/ruby/file.rb some/other_file.txt some/.dir/file]
end
end
it 'is satisfied by matching literal path' do
policy = described_class.new(%w[some/other_file.txt])
expect(policy).to be_satisfied_by(pipeline, seed)
end
it 'is satisfied by matching simple pattern' do
policy = described_class.new(%w[some/*.txt])
expect(policy).to be_satisfied_by(pipeline, seed)
end
it 'is satisfied by matching recusive pattern' do
policy = described_class.new(%w[some/**/*.rb])
expect(policy).to be_satisfied_by(pipeline, seed)
end
it 'is satisfied by matching a pattern with a dot' do
policy = described_class.new(%w[some/*/file])
expect(policy).to be_satisfied_by(pipeline, seed)
end
it 'is not satisfied when pattern does not match path' do
policy = described_class.new(%w[some/*.rb])
expect(policy).not_to be_satisfied_by(pipeline, seed)
end
it 'is not satisfied when pattern does not match' do
policy = described_class.new(%w[invalid/*.md])
expect(policy).not_to be_satisfied_by(pipeline, seed)
end
context 'when pipelines does not run for a branch update' do
before do
pipeline.before_sha = Gitlab::Git::BLANK_SHA
end
it 'is always satisfied' do
policy = described_class.new(%w[invalid/*])
expect(policy).to be_satisfied_by(pipeline, seed)
end
end
end
describe 'gitaly integration' do
set(:project) { create(:project, :repository) }
let(:pipeline) do
create(:ci_empty_pipeline, project: project,
ref: 'master',
source: :push,
sha: '498214d',
before_sha: '281d3a7')
end
let(:build) do
create(:ci_build, pipeline: pipeline, project: project, ref: 'master')
end
let(:seed) { double('build seed', to_resource: build) }
it 'is satisfied by changes introduced by a push' do
policy = described_class.new(['with space/*.md'])
expect(policy).to be_satisfied_by(pipeline, seed)
end
it 'is not satisfied by changes that are not in the push' do
policy = described_class.new(%w[files/js/commit.js])
expect(policy).not_to be_satisfied_by(pipeline, seed)
end
end
end
end
require 'spec_helper'
require 'fast_spec_helper'
require_dependency 'active_model'
describe Gitlab::Ci::Config::Entry::Policy do
let(:entry) { described_class.new(config) }
......@@ -124,6 +125,23 @@ describe Gitlab::Ci::Config::Entry::Policy do
end
end
context 'when specifying a valid changes policy' do
let(:config) { { changes: %w[some/* paths/**/*.rb] } }
it 'is a correct configuraton' do
expect(entry).to be_valid
expect(entry.value).to eq(config)
end
end
context 'when changes policy is invalid' do
let(:config) { { changes: [1, 2] } }
it 'returns errors' do
expect(entry.errors).to include /changes should be an array of strings/
end
end
context 'when specifying unknown policy' do
let(:config) { { refs: ['master'], invalid: :something } }
......
......@@ -1354,7 +1354,7 @@ module Gitlab
end.to raise_error(Gitlab::Ci::YamlProcessor::ValidationError, "jobs:rspec dependencies should be an array of strings")
end
it 'returns errors if pipeline variables expression is invalid' do
it 'returns errors if pipeline variables expression policy is invalid' do
config = YAML.dump({ rspec: { script: 'test', only: { variables: ['== null'] } } })
expect { Gitlab::Ci::YamlProcessor.new(config) }
......@@ -1362,6 +1362,14 @@ module Gitlab
'jobs:rspec:only variables invalid expression syntax')
end
it 'returns errors if pipeline changes policy is invalid' do
config = YAML.dump({ rspec: { script: 'test', only: { changes: [1] } } })
expect { Gitlab::Ci::YamlProcessor.new(config) }
.to raise_error(Gitlab::Ci::YamlProcessor::ValidationError,
'jobs:rspec:only changes should be an array of strings')
end
it 'returns errors if extended hash configuration is invalid' do
config = YAML.dump({ rspec: { extends: 'something', script: 'test' } })
......
......@@ -14,7 +14,7 @@ describe Gitlab::Git::DiffStatsCollection do
let(:diff_stats) { [stats_a, stats_b] }
let(:collection) { described_class.new(diff_stats) }
describe '.find_by_path' do
describe '#find_by_path' do
it 'returns stats by path when found' do
expect(collection.find_by_path('foo')).to eq(stats_a)
end
......@@ -23,4 +23,10 @@ describe Gitlab::Git::DiffStatsCollection do
expect(collection.find_by_path('no-file')).to be_nil
end
end
describe '#paths' do
it 'returns only modified paths' do
expect(collection.paths).to eq %w[foo bar]
end
end
end
require 'spec_helper'
describe Gitlab::Git::Push do
set(:project) { create(:project, :repository) }
let(:oldrev) { project.commit('HEAD~2').id }
let(:newrev) { project.commit.id }
let(:ref) { 'refs/heads/some-branch' }
subject { described_class.new(project, oldrev, newrev, ref) }
describe '#branch_name' do
context 'when it is a branch push' do
let(:ref) { 'refs/heads/my-branch' }
it 'returns branch name' do
expect(subject.branch_name).to eq 'my-branch'
end
end
context 'when it is a tag push' do
let(:ref) { 'refs/tags/my-branch' }
it 'returns nil' do
expect(subject.branch_name).to be_nil
end
end
end
describe '#branch_push?' do
context 'when pushing a branch ref' do
let(:ref) { 'refs/heads/my-branch' }
it { is_expected.to be_branch_push }
end
context 'when it is a tag push' do
let(:ref) { 'refs/tags/my-tag' }
it { is_expected.not_to be_branch_push }
end
end
describe '#branch_updated?' do
context 'when it is a branch push with correct old and new revisions' do
it { is_expected.to be_branch_updated }
end
context 'when it is not a branch push' do
let(:ref) { 'refs/tags/my-tag' }
it { is_expected.not_to be_branch_updated }
end
context 'when old revision is blank' do
let(:oldrev) { Gitlab::Git::BLANK_SHA }
it { is_expected.not_to be_branch_updated }
end
context 'when it is not a branch push' do
let(:newrev) { Gitlab::Git::BLANK_SHA }
it { is_expected.not_to be_branch_updated }
end
context 'when oldrev is nil' do
let(:oldrev) { nil }
it { is_expected.not_to be_branch_updated }
end
end
describe '#force_push?' do
context 'when old revision is an ancestor of the new revision' do
let(:oldrev) { 'HEAD~3' }
let(:newrev) { 'HEAD~1' }
it { is_expected.not_to be_force_push }
end
context 'when old revision is not an ancestor of the new revision' do
let(:oldrev) { 'HEAD~3' }
let(:newrev) { '123456' }
it { is_expected.to be_force_push }
end
end
describe '#branch_added?' do
context 'when old revision is defined' do
it { is_expected.not_to be_branch_added }
end
context 'when old revision is not defined' do
let(:oldrev) { Gitlab::Git::BLANK_SHA }
it { is_expected.to be_branch_added }
end
end
describe '#branch_removed?' do
context 'when new revision is defined' do
it { is_expected.not_to be_branch_removed }
end
context 'when new revision is not defined' do
let(:newrev) { Gitlab::Git::BLANK_SHA }
it { is_expected.to be_branch_removed }
end
end
describe '#modified_paths' do
context 'when a push is a branch update' do
let(:newrev) { '498214d' }
let(:oldrev) { '281d3a7' }
it 'returns modified paths' do
expect(subject.modified_paths).to eq ['bar/branch-test.txt',
'files/js/commit.coffee',
'with space/README.md']
end
end
context 'when a push is not a branch update' do
let(:oldrev) { Gitlab::Git::BLANK_SHA }
it 'raises an error' do
expect { subject.modified_paths }.to raise_error(ArgumentError)
end
end
end
describe '#oldrev' do
context 'when a valid oldrev is provided' do
it 'returns oldrev' do
expect(subject.oldrev).to eq oldrev
end
end
context 'when a nil valud is provided' do
let(:oldrev) { nil }
it 'returns blank SHA' do
expect(subject.oldrev).to eq Gitlab::Git::BLANK_SHA
end
end
end
describe '#newrev' do
context 'when valid newrev is provided' do
it 'returns newrev' do
expect(subject.newrev).to eq newrev
end
end
context 'when a nil valud is provided' do
let(:newrev) { nil }
it 'returns blank SHA' do
expect(subject.newrev).to eq Gitlab::Git::BLANK_SHA
end
end
end
end
......@@ -825,6 +825,57 @@ describe Ci::Pipeline, :mailer do
end
end
describe '#branch_updated?' do
context 'when pipeline has before SHA' do
before do
pipeline.update_column(:before_sha, 'a1b2c3d4')
end
it 'runs on a branch update push' do
expect(pipeline.before_sha).not_to be Gitlab::Git::BLANK_SHA
expect(pipeline.branch_updated?).to be true
end
end
context 'when pipeline does not have before SHA' do
before do
pipeline.update_column(:before_sha, Gitlab::Git::BLANK_SHA)
end
it 'does not run on a branch updating push' do
expect(pipeline.branch_updated?).to be false
end
end
end
describe '#modified_paths' do
context 'when old and new revisions are set' do
let(:project) { create(:project, :repository) }
before do
pipeline.update(before_sha: '1234abcd', sha: '2345bcde')
end
it 'fetches stats for changes between commits' do
expect(project.repository)
.to receive(:diff_stats).with('1234abcd', '2345bcde')
.and_call_original
pipeline.modified_paths
end
end
context 'when either old or new revision is missing' do
before do
pipeline.update_column(:before_sha, Gitlab::Git::BLANK_SHA)
end
it 'raises an error' do
expect { pipeline.modified_paths }.to raise_error(ArgumentError)
end
end
end
describe '#has_kubernetes_active?' do
context 'when kubernetes is active' do
shared_examples 'same behavior between KubernetesService and Platform::Kubernetes' do
......
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