From 964455fd184b5b309a87a4f874a3a16900a41dee Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Thu, 13 Apr 2023 11:00:19 -0600 Subject: [PATCH] Use real logging, fix tests, support it in regular email attachment --- .../elasticsearch/xpack/watcher/Watcher.java | 2 +- .../watcher/notification/WebhookService.java | 26 +++++++++---------- .../HttpEmailAttachementParser.java | 13 ++++------ .../actions/email/EmailActionTests.java | 22 ++++++++++++++-- .../HttpEmailAttachementParserTests.java | 26 ++++++++++++++++++- 5 files changed, 63 insertions(+), 26 deletions(-) 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 ae1bdc3b02ef3..25233d068d4a1 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 @@ -354,7 +354,7 @@ public Collection createComponents( TextTemplateEngine templateEngine = new TextTemplateEngine(scriptService); Map> emailAttachmentParsers = new HashMap<>(); - emailAttachmentParsers.put(HttpEmailAttachementParser.TYPE, new HttpEmailAttachementParser(httpClient, templateEngine)); + emailAttachmentParsers.put(HttpEmailAttachementParser.TYPE, new HttpEmailAttachementParser(webhookService, templateEngine)); emailAttachmentParsers.put(DataAttachmentParser.TYPE, new DataAttachmentParser()); emailAttachmentParsers.put( ReportingAttachmentParser.TYPE, 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 index a149fa04339f1..a7e3fe8bf368d 100644 --- 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 @@ -158,26 +158,19 @@ public Action.Result execute( */ private HttpRequest maybeModifyHttpResponse(HttpRequest request) { WebhookAccount account = getAccount(NAME); - logger.info( - "--> executing webhook request: {} — hostTokenMap: {} — addl token? {}", - request, - account.hostTokenMap, - this.additionalTokenEnabled - ); if (this.additionalTokenEnabled && account.hostTokenMap.size() > 0) { // Generate a string like example.com:9200 to match against the list of hosts where the // additional token should be provided. The token will only be added to the headers if // the request matches the list. String reqHostAndPort = request.host() + ":" + request.port(); - logger.info("--> reqHostAndPort: {}", reqHostAndPort); if (Strings.hasText(account.hostTokenMap.get(reqHostAndPort))) { // Add the additional token - logger.info( - "--> token added to request for {}://{}:{}{}", - request.scheme(), + logger.debug( + "additional [{}] header token added to watcher webhook request for {}://{}:{}", + TOKEN_HEADER_NAME, + request.scheme().scheme(), request.host(), - request.port(), - request.path() + request.port() ); return request.copy().setHeader(TOKEN_HEADER_NAME, account.hostTokenMap.get(reqHostAndPort)).build(); } @@ -192,9 +185,14 @@ private HttpRequest maybeModifyHttpResponse(HttpRequest request) { */ public Tuple modifyAndExecuteHttpRequest(HttpRequest request) throws IOException { final HttpRequest modifiedRequest = maybeModifyHttpResponse(request); - logger.info("--> executing request: {}", modifiedRequest); final HttpResponse response = httpClient.execute(modifiedRequest); - logger.info("--> webhook response status: {} — {}", response.status(), response); + logger.debug( + "executed watcher webhook request for {}://{}:{}, response code: {}", + modifiedRequest.scheme().scheme(), + modifiedRequest.host(), + modifiedRequest.port(), + response.status() + ); return Tuple.tuple(modifiedRequest, response); } diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/email/attachment/HttpEmailAttachementParser.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/email/attachment/HttpEmailAttachementParser.java index 05ada3ac9231b..e4d7fcc3a2935 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/email/attachment/HttpEmailAttachementParser.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/email/attachment/HttpEmailAttachementParser.java @@ -14,11 +14,11 @@ import org.elasticsearch.xcontent.XContentParser; 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.HttpRequestTemplate; import org.elasticsearch.xpack.watcher.common.http.HttpResponse; import org.elasticsearch.xpack.watcher.common.text.TextTemplateEngine; +import org.elasticsearch.xpack.watcher.notification.WebhookService; import org.elasticsearch.xpack.watcher.notification.email.Attachment; import org.elasticsearch.xpack.watcher.support.Variables; @@ -34,11 +34,11 @@ public interface Fields { } public static final String TYPE = "http"; - private final HttpClient httpClient; + private final WebhookService webhookService; private final TextTemplateEngine templateEngine; - public HttpEmailAttachementParser(HttpClient httpClient, TextTemplateEngine templateEngine) { - this.httpClient = httpClient; + public HttpEmailAttachementParser(WebhookService webhookService, TextTemplateEngine templateEngine) { + this.webhookService = webhookService; this.templateEngine = templateEngine; } @@ -82,10 +82,7 @@ public Attachment toAttachment(WatchExecutionContext context, Payload payload, H Map model = Variables.createCtxParamsMap(context, payload); HttpRequest httpRequest = attachment.getRequestTemplate().render(templateEngine, model); - // TODO: it would be possible to make this go through WebhookService the - // way that ReportingAttachmentParser does, but this work has not yet - // been done. - HttpResponse response = httpClient.execute(httpRequest); + HttpResponse response = webhookService.modifyAndExecuteHttpRequest(httpRequest).v2(); // check for status 200, only then append attachment if (response.status() >= 200 && response.status() < 300) { if (response.hasContent()) { diff --git a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/actions/email/EmailActionTests.java b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/actions/email/EmailActionTests.java index 0303e67932549..f097ffe5c4da2 100644 --- a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/actions/email/EmailActionTests.java +++ b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/actions/email/EmailActionTests.java @@ -9,6 +9,7 @@ import io.netty.handler.codec.http.HttpHeaders; import org.elasticsearch.ElasticsearchParseException; +import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.Strings; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.collect.MapBuilder; @@ -35,6 +36,7 @@ import org.elasticsearch.xpack.watcher.common.http.HttpResponse; import org.elasticsearch.xpack.watcher.common.text.TextTemplate; import org.elasticsearch.xpack.watcher.common.text.TextTemplateEngine; +import org.elasticsearch.xpack.watcher.notification.WebhookService; import org.elasticsearch.xpack.watcher.notification.email.Attachment; import org.elasticsearch.xpack.watcher.notification.email.Authentication; import org.elasticsearch.xpack.watcher.notification.email.Email; @@ -62,6 +64,7 @@ import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import static java.util.Collections.emptyMap; import static java.util.Collections.singletonMap; @@ -92,7 +95,10 @@ public void addEmailAttachmentParsers() { Map> emailAttachmentParsers = new HashMap<>(); emailAttachmentParsers.put( HttpEmailAttachementParser.TYPE, - new HttpEmailAttachementParser(httpClient, new MockTextTemplateEngine()) + new HttpEmailAttachementParser( + new WebhookService(Settings.EMPTY, httpClient, mockClusterService().getClusterSettings()), + new MockTextTemplateEngine() + ) ); emailAttachmentParsers.put(DataAttachmentParser.TYPE, new DataAttachmentParser()); emailAttachmentParser = new EmailAttachmentsParser(emailAttachmentParsers); @@ -547,7 +553,13 @@ public void testThatOneFailedEmailAttachmentResultsInActionFailure() throws Exce // setup email attachment parsers Map> attachmentParsers = new HashMap<>(); - attachmentParsers.put(HttpEmailAttachementParser.TYPE, new HttpEmailAttachementParser(httpClient, engine)); + attachmentParsers.put( + HttpEmailAttachementParser.TYPE, + new HttpEmailAttachementParser( + new WebhookService(Settings.EMPTY, httpClient, mockClusterService().getClusterSettings()), + engine + ) + ); EmailAttachmentsParser emailAttachmentsParser = new EmailAttachmentsParser(attachmentParsers); XContentBuilder builder = jsonBuilder().startObject() @@ -666,4 +678,10 @@ public EmailSent send(Email email, Authentication auth, Profile profile, String } } + private ClusterService mockClusterService() { + ClusterService clusterService = mock(ClusterService.class); + ClusterSettings clusterSettings = new ClusterSettings(Settings.EMPTY, Set.of()); + when(clusterService.getClusterSettings()).thenReturn(clusterSettings); + return clusterService; + } } diff --git a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/notification/email/attachment/HttpEmailAttachementParserTests.java b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/notification/email/attachment/HttpEmailAttachementParserTests.java index 0a79ea101f7aa..3ea4a7c787ec2 100644 --- a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/notification/email/attachment/HttpEmailAttachementParserTests.java +++ b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/notification/email/attachment/HttpEmailAttachementParserTests.java @@ -7,8 +7,11 @@ package org.elasticsearch.xpack.watcher.notification.email.attachment; import org.elasticsearch.ElasticsearchException; +import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.Strings; import org.elasticsearch.common.collect.MapBuilder; +import org.elasticsearch.common.settings.ClusterSettings; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xcontent.ToXContent; import org.elasticsearch.xcontent.XContentBuilder; @@ -20,6 +23,7 @@ import org.elasticsearch.xpack.watcher.common.http.HttpRequest; import org.elasticsearch.xpack.watcher.common.http.HttpRequestTemplate; import org.elasticsearch.xpack.watcher.common.http.HttpResponse; +import org.elasticsearch.xpack.watcher.notification.WebhookService; import org.elasticsearch.xpack.watcher.notification.email.attachment.EmailAttachmentParser.EmailAttachment; import org.elasticsearch.xpack.watcher.test.MockTextTemplateEngine; import org.junit.Before; @@ -31,9 +35,14 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Set; import static java.nio.charset.StandardCharsets.UTF_8; import static org.elasticsearch.xcontent.XContentFactory.jsonBuilder; +import static org.elasticsearch.xpack.watcher.notification.email.attachment.ReportingAttachmentParser.INTERVAL_SETTING; +import static org.elasticsearch.xpack.watcher.notification.email.attachment.ReportingAttachmentParser.REPORT_WARNING_ENABLED_SETTING; +import static org.elasticsearch.xpack.watcher.notification.email.attachment.ReportingAttachmentParser.REPORT_WARNING_TEXT; +import static org.elasticsearch.xpack.watcher.notification.email.attachment.ReportingAttachmentParser.RETRIES_SETTING; import static org.elasticsearch.xpack.watcher.test.WatcherTestUtils.mockExecutionContextBuilder; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.core.Is.is; @@ -52,7 +61,13 @@ public void init() throws Exception { httpClient = mock(HttpClient.class); attachmentParsers = new HashMap<>(); - attachmentParsers.put(HttpEmailAttachementParser.TYPE, new HttpEmailAttachementParser(httpClient, new MockTextTemplateEngine())); + attachmentParsers.put( + HttpEmailAttachementParser.TYPE, + new HttpEmailAttachementParser( + new WebhookService(Settings.EMPTY, httpClient, mockClusterService().getClusterSettings()), + new MockTextTemplateEngine() + ) + ); emailAttachmentsParser = new EmailAttachmentsParser(attachmentParsers); } @@ -172,4 +187,13 @@ private WatchExecutionContext createWatchExecutionContext() { .buildMock(); } + private ClusterService mockClusterService() { + ClusterService clusterService = mock(ClusterService.class); + ClusterSettings clusterSettings = new ClusterSettings( + Settings.EMPTY, + Set.of(INTERVAL_SETTING, RETRIES_SETTING, REPORT_WARNING_ENABLED_SETTING, REPORT_WARNING_TEXT) + ); + when(clusterService.getClusterSettings()).thenReturn(clusterSettings); + return clusterService; + } }