Add validation for dynamic templates (#51233)

Tries to load a `Mapper` instance for the mapping snippet of a dynamic template.
This should catch things like using an analyzer that is undefined or mapping attributes that are unused.

This is best effort:
* If `{{name}}` placeholder is used in the mapping snippet then validation is skipped.
* If `match_mapping_type` is not specified then validation is performed for all mapping types.
  If parsing succeeds with a single mapping type then this the dynamic mapping is considered valid.

If is detected that a dynamic template mapping snippet is invalid at mapping update time then the mapping update is failed for indices created on 8.0.0-alpha1 and later. For indices created on prior version a deprecation warning is omitted instead. In 7.x clusters the mapping update will never fail in case of an invalid dynamic template mapping snippet and a deprecation warning will always be omitted.

Closes #17411
Closes #24419

Co-authored-by: Adrien Grand <[email protected]>

Author: martijnvg

docs/reference/mapping/dynamic/templates.asciidoc

@@ -37,6 +37,19 @@ Dynamic templates are specified as an array of named objects:
 <2> The match conditions can include any of : `match_mapping_type`, `match`, `match_pattern`, `unmatch`, `path_match`, `path_unmatch`.
 <3> The mapping that the matched field should use.
 
+If a provided mapping contains an invalid mapping snippet then that results in
+a validation error. Validation always occurs when applying the dynamic template
+at index time or in most cases when updating the dynamic template.
+
+Whether updating the dynamic template fails when supplying an invalid mapping snippet depends on the following:
+* If no `match_mapping_type` has been specified then if the template is valid with one predefined mapping type then
+  the mapping snippet is considered valid. However if at index time a field that matches with the template is indexed
+  as a different type then an validation error will occur at index time instead. For example configuring a dynamic
+  template with no `match_mapping_type` is considered valid as string type, but at index time a field that matches with
+  the dynamic template is indexed as a long, then at index time a validation error may still occur.
+* If the `{{name}}` placeholder is used in the mapping snippet then the validation is skipped when updating
+  the dynamic template. This is because the field name is unknown at that time. The validation will then occur
+  when applying the template at index time.
 
 Templates are processed in order -- the first matching template wins. When
 putting new dynamic templates through the <<indices-put-mapping, put mapping>> API,
@@ -409,4 +422,3 @@ PUT my_index
 
 <1> Like the default dynamic mapping rules, doubles are mapped as floats, which
     are usually accurate enough, yet require half the disk space.
-

docs/reference/release-notes/8.0.0-alpha1.asciidoc

@@ -11,4 +11,8 @@ The changes listed below have been released for the first time in {es}
 Aggregations::
 * Disallow specifying the same percentile multiple times in percentiles aggregation {pull}52257[#52257]
 
+Mapping::
+* Dynamic mappings in indices created on 8.0 and later have stricter validation at mapping update time.
+  (e.g. incorrect analyzer settings or unknown field types). {pull}51233[#51233]
+
 coming[8.0.0]

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

@@ -334,6 +334,18 @@ private List processList(List list, String name, String dynamicType) {
         return processedList;
     }
 
+    String getName() {
+        return name;
+    }
+
+    XContentFieldType getXContentFieldType() {
+        return xcontentFieldType;
+    }
+
+    Map<String, Object> getMapping() {
+        return mapping;
+    }
+
     @Override
     public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
         builder.startObject();

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

@@ -716,4 +716,5 @@ public synchronized List<String> reloadSearchAnalyzers(AnalysisRegistry registry
         }
         return reloadedAnalyzers;
     }
+
 }

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

@@ -19,9 +19,13 @@
 
 package org.elasticsearch.index.mapper;
 
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 import org.elasticsearch.Version;
 import org.elasticsearch.common.Explicit;
 import org.elasticsearch.common.Nullable;
+import org.elasticsearch.common.Strings;
+import org.elasticsearch.common.logging.DeprecationLogger;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.time.DateFormatter;
 import org.elasticsearch.common.xcontent.ToXContent;
@@ -34,13 +38,17 @@
 import java.util.Collections;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Locale;
 import java.util.Map;
 
 import static org.elasticsearch.common.xcontent.support.XContentMapValues.nodeBooleanValue;
 import static org.elasticsearch.index.mapper.TypeParsers.parseDateTimeFormatter;
 
 public class RootObjectMapper extends ObjectMapper {
 
+    private static final Logger LOGGER = LogManager.getLogger(RootObjectMapper.class);
+    private static final DeprecationLogger DEPRECATION_LOGGER = new DeprecationLogger(LOGGER);
+
     public static class Defaults {
         public static final DateFormatter[] DYNAMIC_DATE_TIME_FORMATTERS =
                 new DateFormatter[]{
@@ -130,7 +138,7 @@ public Mapper.Builder parse(String name, Map<String, Object> node, ParserContext
                 String fieldName = entry.getKey();
                 Object fieldNode = entry.getValue();
                 if (parseObjectOrDocumentTypeProperties(fieldName, fieldNode, parserContext, builder)
-                        || processField(builder, fieldName, fieldNode, parserContext.indexVersionCreated())) {
+                        || processField(builder, fieldName, fieldNode, parserContext)) {
                     iterator.remove();
                 }
             }
@@ -139,7 +147,7 @@ public Mapper.Builder parse(String name, Map<String, Object> node, ParserContext
 
         @SuppressWarnings("unchecked")
         protected boolean processField(RootObjectMapper.Builder builder, String fieldName, Object fieldNode,
-                                       Version indexVersionCreated) {
+                                       ParserContext parserContext) {
             if (fieldName.equals("date_formats") || fieldName.equals("dynamic_date_formats")) {
                 if (fieldNode instanceof List) {
                     List<DateFormatter> formatters = new ArrayList<>();
@@ -163,7 +171,7 @@ protected boolean processField(RootObjectMapper.Builder builder, String fieldNam
                           "template_1" : {
                               "match" : "*_test",
                               "match_mapping_type" : "string",
-                              "mapping" : { "type" : "string", "store" : "yes" }
+                              "mapping" : { "type" : "keyword", "store" : "yes" }
                           }
                       }
                   ]
@@ -183,6 +191,7 @@ protected boolean processField(RootObjectMapper.Builder builder, String fieldNam
                     Map<String, Object> templateParams = (Map<String, Object>) entry.getValue();
                     DynamicTemplate template = DynamicTemplate.parse(templateName, templateParams);
                     if (template != null) {
+                        validateDynamicTemplate(parserContext, template);
                         templates.add(template);
                     }
                 }
@@ -333,4 +342,114 @@ protected void doXContent(XContentBuilder builder, ToXContent.Params params) thr
             builder.field("numeric_detection", numericDetection.value());
         }
     }
+
+    private static void validateDynamicTemplate(Mapper.TypeParser.ParserContext parserContext,
+                                                DynamicTemplate dynamicTemplate) {
+
+        if (containsSnippet(dynamicTemplate.getMapping(), "{name}")) {
+            // Can't validate template, because field names can't be guessed up front.
+            return;
+        }
+
+        final XContentFieldType[] types;
+        if (dynamicTemplate.getXContentFieldType() != null) {
+            types = new XContentFieldType[]{dynamicTemplate.getXContentFieldType()};
+        } else {
+            types = XContentFieldType.values();
+        }
+
+        Exception lastError = null;
+        boolean dynamicTemplateInvalid = true;
+
+        for (XContentFieldType contentFieldType : types) {
+            String defaultDynamicType = contentFieldType.defaultMappingType();
+            String mappingType = dynamicTemplate.mappingType(defaultDynamicType);
+            Mapper.TypeParser typeParser = parserContext.typeParser(mappingType);
+            if (typeParser == null) {
+                lastError = new IllegalArgumentException("No mapper found for type [" + mappingType + "]");
+                continue;
+            }
+
+            Map<String, Object> fieldTypeConfig = dynamicTemplate.mappingForName("__dummy__", defaultDynamicType);
+            fieldTypeConfig.remove("type");
+            try {
+                Mapper.Builder<?, ?> dummyBuilder = typeParser.parse("__dummy__", fieldTypeConfig, parserContext);
+                if (fieldTypeConfig.isEmpty()) {
+                    Settings indexSettings = parserContext.mapperService().getIndexSettings().getSettings();
+                    BuilderContext builderContext = new BuilderContext(indexSettings, new ContentPath(1));
+                    dummyBuilder.build(builderContext);
+                    dynamicTemplateInvalid = false;
+                    break;
+                } else {
+                    lastError = new IllegalArgumentException("Unused mapping attributes [" + fieldTypeConfig + "]");
+                }
+            } catch (Exception e) {
+                lastError = e;
+            }
+        }
+
+        final boolean failInvalidDynamicTemplates = parserContext.indexVersionCreated().onOrAfter(Version.V_8_0_0);
+        if (dynamicTemplateInvalid) {
+            String message = String.format(Locale.ROOT, "dynamic template [%s] has invalid content [%s]",
+                dynamicTemplate.getName(), Strings.toString(dynamicTemplate));
+            if (failInvalidDynamicTemplates) {
+                throw new IllegalArgumentException(message, lastError);
+            } else {
+                final String deprecationMessage;
+                if (lastError != null) {
+                     deprecationMessage = String.format(Locale.ROOT, "%s, caused by [%s]", message, lastError.getMessage());
+                } else {
+                    deprecationMessage = message;
+                }
+                DEPRECATION_LOGGER.deprecatedAndMaybeLog("invalid_dynamic_template", deprecationMessage);
+            }
+        }
+    }
+
+    private static boolean containsSnippet(Map<?, ?> map, String snippet) {
+        for (Map.Entry<?, ?> entry : map.entrySet()) {
+            String key = entry.getKey().toString();
+            if (key.contains(snippet)) {
+                return true;
+            }
+
+            Object value = entry.getValue();
+            if (value instanceof Map) {
+                if (containsSnippet((Map<?, ?>) value, snippet)) {
+                    return true;
+                }
+            } else if (value instanceof List) {
+                if (containsSnippet((List<?>) value, snippet)) {
+                    return true;
+                }
+            } else if (value instanceof String) {
+                String valueString = (String) value;
+                if (valueString.contains(snippet)) {
+                    return true;
+                }
+            }
+        }
+
+        return false;
+    }
+
+    private static boolean containsSnippet(List<?> list, String snippet) {
+        for (Object value : list) {
+            if (value instanceof Map) {
+                if (containsSnippet((Map<?, ?>) value, snippet)) {
+                    return true;
+                }
+            } else if (value instanceof List) {
+                if (containsSnippet((List<?>) value, snippet)) {
+                    return true;
+                }
+            } else if (value instanceof String) {
+                String valueString = (String) value;
+                if (valueString.contains(snippet)) {
+                    return true;
+                }
+            }
+        }
+        return false;
+    }
 }