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

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

@@ -349,3 +349,37 @@
           index_patterns: ["purple-index"]
           composed_of: ["red", "blue"]
           ignore_missing_component_templates: ["foo"]
+
+---
+"Composable index templates that include subobjects: false":
+  - skip:
+      version: ' - 8.9.1'
+      reason: 'https://github.com/elastic/elasticsearch/issues/96768 fixed after 8.9.0'
+
+  - 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:
+      indices.put_index_template:
+        name: logs-composable-template
+        body:
+          index_patterns:
+            - test-*
+          composed_of:
+            - test-subobjects
+            - test-field

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

@@ -1595,9 +1595,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());

server/src/main/java/org/elasticsearch/index/mapper/MapperService.java

@@ -12,6 +12,7 @@
 import org.elasticsearch.cluster.metadata.IndexMetadata;
 import org.elasticsearch.cluster.metadata.MappingMetadata;
 import org.elasticsearch.cluster.service.ClusterService;
+import org.elasticsearch.common.Explicit;
 import org.elasticsearch.common.compress.CompressedXContent;
 import org.elasticsearch.common.compress.CompressorFactory;
 import org.elasticsearch.common.settings.Setting;
@@ -39,6 +40,7 @@
 import java.io.InputStream;
 import java.util.Collections;
 import java.util.LinkedHashMap;
+import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
@@ -353,24 +355,62 @@ public void merge(IndexMetadata indexMetadata, MergeReason reason) {
         }
     }
 
