Skip to content

Commit

Permalink
Add settings validation
Browse files Browse the repository at this point in the history 
Signed-off-by: Mohit Godwani <[email protected]>
  • Loading branch information
@mgodwan
mgodwan committed Aug 21, 2024
1 parent 6b43e0b commit 6a6b51f
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 9 deletions.
26 changes: 23 additions & 3 deletions server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand All @@ -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<String> validateOverlap(Set<String> 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.
Expand Down Expand Up @@ -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(),
Expand Down
17 changes: 11 additions & 6 deletions server/src/main/java/org/opensearch/cluster/metadata/MetadataIndexTemplateService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,7 @@ static void validateNotInUse(Metadata metadata, String templateNameOrWildcard) {
final Set<String> componentsBeingUsed = new HashSet<>();
final List<String> templatesStillUsing = metadata.templatesV2().entrySet().stream().filter(e -> {
Set<String> 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);
}
Expand Down Expand Up @@ -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."
Expand All @@ -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;
}
Expand Down Expand Up @@ -1246,7 +1251,7 @@ public static List<CompressedXContent> 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)
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -1367,7 +1372,7 @@ public static List<Map<String, AliasMetadata>> 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);
}
Expand Down
18 changes: 18 additions & 0 deletions server/src/main/java/org/opensearch/cluster/metadata/MetadataUpdateSettingsService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -196,6 +200,7 @@ public ClusterState execute(ClusterState currentState) {
Set<Index> openIndices = new HashSet<>();
Set<Index> closeIndices = new HashSet<>();
final String[] actualIndices = new String[request.indices().length];
final List<String> validationErrors = new ArrayList<>();
for (int i = 0; i < request.indices().length; i++) {
Index index = request.indices()[i];
actualIndices[i] = index.getName();
Expand All @@ -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()) {
Expand Down
1 change: 1 addition & 0 deletions server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) -> {
Expand Down
9 changes: 9 additions & 0 deletions server/src/main/java/org/opensearch/index/IndexSettings.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -733,6 +733,15 @@ public static IndexMergePolicy fromString(String text) {
Property.IndexScope
);

public static final Setting<Long> 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;
Expand Down

0 comments on commit 6a6b51f

Please sign in to comment.