From 6438734926c7df560e11796c9217a17114b6ec46 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Mon, 15 Jul 2024 10:34:26 -0400 Subject: [PATCH 1/6] Add matchesPluginSystemIndexPattern to SystemIndexRegistry Signed-off-by: Craig Perkins --- .../indices/SystemIndexRegistry.java | 23 ++++-- .../main/java/org/opensearch/node/Node.java | 5 +- .../indices/SystemIndicesTests.java | 80 ++++++++++++++----- 3 files changed, 82 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 d9608e220d924..ba57532502d82 100644 --- a/server/src/main/java/org/opensearch/indices/SystemIndexRegistry.java +++ b/server/src/main/java/org/opensearch/indices/SystemIndexRegistry.java @@ -15,13 +15,13 @@ 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.Set; import java.util.stream.Collectors; import static java.util.Collections.singletonList; @@ -46,6 +46,7 @@ public class SystemIndexRegistry { private volatile static String[] SYSTEM_INDEX_PATTERNS = new String[0]; volatile static Collection SYSTEM_INDEX_DESCRIPTORS = Collections.emptyList(); + volatile static Map> SYSTEM_INDEX_DESCRIPTORS_MAP = Collections.emptyMap(); static void register(Map> pluginAndModulesDescriptors) { final Map> descriptorsMap = buildSystemIndexDescriptorMap(pluginAndModulesDescriptors); @@ -56,14 +57,26 @@ static void register(Map> pluginAndMod .collect(Collectors.toList()); descriptors.add(TASK_INDEX_DESCRIPTOR); + SYSTEM_INDEX_DESCRIPTORS_MAP = descriptorsMap; 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()); + public static Set matchesSystemIndexPattern(Set indexExpressions) { + return indexExpressions.stream().filter(pattern -> Regex.simpleMatch(SYSTEM_INDEX_PATTERNS, pattern)).collect(Collectors.toSet()); + } + + public static Set matchesPluginSystemIndexPattern(String pluginClassName, Set indexExpressions) { + if (!SYSTEM_INDEX_DESCRIPTORS_MAP.containsKey(pluginClassName)) { + return Collections.emptySet(); + } + String[] pluginSystemIndexPatterns = SYSTEM_INDEX_DESCRIPTORS_MAP.get(pluginClassName) + .stream() + .map(SystemIndexDescriptor::getIndexPattern) + .toArray(String[]::new); + return indexExpressions.stream() + .filter(pattern -> Regex.simpleMatch(pluginSystemIndexPatterns, pattern)) + .collect(Collectors.toSet()); } /** diff --git a/server/src/main/java/org/opensearch/node/Node.java b/server/src/main/java/org/opensearch/node/Node.java index 96a716af7f1a1..281f697d9bb79 100644 --- a/server/src/main/java/org/opensearch/node/Node.java +++ b/server/src/main/java/org/opensearch/node/Node.java @@ -699,7 +699,10 @@ protected Node( pluginsService.filterPlugins(SystemIndexPlugin.class) .stream() .collect( - Collectors.toMap(plugin -> plugin.getClass().getSimpleName(), plugin -> plugin.getSystemIndexDescriptors(settings)) + Collectors.toMap( + plugin -> plugin.getClass().getCanonicalName(), + plugin -> plugin.getSystemIndexDescriptors(settings) + ) ) ); final SystemIndices systemIndices = new SystemIndices(systemIndexDescriptorMap); diff --git a/server/src/test/java/org/opensearch/indices/SystemIndicesTests.java b/server/src/test/java/org/opensearch/indices/SystemIndicesTests.java index 8ac457c32d53a..762a651d7ec50 100644 --- a/server/src/test/java/org/opensearch/indices/SystemIndicesTests.java +++ b/server/src/test/java/org/opensearch/indices/SystemIndicesTests.java @@ -42,8 +42,8 @@ import java.util.Collection; import java.util.Collections; import java.util.HashMap; -import java.util.List; import java.util.Map; +import java.util.Set; import static java.util.Collections.emptyMap; import static java.util.Collections.singletonList; @@ -155,32 +155,41 @@ public void testSystemIndexMatching() { ); assertThat( - SystemIndexRegistry.matchesSystemIndexPattern(".system-index1", ".system-index2"), - equalTo(List.of(SystemIndexPlugin1.SYSTEM_INDEX_1, SystemIndexPlugin2.SYSTEM_INDEX_2)) + SystemIndexRegistry.matchesSystemIndexPattern(Set.of(".system-index1", ".system-index2")), + equalTo(Set.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*")) + SystemIndexRegistry.matchesSystemIndexPattern(Set.of(".system-index1")), + equalTo(Set.of(SystemIndexPlugin1.SYSTEM_INDEX_1)) ); assertThat( - SystemIndexRegistry.matchesSystemIndexPattern(".system-index-pattern1", ".system-index-pattern2"), - equalTo(List.of(".system-index-pattern1", ".system-index-pattern2")) + SystemIndexRegistry.matchesSystemIndexPattern(Set.of(".system-index2")), + equalTo(Set.of(SystemIndexPlugin2.SYSTEM_INDEX_2)) ); assertThat( - SystemIndexRegistry.matchesSystemIndexPattern(".system-index1", ".system-index-pattern1"), - equalTo(List.of(".system-index1", ".system-index-pattern1")) + SystemIndexRegistry.matchesSystemIndexPattern(Set.of(".system-index-pattern1")), + equalTo(Set.of(".system-index-pattern1")) ); assertThat( - SystemIndexRegistry.matchesSystemIndexPattern(".system-index1", ".system-index-pattern1", ".not-system"), - equalTo(List.of(".system-index1", ".system-index-pattern1")) + SystemIndexRegistry.matchesSystemIndexPattern(Set.of(".system-index-pattern-sub*")), + equalTo(Set.of(".system-index-pattern-sub*")) ); - assertThat(SystemIndexRegistry.matchesSystemIndexPattern(".not-system"), equalTo(Collections.emptyList())); + assertThat( + SystemIndexRegistry.matchesSystemIndexPattern(Set.of(".system-index-pattern1", ".system-index-pattern2")), + equalTo(Set.of(".system-index-pattern1", ".system-index-pattern2")) + ); + assertThat( + SystemIndexRegistry.matchesSystemIndexPattern(Set.of(".system-index1", ".system-index-pattern1")), + equalTo(Set.of(".system-index1", ".system-index-pattern1")) + ); + assertThat( + SystemIndexRegistry.matchesSystemIndexPattern(Set.of(".system-index1", ".system-index-pattern1", ".not-system")), + equalTo(Set.of(".system-index1", ".system-index-pattern1")) + ); + assertThat(SystemIndexRegistry.matchesSystemIndexPattern(Set.of(".not-system")), equalTo(Collections.emptySet())); } - public void testRegisteredSystemIndexExpansion() { + public void testRegisteredSystemIndexMatching() { SystemIndexPlugin plugin1 = new SystemIndexPlugin1(); SystemIndexPlugin plugin2 = new SystemIndexPlugin2(); SystemIndices pluginSystemIndices = new SystemIndices( @@ -191,12 +200,43 @@ public void testRegisteredSystemIndexExpansion() { plugin2.getSystemIndexDescriptors(Settings.EMPTY) ) ); - List systemIndices = SystemIndexRegistry.matchesSystemIndexPattern( - SystemIndexPlugin1.SYSTEM_INDEX_1, - SystemIndexPlugin2.SYSTEM_INDEX_2 + Set systemIndices = SystemIndexRegistry.matchesSystemIndexPattern( + Set.of(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))); + assertTrue(systemIndices.containsAll(Set.of(SystemIndexPlugin1.SYSTEM_INDEX_1, SystemIndexPlugin2.SYSTEM_INDEX_2))); + } + + public void testRegisteredSystemIndexMatchingForPlugin() { + 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) + ) + ); + Set systemIndicesForPlugin1 = SystemIndexRegistry.matchesPluginSystemIndexPattern( + SystemIndexPlugin1.class.getCanonicalName(), + Set.of(SystemIndexPlugin1.SYSTEM_INDEX_1, SystemIndexPlugin2.SYSTEM_INDEX_2, "other-index") + ); + assertEquals(1, systemIndicesForPlugin1.size()); + assertTrue(systemIndicesForPlugin1.contains(SystemIndexPlugin1.SYSTEM_INDEX_1)); + + Set systemIndicesForPlugin2 = SystemIndexRegistry.matchesPluginSystemIndexPattern( + SystemIndexPlugin2.class.getCanonicalName(), + Set.of(SystemIndexPlugin1.SYSTEM_INDEX_1, SystemIndexPlugin2.SYSTEM_INDEX_2, "other-index") + ); + assertEquals(1, systemIndicesForPlugin2.size()); + assertTrue(systemIndicesForPlugin2.contains(SystemIndexPlugin2.SYSTEM_INDEX_2)); + + Set noMatchingSystemIndices = SystemIndexRegistry.matchesPluginSystemIndexPattern( + SystemIndexPlugin2.class.getCanonicalName(), + Set.of("other-index") + ); + assertEquals(0, noMatchingSystemIndices.size()); } static final class SystemIndexPlugin1 extends Plugin implements SystemIndexPlugin { From 9e20ebcabe8d5f2c5efee877617b58ec8c0a2269 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Mon, 15 Jul 2024 10:47:50 -0400 Subject: [PATCH 2/6] 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 fbe2b7f50d446..cbb45da8591e4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - 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)) - Print reason why parent task was cancelled ([#14604](https://github.com/opensearch-project/OpenSearch/issues/14604)) +- Add matchesPluginSystemIndexPattern to SystemIndexRegistry ([#14750](https://github.com/opensearch-project/OpenSearch/pull/14750)) ### Dependencies - Bump `org.gradle.test-retry` from 1.5.8 to 1.5.9 ([#13442](https://github.com/opensearch-project/OpenSearch/pull/13442)) From 541b8bd802df3e90a2acdf6898871f9ee85afe34 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Mon, 15 Jul 2024 14:31:38 -0400 Subject: [PATCH 3/6] Use single data structure to keep track of system indices Signed-off-by: Craig Perkins --- .../org/opensearch/indices/SystemIndexRegistry.java | 2 -- .../java/org/opensearch/indices/SystemIndices.java | 10 ++++++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/org/opensearch/indices/SystemIndexRegistry.java b/server/src/main/java/org/opensearch/indices/SystemIndexRegistry.java index ba57532502d82..9f24a687ba51a 100644 --- a/server/src/main/java/org/opensearch/indices/SystemIndexRegistry.java +++ b/server/src/main/java/org/opensearch/indices/SystemIndexRegistry.java @@ -45,7 +45,6 @@ public class SystemIndexRegistry { ); private volatile static String[] SYSTEM_INDEX_PATTERNS = new String[0]; - volatile static Collection SYSTEM_INDEX_DESCRIPTORS = Collections.emptyList(); volatile static Map> SYSTEM_INDEX_DESCRIPTORS_MAP = Collections.emptyMap(); static void register(Map> pluginAndModulesDescriptors) { @@ -58,7 +57,6 @@ static void register(Map> pluginAndMod descriptors.add(TASK_INDEX_DESCRIPTOR); SYSTEM_INDEX_DESCRIPTORS_MAP = descriptorsMap; - SYSTEM_INDEX_DESCRIPTORS = descriptors.stream().collect(Collectors.toUnmodifiableList()); SYSTEM_INDEX_PATTERNS = descriptors.stream().map(SystemIndexDescriptor::getIndexPattern).toArray(String[]::new); } diff --git a/server/src/main/java/org/opensearch/indices/SystemIndices.java b/server/src/main/java/org/opensearch/indices/SystemIndices.java index bbf58fe91512f..a0b2cab060214 100644 --- a/server/src/main/java/org/opensearch/indices/SystemIndices.java +++ b/server/src/main/java/org/opensearch/indices/SystemIndices.java @@ -63,7 +63,9 @@ public class SystemIndices { public SystemIndices(Map> pluginAndModulesDescriptors) { SystemIndexRegistry.register(pluginAndModulesDescriptors); - this.runAutomaton = buildCharacterRunAutomaton(SystemIndexRegistry.SYSTEM_INDEX_DESCRIPTORS); + this.runAutomaton = buildCharacterRunAutomaton( + SystemIndexRegistry.SYSTEM_INDEX_DESCRIPTORS_MAP.values().stream().flatMap(Collection::stream).collect(Collectors.toList()) + ); } /** @@ -91,7 +93,11 @@ public boolean isSystemIndex(String indexName) { * @throws IllegalStateException if multiple descriptors match the name */ public @Nullable SystemIndexDescriptor findMatchingDescriptor(String name) { - final List matchingDescriptors = SystemIndexRegistry.SYSTEM_INDEX_DESCRIPTORS.stream() + List allDescriptors = SystemIndexRegistry.SYSTEM_INDEX_DESCRIPTORS_MAP.values() + .stream() + .flatMap(Collection::stream) + .collect(Collectors.toList()); + final List matchingDescriptors = allDescriptors.stream() .filter(descriptor -> descriptor.matchesIndexPattern(name)) .collect(Collectors.toList()); From 7992ed211546390398ceac1f83c808c97bbd69a7 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Mon, 15 Jul 2024 16:31:47 -0400 Subject: [PATCH 4/6] Address code review comments Signed-off-by: Craig Perkins --- .../org/opensearch/indices/SystemIndexRegistry.java | 13 ++++++------- .../java/org/opensearch/indices/SystemIndices.java | 9 ++------- 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/server/src/main/java/org/opensearch/indices/SystemIndexRegistry.java b/server/src/main/java/org/opensearch/indices/SystemIndexRegistry.java index 9f24a687ba51a..a0aafaca65002 100644 --- a/server/src/main/java/org/opensearch/indices/SystemIndexRegistry.java +++ b/server/src/main/java/org/opensearch/indices/SystemIndexRegistry.java @@ -45,19 +45,14 @@ public class SystemIndexRegistry { ); private volatile static String[] SYSTEM_INDEX_PATTERNS = new String[0]; - volatile static Map> SYSTEM_INDEX_DESCRIPTORS_MAP = Collections.emptyMap(); + private volatile static Map> SYSTEM_INDEX_DESCRIPTORS_MAP = Collections.emptyMap(); 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_MAP = descriptorsMap; - SYSTEM_INDEX_PATTERNS = descriptors.stream().map(SystemIndexDescriptor::getIndexPattern).toArray(String[]::new); + SYSTEM_INDEX_PATTERNS = getAllDescriptors().stream().map(SystemIndexDescriptor::getIndexPattern).toArray(String[]::new); } public static Set matchesSystemIndexPattern(Set indexExpressions) { @@ -77,6 +72,10 @@ public static Set matchesPluginSystemIndexPattern(String pluginClassName .collect(Collectors.toSet()); } + public static List getAllDescriptors() { + return SYSTEM_INDEX_DESCRIPTORS_MAP.values().stream().flatMap(Collection::stream).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. diff --git a/server/src/main/java/org/opensearch/indices/SystemIndices.java b/server/src/main/java/org/opensearch/indices/SystemIndices.java index a0b2cab060214..6e9e5e7707877 100644 --- a/server/src/main/java/org/opensearch/indices/SystemIndices.java +++ b/server/src/main/java/org/opensearch/indices/SystemIndices.java @@ -63,9 +63,7 @@ public class SystemIndices { public SystemIndices(Map> pluginAndModulesDescriptors) { SystemIndexRegistry.register(pluginAndModulesDescriptors); - this.runAutomaton = buildCharacterRunAutomaton( - SystemIndexRegistry.SYSTEM_INDEX_DESCRIPTORS_MAP.values().stream().flatMap(Collection::stream).collect(Collectors.toList()) - ); + this.runAutomaton = buildCharacterRunAutomaton(SystemIndexRegistry.getAllDescriptors()); } /** @@ -93,11 +91,8 @@ public boolean isSystemIndex(String indexName) { * @throws IllegalStateException if multiple descriptors match the name */ public @Nullable SystemIndexDescriptor findMatchingDescriptor(String name) { - List allDescriptors = SystemIndexRegistry.SYSTEM_INDEX_DESCRIPTORS_MAP.values() + final List matchingDescriptors = SystemIndexRegistry.getAllDescriptors() .stream() - .flatMap(Collection::stream) - .collect(Collectors.toList()); - final List matchingDescriptors = allDescriptors.stream() .filter(descriptor -> descriptor.matchesIndexPattern(name)) .collect(Collectors.toList()); From 8c3d0b58fa4bf7558427aba3ffafc89463650cb2 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Mon, 15 Jul 2024 16:38:27 -0400 Subject: [PATCH 5/6] Add test for getAllDescriptors Signed-off-by: Craig Perkins --- .../indices/SystemIndicesTests.java | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/server/src/test/java/org/opensearch/indices/SystemIndicesTests.java b/server/src/test/java/org/opensearch/indices/SystemIndicesTests.java index 762a651d7ec50..ca9370645dec3 100644 --- a/server/src/test/java/org/opensearch/indices/SystemIndicesTests.java +++ b/server/src/test/java/org/opensearch/indices/SystemIndicesTests.java @@ -42,8 +42,10 @@ import java.util.Collection; import java.util.Collections; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.Set; +import java.util.stream.Collectors; import static java.util.Collections.emptyMap; import static java.util.Collections.singletonList; @@ -189,6 +191,26 @@ public void testSystemIndexMatching() { assertThat(SystemIndexRegistry.matchesSystemIndexPattern(Set.of(".not-system")), equalTo(Collections.emptySet())); } + public void testRegisteredSystemIndexGetAllDescriptors() { + 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) + ) + ); + assertEquals( + SystemIndexRegistry.getAllDescriptors() + .stream() + .map(SystemIndexDescriptor::getIndexPattern) + .collect(Collectors.toUnmodifiableList()), + List.of(SystemIndexPlugin1.SYSTEM_INDEX_1, SystemIndexPlugin2.SYSTEM_INDEX_2, TASK_INDEX + "*") + ); + } + public void testRegisteredSystemIndexMatching() { SystemIndexPlugin plugin1 = new SystemIndexPlugin1(); SystemIndexPlugin plugin2 = new SystemIndexPlugin2(); From 609b715801a13d2400c99ac27a28ab5e6b1fea0c Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Mon, 15 Jul 2024 21:50:15 -0400 Subject: [PATCH 6/6] Update server/src/main/java/org/opensearch/indices/SystemIndexRegistry.java Co-authored-by: Andriy Redko Signed-off-by: Craig Perkins --- .../main/java/org/opensearch/indices/SystemIndexRegistry.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/opensearch/indices/SystemIndexRegistry.java b/server/src/main/java/org/opensearch/indices/SystemIndexRegistry.java index a0aafaca65002..ab2cbd4ef1a73 100644 --- a/server/src/main/java/org/opensearch/indices/SystemIndexRegistry.java +++ b/server/src/main/java/org/opensearch/indices/SystemIndexRegistry.java @@ -72,7 +72,7 @@ public static Set matchesPluginSystemIndexPattern(String pluginClassName .collect(Collectors.toSet()); } - public static List getAllDescriptors() { + static List getAllDescriptors() { return SYSTEM_INDEX_DESCRIPTORS_MAP.values().stream().flatMap(Collection::stream).collect(Collectors.toList()); }