Read about our upcoming Code of Conduct on this issue

Commit 920b5915 authored by Kamil Trzciński's avatar Kamil Trzciński
Browse files

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
......@@ -627,6 +627,18 @@ def test_reports
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 @@ def pipeline_data
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 @@ def create_assignee_note(merge_request)
# 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 @@ def do_execute(oldrev, newrev, ref)
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 @@ def close_upon_missing_source_branch_ref
# 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 @@ def post_merge_manually_merged
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 @@ def mark_pending_todos_done
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 @@ def find_new_commits
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 @@ def notify_about_push
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 @@ def mark_mr_as_wip_from_commits
# 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 @@ def execute_mr_web_hooks
# `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 @@ def filter_merge_requests(merge_requests)
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 @@ class ComplexPolicy < Entry::Node
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 @@ def find_by_path(path)
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 @@
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 Ci
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 Ci
'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 @@
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 @@
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' }