diff --git a/docs/en/watcher/actions/pagerduty.asciidoc b/docs/en/watcher/actions/pagerduty.asciidoc index a7a3cfd8545..ce19020ce4a 100644 --- a/docs/en/watcher/actions/pagerduty.asciidoc +++ b/docs/en/watcher/actions/pagerduty.asciidoc @@ -45,7 +45,7 @@ payload as well as an array of contexts to the action. "attach_payload" : true, "client" : "/foo/bar/{{ctx.watch_id}}", "client_url" : "http://www.example.org/", - "context" : [ + "contexts" : [ { "type": "link", "href": "http://acme.pagerduty.com" diff --git a/plugin/src/main/java/org/elasticsearch/xpack/watcher/notification/pagerduty/IncidentEvent.java b/plugin/src/main/java/org/elasticsearch/xpack/watcher/notification/pagerduty/IncidentEvent.java index 28092ef460f..4353b099c75 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/watcher/notification/pagerduty/IncidentEvent.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/watcher/notification/pagerduty/IncidentEvent.java @@ -121,7 +121,7 @@ public class IncidentEvent implements ToXContentObject { builder.endObject(); } if (contexts != null && contexts.length > 0) { - builder.startArray(Fields.CONTEXT.getPreferredName()); + builder.startArray(Fields.CONTEXTS.getPreferredName()); for (IncidentEventContext context : contexts) { context.toXContent(builder, params); } @@ -154,7 +154,7 @@ public class IncidentEvent implements ToXContentObject { } builder.field(Fields.ATTACH_PAYLOAD.getPreferredName(), attachPayload); if (contexts != null) { - builder.startArray(Fields.CONTEXT.getPreferredName()); + builder.startArray(Fields.CONTEXTS.getPreferredName()); for (IncidentEventContext context : contexts) { context.toXContent(builder, params); } @@ -265,7 +265,7 @@ public class IncidentEvent implements ToXContentObject { proxy.toXContent(builder, params); } if (contexts != null) { - builder.startArray(Fields.CONTEXT.getPreferredName()); + builder.startArray(Fields.CONTEXTS.getPreferredName()); for (IncidentEventContext.Template context : contexts) { context.toXContent(builder, params); } @@ -341,7 +341,7 @@ public class IncidentEvent implements ToXContentObject { throw new ElasticsearchParseException("could not parse pager duty event template. failed to parse field [{}], " + "expected a boolean value but found [{}] instead", Fields.ATTACH_PAYLOAD.getPreferredName(), token); } - } else if (Fields.CONTEXT.match(currentFieldName)) { + } else if (Fields.CONTEXTS.match(currentFieldName) || Fields.CONTEXT_DEPRECATED.match(currentFieldName)) { if (token == XContentParser.Token.START_ARRAY) { List list = new ArrayList<>(); while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) { @@ -349,7 +349,7 @@ public class IncidentEvent implements ToXContentObject { list.add(IncidentEventContext.Template.parse(parser)); } catch (ElasticsearchParseException e) { throw new ElasticsearchParseException("could not parse pager duty event template. failed to parse field " + - "[{}]", Fields.CONTEXT.getPreferredName()); + "[{}]", parser.currentName()); } } contexts = list.toArray(new IncidentEventContext.Template[list.size()]); @@ -438,7 +438,11 @@ public class IncidentEvent implements ToXContentObject { ParseField CLIENT = new ParseField("client"); ParseField CLIENT_URL = new ParseField("client_url"); ParseField ATTACH_PAYLOAD = new ParseField("attach_payload"); - ParseField CONTEXT = new ParseField("context"); + ParseField CONTEXTS = new ParseField("contexts"); + // this field exists because in versions prior 6.0 we accidentally used context instead of contexts and thus the correct data + // was never picked up on the pagerduty side + // we need to keep this for BWC + ParseField CONTEXT_DEPRECATED = new ParseField("context"); ParseField SERVICE_KEY = new ParseField("service_key"); ParseField PAYLOAD = new ParseField("payload"); diff --git a/plugin/src/test/java/org/elasticsearch/xpack/watcher/actions/pagerduty/PagerDutyActionTests.java b/plugin/src/test/java/org/elasticsearch/xpack/watcher/actions/pagerduty/PagerDutyActionTests.java index 7cdd96cc457..6b199a50e44 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/watcher/actions/pagerduty/PagerDutyActionTests.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/watcher/actions/pagerduty/PagerDutyActionTests.java @@ -14,20 +14,20 @@ import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.json.JsonXContent; import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.xpack.watcher.actions.Action; 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.text.TextTemplate; import org.elasticsearch.xpack.watcher.common.text.TextTemplateEngine; +import org.elasticsearch.xpack.watcher.execution.WatchExecutionContext; +import org.elasticsearch.xpack.watcher.execution.Wid; import org.elasticsearch.xpack.watcher.notification.pagerduty.IncidentEvent; import org.elasticsearch.xpack.watcher.notification.pagerduty.IncidentEventContext; import org.elasticsearch.xpack.watcher.notification.pagerduty.IncidentEventDefaults; import org.elasticsearch.xpack.watcher.notification.pagerduty.PagerDutyAccount; import org.elasticsearch.xpack.watcher.notification.pagerduty.PagerDutyService; import org.elasticsearch.xpack.watcher.notification.pagerduty.SentEvent; -import org.elasticsearch.xpack.watcher.actions.Action; -import org.elasticsearch.xpack.watcher.execution.WatchExecutionContext; -import org.elasticsearch.xpack.watcher.execution.Wid; import org.elasticsearch.xpack.watcher.watch.Payload; import org.joda.time.DateTime; import org.joda.time.DateTimeZone; @@ -176,7 +176,8 @@ public class PagerDutyActionTests extends ESTestCase { IncidentEventContext.Template.link(new TextTemplate("_href"), new TextTemplate("_text")), IncidentEventContext.Template.image(new TextTemplate("_src"), new TextTemplate("_href"), new TextTemplate("_alt")) }; - builder.array("context", (Object) contexts); + String fieldName = randomBoolean() ? "contexts" : "context"; + builder.array(fieldName, (Object) contexts); } builder.endObject(); diff --git a/plugin/src/test/java/org/elasticsearch/xpack/watcher/notification/pagerduty/PagerDutyAccountsTests.java b/plugin/src/test/java/org/elasticsearch/xpack/watcher/notification/pagerduty/PagerDutyAccountsTests.java index 88e5b90fa69..fd476629203 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/watcher/notification/pagerduty/PagerDutyAccountsTests.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/watcher/notification/pagerduty/PagerDutyAccountsTests.java @@ -5,10 +5,12 @@ */ package org.elasticsearch.xpack.watcher.notification.pagerduty; +import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.settings.ClusterSettings; import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.common.settings.SettingsException; +import org.elasticsearch.common.xcontent.json.JsonXContent; import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.test.rest.yaml.ObjectPath; import org.elasticsearch.xpack.watcher.common.http.HttpClient; import org.elasticsearch.xpack.watcher.common.http.HttpProxy; import org.elasticsearch.xpack.watcher.common.http.HttpRequest; @@ -20,9 +22,7 @@ import org.mockito.ArgumentCaptor; import java.util.Collections; -import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; -import static org.hamcrest.Matchers.isOneOf; import static org.hamcrest.Matchers.notNullValue; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -36,102 +36,6 @@ public class PagerDutyAccountsTests extends ESTestCase { httpClient = mock(HttpClient.class); } - public void testSingleAccount() throws Exception { - Settings.Builder builder = Settings.builder() - .put("xpack.notification.pagerduty.default_account", "account1"); - addAccountSettings("account1", builder); - PagerDutyService service = new PagerDutyService(builder.build(), httpClient, new ClusterSettings(Settings.EMPTY, - Collections.singleton(PagerDutyService.PAGERDUTY_ACCOUNT_SETTING))); - PagerDutyAccount account = service.getAccount("account1"); - assertThat(account, notNullValue()); - assertThat(account.name, equalTo("account1")); - account = service.getAccount(null); // falling back on the default - assertThat(account, notNullValue()); - assertThat(account.name, equalTo("account1")); - } - - public void testSingleAccountNoExplicitDefault() throws Exception { - Settings.Builder builder = Settings.builder(); - addAccountSettings("account1", builder); - - PagerDutyService service = new PagerDutyService(builder.build(), httpClient, new ClusterSettings(Settings.EMPTY, - Collections.singleton(PagerDutyService.PAGERDUTY_ACCOUNT_SETTING))); - PagerDutyAccount account = service.getAccount("account1"); - assertThat(account, notNullValue()); - assertThat(account.name, equalTo("account1")); - account = service.getAccount(null); // falling back on the default - assertThat(account, notNullValue()); - assertThat(account.name, equalTo("account1")); - } - - public void testMultipleAccounts() throws Exception { - Settings.Builder builder = Settings.builder() - .put("xpack.notification.pagerduty.default_account", "account1"); - addAccountSettings("account1", builder); - addAccountSettings("account2", builder); - - PagerDutyService service = new PagerDutyService(builder.build(), httpClient, new ClusterSettings(Settings.EMPTY, - Collections.singleton(PagerDutyService.PAGERDUTY_ACCOUNT_SETTING))); - PagerDutyAccount account = service.getAccount("account1"); - assertThat(account, notNullValue()); - assertThat(account.name, equalTo("account1")); - account = service.getAccount("account2"); - assertThat(account, notNullValue()); - assertThat(account.name, equalTo("account2")); - account = service.getAccount(null); // falling back on the default - assertThat(account, notNullValue()); - assertThat(account.name, equalTo("account1")); - } - - public void testMultipleAccounts_NoExplicitDefault() throws Exception { - Settings.Builder builder = Settings.builder() - .put("xpack.notification.pagerduty.default_account", "account1"); - addAccountSettings("account1", builder); - addAccountSettings("account2", builder); - - PagerDutyService service = new PagerDutyService(builder.build(), httpClient, new ClusterSettings(Settings.EMPTY, - Collections.singleton(PagerDutyService.PAGERDUTY_ACCOUNT_SETTING))); - PagerDutyAccount account = service.getAccount("account1"); - assertThat(account, notNullValue()); - assertThat(account.name, equalTo("account1")); - account = service.getAccount("account2"); - assertThat(account, notNullValue()); - assertThat(account.name, equalTo("account2")); - account = service.getAccount(null); - assertThat(account, notNullValue()); - assertThat(account.name, isOneOf("account1", "account2")); - } - - public void testMultipleAccounts_UnknownDefault() throws Exception { - expectThrows(SettingsException.class, () -> { - Settings.Builder builder = Settings.builder() - .put("xpack.notification.pagerduty.default_account", "unknown"); - addAccountSettings("account1", builder); - addAccountSettings("account2", builder); - new PagerDutyService(builder.build(), httpClient, new ClusterSettings(Settings.EMPTY, - Collections.singleton(PagerDutyService.PAGERDUTY_ACCOUNT_SETTING))); - }); - } - - public void testNoAccount() throws Exception { - expectThrows(IllegalArgumentException.class, () -> { - Settings.Builder builder = Settings.builder(); - PagerDutyService service = new PagerDutyService(builder.build(), httpClient, new ClusterSettings(Settings.EMPTY, - Collections.singleton(PagerDutyService.PAGERDUTY_ACCOUNT_SETTING))); - service.getAccount(null); - }); - } - - public void testNoAccount_WithDefaultAccount() throws Exception { - try { - Settings.Builder builder = Settings.builder() - .put("xpack.notification.pagerduty.default_account", "unknown"); - new PagerDutyService(builder.build(), httpClient, new ClusterSettings(Settings.EMPTY, - Collections.singleton(PagerDutyService.PAGERDUTY_ACCOUNT_SETTING))); - fail("Expected a SettingsException to happen"); - } catch (SettingsException e) {} - } - public void testProxy() throws Exception { Settings.Builder builder = Settings.builder().put("xpack.notification.pagerduty.default_account", "account1"); addAccountSettings("account1", builder); @@ -150,6 +54,31 @@ public class PagerDutyAccountsTests extends ESTestCase { assertThat(request.proxy(), is(proxy)); } + // in earlier versions of the PD action the wrong JSON was sent, because the contexts field was named context + // the pagerduty API accepts any JSON, thus this was never caught + public void testContextIsSentCorrect() throws Exception { + Settings.Builder builder = Settings.builder().put("xpack.notification.pagerduty.default_account", "account1"); + addAccountSettings("account1", builder); + PagerDutyService service = new PagerDutyService(builder.build(), httpClient, new ClusterSettings(Settings.EMPTY, + Collections.singleton(PagerDutyService.PAGERDUTY_ACCOUNT_SETTING))); + PagerDutyAccount account = service.getAccount("account1"); + + ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(HttpRequest.class); + when(httpClient.execute(argumentCaptor.capture())).thenReturn(new HttpResponse(200)); + + IncidentEventContext[] contexts = { + IncidentEventContext.link("https://www.elastic.co/products/x-pack/alerting", "Go to the Elastic.co Alerting website"), + IncidentEventContext.image("https://www.elastic.co/assets/blte5d899fd0b0e6808/icon-alerting-bb.svg", + "https://www.elastic.co/products/x-pack/alerting", "X-Pack-Alerting website link with log") + }; + IncidentEvent event = new IncidentEvent("foo", null, null, null, null, account.getName(), true, contexts, HttpProxy.NO_PROXY); + account.send(event, Payload.EMPTY); + + HttpRequest request = argumentCaptor.getValue(); + ObjectPath source = ObjectPath.createFromXContent(JsonXContent.jsonXContent, new BytesArray(request.body())); + assertThat(source.evaluate("contexts"), notNullValue()); + } + private void addAccountSettings(String name, Settings.Builder builder) { builder.put("xpack.notification.pagerduty.account." + name + ".service_api_key", randomAlphaOfLength(50)); Settings defaults = SlackMessageDefaultsTests.randomSettings(); diff --git a/plugin/src/test/java/org/elasticsearch/xpack/watcher/test/integration/PagerDutyServiceTests.java b/plugin/src/test/java/org/elasticsearch/xpack/watcher/test/integration/PagerDutyServiceTests.java index 0998f578a4f..d0275a94c79 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/watcher/test/integration/PagerDutyServiceTests.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/watcher/test/integration/PagerDutyServiceTests.java @@ -48,8 +48,9 @@ public class PagerDutyServiceTests extends AbstractWatcherIntegrationTestCase { IncidentEvent event = new IncidentEvent("#testIncidentEvent()", null, null, "PagerDutyServiceTests", "_client_url", "_account", true, new IncidentEventContext[] { - IncidentEventContext.link("_href", "_text"), - IncidentEventContext.image("_src", "_href", "_alt") + IncidentEventContext.link("https://www.elastic.co/products/x-pack/alerting", "Go to the Elastic.co Alerting website"), + IncidentEventContext.image("https://www.elastic.co/assets/blte5d899fd0b0e6808/icon-alerting-bb.svg", + "https://www.elastic.co/products/x-pack/alerting", "X-Pack-Alerting website link with log") }, null); Payload payload = new Payload.Simple("_key", "_val");