Skip to content

Commit

Permalink
Additional index settings provider validation (elastic#113838)
Browse files Browse the repository at this point in the history
Fail if an index settings provider adds a setting that was added by another index settings provider.
  • Loading branch information
martijnvg authored Oct 11, 2024
1 parent 78dd787 commit bebcaf9
Show file tree
Hide file tree
Showing 4 changed files with 141 additions and 22 deletions.
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);
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;
}
}
}
}

0 comments on commit bebcaf9

Please sign in to comment.