Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,7 @@ public static Template resolveTemplate(
templateSettings,
mappings
);
MetadataCreateIndexService.validateAdditionalSettings(provider, result, additionalSettings);
dummySettings.put(result);
additionalSettings.put(result);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -980,7 +980,6 @@ static Settings aggregateIndexSettings(

final Settings.Builder indexSettingsBuilder = Settings.builder();
if (sourceMetadata == null) {
final Settings.Builder additionalIndexSettings = Settings.builder();
final Settings templateAndRequestSettings = Settings.builder().put(combinedTemplateSettings).put(request.settings()).build();

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

// Loop through all the explicit index setting providers, adding them to the
// additionalIndexSettings map
final Settings.Builder additionalIndexSettings = Settings.builder();
final var resolvedAt = Instant.ofEpochMilli(request.getNameResolvedAt());
for (IndexSettingProvider provider : indexSettingProviders) {
additionalIndexSettings.put(
provider.getAdditionalIndexSettings(
request.index(),
request.dataStreamName(),
timeSeriesTemplate,
currentState.getMetadata(),
resolvedAt,
templateAndRequestSettings,
combinedTemplateMappings
)
var newAdditionalSettings = provider.getAdditionalIndexSettings(
request.index(),
request.dataStreamName(),
timeSeriesTemplate,
currentState.getMetadata(),
resolvedAt,
templateAndRequestSettings,
combinedTemplateMappings
);
validateAdditionalSettings(provider, newAdditionalSettings, additionalIndexSettings);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we validate the newAdditionalSettings against the additionalSettings seen so far and not values previously set by the user for example templateAndRequestSettings. Are these 2 settings sets disjoint ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The newAdditionalSettings variable contains any settings that the current index settings provider might have set. The additionalIndexSettings variable tracks any settings that any index settings provider may have set while looping over the list of index settings providers. The new validation requires that newAdditionalSettings cannot contain any settings that are already defined in additionalIndexSettings. So yes, these two Settings variables should be a disjoint.

additionalIndexSettings.put(newAdditionalSettings);
}

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

/**
* Validates whether additional settings don't have keys that are already defined in all additional settings.
*
* @param provider The {@link IndexSettingProvider} that produced <code>additionalSettings</code>
* @param additionalSettings The settings produced by the specified <code>provider</code>
* @param allAdditionalSettings A settings builder containing all additional settings produced by any {@link IndexSettingProvider}
* that already executed
* @throws IllegalArgumentException If keys in additionalSettings are already defined in allAdditionalSettings
*/
public static void validateAdditionalSettings(
IndexSettingProvider provider,
Settings additionalSettings,
Settings.Builder allAdditionalSettings
) throws IllegalArgumentException {
for (String settingName : additionalSettings.keySet()) {
if (allAdditionalSettings.keys().contains(settingName)) {
var name = provider.getClass().getSimpleName();
var message = Strings.format("additional index setting [%s] added by [%s] is already present", settingName, name);
throw new IllegalArgumentException(message);
}
}
}

private static void validateSoftDeleteSettings(Settings indexSettings) {
if (IndexSettings.INDEX_SOFT_DELETES_SETTING.get(indexSettings) == false
&& IndexMetadata.SETTING_INDEX_VERSION_CREATED.get(indexSettings).onOrAfter(IndexVersions.V_8_0_0)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -694,25 +694,25 @@ private void validateIndexTemplateV2(String name, ComposableIndexTemplate indexT
// Workaround for the fact that start_time and end_time are injected by the MetadataCreateDataStreamService upon creation,
// but when validating templates that create data streams the MetadataCreateDataStreamService isn't used.
var finalTemplate = indexTemplate.template();
var finalSettings = Settings.builder();
final var now = Instant.now();
final var metadata = currentState.getMetadata();

final var combinedMappings = collectMappings(indexTemplate, metadata.componentTemplates(), "tmp_idx");
final var combinedSettings = resolveSettings(indexTemplate, metadata.componentTemplates());
// First apply settings sourced from index setting providers:
var finalSettings = Settings.builder();
for (var provider : indexSettingProviders) {
finalSettings.put(
provider.getAdditionalIndexSettings(
"validate-index-name",
indexTemplate.getDataStreamTemplate() != null ? "validate-data-stream-name" : null,
indexTemplate.getDataStreamTemplate() != null && metadata.isTimeSeriesTemplate(indexTemplate),
currentState.getMetadata(),
now,
combinedSettings,
combinedMappings
)
var newAdditionalSettings = provider.getAdditionalIndexSettings(
"validate-index-name",
indexTemplate.getDataStreamTemplate() != null ? "validate-data-stream-name" : null,
indexTemplate.getDataStreamTemplate() != null && metadata.isTimeSeriesTemplate(indexTemplate),
currentState.getMetadata(),
now,
combinedSettings,
combinedMappings
);
MetadataCreateIndexService.validateAdditionalSettings(provider, newAdditionalSettings, finalSettings);
finalSettings.put(newAdditionalSettings);
}
// Then apply setting from component templates:
finalSettings.put(combinedSettings);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the "Elastic License
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
* Public License v 1"; you may not use this file except in compliance with, at
* your election, the "Elastic License 2.0", the "GNU Affero General Public
* License v3.0 only", or the "Server Side Public License, v 1".
*/

package org.elasticsearch.index;

import org.elasticsearch.cluster.metadata.Metadata;
import org.elasticsearch.common.compress.CompressedXContent;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.test.ESSingleNodeTestCase;

import java.time.Instant;
import java.util.Collection;
import java.util.List;
import java.util.concurrent.atomic.AtomicBoolean;

public class IndexSettingProviderTests extends ESSingleNodeTestCase {

public void testIndexCreation() throws Exception {
var indexService = createIndex("my-index1");
assertFalse(indexService.getIndexSettings().getSettings().hasValue("index.refresh_interval"));

INDEX_SETTING_PROVIDER1_ENABLED.set(true);
indexService = createIndex("my-index2");
assertTrue(indexService.getIndexSettings().getSettings().hasValue("index.refresh_interval"));

INDEX_SETTING_PROVIDER2_ENABLED.set(true);
var e = expectThrows(IllegalArgumentException.class, () -> createIndex("my-index3"));
assertEquals(
"additional index setting [index.refresh_interval] added by [TestIndexSettingsProvider] is already present",
e.getMessage()
);
}

@Override
protected Collection<Class<? extends Plugin>> getPlugins() {
return List.of(Plugin1.class, Plugin2.class);
}

public static class Plugin1 extends Plugin {

@Override
public Collection<IndexSettingProvider> getAdditionalIndexSettingProviders(IndexSettingProvider.Parameters parameters) {
return List.of(new TestIndexSettingsProvider("index.refresh_interval", "-1", INDEX_SETTING_PROVIDER1_ENABLED));
}

}

public static class Plugin2 extends Plugin {

@Override
public Collection<IndexSettingProvider> getAdditionalIndexSettingProviders(IndexSettingProvider.Parameters parameters) {
return List.of(new TestIndexSettingsProvider("index.refresh_interval", "100s", INDEX_SETTING_PROVIDER2_ENABLED));
}
}

private static final AtomicBoolean INDEX_SETTING_PROVIDER1_ENABLED = new AtomicBoolean(false);
private static final AtomicBoolean INDEX_SETTING_PROVIDER2_ENABLED = new AtomicBoolean(false);

static class TestIndexSettingsProvider implements IndexSettingProvider {

private final String settingName;
private final String settingValue;
private final AtomicBoolean enabled;

TestIndexSettingsProvider(String settingName, String settingValue, AtomicBoolean enabled) {
this.settingName = settingName;
this.settingValue = settingValue;
this.enabled = enabled;
}

@Override
public Settings getAdditionalIndexSettings(
String indexName,
String dataStreamName,
boolean isTimeSeries,
Metadata metadata,
Instant resolvedAt,
Settings indexTemplateAndCreateRequestSettings,
List<CompressedXContent> combinedTemplateMappings
) {
if (enabled.get()) {
return Settings.builder().put(settingName, settingValue).build();
} else {
return Settings.EMPTY;
}
}
}
}