From e4076188a5e3e3d59f7600eb77a0464ddde0cf6c Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Tue, 27 Dec 2022 11:22:00 -0700 Subject: [PATCH] Refactor Watcher webhook execution into WebhookService 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. --- .../elasticsearch/xpack/watcher/Watcher.java | 5 +- .../webhook/ExecutableWebhookAction.java | 29 ++----- .../actions/webhook/WebhookActionFactory.java | 10 +-- .../watcher/notification/WebhookService.java | 77 +++++++++++++++++++ .../actions/webhook/WebhookActionTests.java | 37 +++++++-- .../xpack/watcher/test/WatcherTestUtils.java | 10 ++- .../xpack/watcher/watch/WatchTests.java | 7 +- 7 files changed, 137 insertions(+), 38 deletions(-) create mode 100644 x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/WebhookService.java diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java index 89283cfc7a54a..b60726550ab95 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java @@ -131,6 +131,7 @@ import org.elasticsearch.xpack.watcher.input.transform.TransformInput; import org.elasticsearch.xpack.watcher.input.transform.TransformInputFactory; import org.elasticsearch.xpack.watcher.notification.NotificationService; +import org.elasticsearch.xpack.watcher.notification.WebhookService; import org.elasticsearch.xpack.watcher.notification.email.Account; import org.elasticsearch.xpack.watcher.notification.email.EmailService; import org.elasticsearch.xpack.watcher.notification.email.HtmlSanitizer; @@ -341,11 +342,13 @@ public Collection createComponents( JiraService jiraService = new JiraService(settings, httpClient, clusterService.getClusterSettings()); SlackService slackService = new SlackService(settings, httpClient, clusterService.getClusterSettings()); PagerDutyService pagerDutyService = new PagerDutyService(settings, httpClient, clusterService.getClusterSettings()); + WebhookService webhookService = new WebhookService(settings, httpClient, clusterService.getClusterSettings()); reloadableServices.add(emailService); reloadableServices.add(jiraService); reloadableServices.add(slackService); reloadableServices.add(pagerDutyService); + reloadableServices.add(webhookService); TextTemplateEngine templateEngine = new TextTemplateEngine(scriptService); Map> emailAttachmentParsers = new HashMap<>(); @@ -386,7 +389,7 @@ public Collection createComponents( // actions final Map actionFactoryMap = new HashMap<>(); actionFactoryMap.put(EmailAction.TYPE, new EmailActionFactory(settings, emailService, templateEngine, emailAttachmentsParser)); - actionFactoryMap.put(WebhookAction.TYPE, new WebhookActionFactory(httpClient, templateEngine)); + actionFactoryMap.put(WebhookAction.TYPE, new WebhookActionFactory(webhookService, templateEngine)); actionFactoryMap.put(IndexAction.TYPE, new IndexActionFactory(settings, client)); actionFactoryMap.put(LoggingAction.TYPE, new LoggingActionFactory(templateEngine)); actionFactoryMap.put(JiraAction.TYPE, new JiraActionFactory(templateEngine, jiraService)); diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/actions/webhook/ExecutableWebhookAction.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/actions/webhook/ExecutableWebhookAction.java index 45fce5cce8901..e0787b200bc14 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/actions/webhook/ExecutableWebhookAction.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/actions/webhook/ExecutableWebhookAction.java @@ -11,41 +11,22 @@ import org.elasticsearch.xpack.core.watcher.actions.ExecutableAction; import org.elasticsearch.xpack.core.watcher.execution.WatchExecutionContext; import org.elasticsearch.xpack.core.watcher.watch.Payload; -import org.elasticsearch.xpack.watcher.common.http.HttpClient; -import org.elasticsearch.xpack.watcher.common.http.HttpRequest; -import org.elasticsearch.xpack.watcher.common.http.HttpResponse; import org.elasticsearch.xpack.watcher.common.text.TextTemplateEngine; -import org.elasticsearch.xpack.watcher.support.Variables; - -import java.util.Map; +import org.elasticsearch.xpack.watcher.notification.WebhookService; public class ExecutableWebhookAction extends ExecutableAction { - private final HttpClient httpClient; + private final WebhookService webhookService; private final TextTemplateEngine templateEngine; - public ExecutableWebhookAction(WebhookAction action, Logger logger, HttpClient httpClient, TextTemplateEngine templateEngine) { + public ExecutableWebhookAction(WebhookAction action, Logger logger, WebhookService webhookService, TextTemplateEngine templateEngine) { super(action, logger); - this.httpClient = httpClient; + this.webhookService = webhookService; this.templateEngine = templateEngine; } @Override public Action.Result execute(String actionId, WatchExecutionContext ctx, Payload payload) throws Exception { - Map model = Variables.createCtxParamsMap(ctx, payload); - - HttpRequest request = action.requestTemplate.render(templateEngine, model); - - if (ctx.simulateAction(actionId)) { - return new WebhookAction.Result.Simulated(request); - } - - HttpResponse response = httpClient.execute(request); - - if (response.status() >= 400) { - return new WebhookAction.Result.Failure(request, response); - } else { - return new WebhookAction.Result.Success(request, response); - } + return webhookService.execute(actionId, action, templateEngine, ctx, payload); } } diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/actions/webhook/WebhookActionFactory.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/actions/webhook/WebhookActionFactory.java index 5cd648a148cef..2937b19276a59 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/actions/webhook/WebhookActionFactory.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/actions/webhook/WebhookActionFactory.java @@ -9,25 +9,25 @@ import org.apache.logging.log4j.LogManager; import org.elasticsearch.xcontent.XContentParser; import org.elasticsearch.xpack.core.watcher.actions.ActionFactory; -import org.elasticsearch.xpack.watcher.common.http.HttpClient; import org.elasticsearch.xpack.watcher.common.text.TextTemplateEngine; +import org.elasticsearch.xpack.watcher.notification.WebhookService; import java.io.IOException; public class WebhookActionFactory extends ActionFactory { - private final HttpClient httpClient; private final TextTemplateEngine templateEngine; + private final WebhookService webhookService; - public WebhookActionFactory(HttpClient httpClient, TextTemplateEngine templateEngine) { + public WebhookActionFactory(WebhookService webhookService, TextTemplateEngine templateEngine) { super(LogManager.getLogger(ExecutableWebhookAction.class)); - this.httpClient = httpClient; this.templateEngine = templateEngine; + this.webhookService = webhookService; } @Override public ExecutableWebhookAction parseExecutable(String watchId, String actionId, XContentParser parser) throws IOException { - return new ExecutableWebhookAction(WebhookAction.parse(watchId, actionId, parser), actionLogger, httpClient, templateEngine); + return new ExecutableWebhookAction(WebhookAction.parse(watchId, actionId, parser), actionLogger, webhookService, templateEngine); } } diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/WebhookService.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/WebhookService.java new file mode 100644 index 0000000000000..eac4cc446e78d --- /dev/null +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/WebhookService.java @@ -0,0 +1,77 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.watcher.notification; + +import org.elasticsearch.common.settings.ClusterSettings; +import org.elasticsearch.common.settings.Setting; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.xpack.core.watcher.actions.Action; +import org.elasticsearch.xpack.core.watcher.execution.WatchExecutionContext; +import org.elasticsearch.xpack.core.watcher.watch.Payload; +import org.elasticsearch.xpack.watcher.actions.webhook.WebhookAction; +import org.elasticsearch.xpack.watcher.common.http.HttpClient; +import org.elasticsearch.xpack.watcher.common.http.HttpRequest; +import org.elasticsearch.xpack.watcher.common.http.HttpResponse; +import org.elasticsearch.xpack.watcher.common.text.TextTemplateEngine; +import org.elasticsearch.xpack.watcher.support.Variables; + +import java.io.IOException; +import java.util.List; +import java.util.Map; + +/** + * The WebhookService class handles executing webhook requests for Watcher actions. These can be + * regular "webhook" actions as well as parts of an "email" action with attachments that make HTTP + * requests. + */ +public class WebhookService extends NotificationService { + + private final HttpClient httpClient; + + public WebhookService(Settings settings, HttpClient httpClient, ClusterSettings clusterSettings) { + super("webhook", settings, clusterSettings, List.of(), getSecureSettings()); + this.httpClient = httpClient; + // do an initial load + reload(settings); + } + + private static List> getSecureSettings() { + return List.of(); + } + + @Override + protected WebhookAccount createAccount(String name, Settings accountSettings) { + return new WebhookAccount(); + } + + public Action.Result execute( + String actionId, + WebhookAction action, + TextTemplateEngine templateEngine, + WatchExecutionContext ctx, + Payload payload + ) throws IOException { + Map model = Variables.createCtxParamsMap(ctx, payload); + + HttpRequest request = action.getRequest().render(templateEngine, model); + + if (ctx.simulateAction(actionId)) { + return new WebhookAction.Result.Simulated(request); + } + + HttpResponse response = httpClient.execute(request); + + if (response.status() >= 400) { + return new WebhookAction.Result.Failure(request, response); + } else { + return new WebhookAction.Result.Success(request, response); + } + } + + public static final class WebhookAccount {} +} diff --git a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/actions/webhook/WebhookActionTests.java b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/actions/webhook/WebhookActionTests.java index 371560b1631f6..f1019614d2294 100644 --- a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/actions/webhook/WebhookActionTests.java +++ b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/actions/webhook/WebhookActionTests.java @@ -8,6 +8,7 @@ import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.client.internal.Client; +import org.elasticsearch.common.settings.ClusterSettings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.env.Environment; import org.elasticsearch.env.TestEnvironment; @@ -33,6 +34,7 @@ import org.elasticsearch.xpack.watcher.common.text.TextTemplate; import org.elasticsearch.xpack.watcher.common.text.TextTemplateEngine; import org.elasticsearch.xpack.watcher.execution.TriggeredExecutionContext; +import org.elasticsearch.xpack.watcher.notification.WebhookService; import org.elasticsearch.xpack.watcher.notification.email.Attachment; import org.elasticsearch.xpack.watcher.support.search.WatcherSearchTemplateService; import org.elasticsearch.xpack.watcher.test.MockTextTemplateEngine; @@ -92,7 +94,12 @@ public void testExecute() throws Exception { HttpRequestTemplate httpRequest = getHttpRequestTemplate(method, TEST_HOST, TEST_PORT, testPath, testBody, null); WebhookAction action = new WebhookAction(httpRequest); - ExecutableWebhookAction executable = new ExecutableWebhookAction(action, logger, httpClient, templateEngine); + WebhookService webhookService = new WebhookService( + Settings.EMPTY, + httpClient, + new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS) + ); + ExecutableWebhookAction executable = new ExecutableWebhookAction(action, logger, webhookService, templateEngine); WatchExecutionContext ctx = WatcherTestUtils.mockExecutionContext("_id", new Payload.Simple("foo", "bar")); Action.Result actionResult = executable.execute("_id", ctx, Payload.EMPTY); @@ -155,7 +162,12 @@ public void testParserSelfGenerated() throws Exception { HttpRequestTemplate request = getHttpRequestTemplate(method, host, TEST_PORT, path, body, null); WebhookAction action = new WebhookAction(request); - ExecutableWebhookAction executable = new ExecutableWebhookAction(action, logger, ExecuteScenario.Success.client(), templateEngine); + WebhookService webhookService = new WebhookService( + Settings.EMPTY, + ExecuteScenario.Success.client(), + new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS) + ); + ExecutableWebhookAction executable = new ExecutableWebhookAction(action, logger, webhookService, templateEngine); XContentBuilder builder = jsonBuilder(); executable.toXContent(builder, ToXContent.EMPTY_PARAMS); @@ -217,7 +229,12 @@ public void testParserFailure() throws Exception { } private WebhookActionFactory webhookFactory(HttpClient client) { - return new WebhookActionFactory(client, templateEngine); + WebhookService webhookService = new WebhookService( + Settings.EMPTY, + client, + new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS) + ); + return new WebhookActionFactory(webhookService, templateEngine); } public void testThatSelectingProxyWorks() throws Exception { @@ -235,7 +252,12 @@ public void testThatSelectingProxyWorks() throws Exception { .proxy(new HttpProxy("localhost", proxyServer.getPort())); WebhookAction action = new WebhookAction(builder.build()); - ExecutableWebhookAction executable = new ExecutableWebhookAction(action, logger, httpClient, templateEngine); + WebhookService webhookService = new WebhookService( + Settings.EMPTY, + httpClient, + new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS) + ); + ExecutableWebhookAction executable = new ExecutableWebhookAction(action, logger, webhookService, templateEngine); String watchId = "test_url_encode" + randomAlphaOfLength(10); ScheduleTriggerEvent triggerEvent = new ScheduleTriggerEvent( watchId, @@ -268,7 +290,12 @@ public void testValidUrls() throws Exception { HttpRequestTemplate requestTemplate = getHttpRequestTemplate(method, host, TEST_PORT, path, testBody, null); WebhookAction action = new WebhookAction(requestTemplate); - ExecutableWebhookAction executable = new ExecutableWebhookAction(action, logger, client, templateEngine); + WebhookService webhookService = new WebhookService( + Settings.EMPTY, + client, + new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS) + ); + ExecutableWebhookAction executable = new ExecutableWebhookAction(action, logger, webhookService, templateEngine); ScheduleTriggerEvent triggerEvent = new ScheduleTriggerEvent( watchId, diff --git a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/test/WatcherTestUtils.java b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/test/WatcherTestUtils.java index f201289a5d1ac..91124771bdaff 100644 --- a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/test/WatcherTestUtils.java +++ b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/test/WatcherTestUtils.java @@ -10,6 +10,7 @@ import org.elasticsearch.action.search.SearchType; import org.elasticsearch.client.internal.Client; import org.elasticsearch.common.bytes.BytesReference; +import org.elasticsearch.common.settings.ClusterSettings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.core.TimeValue; @@ -43,6 +44,7 @@ import org.elasticsearch.xpack.watcher.execution.TriggeredExecutionContext; import org.elasticsearch.xpack.watcher.input.simple.ExecutableSimpleInput; import org.elasticsearch.xpack.watcher.input.simple.SimpleInput; +import org.elasticsearch.xpack.watcher.notification.WebhookService; import org.elasticsearch.xpack.watcher.notification.email.Authentication; import org.elasticsearch.xpack.watcher.notification.email.EmailService; import org.elasticsearch.xpack.watcher.notification.email.EmailTemplate; @@ -179,13 +181,19 @@ public static Watch createTestWatch( httpRequest.method(HttpMethod.POST); httpRequest.path(new TextTemplate("/foobarbaz/{{ctx.watch_id}}")); httpRequest.body(new TextTemplate("{{ctx.watch_id}} executed with {{ctx.payload.response.hits.total_hits}} hits")); + + WebhookService webhookService = new WebhookService( + Settings.EMPTY, + httpClient, + new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS) + ); actions.add( new ActionWrapper( "_webhook", actionThrottler, null, null, - new ExecutableWebhookAction(new WebhookAction(httpRequest.build()), logger, httpClient, engine), + new ExecutableWebhookAction(new WebhookAction(httpRequest.build()), logger, webhookService, engine), null, null ) diff --git a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/watch/WatchTests.java b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/watch/WatchTests.java index 64ca51128e72d..b29dfa182cb2b 100644 --- a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/watch/WatchTests.java +++ b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/watch/WatchTests.java @@ -79,6 +79,7 @@ import org.elasticsearch.xpack.watcher.input.simple.ExecutableSimpleInput; import org.elasticsearch.xpack.watcher.input.simple.SimpleInput; import org.elasticsearch.xpack.watcher.input.simple.SimpleInputFactory; +import org.elasticsearch.xpack.watcher.notification.WebhookService; import org.elasticsearch.xpack.watcher.notification.email.DataAttachment; import org.elasticsearch.xpack.watcher.notification.email.EmailService; import org.elasticsearch.xpack.watcher.notification.email.EmailTemplate; @@ -153,6 +154,7 @@ public class WatchTests extends ESTestCase { private Client client; private HttpClient httpClient; private EmailService emailService; + private WebhookService webhookService; private TextTemplateEngine templateEngine; private HtmlSanitizer htmlSanitizer; private XPackLicenseState licenseState; @@ -166,6 +168,7 @@ public void init() throws Exception { client = mock(Client.class); httpClient = mock(HttpClient.class); emailService = mock(EmailService.class); + webhookService = mock(WebhookService.class); templateEngine = mock(TextTemplateEngine.class); htmlSanitizer = mock(HtmlSanitizer.class); licenseState = mock(XPackLicenseState.class); @@ -658,7 +661,7 @@ private List randomActions() { randomThrottler(), AlwaysConditionTests.randomCondition(scriptService), randomTransform(), - new ExecutableWebhookAction(action, logger, httpClient, templateEngine), + new ExecutableWebhookAction(action, logger, webhookService, templateEngine), null, null ) @@ -676,7 +679,7 @@ private ActionRegistry registry(List actions, ConditionRegistry c new EmailActionFactory(settings, emailService, templateEngine, new EmailAttachmentsParser(Collections.emptyMap())) ); case IndexAction.TYPE -> parsers.put(IndexAction.TYPE, new IndexActionFactory(settings, client)); - case WebhookAction.TYPE -> parsers.put(WebhookAction.TYPE, new WebhookActionFactory(httpClient, templateEngine)); + case WebhookAction.TYPE -> parsers.put(WebhookAction.TYPE, new WebhookActionFactory(webhookService, templateEngine)); case LoggingAction.TYPE -> parsers.put(LoggingAction.TYPE, new LoggingActionFactory(new MockTextTemplateEngine())); } }