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 @@ -490,20 +490,7 @@ public void testProvidersAffectMode() {
ClusterSettings.createBuiltInClusterSettings(),
dataStreamGlobalRetentionSettings,
emptyDataStreamFailureStoreSettings,
new IndexSettingProviders(
Set.of(
(
indexName,
dataStreamName,
templateIndexMode,
metadata,
resolvedAt,
indexTemplateAndCreateRequestSettings,
combinedTemplateMappings,
additionalSettings,
additionalCustomMetadata) -> additionalSettings.put("index.mode", IndexMode.LOOKUP)
)
),
IndexSettingProviders.of((additionalSettings) -> additionalSettings.put("index.mode", IndexMode.LOOKUP)),
null
);
assertThat(response.getDataStreams().getFirst().getIndexModeName(), equalTo("lookup"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import org.elasticsearch.cluster.metadata.IndexMetadata;
import org.elasticsearch.cluster.metadata.Metadata;
import org.elasticsearch.cluster.metadata.ProjectId;
import org.elasticsearch.common.settings.Setting;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.index.Index;
import org.elasticsearch.indices.SystemDataStreamDescriptor;
Expand Down Expand Up @@ -52,6 +53,8 @@ public class CreateIndexClusterStateUpdateRequest {

private ComposableIndexTemplate matchingTemplate;

private boolean settingsSystemProvided = false;

/**
* @deprecated project id ought always be specified
*/
Expand Down Expand Up @@ -223,6 +226,20 @@ public CreateIndexClusterStateUpdateRequest setMatchingTemplate(ComposableIndexT
return this;
}

/**
* Indicates whether the {@link #settings} of this request are system provided.
* System-provided settings are allowed to configure {@linkplain Setting.Property#PrivateIndex private} settings.
* These are typically coming from an {@link org.elasticsearch.index.IndexSettingProvider}.
*/
public CreateIndexClusterStateUpdateRequest settingsSystemProvided(boolean settingsSystemProvided) {
this.settingsSystemProvided = settingsSystemProvided;
return this;
}

public boolean settingsSystemProvided() {
return settingsSystemProvided;
}

@Override
public String toString() {
return "CreateIndexClusterStateUpdateRequest{"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1607,12 +1607,12 @@ private static void validateActiveShardCount(ActiveShardCount waitForActiveShard

private void validate(CreateIndexClusterStateUpdateRequest request, ProjectMetadata projectMetadata, RoutingTable routingTable) {
validateIndexName(request.index(), projectMetadata, routingTable);
validateIndexSettings(request.index(), request.settings(), forbidPrivateIndexSettings);
validateIndexSettings(request.index(), request.settings(), forbidPrivateIndexSettings && request.settingsSystemProvided() == false);
}

public void validateIndexSettings(String indexName, final Settings settings, final boolean forbidPrivateIndexSettings)
throws IndexCreationException {
List<String> validationErrors = getIndexSettingsValidationErrors(settings, forbidPrivateIndexSettings);
List<String> validationErrors = getIndexSettingsValidationErrors(settings, null, forbidPrivateIndexSettings);

if (validationErrors.isEmpty() == false) {
ValidationException validationException = new ValidationException();
Expand All @@ -1621,27 +1621,43 @@ public void validateIndexSettings(String indexName, final Settings settings, fin
}
}

List<String> getIndexSettingsValidationErrors(final Settings settings, final boolean forbidPrivateIndexSettings) {
List<String> getIndexSettingsValidationErrors(
final Settings settings,
@Nullable Settings systemProvided,
final boolean forbidPrivateIndexSettings
) {
List<String> validationErrors = validateIndexCustomPath(settings, env.sharedDataDir());
if (forbidPrivateIndexSettings) {
validationErrors.addAll(validatePrivateSettingsNotExplicitlySet(settings, indexScopedSettings));
validationErrors.addAll(validatePrivateSettingsNotExplicitlySet(settings, systemProvided, indexScopedSettings));
}
return validationErrors;
}

private static List<String> validatePrivateSettingsNotExplicitlySet(Settings settings, IndexScopedSettings indexScopedSettings) {
private static List<String> validatePrivateSettingsNotExplicitlySet(
Settings settings,
@Nullable Settings systemProvided,
IndexScopedSettings indexScopedSettings
) {
List<String> validationErrors = new ArrayList<>();
for (final String key : settings.keySet()) {
final Setting<?> setting = indexScopedSettings.get(key);
if (setting == null) {
assert indexScopedSettings.isPrivateSetting(key) : "expected [" + key + "] to be private but it was not";
} else if (setting.isPrivateIndex()) {
} else if (setting.isPrivateIndex() && isSystemProvided(key, settings, systemProvided) == false) {
validationErrors.add("private index setting [" + key + "] can not be set explicitly");
}
}
return validationErrors;
}

/*
* System-provided settings are always allowed to configure private settings.
* These are typically coming from an IndexSettingProvider.
*/
private static boolean isSystemProvided(String key, Settings settings, @Nullable Settings systemProvided) {
return systemProvided != null && settings.get(key).equals(systemProvided.get(key));
}

/**
* Validates that the configured index data path (if any) is a sub-path of the configured shared data path (if any)
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -786,8 +786,7 @@ void validateIndexTemplateV2(ProjectMetadata projectMetadata, String name, Compo

final var combinedMappings = collectMappings(indexTemplate, projectMetadata.componentTemplates(), "tmp_idx");
final var combinedSettings = resolveSettings(indexTemplate, projectMetadata.componentTemplates());
// First apply settings sourced from index setting providers:
var finalSettings = Settings.builder();
var additionalSettingsBuilder = Settings.builder();
Copy link
Member

Choose a reason for hiding this comment

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

Minor suggestion (take it or leave it), perhaps call this systemSettingsBuilder and systemSettings instead of additional… since it gets passed to the validation as the system-provided settings?

Copy link
Member Author

Choose a reason for hiding this comment

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

OTOH, it's coming from the provideAdditionalSettings method and it's being passed in to the validateAdditionalSettings method. I think it make sense as-is. The main thing to convey about these within validateIndexTemplateV2 are that these are additional settings. What's more important about these in the context of validation is that these additional settings are indeed system provided.

ImmutableOpenMap.Builder<String, Map<String, String>> customMetadataBuilder = ImmutableOpenMap.builder();
for (var provider : indexSettingProviders) {
Settings.Builder builder = Settings.builder();
Expand All @@ -803,9 +802,13 @@ void validateIndexTemplateV2(ProjectMetadata projectMetadata, String name, Compo
customMetadataBuilder::put
);
var newAdditionalSettings = builder.build();
MetadataCreateIndexService.validateAdditionalSettings(provider, newAdditionalSettings, finalSettings);
finalSettings.put(newAdditionalSettings);
MetadataCreateIndexService.validateAdditionalSettings(provider, newAdditionalSettings, additionalSettingsBuilder);
additionalSettingsBuilder.put(newAdditionalSettings);
}
Settings additionalSettings = additionalSettingsBuilder.build();
var finalSettings = Settings.builder();
// First apply settings sourced from index setting providers:
finalSettings.put(additionalSettings);
// Then apply setting from component templates:
finalSettings.put(combinedSettings);
// Then finally apply settings resolved from index template:
Expand All @@ -815,7 +818,7 @@ void validateIndexTemplateV2(ProjectMetadata projectMetadata, String name, Compo

var templateToValidate = indexTemplate.toBuilder().template(Template.builder(finalTemplate).settings(finalSettings)).build();

validate(name, templateToValidate);
validate(name, templateToValidate, additionalSettings);
validateDataStreamsStillReferenced(projectMetadata, name, templateToValidate);
validateLifecycle(projectMetadata, name, templateToValidate, globalRetentionSettings.get(false));
validateDataStreamOptions(projectMetadata, name, templateToValidate, globalRetentionSettings.get(true));
Expand Down Expand Up @@ -2059,18 +2062,19 @@ public static void validateTemplate(Settings validateSettings, CompressedXConten
}

public void validate(String name, ComponentTemplate template) {
validate(name, template.template(), Collections.emptyList());
validate(name, template.template(), Collections.emptyList(), null);
}

private void validate(String name, ComposableIndexTemplate template) {
validate(name, template.template(), template.indexPatterns());
private void validate(String name, ComposableIndexTemplate template, @Nullable Settings systemProvided) {
validate(name, template.template(), template.indexPatterns(), systemProvided);
}

private void validate(String name, Template template, List<String> indexPatterns) {
private void validate(String name, Template template, List<String> indexPatterns, @Nullable Settings systemProvided) {
Optional<Template> maybeTemplate = Optional.ofNullable(template);
validate(
name,
maybeTemplate.map(Template::settings).orElse(Settings.EMPTY),
systemProvided,
indexPatterns,
maybeTemplate.map(Template::aliases).orElse(emptyMap()).values().stream().map(MetadataIndexTemplateService::toAlias).toList()
);
Expand All @@ -2089,10 +2093,16 @@ private static Alias toAlias(AliasMetadata aliasMeta) {
}

private void validate(PutRequest putRequest) {
validate(putRequest.name, putRequest.settings, putRequest.indexPatterns, putRequest.aliases);
validate(putRequest.name, putRequest.settings, null, putRequest.indexPatterns, putRequest.aliases);
}

private void validate(String name, @Nullable Settings settings, List<String> indexPatterns, List<Alias> aliases) {
private void validate(
String name,
@Nullable Settings settings,
@Nullable Settings systemProvided,
List<String> indexPatterns,
List<Alias> aliases
) {
List<String> validationErrors = new ArrayList<>();
if (name.contains(" ")) {
validationErrors.add("name must not contain a space");
Expand Down Expand Up @@ -2145,7 +2155,11 @@ private void validate(String name, @Nullable Settings settings, List<String> ind
validationErrors.add(t.getMessage());
}
}
List<String> indexSettingsValidation = metadataCreateIndexService.getIndexSettingsValidationErrors(settings, true);
List<String> indexSettingsValidation = metadataCreateIndexService.getIndexSettingsValidationErrors(
settings,
systemProvided,
true
);
validationErrors.addAll(indexSettingsValidation);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,11 @@

package org.elasticsearch.index;

import org.elasticsearch.common.settings.Settings;

import java.util.Collections;
import java.util.Set;
import java.util.function.Consumer;

/**
* Keeps track of the {@link IndexSettingProvider} instances defined by plugins and
Expand All @@ -22,6 +25,31 @@ public final class IndexSettingProviders {

private final Set<IndexSettingProvider> indexSettingProviders;

/**
* Utility method which creates an {@link IndexSettingProviders} instance that uses the provided consumer to add settings
* to the index being created.
* The primary use case is for tests that want to add specific settings without having to create a full implementation.
*
* @param settingsBuilderConsumer A consumer that adds index settings
* @return An {@link IndexSettingProviders} instance that uses the provided consumer to add settings
*/
public static IndexSettingProviders of(Consumer<Settings.Builder> settingsBuilderConsumer) {
return new IndexSettingProviders(
Set.of(
(
indexName,
dataStreamName,
templateIndexMode,
projectMetadata,
resolvedAt,
indexTemplateAndCreateRequestSettings,
combinedTemplateMappings,
additionalSettings,
additionalCustomMetadata) -> settingsBuilderConsumer.accept(additionalSettings)
)
);
}

public IndexSettingProviders(Set<IndexSettingProvider> indexSettingProviders) {
this.indexSettingProviders = Collections.unmodifiableSet(indexSettingProviders);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@
import org.elasticsearch.index.shard.IndexLongFieldRange;
import org.elasticsearch.indices.EmptySystemIndices;
import org.elasticsearch.indices.IndexCreationException;
import org.elasticsearch.indices.IndicesService;
import org.elasticsearch.indices.InvalidAliasNameException;
import org.elasticsearch.indices.InvalidIndexNameException;
import org.elasticsearch.indices.ShardLimitValidator;
Expand All @@ -82,6 +83,7 @@
import org.elasticsearch.xcontent.XContentFactory;
import org.hamcrest.Matchers;
import org.junit.Before;
import org.mockito.ArgumentCaptor;

import java.io.IOException;
import java.time.Instant;
Expand Down Expand Up @@ -128,6 +130,9 @@
import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.nullValue;
import static org.hamcrest.Matchers.startsWith;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;

public class MetadataCreateIndexServiceTests extends ESTestCase {

Expand Down Expand Up @@ -1739,6 +1744,106 @@ public void testCreateClusterBlocksTransformerForIndexCreation() {
);
}

public void testSetPrivateSettingsFails() throws Exception {
request.settings(Settings.builder().put(IndexMetadata.INDEX_DOWNSAMPLE_SOURCE_UUID.getKey(), "private_setting").build());

IndicesService indicesService = mock(IndicesService.class);
withTemporaryClusterService(((clusterService, threadPool) -> {
MetadataCreateIndexService service = new MetadataCreateIndexService(
Settings.EMPTY,
clusterService,
indicesService,
null,
createTestShardLimitService(randomIntBetween(1, 1000), clusterService),
newEnvironment(),
new IndexScopedSettings(Settings.EMPTY, IndexScopedSettings.BUILT_IN_INDEX_SETTINGS),
threadPool,
null,
EmptySystemIndices.INSTANCE,
true,
IndexSettingProviders.EMPTY
);

IndexCreationException exception = assertThrows(
IndexCreationException.class,
() -> service.applyCreateIndexRequest(clusterService.state(), request, false, ActionListener.wrap(r -> {}, e -> {}))
);
assertThat(
exception.getCause().getMessage(),
containsString(
"private index setting [" + IndexMetadata.INDEX_DOWNSAMPLE_SOURCE_UUID.getKey() + "] can not be set explicitly"
)
);
}));
}

public void testSetPrivateSettingsSucceedsWhenSystemProvided() throws Exception {
request.settings(Settings.builder().put(IndexMetadata.INDEX_DOWNSAMPLE_SOURCE_UUID.getKey(), "private_setting").build());
request.settingsSystemProvided(true);

IndicesService indicesService = mock(IndicesService.class);
withTemporaryClusterService(((clusterService, threadPool) -> {
MetadataCreateIndexService service = new MetadataCreateIndexService(
Settings.EMPTY,
clusterService,
indicesService,
null,
createTestShardLimitService(randomIntBetween(1, 1000), clusterService),
newEnvironment(),
new IndexScopedSettings(Settings.EMPTY, IndexScopedSettings.BUILT_IN_INDEX_SETTINGS),
threadPool,
null,
EmptySystemIndices.INSTANCE,
true,
IndexSettingProviders.EMPTY
);

try {
service.applyCreateIndexRequest(clusterService.state(), request, false, ActionListener.wrap(r -> {}, e -> {}));
} catch (Exception e) {
fail(e, "did not expect private setting to be rejected when system provided");
}
}));

ArgumentCaptor<IndexMetadata> captor = ArgumentCaptor.forClass(IndexMetadata.class);
verify(indicesService).withTempIndexService(captor.capture(), any());
IndexMetadata indexMetadata = captor.getValue();
assertThat(indexMetadata.getSettings().get(IndexMetadata.INDEX_DOWNSAMPLE_SOURCE_UUID.getKey()), equalTo("private_setting"));
}

public void testIndexSettingProviderPrivateSetting() throws Exception {
IndicesService indicesService = mock(IndicesService.class);
withTemporaryClusterService(((clusterService, threadPool) -> {
MetadataCreateIndexService service = new MetadataCreateIndexService(
Settings.EMPTY,
clusterService,
indicesService,
null,
createTestShardLimitService(randomIntBetween(1, 1000), clusterService),
newEnvironment(),
new IndexScopedSettings(Settings.EMPTY, IndexScopedSettings.BUILT_IN_INDEX_SETTINGS),
threadPool,
null,
EmptySystemIndices.INSTANCE,
true,
IndexSettingProviders.of(
(additionalSettings) -> additionalSettings.put(IndexMetadata.INDEX_DOWNSAMPLE_SOURCE_NAME.getKey(), "private_setting")
)
);

try {
service.applyCreateIndexRequest(clusterService.state(), request, false, ActionListener.wrap(r -> {}, e -> {}));
} catch (Exception e) {
fail(e, "did not expect private setting to be rejected when added via IndexSettingProvider");
}
}));

ArgumentCaptor<IndexMetadata> captor = ArgumentCaptor.forClass(IndexMetadata.class);
verify(indicesService).withTempIndexService(captor.capture(), any());
IndexMetadata indexMetadata = captor.getValue();
assertThat(indexMetadata.getSettings().get(IndexMetadata.INDEX_DOWNSAMPLE_SOURCE_NAME.getKey()), equalTo("private_setting"));
}

private IndexTemplateMetadata addMatchingTemplate(Consumer<IndexTemplateMetadata.Builder> configurator) {
IndexTemplateMetadata.Builder builder = templateMetadataBuilder("template1", "te*");
configurator.accept(builder);
Expand Down Expand Up @@ -1776,7 +1881,7 @@ private ShardLimitValidator randomShardLimitService() {

private void withTemporaryClusterService(BiConsumer<ClusterService, ThreadPool> consumer) {
final ThreadPool threadPool = new TestThreadPool(getTestName());
final ClusterService clusterService = ClusterServiceUtils.createClusterService(threadPool);
final ClusterService clusterService = ClusterServiceUtils.createClusterService(threadPool, projectId);
try {
consumer.accept(clusterService, threadPool);
} finally {
Expand Down
Loading