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

[Security Solution] Use sourcerer selected indices in resolver #90727

Conversation

kqualters-elastic
Copy link
Contributor

@kqualters-elastic kqualters-elastic commented Feb 9, 2021

Summary

fixes #89337

This pr changes the list of indices passed to resolver to come from sourcer, matching the rest of the SIEM app. Previously this used only the default indices. Also fixes a bug in the selectors used by data fetching middleware that caused api requests to always use an empty array. Event requests now use the same set of indices used for the tree the user is viewing.

image

Checklist

Delete any items that are not applicable to this PR.

@kqualters-elastic kqualters-elastic added release_note:skip Skip the PR/issue when compiling release notes Feature:Resolver Security Solution Resolver feature v7.12.0 Team:Threat Hunting Security Solution Threat Hunting Team labels Feb 9, 2021
@kqualters-elastic kqualters-elastic requested a review from a team as a code owner February 9, 2021 07:37
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-threat-hunting (Team:Threat Hunting)

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-threat-hunting (Feature:Resolver)

@kqualters-elastic kqualters-elastic changed the title [Security Solution] Use sourcerer selected indices in resolver fixes https://github.com/elastic/kibana/issues/89337 [Security Solution] Use sourcerer selected indices in resolver fixes elastic/kibana#89337 Feb 9, 2021
@kqualters-elastic kqualters-elastic changed the title [Security Solution] Use sourcerer selected indices in resolver fixes elastic/kibana#89337 [Security Solution] Use sourcerer selected indices in resolver Feb 9, 2021
Copy link
Contributor

@jonathan-buttner jonathan-buttner left a comment

Choose a reason for hiding this comment

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

Thanks for the tests. Could you post a pic of the network request including the indices?

@@ -336,10 +349,22 @@ export const timeRangeFilters = createSelector(
/**
* The indices to use for the requests with the backend.
*/
export const treeParamterIndices = createSelector(treeParametersToFetch, (parameters) => {
export const treeParameterIndices = createSelector(treeParametersToFetch, (parameters) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

💯

Copy link
Contributor

Choose a reason for hiding this comment

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

Spelling is hard 😂 Thanks

@@ -71,6 +78,12 @@ const resolverTreeResponse = (state: DataState): NewResolverTree | undefined =>
return state.tree?.lastResponse?.successful ? state.tree?.lastResponse.result : undefined;
};

const lastResponseIndices = (state: DataState): string[] | undefined => {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm probably missing something but could we get away without needing to look at the last response? Could we just use the current indices all the time?

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 this helps us maintain the original indices which were used to successfully get data back. These are the "current" indices technically as far as I can see

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jonathan-buttner The reason for doing this is because all of these event api requests are inherently tied to the tree the user is currently viewing, if a separate set of indices is passed to resolver as the middlewares are running, that would be a bug. Should not happen currently, as changing the indices with sourcerer forces the store to be recreated, but that won't necessarily always be the case. Added the sequence of actions that show the need for this here: https://github.com/elastic/kibana/pull/90727/files#diff-0a7459663dc51fee4faacb117efa940c3a4f5dfdfa1abc44ea873507e8de80aaR706

@kqualters-elastic
Copy link
Contributor Author

@jonathan-buttner added a screenshot of the request

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
securitySolution 7.5MB 7.5MB +1.1KB

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@@ -280,6 +280,7 @@ export enum TimelineId {
active = 'timeline-1',
casePage = 'timeline-case',
test = 'test', // Reserved for testing purposes
test2 = 'test2',
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe name this alternateTest or updateTester or something?

@kqualters-elastic kqualters-elastic merged commit f563a82 into elastic:master Feb 10, 2021
@kqualters-elastic kqualters-elastic deleted the resolver-use-sourcerer-selected-indices branch February 10, 2021 16:15
kqualters-elastic added a commit to kqualters-elastic/kibana that referenced this pull request Feb 10, 2021
…ic#90727)

* Use sourcer indices

* Add indices to panel requests

* Use a separate indices selector for resolver events

* Use valid timeline id in tests

* Update TimelineId type usage, make selector test clearer

* Update tests to use TimelineId type
kqualters-elastic added a commit that referenced this pull request Feb 11, 2021
…#90727) (#91009)

* [Security Solution] Use sourcerer selected indices in resolver  (#90727)

* Use sourcer indices

* Add indices to panel requests

* Use a separate indices selector for resolver events

* Use valid timeline id in tests

* Update TimelineId type usage, make selector test clearer

* Update tests to use TimelineId type

* Remove non-existant 7.11 mocked import
kqualters-elastic added a commit that referenced this pull request Feb 11, 2021
… (#91008)

* Use sourcer indices

* Add indices to panel requests

* Use a separate indices selector for resolver events

* Use valid timeline id in tests

* Update TimelineId type usage, make selector test clearer

* Update tests to use TimelineId type
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Resolver Security Solution Resolver feature release_note:skip Skip the PR/issue when compiling release notes Team:Threat Hunting Security Solution Threat Hunting Team v7.12.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Analyzer does not include indices to use in requests
5 participants