Skip to content

Commit bebcaf9

Browse files
martijnvgmartijnvg
authored
Additional index settings provider validation (#113838)
Fail if an index settings provider adds a setting that was added by another index settings provider.
1 parent 78dd787 commit bebcaf9

File tree

4 files changed

+141
-22
lines changed

4 files changed

+141
-22
lines changed

‎server/src/main/java/org/elasticsearch/action/admin/indices/template/post/TransportSimulateIndexTemplateAction.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,7 @@ public static Template resolveTemplate(
280280
templateSettings,
281281
mappings
282282
);
283+
MetadataCreateIndexService.validateAdditionalSettings(provider, result, additionalSettings);
283284
dummySettings.put(result);
284285
additionalSettings.put(result);
285286
}

‎server/src/main/java/org/elasticsearch/cluster/metadata/MetadataCreateIndexService.java‎

Lines changed: 34 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -980,7 +980,6 @@ static Settings aggregateIndexSettings(
980980

981981
final Settings.Builder indexSettingsBuilder = Settings.builder();
982982
if (sourceMetadata == null) {
983-
final Settings.Builder additionalIndexSettings = Settings.builder();
984983
final Settings templateAndRequestSettings = Settings.builder().put(combinedTemplateSettings).put(request.settings()).build();
985984

986985
final boolean timeSeriesTemplate = Optional.of(request)
@@ -990,19 +989,20 @@ static Settings aggregateIndexSettings(
990989

991990
// Loop through all the explicit index setting providers, adding them to the
992991
// additionalIndexSettings map
992+
final Settings.Builder additionalIndexSettings = Settings.builder();
993993
final var resolvedAt = Instant.ofEpochMilli(request.getNameResolvedAt());
994994
for (IndexSettingProvider provider : indexSettingProviders) {
995-
additionalIndexSettings.put(
996-
provider.getAdditionalIndexSettings(
997-
request.index(),
998-
request.dataStreamName(),
999-
timeSeriesTemplate,
1000-
currentState.getMetadata(),
1001-
resolvedAt,
1002-
templateAndRequestSettings,
1003-
combinedTemplateMappings
1004-
)
995+
var newAdditionalSettings = provider.getAdditionalIndexSettings(
996+
request.index(),
997+
request.dataStreamName(),
998+
timeSeriesTemplate,
999+
currentState.getMetadata(),
1000+
resolvedAt,
1001+
templateAndRequestSettings,
1002+
combinedTemplateMappings
10051003
);
1004+
validateAdditionalSettings(provider, newAdditionalSettings, additionalIndexSettings);
1005+
additionalIndexSettings.put(newAdditionalSettings);
10061006
}
10071007

10081008
// For all the explicit settings, we go through the template and request level settings
@@ -1111,6 +1111,29 @@ static Settings aggregateIndexSettings(
11111111
return indexSettings;
11121112
}
11131113

1114+
/**
1115+
* Validates whether additional settings don't have keys that are already defined in all additional settings.
1116+
*
1117+
* @param provider The {@link IndexSettingProvider} that produced <code>additionalSettings</code>
1118+
* @param additionalSettings The settings produced by the specified <code>provider</code>
1119+
* @param allAdditionalSettings A settings builder containing all additional settings produced by any {@link IndexSettingProvider}
1120+
* that already executed
1121+
* @throws IllegalArgumentException If keys in additionalSettings are already defined in allAdditionalSettings
1122+
*/
1123+
public static void validateAdditionalSettings(
1124+
IndexSettingProvider provider,
1125+
Settings additionalSettings,
1126+
Settings.Builder allAdditionalSettings
1127+
) throws IllegalArgumentException {
1128+
for (String settingName : additionalSettings.keySet()) {
1129+
if (allAdditionalSettings.keys().contains(settingName)) {
1130+
var name = provider.getClass().getSimpleName();
1131+
var message = Strings.format("additional index setting [%s] added by [%s] is already present", settingName, name);
1132+
throw new IllegalArgumentException(message);
1133+
}
1134+
}
1135+
}
1136+
11141137
private static void validateSoftDeleteSettings(Settings indexSettings) {
11151138
if (IndexSettings.INDEX_SOFT_DELETES_SETTING.get(indexSettings) == false
11161139
&& IndexMetadata.SETTING_INDEX_VERSION_CREATED.get(indexSettings).onOrAfter(IndexVersions.V_8_0_0)) {

‎server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateService.java‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -694,25 +694,25 @@ private void validateIndexTemplateV2(String name, ComposableIndexTemplate indexT
694694
// Workaround for the fact that start_time and end_time are injected by the MetadataCreateDataStreamService upon creation,
695695
// but when validating templates that create data streams the MetadataCreateDataStreamService isn't used.
696696
var finalTemplate = indexTemplate.template();
697-
var finalSettings = Settings.builder();
698697
final var now = Instant.now();
699698
final var metadata = currentState.getMetadata();
700699

701700
final var combinedMappings = collectMappings(indexTemplate, metadata.componentTemplates(), "tmp_idx");
702701
final var combinedSettings = resolveSettings(indexTemplate, metadata.componentTemplates());
703702
// First apply settings sourced from index setting providers:
703+
var finalSettings = Settings.builder();
704704
for (var provider : indexSettingProviders) {
705-
finalSettings.put(
706-
provider.getAdditionalIndexSettings(
707-
"validate-index-name",
708-
indexTemplate.getDataStreamTemplate() != null ? "validate-data-stream-name" : null,
709-
indexTemplate.getDataStreamTemplate() != null && metadata.isTimeSeriesTemplate(indexTemplate),
710-
currentState.getMetadata(),
711-
now,
712-
combinedSettings,
713-
combinedMappings
714-
)
705+
var newAdditionalSettings = provider.getAdditionalIndexSettings(
706+
"validate-index-name",
707+
indexTemplate.getDataStreamTemplate() != null ? "validate-data-stream-name" : null,
708+
indexTemplate.getDataStreamTemplate() != null && metadata.isTimeSeriesTemplate(indexTemplate),
709+
currentState.getMetadata(),
710+
now,
711+
combinedSettings,
712+
combinedMappings
715713
);
714+
MetadataCreateIndexService.validateAdditionalSettings(provider, newAdditionalSettings, finalSettings);
715+
finalSettings.put(newAdditionalSettings);
716716
}
717717
// Then apply setting from component templates:
718718
finalSettings.put(combinedSettings);

‎server/src/test/java/org/elasticsearch/index/IndexSettingProviderTests.java‎

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the "Elastic License
4+
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
5+
* Public License v 1"; you may not use this file except in compliance with, at
6+
* your election, the "Elastic License 2.0", the "GNU Affero General Public
7+
* License v3.0 only", or the "Server Side Public License, v 1".
8+
*/
9+
10+
package org.elasticsearch.index;
11+
12+
import org.elasticsearch.cluster.metadata.Metadata;
13+
import org.elasticsearch.common.compress.CompressedXContent;
14+
import org.elasticsearch.common.settings.Settings;
15+
import org.elasticsearch.plugins.Plugin;
16+
import org.elasticsearch.test.ESSingleNodeTestCase;
17+
18+
import java.time.Instant;
19+
import java.util.Collection;
20+
import java.util.List;
21+
import java.util.concurrent.atomic.AtomicBoolean;
22+
23+
public class IndexSettingProviderTests extends ESSingleNodeTestCase {
24+
25+
public void testIndexCreation() throws Exception {
26+
var indexService = createIndex("my-index1");
27+
assertFalse(indexService.getIndexSettings().getSettings().hasValue("index.refresh_interval"));
28+
29+
INDEX_SETTING_PROVIDER1_ENABLED.set(true);
30+
indexService = createIndex("my-index2");
31+
assertTrue(indexService.getIndexSettings().getSettings().hasValue("index.refresh_interval"));
32+
33+
INDEX_SETTING_PROVIDER2_ENABLED.set(true);
34+
var e = expectThrows(IllegalArgumentException.class, () -> createIndex("my-index3"));
35+
assertEquals(
36+
"additional index setting [index.refresh_interval] added by [TestIndexSettingsProvider] is already present",
37+
e.getMessage()
38+
);
39+
}
40+
41+
@Override
42+
protected Collection<Class<? extends Plugin>> getPlugins() {
43+
return List.of(Plugin1.class, Plugin2.class);
44+
}
45+
46+
public static class Plugin1 extends Plugin {
47+
48+
@Override
49+
public Collection<IndexSettingProvider> getAdditionalIndexSettingProviders(IndexSettingProvider.Parameters parameters) {
50+
return List.of(new TestIndexSettingsProvider("index.refresh_interval", "-1", INDEX_SETTING_PROVIDER1_ENABLED));
51+
}
52+
53+
}
54+
55+
public static class Plugin2 extends Plugin {
56+
57+
@Override
58+
public Collection<IndexSettingProvider> getAdditionalIndexSettingProviders(IndexSettingProvider.Parameters parameters) {
59+
return List.of(new TestIndexSettingsProvider("index.refresh_interval", "100s", INDEX_SETTING_PROVIDER2_ENABLED));
60+
}
61+
}
62+
63+
private static final AtomicBoolean INDEX_SETTING_PROVIDER1_ENABLED = new AtomicBoolean(false);
64+
private static final AtomicBoolean INDEX_SETTING_PROVIDER2_ENABLED = new AtomicBoolean(false);
65+
66+
static class TestIndexSettingsProvider implements IndexSettingProvider {
67+
68+
private final String settingName;
69+
private final String settingValue;
70+
private final AtomicBoolean enabled;
71+
72+
TestIndexSettingsProvider(String settingName, String settingValue, AtomicBoolean enabled) {
73+
this.settingName = settingName;
74+
this.settingValue = settingValue;
75+
this.enabled = enabled;
76+
}
77+
78+
@Override
79+
public Settings getAdditionalIndexSettings(
80+
String indexName,
81+
String dataStreamName,
82+
boolean isTimeSeries,
83+
Metadata metadata,
84+
Instant resolvedAt,
85+
Settings indexTemplateAndCreateRequestSettings,
86+
List<CompressedXContent> combinedTemplateMappings
87+
) {
88+
if (enabled.get()) {
89+
return Settings.builder().put(settingName, settingValue).build();
90+
} else {
91+
return Settings.EMPTY;
92+
}
93+
}
94+
}
95+
}

0 commit comments

Comments
 (0)