Deprecate creation of non-system dot-prefixed index names

gwbrown · gwbrown · commit c73164670f88 · 2019-12-06T18:05:04.000-07:00
This commit deprecates the creation of dot-prefixed index names (e.g.
`.watches`) unless they are registered by a plugin that extends
`SystemIndexPlugin`. This is the first step towards more thorough
proections for system indices.

This commit also modifies several plugins which use dot-prefixed indices
to register indices they own as system indices.
diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverAction.java b/server/src/main/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverAction.java
@@ -57,6 +57,7 @@
 
 import java.io.IOException;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
@@ -122,7 +123,7 @@ protected void masterOperation(Task task, final RolloverRequest rolloverRequest,
             ? rolloverRequest.getNewIndexName()
             : generateRolloverIndexName(sourceProvidedName, indexNameExpressionResolver);
         final String rolloverIndexName = indexNameExpressionResolver.resolveDateMathExpression(unresolvedName);
-        MetaDataCreateIndexService.validateIndexName(rolloverIndexName, state); // will fail if the index already exists
+        MetaDataCreateIndexService.validateIndexName(rolloverIndexName, state, Collections.emptySet()); // fails if the index already exists
         checkNoDuplicatedAliasInIndexTemplate(metaData, rolloverIndexName, rolloverRequest.getAlias());
         IndicesStatsRequest statsRequest = new IndicesStatsRequest().indices(rolloverRequest.getAlias())
             .clear()
diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java b/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java
@@ -46,12 +46,14 @@
 import org.elasticsearch.cluster.routing.ShardRoutingState;
 import org.elasticsearch.cluster.routing.allocation.AllocationService;
 import org.elasticsearch.cluster.service.ClusterService;
+import org.elasticsearch.common.Nullable;
 import org.elasticsearch.common.Priority;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.UUIDs;
 import org.elasticsearch.common.ValidationException;
 import org.elasticsearch.common.compress.CompressedXContent;
 import org.elasticsearch.common.io.PathUtils;
+import org.elasticsearch.common.logging.DeprecationLogger;
 import org.elasticsearch.common.settings.IndexScopedSettings;
 import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.settings.Settings;
@@ -68,6 +70,7 @@
 import org.elasticsearch.indices.IndexCreationException;
 import org.elasticsearch.indices.IndicesService;
 import org.elasticsearch.indices.InvalidIndexNameException;
+import org.elasticsearch.indices.SystemIndexDescriptor;
 import org.elasticsearch.indices.cluster.IndicesClusterStateService.AllocatedIndices.IndexRemovalReason;
 import org.elasticsearch.threadpool.ThreadPool;
 
@@ -80,6 +83,7 @@
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicInteger;
@@ -98,6 +102,7 @@
  */
 public class MetaDataCreateIndexService {
     private static final Logger logger = LogManager.getLogger(MetaDataCreateIndexService.class);
+    private static final DeprecationLogger deprecationLogger = new DeprecationLogger(logger);
 
     public static final int MAX_INDEX_NAME_BYTES = 255;
 
@@ -110,19 +115,21 @@ public class MetaDataCreateIndexService {
     private final IndexScopedSettings indexScopedSettings;
     private final ActiveShardsObserver activeShardsObserver;
     private final NamedXContentRegistry xContentRegistry;
+    private final Map<String, SystemIndexDescriptor> systemIndexDescriptors;
     private final boolean forbidPrivateIndexSettings;
 
     public MetaDataCreateIndexService(
-            final Settings settings,
-            final ClusterService clusterService,
-            final IndicesService indicesService,
-            final AllocationService allocationService,
-            final AliasValidator aliasValidator,
-            final Environment env,
-            final IndexScopedSettings indexScopedSettings,
-            final ThreadPool threadPool,
-            final NamedXContentRegistry xContentRegistry,
-            final boolean forbidPrivateIndexSettings) {
+        final Settings settings,
+        final ClusterService clusterService,
+        final IndicesService indicesService,
+        final AllocationService allocationService,
+        final AliasValidator aliasValidator,
+        final Environment env,
+        final IndexScopedSettings indexScopedSettings,
+        final ThreadPool threadPool,
+        final NamedXContentRegistry xContentRegistry,
+        final Map<String, SystemIndexDescriptor> systemIndexDescriptors,
+        final boolean forbidPrivateIndexSettings) {
         this.settings = settings;
         this.clusterService = clusterService;
         this.indicesService = indicesService;
@@ -132,17 +139,23 @@ public MetaDataCreateIndexService(
         this.indexScopedSettings = indexScopedSettings;
         this.activeShardsObserver = new ActiveShardsObserver(clusterService, threadPool);
         this.xContentRegistry = xContentRegistry;
+        this.systemIndexDescriptors = systemIndexDescriptors;
         this.forbidPrivateIndexSettings = forbidPrivateIndexSettings;
     }
 
     /**
      * Validate the name for an index against some static rules and a cluster state.
+     * Set of allowed system index names is *optional* - provide it only if you want to allow certain dot-prefixed names.
      */
-    public static void validateIndexName(String index, ClusterState state) {
+    public static void validateIndexName(String index, ClusterState state, @Nullable final Set<String> allowedSystemIndices) {
         validateIndexOrAliasName(index, InvalidIndexNameException::new);
         if (!index.toLowerCase(Locale.ROOT).equals(index)) {
             throw new InvalidIndexNameException(index, "must be lowercase");
         }
+        if (index.charAt(0) == '.' && (Objects.isNull(allowedSystemIndices) || allowedSystemIndices.contains(index) == false)) {
+            deprecationLogger.deprecated("index name [{}] starts with a dot '.', in the next major version, creating indices with " +
+                "names starting with a dot will fail as these names are reserved for system indices", index);
+        }
         if (state.routingTable().hasIndex(index)) {
             throw new ResourceAlreadyExistsException(state.routingTable().index(index).getIndex());
         }
@@ -591,7 +604,7 @@ public void onFailure(String source, Exception e) {
     }
 
     private void validate(CreateIndexClusterStateUpdateRequest request, ClusterState state) {
-        validateIndexName(request.index(), state);
+        validateIndexName(request.index(), state, systemIndexDescriptors.keySet());
         validateIndexSettings(request.index(), request.settings(), forbidPrivateIndexSettings);
     }
 
diff --git a/server/src/main/java/org/elasticsearch/indices/SystemIndexDescriptor.java b/server/src/main/java/org/elasticsearch/indices/SystemIndexDescriptor.java
@@ -0,0 +1,59 @@
+/*
+ * 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.indices;
+
+/**
+ * Describes a system index. Provides the information required to create and maintain the system index.
+ */
+public class SystemIndexDescriptor {
+    private final String name;
+    private final String sourcePluginName;
+
+    /**
+     *
+     * @param indexName The name of the system index.
+     * @param sourcePluginName The name of the plugin responsible for this system index.
+     */
+    public SystemIndexDescriptor(String indexName, String sourcePluginName) {
+        this.name = indexName;
+        this.sourcePluginName = sourcePluginName;
+    }
+
+    /**
+     * Get the name of the system index.
+     * @return The name of the system index.
+     */
+    public String getIndexName() {
+        return name;
+    }
+
+    /**
+     * Get the name of the plugin responsible for this system index. It is recommended, but not required, that this is
+     * the name of the class implementing {@link org.elasticsearch.plugins.SystemIndexPlugin}.
+     * @return The name of the plugin responsible for this system index.
+     */
+    public String getSourcePluginName() {
+        return sourcePluginName;
+    }
+
+    // TODO: Index settings and mapping
+    // TODO: getThreadpool()
+    // TODO: Upgrade handling (reindex script?)
+}
diff --git a/server/src/main/java/org/elasticsearch/node/Node.java b/server/src/main/java/org/elasticsearch/node/Node.java
@@ -100,6 +100,7 @@
 import org.elasticsearch.index.engine.EngineFactory;
 import org.elasticsearch.indices.IndicesModule;
 import org.elasticsearch.indices.IndicesService;
+import org.elasticsearch.indices.SystemIndexDescriptor;
 import org.elasticsearch.indices.analysis.AnalysisModule;
 import org.elasticsearch.indices.breaker.CircuitBreakerService;
 import org.elasticsearch.indices.breaker.HierarchyCircuitBreakerService;
@@ -132,6 +133,7 @@
 import org.elasticsearch.plugins.RepositoryPlugin;
 import org.elasticsearch.plugins.ScriptPlugin;
 import org.elasticsearch.plugins.SearchPlugin;
+import org.elasticsearch.plugins.SystemIndexPlugin;
 import org.elasticsearch.repositories.RepositoriesModule;
 import org.elasticsearch.repositories.RepositoriesService;
 import org.elasticsearch.rest.RestController;
@@ -422,6 +424,15 @@ protected Node(
                             .flatMap(m -> m.entrySet().stream())
                             .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
 
+            final Map<String, SystemIndexDescriptor> systemIndexDescriptors = pluginsService.filterPlugins(SystemIndexPlugin.class)
+                .stream()
+                .flatMap(plugin -> plugin.getSystemIndexDescriptors().stream())
+                .collect(Collectors.toUnmodifiableMap(SystemIndexDescriptor::getIndexName, Function.identity(),
+                    (descA, descB) -> {
+                        throw new IllegalStateException("\"multiple plugins, [" + descA.getSourcePluginName() + "] and [" +
+                            descB.getSourcePluginName() + "], attempted to register index name [" + descA.getIndexName() + "]," +
+                            "but each system index can only be managed by a single plugin");
+                    }));
             final IndicesService indicesService =
                 new IndicesService(settings, pluginsService, nodeEnvironment, xContentRegistry, analysisModule.getAnalysisRegistry(),
                     clusterModule.getIndexNameExpressionResolver(), indicesModule.getMapperRegistry(), namedWriteableRegistry,
@@ -440,6 +451,7 @@ protected Node(
                     settingsModule.getIndexScopedSettings(),
                     threadPool,
                     xContentRegistry,
+                    systemIndexDescriptors,
                     forbidPrivateIndexSettings);
 
             Collection<Object> pluginComponents = pluginsService.filterPlugins(Plugin.class).stream()
diff --git a/server/src/main/java/org/elasticsearch/plugins/SystemIndexPlugin.java b/server/src/main/java/org/elasticsearch/plugins/SystemIndexPlugin.java
@@ -0,0 +1,41 @@
+/*
+ * 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.plugins;
+
+import org.elasticsearch.indices.SystemIndexDescriptor;
+
+import java.util.Collection;
+import java.util.Collections;
+
+/**
+ * Plugin for defining system indices. Extends {@link ActionPlugin} because system indices must be accessed via APIs
+ * added by the plugin that owns the system index, rather than standard APIs.
+ */
+public interface SystemIndexPlugin extends ActionPlugin {
+
+    /**
+     * Returns a {@link Collection} of {@link SystemIndexDescriptor}s that describe this plugin's system indices, including
+     * name, mapping, and settings.
+     * @return Descriptions of the system indices managed by this plugin.
+     */
+    default Collection<SystemIndexDescriptor> getSystemIndexDescriptors() {
+        return Collections.emptyList();
+    }
+}
diff --git a/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java b/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java
@@ -269,7 +269,7 @@ public ClusterState execute(ClusterState currentState) {
                                 if (currentIndexMetaData == null) {
                                     // Index doesn't exist - create it and start recovery
                                     // Make sure that the index we are about to create has a validate name
-                                    MetaDataCreateIndexService.validateIndexName(renamedIndexName, currentState);
+                                    MetaDataCreateIndexService.validateIndexName(renamedIndexName, currentState, Collections.emptySet());
                                     createIndexService.validateIndexSettings(renamedIndexName, snapshotIndexMetaData.getSettings(), false);
                                     IndexMetaData.Builder indexMdBuilder = IndexMetaData.builder(snapshotIndexMetaData)
                                         .state(IndexMetaData.State.OPEN)
diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/template/put/MetaDataIndexTemplateServiceTests.java b/server/src/test/java/org/elasticsearch/action/admin/indices/template/put/MetaDataIndexTemplateServiceTests.java
@@ -160,6 +160,7 @@ private static List<Throwable> putTemplate(NamedXContentRegistry xContentRegistr
                 IndexScopedSettings.DEFAULT_SCOPED_SETTINGS,
                 null,
                 xContentRegistry,
+                Collections.emptyMap(),
                 true);
         MetaDataIndexTemplateService service = new MetaDataIndexTemplateService(null, createIndexService,
                 new AliasValidator(), null,
@@ -193,6 +194,7 @@ private List<Throwable> putTemplateDetail(PutRequest request) throws Exception {
                 null,
                 null,
                 xContentRegistry(),
+                Collections.emptyMap(),
                 true);
         MetaDataIndexTemplateService service = new MetaDataIndexTemplateService(
                 clusterService, createIndexService, new AliasValidator(), indicesService,
diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexServiceTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexServiceTests.java
@@ -51,6 +51,7 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Comparator;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Locale;
 import java.util.Set;
@@ -423,7 +424,7 @@ public void testValidateIndexName() throws Exception {
     private void validateIndexName(String indexName, String errorMessage) {
         InvalidIndexNameException e = expectThrows(InvalidIndexNameException.class,
             () -> MetaDataCreateIndexService.validateIndexName(indexName, ClusterState.builder(ClusterName.CLUSTER_NAME_SETTING
-                .getDefault(Settings.EMPTY)).build()));
+                .getDefault(Settings.EMPTY)).build(), Collections.emptySet()));
         assertThat(e.getMessage(), endsWith(errorMessage));
     }
 
@@ -486,4 +487,22 @@ public void testShardLimit() {
         assertThat(e, hasToString(containsString(expectedMessage)));
     }
 
+    public void testValidateIndexNameChecksSystemIndexNames() {
+        // Check deprecations
+        MetaDataCreateIndexService.validateIndexName(".test", ClusterState.EMPTY_STATE, null);
+        assertWarnings("index name [.test] starts with a dot '.', in the next major version, creating indices with " +
+            "names starting with a dot will fail as these names are reserved for system indices");
+        MetaDataCreateIndexService.validateIndexName(".test2", ClusterState.EMPTY_STATE, Collections.emptySet());
+        assertWarnings("index name [.test2] starts with a dot '.', in the next major version, creating indices with " +
+            "names starting with a dot will fail as these names are reserved for system indices");
+        MetaDataCreateIndexService.validateIndexName(".test3", ClusterState.EMPTY_STATE,
+            new HashSet<>(Arrays.asList(".test1", ".test2", ".foobar")));
+        assertWarnings("index name [.test3] starts with a dot '.', in the next major version, creating indices with " +
+            "names starting with a dot will fail as these names are reserved for system indices");
+
+        // Check NO deprecation warnings if we give the index name
+        MetaDataCreateIndexService.validateIndexName(".test4", ClusterState.EMPTY_STATE, new HashSet<>(Arrays.asList(".test4")));
+        MetaDataCreateIndexService.validateIndexName(".test5", ClusterState.EMPTY_STATE,
+            new HashSet<>(Arrays.asList(".foo", ".bar", ".baz", ".test5", ".quux")));
+    }
 }
diff --git a/server/src/test/java/org/elasticsearch/indices/cluster/ClusterStateChanges.java b/server/src/test/java/org/elasticsearch/indices/cluster/ClusterStateChanges.java
@@ -204,7 +204,7 @@ public IndexMetaData upgradeIndexMetaData(IndexMetaData indexMetaData, Version m
             allocationService, IndexScopedSettings.DEFAULT_SCOPED_SETTINGS, indicesService, threadPool);
         MetaDataCreateIndexService createIndexService = new MetaDataCreateIndexService(SETTINGS, clusterService, indicesService,
             allocationService, new AliasValidator(), environment,
-            IndexScopedSettings.DEFAULT_SCOPED_SETTINGS, threadPool, xContentRegistry, true);
+            IndexScopedSettings.DEFAULT_SCOPED_SETTINGS, threadPool, xContentRegistry, Collections.emptyMap(), true);
 
         transportCloseIndexAction = new TransportCloseIndexAction(SETTINGS, transportService, clusterService, threadPool,
             indexStateService, clusterSettings, actionFilters, indexNameExpressionResolver, destructiveOperations);
diff --git a/server/src/test/java/org/elasticsearch/snapshots/SnapshotResiliencyTests.java b/server/src/test/java/org/elasticsearch/snapshots/SnapshotResiliencyTests.java
@@ -1138,7 +1138,7 @@ public void onFailure(final Exception e) {
                 final MetaDataCreateIndexService metaDataCreateIndexService = new MetaDataCreateIndexService(settings, clusterService,
                     indicesService,
                     allocationService, new AliasValidator(), environment, indexScopedSettings,
-                    threadPool, namedXContentRegistry, false);
+                    threadPool, namedXContentRegistry, Collections.emptyMap(), false);
                 actions.put(CreateIndexAction.INSTANCE,
                     new TransportCreateIndexAction(
                         transportService, clusterService, threadPool,
diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java
diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java
