Prevent constructing index template without patterns (#27662)

Today, we prevent the system from storing a broken index template in the
transport layer, however we don't prevent this in XContent. A broken
index template can break the whole cluster state.

This commit attempts to prevent the system from constructing an index
template without a proper index patterns.

core/src/main/java/org/elasticsearch/cluster/metadata/IndexTemplateMetaData.java

@@ -93,6 +93,9 @@ public class IndexTemplateMetaData extends AbstractDiffable<IndexTemplateMetaDat
                                  ImmutableOpenMap<String, CompressedXContent> mappings,
                                  ImmutableOpenMap<String, AliasMetaData> aliases,
                                  ImmutableOpenMap<String, IndexMetaData.Custom> customs) {
+        if (patterns == null || patterns.isEmpty()) {
+            throw new IllegalArgumentException("Index patterns must not be null or empty; got " + patterns);
+        }
         this.name = name;
         this.order = order;
         this.version = version;
@@ -244,7 +247,7 @@ public class IndexTemplateMetaData extends AbstractDiffable<IndexTemplateMetaDat
         if (out.getVersion().onOrAfter(Version.V_6_0_0_alpha1)) {
             out.writeStringList(patterns);
         } else {
-            out.writeString(patterns.size() > 0 ? patterns.get(0) : "");
+            out.writeString(patterns.get(0));
         }
         Settings.writeSettingsToStream(settings, out);
         out.writeVInt(mappings.size());

core/src/test/java/org/elasticsearch/cluster/metadata/IndexTemplateMetaDataTests.java

@@ -21,6 +21,7 @@ package org.elasticsearch.cluster.metadata;
 import org.elasticsearch.Version;
 import org.elasticsearch.common.bytes.BytesArray;
 import org.elasticsearch.common.bytes.BytesReference;
+import org.elasticsearch.common.collect.ImmutableOpenMap;
 import org.elasticsearch.common.io.stream.BytesStreamOutput;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.settings.Settings;
@@ -120,4 +121,45 @@ public class IndexTemplateMetaDataTests extends ESTestCase {
         assertThat(indexTemplateMetaData, equalTo(indexTemplateMetaDataRoundTrip));
     }
 
+    public void testValidateInvalidIndexPatterns() throws Exception {
+        final IllegalArgumentException emptyPatternError = expectThrows(IllegalArgumentException.class, () -> {
+            new IndexTemplateMetaData(randomRealisticUnicodeOfLengthBetween(5, 10), randomInt(), randomInt(),
+                Collections.emptyList(), Settings.EMPTY, ImmutableOpenMap.of(), ImmutableOpenMap.of(), ImmutableOpenMap.of());
+        });
+        assertThat(emptyPatternError.getMessage(), equalTo("Index patterns must not be null or empty; got []"));
+
+        final IllegalArgumentException nullPatternError = expectThrows(IllegalArgumentException.class, () -> {
+            new IndexTemplateMetaData(randomRealisticUnicodeOfLengthBetween(5, 10), randomInt(), randomInt(),
+                null, Settings.EMPTY, ImmutableOpenMap.of(), ImmutableOpenMap.of(), ImmutableOpenMap.of());
+        });
+        assertThat(nullPatternError.getMessage(), equalTo("Index patterns must not be null or empty; got null"));
+
+        final String templateWithEmptyPattern = "{\"index_patterns\" : [],\"order\" : 1000," +
+            "\"settings\" : {\"number_of_shards\" : 10,\"number_of_replicas\" : 1}," +
+            "\"mappings\" : {\"doc\" :" +
+            "{\"properties\":{\"" +
+            randomAlphaOfLength(10) + "\":{\"type\":\"text\"},\"" +
+            randomAlphaOfLength(10) + "\":{\"type\":\"keyword\"}}" +
+            "}}}";
+        try (XContentParser parser =
+                 XContentHelper.createParser(NamedXContentRegistry.EMPTY, new BytesArray(templateWithEmptyPattern), XContentType.JSON)) {
+            final IllegalArgumentException ex = expectThrows(IllegalArgumentException.class,
+                () -> IndexTemplateMetaData.Builder.fromXContent(parser, randomAlphaOfLengthBetween(1, 100)));
+            assertThat(ex.getMessage(), equalTo("Index patterns must not be null or empty; got []"));
+        }
+
+        final String templateWithoutPattern = "{\"order\" : 1000," +
+            "\"settings\" : {\"number_of_shards\" : 10,\"number_of_replicas\" : 1}," +
+            "\"mappings\" : {\"doc\" :" +
+            "{\"properties\":{\"" +
+            randomAlphaOfLength(10) + "\":{\"type\":\"text\"},\"" +
+            randomAlphaOfLength(10) + "\":{\"type\":\"keyword\"}}" +
+            "}}}";
+        try (XContentParser parser =
+                 XContentHelper.createParser(NamedXContentRegistry.EMPTY, new BytesArray(templateWithoutPattern), XContentType.JSON)) {
+            final IllegalArgumentException ex = expectThrows(IllegalArgumentException.class,
+                () -> IndexTemplateMetaData.Builder.fromXContent(parser, randomAlphaOfLengthBetween(1, 100)));
+            assertThat(ex.getMessage(), equalTo("Index patterns must not be null or empty; got null"));
+        }
+    }
 }

