[SIEM][Detections] Adds large list support using REST endpoints #62552
Conversation
This file contains 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
FrankHassanabad
changed the title
FrankHassanabad
changed the title
…e/api from feedback
|
Link to issue https://github.com/elastic/siem-team/issues/615 |
|
@elasticmachine merge upstream |
This comment has been minimized.
This comment has been minimized.
spong
reviewed
Apr 23, 2020
…a into add-list-endpoints
yctercero
approved these changes
Apr 24, 2020
| '@typescript-eslint/no-useless-constructor': 'error', | ||
| '@typescript-eslint/unified-signatures': 'error', | ||
| '@typescript-eslint/explicit-function-return-type': 'error', | ||
| '@typescript-eslint/no-non-null-assertion': 'error', |
There was a problem hiding this comment.
Look forward to having some of these over in the detections engine too :)
| import * as t from 'io-ts'; | ||
|
|
||
| export const listItemIndexExistSchema = t.type({ | ||
| lists_index: t.boolean, |
There was a problem hiding this comment.
Nit - Would you want these changed to be singular list... to match the changes you made in the ConfigSchema to make them singular?
There was a problem hiding this comment.
Yeah I will change it to singular form. I left the REST endpoint plural and folder naming but made everything else singular form.
| }); | ||
| } else if (type != null) { | ||
| const { filename } = request.body.file.hapi; | ||
| // TODO: Should we prevent the same file from being uploaded multiple times? |
There was a problem hiding this comment.
Could we surface up more info on imports? Even on a successful import I wasn't seeing any sort of response message showing up. And maybe on duplicates (looks like right now we're not allowing and just returning the existing list?) also add a message letting user know of current behavior?
There was a problem hiding this comment.
Duplicates are tricky as its an async operation when it does imports at a particular lower level. The counting of the duplicates and what not become wrong values or false values as the streams can be independent parallel things.
I noticed a lot of odd streaming things and I don't want to add penalties of waiting in any other areas as I think we want to introduce streaming of large list items as a feature from the REST endpoint all the way to the backend and hopefully 🤞back-pressure works auto-magically with the streams from the browser to the backend.
If the imports become pure streaming in, we might be able to re-introduce duplicate counting but the way this works right now it "sinks" all the lists into memory on the import and then begins streaming that list into the backend and returns the 200 so it does not time out.
There was a problem hiding this comment.
I will bubble up more information here on an import where it auto-creates the list. I think that is a good idea.
This comment has been minimized.
This comment has been minimized.
…ng message and a few import fix ups
💛 Build succeeded, but was flaky
Test FailuresKibana Pipeline / kibana-xpack-agent / Firefox XPack UI Functional Tests.x-pack/test/functional/apps/spaces/enter_space·ts.Spaces app Enter Space "after each" hook for "falls back to the default home page when the configured default route is malformed"Standard OutStack TraceHistory
To update your PR or re-run it, just comment with: |
This was referenced Apr 28, 2020
FrankHassanabad
added a commit
that referenced
this pull request
Apr 29, 2020
…#62552) (#64710) * [SIEM][Detections] Adds large list support using REST endpoints * Adds large list support using REST endpoints. Status: --- * Currently ready to be merged behind the feature flag of it being disabled with ongoing work happening after it is merged. * REST Endpoints shouldn't have large refactoring at this point * Team meeting occurred where the pieces were discussed in person. What is left? --- - [ ] Add other data types. At the moment `ip` and `keyword` are the two types of lists. See: https://www.elastic.co/guide/en/elasticsearch/reference/current/mapping-types.html - [x] Unit tests - [x] Lots of misc TODO's in the code base still - [ ] Import loads everything into memory first when it should attempt streaming - [ ] Add end to end backend tests - [x] Add transform and io-ts validation for returns Testing --- Ensure you set this in your ENV before starting Kibana: ```ts export ELASTIC_XPACK_SIEM_LISTS_FEATURE=true ``` Download or create a large list file such as this one filled with IP's: https://cinsscore.com/list/ci-badguys.txt Go to your REST endpoint folder of scripts: ```ts cd kibana/x-pack/plugins/lists/server/scripts ``` Do a hard reset: ```ts ./hard_reset ``` Then import it as either a data type of `ip`: ```ts ./import_list_items_by_filename.sh ip ~/Downloads/ci-badguys-smaller.txt ``` Or as a `keyword` ```ts ./import_list_items_by_filename.sh keyword ~/Downloads/ci-badguys-smaller.txt ``` Then you can export it through: ```ts ./export_list_items.sh ci-badgusy-smaller.txt ``` For all the other endpoints and testing of the CRUD operations you have access to: ```ts delete_all_lists.sh delete_list.sh delete_list_index.sh delete_list_item.sh delete_list_item_by_id.sh delete_list_item_by_value.sh export_list_items.sh export_list_items_to_file.sh get_list.sh get_list_item_by_id.sh get_list_item_by_value.sh import_list_items.sh import_list_items_by_filename.sh lists_index_exists.sh patch_list.sh patch_list_item.sh post_list.sh post_list_index.sh post_list_item.sh ``` Delete any items that are not applicable to this PR. - [ ] [Unit or functional tests](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility) were updated or added to match the most common scenarios - [ ] This was checked for breaking API changes and was [labeled appropriately](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#release-notes-process) * Delete CODEOWNERS Code owners should not be backported Co-authored-by: Elastic Machine <[email protected]>
This was referenced May 8, 2020
MindyRS
added
the
Team: SecuritySolution
|
Pinging @elastic/security-solution (Team: SecuritySolution) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Status:
What is left?
ipandkeywordare the two types of lists. See: https://www.elastic.co/guide/en/elasticsearch/reference/current/mapping-types.htmlTesting
Ensure you set this in your ENV before starting Kibana:
Download or create a large list file such as this one filled with IP's:
https://cinsscore.com/list/ci-badguys.txt
Go to your REST endpoint folder of scripts:
Do a hard reset:
Then import it as either a data type of
ip:Or as a
keywordThen you can export it through:
For all the other endpoints and testing of the CRUD operations you have access to:
Checklist
Delete any items that are not applicable to this PR.
For maintainers