From 6a6b51f3beb6ac7f34318b9cb0fb2a1f1a7f7e00 Mon Sep 17 00:00:00 2001 From: Mohit Godwani Date: Wed, 21 Aug 2024 11:55:30 +0530 Subject: [PATCH] Add settings validation Signed-off-by: Mohit Godwani --- .../metadata/MetadataCreateIndexService.java | 26 ++++++++++++++++--- .../MetadataIndexTemplateService.java | 17 +++++++----- .../MetadataUpdateSettingsService.java | 18 +++++++++++++ .../common/settings/IndexScopedSettings.java | 1 + .../org/opensearch/index/IndexSettings.java | 9 +++++++ 5 files changed, 62 insertions(+), 9 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java b/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java index c50b180996e8d..5299602986793 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java @@ -77,6 +77,7 @@ import org.opensearch.common.settings.Settings; import org.opensearch.common.unit.TimeValue; import org.opensearch.common.util.FeatureFlags; +import org.opensearch.common.util.set.Sets; import org.opensearch.common.xcontent.XContentHelper; import org.opensearch.core.action.ActionListener; import org.opensearch.core.common.Strings; @@ -148,7 +149,7 @@ import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REPLICATION_TYPE; import static org.opensearch.cluster.metadata.Metadata.DEFAULT_REPLICA_COUNT_SETTING; -import static org.opensearch.cluster.metadata.MetadataIndexTemplateService.findContextTemplate; +import static org.opensearch.cluster.metadata.MetadataIndexTemplateService.findContextTemplateName; import static org.opensearch.index.IndexModule.INDEX_STORE_TYPE_SETTING; import static org.opensearch.indices.IndicesService.CLUSTER_REPLICATION_TYPE_SETTING; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.isRemoteDataAttributePresent; @@ -563,7 +564,7 @@ private Template applyContext( Settings.Builder settingsBuilder ) throws IOException { if (request.context() != null) { - String contextTemplate = MetadataIndexTemplateService.findContextTemplate(currentState.metadata(), request.context()); + String contextTemplate = MetadataIndexTemplateService.findContextTemplateName(currentState.metadata(), request.context()); ComponentTemplate componentTemplate = currentState.metadata().componentTemplates().get(contextTemplate); if (componentTemplate.template().mappings() != null) { @@ -572,17 +573,36 @@ private Template applyContext( } if (componentTemplate.template().settings() != null) { + validateOverlap(settingsBuilder.keys(), componentTemplate.template().settings(), request.index()).ifPresent(message -> { + ValidationException validationException = new ValidationException(); + validationException.addValidationError(message); + throw validationException; + }); // Settings applied at last settingsBuilder.put(componentTemplate.template().settings()); } settingsBuilder.put(IndexSettings.INDEX_CONTEXT_CREATED_VERSION.getKey(), componentTemplate.version()); + settingsBuilder.put(IndexSettings.INDEX_CONTEXT_CURRENT_VERSION.getKey(), componentTemplate.version()); return componentTemplate.template(); } return null; } + static Optional validateOverlap(Set requestSettings, Settings contextTemplateSettings, String indexName) { + if (requestSettings.stream().anyMatch(contextTemplateSettings::hasValue)) { + return Optional.of( + "Cannot apply context template as user provide settings have overlap with the included context template." + + "Please remove the settings [" + + Sets.intersection(requestSettings, contextTemplateSettings.keySet()) + + "] to continue using the context for index: " + + indexName + ); + } + return Optional.empty(); + } + /** * Given a state and index settings calculated after applying templates, validate metadata for * the new index, returning an {@link IndexMetadata} for the new index. @@ -1760,7 +1780,7 @@ static void validateContext(CreateIndexClusterStateUpdateRequest request, Cluste ); } - if (request.context() != null && findContextTemplate(clusterState.metadata(), request.context()) == null) { + if (request.context() != null && findContextTemplateName(clusterState.metadata(), request.context()) == null) { throw new InvalidIndexContextException( request.context().name(), request.index(), diff --git a/server/src/main/java/org/opensearch/cluster/metadata/MetadataIndexTemplateService.java b/server/src/main/java/org/opensearch/cluster/metadata/MetadataIndexTemplateService.java index 0d4ef8a7f6c74..43d1551831950 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/MetadataIndexTemplateService.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/MetadataIndexTemplateService.java @@ -452,7 +452,7 @@ static void validateNotInUse(Metadata metadata, String templateNameOrWildcard) { final Set componentsBeingUsed = new HashSet<>(); final List templatesStillUsing = metadata.templatesV2().entrySet().stream().filter(e -> { Set referredComponentTemplates = new HashSet<>(e.getValue().composedOf()); - String systemTemplateUsed = findContextTemplate(metadata, e.getValue().context()); + String systemTemplateUsed = findContextTemplateName(metadata, e.getValue().context()); if (systemTemplateUsed != null) { referredComponentTemplates.add(systemTemplateUsed); } @@ -568,7 +568,7 @@ public static void validateV2TemplateRequest( ); } - if (template.context() != null && findContextTemplate(metadata, template.context()) == null) { + if (template.context() != null && findContextTemplateName(metadata, template.context()) == null) { throw new InvalidIndexTemplateException( name, "index template [" + name + "] specifies a context which is not loaded on the cluster." @@ -585,7 +585,12 @@ private void validateComponentTemplateRequest(ComponentTemplate componentTemplat } } - static String findContextTemplate(Metadata metadata, Context context) { + static ComponentTemplate findComponentTemplate(Metadata metadata, Context context) { + String contextTemplateName = findContextTemplateName(metadata, context); + return metadata.componentTemplates().getOrDefault(contextTemplateName, null); + } + + static String findContextTemplateName(Metadata metadata, Context context) { if (context == null) { return null; } @@ -1246,7 +1251,7 @@ public static List collectMappings(final ClusterState state, // Now use context mappings which take the highest precedence Optional.ofNullable(template.context()) - .map(ctx -> findContextTemplate(state.metadata(), ctx)) + .map(ctx -> findContextTemplateName(state.metadata(), ctx)) .map(name -> state.metadata().componentTemplates().get(name)) .map(ComponentTemplate::template) .map(Template::mappings) @@ -1317,7 +1322,7 @@ private static Settings resolveSettings(Metadata metadata, ComposableIndexTempla Optional.ofNullable(template.template()).map(Template::settings).ifPresent(templateSettings::put); // Add the template referred by context since it will take the highest precedence. - final String systemTemplate = findContextTemplate(metadata, template.context()); + final String systemTemplate = findContextTemplateName(metadata, template.context()); final ComponentTemplate componentTemplate = metadata.componentTemplates().get(systemTemplate); Optional.ofNullable(componentTemplate).map(ComponentTemplate::template).map(Template::settings).ifPresent(templateSettings::put); @@ -1367,7 +1372,7 @@ public static List> resolveAliases(final Metadata met // Now use context referenced template's aliases which take the highest precedence if (template.context() != null) { - final String systemTemplate = findContextTemplate(metadata, template.context()); + final String systemTemplate = findContextTemplateName(metadata, template.context()); final ComponentTemplate componentTemplate = metadata.componentTemplates().get(systemTemplate); Optional.ofNullable(componentTemplate.template()).map(Template::aliases).ifPresent(aliases::add); } diff --git a/server/src/main/java/org/opensearch/cluster/metadata/MetadataUpdateSettingsService.java b/server/src/main/java/org/opensearch/cluster/metadata/MetadataUpdateSettingsService.java index 7d4c3512ed757..5eaae5ce60c76 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/MetadataUpdateSettingsService.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/MetadataUpdateSettingsService.java @@ -65,16 +65,20 @@ import org.opensearch.threadpool.ThreadPool; import java.io.IOException; +import java.util.ArrayList; import java.util.Arrays; import java.util.HashSet; +import java.util.List; import java.util.Locale; import java.util.Map; import java.util.Optional; import java.util.Set; import static org.opensearch.action.support.ContextPreservingActionListener.wrapPreservingContext; +import static org.opensearch.cluster.metadata.MetadataCreateIndexService.validateOverlap; import static org.opensearch.cluster.metadata.MetadataCreateIndexService.validateRefreshIntervalSettings; import static org.opensearch.cluster.metadata.MetadataCreateIndexService.validateTranslogDurabilitySettings; +import static org.opensearch.cluster.metadata.MetadataIndexTemplateService.findComponentTemplate; import static org.opensearch.common.settings.AbstractScopedSettings.ARCHIVED_SETTINGS_PREFIX; import static org.opensearch.index.IndexSettings.same; @@ -196,6 +200,7 @@ public ClusterState execute(ClusterState currentState) { Set openIndices = new HashSet<>(); Set closeIndices = new HashSet<>(); final String[] actualIndices = new String[request.indices().length]; + final List validationErrors = new ArrayList<>(); for (int i = 0; i < request.indices().length; i++) { Index index = request.indices()[i]; actualIndices[i] = index.getName(); @@ -205,6 +210,19 @@ public ClusterState execute(ClusterState currentState) { } else { closeIndices.add(index); } + if (metadata.context() != null) { + validateOverlap( + normalizedSettings.keySet(), + findComponentTemplate(currentState.metadata(), metadata.context()).template().settings(), + index.getName() + ).ifPresent(validationErrors::add); + } + } + + if (validationErrors.size() > 0) { + ValidationException exception = new ValidationException(); + exception.addValidationErrors(validationErrors); + throw exception; } if (!skippedSettings.isEmpty() && !openIndices.isEmpty()) { diff --git a/server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java b/server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java index 409f02f5abff6..6cc0f679c7a13 100644 --- a/server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java @@ -249,6 +249,7 @@ public final class IndexScopedSettings extends AbstractScopedSettings { StarTreeIndexSettings.STAR_TREE_MAX_DATE_INTERVALS_SETTING, IndexSettings.INDEX_CONTEXT_CREATED_VERSION, + IndexSettings.INDEX_CONTEXT_CURRENT_VERSION, // validate that built-in similarities don't get redefined Setting.groupSetting("index.similarity.", (s) -> { diff --git a/server/src/main/java/org/opensearch/index/IndexSettings.java b/server/src/main/java/org/opensearch/index/IndexSettings.java index 932984873a10e..7a7577b3d4b7d 100644 --- a/server/src/main/java/org/opensearch/index/IndexSettings.java +++ b/server/src/main/java/org/opensearch/index/IndexSettings.java @@ -733,6 +733,15 @@ public static IndexMergePolicy fromString(String text) { Property.IndexScope ); + public static final Setting INDEX_CONTEXT_CURRENT_VERSION = Setting.longSetting( + "index.context.current_version", + 0, + 0, + Property.PrivateIndex, + Property.Dynamic, + Property.IndexScope + ); + private final Index index; private final Version version; private final Logger logger;