Skip to content

Commit

Permalink
Feed the settings from non-overruling providers to overruling ones.
Browse files Browse the repository at this point in the history
  • Loading branch information
kkrik-es committed Oct 23, 2024
1 parent ba7d095 commit b6433e2
Show file tree
Hide file tree
Showing 9 changed files with 192 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ public static Template resolveTemplate(
MetadataCreateIndexService.validateAdditionalSettings(provider, result, additionalSettings);
dummySettings.put(result);
additionalSettings.put(result);
if (provider.overrulesTemplateAndRequestSettings()) {
if (provider.overrulesSettings()) {
overrulingSettings.addAll(result.keySet());
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -980,34 +980,55 @@ static Settings aggregateIndexSettings(

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

final IndexMode templateIndexMode = Optional.of(request)
.filter(r -> r.isFailureIndex() == false)
.map(CreateIndexClusterStateUpdateRequest::matchingTemplate)
.map(metadata::retrieveIndexModeFromTemplate)
.orElse(null);

// Loop through all the explicit index setting providers, adding them to the
// Loop through non-overruling setting providers, adding them to the
// additionalIndexSettings map
final Settings.Builder additionalIndexSettings = Settings.builder();
final var resolvedAt = Instant.ofEpochMilli(request.getNameResolvedAt());
Set<String> overrulingSettings = new HashSet<>();
for (IndexSettingProvider provider : indexSettingProviders) {
if (provider.overrulesSettings()) {
continue;
}
var newAdditionalSettings = provider.getAdditionalIndexSettings(
request.index(),
request.dataStreamName(),
templateIndexMode,
currentState.getMetadata(),
metadata,
resolvedAt,
templateAndRequestSettings,
combinedTemplateMappings
);
validateAdditionalSettings(provider, newAdditionalSettings, additionalIndexSettings);
additionalIndexSettings.put(newAdditionalSettings);
if (provider.overrulesTemplateAndRequestSettings()) {
overrulingSettings.addAll(newAdditionalSettings.keySet());
}

// Feed the settings generated by non-overruling to overruling setting providers,
// along with template and request settings.
final Settings templateAndRequestAndProviderSettings = settingsBuilder.put(additionalIndexSettings.build()).build();
Set<String> overrulingSettings = new HashSet<>();
for (IndexSettingProvider provider : indexSettingProviders) {
if (provider.overrulesSettings() == false) {
continue;
}
var newAdditionalSettings = provider.getAdditionalIndexSettings(
request.index(),
request.dataStreamName(),
templateIndexMode,
metadata,
resolvedAt,
templateAndRequestAndProviderSettings,
combinedTemplateMappings
);
additionalIndexSettings.put(newAdditionalSettings);
overrulingSettings.addAll(newAdditionalSettings.keySet());
}

for (String explicitSetting : additionalIndexSettings.keys()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,9 @@ public interface IndexSettingProvider {
* otherwise <code>null</code> is returned.
* @param metadata The current metadata instance that doesn't yet contain the index to be created
* @param resolvedAt The time the request to create this new index was accepted.
* @param indexTemplateAndCreateRequestSettings All the settings resolved from the template that matches and any settings
* defined on the create index request
* @param incomingSettings All the settings resolved from the templates that match, the create index request,
* and from non-overruling providers if this is an overruling one
* per {@link #overrulesSettings()}
* @param combinedTemplateMappings All the mappings resolved from the template that matches
*/
Settings getAdditionalIndexSettings(
Expand All @@ -47,7 +48,7 @@ Settings getAdditionalIndexSettings(
@Nullable IndexMode templateIndexMode,
Metadata metadata,
Instant resolvedAt,
Settings indexTemplateAndCreateRequestSettings,
Settings incomingSettings,
List<CompressedXContent> combinedTemplateMappings
);

Expand All @@ -59,13 +60,16 @@ record Parameters(CheckedFunction<IndexMetadata, MapperService, IOException> map
}

/**
* Indicates whether the additional settings that this provider returns can overrule the settings defined in matching template
* or in create index request.
* Indicates whether the additional settings that this provider returns can overrule the settings defined in
* matching templates, create index request, and settings returned by non-overruling providers. All these
* settings are included as input to
* {@link #getAdditionalIndexSettings(String, String, IndexMode, Metadata, Instant, Settings, List)}
* and can therefore be used to decide what settings to provide/overwrite.
*
* Note that this is not used during index template validation, to avoid overruling template settings that may apply to
* different contexts (e.g. the provider is not used, or it returns different setting values).
*/
default boolean overrulesTemplateAndRequestSettings() {
default boolean overrulesSettings() {
return false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ public Settings getAdditionalIndexSettings(
}

@Override
public boolean overrulesTemplateAndRequestSettings() {
public boolean overrulesSettings() {
return true;
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -726,7 +726,7 @@ public Settings getAdditionalIndexSettings(
}

@Override
public boolean overrulesTemplateAndRequestSettings() {
public boolean overrulesSettings() {
return true;
}
})
Expand Down Expand Up @@ -769,7 +769,7 @@ public Settings getAdditionalIndexSettings(
}

@Override
public boolean overrulesTemplateAndRequestSettings() {
public boolean overrulesSettings() {
return true;
}
})
Expand Down Expand Up @@ -812,7 +812,7 @@ public Settings getAdditionalIndexSettings(
}

@Override
public boolean overrulesTemplateAndRequestSettings() {
public boolean overrulesSettings() {
return true;
}
})
Expand Down Expand Up @@ -855,7 +855,7 @@ public Settings getAdditionalIndexSettings(
}

@Override
public boolean overrulesTemplateAndRequestSettings() {
public boolean overrulesSettings() {
return true;
}
})
Expand All @@ -866,6 +866,66 @@ public boolean overrulesTemplateAndRequestSettings() {
assertThat(aggregatedIndexSettings.get("other_setting"), equalTo("other_value"));
}

public void testAggregateSettingsProviderUsesSettingsFromProvider() {
IndexTemplateMetadata templateMetadata = addMatchingTemplate(builder -> {
builder.settings(Settings.builder().put("template_setting", "value1"));
});
Metadata metadata = new Metadata.Builder().templates(Map.of("template_1", templateMetadata)).build();
ClusterState clusterState = ClusterState.builder(ClusterName.DEFAULT).metadata(metadata).build();
request.settings(Settings.builder().put("request_setting", "value2").build());

Settings aggregatedIndexSettings = aggregateIndexSettings(
clusterState,
request,
templateMetadata.settings(),
null,
null,
Settings.EMPTY,
IndexScopedSettings.DEFAULT_SCOPED_SETTINGS,
randomShardLimitService(),
Set.of(new IndexSettingProvider() {
@Override
public Settings getAdditionalIndexSettings(
String indexName,
String dataStreamName,
IndexMode templateIndexMode,
Metadata metadata,
Instant resolvedAt,
Settings incomingSettings,
List<CompressedXContent> combinedTemplateMappings
) {
return Settings.builder().put("provided_setting_1", "value_3").put("provided_setting_2", "value_4").build();
}
}, new IndexSettingProvider() {
@Override
public Settings getAdditionalIndexSettings(
String indexName,
String dataStreamName,
IndexMode templateIndexMode,
Metadata metadata,
Instant resolvedAt,
Settings incomingSettings,
List<CompressedXContent> combinedTemplateMappings
) {
if (incomingSettings.hasValue("provided_setting_1")) {
return Settings.builder().put("provided_setting_2", "value_6").put("request_setting", "value_5").build();
}
return Settings.builder().put("request_setting", "value_5").build();
}

@Override
public boolean overrulesSettings() {
return true;
}
})
);

assertThat(aggregatedIndexSettings.get("template_setting"), equalTo("value1"));
assertThat(aggregatedIndexSettings.get("request_setting"), equalTo("value_5"));
assertThat(aggregatedIndexSettings.get("provided_setting_1"), equalTo("value_3"));
assertThat(aggregatedIndexSettings.get("provided_setting_2"), equalTo("value_6"));
}

public void testInvalidAliasName() {
final String[] invalidAliasNames = new String[] { "-alias1", "+alias2", "_alias3", "a#lias", "al:ias", ".", ".." };
String aliasName = randomFrom(invalidAliasNames);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,14 @@ public void testIndexCreation() throws Exception {
assertTrue(indexService.getIndexSettings().getSettings().hasValue("index.refresh_interval"));
assertEquals("10", indexService.getIndexSettings().getSettings().get("index.mapping.depth.limit"));

INDEX_SETTING_OVERRULING.set(true);
INDEX_SETTING_PROVIDER2_ENABLED.set(true);
indexService = createIndex("my-index3", settings);
assertTrue(indexService.getIndexSettings().getSettings().hasValue("index.refresh_interval"));
assertEquals("100", indexService.getIndexSettings().getSettings().get("index.mapping.depth.limit"));

INDEX_SETTING_DEPTH_ENABLED.set(false);
INDEX_SETTING_PROVIDER2_ENABLED.set(true);
INDEX_SETTING_PROVIDER2_ENABLED.set(false);
INDEX_SETTING_PROVIDER3_ENABLED.set(true);
var e = expectThrows(IllegalArgumentException.class, () -> createIndex("my-index4", settings));
assertEquals(
"additional index setting [index.refresh_interval] added by [TestIndexSettingsProvider] is already present",
Expand All @@ -49,39 +50,48 @@ public void testIndexCreation() throws Exception {

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

public static class Plugin1 extends Plugin {

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

}

public static class Plugin2 extends Plugin {

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

public static class Plugin3 extends Plugin {

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

private static final AtomicBoolean INDEX_SETTING_PROVIDER1_ENABLED = new AtomicBoolean(false);
private static final AtomicBoolean INDEX_SETTING_PROVIDER2_ENABLED = new AtomicBoolean(false);
private static final AtomicBoolean INDEX_SETTING_PROVIDER3_ENABLED = new AtomicBoolean(false);
private static final AtomicBoolean INDEX_SETTING_DEPTH_ENABLED = new AtomicBoolean(true);
private static final AtomicBoolean INDEX_SETTING_OVERRULING = new AtomicBoolean(false);

static class TestIndexSettingsProvider implements IndexSettingProvider {

private final String intervalValue;
private final AtomicBoolean enabled;
private final boolean overruling;

TestIndexSettingsProvider(String intervalValue, AtomicBoolean enabled) {
TestIndexSettingsProvider(String intervalValue, AtomicBoolean enabled, boolean overruling) {
this.intervalValue = intervalValue;
this.enabled = enabled;
this.overruling = overruling;
}

@Override
Expand All @@ -91,13 +101,14 @@ public Settings getAdditionalIndexSettings(
IndexMode templateIndexMode,
Metadata metadata,
Instant resolvedAt,
Settings indexTemplateAndCreateRequestSettings,
Settings incomingSettings,
List<CompressedXContent> combinedTemplateMappings
) {
if (enabled.get()) {
var builder = Settings.builder().put("index.refresh_interval", intervalValue);
if (INDEX_SETTING_DEPTH_ENABLED.get()) {
builder.put("index.mapping.depth.limit", 100);
// Verify that the value can be passed from non-overruling to overruling instance.
builder.put("index.mapping.depth.limit", incomingSettings.hasValue("index.refresh_interval") ? 100 : 1);
}
return builder.build();
} else {
Expand All @@ -106,8 +117,8 @@ public Settings getAdditionalIndexSettings(
}

@Override
public boolean overrulesTemplateAndRequestSettings() {
return INDEX_SETTING_OVERRULING.get();
public boolean overrulesSettings() {
return overruling;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
package org.elasticsearch.xpack.logsdb;

import org.elasticsearch.client.Request;
import org.elasticsearch.cluster.metadata.DataStream;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.index.mapper.SourceFieldMapper;
import org.elasticsearch.test.cluster.ElasticsearchCluster;
Expand Down Expand Up @@ -171,4 +172,35 @@ public void testLogsdbOverrideNullInTemplate() throws IOException {
assertEquals("logsdb", settings.get("index.mode"));
assertEquals(SourceFieldMapper.Mode.STORED.toString(), settings.get("index.mapping.source.mode"));
}

public void testLogsdbOverrideDefaultModeForLogsIndex() throws IOException {
Request request = new Request("PUT", "/_cluster/settings");
request.setJsonEntity("{ \"transient\": { \"cluster.logsdb.enabled\": true } }");
assertOK(client().performRequest(request));

request = new Request("POST", "/_index_template/1");
request.setJsonEntity("""
{
"index_patterns": ["logs-test-*"],
"data_stream": {
}
}
""");
assertOK(client().performRequest(request));

request = new Request("POST", "/logs-test-foo/_doc");
request.setJsonEntity("""
{
"@timestamp": "2020-01-01T00:00:00.000Z",
"host.name": "foo",
"message": "bar"
}
""");
assertOK(client().performRequest(request));

String index = DataStream.getDefaultBackingIndexName("logs-test-foo", 1);
var settings = (Map<?, ?>) ((Map<?, ?>) getIndexSettings(index).get(index)).get("settings");
assertEquals("logsdb", settings.get("index.mode"));
assertEquals(SourceFieldMapper.Mode.STORED.toString(), settings.get("index.mapping.source.mode"));
}
}
Loading

0 comments on commit b6433e2

Please sign in to comment.