Validate V2 templates more strictly (elastic#56170)

This commit changes the validation for V2 index and component templates to re-use the same
validation that V1 templates used. This includes things like invalid template names, index patterns,
etc.

This also adds validation that template names do not contain `*` and index patterns do not contain
`:` (index names can't contain this regardless).

Supercedes elastic#53970
Relates to elastic#53101
Resolves elastic#43737
Resolves elastic#46045

server/src/main/java/org/elasticsearch/action/admin/indices/template/post/TransportSimulateIndexTemplateAction.java

@@ -56,6 +56,7 @@
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
+import java.util.Locale;
 import java.util.Map;
 import java.util.function.Function;
 import java.util.stream.Collectors;
@@ -104,7 +105,7 @@ protected void masterOperation(SimulateIndexTemplateRequest request, ClusterStat
         ClusterState simulateOnClusterState = state;
         if (request.getIndexTemplateRequest() != null) {
             // we'll "locally" add the template defined by the user in the cluster state (as if it existed in the system)
-            String simulateTemplateToAdd = "simulate_new_template_" + UUIDs.randomBase64UUID();
+            String simulateTemplateToAdd = "simulate_new_template_" + UUIDs.randomBase64UUID().toLowerCase(Locale.ROOT);
             simulateOnClusterState = indexTemplateService.addIndexTemplateV2(state, request.getIndexTemplateRequest().create(),
                 simulateTemplateToAdd, request.getIndexTemplateRequest().indexTemplate());
         }

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

@@ -1055,7 +1055,7 @@ private static List<String> validatePrivateSettingsNotExplicitlySet(Settings set
         for (final String key : settings.keySet()) {
             final Setting<?> setting = indexScopedSettings.get(key);
             if (setting == null) {
-                assert indexScopedSettings.isPrivateSetting(key);
+                assert indexScopedSettings.isPrivateSetting(key) : "expected [" + key + "] to be private but it was not";
             } else if (setting.isPrivateIndex()) {
                 validationErrors.add("private index setting [" + key + "] can not be set explicitly");
             }

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

@@ -233,6 +233,7 @@ ClusterState addComponentTemplate(final ClusterState currentState, final boolean
         final Template finalTemplate = new Template(finalSettings,
             stringMappings == null ? null : new CompressedXContent(stringMappings), template.template().aliases());
         final ComponentTemplate finalComponentTemplate = new ComponentTemplate(finalTemplate, template.version(), template.metadata());
+        validate(name, finalComponentTemplate);
         logger.info("adding component template [{}]", name);
         return ClusterState.builder(currentState)
             .metadata(Metadata.builder(currentState.metadata()).put(name, finalComponentTemplate))
@@ -402,6 +403,7 @@ public ClusterState addIndexTemplateV2(final ClusterState currentState, final bo
                 template.priority(), template.version(), template.metadata());
         }
 
+        validate(name, finalIndexTemplate);
         logger.info("adding index template [{}]", name);
         return ClusterState.builder(currentState)
             .metadata(Metadata.builder(currentState.metadata()).put(name, finalIndexTemplate))
@@ -914,70 +916,117 @@ private static void validateTemplate(Settings validateSettings, Map<String, Stri
         }
     }
 
-    private void validate(PutRequest request) {
+    private void validate(String name, ComponentTemplate template) {
+        validate(name, template.template(), Collections.emptyList());
+    }
+
+    private void validate(String name, IndexTemplateV2 template) {
+        validate(name, template.template(), template.indexPatterns());
+    }
+
+    private void validate(String name, Template template, List<String> indexPatterns) {
+        Optional<Template> maybeTemplate = Optional.ofNullable(template);
+        validate(name,
+            maybeTemplate.map(Template::settings).orElse(Settings.EMPTY),
+            indexPatterns,
+            maybeTemplate.map(Template::aliases)
+                .orElse(Collections.emptyMap())
+                .values().stream()
+                .map(MetadataIndexTemplateService::toAlias)
+                .collect(Collectors.toList()));
+    }
+
+    private static Alias toAlias(AliasMetadata aliasMeta) {
+        Alias a = new Alias(aliasMeta.alias());
+        if (aliasMeta.filter() != null) {
+            a.filter(aliasMeta.filter().string());
+        }
+        a.searchRouting(aliasMeta.searchRouting());
+        a.indexRouting(aliasMeta.indexRouting());
+        a.isHidden(aliasMeta.isHidden());
+        a.writeIndex(aliasMeta.writeIndex());
+        return a;
+    }
+
+    private void validate(PutRequest putRequest) {
+        validate(putRequest.name, putRequest.settings, putRequest.indexPatterns, putRequest.aliases);
+    }
+
+    private void validate(String name, @Nullable Settings settings, List<String> indexPatterns, List<Alias> aliases) {
         List<String> validationErrors = new ArrayList<>();
-        if (request.name.contains(" ")) {
+        if (name.contains(" ")) {
             validationErrors.add("name must not contain a space");
         }
-        if (request.name.contains(",")) {
+        if (name.contains(",")) {
             validationErrors.add("name must not contain a ','");
         }
-        if (request.name.contains("#")) {
+        if (name.contains("#")) {
             validationErrors.add("name must not contain a '#'");
         }
-        if (request.name.startsWith("_")) {
+        if (name.contains("*")) {
+            validationErrors.add("name must not contain a '*'");
+        }
+        if (name.startsWith("_")) {
             validationErrors.add("name must not start with '_'");
         }
-        if (!request.name.toLowerCase(Locale.ROOT).equals(request.name)) {
+        if (name.toLowerCase(Locale.ROOT).equals(name) == false) {
             validationErrors.add("name must be lower cased");
         }
-        for(String indexPattern : request.indexPatterns) {
+        for(String indexPattern : indexPatterns) {
             if (indexPattern.contains(" ")) {
-                validationErrors.add("template must not contain a space");
+                validationErrors.add("index_patterns [" + indexPattern + "] must not contain a space");
             }
             if (indexPattern.contains(",")) {
-                validationErrors.add("template must not contain a ','");
+                validationErrors.add("index_pattern [" + indexPattern + "] must not contain a ','");
             }
             if (indexPattern.contains("#")) {
-                validationErrors.add("template must not contain a '#'");
+                validationErrors.add("index_pattern [" + indexPattern + "] must not contain a '#'");
+            }
+            if (indexPattern.contains(":")) {
+                validationErrors.add("index_pattern [" + indexPattern + "] must not contain a ':'");
             }
             if (indexPattern.startsWith("_")) {
-                validationErrors.add("template must not start with '_'");
+                validationErrors.add("index_pattern [" + indexPattern + "] must not start with '_'");
             }
-            if (!Strings.validFileNameExcludingAstrix(indexPattern)) {
-                validationErrors.add("template must not contain the following characters " + Strings.INVALID_FILENAME_CHARS);
+            if (Strings.validFileNameExcludingAstrix(indexPattern) == false) {
+                validationErrors.add("index_pattern [" + indexPattern + "] must not contain the following characters " +
+                    Strings.INVALID_FILENAME_CHARS);
             }
         }
 
-        try {
-            indexScopedSettings.validate(request.settings, true); // templates must be consistent with regards to dependencies
-        } catch (IllegalArgumentException iae) {
-            validationErrors.add(iae.getMessage());
-            for (Throwable t : iae.getSuppressed()) {
-                validationErrors.add(t.getMessage());
+
+        if (settings != null) {
+            try {
+                // templates must be consistent with regards to dependencies
+                indexScopedSettings.validate(settings, true);
+            } catch (IllegalArgumentException iae) {
+                validationErrors.add(iae.getMessage());
+                for (Throwable t : iae.getSuppressed()) {
+                    validationErrors.add(t.getMessage());
+                }
             }
+            List<String> indexSettingsValidation = metadataCreateIndexService.getIndexSettingsValidationErrors(settings, true);
+            validationErrors.addAll(indexSettingsValidation);
         }
-        List<String> indexSettingsValidation = metadataCreateIndexService.getIndexSettingsValidationErrors(request.settings, true);
-        validationErrors.addAll(indexSettingsValidation);
 
-        if (request.indexPatterns.stream().anyMatch(Regex::isMatchAllPattern)) {
-            if (IndexMetadata.INDEX_HIDDEN_SETTING.exists(request.settings)) {
+        if (indexPatterns.stream().anyMatch(Regex::isMatchAllPattern)) {
+            if (settings != null && IndexMetadata.INDEX_HIDDEN_SETTING.exists(settings)) {
                 validationErrors.add("global templates may not specify the setting " + IndexMetadata.INDEX_HIDDEN_SETTING.getKey());
             }
         }
 
-        if (!validationErrors.isEmpty()) {
+        if (validationErrors.size() > 0) {
             ValidationException validationException = new ValidationException();
             validationException.addValidationErrors(validationErrors);
-            throw new InvalidIndexTemplateException(request.name, validationException.getMessage());
+            throw new InvalidIndexTemplateException(name, validationException.getMessage());
         }
 
-        for (Alias alias : request.aliases) {
-            //we validate the alias only partially, as we don't know yet to which index it'll get applied to
+        for (Alias alias : aliases) {
+            // we validate the alias only partially, as we don't know yet to which index it'll get applied to
             aliasValidator.validateAliasStandalone(alias);
-            if (request.indexPatterns.contains(alias.name())) {
-                throw new IllegalArgumentException("Alias [" + alias.name() +
-                    "] cannot be the same as any pattern in [" + String.join(", ", request.indexPatterns) + "]");
+            if (indexPatterns.contains(alias.name())) {
+                throw new IllegalArgumentException("alias [" + alias.name() +
+                    "] cannot be the same as any pattern in [" + String.join(", ", indexPatterns) + "]");
             }
         }
     }

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

@@ -145,7 +145,7 @@ public void testIndexTemplateValidationAccumulatesValidationErrors() {
         assertEquals(throwables.size(), 1);
         assertThat(throwables.get(0), instanceOf(InvalidIndexTemplateException.class));
         assertThat(throwables.get(0).getMessage(), containsString("name must not contain a space"));
-        assertThat(throwables.get(0).getMessage(), containsString("template must not start with '_'"));
+        assertThat(throwables.get(0).getMessage(), containsString("index_pattern [_test_shards*] must not start with '_'"));
         assertThat(throwables.get(0).getMessage(),
                 containsString("Failed to parse value [0] for setting [index.number_of_shards] must be >= 1"));
     }
@@ -158,7 +158,7 @@ public void testIndexTemplateWithAliasNameEqualToTemplatePattern() {
         List<Throwable> errors = putTemplate(xContentRegistry(), request);
         assertThat(errors.size(), equalTo(1));
         assertThat(errors.get(0), instanceOf(IllegalArgumentException.class));
-        assertThat(errors.get(0).getMessage(), equalTo("Alias [foobar] cannot be the same as any pattern in [foo, foobar]"));
+        assertThat(errors.get(0).getMessage(), equalTo("alias [foobar] cannot be the same as any pattern in [foo, foobar]"));
     }
 
     public void testIndexTemplateWithValidateMapping() throws Exception {
@@ -309,17 +309,17 @@ public void testUpdateComponentTemplateWithIndexHiddenSetting() throws Exception
         IndexTemplateV2 firstGlobalIndexTemplate =
             new IndexTemplateV2(org.elasticsearch.common.collect.List.of("*"), template, org.elasticsearch.common.collect.List.of("foo"),
                 1L, null, null);
-        state = metadataIndexTemplateService.addIndexTemplateV2(state, true, "globalIndexTemplate1", firstGlobalIndexTemplate);
+        state = metadataIndexTemplateService.addIndexTemplateV2(state, true, "globalindextemplate1", firstGlobalIndexTemplate);
 
         IndexTemplateV2 secondGlobalIndexTemplate =
             new IndexTemplateV2(org.elasticsearch.common.collect.List.of("*"), template, org.elasticsearch.common.collect.List.of("foo"),
                 2L, null, null);
-        state = metadataIndexTemplateService.addIndexTemplateV2(state, true, "globalIndexTemplate2", secondGlobalIndexTemplate);
+        state = metadataIndexTemplateService.addIndexTemplateV2(state, true, "globalindextemplate2", secondGlobalIndexTemplate);
 
         IndexTemplateV2 fooPatternIndexTemplate =
             new IndexTemplateV2(org.elasticsearch.common.collect.List.of("foo-*"), template, org.elasticsearch.common.collect.List.of(
                 "foo"), 3L, null, null);
-        state = metadataIndexTemplateService.addIndexTemplateV2(state, true, "fooPatternIndexTemplate", fooPatternIndexTemplate);
+        state = metadataIndexTemplateService.addIndexTemplateV2(state, true, "foopatternindextemplate", fooPatternIndexTemplate);
 
         // update the component template to set the index.hidden setting
         Template templateWithIndexHiddenSetting = new Template(Settings.builder().put(IndexMetadata.SETTING_INDEX_HIDDEN, true).build(),
@@ -330,9 +330,9 @@ public void testUpdateComponentTemplateWithIndexHiddenSetting() throws Exception
             fail("expecting an exception as updating the component template would yield the global templates to include the index.hidden " +
                 "setting");
         } catch (IllegalArgumentException e) {
-            assertThat(e.getMessage(), containsStringIgnoringCase("globalIndexTemplate1"));
-            assertThat(e.getMessage(), containsStringIgnoringCase("globalIndexTemplate2"));
-            assertThat(e.getMessage(), not(containsStringIgnoringCase("fooPatternIndexTemplate")));
+            assertThat(e.getMessage(), containsStringIgnoringCase("globalindextemplate1"));
+            assertThat(e.getMessage(), containsStringIgnoringCase("globalindextemplate2"));
+            assertThat(e.getMessage(), not(containsStringIgnoringCase("foopatternindextemplate")));
         }
     }
 

server/src/test/java/org/elasticsearch/cluster/shards/ClusterShardLimitIT.java

@@ -121,7 +121,7 @@ public void testIndexCreationOverLimitFromTemplate() {
 
         assertAcked(client().admin()
             .indices()
-            .preparePutTemplate("should-fail*")
+            .preparePutTemplate("should-fail")
             .setPatterns(Collections.singletonList("should-fail"))
             .setOrder(1)
             .setSettings(Settings.builder()