From 5aee5f9a6b9c3e0d7d5ae0b24f07f901075020cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Mon, 25 Jan 2021 16:36:30 +0100 Subject: [PATCH] Add bwc for parsing mappings from dynamic templates (#67763) During refactoring of the mapper parsing code, the part that checks for unknown mapper parameters and throws an error when one is found was moved to FieldMapper.Builder#parse which gets also executed when applying matching dynamic templates. However, dynamic templates introduced during the 7.x versions may still contain arbitrary parameters, although we have deprecation warnings for that in place on latest 7.x now. When using these templates, indexing a new document with a new field that triggers one of these mappings to be parsed can create an error as demonstrated in #66765. Instead we need to be lenient in these cases and simply ignore the unknown parameter while issuing a deprecation warning here as well. Closes #66765 --- .../mapping/MalformedDynamicTemplateIT.java | 68 +++++++++++++++++++ .../index/mapper/DynamicFieldsBuilder.java | 7 +- .../index/mapper/FieldMapper.java | 49 ++++++++----- .../elasticsearch/index/mapper/Mapper.java | 24 ++++++- .../index/mapper/ParametrizedMapperTests.java | 24 ++++++- 5 files changed, 148 insertions(+), 24 deletions(-) create mode 100644 server/src/internalClusterTest/java/org/elasticsearch/indices/mapping/MalformedDynamicTemplateIT.java diff --git a/server/src/internalClusterTest/java/org/elasticsearch/indices/mapping/MalformedDynamicTemplateIT.java b/server/src/internalClusterTest/java/org/elasticsearch/indices/mapping/MalformedDynamicTemplateIT.java new file mode 100644 index 0000000000000..903e2645b5585 --- /dev/null +++ b/server/src/internalClusterTest/java/org/elasticsearch/indices/mapping/MalformedDynamicTemplateIT.java @@ -0,0 +1,68 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.indices.mapping; + +import org.elasticsearch.Version; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.xcontent.XContentType; +import org.elasticsearch.test.ESIntegTestCase; +import org.elasticsearch.test.VersionUtils; + +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoFailures; + +public class MalformedDynamicTemplateIT extends ESIntegTestCase { + + @Override + protected boolean forbidPrivateIndexSettings() { + return false; + } + + /** + * Check that we can index a document into an 7.x index with a matching dynamic template that + * contains unknown parameters. We were able to create those templates in 7.x still, so we need + * to be able to index new documents into them. Indexing should issue a deprecation warning though. + */ + public void testBWCMalformedDynamicTemplate() { + String mapping = "{ \"dynamic_templates\": [\n" + + " {\n" + + " \"my_template\": {\n" + + " \"mapping\": {\n" + + " \"ignore_malformed\": true,\n" // this parameter is not supported by "keyword" field type + + " \"type\": \"keyword\"\n" + + " },\n" + + " \"path_match\": \"*\"\n" + + " }\n" + + " }\n" + + " ]\n" + + " }\n" + + "}}"; + String indexName = "malformed_dynamic_template"; + assertAcked(prepareCreate(indexName).setSettings(Settings.builder().put(indexSettings()) + .put("number_of_shards", 1) + .put("index.version.created", VersionUtils.randomCompatibleVersion(random(), Version.CURRENT)) + ).addMapping("_doc", mapping, XContentType.JSON).get()); + client().prepareIndex(indexName, "_doc").setSource("{\"foo\" : \"bar\"}", XContentType.JSON).get(); + assertNoFailures((client().admin().indices().prepareRefresh(indexName)).get()); + assertHitCount(client().prepareSearch(indexName).get(), 1); + } + +} \ No newline at end of file diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DynamicFieldsBuilder.java b/server/src/main/java/org/elasticsearch/index/mapper/DynamicFieldsBuilder.java index c6d77d7b7d26e..d9f346ba46623 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/DynamicFieldsBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/DynamicFieldsBuilder.java @@ -212,7 +212,7 @@ private static boolean applyMatchingTemplate(ParseContext context, RuntimeFieldType runtimeFieldType = parser.parse(fullName, mapping, parserContext); Runtime.createDynamicField(runtimeFieldType, context); } else { - Mapper.Builder builder = parseMapping(name, mappingType, mapping, dateFormatter, context); + Mapper.Builder builder = parseDynamicTemplateMapping(name, mappingType, mapping, dateFormatter, context); CONCRETE.createDynamicField(builder, context); } return true; @@ -227,15 +227,16 @@ private static Mapper.Builder findTemplateBuilderForObject(ParseContext context, String dynamicType = matchType.defaultMappingType(); String mappingType = dynamicTemplate.mappingType(dynamicType); Map mapping = dynamicTemplate.mappingForName(name, dynamicType); - return parseMapping(name, mappingType, mapping, null, context); + return parseDynamicTemplateMapping(name, mappingType, mapping, null, context); } - private static Mapper.Builder parseMapping(String name, + private static Mapper.Builder parseDynamicTemplateMapping(String name, String mappingType, Map mapping, DateFormatter dateFormatter, ParseContext context) { Mapper.TypeParser.ParserContext parserContext = context.parserContext(dateFormatter); + parserContext = parserContext.createDynamicTemplateFieldContext(parserContext); Mapper.TypeParser typeParser = parserContext.typeParser(mappingType); if (typeParser == null) { throw new MapperParsingException("failed to find type parsed [" + mappingType + "] for [" + name + "]"); diff --git a/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java index 90bbc2f6237de..8d0cad85541a6 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java @@ -19,6 +19,22 @@ package org.elasticsearch.index.mapper; +import org.apache.lucene.document.Field; +import org.elasticsearch.Version; +import org.elasticsearch.common.Explicit; +import org.elasticsearch.common.TriFunction; +import org.elasticsearch.common.logging.DeprecationLogger; +import org.elasticsearch.common.settings.Setting; +import org.elasticsearch.common.settings.Setting.Property; +import org.elasticsearch.common.xcontent.ToXContent; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.common.xcontent.support.AbstractXContentParser; +import org.elasticsearch.common.xcontent.support.XContentMapValues; +import org.elasticsearch.index.analysis.NamedAnalyzer; +import org.elasticsearch.index.mapper.FieldNamesFieldMapper.FieldNamesFieldType; +import org.elasticsearch.index.mapper.Mapper.TypeParser.ParserContext; + import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; @@ -37,22 +53,6 @@ import java.util.function.Function; import java.util.function.Supplier; -import org.apache.lucene.document.Field; -import org.elasticsearch.Version; -import org.elasticsearch.common.Explicit; -import org.elasticsearch.common.TriFunction; -import org.elasticsearch.common.logging.DeprecationLogger; -import org.elasticsearch.common.settings.Setting; -import org.elasticsearch.common.settings.Setting.Property; -import org.elasticsearch.common.xcontent.ToXContent; -import org.elasticsearch.common.xcontent.XContentBuilder; -import org.elasticsearch.common.xcontent.XContentParser; -import org.elasticsearch.common.xcontent.support.AbstractXContentParser; -import org.elasticsearch.common.xcontent.support.XContentMapValues; -import org.elasticsearch.index.analysis.NamedAnalyzer; -import org.elasticsearch.index.mapper.FieldNamesFieldMapper.FieldNamesFieldType; -import org.elasticsearch.index.mapper.Mapper.TypeParser.ParserContext; - public abstract class FieldMapper extends Mapper implements Cloneable { public static final Setting IGNORE_MALFORMED_SETTING = Setting.boolSetting("index.mapping.ignore_malformed", false, Property.IndexScope); @@ -981,8 +981,21 @@ public final void parse(String name, ParserContext parserContext, Map similarityLookupService() { return similarityLookupService; } /** @@ -153,11 +158,15 @@ public ScriptService scriptService() { return scriptService; } - public ParserContext createMultiFieldContext(ParserContext in) { + ParserContext createMultiFieldContext(ParserContext in) { return new MultiFieldParserContext(in); } - static class MultiFieldParserContext extends ParserContext { + ParserContext createDynamicTemplateFieldContext(ParserContext in) { + return new DynamicTemplateParserContext(in); + } + + private static class MultiFieldParserContext extends ParserContext { MultiFieldParserContext(ParserContext in) { super(in.similarityLookupService, in.typeParsers, in.runtimeTypeParsers, in.indexVersionCreated, in.searchExecutionContextSupplier, in.dateFormatter, in.scriptService, in.indexAnalyzers, in.indexSettings, @@ -167,6 +176,17 @@ static class MultiFieldParserContext extends ParserContext { @Override public boolean isWithinMultiField() { return true; } } + + private static class DynamicTemplateParserContext extends ParserContext { + DynamicTemplateParserContext(ParserContext in) { + super(in.similarityLookupService, in.typeParsers, in.runtimeTypeParsers, in.indexVersionCreated, + in.searchExecutionContextSupplier, in.dateFormatter, in.scriptService, in.indexAnalyzers, in.indexSettings, + in.idFieldDataEnabled, in.supportsDynamicRuntimeMappings); + } + + @Override + public boolean isFromDynamicTemplate() { return true; } + } } Mapper.Builder parse(String name, Map node, ParserContext parserContext) throws MapperParsingException; diff --git a/server/src/test/java/org/elasticsearch/index/mapper/ParametrizedMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/ParametrizedMapperTests.java index 637e32a66680f..42c040f87aacb 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/ParametrizedMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/ParametrizedMapperTests.java @@ -34,6 +34,7 @@ import org.elasticsearch.index.mapper.FieldMapper.Parameter; import org.elasticsearch.plugins.MapperPlugin; import org.elasticsearch.plugins.Plugin; +import org.elasticsearch.test.VersionUtils; import java.io.IOException; import java.util.Arrays; @@ -197,7 +198,7 @@ protected String contentType() { } } - private static TestMapper fromMapping(String mapping, Version version) { + private static TestMapper fromMapping(String mapping, Version version, boolean fromDynamicTemplate) { MapperService mapperService = mock(MapperService.class); IndexAnalyzers indexAnalyzers = new IndexAnalyzers( org.elasticsearch.common.collect.Map.of( @@ -218,11 +219,18 @@ private static TestMapper fromMapping(String mapping, Version version) { mapperService.getIndexAnalyzers(), mapperService.getIndexSettings(), () -> { throw new UnsupportedOperationException(); }, false); + if (fromDynamicTemplate) { + pc = pc.createDynamicTemplateFieldContext(pc); + } return (TestMapper) new TypeParser() .parse("field", XContentHelper.convertToMap(JsonXContent.jsonXContent, mapping, true), pc) .build(new ContentPath()); } + private static TestMapper fromMapping(String mapping, Version version) { + return fromMapping(mapping, version, false); + } + private static TestMapper fromMapping(String mapping) { return fromMapping(mapping, Version.CURRENT); } @@ -381,6 +389,20 @@ public void testDeprecatedParameterName() { assertEquals("{\"field\":{\"type\":\"test_mapper\",\"fixed2\":true,\"required\":\"value\"}}", Strings.toString(mapper)); } + /** + * test parsing mapping from dynamic templates, should ignore unknown parameters for bwc and log deprecation warning before 8.0.0 + */ + public void testBWCunknownParametersfromDynamicTemplates() { + String mapping = "{\"type\":\"test_mapper\",\"some_unknown_parameter\":true,\"required\":\"value\"}"; + TestMapper mapper = fromMapping(mapping, VersionUtils.randomCompatibleVersion(random(), Version.CURRENT), true); + assertNotNull(mapper); + assertWarnings( + "Parameter [some_unknown_parameter] is used in a dynamic template mapping and has no effect on type [test_mapper]. " + + "Usage will result in an error in future major versions and should be removed." + ); + assertEquals("{\"field\":{\"type\":\"test_mapper\",\"required\":\"value\"}}", Strings.toString(mapper)); + } + public void testAnalyzers() { String mapping = "{\"type\":\"test_mapper\",\"analyzer\":\"_standard\",\"required\":\"value\"}"; TestMapper mapper = fromMapping(mapping);