From 366685888e3a8b9d8d17164a0b8a3f2d4f5646da Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Mon, 17 Jun 2024 17:02:35 -0400 Subject: [PATCH 01/21] Create new extension point in SystemIndexPlugin for a single plugin to get registered system indices Signed-off-by: Craig Perkins --- .../plugins/SystemIndexPluginIT.java | 73 +++++++++++++++++++ .../main/java/org/opensearch/node/Node.java | 34 ++++++++- .../opensearch/plugins/SystemIndexPlugin.java | 12 +++ 3 files changed, 117 insertions(+), 2 deletions(-) create mode 100644 server/src/internalClusterTest/java/org/opensearch/plugins/SystemIndexPluginIT.java diff --git a/server/src/internalClusterTest/java/org/opensearch/plugins/SystemIndexPluginIT.java b/server/src/internalClusterTest/java/org/opensearch/plugins/SystemIndexPluginIT.java new file mode 100644 index 0000000000000..7e7aa6fbe84a3 --- /dev/null +++ b/server/src/internalClusterTest/java/org/opensearch/plugins/SystemIndexPluginIT.java @@ -0,0 +1,73 @@ +/* + * 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.plugins; + +import org.opensearch.common.settings.Settings; +import org.opensearch.indices.SystemIndexDescriptor; +import org.opensearch.test.OpenSearchIntegTestCase; + +import java.util.Collection; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.function.Consumer; + +import static org.hamcrest.Matchers.containsString; + +@OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.TEST, numDataNodes = 0, numClientNodes = 0) +public class SystemIndexPluginIT extends OpenSearchIntegTestCase { + + @Override + protected Collection> nodePlugins() { + return List.of(SystemIndexPlugin1.class, SystemIndexPlugin2.class); + } + + public void test2SystemIndexPluginsImplementOnSystemIndices_shouldFail() throws Exception { + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> internalCluster().startNode()); + assertThat(e.getMessage(), containsString("Cannot have more than one plugin implementing onSystemIndex")); + } + +} + +final class SystemIndexPlugin1 extends Plugin implements SystemIndexPlugin { + + private Map> systemIndices; + + public SystemIndexPlugin1() {} + + @Override + public Collection getSystemIndexDescriptors(Settings settings) { + final SystemIndexDescriptor systemIndexDescriptor = new SystemIndexDescriptor(".system-index1", "System index 1"); + return Collections.singletonList(systemIndexDescriptor); + } + + @Override + public Consumer>> onSystemIndices() { + return (systemIndices) -> { this.systemIndices = systemIndices; }; + } +} + +class SystemIndexPlugin2 extends Plugin implements SystemIndexPlugin { + + private Map> systemIndices; + + public SystemIndexPlugin2() {} + + @Override + public Collection getSystemIndexDescriptors(Settings settings) { + final SystemIndexDescriptor systemIndexDescriptor = new SystemIndexDescriptor(".system-index2", "System index 2"); + return Collections.singletonList(systemIndexDescriptor); + } + + @Override + public Consumer>> onSystemIndices() { + return (systemIndices) -> { this.systemIndices = systemIndices; }; + } +} diff --git a/server/src/main/java/org/opensearch/node/Node.java b/server/src/main/java/org/opensearch/node/Node.java index 397949525a3ec..4d143fa3d62bc 100644 --- a/server/src/main/java/org/opensearch/node/Node.java +++ b/server/src/main/java/org/opensearch/node/Node.java @@ -286,6 +286,7 @@ import java.util.concurrent.Executor; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Consumer; import java.util.function.Function; import java.util.function.UnaryOperator; import java.util.stream.Collectors; @@ -814,15 +815,44 @@ protected Node( .flatMap(m -> m.entrySet().stream()) .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); + List systemIndexPlugins = pluginsService.filterPlugins(SystemIndexPlugin.class); final Map> systemIndexDescriptorMap = Collections.unmodifiableMap( - pluginsService.filterPlugins(SystemIndexPlugin.class) - .stream() + systemIndexPlugins.stream() .collect( Collectors.toMap(plugin -> plugin.getClass().getSimpleName(), plugin -> plugin.getSystemIndexDescriptors(settings)) ) ); final SystemIndices systemIndices = new SystemIndices(systemIndexDescriptorMap); + final Map> systemIndexMap = Collections.unmodifiableMap( + systemIndexPlugins.stream() + .collect( + Collectors.toMap( + plugin -> plugin.getClass().getCanonicalName(), + plugin -> plugin.getSystemIndexDescriptors(settings) + .stream() + .map(SystemIndexDescriptor::getIndexPattern) + .collect(Collectors.toSet()) + ) + ) + ); + + Consumer>> onSystemIndex = null; + for (SystemIndexPlugin plugin : systemIndexPlugins) { + Consumer>> newOnSystemIndex = plugin.onSystemIndices(); + if (newOnSystemIndex != null) { + logger.debug("Using onSystemIndex from plugin " + plugin.getClass().getCanonicalName()); + if (onSystemIndex != null) { + throw new IllegalArgumentException("Cannot have more than one plugin implementing onSystemIndex"); + } + onSystemIndex = newOnSystemIndex; + } + } + + if (onSystemIndex != null) { + onSystemIndex.accept(systemIndexMap); + } + final RerouteService rerouteService = new BatchedRerouteService(clusterService, clusterModule.getAllocationService()::reroute); rerouteServiceReference.set(rerouteService); clusterService.setRerouteService(rerouteService); diff --git a/server/src/main/java/org/opensearch/plugins/SystemIndexPlugin.java b/server/src/main/java/org/opensearch/plugins/SystemIndexPlugin.java index 4937a5ed091dc..b80b206c9f436 100644 --- a/server/src/main/java/org/opensearch/plugins/SystemIndexPlugin.java +++ b/server/src/main/java/org/opensearch/plugins/SystemIndexPlugin.java @@ -37,6 +37,9 @@ import java.util.Collection; import java.util.Collections; +import java.util.Map; +import java.util.Set; +import java.util.function.Consumer; /** * Plugin for defining system indices. Extends {@link ActionPlugin} because system indices must be accessed via APIs @@ -55,4 +58,13 @@ public interface SystemIndexPlugin extends ActionPlugin { default Collection getSystemIndexDescriptors(Settings settings) { return Collections.emptyList(); } + + /** + * This function passes the registered system indices to a plugin. + * + * Note: Only one installed plugin may implement onSystemIndices. + */ + default Consumer>> onSystemIndices() { + return null; + } } From 0a54f0ae524666232bbb5dddc6ccdeb35f3c28c3 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Mon, 17 Jun 2024 17:12:54 -0400 Subject: [PATCH 02/21] Add to CHANGELOG Signed-off-by: Craig Perkins --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 347c28792b35b..5e42b558f2671 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ## [Unreleased 2.x] ### Added - Add fingerprint ingest processor ([#13724](https://github.com/opensearch-project/OpenSearch/pull/13724)) +- Create new extension point in SystemIndexPlugin for a single plugin to get registered system indices ([#14415](https://github.com/opensearch-project/OpenSearch/pull/14415)) ### Dependencies - Bump `org.gradle.test-retry` from 1.5.8 to 1.5.9 ([#13442](https://github.com/opensearch-project/OpenSearch/pull/13442)) From 8f2a60bd9053eb4c07becba9b394658770159981 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Thu, 20 Jun 2024 14:40:24 -0400 Subject: [PATCH 03/21] WIP on system indices from IndexNameExpressionResolver Signed-off-by: Craig Perkins --- .../org/opensearch/cluster/ClusterModule.java | 6 ++- .../metadata/IndexNameExpressionResolver.java | 15 ++++++ .../indices/SystemIndexDescriptor.java | 4 +- .../org/opensearch/indices/SystemIndices.java | 10 +++- .../main/java/org/opensearch/node/Node.java | 50 ++++--------------- .../cluster/ClusterModuleTests.java | 34 ++++++++++--- 6 files changed, 69 insertions(+), 50 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/ClusterModule.java b/server/src/main/java/org/opensearch/cluster/ClusterModule.java index c7fd263bda56a..18bc402f41fa5 100644 --- a/server/src/main/java/org/opensearch/cluster/ClusterModule.java +++ b/server/src/main/java/org/opensearch/cluster/ClusterModule.java @@ -93,6 +93,7 @@ import org.opensearch.core.xcontent.NamedXContentRegistry; import org.opensearch.gateway.GatewayAllocator; import org.opensearch.gateway.ShardsBatchGatewayAllocator; +import org.opensearch.indices.SystemIndices; import org.opensearch.ingest.IngestMetadata; import org.opensearch.persistent.PersistentTasksCustomMetadata; import org.opensearch.persistent.PersistentTasksNodeService; @@ -146,14 +147,15 @@ public ClusterModule( ClusterInfoService clusterInfoService, SnapshotsInfoService snapshotsInfoService, ThreadContext threadContext, - ClusterManagerMetrics clusterManagerMetrics + ClusterManagerMetrics clusterManagerMetrics, + SystemIndices systemIndices ) { this.clusterPlugins = clusterPlugins; this.deciderList = createAllocationDeciders(settings, clusterService.getClusterSettings(), clusterPlugins); this.allocationDeciders = new AllocationDeciders(deciderList); this.shardsAllocator = createShardsAllocator(settings, clusterService.getClusterSettings(), clusterPlugins); this.clusterService = clusterService; - this.indexNameExpressionResolver = new IndexNameExpressionResolver(threadContext); + this.indexNameExpressionResolver = new IndexNameExpressionResolver(threadContext, systemIndices); this.allocationService = new AllocationService( allocationDeciders, shardsAllocator, diff --git a/server/src/main/java/org/opensearch/cluster/metadata/IndexNameExpressionResolver.java b/server/src/main/java/org/opensearch/cluster/metadata/IndexNameExpressionResolver.java index 24ff83d638d4b..0ca3a9f063b20 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/IndexNameExpressionResolver.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/IndexNameExpressionResolver.java @@ -53,6 +53,7 @@ import org.opensearch.index.IndexNotFoundException; import org.opensearch.indices.IndexClosedException; import org.opensearch.indices.InvalidIndexNameException; +import org.opensearch.indices.SystemIndices; import java.time.Instant; import java.time.ZoneId; @@ -91,9 +92,16 @@ public class IndexNameExpressionResolver { private final List expressionResolvers = List.of(dateMathExpressionResolver, wildcardExpressionResolver); private final ThreadContext threadContext; + private final SystemIndices systemIndices; public IndexNameExpressionResolver(ThreadContext threadContext) { this.threadContext = Objects.requireNonNull(threadContext, "Thread Context must not be null"); + this.systemIndices = new SystemIndices(Collections.emptyMap()); + } + + public IndexNameExpressionResolver(ThreadContext threadContext, SystemIndices systemIndices) { + this.threadContext = Objects.requireNonNull(threadContext, "Thread Context must not be null"); + this.systemIndices = Objects.requireNonNullElseGet(systemIndices, () -> new SystemIndices(Collections.emptyMap())); } /** @@ -112,6 +120,13 @@ public String[] concreteIndexNames(ClusterState state, IndicesRequest request) { return concreteIndexNames(context, request.indices()); } + /** + * Returns the registry of system indices that have been reserved by modules and plugins + */ + public SystemIndices systemIndices() { + return this.systemIndices; + } + /** * Same as {@link #concreteIndexNames(ClusterState, IndicesRequest)}, but access to system indices is always allowed. */ diff --git a/server/src/main/java/org/opensearch/indices/SystemIndexDescriptor.java b/server/src/main/java/org/opensearch/indices/SystemIndexDescriptor.java index f3592a3561d3a..f3212b1e2fae1 100644 --- a/server/src/main/java/org/opensearch/indices/SystemIndexDescriptor.java +++ b/server/src/main/java/org/opensearch/indices/SystemIndexDescriptor.java @@ -33,6 +33,7 @@ package org.opensearch.indices; import org.apache.lucene.util.automaton.CharacterRunAutomaton; +import org.opensearch.common.annotation.PublicApi; import org.opensearch.common.regex.Regex; import java.util.Objects; @@ -40,8 +41,9 @@ /** * Describes a system index. Provides the information required to create and maintain the system index. * - * @opensearch.internal + * @opensearch.api */ +@PublicApi(since = "2.16.0") public class SystemIndexDescriptor { private final String indexPattern; private final String description; diff --git a/server/src/main/java/org/opensearch/indices/SystemIndices.java b/server/src/main/java/org/opensearch/indices/SystemIndices.java index a85e938c61b7a..218a7a9e8b788 100644 --- a/server/src/main/java/org/opensearch/indices/SystemIndices.java +++ b/server/src/main/java/org/opensearch/indices/SystemIndices.java @@ -40,6 +40,7 @@ import org.apache.lucene.util.automaton.MinimizationOperations; import org.apache.lucene.util.automaton.Operations; import org.opensearch.common.Nullable; +import org.opensearch.common.annotation.PublicApi; import org.opensearch.common.collect.Tuple; import org.opensearch.common.regex.Regex; import org.opensearch.core.index.Index; @@ -64,8 +65,9 @@ * node knows about. Methods for determining if an index should be a system index are also provided * to reduce the locations within the code that need to deal with {@link SystemIndexDescriptor}s. * - * @opensearch.internal + * @opensearch.api */ +@PublicApi(since = "2.16.0") public class SystemIndices { private static final Logger logger = LogManager.getLogger(SystemIndices.class); @@ -76,9 +78,11 @@ public class SystemIndices { private final CharacterRunAutomaton runAutomaton; private final Collection systemIndexDescriptors; + private final Map> descriptorsMap; public SystemIndices(Map> pluginAndModulesDescriptors) { final Map> descriptorsMap = buildSystemIndexDescriptorMap(pluginAndModulesDescriptors); + this.descriptorsMap = descriptorsMap; checkForOverlappingPatterns(descriptorsMap); this.systemIndexDescriptors = unmodifiableList( descriptorsMap.values().stream().flatMap(Collection::stream).collect(Collectors.toList()) @@ -104,6 +108,10 @@ public boolean isSystemIndex(String indexName) { return runAutomaton.run(indexName); } + public Map> getSystemIndexDescriptors() { + return this.descriptorsMap; + } + /** * Finds a single matching {@link SystemIndexDescriptor}, if any, for the given index name. * @param name the name of the index diff --git a/server/src/main/java/org/opensearch/node/Node.java b/server/src/main/java/org/opensearch/node/Node.java index 4d143fa3d62bc..9e5a7fecdc5e6 100644 --- a/server/src/main/java/org/opensearch/node/Node.java +++ b/server/src/main/java/org/opensearch/node/Node.java @@ -286,7 +286,6 @@ import java.util.concurrent.Executor; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; -import java.util.function.Consumer; import java.util.function.Function; import java.util.function.UnaryOperator; import java.util.stream.Collectors; @@ -691,6 +690,14 @@ protected Node( repositoriesServiceReference::get, rerouteServiceReference::get ); + final Map> systemIndexDescriptorMap = Collections.unmodifiableMap( + pluginsService.filterPlugins(SystemIndexPlugin.class) + .stream() + .collect( + Collectors.toMap(plugin -> plugin.getClass().getSimpleName(), plugin -> plugin.getSystemIndexDescriptors(settings)) + ) + ); + final SystemIndices systemIndices = new SystemIndices(systemIndexDescriptorMap); final ClusterModule clusterModule = new ClusterModule( settings, clusterService, @@ -698,7 +705,8 @@ protected Node( clusterInfoService, snapshotsInfoService, threadPool.getThreadContext(), - clusterManagerMetrics + clusterManagerMetrics, + systemIndices ); modules.add(clusterModule); IndicesModule indicesModule = new IndicesModule(pluginsService.filterPlugins(MapperPlugin.class)); @@ -815,44 +823,6 @@ protected Node( .flatMap(m -> m.entrySet().stream()) .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); - List systemIndexPlugins = pluginsService.filterPlugins(SystemIndexPlugin.class); - final Map> systemIndexDescriptorMap = Collections.unmodifiableMap( - systemIndexPlugins.stream() - .collect( - Collectors.toMap(plugin -> plugin.getClass().getSimpleName(), plugin -> plugin.getSystemIndexDescriptors(settings)) - ) - ); - final SystemIndices systemIndices = new SystemIndices(systemIndexDescriptorMap); - - final Map> systemIndexMap = Collections.unmodifiableMap( - systemIndexPlugins.stream() - .collect( - Collectors.toMap( - plugin -> plugin.getClass().getCanonicalName(), - plugin -> plugin.getSystemIndexDescriptors(settings) - .stream() - .map(SystemIndexDescriptor::getIndexPattern) - .collect(Collectors.toSet()) - ) - ) - ); - - Consumer>> onSystemIndex = null; - for (SystemIndexPlugin plugin : systemIndexPlugins) { - Consumer>> newOnSystemIndex = plugin.onSystemIndices(); - if (newOnSystemIndex != null) { - logger.debug("Using onSystemIndex from plugin " + plugin.getClass().getCanonicalName()); - if (onSystemIndex != null) { - throw new IllegalArgumentException("Cannot have more than one plugin implementing onSystemIndex"); - } - onSystemIndex = newOnSystemIndex; - } - } - - if (onSystemIndex != null) { - onSystemIndex.accept(systemIndexMap); - } - final RerouteService rerouteService = new BatchedRerouteService(clusterService, clusterModule.getAllocationService()::reroute); rerouteServiceReference.set(rerouteService); clusterService.setRerouteService(rerouteService); diff --git a/server/src/test/java/org/opensearch/cluster/ClusterModuleTests.java b/server/src/test/java/org/opensearch/cluster/ClusterModuleTests.java index f2d99a51f1c9a..d7210adc6cb85 100644 --- a/server/src/test/java/org/opensearch/cluster/ClusterModuleTests.java +++ b/server/src/test/java/org/opensearch/cluster/ClusterModuleTests.java @@ -70,6 +70,7 @@ import org.opensearch.common.settings.SettingsModule; import org.opensearch.common.util.concurrent.ThreadContext; import org.opensearch.gateway.GatewayAllocator; +import org.opensearch.indices.SystemIndices; import org.opensearch.plugins.ClusterPlugin; import org.opensearch.telemetry.metrics.noop.NoopMetricsRegistry; import org.opensearch.test.ClusterServiceUtils; @@ -168,7 +169,13 @@ public void testRegisterAllocationDeciderDuplicate() { public Collection createAllocationDeciders(Settings settings, ClusterSettings clusterSettings) { return Collections.singletonList(new EnableAllocationDecider(settings, clusterSettings)); } - }), clusterInfoService, null, threadContext, new ClusterManagerMetrics(NoopMetricsRegistry.INSTANCE)) + }), + clusterInfoService, + null, + threadContext, + new ClusterManagerMetrics(NoopMetricsRegistry.INSTANCE), + new SystemIndices(Collections.emptyMap()) + ) ); assertEquals(e.getMessage(), "Cannot specify allocation decider [" + EnableAllocationDecider.class.getName() + "] twice"); } @@ -179,7 +186,13 @@ public void testRegisterAllocationDecider() { public Collection createAllocationDeciders(Settings settings, ClusterSettings clusterSettings) { return Collections.singletonList(new FakeAllocationDecider()); } - }), clusterInfoService, null, threadContext, new ClusterManagerMetrics(NoopMetricsRegistry.INSTANCE)); + }), + clusterInfoService, + null, + threadContext, + new ClusterManagerMetrics(NoopMetricsRegistry.INSTANCE), + new SystemIndices(Collections.emptyMap()) + ); assertTrue(module.deciderList.stream().anyMatch(d -> d.getClass().equals(FakeAllocationDecider.class))); } @@ -189,7 +202,13 @@ private ClusterModule newClusterModuleWithShardsAllocator(Settings settings, Str public Map> getShardsAllocators(Settings settings, ClusterSettings clusterSettings) { return Collections.singletonMap(name, supplier); } - }), clusterInfoService, null, threadContext, new ClusterManagerMetrics(NoopMetricsRegistry.INSTANCE)); + }), + clusterInfoService, + null, + threadContext, + new ClusterManagerMetrics(NoopMetricsRegistry.INSTANCE), + new SystemIndices(Collections.emptyMap()) + ); } public void testRegisterShardsAllocator() { @@ -217,7 +236,8 @@ public void testUnknownShardsAllocator() { clusterInfoService, null, threadContext, - new ClusterManagerMetrics(NoopMetricsRegistry.INSTANCE) + new ClusterManagerMetrics(NoopMetricsRegistry.INSTANCE), + new SystemIndices(Collections.emptyMap()) ) ); assertEquals("Unknown ShardsAllocator [dne]", e.getMessage()); @@ -303,7 +323,8 @@ public void testRejectsReservedExistingShardsAllocatorName() { clusterInfoService, null, threadContext, - new ClusterManagerMetrics(NoopMetricsRegistry.INSTANCE) + new ClusterManagerMetrics(NoopMetricsRegistry.INSTANCE), + new SystemIndices(Collections.emptyMap()) ); expectThrows( IllegalArgumentException.class, @@ -319,7 +340,8 @@ public void testRejectsDuplicateExistingShardsAllocatorName() { clusterInfoService, null, threadContext, - new ClusterManagerMetrics(NoopMetricsRegistry.INSTANCE) + new ClusterManagerMetrics(NoopMetricsRegistry.INSTANCE), + new SystemIndices(Collections.emptyMap()) ); expectThrows( IllegalArgumentException.class, From c8312ad833ad18a93c5cd923aac97c18eea846bb Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Mon, 24 Jun 2024 15:46:25 -0400 Subject: [PATCH 04/21] Add test in IndexNameExpressionResolverTests Signed-off-by: Craig Perkins --- .../plugins/SystemIndexPluginIT.java | 73 ------------------- .../indices/SystemIndexDescriptor.java | 13 ++++ .../IndexNameExpressionResolverTests.java | 51 +++++++++++++ .../indices/SystemIndexDescriptorTests.java | 8 ++ 4 files changed, 72 insertions(+), 73 deletions(-) delete mode 100644 server/src/internalClusterTest/java/org/opensearch/plugins/SystemIndexPluginIT.java diff --git a/server/src/internalClusterTest/java/org/opensearch/plugins/SystemIndexPluginIT.java b/server/src/internalClusterTest/java/org/opensearch/plugins/SystemIndexPluginIT.java deleted file mode 100644 index 7e7aa6fbe84a3..0000000000000 --- a/server/src/internalClusterTest/java/org/opensearch/plugins/SystemIndexPluginIT.java +++ /dev/null @@ -1,73 +0,0 @@ -/* - * 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.plugins; - -import org.opensearch.common.settings.Settings; -import org.opensearch.indices.SystemIndexDescriptor; -import org.opensearch.test.OpenSearchIntegTestCase; - -import java.util.Collection; -import java.util.Collections; -import java.util.List; -import java.util.Map; -import java.util.Set; -import java.util.function.Consumer; - -import static org.hamcrest.Matchers.containsString; - -@OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.TEST, numDataNodes = 0, numClientNodes = 0) -public class SystemIndexPluginIT extends OpenSearchIntegTestCase { - - @Override - protected Collection> nodePlugins() { - return List.of(SystemIndexPlugin1.class, SystemIndexPlugin2.class); - } - - public void test2SystemIndexPluginsImplementOnSystemIndices_shouldFail() throws Exception { - IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> internalCluster().startNode()); - assertThat(e.getMessage(), containsString("Cannot have more than one plugin implementing onSystemIndex")); - } - -} - -final class SystemIndexPlugin1 extends Plugin implements SystemIndexPlugin { - - private Map> systemIndices; - - public SystemIndexPlugin1() {} - - @Override - public Collection getSystemIndexDescriptors(Settings settings) { - final SystemIndexDescriptor systemIndexDescriptor = new SystemIndexDescriptor(".system-index1", "System index 1"); - return Collections.singletonList(systemIndexDescriptor); - } - - @Override - public Consumer>> onSystemIndices() { - return (systemIndices) -> { this.systemIndices = systemIndices; }; - } -} - -class SystemIndexPlugin2 extends Plugin implements SystemIndexPlugin { - - private Map> systemIndices; - - public SystemIndexPlugin2() {} - - @Override - public Collection getSystemIndexDescriptors(Settings settings) { - final SystemIndexDescriptor systemIndexDescriptor = new SystemIndexDescriptor(".system-index2", "System index 2"); - return Collections.singletonList(systemIndexDescriptor); - } - - @Override - public Consumer>> onSystemIndices() { - return (systemIndices) -> { this.systemIndices = systemIndices; }; - } -} diff --git a/server/src/main/java/org/opensearch/indices/SystemIndexDescriptor.java b/server/src/main/java/org/opensearch/indices/SystemIndexDescriptor.java index f3212b1e2fae1..8bb45c559cb98 100644 --- a/server/src/main/java/org/opensearch/indices/SystemIndexDescriptor.java +++ b/server/src/main/java/org/opensearch/indices/SystemIndexDescriptor.java @@ -106,6 +106,19 @@ public String toString() { return "SystemIndexDescriptor[pattern=[" + indexPattern + "], description=[" + description + "]]"; } + @Override + public boolean equals(Object o) { + if (o == this) return true; + if (!(o instanceof SystemIndexDescriptor)) return false; + SystemIndexDescriptor descriptor = (SystemIndexDescriptor) o; + return descriptor.indexPattern != null && descriptor.indexPattern.equals(indexPattern); + } + + @Override + public int hashCode() { + return Objects.hash(indexPattern); + } + // TODO: Index settings and mapping // TODO: getThreadpool() // TODO: Upgrade handling (reindex script?) diff --git a/server/src/test/java/org/opensearch/cluster/metadata/IndexNameExpressionResolverTests.java b/server/src/test/java/org/opensearch/cluster/metadata/IndexNameExpressionResolverTests.java index fda2f411b1994..4f3683bfd4d93 100644 --- a/server/src/test/java/org/opensearch/cluster/metadata/IndexNameExpressionResolverTests.java +++ b/server/src/test/java/org/opensearch/cluster/metadata/IndexNameExpressionResolverTests.java @@ -53,14 +53,20 @@ import org.opensearch.index.IndexSettings; import org.opensearch.indices.IndexClosedException; import org.opensearch.indices.InvalidIndexNameException; +import org.opensearch.indices.SystemIndexDescriptor; +import org.opensearch.indices.SystemIndices; +import org.opensearch.plugins.Plugin; +import org.opensearch.plugins.SystemIndexPlugin; import org.opensearch.test.OpenSearchTestCase; import java.util.Arrays; +import java.util.Collection; import java.util.Collections; import java.util.Comparator; import java.util.EnumSet; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Set; import java.util.function.Function; import java.util.stream.Collectors; @@ -79,6 +85,7 @@ import static org.hamcrest.Matchers.emptyArray; import static org.hamcrest.Matchers.endsWith; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasKey; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; @@ -2520,6 +2527,30 @@ public void testDataStreamsNames() { assertThat(names, empty()); } + public void testSystemIndexRetrieval() { + SystemIndexPlugin plugin1 = new SystemIndexPlugin1(); + SystemIndexPlugin plugin2 = new SystemIndexPlugin2(); + SystemIndices pluginSystemIndices = new SystemIndices( + Map.of( + SystemIndexPlugin1.class.getCanonicalName(), + plugin1.getSystemIndexDescriptors(Settings.EMPTY), + SystemIndexPlugin2.class.getCanonicalName(), + plugin2.getSystemIndexDescriptors(Settings.EMPTY) + ) + ); + IndexNameExpressionResolver resolver = new IndexNameExpressionResolver(threadContext, pluginSystemIndices); + + SystemIndices systemIndices = resolver.systemIndices(); + List pluginClassNames = List.of(plugin1.getClass().getCanonicalName(), plugin2.getClass().getCanonicalName()); + for (String pluginClassName : pluginClassNames) { + assertThat(systemIndices.getSystemIndexDescriptors(), hasKey(pluginClassName)); + assertThat( + systemIndices.getSystemIndexDescriptors().get(pluginClassName), + equalTo(pluginSystemIndices.getSystemIndexDescriptors().get(pluginClassName)) + ); + } + } + private ClusterState systemIndexTestClusterState() { Settings settings = Settings.builder().build(); Metadata.Builder mdBuilder = Metadata.builder() @@ -2534,4 +2565,24 @@ private List resolveConcreteIndexNameList(ClusterState state, SearchRequ .map(i -> i.getName()) .collect(Collectors.toList()); } + + static final class SystemIndexPlugin1 extends Plugin implements SystemIndexPlugin { + public static final String SYSTEM_INDEX_1 = ".system-index1"; + + @Override + public Collection getSystemIndexDescriptors(Settings settings) { + final SystemIndexDescriptor systemIndexDescriptor = new SystemIndexDescriptor(SYSTEM_INDEX_1, "System index 1"); + return Collections.singletonList(systemIndexDescriptor); + } + } + + static final class SystemIndexPlugin2 extends Plugin implements SystemIndexPlugin { + public static final String SYSTEM_INDEX_2 = ".system-index2"; + + @Override + public Collection getSystemIndexDescriptors(Settings settings) { + final SystemIndexDescriptor systemIndexDescriptor = new SystemIndexDescriptor(SYSTEM_INDEX_2, "System index 2"); + return Collections.singletonList(systemIndexDescriptor); + } + } } diff --git a/server/src/test/java/org/opensearch/indices/SystemIndexDescriptorTests.java b/server/src/test/java/org/opensearch/indices/SystemIndexDescriptorTests.java index f2bdaa1fe8c80..d59469bcbb1f2 100644 --- a/server/src/test/java/org/opensearch/indices/SystemIndexDescriptorTests.java +++ b/server/src/test/java/org/opensearch/indices/SystemIndexDescriptorTests.java @@ -35,6 +35,7 @@ import org.opensearch.test.OpenSearchTestCase; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; public class SystemIndexDescriptorTests extends OpenSearchTestCase { @@ -74,4 +75,11 @@ public void testValidation() { assertThat(ex.getMessage(), containsString("must not start with the character sequence [.*] to prevent conflicts")); } } + + public void testEquals() { + SystemIndexDescriptor descriptor1 = new SystemIndexDescriptor(".system-index", "Descriptor"); + SystemIndexDescriptor descriptor2 = new SystemIndexDescriptor(".system-index", "Other Descriptor"); + + assertThat(descriptor1, equalTo(descriptor2)); + } } From a9cffb3104c13eba77174d05294fd0444b9f4c55 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Mon, 24 Jun 2024 15:49:49 -0400 Subject: [PATCH 05/21] Remove changes in SystemIndexPlugin Signed-off-by: Craig Perkins --- CHANGELOG.md | 2 +- .../org/opensearch/plugins/SystemIndexPlugin.java | 12 ------------ 2 files changed, 1 insertion(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5e42b558f2671..4dd246a56300f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ## [Unreleased 2.x] ### Added - Add fingerprint ingest processor ([#13724](https://github.com/opensearch-project/OpenSearch/pull/13724)) -- Create new extension point in SystemIndexPlugin for a single plugin to get registered system indices ([#14415](https://github.com/opensearch-project/OpenSearch/pull/14415)) +- Add method to IndexNameExpressionResolver to get registered system indices ([#14415](https://github.com/opensearch-project/OpenSearch/pull/14415)) ### 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/plugins/SystemIndexPlugin.java b/server/src/main/java/org/opensearch/plugins/SystemIndexPlugin.java index b80b206c9f436..4937a5ed091dc 100644 --- a/server/src/main/java/org/opensearch/plugins/SystemIndexPlugin.java +++ b/server/src/main/java/org/opensearch/plugins/SystemIndexPlugin.java @@ -37,9 +37,6 @@ import java.util.Collection; import java.util.Collections; -import java.util.Map; -import java.util.Set; -import java.util.function.Consumer; /** * Plugin for defining system indices. Extends {@link ActionPlugin} because system indices must be accessed via APIs @@ -58,13 +55,4 @@ public interface SystemIndexPlugin extends ActionPlugin { default Collection getSystemIndexDescriptors(Settings settings) { return Collections.emptyList(); } - - /** - * This function passes the registered system indices to a plugin. - * - * Note: Only one installed plugin may implement onSystemIndices. - */ - default Consumer>> onSystemIndices() { - return null; - } } From 8cc647e0e41dcffb001caf16c4ca9b5f8118db21 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Wed, 26 Jun 2024 11:01:27 -0400 Subject: [PATCH 06/21] Add method in IndexNameExpressionResolver to get matching system indices Signed-off-by: Craig Perkins --- .../opensearch/common/WildcardMatcher.java | 498 ++++++++++++++++++ .../common/WildcardMatcherTests.java | 80 +++ .../metadata/IndexNameExpressionResolver.java | 19 +- .../org/opensearch/indices/SystemIndices.java | 6 +- .../IndexNameExpressionResolverTests.java | 19 +- 5 files changed, 597 insertions(+), 25 deletions(-) create mode 100644 libs/common/src/main/java/org/opensearch/common/WildcardMatcher.java create mode 100644 libs/common/src/test/java/org/opensearch/common/WildcardMatcherTests.java diff --git a/libs/common/src/main/java/org/opensearch/common/WildcardMatcher.java b/libs/common/src/main/java/org/opensearch/common/WildcardMatcher.java new file mode 100644 index 0000000000000..c9b812bad3852 --- /dev/null +++ b/libs/common/src/main/java/org/opensearch/common/WildcardMatcher.java @@ -0,0 +1,498 @@ +/* + * 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.common; + +import java.util.Arrays; +import java.util.Collection; +import java.util.List; +import java.util.Objects; +import java.util.Optional; +import java.util.function.Function; +import java.util.function.Predicate; +import java.util.regex.Pattern; +import java.util.stream.Collector; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +/** + * Class used to create matchers that can be used to test for matching patterns. + * + * WildcardMatcher supports matching using regex patterns. + * + * WildcardMatcher as supports wildcard (*) for matching any pattern and (?) for matching any single character. + * + * @opensearch.api + */ +public abstract class WildcardMatcher implements Predicate { + + public static final WildcardMatcher ANY = new WildcardMatcher() { + + @Override + public boolean matchAny(Stream candidates) { + return true; + } + + @Override + public boolean matchAny(Collection candidates) { + return true; + } + + @Override + public boolean matchAny(String... candidates) { + return true; + } + + @Override + public boolean matchAll(Stream candidates) { + return true; + } + + @Override + public boolean matchAll(Collection candidates) { + return true; + } + + @Override + public boolean matchAll(String[] candidates) { + return true; + } + + @Override + public > T getMatchAny(Stream candidates, Collector collector) { + return candidates.collect(collector); + } + + @Override + public boolean test(String candidate) { + return true; + } + + @Override + public String toString() { + return "*"; + } + }; + + public static final WildcardMatcher NONE = new WildcardMatcher() { + + @Override + public boolean matchAny(Stream candidates) { + return false; + } + + @Override + public boolean matchAny(Collection candidates) { + return false; + } + + @Override + public boolean matchAny(String... candidates) { + return false; + } + + @Override + public boolean matchAll(Stream candidates) { + return false; + } + + @Override + public boolean matchAll(Collection candidates) { + return false; + } + + @Override + public boolean matchAll(String[] candidates) { + return false; + } + + @Override + public > T getMatchAny(Stream candidates, Collector collector) { + return Stream.empty().collect(collector); + } + + @Override + public > T getMatchAny(Collection candidate, Collector collector) { + return Stream.empty().collect(collector); + } + + @Override + public > T getMatchAny(String[] candidate, Collector collector) { + return Stream.empty().collect(collector); + } + + @Override + public boolean test(String candidate) { + return false; + } + + @Override + public String toString() { + return ""; + } + }; + + public static WildcardMatcher from(String pattern, boolean caseSensitive) { + if (pattern == null) { + return NONE; + } else if (pattern.equals("*")) { + return ANY; + } else if (pattern.startsWith("/") && pattern.endsWith("/")) { + return new RegexMatcher(pattern, caseSensitive); + } else if (pattern.indexOf('?') >= 0 || pattern.indexOf('*') >= 0) { + return caseSensitive ? new SimpleMatcher(pattern) : new CasefoldingMatcher(pattern, SimpleMatcher::new); + } else { + return caseSensitive ? new Exact(pattern) : new CasefoldingMatcher(pattern, Exact::new); + } + } + + public static WildcardMatcher from(String pattern) { + return from(pattern, true); + } + + // This may in future use more optimized techniques to combine multiple WildcardMatchers in a single automaton + public static WildcardMatcher from(Stream stream, boolean caseSensitive) { + Collection matchers = stream.map(t -> { + if (t == null) { + return NONE; + } else if (t instanceof String) { + return WildcardMatcher.from(((String) t), caseSensitive); + } else if (t instanceof WildcardMatcher) { + return ((WildcardMatcher) t); + } + throw new UnsupportedOperationException("WildcardMatcher can't be constructed from " + t.getClass().getSimpleName()); + }).collect(Collectors.toSet()); + + if (matchers.isEmpty()) { + return NONE; + } else if (matchers.size() == 1) { + return matchers.stream().findFirst().get(); + } + return new MatcherCombiner(matchers); + } + + public static WildcardMatcher from(Collection collection, boolean caseSensitive) { + if (collection == null || collection.isEmpty()) { + return NONE; + } else if (collection.size() == 1) { + T t = collection.stream().findFirst().get(); + if (t instanceof String) { + return from(((String) t), caseSensitive); + } else if (t instanceof WildcardMatcher) { + return ((WildcardMatcher) t); + } + throw new UnsupportedOperationException("WildcardMatcher can't be constructed from " + t.getClass().getSimpleName()); + } + return from(collection.stream(), caseSensitive); + } + + public static WildcardMatcher from(String[] patterns, boolean caseSensitive) { + if (patterns == null || patterns.length == 0) { + return NONE; + } else if (patterns.length == 1) { + return from(patterns[0], caseSensitive); + } + return from(Arrays.stream(patterns), caseSensitive); + } + + public static WildcardMatcher from(Stream patterns) { + return from(patterns, true); + } + + public static WildcardMatcher from(Collection patterns) { + return from(patterns, true); + } + + public static WildcardMatcher from(String... patterns) { + return from(patterns, true); + } + + public WildcardMatcher concat(Stream matchers) { + return new WildcardMatcher.MatcherCombiner(Stream.concat(matchers, Stream.of(this)).collect(Collectors.toSet())); + } + + public WildcardMatcher concat(Collection matchers) { + if (matchers.isEmpty()) { + return this; + } + return concat(matchers.stream()); + } + + public WildcardMatcher concat(WildcardMatcher... matchers) { + if (matchers.length == 0) { + return this; + } + return concat(Arrays.stream(matchers)); + } + + public boolean matchAny(Stream candidates) { + return candidates.anyMatch(this); + } + + public boolean matchAny(Collection candidates) { + return matchAny(candidates.stream()); + } + + public boolean matchAny(String... candidates) { + return matchAny(Arrays.stream(candidates)); + } + + public boolean matchAll(Stream candidates) { + return candidates.allMatch(this); + } + + public boolean matchAll(Collection candidates) { + return matchAll(candidates.stream()); + } + + public boolean matchAll(String[] candidates) { + return matchAll(Arrays.stream(candidates)); + } + + public > T getMatchAny(Stream candidates, Collector collector) { + return candidates.filter(this).collect(collector); + } + + public > T getMatchAny(Collection candidate, Collector collector) { + return getMatchAny(candidate.stream(), collector); + } + + public > T getMatchAny(final String[] candidate, Collector collector) { + return getMatchAny(Arrays.stream(candidate), collector); + } + + public Optional findFirst(final String candidate) { + return Optional.ofNullable(test(candidate) ? this : null); + } + + public static List matchers(Collection patterns) { + return patterns.stream().map(p -> WildcardMatcher.from(p, true)).collect(Collectors.toList()); + } + + public static List getAllMatchingPatterns(final Collection matchers, final String candidate) { + return matchers.stream().filter(p -> p.test(candidate)).map(Objects::toString).collect(Collectors.toList()); + } + + public static List getAllMatchingPatterns(final Collection pattern, final Collection candidates) { + return pattern.stream().filter(p -> p.matchAny(candidates)).map(Objects::toString).collect(Collectors.toList()); + } + + // + // --- Implementation specializations --- + // + // Casefolding matcher - sits on top of case-sensitive matcher + // and proxies toLower() of input string to the wrapped matcher + private static final class CasefoldingMatcher extends WildcardMatcher { + + private final WildcardMatcher inner; + + public CasefoldingMatcher(String pattern, Function simpleWildcardMatcher) { + this.inner = simpleWildcardMatcher.apply(pattern.toLowerCase()); + } + + @Override + public boolean test(String candidate) { + return inner.test(candidate.toLowerCase()); + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + CasefoldingMatcher that = (CasefoldingMatcher) o; + return inner.equals(that.inner); + } + + @Override + public int hashCode() { + return inner.hashCode(); + } + + @Override + public String toString() { + return inner.toString(); + } + } + + /** + * Exact is a subclass of WildcardMatcher that requires the tested candidates to exactly match the pattern + * of the Matcher + */ + public static final class Exact extends WildcardMatcher { + + private final String pattern; + + private Exact(String pattern) { + this.pattern = pattern; + } + + @Override + public boolean test(String candidate) { + return pattern.equals(candidate); + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + Exact that = (Exact) o; + return pattern.equals(that.pattern); + } + + @Override + public int hashCode() { + return pattern.hashCode(); + } + + @Override + public String toString() { + return pattern; + } + } + + // RegexMatcher uses JDK Pattern to test for matching, + // assumes "//" strings as input pattern + private static final class RegexMatcher extends WildcardMatcher { + + private final Pattern pattern; + + private RegexMatcher(String pattern, boolean caseSensitive) { + if (!(pattern.length() > 1 && pattern.startsWith("/") && pattern.endsWith("/"))) { + throw new IllegalArgumentException("Invalid regex. Must have length greater than 1 and begin and end with a /"); + } + final String stripSlashesPattern = pattern.substring(1, pattern.length() - 1); + this.pattern = caseSensitive + ? Pattern.compile(stripSlashesPattern) + : Pattern.compile(stripSlashesPattern, Pattern.CASE_INSENSITIVE); + } + + @Override + public boolean test(String candidate) { + return pattern.matcher(candidate).matches(); + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + RegexMatcher that = (RegexMatcher) o; + return pattern.pattern().equals(that.pattern.pattern()); + } + + @Override + public int hashCode() { + return pattern.pattern().hashCode(); + } + + @Override + public String toString() { + return "/" + pattern.pattern() + "/"; + } + } + + // Simple implementation of WildcardMatcher matcher with * and ? without + // using exlicit stack or recursion (as long as we don't need sub-matches it does work) + // allows us to save on resources and heap allocations unless Regex is required + private static final class SimpleMatcher extends WildcardMatcher { + + private final String pattern; + + SimpleMatcher(String pattern) { + this.pattern = pattern; + } + + @Override + public boolean test(String candidate) { + int i = 0; + int j = 0; + int n = candidate.length(); + int m = pattern.length(); + int text_backup = -1; + int wild_backup = -1; + while (i < n) { + if (j < m && pattern.charAt(j) == '*') { + text_backup = i; + wild_backup = ++j; + } else if (j < m && (pattern.charAt(j) == '?' || pattern.charAt(j) == candidate.charAt(i))) { + i++; + j++; + } else { + if (wild_backup == -1) return false; + i = ++text_backup; + j = wild_backup; + } + } + while (j < m && pattern.charAt(j) == '*') + j++; + return j >= m; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + SimpleMatcher that = (SimpleMatcher) o; + return pattern.equals(that.pattern); + } + + @Override + public int hashCode() { + return pattern.hashCode(); + } + + @Override + public String toString() { + return pattern; + } + } + + // MatcherCombiner is a combination of a set of matchers + // matches if any of the set do + // Empty MultiMatcher always returns false + private static final class MatcherCombiner extends WildcardMatcher { + + private final Collection wildcardMatchers; + private final int hashCode; + + MatcherCombiner(Collection wildcardMatchers) { + if (!(wildcardMatchers.size() > 1)) { + throw new IllegalArgumentException("Must provide at least 1 wildcard matcher"); + } + this.wildcardMatchers = wildcardMatchers; + hashCode = wildcardMatchers.hashCode(); + } + + @Override + public boolean test(String candidate) { + return wildcardMatchers.stream().anyMatch(m -> m.test(candidate)); + } + + @Override + public Optional findFirst(final String candidate) { + return wildcardMatchers.stream().filter(m -> m.test(candidate)).findFirst(); + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + MatcherCombiner that = (MatcherCombiner) o; + return wildcardMatchers.equals(that.wildcardMatchers); + } + + @Override + public int hashCode() { + return hashCode; + } + + @Override + public String toString() { + return wildcardMatchers.toString(); + } + } +} diff --git a/libs/common/src/test/java/org/opensearch/common/WildcardMatcherTests.java b/libs/common/src/test/java/org/opensearch/common/WildcardMatcherTests.java new file mode 100644 index 0000000000000..14c21901f276d --- /dev/null +++ b/libs/common/src/test/java/org/opensearch/common/WildcardMatcherTests.java @@ -0,0 +1,80 @@ +/* + * 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.common; + +import org.junit.Test; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +public class WildcardMatcherTests { + static private WildcardMatcher wc(String pattern) { + return WildcardMatcher.from(pattern); + } + + static private WildcardMatcher iwc(String pattern) { + return WildcardMatcher.from(pattern, false); + } + + @Test + public void testWildcardMatcherClasses() { + assertFalse(wc("a*?").test("a")); + assertTrue(wc("a*?").test("aa")); + assertTrue(wc("a*?").test("ab")); + assertTrue(wc("a*?").test("abb")); + assertTrue(wc("*my*index").test("myindex")); + assertFalse(wc("*my*index").test("myindex1")); + assertTrue(wc("*my*index?").test("myindex1")); + assertTrue(wc("*my*index").test("this_is_my_great_index")); + assertFalse(wc("*my*index").test("MYindex")); + assertFalse(wc("?kibana").test("kibana")); + assertTrue(wc("?kibana").test(".kibana")); + assertFalse(wc("?kibana").test("kibana.")); + assertTrue(wc("?kibana?").test("?kibana.")); + assertTrue(wc("/(\\d{3}-?\\d{2}-?\\d{4})/").test("123-45-6789")); + assertFalse(wc("(\\d{3}-?\\d{2}-?\\d{4})").test("123-45-6789")); + assertTrue(wc("/\\S+/").test("abc")); + assertTrue(wc("abc").test("abc")); + assertFalse(wc("ABC").test("abc")); + assertFalse(wc(null).test("abc")); + assertTrue(WildcardMatcher.from(null, "abc").test("abc")); + } + + @Test + public void testWildcardMatcherClassesCaseInsensitive() { + assertTrue(iwc("AbC").test("abc")); + assertTrue(iwc("abc").test("aBC")); + assertTrue(iwc("A*b").test("ab")); + assertTrue(iwc("A*b").test("aab")); + assertTrue(iwc("A*b").test("abB")); + assertFalse(iwc("abc").test("AB")); + assertTrue(iwc("/^\\w+$/").test("AbCd")); + } + + @Test + public void testWildcardMatchers() { + assertTrue(!WildcardMatcher.from("a*?").test("a")); + assertTrue(WildcardMatcher.from("a*?").test("aa")); + assertTrue(WildcardMatcher.from("a*?").test("ab")); + assertTrue(WildcardMatcher.from("*my*index").test("myindex")); + assertTrue(!WildcardMatcher.from("*my*index").test("myindex1")); + assertTrue(WildcardMatcher.from("*my*index?").test("myindex1")); + assertTrue(WildcardMatcher.from("*my*index").test("this_is_my_great_index")); + assertTrue(!WildcardMatcher.from("*my*index").test("MYindex")); + assertTrue(!WildcardMatcher.from("?kibana").test("kibana")); + assertTrue(WildcardMatcher.from("?kibana").test(".kibana")); + assertTrue(!WildcardMatcher.from("?kibana").test("kibana.")); + assertTrue(WildcardMatcher.from("?kibana?").test("?kibana.")); + assertTrue(WildcardMatcher.from("/(\\d{3}-?\\d{2}-?\\d{4})/").test("123-45-6789")); + assertTrue(!WildcardMatcher.from("(\\d{3}-?\\d{2}-?\\d{4})").test("123-45-6789")); + assertTrue(WildcardMatcher.from("/\\S*/").test("abc")); + assertTrue(WildcardMatcher.from("abc").test("abc")); + assertTrue(!WildcardMatcher.from("ABC").test("abc")); + } +} diff --git a/server/src/main/java/org/opensearch/cluster/metadata/IndexNameExpressionResolver.java b/server/src/main/java/org/opensearch/cluster/metadata/IndexNameExpressionResolver.java index 0ca3a9f063b20..0c81c014fcec0 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/IndexNameExpressionResolver.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/IndexNameExpressionResolver.java @@ -38,6 +38,7 @@ import org.opensearch.cluster.ClusterState; import org.opensearch.common.Booleans; import org.opensearch.common.Nullable; +import org.opensearch.common.WildcardMatcher; import org.opensearch.common.annotation.PublicApi; import org.opensearch.common.collect.Tuple; import org.opensearch.common.logging.DeprecationLogger; @@ -92,16 +93,17 @@ public class IndexNameExpressionResolver { private final List expressionResolvers = List.of(dateMathExpressionResolver, wildcardExpressionResolver); private final ThreadContext threadContext; - private final SystemIndices systemIndices; + private final WildcardMatcher systemIndexMatcher; public IndexNameExpressionResolver(ThreadContext threadContext) { this.threadContext = Objects.requireNonNull(threadContext, "Thread Context must not be null"); - this.systemIndices = new SystemIndices(Collections.emptyMap()); + this.systemIndexMatcher = WildcardMatcher.NONE; } public IndexNameExpressionResolver(ThreadContext threadContext, SystemIndices systemIndices) { this.threadContext = Objects.requireNonNull(threadContext, "Thread Context must not be null"); - this.systemIndices = Objects.requireNonNullElseGet(systemIndices, () -> new SystemIndices(Collections.emptyMap())); + List allSystemIndexPatterns = systemIndices.getAllSystemIndexPatterns(); + this.systemIndexMatcher = WildcardMatcher.from(allSystemIndexPatterns); } /** @@ -120,13 +122,6 @@ public String[] concreteIndexNames(ClusterState state, IndicesRequest request) { return concreteIndexNames(context, request.indices()); } - /** - * Returns the registry of system indices that have been reserved by modules and plugins - */ - public SystemIndices systemIndices() { - return this.systemIndices; - } - /** * Same as {@link #concreteIndexNames(ClusterState, IndicesRequest)}, but access to system indices is always allowed. */ @@ -179,6 +174,10 @@ public String[] concreteIndexNames(ClusterState state, IndicesOptions options, I return concreteIndexNames(context, request.indices()); } + public List concreteSystemIndices(String... concreteIndices) { + return systemIndexMatcher.getMatchAny(concreteIndices, Collectors.toList()); + } + public List dataStreamNames(ClusterState state, IndicesOptions options, String... indexExpressions) { // Allow system index access - they'll be filtered out below as there's no such thing (yet) as system data streams Context context = new Context(state, options, false, false, true, true, true); diff --git a/server/src/main/java/org/opensearch/indices/SystemIndices.java b/server/src/main/java/org/opensearch/indices/SystemIndices.java index 218a7a9e8b788..860ae5a65a12f 100644 --- a/server/src/main/java/org/opensearch/indices/SystemIndices.java +++ b/server/src/main/java/org/opensearch/indices/SystemIndices.java @@ -78,11 +78,9 @@ public class SystemIndices { private final CharacterRunAutomaton runAutomaton; private final Collection systemIndexDescriptors; - private final Map> descriptorsMap; public SystemIndices(Map> pluginAndModulesDescriptors) { final Map> descriptorsMap = buildSystemIndexDescriptorMap(pluginAndModulesDescriptors); - this.descriptorsMap = descriptorsMap; checkForOverlappingPatterns(descriptorsMap); this.systemIndexDescriptors = unmodifiableList( descriptorsMap.values().stream().flatMap(Collection::stream).collect(Collectors.toList()) @@ -108,8 +106,8 @@ public boolean isSystemIndex(String indexName) { return runAutomaton.run(indexName); } - public Map> getSystemIndexDescriptors() { - return this.descriptorsMap; + public List getAllSystemIndexPatterns() { + return systemIndexDescriptors.stream().map(SystemIndexDescriptor::getIndexPattern).collect(Collectors.toList()); } /** diff --git a/server/src/test/java/org/opensearch/cluster/metadata/IndexNameExpressionResolverTests.java b/server/src/test/java/org/opensearch/cluster/metadata/IndexNameExpressionResolverTests.java index 4f3683bfd4d93..d95d3e803c198 100644 --- a/server/src/test/java/org/opensearch/cluster/metadata/IndexNameExpressionResolverTests.java +++ b/server/src/test/java/org/opensearch/cluster/metadata/IndexNameExpressionResolverTests.java @@ -85,7 +85,6 @@ import static org.hamcrest.Matchers.emptyArray; import static org.hamcrest.Matchers.endsWith; import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.hasKey; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; @@ -2527,7 +2526,7 @@ public void testDataStreamsNames() { assertThat(names, empty()); } - public void testSystemIndexRetrieval() { + public void testSystemIndexMatching() { SystemIndexPlugin plugin1 = new SystemIndexPlugin1(); SystemIndexPlugin plugin2 = new SystemIndexPlugin2(); SystemIndices pluginSystemIndices = new SystemIndices( @@ -2540,15 +2539,13 @@ public void testSystemIndexRetrieval() { ); IndexNameExpressionResolver resolver = new IndexNameExpressionResolver(threadContext, pluginSystemIndices); - SystemIndices systemIndices = resolver.systemIndices(); - List pluginClassNames = List.of(plugin1.getClass().getCanonicalName(), plugin2.getClass().getCanonicalName()); - for (String pluginClassName : pluginClassNames) { - assertThat(systemIndices.getSystemIndexDescriptors(), hasKey(pluginClassName)); - assertThat( - systemIndices.getSystemIndexDescriptors().get(pluginClassName), - equalTo(pluginSystemIndices.getSystemIndexDescriptors().get(pluginClassName)) - ); - } + assertThat( + resolver.concreteSystemIndices(".system-index1", ".system-index2"), + equalTo(List.of(SystemIndexPlugin1.SYSTEM_INDEX_1, SystemIndexPlugin2.SYSTEM_INDEX_2)) + ); + assertThat(resolver.concreteSystemIndices(".system-index1"), equalTo(List.of(SystemIndexPlugin1.SYSTEM_INDEX_1))); + assertThat(resolver.concreteSystemIndices(".system-index2"), equalTo(List.of(SystemIndexPlugin2.SYSTEM_INDEX_2))); + assertThat(resolver.concreteSystemIndices(".not-exists"), equalTo(Collections.EMPTY_LIST)); } private ClusterState systemIndexTestClusterState() { From 81dff6a597687896f87b8ae1ea1f866ab161950f Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Wed, 26 Jun 2024 11:28:14 -0400 Subject: [PATCH 07/21] Show how resolver can be chained to get system indices Signed-off-by: Craig Perkins --- .../IndexNameExpressionResolverTests.java | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/server/src/test/java/org/opensearch/cluster/metadata/IndexNameExpressionResolverTests.java b/server/src/test/java/org/opensearch/cluster/metadata/IndexNameExpressionResolverTests.java index d95d3e803c198..197bbb1191e5d 100644 --- a/server/src/test/java/org/opensearch/cluster/metadata/IndexNameExpressionResolverTests.java +++ b/server/src/test/java/org/opensearch/cluster/metadata/IndexNameExpressionResolverTests.java @@ -2548,6 +2548,38 @@ public void testSystemIndexMatching() { assertThat(resolver.concreteSystemIndices(".not-exists"), equalTo(Collections.EMPTY_LIST)); } + public void testRegisteredSystemIndexExpansion() { + SystemIndexPlugin plugin1 = new SystemIndexPlugin1(); + SystemIndexPlugin plugin2 = new SystemIndexPlugin2(); + SystemIndices pluginSystemIndices = new SystemIndices( + Map.of( + SystemIndexPlugin1.class.getCanonicalName(), + plugin1.getSystemIndexDescriptors(Settings.EMPTY), + SystemIndexPlugin2.class.getCanonicalName(), + plugin2.getSystemIndexDescriptors(Settings.EMPTY) + ) + ); + IndexNameExpressionResolver resolver = new IndexNameExpressionResolver(threadContext, pluginSystemIndices); + Metadata.Builder mdBuilder = Metadata.builder() + .put(indexBuilder("foo")) + .put(indexBuilder("bar")) + .put(indexBuilder(SystemIndexPlugin1.SYSTEM_INDEX_1)) + .put(indexBuilder(SystemIndexPlugin2.SYSTEM_INDEX_2)); + ClusterState state = ClusterState.builder(new ClusterName("_name")).metadata(mdBuilder).build(); + + // Only closed + IndicesOptions options = IndicesOptions.strictExpand(); + IndexNameExpressionResolver.Context context = new IndexNameExpressionResolver.Context(state, options, true); + String[] results = resolver.concreteIndexNames(context, Strings.EMPTY_ARRAY); + assertEquals(4, results.length); + assertTrue( + Arrays.asList(results).containsAll(List.of("foo", "bar", SystemIndexPlugin1.SYSTEM_INDEX_1, SystemIndexPlugin2.SYSTEM_INDEX_2)) + ); + List systemIndices = resolver.concreteSystemIndices(results); + assertEquals(2, systemIndices.size()); + assertTrue(systemIndices.containsAll(List.of(SystemIndexPlugin1.SYSTEM_INDEX_1, SystemIndexPlugin2.SYSTEM_INDEX_2))); + } + private ClusterState systemIndexTestClusterState() { Settings settings = Settings.builder().build(); Metadata.Builder mdBuilder = Metadata.builder() From 5e8e9af5d0778e8a41e0f4765c9d3e59e0d2f476 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Wed, 26 Jun 2024 11:55:27 -0400 Subject: [PATCH 08/21] Fix forbiddenApis check Signed-off-by: Craig Perkins --- .../src/main/java/org/opensearch/common/WildcardMatcher.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/libs/common/src/main/java/org/opensearch/common/WildcardMatcher.java b/libs/common/src/main/java/org/opensearch/common/WildcardMatcher.java index c9b812bad3852..3ded8a946a3fd 100644 --- a/libs/common/src/main/java/org/opensearch/common/WildcardMatcher.java +++ b/libs/common/src/main/java/org/opensearch/common/WildcardMatcher.java @@ -11,6 +11,7 @@ import java.util.Arrays; import java.util.Collection; import java.util.List; +import java.util.Locale; import java.util.Objects; import java.util.Optional; import java.util.function.Function; @@ -292,12 +293,12 @@ private static final class CasefoldingMatcher extends WildcardMatcher { private final WildcardMatcher inner; public CasefoldingMatcher(String pattern, Function simpleWildcardMatcher) { - this.inner = simpleWildcardMatcher.apply(pattern.toLowerCase()); + this.inner = simpleWildcardMatcher.apply(pattern.toLowerCase(Locale.ROOT)); } @Override public boolean test(String candidate) { - return inner.test(candidate.toLowerCase()); + return inner.test(candidate.toLowerCase(Locale.ROOT)); } @Override From a81f2dbb8b3a56ec8e85d8da1a3cb7080076d0f5 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Wed, 26 Jun 2024 11:58:55 -0400 Subject: [PATCH 09/21] Update CHANGELOG Signed-off-by: Craig Perkins --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2829089b08ef7..1f9778b9616be 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Apply the date histogram rewrite optimization to range aggregation ([#13865](https://github.com/opensearch-project/OpenSearch/pull/13865)) - [Writable Warm] Add composite directory implementation and integrate it with FileCache ([12782](https://github.com/opensearch-project/OpenSearch/pull/12782)) - Fix race condition while parsing derived fields from search definition ([14445](https://github.com/opensearch-project/OpenSearch/pull/14445)) -- Add method to IndexNameExpressionResolver to get registered system indices ([#14415](https://github.com/opensearch-project/OpenSearch/pull/14415)) +- Adds a method in IndexNameExpressionResolver to determine if a request matches system indices ([#14415](https://github.com/opensearch-project/OpenSearch/pull/14415)) ### Dependencies - Bump `org.gradle.test-retry` from 1.5.8 to 1.5.9 ([#13442](https://github.com/opensearch-project/OpenSearch/pull/13442)) From aa87d7fd60f68976b53ef5ab5275a54a5c838cee Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Wed, 26 Jun 2024 12:01:25 -0400 Subject: [PATCH 10/21] Make SystemIndices internal Signed-off-by: Craig Perkins --- .../src/main/java/org/opensearch/indices/SystemIndices.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/server/src/main/java/org/opensearch/indices/SystemIndices.java b/server/src/main/java/org/opensearch/indices/SystemIndices.java index 860ae5a65a12f..7d9c3923e1ad1 100644 --- a/server/src/main/java/org/opensearch/indices/SystemIndices.java +++ b/server/src/main/java/org/opensearch/indices/SystemIndices.java @@ -40,7 +40,6 @@ import org.apache.lucene.util.automaton.MinimizationOperations; import org.apache.lucene.util.automaton.Operations; import org.opensearch.common.Nullable; -import org.opensearch.common.annotation.PublicApi; import org.opensearch.common.collect.Tuple; import org.opensearch.common.regex.Regex; import org.opensearch.core.index.Index; @@ -65,9 +64,8 @@ * node knows about. Methods for determining if an index should be a system index are also provided * to reduce the locations within the code that need to deal with {@link SystemIndexDescriptor}s. * - * @opensearch.api + * @opensearch.internal */ -@PublicApi(since = "2.16.0") public class SystemIndices { private static final Logger logger = LogManager.getLogger(SystemIndices.class); From 766368e1dafc972186ae29b2be193c4d8a97bc52 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Wed, 26 Jun 2024 12:12:13 -0400 Subject: [PATCH 11/21] Remove unneeded changes Signed-off-by: Craig Perkins --- .../opensearch/indices/SystemIndexDescriptor.java | 13 ------------- .../indices/SystemIndexDescriptorTests.java | 7 ------- 2 files changed, 20 deletions(-) diff --git a/server/src/main/java/org/opensearch/indices/SystemIndexDescriptor.java b/server/src/main/java/org/opensearch/indices/SystemIndexDescriptor.java index 8bb45c559cb98..f3212b1e2fae1 100644 --- a/server/src/main/java/org/opensearch/indices/SystemIndexDescriptor.java +++ b/server/src/main/java/org/opensearch/indices/SystemIndexDescriptor.java @@ -106,19 +106,6 @@ public String toString() { return "SystemIndexDescriptor[pattern=[" + indexPattern + "], description=[" + description + "]]"; } - @Override - public boolean equals(Object o) { - if (o == this) return true; - if (!(o instanceof SystemIndexDescriptor)) return false; - SystemIndexDescriptor descriptor = (SystemIndexDescriptor) o; - return descriptor.indexPattern != null && descriptor.indexPattern.equals(indexPattern); - } - - @Override - public int hashCode() { - return Objects.hash(indexPattern); - } - // TODO: Index settings and mapping // TODO: getThreadpool() // TODO: Upgrade handling (reindex script?) diff --git a/server/src/test/java/org/opensearch/indices/SystemIndexDescriptorTests.java b/server/src/test/java/org/opensearch/indices/SystemIndexDescriptorTests.java index d59469bcbb1f2..9044b239b3407 100644 --- a/server/src/test/java/org/opensearch/indices/SystemIndexDescriptorTests.java +++ b/server/src/test/java/org/opensearch/indices/SystemIndexDescriptorTests.java @@ -75,11 +75,4 @@ public void testValidation() { assertThat(ex.getMessage(), containsString("must not start with the character sequence [.*] to prevent conflicts")); } } - - public void testEquals() { - SystemIndexDescriptor descriptor1 = new SystemIndexDescriptor(".system-index", "Descriptor"); - SystemIndexDescriptor descriptor2 = new SystemIndexDescriptor(".system-index", "Other Descriptor"); - - assertThat(descriptor1, equalTo(descriptor2)); - } } From 439cc647cc5dbf9d5e78ab6ae121bd05097dd02c Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Wed, 26 Jun 2024 14:00:46 -0400 Subject: [PATCH 12/21] Fix CI failures Signed-off-by: Craig Perkins --- .../org/opensearch/common/WildcardMatcherTests.java | 10 ++-------- .../opensearch/indices/SystemIndexDescriptorTests.java | 1 - 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/libs/common/src/test/java/org/opensearch/common/WildcardMatcherTests.java b/libs/common/src/test/java/org/opensearch/common/WildcardMatcherTests.java index 14c21901f276d..590fa997197a5 100644 --- a/libs/common/src/test/java/org/opensearch/common/WildcardMatcherTests.java +++ b/libs/common/src/test/java/org/opensearch/common/WildcardMatcherTests.java @@ -8,12 +8,9 @@ package org.opensearch.common; -import org.junit.Test; +import org.opensearch.test.OpenSearchTestCase; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; - -public class WildcardMatcherTests { +public class WildcardMatcherTests extends OpenSearchTestCase { static private WildcardMatcher wc(String pattern) { return WildcardMatcher.from(pattern); } @@ -22,7 +19,6 @@ static private WildcardMatcher iwc(String pattern) { return WildcardMatcher.from(pattern, false); } - @Test public void testWildcardMatcherClasses() { assertFalse(wc("a*?").test("a")); assertTrue(wc("a*?").test("aa")); @@ -46,7 +42,6 @@ public void testWildcardMatcherClasses() { assertTrue(WildcardMatcher.from(null, "abc").test("abc")); } - @Test public void testWildcardMatcherClassesCaseInsensitive() { assertTrue(iwc("AbC").test("abc")); assertTrue(iwc("abc").test("aBC")); @@ -57,7 +52,6 @@ public void testWildcardMatcherClassesCaseInsensitive() { assertTrue(iwc("/^\\w+$/").test("AbCd")); } - @Test public void testWildcardMatchers() { assertTrue(!WildcardMatcher.from("a*?").test("a")); assertTrue(WildcardMatcher.from("a*?").test("aa")); diff --git a/server/src/test/java/org/opensearch/indices/SystemIndexDescriptorTests.java b/server/src/test/java/org/opensearch/indices/SystemIndexDescriptorTests.java index 9044b239b3407..f2bdaa1fe8c80 100644 --- a/server/src/test/java/org/opensearch/indices/SystemIndexDescriptorTests.java +++ b/server/src/test/java/org/opensearch/indices/SystemIndexDescriptorTests.java @@ -35,7 +35,6 @@ import org.opensearch.test.OpenSearchTestCase; import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.equalTo; public class SystemIndexDescriptorTests extends OpenSearchTestCase { From c6a76795c1e7ac5bb762cfb012b8fca4e7ab894c Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Wed, 26 Jun 2024 14:24:05 -0400 Subject: [PATCH 13/21] Fix precommit errors Signed-off-by: Craig Perkins --- .../cluster/metadata/IndexNameExpressionResolverTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/test/java/org/opensearch/cluster/metadata/IndexNameExpressionResolverTests.java b/server/src/test/java/org/opensearch/cluster/metadata/IndexNameExpressionResolverTests.java index 197bbb1191e5d..c5f7b03a37d7d 100644 --- a/server/src/test/java/org/opensearch/cluster/metadata/IndexNameExpressionResolverTests.java +++ b/server/src/test/java/org/opensearch/cluster/metadata/IndexNameExpressionResolverTests.java @@ -2545,7 +2545,7 @@ public void testSystemIndexMatching() { ); assertThat(resolver.concreteSystemIndices(".system-index1"), equalTo(List.of(SystemIndexPlugin1.SYSTEM_INDEX_1))); assertThat(resolver.concreteSystemIndices(".system-index2"), equalTo(List.of(SystemIndexPlugin2.SYSTEM_INDEX_2))); - assertThat(resolver.concreteSystemIndices(".not-exists"), equalTo(Collections.EMPTY_LIST)); + assertThat(resolver.concreteSystemIndices(".not-exists"), equalTo(Collections.emptyList())); } public void testRegisteredSystemIndexExpansion() { From 8b4196e075165c45c5b0f10a26e9969fb41cd3ea Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Mon, 1 Jul 2024 14:08:43 -0400 Subject: [PATCH 14/21] Use Regex instead of WildcardMatcher Signed-off-by: Craig Perkins --- .../opensearch/common/WildcardMatcher.java | 499 ------------------ .../common/WildcardMatcherTests.java | 74 --- .../metadata/IndexNameExpressionResolver.java | 9 +- 3 files changed, 4 insertions(+), 578 deletions(-) delete mode 100644 libs/common/src/main/java/org/opensearch/common/WildcardMatcher.java delete mode 100644 libs/common/src/test/java/org/opensearch/common/WildcardMatcherTests.java diff --git a/libs/common/src/main/java/org/opensearch/common/WildcardMatcher.java b/libs/common/src/main/java/org/opensearch/common/WildcardMatcher.java deleted file mode 100644 index 3ded8a946a3fd..0000000000000 --- a/libs/common/src/main/java/org/opensearch/common/WildcardMatcher.java +++ /dev/null @@ -1,499 +0,0 @@ -/* - * 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.common; - -import java.util.Arrays; -import java.util.Collection; -import java.util.List; -import java.util.Locale; -import java.util.Objects; -import java.util.Optional; -import java.util.function.Function; -import java.util.function.Predicate; -import java.util.regex.Pattern; -import java.util.stream.Collector; -import java.util.stream.Collectors; -import java.util.stream.Stream; - -/** - * Class used to create matchers that can be used to test for matching patterns. - * - * WildcardMatcher supports matching using regex patterns. - * - * WildcardMatcher as supports wildcard (*) for matching any pattern and (?) for matching any single character. - * - * @opensearch.api - */ -public abstract class WildcardMatcher implements Predicate { - - public static final WildcardMatcher ANY = new WildcardMatcher() { - - @Override - public boolean matchAny(Stream candidates) { - return true; - } - - @Override - public boolean matchAny(Collection candidates) { - return true; - } - - @Override - public boolean matchAny(String... candidates) { - return true; - } - - @Override - public boolean matchAll(Stream candidates) { - return true; - } - - @Override - public boolean matchAll(Collection candidates) { - return true; - } - - @Override - public boolean matchAll(String[] candidates) { - return true; - } - - @Override - public > T getMatchAny(Stream candidates, Collector collector) { - return candidates.collect(collector); - } - - @Override - public boolean test(String candidate) { - return true; - } - - @Override - public String toString() { - return "*"; - } - }; - - public static final WildcardMatcher NONE = new WildcardMatcher() { - - @Override - public boolean matchAny(Stream candidates) { - return false; - } - - @Override - public boolean matchAny(Collection candidates) { - return false; - } - - @Override - public boolean matchAny(String... candidates) { - return false; - } - - @Override - public boolean matchAll(Stream candidates) { - return false; - } - - @Override - public boolean matchAll(Collection candidates) { - return false; - } - - @Override - public boolean matchAll(String[] candidates) { - return false; - } - - @Override - public > T getMatchAny(Stream candidates, Collector collector) { - return Stream.empty().collect(collector); - } - - @Override - public > T getMatchAny(Collection candidate, Collector collector) { - return Stream.empty().collect(collector); - } - - @Override - public > T getMatchAny(String[] candidate, Collector collector) { - return Stream.empty().collect(collector); - } - - @Override - public boolean test(String candidate) { - return false; - } - - @Override - public String toString() { - return ""; - } - }; - - public static WildcardMatcher from(String pattern, boolean caseSensitive) { - if (pattern == null) { - return NONE; - } else if (pattern.equals("*")) { - return ANY; - } else if (pattern.startsWith("/") && pattern.endsWith("/")) { - return new RegexMatcher(pattern, caseSensitive); - } else if (pattern.indexOf('?') >= 0 || pattern.indexOf('*') >= 0) { - return caseSensitive ? new SimpleMatcher(pattern) : new CasefoldingMatcher(pattern, SimpleMatcher::new); - } else { - return caseSensitive ? new Exact(pattern) : new CasefoldingMatcher(pattern, Exact::new); - } - } - - public static WildcardMatcher from(String pattern) { - return from(pattern, true); - } - - // This may in future use more optimized techniques to combine multiple WildcardMatchers in a single automaton - public static WildcardMatcher from(Stream stream, boolean caseSensitive) { - Collection matchers = stream.map(t -> { - if (t == null) { - return NONE; - } else if (t instanceof String) { - return WildcardMatcher.from(((String) t), caseSensitive); - } else if (t instanceof WildcardMatcher) { - return ((WildcardMatcher) t); - } - throw new UnsupportedOperationException("WildcardMatcher can't be constructed from " + t.getClass().getSimpleName()); - }).collect(Collectors.toSet()); - - if (matchers.isEmpty()) { - return NONE; - } else if (matchers.size() == 1) { - return matchers.stream().findFirst().get(); - } - return new MatcherCombiner(matchers); - } - - public static WildcardMatcher from(Collection collection, boolean caseSensitive) { - if (collection == null || collection.isEmpty()) { - return NONE; - } else if (collection.size() == 1) { - T t = collection.stream().findFirst().get(); - if (t instanceof String) { - return from(((String) t), caseSensitive); - } else if (t instanceof WildcardMatcher) { - return ((WildcardMatcher) t); - } - throw new UnsupportedOperationException("WildcardMatcher can't be constructed from " + t.getClass().getSimpleName()); - } - return from(collection.stream(), caseSensitive); - } - - public static WildcardMatcher from(String[] patterns, boolean caseSensitive) { - if (patterns == null || patterns.length == 0) { - return NONE; - } else if (patterns.length == 1) { - return from(patterns[0], caseSensitive); - } - return from(Arrays.stream(patterns), caseSensitive); - } - - public static WildcardMatcher from(Stream patterns) { - return from(patterns, true); - } - - public static WildcardMatcher from(Collection patterns) { - return from(patterns, true); - } - - public static WildcardMatcher from(String... patterns) { - return from(patterns, true); - } - - public WildcardMatcher concat(Stream matchers) { - return new WildcardMatcher.MatcherCombiner(Stream.concat(matchers, Stream.of(this)).collect(Collectors.toSet())); - } - - public WildcardMatcher concat(Collection matchers) { - if (matchers.isEmpty()) { - return this; - } - return concat(matchers.stream()); - } - - public WildcardMatcher concat(WildcardMatcher... matchers) { - if (matchers.length == 0) { - return this; - } - return concat(Arrays.stream(matchers)); - } - - public boolean matchAny(Stream candidates) { - return candidates.anyMatch(this); - } - - public boolean matchAny(Collection candidates) { - return matchAny(candidates.stream()); - } - - public boolean matchAny(String... candidates) { - return matchAny(Arrays.stream(candidates)); - } - - public boolean matchAll(Stream candidates) { - return candidates.allMatch(this); - } - - public boolean matchAll(Collection candidates) { - return matchAll(candidates.stream()); - } - - public boolean matchAll(String[] candidates) { - return matchAll(Arrays.stream(candidates)); - } - - public > T getMatchAny(Stream candidates, Collector collector) { - return candidates.filter(this).collect(collector); - } - - public > T getMatchAny(Collection candidate, Collector collector) { - return getMatchAny(candidate.stream(), collector); - } - - public > T getMatchAny(final String[] candidate, Collector collector) { - return getMatchAny(Arrays.stream(candidate), collector); - } - - public Optional findFirst(final String candidate) { - return Optional.ofNullable(test(candidate) ? this : null); - } - - public static List matchers(Collection patterns) { - return patterns.stream().map(p -> WildcardMatcher.from(p, true)).collect(Collectors.toList()); - } - - public static List getAllMatchingPatterns(final Collection matchers, final String candidate) { - return matchers.stream().filter(p -> p.test(candidate)).map(Objects::toString).collect(Collectors.toList()); - } - - public static List getAllMatchingPatterns(final Collection pattern, final Collection candidates) { - return pattern.stream().filter(p -> p.matchAny(candidates)).map(Objects::toString).collect(Collectors.toList()); - } - - // - // --- Implementation specializations --- - // - // Casefolding matcher - sits on top of case-sensitive matcher - // and proxies toLower() of input string to the wrapped matcher - private static final class CasefoldingMatcher extends WildcardMatcher { - - private final WildcardMatcher inner; - - public CasefoldingMatcher(String pattern, Function simpleWildcardMatcher) { - this.inner = simpleWildcardMatcher.apply(pattern.toLowerCase(Locale.ROOT)); - } - - @Override - public boolean test(String candidate) { - return inner.test(candidate.toLowerCase(Locale.ROOT)); - } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - CasefoldingMatcher that = (CasefoldingMatcher) o; - return inner.equals(that.inner); - } - - @Override - public int hashCode() { - return inner.hashCode(); - } - - @Override - public String toString() { - return inner.toString(); - } - } - - /** - * Exact is a subclass of WildcardMatcher that requires the tested candidates to exactly match the pattern - * of the Matcher - */ - public static final class Exact extends WildcardMatcher { - - private final String pattern; - - private Exact(String pattern) { - this.pattern = pattern; - } - - @Override - public boolean test(String candidate) { - return pattern.equals(candidate); - } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - Exact that = (Exact) o; - return pattern.equals(that.pattern); - } - - @Override - public int hashCode() { - return pattern.hashCode(); - } - - @Override - public String toString() { - return pattern; - } - } - - // RegexMatcher uses JDK Pattern to test for matching, - // assumes "//" strings as input pattern - private static final class RegexMatcher extends WildcardMatcher { - - private final Pattern pattern; - - private RegexMatcher(String pattern, boolean caseSensitive) { - if (!(pattern.length() > 1 && pattern.startsWith("/") && pattern.endsWith("/"))) { - throw new IllegalArgumentException("Invalid regex. Must have length greater than 1 and begin and end with a /"); - } - final String stripSlashesPattern = pattern.substring(1, pattern.length() - 1); - this.pattern = caseSensitive - ? Pattern.compile(stripSlashesPattern) - : Pattern.compile(stripSlashesPattern, Pattern.CASE_INSENSITIVE); - } - - @Override - public boolean test(String candidate) { - return pattern.matcher(candidate).matches(); - } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - RegexMatcher that = (RegexMatcher) o; - return pattern.pattern().equals(that.pattern.pattern()); - } - - @Override - public int hashCode() { - return pattern.pattern().hashCode(); - } - - @Override - public String toString() { - return "/" + pattern.pattern() + "/"; - } - } - - // Simple implementation of WildcardMatcher matcher with * and ? without - // using exlicit stack or recursion (as long as we don't need sub-matches it does work) - // allows us to save on resources and heap allocations unless Regex is required - private static final class SimpleMatcher extends WildcardMatcher { - - private final String pattern; - - SimpleMatcher(String pattern) { - this.pattern = pattern; - } - - @Override - public boolean test(String candidate) { - int i = 0; - int j = 0; - int n = candidate.length(); - int m = pattern.length(); - int text_backup = -1; - int wild_backup = -1; - while (i < n) { - if (j < m && pattern.charAt(j) == '*') { - text_backup = i; - wild_backup = ++j; - } else if (j < m && (pattern.charAt(j) == '?' || pattern.charAt(j) == candidate.charAt(i))) { - i++; - j++; - } else { - if (wild_backup == -1) return false; - i = ++text_backup; - j = wild_backup; - } - } - while (j < m && pattern.charAt(j) == '*') - j++; - return j >= m; - } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - SimpleMatcher that = (SimpleMatcher) o; - return pattern.equals(that.pattern); - } - - @Override - public int hashCode() { - return pattern.hashCode(); - } - - @Override - public String toString() { - return pattern; - } - } - - // MatcherCombiner is a combination of a set of matchers - // matches if any of the set do - // Empty MultiMatcher always returns false - private static final class MatcherCombiner extends WildcardMatcher { - - private final Collection wildcardMatchers; - private final int hashCode; - - MatcherCombiner(Collection wildcardMatchers) { - if (!(wildcardMatchers.size() > 1)) { - throw new IllegalArgumentException("Must provide at least 1 wildcard matcher"); - } - this.wildcardMatchers = wildcardMatchers; - hashCode = wildcardMatchers.hashCode(); - } - - @Override - public boolean test(String candidate) { - return wildcardMatchers.stream().anyMatch(m -> m.test(candidate)); - } - - @Override - public Optional findFirst(final String candidate) { - return wildcardMatchers.stream().filter(m -> m.test(candidate)).findFirst(); - } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - MatcherCombiner that = (MatcherCombiner) o; - return wildcardMatchers.equals(that.wildcardMatchers); - } - - @Override - public int hashCode() { - return hashCode; - } - - @Override - public String toString() { - return wildcardMatchers.toString(); - } - } -} diff --git a/libs/common/src/test/java/org/opensearch/common/WildcardMatcherTests.java b/libs/common/src/test/java/org/opensearch/common/WildcardMatcherTests.java deleted file mode 100644 index 590fa997197a5..0000000000000 --- a/libs/common/src/test/java/org/opensearch/common/WildcardMatcherTests.java +++ /dev/null @@ -1,74 +0,0 @@ -/* - * 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.common; - -import org.opensearch.test.OpenSearchTestCase; - -public class WildcardMatcherTests extends OpenSearchTestCase { - static private WildcardMatcher wc(String pattern) { - return WildcardMatcher.from(pattern); - } - - static private WildcardMatcher iwc(String pattern) { - return WildcardMatcher.from(pattern, false); - } - - public void testWildcardMatcherClasses() { - assertFalse(wc("a*?").test("a")); - assertTrue(wc("a*?").test("aa")); - assertTrue(wc("a*?").test("ab")); - assertTrue(wc("a*?").test("abb")); - assertTrue(wc("*my*index").test("myindex")); - assertFalse(wc("*my*index").test("myindex1")); - assertTrue(wc("*my*index?").test("myindex1")); - assertTrue(wc("*my*index").test("this_is_my_great_index")); - assertFalse(wc("*my*index").test("MYindex")); - assertFalse(wc("?kibana").test("kibana")); - assertTrue(wc("?kibana").test(".kibana")); - assertFalse(wc("?kibana").test("kibana.")); - assertTrue(wc("?kibana?").test("?kibana.")); - assertTrue(wc("/(\\d{3}-?\\d{2}-?\\d{4})/").test("123-45-6789")); - assertFalse(wc("(\\d{3}-?\\d{2}-?\\d{4})").test("123-45-6789")); - assertTrue(wc("/\\S+/").test("abc")); - assertTrue(wc("abc").test("abc")); - assertFalse(wc("ABC").test("abc")); - assertFalse(wc(null).test("abc")); - assertTrue(WildcardMatcher.from(null, "abc").test("abc")); - } - - public void testWildcardMatcherClassesCaseInsensitive() { - assertTrue(iwc("AbC").test("abc")); - assertTrue(iwc("abc").test("aBC")); - assertTrue(iwc("A*b").test("ab")); - assertTrue(iwc("A*b").test("aab")); - assertTrue(iwc("A*b").test("abB")); - assertFalse(iwc("abc").test("AB")); - assertTrue(iwc("/^\\w+$/").test("AbCd")); - } - - public void testWildcardMatchers() { - assertTrue(!WildcardMatcher.from("a*?").test("a")); - assertTrue(WildcardMatcher.from("a*?").test("aa")); - assertTrue(WildcardMatcher.from("a*?").test("ab")); - assertTrue(WildcardMatcher.from("*my*index").test("myindex")); - assertTrue(!WildcardMatcher.from("*my*index").test("myindex1")); - assertTrue(WildcardMatcher.from("*my*index?").test("myindex1")); - assertTrue(WildcardMatcher.from("*my*index").test("this_is_my_great_index")); - assertTrue(!WildcardMatcher.from("*my*index").test("MYindex")); - assertTrue(!WildcardMatcher.from("?kibana").test("kibana")); - assertTrue(WildcardMatcher.from("?kibana").test(".kibana")); - assertTrue(!WildcardMatcher.from("?kibana").test("kibana.")); - assertTrue(WildcardMatcher.from("?kibana?").test("?kibana.")); - assertTrue(WildcardMatcher.from("/(\\d{3}-?\\d{2}-?\\d{4})/").test("123-45-6789")); - assertTrue(!WildcardMatcher.from("(\\d{3}-?\\d{2}-?\\d{4})").test("123-45-6789")); - assertTrue(WildcardMatcher.from("/\\S*/").test("abc")); - assertTrue(WildcardMatcher.from("abc").test("abc")); - assertTrue(!WildcardMatcher.from("ABC").test("abc")); - } -} diff --git a/server/src/main/java/org/opensearch/cluster/metadata/IndexNameExpressionResolver.java b/server/src/main/java/org/opensearch/cluster/metadata/IndexNameExpressionResolver.java index 0c81c014fcec0..17d6c72187787 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/IndexNameExpressionResolver.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/IndexNameExpressionResolver.java @@ -38,7 +38,6 @@ import org.opensearch.cluster.ClusterState; import org.opensearch.common.Booleans; import org.opensearch.common.Nullable; -import org.opensearch.common.WildcardMatcher; import org.opensearch.common.annotation.PublicApi; import org.opensearch.common.collect.Tuple; import org.opensearch.common.logging.DeprecationLogger; @@ -93,17 +92,17 @@ public class IndexNameExpressionResolver { private final List expressionResolvers = List.of(dateMathExpressionResolver, wildcardExpressionResolver); private final ThreadContext threadContext; - private final WildcardMatcher systemIndexMatcher; + private final String[] systemIndices; public IndexNameExpressionResolver(ThreadContext threadContext) { this.threadContext = Objects.requireNonNull(threadContext, "Thread Context must not be null"); - this.systemIndexMatcher = WildcardMatcher.NONE; + this.systemIndices = new String[0]; } public IndexNameExpressionResolver(ThreadContext threadContext, SystemIndices systemIndices) { this.threadContext = Objects.requireNonNull(threadContext, "Thread Context must not be null"); List allSystemIndexPatterns = systemIndices.getAllSystemIndexPatterns(); - this.systemIndexMatcher = WildcardMatcher.from(allSystemIndexPatterns); + this.systemIndices = allSystemIndexPatterns.toArray(new String[0]); } /** @@ -175,7 +174,7 @@ public String[] concreteIndexNames(ClusterState state, IndicesOptions options, I } public List concreteSystemIndices(String... concreteIndices) { - return systemIndexMatcher.getMatchAny(concreteIndices, Collectors.toList()); + return Arrays.stream(concreteIndices).filter(idx -> Regex.simpleMatch(systemIndices, idx)).collect(Collectors.toList()); } public List dataStreamNames(ClusterState state, IndicesOptions options, String... indexExpressions) { From 11192dc949ba1a35665db4ef2e8e83a79ac0364c Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Mon, 1 Jul 2024 15:30:15 -0400 Subject: [PATCH 15/21] Address code review feedback Signed-off-by: Craig Perkins --- CHANGELOG.md | 2 +- .../cluster/metadata/IndexNameExpressionResolver.java | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 893ffdd70a49e..d9ba123d63bc4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Add batching supported processor base type AbstractBatchingProcessor ([#14554](https://github.com/opensearch-project/OpenSearch/pull/14554)) - Fix race condition while parsing derived fields from search definition ([14445](https://github.com/opensearch-project/OpenSearch/pull/14445)) - Add allowlist setting for ingest-common and search-pipeline-common processors ([#14439](https://github.com/opensearch-project/OpenSearch/issues/14439)) -- Adds a method in IndexNameExpressionResolver to determine if a request matches system indices ([#14415](https://github.com/opensearch-project/OpenSearch/pull/14415)) +- Add a method in IndexNameExpressionResolver to determine if a request matches system indices ([#14415](https://github.com/opensearch-project/OpenSearch/pull/14415)) ### 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/cluster/metadata/IndexNameExpressionResolver.java b/server/src/main/java/org/opensearch/cluster/metadata/IndexNameExpressionResolver.java index 17d6c72187787..9c6d422646c38 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/IndexNameExpressionResolver.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/IndexNameExpressionResolver.java @@ -38,6 +38,7 @@ import org.opensearch.cluster.ClusterState; import org.opensearch.common.Booleans; import org.opensearch.common.Nullable; +import org.opensearch.common.annotation.InternalApi; import org.opensearch.common.annotation.PublicApi; import org.opensearch.common.collect.Tuple; import org.opensearch.common.logging.DeprecationLogger; @@ -99,6 +100,7 @@ public IndexNameExpressionResolver(ThreadContext threadContext) { this.systemIndices = new String[0]; } + @InternalApi public IndexNameExpressionResolver(ThreadContext threadContext, SystemIndices systemIndices) { this.threadContext = Objects.requireNonNull(threadContext, "Thread Context must not be null"); List allSystemIndexPatterns = systemIndices.getAllSystemIndexPatterns(); From 884fa4c48a2c127123fbc02f078bbd445f356d02 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Tue, 2 Jul 2024 14:17:17 -0400 Subject: [PATCH 16/21] Allow caller to pass index expressions Signed-off-by: Craig Perkins --- .../metadata/IndexNameExpressionResolver.java | 4 +- .../IndexNameExpressionResolverTests.java | 39 ++++++++++++++++--- 2 files changed, 35 insertions(+), 8 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/metadata/IndexNameExpressionResolver.java b/server/src/main/java/org/opensearch/cluster/metadata/IndexNameExpressionResolver.java index 9c6d422646c38..4e57f9e1ddc88 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/IndexNameExpressionResolver.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/IndexNameExpressionResolver.java @@ -175,8 +175,8 @@ public String[] concreteIndexNames(ClusterState state, IndicesOptions options, I return concreteIndexNames(context, request.indices()); } - public List concreteSystemIndices(String... concreteIndices) { - return Arrays.stream(concreteIndices).filter(idx -> Regex.simpleMatch(systemIndices, idx)).collect(Collectors.toList()); + public List matchesSystemIndexPattern(String... indexExpressions) { + return Arrays.stream(indexExpressions).filter(pattern -> Regex.simpleMatch(systemIndices, pattern)).collect(Collectors.toList()); } public List dataStreamNames(ClusterState state, IndicesOptions options, String... indexExpressions) { diff --git a/server/src/test/java/org/opensearch/cluster/metadata/IndexNameExpressionResolverTests.java b/server/src/test/java/org/opensearch/cluster/metadata/IndexNameExpressionResolverTests.java index c5f7b03a37d7d..20760e59164b0 100644 --- a/server/src/test/java/org/opensearch/cluster/metadata/IndexNameExpressionResolverTests.java +++ b/server/src/test/java/org/opensearch/cluster/metadata/IndexNameExpressionResolverTests.java @@ -2529,23 +2529,40 @@ public void testDataStreamsNames() { public void testSystemIndexMatching() { SystemIndexPlugin plugin1 = new SystemIndexPlugin1(); SystemIndexPlugin plugin2 = new SystemIndexPlugin2(); + SystemIndexPlugin plugin3 = new SystemIndexPatternPlugin(); SystemIndices pluginSystemIndices = new SystemIndices( Map.of( SystemIndexPlugin1.class.getCanonicalName(), plugin1.getSystemIndexDescriptors(Settings.EMPTY), SystemIndexPlugin2.class.getCanonicalName(), - plugin2.getSystemIndexDescriptors(Settings.EMPTY) + plugin2.getSystemIndexDescriptors(Settings.EMPTY), + SystemIndexPatternPlugin.class.getCanonicalName(), + plugin3.getSystemIndexDescriptors(Settings.EMPTY) ) ); IndexNameExpressionResolver resolver = new IndexNameExpressionResolver(threadContext, pluginSystemIndices); assertThat( - resolver.concreteSystemIndices(".system-index1", ".system-index2"), + resolver.matchesSystemIndexPattern(".system-index1", ".system-index2"), equalTo(List.of(SystemIndexPlugin1.SYSTEM_INDEX_1, SystemIndexPlugin2.SYSTEM_INDEX_2)) ); - assertThat(resolver.concreteSystemIndices(".system-index1"), equalTo(List.of(SystemIndexPlugin1.SYSTEM_INDEX_1))); - assertThat(resolver.concreteSystemIndices(".system-index2"), equalTo(List.of(SystemIndexPlugin2.SYSTEM_INDEX_2))); - assertThat(resolver.concreteSystemIndices(".not-exists"), equalTo(Collections.emptyList())); + assertThat(resolver.matchesSystemIndexPattern(".system-index1"), equalTo(List.of(SystemIndexPlugin1.SYSTEM_INDEX_1))); + assertThat(resolver.matchesSystemIndexPattern(".system-index2"), equalTo(List.of(SystemIndexPlugin2.SYSTEM_INDEX_2))); + assertThat(resolver.matchesSystemIndexPattern(".system-index-pattern1"), equalTo(List.of(".system-index-pattern1"))); + assertThat(resolver.matchesSystemIndexPattern(".system-index-pattern-sub*"), equalTo(List.of(".system-index-pattern-sub*"))); + assertThat( + resolver.matchesSystemIndexPattern(".system-index-pattern1", ".system-index-pattern2"), + equalTo(List.of(".system-index-pattern1", ".system-index-pattern2")) + ); + assertThat( + resolver.matchesSystemIndexPattern(".system-index1", ".system-index-pattern1"), + equalTo(List.of(".system-index1", ".system-index-pattern1")) + ); + assertThat( + resolver.matchesSystemIndexPattern(".system-index1", ".system-index-pattern1", ".not-system"), + equalTo(List.of(".system-index1", ".system-index-pattern1")) + ); + assertThat(resolver.matchesSystemIndexPattern(".not-system"), equalTo(Collections.emptyList())); } public void testRegisteredSystemIndexExpansion() { @@ -2575,7 +2592,7 @@ public void testRegisteredSystemIndexExpansion() { assertTrue( Arrays.asList(results).containsAll(List.of("foo", "bar", SystemIndexPlugin1.SYSTEM_INDEX_1, SystemIndexPlugin2.SYSTEM_INDEX_2)) ); - List systemIndices = resolver.concreteSystemIndices(results); + List systemIndices = resolver.matchesSystemIndexPattern(results); assertEquals(2, systemIndices.size()); assertTrue(systemIndices.containsAll(List.of(SystemIndexPlugin1.SYSTEM_INDEX_1, SystemIndexPlugin2.SYSTEM_INDEX_2))); } @@ -2614,4 +2631,14 @@ public Collection getSystemIndexDescriptors(Settings sett return Collections.singletonList(systemIndexDescriptor); } } + + static final class SystemIndexPatternPlugin extends Plugin implements SystemIndexPlugin { + public static final String SYSTEM_INDEX_PATTERN = ".system-index-pattern*"; + + @Override + public Collection getSystemIndexDescriptors(Settings settings) { + final SystemIndexDescriptor systemIndexDescriptor = new SystemIndexDescriptor(SYSTEM_INDEX_PATTERN, "System index pattern"); + return Collections.singletonList(systemIndexDescriptor); + } + } } From a75e1679ec1fa0d1da9216e5e0fb44e622a91e24 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Wed, 3 Jul 2024 15:48:09 -0400 Subject: [PATCH 17/21] Create SystemIndexRegistry Signed-off-by: Craig Perkins --- .../org/opensearch/cluster/ClusterModule.java | 6 +- .../metadata/IndexNameExpressionResolver.java | 15 -- .../indices/SystemIndexRegistry.java | 131 ++++++++++++++++++ .../org/opensearch/indices/SystemIndices.java | 97 +------------ .../main/java/org/opensearch/node/Node.java | 3 +- .../cluster/ClusterModuleTests.java | 34 +---- .../IndexNameExpressionResolverTests.java | 107 -------------- .../indices/SystemIndicesTests.java | 105 +++++++++++++- 8 files changed, 250 insertions(+), 248 deletions(-) create mode 100644 server/src/main/java/org/opensearch/indices/SystemIndexRegistry.java diff --git a/server/src/main/java/org/opensearch/cluster/ClusterModule.java b/server/src/main/java/org/opensearch/cluster/ClusterModule.java index 18bc402f41fa5..c7fd263bda56a 100644 --- a/server/src/main/java/org/opensearch/cluster/ClusterModule.java +++ b/server/src/main/java/org/opensearch/cluster/ClusterModule.java @@ -93,7 +93,6 @@ import org.opensearch.core.xcontent.NamedXContentRegistry; import org.opensearch.gateway.GatewayAllocator; import org.opensearch.gateway.ShardsBatchGatewayAllocator; -import org.opensearch.indices.SystemIndices; import org.opensearch.ingest.IngestMetadata; import org.opensearch.persistent.PersistentTasksCustomMetadata; import org.opensearch.persistent.PersistentTasksNodeService; @@ -147,15 +146,14 @@ public ClusterModule( ClusterInfoService clusterInfoService, SnapshotsInfoService snapshotsInfoService, ThreadContext threadContext, - ClusterManagerMetrics clusterManagerMetrics, - SystemIndices systemIndices + ClusterManagerMetrics clusterManagerMetrics ) { this.clusterPlugins = clusterPlugins; this.deciderList = createAllocationDeciders(settings, clusterService.getClusterSettings(), clusterPlugins); this.allocationDeciders = new AllocationDeciders(deciderList); this.shardsAllocator = createShardsAllocator(settings, clusterService.getClusterSettings(), clusterPlugins); this.clusterService = clusterService; - this.indexNameExpressionResolver = new IndexNameExpressionResolver(threadContext, systemIndices); + this.indexNameExpressionResolver = new IndexNameExpressionResolver(threadContext); this.allocationService = new AllocationService( allocationDeciders, shardsAllocator, diff --git a/server/src/main/java/org/opensearch/cluster/metadata/IndexNameExpressionResolver.java b/server/src/main/java/org/opensearch/cluster/metadata/IndexNameExpressionResolver.java index 4e57f9e1ddc88..24ff83d638d4b 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/IndexNameExpressionResolver.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/IndexNameExpressionResolver.java @@ -38,7 +38,6 @@ import org.opensearch.cluster.ClusterState; import org.opensearch.common.Booleans; import org.opensearch.common.Nullable; -import org.opensearch.common.annotation.InternalApi; import org.opensearch.common.annotation.PublicApi; import org.opensearch.common.collect.Tuple; import org.opensearch.common.logging.DeprecationLogger; @@ -54,7 +53,6 @@ import org.opensearch.index.IndexNotFoundException; import org.opensearch.indices.IndexClosedException; import org.opensearch.indices.InvalidIndexNameException; -import org.opensearch.indices.SystemIndices; import java.time.Instant; import java.time.ZoneId; @@ -93,18 +91,9 @@ public class IndexNameExpressionResolver { private final List expressionResolvers = List.of(dateMathExpressionResolver, wildcardExpressionResolver); private final ThreadContext threadContext; - private final String[] systemIndices; public IndexNameExpressionResolver(ThreadContext threadContext) { this.threadContext = Objects.requireNonNull(threadContext, "Thread Context must not be null"); - this.systemIndices = new String[0]; - } - - @InternalApi - public IndexNameExpressionResolver(ThreadContext threadContext, SystemIndices systemIndices) { - this.threadContext = Objects.requireNonNull(threadContext, "Thread Context must not be null"); - List allSystemIndexPatterns = systemIndices.getAllSystemIndexPatterns(); - this.systemIndices = allSystemIndexPatterns.toArray(new String[0]); } /** @@ -175,10 +164,6 @@ public String[] concreteIndexNames(ClusterState state, IndicesOptions options, I return concreteIndexNames(context, request.indices()); } - public List matchesSystemIndexPattern(String... indexExpressions) { - return Arrays.stream(indexExpressions).filter(pattern -> Regex.simpleMatch(systemIndices, pattern)).collect(Collectors.toList()); - } - public List dataStreamNames(ClusterState state, IndicesOptions options, String... indexExpressions) { // Allow system index access - they'll be filtered out below as there's no such thing (yet) as system data streams Context context = new Context(state, options, false, false, true, true, true); diff --git a/server/src/main/java/org/opensearch/indices/SystemIndexRegistry.java b/server/src/main/java/org/opensearch/indices/SystemIndexRegistry.java new file mode 100644 index 0000000000000..a784adb6e82b5 --- /dev/null +++ b/server/src/main/java/org/opensearch/indices/SystemIndexRegistry.java @@ -0,0 +1,131 @@ +/* + * 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.indices; + +import org.apache.lucene.util.automaton.Automaton; +import org.apache.lucene.util.automaton.Operations; +import org.opensearch.common.collect.Tuple; +import org.opensearch.common.regex.Regex; +import org.opensearch.tasks.TaskResultsService; + +import java.util.Arrays; +import java.util.Collection; +import java.util.Comparator; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; + +import static java.util.Collections.singletonList; +import static java.util.Collections.singletonMap; +import static java.util.Collections.unmodifiableMap; +import static org.opensearch.tasks.TaskResultsService.TASK_INDEX; + +class SystemIndexRegistry { + private static SystemIndexRegistry INSTANCE = null; + private static final SystemIndexDescriptor TASK_INDEX_DESCRIPTOR = new SystemIndexDescriptor(TASK_INDEX + "*", "Task Result Index"); + static final Map> SERVER_SYSTEM_INDEX_DESCRIPTORS = singletonMap( + TaskResultsService.class.getName(), + singletonList(TASK_INDEX_DESCRIPTOR) + ); + private static String[] SYSTEM_INDEX_PATTERNS = new String[0]; + static Collection SYSTEM_INDEX_DESCRIPTORS; + + private SystemIndexRegistry(Map> pluginAndModulesDescriptors) { + final Map> descriptorsMap = buildSystemIndexDescriptorMap(pluginAndModulesDescriptors); + checkForOverlappingPatterns(descriptorsMap); + List descriptors = pluginAndModulesDescriptors.values() + .stream() + .flatMap(Collection::stream) + .collect(Collectors.toList()); + descriptors.add(TASK_INDEX_DESCRIPTOR); + SYSTEM_INDEX_DESCRIPTORS = descriptors.stream().collect(Collectors.toUnmodifiableList()); + SYSTEM_INDEX_PATTERNS = descriptors.stream().map(SystemIndexDescriptor::getIndexPattern).toArray(String[]::new); + } + + public static synchronized SystemIndexRegistry initialize(Map> pluginAndModulesDescriptors) { + if (INSTANCE == null) { + INSTANCE = new SystemIndexRegistry(pluginAndModulesDescriptors); + } + return INSTANCE; + } + + public static List matchesSystemIndexPattern(String... indexExpressions) { + return Arrays.stream(indexExpressions) + .filter(pattern -> Regex.simpleMatch(SYSTEM_INDEX_PATTERNS, pattern)) + .collect(Collectors.toList()); + } + + /** + * Given a collection of {@link SystemIndexDescriptor}s and their sources, checks to see if the index patterns of the listed + * descriptors overlap with any of the other patterns. If any do, throws an exception. + * + * @param sourceToDescriptors A map of source (plugin) names to the SystemIndexDescriptors they provide. + * @throws IllegalStateException Thrown if any of the index patterns overlaps with another. + */ + static void checkForOverlappingPatterns(Map> sourceToDescriptors) { + List> sourceDescriptorPair = sourceToDescriptors.entrySet() + .stream() + .flatMap(entry -> entry.getValue().stream().map(descriptor -> new Tuple<>(entry.getKey(), descriptor))) + .sorted(Comparator.comparing(d -> d.v1() + ":" + d.v2().getIndexPattern())) // Consistent ordering -> consistent error message + .collect(Collectors.toList()); + + // This is O(n^2) with the number of system index descriptors, and each check is quadratic with the number of states in the + // automaton, but the absolute number of system index descriptors should be quite small (~10s at most), and the number of states + // per pattern should be low as well. If these assumptions change, this might need to be reworked. + sourceDescriptorPair.forEach(descriptorToCheck -> { + List> descriptorsMatchingThisPattern = sourceDescriptorPair.stream() + + .filter(d -> descriptorToCheck.v2() != d.v2()) // Exclude the pattern currently being checked + .filter(d -> overlaps(descriptorToCheck.v2(), d.v2())) + .collect(Collectors.toList()); + if (descriptorsMatchingThisPattern.isEmpty() == false) { + throw new IllegalStateException( + "a system index descriptor [" + + descriptorToCheck.v2() + + "] from [" + + descriptorToCheck.v1() + + "] overlaps with other system index descriptors: [" + + descriptorsMatchingThisPattern.stream() + .map(descriptor -> descriptor.v2() + " from [" + descriptor.v1() + "]") + .collect(Collectors.joining(", ")) + ); + } + }); + } + + private static boolean overlaps(SystemIndexDescriptor a1, SystemIndexDescriptor a2) { + Automaton a1Automaton = Regex.simpleMatchToAutomaton(a1.getIndexPattern()); + Automaton a2Automaton = Regex.simpleMatchToAutomaton(a2.getIndexPattern()); + return Operations.isEmpty(Operations.intersection(a1Automaton, a2Automaton)) == false; + } + + private static Map> buildSystemIndexDescriptorMap( + Map> pluginAndModulesMap + ) { + final Map> map = new HashMap<>( + pluginAndModulesMap.size() + SERVER_SYSTEM_INDEX_DESCRIPTORS.size() + ); + map.putAll(pluginAndModulesMap); + // put the server items last since we expect less of them + SERVER_SYSTEM_INDEX_DESCRIPTORS.forEach((source, descriptors) -> { + if (map.putIfAbsent(source, descriptors) != null) { + throw new IllegalArgumentException( + "plugin or module attempted to define the same source [" + source + "] as a built-in system index" + ); + } + }); + return unmodifiableMap(map); + } + + // visible for testing + static void clear() { + INSTANCE = null; + } +} diff --git a/server/src/main/java/org/opensearch/indices/SystemIndices.java b/server/src/main/java/org/opensearch/indices/SystemIndices.java index 7d9c3923e1ad1..88fd9a920dca1 100644 --- a/server/src/main/java/org/opensearch/indices/SystemIndices.java +++ b/server/src/main/java/org/opensearch/indices/SystemIndices.java @@ -40,50 +40,33 @@ import org.apache.lucene.util.automaton.MinimizationOperations; import org.apache.lucene.util.automaton.Operations; import org.opensearch.common.Nullable; -import org.opensearch.common.collect.Tuple; +import org.opensearch.common.annotation.PublicApi; import org.opensearch.common.regex.Regex; import org.opensearch.core.index.Index; -import org.opensearch.tasks.TaskResultsService; import java.util.Collection; -import java.util.Comparator; -import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Optional; import java.util.stream.Collectors; -import static java.util.Collections.singletonList; -import static java.util.Collections.singletonMap; -import static java.util.Collections.unmodifiableList; -import static java.util.Collections.unmodifiableMap; -import static org.opensearch.tasks.TaskResultsService.TASK_INDEX; - /** * This class holds the {@link SystemIndexDescriptor} objects that represent system indices the * node knows about. Methods for determining if an index should be a system index are also provided * to reduce the locations within the code that need to deal with {@link SystemIndexDescriptor}s. * - * @opensearch.internal + * @opensearch.api */ +@PublicApi(since = "2.16.0") public class SystemIndices { private static final Logger logger = LogManager.getLogger(SystemIndices.class); - private static final Map> SERVER_SYSTEM_INDEX_DESCRIPTORS = singletonMap( - TaskResultsService.class.getName(), - singletonList(new SystemIndexDescriptor(TASK_INDEX + "*", "Task Result Index")) - ); - private final CharacterRunAutomaton runAutomaton; - private final Collection systemIndexDescriptors; public SystemIndices(Map> pluginAndModulesDescriptors) { - final Map> descriptorsMap = buildSystemIndexDescriptorMap(pluginAndModulesDescriptors); - checkForOverlappingPatterns(descriptorsMap); - this.systemIndexDescriptors = unmodifiableList( - descriptorsMap.values().stream().flatMap(Collection::stream).collect(Collectors.toList()) - ); - this.runAutomaton = buildCharacterRunAutomaton(systemIndexDescriptors); + SystemIndexRegistry.initialize(pluginAndModulesDescriptors); + SystemIndexRegistry.initialize(pluginAndModulesDescriptors); + this.runAutomaton = buildCharacterRunAutomaton(SystemIndexRegistry.SYSTEM_INDEX_DESCRIPTORS); } /** @@ -104,10 +87,6 @@ public boolean isSystemIndex(String indexName) { return runAutomaton.run(indexName); } - public List getAllSystemIndexPatterns() { - return systemIndexDescriptors.stream().map(SystemIndexDescriptor::getIndexPattern).collect(Collectors.toList()); - } - /** * Finds a single matching {@link SystemIndexDescriptor}, if any, for the given index name. * @param name the name of the index @@ -115,7 +94,7 @@ public List getAllSystemIndexPatterns() { * @throws IllegalStateException if multiple descriptors match the name */ public @Nullable SystemIndexDescriptor findMatchingDescriptor(String name) { - final List matchingDescriptors = systemIndexDescriptors.stream() + final List matchingDescriptors = SystemIndexRegistry.SYSTEM_INDEX_DESCRIPTORS.stream() .filter(descriptor -> descriptor.matchesIndexPattern(name)) .collect(Collectors.toList()); @@ -172,66 +151,4 @@ private static CharacterRunAutomaton buildCharacterRunAutomaton(Collection> sourceToDescriptors) { - List> sourceDescriptorPair = sourceToDescriptors.entrySet() - .stream() - .flatMap(entry -> entry.getValue().stream().map(descriptor -> new Tuple<>(entry.getKey(), descriptor))) - .sorted(Comparator.comparing(d -> d.v1() + ":" + d.v2().getIndexPattern())) // Consistent ordering -> consistent error message - .collect(Collectors.toList()); - - // This is O(n^2) with the number of system index descriptors, and each check is quadratic with the number of states in the - // automaton, but the absolute number of system index descriptors should be quite small (~10s at most), and the number of states - // per pattern should be low as well. If these assumptions change, this might need to be reworked. - sourceDescriptorPair.forEach(descriptorToCheck -> { - List> descriptorsMatchingThisPattern = sourceDescriptorPair.stream() - - .filter(d -> descriptorToCheck.v2() != d.v2()) // Exclude the pattern currently being checked - .filter(d -> overlaps(descriptorToCheck.v2(), d.v2())) - .collect(Collectors.toList()); - if (descriptorsMatchingThisPattern.isEmpty() == false) { - throw new IllegalStateException( - "a system index descriptor [" - + descriptorToCheck.v2() - + "] from [" - + descriptorToCheck.v1() - + "] overlaps with other system index descriptors: [" - + descriptorsMatchingThisPattern.stream() - .map(descriptor -> descriptor.v2() + " from [" + descriptor.v1() + "]") - .collect(Collectors.joining(", ")) - ); - } - }); - } - - private static boolean overlaps(SystemIndexDescriptor a1, SystemIndexDescriptor a2) { - Automaton a1Automaton = Regex.simpleMatchToAutomaton(a1.getIndexPattern()); - Automaton a2Automaton = Regex.simpleMatchToAutomaton(a2.getIndexPattern()); - return Operations.isEmpty(Operations.intersection(a1Automaton, a2Automaton)) == false; - } - - private static Map> buildSystemIndexDescriptorMap( - Map> pluginAndModulesMap - ) { - final Map> map = new HashMap<>( - pluginAndModulesMap.size() + SERVER_SYSTEM_INDEX_DESCRIPTORS.size() - ); - map.putAll(pluginAndModulesMap); - // put the server items last since we expect less of them - SERVER_SYSTEM_INDEX_DESCRIPTORS.forEach((source, descriptors) -> { - if (map.putIfAbsent(source, descriptors) != null) { - throw new IllegalArgumentException( - "plugin or module attempted to define the same source [" + source + "] as a built-in system index" - ); - } - }); - return unmodifiableMap(map); - } } diff --git a/server/src/main/java/org/opensearch/node/Node.java b/server/src/main/java/org/opensearch/node/Node.java index 09e5e8eb394fb..96a716af7f1a1 100644 --- a/server/src/main/java/org/opensearch/node/Node.java +++ b/server/src/main/java/org/opensearch/node/Node.java @@ -710,8 +710,7 @@ protected Node( clusterInfoService, snapshotsInfoService, threadPool.getThreadContext(), - clusterManagerMetrics, - systemIndices + clusterManagerMetrics ); modules.add(clusterModule); IndicesModule indicesModule = new IndicesModule(pluginsService.filterPlugins(MapperPlugin.class)); diff --git a/server/src/test/java/org/opensearch/cluster/ClusterModuleTests.java b/server/src/test/java/org/opensearch/cluster/ClusterModuleTests.java index d7210adc6cb85..f2d99a51f1c9a 100644 --- a/server/src/test/java/org/opensearch/cluster/ClusterModuleTests.java +++ b/server/src/test/java/org/opensearch/cluster/ClusterModuleTests.java @@ -70,7 +70,6 @@ import org.opensearch.common.settings.SettingsModule; import org.opensearch.common.util.concurrent.ThreadContext; import org.opensearch.gateway.GatewayAllocator; -import org.opensearch.indices.SystemIndices; import org.opensearch.plugins.ClusterPlugin; import org.opensearch.telemetry.metrics.noop.NoopMetricsRegistry; import org.opensearch.test.ClusterServiceUtils; @@ -169,13 +168,7 @@ public void testRegisterAllocationDeciderDuplicate() { public Collection createAllocationDeciders(Settings settings, ClusterSettings clusterSettings) { return Collections.singletonList(new EnableAllocationDecider(settings, clusterSettings)); } - }), - clusterInfoService, - null, - threadContext, - new ClusterManagerMetrics(NoopMetricsRegistry.INSTANCE), - new SystemIndices(Collections.emptyMap()) - ) + }), clusterInfoService, null, threadContext, new ClusterManagerMetrics(NoopMetricsRegistry.INSTANCE)) ); assertEquals(e.getMessage(), "Cannot specify allocation decider [" + EnableAllocationDecider.class.getName() + "] twice"); } @@ -186,13 +179,7 @@ public void testRegisterAllocationDecider() { public Collection createAllocationDeciders(Settings settings, ClusterSettings clusterSettings) { return Collections.singletonList(new FakeAllocationDecider()); } - }), - clusterInfoService, - null, - threadContext, - new ClusterManagerMetrics(NoopMetricsRegistry.INSTANCE), - new SystemIndices(Collections.emptyMap()) - ); + }), clusterInfoService, null, threadContext, new ClusterManagerMetrics(NoopMetricsRegistry.INSTANCE)); assertTrue(module.deciderList.stream().anyMatch(d -> d.getClass().equals(FakeAllocationDecider.class))); } @@ -202,13 +189,7 @@ private ClusterModule newClusterModuleWithShardsAllocator(Settings settings, Str public Map> getShardsAllocators(Settings settings, ClusterSettings clusterSettings) { return Collections.singletonMap(name, supplier); } - }), - clusterInfoService, - null, - threadContext, - new ClusterManagerMetrics(NoopMetricsRegistry.INSTANCE), - new SystemIndices(Collections.emptyMap()) - ); + }), clusterInfoService, null, threadContext, new ClusterManagerMetrics(NoopMetricsRegistry.INSTANCE)); } public void testRegisterShardsAllocator() { @@ -236,8 +217,7 @@ public void testUnknownShardsAllocator() { clusterInfoService, null, threadContext, - new ClusterManagerMetrics(NoopMetricsRegistry.INSTANCE), - new SystemIndices(Collections.emptyMap()) + new ClusterManagerMetrics(NoopMetricsRegistry.INSTANCE) ) ); assertEquals("Unknown ShardsAllocator [dne]", e.getMessage()); @@ -323,8 +303,7 @@ public void testRejectsReservedExistingShardsAllocatorName() { clusterInfoService, null, threadContext, - new ClusterManagerMetrics(NoopMetricsRegistry.INSTANCE), - new SystemIndices(Collections.emptyMap()) + new ClusterManagerMetrics(NoopMetricsRegistry.INSTANCE) ); expectThrows( IllegalArgumentException.class, @@ -340,8 +319,7 @@ public void testRejectsDuplicateExistingShardsAllocatorName() { clusterInfoService, null, threadContext, - new ClusterManagerMetrics(NoopMetricsRegistry.INSTANCE), - new SystemIndices(Collections.emptyMap()) + new ClusterManagerMetrics(NoopMetricsRegistry.INSTANCE) ); expectThrows( IllegalArgumentException.class, diff --git a/server/src/test/java/org/opensearch/cluster/metadata/IndexNameExpressionResolverTests.java b/server/src/test/java/org/opensearch/cluster/metadata/IndexNameExpressionResolverTests.java index 20760e59164b0..fda2f411b1994 100644 --- a/server/src/test/java/org/opensearch/cluster/metadata/IndexNameExpressionResolverTests.java +++ b/server/src/test/java/org/opensearch/cluster/metadata/IndexNameExpressionResolverTests.java @@ -53,20 +53,14 @@ import org.opensearch.index.IndexSettings; import org.opensearch.indices.IndexClosedException; import org.opensearch.indices.InvalidIndexNameException; -import org.opensearch.indices.SystemIndexDescriptor; -import org.opensearch.indices.SystemIndices; -import org.opensearch.plugins.Plugin; -import org.opensearch.plugins.SystemIndexPlugin; import org.opensearch.test.OpenSearchTestCase; import java.util.Arrays; -import java.util.Collection; import java.util.Collections; import java.util.Comparator; import java.util.EnumSet; import java.util.HashSet; import java.util.List; -import java.util.Map; import java.util.Set; import java.util.function.Function; import java.util.stream.Collectors; @@ -2526,77 +2520,6 @@ public void testDataStreamsNames() { assertThat(names, empty()); } - public void testSystemIndexMatching() { - SystemIndexPlugin plugin1 = new SystemIndexPlugin1(); - SystemIndexPlugin plugin2 = new SystemIndexPlugin2(); - SystemIndexPlugin plugin3 = new SystemIndexPatternPlugin(); - SystemIndices pluginSystemIndices = new SystemIndices( - Map.of( - SystemIndexPlugin1.class.getCanonicalName(), - plugin1.getSystemIndexDescriptors(Settings.EMPTY), - SystemIndexPlugin2.class.getCanonicalName(), - plugin2.getSystemIndexDescriptors(Settings.EMPTY), - SystemIndexPatternPlugin.class.getCanonicalName(), - plugin3.getSystemIndexDescriptors(Settings.EMPTY) - ) - ); - IndexNameExpressionResolver resolver = new IndexNameExpressionResolver(threadContext, pluginSystemIndices); - - assertThat( - resolver.matchesSystemIndexPattern(".system-index1", ".system-index2"), - equalTo(List.of(SystemIndexPlugin1.SYSTEM_INDEX_1, SystemIndexPlugin2.SYSTEM_INDEX_2)) - ); - assertThat(resolver.matchesSystemIndexPattern(".system-index1"), equalTo(List.of(SystemIndexPlugin1.SYSTEM_INDEX_1))); - assertThat(resolver.matchesSystemIndexPattern(".system-index2"), equalTo(List.of(SystemIndexPlugin2.SYSTEM_INDEX_2))); - assertThat(resolver.matchesSystemIndexPattern(".system-index-pattern1"), equalTo(List.of(".system-index-pattern1"))); - assertThat(resolver.matchesSystemIndexPattern(".system-index-pattern-sub*"), equalTo(List.of(".system-index-pattern-sub*"))); - assertThat( - resolver.matchesSystemIndexPattern(".system-index-pattern1", ".system-index-pattern2"), - equalTo(List.of(".system-index-pattern1", ".system-index-pattern2")) - ); - assertThat( - resolver.matchesSystemIndexPattern(".system-index1", ".system-index-pattern1"), - equalTo(List.of(".system-index1", ".system-index-pattern1")) - ); - assertThat( - resolver.matchesSystemIndexPattern(".system-index1", ".system-index-pattern1", ".not-system"), - equalTo(List.of(".system-index1", ".system-index-pattern1")) - ); - assertThat(resolver.matchesSystemIndexPattern(".not-system"), equalTo(Collections.emptyList())); - } - - public void testRegisteredSystemIndexExpansion() { - SystemIndexPlugin plugin1 = new SystemIndexPlugin1(); - SystemIndexPlugin plugin2 = new SystemIndexPlugin2(); - SystemIndices pluginSystemIndices = new SystemIndices( - Map.of( - SystemIndexPlugin1.class.getCanonicalName(), - plugin1.getSystemIndexDescriptors(Settings.EMPTY), - SystemIndexPlugin2.class.getCanonicalName(), - plugin2.getSystemIndexDescriptors(Settings.EMPTY) - ) - ); - IndexNameExpressionResolver resolver = new IndexNameExpressionResolver(threadContext, pluginSystemIndices); - Metadata.Builder mdBuilder = Metadata.builder() - .put(indexBuilder("foo")) - .put(indexBuilder("bar")) - .put(indexBuilder(SystemIndexPlugin1.SYSTEM_INDEX_1)) - .put(indexBuilder(SystemIndexPlugin2.SYSTEM_INDEX_2)); - ClusterState state = ClusterState.builder(new ClusterName("_name")).metadata(mdBuilder).build(); - - // Only closed - IndicesOptions options = IndicesOptions.strictExpand(); - IndexNameExpressionResolver.Context context = new IndexNameExpressionResolver.Context(state, options, true); - String[] results = resolver.concreteIndexNames(context, Strings.EMPTY_ARRAY); - assertEquals(4, results.length); - assertTrue( - Arrays.asList(results).containsAll(List.of("foo", "bar", SystemIndexPlugin1.SYSTEM_INDEX_1, SystemIndexPlugin2.SYSTEM_INDEX_2)) - ); - List systemIndices = resolver.matchesSystemIndexPattern(results); - assertEquals(2, systemIndices.size()); - assertTrue(systemIndices.containsAll(List.of(SystemIndexPlugin1.SYSTEM_INDEX_1, SystemIndexPlugin2.SYSTEM_INDEX_2))); - } - private ClusterState systemIndexTestClusterState() { Settings settings = Settings.builder().build(); Metadata.Builder mdBuilder = Metadata.builder() @@ -2611,34 +2534,4 @@ private List resolveConcreteIndexNameList(ClusterState state, SearchRequ .map(i -> i.getName()) .collect(Collectors.toList()); } - - static final class SystemIndexPlugin1 extends Plugin implements SystemIndexPlugin { - public static final String SYSTEM_INDEX_1 = ".system-index1"; - - @Override - public Collection getSystemIndexDescriptors(Settings settings) { - final SystemIndexDescriptor systemIndexDescriptor = new SystemIndexDescriptor(SYSTEM_INDEX_1, "System index 1"); - return Collections.singletonList(systemIndexDescriptor); - } - } - - static final class SystemIndexPlugin2 extends Plugin implements SystemIndexPlugin { - public static final String SYSTEM_INDEX_2 = ".system-index2"; - - @Override - public Collection getSystemIndexDescriptors(Settings settings) { - final SystemIndexDescriptor systemIndexDescriptor = new SystemIndexDescriptor(SYSTEM_INDEX_2, "System index 2"); - return Collections.singletonList(systemIndexDescriptor); - } - } - - static final class SystemIndexPatternPlugin extends Plugin implements SystemIndexPlugin { - public static final String SYSTEM_INDEX_PATTERN = ".system-index-pattern*"; - - @Override - public Collection getSystemIndexDescriptors(Settings settings) { - final SystemIndexDescriptor systemIndexDescriptor = new SystemIndexDescriptor(SYSTEM_INDEX_PATTERN, "System index pattern"); - return Collections.singletonList(systemIndexDescriptor); - } - } } diff --git a/server/src/test/java/org/opensearch/indices/SystemIndicesTests.java b/server/src/test/java/org/opensearch/indices/SystemIndicesTests.java index 2b40c01c61242..12836ae7892ea 100644 --- a/server/src/test/java/org/opensearch/indices/SystemIndicesTests.java +++ b/server/src/test/java/org/opensearch/indices/SystemIndicesTests.java @@ -32,12 +32,18 @@ package org.opensearch.indices; +import org.opensearch.common.settings.Settings; +import org.opensearch.plugins.Plugin; +import org.opensearch.plugins.SystemIndexPlugin; import org.opensearch.tasks.TaskResultsService; import org.opensearch.test.OpenSearchTestCase; +import org.junit.Before; import java.util.Arrays; import java.util.Collection; +import java.util.Collections; import java.util.HashMap; +import java.util.List; import java.util.Map; import static java.util.Collections.emptyMap; @@ -50,6 +56,11 @@ public class SystemIndicesTests extends OpenSearchTestCase { + @Before + public void setup() { + SystemIndexRegistry.clear(); + } + public void testBasicOverlappingPatterns() { SystemIndexDescriptor broadPattern = new SystemIndexDescriptor(".a*c*", "test"); SystemIndexDescriptor notOverlapping = new SystemIndexDescriptor(".bbbddd*", "test"); @@ -67,7 +78,7 @@ public void testBasicOverlappingPatterns() { IllegalStateException exception = expectThrows( IllegalStateException.class, - () -> SystemIndices.checkForOverlappingPatterns(descriptors) + () -> SystemIndexRegistry.checkForOverlappingPatterns(descriptors) ); assertThat( exception.getMessage(), @@ -104,7 +115,7 @@ public void testComplexOverlappingPatterns() { IllegalStateException exception = expectThrows( IllegalStateException.class, - () -> SystemIndices.checkForOverlappingPatterns(descriptors) + () -> SystemIndexRegistry.checkForOverlappingPatterns(descriptors) ); assertThat( exception.getMessage(), @@ -133,4 +144,94 @@ public void testPluginCannotOverrideBuiltInSystemIndex() { IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> new SystemIndices(pluginMap)); assertThat(e.getMessage(), containsString("plugin or module attempted to define the same source")); } + + public void testSystemIndexMatching() { + SystemIndexPlugin plugin1 = new SystemIndexPlugin1(); + SystemIndexPlugin plugin2 = new SystemIndexPlugin2(); + SystemIndexPlugin plugin3 = new SystemIndexPatternPlugin(); + SystemIndices pluginSystemIndices = new SystemIndices( + Map.of( + SystemIndexPlugin1.class.getCanonicalName(), + plugin1.getSystemIndexDescriptors(Settings.EMPTY), + SystemIndexPlugin2.class.getCanonicalName(), + plugin2.getSystemIndexDescriptors(Settings.EMPTY), + SystemIndexPatternPlugin.class.getCanonicalName(), + plugin3.getSystemIndexDescriptors(Settings.EMPTY) + ) + ); + + assertThat( + SystemIndexRegistry.matchesSystemIndexPattern(".system-index1", ".system-index2"), + equalTo(List.of(SystemIndexPlugin1.SYSTEM_INDEX_1, SystemIndexPlugin2.SYSTEM_INDEX_2)) + ); + assertThat(SystemIndexRegistry.matchesSystemIndexPattern(".system-index1"), equalTo(List.of(SystemIndexPlugin1.SYSTEM_INDEX_1))); + assertThat(SystemIndexRegistry.matchesSystemIndexPattern(".system-index2"), equalTo(List.of(SystemIndexPlugin2.SYSTEM_INDEX_2))); + assertThat(SystemIndexRegistry.matchesSystemIndexPattern(".system-index-pattern1"), equalTo(List.of(".system-index-pattern1"))); + assertThat( + SystemIndexRegistry.matchesSystemIndexPattern(".system-index-pattern-sub*"), + equalTo(List.of(".system-index-pattern-sub*")) + ); + assertThat( + SystemIndexRegistry.matchesSystemIndexPattern(".system-index-pattern1", ".system-index-pattern2"), + equalTo(List.of(".system-index-pattern1", ".system-index-pattern2")) + ); + assertThat( + SystemIndexRegistry.matchesSystemIndexPattern(".system-index1", ".system-index-pattern1"), + equalTo(List.of(".system-index1", ".system-index-pattern1")) + ); + assertThat( + SystemIndexRegistry.matchesSystemIndexPattern(".system-index1", ".system-index-pattern1", ".not-system"), + equalTo(List.of(".system-index1", ".system-index-pattern1")) + ); + assertThat(SystemIndexRegistry.matchesSystemIndexPattern(".not-system"), equalTo(Collections.emptyList())); + } + + public void testRegisteredSystemIndexExpansion() { + SystemIndexPlugin plugin1 = new SystemIndexPlugin1(); + SystemIndexPlugin plugin2 = new SystemIndexPlugin2(); + SystemIndices pluginSystemIndices = new SystemIndices( + Map.of( + SystemIndexPlugin1.class.getCanonicalName(), + plugin1.getSystemIndexDescriptors(Settings.EMPTY), + SystemIndexPlugin2.class.getCanonicalName(), + plugin2.getSystemIndexDescriptors(Settings.EMPTY) + ) + ); + List systemIndices = SystemIndexRegistry.matchesSystemIndexPattern( + SystemIndexPlugin1.SYSTEM_INDEX_1, + SystemIndexPlugin2.SYSTEM_INDEX_2 + ); + assertEquals(2, systemIndices.size()); + assertTrue(systemIndices.containsAll(List.of(SystemIndexPlugin1.SYSTEM_INDEX_1, SystemIndexPlugin2.SYSTEM_INDEX_2))); + } + + static final class SystemIndexPlugin1 extends Plugin implements SystemIndexPlugin { + public static final String SYSTEM_INDEX_1 = ".system-index1"; + + @Override + public Collection getSystemIndexDescriptors(Settings settings) { + final SystemIndexDescriptor systemIndexDescriptor = new SystemIndexDescriptor(SYSTEM_INDEX_1, "System index 1"); + return Collections.singletonList(systemIndexDescriptor); + } + } + + static final class SystemIndexPlugin2 extends Plugin implements SystemIndexPlugin { + public static final String SYSTEM_INDEX_2 = ".system-index2"; + + @Override + public Collection getSystemIndexDescriptors(Settings settings) { + final SystemIndexDescriptor systemIndexDescriptor = new SystemIndexDescriptor(SYSTEM_INDEX_2, "System index 2"); + return Collections.singletonList(systemIndexDescriptor); + } + } + + static final class SystemIndexPatternPlugin extends Plugin implements SystemIndexPlugin { + public static final String SYSTEM_INDEX_PATTERN = ".system-index-pattern*"; + + @Override + public Collection getSystemIndexDescriptors(Settings settings) { + final SystemIndexDescriptor systemIndexDescriptor = new SystemIndexDescriptor(SYSTEM_INDEX_PATTERN, "System index pattern"); + return Collections.singletonList(systemIndexDescriptor); + } + } } From f7365b5e8e1283e5970636ba42d590d985ddf07e Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Wed, 3 Jul 2024 16:10:23 -0400 Subject: [PATCH 18/21] Update CHANGELOG Signed-off-by: Craig Perkins --- CHANGELOG.md | 2 +- .../src/main/java/org/opensearch/indices/SystemIndices.java | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d9ba123d63bc4..3e1763667973d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Add batching supported processor base type AbstractBatchingProcessor ([#14554](https://github.com/opensearch-project/OpenSearch/pull/14554)) - Fix race condition while parsing derived fields from search definition ([14445](https://github.com/opensearch-project/OpenSearch/pull/14445)) - Add allowlist setting for ingest-common and search-pipeline-common processors ([#14439](https://github.com/opensearch-project/OpenSearch/issues/14439)) -- Add a method in IndexNameExpressionResolver to determine if a request matches system indices ([#14415](https://github.com/opensearch-project/OpenSearch/pull/14415)) +- Create SystemIndexRegistry with helper method matchesSystemIndex ([#14415](https://github.com/opensearch-project/OpenSearch/pull/14415)) ### 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/indices/SystemIndices.java b/server/src/main/java/org/opensearch/indices/SystemIndices.java index 88fd9a920dca1..28e3f50b444a3 100644 --- a/server/src/main/java/org/opensearch/indices/SystemIndices.java +++ b/server/src/main/java/org/opensearch/indices/SystemIndices.java @@ -40,7 +40,6 @@ import org.apache.lucene.util.automaton.MinimizationOperations; import org.apache.lucene.util.automaton.Operations; import org.opensearch.common.Nullable; -import org.opensearch.common.annotation.PublicApi; import org.opensearch.common.regex.Regex; import org.opensearch.core.index.Index; @@ -55,9 +54,8 @@ * node knows about. Methods for determining if an index should be a system index are also provided * to reduce the locations within the code that need to deal with {@link SystemIndexDescriptor}s. * - * @opensearch.api + * @opensearch.internal */ -@PublicApi(since = "2.16.0") public class SystemIndices { private static final Logger logger = LogManager.getLogger(SystemIndices.class); From 179e8d8801a84d317372c3d952e4787f704a57bd Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Wed, 3 Jul 2024 17:06:09 -0400 Subject: [PATCH 19/21] Remove singleton limitation Signed-off-by: Craig Perkins --- .../indices/SystemIndexRegistry.java | 26 ++++++------------- .../org/opensearch/indices/SystemIndices.java | 3 +-- .../indices/SystemIndicesTests.java | 6 ----- 3 files changed, 9 insertions(+), 26 deletions(-) diff --git a/server/src/main/java/org/opensearch/indices/SystemIndexRegistry.java b/server/src/main/java/org/opensearch/indices/SystemIndexRegistry.java index a784adb6e82b5..27973839c6167 100644 --- a/server/src/main/java/org/opensearch/indices/SystemIndexRegistry.java +++ b/server/src/main/java/org/opensearch/indices/SystemIndexRegistry.java @@ -16,6 +16,7 @@ import java.util.Arrays; import java.util.Collection; +import java.util.Collections; import java.util.Comparator; import java.util.HashMap; import java.util.List; @@ -27,17 +28,17 @@ import static java.util.Collections.unmodifiableMap; import static org.opensearch.tasks.TaskResultsService.TASK_INDEX; -class SystemIndexRegistry { - private static SystemIndexRegistry INSTANCE = null; +public class SystemIndexRegistry { private static final SystemIndexDescriptor TASK_INDEX_DESCRIPTOR = new SystemIndexDescriptor(TASK_INDEX + "*", "Task Result Index"); - static final Map> SERVER_SYSTEM_INDEX_DESCRIPTORS = singletonMap( + private static final Map> SERVER_SYSTEM_INDEX_DESCRIPTORS = singletonMap( TaskResultsService.class.getName(), singletonList(TASK_INDEX_DESCRIPTOR) ); - private static String[] SYSTEM_INDEX_PATTERNS = new String[0]; - static Collection SYSTEM_INDEX_DESCRIPTORS; - private SystemIndexRegistry(Map> pluginAndModulesDescriptors) { + private volatile static String[] SYSTEM_INDEX_PATTERNS = new String[0]; + volatile static Collection SYSTEM_INDEX_DESCRIPTORS = Collections.emptyList(); + + static void register(Map> pluginAndModulesDescriptors) { final Map> descriptorsMap = buildSystemIndexDescriptorMap(pluginAndModulesDescriptors); checkForOverlappingPatterns(descriptorsMap); List descriptors = pluginAndModulesDescriptors.values() @@ -45,17 +46,11 @@ private SystemIndexRegistry(Map> plugi .flatMap(Collection::stream) .collect(Collectors.toList()); descriptors.add(TASK_INDEX_DESCRIPTOR); + SYSTEM_INDEX_DESCRIPTORS = descriptors.stream().collect(Collectors.toUnmodifiableList()); SYSTEM_INDEX_PATTERNS = descriptors.stream().map(SystemIndexDescriptor::getIndexPattern).toArray(String[]::new); } - public static synchronized SystemIndexRegistry initialize(Map> pluginAndModulesDescriptors) { - if (INSTANCE == null) { - INSTANCE = new SystemIndexRegistry(pluginAndModulesDescriptors); - } - return INSTANCE; - } - public static List matchesSystemIndexPattern(String... indexExpressions) { return Arrays.stream(indexExpressions) .filter(pattern -> Regex.simpleMatch(SYSTEM_INDEX_PATTERNS, pattern)) @@ -123,9 +118,4 @@ private static Map> buildSystemIndexDe }); return unmodifiableMap(map); } - - // visible for testing - static void clear() { - INSTANCE = null; - } } diff --git a/server/src/main/java/org/opensearch/indices/SystemIndices.java b/server/src/main/java/org/opensearch/indices/SystemIndices.java index 28e3f50b444a3..bbf58fe91512f 100644 --- a/server/src/main/java/org/opensearch/indices/SystemIndices.java +++ b/server/src/main/java/org/opensearch/indices/SystemIndices.java @@ -62,8 +62,7 @@ public class SystemIndices { private final CharacterRunAutomaton runAutomaton; public SystemIndices(Map> pluginAndModulesDescriptors) { - SystemIndexRegistry.initialize(pluginAndModulesDescriptors); - SystemIndexRegistry.initialize(pluginAndModulesDescriptors); + SystemIndexRegistry.register(pluginAndModulesDescriptors); this.runAutomaton = buildCharacterRunAutomaton(SystemIndexRegistry.SYSTEM_INDEX_DESCRIPTORS); } diff --git a/server/src/test/java/org/opensearch/indices/SystemIndicesTests.java b/server/src/test/java/org/opensearch/indices/SystemIndicesTests.java index 12836ae7892ea..8ac457c32d53a 100644 --- a/server/src/test/java/org/opensearch/indices/SystemIndicesTests.java +++ b/server/src/test/java/org/opensearch/indices/SystemIndicesTests.java @@ -37,7 +37,6 @@ import org.opensearch.plugins.SystemIndexPlugin; import org.opensearch.tasks.TaskResultsService; import org.opensearch.test.OpenSearchTestCase; -import org.junit.Before; import java.util.Arrays; import java.util.Collection; @@ -56,11 +55,6 @@ public class SystemIndicesTests extends OpenSearchTestCase { - @Before - public void setup() { - SystemIndexRegistry.clear(); - } - public void testBasicOverlappingPatterns() { SystemIndexDescriptor broadPattern = new SystemIndexDescriptor(".a*c*", "test"); SystemIndexDescriptor notOverlapping = new SystemIndexDescriptor(".bbbddd*", "test"); From e2ef9c4b9d9da1b5122c5daa19c2712d988fd40b Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Wed, 3 Jul 2024 17:11:36 -0400 Subject: [PATCH 20/21] Add javadoc Signed-off-by: Craig Perkins --- .../java/org/opensearch/indices/SystemIndexRegistry.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/server/src/main/java/org/opensearch/indices/SystemIndexRegistry.java b/server/src/main/java/org/opensearch/indices/SystemIndexRegistry.java index 27973839c6167..48c239213bba1 100644 --- a/server/src/main/java/org/opensearch/indices/SystemIndexRegistry.java +++ b/server/src/main/java/org/opensearch/indices/SystemIndexRegistry.java @@ -28,6 +28,13 @@ import static java.util.Collections.unmodifiableMap; import static org.opensearch.tasks.TaskResultsService.TASK_INDEX; +/** + * This class holds the {@link SystemIndexDescriptor} objects that represent system indices the + * node knows about. This class also contains static methods that identify if index expressions match + * registered system index patterns + * + * @opensearch.api + */ public class SystemIndexRegistry { private static final SystemIndexDescriptor TASK_INDEX_DESCRIPTOR = new SystemIndexDescriptor(TASK_INDEX + "*", "Task Result Index"); private static final Map> SERVER_SYSTEM_INDEX_DESCRIPTORS = singletonMap( From e71aa093b718c71960e2848979d4d5b4b1a8d69a Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Wed, 3 Jul 2024 21:29:46 -0400 Subject: [PATCH 21/21] Add @ExperimentalApi annotation Signed-off-by: Craig Perkins --- .../main/java/org/opensearch/indices/SystemIndexRegistry.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/server/src/main/java/org/opensearch/indices/SystemIndexRegistry.java b/server/src/main/java/org/opensearch/indices/SystemIndexRegistry.java index 48c239213bba1..d9608e220d924 100644 --- a/server/src/main/java/org/opensearch/indices/SystemIndexRegistry.java +++ b/server/src/main/java/org/opensearch/indices/SystemIndexRegistry.java @@ -10,6 +10,7 @@ import org.apache.lucene.util.automaton.Automaton; import org.apache.lucene.util.automaton.Operations; +import org.opensearch.common.annotation.ExperimentalApi; import org.opensearch.common.collect.Tuple; import org.opensearch.common.regex.Regex; import org.opensearch.tasks.TaskResultsService; @@ -35,6 +36,7 @@ * * @opensearch.api */ +@ExperimentalApi public class SystemIndexRegistry { private static final SystemIndexDescriptor TASK_INDEX_DESCRIPTOR = new SystemIndexDescriptor(TASK_INDEX + "*", "Task Result Index"); private static final Map> SERVER_SYSTEM_INDEX_DESCRIPTORS = singletonMap(