Validate non-negative priorities for V2 index templates (#56139)

This also fixes an issue where a `null` priority was treated as below a 0 priority. `null` is now
treated as 0 priority when it comes to comparing V2 templates.

Relates to #53101

docs/reference/indices/index-templates.asciidoc

@@ -212,7 +212,8 @@ specified, meaning that the last component template specified has the highest pr
 `priority`::
 (Optional, integer)
 Priority to determine index template precedence when a new index is created. The index template with
-the highest priority is chosen.
+the highest priority is chosen. If no priority is specified the template is treated as though it is
+of priority 0 (lowest priority).
 This number is not automatically generated by {es}.
 
 `version`::

server/src/main/java/org/elasticsearch/action/admin/indices/template/put/PutIndexTemplateV2Action.java

@@ -103,6 +103,9 @@ public class PutIndexTemplateV2Action extends ActionType<AcknowledgedResponse> {
                         );
                     }
                 }
+                if (indexTemplate.priority() != null && indexTemplate.priority() < 0) {
+                    validationException = addValidationError("index template priority must be >= 0", validationException);
+                }
             }
             return validationException;
         }

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

@@ -130,6 +130,13 @@ public class IndexTemplateV2 extends AbstractDiffable<IndexTemplateV2> implement
         return priority;
     }
 
+    public long priorityOrZero() {
+        if (priority == null) {
+            return 0L;
+        }
+        return priority;
+    }
+
     public Long version() {
         return version;
     }

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

@@ -339,7 +339,7 @@ public class MetadataIndexTemplateService {
         }
 
         Map<String, List<String>> overlaps = findConflictingV2Templates(currentState, name, template.indexPatterns(), true,
-            template.priority());
+            template.priorityOrZero());
         overlaps.remove(name);
         if (overlaps.size() > 0) {
             String error = String.format(Locale.ROOT, "index template [%s] has index patterns %s matching patterns from " +
@@ -351,7 +351,7 @@ public class MetadataIndexTemplateService {
                 overlaps.entrySet().stream()
                     .map(e -> e.getKey() + " => " + e.getValue())
                     .collect(Collectors.joining(",")),
-                template.priority());
+                template.priorityOrZero());
             throw new IllegalArgumentException(error);
         }
 
