From 6bd5e9ef913786260e807e239dd0720174343003 Mon Sep 17 00:00:00 2001 From: Alexander Reelsen Date: Mon, 9 Apr 2018 15:42:16 +0200 Subject: [PATCH] Watcher: Reenable email property settings (elastic/x-pack-elasticsearch#4319) With the change of requiring to configure account settings properly by using affix settings, we forgot another special snowflake, namely the configuration of mail properties, which can be arbitrary in the configuration. Those properties are used when an email is sent. This commit adds a few (but not all of those) options back and removes the link in the documentation to refer to all of those settings. Some settings are useless, as they only change the execution expectations when a mail is sent, which the watch has control over. The following settings are supported * smtp.{host,port,user,password} * smtp.auth * smtp.starttls.{enable,required} * smtp.{timeout,connection_timeout,write_timeout} * smtp.{local_address,local_port} * smtp.send_partial * smtp.wait_on_quit relates elastic/x-pack-elasticsearch#4048 Original commit: elastic/x-pack-elasticsearch@39d56247103a3b0cb97b397af10b71c8c236bc20 --- .../settings/notification-settings.asciidoc | 31 +++++++++-- .../execution/TriggeredWatchStore.java | 1 + .../watcher/notification/email/Account.java | 8 +-- .../notification/email/EmailService.java | 46 +++++++++++++++- .../notification/email/EmailServiceTests.java | 55 +++++++++++++++++++ 5 files changed, 127 insertions(+), 14 deletions(-) diff --git a/docs/en/settings/notification-settings.asciidoc b/docs/en/settings/notification-settings.asciidoc index bb87ca7f1d3..7a3d832ed34 100644 --- a/docs/en/settings/notification-settings.asciidoc +++ b/docs/en/settings/notification-settings.asciidoc @@ -116,12 +116,31 @@ can specify the following email account attributes: an appropriate trust store must configured so that the client will trust the server's certificate. Defaults to `false`. - `smtp.*`;; - SMTP attributes that enable fine control over the SMTP - protocol when sending messages. See - https://javaee.github.io/javamail/docs/api/com/sun/mail/smtp/package-summary.html[com.sun.mail.smtp] - for the full list of SMTP properties you can set. Note that all timeouts - (`writetimeout`, `connection_timeout` and `timeout`) default to 2 minutes. + `smtp.starttls.required`;; + If `true`, then `STARTTLS` will be required. If that command fails, the + connection will fail. Defaults to `false`. + + `smtp.timeout`;; + The socket read timeout. Default is two minutes. + + `smtp.connection_timeout`;; + The socket connection timeout. Default is two minutes. + + `smtp.write_timeout`;; + The socket write timeout. Default is two minutes. + + `smtp.local_address`;; + A configurable local address when sending emails. Not configured by default. + + `smtp.local_port`;; + A configurable local port when sending emails. Not configured by default. + + `smtp.send_partial`;; + Send an email, despite one of the receiver addresses being invalid. + + `smtp.wait_on_quit`;; + If set to false the QUIT command is sent and the connection closed. If set to + true, the QUIT command is sent and a reply is waited for. True by default. `xpack.notification.email.html.sanitization.allow`:: Specifies the HTML elements that are allowed in email notifications. For diff --git a/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/execution/TriggeredWatchStore.java b/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/execution/TriggeredWatchStore.java index dd889dfe4dc..35bc805fc59 100644 --- a/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/execution/TriggeredWatchStore.java +++ b/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/execution/TriggeredWatchStore.java @@ -24,6 +24,7 @@ import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.cluster.routing.Preference; import org.elasticsearch.common.component.AbstractComponent; +import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.util.concurrent.ThreadContext; diff --git a/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/email/Account.java b/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/email/Account.java index d52dc91d5fd..8ba8d030524 100644 --- a/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/email/Account.java +++ b/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/email/Account.java @@ -19,7 +19,6 @@ import javax.mail.Session; import javax.mail.Transport; import javax.mail.internet.InternetAddress; import javax.mail.internet.MimeMessage; - import java.security.AccessController; import java.security.PrivilegedAction; import java.security.PrivilegedActionException; @@ -227,14 +226,9 @@ public class Account { replace(builder, "local_address", "localaddress"); replace(builder, "local_port", "localport"); - replace(builder, "allow_8bitmime", "allow8bitmime"); replace(builder, "send_partial", "sendpartial"); - replace(builder, "sasl.authorization_id", "sasl.authorizationid"); - replace(builder, "sasl.use_canonical_hostname", "sasl.usecanonicalhostname"); replace(builder, "wait_on_quit", "quitwait"); - replace(builder, "report_success", "reportsuccess"); - replace(builder, "mail_extension", "mailextension"); - replace(builder, "use_rset", "userset"); + settings = builder.build(); Properties props = new Properties(); for (String key : settings.keySet()) { diff --git a/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/email/EmailService.java b/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/email/EmailService.java index 039695cc86a..41a2ecc3bcc 100644 --- a/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/email/EmailService.java +++ b/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/email/EmailService.java @@ -10,6 +10,7 @@ import org.elasticsearch.common.settings.ClusterSettings; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Setting.Property; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.xpack.core.watcher.crypto.CryptoService; import org.elasticsearch.xpack.watcher.notification.NotificationService; @@ -33,6 +34,7 @@ public class EmailService extends NotificationService { Setting.affixKeySetting("xpack.notification.email.account.", "email_defaults", (key) -> Setting.groupSetting(key + ".", Property.Dynamic, Property.NodeScope)); + // settings that can be configured as smtp properties private static final Setting.AffixSetting SETTING_SMTP_AUTH = Setting.affixKeySetting("xpack.notification.email.account.", "smtp.auth", (key) -> Setting.boolSetting(key, false, Property.Dynamic, Property.NodeScope)); @@ -41,6 +43,10 @@ public class EmailService extends NotificationService { Setting.affixKeySetting("xpack.notification.email.account.", "smtp.starttls.enable", (key) -> Setting.boolSetting(key, false, Property.Dynamic, Property.NodeScope)); + private static final Setting.AffixSetting SETTING_SMTP_STARTTLS_REQUIRED = + Setting.affixKeySetting("xpack.notification.email.account.", "smtp.starttls.required", + (key) -> Setting.boolSetting(key, false, Property.Dynamic, Property.NodeScope)); + private static final Setting.AffixSetting SETTING_SMTP_HOST = Setting.affixKeySetting("xpack.notification.email.account.", "smtp.host", (key) -> Setting.simpleString(key, Property.Dynamic, Property.NodeScope)); @@ -57,6 +63,34 @@ public class EmailService extends NotificationService { Setting.affixKeySetting("xpack.notification.email.account.", "smtp.password", (key) -> Setting.simpleString(key, Property.Dynamic, Property.NodeScope, Property.Filtered)); + private static final Setting.AffixSetting SETTING_SMTP_TIMEOUT = + Setting.affixKeySetting("xpack.notification.email.account.", "smtp.timeout", + (key) -> Setting.timeSetting(key, TimeValue.timeValueMinutes(2), Property.Dynamic, Property.NodeScope)); + + private static final Setting.AffixSetting SETTING_SMTP_CONNECTION_TIMEOUT = + Setting.affixKeySetting("xpack.notification.email.account.", "smtp.connection_timeout", + (key) -> Setting.timeSetting(key, TimeValue.timeValueMinutes(2), Property.Dynamic, Property.NodeScope)); + + private static final Setting.AffixSetting SETTING_SMTP_WRITE_TIMEOUT = + Setting.affixKeySetting("xpack.notification.email.account.", "smtp.write_timeout", + (key) -> Setting.timeSetting(key, TimeValue.timeValueMinutes(2), Property.Dynamic, Property.NodeScope)); + + private static final Setting.AffixSetting SETTING_SMTP_LOCAL_ADDRESS = + Setting.affixKeySetting("xpack.notification.email.account.", "smtp.local_address", + (key) -> Setting.simpleString(key, Property.Dynamic, Property.NodeScope)); + + private static final Setting.AffixSetting SETTING_SMTP_LOCAL_PORT = + Setting.affixKeySetting("xpack.notification.email.account.", "smtp.local_port", + (key) -> Setting.intSetting(key, 25, Property.Dynamic, Property.NodeScope)); + + private static final Setting.AffixSetting SETTING_SMTP_SEND_PARTIAL = + Setting.affixKeySetting("xpack.notification.email.account.", "smtp.send_partial", + (key) -> Setting.boolSetting(key, false, Property.Dynamic, Property.NodeScope)); + + private static final Setting.AffixSetting SETTING_SMTP_WAIT_ON_QUIT = + Setting.affixKeySetting("xpack.notification.email.account.", "smtp.wait_on_quit", + (key) -> Setting.boolSetting(key, true, Property.Dynamic, Property.NodeScope)); + private final CryptoService cryptoService; public EmailService(Settings settings, @Nullable CryptoService cryptoService, ClusterSettings clusterSettings) { @@ -69,10 +103,18 @@ public class EmailService extends NotificationService { clusterSettings.addAffixUpdateConsumer(SETTING_EMAIL_DEFAULTS, (s, o) -> {}, (s, o) -> {}); clusterSettings.addAffixUpdateConsumer(SETTING_SMTP_AUTH, (s, o) -> {}, (s, o) -> {}); clusterSettings.addAffixUpdateConsumer(SETTING_SMTP_STARTTLS_ENABLE, (s, o) -> {}, (s, o) -> {}); + clusterSettings.addAffixUpdateConsumer(SETTING_SMTP_STARTTLS_REQUIRED, (s, o) -> {}, (s, o) -> {}); clusterSettings.addAffixUpdateConsumer(SETTING_SMTP_HOST, (s, o) -> {}, (s, o) -> {}); clusterSettings.addAffixUpdateConsumer(SETTING_SMTP_PORT, (s, o) -> {}, (s, o) -> {}); clusterSettings.addAffixUpdateConsumer(SETTING_SMTP_USER, (s, o) -> {}, (s, o) -> {}); clusterSettings.addAffixUpdateConsumer(SETTING_SMTP_PASSWORD, (s, o) -> {}, (s, o) -> {}); + clusterSettings.addAffixUpdateConsumer(SETTING_SMTP_TIMEOUT, (s, o) -> {}, (s, o) -> {}); + clusterSettings.addAffixUpdateConsumer(SETTING_SMTP_CONNECTION_TIMEOUT, (s, o) -> {}, (s, o) -> {}); + clusterSettings.addAffixUpdateConsumer(SETTING_SMTP_WRITE_TIMEOUT, (s, o) -> {}, (s, o) -> {}); + clusterSettings.addAffixUpdateConsumer(SETTING_SMTP_LOCAL_ADDRESS, (s, o) -> {}, (s, o) -> {}); + clusterSettings.addAffixUpdateConsumer(SETTING_SMTP_LOCAL_PORT, (s, o) -> {}, (s, o) -> {}); + 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); } @@ -124,7 +166,9 @@ public class EmailService extends NotificationService { public static List> getSettings() { return Arrays.asList(SETTING_DEFAULT_ACCOUNT, SETTING_PROFILE, SETTING_EMAIL_DEFAULTS, SETTING_SMTP_AUTH, SETTING_SMTP_HOST, - SETTING_SMTP_PASSWORD, SETTING_SMTP_PORT, SETTING_SMTP_STARTTLS_ENABLE, SETTING_SMTP_USER); + SETTING_SMTP_PASSWORD, SETTING_SMTP_PORT, SETTING_SMTP_STARTTLS_ENABLE, SETTING_SMTP_USER, SETTING_SMTP_STARTTLS_REQUIRED, + SETTING_SMTP_TIMEOUT, SETTING_SMTP_CONNECTION_TIMEOUT, SETTING_SMTP_WRITE_TIMEOUT, SETTING_SMTP_LOCAL_ADDRESS, + SETTING_SMTP_LOCAL_PORT, SETTING_SMTP_SEND_PARTIAL, SETTING_SMTP_WAIT_ON_QUIT); } } diff --git a/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/notification/email/EmailServiceTests.java b/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/notification/email/EmailServiceTests.java index a5e2e0a7acd..42ae7f8d9ce 100644 --- a/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/notification/email/EmailServiceTests.java +++ b/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/notification/email/EmailServiceTests.java @@ -13,8 +13,14 @@ import org.junit.Before; import java.util.Collections; import java.util.HashSet; +import java.util.Properties; +import static org.apache.logging.log4j.ThreadContext.containsKey; +import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.hasEntry; +import static org.hamcrest.Matchers.hasKey; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.sameInstance; import static org.mockito.Mockito.mock; @@ -49,4 +55,53 @@ public class EmailServiceTests extends ESTestCase { assertThat(sent.email(), sameInstance(email)); assertThat(sent.account(), is("account1")); } + + public void testAccountSmtpPropertyConfiguration() { + Settings settings = Settings.builder() + .put("xpack.notification.email.account.account1.smtp.host", "localhost") + .put("xpack.notification.email.account.account1.smtp.starttls.required", "true") + .put("xpack.notification.email.account.account2.smtp.host", "localhost") + .put("xpack.notification.email.account.account2.smtp.connection_timeout", "1m") + .put("xpack.notification.email.account.account2.smtp.timeout", "1m") + .put("xpack.notification.email.account.account2.smtp.write_timeout", "1m") + .put("xpack.notification.email.account.account3.smtp.host", "localhost") + .put("xpack.notification.email.account.account3.smtp.send_partial", true) + .put("xpack.notification.email.account.account4.smtp.host", "localhost") + .put("xpack.notification.email.account.account4.smtp.local_address", "localhost") + .put("xpack.notification.email.account.account4.smtp.local_port", "1025") + .put("xpack.notification.email.account.account5.smtp.host", "localhost") + .put("xpack.notification.email.account.account5.smtp.wait_on_quit", true) + .build(); + EmailService emailService = new EmailService(settings, null, + new ClusterSettings(Settings.EMPTY, new HashSet<>(EmailService.getSettings()))); + + Account account1 = emailService.getAccount("account1"); + Properties properties1 = account1.getConfig().smtp.properties; + assertThat(properties1, hasEntry("mail.smtp.starttls.required", "true")); + assertThat(properties1, hasEntry("mail.smtp.connectiontimeout", "120000")); + assertThat(properties1, hasEntry("mail.smtp.writetimeout", "120000")); + assertThat(properties1, hasEntry("mail.smtp.timeout", "120000")); + assertThat(properties1, not(hasKey("mail.smtp.sendpartial"))); + assertThat(properties1, not(hasKey("mail.smtp.waitonquit"))); + assertThat(properties1, not(hasKey("mail.smtp.localport"))); + + Account account2 = emailService.getAccount("account2"); + Properties properties2 = account2.getConfig().smtp.properties; + assertThat(properties2, hasEntry("mail.smtp.connectiontimeout", "60000")); + assertThat(properties2, hasEntry("mail.smtp.writetimeout", "60000")); + assertThat(properties2, hasEntry("mail.smtp.timeout", "60000")); + + Account account3 = emailService.getAccount("account3"); + Properties properties3 = account3.getConfig().smtp.properties; + assertThat(properties3, hasEntry("mail.smtp.sendpartial", "true")); + + Account account4 = emailService.getAccount("account4"); + Properties properties4 = account4.getConfig().smtp.properties; + assertThat(properties4, hasEntry("mail.smtp.localaddress", "localhost")); + assertThat(properties4, hasEntry("mail.smtp.localport", "1025")); + + Account account5 = emailService.getAccount("account5"); + Properties properties5 = account5.getConfig().smtp.properties; + assertThat(properties5, hasEntry("mail.smtp.quitwait", "true")); + } }