From a4a6bb8e5436cfcdf997aa32a5b81fdae00c95f9 Mon Sep 17 00:00:00 2001 From: "opensearch-trigger-bot[bot]" <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com> Date: Tue, 9 Jul 2024 15:38:33 -0400 Subject: [PATCH] Create SystemIndexRegistry with helper method matchesSystemIndex (#14415) (#14692) * Create new extension point in SystemIndexPlugin for a single plugin to get registered system indices * Add to CHANGELOG * WIP on system indices from IndexNameExpressionResolver * Add test in IndexNameExpressionResolverTests * Remove changes in SystemIndexPlugin * Add method in IndexNameExpressionResolver to get matching system indices * Show how resolver can be chained to get system indices * Fix forbiddenApis check * Update CHANGELOG * Make SystemIndices internal * Remove unneeded changes * Fix CI failures * Fix precommit errors * Use Regex instead of WildcardMatcher * Address code review feedback * Allow caller to pass index expressions * Create SystemIndexRegistry * Update CHANGELOG * Remove singleton limitation * Add javadoc * Add @ExperimentalApi annotation --------- (cherry picked from commit bf56227ad1d679573beffd94f419f7b73cd07188) Signed-off-by: Craig Perkins Signed-off-by: github-actions[bot] Co-authored-by: github-actions[bot] --- CHANGELOG.md | 1 + .../indices/SystemIndexDescriptor.java | 4 +- .../indices/SystemIndexRegistry.java | 130 ++++++++++++++++++ .../org/opensearch/indices/SystemIndices.java | 88 +----------- .../main/java/org/opensearch/node/Node.java | 17 ++- .../indices/SystemIndicesTests.java | 99 ++++++++++++- 6 files changed, 242 insertions(+), 97 deletions(-) create mode 100644 server/src/main/java/org/opensearch/indices/SystemIndexRegistry.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 5509e3a4a160f..b0e3d668a0505 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +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)) +- Create SystemIndexRegistry with helper method matchesSystemIndex ([#14415](https://github.com/opensearch-project/OpenSearch/pull/14415)) ### Dependencies - Update to Apache Lucene 9.11.1 ([#14042](https://github.com/opensearch-project/OpenSearch/pull/14042), [#14576](https://github.com/opensearch-project/OpenSearch/pull/14576)) 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/SystemIndexRegistry.java b/server/src/main/java/org/opensearch/indices/SystemIndexRegistry.java new file mode 100644 index 0000000000000..d9608e220d924 --- /dev/null +++ b/server/src/main/java/org/opensearch/indices/SystemIndexRegistry.java @@ -0,0 +1,130 @@ +/* + * 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.annotation.ExperimentalApi; +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.Collections; +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; + +/** + * 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 + */ +@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( + TaskResultsService.class.getName(), + singletonList(TASK_INDEX_DESCRIPTOR) + ); + + 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() + .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 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); + } +} diff --git a/server/src/main/java/org/opensearch/indices/SystemIndices.java b/server/src/main/java/org/opensearch/indices/SystemIndices.java index a85e938c61b7a..bbf58fe91512f 100644 --- a/server/src/main/java/org/opensearch/indices/SystemIndices.java +++ b/server/src/main/java/org/opensearch/indices/SystemIndices.java @@ -40,25 +40,15 @@ 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.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 @@ -69,21 +59,11 @@ 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.register(pluginAndModulesDescriptors); + this.runAutomaton = buildCharacterRunAutomaton(SystemIndexRegistry.SYSTEM_INDEX_DESCRIPTORS); } /** @@ -111,7 +91,7 @@ public boolean isSystemIndex(String indexName) { * @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()); @@ -168,66 +148,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 c916c24c702ec..449fa23c5dffb 100644 --- a/server/src/main/java/org/opensearch/node/Node.java +++ b/server/src/main/java/org/opensearch/node/Node.java @@ -693,6 +693,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, @@ -817,15 +825,6 @@ protected Node( .flatMap(m -> m.entrySet().stream()) .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); - 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 RerouteService rerouteService = new BatchedRerouteService(clusterService, clusterModule.getAllocationService()::reroute); rerouteServiceReference.set(rerouteService); clusterService.setRerouteService(rerouteService); diff --git a/server/src/test/java/org/opensearch/indices/SystemIndicesTests.java b/server/src/test/java/org/opensearch/indices/SystemIndicesTests.java index 2b40c01c61242..8ac457c32d53a 100644 --- a/server/src/test/java/org/opensearch/indices/SystemIndicesTests.java +++ b/server/src/test/java/org/opensearch/indices/SystemIndicesTests.java @@ -32,12 +32,17 @@ 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 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; @@ -67,7 +72,7 @@ public void testBasicOverlappingPatterns() { IllegalStateException exception = expectThrows( IllegalStateException.class, - () -> SystemIndices.checkForOverlappingPatterns(descriptors) + () -> SystemIndexRegistry.checkForOverlappingPatterns(descriptors) ); assertThat( exception.getMessage(), @@ -104,7 +109,7 @@ public void testComplexOverlappingPatterns() { IllegalStateException exception = expectThrows( IllegalStateException.class, - () -> SystemIndices.checkForOverlappingPatterns(descriptors) + () -> SystemIndexRegistry.checkForOverlappingPatterns(descriptors) ); assertThat( exception.getMessage(), @@ -133,4 +138,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); + } + } }