Merge branch 'filter-web-hooks-by-branch' into 'master'
Filter web hooks by branch See merge request gitlab-org/gitlab-ce!19513
This commit is contained in:
commit
464b0de1ac
|
@ -66,6 +66,7 @@ class Projects::HooksController < Projects::ApplicationController
|
|||
:enable_ssl_verification,
|
||||
:token,
|
||||
:url,
|
||||
:push_events_branch_filter,
|
||||
*ProjectHook.triggers.values
|
||||
)
|
||||
end
|
||||
|
|
|
@ -50,14 +50,20 @@ module ProtectedRef
|
|||
.map(&:"#{action}_access_levels").flatten
|
||||
end
|
||||
|
||||
# Returns all protected refs that match the given ref name.
|
||||
# This checks all records from the scope built up so far, and does
|
||||
# _not_ return a relation.
|
||||
#
|
||||
# This method optionally takes in a list of `protected_refs` to search
|
||||
# through, to avoid calling out to the database.
|
||||
def matching(ref_name, protected_refs: nil)
|
||||
ProtectedRefMatcher.matching(self, ref_name, protected_refs: protected_refs)
|
||||
(protected_refs || self.all).select { |protected_ref| protected_ref.matches?(ref_name) }
|
||||
end
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def ref_matcher
|
||||
@ref_matcher ||= ProtectedRefMatcher.new(self)
|
||||
@ref_matcher ||= RefMatcher.new(self.name)
|
||||
end
|
||||
end
|
||||
|
|
|
@ -29,6 +29,12 @@ module TriggerableHooks
|
|||
public_send(trigger) # rubocop:disable GitlabSecurity/PublicSend
|
||||
end
|
||||
|
||||
def select_active(hooks_scope, data)
|
||||
select do |hook|
|
||||
ActiveHookFilter.new(hook).matches?(hooks_scope, data)
|
||||
end
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def triggerable_hooks(hooks)
|
||||
|
|
|
@ -0,0 +1,14 @@
|
|||
class ActiveHookFilter
|
||||
def initialize(hook)
|
||||
@hook = hook
|
||||
@push_events_filter_matcher = RefMatcher.new(@hook.push_events_branch_filter)
|
||||
end
|
||||
|
||||
def matches?(hooks_scope, data)
|
||||
return true if hooks_scope != :push_hooks
|
||||
return true if @hook.push_events_branch_filter.blank?
|
||||
|
||||
branch_name = Gitlab::Git.branch_name(data[:ref])
|
||||
@push_events_filter_matcher.matches?(branch_name)
|
||||
end
|
||||
end
|
|
@ -9,6 +9,7 @@ class WebHook < ActiveRecord::Base
|
|||
allow_local_network: lambda(&:allow_local_requests?) }
|
||||
|
||||
validates :token, format: { without: /\n/ }
|
||||
validates :push_events_branch_filter, branch_filter: true
|
||||
|
||||
def execute(data, hook_name)
|
||||
WebHookService.new(self, data, hook_name).execute
|
||||
|
|
|
@ -1184,10 +1184,9 @@ class Project < ActiveRecord::Base
|
|||
|
||||
def execute_hooks(data, hooks_scope = :push_hooks)
|
||||
run_after_commit_or_now do
|
||||
hooks.hooks_for(hooks_scope).each do |hook|
|
||||
hooks.hooks_for(hooks_scope).select_active(hooks_scope, data).each do |hook|
|
||||
hook.async_execute(data, hooks_scope.to_s)
|
||||
end
|
||||
|
||||
SystemHooksService.new.execute_hooks(data, hooks_scope)
|
||||
end
|
||||
end
|
||||
|
|
|
@ -1,56 +0,0 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
class ProtectedRefMatcher
|
||||
def initialize(protected_ref)
|
||||
@protected_ref = protected_ref
|
||||
end
|
||||
|
||||
# Returns all protected refs that match the given ref name.
|
||||
# This checks all records from the scope built up so far, and does
|
||||
# _not_ return a relation.
|
||||
#
|
||||
# This method optionally takes in a list of `protected_refs` to search
|
||||
# through, to avoid calling out to the database.
|
||||
def self.matching(type, ref_name, protected_refs: nil)
|
||||
(protected_refs || type.all).select { |protected_ref| protected_ref.matches?(ref_name) }
|
||||
end
|
||||
|
||||
# Returns all branches/tags (among the given list of refs [`Gitlab::Git::Branch`])
|
||||
# that match the current protected ref.
|
||||
def matching(refs)
|
||||
refs.select { |ref| @protected_ref.matches?(ref.name) }
|
||||
end
|
||||
|
||||
# Checks if the protected ref matches the given ref name.
|
||||
def matches?(ref_name)
|
||||
return false if @protected_ref.name.blank?
|
||||
|
||||
exact_match?(ref_name) || wildcard_match?(ref_name)
|
||||
end
|
||||
|
||||
# Checks if this protected ref contains a wildcard
|
||||
def wildcard?
|
||||
@protected_ref.name && @protected_ref.name.include?('*')
|
||||
end
|
||||
|
||||
protected
|
||||
|
||||
def exact_match?(ref_name)
|
||||
@protected_ref.name == ref_name
|
||||
end
|
||||
|
||||
def wildcard_match?(ref_name)
|
||||
return false unless wildcard?
|
||||
|
||||
wildcard_regex === ref_name
|
||||
end
|
||||
|
||||
def wildcard_regex
|
||||
@wildcard_regex ||= begin
|
||||
name = @protected_ref.name.gsub('*', 'STAR_DONT_ESCAPE')
|
||||
quoted_name = Regexp.quote(name)
|
||||
regex_string = quoted_name.gsub('STAR_DONT_ESCAPE', '.*?')
|
||||
/\A#{regex_string}\z/
|
||||
end
|
||||
end
|
||||
end
|
|
@ -0,0 +1,46 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
class RefMatcher
|
||||
def initialize(ref_name_or_pattern)
|
||||
@ref_name_or_pattern = ref_name_or_pattern
|
||||
end
|
||||
|
||||
# Returns all branches/tags (among the given list of refs [`Gitlab::Git::Branch`])
|
||||
# that match the current protected ref.
|
||||
def matching(refs)
|
||||
refs.select { |ref| matches?(ref.name) }
|
||||
end
|
||||
|
||||
# Checks if the protected ref matches the given ref name.
|
||||
def matches?(ref_name)
|
||||
return false if @ref_name_or_pattern.blank?
|
||||
|
||||
exact_match?(ref_name) || wildcard_match?(ref_name)
|
||||
end
|
||||
|
||||
# Checks if this protected ref contains a wildcard
|
||||
def wildcard?
|
||||
@ref_name_or_pattern && @ref_name_or_pattern.include?('*')
|
||||
end
|
||||
|
||||
protected
|
||||
|
||||
def exact_match?(ref_name)
|
||||
@ref_name_or_pattern == ref_name
|
||||
end
|
||||
|
||||
def wildcard_match?(ref_name)
|
||||
return false unless wildcard?
|
||||
|
||||
wildcard_regex === ref_name
|
||||
end
|
||||
|
||||
def wildcard_regex
|
||||
@wildcard_regex ||= begin
|
||||
name = @ref_name_or_pattern.gsub('*', 'STAR_DONT_ESCAPE')
|
||||
quoted_name = Regexp.quote(name)
|
||||
regex_string = quoted_name.gsub('STAR_DONT_ESCAPE', '.*?')
|
||||
/\A#{regex_string}\z/
|
||||
end
|
||||
end
|
||||
end
|
|
@ -0,0 +1,35 @@
|
|||
# BranchFilterValidator
|
||||
#
|
||||
# Custom validator for branch names. Squishes whitespace and ignores empty
|
||||
# string. This only checks that a string is a valid git branch name. It does
|
||||
# not check whether a branch already exists.
|
||||
#
|
||||
# Example:
|
||||
#
|
||||
# class Webhook < ActiveRecord::Base
|
||||
# validates :push_events_branch_filter, branch_name: true
|
||||
# end
|
||||
#
|
||||
class BranchFilterValidator < ActiveModel::EachValidator
|
||||
def validate_each(record, attribute, value)
|
||||
value.squish! unless value.nil?
|
||||
|
||||
if value.present?
|
||||
value_without_wildcards = value.tr('*', 'x')
|
||||
|
||||
unless Gitlab::GitRefValidator.validate(value_without_wildcards)
|
||||
record.errors[attribute] << "is not a valid branch name"
|
||||
end
|
||||
|
||||
unless value.length <= 4000
|
||||
record.errors[attribute] << "is longer than the allowed length of 4000 characters."
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def contains_wildcard?(value)
|
||||
value.include?('*')
|
||||
end
|
||||
end
|
|
@ -17,6 +17,7 @@
|
|||
%strong Push events
|
||||
%p.light.ml-1
|
||||
This URL will be triggered by a push to the repository
|
||||
= form.text_field :push_events_branch_filter, class: 'form-control', placeholder: 'Branch name or wildcard pattern to trigger on (leave blank for all)'
|
||||
%li
|
||||
= form.check_box :tag_push_events, class: 'form-check-input'
|
||||
= form.label :tag_push_events, class: 'list-label form-check-label ml-1' do
|
||||
|
|
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
title: Add branch filter to project webhooks
|
||||
merge_request: 20338
|
||||
author: Duana Saskia
|
||||
type: added
|
|
@ -0,0 +1,12 @@
|
|||
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
|
||||
# for more information on how to write migrations for GitLab.
|
||||
|
||||
class AddPushEventsBranchFilterToWebHooks < ActiveRecord::Migration
|
||||
include Gitlab::Database::MigrationHelpers
|
||||
|
||||
DOWNTIME = false
|
||||
|
||||
def change
|
||||
add_column :web_hooks, :push_events_branch_filter, :text
|
||||
end
|
||||
end
|
|
@ -2245,6 +2245,7 @@ ActiveRecord::Schema.define(version: 20180826111825) do
|
|||
t.boolean "repository_update_events", default: false, null: false
|
||||
t.boolean "job_events", default: false, null: false
|
||||
t.boolean "confidential_note_events"
|
||||
t.text "push_events_branch_filter"
|
||||
end
|
||||
|
||||
add_index "web_hooks", ["project_id"], name: "index_web_hooks_on_project_id", using: :btree
|
||||
|
|
|
@ -1334,6 +1334,7 @@ GET /projects/:id/hooks/:hook_id
|
|||
"url": "http://example.com/hook",
|
||||
"project_id": 3,
|
||||
"push_events": true,
|
||||
"push_events_branch_filter": "",
|
||||
"issues_events": true,
|
||||
"confidential_issues_events": true,
|
||||
"merge_requests_events": true,
|
||||
|
@ -1360,6 +1361,7 @@ POST /projects/:id/hooks
|
|||
| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) |
|
||||
| `url` | string | yes | The hook URL |
|
||||
| `push_events` | boolean | no | Trigger hook on push events |
|
||||
| `push_events_branch_filter` | string | no | Trigger hook on push events for matching branches only |
|
||||
| `issues_events` | boolean | no | Trigger hook on issues events |
|
||||
| `confidential_issues_events` | boolean | no | Trigger hook on confidential issues events |
|
||||
| `merge_requests_events` | boolean | no | Trigger hook on merge requests events |
|
||||
|
@ -1385,6 +1387,7 @@ PUT /projects/:id/hooks/:hook_id
|
|||
| `hook_id` | integer | yes | The ID of the project hook |
|
||||
| `url` | string | yes | The hook URL |
|
||||
| `push_events` | boolean | no | Trigger hook on push events |
|
||||
| `push_events_branch_filter` | string | no | Trigger hook on push events for matching branches only |
|
||||
| `issues_events` | boolean | no | Trigger hook on issues events |
|
||||
| `confidential_issues_events` | boolean | no | Trigger hook on confidential issues events |
|
||||
| `merge_requests_events` | boolean | no | Trigger hook on merge requests events |
|
||||
|
|
|
@ -65,8 +65,8 @@ Below are described the supported events.
|
|||
|
||||
Triggered when you push to the repository except when pushing tags.
|
||||
|
||||
> **Note:** When more than 20 commits are pushed at once, the `commits` web hook
|
||||
attribute will only contain the first 20 for performance reasons. Loading
|
||||
> **Note:** When more than 20 commits are pushed at once, the `commits` web hook
|
||||
attribute will only contain the first 20 for performance reasons. Loading
|
||||
detailed commit data is expensive. Note that despite only 20 commits being
|
||||
present in the `commits` attribute, the `total_commits_count` attribute will
|
||||
contain the actual total.
|
||||
|
|
|
@ -105,6 +105,7 @@ module API
|
|||
expose :project_id, :issues_events, :confidential_issues_events
|
||||
expose :note_events, :confidential_note_events, :pipeline_events, :wiki_page_events
|
||||
expose :job_events
|
||||
expose :push_events_branch_filter
|
||||
end
|
||||
|
||||
class SharedGroup < Grape::Entity
|
||||
|
|
|
@ -20,6 +20,7 @@ module API
|
|||
optional :wiki_page_events, type: Boolean, desc: "Trigger hook on wiki events"
|
||||
optional :enable_ssl_verification, type: Boolean, desc: "Do SSL verification when triggering the hook"
|
||||
optional :token, type: String, desc: "Secret token to validate received payloads; this will not be returned in the response"
|
||||
optional :push_events_branch_filter, type: String, desc: "Trigger hook on specified branch only"
|
||||
end
|
||||
end
|
||||
|
||||
|
@ -63,6 +64,7 @@ module API
|
|||
present hook, with: Entities::ProjectHook
|
||||
else
|
||||
error!("Invalid url given", 422) if hook.errors[:url].present?
|
||||
error!("Invalid branch filter given", 422) if hook.errors[:push_events_branch_filter].present?
|
||||
|
||||
not_found!("Project hook #{hook.errors.messages}")
|
||||
end
|
||||
|
@ -84,6 +86,7 @@ module API
|
|||
present hook, with: Entities::ProjectHook
|
||||
else
|
||||
error!("Invalid url given", 422) if hook.errors[:url].present?
|
||||
error!("Invalid branch filter given", 422) if hook.errors[:push_events_branch_filter].present?
|
||||
|
||||
not_found!("Project hook #{hook.errors.messages}")
|
||||
end
|
||||
|
|
|
@ -51,6 +51,7 @@ describe 'Projects > Settings > Integration settings' do
|
|||
|
||||
fill_in 'hook_url', with: url
|
||||
check 'Tag push events'
|
||||
fill_in 'hook_push_events_branch_filter', with: 'master'
|
||||
check 'Enable SSL verification'
|
||||
check 'Job events'
|
||||
|
||||
|
|
|
@ -416,6 +416,7 @@ ProjectHook:
|
|||
- type
|
||||
- service_id
|
||||
- push_events
|
||||
- push_events_branch_filter
|
||||
- issues_events
|
||||
- merge_requests_events
|
||||
- tag_push_events
|
||||
|
|
|
@ -40,4 +40,28 @@ RSpec.describe TriggerableHooks do
|
|||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe '.select_active' do
|
||||
it 'returns hooks that match the active filter' do
|
||||
TestableHook.create!(url: 'http://example1.com', push_events: true)
|
||||
TestableHook.create!(url: 'http://example2.com', push_events: true)
|
||||
filter1 = double(:filter1)
|
||||
filter2 = double(:filter2)
|
||||
allow(ActiveHookFilter).to receive(:new).exactly(2).times.and_return(filter1, filter2)
|
||||
expect(filter1).to receive(:matches?).and_return(true)
|
||||
expect(filter2).to receive(:matches?).and_return(false)
|
||||
|
||||
hooks = TestableHook.push_hooks.order_id_asc
|
||||
expect(hooks.select_active(:push_hooks, {})).to eq [hooks.first]
|
||||
end
|
||||
|
||||
it 'returns empty list if no hooks match the active filter' do
|
||||
TestableHook.create!(url: 'http://example1.com', push_events: true)
|
||||
filter = double(:filter)
|
||||
allow(ActiveHookFilter).to receive(:new).and_return(filter)
|
||||
expect(filter).to receive(:matches?).and_return(false)
|
||||
|
||||
expect(TestableHook.push_hooks.select_active(:push_hooks, {})).to eq []
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -0,0 +1,68 @@
|
|||
require 'spec_helper'
|
||||
|
||||
describe ActiveHookFilter do
|
||||
subject(:filter) { described_class.new(hook) }
|
||||
|
||||
describe '#matches?' do
|
||||
context 'for push event hooks' do
|
||||
let(:hook) do
|
||||
create(:project_hook, push_events: true, push_events_branch_filter: branch_filter)
|
||||
end
|
||||
|
||||
context 'branch filter is specified' do
|
||||
let(:branch_filter) { 'master' }
|
||||
|
||||
it 'returns true if branch matches' do
|
||||
expect(filter.matches?(:push_hooks, { ref: 'refs/heads/master' })).to be true
|
||||
end
|
||||
|
||||
it 'returns false if branch does not match' do
|
||||
expect(filter.matches?(:push_hooks, { ref: 'refs/heads/my_branch' })).to be false
|
||||
end
|
||||
|
||||
it 'returns false if ref is nil' do
|
||||
expect(filter.matches?(:push_hooks, {})).to be false
|
||||
end
|
||||
|
||||
context 'branch filter contains wildcard' do
|
||||
let(:branch_filter) { 'features/*' }
|
||||
|
||||
it 'returns true if branch matches' do
|
||||
expect(filter.matches?(:push_hooks, { ref: 'refs/heads/features/my-branch' })).to be true
|
||||
expect(filter.matches?(:push_hooks, { ref: 'refs/heads/features/my-branch/something' })).to be true
|
||||
end
|
||||
|
||||
it 'returns false if branch does not match' do
|
||||
expect(filter.matches?(:push_hooks, { ref: 'refs/heads/master' })).to be false
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context 'branch filter is not specified' do
|
||||
let(:branch_filter) { nil }
|
||||
|
||||
it 'returns true' do
|
||||
expect(filter.matches?(:push_hooks, { ref: 'refs/heads/master' })).to be true
|
||||
end
|
||||
end
|
||||
|
||||
context 'branch filter is empty string' do
|
||||
let(:branch_filter) { '' }
|
||||
|
||||
it 'acts like branch is not specified' do
|
||||
expect(filter.matches?(:push_hooks, { ref: 'refs/heads/master' })).to be true
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context 'for non-push-events hooks' do
|
||||
let(:hook) do
|
||||
create(:project_hook, issues_events: true, push_events: false, push_events_branch_filter: '')
|
||||
end
|
||||
|
||||
it 'returns true as branch filters are not yet supported for these' do
|
||||
expect(filter.matches?(:issues_events, { ref: 'refs/heads/master' })).to be true
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
|
@ -35,6 +35,26 @@ describe WebHook do
|
|||
|
||||
it { is_expected.not_to allow_values("foo\nbar", "foo\r\nbar").for(:token) }
|
||||
end
|
||||
|
||||
describe 'push_events_branch_filter' do
|
||||
it { is_expected.to allow_values("good_branch_name", "another/good-branch_name").for(:push_events_branch_filter) }
|
||||
it { is_expected.to allow_values("").for(:push_events_branch_filter) }
|
||||
it { is_expected.not_to allow_values("bad branch name", "bad~branchname").for(:push_events_branch_filter) }
|
||||
|
||||
it 'gets rid of whitespace' do
|
||||
hook.push_events_branch_filter = ' branch '
|
||||
hook.save
|
||||
|
||||
expect(hook.push_events_branch_filter).to eq('branch')
|
||||
end
|
||||
|
||||
it 'stores whitespace only as empty' do
|
||||
hook.push_events_branch_filter = ' '
|
||||
hook.save
|
||||
|
||||
expect(hook.push_events_branch_filter).to eq('')
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe 'execute' do
|
||||
|
|
|
@ -3734,21 +3734,45 @@ describe Project do
|
|||
end
|
||||
|
||||
describe '#execute_hooks' do
|
||||
it 'executes the projects hooks with the specified scope' do
|
||||
hook1 = create(:project_hook, merge_requests_events: true, tag_push_events: false)
|
||||
hook2 = create(:project_hook, merge_requests_events: false, tag_push_events: true)
|
||||
project = create(:project, hooks: [hook1, hook2])
|
||||
let(:data) { { ref: 'refs/heads/master', data: 'data' } }
|
||||
it 'executes active projects hooks with the specified scope' do
|
||||
hook = create(:project_hook, merge_requests_events: false, push_events: true)
|
||||
expect(ProjectHook).to receive(:select_active)
|
||||
.with(:push_hooks, data)
|
||||
.and_return([hook])
|
||||
project = create(:project, hooks: [hook])
|
||||
|
||||
expect_any_instance_of(ProjectHook).to receive(:async_execute).once
|
||||
|
||||
project.execute_hooks({}, :tag_push_hooks)
|
||||
project.execute_hooks(data, :push_hooks)
|
||||
end
|
||||
|
||||
it 'does not execute project hooks that dont match the specified scope' do
|
||||
hook = create(:project_hook, merge_requests_events: true, push_events: false)
|
||||
project = create(:project, hooks: [hook])
|
||||
|
||||
expect_any_instance_of(ProjectHook).not_to receive(:async_execute).once
|
||||
|
||||
project.execute_hooks(data, :push_hooks)
|
||||
end
|
||||
|
||||
it 'does not execute project hooks which are not active' do
|
||||
hook = create(:project_hook, push_events: true)
|
||||
expect(ProjectHook).to receive(:select_active)
|
||||
.with(:push_hooks, data)
|
||||
.and_return([])
|
||||
project = create(:project, hooks: [hook])
|
||||
|
||||
expect_any_instance_of(ProjectHook).not_to receive(:async_execute).once
|
||||
|
||||
project.execute_hooks(data, :push_hooks)
|
||||
end
|
||||
|
||||
it 'executes the system hooks with the specified scope' do
|
||||
expect_any_instance_of(SystemHooksService).to receive(:execute_hooks).with({ data: 'data' }, :merge_request_hooks)
|
||||
expect_any_instance_of(SystemHooksService).to receive(:execute_hooks).with(data, :merge_request_hooks)
|
||||
|
||||
project = build(:project)
|
||||
project.execute_hooks({ data: 'data' }, :merge_request_hooks)
|
||||
project.execute_hooks(data, :merge_request_hooks)
|
||||
end
|
||||
|
||||
it 'executes the system hooks when inside a transaction' do
|
||||
|
@ -3763,7 +3787,7 @@ describe Project do
|
|||
# actually get to the `after_commit` hook that queues these jobs.
|
||||
expect do
|
||||
project.transaction do
|
||||
project.execute_hooks({ data: 'data' }, :merge_request_hooks)
|
||||
project.execute_hooks(data, :merge_request_hooks)
|
||||
end
|
||||
end.not_to raise_error # Sidekiq::Worker::EnqueueFromTransactionError
|
||||
end
|
||||
|
|
|
@ -9,7 +9,8 @@ describe API::ProjectHooks, 'ProjectHooks' do
|
|||
:all_events_enabled,
|
||||
project: project,
|
||||
url: 'http://example.com',
|
||||
enable_ssl_verification: true)
|
||||
enable_ssl_verification: true,
|
||||
push_events_branch_filter: 'master')
|
||||
end
|
||||
|
||||
before do
|
||||
|
@ -38,6 +39,7 @@ describe API::ProjectHooks, 'ProjectHooks' do
|
|||
expect(json_response.first['pipeline_events']).to eq(true)
|
||||
expect(json_response.first['wiki_page_events']).to eq(true)
|
||||
expect(json_response.first['enable_ssl_verification']).to eq(true)
|
||||
expect(json_response.first['push_events_branch_filter']).to eq('master')
|
||||
end
|
||||
end
|
||||
|
||||
|
@ -90,7 +92,7 @@ describe API::ProjectHooks, 'ProjectHooks' do
|
|||
expect do
|
||||
post api("/projects/#{project.id}/hooks", user),
|
||||
url: "http://example.com", issues_events: true, confidential_issues_events: true, wiki_page_events: true,
|
||||
job_events: true
|
||||
job_events: true, push_events_branch_filter: 'some-feature-branch'
|
||||
end.to change {project.hooks.count}.by(1)
|
||||
|
||||
expect(response).to have_gitlab_http_status(201)
|
||||
|
@ -106,6 +108,7 @@ describe API::ProjectHooks, 'ProjectHooks' do
|
|||
expect(json_response['pipeline_events']).to eq(false)
|
||||
expect(json_response['wiki_page_events']).to eq(true)
|
||||
expect(json_response['enable_ssl_verification']).to eq(true)
|
||||
expect(json_response['push_events_branch_filter']).to eq('some-feature-branch')
|
||||
expect(json_response).not_to include('token')
|
||||
end
|
||||
|
||||
|
@ -132,7 +135,12 @@ describe API::ProjectHooks, 'ProjectHooks' do
|
|||
end
|
||||
|
||||
it "returns a 422 error if url not valid" do
|
||||
post api("/projects/#{project.id}/hooks", user), "url" => "ftp://example.com"
|
||||
post api("/projects/#{project.id}/hooks", user), url: "ftp://example.com"
|
||||
expect(response).to have_gitlab_http_status(422)
|
||||
end
|
||||
|
||||
it "returns a 422 error if branch filter is not valid" do
|
||||
post api("/projects/#{project.id}/hooks", user), url: "http://example.com", push_events_branch_filter: '~badbranchname/'
|
||||
expect(response).to have_gitlab_http_status(422)
|
||||
end
|
||||
end
|
||||
|
|
|
@ -0,0 +1,42 @@
|
|||
require 'spec_helper'
|
||||
|
||||
describe BranchFilterValidator do
|
||||
let(:validator) { described_class.new(attributes: [:push_events_branch_filter]) }
|
||||
let(:hook) { build(:project_hook) }
|
||||
|
||||
describe '#validates_each' do
|
||||
it 'allows valid branch names' do
|
||||
validator.validate_each(hook, :push_events_branch_filter, "good_branch_name")
|
||||
validator.validate_each(hook, :push_events_branch_filter, "another/good_branch_name")
|
||||
expect(hook.errors.empty?).to be true
|
||||
end
|
||||
|
||||
it 'disallows bad branch names' do
|
||||
validator.validate_each(hook, :push_events_branch_filter, "bad branch~name")
|
||||
expect(hook.errors[:push_events_branch_filter].empty?).to be false
|
||||
end
|
||||
|
||||
it 'allows wildcards' do
|
||||
validator.validate_each(hook, :push_events_branch_filter, "features/*")
|
||||
validator.validate_each(hook, :push_events_branch_filter, "features/*/bla")
|
||||
validator.validate_each(hook, :push_events_branch_filter, "*-stable")
|
||||
expect(hook.errors.empty?).to be true
|
||||
end
|
||||
|
||||
it 'gets rid of whitespace' do
|
||||
filter = ' master '
|
||||
validator.validate_each(hook, :push_events_branch_filter, filter)
|
||||
|
||||
expect(filter).to eq 'master'
|
||||
end
|
||||
|
||||
# Branch names can be quite long but in practice aren't over 255 so 4000 should
|
||||
# be enough space for a list of branch names but we can increase if needed.
|
||||
it 'limits length to 4000 chars' do
|
||||
filter = 'a' * 4001
|
||||
validator.validate_each(hook, :push_events_branch_filter, filter)
|
||||
|
||||
expect(hook.errors[:push_events_branch_filter].empty?).to be false
|
||||
end
|
||||
end
|
||||
end
|
Loading…
Reference in New Issue