From 274e37195fa0bce54ff3bb04c61cdaa571fe191c Mon Sep 17 00:00:00 2001 From: Phawin Khongkhasawan Date: Tue, 23 Apr 2024 07:28:32 +0700 Subject: [PATCH 1/6] Move to test module --- lib/api/hooks/test.rb | 19 +++++++++++++++++++ lib/api/project_hooks.rb | 28 ++++------------------------ 2 files changed, 23 insertions(+), 24 deletions(-) diff --git a/lib/api/hooks/test.rb b/lib/api/hooks/test.rb index 4871955c6e02d5..f52fe1c2f84a9a 100644 --- a/lib/api/hooks/test.rb +++ b/lib/api/hooks/test.rb @@ -15,7 +15,26 @@ class Test < ::Grape::API hook.execute(data, configuration[:kind]) data end + + params do + requires :hook_id, type: Integer, desc: 'The ID of the hook' + requires :trigger, + type: String, + desc: 'The type of trigger hook', + values: (configuration[:entity] || ProjectHook).triggers.values.map(&:to_s) + end + post ":hook_id/test/:trigger" do + hook = find_hook + result = TestHooks::ProjectService.new(hook, current_user, params[:trigger]).execute + success = (200..299).cover?(result.payload[:http_status]) + if success + created! + else + render_api_error!(result.message, 422) + end + end end + # rubocop: enable API/Base end end diff --git a/lib/api/project_hooks.rb b/lib/api/project_hooks.rb index 86bd9a203ad102..e3d997b9b58c77 100644 --- a/lib/api/project_hooks.rb +++ b/lib/api/project_hooks.rb @@ -137,30 +137,10 @@ def hook_scope end end - desc 'Triggers a project hook test' do - detail 'Triggers a project hook test' - success code: 201 - failure [ - { code: 400, message: 'Bad request' }, - { code: 404, message: 'Not found' }, - { code: 422, message: 'Unprocessable entity' } - ] - end - params do - requires :trigger, - type: String, - desc: 'The type of trigger hook', - values: ProjectHook.triggers.values.map(&:to_s) - end - post ":id/hooks/:hook_id/test/:trigger" do - hook = find_hook - result = TestHooks::ProjectService.new(hook, current_user, params[:trigger]).execute - success = (200..299).cover?(result.payload[:http_status]) - if success - created! - else - render_api_error!(result.message, 422) - end + namespace ':id/hooks' do + mount ::API::Hooks::Test, with: { + entity: ProjectHook + } end end end -- GitLab From 99ec200c3ba759487a00d8c6cf5955f5fe59ac45 Mon Sep 17 00:00:00 2001 From: Phawin Khongkhasawan Date: Tue, 23 Apr 2024 20:57:46 +0700 Subject: [PATCH 2/6] Add API for trigger group test webhook Changelog: added EE: true --- doc/api/groups.md | 16 ++ ee/lib/api/group_hooks.rb | 3 + ee/spec/requests/api/group_hooks_spec.rb | 15 ++ lib/api/hooks/test.rb | 20 +- spec/requests/api/project_hooks_spec.rb | 210 +----------------- .../requests/api/hooks_shared_examples.rb | 209 +++++++++++++++++ 6 files changed, 262 insertions(+), 211 deletions(-) diff --git a/doc/api/groups.md b/doc/api/groups.md index 0c7914b7362f99..126a58f1d657c4 100644 --- a/doc/api/groups.md +++ b/doc/api/groups.md @@ -1689,6 +1689,22 @@ DELETE /groups/:id/hooks/:hook_id | `id` | integer/string | yes | The ID or [URL-encoded path of the group](rest/index.md#namespaced-path-encoding) | | `hook_id` | integer | yes | The ID of the group hook. | +### Trigger a test group hook + +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/455589) in GitLab 17.0. + +Trigger a test hook for a specified group. + +```plaintext +POST /groups/:id/hooks/:hook_id/test/:trigger +``` + +| Attribute | Type | Required | Description | +|-----------|-------------------|----------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| `hook_id` | integer | Yes | The ID of the group hook. | +| `id` | integer or string | Yes | The ID or [URL-encoded path of the group](rest/index.md#namespaced-path-encoding). | +| `trigger` | string | Yes | One of `push_events`, `tag_push_events`, `issues_events`, `confidential_issues_events`, `note_events`, `merge_requests_events`, `job_events`, `pipeline_events`, `wiki_page_events`, `releases_events`, `emoji_events`, or `resource_access_token_events`. | + ## Group Audit Events DETAILS: diff --git a/ee/lib/api/group_hooks.rb b/ee/lib/api/group_hooks.rb index d087489608da5a..fd2752806f3aa5 100644 --- a/ee/lib/api/group_hooks.rb +++ b/ee/lib/api/group_hooks.rb @@ -138,6 +138,9 @@ def hook_scope namespace ':id/hooks' do mount ::API::Hooks::UrlVariables + mount ::API::Hooks::Test, with: { + entity: GroupHook + } end end end diff --git a/ee/spec/requests/api/group_hooks_spec.rb b/ee/spec/requests/api/group_hooks_spec.rb index 42edb8f2f3990f..fd826cfe69e3b7 100644 --- a/ee/spec/requests/api/group_hooks_spec.rb +++ b/ee/spec/requests/api/group_hooks_spec.rb @@ -67,6 +67,21 @@ def event_names { push_events: true, confidential_note_events: nil } end + context 'when group does not have a project' do + it 'return error' do + post api("#{hook_uri}/test/push_events", user, admin_mode: user.admin?), params: {} + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to eq('Hook execution failed. Ensure the group has a project with commits.') + end + end + + context 'when group has a project' do + let_it_be(:project) { create(:project, :repository, group: group) } + + it_behaves_like 'test web-hook endpoint' + end + it_behaves_like 'web-hook API endpoints with branch-filter', '/projects/:id' end end diff --git a/lib/api/hooks/test.rb b/lib/api/hooks/test.rb index f52fe1c2f84a9a..ab1b62a21ded8f 100644 --- a/lib/api/hooks/test.rb +++ b/lib/api/hooks/test.rb @@ -16,6 +16,15 @@ class Test < ::Grape::API data end + desc 'Triggers a hook test' do + detail 'Triggers a hook test' + success code: 201 + failure [ + { code: 400, message: 'Bad request' }, + { code: 404, message: 'Not found' }, + { code: 422, message: 'Unprocessable entity' } + ] + end params do requires :hook_id, type: Integer, desc: 'The ID of the hook' requires :trigger, @@ -25,8 +34,16 @@ class Test < ::Grape::API end post ":hook_id/test/:trigger" do hook = find_hook - result = TestHooks::ProjectService.new(hook, current_user, params[:trigger]).execute + service = TestHooks::ProjectService.new(hook, current_user, params[:trigger]) + + if configuration[:entity] == GroupHook + render_api_error!(_('Hook execution failed. Ensure the group has a project with commits.'), 400) unless @group.first_non_empty_project # rubocop:disable Layout/LineLength -- Error is just too long. + service.project = @group.first_non_empty_project + end + + result = service.execute success = (200..299).cover?(result.payload[:http_status]) + if success created! else @@ -34,7 +51,6 @@ class Test < ::Grape::API end end end - # rubocop: enable API/Base end end diff --git a/spec/requests/api/project_hooks_spec.rb b/spec/requests/api/project_hooks_spec.rb index 0395a12da8a4be..3470322bd75857 100644 --- a/spec/requests/api/project_hooks_spec.rb +++ b/spec/requests/api/project_hooks_spec.rb @@ -3,7 +3,6 @@ require 'spec_helper' RSpec.describe API::ProjectHooks, 'ProjectHooks', feature_category: :webhooks do - include StubRequests let_it_be(:user) { create(:user) } let_it_be(:user3) { create(:user) } let_it_be_with_reload(:project) do @@ -67,214 +66,7 @@ def event_names { push_events: true, confidential_note_events: nil } end - context "when trigger project webhook test", :aggregate_failures do - using RSpec::Parameterized::TableSyntax - - before do - stub_full_request(hook.url, method: :post).to_return(status: 200) - end - - context 'when testing is not available for trigger' do - where(:trigger_name) do - %w[confidential_note_events deployment_events feature_flag_events] - end - with_them do - it 'returns error message that testing is not available' do - post api("#{hook_uri}/test/#{trigger_name}", user, admin_mode: user.admin?), params: {} - - expect(response).to have_gitlab_http_status(:unprocessable_entity) - expect(json_response['message']).to eq('Testing not available for this hook') - end - end - end - - context 'when push_events' do - where(:trigger_name) do - [ - ["push_events"], - ["tag_push_events"] - ] - end - with_them do - it 'executes hook' do - post api("#{hook_uri}/test/#{trigger_name}", user, admin_mode: user.admin?), params: {} - - expect(response).to have_gitlab_http_status(:created) - end - end - end - - context 'when issue_events' do - where(:trigger_name) do - %w[issues_events confidential_issues_events] - end - with_them do - it 'executes hook' do - create(:issue, project: project) - - post api("#{hook_uri}/test/#{trigger_name}", user, admin_mode: user.admin?), params: {} - - expect(response).to have_gitlab_http_status(:created) - end - - it 'returns error message if not enough data' do - post api("#{hook_uri}/test/#{trigger_name}", user, admin_mode: user.admin?), params: {} - - expect(response).to have_gitlab_http_status(:unprocessable_entity) - expect(json_response['message']).to eq('Ensure the project has issues.') - end - end - end - - context 'when note_events' do - where(:trigger_name) do - [ - ["note_events"], - ["emoji_events"] - ] - end - with_them do - it 'executes hook' do - create(:note, :on_issue, project: project) - - post api("#{hook_uri}/test/#{trigger_name}", user, admin_mode: user.admin?), params: {} - - expect(response).to have_gitlab_http_status(:created) - end - - it 'returns error message if not enough data' do - post api("#{hook_uri}/test/#{trigger_name}", user, admin_mode: user.admin?), params: {} - - expect(response).to have_gitlab_http_status(:unprocessable_entity) - expect(json_response['message']).to eq('Ensure the project has notes.') - end - end - end - - context 'when merge_request_events' do - let(:trigger_name) { 'merge_requests_events' } - - it 'executes hook' do - create(:merge_request, source_project: project, target_project: project) - - post api("#{hook_uri}/test/#{trigger_name}", user, admin_mode: user.admin?), params: {} - - expect(response).to have_gitlab_http_status(:created) - end - - it 'returns error message if not enough data' do - post api("#{hook_uri}/test/#{trigger_name}", user, admin_mode: user.admin?), params: {} - - expect(response).to have_gitlab_http_status(:unprocessable_entity) - expect(json_response['message']).to eq('Ensure the project has merge requests.') - end - end - - context 'when job_events' do - let(:trigger_name) { 'job_events' } - - it 'executes hook' do - create(:ci_build, project: project, name: 'build') - - post api("#{hook_uri}/test/#{trigger_name}", user, admin_mode: user.admin?), params: {} - - expect(response).to have_gitlab_http_status(:created) - end - - it 'returns error message if not enough data' do - post api("#{hook_uri}/test/#{trigger_name}", user, admin_mode: user.admin?), params: {} - - expect(response).to have_gitlab_http_status(:unprocessable_entity) - expect(json_response['message']).to eq('Ensure the project has CI jobs.') - end - end - - context 'when pipeline_events' do - let(:trigger_name) { 'pipeline_events' } - - it 'executes hook' do - create(:ci_pipeline, project: project, user: user) - - post api("#{hook_uri}/test/#{trigger_name}", user, admin_mode: user.admin?), params: {} - - expect(response).to have_gitlab_http_status(:created) - end - - it 'returns error message if not enough data' do - post api("#{hook_uri}/test/#{trigger_name}", user, admin_mode: user.admin?), params: {} - - expect(response).to have_gitlab_http_status(:unprocessable_entity) - expect(json_response['message']).to eq('Ensure the project has CI pipelines.') - end - end - - context 'when wiki_page_events' do - let(:trigger_name) { 'wiki_page_events' } - - it 'executes hook' do - create(:wiki_page, wiki: project.wiki) - - post api("#{hook_uri}/test/#{trigger_name}", user, admin_mode: user.admin?), params: {} - - expect(response).to have_gitlab_http_status(:created) - end - - it 'returns error message if wiki is not enabled' do - project.project_feature.update!(wiki_access_level: ProjectFeature::DISABLED) - - post api("#{hook_uri}/test/#{trigger_name}", user, admin_mode: user.admin?), params: {} - - expect(response).to have_gitlab_http_status(:unprocessable_entity) - expect(json_response['message']).to eq('Ensure the wiki is enabled and has pages.') - end - end - - context 'when release_events' do - let(:trigger_name) { 'releases_events' } - - it 'executes hook' do - create(:release, project: project) - - post api("#{hook_uri}/test/#{trigger_name}", user, admin_mode: user.admin?), params: {} - - expect(response).to have_gitlab_http_status(:created) - end - - it 'returns error message if not enough data' do - post api("#{hook_uri}/test/#{trigger_name}", user, admin_mode: user.admin?), params: {} - - expect(response).to have_gitlab_http_status(:unprocessable_entity) - expect(json_response['message']).to eq('Ensure the project has releases.') - end - end - - context 'when resource_access_token_events' do - let(:trigger_name) { 'resource_access_token_events' } - - it 'executes hook' do - post api("#{hook_uri}/test/#{trigger_name}", user, admin_mode: user.admin?), params: {} - - expect(response).to have_gitlab_http_status(:created) - end - end - - it "returns a 400 error when trigger is invalid" do - post api("#{hook_uri}/test/xyz", user, admin_mode: user.admin?), params: {} - - expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['error']).to eq('trigger does not have a valid value') - end - - it "returns a 422 error when request trigger test is not successful" do - stub_full_request(hook.url, method: :post).to_return(status: 400, body: 'Error response') - - post api("#{hook_uri}/test/push_events", user, admin_mode: user.admin?), params: {} - - expect(response).to have_gitlab_http_status(:unprocessable_entity) - expect(json_response['message']).to eq('Error response') - end - end - + it_behaves_like 'test web-hook endpoint' it_behaves_like 'web-hook API endpoints with branch-filter', '/projects/:id' end end diff --git a/spec/support/shared_examples/requests/api/hooks_shared_examples.rb b/spec/support/shared_examples/requests/api/hooks_shared_examples.rb index de458bc87db28c..d46e812363e6a5 100644 --- a/spec/support/shared_examples/requests/api/hooks_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/hooks_shared_examples.rb @@ -416,3 +416,212 @@ def hook_param_overrides end end end + +RSpec.shared_examples 'test web-hook endpoint' do + include StubRequests + context "when trigger project webhook test", :aggregate_failures do + before do + stub_full_request(hook.url, method: :post).to_return(status: 200) + end + + context 'when testing is not available for trigger' do + where(:trigger_name) do + %w[confidential_note_events deployment_events feature_flag_events] + end + with_them do + it 'returns error message that testing is not available' do + post api("#{hook_uri}/test/#{trigger_name}", user, admin_mode: user.admin?), params: {} + + expect(response).to have_gitlab_http_status(:unprocessable_entity) + expect(json_response['message']).to eq('Testing not available for this hook') + end + end + end + + context 'when push_events' do + where(:trigger_name) do + [ + ["push_events"], + ["tag_push_events"] + ] + end + with_them do + it 'executes hook' do + post api("#{hook_uri}/test/#{trigger_name}", user, admin_mode: user.admin?), params: {} + + expect(response).to have_gitlab_http_status(:created) + end + end + end + + context 'when issue_events' do + where(:trigger_name) do + %w[issues_events confidential_issues_events] + end + with_them do + it 'executes hook' do + create(:issue, project: project) + + post api("#{hook_uri}/test/#{trigger_name}", user, admin_mode: user.admin?), params: {} + + expect(response).to have_gitlab_http_status(:created) + end + + it 'returns error message if not enough data' do + post api("#{hook_uri}/test/#{trigger_name}", user, admin_mode: user.admin?), params: {} + + expect(response).to have_gitlab_http_status(:unprocessable_entity) + expect(json_response['message']).to eq('Ensure the project has issues.') + end + end + end + + context 'when note_events' do + where(:trigger_name) do + [ + ["note_events"], + ["emoji_events"] + ] + end + with_them do + it 'executes hook' do + create(:note, :on_issue, project: project) + + post api("#{hook_uri}/test/#{trigger_name}", user, admin_mode: user.admin?), params: {} + + expect(response).to have_gitlab_http_status(:created) + end + + it 'returns error message if not enough data' do + post api("#{hook_uri}/test/#{trigger_name}", user, admin_mode: user.admin?), params: {} + + expect(response).to have_gitlab_http_status(:unprocessable_entity) + expect(json_response['message']).to eq('Ensure the project has notes.') + end + end + end + + context 'when merge_request_events' do + let(:trigger_name) { 'merge_requests_events' } + + it 'executes hook' do + create(:merge_request, source_project: project, target_project: project) + + post api("#{hook_uri}/test/#{trigger_name}", user, admin_mode: user.admin?), params: {} + + expect(response).to have_gitlab_http_status(:created) + end + + it 'returns error message if not enough data' do + post api("#{hook_uri}/test/#{trigger_name}", user, admin_mode: user.admin?), params: {} + + expect(response).to have_gitlab_http_status(:unprocessable_entity) + expect(json_response['message']).to eq('Ensure the project has merge requests.') + end + end + + context 'when job_events' do + let(:trigger_name) { 'job_events' } + + it 'executes hook' do + create(:ci_build, project: project, name: 'build') + + post api("#{hook_uri}/test/#{trigger_name}", user, admin_mode: user.admin?), params: {} + + expect(response).to have_gitlab_http_status(:created) + end + + it 'returns error message if not enough data' do + post api("#{hook_uri}/test/#{trigger_name}", user, admin_mode: user.admin?), params: {} + + expect(response).to have_gitlab_http_status(:unprocessable_entity) + expect(json_response['message']).to eq('Ensure the project has CI jobs.') + end + end + + context 'when pipeline_events' do + let(:trigger_name) { 'pipeline_events' } + + it 'executes hook' do + create(:ci_pipeline, project: project, user: user) + + post api("#{hook_uri}/test/#{trigger_name}", user, admin_mode: user.admin?), params: {} + + expect(response).to have_gitlab_http_status(:created) + end + + it 'returns error message if not enough data' do + post api("#{hook_uri}/test/#{trigger_name}", user, admin_mode: user.admin?), params: {} + + expect(response).to have_gitlab_http_status(:unprocessable_entity) + expect(json_response['message']).to eq('Ensure the project has CI pipelines.') + end + end + + context 'when wiki_page_events' do + let(:trigger_name) { 'wiki_page_events' } + + it 'executes hook' do + create(:wiki_page, wiki: project.wiki) + + post api("#{hook_uri}/test/#{trigger_name}", user, admin_mode: user.admin?), params: {} + + expect(response).to have_gitlab_http_status(:created) + end + + it 'returns error message if wiki is not enabled' do + project.project_feature.update!(wiki_access_level: ProjectFeature::DISABLED) + + post api("#{hook_uri}/test/#{trigger_name}", user, admin_mode: user.admin?), params: {} + + expect(response).to have_gitlab_http_status(:unprocessable_entity) + expect(json_response['message']).to eq('Ensure the wiki is enabled and has pages.') + end + end + + context 'when release_events' do + let(:trigger_name) { 'releases_events' } + + it 'executes hook' do + create(:release, project: project) + + post api("#{hook_uri}/test/#{trigger_name}", user, admin_mode: user.admin?), params: {} + + expect(response).to have_gitlab_http_status(:created) + end + + it 'returns error message if not enough data' do + post api("#{hook_uri}/test/#{trigger_name}", user, admin_mode: user.admin?), params: {} + + expect(response).to have_gitlab_http_status(:unprocessable_entity) + expect(json_response['message']).to eq('Ensure the project has releases.') + end + end + + context 'when resource_access_token_events' do + let(:trigger_name) { 'resource_access_token_events' } + + it 'executes hook' do + post api("#{hook_uri}/test/#{trigger_name}", user, admin_mode: user.admin?), params: {} + + expect(response).to have_gitlab_http_status(:created) + end + end + + it "returns a 400 error when trigger is invalid" do + post api("#{hook_uri}/test/xyz", user, admin_mode: user.admin?), params: {} + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['error']).to eq('trigger does not have a valid value') + end + + it "returns a 422 error when request trigger test is not successful" do + stub_full_request(hook.url, method: :post).to_return(status: 400, body: 'Error response') + + post api("#{hook_uri}/test/push_events", user, admin_mode: user.admin?), params: {} + + expect(response).to have_gitlab_http_status(:unprocessable_entity) + expect(json_response['message']).to eq('Error response') + end + end +end -- GitLab From 66e25b0669dbcdc5a2ef8b4a694885f32114e54a Mon Sep 17 00:00:00 2001 From: Phawin Khongkhasawan Date: Wed, 1 May 2024 14:55:19 +0700 Subject: [PATCH 3/6] Add Group hook --- doc/api/groups.md | 9 + ee/lib/api/group_hooks.rb | 2 +- lib/api/hooks/test.rb | 35 --- lib/api/hooks/trigger_test.rb | 49 ++++ lib/api/project_hooks.rb | 34 +-- spec/requests/api/project_hooks_spec.rb | 233 ------------------ .../requests/api/hooks_shared_examples.rb | 27 +- 7 files changed, 87 insertions(+), 302 deletions(-) create mode 100644 lib/api/hooks/trigger_test.rb diff --git a/doc/api/groups.md b/doc/api/groups.md index c34fde6119e3c7..bd269f62f43f23 100644 --- a/doc/api/groups.md +++ b/doc/api/groups.md @@ -1631,9 +1631,14 @@ DELETE /groups/:id/hooks/:hook_id ### Trigger a test group hook > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/455589) in GitLab 17.0. +> - Special rate limit [introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/150486) in GitLab 17.0 [with a flag](../administration/feature_flags.md) named `web_hook_test_api_endpoint_rate_limit`. Enabled by default. Trigger a test hook for a specified group. +In GitLab 17.0 and later, this endpoint has a special rate limit of three requests per minute per group hook. +To disable this limit on self-managed GitLab and GitLab Dedicated, an administrator can +[disable the feature flag](../administration/feature_flags.md) named `web_hook_test_api_endpoint_rate_limit`. + ```plaintext POST /groups/:id/hooks/:hook_id/test/:trigger ``` @@ -1644,6 +1649,10 @@ POST /groups/:id/hooks/:hook_id/test/:trigger | `id` | integer or string | Yes | The ID or [URL-encoded path of the group](rest/index.md#namespaced-path-encoding). | | `trigger` | string | Yes | One of `push_events`, `tag_push_events`, `issues_events`, `confidential_issues_events`, `note_events`, `merge_requests_events`, `job_events`, `pipeline_events`, `wiki_page_events`, `releases_events`, `emoji_events`, or `resource_access_token_events`. | +```json +{"message":"201 Created"} +``` + ## Group Audit Events DETAILS: diff --git a/ee/lib/api/group_hooks.rb b/ee/lib/api/group_hooks.rb index cd2e7c61fa18b0..aed421a4406e51 100644 --- a/ee/lib/api/group_hooks.rb +++ b/ee/lib/api/group_hooks.rb @@ -141,7 +141,7 @@ def hook_scope namespace ':id/hooks' do mount ::API::Hooks::UrlVariables - mount ::API::Hooks::Test, with: { + mount ::API::Hooks::TriggerTest, with: { entity: GroupHook } end diff --git a/lib/api/hooks/test.rb b/lib/api/hooks/test.rb index ab1b62a21ded8f..4871955c6e02d5 100644 --- a/lib/api/hooks/test.rb +++ b/lib/api/hooks/test.rb @@ -15,41 +15,6 @@ class Test < ::Grape::API hook.execute(data, configuration[:kind]) data end - - desc 'Triggers a hook test' do - detail 'Triggers a hook test' - success code: 201 - failure [ - { code: 400, message: 'Bad request' }, - { code: 404, message: 'Not found' }, - { code: 422, message: 'Unprocessable entity' } - ] - end - params do - requires :hook_id, type: Integer, desc: 'The ID of the hook' - requires :trigger, - type: String, - desc: 'The type of trigger hook', - values: (configuration[:entity] || ProjectHook).triggers.values.map(&:to_s) - end - post ":hook_id/test/:trigger" do - hook = find_hook - service = TestHooks::ProjectService.new(hook, current_user, params[:trigger]) - - if configuration[:entity] == GroupHook - render_api_error!(_('Hook execution failed. Ensure the group has a project with commits.'), 400) unless @group.first_non_empty_project # rubocop:disable Layout/LineLength -- Error is just too long. - service.project = @group.first_non_empty_project - end - - result = service.execute - success = (200..299).cover?(result.payload[:http_status]) - - if success - created! - else - render_api_error!(result.message, 422) - end - end end # rubocop: enable API/Base end diff --git a/lib/api/hooks/trigger_test.rb b/lib/api/hooks/trigger_test.rb new file mode 100644 index 00000000000000..acae7ee319b6d6 --- /dev/null +++ b/lib/api/hooks/trigger_test.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +module API + module Hooks + # rubocop: disable API/Base -- re-usable module + class TriggerTest < ::Grape::API + desc 'Triggers a hook test' do + detail 'Triggers a hook test' + success code: 201 + failure [ + { code: 400, message: 'Bad request' }, + { code: 404, message: 'Not found' }, + { code: 422, message: 'Unprocessable entity' }, + { code: 429, message: 'Too many requests' } + ] + end + params do + requires :hook_id, type: Integer, desc: 'The ID of the hook' + requires :trigger, + type: String, + desc: 'The type of trigger hook', + values: (configuration[:entity] || ProjectHook).triggers.values.map(&:to_s) + end + post ":hook_id/test/:trigger", urgency: :low do + hook = find_hook + service = TestHooks::ProjectService.new(hook, current_user, params[:trigger]) + + if Feature.enabled?(:web_hook_test_api_endpoint_rate_limit) + check_rate_limit!(:web_hook_test_api_endpoint, scope: hook) + end + + if configuration[:entity] == GroupHook + render_api_error!(_('Hook execution failed. Ensure the group has a project with commits.'), 400) unless @group.first_non_empty_project # rubocop:disable Layout/LineLength -- Error is just too long. + service.project = @group.first_non_empty_project + end + + result = service.execute + success = (200..299).cover?(result.payload[:http_status]) + + if success + created! + else + render_api_error!(result.message, 422) + end + end + end + # rubocop: enable API/Base + end +end diff --git a/lib/api/project_hooks.rb b/lib/api/project_hooks.rb index f7df54b4e3fb8f..7239db52a0cca1 100644 --- a/lib/api/project_hooks.rb +++ b/lib/api/project_hooks.rb @@ -140,40 +140,10 @@ def hook_scope end end - namespace ':id/hooks' do - mount ::API::Hooks::Test, with: { + namespace ':id/hooks/' do + mount ::API::Hooks::TriggerTest, with: { entity: ProjectHook } - desc 'Triggers a project hook test' do - detail 'Triggers a project hook test' - success code: 201 - failure [ - { code: 400, message: 'Bad request' }, - { code: 404, message: 'Not found' }, - { code: 422, message: 'Unprocessable entity' }, - { code: 429, message: 'Too many requests' } - ] - end - params do - requires :trigger, - type: String, - desc: 'The type of trigger hook', - values: ProjectHook.triggers.values.map(&:to_s) - end - post ":id/hooks/:hook_id/test/:trigger", urgency: :low do - hook = find_hook - - if Feature.enabled?(:web_hook_test_api_endpoint_rate_limit) - check_rate_limit!(:web_hook_test_api_endpoint, scope: hook) - end - - result = TestHooks::ProjectService.new(hook, current_user, params[:trigger]).execute - success = (200..299).cover?(result.payload[:http_status]) - if success - created! - else - render_api_error!(result.message, 422) - end end end end diff --git a/spec/requests/api/project_hooks_spec.rb b/spec/requests/api/project_hooks_spec.rb index 4a69e426414212..3470322bd75857 100644 --- a/spec/requests/api/project_hooks_spec.rb +++ b/spec/requests/api/project_hooks_spec.rb @@ -67,239 +67,6 @@ def event_names end it_behaves_like 'test web-hook endpoint' - context "when trigger project webhook test", :aggregate_failures, :clean_gitlab_redis_rate_limiting do - using RSpec::Parameterized::TableSyntax - - before do - stub_full_request(hook.url, method: :post).to_return(status: 200) - end - - specify do - expect(post(api("#{hook_uri}/test/push_events", user), params: {})) - .to have_request_urgency(:low) - end - - it_behaves_like 'rate limited endpoint', rate_limit_key: :web_hook_test_api_endpoint do - let(:current_user) { user } - - def request - post api("#{hook_uri}/test/push_events", user), params: {} - end - - context 'when ops flag is disabled' do - before do - stub_feature_flags(web_hook_test_api_endpoint_rate_limit: false) - end - - it 'does not block the request' do - request - - expect(response).to have_gitlab_http_status(:created) - end - end - end - - context 'when testing is not available for trigger' do - where(:trigger_name) do - %w[confidential_note_events deployment_events feature_flag_events] - end - with_them do - it 'returns error message that testing is not available' do - post api("#{hook_uri}/test/#{trigger_name}", user, admin_mode: user.admin?), params: {} - - expect(response).to have_gitlab_http_status(:unprocessable_entity) - expect(json_response['message']).to eq('Testing not available for this hook') - end - end - end - - context 'when push_events' do - where(:trigger_name) do - [ - ["push_events"], - ["tag_push_events"] - ] - end - with_them do - it 'executes hook' do - post api("#{hook_uri}/test/#{trigger_name}", user, admin_mode: user.admin?), params: {} - - expect(response).to have_gitlab_http_status(:created) - end - end - end - - context 'when issue_events' do - where(:trigger_name) do - %w[issues_events confidential_issues_events] - end - with_them do - it 'executes hook' do - create(:issue, project: project) - - post api("#{hook_uri}/test/#{trigger_name}", user, admin_mode: user.admin?), params: {} - - expect(response).to have_gitlab_http_status(:created) - end - - it 'returns error message if not enough data' do - post api("#{hook_uri}/test/#{trigger_name}", user, admin_mode: user.admin?), params: {} - - expect(response).to have_gitlab_http_status(:unprocessable_entity) - expect(json_response['message']).to eq('Ensure the project has issues.') - end - end - end - - context 'when note_events' do - where(:trigger_name) do - [ - ["note_events"], - ["emoji_events"] - ] - end - with_them do - it 'executes hook' do - create(:note, :on_issue, project: project) - - post api("#{hook_uri}/test/#{trigger_name}", user, admin_mode: user.admin?), params: {} - - expect(response).to have_gitlab_http_status(:created) - end - - it 'returns error message if not enough data' do - post api("#{hook_uri}/test/#{trigger_name}", user, admin_mode: user.admin?), params: {} - - expect(response).to have_gitlab_http_status(:unprocessable_entity) - expect(json_response['message']).to eq('Ensure the project has notes.') - end - end - end - - context 'when merge_request_events' do - let(:trigger_name) { 'merge_requests_events' } - - it 'executes hook' do - create(:merge_request, source_project: project, target_project: project) - - post api("#{hook_uri}/test/#{trigger_name}", user, admin_mode: user.admin?), params: {} - - expect(response).to have_gitlab_http_status(:created) - end - - it 'returns error message if not enough data' do - post api("#{hook_uri}/test/#{trigger_name}", user, admin_mode: user.admin?), params: {} - - expect(response).to have_gitlab_http_status(:unprocessable_entity) - expect(json_response['message']).to eq('Ensure the project has merge requests.') - end - end - - context 'when job_events' do - let(:trigger_name) { 'job_events' } - - it 'executes hook' do - create(:ci_build, project: project, name: 'build') - - post api("#{hook_uri}/test/#{trigger_name}", user, admin_mode: user.admin?), params: {} - - expect(response).to have_gitlab_http_status(:created) - end - - it 'returns error message if not enough data' do - post api("#{hook_uri}/test/#{trigger_name}", user, admin_mode: user.admin?), params: {} - - expect(response).to have_gitlab_http_status(:unprocessable_entity) - expect(json_response['message']).to eq('Ensure the project has CI jobs.') - end - end - - context 'when pipeline_events' do - let(:trigger_name) { 'pipeline_events' } - - it 'executes hook' do - create(:ci_pipeline, project: project, user: user) - - post api("#{hook_uri}/test/#{trigger_name}", user, admin_mode: user.admin?), params: {} - - expect(response).to have_gitlab_http_status(:created) - end - - it 'returns error message if not enough data' do - post api("#{hook_uri}/test/#{trigger_name}", user, admin_mode: user.admin?), params: {} - - expect(response).to have_gitlab_http_status(:unprocessable_entity) - expect(json_response['message']).to eq('Ensure the project has CI pipelines.') - end - end - - context 'when wiki_page_events' do - let(:trigger_name) { 'wiki_page_events' } - - it 'executes hook' do - create(:wiki_page, wiki: project.wiki) - - post api("#{hook_uri}/test/#{trigger_name}", user, admin_mode: user.admin?), params: {} - - expect(response).to have_gitlab_http_status(:created) - end - - it 'returns error message if wiki is not enabled' do - project.project_feature.update!(wiki_access_level: ProjectFeature::DISABLED) - - post api("#{hook_uri}/test/#{trigger_name}", user, admin_mode: user.admin?), params: {} - - expect(response).to have_gitlab_http_status(:unprocessable_entity) - expect(json_response['message']).to eq('Ensure the wiki is enabled and has pages.') - end - end - - context 'when release_events' do - let(:trigger_name) { 'releases_events' } - - it 'executes hook' do - create(:release, project: project) - - post api("#{hook_uri}/test/#{trigger_name}", user, admin_mode: user.admin?), params: {} - - expect(response).to have_gitlab_http_status(:created) - end - - it 'returns error message if not enough data' do - post api("#{hook_uri}/test/#{trigger_name}", user, admin_mode: user.admin?), params: {} - - expect(response).to have_gitlab_http_status(:unprocessable_entity) - expect(json_response['message']).to eq('Ensure the project has releases.') - end - end - - context 'when resource_access_token_events' do - let(:trigger_name) { 'resource_access_token_events' } - - it 'executes hook' do - post api("#{hook_uri}/test/#{trigger_name}", user, admin_mode: user.admin?), params: {} - - expect(response).to have_gitlab_http_status(:created) - end - end - - it "returns a 400 error when trigger is invalid" do - post api("#{hook_uri}/test/xyz", user, admin_mode: user.admin?), params: {} - - expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['error']).to eq('trigger does not have a valid value') - end - - it "returns a 422 error when request trigger test is not successful" do - stub_full_request(hook.url, method: :post).to_return(status: 400, body: 'Error response') - - post api("#{hook_uri}/test/push_events", user, admin_mode: user.admin?), params: {} - - expect(response).to have_gitlab_http_status(:unprocessable_entity) - expect(json_response['message']).to eq('Error response') - end - end - it_behaves_like 'web-hook API endpoints with branch-filter', '/projects/:id' end end diff --git a/spec/support/shared_examples/requests/api/hooks_shared_examples.rb b/spec/support/shared_examples/requests/api/hooks_shared_examples.rb index e0b60e9929d5b5..b00aa0f4612a31 100644 --- a/spec/support/shared_examples/requests/api/hooks_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/hooks_shared_examples.rb @@ -419,11 +419,36 @@ def hook_param_overrides RSpec.shared_examples 'test web-hook endpoint' do include StubRequests - context "when trigger project webhook test", :aggregate_failures do + context "when trigger webhook test", :aggregate_failures, :clean_gitlab_redis_rate_limiting do before do stub_full_request(hook.url, method: :post).to_return(status: 200) end + specify do + expect(post(api("#{hook_uri}/test/push_events", user), params: {})) + .to have_request_urgency(:low) + end + + it_behaves_like 'rate limited endpoint', rate_limit_key: :web_hook_test_api_endpoint do + let(:current_user) { user } + + def request + post api("#{hook_uri}/test/push_events", user), params: {} + end + + context 'when ops flag is disabled' do + before do + stub_feature_flags(web_hook_test_api_endpoint_rate_limit: false) + end + + it 'does not block the request' do + request + + expect(response).to have_gitlab_http_status(:created) + end + end + end + context 'when testing is not available for trigger' do where(:trigger_name) do %w[confidential_note_events deployment_events feature_flag_events] -- GitLab From 548b96d8e91794c58bf3c38eddc7e0bffca9fee0 Mon Sep 17 00:00:00 2001 From: Phawin Khongkhasawan Date: Tue, 7 May 2024 20:20:50 +0700 Subject: [PATCH 4/6] Apply suggestion from @.luke --- ee/app/controllers/groups/hooks_controller.rb | 8 +++---- ee/app/services/test_hooks/group_service.rb | 15 +++++++++++++ ee/lib/ee/api/hooks/trigger_test.rb | 21 +++++++++++++++++++ .../groups/hooks_controller_spec.rb | 2 +- ee/spec/requests/api/group_hooks_spec.rb | 6 +++--- lib/api/hooks/trigger_test.rb | 17 ++++++++------- locale/gitlab.pot | 2 +- .../requests/api/hooks_shared_examples.rb | 5 ----- 8 files changed, 54 insertions(+), 22 deletions(-) create mode 100644 ee/app/services/test_hooks/group_service.rb create mode 100644 ee/lib/ee/api/hooks/trigger_test.rb diff --git a/ee/app/controllers/groups/hooks_controller.rb b/ee/app/controllers/groups/hooks_controller.rb index c62d1aa271ebef..b5b1aad784a03a 100644 --- a/ee/app/controllers/groups/hooks_controller.rb +++ b/ee/app/controllers/groups/hooks_controller.rb @@ -17,14 +17,12 @@ class Groups::HooksController < Groups::ApplicationController urgency :low, [:test] def test - if @group.first_non_empty_project - service = TestHooks::ProjectService.new(hook, current_user, params[:trigger] || 'push_events') - service.project = @group.first_non_empty_project - result = service.execute + result = TestHooks::GroupService.new(hook, current_user, params[:trigger]).execute + if result.success? set_hook_execution_notice(result) else - flash[:alert] = _('Hook execution failed. Ensure the group has a project with commits.') + flash[:alert] = format(_('Hook execution failed. %{error}'), error: result.message) end redirect_back_or_default(default: { action: 'index' }) diff --git a/ee/app/services/test_hooks/group_service.rb b/ee/app/services/test_hooks/group_service.rb new file mode 100644 index 00000000000000..02f01f3ab0d314 --- /dev/null +++ b/ee/app/services/test_hooks/group_service.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +module TestHooks + class GroupService < TestHooks::BaseService + def execute + project = @hook.group.first_non_empty_project + + return error('Ensure the group has a project with commits.') unless project + + service = TestHooks::ProjectService.new(hook, current_user, @trigger || 'push_events') + service.project = project + service.execute + end + end +end diff --git a/ee/lib/ee/api/hooks/trigger_test.rb b/ee/lib/ee/api/hooks/trigger_test.rb new file mode 100644 index 00000000000000..19285e0bc77a36 --- /dev/null +++ b/ee/lib/ee/api/hooks/trigger_test.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +module EE + module API + module Hooks + module TriggerTest + extend ActiveSupport::Concern + + prepended do + helpers do + def hook_test_service(hook, entity) + return super unless entity == GroupHook + + TestHooks::GroupService.new(hook, current_user, params[:trigger]) + end + end + end + end + end + end +end diff --git a/ee/spec/controllers/groups/hooks_controller_spec.rb b/ee/spec/controllers/groups/hooks_controller_spec.rb index 8303f9ffa7d8e2..28ed8b65530621 100644 --- a/ee/spec/controllers/groups/hooks_controller_spec.rb +++ b/ee/spec/controllers/groups/hooks_controller_spec.rb @@ -203,7 +203,7 @@ def it_renders_correctly expect(response).to have_gitlab_http_status(:found) expect(flash[:notice]).to be_nil - expect(flash[:alert]).to eq('Hook execution failed: Ensure the project has CI jobs.') + expect(flash[:alert]).to eq('Hook execution failed. Ensure the project has CI jobs.') end end end diff --git a/ee/spec/requests/api/group_hooks_spec.rb b/ee/spec/requests/api/group_hooks_spec.rb index 8339b513953175..bdc2f618d14095 100644 --- a/ee/spec/requests/api/group_hooks_spec.rb +++ b/ee/spec/requests/api/group_hooks_spec.rb @@ -64,11 +64,11 @@ def event_names end context 'when group does not have a project' do - it 'return error' do + it 'returns error' do post api("#{hook_uri}/test/push_events", user, admin_mode: user.admin?), params: {} - expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['message']).to eq('Hook execution failed. Ensure the group has a project with commits.') + expect(response).to have_gitlab_http_status(:unprocessable_entity) + expect(json_response['message']).to eq('Ensure the group has a project with commits.') end end diff --git a/lib/api/hooks/trigger_test.rb b/lib/api/hooks/trigger_test.rb index acae7ee319b6d6..9a738610a380ac 100644 --- a/lib/api/hooks/trigger_test.rb +++ b/lib/api/hooks/trigger_test.rb @@ -4,6 +4,12 @@ module API module Hooks # rubocop: disable API/Base -- re-usable module class TriggerTest < ::Grape::API + helpers do + # EE::API::Hooks::TriggerTest overrides this helper + def hook_test_service(hook, _) + TestHooks::ProjectService.new(hook, current_user, params[:trigger]) + end + end desc 'Triggers a hook test' do detail 'Triggers a hook test' success code: 201 @@ -19,21 +25,16 @@ class TriggerTest < ::Grape::API requires :trigger, type: String, desc: 'The type of trigger hook', - values: (configuration[:entity] || ProjectHook).triggers.values.map(&:to_s) + values: ProjectHook.triggers.values.map(&:to_s) end post ":hook_id/test/:trigger", urgency: :low do hook = find_hook - service = TestHooks::ProjectService.new(hook, current_user, params[:trigger]) if Feature.enabled?(:web_hook_test_api_endpoint_rate_limit) check_rate_limit!(:web_hook_test_api_endpoint, scope: hook) end - if configuration[:entity] == GroupHook - render_api_error!(_('Hook execution failed. Ensure the group has a project with commits.'), 400) unless @group.first_non_empty_project # rubocop:disable Layout/LineLength -- Error is just too long. - service.project = @group.first_non_empty_project - end - + service = hook_test_service(hook, configuration[:entity]) result = service.execute success = (200..299).cover?(result.payload[:http_status]) @@ -47,3 +48,5 @@ class TriggerTest < ::Grape::API # rubocop: enable API/Base end end + +API::Hooks::TriggerTest.prepend_mod diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 96494c1ff1485e..3dc43c3ccd7021 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -25863,7 +25863,7 @@ msgstr "" msgid "Homepage" msgstr "" -msgid "Hook execution failed. Ensure the group has a project with commits." +msgid "Hook execution failed. %{error}" msgstr "" msgid "Horizontal rule" diff --git a/spec/support/shared_examples/requests/api/hooks_shared_examples.rb b/spec/support/shared_examples/requests/api/hooks_shared_examples.rb index b00aa0f4612a31..2d201964d156df 100644 --- a/spec/support/shared_examples/requests/api/hooks_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/hooks_shared_examples.rb @@ -424,11 +424,6 @@ def hook_param_overrides stub_full_request(hook.url, method: :post).to_return(status: 200) end - specify do - expect(post(api("#{hook_uri}/test/push_events", user), params: {})) - .to have_request_urgency(:low) - end - it_behaves_like 'rate limited endpoint', rate_limit_key: :web_hook_test_api_endpoint do let(:current_user) { user } -- GitLab From 4d08730e74fc3ac1e3ae543e1064747f30b7dd98 Mon Sep 17 00:00:00 2001 From: Phawin Khongkhasawan Date: Fri, 10 May 2024 02:08:08 +0000 Subject: [PATCH 5/6] Apply 2 suggestion(s) to 2 file(s) Co-authored-by: Ashraf Khamis Co-authored-by: Luke Duncalfe --- doc/api/groups.md | 2 +- lib/api/hooks/trigger_test.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/api/groups.md b/doc/api/groups.md index bd269f62f43f23..260b786e910cd9 100644 --- a/doc/api/groups.md +++ b/doc/api/groups.md @@ -1635,7 +1635,7 @@ DELETE /groups/:id/hooks/:hook_id Trigger a test hook for a specified group. -In GitLab 17.0 and later, this endpoint has a special rate limit of three requests per minute per group hook. +This endpoint has a special rate limit of three requests per minute per group hook. To disable this limit on self-managed GitLab and GitLab Dedicated, an administrator can [disable the feature flag](../administration/feature_flags.md) named `web_hook_test_api_endpoint_rate_limit`. diff --git a/lib/api/hooks/trigger_test.rb b/lib/api/hooks/trigger_test.rb index 9a738610a380ac..9c033f96279f12 100644 --- a/lib/api/hooks/trigger_test.rb +++ b/lib/api/hooks/trigger_test.rb @@ -27,7 +27,7 @@ def hook_test_service(hook, _) desc: 'The type of trigger hook', values: ProjectHook.triggers.values.map(&:to_s) end - post ":hook_id/test/:trigger", urgency: :low do + post ":hook_id/test/:trigger" do hook = find_hook if Feature.enabled?(:web_hook_test_api_endpoint_rate_limit) -- GitLab From 9a8d2d3904de610a7f5c7f713076303dbb14d7e6 Mon Sep 17 00:00:00 2001 From: Phawin Khongkhasawan Date: Mon, 13 May 2024 11:52:21 +0000 Subject: [PATCH 6/6] Apply 1 suggestion(s) to 1 file(s) Co-authored-by: Ashraf Khamis --- doc/api/groups.md | 4 ++-- lib/api/hooks/trigger_test.rb | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/api/groups.md b/doc/api/groups.md index 260b786e910cd9..958944cb3cc71b 100644 --- a/doc/api/groups.md +++ b/doc/api/groups.md @@ -1630,8 +1630,8 @@ DELETE /groups/:id/hooks/:hook_id ### Trigger a test group hook -> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/455589) in GitLab 17.0. -> - Special rate limit [introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/150486) in GitLab 17.0 [with a flag](../administration/feature_flags.md) named `web_hook_test_api_endpoint_rate_limit`. Enabled by default. +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/455589) in GitLab 17.1. +> - Special rate limit [introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/150486) in GitLab 17.1 [with a flag](../administration/feature_flags.md) named `web_hook_test_api_endpoint_rate_limit`. Enabled by default. Trigger a test hook for a specified group. diff --git a/lib/api/hooks/trigger_test.rb b/lib/api/hooks/trigger_test.rb index 9c033f96279f12..a0cf0b1203e5d7 100644 --- a/lib/api/hooks/trigger_test.rb +++ b/lib/api/hooks/trigger_test.rb @@ -30,7 +30,7 @@ def hook_test_service(hook, _) post ":hook_id/test/:trigger" do hook = find_hook - if Feature.enabled?(:web_hook_test_api_endpoint_rate_limit) + if Feature.enabled?(:web_hook_test_api_endpoint_rate_limit, Feature.current_request) check_rate_limit!(:web_hook_test_api_endpoint, scope: hook) end -- GitLab