Fix composable templates with subobjects: false

docs/changelog/97317.yaml

@@ -0,0 +1,6 @@
+pr: 97317
+summary: "Fix merges of mappings with `subobjects: false` for composable index templates"
+area: Mapping
+type: bug
+issues:
+ - 96768

modules/data-streams/src/internalClusterTest/java/org/elasticsearch/datastreams/DataStreamIT.java

@@ -514,7 +514,7 @@ public void testTimeStampValidationInvalidFieldMapping() throws Exception {
             {
                   "properties": {
                     "@timestamp": {
-                      "type": "keyword"
+                      "type": "long"
                     }
                   }
                 }""";
@@ -538,7 +538,7 @@ public void testTimeStampValidationInvalidFieldMapping() throws Exception {
         );
         assertThat(
             e.getCause().getCause().getMessage(),
-            equalTo("data stream timestamp field [@timestamp] is of type [keyword], but [date,date_nanos] is expected")
+            equalTo("data stream timestamp field [@timestamp] is of type [long], but [date,date_nanos] is expected")
         );
     }
 
@@ -1988,18 +1988,13 @@ public void testPartitionedTemplate() throws IOException {
         ).actionGet();
 
         /**
-         * routing enable with allow custom routing false
+         * routing settings with allow custom routing false
          */
         template = new ComposableIndexTemplate(
             List.of("logs"),
             new Template(
                 Settings.builder().put("index.number_of_shards", "3").put("index.routing_partition_size", "2").build(),
-                new CompressedXContent("""
-                    {
-                          "_routing": {
-                            "required": true
-                          }
-                        }"""),
+                null,
                 null
             ),
             null,
@@ -2024,6 +2019,36 @@ public void testPartitionedTemplate() throws IOException {
         );
     }
 
+    public void testRoutingEnabledInMappingDisabledInDataStreamTemplate() throws IOException {
+        ComposableIndexTemplate template = new ComposableIndexTemplate(
+            List.of("logs"),
+            new Template(
+                Settings.builder().put("index.number_of_shards", "3").put("index.routing_partition_size", "2").build(),
+                new CompressedXContent("""
+                    {
+                          "_routing": {
+                            "required": true
+                          }
+                        }"""),
+                null
+            ),
+            null,
+            null,
+            null,
+            null,
+            new ComposableIndexTemplate.DataStreamTemplate(false, false)
+        );
+        Exception e = expectThrows(
+            IllegalArgumentException.class,
+            () -> client().execute(
+                PutComposableIndexTemplateAction.INSTANCE,
+                new PutComposableIndexTemplateAction.Request("my-it").indexTemplate(template)
+            ).actionGet()
+        );
+        Exception actualException = (Exception) e.getCause();
+        assertTrue(Throwables.getRootCause(actualException).getMessage().contains("contradicting `_routing.required` settings"));
+    }
+
     public void testSearchWithRouting() throws IOException, ExecutionException, InterruptedException {
         /**
          * partition size with routing required

modules/data-streams/src/main/java/org/elasticsearch/datastreams/DataStreamIndexSettingsProvider.java

@@ -158,10 +158,7 @@ private List<String> findRoutingPaths(String indexName, Settings allSettings, Li
         tmpIndexMetadata.settings(finalResolvedSettings);
         // Create MapperService just to extract keyword dimension fields:
         try (var mapperService = mapperServiceFactory.apply(tmpIndexMetadata.build())) {
-            for (var mapping : combinedTemplateMappings) {
-                mapperService.merge(MapperService.SINGLE_MAPPING_NAME, mapping, MapperService.MergeReason.INDEX_TEMPLATE);
-            }
-
+            mapperService.merge(MapperService.SINGLE_MAPPING_NAME, combinedTemplateMappings, MapperService.MergeReason.INDEX_TEMPLATE);
             List<String> routingPaths = new ArrayList<>();
             for (var fieldMapper : mapperService.documentMapper().mappers().fieldMappers()) {
                 extractPath(routingPaths, fieldMapper);

rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.put_index_template/15_composition.yml

@@ -349,3 +349,146 @@
           index_patterns: ["purple-index"]
           composed_of: ["red", "blue"]
           ignore_missing_component_templates: ["foo"]
+
+---
+"Composable index templates that include subobjects: false at root":
+  - skip:
+      version: ' - 8.10.1'
+      reason: 'https://github.com/elastic/elasticsearch/issues/96768 fixed after 8.10.0'
+      features: allowed_warnings
+
+  - do:
+      cluster.put_component_template:
+        name: test-subobjects
+        body:
+          template:
+            mappings:
+              subobjects: false
+
+  - do:
+      cluster.put_component_template:
+        name: test-field
+        body:
+          template:
+            mappings:
+              properties:
+                parent.subfield:
+                  type: keyword
+
+  - do:
+      allowed_warnings:
+        - "index template [test-composable-template] has index patterns [test-*] matching patterns from existing older templates [global] with patterns (global => [*]); this template [test-composable-template] will take precedence during new index creation"
+      indices.put_index_template:
+        name: test-composable-template
+        body:
+          index_patterns:
+            - test-*
+          composed_of:
+            - test-subobjects
+            - test-field
+  - is_true: acknowledged
+
+  - do:
+      indices.create:
+        index: test-generic
+
+  - do:
+      indices.get_mapping:
+        index: test-generic
+  - match: { test-generic.mappings.properties.parent\.subfield.type: "keyword" }
+
+---
+"Composable index templates that include subobjects: false on arbitrary field":
+  - skip:
+      version: ' - 8.10.1'
+      reason: 'https://github.com/elastic/elasticsearch/issues/96768 fixed after 8.10.0'
+      features: allowed_warnings
+
+  - do:
+      cluster.put_component_template:
+        name: test-subobjects
+        body:
+          template:
+            mappings:
+              properties:
+                parent:
+                  type: object
+                  subobjects: false
+
+  - do:
+      cluster.put_component_template:
+        name: test-subfield
+        body:
+          template:
+            mappings:
+              properties:
+                parent:
+                  properties:
+                    child.grandchild:
+                      type: keyword
+
+  - do:
+      allowed_warnings:
+        - "index template [test-composable-template] has index patterns [test-*] matching patterns from existing older templates [global] with patterns (global => [*]); this template [test-composable-template] will take precedence during new index creation"
+      indices.put_index_template:
+        name: test-composable-template
+        body:
+          index_patterns:
+            - test-*
+          composed_of:
+            - test-subobjects
+            - test-subfield
+  - is_true: acknowledged
+
+  - do:
+      indices.create:
+        index: test-generic
+
+  - do:
+      indices.get_mapping:
+        index: test-generic
+  - match: { test-generic.mappings.properties.parent.properties.child\.grandchild.type: "keyword" }
+
+---
+"Composition of component templates with different legal field mappings":
+  - skip:
+      features: allowed_warnings
+
+  - do:
+      cluster.put_component_template:
+        name: mapping
+        body:
+          template:
+            mappings:
+              properties:
+                field:
+                  type: long
+                  coerce: true
+
+  - do:
+      allowed_warnings:
+        - "index template [test-composable-template] has index patterns [test-*] matching patterns from existing older templates [global] with patterns (global => [*]); this template [test-composable-template] will take precedence during new index creation"
+      indices.put_index_template:
+        name: test-composable-template
+        body:
+          index_patterns:
+            - test-*
+          composed_of:
+            - mapping
+          template:
+            mappings:
+              properties:
+                field:
+                  type: keyword
+                  ignore_above: 1024
+  - is_true: acknowledged
+
+  - do:
+      indices.create:
+        index: test-generic
+
+  - do:
+      indices.get_mapping:
+        index: test-generic
+  - match: { test-generic.mappings.properties.field.type: "keyword" }
+  - match: { test-generic.mappings.properties.field.ignore_above: 1024 }

rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/15_timestamp_mapping.yml

@@ -148,7 +148,7 @@ reject @timestamp with wrong type:
       reason: introduced in 8.1.0
 
   - do:
-      catch: /data stream timestamp field \[@timestamp\] is of type \[keyword\], but \[date,date_nanos\] is expected/
+      catch: /data stream timestamp field \[@timestamp\] is of type \[long\], but \[date,date_nanos\] is expected/
       indices.create:
           index: test
           body:
@@ -163,7 +163,7 @@ reject @timestamp with wrong type:
             mappings:
               properties:
                 "@timestamp":
-                  type: keyword
+                  type: long
 
 ---
 reject timestamp meta field with wrong type:

server/src/main/java/org/elasticsearch/action/admin/indices/template/post/TransportSimulateIndexTemplateAction.java

@@ -294,9 +294,7 @@ public static Template resolveTemplate(
             indexMetadata,
             tempIndexService -> {
                 MapperService mapperService = tempIndexService.mapperService();
-                for (CompressedXContent mapping : mappings) {
-                    mapperService.merge(MapperService.SINGLE_MAPPING_NAME, mapping, MapperService.MergeReason.INDEX_TEMPLATE);
-                }
+                mapperService.merge(MapperService.SINGLE_MAPPING_NAME, mappings, MapperService.MergeReason.INDEX_TEMPLATE);
 
                 DocumentMapper documentMapper = mapperService.documentMapper();
                 return documentMapper != null ? documentMapper.mappingSource() : null;

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

@@ -1331,13 +1331,14 @@ private static void updateIndexMappingsAndBuildSortOrder(
     ) throws IOException {
         MapperService mapperService = indexService.mapperService();
         IndexMode indexMode = indexService.getIndexSettings() != null ? indexService.getIndexSettings().getMode() : IndexMode.STANDARD;
+        List<CompressedXContent> allMappings = new ArrayList<>();
         final CompressedXContent defaultMapping = indexMode.getDefaultMapping();
         if (defaultMapping != null) {
-            mapperService.merge(MapperService.SINGLE_MAPPING_NAME, defaultMapping, MergeReason.INDEX_TEMPLATE);
-        }
-        for (CompressedXContent mapping : mappings) {
-            mapperService.merge(MapperService.SINGLE_MAPPING_NAME, mapping, MergeReason.INDEX_TEMPLATE);
+            allMappings.add(defaultMapping);
         }
+        allMappings.addAll(mappings);
+        mapperService.merge(MapperService.SINGLE_MAPPING_NAME, allMappings, MergeReason.INDEX_TEMPLATE);
+
         indexMode.validateTimestampFieldMapping(request.dataStreamName() != null, mapperService.mappingLookup());
 
         if (sourceMetadata == null) {

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

@@ -84,7 +84,7 @@
 public class MetadataIndexTemplateService {
 
     public static final String DEFAULT_TIMESTAMP_FIELD = "@timestamp";
-    public static final CompressedXContent DEFAULT_TIMESTAMP_MAPPING;
+    public static final CompressedXContent DEFAULT_TIMESTAMP_MAPPING_WITHOUT_ROUTING;
 
     private static final CompressedXContent DEFAULT_TIMESTAMP_MAPPING_WITH_ROUTING;
 
@@ -96,8 +96,14 @@ public class MetadataIndexTemplateService {
             Map.of("type", DateFieldMapper.CONTENT_TYPE, "ignore_malformed", "false")
         );
         try {
-            DEFAULT_TIMESTAMP_MAPPING = new CompressedXContent(
+            DEFAULT_TIMESTAMP_MAPPING_WITHOUT_ROUTING = new CompressedXContent(
                 (builder, params) -> builder.startObject(MapperService.SINGLE_MAPPING_NAME)
+                    // adding explicit "_routing": {"required": false}, even though this is the default, because this snippet is used
+                    // later for resolving a RoutingFieldMapper, where we need this information to validate that does not conflict with
+                    // any mapping.
+                    .startObject(RoutingFieldMapper.NAME)
+                    .field("required", false)
+                    .endObject()
                     .field("properties", defaultTimestampField)
                     .endObject()
             );
@@ -1303,7 +1309,7 @@ public static List<CompressedXContent> collectMappings(
             if (template.getDataStreamTemplate().isAllowCustomRouting()) {
                 mappings.add(0, DEFAULT_TIMESTAMP_MAPPING_WITH_ROUTING);
             } else {
-                mappings.add(0, DEFAULT_TIMESTAMP_MAPPING);
+                mappings.add(0, DEFAULT_TIMESTAMP_MAPPING_WITHOUT_ROUTING);
             }
         }
 
@@ -1599,9 +1605,7 @@ private static void validateCompositeTemplate(
             List<CompressedXContent> mappings = collectMappings(stateWithIndex, templateName, indexName);
             try {
                 MapperService mapperService = tempIndexService.mapperService();
-                for (CompressedXContent mapping : mappings) {
-                    mapperService.merge(MapperService.SINGLE_MAPPING_NAME, mapping, MapperService.MergeReason.INDEX_TEMPLATE);
-                }
+                mapperService.merge(MapperService.SINGLE_MAPPING_NAME, mappings, MapperService.MergeReason.INDEX_TEMPLATE);
 
                 if (template.getDataStreamTemplate() != null) {
                     validateTimestampFieldMapping(mapperService.mappingLookup());