From 8fa14b333dc965fab6d7d967c3afb7b948b5dcaa Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Mon, 4 May 2020 16:19:13 -0600 Subject: [PATCH] [7.x] Validate non-negative priorities for V2 index templates (#56139) (#56163) Backports the following commits to 7.x: - Validate non-negative priorities for V2 index templates (#56139) --- .../indices/index-templates.asciidoc | 3 +- .../put/PutIndexTemplateV2Action.java | 3 ++ .../cluster/metadata/IndexTemplateV2.java | 7 ++++ .../MetadataIndexTemplateService.java | 13 +++---- .../rest/action/cat/RestTemplatesAction.java | 2 +- .../put/PutIndexTemplateV2RequestTests.java | 12 ++++++ .../MetadataIndexTemplateServiceTests.java | 38 +++++++++++++------ 7 files changed, 57 insertions(+), 21 deletions(-) diff --git a/docs/reference/indices/index-templates.asciidoc b/docs/reference/indices/index-templates.asciidoc index fc45c24e47a..59d37804a98 100644 --- a/docs/reference/indices/index-templates.asciidoc +++ b/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`:: diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/template/put/PutIndexTemplateV2Action.java b/server/src/main/java/org/elasticsearch/action/admin/indices/template/put/PutIndexTemplateV2Action.java index 3cceeebac7f..53e0ee7898f 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/template/put/PutIndexTemplateV2Action.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/template/put/PutIndexTemplateV2Action.java @@ -103,6 +103,9 @@ public class PutIndexTemplateV2Action extends ActionType { ); } } + if (indexTemplate.priority() != null && indexTemplate.priority() < 0) { + validationException = addValidationError("index template priority must be >= 0", validationException); + } } return validationException; } diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexTemplateV2.java b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexTemplateV2.java index b77366c8ce1..bf4279d5ab7 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexTemplateV2.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexTemplateV2.java @@ -131,6 +131,13 @@ public class IndexTemplateV2 extends AbstractDiffable implement return priority; } + public long priorityOrZero() { + if (priority == null) { + return 0L; + } + return priority; + } + public Long version() { return version; } diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateService.java b/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateService.java index 17a41de9d40..2d425f1e015 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateService.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateService.java @@ -339,7 +339,7 @@ public class MetadataIndexTemplateService { } Map> 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> findConflictingV2Templates(final ClusterState state, final String candidateName, final List 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> findConflictingV2Templates(final ClusterState state, final String candidateName, - final List indexPatterns, boolean checkPriority, Long priority) { + final List indexPatterns, boolean checkPriority, long priority) { Automaton v1automaton = Regex.simpleMatchToAutomaton(indexPatterns.toArray(Strings.EMPTY_ARRAY)); Map> overlappingTemplates = new HashMap<>(); for (Map.Entry 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()); @@ -716,8 +716,7 @@ public class MetadataIndexTemplateService { } final List 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); diff --git a/server/src/main/java/org/elasticsearch/rest/action/cat/RestTemplatesAction.java b/server/src/main/java/org/elasticsearch/rest/action/cat/RestTemplatesAction.java index 75b1fcfbc1e..5cfd7e2aad4 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/cat/RestTemplatesAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/cat/RestTemplatesAction.java @@ -110,7 +110,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(); diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/template/put/PutIndexTemplateV2RequestTests.java b/server/src/test/java/org/elasticsearch/action/admin/indices/template/put/PutIndexTemplateV2RequestTests.java index f89244325a4..e50f62d18bc 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/indices/template/put/PutIndexTemplateV2RequestTests.java +++ b/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; @@ -80,4 +81,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 validationErrors = validationException.validationErrors(); + assertThat(validationErrors.size(), is(1)); + String error = validationErrors.get(0); + assertThat(error, is("index template priority must be >= 0")); + } } diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateServiceTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateServiceTests.java index 891f601e8af..ca67d3cbc3e 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateServiceTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateServiceTests.java @@ -415,7 +415,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 { @@ -525,7 +525,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 @@ -565,7 +565,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 @@ -580,15 +580,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 {