diff --git a/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/model/destination/CustomWebhook.kt b/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/model/destination/CustomWebhook.kt index d4c74744..3be8d754 100644 --- a/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/model/destination/CustomWebhook.kt +++ b/alerting/src/main/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, val headerParams: Map, 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 = mutableMapOf() var headerParams: Map = 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/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/model/destination/Destination.kt b/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/model/destination/Destination.kt index 6d60a804..dc21da93 100644 --- a/alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/model/destination/Destination.kt +++ b/alerting/src/main/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/alerting/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/model/DestinationTests.kt b/alerting/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/model/DestinationTests.kt index 7bae2a3d..a14ba1dc 100644 --- a/alerting/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/model/DestinationTests.kt +++ b/alerting/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/model/DestinationTests.kt @@ -86,13 +86,13 @@ 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) { } @@ -100,13 +100,13 @@ class DestinationTests : ESTestCase() { 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/alerting/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/resthandler/DestinationRestApiIT.kt b/alerting/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/resthandler/DestinationRestApiIT.kt index 64d9fbe7..e5ea7f0b 100644 --- a/alerting/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/resthandler/DestinationRestApiIT.kt +++ b/alerting/src/test/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,12 +137,13 @@ 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)) @@ -150,12 +151,14 @@ class DestinationRestApiIT : AlertingRestTestCase() { 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/notification/src/main/java/com/amazon/opendistroforelasticsearch/alerting/destination/client/DestinationHttpClient.java b/notification/src/main/java/com/amazon/opendistroforelasticsearch/alerting/destination/client/DestinationHttpClient.java index 93d363ed..489464cb 100644 --- a/notification/src/main/java/com/amazon/opendistroforelasticsearch/alerting/destination/client/DestinationHttpClient.java +++ b/notification/src/main/java/com/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 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 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/notification/src/main/java/com/amazon/opendistroforelasticsearch/alerting/destination/message/CustomWebhookMessage.java b/notification/src/main/java/com/amazon/opendistroforelasticsearch/alerting/destination/message/CustomWebhookMessage.java index 27a3140f..e134703f 100644 --- a/notification/src/main/java/com/amazon/opendistroforelasticsearch/alerting/destination/message/CustomWebhookMessage.java +++ b/notification/src/main/java/com/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 queryParams; @@ -42,6 +46,7 @@ private CustomWebhookMessage(final DestinationType destinationType, final String host, final Integer port, final String path, + final String method, final Map queryParams, final Map 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 queryParams; private Map 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 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 getQueryParams() { return queryParams; } diff --git a/notification/src/test/java/com/amazon/opendistroforelasticsearch/alerting/destination/CustomWebhookMessageTest.java b/notification/src/test/java/com/amazon/opendistroforelasticsearch/alerting/destination/CustomWebhookMessageTest.java index 806c7c31..48a935b6 100644 --- a/notification/src/test/java/com/amazon/opendistroforelasticsearch/alerting/destination/CustomWebhookMessageTest.java +++ b/notification/src/test/java/com/amazon/opendistroforelasticsearch/alerting/destination/CustomWebhookMessageTest.java @@ -23,20 +23,41 @@ import com.amazon.opendistroforelasticsearch.alerting.destination.message.DestinationType; import com.amazon.opendistroforelasticsearch.alerting.destination.response.DestinationResponse; import org.apache.http.client.methods.CloseableHttpResponse; +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.HttpUriRequest; import org.apache.http.entity.StringEntity; import org.apache.http.impl.client.CloseableHttpClient; import org.apache.http.message.BasicStatusLine; import org.easymock.EasyMock; import org.elasticsearch.rest.RestStatus; import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; import java.util.HashMap; import java.util.Map; import static org.junit.Assert.assertEquals; + +@RunWith(Parameterized.class) public class CustomWebhookMessageTest { + @Parameterized.Parameters(name = "Param: {0}={1}") + public static Object[][] params() { + return new Object[][]{ + {"POST", HttpPost.class}, + {"PUT", HttpPut.class}, + {"PATCH", HttpPatch.class}, + }; + } + + @Parameterized.Parameter(0) + public String method; + + @Parameterized.Parameter(1) + public Class expectedHttpClass; @Test public void testCustomWebhookMessage_NullEntityResponse() throws Exception { @@ -49,6 +70,7 @@ public void testCustomWebhookMessage_NullEntityResponse() throws Exception { .build(); CloseableHttpResponse httpResponse = EasyMock.createMock(CloseableHttpResponse.class); EasyMock.expect(mockHttpClient.execute(EasyMock.anyObject(HttpPost.class))).andReturn(httpResponse); + EasyMock.expect(mockHttpClient.execute(EasyMock.isA(expectedHttpClass))).andReturn(httpResponse); BasicStatusLine mockStatusLine = EasyMock.createMock(BasicStatusLine.class); @@ -74,7 +96,7 @@ public void testCustomWebhookMessage_NullEntityResponse() throws Exception { "All member callout: @All All Present member callout: @Present\"}"; BaseMessage bm = new CustomWebhookMessage.Builder("abc").withHost("hooks.chime.aws"). withPath("incomingwebhooks/383c0e2b-d028-44f4-8d38-696754bc4574"). - withMessage(message). + withMessage(message).withMethod(method). withQueryParams(queryParams).build(); DestinationResponse actualCustomResponse = (DestinationResponse) Notification.publish(bm); @@ -91,7 +113,7 @@ public void testCustomWebhookMessage_EmptyEntityResponse() throws Exception { .withStatusCode(RestStatus.OK.getStatus()) .build(); CloseableHttpResponse httpResponse = EasyMock.createMock(CloseableHttpResponse.class); - EasyMock.expect(mockHttpClient.execute(EasyMock.anyObject(HttpPost.class))).andReturn(httpResponse); + EasyMock.expect(mockHttpClient.execute(EasyMock.isA(expectedHttpClass))).andReturn(httpResponse); BasicStatusLine mockStatusLine = EasyMock.createMock(BasicStatusLine.class); @@ -117,7 +139,7 @@ public void testCustomWebhookMessage_EmptyEntityResponse() throws Exception { "All member callout: @All All Present member callout: @Present\"}"; BaseMessage bm = new CustomWebhookMessage.Builder("abc").withHost("hooks.chime.aws"). withPath("incomingwebhooks/383c0e2b-d028-44f4-8d38-696754bc4574"). - withMessage(message). + withMessage(message).withMethod(method). withQueryParams(queryParams).build(); DestinationResponse actualCustomResponse = (DestinationResponse) Notification.publish(bm); @@ -136,7 +158,7 @@ public void testCustomWebhookMessage_NonemptyEntityResponse() throws Exception { .withStatusCode(RestStatus.OK.getStatus()) .build(); CloseableHttpResponse httpResponse = EasyMock.createMock(CloseableHttpResponse.class); - EasyMock.expect(mockHttpClient.execute(EasyMock.anyObject(HttpPost.class))).andReturn(httpResponse); + EasyMock.expect(mockHttpClient.execute(EasyMock.isA(expectedHttpClass))).andReturn(httpResponse); BasicStatusLine mockStatusLine = EasyMock.createMock(BasicStatusLine.class); @@ -162,7 +184,7 @@ public void testCustomWebhookMessage_NonemptyEntityResponse() throws Exception { "All member callout: @All All Present member callout: @Present\"}"; BaseMessage bm = new CustomWebhookMessage.Builder("abc").withHost("hooks.chime.aws"). withPath("incomingwebhooks/383c0e2b-d028-44f4-8d38-696754bc4574"). - withMessage(message). + withMessage(message).withMethod(method). withQueryParams(queryParams).build(); DestinationResponse actualCustomResponse = (DestinationResponse) Notification.publish(bm);