Skip to content

Commit

Permalink
Tolerate empty types array in Watch definitions (elastic#83524)
Browse files Browse the repository at this point in the history
In 6.x internal system components created Watches with an empty
types array in their definition. Types do not exist in 8.x, so
these system-created Watches would need to be modified in 7.x
to remove the types field. Because doing such modifications as
a secret background task could be risky, instead the 8.x Watch
parser will tolerate and ignore an empty types array. It is
clear that an empty types array can be considered identical to
typeless. Non-empty types arrays will still be considered fatal
errors in 8.x, as silently ignoring them could change the meaning
of the search.

Fixes elastic#83235
  • Loading branch information
droberts195 committed Feb 9, 2022
1 parent 315a361 commit 8eef2eb
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 0 deletions.
6 changes: 6 additions & 0 deletions docs/changelog/83524.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
pr: 83524
summary: Tolerate empty types array in Watch definitions
area: Watcher
type: bug
issues:
- 83235
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.logging.DeprecationCategory;
import org.elasticsearch.common.logging.DeprecationLogger;
import org.elasticsearch.core.Nullable;
import org.elasticsearch.script.Script;
import org.elasticsearch.script.ScriptType;
Expand Down Expand Up @@ -43,6 +45,10 @@ public class WatcherSearchTemplateRequest implements ToXContentObject {
private final BytesReference searchSource;
private boolean restTotalHitsAsInt = true;

private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(WatcherSearchTemplateRequest.class);
static final String TYPES_DEPRECATION_MESSAGE =
"[types removal] Specifying empty types array in a watcher search request is deprecated.";

public WatcherSearchTemplateRequest(
String[] indices,
SearchType searchType,
Expand Down Expand Up @@ -190,6 +196,17 @@ public static WatcherSearchTemplateRequest fromXContent(XContentParser parser, S
);
}
}
} else if (TYPES_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
// Tolerate an empty types array, because some watches created internally in 6.x have
// an empty types array in their search, and it's clearly equivalent to typeless.
if (parser.nextToken() != XContentParser.Token.END_ARRAY) {
throw new ElasticsearchParseException(
"could not read search request. unsupported non-empty array field [" + currentFieldName + "]"
);
}
// Empty types arrays still generate the same deprecation warning they did in 7.x.
// Ideally they should be removed from the definition.
deprecationLogger.critical(DeprecationCategory.PARSING, "watcher_search_input", TYPES_DEPRECATION_MESSAGE);
} else {
throw new ElasticsearchParseException(
"could not read search request. unexpected array field [" + currentFieldName + "]"
Expand Down Expand Up @@ -272,6 +289,7 @@ public int hashCode() {
}

private static final ParseField INDICES_FIELD = new ParseField("indices");
private static final ParseField TYPES_FIELD = new ParseField("types");
private static final ParseField BODY_FIELD = new ParseField("body");
private static final ParseField SEARCH_TYPE_FIELD = new ParseField("search_type");
private static final ParseField INDICES_OPTIONS_FIELD = new ParseField("indices_options");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,18 @@
*/
package org.elasticsearch.xpack.watcher.support.search;

import org.elasticsearch.ElasticsearchParseException;
import org.elasticsearch.action.search.SearchType;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.xcontent.XContentParser;
import org.elasticsearch.xcontent.json.JsonXContent;

import java.io.IOException;
import java.util.List;
import java.util.Map;

import static java.util.Collections.singletonMap;
import static org.hamcrest.Matchers.arrayContaining;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is;

Expand All @@ -32,6 +35,49 @@ public void testFromXContentWithTemplateCustomLang() throws IOException {
assertTemplate(source, "custom-script", "painful", singletonMap("bar", "baz"));
}

public void testFromXContentWithEmptyTypes() throws IOException {
String source = """
{
"search_type" : "query_then_fetch",
"indices" : [ ".ml-anomalies-*" ],
"types" : [ ],
"body" : {
"query" : {
"bool" : {
"filter" : [ { "term" : { "job_id" : "my-job" } }, { "range" : { "timestamp" : { "gte" : "now-30m" } } } ]
}
}
}
}
""";
try (XContentParser parser = createParser(JsonXContent.jsonXContent, source)) {
parser.nextToken();
WatcherSearchTemplateRequest result = WatcherSearchTemplateRequest.fromXContent(parser, randomFrom(SearchType.values()));
assertThat(result.getIndices(), arrayContaining(".ml-anomalies-*"));
}
}

public void testFromXContentWithNonEmptyTypes() throws IOException {
String source = """
{
"search_type" : "query_then_fetch",
"indices" : [ "my-index" ],
"types" : [ "my-type" ],
"body" : {
"query" : { "match_all" : {} }
}
}
""";
try (XContentParser parser = createParser(JsonXContent.jsonXContent, source)) {
parser.nextToken();
ElasticsearchParseException e = expectThrows(
ElasticsearchParseException.class,
() -> WatcherSearchTemplateRequest.fromXContent(parser, randomFrom(SearchType.values()))
);
assertThat(e.getMessage(), is("could not read search request. unsupported non-empty array field [types]"));
}
}

public void testDefaultHitCountsDefaults() throws IOException {
assertHitCount("{}", true);
}
Expand Down Expand Up @@ -61,4 +107,8 @@ private void assertTemplate(String source, String expectedScript, String expecte
assertThat(result.getTemplate().getParams(), equalTo(expectedParams));
}
}

protected List<String> filteredWarnings() {
return List.of(WatcherSearchTemplateRequest.TYPES_DEPRECATION_MESSAGE);
}
}

0 comments on commit 8eef2eb

Please sign in to comment.