[7.8] Throw exception on duplicate mappings metadata fields (#57840)

This is a backport of #57835 In #57701 we changed mappings merging so that duplicate fields specified in mappings caused an exception during validation. This change makes the same exception thrown when metadata fields are duplicated. This will allow us to be strict currently with plans to make the merging more fine-grained in a later release.
elastic · Jun 8, 2020 · e142d69 · e142d69
1 parent e070789
commit e142d69
Show file tree

Hide file tree

Showing 3 changed files with 8 additions and 11 deletions.
diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataCreateIndexService.java b/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataCreateIndexService.java
@@ -585,9 +585,7 @@ static Map<String, Map<String, Object>> parseV2Mappings(String mappingsJson, Lis
                 Map<String, Object> innerTemplateNonProperties = new HashMap<>(innerTemplateMapping);
                 Map<String, Object> maybeProperties = (Map<String, Object>) innerTemplateNonProperties.remove("properties");
 
-                nonProperties = removeDuplicatedDynamicTemplates(nonProperties, innerTemplateNonProperties);
-                XContentHelper.mergeDefaults(innerTemplateNonProperties, nonProperties);
-                nonProperties = innerTemplateNonProperties;
+                nonProperties = mergeFailingOnReplacement(nonProperties, innerTemplateNonProperties);
 
                 if (maybeProperties != null) {
                     properties = mergeFailingOnReplacement(properties, maybeProperties);
@@ -600,9 +598,7 @@ static Map<String, Map<String, Object>> parseV2Mappings(String mappingsJson, Lis
             Map<String, Object> innerRequestNonProperties = new HashMap<>(innerRequestMappings);
             Map<String, Object> maybeRequestProperties = (Map<String, Object>) innerRequestNonProperties.remove("properties");
 
-            nonProperties = removeDuplicatedDynamicTemplates(nonProperties, innerRequestMappings);
-            XContentHelper.mergeDefaults(innerRequestNonProperties, nonProperties);
-            nonProperties = innerRequestNonProperties;
+            nonProperties = mergeFailingOnReplacement(nonProperties, innerRequestNonProperties);
 
             if (maybeRequestProperties != null) {
                 properties = mergeFailingOnReplacement(properties, maybeRequestProperties);

diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateService.java b/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateService.java
@@ -949,12 +949,11 @@ private static void validateCompositeTemplate(final ClusterState state,
                 // Parse mappings to ensure they are valid after being composed
                 List<CompressedXContent> mappings = resolveMappings(stateWithIndex, templateName);
                 try {
+                    Map<String, Map<String, Object>> finalMappings =
+                        MetadataCreateIndexService.parseV2Mappings("{}", mappings, xContentRegistry);
                     MapperService dummyMapperService = tempIndexService.mapperService();
-                    for (CompressedXContent mapping : mappings) {
-                        // TODO: Eventually change this to:
-                        // dummyMapperService.merge(MapperService.SINGLE_MAPPING_NAME, mapping, MergeReason.INDEX_TEMPLATE);
-                        dummyMapperService.merge(MapperService.SINGLE_MAPPING_NAME, mapping, MergeReason.MAPPING_UPDATE);
-                    }
+                        // TODO: Eventually change this to use MergeReason.INDEX_TEMPLATE
+                        dummyMapperService.merge(finalMappings, MergeReason.MAPPING_UPDATE);
                 } catch (Exception e) {
                     throw new IllegalArgumentException("invalid composite mappings for [" + templateName + "]", e);
                 }

diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataCreateIndexServiceTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataCreateIndexServiceTests.java
@@ -1174,6 +1174,7 @@ public void testDedupTemplateDynamicTemplates() throws Exception {
             dynamicMapping.get("path_match"), is("docker.container.labels.*"));
     }
 
+    @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/pull/57393")
     public void testDedupRequestDynamicTemplates() throws Exception {
         String requestMappingJson = "{\"_doc\":{\"_source\":{\"enabled\": false}, \"dynamic_templates\": [" +
             "{\n" +
@@ -1235,6 +1236,7 @@ public void testDedupRequestDynamicTemplates() throws Exception {
         assertThat(mapping.get("type"), is("keyword"));
     }
 
+    @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/pull/57393")
     public void testMultipleComponentTemplatesDefineSameDynamicTemplate() throws Exception {
         String ct1Mapping = "{\"_doc\":{\"_source\":{\"enabled\": false}, \"dynamic_templates\": [" +
             "{\n" +