Add explicit tests for dynamic template validation on bad mapping parameters (#74177)

When a dynamic template is applied to a 7.x index, we do some validation checks
against the generated mappings and issue a deprecation warning if the validation
fails. We had some tests that were checking this at a low level, but nothing that
exercised the full logic.

When dynamic runtimes were added, we expected this behaviour to carry over;
however, a bug in building the ParserContext to pass to the runtime Builder meant
that we would instead always throw an error. In fact, erroring here is a good result,
as the only reason we issue a deprecation warning for non-runtime fields is to
preserve backward compatibility; given that runtime fields are new, it has never
been possible to add a bad dynamic runtime template.

This commit adds specific tests for both situations. It ensures that the parser
context passed to a runtime builder will know that it is in a dynamic context,
but it also removes the version check and deprecation warning, as they were
never actually triggered and we can be stricter here.

Author: romseygeek

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

@@ -193,7 +193,7 @@ private static boolean applyMatchingTemplate(ParseContext context,
         String mappingType = dynamicTemplate.mappingType(dynamicType);
         Map<String, Object> mapping = dynamicTemplate.mappingForName(name, dynamicType);
         if (dynamicTemplate.isRuntimeMapping()) {
-            Mapper.TypeParser.ParserContext parserContext = context.parserContext(dateFormatter);
+            Mapper.TypeParser.ParserContext parserContext = context.dynamicTemplateParserContext(dateFormatter);
             RuntimeField.Parser parser = parserContext.runtimeFieldParser(mappingType);
             String fullName = context.path().pathAsText(name);
             if (parser == null) {
@@ -225,8 +225,7 @@ private static Mapper.Builder parseDynamicTemplateMapping(String name,
                                                Map<String, Object> mapping,
                                                DateFormatter dateFormatter,
                                                ParseContext context) {
-        Mapper.TypeParser.ParserContext parserContext = context.parserContext(dateFormatter);
-        parserContext = parserContext.createDynamicTemplateFieldContext(parserContext);
+        Mapper.TypeParser.ParserContext parserContext = context.dynamicTemplateParserContext(dateFormatter);
         Mapper.TypeParser typeParser = parserContext.typeParser(mappingType);
         if (typeParser == null) {
             throw new MapperParsingException("failed to find type parsed [" + mappingType + "] for [" + name + "]");

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

@@ -144,8 +144,8 @@ ParserContext createMultiFieldContext(ParserContext in) {
                 return new MultiFieldParserContext(in);
             }
 
-            ParserContext createDynamicTemplateFieldContext(ParserContext in) {
-                return new DynamicTemplateParserContext(in);
+            ParserContext createDynamicTemplateFieldContext() {
+                return new DynamicTemplateParserContext(this);
             }
 
             private static class MultiFieldParserContext extends ParserContext {

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

@@ -164,8 +164,8 @@ public Iterable<Document> nonRootDocuments() {
         }
 
         @Override
-        public Mapper.TypeParser.ParserContext parserContext(DateFormatter dateFormatter) {
-            return in.parserContext(dateFormatter);
+        public Mapper.TypeParser.ParserContext dynamicTemplateParserContext(DateFormatter dateFormatter) {
+            return in.dynamicTemplateParserContext(dateFormatter);
         }
 
         @Override
@@ -336,8 +336,8 @@ public InternalParseContext(MappingLookup mappingLookup,
         }
 
         @Override
-        public Mapper.TypeParser.ParserContext parserContext(DateFormatter dateFormatter) {
-            return parserContextFunction.apply(dateFormatter);
+        public Mapper.TypeParser.ParserContext dynamicTemplateParserContext(DateFormatter dateFormatter) {
+            return parserContextFunction.apply(dateFormatter).createDynamicTemplateFieldContext();
         }
 
         @Override
@@ -548,7 +548,7 @@ public Collection<String> getFieldNames() {
      */
     public abstract Collection<String> getFieldNames();
 
-    public abstract Mapper.TypeParser.ParserContext parserContext(DateFormatter dateFormatter);
+    public abstract Mapper.TypeParser.ParserContext dynamicTemplateParserContext(DateFormatter dateFormatter);
 
     /**
      * Return a new context that will be within a copy-to operation.

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

@@ -8,9 +8,6 @@
 
 package org.elasticsearch.index.mapper;
 
-import org.elasticsearch.Version;
-import org.elasticsearch.common.logging.DeprecationCategory;
-import org.elasticsearch.common.logging.DeprecationLogger;
 import org.elasticsearch.common.xcontent.ToXContent;
 import org.elasticsearch.common.xcontent.ToXContentFragment;
 import org.elasticsearch.common.xcontent.XContentBuilder;
@@ -67,8 +64,6 @@ abstract class Builder implements ToXContent {
         final String name;
         final Parameter<Map<String, String>> meta = Parameter.metaParam();
 
-        private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(RuntimeField.class);
-
         protected Builder(String name) {
             this.name = name;
         }
@@ -104,18 +99,6 @@ public final void parse(String name, Mapper.TypeParser.ParserContext parserConte
                 final Object propNode = entry.getValue();
                 Parameter<?> parameter = paramsMap.get(propName);
                 if (parameter == null) {
-                    if (parserContext.isFromDynamicTemplate() && parserContext.indexVersionCreated().before(Version.V_8_0_0)) {
-                        // The parameter is unknown, but this mapping is from a dynamic template.
-                        // Until 7.x it was possible to use unknown parameters there, so for bwc we need to ignore it
-                        deprecationLogger.deprecate(DeprecationCategory.API, propName,
-                            "Parameter [{}] is used in a dynamic template mapping and has no effect on type [{}]. "
-                                + "Usage will result in an error in future major versions and should be removed.",
-                            propName,
-                            type
-                        );
-                        iterator.remove();
-                        continue;
-                    }
                     throw new MapperParsingException(
                         "unknown parameter [" + propName + "] on runtime field [" + name + "] of type [" + type + "]"
                     );

server/src/test/java/org/elasticsearch/index/mapper/DynamicTemplatesTests.java

@@ -11,9 +11,14 @@
 import org.apache.lucene.index.IndexOptions;
 import org.apache.lucene.index.IndexableField;
 import org.apache.lucene.util.BytesRef;
+import org.elasticsearch.Version;
 import org.elasticsearch.index.mapper.ParseContext.Document;
+import org.elasticsearch.test.VersionUtils;
+
+import java.io.IOException;
 
 import static org.elasticsearch.test.StreamsUtils.copyToStringFromClasspath;
+import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.notNullValue;
 
@@ -159,4 +164,78 @@ public void testSimpleWithXContentTraverse() throws Exception {
         fieldMapper = mapperService.documentMapper().mappers().getMapper("multi2.org");
         assertNotNull(fieldMapper);
     }
+
+    public void testDynamicMapperWithBadMapping() throws IOException {
+        {
+            // in 7.x versions this will issue a deprecation warning
+            Version version = VersionUtils.randomCompatibleVersion(random(), Version.V_7_0_0);
+            DocumentMapper mapper = createDocumentMapper(version, topMapping(b -> {
+                b.startArray("dynamic_templates");
+                {
+                    b.startObject();
+                    {
+                        b.startObject("test");
+                        {
+                            b.field("match_mapping_type", "string");
+                            b.startObject("mapping").field("badparam", false).endObject();
+                        }
+                        b.endObject();
+                    }
+                    b.endObject();
+                }
+                b.endArray();
+            }));
+            assertWarnings(
+                "dynamic template [test] has invalid content [{\"match_mapping_type\":\"string\",\"mapping\":{\"badparam\":false}}], " +
+                "attempted to validate it with the following match_mapping_type: [string], last error: " +
+                "[unknown parameter [badparam] on mapper [__dynamic__test] of type [null]]");
+
+            mapper.parse(source(b -> b.field("field", "foo")));
+            assertWarnings("Parameter [badparam] is used in a dynamic template mapping and has no effect on type [null]. " +
+                    "Usage will result in an error in future major versions and should be removed.");
+        }
+
+        {
+            // in 8.x it will error out
+            Exception e = expectThrows(MapperParsingException.class, () -> createMapperService(topMapping(b -> {
+                b.startArray("dynamic_templates");
+                {
+                    b.startObject();
+                    {
+                        b.startObject("test");
+                        {
+                            b.field("match_mapping_type", "string");
+                            b.startObject("mapping").field("badparam", false).endObject();
+                        }
+                        b.endObject();
+                    }
+                    b.endObject();
+                }
+                b.endArray();
+            })));
+            assertThat(e.getMessage(), containsString("dynamic template [test] has invalid content"));
+            assertThat(e.getCause().getMessage(), containsString("badparam"));
+        }
+    }
+
+    public void testDynamicRuntimeWithBadMapping() {
+        Exception e = expectThrows(MapperParsingException.class, () -> createMapperService(topMapping(b -> {
+            b.startArray("dynamic_templates");
+            {
+                b.startObject();
+                {
+                    b.startObject("test");
+                    {
+                        b.field("match_mapping_type", "string");
+                        b.startObject("runtime").field("badparam", false).endObject();
+                    }
+                    b.endObject();
+                }
+                b.endObject();
+            }
+            b.endArray();
+        })));
+        assertThat(e.getMessage(), containsString("dynamic template [test] has invalid content"));
+        assertThat(e.getCause().getMessage(), containsString("badparam"));
+    }
 }

server/src/test/java/org/elasticsearch/index/mapper/ParametrizedMapperTests.java

@@ -208,7 +208,7 @@ private static TestMapper fromMapping(String mapping, Version version, boolean f
             throw new UnsupportedOperationException();
         });
         if (fromDynamicTemplate) {
-            pc = pc.createDynamicTemplateFieldContext(pc);
+            pc = pc.createDynamicTemplateFieldContext();
         }
         return (TestMapper) new TypeParser()
             .parse("field", XContentHelper.convertToMap(JsonXContent.jsonXContent, mapping, true), pc)