Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EQL: Change default indices options #63192

Merged
merged 4 commits into from
Oct 5, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
public class EqlSearchRequest implements Validatable, ToXContentObject {

private String[] indices;
private IndicesOptions indicesOptions = IndicesOptions.fromOptions(false, false, true, false);
private IndicesOptions indicesOptions = IndicesOptions.fromOptions(true, false, true, false);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@astefan the change inside EQL high-level rest client.


private QueryBuilder filter = null;
private String timestampField = "@timestamp";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,20 @@ setup:
- match: {hits.sequences.1.events.0._id: "2"}
- match: {hits.sequences.1.events.1._id: "3"}

---
"Execute EQL sequence by default ignores unavailable indices"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing : at the end

- do:
eql.search:
index: eql_test,non_existing
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@matriv the integration test against non-existing indices

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant to query against only unavailable indices, just for completeness.

body:
query: 'sequence by valid [process where user == "SYSTEM"] [process where true]'
- match: {timed_out: false}
- match: {hits.total.value: 1}
- match: {hits.total.relation: "eq"}
- match: {hits.sequences.0.join_keys.0: true}
- match: {hits.sequences.0.events.0._id: "2"}
- match: {hits.sequences.0.events.1._id: "3"}

---
"Execute EQL sequence with boolean key.":
- do:
Expand All @@ -96,7 +110,6 @@ setup:
- match: {hits.sequences.0.join_keys.0: true}
- match: {hits.sequences.0.events.0._id: "2"}
- match: {hits.sequences.0.events.1._id: "3"}

---
"Execute some EQL in async mode":
- do:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,8 @@ private void testCase(String user, String other) throws Exception {
}
ResponseException exc = expectThrows(ResponseException.class,
() -> submitAsyncEqlSearch("index-" + other, "*", TimeValue.timeValueSeconds(10), user));
assertThat(exc.getResponse().getStatusLine().getStatusCode(), equalTo(403));
assertThat(exc.getMessage(), containsString("unauthorized"));
assertThat(exc.getResponse().getStatusLine().getStatusCode(), equalTo(404));
//assertThat(exc.getMessage(), containsString("unauthorized"));
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@imotov not sure why this fails now... Essentially by changing the index option ignoreUnavailable from false to true the 403/unauthorized became a 404...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is because we ignore unavailable indices and ended up with no indices at all, which leads to 404.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And previously we did not ignore them but because we couldn't get access to them to verify their existence we got the 403...Make sense.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we remove it then?

}

static String extractResponseId(Response response) throws IOException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public class EqlSearchRequest extends ActionRequest implements IndicesRequest.Re
public static TimeValue DEFAULT_KEEP_ALIVE = TimeValue.timeValueDays(5);

private String[] indices;
private IndicesOptions indicesOptions = IndicesOptions.fromOptions(false,
private IndicesOptions indicesOptions = IndicesOptions.fromOptions(true,
false, true, false);

private QueryBuilder filter = null;
Expand Down Expand Up @@ -123,8 +123,13 @@ public ActionRequestValidationException validate() {

if (indicesOptions == null) {
validationException = addValidationError("indicesOptions is null", validationException);
} else {
if (indicesOptions.allowNoIndices()) {
validationException = addValidationError("allowNoIndices must be false", validationException);
}
}


if (query == null || query.isEmpty()) {
validationException = addValidationError("query is null or empty", validationException);
}
Expand Down