Add warnings/errors when V2 templates would match same indices… (#54367)

* Add warnings/errors when V2 templates would match same indices as V1

With the introduction of V2 index templates, we want to warn users that templates they put in place
might not take precedence (because v2 templates are going to "win"). This adds this validation at
`PUT` time for both V1 and V2 templates with the following rules:

** When creating or updating a V2 template
- If the v2 template would match indices for an existing v1 template or templates, provide a
warning (through the deprecation logging so it shows up to the client) as well as logging the
warning

The v2 warning looks like:

```
index template [my-v2-template] has index patterns [foo-*] matching patterns from existing older
templates [old-v1-template,match-all-template] with patterns (old-v1-template =>
[foo*],match-all-template => [*]); this template [my-v2-template] will take
precedence during new index creation
```

** When creating a V1 template
- If the v1 template is for index patterns of `"*"` and a v2 template exists, warn that the v2
template may take precedence
- If the v1 template is for index patterns other than all indices, and a v2 template exists that
would match, throw an error preventing creation of the v1 template

** When updating a V1 template (without changing its existing `index_patterns`!)
- If the v1 template is for index patterns that would match an existing v2 template, warn that the
v2 template may take precedence.

The v1 warning looks like:

```
template [my-v1-template] has index patterns [*] matching patterns from existing index templates
[existing-v2-template] with patterns (existing-v2-template => [foo*]); this template [my-v1-template] may be ignored in favor of an index template at index creation time
```

And the v1 error looks like:

```
template [my-v1-template] has index patterns [foo*] matching patterns from existing index templates
[existing-v2-template] with patterns (existing-v2-template => [f*]), use index templates (/_index_template) instead
```

Relates to #53101

* Remove v2 index and component templates when cleaning up tests

* Finish half-finished comment sentence

* Guard template removal and ignore for earlier versions of ES

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

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

@@ -19,9 +19,12 @@
 package org.elasticsearch.cluster.metadata;
 
 import com.carrotsearch.hppc.cursors.ObjectCursor;
+import com.carrotsearch.hppc.cursors.ObjectObjectCursor;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.apache.lucene.util.CollectionUtil;
+import org.apache.lucene.util.automaton.Automaton;
+import org.apache.lucene.util.automaton.Operations;
 import org.elasticsearch.Version;
 import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.action.admin.indices.alias.Alias;
@@ -37,6 +40,7 @@ import org.elasticsearch.common.UUIDs;
 import org.elasticsearch.common.ValidationException;
 import org.elasticsearch.common.compress.CompressedXContent;
 import org.elasticsearch.common.inject.Inject;
+import org.elasticsearch.common.logging.DeprecationLogger;
 import org.elasticsearch.common.regex.Regex;
 import org.elasticsearch.common.settings.IndexScopedSettings;
 import org.elasticsearch.common.settings.Settings;
@@ -54,12 +58,15 @@ import org.elasticsearch.indices.InvalidIndexTemplateException;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Comparator;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Locale;
+import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
 import java.util.function.Predicate;
+import java.util.stream.Collectors;
 
 import static org.elasticsearch.indices.cluster.IndicesClusterStateService.AllocatedIndices.IndexRemovalReason.NO_LONGER_ASSIGNED;
 
@@ -69,6 +76,7 @@ import static org.elasticsearch.indices.cluster.IndicesClusterStateService.Alloc
 public class MetaDataIndexTemplateService {
 
     private static final Logger logger = LogManager.getLogger(MetaDataIndexTemplateService.class);
+    private static final DeprecationLogger deprecationLogger = new DeprecationLogger(logger);
 
     private final ClusterService clusterService;
     private final AliasValidator aliasValidator;
@@ -272,6 +280,21 @@ public class MetaDataIndexTemplateService {
             throw new IllegalArgumentException("index template [" + name + "] already exists");
         }
 
+        Map<String, List<String>> overlaps = findConflictingV1Templates(currentState, name, template.indexPatterns());
+        if (overlaps.size() > 0) {
+            String warning = String.format(Locale.ROOT, "index template [%s] has index patterns %s matching patterns from " +
+                    "existing older templates [%s] with patterns (%s); this template [%s] will take precedence during new index creation",
+                name,
+                template.indexPatterns(),
+                Strings.collectionToCommaDelimitedString(overlaps.keySet()),
+                overlaps.entrySet().stream()
+                    .map(e -> e.getKey() + " => " + e.getValue())
+                    .collect(Collectors.joining(",")),
+                name);
+            logger.warn(warning);
+            deprecationLogger.deprecated(warning);
+        }
+
         // TODO: validation of index template
         // validateAndAddTemplate(request, templateBuilder, indicesService, xContentRegistry);
 
@@ -281,6 +304,48 @@ public class MetaDataIndexTemplateService {
             .build();
     }
 
+    /**
+     * Return a map of v1 template names to their index patterns for v1 templates that would overlap
+     * with the given v2 template's index patterns.
+     */
+    static Map<String, List<String>> findConflictingV1Templates(final ClusterState state, final String candidateName,
+                                                                final List<String> indexPatterns) {
+        Automaton v2automaton = Regex.simpleMatchToAutomaton(indexPatterns.toArray(Strings.EMPTY_ARRAY));
+        Map<String, List<String>> overlappingTemplates = new HashMap<>();
+        for (ObjectObjectCursor<String, IndexTemplateMetaData> cursor : state.metaData().templates()) {
+            String name = cursor.key;
+            IndexTemplateMetaData template = cursor.value;
+            Automaton v1automaton = Regex.simpleMatchToAutomaton(template.patterns().toArray(Strings.EMPTY_ARRAY));
+            if (Operations.isEmpty(Operations.intersection(v2automaton, v1automaton)) == false) {
+                logger.debug("index template {} and old template {} would overlap: {} <=> {}",
+                    candidateName, name, indexPatterns, template.patterns());
+                overlappingTemplates.put(name, template.patterns());
+            }
+        }
+        return overlappingTemplates;
+    }
+
+    /**
+     * Return a map of v2 template names to their index patterns for v2 templates that would overlap
+     * with the given v1 template's index patterns.
+     */
+    static Map<String, List<String>> findConflictingV2Templates(final ClusterState state, final String candidateName,
+                                                                final List<String> indexPatterns) {
+        Automaton v1automaton = Regex.simpleMatchToAutomaton(indexPatterns.toArray(Strings.EMPTY_ARRAY));
+        Map<String, List<String>> overlappingTemplates = new HashMap<>();
+        for (Map.Entry<String, IndexTemplateV2> entry : state.metaData().templatesV2().entrySet()) {
+            String name = entry.getKey();
+            IndexTemplateV2 template = entry.getValue();
+            Automaton v2automaton = Regex.simpleMatchToAutomaton(template.indexPatterns().toArray(Strings.EMPTY_ARRAY));
+            if (Operations.isEmpty(Operations.intersection(v1automaton, v2automaton)) == false) {
+                logger.debug("old template {} and index template {} would overlap: {} <=> {}",
+                    candidateName, name, indexPatterns, template.indexPatterns());
+                overlappingTemplates.put(name, template.indexPatterns());
+            }
+        }
+        return overlappingTemplates;
+    }
+
     /**
      * Remove the given index template from the cluster state. The index template name
      * supports simple regex wildcards for removing multiple index templates at a time.
@@ -374,36 +439,8 @@ public class MetaDataIndexTemplateService {
 
             @Override
             public ClusterState execute(ClusterState currentState) throws Exception {
-                if (request.create && currentState.metaData().templates().containsKey(request.name)) {
-                    throw new IllegalArgumentException("index_template [" + request.name + "] already exists");
-                }
-
-                templateBuilder.order(request.order);
-                templateBuilder.version(request.version);
-                templateBuilder.patterns(request.indexPatterns);
-                templateBuilder.settings(request.settings);
-
-                if (request.mappings != null) {
-                    try {
-                        templateBuilder.putMapping(MapperService.SINGLE_MAPPING_NAME, request.mappings);
-                    } catch (Exception e) {
-                        throw new MapperParsingException("Failed to parse mapping: {}", e, request.mappings);
-                    }
-                }
-
                 validateTemplate(request.settings, request.mappings, indicesService, xContentRegistry);
-
-                for (Alias alias : request.aliases) {
-                    AliasMetaData aliasMetaData = AliasMetaData.builder(alias.name()).filter(alias.filter())
-                        .indexRouting(alias.indexRouting()).searchRouting(alias.searchRouting()).build();
-                    templateBuilder.putAlias(aliasMetaData);
-                }
-                IndexTemplateMetaData template = templateBuilder.build();
-
-                MetaData.Builder builder = MetaData.builder(currentState.metaData()).put(template);
-
-                logger.info("adding template [{}] for index patterns {}", request.name, request.indexPatterns);
-                return ClusterState.builder(currentState).metaData(builder).build();
+                return innerPutTemplate(currentState, request, templateBuilder);
             }
 
             @Override
@@ -413,6 +450,75 @@ public class MetaDataIndexTemplateService {
         });
     }
 
+    // Package visible for testing
+    static ClusterState innerPutTemplate(final ClusterState currentState, PutRequest request,
+                                         IndexTemplateMetaData.Builder templateBuilder) {
+        // Flag for whether this is updating an existing template or adding a new one
+        // TODO: in 8.0+, only allow updating index templates, not adding new ones
+        boolean isUpdate = currentState.metaData().templates().containsKey(request.name);
+        if (request.create && isUpdate) {
+            throw new IllegalArgumentException("index_template [" + request.name + "] already exists");
+        }
+        boolean isUpdateAndPatternsAreUnchanged = isUpdate &&
+            currentState.metaData().templates().get(request.name).patterns().equals(request.indexPatterns);
+
+        Map<String, List<String>> overlaps = findConflictingV2Templates(currentState, request.name, request.indexPatterns);
+        if (overlaps.size() > 0) {
+            // Be less strict (just a warning) if we're updating an existing template or this is a match-all template
+            if (isUpdateAndPatternsAreUnchanged || request.indexPatterns.stream().anyMatch(Regex::isMatchAllPattern)) {
+                String warning = String.format(Locale.ROOT, "template [%s] has index patterns %s matching patterns" +
+                        " from existing index templates [%s] with patterns (%s); this template [%s] may be ignored in favor of " +
+                        "an index template at index creation time",
+                    request.name,
+                    request.indexPatterns,
+                    Strings.collectionToCommaDelimitedString(overlaps.keySet()),
+                    overlaps.entrySet().stream()
+                        .map(e -> e.getKey() + " => " + e.getValue())
+                        .collect(Collectors.joining(",")),
+                    request.name);
+                logger.warn(warning);
+                deprecationLogger.deprecated(warning);
+            } else {
+                // Otherwise, this is a hard error, the user should use V2 index templates instead
+                String error = String.format(Locale.ROOT, "template [%s] has index patterns %s matching patterns" +
+                        " from existing index templates [%s] with patterns (%s), use index templates (/_index_template) instead",
+                    request.name,
+                    request.indexPatterns,
+                    Strings.collectionToCommaDelimitedString(overlaps.keySet()),
+                    overlaps.entrySet().stream()
+                        .map(e -> e.getKey() + " => " + e.getValue())
+                        .collect(Collectors.joining(",")));
+                logger.error(error);
+                throw new IllegalArgumentException(error);
+            }
+        }
+
+        templateBuilder.order(request.order);
+        templateBuilder.version(request.version);
+        templateBuilder.patterns(request.indexPatterns);
+        templateBuilder.settings(request.settings);
+
+        if (request.mappings != null) {
+            try {
+                templateBuilder.putMapping(MapperService.SINGLE_MAPPING_NAME, request.mappings);
+            } catch (Exception e) {
+                throw new MapperParsingException("Failed to parse mapping: {}", e, request.mappings);
+            }
+        }
+
+        for (Alias alias : request.aliases) {
+            AliasMetaData aliasMetaData = AliasMetaData.builder(alias.name()).filter(alias.filter())
+                .indexRouting(alias.indexRouting()).searchRouting(alias.searchRouting()).build();
+            templateBuilder.putAlias(aliasMetaData);
+        }
+        IndexTemplateMetaData template = templateBuilder.build();
+
+        MetaData.Builder builder = MetaData.builder(currentState.metaData()).put(template);
+
+        logger.info("adding template [{}] for index patterns {}", request.name, request.indexPatterns);
+        return ClusterState.builder(currentState).metaData(builder).build();
+    }
+
     /**
      * Finds index templates whose index pattern matched with the given index name. In the case of
      * hidden indices, a template with a match all pattern or global template will not be returned.
@@ -474,6 +580,15 @@ public class MetaDataIndexTemplateService {
 
     private static void validateTemplate(Settings settings, String mappings,
                                          IndicesService indicesService, NamedXContentRegistry xContentRegistry) throws Exception {
+        // First check to see if mappings are valid XContent
+        if (mappings != null) {
+            try {
+                new CompressedXContent(mappings);
+            } catch (Exception e) {
+                throw new MapperParsingException("Failed to parse mapping: {}", e, mappings);
+            }
+        }
+
         Index createdIndex = null;
         final String temporaryIndexName = UUIDs.randomBase64UUID();
         try {

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

@@ -316,6 +316,147 @@ public class MetaDataIndexTemplateServiceTests extends ESSingleNodeTestCase {
         assertNull(updatedState.metaData().templatesV2().get("foo"));
     }
 
+    /**
+     * Test that if we have a pre-existing v1 template and put a v2 template that would match the same indices, we generate a warning
+     */
+    public void testPuttingV2TemplateGeneratesWarning() {
+        IndexTemplateMetaData v1Template = IndexTemplateMetaData.builder("v1-template")
+            .patterns(Arrays.asList("fo*", "baz"))
+            .build();
+
+        ClusterState state = ClusterState.builder(ClusterState.EMPTY_STATE)
+            .metaData(MetaData.builder(MetaData.EMPTY_META_DATA)
+                .put(v1Template)
+                .build())
+            .build();
+
+        IndexTemplateV2 v2Template = new IndexTemplateV2(Arrays.asList("foo-bar-*", "eggplant"), null, null, null, null, null);
+        state = MetaDataIndexTemplateService.addIndexTemplateV2(state, false, "v2-template", v2Template);
+
+        assertWarnings("index template [v2-template] has index patterns [foo-bar-*, eggplant] matching patterns " +
+            "from existing older templates [v1-template] with patterns (v1-template => [fo*, baz]); this template [v2-template] will " +
+            "take precedence during new index creation");
+
+        assertNotNull(state.metaData().templatesV2().get("v2-template"));
+        assertThat(state.metaData().templatesV2().get("v2-template"), equalTo(v2Template));
+    }
+
+    /**
+     * Test that if we have a pre-existing v2 template and put a "*" v1 template, we generate a warning
+     */
+    public void testPuttingV1StarTemplateGeneratesWarning() {
+        IndexTemplateV2 v2Template = new IndexTemplateV2(Arrays.asList("foo-bar-*", "eggplant"), null, null, null, null, null);
+        ClusterState state = MetaDataIndexTemplateService.addIndexTemplateV2(ClusterState.EMPTY_STATE, false, "v2-template", v2Template);
+
+        MetaDataIndexTemplateService.PutRequest req = new MetaDataIndexTemplateService.PutRequest("cause", "v1-template");
+        req.patterns(Arrays.asList("*", "baz"));
+        state = MetaDataIndexTemplateService.innerPutTemplate(state, req, IndexTemplateMetaData.builder("v1-template"));
+
+        assertWarnings("template [v1-template] has index patterns [*, baz] matching patterns from existing " +
+            "index templates [v2-template] with patterns (v2-template => [foo-bar-*, eggplant]); this template [v1-template] may " +
+            "be ignored in favor of an index template at index creation time");
+
+        assertNotNull(state.metaData().templates().get("v1-template"));
+        assertThat(state.metaData().templates().get("v1-template").patterns(), containsInAnyOrder("*", "baz"));
+    }
+
+    /**
+     * Test that if we have a pre-existing v2 template and put a v1 template that would match the same indices, we generate a hard error
+     */
+    public void testPuttingV1NonStarTemplateGeneratesError() {
+        IndexTemplateV2 v2Template = new IndexTemplateV2(Arrays.asList("foo-bar-*", "eggplant"), null, null, null, null, null);
+        ClusterState state = MetaDataIndexTemplateService.addIndexTemplateV2(ClusterState.EMPTY_STATE, false, "v2-template", v2Template);
+
+        MetaDataIndexTemplateService.PutRequest req = new MetaDataIndexTemplateService.PutRequest("cause", "v1-template");
+        req.patterns(Arrays.asList("egg*", "baz"));
+        IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
+            () -> MetaDataIndexTemplateService.innerPutTemplate(state, req, IndexTemplateMetaData.builder("v1-template")));
+
+        assertThat(e.getMessage(),
+            equalTo("template [v1-template] has index patterns [egg*, baz] matching patterns from existing index " +
+                "templates [v2-template] with patterns (v2-template => [foo-bar-*, eggplant]), use index templates " +
+                "(/_index_template) instead"));
+
+        assertNull(state.metaData().templates().get("v1-template"));
+    }
+
+    /**
+     * Test that if we have a pre-existing v1 and v2 template, and we update the existing v1
+     * template without changing its index patterns, a warning is generated
+     */
+    public void testUpdatingV1NonStarTemplateWithUnchangedPatternsGeneratesWarning() {
+        IndexTemplateMetaData v1Template = IndexTemplateMetaData.builder("v1-template")
+            .patterns(Arrays.asList("fo*", "baz"))
+            .build();
+
+        ClusterState state = ClusterState.builder(ClusterState.EMPTY_STATE)
+            .metaData(MetaData.builder(MetaData.EMPTY_META_DATA)
+                .put(v1Template)
+                .build())
+            .build();
+
+        IndexTemplateV2 v2Template = new IndexTemplateV2(Arrays.asList("foo-bar-*", "eggplant"), null, null, null, null, null);
+        state = MetaDataIndexTemplateService.addIndexTemplateV2(state, false, "v2-template", v2Template);
+
+        assertWarnings("index template [v2-template] has index patterns [foo-bar-*, eggplant] matching patterns " +
+            "from existing older templates [v1-template] with patterns (v1-template => [fo*, baz]); this template [v2-template] will " +
+            "take precedence during new index creation");
+
+        assertNotNull(state.metaData().templatesV2().get("v2-template"));
+        assertThat(state.metaData().templatesV2().get("v2-template"), equalTo(v2Template));
+
+        // Now try to update the existing v1-template
+
+        MetaDataIndexTemplateService.PutRequest req = new MetaDataIndexTemplateService.PutRequest("cause", "v1-template");
+        req.patterns(Arrays.asList("fo*", "baz"));
+        state = MetaDataIndexTemplateService.innerPutTemplate(state, req, IndexTemplateMetaData.builder("v1-template"));
+
+        assertWarnings("template [v1-template] has index patterns [fo*, baz] matching patterns from existing " +
+            "index templates [v2-template] with patterns (v2-template => [foo-bar-*, eggplant]); this template [v1-template] may " +
+            "be ignored in favor of an index template at index creation time");
+
+        assertNotNull(state.metaData().templates().get("v1-template"));
+        assertThat(state.metaData().templates().get("v1-template").patterns(), containsInAnyOrder("fo*", "baz"));
+    }
+
+    /**
+     * Test that if we have a pre-existing v1 and v2 template, and we update the existing v1
+     * template *AND* change the index patterns that an error is generated
+     */
+    public void testUpdatingV1NonStarWithChangedPatternsTemplateGeneratesError() {
+        IndexTemplateMetaData v1Template = IndexTemplateMetaData.builder("v1-template")
+            .patterns(Arrays.asList("fo*", "baz"))
+            .build();
+
+        ClusterState state = ClusterState.builder(ClusterState.EMPTY_STATE)
+            .metaData(MetaData.builder(MetaData.EMPTY_META_DATA)
+                .put(v1Template)
+                .build())
+            .build();
+
+        IndexTemplateV2 v2Template = new IndexTemplateV2(Arrays.asList("foo-bar-*", "eggplant"), null, null, null, null, null);
+        state = MetaDataIndexTemplateService.addIndexTemplateV2(state, false, "v2-template", v2Template);
+
+        assertWarnings("index template [v2-template] has index patterns [foo-bar-*, eggplant] matching patterns " +
+            "from existing older templates [v1-template] with patterns (v1-template => [fo*, baz]); this template [v2-template] will " +
+            "take precedence during new index creation");
+
+        assertNotNull(state.metaData().templatesV2().get("v2-template"));
+        assertThat(state.metaData().templatesV2().get("v2-template"), equalTo(v2Template));
+
+        // Now try to update the existing v1-template
+
+        MetaDataIndexTemplateService.PutRequest req = new MetaDataIndexTemplateService.PutRequest("cause", "v1-template");
+        req.patterns(Arrays.asList("egg*", "baz"));
+        final ClusterState finalState = state;
+        IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
+            () -> MetaDataIndexTemplateService.innerPutTemplate(finalState, req, IndexTemplateMetaData.builder("v1-template")));
+
+        assertThat(e.getMessage(), equalTo("template [v1-template] has index patterns [egg*, baz] matching patterns " +
+            "from existing index templates [v2-template] with patterns (v2-template => [foo-bar-*, eggplant]), use index " +
+            "templates (/_index_template) instead"));
+    }
+
     private static List<Throwable> putTemplate(NamedXContentRegistry xContentRegistry, PutRequest request) {
         MetaDataCreateIndexService createIndexService = new MetaDataCreateIndexService(
                 Settings.EMPTY,

test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java

@@ -549,9 +549,29 @@ public abstract class ESRestTestCase extends ESTestCase {
                         adminClient().performRequest(new Request("DELETE", "_template/" + template));
                     }
                 }
+                try {
+                    adminClient().performRequest(new Request("DELETE", "_index_template/*"));
+                    adminClient().performRequest(new Request("DELETE", "_component_template/*"));
+                } catch (ResponseException e) {
+                    if (e.getResponse().getStatusLine().getStatusCode() == 405) {
+                        // We hit a version of ES that doesn't support index templates v2 yet, so it's safe to ignore
+                    } else {
+                        throw e;
+                    }
+                }
             } else {
                 logger.debug("Clearing all templates");
                 adminClient().performRequest(new Request("DELETE", "_template/*"));
+                try {
+                    adminClient().performRequest(new Request("DELETE", "_index_template/*"));
+                    adminClient().performRequest(new Request("DELETE", "_component_template/*"));
+                } catch (ResponseException e) {
+                    if (e.getResponse().getStatusLine().getStatusCode() == 405) {
+                        // We hit a version of ES that doesn't support index templates v2 yet, so it's safe to ignore
+                    } else {
+                        throw e;
+                    }
+                }
             }
         }