Validate index name time format setting at parse time (#47911) (#48881)

elastic · Nov 7, 2019 · 5805560 · 5805560
1 parent dfc6a13
commit 5805560
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 12 deletions.
diff --git a/...plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/Exporter.java b/...plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/Exporter.java
@@ -15,7 +15,6 @@
 import org.elasticsearch.license.XPackLicenseState;
 import org.elasticsearch.xpack.monitoring.exporter.http.HttpExporter;
 
-import java.time.ZoneOffset;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.Iterator;
@@ -103,9 +102,14 @@ public Iterator<Setting<?>> settings() {
     /**
      * Every {@code Exporter} allows users to use a different index time format.
      */
-    private static final Setting.AffixSetting<String> INDEX_NAME_TIME_FORMAT_SETTING =
-            Setting.affixKeySetting("xpack.monitoring.exporters.","index.name.time_format",
-                    key -> Setting.simpleString(key, Property.Dynamic, Property.NodeScope));
+    static final Setting.AffixSetting<DateFormatter> INDEX_NAME_TIME_FORMAT_SETTING =
+                Setting.affixKeySetting("xpack.monitoring.exporters.","index.name.time_format",
+                        key -> new Setting<DateFormatter>(
+                                key,
+                                Exporter.INDEX_FORMAT,
+                                DateFormatter::forPattern,
+                                Property.Dynamic,
+                                Property.NodeScope));
 
     private static final String INDEX_FORMAT = "yyyy.MM.dd";
 
@@ -151,14 +155,8 @@ public void close() {
     protected abstract void doClose();
 
     protected static DateFormatter dateTimeFormatter(final Config config) {
-        Setting<String> setting = INDEX_NAME_TIME_FORMAT_SETTING.getConcreteSettingForNamespace(config.name);
-        String format = setting.exists(config.settings()) ? setting.get(config.settings()) : INDEX_FORMAT;
-        try {
-            return DateFormatter.forPattern(format).withZone(ZoneOffset.UTC);
-        } catch (IllegalArgumentException e) {
-            throw new SettingsException("[" + INDEX_NAME_TIME_FORMAT_SETTING.getKey() + "] invalid index name time format: ["
-                    + format + "]", e);
-        }
+        Setting<DateFormatter> setting = INDEX_NAME_TIME_FORMAT_SETTING.getConcreteSettingForNamespace(config.name);
+        return setting.get(config.settings());
     }
 
     public static List<Setting.AffixSetting<?>> getSettings() {

diff --git a/.../monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/ExportersTests.java b/.../monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/ExportersTests.java
@@ -111,6 +111,19 @@ public void testHostsMustBeSetIfTypeIsHttp() {
         assertThat(e.getCause(), hasToString(containsString("host list for [" + prefix + ".host] is empty")));
     }
 
+    public void testIndexNameTimeFormatMustBeValid() {
+        final String prefix = "xpack.monitoring.exporters.example";
+        final String setting = ".index.name.time_format";
+        final String value = "yyyy.MM.dd.j";
+        final Settings settings = Settings.builder().put(prefix + setting, value).build();
+        final IllegalArgumentException e = expectThrows(
+            IllegalArgumentException.class,
+            () -> Exporter.INDEX_NAME_TIME_FORMAT_SETTING.getConcreteSetting(prefix + setting).get(settings));
+        assertThat(e, hasToString(containsString("Invalid format: [" + value + "]: Unknown pattern letter: j")));
+        assertThat(e.getCause(), instanceOf(IllegalArgumentException.class));
+        assertThat(e.getCause(), hasToString(containsString("Unknown pattern letter: j")));
+    }
+
     public void testExporterIndexPattern() {
         Exporter.Config config = mock(Exporter.Config.class);
         when(config.name()).thenReturn("anything");