Disallow index.time_series.end_time setting from being set or updated…

… in normal indices (#110268) The index.mode setting validates other index settings. When updating the index.time_series.end_time setting and the index.mode setting isn't wasn't defined at index creation time (meaning that default is active), then this validation is skipped which results into (worse) errors at a later point in time. This problem is fixed by enforced by making index.mode setting a dependency of index.time_series.end_time setting. Note that this problem doesn't exist for the index.time_series.start_time and index.routing_path index settings, because these index settings are final, which mean these can only be defined when an index is being created. Closes #110265
elastic · Jul 2, 2024 · e0d71d6 · e0d71d6
1 parent 58b73c1
commit e0d71d6
Show file tree

Hide file tree

Showing 6 changed files with 133 additions and 22 deletions.
diff --git a/docs/changelog/110268.yaml b/docs/changelog/110268.yaml
@@ -0,0 +1,6 @@
+pr: 110268
+summary: Disallow index.time_series.end_time setting from being set or updated in normal indices
+area: TSDB
+type: bug
+issues:
+ - 110265
diff --git a/...ams/src/test/java/org/elasticsearch/datastreams/DataStreamIndexSettingsProviderTests.java b/...ams/src/test/java/org/elasticsearch/datastreams/DataStreamIndexSettingsProviderTests.java
@@ -83,7 +83,11 @@ public void testGetAdditionalIndexSettings() throws Exception {
             settings,
             List.of(new CompressedXContent(mapping))
         );
-        assertThat(result.size(), equalTo(3));
+        // The index.time_series.end_time setting requires index.mode to be set to time_series adding it here so that we read this setting:
+        // (in production the index.mode setting is usually provided in an index or component template)
+        result = builder().put(result).put("index.mode", "time_series").build();
+        assertThat(result.size(), equalTo(4));
+        assertThat(IndexSettings.MODE.get(result), equalTo(IndexMode.TIME_SERIES));
         assertThat(IndexSettings.TIME_SERIES_START_TIME.get(result), equalTo(now.minusMillis(DEFAULT_LOOK_BACK_TIME.getMillis())));
         assertThat(IndexSettings.TIME_SERIES_END_TIME.get(result), equalTo(now.plusMillis(DEFAULT_LOOK_AHEAD_TIME.getMillis())));
         assertThat(IndexMetadata.INDEX_ROUTING_PATH.get(result), contains("field3"));
@@ -124,7 +128,11 @@ public void testGetAdditionalIndexSettingsIndexRoutingPathAlreadyDefined() throw
             settings,
             List.of(new CompressedXContent(mapping))
         );
-        assertThat(result.size(), equalTo(2));
+        // The index.time_series.end_time setting requires index.mode to be set to time_series adding it here so that we read this setting:
+        // (in production the index.mode setting is usually provided in an index or component template)
+        result = builder().put(result).put("index.mode", "time_series").build();
+        assertThat(result.size(), equalTo(3));
+        assertThat(result.get(IndexSettings.MODE.getKey()), equalTo("time_series"));
         assertThat(IndexSettings.TIME_SERIES_START_TIME.get(result), equalTo(now.minusMillis(DEFAULT_LOOK_BACK_TIME.getMillis())));
         assertThat(IndexSettings.TIME_SERIES_END_TIME.get(result), equalTo(now.plusMillis(DEFAULT_LOOK_AHEAD_TIME.getMillis())));
     }
@@ -190,7 +198,11 @@ public void testGetAdditionalIndexSettingsMappingsMerging() throws Exception {
             settings,
             List.of(new CompressedXContent(mapping1), new CompressedXContent(mapping2), new CompressedXContent(mapping3))
         );
-        assertThat(result.size(), equalTo(3));
+        // The index.time_series.end_time setting requires index.mode to be set to time_series adding it here so that we read this setting:
+        // (in production the index.mode setting is usually provided in an index or component template)
+        result = builder().put(result).put("index.mode", "time_series").build();
+        assertThat(result.size(), equalTo(4));
+        assertThat(IndexSettings.MODE.get(result), equalTo(IndexMode.TIME_SERIES));
         assertThat(IndexSettings.TIME_SERIES_START_TIME.get(result), equalTo(now.minusMillis(DEFAULT_LOOK_BACK_TIME.getMillis())));
         assertThat(IndexSettings.TIME_SERIES_END_TIME.get(result), equalTo(now.plusMillis(DEFAULT_LOOK_AHEAD_TIME.getMillis())));
         assertThat(IndexMetadata.INDEX_ROUTING_PATH.get(result), containsInAnyOrder("field1", "field3"));
@@ -211,7 +223,11 @@ public void testGetAdditionalIndexSettingsNoMappings() {
             settings,
             List.of()
         );
-        assertThat(result.size(), equalTo(2));
+        // The index.time_series.end_time setting requires index.mode to be set to time_series adding it here so that we read this setting:
+        // (in production the index.mode setting is usually provided in an index or component template)
+        result = builder().put(result).put("index.mode", "time_series").build();
+        assertThat(result.size(), equalTo(3));
+        assertThat(result.get(IndexSettings.MODE.getKey()), equalTo("time_series"));
         assertThat(IndexSettings.TIME_SERIES_START_TIME.get(result), equalTo(now.minusMillis(DEFAULT_LOOK_BACK_TIME.getMillis())));
         assertThat(IndexSettings.TIME_SERIES_END_TIME.get(result), equalTo(now.plusMillis(DEFAULT_LOOK_AHEAD_TIME.getMillis())));
     }
@@ -232,7 +248,11 @@ public void testGetAdditionalIndexSettingsLookAheadTime() throws Exception {
             settings,
             List.of(new CompressedXContent("{}"))
         );
-        assertThat(result.size(), equalTo(2));
+        // The index.time_series.end_time setting requires index.mode to be set to time_series adding it here so that we read this setting:
+        // (in production the index.mode setting is usually provided in an index or component template)
+        result = builder().put(result).put("index.mode", "time_series").build();
+        assertThat(result.size(), equalTo(3));
+        assertThat(result.get(IndexSettings.MODE.getKey()), equalTo("time_series"));
         assertThat(IndexSettings.TIME_SERIES_START_TIME.get(result), equalTo(now.minusMillis(DEFAULT_LOOK_BACK_TIME.getMillis())));
         assertThat(IndexSettings.TIME_SERIES_END_TIME.get(result), equalTo(now.plusMillis(lookAheadTime.getMillis())));
     }
@@ -253,7 +273,11 @@ public void testGetAdditionalIndexSettingsLookBackTime() throws Exception {
             settings,
             List.of(new CompressedXContent("{}"))
         );
-        assertThat(result.size(), equalTo(2));
+        // The index.time_series.end_time setting requires index.mode to be set to time_series adding it here so that we read this setting:
+        // (in production the index.mode setting is usually provided in an index or component template)
+        result = builder().put(result).put("index.mode", "time_series").build();
+        assertThat(result.size(), equalTo(3));
+        assertThat(result.get(IndexSettings.MODE.getKey()), equalTo("time_series"));
         assertThat(IndexSettings.TIME_SERIES_START_TIME.get(result), equalTo(now.minusMillis(lookBackTime.getMillis())));
         assertThat(IndexSettings.TIME_SERIES_END_TIME.get(result), equalTo(now.plusMillis(DEFAULT_LOOK_AHEAD_TIME.getMillis())));
     }
@@ -363,7 +387,11 @@ public void testGetAdditionalIndexSettingsMigrateToTsdb() {
             settings,
             List.of()
         );
-        assertThat(result.size(), equalTo(2));
+        // The index.time_series.end_time setting requires index.mode to be set to time_series adding it here so that we read this setting:
+        // (in production the index.mode setting is usually provided in an index or component template)
+        result = builder().put(result).put("index.mode", "time_series").build();
+        assertThat(result.size(), equalTo(3));
+        assertThat(result.get(IndexSettings.MODE.getKey()), equalTo("time_series"));
         assertThat(IndexSettings.TIME_SERIES_START_TIME.get(result), equalTo(now.minusMillis(DEFAULT_LOOK_BACK_TIME.getMillis())));
         assertThat(IndexSettings.TIME_SERIES_END_TIME.get(result), equalTo(now.plusMillis(DEFAULT_LOOK_AHEAD_TIME.getMillis())));
     }
@@ -428,7 +456,8 @@ public void testGenerateRoutingPathFromDynamicTemplate() throws Exception {
             }
             """;
         Settings result = generateTsdbSettings(mapping, now);
-        assertThat(result.size(), equalTo(3));
+        assertThat(result.size(), equalTo(4));
+        assertThat(IndexSettings.MODE.get(result), equalTo(IndexMode.TIME_SERIES));
         assertThat(IndexSettings.TIME_SERIES_START_TIME.get(result), equalTo(now.minusMillis(DEFAULT_LOOK_BACK_TIME.getMillis())));
         assertThat(IndexSettings.TIME_SERIES_END_TIME.get(result), equalTo(now.plusMillis(DEFAULT_LOOK_AHEAD_TIME.getMillis())));
         assertThat(IndexMetadata.INDEX_ROUTING_PATH.get(result), containsInAnyOrder("host.id", "prometheus.labels.*"));
@@ -467,7 +496,8 @@ public void testGenerateRoutingPathFromDynamicTemplateWithMultiplePathMatchEntri
             }
             """;
         Settings result = generateTsdbSettings(mapping, now);
-        assertThat(result.size(), equalTo(3));
+        assertThat(result.size(), equalTo(4));
+        assertThat(IndexSettings.MODE.get(result), equalTo(IndexMode.TIME_SERIES));
         assertThat(IndexSettings.TIME_SERIES_START_TIME.get(result), equalTo(now.minusMillis(DEFAULT_LOOK_BACK_TIME.getMillis())));
         assertThat(IndexSettings.TIME_SERIES_END_TIME.get(result), equalTo(now.plusMillis(DEFAULT_LOOK_AHEAD_TIME.getMillis())));
         assertThat(
@@ -516,7 +546,8 @@ public void testGenerateRoutingPathFromDynamicTemplateWithMultiplePathMatchEntri
             }
             """;
         Settings result = generateTsdbSettings(mapping, now);
-        assertThat(result.size(), equalTo(3));
+        assertThat(result.size(), equalTo(4));
+        assertThat(IndexSettings.MODE.get(result), equalTo(IndexMode.TIME_SERIES));
         assertThat(IndexSettings.TIME_SERIES_START_TIME.get(result), equalTo(now.minusMillis(DEFAULT_LOOK_BACK_TIME.getMillis())));
         assertThat(IndexSettings.TIME_SERIES_END_TIME.get(result), equalTo(now.plusMillis(DEFAULT_LOOK_AHEAD_TIME.getMillis())));
         assertThat(
@@ -569,7 +600,8 @@ public void testGenerateRoutingPathFromDynamicTemplate_templateWithNoPathMatch()
             }
             """;
         Settings result = generateTsdbSettings(mapping, now);
-        assertThat(result.size(), equalTo(3));
+        assertThat(result.size(), equalTo(4));
+        assertThat(IndexSettings.MODE.get(result), equalTo(IndexMode.TIME_SERIES));
         assertThat(IndexSettings.TIME_SERIES_START_TIME.get(result), equalTo(now.minusMillis(DEFAULT_LOOK_BACK_TIME.getMillis())));
         assertThat(IndexSettings.TIME_SERIES_END_TIME.get(result), equalTo(now.plusMillis(DEFAULT_LOOK_AHEAD_TIME.getMillis())));
         assertThat(IndexMetadata.INDEX_ROUTING_PATH.get(result), containsInAnyOrder("host.id", "prometheus.labels.*"));
@@ -646,7 +678,8 @@ public void testGenerateRoutingPathFromPassThroughObject() throws Exception {
             }
             """;
         Settings result = generateTsdbSettings(mapping, now);
-        assertThat(result.size(), equalTo(3));
+        assertThat(result.size(), equalTo(4));
+        assertThat(IndexSettings.MODE.get(result), equalTo(IndexMode.TIME_SERIES));
         assertThat(IndexSettings.TIME_SERIES_START_TIME.get(result), equalTo(now.minusMillis(DEFAULT_LOOK_BACK_TIME.getMillis())));
         assertThat(IndexSettings.TIME_SERIES_END_TIME.get(result), equalTo(now.plusMillis(DEFAULT_LOOK_AHEAD_TIME.getMillis())));
         assertThat(IndexMetadata.INDEX_ROUTING_PATH.get(result), containsInAnyOrder("labels.*"));
@@ -657,7 +690,7 @@ private Settings generateTsdbSettings(String mapping, Instant now) throws IOExce
         String dataStreamName = "logs-app1";
         Settings settings = Settings.EMPTY;
 
-        return provider.getAdditionalIndexSettings(
+        var result = provider.getAdditionalIndexSettings(
             DataStream.getDefaultBackingIndexName(dataStreamName, 1),
             dataStreamName,
             true,
@@ -666,6 +699,9 @@ private Settings generateTsdbSettings(String mapping, Instant now) throws IOExce
             settings,
             List.of(new CompressedXContent(mapping))
         );
+        // The index.time_series.end_time setting requires index.mode to be set to time_series adding it here so that we read this setting:
+        // (in production the index.mode setting is usually provided in an index or component template)
+        return builder().put(result).put("index.mode", "time_series").build();
     }
 
 }
diff --git a/...s/src/test/java/org/elasticsearch/datastreams/MetadataDataStreamRolloverServiceTests.java b/...s/src/test/java/org/elasticsearch/datastreams/MetadataDataStreamRolloverServiceTests.java
@@ -24,7 +24,6 @@
 import org.elasticsearch.cluster.metadata.Metadata;
 import org.elasticsearch.cluster.metadata.Template;
 import org.elasticsearch.common.settings.Settings;
-import org.elasticsearch.common.time.DateUtils;
 import org.elasticsearch.index.Index;
 import org.elasticsearch.index.IndexMode;
 import org.elasticsearch.index.IndexSettings;
@@ -398,11 +397,9 @@ public void testRolloverClusterStateWithBrokenOlderTsdbDataStream() throws Excep
 
             for (int i = 0; i < numberOfBackingIndices; i++) {
                 var im = rolloverMetadata.index(rolloverMetadata.dataStreams().get(dataStreamName).getIndices().get(i));
-                var startTime1 = IndexSettings.TIME_SERIES_START_TIME.get(im.getSettings());
-                var endTime1 = IndexSettings.TIME_SERIES_END_TIME.get(im.getSettings());
-                assertThat(startTime1.toEpochMilli(), equalTo(DateUtils.MAX_MILLIS_BEFORE_MINUS_9999));
-                assertThat(endTime1.toEpochMilli(), equalTo(DateUtils.MAX_MILLIS_BEFORE_9999));
-                assertThat(im.getIndexMode(), equalTo(null));
+                assertThat(im.getTimeSeriesStart(), nullValue());
+                assertThat(im.getTimeSeriesEnd(), nullValue());
+                assertThat(im.getIndexMode(), nullValue());
             }
             {
                 var im = rolloverMetadata.index(

diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/10_settings.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/10_settings.yml
@@ -336,6 +336,67 @@ set start_time and end_time without timeseries mode:
               time_series:
                 end_time: 1632625782000
 
+---
+set start_time, end_time and routing_path via put settings api without time_series mode:
+  - requires:
+      cluster_features: [ "gte_v8.15.0" ]
+      reason: bug fixed in 8.15.0
+
+  - do:
+      indices.create:
+        index: test-index
+  - match: { acknowledged: true }
+
+  - do:
+      catch: /\[index.time_series.end_time\] requires \[index.mode=time_series\]/
+      indices.put_settings:
+        index: test-index
+        body:
+          index.time_series.end_time: 1632625782000
+
+  - do:
+      catch: /Can't update non dynamic settings \[\[index.time_series.start_time\]\] for open indices/
+      indices.put_settings:
+        index: test-index
+        body:
+          index.time_series.start_time: 1632625782000
+
+  - do:
+      catch: /Can't update non dynamic settings \[\[index.routing_path\]\] for open indices/
+      indices.put_settings:
+        index: test-index
+        body:
+          settings:
+            index:
+              routing_path: foo
+
+  - do:
+      indices.close:
+        index: test-index
+
+  - do:
+      catch: /\[index.time_series.end_time\] requires \[index.mode=time_series\]/
+      indices.put_settings:
+        index: test-index
+        body:
+          index.time_series.end_time: 1632625782000
+
+  - do:
+      catch: /final test-index setting \[index.time_series.start_time\], not updateable/
+      indices.put_settings:
+        index: test-index
+        body:
+          index.time_series.start_time: 1632625782000
+
+  - do:
+      catch: /final test-index setting \[index.routing_path\], not updateable/
+      indices.put_settings:
+        index: test-index
+        body:
+          settings:
+            index:
+              routing_path: foo
+
 ---
 set bad start_time and end_time:
   - requires:

diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateService.java b/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateService.java
@@ -714,7 +714,9 @@ private void validateIndexTemplateV2(String name, ComposableIndexTemplate indexT
                 )
             );
         }
-        // Then apply settings resolved from templates:
+        // Then apply setting from component templates:
+        finalSettings.put(combinedSettings);
+        // Then finally apply settings resolved from index template:
         finalSettings.put(finalTemplate.map(Template::settings).orElse(Settings.EMPTY));
 
         var templateToValidate = indexTemplate.toBuilder()

diff --git a/server/src/main/java/org/elasticsearch/index/IndexSettings.java b/server/src/main/java/org/elasticsearch/index/IndexSettings.java
@@ -618,16 +618,25 @@ public void validate(Instant value) {}
 
             @Override
             public void validate(Instant value, Map<Setting<?>, Object> settings) {
-                @SuppressWarnings("unchecked")
                 Instant startTime = (Instant) settings.get(TIME_SERIES_START_TIME);
                 if (startTime.toEpochMilli() > value.toEpochMilli()) {
                     throw new IllegalArgumentException("index.time_series.end_time must be larger than index.time_series.start_time");
                 }
+
+                // The index.time_series.end_time setting can only be specified if the index.mode setting has been set to time_series
+                // This check here is specifically needed because in case of updating index settings the validation the gets executed
+                // in IndexSettings constructor when reading the index.mode setting doesn't get executed.
+                IndexMode indexMode = (IndexMode) settings.get(MODE);
+                if (indexMode != IndexMode.TIME_SERIES) {
+                    throw new IllegalArgumentException(
+                        "[" + TIME_SERIES_END_TIME.getKey() + "] requires [index.mode=" + IndexMode.TIME_SERIES + "]"
+                    );
+                }
             }
 
             @Override
             public Iterator<Setting<?>> settings() {
-                List<Setting<?>> settings = List.of(TIME_SERIES_START_TIME);
+                List<Setting<?>> settings = List.of(TIME_SERIES_START_TIME, MODE);
                 return settings.iterator();
             }
         },