From 95138bfd5d1d097785adb2719a6713951c4a48c7 Mon Sep 17 00:00:00 2001 From: Mohit Godwani Date: Tue, 16 Jul 2024 19:10:04 +0530 Subject: [PATCH 1/6] Template creation using context Signed-off-by: Mohit Godwani --- CHANGELOG.md | 1 + .../TransportSimulateIndexTemplateAction.java | 3 +- .../post/TransportSimulateTemplateAction.java | 3 +- .../SystemTemplateMetadata.java | 9 +- .../SystemTemplatesService.java | 2 +- .../metadata/ComposableIndexTemplate.java | 32 +++- .../opensearch/cluster/metadata/Context.java | 110 ++++++++++++++ .../MetadataIndexTemplateService.java | 143 +++++++++++++++++- .../SystemTemplatesServiceTests.java | 42 +++-- .../MetadataIndexTemplateServiceTests.java | 47 +++++- 10 files changed, 359 insertions(+), 33 deletions(-) create mode 100644 server/src/main/java/org/opensearch/cluster/metadata/Context.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 0417cc14ee86f..49eb421dc9206 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Add matchesPluginSystemIndexPattern to SystemIndexRegistry ([#14750](https://github.com/opensearch-project/OpenSearch/pull/14750)) - Add Plugin interface for loading application based configuration templates (([#14659](https://github.com/opensearch-project/OpenSearch/issues/14659))) - Refactor remote-routing-table service inline with remote state interfaces([#14668](https://github.com/opensearch-project/OpenSearch/pull/14668)) +- Add logic to create index templates (v2) using context field ([#14811](https://github.com/opensearch-project/OpenSearch/pull/14811)) ### Dependencies - Bump `org.gradle.test-retry` from 1.5.8 to 1.5.9 ([#13442](https://github.com/opensearch-project/OpenSearch/pull/13442)) diff --git a/server/src/main/java/org/opensearch/action/admin/indices/template/post/TransportSimulateIndexTemplateAction.java b/server/src/main/java/org/opensearch/action/admin/indices/template/post/TransportSimulateIndexTemplateAction.java index c1a02d813ffb2..22f1831a54164 100644 --- a/server/src/main/java/org/opensearch/action/admin/indices/template/post/TransportSimulateIndexTemplateAction.java +++ b/server/src/main/java/org/opensearch/action/admin/indices/template/post/TransportSimulateIndexTemplateAction.java @@ -140,7 +140,8 @@ protected void clusterManagerOperation( MetadataIndexTemplateService.validateV2TemplateRequest( state.metadata(), simulateTemplateToAdd, - request.getIndexTemplateRequest().indexTemplate() + request.getIndexTemplateRequest().indexTemplate(), + clusterService.getClusterSettings() ); stateWithTemplate = indexTemplateService.addIndexTemplateV2( state, diff --git a/server/src/main/java/org/opensearch/action/admin/indices/template/post/TransportSimulateTemplateAction.java b/server/src/main/java/org/opensearch/action/admin/indices/template/post/TransportSimulateTemplateAction.java index 6565896fd3db2..03190445647ad 100644 --- a/server/src/main/java/org/opensearch/action/admin/indices/template/post/TransportSimulateTemplateAction.java +++ b/server/src/main/java/org/opensearch/action/admin/indices/template/post/TransportSimulateTemplateAction.java @@ -134,7 +134,8 @@ protected void clusterManagerOperation( MetadataIndexTemplateService.validateV2TemplateRequest( state.metadata(), simulateTemplateToAdd, - request.getIndexTemplateRequest().indexTemplate() + request.getIndexTemplateRequest().indexTemplate(), + clusterService.getClusterSettings() ); stateWithTemplate = indexTemplateService.addIndexTemplateV2( state, diff --git a/server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplateMetadata.java b/server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplateMetadata.java index 9bbe27ac0e281..12dfadc9e8d43 100644 --- a/server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplateMetadata.java +++ b/server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplateMetadata.java @@ -48,13 +48,14 @@ public long version() { * @return Metadata object based on name */ public static SystemTemplateMetadata fromComponentTemplate(String fullyQualifiedName) { - assert fullyQualifiedName.length() > 1 : "System template name must have at least one component"; - assert fullyQualifiedName.substring(1, fullyQualifiedName.indexOf(DELIMITER, 1)).equals(COMPONENT_TEMPLATE_TYPE); + assert fullyQualifiedName.length() > DELIMITER.length() * 3 + 2 + COMPONENT_TEMPLATE_TYPE.length() + : "System template name must have all defined components"; + assert (DELIMITER + fullyQualifiedName.substring(1, fullyQualifiedName.indexOf(DELIMITER, 1))).equals(COMPONENT_TEMPLATE_TYPE); return new SystemTemplateMetadata( - Long.parseLong(fullyQualifiedName.substring(fullyQualifiedName.lastIndexOf(DELIMITER))), + Long.parseLong(fullyQualifiedName.substring(fullyQualifiedName.lastIndexOf(DELIMITER) + 1)), COMPONENT_TEMPLATE_TYPE, - fullyQualifiedName.substring(0, fullyQualifiedName.lastIndexOf(DELIMITER)) + fullyQualifiedName.substring(fullyQualifiedName.indexOf(DELIMITER, 2) + 1, fullyQualifiedName.lastIndexOf(DELIMITER)) ); } diff --git a/server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplatesService.java b/server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplatesService.java index ccb9272fa57b1..90652192e5c28 100644 --- a/server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplatesService.java +++ b/server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplatesService.java @@ -85,7 +85,7 @@ void refreshTemplates(boolean verification) { int failedLoadingRepositories = 0; List exceptions = new ArrayList<>(); - if (loaded.compareAndSet(false, true) && enabledTemplates) { + if ((verification || loaded.compareAndSet(false, true)) && enabledTemplates) { for (SystemTemplatesPlugin plugin : systemTemplatesPluginList) { try (SystemTemplateRepository repository = plugin.loadRepository()) { diff --git a/server/src/main/java/org/opensearch/cluster/metadata/ComposableIndexTemplate.java b/server/src/main/java/org/opensearch/cluster/metadata/ComposableIndexTemplate.java index e7f1b97f28842..619e4e2b21844 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/ComposableIndexTemplate.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/ComposableIndexTemplate.java @@ -75,6 +75,7 @@ public class ComposableIndexTemplate extends AbstractDiffable PARSER = new ConstructingObjectParser<>( @@ -87,7 +88,8 @@ public class ComposableIndexTemplate extends AbstractDiffable) a[5], - (DataStreamTemplate) a[6] + (DataStreamTemplate) a[6], + (Context) a[7] ) ); @@ -99,6 +101,7 @@ public class ComposableIndexTemplate extends AbstractDiffable p.map(), METADATA); PARSER.declareObject(ConstructingObjectParser.optionalConstructorArg(), DataStreamTemplate.PARSER, DATA_STREAM); + PARSER.declareObject(ConstructingObjectParser.optionalConstructorArg(), Context.PARSER, CONTEXT); } private final List indexPatterns; @@ -114,6 +117,8 @@ public class ComposableIndexTemplate extends AbstractDiffable metadata; @Nullable private final DataStreamTemplate dataStreamTemplate; + @Nullable + private final Context context; static Diff readITV2DiffFrom(StreamInput in) throws IOException { return AbstractDiffable.readDiffFrom(ComposableIndexTemplate::new, in); @@ -131,7 +136,7 @@ public ComposableIndexTemplate( @Nullable Long version, @Nullable Map metadata ) { - this(indexPatterns, template, componentTemplates, priority, version, metadata, null); + this(indexPatterns, template, componentTemplates, priority, version, metadata, null, null); } public ComposableIndexTemplate( @@ -142,6 +147,19 @@ public ComposableIndexTemplate( @Nullable Long version, @Nullable Map metadata, @Nullable DataStreamTemplate dataStreamTemplate + ) { + this(indexPatterns, template, componentTemplates, priority, version, metadata, dataStreamTemplate, null); + } + + public ComposableIndexTemplate( + List indexPatterns, + @Nullable Template template, + @Nullable List componentTemplates, + @Nullable Long priority, + @Nullable Long version, + @Nullable Map metadata, + @Nullable DataStreamTemplate dataStreamTemplate, + @Nullable Context context ) { this.indexPatterns = indexPatterns; this.template = template; @@ -150,6 +168,7 @@ public ComposableIndexTemplate( this.version = version; this.metadata = metadata; this.dataStreamTemplate = dataStreamTemplate; + this.context = context; } public ComposableIndexTemplate(StreamInput in) throws IOException { @@ -164,6 +183,7 @@ public ComposableIndexTemplate(StreamInput in) throws IOException { this.version = in.readOptionalVLong(); this.metadata = in.readMap(); this.dataStreamTemplate = in.readOptionalWriteable(DataStreamTemplate::new); + this.context = in.readOptionalWriteable(Context::new); } public List indexPatterns() { @@ -205,6 +225,10 @@ public DataStreamTemplate getDataStreamTemplate() { return dataStreamTemplate; } + public Context context() { + return context; + } + @Override public void writeTo(StreamOutput out) throws IOException { out.writeStringCollection(this.indexPatterns); @@ -219,6 +243,7 @@ public void writeTo(StreamOutput out) throws IOException { out.writeOptionalVLong(this.version); out.writeMap(this.metadata); out.writeOptionalWriteable(dataStreamTemplate); + out.writeOptionalWriteable(context); } @Override @@ -243,6 +268,9 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws if (this.dataStreamTemplate != null) { builder.field(DATA_STREAM.getPreferredName(), dataStreamTemplate); } + if (this.context != null) { + builder.field(CONTEXT.getPreferredName(), context); + } builder.endObject(); return builder; } diff --git a/server/src/main/java/org/opensearch/cluster/metadata/Context.java b/server/src/main/java/org/opensearch/cluster/metadata/Context.java new file mode 100644 index 0000000000000..8e63b1f7341d6 --- /dev/null +++ b/server/src/main/java/org/opensearch/cluster/metadata/Context.java @@ -0,0 +1,110 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.cluster.metadata; + +import org.opensearch.cluster.AbstractDiffable; +import org.opensearch.common.annotation.ExperimentalApi; +import org.opensearch.core.ParseField; +import org.opensearch.core.common.io.stream.StreamInput; +import org.opensearch.core.common.io.stream.StreamOutput; +import org.opensearch.core.xcontent.ConstructingObjectParser; +import org.opensearch.core.xcontent.ToXContentObject; +import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.core.xcontent.XContentParser; + +import java.io.IOException; +import java.util.Map; + +/** + * Class encapsulating the context metadata associated with an index template/index. + */ +@ExperimentalApi +public class Context extends AbstractDiffable implements ToXContentObject { + + private static final ParseField NAME = new ParseField("name"); + private static final ParseField VERSION = new ParseField("version"); + private static final ParseField PARAMS = new ParseField("params"); + + public static final String LATEST_VERSION = "_latest"; + + private String name; + private String version = LATEST_VERSION; + private Map params; + + public static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>( + "index_template", + false, + a -> new Context((String) a[0], (String) a[1], (Map) a[2]) + ); + + static { + PARSER.declareString(ConstructingObjectParser.constructorArg(), NAME); + PARSER.declareString(ConstructingObjectParser.optionalConstructorArg(), VERSION); + PARSER.declareObject(ConstructingObjectParser.optionalConstructorArg(), (p, c) -> p.map(), PARAMS); + } + + public Context(String name, String version, Map params) { + this.name = name; + this.version = version; + this.params = params; + } + + public Context(StreamInput in) throws IOException { + this.name = in.readString(); + this.version = in.readOptionalString(); + this.params = in.readMap(); + } + + public String name() { + return name; + } + + public void name(String name) { + this.name = name; + } + + public String version() { + return version; + } + + public void version(String version) { + this.version = version; + } + + public Map params() { + return params; + } + + public void params(Map params) { + this.params = params; + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + out.writeString(name); + out.writeOptionalString(version); + out.writeMap(params); + } + + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + builder.startObject(); + builder.field(NAME.getPreferredName(), this.name); + builder.field("version", this.version); + if (params != null) { + builder.field("params", this.params); + } + builder.endObject(); + return builder; + } + + public static Context fromXContent(XContentParser parser) { + return PARSER.apply(parser, null); + } +} 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 5b03d3f7b19ce..7580f48656a2f 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/MetadataIndexTemplateService.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/MetadataIndexTemplateService.java @@ -42,6 +42,9 @@ import org.opensearch.action.support.master.AcknowledgedResponse; import org.opensearch.cluster.ClusterState; import org.opensearch.cluster.ClusterStateUpdateTask; +import org.opensearch.cluster.applicationtemplates.ClusterStateSystemTemplateLoader; +import org.opensearch.cluster.applicationtemplates.SystemTemplateMetadata; +import org.opensearch.cluster.applicationtemplates.SystemTemplatesService; import org.opensearch.cluster.service.ClusterManagerTaskKeys; import org.opensearch.cluster.service.ClusterManagerTaskThrottler; import org.opensearch.cluster.service.ClusterService; @@ -53,7 +56,9 @@ import org.opensearch.common.inject.Inject; import org.opensearch.common.logging.HeaderWarning; import org.opensearch.common.regex.Regex; +import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.IndexScopedSettings; +import org.opensearch.common.settings.SecureSettings; import org.opensearch.common.settings.Settings; import org.opensearch.common.unit.TimeValue; import org.opensearch.common.util.set.Sets; @@ -72,6 +77,7 @@ import org.opensearch.indices.IndexTemplateMissingException; import org.opensearch.indices.IndicesService; import org.opensearch.indices.InvalidIndexTemplateException; +import org.opensearch.threadpool.ThreadPool; import java.io.IOException; import java.io.UncheckedIOException; @@ -94,6 +100,7 @@ import static org.opensearch.cluster.metadata.MetadataCreateDataStreamService.validateTimestampFieldMapping; import static org.opensearch.cluster.metadata.MetadataCreateIndexService.validateRefreshIntervalSettings; +import static org.opensearch.common.util.concurrent.ThreadContext.ACTION_ORIGIN_TRANSIENT_NAME; import static org.opensearch.indices.cluster.IndicesClusterStateService.AllocatedIndices.IndexRemovalReason.NO_LONGER_ASSIGNED; /** @@ -116,6 +123,7 @@ public class MetadataIndexTemplateService { private final ClusterManagerTaskThrottler.ThrottlingKey removeIndexTemplateV2TaskKey; private final ClusterManagerTaskThrottler.ThrottlingKey createComponentTemplateTaskKey; private final ClusterManagerTaskThrottler.ThrottlingKey removeComponentTemplateTaskKey; + private final ThreadPool threadPool; @Inject public MetadataIndexTemplateService( @@ -124,7 +132,8 @@ public MetadataIndexTemplateService( AliasValidator aliasValidator, IndicesService indicesService, IndexScopedSettings indexScopedSettings, - NamedXContentRegistry xContentRegistry + NamedXContentRegistry xContentRegistry, + ThreadPool threadPool ) { this.clusterService = clusterService; this.aliasValidator = aliasValidator; @@ -132,6 +141,7 @@ public MetadataIndexTemplateService( this.metadataCreateIndexService = metadataCreateIndexService; this.indexScopedSettings = indexScopedSettings; this.xContentRegistry = xContentRegistry; + this.threadPool = threadPool; // Task is onboarded for throttling, it will get retried from associated TransportClusterManagerNodeAction. createIndexTemplateTaskKey = clusterService.registerClusterManagerTask(ClusterManagerTaskKeys.CREATE_INDEX_TEMPLATE_KEY, true); @@ -209,6 +219,7 @@ public void putComponentTemplate( final ComponentTemplate template, final ActionListener listener ) { + validateComponentTemplateRequest(template); clusterService.submitStateUpdateTask( "create-component-template [" + name + "], cause [" + cause + "]", new ClusterStateUpdateTask(Priority.URGENT) { @@ -378,6 +389,7 @@ public void removeComponentTemplate( final ActionListener listener ) { validateNotInUse(clusterService.state().metadata(), name); + validateComponentTemplateRequest(clusterService.state().metadata().componentTemplates().get(name)); clusterService.submitStateUpdateTask("remove-component-template [" + name + "]", new ClusterStateUpdateTask(Priority.URGENT) { @Override @@ -439,7 +451,12 @@ static void validateNotInUse(Metadata metadata, String templateNameOrWildcard) { .collect(Collectors.toSet()); final Set componentsBeingUsed = new HashSet<>(); final List templatesStillUsing = metadata.templatesV2().entrySet().stream().filter(e -> { - Set intersecting = Sets.intersection(new HashSet<>(e.getValue().composedOf()), matchingComponentTemplates); + Set referredComponentTemplates = new HashSet<>(e.getValue().composedOf()); + String systemTemplateUsed = findContextTemplate(metadata, e.getValue().context()); + if (systemTemplateUsed != null) { + referredComponentTemplates.add(systemTemplateUsed); + } + Set intersecting = Sets.intersection(referredComponentTemplates, matchingComponentTemplates); if (intersecting.size() > 0) { componentsBeingUsed.addAll(intersecting); return true; @@ -469,7 +486,7 @@ public void putIndexTemplateV2( final ComposableIndexTemplate template, final ActionListener listener ) { - validateV2TemplateRequest(clusterService.state().metadata(), name, template); + validateV2TemplateRequest(clusterService.state().metadata(), name, template, clusterService.getClusterSettings()); clusterService.submitStateUpdateTask( "create-index-template-v2 [" + name + "], cause [" + cause + "]", new ClusterStateUpdateTask(Priority.URGENT) { @@ -502,7 +519,12 @@ public void clusterStateProcessed(String source, ClusterState oldState, ClusterS ); } - public static void validateV2TemplateRequest(Metadata metadata, String name, ComposableIndexTemplate template) { + public static void validateV2TemplateRequest( + Metadata metadata, + String name, + ComposableIndexTemplate template, + ClusterSettings settings + ) { if (template.indexPatterns().stream().anyMatch(Regex::isMatchAllPattern)) { Settings mergedSettings = resolveSettings(metadata, template); if (IndexMetadata.INDEX_HIDDEN_SETTING.exists(mergedSettings)) { @@ -514,6 +536,8 @@ public static void validateV2TemplateRequest(Metadata metadata, String name, Com } final Map componentTemplates = metadata.componentTemplates(); + final boolean isContextAllowed = settings.get(SystemTemplatesService.SETTING_APPLICATION_BASED_CONFIGURATION_TEMPLATES_ENABLED); + final List missingComponentTemplates = template.composedOf() .stream() .filter(componentTemplate -> componentTemplates.containsKey(componentTemplate) == false) @@ -525,6 +549,78 @@ public static void validateV2TemplateRequest(Metadata metadata, String name, Com "index template [" + name + "] specifies component templates " + missingComponentTemplates + " that do not exist" ); } + + if (template.context() != null && !isContextAllowed) { + throw new InvalidIndexTemplateException( + name, + "index template [" + + name + + "] specifies a context which cannot be used without enabling: " + + SystemTemplatesService.SETTING_APPLICATION_BASED_CONFIGURATION_TEMPLATES_ENABLED.getKey() + ); + } + + if (isContextAllowed + && template.composedOf().stream().anyMatch(componentTemplate -> isSystemTemplate(componentTemplates.get(componentTemplate)))) { + throw new InvalidIndexTemplateException( + name, + "index template [" + name + "] specifies a component templates which can only be used in context." + ); + } + + if (template.context() != null && findContextTemplate(metadata, template.context()) == null) { + throw new InvalidIndexTemplateException( + name, + "index template [" + name + "] specifies a context which is not loaded on the cluster." + ); + } + } + + private void validateComponentTemplateRequest(ComponentTemplate componentTemplate) { + if (isSystemTemplate(componentTemplate) + && !ClusterStateSystemTemplateLoader.TEMPLATE_LOADER_IDENTIFIER.equals( + threadPool.getThreadContext().getTransient(ACTION_ORIGIN_TRANSIENT_NAME) + )) { + throw new IllegalArgumentException("A system template can only be created with a repository"); + } + } + + private static String findContextTemplate(Metadata metadata, Context context) { + if (context == null) { + return null; + } + long maxVersionSeen = -1L; + final boolean searchSpecificVersion = !Context.LATEST_VERSION.equals(context.version()); + + String systemTemplateToUse = null; + for (Map.Entry entry : metadata.componentTemplates().entrySet()) { + if (isSystemTemplate(entry.getValue())) { + SystemTemplateMetadata templateMetadata = SystemTemplateMetadata.fromComponentTemplate(entry.getKey()); + if (templateMetadata.name().equals(context.name())) { + if (!searchSpecificVersion && maxVersionSeen < templateMetadata.version()) { + maxVersionSeen = templateMetadata.version(); + systemTemplateToUse = entry.getKey(); + } else if (searchSpecificVersion && templateMetadata.version() == Long.parseLong(context.version())) { + systemTemplateToUse = entry.getKey(); + } + } + } + } + return systemTemplateToUse; + } + + private static boolean isSystemTemplate(ComponentTemplate componentTemplate) { + return Optional.ofNullable(componentTemplate) + .map(ComponentTemplate::metadata) + .map(md -> md.get(ClusterStateSystemTemplateLoader.TEMPLATE_TYPE_KEY)) + .filter(ob -> SystemTemplateMetadata.COMPONENT_TEMPLATE_TYPE.equals(ob.toString())) + .isPresent(); + } + + private static boolean hasOverlap(Settings.Builder userCollection, Settings baseSettings) { + final Set userSet = new HashSet<>(userCollection.keys()); + Optional.ofNullable(userCollection.getSecureSettings()).map(SecureSettings::getSettingNames).ifPresent(userSet::addAll); + return !Sets.intersection(userSet, baseSettings.names()).isEmpty(); } public ClusterState addIndexTemplateV2( @@ -613,7 +709,8 @@ public ClusterState addIndexTemplateV2( template.priority(), template.version(), template.metadata(), - template.getDataStreamTemplate() + template.getDataStreamTemplate(), + template.context() ); } @@ -1140,7 +1237,7 @@ public static List collectMappings(final ClusterState state, .map(Template::mappings) .filter(Objects::nonNull) .collect(Collectors.toCollection(LinkedList::new)); - // Add the actual index template's mappings, since it takes the highest precedence + // Add the actual index template's mappings, since it takes the next precedence Optional.ofNullable(template.template()).map(Template::mappings).ifPresent(mappings::add); if (template.getDataStreamTemplate() != null && indexName.startsWith(DataStream.BACKING_INDEX_PREFIX)) { // add a default mapping for the timestamp field, at the lowest precedence, to make bootstrapping data streams more @@ -1165,6 +1262,15 @@ public static List collectMappings(final ClusterState state, }) .ifPresent(mappings::add); } + + // Now use context mappings which take the highest precedence + Optional.ofNullable(template.context()) + .map(ctx -> findContextTemplate(state.metadata(), ctx)) + .map(name -> state.metadata().componentTemplates().get(name)) + .map(ComponentTemplate::template) + .map(Template::mappings) + .ifPresent(mappings::add); + return Collections.unmodifiableList(mappings); } @@ -1226,8 +1332,21 @@ private static Settings resolveSettings(Metadata metadata, ComposableIndexTempla Settings.Builder templateSettings = Settings.builder(); componentSettings.forEach(templateSettings::put); - // Add the actual index template's settings to the end, since it takes the highest precedence. + // Add the actual index template's settings now, since it takes the next precedence. 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 ComponentTemplate componentTemplate = metadata.componentTemplates().get(systemTemplate); + Optional.ofNullable(componentTemplate).map(ComponentTemplate::template).map(Template::settings).ifPresent(s -> { + if (hasOverlap(templateSettings, s)) { + throw new IllegalArgumentException( + "Template override settings which are declared in " + "template referred through context" + ); + } + templateSettings.put(s); + }); + return templateSettings.build(); } @@ -1269,8 +1388,16 @@ public static List> resolveAliases(final Metadata met .filter(Objects::nonNull) .collect(Collectors.toList()); - // Add the actual index template's aliases to the end if they exist + // Add the actual index template's aliases now if they exist Optional.ofNullable(template.template()).map(Template::aliases).ifPresent(aliases::add); + + // Now use context referenced template's aliases which take the highest precedence + if (template.context() != null) { + final String systemTemplate = findContextTemplate(metadata, template.context()); + final ComponentTemplate componentTemplate = metadata.componentTemplates().get(systemTemplate); + Optional.ofNullable(componentTemplate.template()).map(Template::aliases).ifPresent(aliases::add); + } + // Aliases are applied in order, but subsequent alias configuration from the same name is // ignored, so in order for the order to be correct, alias configuration should be in order // of precedence (with the index template first) diff --git a/server/src/test/java/org/opensearch/cluster/applicationtemplates/SystemTemplatesServiceTests.java b/server/src/test/java/org/opensearch/cluster/applicationtemplates/SystemTemplatesServiceTests.java index 4addf3802b40d..affb017264fdf 100644 --- a/server/src/test/java/org/opensearch/cluster/applicationtemplates/SystemTemplatesServiceTests.java +++ b/server/src/test/java/org/opensearch/cluster/applicationtemplates/SystemTemplatesServiceTests.java @@ -32,39 +32,51 @@ public class SystemTemplatesServiceTests extends OpenSearchTestCase { public void testSystemTemplatesLoaded() throws IOException { setupService(true); - systemTemplatesService.onClusterManager(); - SystemTemplatesService.Stats stats = systemTemplatesService.stats(); - assertNotNull(stats); - assertEquals(stats.getTemplatesLoaded(), 1L); - assertEquals(stats.getFailedLoadingTemplates(), 0L); - assertEquals(stats.getFailedLoadingRepositories(), 1L); + // First time load should happen, second time should short circuit. + for (int iter = 1; iter <= 2; iter++) { + systemTemplatesService.onClusterManager(); + SystemTemplatesService.Stats stats = systemTemplatesService.stats(); + assertNotNull(stats); + assertEquals(stats.getTemplatesLoaded(), iter % 2); + assertEquals(stats.getFailedLoadingTemplates(), 0L); + assertEquals(stats.getFailedLoadingRepositories(), iter % 2); + } } - public void testSystemTemplatesVerify() throws IOException { + public void testSystemTemplatesVerifyAndLoad() throws IOException { setupService(false); systemTemplatesService.verifyRepositories(); - SystemTemplatesService.Stats stats = systemTemplatesService.stats(); assertNotNull(stats); assertEquals(stats.getTemplatesLoaded(), 0L); assertEquals(stats.getFailedLoadingTemplates(), 0L); assertEquals(stats.getFailedLoadingRepositories(), 0L); + + systemTemplatesService.onClusterManager(); + stats = systemTemplatesService.stats(); + assertNotNull(stats); + assertEquals(stats.getTemplatesLoaded(), 1L); + assertEquals(stats.getFailedLoadingTemplates(), 0L); + assertEquals(stats.getFailedLoadingRepositories(), 0L); } public void testSystemTemplatesVerifyWithFailingRepository() throws IOException { setupService(true); - assertThrows(IllegalStateException.class, () -> systemTemplatesService.verifyRepositories()); + // Do it multiple times to ensure verify checks are always executed. + for (int i = 0; i < 2; i++) { + assertThrows(IllegalStateException.class, () -> systemTemplatesService.verifyRepositories()); - SystemTemplatesService.Stats stats = systemTemplatesService.stats(); - assertNotNull(stats); - assertEquals(stats.getTemplatesLoaded(), 0L); - assertEquals(stats.getFailedLoadingTemplates(), 0L); - assertEquals(stats.getFailedLoadingRepositories(), 1L); + SystemTemplatesService.Stats stats = systemTemplatesService.stats(); + assertNotNull(stats); + assertEquals(stats.getTemplatesLoaded(), 0L); + assertEquals(stats.getFailedLoadingTemplates(), 0L); + assertEquals(stats.getFailedLoadingRepositories(), 1L); + } } - void setupService(boolean errorFromMockPlugin) throws IOException { + private void setupService(boolean errorFromMockPlugin) throws IOException { FeatureFlags.initializeFeatureFlags(Settings.builder().put(FeatureFlags.APPLICATION_BASED_CONFIGURATION_TEMPLATES, true).build()); ThreadPool mockPool = Mockito.mock(ThreadPool.class); diff --git a/server/src/test/java/org/opensearch/cluster/metadata/MetadataIndexTemplateServiceTests.java b/server/src/test/java/org/opensearch/cluster/metadata/MetadataIndexTemplateServiceTests.java index 6643d6e13289b..71ee4865f397a 100644 --- a/server/src/test/java/org/opensearch/cluster/metadata/MetadataIndexTemplateServiceTests.java +++ b/server/src/test/java/org/opensearch/cluster/metadata/MetadataIndexTemplateServiceTests.java @@ -656,6 +656,50 @@ public void onFailure(Exception e) { ); } + public void testPutGlobalV2TemplateWhichProvidesContextWithContextDisabled() throws Exception { + + } + + public void testPutGlobalV2TemplateWhichProvidesContextNotPresentInState() throws Exception { + + } + + public void testPutGlobalV2TemplateWhichProvidesContextWithNonExistingVersion() throws Exception { + + } + + public void testPutGlobalV2TemplateWhichProvidesContextInComposedOfSection() throws Exception { + + } + + public void testPutGlobalV2TemplateWhichProvidesContextWithSpecificVersion() throws Exception { + + } + + public void testPutGlobalV2TemplateWhichProvidesContextWithLatestVersion() throws Exception { + + } + + public void testRemoveSystemTemplateViaUnknownSource() throws Exception { + + } + + public void testRemoveSystemTemplateInUse() throws Exception { + + } + + public void testRemoveSystemTemplateNotInUse() throws Exception { + + } + + public void testPutSystemTemplateViaUnknownSource() throws Exception { + + } + + public void testPutSystemTemplate() throws Exception { + + } + /** * Test that if we have a pre-existing v2 template and put a "*" v1 template, we generate a warning */ @@ -2067,7 +2111,8 @@ private static List putTemplate(NamedXContentRegistry xContentRegistr new AliasValidator(), null, new IndexScopedSettings(Settings.EMPTY, IndexScopedSettings.BUILT_IN_INDEX_SETTINGS), - xContentRegistry + xContentRegistry, + null ); final List throwables = new ArrayList<>(); From 9b1f5e5c11b5592f21ea7478a378a74379a8a15f Mon Sep 17 00:00:00 2001 From: Mohit Godwani Date: Thu, 18 Jul 2024 21:06:16 +0530 Subject: [PATCH 2/6] Address PR comments Signed-off-by: Mohit Godwani --- .../SystemTemplateMetadata.java | 24 +++++++++++++++++++ .../TemplateRepositoryMetadata.java | 23 ++++++++++++++++++ .../metadata/ComposableIndexTemplate.java | 15 ++++++++---- .../opensearch/cluster/metadata/Context.java | 18 ++++++++++++++ .../MetadataIndexTemplateService.java | 2 +- 5 files changed, 77 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplateMetadata.java b/server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplateMetadata.java index 12dfadc9e8d43..bb1996727d16d 100644 --- a/server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplateMetadata.java +++ b/server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplateMetadata.java @@ -10,6 +10,8 @@ import org.opensearch.common.annotation.ExperimentalApi; +import java.util.Objects; + /** * Metadata information about a template available in a template repository. */ @@ -66,4 +68,26 @@ public static SystemTemplateMetadata fromComponentTemplateInfo(String name, long public final String fullyQualifiedName() { return type + DELIMITER + name + DELIMITER + version; } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + SystemTemplateMetadata that = (SystemTemplateMetadata) o; + return version == that.version && Objects.equals(type, that.type) && Objects.equals(name, that.name); + } + + @Override + public int hashCode() { + return Objects.hash(version, type, name); + } + + @Override + public String toString() { + return "SystemTemplateMetadata{" + + "version=" + version + + ", type='" + type + '\'' + + ", name='" + name + '\'' + + '}'; + } } diff --git a/server/src/main/java/org/opensearch/cluster/applicationtemplates/TemplateRepositoryMetadata.java b/server/src/main/java/org/opensearch/cluster/applicationtemplates/TemplateRepositoryMetadata.java index 7ab4553aade0e..c0457d57acccc 100644 --- a/server/src/main/java/org/opensearch/cluster/applicationtemplates/TemplateRepositoryMetadata.java +++ b/server/src/main/java/org/opensearch/cluster/applicationtemplates/TemplateRepositoryMetadata.java @@ -10,6 +10,8 @@ import org.opensearch.common.annotation.ExperimentalApi; +import java.util.Objects; + /** * The information to uniquely identify a template repository. */ @@ -31,4 +33,25 @@ public String id() { public long version() { return version; } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + TemplateRepositoryMetadata that = (TemplateRepositoryMetadata) o; + return version == that.version && Objects.equals(id, that.id); + } + + @Override + public int hashCode() { + return Objects.hash(id, version); + } + + @Override + public String toString() { + return "TemplateRepositoryMetadata{" + + "id='" + id + '\'' + + ", version=" + version + + '}'; + } } diff --git a/server/src/main/java/org/opensearch/cluster/metadata/ComposableIndexTemplate.java b/server/src/main/java/org/opensearch/cluster/metadata/ComposableIndexTemplate.java index 619e4e2b21844..d5304cb14f66c 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/ComposableIndexTemplate.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/ComposableIndexTemplate.java @@ -32,6 +32,7 @@ package org.opensearch.cluster.metadata; +import org.opensearch.Version; import org.opensearch.cluster.AbstractDiffable; import org.opensearch.cluster.Diff; import org.opensearch.cluster.metadata.DataStream.TimestampField; @@ -183,7 +184,9 @@ public ComposableIndexTemplate(StreamInput in) throws IOException { this.version = in.readOptionalVLong(); this.metadata = in.readMap(); this.dataStreamTemplate = in.readOptionalWriteable(DataStreamTemplate::new); - this.context = in.readOptionalWriteable(Context::new); + if (in.getVersion().onOrAfter(Version.V_3_0_0)) { + this.context = in.readOptionalWriteable(Context::new); + } } public List indexPatterns() { @@ -243,7 +246,9 @@ public void writeTo(StreamOutput out) throws IOException { out.writeOptionalVLong(this.version); out.writeMap(this.metadata); out.writeOptionalWriteable(dataStreamTemplate); - out.writeOptionalWriteable(context); + if (out.getVersion().onOrAfter(Version.V_3_0_0)) { + out.writeOptionalWriteable(context); + } } @Override @@ -284,7 +289,8 @@ public int hashCode() { this.priority, this.version, this.metadata, - this.dataStreamTemplate + this.dataStreamTemplate, + this.context ); } @@ -303,7 +309,8 @@ public boolean equals(Object obj) { && Objects.equals(this.priority, other.priority) && Objects.equals(this.version, other.version) && Objects.equals(this.metadata, other.metadata) - && Objects.equals(this.dataStreamTemplate, other.dataStreamTemplate); + && Objects.equals(this.dataStreamTemplate, other.dataStreamTemplate) + && Objects.equals(this.context, other.context); } @Override diff --git a/server/src/main/java/org/opensearch/cluster/metadata/Context.java b/server/src/main/java/org/opensearch/cluster/metadata/Context.java index 8e63b1f7341d6..397e1e1dc1e94 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/Context.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/Context.java @@ -20,6 +20,7 @@ import java.io.IOException; import java.util.Map; +import java.util.Objects; /** * Class encapsulating the context metadata associated with an index template/index. @@ -49,6 +50,10 @@ public class Context extends AbstractDiffable implement PARSER.declareObject(ConstructingObjectParser.optionalConstructorArg(), (p, c) -> p.map(), PARAMS); } + public Context(String name) { + this(name, LATEST_VERSION, Map.of()); + } + public Context(String name, String version, Map params) { this.name = name; this.version = version; @@ -107,4 +112,17 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws public static Context fromXContent(XContentParser parser) { return PARSER.apply(parser, null); } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + Context context = (Context) o; + return Objects.equals(name, context.name) && Objects.equals(version, context.version) && Objects.equals(params, context.params); + } + + @Override + public int hashCode() { + return Objects.hash(name, version, params); + } } 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 7580f48656a2f..4feaab91acb5a 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/MetadataIndexTemplateService.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/MetadataIndexTemplateService.java @@ -581,7 +581,7 @@ private void validateComponentTemplateRequest(ComponentTemplate componentTemplat && !ClusterStateSystemTemplateLoader.TEMPLATE_LOADER_IDENTIFIER.equals( threadPool.getThreadContext().getTransient(ACTION_ORIGIN_TRANSIENT_NAME) )) { - throw new IllegalArgumentException("A system template can only be created with a repository"); + throw new IllegalArgumentException("A system template can only be created/updated/deleted with a repository"); } } From 0d69b7875d2d928959d44b045153bdaf292f5d33 Mon Sep 17 00:00:00 2001 From: Mohit Godwani Date: Fri, 19 Jul 2024 12:09:54 +0530 Subject: [PATCH 3/6] Fast lookup and tests Signed-off-by: Mohit Godwani --- .../SystemTemplateMetadata.java | 6 +- .../TemplateRepositoryMetadata.java | 5 +- .../metadata/ComposableIndexTemplate.java | 2 + .../opensearch/cluster/metadata/Context.java | 4 +- .../opensearch/cluster/metadata/Metadata.java | 24 + .../MetadataIndexTemplateService.java | 42 +- .../MetadataIndexTemplateServiceTests.java | 432 +++++++++++++++++- 7 files changed, 458 insertions(+), 57 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplateMetadata.java b/server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplateMetadata.java index bb1996727d16d..227b70ffa2ef5 100644 --- a/server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplateMetadata.java +++ b/server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplateMetadata.java @@ -84,10 +84,6 @@ public int hashCode() { @Override public String toString() { - return "SystemTemplateMetadata{" + - "version=" + version + - ", type='" + type + '\'' + - ", name='" + name + '\'' + - '}'; + return "SystemTemplateMetadata{" + "version=" + version + ", type='" + type + '\'' + ", name='" + name + '\'' + '}'; } } diff --git a/server/src/main/java/org/opensearch/cluster/applicationtemplates/TemplateRepositoryMetadata.java b/server/src/main/java/org/opensearch/cluster/applicationtemplates/TemplateRepositoryMetadata.java index c0457d57acccc..1fa79d291480b 100644 --- a/server/src/main/java/org/opensearch/cluster/applicationtemplates/TemplateRepositoryMetadata.java +++ b/server/src/main/java/org/opensearch/cluster/applicationtemplates/TemplateRepositoryMetadata.java @@ -49,9 +49,6 @@ public int hashCode() { @Override public String toString() { - return "TemplateRepositoryMetadata{" + - "id='" + id + '\'' + - ", version=" + version + - '}'; + return "TemplateRepositoryMetadata{" + "id='" + id + '\'' + ", version=" + version + '}'; } } diff --git a/server/src/main/java/org/opensearch/cluster/metadata/ComposableIndexTemplate.java b/server/src/main/java/org/opensearch/cluster/metadata/ComposableIndexTemplate.java index d5304cb14f66c..594dda83c41e2 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/ComposableIndexTemplate.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/ComposableIndexTemplate.java @@ -186,6 +186,8 @@ public ComposableIndexTemplate(StreamInput in) throws IOException { this.dataStreamTemplate = in.readOptionalWriteable(DataStreamTemplate::new); if (in.getVersion().onOrAfter(Version.V_3_0_0)) { this.context = in.readOptionalWriteable(Context::new); + } else { + this.context = null; } } diff --git a/server/src/main/java/org/opensearch/cluster/metadata/Context.java b/server/src/main/java/org/opensearch/cluster/metadata/Context.java index 397e1e1dc1e94..4bd6134e8a318 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/Context.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/Context.java @@ -56,7 +56,9 @@ public Context(String name) { public Context(String name, String version, Map params) { this.name = name; - this.version = version; + if (version != null) { + this.version = version; + } this.params = params; } diff --git a/server/src/main/java/org/opensearch/cluster/metadata/Metadata.java b/server/src/main/java/org/opensearch/cluster/metadata/Metadata.java index 2a54f6444ffda..bc9033c527476 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/Metadata.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/Metadata.java @@ -43,6 +43,7 @@ import org.opensearch.cluster.DiffableUtils; import org.opensearch.cluster.NamedDiffable; import org.opensearch.cluster.NamedDiffableValueSerializer; +import org.opensearch.cluster.applicationtemplates.SystemTemplateMetadata; import org.opensearch.cluster.block.ClusterBlock; import org.opensearch.cluster.block.ClusterBlockLevel; import org.opensearch.cluster.coordination.CoordinationMetadata; @@ -280,6 +281,8 @@ static Custom fromXContent(XContentParser parser, String name) throws IOExceptio private final SortedMap indicesLookup; + private final Map> systemTemplatesLookup; + Metadata( String clusterUUID, boolean clusterUUIDCommitted, @@ -328,6 +331,23 @@ static Custom fromXContent(XContentParser parser, String name) throws IOExceptio this.allClosedIndices = allClosedIndices; this.visibleClosedIndices = visibleClosedIndices; this.indicesLookup = indicesLookup; + + this.systemTemplatesLookup = new HashMap<>(); + Optional.ofNullable((ComponentTemplateMetadata) this.custom(ComponentTemplateMetadata.TYPE)) + .map(ComponentTemplateMetadata::componentTemplates) + .orElseGet(Collections::emptyMap) + .forEach((k, v) -> { + if (MetadataIndexTemplateService.isSystemTemplate(v)) { + SystemTemplateMetadata templateMetadata = SystemTemplateMetadata.fromComponentTemplate(k); + systemTemplatesLookup.compute(templateMetadata.name(), (ik, iv) -> { + if (iv == null) { + iv = new TreeMap<>(); + } + iv.put(templateMetadata.version(), k); + return iv; + }); + } + }); } public long version() { @@ -828,6 +848,10 @@ public Map componentTemplates() { .orElse(Collections.emptyMap()); } + public Map> systemTemplatesLookup() { + return systemTemplatesLookup; + } + public Map templatesV2() { return Optional.ofNullable((ComposableIndexTemplateMetadata) this.custom(ComposableIndexTemplateMetadata.TYPE)) .map(ComposableIndexTemplateMetadata::indexTemplates) 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 4feaab91acb5a..7bc3d279513cd 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/MetadataIndexTemplateService.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/MetadataIndexTemplateService.java @@ -58,9 +58,9 @@ import org.opensearch.common.regex.Regex; import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.IndexScopedSettings; -import org.opensearch.common.settings.SecureSettings; 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.XContentFactory; import org.opensearch.core.action.ActionListener; @@ -536,7 +536,7 @@ public static void validateV2TemplateRequest( } final Map componentTemplates = metadata.componentTemplates(); - final boolean isContextAllowed = settings.get(SystemTemplatesService.SETTING_APPLICATION_BASED_CONFIGURATION_TEMPLATES_ENABLED); + final boolean isContextAllowed = FeatureFlags.isEnabled(FeatureFlags.APPLICATION_BASED_CONFIGURATION_TEMPLATES); final List missingComponentTemplates = template.composedOf() .stream() @@ -589,27 +589,14 @@ private static String findContextTemplate(Metadata metadata, Context context) { if (context == null) { return null; } - long maxVersionSeen = -1L; final boolean searchSpecificVersion = !Context.LATEST_VERSION.equals(context.version()); - - String systemTemplateToUse = null; - for (Map.Entry entry : metadata.componentTemplates().entrySet()) { - if (isSystemTemplate(entry.getValue())) { - SystemTemplateMetadata templateMetadata = SystemTemplateMetadata.fromComponentTemplate(entry.getKey()); - if (templateMetadata.name().equals(context.name())) { - if (!searchSpecificVersion && maxVersionSeen < templateMetadata.version()) { - maxVersionSeen = templateMetadata.version(); - systemTemplateToUse = entry.getKey(); - } else if (searchSpecificVersion && templateMetadata.version() == Long.parseLong(context.version())) { - systemTemplateToUse = entry.getKey(); - } - } - } - } - return systemTemplateToUse; + return Optional.ofNullable(metadata.systemTemplatesLookup()) + .map(coll -> coll.get(context.name())) + .map(coll -> coll.get(searchSpecificVersion ? Long.parseLong(context.version()) : coll.lastKey())) + .orElse(null); } - private static boolean isSystemTemplate(ComponentTemplate componentTemplate) { + public static boolean isSystemTemplate(ComponentTemplate componentTemplate) { return Optional.ofNullable(componentTemplate) .map(ComponentTemplate::metadata) .map(md -> md.get(ClusterStateSystemTemplateLoader.TEMPLATE_TYPE_KEY)) @@ -617,12 +604,6 @@ private static boolean isSystemTemplate(ComponentTemplate componentTemplate) { .isPresent(); } - private static boolean hasOverlap(Settings.Builder userCollection, Settings baseSettings) { - final Set userSet = new HashSet<>(userCollection.keys()); - Optional.ofNullable(userCollection.getSecureSettings()).map(SecureSettings::getSettingNames).ifPresent(userSet::addAll); - return !Sets.intersection(userSet, baseSettings.names()).isEmpty(); - } - public ClusterState addIndexTemplateV2( final ClusterState currentState, final boolean create, @@ -1338,14 +1319,7 @@ private static Settings resolveSettings(Metadata metadata, ComposableIndexTempla // Add the template referred by context since it will take the highest precedence. final String systemTemplate = findContextTemplate(metadata, template.context()); final ComponentTemplate componentTemplate = metadata.componentTemplates().get(systemTemplate); - Optional.ofNullable(componentTemplate).map(ComponentTemplate::template).map(Template::settings).ifPresent(s -> { - if (hasOverlap(templateSettings, s)) { - throw new IllegalArgumentException( - "Template override settings which are declared in " + "template referred through context" - ); - } - templateSettings.put(s); - }); + Optional.ofNullable(componentTemplate).map(ComponentTemplate::template).map(Template::settings).ifPresent(templateSettings::put); return templateSettings.build(); } diff --git a/server/src/test/java/org/opensearch/cluster/metadata/MetadataIndexTemplateServiceTests.java b/server/src/test/java/org/opensearch/cluster/metadata/MetadataIndexTemplateServiceTests.java index 71ee4865f397a..f26f45b69d133 100644 --- a/server/src/test/java/org/opensearch/cluster/metadata/MetadataIndexTemplateServiceTests.java +++ b/server/src/test/java/org/opensearch/cluster/metadata/MetadataIndexTemplateServiceTests.java @@ -36,6 +36,8 @@ import org.opensearch.action.admin.indices.alias.Alias; import org.opensearch.action.support.master.AcknowledgedResponse; import org.opensearch.cluster.ClusterState; +import org.opensearch.cluster.applicationtemplates.ClusterStateSystemTemplateLoader; +import org.opensearch.cluster.applicationtemplates.SystemTemplateMetadata; import org.opensearch.cluster.metadata.MetadataIndexTemplateService.PutRequest; import org.opensearch.cluster.routing.allocation.AwarenessReplicaBalance; import org.opensearch.cluster.service.ClusterService; @@ -45,6 +47,8 @@ import org.opensearch.common.settings.Settings; import org.opensearch.common.settings.SettingsException; import org.opensearch.common.unit.TimeValue; +import org.opensearch.common.util.FeatureFlags; +import org.opensearch.common.util.concurrent.ThreadContext; import org.opensearch.common.xcontent.LoggingDeprecationHandler; import org.opensearch.common.xcontent.XContentFactory; import org.opensearch.core.action.ActionListener; @@ -53,6 +57,8 @@ import org.opensearch.core.xcontent.NamedXContentRegistry; import org.opensearch.core.xcontent.XContentParser; import org.opensearch.env.Environment; +import org.opensearch.index.codec.CodecService; +import org.opensearch.index.engine.EngineConfig; import org.opensearch.index.mapper.MapperParsingException; import org.opensearch.index.mapper.MapperService; import org.opensearch.indices.DefaultRemoteStoreSettings; @@ -62,6 +68,7 @@ import org.opensearch.indices.SystemIndices; import org.opensearch.repositories.RepositoriesService; import org.opensearch.test.OpenSearchSingleNodeTestCase; +import org.opensearch.threadpool.ThreadPool; import java.io.IOException; import java.util.ArrayList; @@ -76,10 +83,14 @@ import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Function; import java.util.stream.Collectors; import static java.util.Collections.singletonList; +import static org.opensearch.cluster.applicationtemplates.ClusterStateSystemTemplateLoader.TEMPLATE_LOADER_IDENTIFIER; +import static org.opensearch.cluster.applicationtemplates.SystemTemplateMetadata.fromComponentTemplateInfo; import static org.opensearch.common.settings.Settings.builder; +import static org.opensearch.common.util.concurrent.ThreadContext.ACTION_ORIGIN_TRANSIENT_NAME; import static org.opensearch.env.Environment.PATH_HOME_SETTING; import static org.opensearch.index.mapper.DataStreamFieldMapper.Defaults.TIMESTAMP_FIELD; import static org.opensearch.indices.ShardLimitValidatorTests.createTestShardLimitService; @@ -657,47 +668,303 @@ public void onFailure(Exception e) { } public void testPutGlobalV2TemplateWhichProvidesContextWithContextDisabled() throws Exception { + MetadataIndexTemplateService metadataIndexTemplateService = getMetadataIndexTemplateService(); + ComposableIndexTemplate globalIndexTemplate = new ComposableIndexTemplate( + List.of("*"), + null, + List.of(), + null, + null, + null, + null, + new Context("any") + ); + InvalidIndexTemplateException ex = expectThrows( + InvalidIndexTemplateException.class, + () -> metadataIndexTemplateService.putIndexTemplateV2( + "testing", + true, + "template-referencing-context-as-ct", + TimeValue.timeValueSeconds(30L), + globalIndexTemplate, + new ActionListener() { + @Override + public void onResponse(AcknowledgedResponse response) { + fail("the listener should not be invoked as validation should fail"); + } + @Override + public void onFailure(Exception e) { + fail("the listener should not be invoked as validation should fail"); + } + } + ) + ); + assertTrue( + "Invalid exception message." + ex.getMessage(), + ex.getMessage().contains("specifies a context which cannot be used without enabling") + ); } public void testPutGlobalV2TemplateWhichProvidesContextNotPresentInState() throws Exception { + FeatureFlags.initializeFeatureFlags(Settings.builder().put(FeatureFlags.APPLICATION_BASED_CONFIGURATION_TEMPLATES, true).build()); + MetadataIndexTemplateService metadataIndexTemplateService = getMetadataIndexTemplateService(); + ComposableIndexTemplate globalIndexTemplate = new ComposableIndexTemplate( + List.of("*"), + null, + List.of(), + null, + null, + null, + null, + new Context("any") + ); + InvalidIndexTemplateException ex = expectThrows( + InvalidIndexTemplateException.class, + () -> metadataIndexTemplateService.putIndexTemplateV2( + "testing", + true, + "template-referencing-context-as-ct", + TimeValue.timeValueSeconds(30L), + globalIndexTemplate, + new ActionListener() { + @Override + public void onResponse(AcknowledgedResponse response) { + fail("the listener should not be invoked as validation should fail"); + } + @Override + public void onFailure(Exception e) { + fail("the listener should not be invoked as validation should fail"); + } + } + ) + ); + assertTrue( + "Invalid exception message." + ex.getMessage(), + ex.getMessage().contains("specifies a context which is not loaded on the cluster") + ); } public void testPutGlobalV2TemplateWhichProvidesContextWithNonExistingVersion() throws Exception { + FeatureFlags.initializeFeatureFlags(Settings.builder().put(FeatureFlags.APPLICATION_BASED_CONFIGURATION_TEMPLATES, true).build()); + MetadataIndexTemplateService metadataIndexTemplateService = getMetadataIndexTemplateService(); + + Function templateApplier = codec -> new Template( + Settings.builder().put(EngineConfig.INDEX_CODEC_SETTING.getKey(), codec).build(), + null, + null + ); + ComponentTemplate systemTemplate = new ComponentTemplate( + templateApplier.apply(CodecService.BEST_COMPRESSION_CODEC), + 1L, + Map.of(ClusterStateSystemTemplateLoader.TEMPLATE_TYPE_KEY, SystemTemplateMetadata.COMPONENT_TEMPLATE_TYPE, "_version", 1L) + ); + SystemTemplateMetadata systemTemplateMetadata = fromComponentTemplateInfo("ct-best-compression-codec" + System.nanoTime(), 1); + + CountDownLatch waitToCreateComponentTemplate = new CountDownLatch(1); + ActionListener createComponentTemplateListener = new ActionListener() { + @Override + public void onResponse(AcknowledgedResponse response) { + waitToCreateComponentTemplate.countDown(); + } + + @Override + public void onFailure(Exception e) { + fail("expecting the component template PUT to succeed but got: " + e.getMessage()); + } + }; + + ThreadContext threadContext = getInstanceFromNode(ThreadPool.class).getThreadContext(); + try (ThreadContext.StoredContext ignore = threadContext.stashContext()) { + getInstanceFromNode(ThreadPool.class).getThreadContext().putTransient(ACTION_ORIGIN_TRANSIENT_NAME, TEMPLATE_LOADER_IDENTIFIER); + metadataIndexTemplateService.putComponentTemplate( + "test", + true, + systemTemplateMetadata.fullyQualifiedName(), + TimeValue.timeValueSeconds(30L), + systemTemplate, + createComponentTemplateListener + ); + } + + assertTrue("Could not create component templates", waitToCreateComponentTemplate.await(10, TimeUnit.SECONDS)); + + Context context = new Context(systemTemplateMetadata.name(), Long.toString(2L), Map.of()); + ComposableIndexTemplate globalIndexTemplate = new ComposableIndexTemplate( + List.of("*"), + templateApplier.apply(CodecService.LZ4), + List.of(), + null, + null, + null, + null, + context + ); + + InvalidIndexTemplateException ex = expectThrows( + InvalidIndexTemplateException.class, + () -> metadataIndexTemplateService.putIndexTemplateV2( + "testing", + true, + "template-referencing-context-as-ct", + TimeValue.timeValueSeconds(30L), + globalIndexTemplate, + new ActionListener() { + @Override + public void onResponse(AcknowledgedResponse response) { + fail("the listener should not be invoked as validation should fail"); + } + + @Override + public void onFailure(Exception e) { + fail("the listener should not be invoked as validation should fail"); + } + } + ) + ); + assertTrue( + "Invalid exception message." + ex.getMessage(), + ex.getMessage().contains("specifies a context which is not loaded on the cluster") + ); } public void testPutGlobalV2TemplateWhichProvidesContextInComposedOfSection() throws Exception { + FeatureFlags.initializeFeatureFlags(Settings.builder().put(FeatureFlags.APPLICATION_BASED_CONFIGURATION_TEMPLATES, true).build()); + MetadataIndexTemplateService metadataIndexTemplateService = getMetadataIndexTemplateService(); - } + Function templateApplier = codec -> new Template( + Settings.builder().put(EngineConfig.INDEX_CODEC_SETTING.getKey(), codec).build(), + null, + null + ); + ComponentTemplate systemTemplate = new ComponentTemplate( + templateApplier.apply(CodecService.BEST_COMPRESSION_CODEC), + 1L, + Map.of(ClusterStateSystemTemplateLoader.TEMPLATE_TYPE_KEY, SystemTemplateMetadata.COMPONENT_TEMPLATE_TYPE, "_version", 1L) + ); + SystemTemplateMetadata systemTemplateMetadata = fromComponentTemplateInfo("context-best-compression-codec" + System.nanoTime(), 1); - public void testPutGlobalV2TemplateWhichProvidesContextWithSpecificVersion() throws Exception { + CountDownLatch waitToCreateComponentTemplate = new CountDownLatch(1); + ActionListener createComponentTemplateListener = new ActionListener() { - } + @Override + public void onResponse(AcknowledgedResponse response) { + waitToCreateComponentTemplate.countDown(); + } - public void testPutGlobalV2TemplateWhichProvidesContextWithLatestVersion() throws Exception { + @Override + public void onFailure(Exception e) { + fail("expecting the component template PUT to succeed but got: " + e.getMessage()); + } + }; + ThreadContext context = getInstanceFromNode(ThreadPool.class).getThreadContext(); + try (ThreadContext.StoredContext ignore = context.stashContext()) { + getInstanceFromNode(ThreadPool.class).getThreadContext().putTransient(ACTION_ORIGIN_TRANSIENT_NAME, TEMPLATE_LOADER_IDENTIFIER); + metadataIndexTemplateService.putComponentTemplate( + "test", + true, + systemTemplateMetadata.fullyQualifiedName(), + TimeValue.timeValueSeconds(30L), + systemTemplate, + createComponentTemplateListener + ); + } + assertTrue("Could not create component templates", waitToCreateComponentTemplate.await(10, TimeUnit.SECONDS)); + ComposableIndexTemplate globalIndexTemplate = new ComposableIndexTemplate( + List.of("*"), + templateApplier.apply(CodecService.LZ4), + List.of(systemTemplateMetadata.fullyQualifiedName()), + null, + null, + null, + null + ); + InvalidIndexTemplateException ex = expectThrows( + InvalidIndexTemplateException.class, + () -> metadataIndexTemplateService.putIndexTemplateV2( + "testing", + true, + "template-referencing-context-as-ct", + TimeValue.timeValueSeconds(30L), + globalIndexTemplate, + new ActionListener() { + @Override + public void onResponse(AcknowledgedResponse response) { + fail("the listener should not be invoked as validation should fail"); + } + + @Override + public void onFailure(Exception e) { + fail("the listener should not be invoked as validation should fail"); + } + } + ) + ); + assertTrue( + "Invalid exception message." + ex.getMessage(), + ex.getMessage().contains("specifies a component templates which can only be used in context") + ); } - public void testRemoveSystemTemplateViaUnknownSource() throws Exception { + public void testPutGlobalV2TemplateWhichProvidesContextWithSpecificVersion() throws Exception { + verifyTemplateCreationUsingContext("1"); + } + public void testPutGlobalV2TemplateWhichProvidesContextWithLatestVersion() throws Exception { + verifyTemplateCreationUsingContext("_latest"); } - public void testRemoveSystemTemplateInUse() throws Exception { + public void testModifySystemTemplateViaUnknownSource() throws Exception { + FeatureFlags.initializeFeatureFlags(Settings.builder().put(FeatureFlags.APPLICATION_BASED_CONFIGURATION_TEMPLATES, true).build()); + MetadataIndexTemplateService metadataIndexTemplateService = getMetadataIndexTemplateService(); - } + Function templateApplier = codec -> new Template( + Settings.builder().put(EngineConfig.INDEX_CODEC_SETTING.getKey(), codec).build(), + null, + null + ); - public void testRemoveSystemTemplateNotInUse() throws Exception { + ComponentTemplate systemTemplate = new ComponentTemplate( + templateApplier.apply(CodecService.BEST_COMPRESSION_CODEC), + 1L, + Map.of(ClusterStateSystemTemplateLoader.TEMPLATE_TYPE_KEY, SystemTemplateMetadata.COMPONENT_TEMPLATE_TYPE, "_version", 1L) + ); + SystemTemplateMetadata systemTemplateMetadata = fromComponentTemplateInfo("ct-best-compression-codec" + System.nanoTime(), 1); + IllegalArgumentException ex = expectThrows( + IllegalArgumentException.class, + () -> metadataIndexTemplateService.putComponentTemplate( + "test", + true, + systemTemplateMetadata.fullyQualifiedName(), + TimeValue.timeValueSeconds(30L), + systemTemplate, + ActionListener.wrap(() -> {}) + ) + ); + assertTrue( + "Invalid exception message." + ex.getMessage(), + ex.getMessage().contains("A system template can only be created/updated/deleted with a repository") + ); } - public void testPutSystemTemplateViaUnknownSource() throws Exception { + public void testResolveSettingsWithContextVersion() throws Exception { + ClusterService clusterService = node().injector().getInstance(ClusterService.class); + final String indexTemplateName = verifyTemplateCreationUsingContext("1"); + Settings settings = MetadataIndexTemplateService.resolveSettings(clusterService.state().metadata(), indexTemplateName); + assertThat(settings.get("index.codec"), equalTo(CodecService.BEST_COMPRESSION_CODEC)); } - public void testPutSystemTemplate() throws Exception { + public void testResolveSettingsWithContextLatest() throws Exception { + ClusterService clusterService = node().injector().getInstance(ClusterService.class); + final String indexTemplateName = verifyTemplateCreationUsingContext(Context.LATEST_VERSION); + Settings settings = MetadataIndexTemplateService.resolveSettings(clusterService.state().metadata(), indexTemplateName); + assertThat(settings.get("index.codec"), equalTo(CodecService.ZLIB)); } /** @@ -1557,6 +1824,16 @@ public void testResolveAliases() throws Exception { ComponentTemplate ct2 = new ComponentTemplate(new Template(null, null, a2), null, null); state = service.addComponentTemplate(state, true, "ct_high", ct1); state = service.addComponentTemplate(state, true, "ct_low", ct2); + + Map a4 = Map.of("sys", AliasMetadata.builder("sys").build()); + ComponentTemplate sysTemplate = new ComponentTemplate( + new Template(null, null, a4), + 1L, + Map.of(ClusterStateSystemTemplateLoader.TEMPLATE_TYPE_KEY, SystemTemplateMetadata.COMPONENT_TEMPLATE_TYPE, "_version", 1) + ); + SystemTemplateMetadata systemTemplateMetadata = SystemTemplateMetadata.fromComponentTemplateInfo("sys-template", 1L); + state = service.addComponentTemplate(state, true, systemTemplateMetadata.fullyQualifiedName(), sysTemplate); + ComposableIndexTemplate it = new ComposableIndexTemplate( Collections.singletonList("i*"), new Template(null, null, a3), @@ -1564,14 +1841,15 @@ public void testResolveAliases() throws Exception { 0L, 1L, null, - null + null, + new Context(systemTemplateMetadata.name()) ); state = service.addIndexTemplateV2(state, true, "my-template", it); List> resolvedAliases = MetadataIndexTemplateService.resolveAliases(state.metadata(), "my-template"); - // These should be order of precedence, so the index template (a3), then ct_high (a1), then ct_low (a2) - assertThat(resolvedAliases, equalTo(Arrays.asList(a3, a1, a2))); + // These should be order of precedence, so the context(a4), index template (a3), then ct_high (a1), then ct_low (a2) + assertThat(resolvedAliases, equalTo(Arrays.asList(a4, a3, a1, a2))); } public void testAddInvalidTemplate() throws Exception { @@ -2235,4 +2513,132 @@ public static void assertTemplatesEqual(ComposableIndexTemplate actual, Composab } } } + + private String verifyTemplateCreationUsingContext(String contextVersion) throws Exception { + FeatureFlags.initializeFeatureFlags(Settings.builder().put(FeatureFlags.APPLICATION_BASED_CONFIGURATION_TEMPLATES, true).build()); + MetadataIndexTemplateService metadataIndexTemplateService = getMetadataIndexTemplateService(); + + Function templateApplier = codec -> new Template( + Settings.builder().put(EngineConfig.INDEX_CODEC_SETTING.getKey(), codec).build(), + null, + null + ); + + ComponentTemplate componentTemplate = new ComponentTemplate(templateApplier.apply(CodecService.DEFAULT_CODEC), 1L, new HashMap<>()); + + ComponentTemplate systemTemplate = new ComponentTemplate( + templateApplier.apply(CodecService.BEST_COMPRESSION_CODEC), + 1L, + Map.of(ClusterStateSystemTemplateLoader.TEMPLATE_TYPE_KEY, SystemTemplateMetadata.COMPONENT_TEMPLATE_TYPE, "_version", 1L) + ); + SystemTemplateMetadata systemTemplateMetadata = fromComponentTemplateInfo("ct-best-compression-codec" + System.nanoTime(), 1); + + ComponentTemplate systemTemplateV2 = new ComponentTemplate( + templateApplier.apply(CodecService.ZLIB), + 2L, + Map.of(ClusterStateSystemTemplateLoader.TEMPLATE_TYPE_KEY, SystemTemplateMetadata.COMPONENT_TEMPLATE_TYPE, "_version", 2L) + ); + SystemTemplateMetadata systemTemplateV2Metadata = fromComponentTemplateInfo(systemTemplateMetadata.name(), 2); + + CountDownLatch waitToCreateComponentTemplate = new CountDownLatch(3); + ActionListener createComponentTemplateListener = new ActionListener() { + + @Override + public void onResponse(AcknowledgedResponse response) { + waitToCreateComponentTemplate.countDown(); + } + + @Override + public void onFailure(Exception e) { + fail("expecting the component template PUT to succeed but got: " + e.getMessage()); + } + }; + + String componentTemplateName = "ct-default-codec" + System.nanoTime(); + metadataIndexTemplateService.putComponentTemplate( + "test", + true, + componentTemplateName, + TimeValue.timeValueSeconds(30L), + componentTemplate, + createComponentTemplateListener + ); + + ThreadContext threadContext = getInstanceFromNode(ThreadPool.class).getThreadContext(); + try (ThreadContext.StoredContext ignore = threadContext.stashContext()) { + getInstanceFromNode(ThreadPool.class).getThreadContext().putTransient(ACTION_ORIGIN_TRANSIENT_NAME, TEMPLATE_LOADER_IDENTIFIER); + metadataIndexTemplateService.putComponentTemplate( + "test", + true, + systemTemplateMetadata.fullyQualifiedName(), + TimeValue.timeValueSeconds(30L), + systemTemplate, + createComponentTemplateListener + ); + + metadataIndexTemplateService.putComponentTemplate( + "test", + true, + systemTemplateV2Metadata.fullyQualifiedName(), + TimeValue.timeValueSeconds(30L), + systemTemplateV2, + createComponentTemplateListener + ); + } + + assertTrue("Could not create component templates", waitToCreateComponentTemplate.await(10, TimeUnit.SECONDS)); + + Context context = new Context(systemTemplateMetadata.name(), contextVersion, Map.of()); + ComposableIndexTemplate globalIndexTemplate = new ComposableIndexTemplate( + List.of("*"), + templateApplier.apply(CodecService.LZ4), + List.of(componentTemplateName), + null, + null, + null, + null, + context + ); + + String indexTemplateName = "template-referencing-ct-and-context"; + CountDownLatch waitForIndexTemplate = new CountDownLatch(1); + metadataIndexTemplateService.putIndexTemplateV2( + "testing", + true, + indexTemplateName, + TimeValue.timeValueSeconds(30L), + globalIndexTemplate, + new ActionListener() { + @Override + public void onResponse(AcknowledgedResponse response) { + waitForIndexTemplate.countDown(); + } + + @Override + public void onFailure(Exception e) { + fail("the listener should not be invoked as the template should succeed"); + } + } + ); + assertTrue("Expected index template to have been created.", waitForIndexTemplate.await(10, TimeUnit.SECONDS)); + assertTemplatesEqual( + node().injector().getInstance(ClusterService.class).state().metadata().templatesV2().get(indexTemplateName), + globalIndexTemplate + ); + + return indexTemplateName; + } + + @Override + protected boolean resetNodeAfterTest() { + return true; + } + + @Override + protected Settings featureFlagSettings() { + return Settings.builder() + .put(super.featureFlagSettings()) + .put(FeatureFlags.APPLICATION_BASED_CONFIGURATION_TEMPLATES, false) + .build(); + } } From c4c8c7bb24f98a7a7cfb562945ec25d95b71481f Mon Sep 17 00:00:00 2001 From: Mohit Godwani Date: Mon, 22 Jul 2024 17:53:34 +0530 Subject: [PATCH 4/6] Move system templates lookup to build Signed-off-by: Mohit Godwani --- .../opensearch/cluster/metadata/Metadata.java | 58 ++++++++++++------- 1 file changed, 38 insertions(+), 20 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/metadata/Metadata.java b/server/src/main/java/org/opensearch/cluster/metadata/Metadata.java index bc9033c527476..440b9e267cf0a 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/Metadata.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/Metadata.java @@ -300,7 +300,8 @@ static Custom fromXContent(XContentParser parser, String name) throws IOExceptio String[] visibleOpenIndices, String[] allClosedIndices, String[] visibleClosedIndices, - SortedMap indicesLookup + SortedMap indicesLookup, + Map> systemTemplatesLookup ) { this.clusterUUID = clusterUUID; this.clusterUUIDCommitted = clusterUUIDCommitted; @@ -331,23 +332,7 @@ static Custom fromXContent(XContentParser parser, String name) throws IOExceptio this.allClosedIndices = allClosedIndices; this.visibleClosedIndices = visibleClosedIndices; this.indicesLookup = indicesLookup; - - this.systemTemplatesLookup = new HashMap<>(); - Optional.ofNullable((ComponentTemplateMetadata) this.custom(ComponentTemplateMetadata.TYPE)) - .map(ComponentTemplateMetadata::componentTemplates) - .orElseGet(Collections::emptyMap) - .forEach((k, v) -> { - if (MetadataIndexTemplateService.isSystemTemplate(v)) { - SystemTemplateMetadata templateMetadata = SystemTemplateMetadata.fromComponentTemplate(k); - systemTemplatesLookup.compute(templateMetadata.name(), (ik, iv) -> { - if (iv == null) { - iv = new TreeMap<>(); - } - iv.put(templateMetadata.version(), k); - return iv; - }); - } - }); + this.systemTemplatesLookup = systemTemplatesLookup; } public long version() { @@ -1213,6 +1198,8 @@ public static class Builder { private final Map customs; private final Metadata previousMetadata; + private Map> systemTemplatesLookup; + public Builder() { clusterUUID = UNKNOWN_CLUSTER_UUID; indices = new HashMap<>(); @@ -1578,6 +1565,8 @@ public Metadata build() { ? (DataStreamMetadata) this.previousMetadata.customs.get(DataStreamMetadata.TYPE) : null; + buildSystemTemplatesLookup(); + boolean recomputeRequiredforIndicesLookups = (previousMetadata == null) || (indices.equals(previousMetadata.indices) == false) || (previousDataStreamMetadata != null && previousDataStreamMetadata.equals(dataStreamMetadata) == false) @@ -1588,6 +1577,33 @@ public Metadata build() { : buildMetadataWithRecomputedIndicesLookups(); } + private void buildSystemTemplatesLookup() { + if (previousMetadata != null + && Objects.equals( + previousMetadata.customs.get(ComponentTemplateMetadata.TYPE), + this.customs.get(ComponentTemplateMetadata.TYPE) + )) { + systemTemplatesLookup = Collections.unmodifiableMap(previousMetadata.systemTemplatesLookup); + } else { + systemTemplatesLookup = new HashMap<>(); + Optional.ofNullable((ComponentTemplateMetadata) this.customs.get(ComponentTemplateMetadata.TYPE)) + .map(ComponentTemplateMetadata::componentTemplates) + .orElseGet(Collections::emptyMap) + .forEach((k, v) -> { + if (MetadataIndexTemplateService.isSystemTemplate(v)) { + SystemTemplateMetadata templateMetadata = SystemTemplateMetadata.fromComponentTemplate(k); + systemTemplatesLookup.compute(templateMetadata.name(), (ik, iv) -> { + if (iv == null) { + iv = new TreeMap<>(); + } + iv.put(templateMetadata.version(), k); + return iv; + }); + } + }); + } + } + protected Metadata buildMetadataWithPreviousIndicesLookups() { return new Metadata( clusterUUID, @@ -1606,7 +1622,8 @@ protected Metadata buildMetadataWithPreviousIndicesLookups() { Arrays.copyOf(previousMetadata.visibleOpenIndices, previousMetadata.visibleOpenIndices.length), Arrays.copyOf(previousMetadata.allClosedIndices, previousMetadata.allClosedIndices.length), Arrays.copyOf(previousMetadata.visibleClosedIndices, previousMetadata.visibleClosedIndices.length), - Collections.unmodifiableSortedMap(previousMetadata.indicesLookup) + Collections.unmodifiableSortedMap(previousMetadata.indicesLookup), + systemTemplatesLookup ); } @@ -1729,7 +1746,8 @@ protected Metadata buildMetadataWithRecomputedIndicesLookups() { visibleOpenIndicesArray, allClosedIndicesArray, visibleClosedIndicesArray, - indicesLookup + indicesLookup, + systemTemplatesLookup ); } From 55df4198f15fd7b51e1aa484095439e9ece50f96 Mon Sep 17 00:00:00 2001 From: Mohit Godwani Date: Mon, 22 Jul 2024 18:59:45 +0530 Subject: [PATCH 5/6] Allow component template meta in os node command Signed-off-by: Mohit Godwani --- .../cluster/coordination/OpenSearchNodeCommand.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/coordination/OpenSearchNodeCommand.java b/server/src/main/java/org/opensearch/cluster/coordination/OpenSearchNodeCommand.java index 259d8961a3e78..896fe6fc8024b 100644 --- a/server/src/main/java/org/opensearch/cluster/coordination/OpenSearchNodeCommand.java +++ b/server/src/main/java/org/opensearch/cluster/coordination/OpenSearchNodeCommand.java @@ -47,6 +47,7 @@ import org.opensearch.cluster.ClusterName; import org.opensearch.cluster.ClusterState; import org.opensearch.cluster.Diff; +import org.opensearch.cluster.metadata.ComponentTemplateMetadata; import org.opensearch.cluster.metadata.DataStreamMetadata; import org.opensearch.cluster.metadata.Metadata; import org.opensearch.common.collect.Tuple; @@ -94,9 +95,10 @@ public abstract class OpenSearchNodeCommand extends EnvironmentAwareCommand { public T parseNamedObject(Class categoryClass, String name, XContentParser parser, C context) throws IOException { // Currently, two unknown top-level objects are present if (Metadata.Custom.class.isAssignableFrom(categoryClass)) { - if (DataStreamMetadata.TYPE.equals(name)) { + if (DataStreamMetadata.TYPE.equals(name) || ComponentTemplateMetadata.TYPE.equals(name)) { // DataStreamMetadata is used inside Metadata class for validation purposes and building the indicesLookup, - // therefor even es node commands need to be able to parse it. + // ComponentTemplateMetadata is used inside Metadata class for building the systemTemplatesLookup, + // therefor even OpenSearch node commands need to be able to parse it. return super.parseNamedObject(categoryClass, name, parser, context); // TODO: Try to parse other named objects (e.g. stored scripts, ingest pipelines) that are part of core es as well? // Note that supporting PersistentTasksCustomMetadata is trickier, because PersistentTaskParams is a named object too. From 480fee5001c3510e80f67ac6aedf48d80c6e4c54 Mon Sep 17 00:00:00 2001 From: Mohit Godwani Date: Tue, 23 Jul 2024 11:34:31 +0530 Subject: [PATCH 6/6] empty commit Signed-off-by: Mohit Godwani