Validate metdata on _msearch
#35938
Validate metdata on _msearch
#35938
Conversation
MultiSearchRequests issues through `_msearch` now validate all keys in the metadata section. Previously unknown keys were ignored while now an exception is thrown. Closes elastic#35869
s1monw
added
>bug
>breaking
:Search/Search
|
Pinging @elastic/es-search |
| [float] | ||
| ==== Multi Search Request metadata validation | ||
|
|
||
| MultiSearchRequests issues through `_msearch` now validate all keys in the metadata section. Previously unknown keys were ignored |
| } else { | ||
| // TODO we should not be lenient here and fail if there is any unknown key in the source map | ||
| throw new IllegalStateException("key [" + entry.getKey() + "] is not supported in the metadata section"); |
| } else if ("allow_no_indices".equals(entry.getKey()) || "allowNoIndices".equals(entry.getKey())) { | ||
| allowNoIndices = value; | ||
| } else if ("ignore_throttled".equals(entry.getKey()) || "ignoreThrottled".equals(entry.getKey())) { | ||
| ignoreThrottled = value; |
There was a problem hiding this comment.
Maybe we should rather improve IndicesOptions#fromMap and make sure that we don't ignore what we don't recognize? I believe we have the same problem in create and restore snapshot API. Could we make the method remove what it parses and check that the map is left empty after all the parsing? Just an idea.
There was a problem hiding this comment.
this is unrelated. In this case we need to know if the key is valid and fromMap doesn't tell me what it consumed.
There was a problem hiding this comment.
that is why I am proposing to change/improve fromMap and to make other callers of it stricter as well. It does not have to be as part of this PR but I think it would be a good change.
There was a problem hiding this comment.
not sure I understand what you mean. can you make an example?
There was a problem hiding this comment.
I haven't tried this so it may not work but I am wondering if fromMap removes what it parses from the map provided as argument, and we also update our current "parsing" code to remove the corresponding entry from the map whenever a field is parsed, we can enforce that the map needs to be empty at the end (hence making parsing strict) without having to duplicate the parsing code. Does that make sense?
MultiSearchRequests issues through
_msearchnow validate all keysin the metadata section. Previously unknown keys were ignored
while now an exception is thrown.
Closes #35869