Allow for http method selection in custom webhook (#101)

* Add http method to custom webhook * Remove unused line * Opendistro 1.3 * Revert "Adding new type of input for Monitors - HttpInput (#82)" This reverts commit c2004f7. * fix alerting stats API (#129) * Bump plugin version (#131) * Restrict custom http alert method to POST PUT and PATCH * Fix indenting * Fix indention * Fix additional indention Co-authored-by: Jason Leezer <[email protected]> Co-authored-by: Lucas Winkelmann <[email protected]> Co-authored-by: Jinsoo <[email protected]> Co-authored-by: Mohammad Qureshi <[email protected]>
opendistro-for-elasticsearch · Nov 30, 2020 · 150b060 · 150b060
1 parent f98879f
commit 150b060
Show file tree

Hide file tree

Showing 7 changed files with 105 additions and 26 deletions.
diff --git a/.../kotlin/com/amazon/opendistroforelasticsearch/alerting/model/destination/CustomWebhook.kt b/.../kotlin/com/amazon/opendistroforelasticsearch/alerting/model/destination/CustomWebhook.kt
@@ -35,6 +35,7 @@ data class CustomWebhook(
     val host: String?,
     val port: Int,
     val path: String?,
+    val method: String?,
     val queryParams: Map<String, String>,
     val headerParams: Map<String, String>,
     val username: String?,
@@ -54,6 +55,7 @@ data class CustomWebhook(
                 .field(HOST_FIELD, host)
                 .field(PORT_FIELD, port)
                 .field(PATH_FIELD, path)
+                .field(METHOD_FIELD, method)
                 .field(QUERY_PARAMS_FIELD, queryParams)
                 .field(HEADER_PARAMS_FIELD, headerParams)
                 .field(USERNAME_FIELD, username)
@@ -68,6 +70,7 @@ data class CustomWebhook(
         out.writeString(host)
         out.writeOptionalInt(port)
         out.writeOptionalString(path)
+        out.writeOptionalString(method)
         out.writeMap(queryParams)
         out.writeMap(headerParams)
         out.writeOptionalString(username)
@@ -81,6 +84,7 @@ data class CustomWebhook(
         const val HOST_FIELD = "host"
         const val PORT_FIELD = "port"
         const val PATH_FIELD = "path"
+        const val METHOD_FIELD = "method"
         const val QUERY_PARAMS_FIELD = "query_params"
         const val HEADER_PARAMS_FIELD = "header_params"
         const val USERNAME_FIELD = "username"
@@ -94,6 +98,7 @@ data class CustomWebhook(
             var host: String? = null
             var port: Int = -1
             var path: String? = null
+            var method: String? = null
             var queryParams: Map<String, String> = mutableMapOf()
             var headerParams: Map<String, String> = mutableMapOf()
             var username: String? = null
@@ -109,6 +114,7 @@ data class CustomWebhook(
                     HOST_FIELD -> host = xcp.textOrNull()
                     PORT_FIELD -> port = xcp.intValue()
                     PATH_FIELD -> path = xcp.textOrNull()
+                    METHOD_FIELD -> method = xcp.textOrNull()
                     QUERY_PARAMS_FIELD -> queryParams = xcp.mapStrings()
                     HEADER_PARAMS_FIELD -> headerParams = xcp.mapStrings()
                     USERNAME_FIELD -> username = xcp.textOrNull()
@@ -118,7 +124,7 @@ data class CustomWebhook(
                     }
                 }
             }
-            return CustomWebhook(url, scheme, host, port, path, queryParams, headerParams, username, password)
+            return CustomWebhook(url, scheme, host, port, path, method, queryParams, headerParams, username, password)
         }
 
         @Suppress("UNCHECKED_CAST")
@@ -136,6 +142,7 @@ data class CustomWebhook(
                     sin.readString(), // host
                     sin.readOptionalInt(), // port
                     sin.readOptionalString(), // path
+                    sin.readOptionalString(), // method
                     suppressWarning(sin.readMap()), // queryParams)
                     suppressWarning(sin.readMap()), // headerParams)
                     sin.readOptionalString(), // username

diff --git a/...in/kotlin/com/amazon/opendistroforelasticsearch/alerting/model/destination/Destination.kt b/...in/kotlin/com/amazon/opendistroforelasticsearch/alerting/model/destination/Destination.kt
@@ -262,6 +262,7 @@ data class Destination(
                         .withHost(customWebhook?.host)
                         .withPort(customWebhook?.port)
                         .withPath(customWebhook?.path)
+                        .withMethod(customWebhook?.method)
                         .withQueryParams(customWebhook?.queryParams)
                         .withHeaderParams(customWebhook?.headerParams)
                         .withMessage(compiledMessage).build()

diff --git a/.../src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/model/DestinationTests.kt b/.../src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/model/DestinationTests.kt
@@ -86,27 +86,27 @@ class DestinationTests : ESTestCase() {
     }
 
     fun `test custom webhook destination with url and no host`() {
-        val customWebhook = CustomWebhook("http://abc.com", null, null, -1, null, emptyMap(), emptyMap(), null, null)
+        val customWebhook = CustomWebhook("http://abc.com", null, null, -1, null, null, emptyMap(), emptyMap(), null, null)
         assertEquals("Url is manipulated", customWebhook.url, "http://abc.com")
     }
 
     fun `test custom webhook destination with host and no url`() {
         try {
-            val customWebhook = CustomWebhook(null, null, "abc.com", 80, null, emptyMap(), emptyMap(), null, null)
+            val customWebhook = CustomWebhook(null, null, "abc.com", 80, null, null, emptyMap(), emptyMap(), null, null)
             assertEquals("host is manipulated", customWebhook.host, "abc.com")
         } catch (ignored: IllegalArgumentException) {
         }
     }
 
     fun `test custom webhook destination with url and host`() {
         // In this case, url will be given priority
-        val customWebhook = CustomWebhook("http://abc.com", null, null, -1, null, emptyMap(), emptyMap(), null, null)
+        val customWebhook = CustomWebhook("http://abc.com", null, null, -1, null, null, emptyMap(), emptyMap(), null, null)
         assertEquals("Url is manipulated", customWebhook.url, "http://abc.com")
     }
 
     fun `test custom webhook destination with no url and no host`() {
         try {
-            CustomWebhook("", null, null, 80, null, emptyMap(), emptyMap(), null, null)
+            CustomWebhook("", null, null, 80, null, null, emptyMap(), emptyMap(), null, null)
             fail("Creating a custom webhook destination with empty url did not fail.")
         } catch (ignored: IllegalArgumentException) {
         }
@@ -175,6 +175,7 @@ class DestinationTests : ESTestCase() {
                     "localhost",
                     162,
                     "/tmp/",
+                    "POST",
                     mutableMapOf(),
                     mutableMapOf(),
                     "admin",
@@ -219,6 +220,7 @@ class DestinationTests : ESTestCase() {
                         "localhost",
                         162,
                         null,
+                        "POST",
                         mutableMapOf(),
                         mutableMapOf(),
                         null,

diff --git a/...kotlin/com/amazon/opendistroforelasticsearch/alerting/resthandler/DestinationRestApiIT.kt b/...kotlin/com/amazon/opendistroforelasticsearch/alerting/resthandler/DestinationRestApiIT.kt
@@ -102,7 +102,7 @@ class DestinationRestApiIT : AlertingRestTestCase() {
     }
 
     fun `test creating a custom webhook destination with url`() {
-        val customWebhook = CustomWebhook("http://abc.com", null, null, 80, null, emptyMap(), emptyMap(), null, null)
+        val customWebhook = CustomWebhook("http://abc.com", null, null, 80, null, "PUT", emptyMap(), emptyMap(), null, null)
         val destination = Destination(
                 type = DestinationType.CUSTOM_WEBHOOK,
                 name = "test",
@@ -119,7 +119,7 @@ class DestinationRestApiIT : AlertingRestTestCase() {
     }
 
     fun `test creating a custom webhook destination with host`() {
-        val customWebhook = CustomWebhook("", "http", "abc.com", 80, "a/b/c",
+        val customWebhook = CustomWebhook("", "http", "abc.com", 80, "a/b/c", "PATCH",
                 mapOf("foo" to "1", "bar" to "2"), mapOf("h1" to "1", "h2" to "2"), null, null)
         val destination = Destination(
                 type = DestinationType.CUSTOM_WEBHOOK,
@@ -137,25 +137,28 @@ class DestinationRestApiIT : AlertingRestTestCase() {
         assertEquals("Incorrect destination port", createdDestination.customWebhook?.port, 80)
         assertEquals("Incorrect destination path", createdDestination.customWebhook?.path, "a/b/c")
         assertEquals("Incorrect destination scheme", createdDestination.customWebhook?.scheme, "http")
+        assertEquals("Incorrect destination method", createdDestination.customWebhook?.method, "PATCH")
         Assert.assertNotNull("custom webhook object should not be null", createdDestination.customWebhook)
     }
 
     fun `test updating a custom webhook destination`() {
         val destination = createDestination()
-        val customWebhook = CustomWebhook("http://update1.com", "http", "abc.com", 80, null, emptyMap(), emptyMap(), null, null)
+        val customWebhook = CustomWebhook("http://update1.com", "http", "abc.com", 80, null, null, emptyMap(), emptyMap(), null, null)
         var updatedDestination = updateDestination(
                 destination.copy(name = "updatedName", customWebhook = customWebhook,
                         type = DestinationType.CUSTOM_WEBHOOK))
         assertEquals("Incorrect destination name after update", updatedDestination.name, "updatedName")
         assertEquals("Incorrect destination ID after update", updatedDestination.id, destination.id)
         assertEquals("Incorrect destination type after update", updatedDestination.type, DestinationType.CUSTOM_WEBHOOK)
         assertEquals("Incorrect destination url after update", "http://update1.com", updatedDestination.customWebhook?.url)
-        var updatedCustomWebhook = CustomWebhook("http://update2.com", "http", "abc.com", 80, null, emptyMap(), emptyMap(), null, null)
+        var updatedCustomWebhook = CustomWebhook("http://update2.com", "http", "abc.com", 80,
+                null, "PUT", emptyMap(), emptyMap(), null, null)
         updatedDestination = updateDestination(
                 destination.copy(name = "updatedName", customWebhook = updatedCustomWebhook,
                         type = DestinationType.CUSTOM_WEBHOOK))
         assertEquals("Incorrect destination url after update", "http://update2.com", updatedDestination.customWebhook?.url)
-        updatedCustomWebhook = CustomWebhook("", "http", "abc.com", 80, null, emptyMap(), emptyMap(), null, null)
+        assertEquals("Incorrect method after update", "PUT", updatedDestination.customWebhook?.method)
+        updatedCustomWebhook = CustomWebhook("", "http", "abc.com", 80, null, null, emptyMap(), emptyMap(), null, null)
         updatedDestination = updateDestination(
                 destination.copy(name = "updatedName", customWebhook = updatedCustomWebhook,
                         type = DestinationType.CUSTOM_WEBHOOK))

diff --git a/.../amazon/opendistroforelasticsearch/alerting/destination/client/DestinationHttpClient.java b/.../amazon/opendistroforelasticsearch/alerting/destination/client/DestinationHttpClient.java
@@ -21,7 +21,11 @@
 import org.apache.http.HttpResponse;
 import org.apache.http.client.config.RequestConfig;
 import org.apache.http.client.methods.CloseableHttpResponse;
+import org.apache.http.client.methods.HttpEntityEnclosingRequestBase;
+import org.apache.http.client.methods.HttpPatch;
 import org.apache.http.client.methods.HttpPost;
+import org.apache.http.client.methods.HttpPut;
+import org.apache.http.client.methods.HttpRequestBase;
 import org.apache.http.client.utils.URIBuilder;
 import org.apache.http.entity.StringEntity;
 import org.apache.http.impl.client.CloseableHttpClient;
@@ -103,30 +107,46 @@ public String execute(BaseMessage message) throws Exception {
 
     private CloseableHttpResponse getHttpResponse(BaseMessage message) throws Exception {
         URI uri = null;
-        HttpPost httpPostRequest = new HttpPost();
+        HttpRequestBase httpRequest;
         if (message instanceof CustomWebhookMessage) {
             CustomWebhookMessage customWebhookMessage = (CustomWebhookMessage) message;
             uri = buildUri(customWebhookMessage.getUrl(), customWebhookMessage.getScheme(), customWebhookMessage.getHost(),
                     customWebhookMessage.getPort(), customWebhookMessage.getPath(), customWebhookMessage.getQueryParams());
-
+            httpRequest = constructHttpRequest(((CustomWebhookMessage) message).getMethod());
             // set headers
             Map<String, String> headerParams = customWebhookMessage.getHeaderParams();
             if(headerParams == null || headerParams.isEmpty()) {
                 // set default header
-                httpPostRequest.setHeader("Content-Type", "application/json");
+                httpRequest.setHeader("Content-Type", "application/json");
             } else {
                 for (Map.Entry<String, String> e : customWebhookMessage.getHeaderParams().entrySet())
-                    httpPostRequest.setHeader(e.getKey(), e.getValue());
+                    httpRequest.setHeader(e.getKey(), e.getValue());
             }
         } else {
-            uri = buildUri(message.getUrl().trim(), null, null, -1, null, null);
+             httpRequest = new HttpPost();
+             uri = buildUri(message.getUrl().trim(), null, null, -1, null, null);
         }
 
-        httpPostRequest.setURI(uri);
-        StringEntity entity = new StringEntity(extractBody(message), StandardCharsets.UTF_8);
-        httpPostRequest.setEntity(entity);
+        httpRequest.setURI(uri);
+        if (httpRequest instanceof HttpEntityEnclosingRequestBase){
+            StringEntity entity = new StringEntity(extractBody(message), StandardCharsets.UTF_8);
+            ((HttpEntityEnclosingRequestBase) httpRequest).setEntity(entity);
+        }
 
-        return HTTP_CLIENT.execute(httpPostRequest);
+        return HTTP_CLIENT.execute(httpRequest);
+    }
+
+    private HttpRequestBase constructHttpRequest(String method) {
+        switch (method){
+            case "POST":
+                return new HttpPost();
+            case "PUT":
+                return new HttpPut();
+            case "PATCH":
+                return new HttpPatch();
+            default:
+                throw new IllegalArgumentException("Invalid method supplied");
+        }
     }
 
     private URI buildUri(String endpoint, String scheme, String host,

diff --git a/.../amazon/opendistroforelasticsearch/alerting/destination/message/CustomWebhookMessage.java b/.../amazon/opendistroforelasticsearch/alerting/destination/message/CustomWebhookMessage.java
@@ -15,6 +15,9 @@
 
 package com.amazon.opendistroforelasticsearch.alerting.destination.message;
 
+import org.apache.http.client.methods.HttpPatch;
+import org.apache.http.client.methods.HttpPost;
+import org.apache.http.client.methods.HttpPut;
 import org.elasticsearch.common.Strings;
 
 import java.util.Map;
@@ -28,6 +31,7 @@ public class CustomWebhookMessage extends BaseMessage {
     private String url;
     private String scheme;
     private String host;
+    private String method;
     private int port;
     private String path;
     private Map<String, String> queryParams;
@@ -42,6 +46,7 @@ private CustomWebhookMessage(final DestinationType destinationType,
                                  final String host,
                                  final Integer port,
                                  final String path,
+                                 final String method,
                                  final Map<String, String> queryParams,
                                  final Map<String, String> headerParams,
                                  final String userName,
@@ -75,6 +80,17 @@ private CustomWebhookMessage(final DestinationType destinationType,
             throw new IllegalArgumentException("Either fully qualified URL or host name should be provided");
         }
 
+        if (Strings.isNullOrEmpty(method)){
+            // Default to POST for backwards compatibility
+            this.method = "POST";
+        } else if (!HttpPost.METHOD_NAME.equals(method) && !HttpPut.METHOD_NAME.equals(method)
+                && !HttpPatch.METHOD_NAME.equals(method)) {
+            throw new IllegalArgumentException("Invalid method supplied. Only POST, PUT and PATCH are allowed");
+        } else {
+            this.method = method;
+        }
+
+
         this.message = message;
         this.url = url;
         this.host = host;
@@ -88,7 +104,7 @@ private CustomWebhookMessage(final DestinationType destinationType,
     public String toString() {
         return "DestinationType: " + destinationType + ", DestinationName:" +  destinationName +
                 ", Url: " + url + ", scheme: " + scheme + ", Host: " + host + ", Port: " +
-                port + ", Path: " + path + ", Message: " + message;
+                port + ", Path: " + path + ", Method: " + method + ", Message: " + message;
     }
 
     public static class Builder {
@@ -100,6 +116,7 @@ public static class Builder {
         private String host;
         private Integer port;
         private String path;
+        private String method;
         private Map<String, String> queryParams;
         private Map<String, String> headerParams;
         private String userName;
@@ -130,6 +147,11 @@ public CustomWebhookMessage.Builder withPath(String path) {
             return this;
         }
 
+        public CustomWebhookMessage.Builder withMethod(String method) {
+            this.method = method;
+            return this;
+        }
+
         public CustomWebhookMessage.Builder withQueryParams(Map<String, String> queryParams) {
             this.queryParams = queryParams;
             return this;
@@ -163,7 +185,7 @@ public CustomWebhookMessage.Builder withPassword(String password) {
         public CustomWebhookMessage build() {
             CustomWebhookMessage customWebhookMessage = new CustomWebhookMessage(
                     this.destinationType, this.destinationName, this.url,
-                    this.scheme, this.host, this.port, this.path, this.queryParams,
+                    this.scheme, this.host, this.port, this.path, this.method, this.queryParams,
                     this.headerParams, this.userName, this.password, this.message);
             return customWebhookMessage;
         }
@@ -185,6 +207,8 @@ public String getPath() {
         return path;
     }
 
+    public String getMethod() { return method; }
+
     public Map<String, String> getQueryParams() {
         return queryParams;
     }