From e93ea67bf10db122c7a02b21ee1034a982122505 Mon Sep 17 00:00:00 2001 From: Keith Massey Date: Wed, 16 Apr 2025 13:40:05 -0500 Subject: [PATCH 01/13] Adding settings to data streams --- .../UpdateTimeSeriesRangeServiceTests.java | 1 + .../org/elasticsearch/TransportVersions.java | 1 + .../cluster/metadata/DataStream.java | 97 +++++++++- .../MetadataCreateDataStreamService.java | 1 + .../cluster/metadata/DataStreamTests.java | 167 +++++++++++++++++- .../metadata/DataStreamTestHelper.java | 14 ++ ...StreamLifecycleUsageTransportActionIT.java | 1 + .../DataStreamDeprecationCheckerTests.java | 3 + .../IndexDeprecationCheckerTests.java | 1 + 9 files changed, 279 insertions(+), 7 deletions(-) diff --git a/modules/data-streams/src/test/java/org/elasticsearch/datastreams/UpdateTimeSeriesRangeServiceTests.java b/modules/data-streams/src/test/java/org/elasticsearch/datastreams/UpdateTimeSeriesRangeServiceTests.java index 03546c126818e..0169b1b7da8cd 100644 --- a/modules/data-streams/src/test/java/org/elasticsearch/datastreams/UpdateTimeSeriesRangeServiceTests.java +++ b/modules/data-streams/src/test/java/org/elasticsearch/datastreams/UpdateTimeSeriesRangeServiceTests.java @@ -257,6 +257,7 @@ public void testUpdateTimeSeriesTemporalOneBadDataStream() { ds2Indices, 2, ds2.getMetadata(), + ds2.getSettings(), ds2.isHidden(), ds2.isReplicated(), ds2.isSystem(), diff --git a/server/src/main/java/org/elasticsearch/TransportVersions.java b/server/src/main/java/org/elasticsearch/TransportVersions.java index 51482a99dc8b1..30c407c11f301 100644 --- a/server/src/main/java/org/elasticsearch/TransportVersions.java +++ b/server/src/main/java/org/elasticsearch/TransportVersions.java @@ -223,6 +223,7 @@ static TransportVersion def(int id) { public static final TransportVersion ESQL_REPORT_SHARD_PARTITIONING = def(9_050_0_00); public static final TransportVersion ESQL_QUERY_PLANNING_DURATION = def(9_051_0_00); public static final TransportVersion ESQL_DOCUMENTS_FOUND_AND_VALUES_LOADED = def(9_052_0_00); + public static final TransportVersion SETTINGS_IN_DATA_STREAMS = def(9_053_00_0); /* * STOP! READ THIS FIRST! No, really, diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/DataStream.java b/server/src/main/java/org/elasticsearch/cluster/metadata/DataStream.java index 607e3a8a07efb..85aa16afc1e4b 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/DataStream.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/DataStream.java @@ -31,6 +31,7 @@ import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.time.DateFormatter; import org.elasticsearch.common.time.DateFormatters; import org.elasticsearch.common.util.FeatureFlag; @@ -57,6 +58,7 @@ import java.util.ArrayList; import java.util.Comparator; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Locale; import java.util.Map; @@ -67,6 +69,7 @@ import java.util.function.Predicate; import java.util.stream.Collectors; +import static org.elasticsearch.cluster.metadata.MetadataCreateDataStreamService.lookupTemplateForDataStream; import static org.elasticsearch.common.xcontent.XContentParserUtils.ensureExpectedToken; import static org.elasticsearch.index.IndexSettings.LIFECYCLE_ORIGINATION_DATE; import static org.elasticsearch.index.IndexSettings.PREFER_ILM_SETTING; @@ -118,6 +121,7 @@ public static boolean isFailureStoreFeatureFlagEnabled() { private final long generation; @Nullable private final Map metadata; + private final Settings settings; private final boolean hidden; private final boolean replicated; private final boolean system; @@ -137,6 +141,7 @@ public DataStream( List indices, long generation, Map metadata, + Settings settings, boolean hidden, boolean replicated, boolean system, @@ -152,6 +157,7 @@ public DataStream( name, generation, metadata, + settings, hidden, replicated, system, @@ -169,6 +175,7 @@ public DataStream( String name, long generation, Map metadata, + Settings settings, boolean hidden, boolean replicated, boolean system, @@ -183,6 +190,7 @@ public DataStream( this.name = name; this.generation = generation; this.metadata = metadata; + this.settings = Objects.requireNonNull(settings); assert system == false || hidden; // system indices must be hidden this.hidden = hidden; this.replicated = replicated; @@ -235,10 +243,17 @@ public static DataStream read(StreamInput in) throws IOException { // is still behind a feature flag in previous version we use the default value instead of explicitly disabling it. dataStreamOptions = failureStoreEnabled ? DataStreamOptions.FAILURE_STORE_ENABLED : null; } + final Settings settings; + if (in.getTransportVersion().onOrAfter(TransportVersions.SETTINGS_IN_DATA_STREAMS)) { + settings = Settings.readSettingsFromStream(in); + } else { + settings = Settings.EMPTY; + } return new DataStream( name, generation, metadata, + settings, hidden, replicated, system, @@ -329,6 +344,52 @@ public boolean rolloverOnWrite() { return backingIndices.rolloverOnWrite; } + public ComposableIndexTemplate getEffectiveIndexTemplate(ProjectMetadata projectMetadata) { + return mergeSettingsIntoTemplate(lookupTemplateForDataStream(name, projectMetadata), settings); + } + + public static ComposableIndexTemplate getEffectiveIndexTemplate( + String dataStreamName, + ProjectMetadata projectMetadata, + Settings settingsOverrides + ) { + return mergeSettingsIntoTemplate(lookupTemplateForDataStream(dataStreamName, projectMetadata), settingsOverrides); + } + + public Settings getEffectiveSettings(ProjectMetadata projectMetadata) { + ComposableIndexTemplate template = getMatchingIndexTemplate(projectMetadata); + return mergeSettings(template.template() == null ? Settings.EMPTY : template.template().settings(), settings); + } + + private ComposableIndexTemplate getMatchingIndexTemplate(ProjectMetadata projectMetadata) { + return lookupTemplateForDataStream(name, projectMetadata); + } + + public static ComposableIndexTemplate mergeSettingsIntoTemplate(ComposableIndexTemplate template, Settings settings) { + if (Settings.EMPTY.equals(settings)) { + return template; + } + ComposableIndexTemplate.Builder mergedIndexTemplateBuilder = template.toBuilder(); + assert template.template() != null : "Template is unexpectedly null"; + Template.Builder mergedTemplateBuilder = Template.builder(template.template()); + mergedTemplateBuilder.settings(mergeSettings(template.template().settings(), settings)); + mergedIndexTemplateBuilder.template(mergedTemplateBuilder); + return mergedIndexTemplateBuilder.build(); + } + + private static Settings mergeSettings(Settings originalSettings, Settings newSettings) { + if (Settings.EMPTY.equals(newSettings)) { + return Objects.requireNonNullElse(originalSettings, Settings.EMPTY); + } + Settings.Builder settingsBuilder = Settings.builder().put(originalSettings).put(newSettings); + for (String settingName : new HashSet<>(settingsBuilder.keys())) { + if (settingsBuilder.get(settingName) == null) { + settingsBuilder.remove(settingName); + } + } + return settingsBuilder.build(); + } + /** * We define that a data stream is considered internal either if it is a system index or if * its name starts with a dot. @@ -440,6 +501,10 @@ public Map getMetadata() { return metadata; } + public Settings getSettings() { + return settings; + } + @Override public boolean isHidden() { return hidden; @@ -1268,6 +1333,9 @@ public void writeTo(StreamOutput out) throws IOException { if (out.getTransportVersion().onOrAfter(DataStream.ADD_DATA_STREAM_OPTIONS_VERSION)) { out.writeOptionalWriteable(dataStreamOptions.isEmpty() ? null : dataStreamOptions); } + if (out.getTransportVersion().onOrAfter(TransportVersions.SETTINGS_IN_DATA_STREAMS)) { + settings.writeTo(out); + } } public static final ParseField NAME_FIELD = new ParseField("name"); @@ -1288,26 +1356,28 @@ public void writeTo(StreamOutput out) throws IOException { public static final ParseField FAILURE_ROLLOVER_ON_WRITE_FIELD = new ParseField("failure_rollover_on_write"); public static final ParseField FAILURE_AUTO_SHARDING_FIELD = new ParseField("failure_auto_sharding"); public static final ParseField DATA_STREAM_OPTIONS_FIELD = new ParseField("options"); + public static final ParseField SETTINGS_FIELD = new ParseField("settings"); @SuppressWarnings("unchecked") private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>("data_stream", args -> { // Fields behind a feature flag need to be parsed last otherwise the parser will fail when the feature flag is disabled. // Until the feature flag is removed we keep them separately to be mindful of this. - boolean failureStoreEnabled = DataStream.isFailureStoreFeatureFlagEnabled() && args[12] != null && (boolean) args[12]; + boolean failureStoreEnabled = DataStream.isFailureStoreFeatureFlagEnabled() && args[13] != null && (boolean) args[13]; DataStreamIndices failureIndices = DataStream.isFailureStoreFeatureFlagEnabled() ? new DataStreamIndices( FAILURE_STORE_PREFIX, - args[13] != null ? (List) args[13] : List.of(), - args[14] != null && (boolean) args[14], - (DataStreamAutoShardingEvent) args[15] + args[14] != null ? (List) args[14] : List.of(), + args[15] != null && (boolean) args[15], + (DataStreamAutoShardingEvent) args[16] ) : new DataStreamIndices(FAILURE_STORE_PREFIX, List.of(), false, null); // We cannot distinguish if failure store was explicitly disabled or not. Given that failure store // is still behind a feature flag in previous version we use the default value instead of explicitly disabling it. DataStreamOptions dataStreamOptions = DataStreamOptions.EMPTY; + if (DataStream.isFailureStoreFeatureFlagEnabled()) { - if (args[16] != null) { - dataStreamOptions = (DataStreamOptions) args[16]; + if (args[17] != null) { + dataStreamOptions = (DataStreamOptions) args[17]; } else if (failureStoreEnabled) { dataStreamOptions = DataStreamOptions.FAILURE_STORE_ENABLED; } @@ -1316,6 +1386,7 @@ public void writeTo(StreamOutput out) throws IOException { (String) args[0], (Long) args[2], (Map) args[3], + args[12] == null ? Settings.EMPTY : (Settings) args[12], args[4] != null && (boolean) args[4], args[5] != null && (boolean) args[5], args[6] != null && (boolean) args[6], @@ -1359,6 +1430,7 @@ public void writeTo(StreamOutput out) throws IOException { (p, c) -> DataStreamAutoShardingEvent.fromXContent(p), AUTO_SHARDING_FIELD ); + PARSER.declareObject(ConstructingObjectParser.optionalConstructorArg(), (p, c) -> Settings.fromXContent(p), SETTINGS_FIELD); // The fields behind the feature flag should always be last. if (DataStream.isFailureStoreFeatureFlagEnabled()) { // Should be removed after backport @@ -1415,6 +1487,9 @@ public XContentBuilder toXContent( builder.field(REPLICATED_FIELD.getPreferredName(), replicated); builder.field(SYSTEM_FIELD.getPreferredName(), system); builder.field(ALLOW_CUSTOM_ROUTING.getPreferredName(), allowCustomRouting); + builder.startObject(SETTINGS_FIELD.getPreferredName()); + this.settings.toXContent(builder, params); + builder.endObject(); if (DataStream.isFailureStoreFeatureFlagEnabled()) { if (failureIndices.indices.isEmpty() == false) { builder.xContentList(FAILURE_INDICES_FIELD.getPreferredName(), failureIndices.indices); @@ -1455,6 +1530,7 @@ public boolean equals(Object o) { return name.equals(that.name) && generation == that.generation && Objects.equals(metadata, that.metadata) + && Objects.equals(settings, that.settings) && hidden == that.hidden && system == that.system && replicated == that.replicated @@ -1472,6 +1548,7 @@ public int hashCode() { name, generation, metadata, + settings, hidden, system, replicated, @@ -1784,6 +1861,7 @@ public static class Builder { private long generation = 1; @Nullable private Map metadata = null; + private Settings settings = Settings.EMPTY; private boolean hidden = false; private boolean replicated = false; private boolean system = false; @@ -1811,6 +1889,7 @@ private Builder(DataStream dataStream) { name = dataStream.name; generation = dataStream.generation; metadata = dataStream.metadata; + settings = dataStream.settings; hidden = dataStream.hidden; replicated = dataStream.replicated; system = dataStream.system; @@ -1842,6 +1921,11 @@ public Builder setMetadata(Map metadata) { return this; } + public Builder setSettings(Settings settings) { + this.settings = settings; + return this; + } + public Builder setHidden(boolean hidden) { this.hidden = hidden; return this; @@ -1902,6 +1986,7 @@ public DataStream build() { name, generation, metadata, + settings, hidden, replicated, system, diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataCreateDataStreamService.java b/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataCreateDataStreamService.java index 3219b5ec32626..15ddfc6b8e93e 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataCreateDataStreamService.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataCreateDataStreamService.java @@ -331,6 +331,7 @@ static ClusterState createDataStream( dataStreamName, initialGeneration, template.metadata() != null ? Map.copyOf(template.metadata()) : null, + Settings.EMPTY, hidden, false, isSystem, diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/DataStreamTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/DataStreamTests.java index fc0b9a291549b..e836277d21dd3 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/DataStreamTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/DataStreamTests.java @@ -17,6 +17,7 @@ import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.common.Strings; import org.elasticsearch.common.UUIDs; +import org.elasticsearch.common.compress.CompressedXContent; import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.common.settings.ClusterSettings; import org.elasticsearch.common.settings.Settings; @@ -52,12 +53,14 @@ import java.util.function.Function; import java.util.stream.Collectors; +import static org.elasticsearch.cluster.metadata.ComponentTemplateTests.randomMappings; import static org.elasticsearch.cluster.metadata.DataStream.getDefaultBackingIndexName; import static org.elasticsearch.cluster.metadata.DataStream.getDefaultFailureStoreName; import static org.elasticsearch.cluster.metadata.DataStreamTestHelper.newInstance; import static org.elasticsearch.cluster.metadata.DataStreamTestHelper.randomGlobalRetention; import static org.elasticsearch.cluster.metadata.DataStreamTestHelper.randomIndexInstances; import static org.elasticsearch.cluster.metadata.DataStreamTestHelper.randomNonEmptyIndexInstances; +import static org.elasticsearch.cluster.metadata.DataStreamTestHelper.randomSettings; import static org.elasticsearch.index.IndexSettings.LIFECYCLE_ORIGINATION_DATE; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; @@ -69,6 +72,7 @@ import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.nullValue; +import static org.junit.Assert.assertThrows; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -95,6 +99,7 @@ protected DataStream mutateInstance(DataStream instance) { var indices = instance.getIndices(); var generation = instance.getGeneration(); var metadata = instance.getMetadata(); + var settings = instance.getSettings(); var isHidden = instance.isHidden(); var isReplicated = instance.isReplicated(); var isSystem = instance.isSystem(); @@ -107,7 +112,7 @@ protected DataStream mutateInstance(DataStream instance) { var autoShardingEvent = instance.getAutoShardingEvent(); var failureRolloverOnWrite = instance.getFailureComponent().isRolloverOnWrite(); var failureAutoShardingEvent = instance.getDataComponent().getAutoShardingEvent(); - switch (between(0, 15)) { + switch (between(0, 16)) { case 0 -> name = randomAlphaOfLength(10); case 1 -> indices = randomNonEmptyIndexInstances(); case 2 -> generation = instance.getGeneration() + randomIntBetween(1, 10); @@ -172,12 +177,14 @@ protected DataStream mutateInstance(DataStream instance) { case 15 -> failureAutoShardingEvent = randomBoolean() && failureAutoShardingEvent != null ? null : new DataStreamAutoShardingEvent(indices.getLast().getName(), randomIntBetween(1, 10), randomMillisUpToYear9999()); + case 16 -> settings = randomValueOtherThan(settings, DataStreamTestHelper::randomSettings); } return new DataStream( name, generation, metadata, + settings, isHidden, isReplicated, isSystem, @@ -2117,6 +2124,7 @@ public void testXContentSerializationWithRolloverAndEffectiveRetention() throws indices, generation, metadata, + randomSettings(), isSystem, randomBoolean(), isSystem, @@ -2311,6 +2319,7 @@ public void testWriteFailureIndex() { randomNonEmptyIndexInstances(), randomNonNegativeInt(), null, + randomSettings(), hidden, replicated, system, @@ -2329,6 +2338,7 @@ public void testWriteFailureIndex() { randomNonEmptyIndexInstances(), randomNonNegativeInt(), null, + randomSettings(), hidden, replicated, system, @@ -2354,6 +2364,7 @@ public void testWriteFailureIndex() { randomNonEmptyIndexInstances(), randomNonNegativeInt(), null, + randomSettings(), hidden, replicated, system, @@ -2378,6 +2389,7 @@ public void testIsFailureIndex() { backingIndices, randomNonNegativeInt(), null, + randomSettings(), hidden, replicated, system, @@ -2400,6 +2412,7 @@ public void testIsFailureIndex() { backingIndices, randomNonNegativeInt(), null, + randomSettings(), hidden, replicated, system, @@ -2431,6 +2444,7 @@ public void testIsFailureIndex() { backingIndices, randomNonNegativeInt(), null, + randomSettings(), hidden, replicated, system, @@ -2618,6 +2632,157 @@ public void testIsFailureStoreEffectivelyEnabled_staticHelperMethod() { ); } + public void testGetEffectiveSettings() { + { + // No matching template, so we expect an IllegalArgumentException + DataStream dataStream = createTestInstance(); + ProjectMetadata.Builder projectMetadataBuilder = ProjectMetadata.builder(randomProjectIdOrDefault()); + assertThrows(IllegalArgumentException.class, () -> dataStream.getEffectiveSettings(projectMetadataBuilder.build())); + } + { + // We only have settings from the template, so we expect to get those back + DataStream dataStream = createDataStream(Settings.EMPTY); + Settings templateSettings = randomSettings(); + Template.Builder templateBuilder = Template.builder().settings(templateSettings); + ComposableIndexTemplate indexTemplate = ComposableIndexTemplate.builder() + .indexPatterns(List.of(dataStream.getName())) + .dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate()) + .template(templateBuilder) + .build(); + ProjectMetadata.Builder projectMetadataBuilder = ProjectMetadata.builder(randomProjectIdOrDefault()) + .indexTemplates(Map.of(dataStream.getName(), indexTemplate)); + assertThat(dataStream.getEffectiveSettings(projectMetadataBuilder.build()), equalTo(templateSettings)); + } + { + // We only have settings from the data stream, so we expect to get those back + Settings dataStreamSettings = randomSettings(); + DataStream dataStream = createDataStream(dataStreamSettings); + Settings templateSettings = Settings.EMPTY; + Template.Builder templateBuilder = Template.builder().settings(templateSettings); + ComposableIndexTemplate indexTemplate = ComposableIndexTemplate.builder() + .indexPatterns(List.of(dataStream.getName())) + .dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate()) + .template(templateBuilder) + .build(); + ProjectMetadata.Builder projectMetadataBuilder = ProjectMetadata.builder(randomProjectIdOrDefault()) + .indexTemplates(Map.of(dataStream.getName(), indexTemplate)); + assertThat(dataStream.getEffectiveSettings(projectMetadataBuilder.build()), equalTo(dataStreamSettings)); + } + { + // Here we have settings from both the template and the data stream, so we expect the data stream settings to take precedence + Settings dataStreamSettings = Settings.builder() + .put("index.setting1", "dataStreamValue") + .put("index.setting2", "dataStreamValue") + .put("index.setting3", (String) null) // This one gets removed from the effective settings + .build(); + DataStream dataStream = createDataStream(dataStreamSettings); + Settings templateSettings = Settings.builder() + .put("index.setting1", "templateValue") + .put("index.setting3", "templateValue") + .put("index.setting4", "templateValue") + .build(); + Template.Builder templateBuilder = Template.builder().settings(templateSettings); + ComposableIndexTemplate indexTemplate = ComposableIndexTemplate.builder() + .indexPatterns(List.of(dataStream.getName())) + .dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate()) + .template(templateBuilder) + .build(); + ProjectMetadata.Builder projectMetadataBuilder = ProjectMetadata.builder(randomProjectIdOrDefault()) + .indexTemplates(Map.of(dataStream.getName(), indexTemplate)); + Settings mergedSettings = Settings.builder() + .put("index.setting1", "dataStreamValue") + .put("index.setting2", "dataStreamValue") + .put("index.setting4", "templateValue") + .build(); + assertThat(dataStream.getEffectiveSettings(projectMetadataBuilder.build()), equalTo(mergedSettings)); + } + } + + public void testGetEffectiveIndexTemplate() { + { + // No matching template, so we expect an IllegalArgumentException + DataStream dataStream = createTestInstance(); + ProjectMetadata.Builder projectMetadataBuilder = ProjectMetadata.builder(randomProjectIdOrDefault()); + assertThrows(IllegalArgumentException.class, () -> dataStream.getEffectiveIndexTemplate(projectMetadataBuilder.build())); + } + { + // We only have settings from the template, so the effective template will just be the original template + DataStream dataStream = createDataStream(Settings.EMPTY); + Settings templateSettings = randomSettings(); + Template.Builder templateBuilder = Template.builder().settings(templateSettings).mappings(randomMappings()); + ComposableIndexTemplate indexTemplate = ComposableIndexTemplate.builder() + .indexPatterns(List.of(dataStream.getName())) + .dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate()) + .template(templateBuilder) + .build(); + ProjectMetadata.Builder projectMetadataBuilder = ProjectMetadata.builder(randomProjectIdOrDefault()) + .indexTemplates(Map.of(dataStream.getName(), indexTemplate)); + assertThat(dataStream.getEffectiveIndexTemplate(projectMetadataBuilder.build()), equalTo(indexTemplate)); + } + { + // We only have settings from the data stream, so we expect to get only those back in the effective template + Settings dataStreamSettings = randomSettings(); + DataStream dataStream = createDataStream(dataStreamSettings); + Settings templateSettings = Settings.EMPTY; + CompressedXContent templateMappings = randomMappings(); + Template.Builder templateBuilder = Template.builder().settings(templateSettings).mappings(templateMappings); + ComposableIndexTemplate indexTemplate = ComposableIndexTemplate.builder() + .indexPatterns(List.of(dataStream.getName())) + .dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate()) + .template(templateBuilder) + .build(); + ProjectMetadata.Builder projectMetadataBuilder = ProjectMetadata.builder(randomProjectIdOrDefault()) + .indexTemplates(Map.of(dataStream.getName(), indexTemplate)); + Template.Builder expectedTemplateBuilder = Template.builder().settings(dataStreamSettings).mappings(templateMappings); + ComposableIndexTemplate expectedEffectiveTemplate = ComposableIndexTemplate.builder() + .indexPatterns(List.of(dataStream.getName())) + .dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate()) + .template(expectedTemplateBuilder) + .build(); + assertThat(dataStream.getEffectiveIndexTemplate(projectMetadataBuilder.build()), equalTo(expectedEffectiveTemplate)); + } + { + // Here we have settings from both the template and the data stream, so we expect the data stream settings to take precedence + Settings dataStreamSettings = Settings.builder() + .put("index.setting1", "dataStreamValue") + .put("index.setting2", "dataStreamValue") + .put("index.setting3", (String) null) // This one gets removed from the effective settings + .build(); + DataStream dataStream = createDataStream(dataStreamSettings); + Settings templateSettings = Settings.builder() + .put("index.setting1", "templateValue") + .put("index.setting3", "templateValue") + .put("index.setting4", "templateValue") + .build(); + CompressedXContent templateMappings = randomMappings(); + Template.Builder templateBuilder = Template.builder().settings(templateSettings).mappings(templateMappings); + ComposableIndexTemplate indexTemplate = ComposableIndexTemplate.builder() + .indexPatterns(List.of(dataStream.getName())) + .dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate()) + .template(templateBuilder) + .build(); + ProjectMetadata.Builder projectMetadataBuilder = ProjectMetadata.builder(randomProjectIdOrDefault()) + .indexTemplates(Map.of(dataStream.getName(), indexTemplate)); + Settings mergedSettings = Settings.builder() + .put("index.setting1", "dataStreamValue") + .put("index.setting2", "dataStreamValue") + .put("index.setting4", "templateValue") + .build(); + Template.Builder expectedTemplateBuilder = Template.builder().settings(mergedSettings).mappings(templateMappings); + ComposableIndexTemplate expectedEffectiveTemplate = ComposableIndexTemplate.builder() + .indexPatterns(List.of(dataStream.getName())) + .dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate()) + .template(expectedTemplateBuilder) + .build(); + assertThat(dataStream.getEffectiveIndexTemplate(projectMetadataBuilder.build()), equalTo(expectedEffectiveTemplate)); + } + } + + private DataStream createDataStream(Settings settings) { + DataStream dataStream = createTestInstance(); + return dataStream.copy().setSettings(settings).build(); + } + private record DataStreamMetadata(Long creationTimeInMillis, Long rolloverTimeInMillis, Long originationTimeInMillis) { public static DataStreamMetadata dataStreamMetadata(Long creationTimeInMillis, Long rolloverTimeInMillis) { return new DataStreamMetadata(creationTimeInMillis, rolloverTimeInMillis, null); diff --git a/test/framework/src/main/java/org/elasticsearch/cluster/metadata/DataStreamTestHelper.java b/test/framework/src/main/java/org/elasticsearch/cluster/metadata/DataStreamTestHelper.java index 9c52b273d057a..7bcde98044ad3 100644 --- a/test/framework/src/main/java/org/elasticsearch/cluster/metadata/DataStreamTestHelper.java +++ b/test/framework/src/main/java/org/elasticsearch/cluster/metadata/DataStreamTestHelper.java @@ -77,8 +77,10 @@ import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_INDEX_UUID; import static org.elasticsearch.test.ESTestCase.generateRandomStringArray; import static org.elasticsearch.test.ESTestCase.randomAlphaOfLength; +import static org.elasticsearch.test.ESTestCase.randomAlphanumericOfLength; import static org.elasticsearch.test.ESTestCase.randomBoolean; import static org.elasticsearch.test.ESTestCase.randomFrom; +import static org.elasticsearch.test.ESTestCase.randomInt; import static org.elasticsearch.test.ESTestCase.randomIntBetween; import static org.elasticsearch.test.ESTestCase.randomMap; import static org.elasticsearch.test.ESTestCase.randomMillisUpToYear9999; @@ -357,6 +359,7 @@ public static DataStream randomInstance(String dataStreamName, LongSupplier time dataStreamName, generation, metadata, + randomSettings(), randomBoolean(), replicated, false, // Some tests don't work well with system data streams, since these data streams require special handling @@ -835,4 +838,15 @@ public static DataStreamOptions.Template createDataStreamOptionsTemplate(Boolean ResettableValue.create(new DataStreamFailureStore.Template(ResettableValue.create(failureStore))) ); } + + static Settings randomSettings() { + Settings.Builder builder = Settings.builder(); + if (randomBoolean()) { + return Settings.EMPTY; + } + for (int i = 1; i < randomInt(100); i++) { + builder.put(randomAlphanumericOfLength(20), randomAlphanumericOfLength(50)); + } + return builder.build(); + } } diff --git a/x-pack/plugin/core/src/internalClusterTest/java/org/elasticsearch/xpack/core/action/DataStreamLifecycleUsageTransportActionIT.java b/x-pack/plugin/core/src/internalClusterTest/java/org/elasticsearch/xpack/core/action/DataStreamLifecycleUsageTransportActionIT.java index 24a59ad1c6393..e7724bf965d58 100644 --- a/x-pack/plugin/core/src/internalClusterTest/java/org/elasticsearch/xpack/core/action/DataStreamLifecycleUsageTransportActionIT.java +++ b/x-pack/plugin/core/src/internalClusterTest/java/org/elasticsearch/xpack/core/action/DataStreamLifecycleUsageTransportActionIT.java @@ -174,6 +174,7 @@ public void testAction() throws Exception { indices, randomLongBetween(0, 1000), Map.of(), + Settings.EMPTY, systemDataStream || randomBoolean(), replicated, systemDataStream, diff --git a/x-pack/plugin/deprecation/src/test/java/org/elasticsearch/xpack/deprecation/DataStreamDeprecationCheckerTests.java b/x-pack/plugin/deprecation/src/test/java/org/elasticsearch/xpack/deprecation/DataStreamDeprecationCheckerTests.java index e0dfaef605af7..7eb1a2c666afa 100644 --- a/x-pack/plugin/deprecation/src/test/java/org/elasticsearch/xpack/deprecation/DataStreamDeprecationCheckerTests.java +++ b/x-pack/plugin/deprecation/src/test/java/org/elasticsearch/xpack/deprecation/DataStreamDeprecationCheckerTests.java @@ -174,6 +174,7 @@ private DataStream createTestDataStream( allIndices, randomNegativeLong(), Map.of(), + Settings.EMPTY, randomBoolean(), false, false, @@ -264,6 +265,7 @@ public void testOldIndicesIgnoredWarningCheck() { allIndices, randomNegativeLong(), Map.of(), + Settings.EMPTY, randomBoolean(), false, false, @@ -331,6 +333,7 @@ public void testOldSystemDataStreamIgnored() { allIndices, randomNegativeLong(), Map.of(), + Settings.EMPTY, true, false, true, diff --git a/x-pack/plugin/deprecation/src/test/java/org/elasticsearch/xpack/deprecation/IndexDeprecationCheckerTests.java b/x-pack/plugin/deprecation/src/test/java/org/elasticsearch/xpack/deprecation/IndexDeprecationCheckerTests.java index 5dd1304890f22..fc69bf1267008 100644 --- a/x-pack/plugin/deprecation/src/test/java/org/elasticsearch/xpack/deprecation/IndexDeprecationCheckerTests.java +++ b/x-pack/plugin/deprecation/src/test/java/org/elasticsearch/xpack/deprecation/IndexDeprecationCheckerTests.java @@ -216,6 +216,7 @@ public void testOldIndicesCheckDataStreamIndex() { List.of(indexMetadata.getIndex()), randomNegativeLong(), Map.of(), + Settings.EMPTY, randomBoolean(), false, false, From cf2b2c28ae18b27eed82d3e14756359c5d618399 Mon Sep 17 00:00:00 2001 From: Keith Massey Date: Wed, 16 Apr 2025 13:48:44 -0500 Subject: [PATCH 02/13] removing unused method --- .../org/elasticsearch/cluster/metadata/DataStream.java | 8 -------- 1 file changed, 8 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/DataStream.java b/server/src/main/java/org/elasticsearch/cluster/metadata/DataStream.java index 85aa16afc1e4b..574b1c86c61d1 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/DataStream.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/DataStream.java @@ -348,14 +348,6 @@ public ComposableIndexTemplate getEffectiveIndexTemplate(ProjectMetadata project return mergeSettingsIntoTemplate(lookupTemplateForDataStream(name, projectMetadata), settings); } - public static ComposableIndexTemplate getEffectiveIndexTemplate( - String dataStreamName, - ProjectMetadata projectMetadata, - Settings settingsOverrides - ) { - return mergeSettingsIntoTemplate(lookupTemplateForDataStream(dataStreamName, projectMetadata), settingsOverrides); - } - public Settings getEffectiveSettings(ProjectMetadata projectMetadata) { ComposableIndexTemplate template = getMatchingIndexTemplate(projectMetadata); return mergeSettings(template.template() == null ? Settings.EMPTY : template.template().settings(), settings); From bec18b31cb4c8643699fb4dab841dbc224f2fc55 Mon Sep 17 00:00:00 2001 From: Keith Massey Date: Wed, 16 Apr 2025 14:44:17 -0500 Subject: [PATCH 03/13] fixing ProjectMetadataTests --- .../org/elasticsearch/cluster/metadata/ProjectMetadataTests.java | 1 + 1 file changed, 1 insertion(+) diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/ProjectMetadataTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/ProjectMetadataTests.java index 3533ebfd7399c..0a925dd0eb542 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/ProjectMetadataTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/ProjectMetadataTests.java @@ -317,6 +317,7 @@ public void testToXContent() throws IOException { "replicated": false, "system": false, "allow_custom_routing": false, + "settings" : { }, "failure_rollover_on_write": false, "rollover_on_write": false } From 9cb64f2778b48c12367d0518e0828bc55e14e52d Mon Sep 17 00:00:00 2001 From: Keith Massey Date: Wed, 16 Apr 2025 15:04:14 -0500 Subject: [PATCH 04/13] removing invalid MetadataTests tests --- .../cluster/metadata/MetadataTests.java | 32 ------------------- 1 file changed, 32 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataTests.java index ef9483165ce54..dd9818fcd484e 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataTests.java @@ -1692,38 +1692,6 @@ public void testMultiProjectSerialization() throws IOException { } } - public void testMetadataSerializationPreMultiProject() throws IOException { - final Metadata orig = randomMetadata(); - TransportVersion version = TransportVersionUtils.getPreviousVersion(TransportVersions.MULTI_PROJECT); - final BytesStreamOutput out = new BytesStreamOutput(); - out.setTransportVersion(version); - orig.writeTo(out); - NamedWriteableRegistry namedWriteableRegistry = new NamedWriteableRegistry(ClusterModule.getNamedWriteables()); - final StreamInput input = out.bytes().streamInput(); - input.setTransportVersion(version); - final Metadata fromStreamMeta = Metadata.readFrom(new NamedWriteableAwareStreamInput(input, namedWriteableRegistry)); - assertTrue(Metadata.isGlobalStateEquals(orig, fromStreamMeta)); - } - - public void testDiffSerializationPreMultiProject() throws IOException { - final Metadata meta1 = randomMetadata(1); - final Metadata meta2 = randomMetadata(2); - TransportVersion version = TransportVersionUtils.getPreviousVersion(TransportVersions.MULTI_PROJECT); - final Diff diff = meta2.diff(meta1); - - final BytesStreamOutput out = new BytesStreamOutput(); - out.setTransportVersion(version); - diff.writeTo(out); - - NamedWriteableRegistry namedWriteableRegistry = new NamedWriteableRegistry(ClusterModule.getNamedWriteables()); - final StreamInput input = out.bytes().streamInput(); - input.setTransportVersion(version); - final Diff read = Metadata.readDiffFrom(new NamedWriteableAwareStreamInput(input, namedWriteableRegistry)); - - final Metadata applied = read.apply(meta1); - assertTrue(Metadata.isGlobalStateEquals(meta2, applied)); - } - public void testGetNonExistingProjectThrows() { final List projects = IntStream.range(0, between(1, 3)) .mapToObj(i -> randomProject(ProjectId.fromId("p_" + i), between(0, 5))) From 781f9c673790837b11944537a1fb8b53db918e04 Mon Sep 17 00:00:00 2001 From: elasticsearchmachine Date: Wed, 16 Apr 2025 20:12:53 +0000 Subject: [PATCH 05/13] [CI] Auto commit changes from spotless --- .../java/org/elasticsearch/cluster/metadata/MetadataTests.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataTests.java index dd9818fcd484e..612c387af856c 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataTests.java @@ -11,7 +11,6 @@ import org.elasticsearch.ResourceNotFoundException; import org.elasticsearch.TransportVersion; -import org.elasticsearch.TransportVersions; import org.elasticsearch.action.admin.indices.alias.get.GetAliasesRequest; import org.elasticsearch.cluster.ClusterModule; import org.elasticsearch.cluster.ClusterState; @@ -27,7 +26,6 @@ import org.elasticsearch.common.io.stream.BytesStreamOutput; import org.elasticsearch.common.io.stream.NamedWriteableAwareStreamInput; import org.elasticsearch.common.io.stream.NamedWriteableRegistry; -import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.settings.ClusterSettings; import org.elasticsearch.common.settings.Setting; @@ -61,7 +59,6 @@ import org.elasticsearch.plugins.MapperPlugin; import org.elasticsearch.test.AbstractChunkedSerializingTestCase; import org.elasticsearch.test.ESTestCase; -import org.elasticsearch.test.TransportVersionUtils; import org.elasticsearch.test.index.IndexVersionUtils; import org.elasticsearch.test.rest.ObjectPath; import org.elasticsearch.threadpool.ThreadPool; From 7432824b50ad5a620862e876abcaf6e992d68585 Mon Sep 17 00:00:00 2001 From: Keith Massey Date: Wed, 16 Apr 2025 15:45:06 -0500 Subject: [PATCH 06/13] adding a unit test --- .../cluster/metadata/DataStream.java | 2 +- .../cluster/metadata/DataStreamTests.java | 68 +++++++++++++++++++ 2 files changed, 69 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/DataStream.java b/server/src/main/java/org/elasticsearch/cluster/metadata/DataStream.java index 574b1c86c61d1..a241251bf6dd6 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/DataStream.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/DataStream.java @@ -345,7 +345,7 @@ public boolean rolloverOnWrite() { } public ComposableIndexTemplate getEffectiveIndexTemplate(ProjectMetadata projectMetadata) { - return mergeSettingsIntoTemplate(lookupTemplateForDataStream(name, projectMetadata), settings); + return mergeSettingsIntoTemplate(getMatchingIndexTemplate(projectMetadata), settings); } public Settings getEffectiveSettings(ProjectMetadata projectMetadata) { diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/DataStreamTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/DataStreamTests.java index e836277d21dd3..06ae7577da464 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/DataStreamTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/DataStreamTests.java @@ -2778,6 +2778,74 @@ public void testGetEffectiveIndexTemplate() { } } + public void testMergeSettingsIntoTemplate() { + { + // We only have settings from the template, so the effective template will just be the original template + DataStream dataStream = createDataStream(Settings.EMPTY); + Settings templateSettings = randomSettings(); + Template.Builder templateBuilder = Template.builder().settings(templateSettings).mappings(randomMappings()); + ComposableIndexTemplate indexTemplate = ComposableIndexTemplate.builder() + .indexPatterns(List.of(dataStream.getName())) + .dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate()) + .template(templateBuilder) + .build(); + assertThat(DataStream.mergeSettingsIntoTemplate(indexTemplate, Settings.EMPTY), equalTo(indexTemplate)); + } + { + // We only have settings from the data stream, so we expect to get only those back in the effective template + Settings dataStreamSettings = randomSettings(); + String dataStreamName = randomAlphaOfLength(10).toLowerCase(Locale.ROOT); + Settings templateSettings = Settings.EMPTY; + CompressedXContent templateMappings = randomMappings(); + Template.Builder templateBuilder = Template.builder().settings(templateSettings).mappings(templateMappings); + ComposableIndexTemplate indexTemplate = ComposableIndexTemplate.builder() + .indexPatterns(List.of(dataStreamName)) + .dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate()) + .template(templateBuilder) + .build(); + Template.Builder expectedTemplateBuilder = Template.builder().settings(dataStreamSettings).mappings(templateMappings); + ComposableIndexTemplate expectedEffectiveTemplate = ComposableIndexTemplate.builder() + .indexPatterns(List.of(dataStreamName)) + .dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate()) + .template(expectedTemplateBuilder) + .build(); + assertThat(DataStream.mergeSettingsIntoTemplate(indexTemplate, dataStreamSettings), equalTo(expectedEffectiveTemplate)); + } + { + // Here we have settings from both the template and the data stream, so we expect the data stream settings to take precedence + Settings dataStreamSettings = Settings.builder() + .put("index.setting1", "dataStreamValue") + .put("index.setting2", "dataStreamValue") + .put("index.setting3", (String) null) // This one gets removed from the effective settings + .build(); + String dataStreamName = randomAlphaOfLength(10).toLowerCase(Locale.ROOT); + Settings templateSettings = Settings.builder() + .put("index.setting1", "templateValue") + .put("index.setting3", "templateValue") + .put("index.setting4", "templateValue") + .build(); + CompressedXContent templateMappings = randomMappings(); + Template.Builder templateBuilder = Template.builder().settings(templateSettings).mappings(templateMappings); + ComposableIndexTemplate indexTemplate = ComposableIndexTemplate.builder() + .indexPatterns(List.of(dataStreamName)) + .dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate()) + .template(templateBuilder) + .build(); + Settings mergedSettings = Settings.builder() + .put("index.setting1", "dataStreamValue") + .put("index.setting2", "dataStreamValue") + .put("index.setting4", "templateValue") + .build(); + Template.Builder expectedTemplateBuilder = Template.builder().settings(mergedSettings).mappings(templateMappings); + ComposableIndexTemplate expectedEffectiveTemplate = ComposableIndexTemplate.builder() + .indexPatterns(List.of(dataStreamName)) + .dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate()) + .template(expectedTemplateBuilder) + .build(); + assertThat(DataStream.mergeSettingsIntoTemplate(indexTemplate, dataStreamSettings), equalTo(expectedEffectiveTemplate)); + } + } + private DataStream createDataStream(Settings settings) { DataStream dataStream = createTestInstance(); return dataStream.copy().setSettings(settings).build(); From c7ffd3e30953010af5d5a749bc9e96df3583a0fb Mon Sep 17 00:00:00 2001 From: Keith Massey Date: Thu, 17 Apr 2025 11:12:53 -0500 Subject: [PATCH 07/13] Adding back a constructor that serverless depends on --- .../cluster/metadata/DataStream.java | 36 +++++++++++++++++++ ...StreamLifecycleUsageTransportActionIT.java | 1 - .../DataStreamDeprecationCheckerTests.java | 3 -- .../IndexDeprecationCheckerTests.java | 1 - 4 files changed, 36 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/DataStream.java b/server/src/main/java/org/elasticsearch/cluster/metadata/DataStream.java index a241251bf6dd6..bba1f52b43c38 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/DataStream.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/DataStream.java @@ -135,6 +135,42 @@ public static boolean isFailureStoreFeatureFlagEnabled() { private final DataStreamIndices backingIndices; private final DataStreamIndices failureIndices; + // visible for testing + public DataStream( + String name, + List indices, + long generation, + Map metadata, + boolean hidden, + boolean replicated, + boolean system, + boolean allowCustomRouting, + IndexMode indexMode, + DataStreamLifecycle lifecycle, + @Nullable DataStreamOptions dataStreamOptions, + List failureIndices, + boolean rolloverOnWrite, + @Nullable DataStreamAutoShardingEvent autoShardingEvent + ) { + this( + name, + indices, + generation, + metadata, + Settings.EMPTY, + hidden, + replicated, + system, + allowCustomRouting, + indexMode, + lifecycle, + dataStreamOptions, + failureIndices, + rolloverOnWrite, + autoShardingEvent + ); + } + // visible for testing public DataStream( String name, diff --git a/x-pack/plugin/core/src/internalClusterTest/java/org/elasticsearch/xpack/core/action/DataStreamLifecycleUsageTransportActionIT.java b/x-pack/plugin/core/src/internalClusterTest/java/org/elasticsearch/xpack/core/action/DataStreamLifecycleUsageTransportActionIT.java index e7724bf965d58..24a59ad1c6393 100644 --- a/x-pack/plugin/core/src/internalClusterTest/java/org/elasticsearch/xpack/core/action/DataStreamLifecycleUsageTransportActionIT.java +++ b/x-pack/plugin/core/src/internalClusterTest/java/org/elasticsearch/xpack/core/action/DataStreamLifecycleUsageTransportActionIT.java @@ -174,7 +174,6 @@ public void testAction() throws Exception { indices, randomLongBetween(0, 1000), Map.of(), - Settings.EMPTY, systemDataStream || randomBoolean(), replicated, systemDataStream, diff --git a/x-pack/plugin/deprecation/src/test/java/org/elasticsearch/xpack/deprecation/DataStreamDeprecationCheckerTests.java b/x-pack/plugin/deprecation/src/test/java/org/elasticsearch/xpack/deprecation/DataStreamDeprecationCheckerTests.java index 7eb1a2c666afa..e0dfaef605af7 100644 --- a/x-pack/plugin/deprecation/src/test/java/org/elasticsearch/xpack/deprecation/DataStreamDeprecationCheckerTests.java +++ b/x-pack/plugin/deprecation/src/test/java/org/elasticsearch/xpack/deprecation/DataStreamDeprecationCheckerTests.java @@ -174,7 +174,6 @@ private DataStream createTestDataStream( allIndices, randomNegativeLong(), Map.of(), - Settings.EMPTY, randomBoolean(), false, false, @@ -265,7 +264,6 @@ public void testOldIndicesIgnoredWarningCheck() { allIndices, randomNegativeLong(), Map.of(), - Settings.EMPTY, randomBoolean(), false, false, @@ -333,7 +331,6 @@ public void testOldSystemDataStreamIgnored() { allIndices, randomNegativeLong(), Map.of(), - Settings.EMPTY, true, false, true, diff --git a/x-pack/plugin/deprecation/src/test/java/org/elasticsearch/xpack/deprecation/IndexDeprecationCheckerTests.java b/x-pack/plugin/deprecation/src/test/java/org/elasticsearch/xpack/deprecation/IndexDeprecationCheckerTests.java index fc69bf1267008..5dd1304890f22 100644 --- a/x-pack/plugin/deprecation/src/test/java/org/elasticsearch/xpack/deprecation/IndexDeprecationCheckerTests.java +++ b/x-pack/plugin/deprecation/src/test/java/org/elasticsearch/xpack/deprecation/IndexDeprecationCheckerTests.java @@ -216,7 +216,6 @@ public void testOldIndicesCheckDataStreamIndex() { List.of(indexMetadata.getIndex()), randomNegativeLong(), Map.of(), - Settings.EMPTY, randomBoolean(), false, false, From 9d09df7aa19e26048b86c087b57ce3ac032292b5 Mon Sep 17 00:00:00 2001 From: Keith Massey Date: Fri, 18 Apr 2025 15:37:53 -0500 Subject: [PATCH 08/13] making getEffectiveSettings more robust --- .../cluster/metadata/DataStream.java | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/DataStream.java b/server/src/main/java/org/elasticsearch/cluster/metadata/DataStream.java index bba1f52b43c38..b8980d3cd8d5a 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/DataStream.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/DataStream.java @@ -398,16 +398,25 @@ public static ComposableIndexTemplate mergeSettingsIntoTemplate(ComposableIndexT return template; } ComposableIndexTemplate.Builder mergedIndexTemplateBuilder = template.toBuilder(); - assert template.template() != null : "Template is unexpectedly null"; - Template.Builder mergedTemplateBuilder = Template.builder(template.template()); - mergedTemplateBuilder.settings(mergeSettings(template.template().settings(), settings)); + Template.Builder mergedTemplateBuilder; + Settings templateSettings; + if (template.template() == null) { + mergedTemplateBuilder = Template.builder(); + templateSettings = null; + } else { + mergedTemplateBuilder = Template.builder(template.template()); + templateSettings = template.template().settings(); + } + mergedTemplateBuilder.settings(mergeSettings(templateSettings, settings)); mergedIndexTemplateBuilder.template(mergedTemplateBuilder); return mergedIndexTemplateBuilder.build(); } private static Settings mergeSettings(Settings originalSettings, Settings newSettings) { - if (Settings.EMPTY.equals(newSettings)) { + if (newSettings == null || Settings.EMPTY.equals(newSettings)) { return Objects.requireNonNullElse(originalSettings, Settings.EMPTY); + } else if (originalSettings == null || Settings.EMPTY.equals(originalSettings)) { + return newSettings; } Settings.Builder settingsBuilder = Settings.builder().put(originalSettings).put(newSettings); for (String settingName : new HashSet<>(settingsBuilder.keys())) { From a39ffbc7d988e4f46b8b36b2b9171d1d9b51c348 Mon Sep 17 00:00:00 2001 From: Keith Massey Date: Tue, 22 Apr 2025 13:37:10 -0500 Subject: [PATCH 09/13] moving merge methods out of DataStream --- .../metadata/ComposableIndexTemplate.java | 28 ++++++++ .../cluster/metadata/DataStream.java | 40 +---------- .../common/settings/Settings.java | 18 +++++ .../ComposableIndexTemplateTests.java | 64 +++++++++++++++++ .../cluster/metadata/DataStreamTests.java | 68 ------------------- .../common/settings/SettingsTests.java | 54 +++++++++++++++ 6 files changed, 167 insertions(+), 105 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/ComposableIndexTemplate.java b/server/src/main/java/org/elasticsearch/cluster/metadata/ComposableIndexTemplate.java index cd85779fc99b3..bf8f44037860a 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/ComposableIndexTemplate.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/ComposableIndexTemplate.java @@ -18,6 +18,7 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.core.Nullable; import org.elasticsearch.index.mapper.DataStreamTimestampFieldMapper; import org.elasticsearch.index.mapper.MapperService; @@ -309,6 +310,33 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params, @Nulla return builder; } + /* + * Merges the given settings into the settings in this ComposableIndexTemplate. Any null values in the + * given settings are removed from the settings in the returned ComposableIndexTemplate. If this + * ComposableIndexTemplate has no settings, the given settings are the only ones in the returned template + * (with any null values removed). If this ComposableIndexTemplate has no template, an empty template with + * those settings is created. If the given settings are null or empty, this ComposableIndexTemplate is just + * returned unchanged. This method never changes this object. + */ + public ComposableIndexTemplate mergeSettings(Settings settings) { + if (settings == null || Settings.EMPTY.equals(settings)) { + return this; + } + ComposableIndexTemplate.Builder mergedIndexTemplateBuilder = this.toBuilder(); + Template.Builder mergedTemplateBuilder; + Settings templateSettings; + if (this.template() == null) { + mergedTemplateBuilder = Template.builder(); + templateSettings = null; + } else { + mergedTemplateBuilder = Template.builder(this.template()); + templateSettings = this.template().settings(); + } + mergedTemplateBuilder.settings(templateSettings == null ? settings : templateSettings.merge(settings)); + mergedIndexTemplateBuilder.template(mergedTemplateBuilder); + return mergedIndexTemplateBuilder.build(); + } + @Override public int hashCode() { return Objects.hash( diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/DataStream.java b/server/src/main/java/org/elasticsearch/cluster/metadata/DataStream.java index 45f31aeccd2e4..261962de96b28 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/DataStream.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/DataStream.java @@ -58,7 +58,6 @@ import java.util.ArrayList; import java.util.Comparator; import java.util.HashMap; -import java.util.HashSet; import java.util.List; import java.util.Locale; import java.util.Map; @@ -377,52 +376,19 @@ public boolean rolloverOnWrite() { } public ComposableIndexTemplate getEffectiveIndexTemplate(ProjectMetadata projectMetadata) { - return mergeSettingsIntoTemplate(getMatchingIndexTemplate(projectMetadata), settings); + return getMatchingIndexTemplate(projectMetadata).mergeSettings(settings); } public Settings getEffectiveSettings(ProjectMetadata projectMetadata) { ComposableIndexTemplate template = getMatchingIndexTemplate(projectMetadata); - return mergeSettings(template.template() == null ? Settings.EMPTY : template.template().settings(), settings); + Settings templateSettings = template.template() == null ? Settings.EMPTY : template.template().settings(); + return templateSettings.merge(settings); } private ComposableIndexTemplate getMatchingIndexTemplate(ProjectMetadata projectMetadata) { return lookupTemplateForDataStream(name, projectMetadata); } - public static ComposableIndexTemplate mergeSettingsIntoTemplate(ComposableIndexTemplate template, Settings settings) { - if (Settings.EMPTY.equals(settings)) { - return template; - } - ComposableIndexTemplate.Builder mergedIndexTemplateBuilder = template.toBuilder(); - Template.Builder mergedTemplateBuilder; - Settings templateSettings; - if (template.template() == null) { - mergedTemplateBuilder = Template.builder(); - templateSettings = null; - } else { - mergedTemplateBuilder = Template.builder(template.template()); - templateSettings = template.template().settings(); - } - mergedTemplateBuilder.settings(mergeSettings(templateSettings, settings)); - mergedIndexTemplateBuilder.template(mergedTemplateBuilder); - return mergedIndexTemplateBuilder.build(); - } - - private static Settings mergeSettings(Settings originalSettings, Settings newSettings) { - if (newSettings == null || Settings.EMPTY.equals(newSettings)) { - return Objects.requireNonNullElse(originalSettings, Settings.EMPTY); - } else if (originalSettings == null || Settings.EMPTY.equals(originalSettings)) { - return newSettings; - } - Settings.Builder settingsBuilder = Settings.builder().put(originalSettings).put(newSettings); - for (String settingName : new HashSet<>(settingsBuilder.keys())) { - if (settingsBuilder.get(settingName) == null) { - settingsBuilder.remove(settingName); - } - } - return settingsBuilder.build(); - } - /** * We define that a data stream is considered internal either if it is a system index or if * its name starts with a dot. diff --git a/server/src/main/java/org/elasticsearch/common/settings/Settings.java b/server/src/main/java/org/elasticsearch/common/settings/Settings.java index 4ceee12c3bc5f..c8ba105c6433b 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/Settings.java +++ b/server/src/main/java/org/elasticsearch/common/settings/Settings.java @@ -876,6 +876,24 @@ public Set keySet() { return newKeySet; } + /* + * This method merges the given newSettings into this Settings, returning either a new Settings object or + * this if the newSettings are null or empty. If any values are null in newSettings, those keys are removed + * from the returned object. + */ + public Settings merge(Settings newSettings) { + if (newSettings == null || Settings.EMPTY.equals(newSettings)) { + return this; + } + Settings.Builder settingsBuilder = Settings.builder().put(this).put(newSettings); + for (String settingName : new HashSet<>(settingsBuilder.keys())) { + if (settingsBuilder.get(settingName) == null) { + settingsBuilder.remove(settingName); + } + } + return settingsBuilder.build(); + } + /** * A builder allowing to put different settings and then {@link #build()} an immutable * settings implementation. Use {@link Settings#builder()} in order to diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/ComposableIndexTemplateTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/ComposableIndexTemplateTests.java index 4bfde764fa3b1..85425197ecfc7 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/ComposableIndexTemplateTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/ComposableIndexTemplateTests.java @@ -26,6 +26,7 @@ import java.io.IOException; import java.util.Collections; import java.util.List; +import java.util.Locale; import java.util.Map; import static org.elasticsearch.cluster.metadata.DataStream.TIMESTAMP_FIELD_NAME; @@ -277,4 +278,67 @@ public void testBuilderRoundtrip() { assertEquals(template.template(), Template.builder(template.template()).build()); } } + + public void testMergeSettings() { + { + // We only have settings from the template, so the effective template will just be the original template + Settings templateSettings = randomSettings(); + Template.Builder templateBuilder = Template.builder().settings(templateSettings).mappings(randomMappings(null)); + ComposableIndexTemplate indexTemplate = randomInstance(); + assertThat(indexTemplate.mergeSettings(Settings.EMPTY), equalTo(indexTemplate)); + } + { + // We only have settings from the data stream, so we expect to get only those back in the effective template + Settings dataStreamSettings = randomSettings(); + String dataStreamName = randomAlphaOfLength(10).toLowerCase(Locale.ROOT); + Settings templateSettings = Settings.EMPTY; + CompressedXContent templateMappings = randomMappings(randomDataStreamTemplate()); + Template.Builder templateBuilder = Template.builder().settings(templateSettings).mappings(templateMappings); + ComposableIndexTemplate indexTemplate = ComposableIndexTemplate.builder() + .indexPatterns(List.of(dataStreamName)) + .dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate()) + .template(templateBuilder) + .build(); + Template.Builder expectedTemplateBuilder = Template.builder().settings(dataStreamSettings).mappings(templateMappings); + ComposableIndexTemplate expectedEffectiveTemplate = ComposableIndexTemplate.builder() + .indexPatterns(List.of(dataStreamName)) + .dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate()) + .template(expectedTemplateBuilder) + .build(); + assertThat(indexTemplate.mergeSettings(dataStreamSettings), equalTo(expectedEffectiveTemplate)); + } + { + // Here we have settings from both the template and the data stream, so we expect the data stream settings to take precedence + Settings dataStreamSettings = Settings.builder() + .put("index.setting1", "dataStreamValue") + .put("index.setting2", "dataStreamValue") + .put("index.setting3", (String) null) // This one gets removed from the effective settings + .build(); + String dataStreamName = randomAlphaOfLength(10).toLowerCase(Locale.ROOT); + Settings templateSettings = Settings.builder() + .put("index.setting1", "templateValue") + .put("index.setting3", "templateValue") + .put("index.setting4", "templateValue") + .build(); + CompressedXContent templateMappings = randomMappings(randomDataStreamTemplate()); + Template.Builder templateBuilder = Template.builder().settings(templateSettings).mappings(templateMappings); + ComposableIndexTemplate indexTemplate = ComposableIndexTemplate.builder() + .indexPatterns(List.of(dataStreamName)) + .dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate()) + .template(templateBuilder) + .build(); + Settings mergedSettings = Settings.builder() + .put("index.setting1", "dataStreamValue") + .put("index.setting2", "dataStreamValue") + .put("index.setting4", "templateValue") + .build(); + Template.Builder expectedTemplateBuilder = Template.builder().settings(mergedSettings).mappings(templateMappings); + ComposableIndexTemplate expectedEffectiveTemplate = ComposableIndexTemplate.builder() + .indexPatterns(List.of(dataStreamName)) + .dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate()) + .template(expectedTemplateBuilder) + .build(); + assertThat(indexTemplate.mergeSettings(dataStreamSettings), equalTo(expectedEffectiveTemplate)); + } + } } diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/DataStreamTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/DataStreamTests.java index 06ae7577da464..e836277d21dd3 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/DataStreamTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/DataStreamTests.java @@ -2778,74 +2778,6 @@ public void testGetEffectiveIndexTemplate() { } } - public void testMergeSettingsIntoTemplate() { - { - // We only have settings from the template, so the effective template will just be the original template - DataStream dataStream = createDataStream(Settings.EMPTY); - Settings templateSettings = randomSettings(); - Template.Builder templateBuilder = Template.builder().settings(templateSettings).mappings(randomMappings()); - ComposableIndexTemplate indexTemplate = ComposableIndexTemplate.builder() - .indexPatterns(List.of(dataStream.getName())) - .dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate()) - .template(templateBuilder) - .build(); - assertThat(DataStream.mergeSettingsIntoTemplate(indexTemplate, Settings.EMPTY), equalTo(indexTemplate)); - } - { - // We only have settings from the data stream, so we expect to get only those back in the effective template - Settings dataStreamSettings = randomSettings(); - String dataStreamName = randomAlphaOfLength(10).toLowerCase(Locale.ROOT); - Settings templateSettings = Settings.EMPTY; - CompressedXContent templateMappings = randomMappings(); - Template.Builder templateBuilder = Template.builder().settings(templateSettings).mappings(templateMappings); - ComposableIndexTemplate indexTemplate = ComposableIndexTemplate.builder() - .indexPatterns(List.of(dataStreamName)) - .dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate()) - .template(templateBuilder) - .build(); - Template.Builder expectedTemplateBuilder = Template.builder().settings(dataStreamSettings).mappings(templateMappings); - ComposableIndexTemplate expectedEffectiveTemplate = ComposableIndexTemplate.builder() - .indexPatterns(List.of(dataStreamName)) - .dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate()) - .template(expectedTemplateBuilder) - .build(); - assertThat(DataStream.mergeSettingsIntoTemplate(indexTemplate, dataStreamSettings), equalTo(expectedEffectiveTemplate)); - } - { - // Here we have settings from both the template and the data stream, so we expect the data stream settings to take precedence - Settings dataStreamSettings = Settings.builder() - .put("index.setting1", "dataStreamValue") - .put("index.setting2", "dataStreamValue") - .put("index.setting3", (String) null) // This one gets removed from the effective settings - .build(); - String dataStreamName = randomAlphaOfLength(10).toLowerCase(Locale.ROOT); - Settings templateSettings = Settings.builder() - .put("index.setting1", "templateValue") - .put("index.setting3", "templateValue") - .put("index.setting4", "templateValue") - .build(); - CompressedXContent templateMappings = randomMappings(); - Template.Builder templateBuilder = Template.builder().settings(templateSettings).mappings(templateMappings); - ComposableIndexTemplate indexTemplate = ComposableIndexTemplate.builder() - .indexPatterns(List.of(dataStreamName)) - .dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate()) - .template(templateBuilder) - .build(); - Settings mergedSettings = Settings.builder() - .put("index.setting1", "dataStreamValue") - .put("index.setting2", "dataStreamValue") - .put("index.setting4", "templateValue") - .build(); - Template.Builder expectedTemplateBuilder = Template.builder().settings(mergedSettings).mappings(templateMappings); - ComposableIndexTemplate expectedEffectiveTemplate = ComposableIndexTemplate.builder() - .indexPatterns(List.of(dataStreamName)) - .dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate()) - .template(expectedTemplateBuilder) - .build(); - assertThat(DataStream.mergeSettingsIntoTemplate(indexTemplate, dataStreamSettings), equalTo(expectedEffectiveTemplate)); - } - } - private DataStream createDataStream(Settings settings) { DataStream dataStream = createTestInstance(); return dataStream.copy().setSettings(settings).build(); diff --git a/server/src/test/java/org/elasticsearch/common/settings/SettingsTests.java b/server/src/test/java/org/elasticsearch/common/settings/SettingsTests.java index 2e87082b581d5..097874671d30a 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/SettingsTests.java +++ b/server/src/test/java/org/elasticsearch/common/settings/SettingsTests.java @@ -722,4 +722,58 @@ public void testGlobValues() throws IOException { assertThat(values, contains("1")); } + public void testMerge() { + { + assertThat(Settings.EMPTY.merge(null), equalTo(Settings.EMPTY)); + assertThat(Settings.EMPTY.merge(Settings.EMPTY), equalTo(Settings.EMPTY)); + } + { + Settings.Builder builder = Settings.builder(); + for (int i = 1; i < randomInt(100); i++) { + builder.put(randomAlphanumericOfLength(20), randomAlphanumericOfLength(50)); + } + Settings settings = builder.build(); + assertThat(settings.merge(null), equalTo(settings)); + assertThat(settings.merge(Settings.EMPTY), equalTo(settings)); + } + { + Settings.Builder builder = Settings.builder(); + for (int i = 1; i < randomInt(100); i++) { + builder.put(randomAlphanumericOfLength(20), randomAlphanumericOfLength(50)); + } + Settings newSettings = builder.build(); + assertThat(Settings.EMPTY.merge(newSettings), equalTo(newSettings)); + } + { + Settings settings = Settings.builder() + .put("index.setting1", "templateValue") + .put("index.setting3", "templateValue") + .put("index.setting4", "templateValue") + .build(); + Settings newSettings = Settings.builder() + .put("index.setting1", "dataStreamValue") + .put("index.setting2", "dataStreamValue") + .put("index.setting3", (String) null) // This one gets removed from the effective settings + .build(); + Settings mergedSettings = Settings.builder() + .put("index.setting1", "dataStreamValue") + .put("index.setting2", "dataStreamValue") + .put("index.setting4", "templateValue") + .build(); + assertThat(settings.merge(newSettings), equalTo(mergedSettings)); + } + { + Settings newSettings = Settings.builder() + .put("index.setting1", "dataStreamValue") + .put("index.setting2", "dataStreamValue") + .put("index.setting3", (String) null) // This one gets removed from the effective settings + .build(); + Settings mergedSettings = Settings.builder() + .put("index.setting1", "dataStreamValue") + .put("index.setting2", "dataStreamValue") + .build(); + assertThat(Settings.EMPTY.merge(newSettings), equalTo(mergedSettings)); + } + } + } From d85b4d78b1bf41676aab4efae41a3361051b7eb9 Mon Sep 17 00:00:00 2001 From: Keith Massey Date: Tue, 22 Apr 2025 17:36:15 -0500 Subject: [PATCH 10/13] updates to Settings::merge from code review --- .../common/settings/Settings.java | 18 ++-- .../common/settings/SettingsTests.java | 101 +++++++++--------- 2 files changed, 63 insertions(+), 56 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/settings/Settings.java b/server/src/main/java/org/elasticsearch/common/settings/Settings.java index c8ba105c6433b..8eb344cb4b13c 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/Settings.java +++ b/server/src/main/java/org/elasticsearch/common/settings/Settings.java @@ -882,16 +882,22 @@ public Set keySet() { * from the returned object. */ public Settings merge(Settings newSettings) { - if (newSettings == null || Settings.EMPTY.equals(newSettings)) { + Objects.requireNonNull(newSettings); + if (Settings.EMPTY.equals(newSettings)) { return this; } - Settings.Builder settingsBuilder = Settings.builder().put(this).put(newSettings); - for (String settingName : new HashSet<>(settingsBuilder.keys())) { - if (settingsBuilder.get(settingName) == null) { - settingsBuilder.remove(settingName); + Settings.Builder builder = Settings.builder().put(this).put(newSettings); + for (String key : newSettings.keySet()) { + String rawValue = newSettings.get(key); + if (builder.get(key) == null) { + if (rawValue == null) { + builder.remove(key); + } else { + builder.put(key, rawValue); + } } } - return settingsBuilder.build(); + return builder.build(); } /** diff --git a/server/src/test/java/org/elasticsearch/common/settings/SettingsTests.java b/server/src/test/java/org/elasticsearch/common/settings/SettingsTests.java index 097874671d30a..da7876b63db61 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/SettingsTests.java +++ b/server/src/test/java/org/elasticsearch/common/settings/SettingsTests.java @@ -722,58 +722,59 @@ public void testGlobValues() throws IOException { assertThat(values, contains("1")); } - public void testMerge() { - { - assertThat(Settings.EMPTY.merge(null), equalTo(Settings.EMPTY)); - assertThat(Settings.EMPTY.merge(Settings.EMPTY), equalTo(Settings.EMPTY)); - } - { - Settings.Builder builder = Settings.builder(); - for (int i = 1; i < randomInt(100); i++) { - builder.put(randomAlphanumericOfLength(20), randomAlphanumericOfLength(50)); - } - Settings settings = builder.build(); - assertThat(settings.merge(null), equalTo(settings)); - assertThat(settings.merge(Settings.EMPTY), equalTo(settings)); - } - { - Settings.Builder builder = Settings.builder(); - for (int i = 1; i < randomInt(100); i++) { - builder.put(randomAlphanumericOfLength(20), randomAlphanumericOfLength(50)); - } - Settings newSettings = builder.build(); - assertThat(Settings.EMPTY.merge(newSettings), equalTo(newSettings)); - } - { - Settings settings = Settings.builder() - .put("index.setting1", "templateValue") - .put("index.setting3", "templateValue") - .put("index.setting4", "templateValue") - .build(); - Settings newSettings = Settings.builder() - .put("index.setting1", "dataStreamValue") - .put("index.setting2", "dataStreamValue") - .put("index.setting3", (String) null) // This one gets removed from the effective settings - .build(); - Settings mergedSettings = Settings.builder() - .put("index.setting1", "dataStreamValue") - .put("index.setting2", "dataStreamValue") - .put("index.setting4", "templateValue") - .build(); - assertThat(settings.merge(newSettings), equalTo(mergedSettings)); + public void testMergeNullOrEmptySettingsIntoEmptySettings() { + expectThrows(NullPointerException.class, () -> Settings.EMPTY.merge(null)); + assertThat(Settings.EMPTY.merge(Settings.EMPTY), equalTo(Settings.EMPTY)); + } + + public void testMergeEmptySettings() { + Settings.Builder builder = Settings.builder(); + for (int i = 1; i < randomInt(100); i++) { + builder.put(randomAlphanumericOfLength(20), randomAlphanumericOfLength(50)); } - { - Settings newSettings = Settings.builder() - .put("index.setting1", "dataStreamValue") - .put("index.setting2", "dataStreamValue") - .put("index.setting3", (String) null) // This one gets removed from the effective settings - .build(); - Settings mergedSettings = Settings.builder() - .put("index.setting1", "dataStreamValue") - .put("index.setting2", "dataStreamValue") - .build(); - assertThat(Settings.EMPTY.merge(newSettings), equalTo(mergedSettings)); + Settings settings = builder.build(); + assertThat(settings.merge(Settings.EMPTY), equalTo(settings)); + } + + public void testMergeNonEmptySettingsIntoEmptySettings() { + Settings.Builder builder = Settings.builder(); + for (int i = 1; i < randomInt(100); i++) { + builder.put(randomAlphanumericOfLength(20), randomAlphanumericOfLength(50)); } + Settings newSettings = builder.build(); + assertThat(Settings.EMPTY.merge(newSettings), equalTo(newSettings)); + } + + public void testMergeNonEmptySettingsIntoNonEmptySettings() { + Settings settings = Settings.builder() + .put("index.setting1", "templateValue") + .put("index.setting3", "templateValue") + .put("index.setting4", "templateValue") + .build(); + Settings newSettings = Settings.builder() + .put("index.setting1", "dataStreamValue") + .put("index.setting2", "dataStreamValue") + .put("index.setting3", (String) null) // This one gets removed from the effective settings + .build(); + Settings mergedSettings = Settings.builder() + .put("index.setting1", "dataStreamValue") + .put("index.setting2", "dataStreamValue") + .put("index.setting4", "templateValue") + .build(); + assertThat(settings.merge(newSettings), equalTo(mergedSettings)); + } + + public void testMergeNonEmptySettingsWithNullIntoEmptySettings() { + Settings newSettings = Settings.builder() + .put("index.setting1", "dataStreamValue") + .put("index.setting2", "dataStreamValue") + .put("index.setting3", (String) null) // This one gets removed from the effective settings + .build(); + Settings mergedSettings = Settings.builder() + .put("index.setting1", "dataStreamValue") + .put("index.setting2", "dataStreamValue") + .build(); + assertThat(Settings.EMPTY.merge(newSettings), equalTo(mergedSettings)); } } From 5df3d4f102a4ce2fa4bebf319bf6673d53d7a365 Mon Sep 17 00:00:00 2001 From: Keith Massey Date: Tue, 22 Apr 2025 17:46:56 -0500 Subject: [PATCH 11/13] splitting up unit tests for code review --- .../ComposableIndexTemplateTests.java | 120 ++++---- .../cluster/metadata/DataStreamTests.java | 282 +++++++++--------- 2 files changed, 202 insertions(+), 200 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/ComposableIndexTemplateTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/ComposableIndexTemplateTests.java index 85425197ecfc7..24200e4b562c2 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/ComposableIndexTemplateTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/ComposableIndexTemplateTests.java @@ -279,66 +279,66 @@ public void testBuilderRoundtrip() { } } + public void testMergeEmptySettingsIntoTemplateWithNonEmptySettings() { + // We only have settings from the template, so the effective template will just be the original template + Settings templateSettings = randomSettings(); + Template.Builder templateBuilder = Template.builder().settings(templateSettings).mappings(randomMappings(null)); + ComposableIndexTemplate indexTemplate = randomInstance(); + assertThat(indexTemplate.mergeSettings(Settings.EMPTY), equalTo(indexTemplate)); + } + + public void testMergeNonEmptySettingsIntoTemplateWithEmptySettings() { + // We only have settings from the data stream, so we expect to get only those back in the effective template + Settings dataStreamSettings = randomSettings(); + String dataStreamName = randomAlphaOfLength(10).toLowerCase(Locale.ROOT); + Settings templateSettings = Settings.EMPTY; + CompressedXContent templateMappings = randomMappings(randomDataStreamTemplate()); + Template.Builder templateBuilder = Template.builder().settings(templateSettings).mappings(templateMappings); + ComposableIndexTemplate indexTemplate = ComposableIndexTemplate.builder() + .indexPatterns(List.of(dataStreamName)) + .dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate()) + .template(templateBuilder) + .build(); + Template.Builder expectedTemplateBuilder = Template.builder().settings(dataStreamSettings).mappings(templateMappings); + ComposableIndexTemplate expectedEffectiveTemplate = ComposableIndexTemplate.builder() + .indexPatterns(List.of(dataStreamName)) + .dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate()) + .template(expectedTemplateBuilder) + .build(); + assertThat(indexTemplate.mergeSettings(dataStreamSettings), equalTo(expectedEffectiveTemplate)); + } + public void testMergeSettings() { - { - // We only have settings from the template, so the effective template will just be the original template - Settings templateSettings = randomSettings(); - Template.Builder templateBuilder = Template.builder().settings(templateSettings).mappings(randomMappings(null)); - ComposableIndexTemplate indexTemplate = randomInstance(); - assertThat(indexTemplate.mergeSettings(Settings.EMPTY), equalTo(indexTemplate)); - } - { - // We only have settings from the data stream, so we expect to get only those back in the effective template - Settings dataStreamSettings = randomSettings(); - String dataStreamName = randomAlphaOfLength(10).toLowerCase(Locale.ROOT); - Settings templateSettings = Settings.EMPTY; - CompressedXContent templateMappings = randomMappings(randomDataStreamTemplate()); - Template.Builder templateBuilder = Template.builder().settings(templateSettings).mappings(templateMappings); - ComposableIndexTemplate indexTemplate = ComposableIndexTemplate.builder() - .indexPatterns(List.of(dataStreamName)) - .dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate()) - .template(templateBuilder) - .build(); - Template.Builder expectedTemplateBuilder = Template.builder().settings(dataStreamSettings).mappings(templateMappings); - ComposableIndexTemplate expectedEffectiveTemplate = ComposableIndexTemplate.builder() - .indexPatterns(List.of(dataStreamName)) - .dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate()) - .template(expectedTemplateBuilder) - .build(); - assertThat(indexTemplate.mergeSettings(dataStreamSettings), equalTo(expectedEffectiveTemplate)); - } - { - // Here we have settings from both the template and the data stream, so we expect the data stream settings to take precedence - Settings dataStreamSettings = Settings.builder() - .put("index.setting1", "dataStreamValue") - .put("index.setting2", "dataStreamValue") - .put("index.setting3", (String) null) // This one gets removed from the effective settings - .build(); - String dataStreamName = randomAlphaOfLength(10).toLowerCase(Locale.ROOT); - Settings templateSettings = Settings.builder() - .put("index.setting1", "templateValue") - .put("index.setting3", "templateValue") - .put("index.setting4", "templateValue") - .build(); - CompressedXContent templateMappings = randomMappings(randomDataStreamTemplate()); - Template.Builder templateBuilder = Template.builder().settings(templateSettings).mappings(templateMappings); - ComposableIndexTemplate indexTemplate = ComposableIndexTemplate.builder() - .indexPatterns(List.of(dataStreamName)) - .dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate()) - .template(templateBuilder) - .build(); - Settings mergedSettings = Settings.builder() - .put("index.setting1", "dataStreamValue") - .put("index.setting2", "dataStreamValue") - .put("index.setting4", "templateValue") - .build(); - Template.Builder expectedTemplateBuilder = Template.builder().settings(mergedSettings).mappings(templateMappings); - ComposableIndexTemplate expectedEffectiveTemplate = ComposableIndexTemplate.builder() - .indexPatterns(List.of(dataStreamName)) - .dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate()) - .template(expectedTemplateBuilder) - .build(); - assertThat(indexTemplate.mergeSettings(dataStreamSettings), equalTo(expectedEffectiveTemplate)); - } + // Here we have settings from both the template and the data stream, so we expect the data stream settings to take precedence + Settings dataStreamSettings = Settings.builder() + .put("index.setting1", "dataStreamValue") + .put("index.setting2", "dataStreamValue") + .put("index.setting3", (String) null) // This one gets removed from the effective settings + .build(); + String dataStreamName = randomAlphaOfLength(10).toLowerCase(Locale.ROOT); + Settings templateSettings = Settings.builder() + .put("index.setting1", "templateValue") + .put("index.setting3", "templateValue") + .put("index.setting4", "templateValue") + .build(); + CompressedXContent templateMappings = randomMappings(randomDataStreamTemplate()); + Template.Builder templateBuilder = Template.builder().settings(templateSettings).mappings(templateMappings); + ComposableIndexTemplate indexTemplate = ComposableIndexTemplate.builder() + .indexPatterns(List.of(dataStreamName)) + .dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate()) + .template(templateBuilder) + .build(); + Settings mergedSettings = Settings.builder() + .put("index.setting1", "dataStreamValue") + .put("index.setting2", "dataStreamValue") + .put("index.setting4", "templateValue") + .build(); + Template.Builder expectedTemplateBuilder = Template.builder().settings(mergedSettings).mappings(templateMappings); + ComposableIndexTemplate expectedEffectiveTemplate = ComposableIndexTemplate.builder() + .indexPatterns(List.of(dataStreamName)) + .dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate()) + .template(expectedTemplateBuilder) + .build(); + assertThat(indexTemplate.mergeSettings(dataStreamSettings), equalTo(expectedEffectiveTemplate)); } } diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/DataStreamTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/DataStreamTests.java index e836277d21dd3..d5a70863ee361 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/DataStreamTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/DataStreamTests.java @@ -2632,150 +2632,152 @@ public void testIsFailureStoreEffectivelyEnabled_staticHelperMethod() { ); } + public void testGetEffectiveSettingsNoMatchingTemplate() { + // No matching template, so we expect an IllegalArgumentException + DataStream dataStream = createTestInstance(); + ProjectMetadata.Builder projectMetadataBuilder = ProjectMetadata.builder(randomProjectIdOrDefault()); + assertThrows(IllegalArgumentException.class, () -> dataStream.getEffectiveSettings(projectMetadataBuilder.build())); + } + + public void testGetEffectiveSettingsTemplateSettingsOnly() { + // We only have settings from the template, so we expect to get those back + DataStream dataStream = createDataStream(Settings.EMPTY); + Settings templateSettings = randomSettings(); + Template.Builder templateBuilder = Template.builder().settings(templateSettings); + ComposableIndexTemplate indexTemplate = ComposableIndexTemplate.builder() + .indexPatterns(List.of(dataStream.getName())) + .dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate()) + .template(templateBuilder) + .build(); + ProjectMetadata.Builder projectMetadataBuilder = ProjectMetadata.builder(randomProjectIdOrDefault()) + .indexTemplates(Map.of(dataStream.getName(), indexTemplate)); + assertThat(dataStream.getEffectiveSettings(projectMetadataBuilder.build()), equalTo(templateSettings)); + } + + public void testGetEffectiveSettingsDataStreamSettingsOnly() { + // We only have settings from the data stream, so we expect to get those back + Settings dataStreamSettings = randomSettings(); + DataStream dataStream = createDataStream(dataStreamSettings); + Settings templateSettings = Settings.EMPTY; + Template.Builder templateBuilder = Template.builder().settings(templateSettings); + ComposableIndexTemplate indexTemplate = ComposableIndexTemplate.builder() + .indexPatterns(List.of(dataStream.getName())) + .dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate()) + .template(templateBuilder) + .build(); + ProjectMetadata.Builder projectMetadataBuilder = ProjectMetadata.builder(randomProjectIdOrDefault()) + .indexTemplates(Map.of(dataStream.getName(), indexTemplate)); + assertThat(dataStream.getEffectiveSettings(projectMetadataBuilder.build()), equalTo(dataStreamSettings)); + } + public void testGetEffectiveSettings() { - { - // No matching template, so we expect an IllegalArgumentException - DataStream dataStream = createTestInstance(); - ProjectMetadata.Builder projectMetadataBuilder = ProjectMetadata.builder(randomProjectIdOrDefault()); - assertThrows(IllegalArgumentException.class, () -> dataStream.getEffectiveSettings(projectMetadataBuilder.build())); - } - { - // We only have settings from the template, so we expect to get those back - DataStream dataStream = createDataStream(Settings.EMPTY); - Settings templateSettings = randomSettings(); - Template.Builder templateBuilder = Template.builder().settings(templateSettings); - ComposableIndexTemplate indexTemplate = ComposableIndexTemplate.builder() - .indexPatterns(List.of(dataStream.getName())) - .dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate()) - .template(templateBuilder) - .build(); - ProjectMetadata.Builder projectMetadataBuilder = ProjectMetadata.builder(randomProjectIdOrDefault()) - .indexTemplates(Map.of(dataStream.getName(), indexTemplate)); - assertThat(dataStream.getEffectiveSettings(projectMetadataBuilder.build()), equalTo(templateSettings)); - } - { - // We only have settings from the data stream, so we expect to get those back - Settings dataStreamSettings = randomSettings(); - DataStream dataStream = createDataStream(dataStreamSettings); - Settings templateSettings = Settings.EMPTY; - Template.Builder templateBuilder = Template.builder().settings(templateSettings); - ComposableIndexTemplate indexTemplate = ComposableIndexTemplate.builder() - .indexPatterns(List.of(dataStream.getName())) - .dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate()) - .template(templateBuilder) - .build(); - ProjectMetadata.Builder projectMetadataBuilder = ProjectMetadata.builder(randomProjectIdOrDefault()) - .indexTemplates(Map.of(dataStream.getName(), indexTemplate)); - assertThat(dataStream.getEffectiveSettings(projectMetadataBuilder.build()), equalTo(dataStreamSettings)); - } - { - // Here we have settings from both the template and the data stream, so we expect the data stream settings to take precedence - Settings dataStreamSettings = Settings.builder() - .put("index.setting1", "dataStreamValue") - .put("index.setting2", "dataStreamValue") - .put("index.setting3", (String) null) // This one gets removed from the effective settings - .build(); - DataStream dataStream = createDataStream(dataStreamSettings); - Settings templateSettings = Settings.builder() - .put("index.setting1", "templateValue") - .put("index.setting3", "templateValue") - .put("index.setting4", "templateValue") - .build(); - Template.Builder templateBuilder = Template.builder().settings(templateSettings); - ComposableIndexTemplate indexTemplate = ComposableIndexTemplate.builder() - .indexPatterns(List.of(dataStream.getName())) - .dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate()) - .template(templateBuilder) - .build(); - ProjectMetadata.Builder projectMetadataBuilder = ProjectMetadata.builder(randomProjectIdOrDefault()) - .indexTemplates(Map.of(dataStream.getName(), indexTemplate)); - Settings mergedSettings = Settings.builder() - .put("index.setting1", "dataStreamValue") - .put("index.setting2", "dataStreamValue") - .put("index.setting4", "templateValue") - .build(); - assertThat(dataStream.getEffectiveSettings(projectMetadataBuilder.build()), equalTo(mergedSettings)); - } + // Here we have settings from both the template and the data stream, so we expect the data stream settings to take precedence + Settings dataStreamSettings = Settings.builder() + .put("index.setting1", "dataStreamValue") + .put("index.setting2", "dataStreamValue") + .put("index.setting3", (String) null) // This one gets removed from the effective settings + .build(); + DataStream dataStream = createDataStream(dataStreamSettings); + Settings templateSettings = Settings.builder() + .put("index.setting1", "templateValue") + .put("index.setting3", "templateValue") + .put("index.setting4", "templateValue") + .build(); + Template.Builder templateBuilder = Template.builder().settings(templateSettings); + ComposableIndexTemplate indexTemplate = ComposableIndexTemplate.builder() + .indexPatterns(List.of(dataStream.getName())) + .dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate()) + .template(templateBuilder) + .build(); + ProjectMetadata.Builder projectMetadataBuilder = ProjectMetadata.builder(randomProjectIdOrDefault()) + .indexTemplates(Map.of(dataStream.getName(), indexTemplate)); + Settings mergedSettings = Settings.builder() + .put("index.setting1", "dataStreamValue") + .put("index.setting2", "dataStreamValue") + .put("index.setting4", "templateValue") + .build(); + assertThat(dataStream.getEffectiveSettings(projectMetadataBuilder.build()), equalTo(mergedSettings)); + } + + public void testGetEffectiveIndexTemplateNoMatchingTemplate() { + // No matching template, so we expect an IllegalArgumentException + DataStream dataStream = createTestInstance(); + ProjectMetadata.Builder projectMetadataBuilder = ProjectMetadata.builder(randomProjectIdOrDefault()); + assertThrows(IllegalArgumentException.class, () -> dataStream.getEffectiveIndexTemplate(projectMetadataBuilder.build())); + } + + public void testGetEffectiveIndexTemplateTemplateSettingsOnly() { + // We only have settings from the template, so the effective template will just be the original template + DataStream dataStream = createDataStream(Settings.EMPTY); + Settings templateSettings = randomSettings(); + Template.Builder templateBuilder = Template.builder().settings(templateSettings).mappings(randomMappings()); + ComposableIndexTemplate indexTemplate = ComposableIndexTemplate.builder() + .indexPatterns(List.of(dataStream.getName())) + .dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate()) + .template(templateBuilder) + .build(); + ProjectMetadata.Builder projectMetadataBuilder = ProjectMetadata.builder(randomProjectIdOrDefault()) + .indexTemplates(Map.of(dataStream.getName(), indexTemplate)); + assertThat(dataStream.getEffectiveIndexTemplate(projectMetadataBuilder.build()), equalTo(indexTemplate)); + } + + public void testGetEffectiveIndexTemplateDataStreamSettingsOnly() { + // We only have settings from the data stream, so we expect to get only those back in the effective template + Settings dataStreamSettings = randomSettings(); + DataStream dataStream = createDataStream(dataStreamSettings); + Settings templateSettings = Settings.EMPTY; + CompressedXContent templateMappings = randomMappings(); + Template.Builder templateBuilder = Template.builder().settings(templateSettings).mappings(templateMappings); + ComposableIndexTemplate indexTemplate = ComposableIndexTemplate.builder() + .indexPatterns(List.of(dataStream.getName())) + .dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate()) + .template(templateBuilder) + .build(); + ProjectMetadata.Builder projectMetadataBuilder = ProjectMetadata.builder(randomProjectIdOrDefault()) + .indexTemplates(Map.of(dataStream.getName(), indexTemplate)); + Template.Builder expectedTemplateBuilder = Template.builder().settings(dataStreamSettings).mappings(templateMappings); + ComposableIndexTemplate expectedEffectiveTemplate = ComposableIndexTemplate.builder() + .indexPatterns(List.of(dataStream.getName())) + .dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate()) + .template(expectedTemplateBuilder) + .build(); + assertThat(dataStream.getEffectiveIndexTemplate(projectMetadataBuilder.build()), equalTo(expectedEffectiveTemplate)); } public void testGetEffectiveIndexTemplate() { - { - // No matching template, so we expect an IllegalArgumentException - DataStream dataStream = createTestInstance(); - ProjectMetadata.Builder projectMetadataBuilder = ProjectMetadata.builder(randomProjectIdOrDefault()); - assertThrows(IllegalArgumentException.class, () -> dataStream.getEffectiveIndexTemplate(projectMetadataBuilder.build())); - } - { - // We only have settings from the template, so the effective template will just be the original template - DataStream dataStream = createDataStream(Settings.EMPTY); - Settings templateSettings = randomSettings(); - Template.Builder templateBuilder = Template.builder().settings(templateSettings).mappings(randomMappings()); - ComposableIndexTemplate indexTemplate = ComposableIndexTemplate.builder() - .indexPatterns(List.of(dataStream.getName())) - .dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate()) - .template(templateBuilder) - .build(); - ProjectMetadata.Builder projectMetadataBuilder = ProjectMetadata.builder(randomProjectIdOrDefault()) - .indexTemplates(Map.of(dataStream.getName(), indexTemplate)); - assertThat(dataStream.getEffectiveIndexTemplate(projectMetadataBuilder.build()), equalTo(indexTemplate)); - } - { - // We only have settings from the data stream, so we expect to get only those back in the effective template - Settings dataStreamSettings = randomSettings(); - DataStream dataStream = createDataStream(dataStreamSettings); - Settings templateSettings = Settings.EMPTY; - CompressedXContent templateMappings = randomMappings(); - Template.Builder templateBuilder = Template.builder().settings(templateSettings).mappings(templateMappings); - ComposableIndexTemplate indexTemplate = ComposableIndexTemplate.builder() - .indexPatterns(List.of(dataStream.getName())) - .dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate()) - .template(templateBuilder) - .build(); - ProjectMetadata.Builder projectMetadataBuilder = ProjectMetadata.builder(randomProjectIdOrDefault()) - .indexTemplates(Map.of(dataStream.getName(), indexTemplate)); - Template.Builder expectedTemplateBuilder = Template.builder().settings(dataStreamSettings).mappings(templateMappings); - ComposableIndexTemplate expectedEffectiveTemplate = ComposableIndexTemplate.builder() - .indexPatterns(List.of(dataStream.getName())) - .dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate()) - .template(expectedTemplateBuilder) - .build(); - assertThat(dataStream.getEffectiveIndexTemplate(projectMetadataBuilder.build()), equalTo(expectedEffectiveTemplate)); - } - { - // Here we have settings from both the template and the data stream, so we expect the data stream settings to take precedence - Settings dataStreamSettings = Settings.builder() - .put("index.setting1", "dataStreamValue") - .put("index.setting2", "dataStreamValue") - .put("index.setting3", (String) null) // This one gets removed from the effective settings - .build(); - DataStream dataStream = createDataStream(dataStreamSettings); - Settings templateSettings = Settings.builder() - .put("index.setting1", "templateValue") - .put("index.setting3", "templateValue") - .put("index.setting4", "templateValue") - .build(); - CompressedXContent templateMappings = randomMappings(); - Template.Builder templateBuilder = Template.builder().settings(templateSettings).mappings(templateMappings); - ComposableIndexTemplate indexTemplate = ComposableIndexTemplate.builder() - .indexPatterns(List.of(dataStream.getName())) - .dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate()) - .template(templateBuilder) - .build(); - ProjectMetadata.Builder projectMetadataBuilder = ProjectMetadata.builder(randomProjectIdOrDefault()) - .indexTemplates(Map.of(dataStream.getName(), indexTemplate)); - Settings mergedSettings = Settings.builder() - .put("index.setting1", "dataStreamValue") - .put("index.setting2", "dataStreamValue") - .put("index.setting4", "templateValue") - .build(); - Template.Builder expectedTemplateBuilder = Template.builder().settings(mergedSettings).mappings(templateMappings); - ComposableIndexTemplate expectedEffectiveTemplate = ComposableIndexTemplate.builder() - .indexPatterns(List.of(dataStream.getName())) - .dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate()) - .template(expectedTemplateBuilder) - .build(); - assertThat(dataStream.getEffectiveIndexTemplate(projectMetadataBuilder.build()), equalTo(expectedEffectiveTemplate)); - } + // Here we have settings from both the template and the data stream, so we expect the data stream settings to take precedence + Settings dataStreamSettings = Settings.builder() + .put("index.setting1", "dataStreamValue") + .put("index.setting2", "dataStreamValue") + .put("index.setting3", (String) null) // This one gets removed from the effective settings + .build(); + DataStream dataStream = createDataStream(dataStreamSettings); + Settings templateSettings = Settings.builder() + .put("index.setting1", "templateValue") + .put("index.setting3", "templateValue") + .put("index.setting4", "templateValue") + .build(); + CompressedXContent templateMappings = randomMappings(); + Template.Builder templateBuilder = Template.builder().settings(templateSettings).mappings(templateMappings); + ComposableIndexTemplate indexTemplate = ComposableIndexTemplate.builder() + .indexPatterns(List.of(dataStream.getName())) + .dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate()) + .template(templateBuilder) + .build(); + ProjectMetadata.Builder projectMetadataBuilder = ProjectMetadata.builder(randomProjectIdOrDefault()) + .indexTemplates(Map.of(dataStream.getName(), indexTemplate)); + Settings mergedSettings = Settings.builder() + .put("index.setting1", "dataStreamValue") + .put("index.setting2", "dataStreamValue") + .put("index.setting4", "templateValue") + .build(); + Template.Builder expectedTemplateBuilder = Template.builder().settings(mergedSettings).mappings(templateMappings); + ComposableIndexTemplate expectedEffectiveTemplate = ComposableIndexTemplate.builder() + .indexPatterns(List.of(dataStream.getName())) + .dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate()) + .template(expectedTemplateBuilder) + .build(); + assertThat(dataStream.getEffectiveIndexTemplate(projectMetadataBuilder.build()), equalTo(expectedEffectiveTemplate)); } private DataStream createDataStream(Settings settings) { From a9998f5c3d98a1bc054f96c4ed0935236e5bfc0f Mon Sep 17 00:00:00 2001 From: Keith Massey Date: Wed, 23 Apr 2025 07:57:04 -0500 Subject: [PATCH 12/13] fixing Settings::merge --- .../elasticsearch/common/settings/Settings.java | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/settings/Settings.java b/server/src/main/java/org/elasticsearch/common/settings/Settings.java index 8eb344cb4b13c..635537d62b2a7 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/Settings.java +++ b/server/src/main/java/org/elasticsearch/common/settings/Settings.java @@ -877,24 +877,21 @@ public Set keySet() { } /* - * This method merges the given newSettings into this Settings, returning either a new Settings object or - * this if the newSettings are null or empty. If any values are null in newSettings, those keys are removed - * from the returned object. + * This method merges the given newSettings into this Settings, returning either a new Settings object or this if the newSettings are + * empty. If any values are null in newSettings, those keys are removed from the returned object. */ public Settings merge(Settings newSettings) { Objects.requireNonNull(newSettings); if (Settings.EMPTY.equals(newSettings)) { return this; } - Settings.Builder builder = Settings.builder().put(this).put(newSettings); + Settings.Builder builder = Settings.builder().put(this); for (String key : newSettings.keySet()) { String rawValue = newSettings.get(key); - if (builder.get(key) == null) { - if (rawValue == null) { - builder.remove(key); - } else { - builder.put(key, rawValue); - } + if (rawValue == null) { + builder.remove(key); + } else { + builder.put(key, rawValue); } } return builder.build(); From 641c645daddd1ace20f383a2f6bef553cb2ff536 Mon Sep 17 00:00:00 2001 From: Keith Massey Date: Wed, 23 Apr 2025 07:57:42 -0500 Subject: [PATCH 13/13] removing support for null settings from ComposableIndexTemplate::mergeSettings --- .../cluster/metadata/ComposableIndexTemplate.java | 5 +++-- .../cluster/metadata/ComposableIndexTemplateTests.java | 1 + 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/ComposableIndexTemplate.java b/server/src/main/java/org/elasticsearch/cluster/metadata/ComposableIndexTemplate.java index bf8f44037860a..4ceb807adece4 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/ComposableIndexTemplate.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/ComposableIndexTemplate.java @@ -315,11 +315,12 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params, @Nulla * given settings are removed from the settings in the returned ComposableIndexTemplate. If this * ComposableIndexTemplate has no settings, the given settings are the only ones in the returned template * (with any null values removed). If this ComposableIndexTemplate has no template, an empty template with - * those settings is created. If the given settings are null or empty, this ComposableIndexTemplate is just + * those settings is created. If the given settings are empty, this ComposableIndexTemplate is just * returned unchanged. This method never changes this object. */ public ComposableIndexTemplate mergeSettings(Settings settings) { - if (settings == null || Settings.EMPTY.equals(settings)) { + Objects.requireNonNull(settings); + if (Settings.EMPTY.equals(settings)) { return this; } ComposableIndexTemplate.Builder mergedIndexTemplateBuilder = this.toBuilder(); diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/ComposableIndexTemplateTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/ComposableIndexTemplateTests.java index 24200e4b562c2..7ec71e89211c4 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/ComposableIndexTemplateTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/ComposableIndexTemplateTests.java @@ -284,6 +284,7 @@ public void testMergeEmptySettingsIntoTemplateWithNonEmptySettings() { Settings templateSettings = randomSettings(); Template.Builder templateBuilder = Template.builder().settings(templateSettings).mappings(randomMappings(null)); ComposableIndexTemplate indexTemplate = randomInstance(); + expectThrows(NullPointerException.class, () -> indexTemplate.mergeSettings(null)); assertThat(indexTemplate.mergeSettings(Settings.EMPTY), equalTo(indexTemplate)); }