diff --git a/plugin/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java b/plugin/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java index e8d3647920a..f5d93725bc8 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java @@ -372,7 +372,7 @@ public class Watcher implements ActionPlugin { return Arrays.asList(registry, watcherClient, inputRegistry, historyStore, triggerService, triggeredWatchParser, watcherLifeCycleService, executionService, triggerEngineListener, watcherService, watchParser, configuredTriggerEngine, triggeredWatchStore, watcherSearchTemplateService, watcherIndexTemplateRegistry, - slackService, pagerDutyService); + slackService, pagerDutyService, hipChatService); } protected TriggerEngine getTriggerEngine(Clock clock, ScheduleRegistry scheduleRegistry) { diff --git a/plugin/src/main/java/org/elasticsearch/xpack/watcher/notification/hipchat/IntegrationAccount.java b/plugin/src/main/java/org/elasticsearch/xpack/watcher/notification/hipchat/IntegrationAccount.java index 64182bcb137..8af00ae8f81 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/watcher/notification/hipchat/IntegrationAccount.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/watcher/notification/hipchat/IntegrationAccount.java @@ -95,10 +95,11 @@ public class IntegrationAccount extends HipChatAccount { } private HttpRequest buildRoomRequest(String room, final HipChatMessage message, HttpProxy proxy) { + String urlEncodedRoom = HttpRequest.encodeUrl(room); HttpRequest.Builder builder = server.httpRequest() .method(HttpMethod.POST) .scheme(Scheme.HTTPS) - .path("/v2/room/" + room + "/notification") + .path("/v2/room/" + urlEncodedRoom + "/notification") .setHeader("Content-Type", "application/json") .setHeader("Authorization", "Bearer " + authToken) .body(Strings.toString((xbuilder, params) -> { diff --git a/plugin/src/main/java/org/elasticsearch/xpack/watcher/notification/hipchat/UserAccount.java b/plugin/src/main/java/org/elasticsearch/xpack/watcher/notification/hipchat/UserAccount.java index b178fcc2953..c0b89cc66ec 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/watcher/notification/hipchat/UserAccount.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/watcher/notification/hipchat/UserAccount.java @@ -23,6 +23,8 @@ import org.elasticsearch.xpack.watcher.notification.hipchat.HipChatMessage.Forma import org.elasticsearch.xpack.watcher.actions.hipchat.HipChatAction; import java.io.IOException; +import java.net.URI; +import java.net.URISyntaxException; import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -108,10 +110,11 @@ public class UserAccount extends HipChatAccount { } public HttpRequest buildRoomRequest(String room, final HipChatMessage message, HttpProxy proxy) { + String urlEncodedRoom = encodeRoom(room); HttpRequest.Builder builder = server.httpRequest() .method(HttpMethod.POST) .scheme(Scheme.HTTPS) - .path("/v2/room/" + room + "/notification") + .path("/v2/room/" + urlEncodedRoom + "/notification") .setHeader("Content-Type", "application/json") .setHeader("Authorization", "Bearer " + authToken) .body(Strings.toString((xbuilder, params) -> { @@ -133,6 +136,18 @@ public class UserAccount extends HipChatAccount { return builder.build(); } + // this specific hipchat API does not accept application-form encoding, but requires real URL encoding + // spaces must not be replaced with a plus, but rather with %20 + // this workaround ensures, that this happens + private String encodeRoom(String text) { + try { + return new URI("//", "", "", text, null).getRawQuery(); + } catch (URISyntaxException e) { + throw new IllegalArgumentException("failed to URL encode text [" + text + "]", e); + } + + } + public HttpRequest buildUserRequest(String user, final HipChatMessage message, HttpProxy proxy) { HttpRequest.Builder builder = server.httpRequest() .method(HttpMethod.POST) diff --git a/plugin/src/main/java/org/elasticsearch/xpack/watcher/notification/hipchat/V1Account.java b/plugin/src/main/java/org/elasticsearch/xpack/watcher/notification/hipchat/V1Account.java index 299a40edf98..084cff2d094 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/watcher/notification/hipchat/V1Account.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/watcher/notification/hipchat/V1Account.java @@ -103,7 +103,7 @@ public class V1Account extends HipChatAccount { builder.proxy(proxy); } StringBuilder body = new StringBuilder(); - body.append("room_id=").append(room); + body.append("room_id=").append(HttpRequest.encodeUrl(room)); body.append("&from=").append(HttpRequest.encodeUrl(message.from)); body.append("&message=").append(HttpRequest.encodeUrl(message.body)); if (message.format != null) { diff --git a/plugin/src/test/java/org/elasticsearch/xpack/watcher/notification/hipchat/IntegrationAccountTests.java b/plugin/src/test/java/org/elasticsearch/xpack/watcher/notification/hipchat/IntegrationAccountTests.java index 1fec9dd2200..d37f472a854 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/watcher/notification/hipchat/IntegrationAccountTests.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/watcher/notification/hipchat/IntegrationAccountTests.java @@ -24,6 +24,7 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; public class IntegrationAccountTests extends ESTestCase { + public void testSettings() throws Exception { String accountName = "_name"; @@ -113,11 +114,12 @@ public class IntegrationAccountTests extends ESTestCase { public void testSend() throws Exception { HttpClient httpClient = mock(HttpClient.class); + String room = "Room with Spaces"; IntegrationAccount account = new IntegrationAccount("_name", Settings.builder() .put("host", "_host") .put("port", "443") .put("auth_token", "_token") - .put("room", "_room") + .put("room", room) .build(), HipChatServer.DEFAULT, httpClient, mock(Logger.class)); HipChatMessage.Format format = randomFrom(HipChatMessage.Format.values()); @@ -128,7 +130,8 @@ public class IntegrationAccountTests extends ESTestCase { HttpRequest req = HttpRequest.builder("_host", 443) .method(HttpMethod.POST) .scheme(Scheme.HTTPS) - .path("/v2/room/_room/notification") + // url encoded already + .path("/v2/room/Room+with+Spaces/notification") .setHeader("Content-Type", "application/json") .setHeader("Authorization", "Bearer _token") .body(Strings.toString((builder, params) -> { diff --git a/plugin/src/test/java/org/elasticsearch/xpack/watcher/notification/hipchat/UserAccountTests.java b/plugin/src/test/java/org/elasticsearch/xpack/watcher/notification/hipchat/UserAccountTests.java index 07a6165b47c..a38c7f5ef13 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/watcher/notification/hipchat/UserAccountTests.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/watcher/notification/hipchat/UserAccountTests.java @@ -12,18 +12,23 @@ import org.elasticsearch.common.settings.SettingsException; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xpack.watcher.common.http.HttpClient; import org.elasticsearch.xpack.watcher.common.http.HttpMethod; +import org.elasticsearch.xpack.watcher.common.http.HttpProxy; import org.elasticsearch.xpack.watcher.common.http.HttpRequest; import org.elasticsearch.xpack.watcher.common.http.HttpResponse; import org.elasticsearch.xpack.watcher.common.http.Scheme; import org.elasticsearch.xpack.watcher.common.text.TextTemplate; import org.elasticsearch.xpack.watcher.test.MockTextTemplateEngine; +import org.mockito.ArgumentCaptor; import java.util.HashMap; import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; import static org.hamcrest.Matchers.arrayContaining; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.nullValue; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; @@ -265,6 +270,29 @@ public class UserAccountTests extends ESTestCase { assertThat(message.format, is(nullValue())); } + public void testRoomNameIsUrlEncoded() throws Exception { + Settings settings = Settings.builder() + .put("user", "testuser") + .put("auth_token", "awesome-auth-token") + .build(); + HipChatServer hipChatServer = mock(HipChatServer.class); + HttpClient httpClient = mock(HttpClient.class); + UserAccount account = new UserAccount("notify-monitoring", settings, hipChatServer, httpClient, logger); + + TextTemplate[] rooms = new TextTemplate[] { new TextTemplate("Room with Spaces")}; + HipChatMessage.Template template = + new HipChatMessage.Template(new TextTemplate("body"), rooms, null, "sender", HipChatMessage.Format.TEXT, null, true); + + HipChatMessage message = account.render("watchId", "actionId", new MockTextTemplateEngine(), template, new HashMap<>()); + account.send(message, HttpProxy.NO_PROXY); + + ArgumentCaptor captor = ArgumentCaptor.forClass(HttpRequest.class); + verify(httpClient).execute(captor.capture()); + assertThat(captor.getAllValues(), hasSize(1)); + assertThat(captor.getValue().path(), not(containsString("Room with Spaces"))); + assertThat(captor.getValue().path(), containsString("Room%20with%20Spaces")); + } + private UserAccount createUserAccount(Settings settings) { HipChatServer hipChatServer = mock(HipChatServer.class); HttpClient httpClient = mock(HttpClient.class); diff --git a/plugin/src/test/java/org/elasticsearch/xpack/watcher/notification/hipchat/V1AccountTests.java b/plugin/src/test/java/org/elasticsearch/xpack/watcher/notification/hipchat/V1AccountTests.java index 08698ff2f64..d976e75330e 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/watcher/notification/hipchat/V1AccountTests.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/watcher/notification/hipchat/V1AccountTests.java @@ -110,7 +110,8 @@ public class V1AccountTests extends ESTestCase { HipChatMessage.Format format = randomFrom(HipChatMessage.Format.values()); HipChatMessage.Color color = randomFrom(HipChatMessage.Color.values()); Boolean notify = randomBoolean(); - HipChatMessage message = new HipChatMessage("_body", new String[] { "_r1", "_r2" }, null, "_from", format, color, notify); + HipChatMessage message = new HipChatMessage("_body", new String[] { "Room with Spaces", "_r2" }, null, "_from", format, + color, notify); HttpRequest req1 = HttpRequest.builder("_host", 443) .method(HttpMethod.POST) @@ -120,7 +121,7 @@ public class V1AccountTests extends ESTestCase { .setParam("format", "json") .setParam("auth_token", "_token") .body(new StringBuilder() - .append("room_id=").append("_r1&") + .append("room_id=").append("Room+with+Spaces&") .append("from=").append("_from&") .append("message=").append("_body&") .append("message_format=").append(format.value()).append("&") diff --git a/plugin/src/test/java/org/elasticsearch/xpack/watcher/test/integration/HipChatServiceTests.java b/plugin/src/test/java/org/elasticsearch/xpack/watcher/test/integration/HipChatServiceTests.java index c96a1afcd55..6222c59abe6 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/watcher/test/integration/HipChatServiceTests.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/watcher/test/integration/HipChatServiceTests.java @@ -5,21 +5,29 @@ */ package org.elasticsearch.xpack.watcher.test.integration; -import org.apache.lucene.util.LuceneTestCase; import org.elasticsearch.ExceptionsHelper; import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.plugins.Plugin; +import org.elasticsearch.script.MockMustacheScriptEngine; import org.elasticsearch.test.junit.annotations.Network; import org.elasticsearch.test.junit.annotations.TestLogging; +import org.elasticsearch.xpack.XPackPlugin; +import org.elasticsearch.xpack.XPackSettings; +import org.elasticsearch.xpack.XPackSingleNodeTestCase; +import org.elasticsearch.xpack.watcher.actions.hipchat.HipChatAction; +import org.elasticsearch.xpack.watcher.client.WatcherClient; +import org.elasticsearch.xpack.watcher.condition.AlwaysCondition; +import org.elasticsearch.xpack.watcher.history.HistoryStore; import org.elasticsearch.xpack.watcher.notification.hipchat.HipChatAccount; import org.elasticsearch.xpack.watcher.notification.hipchat.HipChatMessage; import org.elasticsearch.xpack.watcher.notification.hipchat.HipChatService; import org.elasticsearch.xpack.watcher.notification.hipchat.SentMessages; -import org.elasticsearch.xpack.watcher.actions.hipchat.HipChatAction; -import org.elasticsearch.xpack.watcher.condition.AlwaysCondition; -import org.elasticsearch.xpack.watcher.test.AbstractWatcherIntegrationTestCase; import org.elasticsearch.xpack.watcher.transport.actions.put.PutWatchResponse; +import java.util.Arrays; +import java.util.Collection; + import static org.elasticsearch.index.query.QueryBuilders.boolQuery; import static org.elasticsearch.index.query.QueryBuilders.termQuery; import static org.elasticsearch.search.builder.SearchSourceBuilder.searchSource; @@ -34,21 +42,20 @@ import static org.hamcrest.Matchers.notNullValue; @Network @TestLogging("org.elasticsearch.xpack.watcher.common.http:TRACE") -public class HipChatServiceTests extends AbstractWatcherIntegrationTestCase { +public class HipChatServiceTests extends XPackSingleNodeTestCase { + @Override - protected boolean timeWarped() { - return true; + protected Collection> getPlugins() { + return Arrays.asList(XPackPlugin.class, MockMustacheScriptEngine.TestPlugin.class); } @Override - protected boolean enableSecurity() { - return false; - } - - @Override - protected Settings nodeSettings(int nodeOrdinal) { + protected Settings nodeSettings() { return Settings.builder() - .put(super.nodeSettings(nodeOrdinal)) + .put(super.nodeSettings()) + .put(XPackSettings.WATCHER_ENABLED.getKey(), true) + .put(XPackSettings.SECURITY_ENABLED.getKey(), false) + .put(XPackSettings.MONITORING_ENABLED.getKey(), false) // this is for the `test-watcher-integration` group level integration in HipChat .put("xpack.notification.hipchat.account.integration_account.profile", "integration") @@ -58,7 +65,7 @@ public class HipChatServiceTests extends AbstractWatcherIntegrationTestCase { // this is for the Watcher Test account in HipChat .put("xpack.notification.hipchat.account.user_account.profile", "user") - .put("xpack.notification.hipchat.account.user_account.auth_token", "12rNQUuQ0wObfRVeoVD8OeoAnosCT8tSTV5UjsII") + .put("xpack.notification.hipchat.account.user_account.auth_token", "4UefsFLvKRw01EMN5vo3oyoY6BLiz7IQBQbGug8K") // this is for the `test-watcher-v1` notification token .put("xpack.notification.hipchat.account.v1_account.profile", "v1") @@ -66,12 +73,11 @@ public class HipChatServiceTests extends AbstractWatcherIntegrationTestCase { .build(); } - @LuceneTestCase.AwaitsFix(bugUrl = "https://github.com/elastic/infra/issues/2726") public void testSendMessageV1Account() throws Exception { - HipChatService service = getInstanceFromMaster(HipChatService.class); + HipChatService service = getInstanceFromNode(HipChatService.class); HipChatMessage hipChatMessage = new HipChatMessage( "HipChatServiceTests#testSendMessage_V1Account", - new String[] { "test-watcher", "test-watcher-2" }, + new String[] { "test-watcher", "test-watcher-2", "test watcher with spaces" }, null, // users are unsupported in v1 "watcher-tests", HipChatMessage.Format.TEXT, @@ -81,12 +87,11 @@ public class HipChatServiceTests extends AbstractWatcherIntegrationTestCase { HipChatAccount account = service.getAccount("v1_account"); assertThat(account, notNullValue()); SentMessages messages = account.send(hipChatMessage, null); - assertSentMessagesAreValid(2, messages); + assertSentMessagesAreValid(3, messages); } - @LuceneTestCase.AwaitsFix(bugUrl = "https://github.com/elastic/infra/issues/2726") public void testSendMessageIntegrationAccount() throws Exception { - HipChatService service = getInstanceFromMaster(HipChatService.class); + HipChatService service = getInstanceFromNode(HipChatService.class); HipChatMessage.Color color = randomFrom(HipChatMessage.Color.values()); HipChatMessage hipChatMessage = new HipChatMessage( "HipChatServiceTests#testSendMessage_IntegrationAccount colored " + color.value(), @@ -103,15 +108,14 @@ public class HipChatServiceTests extends AbstractWatcherIntegrationTestCase { assertSentMessagesAreValid(1, messages); } - @LuceneTestCase.AwaitsFix(bugUrl = "https://github.com/elastic/infra/issues/2726") public void testSendMessageUserAccount() throws Exception { - HipChatService service = getInstanceFromMaster(HipChatService.class); + HipChatService service = getInstanceFromNode(HipChatService.class); HipChatMessage.Color color = randomFrom(HipChatMessage.Color.values()); HipChatMessage hipChatMessage = new HipChatMessage( "HipChatServiceTests#testSendMessage_UserAccount colored " + color.value(), - new String[] { "test-watcher", "test-watcher-2" }, + new String[] { "test-watcher", "test-watcher-2", "test watcher with spaces" }, new String[] { "watcher@elastic.co" }, - null, // custom "from" is not supported by integration profiles + null, HipChatMessage.Format.TEXT, color, false); @@ -119,10 +123,9 @@ public class HipChatServiceTests extends AbstractWatcherIntegrationTestCase { HipChatAccount account = service.getAccount("user_account"); assertThat(account, notNullValue()); SentMessages messages = account.send(hipChatMessage, null); - assertSentMessagesAreValid(3, messages); + assertSentMessagesAreValid(4, messages); } - @LuceneTestCase.AwaitsFix(bugUrl = "https://github.com/elastic/infra/issues/2726") public void testWatchWithHipChatAction() throws Exception { HipChatAccount.Profile profile = randomFrom(HipChatAccount.Profile.values()); HipChatMessage.Color color = randomFrom(HipChatMessage.Color.values()); @@ -132,7 +135,7 @@ public class HipChatServiceTests extends AbstractWatcherIntegrationTestCase { case USER: account = "user_account"; actionBuilder = hipchatAction(account, "_message") - .addRooms("test-watcher", "test-watcher-2") + .addRooms("test-watcher", "test-watcher-2", "test watcher with spaces") .addUsers("watcher@elastic.co") .setFormat(HipChatMessage.Format.TEXT) .setColor(color) @@ -151,7 +154,7 @@ public class HipChatServiceTests extends AbstractWatcherIntegrationTestCase { assertThat(profile, is(HipChatAccount.Profile.V1)); account = "v1_account"; actionBuilder = hipchatAction(account, "_message") - .addRooms("test-watcher", "test-watcher-2") + .addRooms("test-watcher", "test-watcher-2", "test watcher with spaces") .setFrom("watcher-test") .setFormat(HipChatMessage.Format.TEXT) .setColor(color) @@ -159,7 +162,8 @@ public class HipChatServiceTests extends AbstractWatcherIntegrationTestCase { } String id = randomAlphaOfLength(10); - PutWatchResponse putWatchResponse = watcherClient().preparePutWatch(id).setSource(watchBuilder() + WatcherClient watcherClient = new WatcherClient(client()); + PutWatchResponse putWatchResponse = watcherClient.preparePutWatch(id).setSource(watchBuilder() .trigger(schedule(interval("10m"))) .input(simpleInput("ref", "HipChatServiceTests#testWatchWithHipChatAction")) .condition(AlwaysCondition.INSTANCE) @@ -168,29 +172,27 @@ public class HipChatServiceTests extends AbstractWatcherIntegrationTestCase { assertThat(putWatchResponse.isCreated(), is(true)); - timeWarp().trigger(id); - flush(); - refresh(); + watcherClient.prepareExecuteWatch(id).setRecordExecution(true).execute().actionGet(); - assertWatchWithMinimumPerformedActionsCount(id, 1L, false); - - SearchResponse response = searchHistory(searchSource().query(boolQuery() + client().admin().indices().prepareRefresh(HistoryStore.INDEX_PREFIX_WITH_TEMPLATE + "*").execute().actionGet(); + SearchResponse response = client().prepareSearch(HistoryStore.INDEX_PREFIX_WITH_TEMPLATE + "*") + .setSource(searchSource().query(boolQuery() .must(termQuery("result.actions.id", "hipchat")) .must(termQuery("result.actions.type", "hipchat")) .must(termQuery("result.actions.status", "success")) .must(termQuery("result.actions.hipchat.account", account)) - .must(termQuery("result.actions.hipchat.sent_messages.status", "success")))); + .must(termQuery("result.actions.hipchat.sent_messages.status", "success")))) + .get(); assertThat(response, notNullValue()); assertThat(response.getHits().getTotalHits(), is(1L)); } - @LuceneTestCase.AwaitsFix(bugUrl = "https://github.com/elastic/infra/issues/2726") public void testDefaultValuesForColorAndFormatWorks() { - HipChatService service = getInstanceFromMaster(HipChatService.class); + HipChatService service = getInstanceFromNode(HipChatService.class); HipChatMessage hipChatMessage = new HipChatMessage( "HipChatServiceTests#testSendMessage_UserAccount with default Color and text", - new String[] { "test-watcher" }, + new String[] { "test-watcher", "test-watcher-2", "test watcher with spaces" }, new String[] { "watcher@elastic.co" }, null, // custom "from" is not supported by integration profiles null, @@ -200,7 +202,7 @@ public class HipChatServiceTests extends AbstractWatcherIntegrationTestCase { HipChatAccount account = service.getAccount("user_account"); assertThat(account, notNullValue()); SentMessages messages = account.send(hipChatMessage, null); - assertSentMessagesAreValid(2, messages); + assertSentMessagesAreValid(4, messages); } private void assertSentMessagesAreValid(int expectedMessageSize, SentMessages messages) {