Check dot-index rules after template application (#52087)

Previously, the dot-index rules (namely, that indices with dot-prefixed
names should be either hidden indices or system indices) was done
before* template application, and so only checked for the `index.hidden`
setting in the request, ignoring if that setting was set via a template.

This commit moves that check to a different method, which is applied
after templates have been resolved and applied to the index settings.

server/src/main/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverAction.java

@@ -132,7 +132,7 @@ public class TransportRolloverAction extends TransportMasterNodeAction<RolloverR
         final String rolloverIndexName = indexNameExpressionResolver.resolveDateMathExpression(unresolvedName);
         final Boolean isHidden = IndexMetaData.INDEX_HIDDEN_SETTING.exists(rolloverRequest.getCreateIndexRequest().settings()) ?
             IndexMetaData.INDEX_HIDDEN_SETTING.get(rolloverRequest.getCreateIndexRequest().settings()) : null;
-        createIndexService.validateIndexName(rolloverIndexName, state, isHidden); // fails if the index already exists
+        createIndexService.validateIndexName(rolloverIndexName, state); // fails if the index already exists
         checkNoDuplicatedAliasInIndexTemplate(metaData, rolloverIndexName, rolloverRequest.getAlias(), isHidden);
         IndicesStatsRequest statsRequest = new IndicesStatsRequest().indices(rolloverRequest.getAlias())
             .clear()

server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java

@@ -169,12 +169,32 @@ public class MetaDataCreateIndexService {
     /**
      * Validate the name for an index against some static rules and a cluster state.
      */
-    public void validateIndexName(String index, ClusterState state, @Nullable Boolean isHidden) {
+    public void validateIndexName(String index, ClusterState state) {
         validateIndexOrAliasName(index, InvalidIndexNameException::new);
         if (!index.toLowerCase(Locale.ROOT).equals(index)) {
             throw new InvalidIndexNameException(index, "must be lowercase");
         }
 
+        // NOTE: dot-prefixed index names are validated after template application, not here
+
+        if (state.routingTable().hasIndex(index)) {
+            throw new ResourceAlreadyExistsException(state.routingTable().index(index).getIndex());
+        }
+        if (state.metaData().hasIndex(index)) {
+            throw new ResourceAlreadyExistsException(state.metaData().index(index).getIndex());
+        }
+        if (state.metaData().hasAlias(index)) {
+            throw new InvalidIndexNameException(index, "already exists as alias");
+        }
+    }
+
+    /**
+     * Validates (if this index has a dot-prefixed name) whether it follows the rules for dot-prefixed indices.
+     * @param index The name of the index in question
+     * @param state The current cluster state
+     * @param isHidden Whether or not this is a hidden index
+     */
+    public void validateDotIndex(String index, ClusterState state, @Nullable Boolean isHidden) {
         if (index.charAt(0) == '.') {
             List<SystemIndexDescriptor> matchingDescriptors = systemIndexDescriptors.stream()
                 .filter(descriptor -> descriptor.matchesIndexPattern(index))
@@ -200,15 +220,6 @@ public class MetaDataCreateIndexService {
                 throw new IllegalStateException(errorMessage.toString());
             }
         }
-        if (state.routingTable().hasIndex(index)) {
-            throw new ResourceAlreadyExistsException(state.routingTable().index(index).getIndex());
-        }
-        if (state.metaData().hasIndex(index)) {
-            throw new ResourceAlreadyExistsException(state.metaData().index(index).getIndex());
-        }
-        if (state.metaData().hasAlias(index)) {
-            throw new InvalidIndexNameException(index, "already exists as alias");
-        }
     }
 
     /**
@@ -322,10 +333,12 @@ public class MetaDataCreateIndexService {
 
         // we only find a template when its an API call (a new index)
         // find templates, highest order are better matching
-        final Boolean isHidden = IndexMetaData.INDEX_HIDDEN_SETTING.exists(request.settings()) ?
+        final Boolean isHiddenFromRequest = IndexMetaData.INDEX_HIDDEN_SETTING.exists(request.settings()) ?
             IndexMetaData.INDEX_HIDDEN_SETTING.get(request.settings()) : null;
         final List<IndexTemplateMetaData> templates = sourceMetaData == null ?
-            Collections.unmodifiableList(MetaDataIndexTemplateService.findTemplates(currentState.metaData(), request.index(), isHidden)) :
+            Collections.unmodifiableList(MetaDataIndexTemplateService.findTemplates(currentState.metaData(),
+                request.index(),
+                isHiddenFromRequest)) :
             Collections.emptyList();
 
         final Map<String, Map<String, Object>> mappings = Collections.unmodifiableMap(parseMappings(request.mappings(), templates,
@@ -335,6 +348,9 @@ public class MetaDataCreateIndexService {
             aggregateIndexSettings(currentState, request, templates, mappings, sourceMetaData, settings, indexScopedSettings);
         int routingNumShards = getIndexNumberOfRoutingShards(aggregatedIndexSettings, sourceMetaData);
 
+        final boolean isHiddenAfterTemplates = IndexMetaData.INDEX_HIDDEN_SETTING.get(aggregatedIndexSettings);
+        validateDotIndex(request.index(), currentState, isHiddenAfterTemplates);
+
         // remove the setting it's temporary and is only relevant once we create the index
         final Settings.Builder settingsBuilder = Settings.builder().put(aggregatedIndexSettings);
         settingsBuilder.remove(IndexMetaData.INDEX_NUMBER_OF_ROUTING_SHARDS_SETTING.getKey());
@@ -742,8 +758,7 @@ public class MetaDataCreateIndexService {
     }
 
     private void validate(CreateIndexClusterStateUpdateRequest request, ClusterState state) {
-        boolean isHidden = IndexMetaData.INDEX_HIDDEN_SETTING.get(request.settings());
+        validateIndexName(request.index(), state);
-        validateIndexName(request.index(), state, isHidden);
         validateIndexSettings(request.index(), request.settings(), forbidPrivateIndexSettings);
     }
 

server/src/main/java/org/elasticsearch/snapshots/RestoreService.java

@@ -279,7 +279,8 @@ public class RestoreService implements ClusterStateApplier {
                                     // Index doesn't exist - create it and start recovery
                                     // Make sure that the index we are about to create has a validate name
                                     boolean isHidden = IndexMetaData.INDEX_HIDDEN_SETTING.get(snapshotIndexMetaData.getSettings());
-                                    createIndexService.validateIndexName(renamedIndexName, currentState, isHidden);
+                                    createIndexService.validateIndexName(renamedIndexName, currentState);
+                                    createIndexService.validateDotIndex(renamedIndexName, currentState, isHidden);
                                     createIndexService.validateIndexSettings(renamedIndexName, snapshotIndexMetaData.getSettings(), false);
                                     IndexMetaData.Builder indexMdBuilder = IndexMetaData.builder(snapshotIndexMetaData)
                                         .state(IndexMetaData.State.OPEN)

server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexServiceTests.java

@@ -517,7 +517,7 @@ public class MetaDataCreateIndexServiceTests extends ESTestCase {
     private void validateIndexName(MetaDataCreateIndexService metaDataCreateIndexService, String indexName, String errorMessage) {
         InvalidIndexNameException e = expectThrows(InvalidIndexNameException.class,
             () -> metaDataCreateIndexService.validateIndexName(indexName, ClusterState.builder(ClusterName.CLUSTER_NAME_SETTING
-                .getDefault(Settings.EMPTY)).build(), false));
+                .getDefault(Settings.EMPTY)).build()));
         assertThat(e.getMessage(), endsWith(errorMessage));
     }
 
@@ -586,7 +586,7 @@ public class MetaDataCreateIndexServiceTests extends ESTestCase {
         assertThat(e, hasToString(containsString(expectedMessage)));
     }
 
-    public void testValidateIndexNameChecksSystemIndexNames() {
+    public void testValidateDotIndex() {
         List<SystemIndexDescriptor> systemIndexDescriptors = new ArrayList<>();
         systemIndexDescriptors.add(new SystemIndexDescriptor(".test", "test"));
         systemIndexDescriptors.add(new SystemIndexDescriptor(".test3", "test"));
@@ -609,25 +609,25 @@ public class MetaDataCreateIndexServiceTests extends ESTestCase {
                 false
             );
             // Check deprecations
-            checkerService.validateIndexName(".test2", ClusterState.EMPTY_STATE, false);
+            checkerService.validateDotIndex(".test2", ClusterState.EMPTY_STATE, false);
             assertWarnings("index name [.test2] starts with a dot '.', in the next major version, index " +
                 "names starting with a dot are reserved for hidden indices and system indices");
 
             // Check non-system hidden indices don't trigger a warning
-            checkerService.validateIndexName(".test2", ClusterState.EMPTY_STATE, true);
+            checkerService.validateDotIndex(".test2", ClusterState.EMPTY_STATE, true);
 
             // Check NO deprecation warnings if we give the index name
-            checkerService.validateIndexName(".test", ClusterState.EMPTY_STATE, false);
+            checkerService.validateDotIndex(".test", ClusterState.EMPTY_STATE, false);
-            checkerService.validateIndexName(".test3", ClusterState.EMPTY_STATE, false);
+            checkerService.validateDotIndex(".test3", ClusterState.EMPTY_STATE, false);
 
             // Check that patterns with wildcards work
-            checkerService.validateIndexName(".pattern-test", ClusterState.EMPTY_STATE, false);
+            checkerService.validateDotIndex(".pattern-test", ClusterState.EMPTY_STATE, false);
-            checkerService.validateIndexName(".pattern-test-with-suffix", ClusterState.EMPTY_STATE, false);
+            checkerService.validateDotIndex(".pattern-test-with-suffix", ClusterState.EMPTY_STATE, false);
-            checkerService.validateIndexName(".pattern-test-other-suffix", ClusterState.EMPTY_STATE, false);
+            checkerService.validateDotIndex(".pattern-test-other-suffix", ClusterState.EMPTY_STATE, false);
 
             // Check that an exception is thrown if more than one descriptor matches the index name
             AssertionError exception = expectThrows(AssertionError.class,
-                () -> checkerService.validateIndexName(".pattern-test-overlapping", ClusterState.EMPTY_STATE, false));
+                () -> checkerService.validateDotIndex(".pattern-test-overlapping", ClusterState.EMPTY_STATE, false));
             assertThat(exception.getMessage(),
                 containsString("index name [.pattern-test-overlapping] is claimed as a system index by multiple system index patterns:"));
             assertThat(exception.getMessage(), containsString("pattern: [.pattern-test*], description: [test-1]"));
@@ -667,11 +667,11 @@ public class MetaDataCreateIndexServiceTests extends ESTestCase {
             );
 
             excludedNames.forEach(name -> {
-                checkerService.validateIndexName(name, ClusterState.EMPTY_STATE, false);
+                checkerService.validateDotIndex(name, ClusterState.EMPTY_STATE, false);
             });
 
             excludedNames.forEach(name -> {
-                expectThrows(AssertionError.class, () -> checkerService.validateIndexName(name, ClusterState.EMPTY_STATE, true));
+                expectThrows(AssertionError.class, () -> checkerService.validateDotIndex(name, ClusterState.EMPTY_STATE, true));
             });
         } finally {
             testThreadPool.shutdown();