Use real logging, fix tests, support it in regular email attachment

x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java

@@ -354,7 +354,7 @@ public Collection<Object> createComponents(
 
         TextTemplateEngine templateEngine = new TextTemplateEngine(scriptService);
         Map<String, EmailAttachmentParser<?>> 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,

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<HttpRequest, HttpResponse> 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);
     }
 

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<String, Object> 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()) {

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<String, EmailAttachmentParser<? extends EmailAttachmentParser.EmailAttachment>> 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<String, EmailAttachmentParser<? extends EmailAttachmentParser.EmailAttachment>> 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;
+    }
 }

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;
+    }
 }