Skip to content

Commit 5ec70e5

Browse files
authored
EQL: make allow_no_indices true by default (elastic#63573)
* Allow all indices options variants Irrespective of allow_no_indices value, throw VerificationException when there is no index validated
1 parent 1e949ca commit 5ec70e5

12 files changed

Lines changed: 179 additions & 22 deletions

File tree

client/rest-high-level/src/main/java/org/elasticsearch/client/eql/EqlSearchRequest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@
3434
public class EqlSearchRequest implements Validatable, ToXContentObject {
3535

3636
private String[] indices;
37-
private IndicesOptions indicesOptions = IndicesOptions.fromOptions(true, false, true, false);
37+
private IndicesOptions indicesOptions = IndicesOptions.fromOptions(true, true, true, false);
3838

3939
private QueryBuilder filter = null;
4040
private String timestampField = "@timestamp";
Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License;
4+
* you may not use this file except in compliance with the Elastic License.
5+
*/
6+
7+
package org.elasticsearch.test.eql;
8+
9+
import org.elasticsearch.client.Request;
10+
import org.elasticsearch.client.Response;
11+
import org.elasticsearch.client.ResponseException;
12+
import org.elasticsearch.common.Strings;
13+
import org.elasticsearch.common.settings.Settings;
14+
import org.elasticsearch.common.xcontent.XContentBuilder;
15+
import org.elasticsearch.common.xcontent.json.JsonXContent;
16+
import org.elasticsearch.test.rest.ESRestTestCase;
17+
import org.junit.Before;
18+
19+
import java.io.IOException;
20+
21+
import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
22+
import static org.elasticsearch.xpack.ql.util.StringUtils.EMPTY;
23+
import static org.hamcrest.Matchers.containsString;
24+
import static org.hamcrest.Matchers.equalTo;
25+
26+
public abstract class EqlRestValidationTestCase extends ESRestTestCase {
27+
28+
// TODO: handle for existent indices the patterns "test,inexistent", "inexistent,test" that seem to work atm as is
29+
private static final String[] existentIndexName = new String[] {"test,inexistent*", "test*,inexistent*", "inexistent*,test"};
30+
private static final String[] inexistentIndexName = new String[] {"inexistent", "inexistent*", "inexistent1*,inexistent2*"};
31+
32+
@Before
33+
public void prepareIndices() throws IOException {
34+
createIndex("test", Settings.EMPTY);
35+
36+
Object[] fieldsAndValues = new Object[] {"event_type", "my_event", "@timestamp", "2020-10-08T12:35:48Z", "val", 0};
37+
XContentBuilder document = jsonBuilder().startObject();
38+
for (int i = 0; i < fieldsAndValues.length; i += 2) {
39+
document.field((String) fieldsAndValues[i], fieldsAndValues[i + 1]);
40+
}
41+
document.endObject();
42+
final Request request = new Request("POST", "/test/_doc/" + 0);
43+
request.setJsonEntity(Strings.toString(document));
44+
assertOK(client().performRequest(request));
45+
46+
assertOK(adminClient().performRequest(new Request("POST", "/test/_refresh")));
47+
}
48+
49+
public void testDefaultIndicesOptions() throws IOException {
50+
String message = "\"root_cause\":[{\"type\":\"verification_exception\",\"reason\":\"Found 1 problem\\nline -1:-1: Unknown index";
51+
assertErrorMessageOnInexistentIndices(EMPTY, true, message, EMPTY);
52+
assertErrorMessageOnExistentIndices("?allow_no_indices=false", false, message, EMPTY);
53+
assertValidRequestOnExistentIndices(EMPTY);
54+
}
55+
56+
public void testAllowNoIndicesOption() throws IOException {
57+
boolean allowNoIndices = randomBoolean();
58+
boolean setAllowNoIndices = randomBoolean();
59+
boolean isAllowNoIndices = allowNoIndices || setAllowNoIndices == false;
60+
61+
String allowNoIndicesTrueMessage = "\"root_cause\":[{\"type\":\"verification_exception\",\"reason\":"
62+
+ "\"Found 1 problem\\nline -1:-1: Unknown index";
63+
String allowNoIndicesFalseMessage = "\"root_cause\":[{\"type\":\"index_not_found_exception\",\"reason\":\"no such index";
64+
String reqParameter = setAllowNoIndices ? "?allow_no_indices=" + allowNoIndices : EMPTY;
65+
66+
assertErrorMessageOnInexistentIndices(reqParameter, isAllowNoIndices, allowNoIndicesTrueMessage, allowNoIndicesFalseMessage);
67+
if (isAllowNoIndices) {
68+
assertValidRequestOnExistentIndices(reqParameter);
69+
}
70+
}
71+
72+
private void assertErrorMessageOnExistentIndices(String reqParameter, boolean isAllowNoIndices, String allowNoIndicesTrueMessage,
73+
String allowNoIndicesFalseMessage) throws IOException {
74+
assertErrorMessages(existentIndexName, reqParameter, isAllowNoIndices, allowNoIndicesTrueMessage, allowNoIndicesFalseMessage);
75+
}
76+
77+
private void assertErrorMessageOnInexistentIndices(String reqParameter, boolean isAllowNoIndices, String allowNoIndicesTrueMessage,
78+
String allowNoIndicesFalseMessage) throws IOException {
79+
assertErrorMessages(inexistentIndexName, reqParameter, isAllowNoIndices, allowNoIndicesTrueMessage, allowNoIndicesFalseMessage);
80+
}
81+
82+
private void assertErrorMessages(String[] indices, String reqParameter, boolean isAllowNoIndices, String allowNoIndicesTrueMessage,
83+
String allowNoIndicesFalseMessage) throws IOException {
84+
for (String indexName : indices) {
85+
final Request request = createRequest(indexName, reqParameter);
86+
ResponseException exc = expectThrows(ResponseException.class, () -> client().performRequest(request));
87+
88+
assertThat(exc.getResponse().getStatusLine().getStatusCode(), equalTo(isAllowNoIndices ? 400 : 404));
89+
// TODO add the index name to the message to be checked. Waiting on https://github.com/elastic/elasticsearch/issues/63529
90+
assertThat(exc.getMessage(), containsString(isAllowNoIndices ? allowNoIndicesTrueMessage : allowNoIndicesFalseMessage));
91+
}
92+
}
93+
94+
private Request createRequest(String indexName, String reqParameter) throws IOException {
95+
final Request request = new Request("POST", "/" + indexName + "/_eql/search" + reqParameter);
96+
request.setJsonEntity(Strings.toString(JsonXContent.contentBuilder()
97+
.startObject()
98+
.field("event_category_field", "event_type")
99+
.field("query", "my_event where true")
100+
.endObject()));
101+
102+
return request;
103+
}
104+
105+
private void assertValidRequestOnExistentIndices(String reqParameter) throws IOException {
106+
for (String indexName : existentIndexName) {
107+
final Request request = createRequest(indexName, reqParameter);
108+
Response response = client().performRequest(request);
109+
assertOK(response);
110+
}
111+
}
112+
}
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
package org.elasticsearch.xpack.eql;
2+
3+
import org.elasticsearch.test.eql.EqlRestValidationTestCase;
4+
5+
public class EqlRestValidationIT extends EqlRestValidationTestCase {
6+
7+
}

x-pack/plugin/eql/qa/security/src/javaRestTest/java/org/elasticsearch/xpack/eql/AsyncEqlSecurityIT.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ private void testCase(String user, String other) throws Exception {
8989
}
9090
ResponseException exc = expectThrows(ResponseException.class,
9191
() -> submitAsyncEqlSearch("index-" + other, "*", TimeValue.timeValueSeconds(10), user));
92-
assertThat(exc.getResponse().getStatusLine().getStatusCode(), equalTo(404));
92+
assertThat(exc.getResponse().getStatusLine().getStatusCode(), equalTo(400));
9393
}
9494

9595
static String extractResponseId(Response response) throws IOException {
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
package org.elasticsearch.xpack.eql;
2+
3+
import org.elasticsearch.common.settings.Settings;
4+
import org.elasticsearch.test.eql.EqlRestValidationTestCase;
5+
6+
import static org.elasticsearch.xpack.eql.SecurityUtils.secureClientSettings;
7+
8+
public class EqlRestValidationIT extends EqlRestValidationTestCase {
9+
10+
@Override
11+
protected Settings restClientSettings() {
12+
return secureClientSettings();
13+
}
14+
}

x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/action/EqlSearchRequest.java

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,7 @@ public class EqlSearchRequest extends ActionRequest implements IndicesRequest.Re
4040
public static TimeValue DEFAULT_KEEP_ALIVE = TimeValue.timeValueDays(5);
4141

4242
private String[] indices;
43-
private IndicesOptions indicesOptions = IndicesOptions.fromOptions(true,
44-
false, true, false);
43+
private IndicesOptions indicesOptions = IndicesOptions.fromOptions(true, true, true, false);
4544

4645
private QueryBuilder filter = null;
4746
private String timestampField = FIELD_TIMESTAMP;
@@ -119,13 +118,8 @@ public ActionRequestValidationException validate() {
119118

120119
if (indicesOptions == null) {
121120
validationException = addValidationError("indicesOptions is null", validationException);
122-
} else {
123-
if (indicesOptions.allowNoIndices()) {
124-
validationException = addValidationError("allowNoIndices must be false", validationException);
125-
}
126121
}
127122

128-
129123
if (query == null || query.isEmpty()) {
130124
validationException = addValidationError("query is null or empty", validationException);
131125
}

x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/analysis/PreAnalyzer.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,21 @@
66

77
package org.elasticsearch.xpack.eql.analysis;
88

9+
import org.elasticsearch.xpack.ql.common.Failure;
910
import org.elasticsearch.xpack.ql.index.IndexResolution;
1011
import org.elasticsearch.xpack.ql.plan.logical.EsRelation;
1112
import org.elasticsearch.xpack.ql.plan.logical.LogicalPlan;
1213
import org.elasticsearch.xpack.ql.plan.logical.UnresolvedRelation;
1314

15+
import java.util.Collections;
16+
1417
public class PreAnalyzer {
1518

1619
public LogicalPlan preAnalyze(LogicalPlan plan, IndexResolution indices) {
20+
// wrap a potential index_not_found_exception with a VerificationException (expected by client)
21+
if (indices.isValid() == false) {
22+
throw new VerificationException(Collections.singletonList(Failure.fail(plan, indices.toString())));
23+
}
1724
if (plan.analyzed() == false) {
1825
final EsRelation esRelation = new EsRelation(plan.source(), indices.get(), false);
1926
// FIXME: includeFrozen needs to be set already

x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/plugin/TransportEqlSearchAction.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,6 @@ public static void operation(PlanExecutor planExecutor, EqlSearchTask task, EqlS
108108
ZoneId zoneId = DateUtils.of("Z");
109109
QueryBuilder filter = request.filter();
110110
TimeValue timeout = TimeValue.timeValueSeconds(30);
111-
boolean includeFrozen = request.indicesOptions().ignoreThrottled() == false;
112111
String clientId = null;
113112

114113
ParserParams params = new ParserParams(zoneId)
@@ -118,8 +117,8 @@ public static void operation(PlanExecutor planExecutor, EqlSearchTask task, EqlS
118117
.size(request.size())
119118
.fetchSize(request.fetchSize());
120119

121-
EqlConfiguration cfg = new EqlConfiguration(request.indices(), zoneId, username, clusterName, filter, timeout, includeFrozen,
122-
request.fetchSize(), clientId, new TaskId(nodeId, task.getId()), task);
120+
EqlConfiguration cfg = new EqlConfiguration(request.indices(), zoneId, username, clusterName, filter, timeout,
121+
request.indicesOptions(), request.fetchSize(), clientId, new TaskId(nodeId, task.getId()), task);
123122
planExecutor.eql(cfg, request.query(), params, wrap(r -> listener.onResponse(createResponse(r, task.getExecutionId())),
124123
listener::onFailure));
125124
}

x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/session/EqlConfiguration.java

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
package org.elasticsearch.xpack.eql.session;
88

9+
import org.elasticsearch.action.support.IndicesOptions;
910
import org.elasticsearch.common.Nullable;
1011
import org.elasticsearch.common.Strings;
1112
import org.elasticsearch.common.unit.TimeValue;
@@ -20,7 +21,7 @@ public class EqlConfiguration extends org.elasticsearch.xpack.ql.session.Configu
2021
private final String[] indices;
2122
private final TimeValue requestTimeout;
2223
private final String clientId;
23-
private final boolean includeFrozenIndices;
24+
private final IndicesOptions indicesOptions;
2425
private final TaskId taskId;
2526
private final EqlSearchTask task;
2627
private final int fetchSize;
@@ -29,15 +30,15 @@ public class EqlConfiguration extends org.elasticsearch.xpack.ql.session.Configu
2930
private final QueryBuilder filter;
3031

3132
public EqlConfiguration(String[] indices, ZoneId zi, String username, String clusterName, QueryBuilder filter, TimeValue requestTimeout,
32-
boolean includeFrozen, int fetchSize, String clientId, TaskId taskId,
33+
IndicesOptions indicesOptions, int fetchSize, String clientId, TaskId taskId,
3334
EqlSearchTask task) {
3435
super(zi, username, clusterName);
3536

3637
this.indices = indices;
3738
this.filter = filter;
3839
this.requestTimeout = requestTimeout;
3940
this.clientId = clientId;
40-
this.includeFrozenIndices = includeFrozen;
41+
this.indicesOptions = indicesOptions;
4142
this.taskId = taskId;
4243
this.task = task;
4344
this.fetchSize = fetchSize;
@@ -67,8 +68,8 @@ public String clientId() {
6768
return clientId;
6869
}
6970

70-
public boolean includeFrozen() {
71-
return includeFrozenIndices;
71+
public IndicesOptions indicesOptions() {
72+
return indicesOptions;
7273
}
7374

7475
public boolean isCancelled() {

x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/session/EqlSession.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ private <T> void preAnalyze(LogicalPlan parsed, ActionListener<LogicalPlan> list
100100
listener.onFailure(new TaskCancelledException("cancelled"));
101101
return;
102102
}
103-
indexResolver.resolveAsMergedMapping(indexWildcard, null, configuration.includeFrozen(), configuration.filter(),
103+
indexResolver.resolveAsMergedMapping(indexWildcard, null, configuration.indicesOptions(), configuration.filter(),
104104
map(listener, r -> preAnalyzer.preAnalyze(parsed, r))
105105
);
106106
}

0 commit comments

Comments
 (0)