Skip to content

Commit

Permalink
Add validation for dynamic templates (elastic#51233)
Browse files Browse the repository at this point in the history
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 elastic#17411
Closes elastic#24419

Co-authored-by: Adrien Grand <jpountz@gmail.com>
  • Loading branch information
martijnvg and jpountz committed Feb 27, 2020
1 parent 52fa465 commit 5e0a808
Show file tree
Hide file tree
Showing 6 changed files with 355 additions and 5 deletions.
14 changes: 13 additions & 1 deletion docs/reference/mapping/dynamic/templates.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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.

13 changes: 13 additions & 0 deletions docs/reference/release-notes/7.7.asciidoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
[[release-notes-7.7.0]]
== {es} version 7.7.0

coming[7.7.0]

[[breaking-7.7.0]]
[float]
=== Breaking changes

Mapping::
* Dynamic mappings in indices created on 8.0 and later have stricter validation at mapping update time and
results in a deprecation warning for indices created in Elasticsearch 7.7.0 and later.
(e.g. incorrect analyzer settings or unknown field types). {pull}51233[#51233]
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -886,4 +886,5 @@ public synchronized List<String> reloadSearchAnalyzers(AnalysisRegistry registry
}
return reloadedAnalyzers;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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[]{
Expand Down Expand Up @@ -128,15 +136,15 @@ 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();
}
}
return builder;
}

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<>();
Expand All @@ -159,7 +167,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" }
// }
// }
// ]
Expand All @@ -176,8 +184,9 @@ protected boolean processField(RootObjectMapper.Builder builder, String fieldNam
Map.Entry<String, Object> entry = tmpl.entrySet().iterator().next();
String templateName = entry.getKey();
Map<String, Object> templateParams = (Map<String, Object>) entry.getValue();
DynamicTemplate template = DynamicTemplate.parse(templateName, templateParams, indexVersionCreated);
DynamicTemplate template = DynamicTemplate.parse(templateName, templateParams, parserContext.indexVersionCreated());
if (template != null) {
validateDynamicTemplate(parserContext, template);
templates.add(template);
}
}
Expand Down Expand Up @@ -326,4 +335,111 @@ 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 shouldEmitDeprecationWarning = parserContext.indexVersionCreated().onOrAfter(Version.V_7_7_0);
if (dynamicTemplateInvalid && shouldEmitDeprecationWarning) {
String message = String.format(Locale.ROOT, "dynamic template [%s] has invalid content [%s]",
dynamicTemplate.getName(), Strings.toString(dynamicTemplate));

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;
}
}
Loading

0 comments on commit 5e0a808

Please sign in to comment.