EQL: make allow_no_indices true by default#63573
Merged
astefan merged 3 commits intoelastic:masterfrom Oct 12, 2020
Merged
Conversation
Irrespective of allow_no_indices value, throw VerificationException when there is no index validated
Collaborator
|
Pinging @elastic/es-ql (:Query Languages/EQL) |
costin
approved these changes
Oct 12, 2020
Member
costin
left a comment
There was a problem hiding this comment.
Left a minor comment but LGTM otherwise.
Comment on lines
+23
to
+28
| // wrap a potential index_not_found_exception with a VerificationException (expected by client) | ||
| try { | ||
| esIndex = indices.get(); | ||
| } catch (MappingException me) { | ||
| throw new VerificationException(Collections.singletonList(Failure.fail(plan, me.getMessage()))); | ||
| } |
Member
There was a problem hiding this comment.
Use indices.isValid instead to check if the index is valid and if it's not, call toString() on it which returns the message instead of the index name.
Potentially pass the listener in the method to call onFailure with the failure instead of throwing it.
| return; | ||
| } | ||
| indexResolver.resolveAsMergedMapping(indexWildcard, null, configuration.includeFrozen(), configuration.filter(), | ||
| indexResolver.resolveAsMergedMapping(indexWildcard, null, configuration.indicesOptions(), configuration.filter(), |
|
While this looks to be the behavior that we want, I'm wondering whether Again, this is a blocker for EQL detections so I'm happy with any solution, but I wanted to re-raise the question since it had been raised previously. |
Member
|
We think this solves it.In ES |
astefan
added a commit
to astefan/elasticsearch
that referenced
this pull request
Oct 12, 2020
* Allow all indices options variants Irrespective of allow_no_indices value, throw VerificationException when there is no index validated (cherry picked from commit 5ec70e5)
astefan
added a commit
that referenced
this pull request
Oct 12, 2020
rylnd
pushed a commit
to rylnd/elasticsearch
that referenced
this pull request
Oct 13, 2020
* Allow all indices options variants Irrespective of allow_no_indices value, throw VerificationException when there is no index validated
astefan
added a commit
that referenced
this pull request
Oct 14, 2020
2 tasks
rylnd
added a commit
to rylnd/kibana
that referenced
this pull request
Nov 10, 2020
These were previously needed to work around an index resolution but, but this has since been resolved upstream in elasticsearch via elastic/elasticsearch#63573.
rylnd
added a commit
to elastic/kibana
that referenced
this pull request
Nov 16, 2020
* Ensure that data is not lost when parsing EQL responses The shared search utilities expect that response data exists in the response's body field. However, in an EQL response this information also exists as a sibling to the body field, and so we must normalize this data into the body before we can leverage these utilities with EQL queries. * Remove unused EQL parameters These were previously needed to work around an index resolution but, but this has since been resolved upstream in elasticsearch via elastic/elasticsearch#63573. * Allow custom test subj for Preview Histogram to propagate to DOM Previously, custom preview histograms were passing a data-test-subj prop to our general histogram, but the general histogram did not know/care about this prop and it did not become a data property on the underlying DOM element. While most of our tests leveraged enzyme, they could still query by this react prop and everything worked as expected. However, now that we want to exercise this behavior in cypress, we need something to propagate to the DOM so that we can determine which histogram has rendered, so the prop has been updated to be `dataTestSubj`, which then becomes a data-test-subj on the histogram's panel. Tests have been updated accordingly. * Exercise Query Preview during EQL rule creation * Asserts that the preview displays a histogram * Asserts that no error toast is displayed * Add integration tests around EQL sequence signal generation * Clearer assertion * Simplify test assertion * Fix typings These were updated on an upstream refactor. Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
rylnd
added a commit
to elastic/kibana
that referenced
this pull request
Nov 17, 2020
* Ensure that data is not lost when parsing EQL responses The shared search utilities expect that response data exists in the response's body field. However, in an EQL response this information also exists as a sibling to the body field, and so we must normalize this data into the body before we can leverage these utilities with EQL queries. * Remove unused EQL parameters These were previously needed to work around an index resolution but, but this has since been resolved upstream in elasticsearch via elastic/elasticsearch#63573. * Allow custom test subj for Preview Histogram to propagate to DOM Previously, custom preview histograms were passing a data-test-subj prop to our general histogram, but the general histogram did not know/care about this prop and it did not become a data property on the underlying DOM element. While most of our tests leveraged enzyme, they could still query by this react prop and everything worked as expected. However, now that we want to exercise this behavior in cypress, we need something to propagate to the DOM so that we can determine which histogram has rendered, so the prop has been updated to be `dataTestSubj`, which then becomes a data-test-subj on the histogram's panel. Tests have been updated accordingly. * Exercise Query Preview during EQL rule creation * Asserts that the preview displays a histogram * Asserts that no error toast is displayed * Add integration tests around EQL sequence signal generation * Clearer assertion * Simplify test assertion * Fix typings These were updated on an upstream refactor. Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> # Conflicts: # x-pack/plugins/security_solution/cypress/tasks/create_new_rule.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
allow_no_indicesis nowtrueby default and, as before, can be set in the_eqlrequest as a parameter. Irrespective ofallow_no_indicesvalue, throw verification_exception when there is no index validated from the provided patterns. Ifallow_no_indicesisfalseand there is a pattern that results in a no index being returned, the Elasticsearch's mapping_exception is thrown.Fixes #62986.