Skip to content
Open
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 @@ -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();
Copy link
Contributor

Choose a reason for hiding this comment

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

Does the order matter here? I mean the fact that we apply additionalIndexSettings after request.settings().

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does, we'd need to filter conflicting ones. I'll add something if we decide to proceed with this.

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);
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need this line?

Copy link
Contributor Author

@kkrik-es kkrik-es Oct 24, 2024

Choose a reason for hiding this comment

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

I think so, additionalIndexSettings contains all provided settings and it's used below.

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