Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor Watcher webhook execution into WebhookService #92576

Merged

Conversation

dakrone
Copy link
Member

@dakrone dakrone commented Dec 27, 2022

This commit creates a new WebhookService which encapsulates the action of executing Watcher's webhook actions. Currently the functionality of webhook is unchanged from its existing behavior, however, this refactoring allows us to extend webhook actions, from both the "webhook" action as well as other actions that use webhooks like the "email" action, to add functionality for adding additional injected tokens.

This commit creates a new `WebhookService` which encapsulates the action of executing Watcher's webhook actions. Currently the functionality of webhook is unchanged from its existing behavior, however, this refactoring allows us to extend webhook actions, from both the "webhook" action as well as other actions that use webhooks like the "email" action, to add functionality for adding additional injected tokens.
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-data-management (Team:Data Management)

@elasticsearchmachine elasticsearchmachine added the Team:Data Management Meta label for data/management team label Dec 27, 2022
Copy link
Contributor

@tvernum tvernum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dakrone dakrone merged commit e887c02 into elastic:main Dec 27, 2022
dakrone added a commit to dakrone/elasticsearch that referenced this pull request Feb 1, 2023
This commit builds on elastic#92576 by adding the `xpack.notification.webhook.additional_token` and
`xpack.notification.webhook.token_hosts` keystore settings to allow an additional token to be sent
when Watcher performs a webhook request. This includes both the regular `webhook` action as well as
the `email` action that uses an `attachment` parameter to a url (which internally uses a webhook to
retrieve the attachment).

These settings can both by reloaded by updating the keystore and using the reload secure settings
API.

`xpack.notification.webhook.additional_token` is the token that will be sent in the header.
`xpack.notification.webhook.token_hosts` is a comma-separated list of `host:port` pairs for which
the header should be sent. For example, if this contains `localhost:1234,aoeu.com:9182` then the
token will only be added to the headers for webhook requests to the `localhost` and `aoeu.com` hosts
on the 1234 and 9182 ports respectively.
dakrone added a commit that referenced this pull request Feb 6, 2023
…93426)

This commit builds on #92576 by adding the `xpack.notification.webhook.host_token_pairs` keystore setting to allow an additional token to be sent when Watcher performs a webhook request. This includes both the regular `webhook` action as well as the `email` action that uses an `attachment` parameter to a url (which internally uses a webhook to retrieve the attachment).

These settings can both by reloaded by updating the keystore and using the reload secure settings API.

- `xpack.notification.webhook.host_token_pairs` is a comma-separated list of `<host>:<port>=<token>` pairs for which the header should be sent. For example, if this contains `localhost:1234=token1,aoeu.com:9182=token2` then the token will only be added to the headers for webhook requests to the `localhost` and `aoeu.com` hosts on the 1234 and 9182 ports with tokens `token1` and `token2` respectively.

Also added is a cluster setting (non-dynamic) — `xpack.notification.webhook.additional_token_enabled` that determines whether the token should be sent. It defaults to `false`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants