diff --git a/docs/changelog/110268.yaml b/docs/changelog/110268.yaml new file mode 100644 index 0000000000000..adfb467f92e8b --- /dev/null +++ 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/modules/data-streams/src/test/java/org/elasticsearch/datastreams/DataStreamIndexSettingsProviderTests.java b/modules/data-streams/src/test/java/org/elasticsearch/datastreams/DataStreamIndexSettingsProviderTests.java index 85f0d354576a4..cf6911850921b 100644 --- a/modules/data-streams/src/test/java/org/elasticsearch/datastreams/DataStreamIndexSettingsProviderTests.java +++ b/modules/data-streams/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/modules/data-streams/src/test/java/org/elasticsearch/datastreams/MetadataDataStreamRolloverServiceTests.java b/modules/data-streams/src/test/java/org/elasticsearch/datastreams/MetadataDataStreamRolloverServiceTests.java index 86f6dea220e84..8156345b83b4c 100644 --- a/modules/data-streams/src/test/java/org/elasticsearch/datastreams/MetadataDataStreamRolloverServiceTests.java +++ b/modules/data-streams/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 index 485b5b1796ec4..46476fd071b30 100644 --- 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 index e9da9629cb6a3..e9658e71f895e 100644 --- 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 index 5d864e4fa1e24..1e718fba0d08d 100644 --- 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, 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> settings() { - List> settings = List.of(TIME_SERIES_START_TIME); + List> settings = List.of(TIME_SERIES_START_TIME, MODE); return settings.iterator(); } },