core/src/test/java/org/elasticsearch/cluster/metadata/TemplateUpgradeServiceTests.java

@@ -54,6 +54,8 @@ import java.util.Set;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicReference;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
 
 import static java.util.Collections.emptyMap;
 import static org.elasticsearch.test.VersionUtils.randomVersion;
@@ -61,9 +63,7 @@ import static org.hamcrest.CoreMatchers.nullValue;
 import static org.hamcrest.CoreMatchers.startsWith;
 import static org.hamcrest.Matchers.empty;
 import static org.hamcrest.Matchers.equalTo;
-import static org.hamcrest.Matchers.greaterThanOrEqualTo;
 import static org.hamcrest.Matchers.hasSize;
-import static org.hamcrest.Matchers.lessThan;
 import static org.hamcrest.Matchers.notNullValue;
 import static org.mockito.Matchers.any;
 import static org.mockito.Mockito.doAnswer;
@@ -83,16 +83,17 @@ public class TemplateUpgradeServiceTests extends ESTestCase {
         boolean shouldChange = randomBoolean();
 
         MetaData metaData = randomMetaData(
-            IndexTemplateMetaData.builder("user_template").build(),
+            IndexTemplateMetaData.builder("user_template").patterns(randomIndexPatterns()).build(),
-            IndexTemplateMetaData.builder("removed_test_template").build(),
+            IndexTemplateMetaData.builder("removed_test_template").patterns(randomIndexPatterns()).build(),
-            IndexTemplateMetaData.builder("changed_test_template").build()
+            IndexTemplateMetaData.builder("changed_test_template").patterns(randomIndexPatterns()).build()
         );
 
         TemplateUpgradeService service = new TemplateUpgradeService(Settings.EMPTY, null, clusterService, null,
             Arrays.asList(
                 templates -> {
                     if (shouldAdd) {
-                        assertNull(templates.put("added_test_template", IndexTemplateMetaData.builder("added_test_template").build()));
+                        assertNull(templates.put("added_test_template",
+                            IndexTemplateMetaData.builder("added_test_template").patterns(randomIndexPatterns()).build()));
                     }
                     return templates;
                 },
@@ -105,7 +106,7 @@ public class TemplateUpgradeServiceTests extends ESTestCase {
                 templates -> {
                     if (shouldChange) {
                         assertNotNull(templates.put("changed_test_template",
-                            IndexTemplateMetaData.builder("changed_test_template").order(10).build()));
+                            IndexTemplateMetaData.builder("changed_test_template").patterns(randomIndexPatterns()).order(10).build()));
                     }
                     return templates;
                 }
@@ -234,9 +235,9 @@ public class TemplateUpgradeServiceTests extends ESTestCase {
         AtomicInteger updateInvocation = new AtomicInteger();
 
         MetaData metaData = randomMetaData(
-            IndexTemplateMetaData.builder("user_template").build(),
+            IndexTemplateMetaData.builder("user_template").patterns(randomIndexPatterns()).build(),
-            IndexTemplateMetaData.builder("removed_test_template").build(),
+            IndexTemplateMetaData.builder("removed_test_template").patterns(randomIndexPatterns()).build(),
-            IndexTemplateMetaData.builder("changed_test_template").build()
+            IndexTemplateMetaData.builder("changed_test_template").patterns(randomIndexPatterns()).build()
         );
 
         ThreadPool threadPool = mock(ThreadPool.class);
@@ -390,4 +391,10 @@ public class TemplateUpgradeServiceTests extends ESTestCase {
         }
         return builder.build();
     }
+
+    List<String> randomIndexPatterns() {
+        return IntStream.range(0, between(1, 10))
+            .mapToObj(n -> randomUnicodeOfCodepointLengthBetween(1, 100))
+            .collect(Collectors.toList());
+    }
 }

core/src/test/java/org/elasticsearch/cluster/serialization/ClusterSerializationTests.java

@@ -46,6 +46,7 @@ import org.elasticsearch.test.VersionUtils;
 
 import java.io.IOException;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
 
 import static org.hamcrest.Matchers.equalTo;
@@ -170,8 +171,8 @@ public class ClusterSerializationTests extends ESAllocationTestCase {
     public void testObjectReuseWhenApplyingClusterStateDiff() throws Exception {
         IndexMetaData indexMetaData
             = IndexMetaData.builder("test").settings(settings(Version.CURRENT)).numberOfShards(10).numberOfReplicas(1).build();
-        IndexTemplateMetaData indexTemplateMetaData
+        IndexTemplateMetaData indexTemplateMetaData = IndexTemplateMetaData.builder("test-template")
-            = IndexTemplateMetaData.builder("test-template").patterns(new ArrayList<>()).build();
+            .patterns(Arrays.asList(generateRandomStringArray(10, 100, false, false))).build();
         MetaData metaData = MetaData.builder().put(indexMetaData, true).put(indexTemplateMetaData).build();
 
         RoutingTable routingTable = RoutingTable.builder().addAsNew(metaData.index("test")).build();

core/src/test/java/org/elasticsearch/cluster/serialization/ClusterStateToStringTests.java

@@ -32,6 +32,8 @@ import org.elasticsearch.cluster.routing.allocation.AllocationService;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.settings.Settings;
 
+import java.util.Arrays;
+
 import static java.util.Collections.emptyMap;
 import static java.util.Collections.emptySet;
 import static org.hamcrest.Matchers.containsString;
@@ -40,7 +42,8 @@ public class ClusterStateToStringTests extends ESAllocationTestCase {
     public void testClusterStateSerialization() throws Exception {
         MetaData metaData = MetaData.builder()
                 .put(IndexMetaData.builder("test_idx").settings(settings(Version.CURRENT)).numberOfShards(10).numberOfReplicas(1))
-                .put(IndexTemplateMetaData.builder("test_template").build())
+                .put(IndexTemplateMetaData.builder("test_template")
+                    .patterns(Arrays.asList(generateRandomStringArray(10, 100, false,false))).build())
                 .build();
 
         RoutingTable routingTable = RoutingTable.builder()

core/src/test/java/org/elasticsearch/gateway/GatewayMetaStateTests.java

@@ -308,7 +308,8 @@ public class GatewayMetaStateTests extends ESAllocationTestCase {
             Collections.emptyList(),
             Collections.singletonList(
                 templates -> {
-                    templates.put("added_test_template", IndexTemplateMetaData.builder("added_test_template").build());
+                    templates.put("added_test_template", IndexTemplateMetaData.builder("added_test_template")
+                        .patterns(Arrays.asList(generateRandomStringArray(10, 100, false, false))).build());
                     return templates;
                 }
             ));
@@ -438,14 +439,17 @@ public class GatewayMetaStateTests extends ESAllocationTestCase {
             Collections.emptyList(),
             Arrays.asList(
                 indexTemplateMetaDatas -> {
-                    indexTemplateMetaDatas.put("template1", IndexTemplateMetaData.builder("template1").settings(
+                    indexTemplateMetaDatas.put("template1", IndexTemplateMetaData.builder("template1")
-                        Settings.builder().put(IndexMetaData.INDEX_NUMBER_OF_SHARDS_SETTING.getKey(), 20).build()).build());
+                        .patterns(Arrays.asList(generateRandomStringArray(10, 100, false, false)))
+                        .settings(Settings.builder().put(IndexMetaData.INDEX_NUMBER_OF_SHARDS_SETTING.getKey(), 20).build())
+                        .build());
                     return indexTemplateMetaDatas;
 
                 },
                 indexTemplateMetaDatas -> {
-                    indexTemplateMetaDatas.put("template2", IndexTemplateMetaData.builder("template2").settings(
+                    indexTemplateMetaDatas.put("template2", IndexTemplateMetaData.builder("template2")
-                        Settings.builder().put(IndexMetaData.INDEX_NUMBER_OF_REPLICAS_SETTING.getKey(), 10).build()).build());
+                        .patterns(Arrays.asList(generateRandomStringArray(10, 100, false, false)))
+                        .settings(Settings.builder().put(IndexMetaData.INDEX_NUMBER_OF_REPLICAS_SETTING.getKey(), 10).build()).build());
                     return indexTemplateMetaDatas;
 
                 }
@@ -535,6 +539,7 @@ public class GatewayMetaStateTests extends ESAllocationTestCase {
                 .settings(settings(Version.CURRENT)
                     .put(IndexMetaData.INDEX_NUMBER_OF_REPLICAS_SETTING.getKey(), randomIntBetween(0, 3))
                     .put(IndexMetaData.INDEX_NUMBER_OF_SHARDS_SETTING.getKey(), randomIntBetween(1, 5)))
+                .patterns(Arrays.asList(generateRandomStringArray(10, 100, false, false)))
                 .build();
             builder.put(templateMetaData);
         }