From f8cfcb4fc3425636f4aede663078ae2caa503969 Mon Sep 17 00:00:00 2001 From: Niklas <mc.taucher2003@gmail.com> Date: Fri, 16 Feb 2024 18:13:53 +0000 Subject: [PATCH] Add custom payload template to webhooks Changelog: added --- .../concerns/web_hooks/hook_actions.rb | 3 +- app/models/hooks/web_hook.rb | 1 + app/services/web_hook_service.rb | 41 ++++++++-- app/views/shared/web_hooks/_form.html.haml | 7 ++ .../beta/custom_webhook_template.yml | 9 ++ ...add_custom_webhook_template_to_web_hook.rb | 12 +++ ...it_to_web_hooks_custom_webhook_template.rb | 14 ++++ db/schema_migrations/20240213181406 | 1 + db/schema_migrations/20240213181407 | 1 + db/structure.sql | 4 +- doc/user/project/integrations/webhooks.md | 31 +++++++ ee/lib/api/group_hooks.rb | 1 + .../api/schemas/public_api/v4/group_hook.json | 9 +- lib/api/entities/hook.rb | 2 + lib/api/project_hooks.rb | 1 + locale/gitlab.pot | 6 ++ .../projects/hooks_controller_spec.rb | 2 + .../schemas/public_api/v4/project_hook.json | 9 +- .../schemas/public_api/v4/system_hook.json | 65 ++++++++++++--- spec/lib/api/entities/hook_spec.rb | 3 +- spec/models/hooks/web_hook_spec.rb | 1 + spec/services/web_hook_service_spec.rb | 82 +++++++++++++++++++ 22 files changed, 282 insertions(+), 23 deletions(-) create mode 100644 config/feature_flags/beta/custom_webhook_template.yml create mode 100644 db/migrate/20240213181406_add_custom_webhook_template_to_web_hook.rb create mode 100644 db/migrate/20240213181407_add_text_limit_to_web_hooks_custom_webhook_template.rb create mode 100644 db/schema_migrations/20240213181406 create mode 100644 db/schema_migrations/20240213181407 diff --git a/app/controllers/concerns/web_hooks/hook_actions.rb b/app/controllers/concerns/web_hooks/hook_actions.rb index 5aeb10dfb878b..aae38e67e2300 100644 --- a/app/controllers/concerns/web_hooks/hook_actions.rb +++ b/app/controllers/concerns/web_hooks/hook_actions.rb @@ -71,7 +71,8 @@ def hook_params end def hook_param_names - %i[enable_ssl_verification name description token url push_events_branch_filter branch_filter_strategy] + %i[enable_ssl_verification name description token url push_events_branch_filter branch_filter_strategy + custom_webhook_template] end def destroy_hook(hook) diff --git a/app/models/hooks/web_hook.rb b/app/models/hooks/web_hook.rb index c0bfe31fb389d..24f19b932e0e7 100644 --- a/app/models/hooks/web_hook.rb +++ b/app/models/hooks/web_hook.rb @@ -44,6 +44,7 @@ class WebHook < ApplicationRecord validates :url_variables, json_schema: { filename: 'web_hooks_url_variables' } validate :no_missing_url_variables validates :interpolated_url, public_url: true, if: ->(hook) { hook.url_variables? && hook.errors.empty? } + validates :custom_webhook_template, length: { maximum: 4096 } enum branch_filter_strategy: { wildcard: 0, diff --git a/app/services/web_hook_service.rb b/app/services/web_hook_service.rb index 035f1754cbbf9..2373e1de3dd80 100644 --- a/app/services/web_hook_service.rb +++ b/app/services/web_hook_service.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class WebHookService + include Gitlab::Utils::StrongMemoize + class InternalErrorResponse ERROR_MESSAGE = 'internal error' @@ -33,6 +35,8 @@ def initialize RESPONSE_HEADERS_COUNT_LIMIT = 50 RESPONSE_HEADERS_SIZE_LIMIT = 1.kilobytes + CUSTOM_TEMPLATE_INTERPOLATION_REGEX = /{{(.+?)}}/ + attr_accessor :hook, :data, :hook_name, :request_options attr_reader :uniqueness_token @@ -87,15 +91,19 @@ def execute ) ServiceResponse.success(message: response.body, payload: { http_status: response.code }) - rescue *Gitlab::HTTP::HTTP_ERRORS, + rescue *Gitlab::HTTP::HTTP_ERRORS, JSON::ParserError, Gitlab::Json::LimitedEncoder::LimitExceeded, URI::InvalidURIError => e execution_duration = ::Gitlab::Metrics::System.monotonic_time - start_time error_message = e.to_s + # An exception raised while rendering the custom template prevents us from calling `#request_payload` + request_data = e.instance_of?(JSON::ParserError) ? {} : request_payload + log_execution( response: InternalErrorResponse.new, execution_duration: execution_duration, - error_message: error_message + error_message: error_message, + request_data: request_data ) Gitlab::AppLogger.error("WebHook Error after #{execution_duration.to_i.seconds}s => #{e}") @@ -129,7 +137,7 @@ def parsed_url def make_request(url, basic_auth = false) Gitlab::HTTP.post(url, - body: Gitlab::Json::LimitedEncoder.encode(data, limit: REQUEST_BODY_SIZE_LIMIT), + body: Gitlab::Json::LimitedEncoder.encode(request_payload, limit: REQUEST_BODY_SIZE_LIMIT), headers: build_headers, verify: hook.enable_ssl_verification, basic_auth: basic_auth, @@ -145,7 +153,7 @@ def make_request_with_auth make_request(post_url, basic_auth) end - def log_execution(response:, execution_duration:, error_message: nil) + def log_execution(response:, execution_duration:, error_message: nil, request_data: request_payload) category = response_category(response) log_data = { trigger: hook_name, @@ -153,7 +161,7 @@ def log_execution(response:, execution_duration:, error_message: nil) interpolated_url: hook.interpolated_url, execution_duration: execution_duration, request_headers: build_headers, - request_data: data, + request_data: request_data, response_headers: safe_response_headers(response), response_body: safe_response_body(response), response_status: response.code, @@ -267,4 +275,27 @@ def string_size_limit(str, limit) def enforce_utf8(str) Gitlab::EncodingHelper.encode_utf8(str) end + + def request_payload + return data unless hook.custom_webhook_template.present? + return data unless Feature.enabled?(:custom_webhook_template, hook.parent, type: :beta) + + start_time = Gitlab::Metrics::System.monotonic_time + rendered_template = render_custom_template(hook.custom_webhook_template, data.deep_stringify_keys) + duration = Gitlab::Metrics::System.monotonic_time - start_time + + Gitlab::AppLogger.info( + message: "Rendered custom webhook template", + hook_id: hook.id, + duration_s: duration + ) + Gitlab::Json.parse(rendered_template) + rescue JSON::ParserError => e + raise JSON::ParserError, "Error while parsing rendered custom webhook template: #{e.message}" + end + strong_memoize_attr :request_payload + + def render_custom_template(template, params) + template.gsub(CUSTOM_TEMPLATE_INTERPOLATION_REGEX) { params.dig(*Regexp.last_match(1).split('.')) } + end end diff --git a/app/views/shared/web_hooks/_form.html.haml b/app/views/shared/web_hooks/_form.html.haml index cdef45a041531..5f920d9c982e9 100644 --- a/app/views/shared/web_hooks/_form.html.haml +++ b/app/views/shared/web_hooks/_form.html.haml @@ -77,6 +77,13 @@ integration_webhook_event_human_name(:emoji_events), help_text: s_('Webhooks|An emoji is awarded or revoked. %{help_link}?').html_safe % { help_link: emoji_help_link } +- if Feature.enabled?(:custom_webhook_template, hook.parent, type: :beta) + .form-group + = form.label :custom_webhook_template, s_('Webhooks|Custom webhook template (optional)'), class: 'label-bold' + = form.text_area :custom_webhook_template, value: hook.custom_webhook_template, class: 'form-control gl-form-input gl-form-input-xl', rows: 8, maxlength: 4096 + %p.form-text.text-muted + = link_to s_('Webhooks|How to create a custom webhook template?'), help_page_path('user/project/integrations/webhooks', anchor: 'custom-webhook-template') + .form-group = form.label :enable_ssl_verification, s_('Webhooks|SSL verification'), class: 'label-bold checkbox' %ul.list-unstyled diff --git a/config/feature_flags/beta/custom_webhook_template.yml b/config/feature_flags/beta/custom_webhook_template.yml new file mode 100644 index 0000000000000..8fa1097d71c84 --- /dev/null +++ b/config/feature_flags/beta/custom_webhook_template.yml @@ -0,0 +1,9 @@ +--- +name: custom_webhook_template +feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/362504 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/142738 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/439610 +milestone: '16.10' +group: group::import and integrate +type: beta +default_enabled: false diff --git a/db/migrate/20240213181406_add_custom_webhook_template_to_web_hook.rb b/db/migrate/20240213181406_add_custom_webhook_template_to_web_hook.rb new file mode 100644 index 0000000000000..87e982b36c580 --- /dev/null +++ b/db/migrate/20240213181406_add_custom_webhook_template_to_web_hook.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +class AddCustomWebhookTemplateToWebHook < Gitlab::Database::Migration[2.2] + enable_lock_retries! + milestone '16.10' + + def change + # rubocop:disable Migration/AddLimitToTextColumns -- limit is added in 20240213181407 + add_column :web_hooks, :custom_webhook_template, :text, null: true + # rubocop:enable Migration/AddLimitToTextColumns + end +end diff --git a/db/migrate/20240213181407_add_text_limit_to_web_hooks_custom_webhook_template.rb b/db/migrate/20240213181407_add_text_limit_to_web_hooks_custom_webhook_template.rb new file mode 100644 index 0000000000000..0f6084b12cc7c --- /dev/null +++ b/db/migrate/20240213181407_add_text_limit_to_web_hooks_custom_webhook_template.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +class AddTextLimitToWebHooksCustomWebhookTemplate < Gitlab::Database::Migration[2.2] + disable_ddl_transaction! + milestone '16.10' + + def up + add_text_limit :web_hooks, :custom_webhook_template, 4096 + end + + def down + remove_text_limit :web_hooks, :custom_webhook_template + end +end diff --git a/db/schema_migrations/20240213181406 b/db/schema_migrations/20240213181406 new file mode 100644 index 0000000000000..2d3ecaba285ee --- /dev/null +++ b/db/schema_migrations/20240213181406 @@ -0,0 +1 @@ +c08cf74a779336874a525308791a9974543591692e0fbcb2c66c13455617c52d \ No newline at end of file diff --git a/db/schema_migrations/20240213181407 b/db/schema_migrations/20240213181407 new file mode 100644 index 0000000000000..b67e12deb557b --- /dev/null +++ b/db/schema_migrations/20240213181407 @@ -0,0 +1 @@ +33e45972396db4163da1089bccf7aeb6da5e2a88e2e7ea4d4e3fe00b6347a54a \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 47f57e1d96085..58aaca9775be5 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -17454,8 +17454,10 @@ CREATE TABLE web_hooks ( emoji_events boolean DEFAULT false NOT NULL, name text, description text, + custom_webhook_template text, CONSTRAINT check_1e4d5cbdc5 CHECK ((char_length(name) <= 255)), - CONSTRAINT check_23a96ad211 CHECK ((char_length(description) <= 2048)) + CONSTRAINT check_23a96ad211 CHECK ((char_length(description) <= 2048)), + CONSTRAINT check_69ef76ee0c CHECK ((char_length(custom_webhook_template) <= 4096)) ); CREATE SEQUENCE web_hooks_id_seq diff --git a/doc/user/project/integrations/webhooks.md b/doc/user/project/integrations/webhooks.md index 084490fdcaaf6..5dd565282db61 100644 --- a/doc/user/project/integrations/webhooks.md +++ b/doc/user/project/integrations/webhooks.md @@ -106,6 +106,37 @@ You must define the following variables: Variable names can contain only lowercase letters (`a-z`), numbers (`0-9`), or underscores (`_`). You can define URL variables directly using the REST API. +## Custom webhook template + +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/142738) in GitLab 16.10 [with a flag](../../../administration/feature_flags.md) named `custom_webhook_template`. Disabled by default. + +FLAG: +On self-managed GitLab, by default this feature is not available. To make it available, an administrator can +[enable the feature flag](../../../administration/feature_flags.md) named `custom_webhook_template`. +On GitLab.com, this feature is not available. + +You can set a custom payload template in the webhook configuration. The request body is rendered from the template +with the data for the current event. The template must render as valid JSON. + +You can use any field from the [payload of any event](webhook_events.md), such as `{{build_name}}` for a job event and `{{deployable_url}}` +for a deployment event. For example: + +Given this custom payload template: + +```json +{ + "event": "{{object_kind}}" +} +``` + +You'll have this request payload that combines the template with a `push` event: + +```json +{ + "event": "push" +} +``` + ## Configure your webhook receiver endpoint Webhook receiver endpoints should be fast and stable. diff --git a/ee/lib/api/group_hooks.rb b/ee/lib/api/group_hooks.rb index b3b19c3c8fac9..3b3a8cec6dbf2 100644 --- a/ee/lib/api/group_hooks.rb +++ b/ee/lib/api/group_hooks.rb @@ -36,6 +36,7 @@ def hook_scope optional :emoji_events, type: Boolean, desc: "Trigger hook on emoji 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 :custom_webhook_template, type: String, desc: "Custom template for the request payload" use :url_variables end end diff --git a/ee/spec/fixtures/api/schemas/public_api/v4/group_hook.json b/ee/spec/fixtures/api/schemas/public_api/v4/group_hook.json index 66cb247d98aa6..496462a235732 100644 --- a/ee/spec/fixtures/api/schemas/public_api/v4/group_hook.json +++ b/ee/spec/fixtures/api/schemas/public_api/v4/group_hook.json @@ -24,7 +24,8 @@ "emoji_events", "alert_status", "disabled_until", - "url_variables" + "url_variables", + "custom_webhook_template" ], "properties": { "id": { @@ -128,6 +129,12 @@ } } } + }, + "custom_webhook_template": { + "type": [ + "string", + "null" + ] } }, "additionalProperties": false diff --git a/lib/api/entities/hook.rb b/lib/api/entities/hook.rb index d92331f7dea4f..61f3d60bd48a2 100644 --- a/lib/api/entities/hook.rb +++ b/lib/api/entities/hook.rb @@ -18,6 +18,8 @@ class Hook < Grape::Entity if: ->(_, options) { options[:with_url_variables] != false }, documentation: { type: 'Hash', example: { "token" => "secr3t" }, is_array: true } + expose :custom_webhook_template, documentation: { type: 'string', example: '{"event":"{{object_kind}}"}' } + def url_variables object.url_variables.keys.map { { key: _1 } } end diff --git a/lib/api/project_hooks.rb b/lib/api/project_hooks.rb index 011d5e69f006a..508026f6fda0a 100644 --- a/lib/api/project_hooks.rb +++ b/lib/api/project_hooks.rb @@ -35,6 +35,7 @@ def hook_scope 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" + optional :custom_webhook_template, type: String, desc: "Custom template for the request payload" use :url_variables end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index b08b9d371ec2d..a162fdc3bed0b 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -55769,6 +55769,9 @@ msgstr "" msgid "Webhooks|Confidential issues events" msgstr "" +msgid "Webhooks|Custom webhook template (optional)" +msgstr "" + msgid "Webhooks|Delete webhook" msgstr "" @@ -55799,6 +55802,9 @@ msgstr "" msgid "Webhooks|How it looks in the UI" msgstr "" +msgid "Webhooks|How to create a custom webhook template?" +msgstr "" + msgid "Webhooks|Issues events" msgstr "" diff --git a/spec/controllers/projects/hooks_controller_spec.rb b/spec/controllers/projects/hooks_controller_spec.rb index 8ba2e2a55fadc..0d1a452350719 100644 --- a/spec/controllers/projects/hooks_controller_spec.rb +++ b/spec/controllers/projects/hooks_controller_spec.rb @@ -138,6 +138,8 @@ def it_renders_correctly wiki_page_events: true, deployment_events: true, + custom_webhook_template: '{"test":"test"}', + url_variables: [{ key: 'token', value: 'some secret value' }] } diff --git a/spec/fixtures/api/schemas/public_api/v4/project_hook.json b/spec/fixtures/api/schemas/public_api/v4/project_hook.json index c42a4cad71255..dcf04ee6a3a2e 100644 --- a/spec/fixtures/api/schemas/public_api/v4/project_hook.json +++ b/spec/fixtures/api/schemas/public_api/v4/project_hook.json @@ -22,7 +22,8 @@ "releases_events", "alert_status", "disabled_until", - "emoji_events" + "emoji_events", + "custom_webhook_template" ], "optional": [ "url_variables" @@ -126,6 +127,12 @@ } } } + }, + "custom_webhook_template": { + "type": [ + "string", + "null" + ] } }, "additionalProperties": false diff --git a/spec/fixtures/api/schemas/public_api/v4/system_hook.json b/spec/fixtures/api/schemas/public_api/v4/system_hook.json index b6f56b948a0ad..62154d92acffc 100644 --- a/spec/fixtures/api/schemas/public_api/v4/system_hook.json +++ b/spec/fixtures/api/schemas/public_api/v4/system_hook.json @@ -11,29 +11,68 @@ "enable_ssl_verification", "alert_status", "disabled_until", - "url_variables" + "url_variables", + "custom_webhook_template" ], "properties": { - "id": { "type": "integer" }, - "url": { "type": "string" }, - "created_at": { "type": "string" }, - "push_events": { "type": "boolean" }, - "tag_push_events": { "type": "boolean" }, - "merge_requests_events": { "type": "boolean" }, - "repository_update_events": { "type": "boolean" }, - "enable_ssl_verification": { "type": "boolean" }, - "alert_status": { "type": "string", "enum": ["executable", "disabled", "temporarily_disabled"] }, - "disabled_until": { "type": ["string", "null"] }, + "id": { + "type": "integer" + }, + "url": { + "type": "string" + }, + "created_at": { + "type": "string" + }, + "push_events": { + "type": "boolean" + }, + "tag_push_events": { + "type": "boolean" + }, + "merge_requests_events": { + "type": "boolean" + }, + "repository_update_events": { + "type": "boolean" + }, + "enable_ssl_verification": { + "type": "boolean" + }, + "alert_status": { + "type": "string", + "enum": [ + "executable", + "disabled", + "temporarily_disabled" + ] + }, + "disabled_until": { + "type": [ + "string", + "null" + ] + }, "url_variables": { "type": "array", "items": { "type": "object", "additionalProperties": false, - "required": ["key"], + "required": [ + "key" + ], "properties": { - "key": { "type": "string" } + "key": { + "type": "string" + } } } + }, + "custom_webhook_template": { + "type": [ + "string", + "null" + ] } }, "additionalProperties": false diff --git a/spec/lib/api/entities/hook_spec.rb b/spec/lib/api/entities/hook_spec.rb index 45648d6fb645e..9b1db1ad258d1 100644 --- a/spec/lib/api/entities/hook_spec.rb +++ b/spec/lib/api/entities/hook_spec.rb @@ -11,7 +11,8 @@ it 'exposes correct attributes' do expect(json.keys).to contain_exactly(:alert_status, :created_at, :disabled_until, :enable_ssl_verification, :id, - :merge_requests_events, :push_events, :repository_update_events, :tag_push_events, :url, :url_variables + :merge_requests_events, :push_events, :repository_update_events, :tag_push_events, :url, :url_variables, + :custom_webhook_template ) end diff --git a/spec/models/hooks/web_hook_spec.rb b/spec/models/hooks/web_hook_spec.rb index 308e16328d7ef..4b5ab3277db55 100644 --- a/spec/models/hooks/web_hook_spec.rb +++ b/spec/models/hooks/web_hook_spec.rb @@ -23,6 +23,7 @@ describe 'validations' do it { is_expected.to validate_presence_of(:url) } + it { is_expected.to validate_length_of(:custom_webhook_template).is_at_most(4096) } describe 'url_variables' do it { is_expected.to allow_value({}).for(:url_variables) } diff --git a/spec/services/web_hook_service_spec.rb b/spec/services/web_hook_service_spec.rb index c33273348f612..c3230dcd64e62 100644 --- a/spec/services/web_hook_service_spec.rb +++ b/spec/services/web_hook_service_spec.rb @@ -369,6 +369,88 @@ end end + context 'when custom_webhook_template is set' do + before do + stub_full_request(project_hook.url, method: :post) + end + + context 'when template is valid' do + before do + project_hook.custom_webhook_template = '{"before":"{{before}}"}' + end + + it 'renders custom_webhook_template for body' do + service_instance.execute + + expect(WebMock).to have_requested(:post, stubbed_hostname(project_hook.url)) + .with(headers: headers, body: '{"before":"oldrev"}') + .once + end + + context 'when using nested values' do + let(:data) do + { before: 'before', nested: { key: 'value' } } + end + + before do + project_hook.custom_webhook_template = '{"before":"{{before}}","nested_key":"{{nested.key}}"}' + end + + it 'renders custom_webhook_template for body' do + service_instance.execute + + expect(WebMock).to have_requested(:post, stubbed_hostname(project_hook.url)) + .with(headers: headers, body: '{"before":"before","nested_key":"value"}') + .once + end + end + + context 'when feature flag is disabled' do + before do + stub_feature_flags(custom_webhook_template: false) + end + + it 'does not render custom template', :aggregate_failures do + service_instance.execute + + expect(WebMock).to have_requested(:post, stubbed_hostname(project_hook.url)) + .with(headers: headers, body: '{"before":"oldrev","after":"newrev","ref":"ref"}') + .once + end + end + end + + context 'when template is invalid' do + before do + project_hook.custom_webhook_template = '{"test":"{{event}"}' + end + + it 'renders without problems', :aggregate_failures do + service_instance.execute + + expect(WebMock).to have_requested(:post, stubbed_hostname(project_hook.url)) + .with(headers: headers, body: '{"test":"{{event}"}') + .once + expect { service_instance.execute }.not_to raise_error + end + end + + context 'when template renders invalid json' do + before do + project_hook.custom_webhook_template = '{"test":"{{before}}}' + end + + it 'handles the error', :aggregate_failures do + expect(service_instance.execute).to have_attributes( + status: :error, + message: 'Error while parsing rendered custom webhook template: quoted string not terminated ' \ + '(after test) at line 1, column 16 [parse.c:379] in \'{"test":"oldrev}' + ) + expect { service_instance.execute }.not_to raise_error + end + end + end + it 'handles 200 status code' do stub_full_request(project_hook.url, method: :post).to_return(status: 200, body: 'Success') -- GitLab