Dynamic Template accepts invalid mapping values

[dmarkhas]

Elasticsearch version: 5.3.0

Plugins installed: []

JVM version: 1.8.0.92

OS version: SLES11SP4

Description of the problem including expected versus actual behavior:

When creating a dynamic template, a user can provide an invalid value to the "index" parameter (true/false instead of "analzyed","not_analyzed"). ElasticSearch accepts the template and does not warn about the invalid value, but any documents that will match this template will be discarded.

ElasticSearch should validate the template and reject it if it's malformed.

Steps to reproduce:

1. Create a dynamic template as follows
   {
   "string_fields": {
   "mapping": {
   "index": true,
   "type": "text",
   "fields": {
   "raw": {
   "ignore_above": 10000,
   "index": "not_analyzed",
   "type": "keyword"
   }
   }
   },
   "match_mapping_type": "string",
   "match": "*"
   }
   }
2. Index any document with a string field
3. The following exception will appear in the log:

java.lang.IllegalArgumentException: Can't parse [index] value [true] for field [YOUR_FIELD_NAME_HERE], expected [no], [not_analyzed] or [analyzed]
at org.elasticsearch.index.mapper.StringFieldMapper$TypeParser.parse(StringFieldMapper.java:210) ~[elasticsearch-5.3.0.jar:5.3.0]

[jimczi]

Thanks @dmarkhas !
Template validation is difficult to achieve with the current model but we're working on an improvement that should allow more validations when the template is created:
#21105

[dmarkhas]

@jimczi I wasn't aware of the other open issue, but just to clarify the issue here is not related to the validity of the logic of a given template in the context of all other templates that exist, but the fact that it actually accepts invalid values (even invalid types) for some of the fields - a field that should be a string accepts a boolean value, which has no meaning....

[jimczi]

Oh right @dmarkhas, I think this is due to a BWC layer that was added to set the type to keyword automatically when not_analyzed is set. Though it seems that this option is not removed from the final mapping. @jpountz can you take a look ?

[jpountz]

Yeah this is a known issue unfortunately, there is a similar bug reported at #17411. I agree we should fix it. The thing that makes validation hard is that in some cases the type of the field is not known until a document is parsed. However in your case, you specify the match_mapping_type explicitly, so even if that does not fix the issue entirely, we should probably try to at least validate mappings when an explicit match_mapping_type is supplied.

[dmarkhas]

Thanks @jpountz.
I believe the index property can only accept a finite set of values ("no","not_analyzed" and "analyzed"), so the basic validation that the supplied value is indeed part of that set should be quite simple, no? (forgive me if I'm missing something obvious :) )

[jpountz]

@dmarkhas This is correct (actually it is even simpler in 5.x as the only options are true/false) for fields that support the index property, but eg. object does not support it.

[PhaedrusTheGreek]

6.0 error for reference

{
  "error": {
    "root_cause": [
      {
        "type": "mapper_parsing_exception",
        "reason": "failed to parse"
      }
    ],
    "type": "mapper_parsing_exception",
    "reason": "failed to parse",
    "caused_by": {
      "type": "illegal_argument_exception",
      "reason": "Could not convert [<FIELD>.index] to boolean",
      "caused_by": {
        "type": "illegal_argument_exception",
        "reason": "Failed to parse value [analyzed] as only [true] or [false] are allowed."
      }
    }
  },
  "status": 400
}

[spinscale]

another issue is, that when upgrading to 6.0, you might accidentally have an old template lying around that then breaks future indexing

PUT _template/foo
{
  "index_patterns": [
    "foo*"
  ],
  "mappings": {
    "bar": {
      "dynamic_templates": [
        {
          "notanalyzed": {
            "match_mapping_type": "string",
            "mapping": {
              "index": "not_analyzed",
              "type": "string"
            },
            "match": "*"
          }
        }
      ]
    }
  }
}

PUT foo/bar/1
{
  "bar" : "anystring"
}

returns

{
  "error": {
    "root_cause": [
      {
        "type": "mapper_parsing_exception",
        "reason": "failed to find type parsed [string] for [bar]"
      }
    ],
    "type": "mapper_parsing_exception",
    "reason": "failed to find type parsed [string] for [bar]"
  },
  "status": 400
}

[T3rm1]

So nobody decided to fix this severe problem (the one spinscale is refering to) before 6.0? For real? This is exactly what is causing hundreds of problems for me now. This should have been a blocker for the release.