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@39d5624710

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.starttls.required`;;
-  SMTP attributes that enable fine control over the SMTP
+  If `true`, then `STARTTLS` will be required. If that command fails, the
-  protocol when sending messages. See
+  connection will fail. Defaults to `false`.
-  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
+  `smtp.timeout`;;
-  (`writetimeout`, `connection_timeout` and `timeout`) default to 2 minutes.
+  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

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;

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()) {

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<Account> {
             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<Boolean> 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<Account> {
             Setting.affixKeySetting("xpack.notification.email.account.", "smtp.starttls.enable",
                     (key) -> Setting.boolSetting(key, false, Property.Dynamic, Property.NodeScope));
 
+    private static final Setting.AffixSetting<Boolean> 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<String> 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<Account> {
             Setting.affixKeySetting("xpack.notification.email.account.", "smtp.password",
                     (key) -> Setting.simpleString(key, Property.Dynamic, Property.NodeScope, Property.Filtered));
 
+    private static final Setting.AffixSetting<TimeValue> 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<TimeValue> 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<TimeValue> 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<String> 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<Integer> 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<Boolean> 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<Boolean> 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<Account> {
         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<Account> {
 
     public static List<Setting<?>> 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);
     }
 
 }

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"));
+    }
 }