Pārlūkot izejas kodu

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 <jpountz@gmail.com>
Martijn van Groningen 5 gadi atpakaļ
vecāks
revīzija
31b29875c9

+ 13 - 1
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.
-

+ 4 - 0
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]

+ 12 - 0
server/src/main/java/org/elasticsearch/index/mapper/DynamicTemplate.java

@@ -334,6 +334,18 @@ public class DynamicTemplate implements ToXContentObject {
         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();

+ 1 - 0
server/src/main/java/org/elasticsearch/index/mapper/MapperService.java

@@ -716,4 +716,5 @@ public class MapperService extends AbstractIndexComponent implements Closeable {
         }
         return reloadedAnalyzers;
     }
+
 }

+ 122 - 3
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,6 +38,7 @@ import java.util.Collection;
 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;
@@ -41,6 +46,9 @@ 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 class RootObjectMapper extends ObjectMapper {
                 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 class RootObjectMapper extends ObjectMapper {
 
         @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 @@ public class RootObjectMapper extends ObjectMapper {
                           "template_1" : {
                               "match" : "*_test",
                               "match_mapping_type" : "string",
-                              "mapping" : { "type" : "string", "store" : "yes" }
+                              "mapping" : { "type" : "keyword", "store" : "yes" }
                           }
                       }
                   ]
@@ -183,6 +191,7 @@ public class RootObjectMapper extends ObjectMapper {
                     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 @@ public class RootObjectMapper extends ObjectMapper {
             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;
+    }
 }

+ 193 - 0
server/src/test/java/org/elasticsearch/index/mapper/RootObjectMapperTests.java

@@ -19,14 +19,23 @@
 
 package org.elasticsearch.index.mapper;
 
+import org.elasticsearch.Version;
+import org.elasticsearch.cluster.metadata.IndexMetaData;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.compress.CompressedXContent;
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentFactory;
 import org.elasticsearch.index.mapper.MapperService.MergeReason;
 import org.elasticsearch.test.ESSingleNodeTestCase;
 
 import java.util.Arrays;
 
+import static org.elasticsearch.test.VersionUtils.randomVersionBetween;
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.instanceOf;
+
 public class RootObjectMapperTests extends ESSingleNodeTestCase {
 
     public void testNumericDetection() throws Exception {
@@ -200,4 +209,188 @@ public class RootObjectMapperTests extends ESSingleNodeTestCase {
                     () -> parser.parse("type", new CompressedXContent(mapping)));
             assertEquals("Dynamic template syntax error. An array of named objects is expected.", e.getMessage());
     }
+
+    public void testIllegalDynamicTemplateUnknownFieldType() throws Exception {
+        XContentBuilder mapping = XContentFactory.jsonBuilder();
+        mapping.startObject();
+        {
+            mapping.startObject("type");
+            mapping.startArray("dynamic_templates");
+            {
+                mapping.startObject();
+                mapping.startObject("my_template");
+                mapping.field("match_mapping_type", "string");
+                mapping.startObject("mapping");
+                mapping.field("type", "string");
+                mapping.endObject();
+                mapping.endObject();
+                mapping.endObject();
+            }
+            mapping.endArray();
+            mapping.endObject();
+        }
+        mapping.endObject();
+        MapperService mapperService = createIndex("test").mapperService();
+        MapperParsingException e = expectThrows(MapperParsingException.class,
+            () -> mapperService.merge("type", new CompressedXContent(Strings.toString(mapping)), MergeReason.MAPPING_UPDATE));
+        assertThat(e.getRootCause(), instanceOf(IllegalArgumentException.class));
+        assertThat(e.getRootCause().getMessage(), equalTo("No mapper found for type [string]"));
+    }
+
+    public void testIllegalDynamicTemplateUnknownAttribute() throws Exception {
+        XContentBuilder mapping = XContentFactory.jsonBuilder();
+        mapping.startObject();
+        {
+            mapping.startObject("type");
+            mapping.startArray("dynamic_templates");
+            {
+                mapping.startObject();
+                mapping.startObject("my_template");
+                mapping.field("match_mapping_type", "string");
+                mapping.startObject("mapping");
+                mapping.field("type", "keyword");
+                mapping.field("foo", "bar");
+                mapping.endObject();
+                mapping.endObject();
+                mapping.endObject();
+            }
+            mapping.endArray();
+            mapping.endObject();
+        }
+        mapping.endObject();
+        MapperService mapperService = createIndex("test").mapperService();
+        MapperParsingException e = expectThrows(MapperParsingException.class,
+            () -> mapperService.merge("type", new CompressedXContent(Strings.toString(mapping)), MergeReason.MAPPING_UPDATE));
+        assertThat(e.getRootCause(), instanceOf(IllegalArgumentException.class));
+        assertThat(e.getRootCause().getMessage(), equalTo("Unused mapping attributes [{foo=bar}]"));
+    }
+
+    public void testIllegalDynamicTemplateInvalidAttribute() throws Exception {
+        XContentBuilder mapping = XContentFactory.jsonBuilder();
+        mapping.startObject();
+        {
+            mapping.startObject("type");
+            mapping.startArray("dynamic_templates");
+            {
+                mapping.startObject();
+                mapping.startObject("my_template");
+                mapping.field("match_mapping_type", "string");
+                mapping.startObject("mapping");
+                mapping.field("type", "text");
+                mapping.field("analyzer", "foobar");
+                mapping.endObject();
+                mapping.endObject();
+                mapping.endObject();
+            }
+            mapping.endArray();
+            mapping.endObject();
+        }
+        mapping.endObject();
+        MapperService mapperService = createIndex("test").mapperService();
+        MapperParsingException e = expectThrows(MapperParsingException.class,
+            () -> mapperService.merge("type", new CompressedXContent(Strings.toString(mapping)), MergeReason.MAPPING_UPDATE));
+        assertThat(e.getRootCause(), instanceOf(MapperParsingException.class));
+        assertThat(e.getRootCause().getMessage(), equalTo("analyzer [foobar] not found for field [__dummy__]"));
+    }
+
+    public void testIllegalDynamicTemplateNoMappingType() throws Exception {
+        MapperService mapperService;
+
+        {
+            XContentBuilder mapping = XContentFactory.jsonBuilder();
+            mapping.startObject();
+            {
+                mapping.startObject("type");
+                mapping.startArray("dynamic_templates");
+                {
+                    mapping.startObject();
+                    mapping.startObject("my_template");
+                    if (randomBoolean()) {
+                        mapping.field("match_mapping_type", "*");
+                    } else {
+                        mapping.field("match", "string_*");
+                    }
+                    mapping.startObject("mapping");
+                    mapping.field("type", "{dynamic_type}");
+                    mapping.field("index_phrases", true);
+                    mapping.endObject();
+                    mapping.endObject();
+                    mapping.endObject();
+                }
+                mapping.endArray();
+                mapping.endObject();
+            }
+            mapping.endObject();
+            mapperService = createIndex("test").mapperService();
+            DocumentMapper mapper =
+                mapperService.merge("type", new CompressedXContent(Strings.toString(mapping)), MergeReason.MAPPING_UPDATE);
+            assertThat(mapper.mappingSource().toString(), containsString("\"index_phrases\":true"));
+        }
+        {
+            XContentBuilder mapping = XContentFactory.jsonBuilder();
+            mapping.startObject();
+            {
+                mapping.startObject("type");
+                mapping.startArray("dynamic_templates");
+                {
+                    mapping.startObject();
+                    mapping.startObject("my_template");
+                    if (randomBoolean()) {
+                        mapping.field("match_mapping_type", "*");
+                    } else {
+                        mapping.field("match", "string_*");
+                    }
+                    mapping.startObject("mapping");
+                    mapping.field("type", "{dynamic_type}");
+                    mapping.field("foo", "bar");
+                    mapping.endObject();
+                    mapping.endObject();
+                    mapping.endObject();
+                }
+                mapping.endArray();
+                mapping.endObject();
+            }
+            mapping.endObject();
+            MapperParsingException e = expectThrows(MapperParsingException.class,
+                () -> mapperService.merge("type", new CompressedXContent(Strings.toString(mapping)), MergeReason.MAPPING_UPDATE));
+            assertThat(e.getRootCause(), instanceOf(IllegalArgumentException.class));
+            assertThat(e.getRootCause().getMessage(), equalTo("Unused mapping attributes [{foo=bar}]"));
+        }
+    }
+
+    @Override
+    protected boolean forbidPrivateIndexSettings() {
+        return false;
+    }
+
+    public void testIllegalDynamicTemplate7DotXIndex() throws Exception {
+        XContentBuilder mapping = XContentFactory.jsonBuilder();
+        mapping.startObject();
+        {
+            mapping.startObject("type");
+            mapping.startArray("dynamic_templates");
+            {
+                mapping.startObject();
+                mapping.startObject("my_template");
+                mapping.field("match_mapping_type", "string");
+                mapping.startObject("mapping");
+                mapping.field("type", "string");
+                mapping.endObject();
+                mapping.endObject();
+                mapping.endObject();
+            }
+            mapping.endArray();
+            mapping.endObject();
+        }
+        mapping.endObject();
+        Version createdVersion = randomVersionBetween(random(), Version.V_7_0_0, Version.V_7_7_0);
+        Settings indexSettings = Settings.builder()
+            .put(IndexMetaData.SETTING_INDEX_VERSION_CREATED.getKey(), createdVersion)
+            .build();
+        MapperService mapperService = createIndex("test", indexSettings).mapperService();
+        DocumentMapper mapper = mapperService.merge("type", new CompressedXContent(Strings.toString(mapping)), MergeReason.MAPPING_UPDATE);
+        assertThat(mapper.mappingSource().toString(), containsString("\"type\":\"string\""));
+        assertWarnings("dynamic template [my_template] has invalid content [{\"match_mapping_type\":\"string\",\"mapping\":{\"type\":" +
+            "\"string\"}}], caused by [No mapper found for type [string]]");
+    }
 }