diff options
-rw-r--r-- | app/services/test_hooks/base_service.rb | 7 | ||||
-rw-r--r-- | app/services/web_hook_service.rb | 2 | ||||
-rw-r--r-- | changelogs/unreleased/37288-fix-wrong-header-when-testing-webhook.yml | 5 | ||||
-rw-r--r-- | spec/services/test_hooks/project_service_spec.rb | 27 | ||||
-rw-r--r-- | spec/services/test_hooks/system_service_spec.rb | 9 | ||||
-rw-r--r-- | spec/services/web_hook_service_spec.rb | 2 |
6 files changed, 34 insertions, 18 deletions
diff --git a/app/services/test_hooks/base_service.rb b/app/services/test_hooks/base_service.rb index 4abd2c44b2f..20d90504bd2 100644 --- a/app/services/test_hooks/base_service.rb +++ b/app/services/test_hooks/base_service.rb @@ -9,18 +9,17 @@ module TestHooks end def execute + trigger_key = hook.class::TRIGGERS.key(trigger.to_sym) trigger_data_method = "#{trigger}_data" - if !self.respond_to?(trigger_data_method, true) || - !hook.class::TRIGGERS.value?(trigger.to_sym) - + if trigger_key.nil? || !self.respond_to?(trigger_data_method, true) return error('Testing not available for this hook') end error_message = catch(:validation_error) do 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 error(error_message) diff --git a/app/services/web_hook_service.rb b/app/services/web_hook_service.rb index 2825478926a..cd99e0b90f9 100644 --- a/app/services/web_hook_service.rb +++ b/app/services/web_hook_service.rb @@ -19,7 +19,7 @@ class WebHookService def initialize(hook, data, hook_name) @hook = hook @data = data - @hook_name = hook_name + @hook_name = hook_name.to_s end def execute diff --git a/changelogs/unreleased/37288-fix-wrong-header-when-testing-webhook.yml b/changelogs/unreleased/37288-fix-wrong-header-when-testing-webhook.yml new file mode 100644 index 00000000000..d6d21ac4c51 --- /dev/null +++ b/changelogs/unreleased/37288-fix-wrong-header-when-testing-webhook.yml @@ -0,0 +1,5 @@ +--- +title: Fix a wrong `X-Gitlab-Event` header when testing webhooks +merge_request: 14108 +author: +type: fixed diff --git a/spec/services/test_hooks/project_service_spec.rb b/spec/services/test_hooks/project_service_spec.rb index 4218c15a3ce..28dfa9cf59c 100644 --- a/spec/services/test_hooks/project_service_spec.rb +++ b/spec/services/test_hooks/project_service_spec.rb @@ -21,6 +21,7 @@ describe TestHooks::ProjectService do context 'push_events' do let(:trigger) { 'push_events' } + let(:trigger_key) { :push_hooks } it 'returns error message if not enough data' do allow(project).to receive(:empty_repo?).and_return(true) @@ -33,13 +34,14 @@ describe TestHooks::ProjectService do allow(project).to receive(:empty_repo?).and_return(false) 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) end end context 'tag_push_events' do let(:trigger) { 'tag_push_events' } + let(:trigger_key) { :tag_push_hooks } it 'returns error message if not enough data' do allow(project).to receive(:empty_repo?).and_return(true) @@ -52,13 +54,14 @@ describe TestHooks::ProjectService do allow(project).to receive(:empty_repo?).and_return(false) 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) end end context 'note_events' do let(:trigger) { 'note_events' } + let(:trigger_key) { :note_hooks } it 'returns error message if not enough data' do expect(hook).not_to receive(:execute) @@ -69,13 +72,14 @@ describe TestHooks::ProjectService do allow(project).to receive(:notes).and_return([Note.new]) 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) end end context 'issues_events' do let(:trigger) { 'issues_events' } + let(:trigger_key) { :issue_hooks } let(:issue) { build(:issue) } it 'returns error message if not enough data' do @@ -87,13 +91,14 @@ describe TestHooks::ProjectService do allow(project).to receive(:issues).and_return([issue]) 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) end end context 'confidential_issues_events' do let(:trigger) { 'confidential_issues_events' } + let(:trigger_key) { :confidential_issue_hooks } let(:issue) { build(:issue) } it 'returns error message if not enough data' do @@ -105,13 +110,14 @@ describe TestHooks::ProjectService do allow(project).to receive(:issues).and_return([issue]) 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) end end context 'merge_requests_events' do let(:trigger) { 'merge_requests_events' } + let(:trigger_key) { :merge_request_hooks } it 'returns error message if not enough data' do expect(hook).not_to receive(:execute) @@ -122,13 +128,14 @@ describe TestHooks::ProjectService do create(:merge_request, source_project: project) 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) end end context 'job_events' do let(:trigger) { 'job_events' } + let(:trigger_key) { :job_hooks } it 'returns error message if not enough data' do expect(hook).not_to receive(:execute) @@ -139,13 +146,14 @@ describe TestHooks::ProjectService do create(:ci_build, project: project) 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) end end context 'pipeline_events' do let(:trigger) { 'pipeline_events' } + let(:trigger_key) { :pipeline_hooks } it 'returns error message if not enough data' do expect(hook).not_to receive(:execute) @@ -156,13 +164,14 @@ describe TestHooks::ProjectService do create(:ci_empty_pipeline, project: project) 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) end end context 'wiki_page_events' do let(:trigger) { 'wiki_page_events' } + let(:trigger_key) { :wiki_page_hooks } it 'returns error message if wiki disabled' do allow(project).to receive(:wiki_enabled?).and_return(false) @@ -180,7 +189,7 @@ describe TestHooks::ProjectService do create(:wiki_page, wiki: project.wiki) 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) end end diff --git a/spec/services/test_hooks/system_service_spec.rb b/spec/services/test_hooks/system_service_spec.rb index a15708bf82f..ff8b9595538 100644 --- a/spec/services/test_hooks/system_service_spec.rb +++ b/spec/services/test_hooks/system_service_spec.rb @@ -24,36 +24,39 @@ describe TestHooks::SystemService do context 'push_events' do let(:trigger) { 'push_events' } + let(:trigger_key) { :push_hooks } it 'executes hook' do allow(project).to receive(:empty_repo?).and_return(false) 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) end end context 'tag_push_events' do let(:trigger) { 'tag_push_events' } + let(:trigger_key) { :tag_push_hooks } it 'executes hook' do allow(project.repository).to receive(:tags).and_return(['tag']) 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) end end context 'repository_update_events' do let(:trigger) { 'repository_update_events' } + let(:trigger_key) { :repository_update_hooks } it 'executes hook' do allow(project).to receive(:empty_repo?).and_return(false) 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) end end diff --git a/spec/services/web_hook_service_spec.rb b/spec/services/web_hook_service_spec.rb index 0726e135b20..a669429ce3e 100644 --- a/spec/services/web_hook_service_spec.rb +++ b/spec/services/web_hook_service_spec.rb @@ -12,7 +12,7 @@ describe WebHookService do let(:data) do { before: 'oldrev', after: 'newrev', ref: 'ref' } 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 before do |