From f89207cd82fd95ac5c586eb189d844ac7db39553 Mon Sep 17 00:00:00 2001 From: Michael Basnight Date: Thu, 28 Jun 2018 08:30:28 -0500 Subject: [PATCH 01/11] Make watcher plugin reloadable This commit allows for rebuilding watcher secure secrets via the reload_secure_settings API call. The commit also renames a method in the Notification Service to make it a bit more readable. --- .../elasticsearch/xpack/watcher/Watcher.java | 63 +++++++++++++++---- .../notification/NotificationService.java | 2 +- .../notification/email/EmailService.java | 4 +- .../notification/hipchat/HipChatService.java | 8 +-- .../notification/jira/JiraService.java | 4 +- .../pagerduty/PagerDutyService.java | 2 +- .../notification/slack/SlackService.java | 4 +- .../NotificationServiceTests.java | 4 +- .../xpack/watcher/WatcherPluginTests.java | 42 +++++++++++++ 9 files changed, 107 insertions(+), 26 deletions(-) diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java index f7d51d328a797..47d4199c45460 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java @@ -6,6 +6,7 @@ package org.elasticsearch.xpack.watcher; import org.apache.logging.log4j.Logger; +import org.apache.lucene.util.SetOnce; import org.elasticsearch.action.ActionRequest; import org.elasticsearch.action.ActionResponse; import org.elasticsearch.bootstrap.BootstrapCheck; @@ -38,6 +39,7 @@ import org.elasticsearch.node.Node; import org.elasticsearch.plugins.ActionPlugin; import org.elasticsearch.plugins.Plugin; +import org.elasticsearch.plugins.ReloadablePlugin; import org.elasticsearch.plugins.ScriptPlugin; import org.elasticsearch.rest.RestController; import org.elasticsearch.rest.RestHandler; @@ -123,6 +125,7 @@ import org.elasticsearch.xpack.watcher.input.simple.SimpleInputFactory; import org.elasticsearch.xpack.watcher.input.transform.TransformInput; import org.elasticsearch.xpack.watcher.input.transform.TransformInputFactory; +import org.elasticsearch.xpack.watcher.notification.NotificationService; import org.elasticsearch.xpack.watcher.notification.email.Account; import org.elasticsearch.xpack.watcher.notification.email.EmailService; import org.elasticsearch.xpack.watcher.notification.email.HtmlSanitizer; @@ -194,7 +197,7 @@ import static java.util.Collections.emptyList; -public class Watcher extends Plugin implements ActionPlugin, ScriptPlugin { +public class Watcher extends Plugin implements ActionPlugin, ScriptPlugin, ReloadablePlugin { // This setting is only here for backward compatibility reasons as 6.x indices made use of it. It can be removed in 8.x. @Deprecated @@ -221,6 +224,11 @@ public class Watcher extends Plugin implements ActionPlugin, ScriptPlugin { protected final boolean transportClient; protected final boolean enabled; protected final Environment env; + private SetOnce emailService = new SetOnce<>(); + private SetOnce hipChatService = new SetOnce<>(); + private SetOnce jiraService = new SetOnce<>(); + private SetOnce slackService = new SetOnce<>(); + private SetOnce pagerDutyService = new SetOnce<>(); public Watcher(final Settings settings) { this.settings = settings; @@ -269,11 +277,11 @@ public Collection createComponents(Client client, ClusterService cluster httpClient = new HttpClient(settings, httpAuthRegistry, getSslService()); // notification - EmailService emailService = new EmailService(settings, cryptoService, clusterService.getClusterSettings()); - HipChatService hipChatService = new HipChatService(settings, httpClient, clusterService.getClusterSettings()); - JiraService jiraService = new JiraService(settings, httpClient, clusterService.getClusterSettings()); - SlackService slackService = new SlackService(settings, httpClient, clusterService.getClusterSettings()); - PagerDutyService pagerDutyService = new PagerDutyService(settings, httpClient, clusterService.getClusterSettings()); + emailService.set(new EmailService(settings, cryptoService, clusterService.getClusterSettings())); + hipChatService.set(new HipChatService(settings, httpClient, clusterService.getClusterSettings())); + jiraService.set(new JiraService(settings, httpClient, clusterService.getClusterSettings())); + slackService.set(new SlackService(settings, httpClient, clusterService.getClusterSettings())); + pagerDutyService.set(new PagerDutyService(settings, httpClient, clusterService.getClusterSettings())); TextTemplateEngine templateEngine = new TextTemplateEngine(settings, scriptService); Map emailAttachmentParsers = new HashMap<>(); @@ -300,14 +308,15 @@ public Collection createComponents(Client client, ClusterService cluster // actions final Map actionFactoryMap = new HashMap<>(); - actionFactoryMap.put(EmailAction.TYPE, new EmailActionFactory(settings, emailService, templateEngine, emailAttachmentsParser)); + actionFactoryMap.put(EmailAction.TYPE, new EmailActionFactory(settings, emailService.get(), templateEngine, + emailAttachmentsParser)); actionFactoryMap.put(WebhookAction.TYPE, new WebhookActionFactory(settings, httpClient, httpTemplateParser, templateEngine)); actionFactoryMap.put(IndexAction.TYPE, new IndexActionFactory(settings, client)); actionFactoryMap.put(LoggingAction.TYPE, new LoggingActionFactory(settings, templateEngine)); - actionFactoryMap.put(HipChatAction.TYPE, new HipChatActionFactory(settings, templateEngine, hipChatService)); - actionFactoryMap.put(JiraAction.TYPE, new JiraActionFactory(settings, templateEngine, jiraService)); - actionFactoryMap.put(SlackAction.TYPE, new SlackActionFactory(settings, templateEngine, slackService)); - actionFactoryMap.put(PagerDutyAction.TYPE, new PagerDutyActionFactory(settings, templateEngine, pagerDutyService)); + actionFactoryMap.put(HipChatAction.TYPE, new HipChatActionFactory(settings, templateEngine, hipChatService.get())); + actionFactoryMap.put(JiraAction.TYPE, new JiraActionFactory(settings, templateEngine, jiraService.get())); + actionFactoryMap.put(SlackAction.TYPE, new SlackActionFactory(settings, templateEngine, slackService.get())); + actionFactoryMap.put(PagerDutyAction.TYPE, new PagerDutyActionFactory(settings, templateEngine, pagerDutyService.get())); final ActionRegistry registry = new ActionRegistry(actionFactoryMap, conditionRegistry, transformRegistry, getClock(), getLicenseState()); @@ -367,7 +376,8 @@ public Collection createComponents(Client client, ClusterService cluster return Arrays.asList(registry, inputRegistry, historyStore, triggerService, triggeredWatchParser, watcherLifeCycleService, executionService, triggerEngineListener, watcherService, watchParser, - configuredTriggerEngine, triggeredWatchStore, watcherSearchTemplateService, slackService, pagerDutyService, hipChatService); + configuredTriggerEngine, triggeredWatchStore, watcherSearchTemplateService, slackService.get(), pagerDutyService.get(), + hipChatService.get()); } protected TriggerEngine getTriggerEngine(Clock clock, ScheduleRegistry scheduleRegistry) { @@ -613,4 +623,33 @@ public List getContexts() { public void close() throws IOException { IOUtils.closeWhileHandlingException(httpClient); } + + /** + * Used to detect which {@link NotificationService} get reloaded. Also useful for testing the reload in tests. + * @return + */ + protected List getReloadableServices() { + return Arrays.asList( + emailService.get(), + hipChatService.get(), + jiraService.get(), + slackService.get(), + pagerDutyService.get()); + } + + /** + * Reloads all reloadable services' settings. + * @param settings + * Settings used while reloading the plugin. All values are + * retrievable, including the values stored in the node's keystore. + * The setting values are the initial ones, from when the node has be + * started, i.e. they don't follow dynamic updates. + * @throws Exception + */ + @Override + public void reload(Settings settings) throws Exception { + for (NotificationService service : getReloadableServices()) { + service.loadSettings(settings); + } + } } diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/NotificationService.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/NotificationService.java index 9870bcd086534..b698fa23b1b79 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/NotificationService.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/NotificationService.java @@ -30,7 +30,7 @@ public NotificationService(Settings settings, String type) { this.type = type; } - protected synchronized void setAccountSetting(Settings settings) { + public synchronized void loadSettings(Settings settings) { Tuple, Account> accounts = buildAccounts(settings, this::createAccount); this.accounts = Collections.unmodifiableMap(accounts.v1()); this.defaultAccount = accounts.v2(); diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/email/EmailService.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/email/EmailService.java index 41a2ecc3bcc80..04fd3de584bb0 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/email/EmailService.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/email/EmailService.java @@ -96,7 +96,7 @@ public class EmailService extends NotificationService { public EmailService(Settings settings, @Nullable CryptoService cryptoService, ClusterSettings clusterSettings) { super(settings, "email"); this.cryptoService = cryptoService; - clusterSettings.addSettingsUpdateConsumer(this::setAccountSetting, getSettings()); + clusterSettings.addSettingsUpdateConsumer(this::loadSettings, getSettings()); // ensure logging of setting changes clusterSettings.addSettingsUpdateConsumer(SETTING_DEFAULT_ACCOUNT, (s) -> {}); clusterSettings.addAffixUpdateConsumer(SETTING_PROFILE, (s, o) -> {}, (s, o) -> {}); @@ -116,7 +116,7 @@ public EmailService(Settings settings, @Nullable CryptoService cryptoService, Cl clusterSettings.addAffixUpdateConsumer(SETTING_SMTP_SEND_PARTIAL, (s, o) -> {}, (s, o) -> {}); clusterSettings.addAffixUpdateConsumer(SETTING_SMTP_WAIT_ON_QUIT, (s, o) -> {}, (s, o) -> {}); // do an initial load - setAccountSetting(settings); + loadSettings(settings); } @Override diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/hipchat/HipChatService.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/hipchat/HipChatService.java index 477b4545294bd..3294017136a7f 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/hipchat/HipChatService.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/hipchat/HipChatService.java @@ -67,7 +67,7 @@ public class HipChatService extends NotificationService { public HipChatService(Settings settings, HttpClient httpClient, ClusterSettings clusterSettings) { super(settings, "hipchat"); this.httpClient = httpClient; - clusterSettings.addSettingsUpdateConsumer(this::setAccountSetting, getSettings()); + clusterSettings.addSettingsUpdateConsumer(this::loadSettings, getSettings()); // ensure logging of setting changes clusterSettings.addSettingsUpdateConsumer(SETTING_DEFAULT_ACCOUNT, (s) -> {}); clusterSettings.addSettingsUpdateConsumer(SETTING_DEFAULT_HOST, (s) -> {}); @@ -80,13 +80,13 @@ public HipChatService(Settings settings, HttpClient httpClient, ClusterSettings clusterSettings.addAffixUpdateConsumer(SETTING_PORT, (s, o) -> {}, (s, o) -> {}); clusterSettings.addAffixUpdateConsumer(SETTING_MESSAGE_DEFAULTS, (s, o) -> {}, (s, o) -> {}); - setAccountSetting(settings); + loadSettings(settings); } @Override - protected synchronized void setAccountSetting(Settings settings) { + public synchronized void loadSettings(Settings settings) { defaultServer = new HipChatServer(settings.getByPrefix("xpack.notification.hipchat.")); - super.setAccountSetting(settings); + super.loadSettings(settings); } @Override diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/jira/JiraService.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/jira/JiraService.java index 297531cbe81ca..39fe2b39c9fed 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/jira/JiraService.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/jira/JiraService.java @@ -62,7 +62,7 @@ public class JiraService extends NotificationService { public JiraService(Settings settings, HttpClient httpClient, ClusterSettings clusterSettings) { super(settings, "jira"); this.httpClient = httpClient; - clusterSettings.addSettingsUpdateConsumer(this::setAccountSetting, getSettings()); + clusterSettings.addSettingsUpdateConsumer(this::loadSettings, getSettings()); // ensure logging of setting changes clusterSettings.addSettingsUpdateConsumer(SETTING_DEFAULT_ACCOUNT, (s) -> {}); clusterSettings.addAffixUpdateConsumer(SETTING_ALLOW_HTTP, (s, o) -> {}, (s, o) -> {}); @@ -74,7 +74,7 @@ public JiraService(Settings settings, HttpClient httpClient, ClusterSettings clu clusterSettings.addAffixUpdateConsumer(SETTING_SECURE_PASSWORD, (s, o) -> {}, (s, o) -> {}); clusterSettings.addAffixUpdateConsumer(SETTING_DEFAULTS, (s, o) -> {}, (s, o) -> {}); // do an initial load - setAccountSetting(settings); + loadSettings(settings); } @Override diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/pagerduty/PagerDutyService.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/pagerduty/PagerDutyService.java index e74e78707beff..d36059a5aa8ad 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/pagerduty/PagerDutyService.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/pagerduty/PagerDutyService.java @@ -45,7 +45,7 @@ public PagerDutyService(Settings settings, HttpClient httpClient, ClusterSetting clusterSettings.addAffixUpdateConsumer(SETTING_SERVICE_API_KEY, (s, o) -> {}, (s, o) -> {}); clusterSettings.addAffixUpdateConsumer(SETTING_SECURE_SERVICE_API_KEY, (s, o) -> {}, (s, o) -> {}); clusterSettings.addAffixUpdateConsumer(SETTING_DEFAULTS, (s, o) -> {}, (s, o) -> {}); - setAccountSetting(settings); + loadSettings(settings); } @Override diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/slack/SlackService.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/slack/SlackService.java index 92f44bfcbe39b..dcd07298da7f7 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/slack/SlackService.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/slack/SlackService.java @@ -41,12 +41,12 @@ public class SlackService extends NotificationService { public SlackService(Settings settings, HttpClient httpClient, ClusterSettings clusterSettings) { super(settings, "slack"); this.httpClient = httpClient; - clusterSettings.addSettingsUpdateConsumer(this::setAccountSetting, getSettings()); + clusterSettings.addSettingsUpdateConsumer(this::loadSettings, getSettings()); clusterSettings.addSettingsUpdateConsumer(SETTING_DEFAULT_ACCOUNT, (s) -> {}); clusterSettings.addAffixUpdateConsumer(SETTING_URL, (s, o) -> {}, (s, o) -> {}); clusterSettings.addAffixUpdateConsumer(SETTING_URL_SECURE, (s, o) -> {}, (s, o) -> {}); clusterSettings.addAffixUpdateConsumer(SETTING_DEFAULTS, (s, o) -> {}, (s, o) -> {}); - setAccountSetting(settings); + loadSettings(settings); } @Override diff --git a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/notification/NotificationServiceTests.java b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/notification/NotificationServiceTests.java index bb5f234ca950a..a5ba7a56ba644 100644 --- a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/notification/NotificationServiceTests.java +++ b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/notification/NotificationServiceTests.java @@ -82,7 +82,7 @@ private static class TestNotificationService extends NotificationService TestNotificationService(Settings settings) { super(settings, "test"); - setAccountSetting(settings); + loadSettings(settings); } @Override @@ -90,4 +90,4 @@ protected String createAccount(String name, Settings accountSettings) { return name; } } -} \ No newline at end of file +} diff --git a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/WatcherPluginTests.java b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/WatcherPluginTests.java index abb981053e730..89d71804a79fb 100644 --- a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/WatcherPluginTests.java +++ b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/WatcherPluginTests.java @@ -15,7 +15,10 @@ import org.elasticsearch.test.IndexSettingsModule; import org.elasticsearch.threadpool.ExecutorBuilder; import org.elasticsearch.xpack.core.watcher.watch.Watch; +import org.elasticsearch.xpack.watcher.notification.NotificationService; +import org.elasticsearch.xpack.watcher.notification.email.Account; +import java.util.Collections; import java.util.List; import static java.util.Collections.emptyMap; @@ -97,4 +100,43 @@ public void testThreadPoolSize() { .build(); assertThat(Watcher.getWatcherThreadPoolSize(noDataNodeSettings), is(1)); } + + private class TestNotificationService extends NotificationService { + + boolean calledCreateAccount = false; + + TestNotificationService(Settings settings, String type) { + super(settings, type); + } + + @Override + protected Account createAccount(String name, Settings accountSettings) { + return null; + } + + @Override + public synchronized void loadSettings(Settings settings) { + calledCreateAccount = true; + super.loadSettings(settings); + } + } + + public void testReload() throws Exception { + Settings settings = Settings.builder() + .put("xpack.watcher.enabled", false) + .put("path.home", createTempDir()) + .build(); + TestNotificationService service = new TestNotificationService(settings, "test"); + Watcher watcher = new Watcher(settings) { + @Override + protected List getReloadableServices() { + return Collections.singletonList(service); + } + }; + + assertFalse(service.calledCreateAccount); + watcher.reload(settings); + assertTrue(service.calledCreateAccount); + + } } From 153a97b0f53d8cd2d3b04434e43c73b45552b8b6 Mon Sep 17 00:00:00 2001 From: Michael Basnight Date: Tue, 3 Jul 2018 09:07:16 -0500 Subject: [PATCH 02/11] rename to reload --- .../java/org/elasticsearch/xpack/watcher/Watcher.java | 2 +- .../xpack/watcher/notification/NotificationService.java | 2 +- .../xpack/watcher/notification/email/EmailService.java | 4 ++-- .../watcher/notification/hipchat/HipChatService.java | 8 ++++---- .../xpack/watcher/notification/jira/JiraService.java | 4 ++-- .../watcher/notification/pagerduty/PagerDutyService.java | 2 +- .../xpack/watcher/notification/slack/SlackService.java | 4 ++-- .../xpack/notification/NotificationServiceTests.java | 2 +- .../elasticsearch/xpack/watcher/WatcherPluginTests.java | 4 ++-- 9 files changed, 16 insertions(+), 16 deletions(-) diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java index 47d4199c45460..f8050c2233dae 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java @@ -649,7 +649,7 @@ protected List getReloadableServices() { @Override public void reload(Settings settings) throws Exception { for (NotificationService service : getReloadableServices()) { - service.loadSettings(settings); + service.reload(settings); } } } diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/NotificationService.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/NotificationService.java index b698fa23b1b79..9a5102d2934cf 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/NotificationService.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/NotificationService.java @@ -30,7 +30,7 @@ public NotificationService(Settings settings, String type) { this.type = type; } - public synchronized void loadSettings(Settings settings) { + public synchronized void reload(Settings settings) { Tuple, Account> accounts = buildAccounts(settings, this::createAccount); this.accounts = Collections.unmodifiableMap(accounts.v1()); this.defaultAccount = accounts.v2(); diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/email/EmailService.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/email/EmailService.java index 04fd3de584bb0..42b7ac6e6481f 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/email/EmailService.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/email/EmailService.java @@ -96,7 +96,7 @@ public class EmailService extends NotificationService { public EmailService(Settings settings, @Nullable CryptoService cryptoService, ClusterSettings clusterSettings) { super(settings, "email"); this.cryptoService = cryptoService; - clusterSettings.addSettingsUpdateConsumer(this::loadSettings, getSettings()); + clusterSettings.addSettingsUpdateConsumer(this::reload, getSettings()); // ensure logging of setting changes clusterSettings.addSettingsUpdateConsumer(SETTING_DEFAULT_ACCOUNT, (s) -> {}); clusterSettings.addAffixUpdateConsumer(SETTING_PROFILE, (s, o) -> {}, (s, o) -> {}); @@ -116,7 +116,7 @@ public EmailService(Settings settings, @Nullable CryptoService cryptoService, Cl clusterSettings.addAffixUpdateConsumer(SETTING_SMTP_SEND_PARTIAL, (s, o) -> {}, (s, o) -> {}); clusterSettings.addAffixUpdateConsumer(SETTING_SMTP_WAIT_ON_QUIT, (s, o) -> {}, (s, o) -> {}); // do an initial load - loadSettings(settings); + reload(settings); } @Override diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/hipchat/HipChatService.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/hipchat/HipChatService.java index 3294017136a7f..ab21c04be5feb 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/hipchat/HipChatService.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/hipchat/HipChatService.java @@ -67,7 +67,7 @@ public class HipChatService extends NotificationService { public HipChatService(Settings settings, HttpClient httpClient, ClusterSettings clusterSettings) { super(settings, "hipchat"); this.httpClient = httpClient; - clusterSettings.addSettingsUpdateConsumer(this::loadSettings, getSettings()); + clusterSettings.addSettingsUpdateConsumer(this::reload, getSettings()); // ensure logging of setting changes clusterSettings.addSettingsUpdateConsumer(SETTING_DEFAULT_ACCOUNT, (s) -> {}); clusterSettings.addSettingsUpdateConsumer(SETTING_DEFAULT_HOST, (s) -> {}); @@ -80,13 +80,13 @@ public HipChatService(Settings settings, HttpClient httpClient, ClusterSettings clusterSettings.addAffixUpdateConsumer(SETTING_PORT, (s, o) -> {}, (s, o) -> {}); clusterSettings.addAffixUpdateConsumer(SETTING_MESSAGE_DEFAULTS, (s, o) -> {}, (s, o) -> {}); - loadSettings(settings); + reload(settings); } @Override - public synchronized void loadSettings(Settings settings) { + public synchronized void reload(Settings settings) { defaultServer = new HipChatServer(settings.getByPrefix("xpack.notification.hipchat.")); - super.loadSettings(settings); + super.reload(settings); } @Override diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/jira/JiraService.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/jira/JiraService.java index 39fe2b39c9fed..77f7aa0490705 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/jira/JiraService.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/jira/JiraService.java @@ -62,7 +62,7 @@ public class JiraService extends NotificationService { public JiraService(Settings settings, HttpClient httpClient, ClusterSettings clusterSettings) { super(settings, "jira"); this.httpClient = httpClient; - clusterSettings.addSettingsUpdateConsumer(this::loadSettings, getSettings()); + clusterSettings.addSettingsUpdateConsumer(this::reload, getSettings()); // ensure logging of setting changes clusterSettings.addSettingsUpdateConsumer(SETTING_DEFAULT_ACCOUNT, (s) -> {}); clusterSettings.addAffixUpdateConsumer(SETTING_ALLOW_HTTP, (s, o) -> {}, (s, o) -> {}); @@ -74,7 +74,7 @@ public JiraService(Settings settings, HttpClient httpClient, ClusterSettings clu clusterSettings.addAffixUpdateConsumer(SETTING_SECURE_PASSWORD, (s, o) -> {}, (s, o) -> {}); clusterSettings.addAffixUpdateConsumer(SETTING_DEFAULTS, (s, o) -> {}, (s, o) -> {}); // do an initial load - loadSettings(settings); + reload(settings); } @Override diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/pagerduty/PagerDutyService.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/pagerduty/PagerDutyService.java index d36059a5aa8ad..791a284e7672f 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/pagerduty/PagerDutyService.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/pagerduty/PagerDutyService.java @@ -45,7 +45,7 @@ public PagerDutyService(Settings settings, HttpClient httpClient, ClusterSetting clusterSettings.addAffixUpdateConsumer(SETTING_SERVICE_API_KEY, (s, o) -> {}, (s, o) -> {}); clusterSettings.addAffixUpdateConsumer(SETTING_SECURE_SERVICE_API_KEY, (s, o) -> {}, (s, o) -> {}); clusterSettings.addAffixUpdateConsumer(SETTING_DEFAULTS, (s, o) -> {}, (s, o) -> {}); - loadSettings(settings); + reload(settings); } @Override diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/slack/SlackService.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/slack/SlackService.java index dcd07298da7f7..0398eb394c104 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/slack/SlackService.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/slack/SlackService.java @@ -41,12 +41,12 @@ public class SlackService extends NotificationService { public SlackService(Settings settings, HttpClient httpClient, ClusterSettings clusterSettings) { super(settings, "slack"); this.httpClient = httpClient; - clusterSettings.addSettingsUpdateConsumer(this::loadSettings, getSettings()); + clusterSettings.addSettingsUpdateConsumer(this::reload, getSettings()); clusterSettings.addSettingsUpdateConsumer(SETTING_DEFAULT_ACCOUNT, (s) -> {}); clusterSettings.addAffixUpdateConsumer(SETTING_URL, (s, o) -> {}, (s, o) -> {}); clusterSettings.addAffixUpdateConsumer(SETTING_URL_SECURE, (s, o) -> {}, (s, o) -> {}); clusterSettings.addAffixUpdateConsumer(SETTING_DEFAULTS, (s, o) -> {}, (s, o) -> {}); - loadSettings(settings); + reload(settings); } @Override diff --git a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/notification/NotificationServiceTests.java b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/notification/NotificationServiceTests.java index a5ba7a56ba644..970d351d2d529 100644 --- a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/notification/NotificationServiceTests.java +++ b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/notification/NotificationServiceTests.java @@ -82,7 +82,7 @@ private static class TestNotificationService extends NotificationService TestNotificationService(Settings settings) { super(settings, "test"); - loadSettings(settings); + reload(settings); } @Override diff --git a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/WatcherPluginTests.java b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/WatcherPluginTests.java index 89d71804a79fb..649443d66db4d 100644 --- a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/WatcherPluginTests.java +++ b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/WatcherPluginTests.java @@ -115,9 +115,9 @@ protected Account createAccount(String name, Settings accountSettings) { } @Override - public synchronized void loadSettings(Settings settings) { + public synchronized void reload(Settings settings) { calledCreateAccount = true; - super.loadSettings(settings); + super.reload(settings); } } From 7be7b49b6f592b7aaa2bdbbc05f214f52f84e5f4 Mon Sep 17 00:00:00 2001 From: Michael Basnight Date: Tue, 3 Jul 2018 09:21:56 -0500 Subject: [PATCH 03/11] Use a list instead of a method and clean test --- .../elasticsearch/xpack/watcher/Watcher.java | 21 +++++++------------ .../xpack/watcher/WatcherPluginTests.java | 21 +++++++++---------- 2 files changed, 18 insertions(+), 24 deletions(-) diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java index f8050c2233dae..fbcf05857b422 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java @@ -229,6 +229,7 @@ public class Watcher extends Plugin implements ActionPlugin, ScriptPlugin, Reloa private SetOnce jiraService = new SetOnce<>(); private SetOnce slackService = new SetOnce<>(); private SetOnce pagerDutyService = new SetOnce<>(); + List reloadableServices = new ArrayList<>(); public Watcher(final Settings settings) { this.settings = settings; @@ -278,10 +279,16 @@ public Collection createComponents(Client client, ClusterService cluster // notification emailService.set(new EmailService(settings, cryptoService, clusterService.getClusterSettings())); + reloadableServices.add(emailService.get()); hipChatService.set(new HipChatService(settings, httpClient, clusterService.getClusterSettings())); + reloadableServices.add(hipChatService.get()); jiraService.set(new JiraService(settings, httpClient, clusterService.getClusterSettings())); + reloadableServices.add(jiraService.get()); slackService.set(new SlackService(settings, httpClient, clusterService.getClusterSettings())); + reloadableServices.add(slackService.get()); pagerDutyService.set(new PagerDutyService(settings, httpClient, clusterService.getClusterSettings())); + reloadableServices.add(pagerDutyService.get()); + TextTemplateEngine templateEngine = new TextTemplateEngine(settings, scriptService); Map emailAttachmentParsers = new HashMap<>(); @@ -624,18 +631,6 @@ public void close() throws IOException { IOUtils.closeWhileHandlingException(httpClient); } - /** - * Used to detect which {@link NotificationService} get reloaded. Also useful for testing the reload in tests. - * @return - */ - protected List getReloadableServices() { - return Arrays.asList( - emailService.get(), - hipChatService.get(), - jiraService.get(), - slackService.get(), - pagerDutyService.get()); - } /** * Reloads all reloadable services' settings. @@ -648,7 +643,7 @@ protected List getReloadableServices() { */ @Override public void reload(Settings settings) throws Exception { - for (NotificationService service : getReloadableServices()) { + for (NotificationService service : reloadableServices) { service.reload(settings); } } diff --git a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/WatcherPluginTests.java b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/WatcherPluginTests.java index 649443d66db4d..87a6321c15b45 100644 --- a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/WatcherPluginTests.java +++ b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/WatcherPluginTests.java @@ -25,6 +25,9 @@ import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.is; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; public class WatcherPluginTests extends ESTestCase { @@ -126,17 +129,13 @@ public void testReload() throws Exception { .put("xpack.watcher.enabled", false) .put("path.home", createTempDir()) .build(); - TestNotificationService service = new TestNotificationService(settings, "test"); - Watcher watcher = new Watcher(settings) { - @Override - protected List getReloadableServices() { - return Collections.singletonList(service); - } - }; - - assertFalse(service.calledCreateAccount); - watcher.reload(settings); - assertTrue(service.calledCreateAccount); + NotificationService mockService = mock(NotificationService.class); + Watcher watcher = new Watcher(settings); + watcher.reloadableServices.clear(); + watcher.reloadableServices.add(mockService); + verify(mockService, times(0)).reload(settings); + watcher.reload(settings); + verify(mockService, times(1)).reload(settings); } } From b97f21b6bebd48c3b953527b0dcab209f1187669 Mon Sep 17 00:00:00 2001 From: Michael Basnight Date: Tue, 3 Jul 2018 09:36:06 -0500 Subject: [PATCH 04/11] Fix enabled status on reload --- .../elasticsearch/xpack/watcher/Watcher.java | 5 ++++- .../xpack/watcher/WatcherPluginTests.java | 17 +++++++++++++++-- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java index fbcf05857b422..fa19a952108aa 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java @@ -642,7 +642,10 @@ public void close() throws IOException { * @throws Exception */ @Override - public void reload(Settings settings) throws Exception { + public void reload(Settings settings) { + if (enabled == false || transportClient) { + return; + } for (NotificationService service : reloadableServices) { service.reload(settings); } diff --git a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/WatcherPluginTests.java b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/WatcherPluginTests.java index 87a6321c15b45..a23830fd99f4f 100644 --- a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/WatcherPluginTests.java +++ b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/WatcherPluginTests.java @@ -124,9 +124,9 @@ public synchronized void reload(Settings settings) { } } - public void testReload() throws Exception { + public void testReload() { Settings settings = Settings.builder() - .put("xpack.watcher.enabled", false) + .put("xpack.watcher.enabled", true) .put("path.home", createTempDir()) .build(); NotificationService mockService = mock(NotificationService.class); @@ -138,4 +138,17 @@ public void testReload() throws Exception { watcher.reload(settings); verify(mockService, times(1)).reload(settings); } + + public void testReloadDisabled() { + Settings settings = Settings.builder() + .put("xpack.watcher.enabled", false) + .put("path.home", createTempDir()) + .build(); + NotificationService mockService = mock(NotificationService.class); + Watcher watcher = new Watcher(settings); + + verify(mockService, times(0)).reload(settings); + watcher.reload(settings); + verify(mockService, times(0)).reload(settings); + } } From 98d41ee61936cacf33a682d95c06f7179d994778 Mon Sep 17 00:00:00 2001 From: Michael Basnight Date: Tue, 3 Jul 2018 09:40:20 -0500 Subject: [PATCH 05/11] Remove unused code --- .../xpack/watcher/WatcherPluginTests.java | 20 ------------------- 1 file changed, 20 deletions(-) diff --git a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/WatcherPluginTests.java b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/WatcherPluginTests.java index a23830fd99f4f..ee2e6be029c5a 100644 --- a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/WatcherPluginTests.java +++ b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/WatcherPluginTests.java @@ -104,26 +104,6 @@ public void testThreadPoolSize() { assertThat(Watcher.getWatcherThreadPoolSize(noDataNodeSettings), is(1)); } - private class TestNotificationService extends NotificationService { - - boolean calledCreateAccount = false; - - TestNotificationService(Settings settings, String type) { - super(settings, type); - } - - @Override - protected Account createAccount(String name, Settings accountSettings) { - return null; - } - - @Override - public synchronized void reload(Settings settings) { - calledCreateAccount = true; - super.reload(settings); - } - } - public void testReload() { Settings settings = Settings.builder() .put("xpack.watcher.enabled", true) From cc09f7b8440174bb4d8c07edc05e667dbb79cb6e Mon Sep 17 00:00:00 2001 From: Michael Basnight Date: Tue, 10 Jul 2018 14:22:28 -0500 Subject: [PATCH 06/11] Fixing merge conflict properly --- .../xpack/watcher/notification/NotificationService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/NotificationService.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/NotificationService.java index 11430ada8eb5d..027825ab77871 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/NotificationService.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/NotificationService.java @@ -31,7 +31,7 @@ public abstract class NotificationService extends AbstractComponent { public NotificationService(Settings settings, String type, ClusterSettings clusterSettings, List> pluginSettings) { this(settings, type); - clusterSettings.addSettingsUpdateConsumer(this::setAccountSetting, pluginSettings); + clusterSettings.addSettingsUpdateConsumer(this::reload, pluginSettings); } // Used for testing only From 0a1d9aff78db575633e7f904b055fb511fd4e076 Mon Sep 17 00:00:00 2001 From: Michael Basnight Date: Wed, 11 Jul 2018 08:37:16 -0500 Subject: [PATCH 07/11] Remove setOnce --- .../elasticsearch/xpack/watcher/Watcher.java | 42 ++++++++----------- 1 file changed, 18 insertions(+), 24 deletions(-) diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java index fa19a952108aa..c9936fb4e66e3 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java @@ -6,7 +6,6 @@ package org.elasticsearch.xpack.watcher; import org.apache.logging.log4j.Logger; -import org.apache.lucene.util.SetOnce; import org.elasticsearch.action.ActionRequest; import org.elasticsearch.action.ActionResponse; import org.elasticsearch.bootstrap.BootstrapCheck; @@ -224,11 +223,6 @@ public class Watcher extends Plugin implements ActionPlugin, ScriptPlugin, Reloa protected final boolean transportClient; protected final boolean enabled; protected final Environment env; - private SetOnce emailService = new SetOnce<>(); - private SetOnce hipChatService = new SetOnce<>(); - private SetOnce jiraService = new SetOnce<>(); - private SetOnce slackService = new SetOnce<>(); - private SetOnce pagerDutyService = new SetOnce<>(); List reloadableServices = new ArrayList<>(); public Watcher(final Settings settings) { @@ -278,17 +272,17 @@ public Collection createComponents(Client client, ClusterService cluster httpClient = new HttpClient(settings, httpAuthRegistry, getSslService()); // notification - emailService.set(new EmailService(settings, cryptoService, clusterService.getClusterSettings())); - reloadableServices.add(emailService.get()); - hipChatService.set(new HipChatService(settings, httpClient, clusterService.getClusterSettings())); - reloadableServices.add(hipChatService.get()); - jiraService.set(new JiraService(settings, httpClient, clusterService.getClusterSettings())); - reloadableServices.add(jiraService.get()); - slackService.set(new SlackService(settings, httpClient, clusterService.getClusterSettings())); - reloadableServices.add(slackService.get()); - pagerDutyService.set(new PagerDutyService(settings, httpClient, clusterService.getClusterSettings())); - reloadableServices.add(pagerDutyService.get()); - + EmailService emailService = new EmailService(settings, cryptoService, clusterService.getClusterSettings()); + HipChatService hipChatService = new HipChatService(settings, httpClient, clusterService.getClusterSettings()); + JiraService jiraService = new JiraService(settings, httpClient, clusterService.getClusterSettings()); + SlackService slackService = new SlackService(settings, httpClient, clusterService.getClusterSettings()); + PagerDutyService pagerDutyService = new PagerDutyService(settings, httpClient, clusterService.getClusterSettings()); + + reloadableServices.add(emailService); + reloadableServices.add(hipChatService); + reloadableServices.add(jiraService); + reloadableServices.add(slackService); + reloadableServices.add(pagerDutyService); TextTemplateEngine templateEngine = new TextTemplateEngine(settings, scriptService); Map emailAttachmentParsers = new HashMap<>(); @@ -315,15 +309,15 @@ public Collection createComponents(Client client, ClusterService cluster // actions final Map actionFactoryMap = new HashMap<>(); - actionFactoryMap.put(EmailAction.TYPE, new EmailActionFactory(settings, emailService.get(), templateEngine, + actionFactoryMap.put(EmailAction.TYPE, new EmailActionFactory(settings, emailService, templateEngine, emailAttachmentsParser)); actionFactoryMap.put(WebhookAction.TYPE, new WebhookActionFactory(settings, httpClient, httpTemplateParser, templateEngine)); actionFactoryMap.put(IndexAction.TYPE, new IndexActionFactory(settings, client)); actionFactoryMap.put(LoggingAction.TYPE, new LoggingActionFactory(settings, templateEngine)); - actionFactoryMap.put(HipChatAction.TYPE, new HipChatActionFactory(settings, templateEngine, hipChatService.get())); - actionFactoryMap.put(JiraAction.TYPE, new JiraActionFactory(settings, templateEngine, jiraService.get())); - actionFactoryMap.put(SlackAction.TYPE, new SlackActionFactory(settings, templateEngine, slackService.get())); - actionFactoryMap.put(PagerDutyAction.TYPE, new PagerDutyActionFactory(settings, templateEngine, pagerDutyService.get())); + actionFactoryMap.put(HipChatAction.TYPE, new HipChatActionFactory(settings, templateEngine, hipChatService)); + actionFactoryMap.put(JiraAction.TYPE, new JiraActionFactory(settings, templateEngine, jiraService)); + actionFactoryMap.put(SlackAction.TYPE, new SlackActionFactory(settings, templateEngine, slackService)); + actionFactoryMap.put(PagerDutyAction.TYPE, new PagerDutyActionFactory(settings, templateEngine, pagerDutyService)); final ActionRegistry registry = new ActionRegistry(actionFactoryMap, conditionRegistry, transformRegistry, getClock(), getLicenseState()); @@ -383,8 +377,8 @@ public Collection createComponents(Client client, ClusterService cluster return Arrays.asList(registry, inputRegistry, historyStore, triggerService, triggeredWatchParser, watcherLifeCycleService, executionService, triggerEngineListener, watcherService, watchParser, - configuredTriggerEngine, triggeredWatchStore, watcherSearchTemplateService, slackService.get(), pagerDutyService.get(), - hipChatService.get()); + configuredTriggerEngine, triggeredWatchStore, watcherSearchTemplateService, slackService, pagerDutyService, + hipChatService); } protected TriggerEngine getTriggerEngine(Clock clock, ScheduleRegistry scheduleRegistry) { From 38df69511cc9f5e10971940cc6bf1a0d760c609f Mon Sep 17 00:00:00 2001 From: Michael Basnight Date: Wed, 11 Jul 2018 08:48:45 -0500 Subject: [PATCH 08/11] Fix test up and cleanup reload method --- .../main/java/org/elasticsearch/xpack/watcher/Watcher.java | 5 +---- .../org/elasticsearch/xpack/watcher/WatcherPluginTests.java | 5 ++--- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java index c9936fb4e66e3..82feda4fcd243 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java @@ -633,15 +633,12 @@ public void close() throws IOException { * retrievable, including the values stored in the node's keystore. * The setting values are the initial ones, from when the node has be * started, i.e. they don't follow dynamic updates. - * @throws Exception */ @Override public void reload(Settings settings) { if (enabled == false || transportClient) { return; } - for (NotificationService service : reloadableServices) { - service.reload(settings); - } + reloadableServices.forEach(s -> s.reload(settings)); } } diff --git a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/WatcherPluginTests.java b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/WatcherPluginTests.java index ee2e6be029c5a..98c94e2cad25e 100644 --- a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/WatcherPluginTests.java +++ b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/WatcherPluginTests.java @@ -28,6 +28,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; public class WatcherPluginTests extends ESTestCase { @@ -114,7 +115,6 @@ public void testReload() { watcher.reloadableServices.clear(); watcher.reloadableServices.add(mockService); - verify(mockService, times(0)).reload(settings); watcher.reload(settings); verify(mockService, times(1)).reload(settings); } @@ -127,8 +127,7 @@ public void testReloadDisabled() { NotificationService mockService = mock(NotificationService.class); Watcher watcher = new Watcher(settings); - verify(mockService, times(0)).reload(settings); watcher.reload(settings); - verify(mockService, times(0)).reload(settings); + verifyNoMoreInteractions(mockService); } } From ad5d8a1978557ee8f415bf057f642b661db13ed2 Mon Sep 17 00:00:00 2001 From: Michael Basnight Date: Wed, 11 Jul 2018 09:00:10 -0500 Subject: [PATCH 09/11] Change pkg private to protected --- .../org/elasticsearch/xpack/watcher/Watcher.java | 2 +- .../xpack/watcher/WatcherPluginTests.java | 16 ++++++++++------ 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java index 82feda4fcd243..5751e55060961 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java @@ -223,7 +223,7 @@ public class Watcher extends Plugin implements ActionPlugin, ScriptPlugin, Reloa protected final boolean transportClient; protected final boolean enabled; protected final Environment env; - List reloadableServices = new ArrayList<>(); + protected List reloadableServices = new ArrayList<>(); public Watcher(final Settings settings) { this.settings = settings; diff --git a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/WatcherPluginTests.java b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/WatcherPluginTests.java index 98c94e2cad25e..dc56ecaf106a0 100644 --- a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/WatcherPluginTests.java +++ b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/WatcherPluginTests.java @@ -16,9 +16,7 @@ import org.elasticsearch.threadpool.ExecutorBuilder; import org.elasticsearch.xpack.core.watcher.watch.Watch; import org.elasticsearch.xpack.watcher.notification.NotificationService; -import org.elasticsearch.xpack.watcher.notification.email.Account; -import java.util.Collections; import java.util.List; import static java.util.Collections.emptyMap; @@ -111,9 +109,7 @@ public void testReload() { .put("path.home", createTempDir()) .build(); NotificationService mockService = mock(NotificationService.class); - Watcher watcher = new Watcher(settings); - watcher.reloadableServices.clear(); - watcher.reloadableServices.add(mockService); + Watcher watcher = new TestWatcher(settings, mockService); watcher.reload(settings); verify(mockService, times(1)).reload(settings); @@ -125,9 +121,17 @@ public void testReloadDisabled() { .put("path.home", createTempDir()) .build(); NotificationService mockService = mock(NotificationService.class); - Watcher watcher = new Watcher(settings); + Watcher watcher = new TestWatcher(settings, mockService); watcher.reload(settings); verifyNoMoreInteractions(mockService); } + + private class TestWatcher extends Watcher { + + public TestWatcher(Settings settings, NotificationService service) { + super(settings); + reloadableServices.add(service); + } + } } From 79f0b7ee40744be0862a8710f9be2ac26bc95350 Mon Sep 17 00:00:00 2001 From: Michael Basnight Date: Wed, 11 Jul 2018 09:05:54 -0500 Subject: [PATCH 10/11] fix checkstyle --- .../org/elasticsearch/xpack/watcher/WatcherPluginTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/WatcherPluginTests.java b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/WatcherPluginTests.java index dc56ecaf106a0..474f69c70edb3 100644 --- a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/WatcherPluginTests.java +++ b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/WatcherPluginTests.java @@ -129,7 +129,7 @@ public void testReloadDisabled() { private class TestWatcher extends Watcher { - public TestWatcher(Settings settings, NotificationService service) { + TestWatcher(Settings settings, NotificationService service) { super(settings); reloadableServices.add(service); } From e956505b35023dce9d288cb9d311ba4aa543f44d Mon Sep 17 00:00:00 2001 From: Michael Basnight Date: Wed, 11 Jul 2018 09:48:32 -0500 Subject: [PATCH 11/11] Minor nit changes --- .../org/elasticsearch/xpack/watcher/Watcher.java | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java index 5751e55060961..78d1d37287f60 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java @@ -309,8 +309,7 @@ public Collection createComponents(Client client, ClusterService cluster // actions final Map actionFactoryMap = new HashMap<>(); - actionFactoryMap.put(EmailAction.TYPE, new EmailActionFactory(settings, emailService, templateEngine, - emailAttachmentsParser)); + actionFactoryMap.put(EmailAction.TYPE, new EmailActionFactory(settings, emailService, templateEngine, emailAttachmentsParser)); actionFactoryMap.put(WebhookAction.TYPE, new WebhookActionFactory(settings, httpClient, httpTemplateParser, templateEngine)); actionFactoryMap.put(IndexAction.TYPE, new IndexActionFactory(settings, client)); actionFactoryMap.put(LoggingAction.TYPE, new LoggingActionFactory(settings, templateEngine)); @@ -377,8 +376,7 @@ public Collection createComponents(Client client, ClusterService cluster return Arrays.asList(registry, inputRegistry, historyStore, triggerService, triggeredWatchParser, watcherLifeCycleService, executionService, triggerEngineListener, watcherService, watchParser, - configuredTriggerEngine, triggeredWatchStore, watcherSearchTemplateService, slackService, pagerDutyService, - hipChatService); + configuredTriggerEngine, triggeredWatchStore, watcherSearchTemplateService, slackService, pagerDutyService, hipChatService); } protected TriggerEngine getTriggerEngine(Clock clock, ScheduleRegistry scheduleRegistry) { @@ -625,14 +623,8 @@ public void close() throws IOException { IOUtils.closeWhileHandlingException(httpClient); } - /** - * Reloads all reloadable services' settings. - * @param settings - * Settings used while reloading the plugin. All values are - * retrievable, including the values stored in the node's keystore. - * The setting values are the initial ones, from when the node has be - * started, i.e. they don't follow dynamic updates. + * Reloads all the reloadable services in watcher. */ @Override public void reload(Settings settings) {