Commit 6cd0ec11 authored by Douwe Maan's avatar Douwe Maan

Merge branch '37288-fix-wrong-header-when-testing-webhook' into 'master'

Fix a wrong `X-Gitlab-Event` header when testing webhooks

Closes #37288

See merge request !14108
parents f88659d0 9e201575
...@@ -9,18 +9,17 @@ module TestHooks ...@@ -9,18 +9,17 @@ module TestHooks
end end
def execute def execute
trigger_key = hook.class::TRIGGERS.key(trigger.to_sym)
trigger_data_method = "#{trigger}_data" trigger_data_method = "#{trigger}_data"
if !self.respond_to?(trigger_data_method, true) || if trigger_key.nil? || !self.respond_to?(trigger_data_method, true)
!hook.class::TRIGGERS.value?(trigger.to_sym)
return error('Testing not available for this hook') return error('Testing not available for this hook')
end end
error_message = catch(:validation_error) do error_message = catch(:validation_error) do
sample_data = self.__send__(trigger_data_method) # rubocop:disable GitlabSecurity/PublicSend sample_data = self.__send__(trigger_data_method) # rubocop:disable GitlabSecurity/PublicSend
return hook.execute(sample_data, trigger) return hook.execute(sample_data, trigger_key)
end end
error(error_message) error(error_message)
......
...@@ -19,7 +19,7 @@ class WebHookService ...@@ -19,7 +19,7 @@ class WebHookService
def initialize(hook, data, hook_name) def initialize(hook, data, hook_name)
@hook = hook @hook = hook
@data = data @data = data
@hook_name = hook_name @hook_name = hook_name.to_s
end end
def execute def execute
......
---
title: Fix a wrong `X-Gitlab-Event` header when testing webhooks
merge_request: 14108
author:
type: fixed
...@@ -21,6 +21,7 @@ describe TestHooks::ProjectService do ...@@ -21,6 +21,7 @@ describe TestHooks::ProjectService do
context 'push_events' do context 'push_events' do
let(:trigger) { 'push_events' } let(:trigger) { 'push_events' }
let(:trigger_key) { :push_hooks }
it 'returns error message if not enough data' do it 'returns error message if not enough data' do
allow(project).to receive(:empty_repo?).and_return(true) allow(project).to receive(:empty_repo?).and_return(true)
...@@ -33,13 +34,14 @@ describe TestHooks::ProjectService do ...@@ -33,13 +34,14 @@ describe TestHooks::ProjectService do
allow(project).to receive(:empty_repo?).and_return(false) allow(project).to receive(:empty_repo?).and_return(false)
allow(Gitlab::DataBuilder::Push).to receive(:build_sample).and_return(sample_data) allow(Gitlab::DataBuilder::Push).to receive(:build_sample).and_return(sample_data)
expect(hook).to receive(:execute).with(sample_data, trigger).and_return(success_result) expect(hook).to receive(:execute).with(sample_data, trigger_key).and_return(success_result)
expect(service.execute).to include(success_result) expect(service.execute).to include(success_result)
end end
end end
context 'tag_push_events' do context 'tag_push_events' do
let(:trigger) { 'tag_push_events' } let(:trigger) { 'tag_push_events' }
let(:trigger_key) { :tag_push_hooks }
it 'returns error message if not enough data' do it 'returns error message if not enough data' do
allow(project).to receive(:empty_repo?).and_return(true) allow(project).to receive(:empty_repo?).and_return(true)
...@@ -52,13 +54,14 @@ describe TestHooks::ProjectService do ...@@ -52,13 +54,14 @@ describe TestHooks::ProjectService do
allow(project).to receive(:empty_repo?).and_return(false) allow(project).to receive(:empty_repo?).and_return(false)
allow(Gitlab::DataBuilder::Push).to receive(:build_sample).and_return(sample_data) allow(Gitlab::DataBuilder::Push).to receive(:build_sample).and_return(sample_data)
expect(hook).to receive(:execute).with(sample_data, trigger).and_return(success_result) expect(hook).to receive(:execute).with(sample_data, trigger_key).and_return(success_result)
expect(service.execute).to include(success_result) expect(service.execute).to include(success_result)
end end
end end
context 'note_events' do context 'note_events' do
let(:trigger) { 'note_events' } let(:trigger) { 'note_events' }
let(:trigger_key) { :note_hooks }
it 'returns error message if not enough data' do it 'returns error message if not enough data' do
expect(hook).not_to receive(:execute) expect(hook).not_to receive(:execute)
...@@ -69,13 +72,14 @@ describe TestHooks::ProjectService do ...@@ -69,13 +72,14 @@ describe TestHooks::ProjectService do
allow(project).to receive(:notes).and_return([Note.new]) allow(project).to receive(:notes).and_return([Note.new])
allow(Gitlab::DataBuilder::Note).to receive(:build).and_return(sample_data) allow(Gitlab::DataBuilder::Note).to receive(:build).and_return(sample_data)
expect(hook).to receive(:execute).with(sample_data, trigger).and_return(success_result) expect(hook).to receive(:execute).with(sample_data, trigger_key).and_return(success_result)
expect(service.execute).to include(success_result) expect(service.execute).to include(success_result)
end end
end end
context 'issues_events' do context 'issues_events' do
let(:trigger) { 'issues_events' } let(:trigger) { 'issues_events' }
let(:trigger_key) { :issue_hooks }
let(:issue) { build(:issue) } let(:issue) { build(:issue) }
it 'returns error message if not enough data' do it 'returns error message if not enough data' do
...@@ -87,13 +91,14 @@ describe TestHooks::ProjectService do ...@@ -87,13 +91,14 @@ describe TestHooks::ProjectService do
allow(project).to receive(:issues).and_return([issue]) allow(project).to receive(:issues).and_return([issue])
allow(issue).to receive(:to_hook_data).and_return(sample_data) allow(issue).to receive(:to_hook_data).and_return(sample_data)
expect(hook).to receive(:execute).with(sample_data, trigger).and_return(success_result) expect(hook).to receive(:execute).with(sample_data, trigger_key).and_return(success_result)
expect(service.execute).to include(success_result) expect(service.execute).to include(success_result)
end end
end end
context 'confidential_issues_events' do context 'confidential_issues_events' do
let(:trigger) { 'confidential_issues_events' } let(:trigger) { 'confidential_issues_events' }
let(:trigger_key) { :confidential_issue_hooks }
let(:issue) { build(:issue) } let(:issue) { build(:issue) }
it 'returns error message if not enough data' do it 'returns error message if not enough data' do
...@@ -105,13 +110,14 @@ describe TestHooks::ProjectService do ...@@ -105,13 +110,14 @@ describe TestHooks::ProjectService do
allow(project).to receive(:issues).and_return([issue]) allow(project).to receive(:issues).and_return([issue])
allow(issue).to receive(:to_hook_data).and_return(sample_data) allow(issue).to receive(:to_hook_data).and_return(sample_data)
expect(hook).to receive(:execute).with(sample_data, trigger).and_return(success_result) expect(hook).to receive(:execute).with(sample_data, trigger_key).and_return(success_result)
expect(service.execute).to include(success_result) expect(service.execute).to include(success_result)
end end
end end
context 'merge_requests_events' do context 'merge_requests_events' do
let(:trigger) { 'merge_requests_events' } let(:trigger) { 'merge_requests_events' }
let(:trigger_key) { :merge_request_hooks }
it 'returns error message if not enough data' do it 'returns error message if not enough data' do
expect(hook).not_to receive(:execute) expect(hook).not_to receive(:execute)
...@@ -122,13 +128,14 @@ describe TestHooks::ProjectService do ...@@ -122,13 +128,14 @@ describe TestHooks::ProjectService do
create(:merge_request, source_project: project) create(:merge_request, source_project: project)
allow_any_instance_of(MergeRequest).to receive(:to_hook_data).and_return(sample_data) allow_any_instance_of(MergeRequest).to receive(:to_hook_data).and_return(sample_data)
expect(hook).to receive(:execute).with(sample_data, trigger).and_return(success_result) expect(hook).to receive(:execute).with(sample_data, trigger_key).and_return(success_result)
expect(service.execute).to include(success_result) expect(service.execute).to include(success_result)
end end
end end
context 'job_events' do context 'job_events' do
let(:trigger) { 'job_events' } let(:trigger) { 'job_events' }
let(:trigger_key) { :job_hooks }
it 'returns error message if not enough data' do it 'returns error message if not enough data' do
expect(hook).not_to receive(:execute) expect(hook).not_to receive(:execute)
...@@ -139,13 +146,14 @@ describe TestHooks::ProjectService do ...@@ -139,13 +146,14 @@ describe TestHooks::ProjectService do
create(:ci_build, project: project) create(:ci_build, project: project)
allow(Gitlab::DataBuilder::Build).to receive(:build).and_return(sample_data) allow(Gitlab::DataBuilder::Build).to receive(:build).and_return(sample_data)
expect(hook).to receive(:execute).with(sample_data, trigger).and_return(success_result) expect(hook).to receive(:execute).with(sample_data, trigger_key).and_return(success_result)
expect(service.execute).to include(success_result) expect(service.execute).to include(success_result)
end end
end end
context 'pipeline_events' do context 'pipeline_events' do
let(:trigger) { 'pipeline_events' } let(:trigger) { 'pipeline_events' }
let(:trigger_key) { :pipeline_hooks }
it 'returns error message if not enough data' do it 'returns error message if not enough data' do
expect(hook).not_to receive(:execute) expect(hook).not_to receive(:execute)
...@@ -156,13 +164,14 @@ describe TestHooks::ProjectService do ...@@ -156,13 +164,14 @@ describe TestHooks::ProjectService do
create(:ci_empty_pipeline, project: project) create(:ci_empty_pipeline, project: project)
allow(Gitlab::DataBuilder::Pipeline).to receive(:build).and_return(sample_data) allow(Gitlab::DataBuilder::Pipeline).to receive(:build).and_return(sample_data)
expect(hook).to receive(:execute).with(sample_data, trigger).and_return(success_result) expect(hook).to receive(:execute).with(sample_data, trigger_key).and_return(success_result)
expect(service.execute).to include(success_result) expect(service.execute).to include(success_result)
end end
end end
context 'wiki_page_events' do context 'wiki_page_events' do
let(:trigger) { 'wiki_page_events' } let(:trigger) { 'wiki_page_events' }
let(:trigger_key) { :wiki_page_hooks }
it 'returns error message if wiki disabled' do it 'returns error message if wiki disabled' do
allow(project).to receive(:wiki_enabled?).and_return(false) allow(project).to receive(:wiki_enabled?).and_return(false)
...@@ -180,7 +189,7 @@ describe TestHooks::ProjectService do ...@@ -180,7 +189,7 @@ describe TestHooks::ProjectService do
create(:wiki_page, wiki: project.wiki) create(:wiki_page, wiki: project.wiki)
allow(Gitlab::DataBuilder::WikiPage).to receive(:build).and_return(sample_data) allow(Gitlab::DataBuilder::WikiPage).to receive(:build).and_return(sample_data)
expect(hook).to receive(:execute).with(sample_data, trigger).and_return(success_result) expect(hook).to receive(:execute).with(sample_data, trigger_key).and_return(success_result)
expect(service.execute).to include(success_result) expect(service.execute).to include(success_result)
end end
end end
......
...@@ -24,36 +24,39 @@ describe TestHooks::SystemService do ...@@ -24,36 +24,39 @@ describe TestHooks::SystemService do
context 'push_events' do context 'push_events' do
let(:trigger) { 'push_events' } let(:trigger) { 'push_events' }
let(:trigger_key) { :push_hooks }
it 'executes hook' do it 'executes hook' do
allow(project).to receive(:empty_repo?).and_return(false) allow(project).to receive(:empty_repo?).and_return(false)
expect(Gitlab::DataBuilder::Push).to receive(:sample_data).and_call_original expect(Gitlab::DataBuilder::Push).to receive(:sample_data).and_call_original
expect(hook).to receive(:execute).with(Gitlab::DataBuilder::Push::SAMPLE_DATA, trigger).and_return(success_result) expect(hook).to receive(:execute).with(Gitlab::DataBuilder::Push::SAMPLE_DATA, trigger_key).and_return(success_result)
expect(service.execute).to include(success_result) expect(service.execute).to include(success_result)
end end
end end
context 'tag_push_events' do context 'tag_push_events' do
let(:trigger) { 'tag_push_events' } let(:trigger) { 'tag_push_events' }
let(:trigger_key) { :tag_push_hooks }
it 'executes hook' do it 'executes hook' do
allow(project.repository).to receive(:tags).and_return(['tag']) allow(project.repository).to receive(:tags).and_return(['tag'])
expect(Gitlab::DataBuilder::Push).to receive(:sample_data).and_call_original expect(Gitlab::DataBuilder::Push).to receive(:sample_data).and_call_original
expect(hook).to receive(:execute).with(Gitlab::DataBuilder::Push::SAMPLE_DATA, trigger).and_return(success_result) expect(hook).to receive(:execute).with(Gitlab::DataBuilder::Push::SAMPLE_DATA, trigger_key).and_return(success_result)
expect(service.execute).to include(success_result) expect(service.execute).to include(success_result)
end end
end end
context 'repository_update_events' do context 'repository_update_events' do
let(:trigger) { 'repository_update_events' } let(:trigger) { 'repository_update_events' }
let(:trigger_key) { :repository_update_hooks }
it 'executes hook' do it 'executes hook' do
allow(project).to receive(:empty_repo?).and_return(false) allow(project).to receive(:empty_repo?).and_return(false)
expect(Gitlab::DataBuilder::Repository).to receive(:sample_data).and_call_original expect(Gitlab::DataBuilder::Repository).to receive(:sample_data).and_call_original
expect(hook).to receive(:execute).with(Gitlab::DataBuilder::Repository::SAMPLE_DATA, trigger).and_return(success_result) expect(hook).to receive(:execute).with(Gitlab::DataBuilder::Repository::SAMPLE_DATA, trigger_key).and_return(success_result)
expect(service.execute).to include(success_result) expect(service.execute).to include(success_result)
end end
end end
......
...@@ -12,7 +12,7 @@ describe WebHookService do ...@@ -12,7 +12,7 @@ describe WebHookService do
let(:data) do let(:data) do
{ before: 'oldrev', after: 'newrev', ref: 'ref' } { before: 'oldrev', after: 'newrev', ref: 'ref' }
end end
let(:service_instance) { described_class.new(project_hook, data, 'push_hooks') } let(:service_instance) { described_class.new(project_hook, data, :push_hooks) }
describe '#execute' do describe '#execute' do
before do before 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