@@ -435,7 +435,7 @@ public class MetadataIndexTemplateService {
      */
     public static Map<String, List<String>> findConflictingV2Templates(final ClusterState state, final String candidateName,
                                                                        final List<String> indexPatterns) {
-        return findConflictingV2Templates(state, candidateName, indexPatterns, false, null);
+        return findConflictingV2Templates(state, candidateName, indexPatterns, false, 0L);
     }
 
     /**
@@ -449,7 +449,7 @@ public class MetadataIndexTemplateService {
      * index templates with the same priority).
      */
     static Map<String, List<String>> findConflictingV2Templates(final ClusterState state, final String candidateName,
-                                                                final List<String> indexPatterns, boolean checkPriority, Long priority) {
+                                                                final List<String> indexPatterns, boolean checkPriority, long priority) {
         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()) {
@@ -457,7 +457,7 @@ public class MetadataIndexTemplateService {
             IndexTemplateV2 template = entry.getValue();
             Automaton v2automaton = Regex.simpleMatchToAutomaton(template.indexPatterns().toArray(Strings.EMPTY_ARRAY));
             if (Operations.isEmpty(Operations.intersection(v1automaton, v2automaton)) == false) {
-                if (checkPriority == false || Objects.equals(priority, template.priority())) {
+                if (checkPriority == false || priority == template.priorityOrZero()) {
                     logger.debug("old template {} and index template {} would overlap: {} <=> {}",
                         candidateName, name, indexPatterns, template.indexPatterns());
                     overlappingTemplates.put(name, template.indexPatterns());
@@ -733,8 +733,7 @@ public class MetadataIndexTemplateService {
         }
 
         final List<IndexTemplateV2> candidates = new ArrayList<>(matchedTemplates.keySet());
-        CollectionUtil.timSort(candidates, Comparator.comparing(IndexTemplateV2::priority,
-            Comparator.nullsLast(Comparator.reverseOrder())));
+        CollectionUtil.timSort(candidates, Comparator.comparing(IndexTemplateV2::priorityOrZero, Comparator.reverseOrder()));
 
         assert candidates.size() > 0 : "we should have returned early with no candidates";
         IndexTemplateV2 winner = candidates.get(0);

server/src/main/java/org/elasticsearch/rest/action/cat/RestTemplatesAction.java

@@ -108,7 +108,7 @@ public class RestTemplatesAction extends AbstractCatAction {
                 table.startRow();
                 table.addCell(name);
                 table.addCell("[" + String.join(", ", template.indexPatterns()) + "]");
-                table.addCell(template.priority());
+                table.addCell(template.priorityOrZero());
                 table.addCell(template.version());
                 table.addCell("[" + String.join(", ", template.composedOf()) + "]");
                 table.endRow();

server/src/test/java/org/elasticsearch/action/admin/indices/template/put/PutIndexTemplateV2RequestTests.java

@@ -29,6 +29,7 @@ import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.test.AbstractWireSerializingTestCase;
 
 import java.io.IOException;
+import java.util.Arrays;
 import java.util.List;
 
 import static org.hamcrest.Matchers.is;
@@ -79,4 +80,15 @@ public class PutIndexTemplateV2RequestTests extends AbstractWireSerializingTestC
         String error = validationErrors.get(0);
         assertThat(error, is("an index template is required"));
     }
+
+    public void testValidationOfPriority() {
+        PutIndexTemplateV2Action.Request req = new PutIndexTemplateV2Action.Request("test");
+        req.indexTemplate(new IndexTemplateV2(Arrays.asList("foo", "bar"), null, null, -5L, null, null));
+        ActionRequestValidationException validationException = req.validate();
+        assertThat(validationException, is(notNullValue()));
+        List<String> validationErrors = validationException.validationErrors();
+        assertThat(validationErrors.size(), is(1));
+        String error = validationErrors.get(0);
+        assertThat(error, is("index template priority must be >= 0"));
+    }
 }

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

@@ -413,7 +413,7 @@ public class MetadataIndexTemplateServiceTests extends ESSingleNodeTestCase {
             "take precedence during new index creation");
 
         assertNotNull(state.metadata().templatesV2().get("v2-template"));
-        assertThat(state.metadata().templatesV2().get("v2-template"), equalTo(v2Template));
+        assertTemplatesEqual(state.metadata().templatesV2().get("v2-template"), v2Template);
     }
 
     public void testPutGlobalV2TemplateWhichResolvesIndexHiddenSetting() throws Exception {
@@ -527,7 +527,7 @@ public class MetadataIndexTemplateServiceTests extends ESSingleNodeTestCase {
             "take precedence during new index creation");
 
         assertNotNull(state.metadata().templatesV2().get("v2-template"));
-        assertThat(state.metadata().templatesV2().get("v2-template"), equalTo(v2Template));
+        assertTemplatesEqual(state.metadata().templatesV2().get("v2-template"), v2Template);
 
         // Now try to update the existing v1-template
 
@@ -567,7 +567,7 @@ public class MetadataIndexTemplateServiceTests extends ESSingleNodeTestCase {
             "take precedence during new index creation");
 
         assertNotNull(state.metadata().templatesV2().get("v2-template"));
-        assertThat(state.metadata().templatesV2().get("v2-template"), equalTo(v2Template));
+        assertTemplatesEqual(state.metadata().templatesV2().get("v2-template"), v2Template);
 
         // Now try to update the existing v1-template
 
@@ -583,15 +583,29 @@ public class MetadataIndexTemplateServiceTests extends ESSingleNodeTestCase {
     }
 
     public void testPuttingOverlappingV2Template() throws Exception {
-        IndexTemplateV2 template = new IndexTemplateV2(Arrays.asList("egg*", "baz"), null, null, 1L, null, null);
-        MetadataIndexTemplateService metadataIndexTemplateService = getMetadataIndexTemplateService();
-        ClusterState state = metadataIndexTemplateService.addIndexTemplateV2(ClusterState.EMPTY_STATE, false, "foo", template);
-        IndexTemplateV2 newTemplate = new IndexTemplateV2(Arrays.asList("abc", "baz*"), null, null, 1L, null, null);
-        IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
-            () -> metadataIndexTemplateService.addIndexTemplateV2(state, false, "foo2", newTemplate));
-        assertThat(e.getMessage(), equalTo("index template [foo2] has index patterns [abc, baz*] matching patterns from existing " +
-            "templates [foo] with patterns (foo => [egg*, baz]) that have the same priority [1], multiple index templates may not " +
-            "match during index creation, please use a different priority"));
+        {
+            IndexTemplateV2 template = new IndexTemplateV2(Arrays.asList("egg*", "baz"), null, null, 1L, null, null);
+            MetadataIndexTemplateService metadataIndexTemplateService = getMetadataIndexTemplateService();
+            ClusterState state = metadataIndexTemplateService.addIndexTemplateV2(ClusterState.EMPTY_STATE, false, "foo", template);
+            IndexTemplateV2 newTemplate = new IndexTemplateV2(Arrays.asList("abc", "baz*"), null, null, 1L, null, null);
+            IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
+                () -> metadataIndexTemplateService.addIndexTemplateV2(state, false, "foo2", newTemplate));
+            assertThat(e.getMessage(), equalTo("index template [foo2] has index patterns [abc, baz*] matching patterns from existing " +
+                "templates [foo] with patterns (foo => [egg*, baz]) that have the same priority [1], multiple index templates may not " +
+                "match during index creation, please use a different priority"));
+        }
+
+        {
+            IndexTemplateV2 template = new IndexTemplateV2(Arrays.asList("egg*", "baz"), null, null, null, null, null);
+            MetadataIndexTemplateService metadataIndexTemplateService = getMetadataIndexTemplateService();
+            ClusterState state = metadataIndexTemplateService.addIndexTemplateV2(ClusterState.EMPTY_STATE, false, "foo", template);
+            IndexTemplateV2 newTemplate = new IndexTemplateV2(Arrays.asList("abc", "baz*"), null, null, 0L, null, null);
+            IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
+                () -> metadataIndexTemplateService.addIndexTemplateV2(state, false, "foo2", newTemplate));
+            assertThat(e.getMessage(), equalTo("index template [foo2] has index patterns [abc, baz*] matching patterns from existing " +
+                "templates [foo] with patterns (foo => [egg*, baz]) that have the same priority [0], multiple index templates may not " +
+                "match during index creation, please use a different priority"));
+        }
     }
 
     public void testFindV2Templates() throws Exception {