Welcome to mirror list, hosted at ThFree Co, Russian Federation.

gitlab.com/gitlab-org/gitlab-foss.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat (limited to 'spec/models/hooks')
-rw-r--r--spec/models/hooks/active_hook_filter_spec.rb113
-rw-r--r--spec/models/hooks/project_hook_spec.rb20
-rw-r--r--spec/models/hooks/system_hook_spec.rb11
-rw-r--r--spec/models/hooks/web_hook_log_spec.rb19
-rw-r--r--spec/models/hooks/web_hook_spec.rb142
5 files changed, 228 insertions, 77 deletions
diff --git a/spec/models/hooks/active_hook_filter_spec.rb b/spec/models/hooks/active_hook_filter_spec.rb
index 1f693ce9fde..47c0fbdb106 100644
--- a/spec/models/hooks/active_hook_filter_spec.rb
+++ b/spec/models/hooks/active_hook_filter_spec.rb
@@ -6,65 +6,102 @@ RSpec.describe ActiveHookFilter do
subject(:filter) { described_class.new(hook) }
describe '#matches?' do
- context 'for push event hooks' do
+ using RSpec::Parameterized::TableSyntax
+
+ context 'for various types of branch_filter' do
let(:hook) do
- create(:project_hook, push_events: true, push_events_branch_filter: branch_filter)
+ build(:project_hook, push_events: true, issues_events: true)
end
- context 'branch filter is specified' do
- let(:branch_filter) { 'master' }
+ where(:branch_filter_strategy, :branch_filter, :ref, :expected_matches?) do
+ 'all_branches' | 'master' | 'refs/heads/master' | true
+ 'all_branches' | '' | 'refs/heads/master' | true
+ 'all_branches' | nil | 'refs/heads/master' | true
+ 'all_branches' | '.*' | 'refs/heads/master' | true
+ 'wildcard' | 'master' | 'refs/heads/master' | true
+ 'wildcard' | 'master' | 'refs/heads/my_branch' | false
+ 'wildcard' | 'features/*' | 'refs/heads/features/my-branch' | true
+ 'wildcard' | 'features/*' | 'refs/heads/features/my-branch/something' | true
+ 'wildcard' | 'features/*' | 'refs/heads/master' | false
+ 'wildcard' | nil | 'refs/heads/master' | true
+ 'wildcard' | '' | 'refs/heads/master' | true
+ 'regex' | 'master' | 'refs/heads/master' | true
+ 'regex' | 'master' | 'refs/heads/my_branch' | false
+ 'regex' | 'features/*' | 'refs/heads/xxxx/features/my-branch' | true
+ 'regex' | 'features/*' | 'refs/heads/features/' | true
+ 'regex' | 'features/*' | 'refs/heads/features' | true
+ 'regex' | 'features/.*' | 'refs/heads/features/my-branch' | true
+ 'regex' | 'features/.*' | 'refs/heads/features/my-branch/something' | true
+ 'regex' | 'features/.*' | 'refs/heads/master' | false
+ 'regex' | '(feature|dev)' | 'refs/heads/feature' | true
+ 'regex' | '(feature|dev)' | 'refs/heads/dev' | true
+ 'regex' | '(feature|dev)' | 'refs/heads/master' | false
+ 'regex' | nil | 'refs/heads/master' | true
+ 'regex' | '' | 'refs/heads/master' | true
+ end
- it 'returns true if branch matches' do
- expect(filter.matches?(:push_hooks, { ref: 'refs/heads/master' })).to be true
+ with_them do
+ before do
+ hook.assign_attributes(
+ push_events_branch_filter: branch_filter,
+ branch_filter_strategy: branch_filter_strategy
+ )
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 { expect(filter.matches?(:push_hooks, { ref: ref })).to be expected_matches? }
+ it { expect(filter.matches?(:issues_events, { ref: ref })).to be true }
+ end
- it 'returns false if ref is nil' do
- expect(filter.matches?(:push_hooks, {})).to be false
+ context 'when the branch filter is a invalid regex' do
+ let(:hook) do
+ build(
+ :project_hook,
+ push_events: true,
+ push_events_branch_filter: 'master',
+ branch_filter_strategy: 'regex'
+ )
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
+ before do
+ allow(hook).to receive(:push_events_branch_filter).and_return("invalid-regex[")
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
+ it { expect(filter.matches?(:push_hooks, { ref: 'refs/heads/master' })).to be false }
end
- context 'branch filter is empty string' do
- let(:branch_filter) { '' }
+ context 'when the branch filter is not properly set to nil' do
+ let(:hook) do
+ build(
+ :project_hook,
+ push_events: true,
+ branch_filter_strategy: 'all_branches'
+ )
+ end
- it 'acts like branch is not specified' do
- expect(filter.matches?(:push_hooks, { ref: 'refs/heads/master' })).to be true
+ before do
+ allow(hook).to receive(:push_events_branch_filter).and_return("master")
end
+
+ it { expect(filter.matches?(:push_hooks, { ref: 'refs/heads/feature1' })).to be true }
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: '')
+ context 'when feature flag is disabled' do
+ before do
+ stub_feature_flags(enhanced_webhook_support_regex: false)
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
+ let(:hook) do
+ build(
+ :project_hook,
+ push_events: true,
+ push_events_branch_filter: '(master)',
+ branch_filter_strategy: 'regex'
+ )
end
+
+ it { expect(filter.matches?(:push_hooks, { ref: 'refs/heads/master' })).to be false }
+ it { expect(filter.matches?(:push_hooks, { ref: 'refs/heads/(master)' })).to be true }
end
end
end
diff --git a/spec/models/hooks/project_hook_spec.rb b/spec/models/hooks/project_hook_spec.rb
index 923a6f92424..3d8c377ab21 100644
--- a/spec/models/hooks/project_hook_spec.rb
+++ b/spec/models/hooks/project_hook_spec.rb
@@ -12,14 +12,14 @@ RSpec.describe ProjectHook do
end
it_behaves_like 'includes Limitable concern' do
- subject { build(:project_hook, project: create(:project)) }
+ subject { build(:project_hook) }
end
describe '.for_projects' do
it 'finds related project hooks' do
- hook_a = create(:project_hook)
- hook_b = create(:project_hook)
- hook_c = create(:project_hook)
+ hook_a = create(:project_hook, project: build(:project))
+ hook_b = create(:project_hook, project: build(:project))
+ hook_c = create(:project_hook, project: build(:project))
expect(described_class.for_projects([hook_a.project, hook_b.project]))
.to contain_exactly(hook_a, hook_b)
@@ -30,16 +30,18 @@ RSpec.describe ProjectHook do
describe '.push_hooks' do
it 'returns hooks for push events only' do
- hook = create(:project_hook, push_events: true)
- create(:project_hook, push_events: false)
+ project = build(:project)
+ hook = create(:project_hook, project: project, push_events: true)
+ create(:project_hook, project: project, push_events: false)
expect(described_class.push_hooks).to eq([hook])
end
end
describe '.tag_push_hooks' do
it 'returns hooks for tag push events only' do
- hook = create(:project_hook, tag_push_events: true)
- create(:project_hook, tag_push_events: false)
+ project = build(:project)
+ hook = create(:project_hook, project: project, tag_push_events: true)
+ create(:project_hook, project: project, tag_push_events: false)
expect(described_class.tag_push_hooks).to eq([hook])
end
end
@@ -65,7 +67,7 @@ RSpec.describe ProjectHook do
end
describe '#update_last_failure', :clean_gitlab_redis_shared_state do
- let_it_be(:hook) { create(:project_hook) }
+ let(:hook) { build(:project_hook) }
it 'is a method of this class' do
expect { hook.update_last_failure }.not_to raise_error
diff --git a/spec/models/hooks/system_hook_spec.rb b/spec/models/hooks/system_hook_spec.rb
index f4786083b75..ba94730b1dd 100644
--- a/spec/models/hooks/system_hook_spec.rb
+++ b/spec/models/hooks/system_hook_spec.rb
@@ -4,7 +4,7 @@ require "spec_helper"
RSpec.describe SystemHook do
context 'default attributes' do
- let(:system_hook) { build(:system_hook) }
+ let(:system_hook) { described_class.new }
it 'sets defined default parameters' do
attrs = {
@@ -32,10 +32,10 @@ RSpec.describe SystemHook do
end
describe "execute", :sidekiq_might_not_need_inline do
- let(:system_hook) { create(:system_hook) }
- let(:user) { create(:user) }
- let(:project) { create(:project, namespace: user.namespace) }
- let(:group) { create(:group) }
+ let_it_be(:system_hook) { create(:system_hook) }
+ let_it_be(:user) { create(:user) }
+ let(:project) { build(:project, namespace: user.namespace) }
+ let(:group) { build(:group) }
let(:params) do
{ name: 'John Doe', username: 'jduser', email: 'jg@example.com', password: User.random_password }
end
@@ -145,6 +145,7 @@ RSpec.describe SystemHook do
end
it 'group member update hook' do
+ group = create(:group)
group.add_guest(user)
group.add_maintainer(user)
diff --git a/spec/models/hooks/web_hook_log_spec.rb b/spec/models/hooks/web_hook_log_spec.rb
index 3441dfda7d6..fafca144cae 100644
--- a/spec/models/hooks/web_hook_log_spec.rb
+++ b/spec/models/hooks/web_hook_log_spec.rb
@@ -12,7 +12,7 @@ RSpec.describe WebHookLog do
it { is_expected.to validate_presence_of(:web_hook) }
describe '.recent' do
- let(:hook) { create(:project_hook) }
+ let(:hook) { build(:project_hook) }
it 'does not return web hook logs that are too old' do
create(:web_hook_log, web_hook: hook, created_at: 10.days.ago)
@@ -30,8 +30,10 @@ RSpec.describe WebHookLog do
end
describe '#save' do
+ let(:hook) { build(:project_hook) }
+
context 'with basic auth credentials' do
- let(:web_hook_log) { build(:web_hook_log, url: 'http://test:123@example.com') }
+ let(:web_hook_log) { build(:web_hook_log, web_hook: hook, url: 'http://test:123@example.com') }
subject { web_hook_log.save! }
@@ -45,9 +47,9 @@ RSpec.describe WebHookLog do
end
context "with users' emails" do
- let(:author) { create(:user) }
- let(:user) { create(:user) }
- let(:web_hook_log) { create(:web_hook_log, request_data: data) }
+ let(:author) { build(:user) }
+ let(:user) { build(:user) }
+ let(:web_hook_log) { create(:web_hook_log, web_hook: hook, request_data: data) }
let(:data) do
{
user: {
@@ -93,11 +95,12 @@ RSpec.describe WebHookLog do
end
describe '.delete_batch_for' do
- let(:hook) { create(:project_hook) }
+ let_it_be(:hook) { build(:project_hook) }
+ let_it_be(:hook2) { build(:project_hook) }
- before do
+ before_all do
create_list(:web_hook_log, 3, web_hook: hook)
- create_list(:web_hook_log, 3)
+ create_list(:web_hook_log, 3, web_hook: hook2)
end
subject { described_class.delete_batch_for(hook, batch_size: batch_size) }
diff --git a/spec/models/hooks/web_hook_spec.rb b/spec/models/hooks/web_hook_spec.rb
index da8c10b67a6..db854670cc3 100644
--- a/spec/models/hooks/web_hook_spec.rb
+++ b/spec/models/hooks/web_hook_spec.rb
@@ -34,6 +34,11 @@ RSpec.describe WebHook do
it { is_expected.to allow_value({ 'x' => ('a' * 100) }).for(:url_variables) }
it { is_expected.to allow_value({ 'foo' => 'bar', 'bar' => 'baz' }).for(:url_variables) }
it { is_expected.to allow_value((1..20).to_h { ["k#{_1}", 'value'] }).for(:url_variables) }
+ it { is_expected.to allow_value({ 'MY-TOKEN' => 'bar' }).for(:url_variables) }
+ it { is_expected.to allow_value({ 'my_secr3t-token' => 'bar' }).for(:url_variables) }
+ it { is_expected.to allow_value({ 'x-y-z' => 'bar' }).for(:url_variables) }
+ it { is_expected.to allow_value({ 'x_y_z' => 'bar' }).for(:url_variables) }
+ it { is_expected.to allow_value({ 'f.o.o' => 'bar' }).for(:url_variables) }
it { is_expected.not_to allow_value([]).for(:url_variables) }
it { is_expected.not_to allow_value({ 'foo' => 1 }).for(:url_variables) }
@@ -45,6 +50,10 @@ RSpec.describe WebHook do
it { is_expected.not_to allow_value({ '' => 'foo' }).for(:url_variables) }
it { is_expected.not_to allow_value({ '1foo' => 'foo' }).for(:url_variables) }
it { is_expected.not_to allow_value((1..21).to_h { ["k#{_1}", 'value'] }).for(:url_variables) }
+ it { is_expected.not_to allow_value({ 'MY--TOKEN' => 'foo' }).for(:url_variables) }
+ it { is_expected.not_to allow_value({ 'MY__SECRET' => 'foo' }).for(:url_variables) }
+ it { is_expected.not_to allow_value({ 'x-_y' => 'foo' }).for(:url_variables) }
+ it { is_expected.not_to allow_value({ 'x..y' => 'foo' }).for(:url_variables) }
end
describe 'url' do
@@ -83,7 +92,7 @@ RSpec.describe WebHook do
subject { hook }
before do
- hook.url_variables = { 'one' => 'a', 'two' => 'b' }
+ hook.url_variables = { 'one' => 'a', 'two' => 'b', 'url' => 'http://example.com' }
end
it { is_expected.to allow_value('http://example.com').for(:url) }
@@ -92,6 +101,8 @@ RSpec.describe WebHook do
it { is_expected.to allow_value('http://example.com/{two}').for(:url) }
it { is_expected.to allow_value('http://user:s3cret@example.com/{two}').for(:url) }
it { is_expected.to allow_value('http://{one}:{two}@example.com').for(:url) }
+ it { is_expected.to allow_value('http://{one}').for(:url) }
+ it { is_expected.to allow_value('{url}').for(:url) }
it { is_expected.not_to allow_value('http://example.com/{one}/{two}/{three}').for(:url) }
it { is_expected.not_to allow_value('http://example.com/{foo}').for(:url) }
@@ -113,24 +124,81 @@ RSpec.describe WebHook do
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!
+ before do
+ subject.branch_filter_strategy = strategy
+ end
+
+ context 'with "all branches" strategy' do
+ let(:strategy) { 'all_branches' }
+
+ it {
+ is_expected.to allow_values(
+ "good_branch_name",
+ "another/good-branch_name",
+ "good branch name",
+ "good~branchname",
+ "good_branchname(",
+ "good_branchname[",
+ ""
+ ).for(:push_events_branch_filter)
+ }
+ end
+
+ context 'with "wildcard" strategy' do
+ let(:strategy) { 'wildcard' }
+
+ it {
+ is_expected.to allow_values(
+ "good_branch_name",
+ "another/good-branch_name",
+ "good_branch_name(",
+ ""
+ ).for(:push_events_branch_filter)
+ }
+
+ it {
+ is_expected.not_to allow_values(
+ "bad branch name",
+ "bad~branchname",
+ "bad_branch_name["
+ ).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
- expect(hook.push_events_branch_filter).to eq('branch')
+ it 'stores whitespace only as empty' do
+ hook.push_events_branch_filter = ' '
+ hook.save!
+ expect(hook.push_events_branch_filter).to eq('')
+ end
end
- it 'stores whitespace only as empty' do
- hook.push_events_branch_filter = ' '
- hook.save!
+ context 'with "regex" strategy' do
+ let(:strategy) { 'regex' }
- expect(hook.push_events_branch_filter).to eq('')
+ it {
+ is_expected.to allow_values(
+ "good_branch_name",
+ "another/good-branch_name",
+ "good branch name",
+ "good~branch~name",
+ ""
+ ).for(:push_events_branch_filter)
+ }
+
+ it { is_expected.not_to allow_values("bad_branch_name(", "bad_branch_name[").for(:push_events_branch_filter) }
end
end
+
+ it "only consider these branch filter strategies are valid" do
+ expected_valid_types = %w[all_branches regex wildcard]
+ expect(described_class.branch_filter_strategies.keys).to contain_exactly(*expected_valid_types)
+ end
end
describe 'encrypted attributes' do
@@ -141,7 +209,7 @@ RSpec.describe WebHook do
describe '.web_hooks_disable_failed?' do
it 'returns true when feature is enabled for parent' do
- second_hook = build(:project_hook, project: create(:project))
+ second_hook = build(:project_hook)
stub_feature_flags(web_hooks_disable_failed: [false, second_hook.project])
expect(described_class.web_hooks_disable_failed?(hook)).to eq(false)
@@ -242,7 +310,7 @@ RSpec.describe WebHook do
end
describe '#executable?' do
- let(:web_hook) { create(:project_hook, project: project) }
+ let_it_be_with_reload(:web_hook) { create(:project_hook, project: project) }
where(:recent_failures, :not_until, :executable) do
[
@@ -389,7 +457,7 @@ RSpec.describe WebHook do
end
end
- describe 'backoff!' do
+ describe '#backoff!' do
context 'when we have not backed off before' do
it 'does not disable the hook' do
expect { hook.backoff! }.not_to change(hook, :executable?).from(true)
@@ -400,6 +468,26 @@ RSpec.describe WebHook do
end
end
+ context 'when the recent failure value is the max value of a smallint' do
+ before do
+ hook.update!(recent_failures: 32767, disabled_until: 1.hour.ago)
+ end
+
+ it 'reduces to MAX_FAILURES' do
+ expect { hook.backoff! }.to change(hook, :recent_failures).to(described_class::MAX_FAILURES)
+ end
+ end
+
+ context 'when the recent failure value is MAX_FAILURES' do
+ before do
+ hook.update!(recent_failures: described_class::MAX_FAILURES, disabled_until: 1.hour.ago)
+ end
+
+ it 'does not change recent_failures' do
+ expect { hook.backoff! }.not_to change(hook, :recent_failures)
+ end
+ end
+
context 'when we have exhausted the grace period' do
before do
hook.update!(recent_failures: described_class::FAILURE_THRESHOLD)
@@ -459,11 +547,21 @@ RSpec.describe WebHook do
end
end
- describe 'failed!' do
+ describe '#failed!' do
it 'increments the failure count' do
expect { hook.failed! }.to change(hook, :recent_failures).by(1)
end
+ context 'when the recent failure value is the max value of a smallint' do
+ before do
+ hook.update!(recent_failures: 32767)
+ end
+
+ it 'does not change recent_failures' do
+ expect { hook.failed! }.not_to change(hook, :recent_failures)
+ end
+ end
+
it 'does not update the hook if the the failure count exceeds the maximum value' do
hook.recent_failures = described_class::MAX_FAILURES
@@ -670,4 +768,14 @@ RSpec.describe WebHook do
expect { described_class.new.update_last_failure }.not_to raise_error
end
end
+
+ describe '#masked_token' do
+ it { expect(hook.masked_token).to be_nil }
+
+ context 'with a token' do
+ let(:hook) { build(:project_hook, :token, project: project) }
+
+ it { expect(hook.masked_token).to eq described_class::SECRET_MASK }
+ end
+ end
end