From cdd2cb7de05422748b2bf489a585f55cde2eb391 Mon Sep 17 00:00:00 2001 From: Jay Modi Date: Wed, 26 Aug 2020 08:34:14 -0600 Subject: [PATCH] Remove tasks module to define tasks system index This commit removes the tasks module that only existed to define the tasks result index, `.tasks`, as a system index. The definition for the tasks results system index descriptor is moved to the `SystemIndices` class with a check that no other plugin or module attempts to define an entry with the same source. Additionally, this change also makes the pattern for the tasks result index a wildcard pattern since we will need this when the index is upgraded (reindex to new name and then alias that to .tasks). Backport of #61540 --- modules/tasks/build.gradle | 25 ---------- .../tasksplugin/TasksPlugin.java | 41 ---------------- .../tasksplugin/TasksPluginTests.java | 33 ------------- .../elasticsearch/indices/SystemIndices.java | 47 ++++++++++++++++--- .../indices/SystemIndicesTests.java | 35 ++++++++++---- 5 files changed, 68 insertions(+), 113 deletions(-) delete mode 100644 modules/tasks/build.gradle delete mode 100644 modules/tasks/src/main/java/org/elasticsearch/tasksplugin/TasksPlugin.java delete mode 100644 modules/tasks/src/test/java/org/elasticsearch/tasksplugin/TasksPluginTests.java diff --git a/modules/tasks/build.gradle b/modules/tasks/build.gradle deleted file mode 100644 index b3ba90ccb6a11..0000000000000 --- a/modules/tasks/build.gradle +++ /dev/null @@ -1,25 +0,0 @@ -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -esplugin { - description 'Supports the Tasks API' - classname 'org.elasticsearch.tasksplugin.TasksPlugin' -} - -integTest.enabled = false diff --git a/modules/tasks/src/main/java/org/elasticsearch/tasksplugin/TasksPlugin.java b/modules/tasks/src/main/java/org/elasticsearch/tasksplugin/TasksPlugin.java deleted file mode 100644 index 0467b9419c778..0000000000000 --- a/modules/tasks/src/main/java/org/elasticsearch/tasksplugin/TasksPlugin.java +++ /dev/null @@ -1,41 +0,0 @@ -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -package org.elasticsearch.tasksplugin; - -import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.indices.SystemIndexDescriptor; -import org.elasticsearch.plugins.Plugin; -import org.elasticsearch.plugins.SystemIndexPlugin; - -import java.util.Collection; -import java.util.Collections; - -import static org.elasticsearch.tasks.TaskResultsService.TASK_INDEX; - -/** - * This plugin currently only exists to register `.tasks` as a system index. - */ -public class TasksPlugin extends Plugin implements SystemIndexPlugin { - - @Override - public Collection getSystemIndexDescriptors(Settings settings) { - return Collections.singletonList(new SystemIndexDescriptor(TASK_INDEX, this.getClass().getSimpleName())); - } -} diff --git a/modules/tasks/src/test/java/org/elasticsearch/tasksplugin/TasksPluginTests.java b/modules/tasks/src/test/java/org/elasticsearch/tasksplugin/TasksPluginTests.java deleted file mode 100644 index 23b873e377eb3..0000000000000 --- a/modules/tasks/src/test/java/org/elasticsearch/tasksplugin/TasksPluginTests.java +++ /dev/null @@ -1,33 +0,0 @@ -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -package org.elasticsearch.tasksplugin; - -import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.test.ESTestCase; -import org.hamcrest.Matchers; - -public class TasksPluginTests extends ESTestCase { - - public void testDummy() { - // This is a dummy test case to satisfy the conventions - TasksPlugin plugin = new TasksPlugin(); - assertThat(plugin.getSystemIndexDescriptors(Settings.EMPTY), Matchers.hasSize(1)); - } -} diff --git a/server/src/main/java/org/elasticsearch/indices/SystemIndices.java b/server/src/main/java/org/elasticsearch/indices/SystemIndices.java index 016458fec48a6..ea72b682cb494 100644 --- a/server/src/main/java/org/elasticsearch/indices/SystemIndices.java +++ b/server/src/main/java/org/elasticsearch/indices/SystemIndices.java @@ -28,15 +28,21 @@ import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.regex.Regex; import org.elasticsearch.index.Index; +import org.elasticsearch.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.elasticsearch.tasks.TaskResultsService.TASK_INDEX; /** * This class holds the {@link SystemIndexDescriptor} objects that represent system indices the @@ -45,12 +51,17 @@ */ public class SystemIndices { + 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> systemIndexDescriptorMap) { - checkForOverlappingPatterns(systemIndexDescriptorMap); - this.systemIndexDescriptors = unmodifiableList(systemIndexDescriptorMap.values() + public SystemIndices(Map> pluginAndModulesDescriptors) { + final Map> descriptorsMap = buildSystemIndexDescriptorMap(pluginAndModulesDescriptors); + checkForOverlappingPatterns(descriptorsMap); + this.systemIndexDescriptors = unmodifiableList(descriptorsMap.values() .stream() .flatMap(Collection::stream) .collect(Collectors.toList())); @@ -63,7 +74,16 @@ public SystemIndices(Map> systemIndexD * @return true if the {@link Index}'s name matches a pattern from a {@link SystemIndexDescriptor} */ public boolean isSystemIndex(Index index) { - return runAutomaton.run(index.getName()); + return isSystemIndex(index.getName()); + } + + /** + * Determines whether a given index is a system index by comparing its name to the collection of loaded {@link SystemIndexDescriptor}s + * @param indexName the index name to check against loaded {@link SystemIndexDescriptor}s + * @return true if the index name matches a pattern from a {@link SystemIndexDescriptor} + */ + public boolean isSystemIndex(String indexName) { + return runAutomaton.run(indexName); } /** @@ -126,10 +146,10 @@ static void checkForOverlappingPatterns(Map overlaps(descriptorToCheck.v2(), d.v2())) .collect(Collectors.toList()); if (descriptorsMatchingThisPattern.isEmpty() == false) { - throw new IllegalStateException("a system index descriptor [" + descriptorToCheck.v2() + "] from plugin [" + + 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 plugin [" + descriptor.v1() + "]") + .map(descriptor -> descriptor.v2() + " from [" + descriptor.v1() + "]") .collect(Collectors.joining(", "))); } }); @@ -140,4 +160,19 @@ private static boolean overlaps(SystemIndexDescriptor a1, SystemIndexDescriptor 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/test/java/org/elasticsearch/indices/SystemIndicesTests.java b/server/src/test/java/org/elasticsearch/indices/SystemIndicesTests.java index 926135782621a..a09dfc4f9ab46 100644 --- a/server/src/test/java/org/elasticsearch/indices/SystemIndicesTests.java +++ b/server/src/test/java/org/elasticsearch/indices/SystemIndicesTests.java @@ -19,14 +19,18 @@ package org.elasticsearch.indices; +import org.elasticsearch.tasks.TaskResultsService; import org.elasticsearch.test.ESTestCase; import java.util.Arrays; import java.util.Collection; -import java.util.Collections; import java.util.HashMap; import java.util.Map; +import static java.util.Collections.emptyMap; +import static java.util.Collections.singletonList; +import static java.util.Collections.singletonMap; +import static org.elasticsearch.tasks.TaskResultsService.TASK_INDEX; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.not; @@ -45,14 +49,14 @@ public void testBasicOverlappingPatterns() { String broadPatternSource = "AAA" + randomAlphaOfLength(5); String otherSource = "ZZZ" + randomAlphaOfLength(6); Map> descriptors = new HashMap<>(); - descriptors.put(broadPatternSource, Collections.singletonList(broadPattern)); + descriptors.put(broadPatternSource, singletonList(broadPattern)); descriptors.put(otherSource, Arrays.asList(notOverlapping, overlapping1, overlapping2, overlapping3)); IllegalStateException exception = expectThrows(IllegalStateException.class, () -> SystemIndices.checkForOverlappingPatterns(descriptors)); assertThat(exception.getMessage(), containsString("a system index descriptor [" + broadPattern + - "] from plugin [" + broadPatternSource + "] overlaps with other system index descriptors:")); - String fromPluginString = " from plugin [" + otherSource + "]"; + "] from [" + broadPatternSource + "] overlaps with other system index descriptors:")); + String fromPluginString = " from [" + otherSource + "]"; assertThat(exception.getMessage(), containsString(overlapping1.toString() + fromPluginString)); assertThat(exception.getMessage(), containsString(overlapping2.toString() + fromPluginString)); assertThat(exception.getMessage(), containsString(overlapping3.toString() + fromPluginString)); @@ -72,16 +76,31 @@ public void testComplexOverlappingPatterns() { String source1 = "AAA" + randomAlphaOfLength(5); String source2 = "ZZZ" + randomAlphaOfLength(6); Map> descriptors = new HashMap<>(); - descriptors.put(source1, Collections.singletonList(pattern1)); - descriptors.put(source2, Collections.singletonList(pattern2)); + descriptors.put(source1, singletonList(pattern1)); + descriptors.put(source2, singletonList(pattern2)); IllegalStateException exception = expectThrows(IllegalStateException.class, () -> SystemIndices.checkForOverlappingPatterns(descriptors)); assertThat(exception.getMessage(), containsString("a system index descriptor [" + pattern1 + - "] from plugin [" + source1 + "] overlaps with other system index descriptors:")); - assertThat(exception.getMessage(), containsString(pattern2.toString() + " from plugin [" + source2 + "]")); + "] from [" + source1 + "] overlaps with other system index descriptors:")); + assertThat(exception.getMessage(), containsString(pattern2.toString() + " from [" + source2 + "]")); IllegalStateException constructorException = expectThrows(IllegalStateException.class, () -> new SystemIndices(descriptors)); assertThat(constructorException.getMessage(), equalTo(exception.getMessage())); } + + public void testBuiltInSystemIndices() { + SystemIndices systemIndices = new SystemIndices(emptyMap()); + assertTrue(systemIndices.isSystemIndex(".tasks")); + assertTrue(systemIndices.isSystemIndex(".tasks1")); + assertTrue(systemIndices.isSystemIndex(".tasks-old")); + } + + public void testPluginCannotOverrideBuiltInSystemIndex() { + Map> pluginMap = singletonMap( + TaskResultsService.class.getName(), singletonList(new SystemIndexDescriptor(TASK_INDEX, "Task Result Index")) + ); + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> new SystemIndices(pluginMap)); + assertThat(e.getMessage(), containsString("plugin or module attempted to define the same source")); + } }