+    /**
+     * Merging the provided mappings. If the provided mapping list is ordered, merging will maintain the original ordering.
+     */
+    public DocumentMapper merge(String type, List<CompressedXContent> mappingSources, MergeReason reason) {
+        final DocumentMapper currentMapper = this.mapper;
+        if (currentMapper != null && mappingSources.size() == 1 && currentMapper.mappingSource().equals(mappingSources.get(0))) {
+            return currentMapper;
+        }
+
+        List<Map<String, Object>> convertedMappings = new LinkedList<>();
+        Explicit<Boolean> explicitSubobjects = null;
+        for (CompressedXContent mappingSource : mappingSources) {
+            Map<String, Object> mapping = MappingParser.convertToMap(mappingSource);
+            convertedMappings.add(mapping);
+            Explicit<Boolean> tmpSubobjects = ObjectMapper.checkSubobjectsInMappingRoot(mapping);
+            if (tmpSubobjects != null) {
+                if (explicitSubobjects != null && tmpSubobjects.value() != explicitSubobjects.value()) {
+                    throw new MapperParsingException("Failed to parse mappings: contradicting subobjects settings provided");
+                }
+                explicitSubobjects = tmpSubobjects;
+            }
+        }
+
+        DocumentMapper ret = null;
+        for (Map<String, Object> mapping : convertedMappings) {
+            ret = doMerge(type, reason, mapping, explicitSubobjects);
+        }
+        return ret;
+    }
+
     public DocumentMapper merge(String type, CompressedXContent mappingSource, MergeReason reason) {
         final DocumentMapper currentMapper = this.mapper;
         if (currentMapper != null && currentMapper.mappingSource().equals(mappingSource)) {
             return currentMapper;
         }
-        synchronized (this) {
-            Mapping incomingMapping = parseMapping(type, mappingSource);
-            Mapping mapping = mergeMappings(this.mapper, incomingMapping, reason);
-            // TODO: In many cases the source here is equal to mappingSource so we need not serialize again.
-            // We should identify these cases reliably and save expensive serialization here
-            DocumentMapper newMapper = newDocumentMapper(mapping, reason, mapping.toCompressedXContent());
-            if (reason == MergeReason.MAPPING_UPDATE_PREFLIGHT) {
-                return newMapper;
-            }
-            this.mapper = newMapper;
-            assert assertSerialization(newMapper);
+        Map<String, Object> mappingSourceAsMap = MappingParser.convertToMap(mappingSource);
+        return doMerge(type, reason, mappingSourceAsMap, null);
+    }
+
+    private synchronized DocumentMapper doMerge(
+        String type,
+        MergeReason reason,
+        Map<String, Object> mappingSourceAsMap,
+        @Nullable Explicit<Boolean> explicitSubobjects
+    ) {
+        Mapping incomingMapping = parseMapping(type, mappingSourceAsMap, explicitSubobjects);
+        Mapping mapping = mergeMappings(this.mapper, incomingMapping, reason);
+        // TODO: In many cases the source here is equal to mappingSource so we need not serialize again.
+        // We should identify these cases reliably and save expensive serialization here
+        DocumentMapper newMapper = newDocumentMapper(mapping, reason, mapping.toCompressedXContent());
+        if (reason == MergeReason.MAPPING_UPDATE_PREFLIGHT) {
             return newMapper;
         }
+        this.mapper = newMapper;
+        assert assertSerialization(newMapper);
+        return newMapper;
     }
 
     private DocumentMapper newDocumentMapper(Mapping mapping, MergeReason reason, CompressedXContent mappingSource) {
@@ -387,6 +427,22 @@ public Mapping parseMapping(String mappingType, CompressedXContent mappingSource
         }
     }
 
+    /**
+     * A method to parse mapping from a source in a map form, that allows to specify explicit/implicit {@code subobjects} configuration.
+     * Since parsing is affected by the {@code subobjects} setting, the resulted mapping may change according to this setting.
+     * @param mappingType the mapping type
+     * @param mappingSource mapping source already converted to a map form, but not yet processed otherwise
+     * @param explicitSubobjects subobjects configuration to use for this parsing operation
+     * @return a parsed mapping
+     */
+    public Mapping parseMapping(String mappingType, Map<String, Object> mappingSource, @Nullable Explicit<Boolean> explicitSubobjects) {
+        try {
+            return mappingParser.parse(mappingType, mappingSource, explicitSubobjects);
+        } catch (Exception e) {
+            throw new MapperParsingException("Failed to parse mapping: {}", e, e.getMessage());
+        }
+    }
+
     public static Mapping mergeMappings(DocumentMapper currentMapper, Mapping incomingMapping, MergeReason reason) {
         Mapping newMapping;
         if (currentMapper == null) {

server/src/main/java/org/elasticsearch/index/mapper/MappingParser.java

@@ -8,6 +8,7 @@
 
 package org.elasticsearch.index.mapper;
 
+import org.elasticsearch.common.Explicit;
 import org.elasticsearch.common.compress.CompressedXContent;
 import org.elasticsearch.common.xcontent.XContentHelper;
 import org.elasticsearch.core.Nullable;
@@ -73,19 +74,37 @@ private static String getRemainingFields(Map<?, ?> map) {
         return remainingFields.toString();
     }
 
-    @SuppressWarnings("unchecked")
-    Mapping parse(@Nullable String type, CompressedXContent source) throws MapperParsingException {
+    static Map<String, Object> convertToMap(CompressedXContent source) {
         Objects.requireNonNull(source, "source cannot be null");
-        Map<String, Object> mapping = XContentHelper.convertToMap(source.compressedReference(), true, XContentType.JSON).v2();
-        if (mapping.isEmpty()) {
+        return XContentHelper.convertToMap(source.compressedReference(), true, XContentType.JSON).v2();
+    }
+
+    Mapping parse(@Nullable String type, CompressedXContent source) throws MapperParsingException {
+        Map<String, Object> mapping = convertToMap(source);
+        return parse(type, mapping, null);
+    }
+
+    /**
+     * A method to parse mapping from a source in a map form, that allows to specify explicit/implicit {@code subobjects} configuration.
+     * Since parsing is affected by the {@code subobjects} setting, the resulted mapping may change according to this setting.
+     * @param type the mapping type
+     * @param mappingSource mapping source already converted to a map form, but not yet processed otherwise
+     * @param explicitSubobjects subobjects configuration to use for this parsing operation
+     * @return a parsed mapping
+     * @throws MapperParsingException in case of parsing error
+     */
+    @SuppressWarnings("unchecked")
+    Mapping parse(@Nullable String type, Map<String, Object> mappingSource, @Nullable Explicit<Boolean> explicitSubobjects)
+        throws MapperParsingException {
+        if (mappingSource.isEmpty()) {
             if (type == null) {
-                throw new MapperParsingException("malformed mapping, no type name found");
+                throw new MapperParsingException("malformed mappingSource, no type name found");
             }
         } else {
-            String rootName = mapping.keySet().iterator().next();
+            String rootName = mappingSource.keySet().iterator().next();
             if (type == null || type.equals(rootName) || documentTypeResolver.apply(type).equals(rootName)) {
                 type = rootName;
-                mapping = (Map<String, Object>) mapping.get(rootName);
+                mappingSource = (Map<String, Object>) mappingSource.get(rootName);
             }
         }
         if (type == null) {
@@ -94,19 +113,16 @@ Mapping parse(@Nullable String type, CompressedXContent source) throws MapperPar
         if (type.isEmpty()) {
             throw new MapperParsingException("type cannot be an empty string");
         }
-        return parse(type, mapping);
-    }
 
-    private Mapping parse(String type, Map<String, Object> mapping) throws MapperParsingException {
         final MappingParserContext mappingParserContext = mappingParserContextSupplier.get();
 
-        RootObjectMapper.Builder rootObjectMapper = RootObjectMapper.parse(type, mapping, mappingParserContext);
+        RootObjectMapper.Builder rootObjectMapper = RootObjectMapper.parse(type, mappingSource, mappingParserContext, explicitSubobjects);
 
         Map<Class<? extends MetadataFieldMapper>, MetadataFieldMapper> metadataMappers = metadataMappersSupplier.get();
         Map<String, Object> meta = null;
         boolean isSourceSynthetic = mappingParserContext.getIndexSettings().getMode().isSyntheticSourceEnabled();
 
-        Iterator<Map.Entry<String, Object>> iterator = mapping.entrySet().iterator();
+        Iterator<Map.Entry<String, Object>> iterator = mappingSource.entrySet().iterator();
         while (iterator.hasNext()) {
             Map.Entry<String, Object> entry = iterator.next();
             String fieldName = entry.getKey();
@@ -135,7 +151,7 @@ private Mapping parse(String type, Map<String, Object> mapping) throws MapperPar
         }
 
         @SuppressWarnings("unchecked")
-        Map<String, Object> removed = (Map<String, Object>) mapping.remove("_meta");
+        Map<String, Object> removed = (Map<String, Object>) mappingSource.remove("_meta");
         if (removed != null) {
             /*
              * It may not be required to copy meta here to maintain immutability but the cost is pretty low here.
@@ -155,7 +171,7 @@ private Mapping parse(String type, Map<String, Object> mapping) throws MapperPar
         }
         if (mappingParserContext.indexVersionCreated().isLegacyIndexVersion() == false) {
             // legacy indices are allowed to have extra definitions that we ignore (we will drop them on import)
-            checkNoRemainingFields(mapping, "Root mapping definition has unsupported parameters: ");
+            checkNoRemainingFields(mappingSource, "Root mapping definition has unsupported parameters: ");
         }
 
         return new Mapping(

server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java

@@ -14,6 +14,7 @@
 import org.elasticsearch.common.logging.DeprecationCategory;
 import org.elasticsearch.common.logging.DeprecationLogger;
 import org.elasticsearch.common.xcontent.support.XContentMapValues;
+import org.elasticsearch.core.Nullable;
 import org.elasticsearch.index.IndexVersion;
 import org.elasticsearch.index.mapper.MapperService.MergeReason;
 import org.elasticsearch.xcontent.ToXContent;
@@ -251,7 +252,7 @@ protected static Explicit<Boolean> parseSubobjects(Map<String, Object> node) {
             if (subobjectsNode != null) {
                 return Explicit.explicitBoolean(XContentMapValues.nodeBooleanValue(subobjectsNode, "subobjects.subobjects"));
             }
-            return Explicit.IMPLICIT_TRUE;
+            return Defaults.SUBOBJECTS;
         }
 
         protected static void parseProperties(
@@ -589,6 +590,27 @@ protected void doXContent(XContentBuilder builder, Params params) throws IOExcep
 
     }
 
+    /**
+     * Checks the {@code subobjects} config in provided mapping's root.
+     * @param mapping a raw mapping in a map form
+     * @return an explicit {@code subobjects} value if such is configured directly under the root, or {@code null} otherwise
+     */
+    @Nullable
+    static Explicit<Boolean> checkSubobjectsInMappingRoot(Map<String, Object> mapping) {
+        if (mapping.size() == 1) {
+            String rootName = mapping.keySet().iterator().next();
+            Object childNode = mapping.get(rootName);
+            if (childNode instanceof Map) {
+                @SuppressWarnings("unchecked")
+                Object subobjectsNode = ((Map<String, Object>) mapping.get(rootName)).get("subobjects");
+                if (subobjectsNode != null) {
+                    return Explicit.explicitBoolean(XContentMapValues.nodeBooleanValue(subobjectsNode, "subobjects.subobjects"));
+                }
+            }
+        }
+        return null;
+    }
+
     public SourceLoader.SyntheticFieldLoader syntheticFieldLoader(Stream<Mapper> extra) {
         return new SyntheticSourceFieldLoader(
             Stream.concat(extra, mappers.values().stream())

server/src/main/java/org/elasticsearch/index/mapper/RootObjectMapper.java

@@ -14,6 +14,7 @@
 import org.elasticsearch.common.logging.DeprecationCategory;
 import org.elasticsearch.common.logging.DeprecationLogger;
 import org.elasticsearch.common.time.DateFormatter;
+import org.elasticsearch.core.Nullable;
 import org.elasticsearch.index.IndexVersion;
 import org.elasticsearch.index.mapper.DynamicTemplate.XContentFieldType;
 import org.elasticsearch.index.mapper.MapperService.MergeReason;
@@ -399,9 +400,16 @@ protected void startSyntheticField(XContentBuilder b) throws IOException {
         b.startObject();
     }
 
-    public static RootObjectMapper.Builder parse(String name, Map<String, Object> node, MappingParserContext parserContext)
-        throws MapperParsingException {
+    public static RootObjectMapper.Builder parse(
+        String name,
+        Map<String, Object> node,
+        MappingParserContext parserContext,
+        @Nullable Explicit<Boolean> explicitSubobjects
+    ) throws MapperParsingException {
         Explicit<Boolean> subobjects = parseSubobjects(node);
+        if (subobjects.explicit() == false && explicitSubobjects != null) {
+            subobjects = explicitSubobjects;
+        }
         RootObjectMapper.Builder builder = new Builder(name, subobjects);
         Iterator<Map.Entry<String, Object>> iterator = node.entrySet().iterator();
         while (iterator.hasNext()) {

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

@@ -2504,6 +2504,61 @@ public void testAddInvalidTemplateIgnoreService() throws Exception {
         assertThat(e.getMessage(), containsString("missing component templates [fail] that does not exist"));
     }
 
+    public void testComposableTemplateWithSubobjectsFalse() throws Exception {
+        final MetadataIndexTemplateService service = getMetadataIndexTemplateService();
+        ClusterState state = ClusterState.EMPTY_STATE;
+
+        ComponentTemplate subobjects = new ComponentTemplate(new Template(null, new CompressedXContent("""
+            {
+              "subobjects": false
+            }
+            """), null), null, null);
+
+        ComponentTemplate fieldMapping = new ComponentTemplate(new Template(null, new CompressedXContent("""
+            {
+              "properties": {
+                "parent.subfield": {
+                  "type": "keyword"
+                }
+              }
+            }
+            """), null), null, null);
+
+        state = service.addComponentTemplate(state, true, "subobjects", subobjects);
+        state = service.addComponentTemplate(state, true, "field_mapping", fieldMapping);
+        ComposableIndexTemplate it = new ComposableIndexTemplate(
+            List.of("test-*"),
+            new Template(null, null, null),
+            List.of("subobjects", "field_mapping"),
+            0L,
+            1L,
+            null,
+            null,
+            null
+        );
+        state = service.addIndexTemplateV2(state, true, "composable-template", it);
+
+        List<CompressedXContent> mappings = MetadataIndexTemplateService.collectMappings(state, "composable-template", "test-index");
+
+        assertNotNull(mappings);
+        assertThat(mappings.size(), equalTo(2));
+        List<Map<String, Object>> parsedMappings = mappings.stream().map(m -> {
+            try {
+                return MapperService.parseMapping(NamedXContentRegistry.EMPTY, m);
+            } catch (Exception e) {
+                logger.error(e);
+                fail("failed to parse mappings: " + m.string());
+                return null;
+            }
+        }).toList();
+
+        assertThat(parsedMappings.get(0), equalTo(Map.of("_doc", Map.of("subobjects", false))));
+        assertThat(
+            parsedMappings.get(1),
+            equalTo(Map.of("_doc", Map.of("properties", Map.of("parent.subfield", Map.of("type", "keyword")))))
+        );
+    }
+
     private static List<Throwable> putTemplate(NamedXContentRegistry xContentRegistry, PutRequest request) {
         ThreadPool testThreadPool = mock(ThreadPool.class);
         ClusterService clusterService = ClusterServiceUtils.createClusterService(testThreadPool);