-
Notifications
You must be signed in to change notification settings - Fork 8.1k
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 Solutions][Detection Engine] Migrates exception lists to saved object references (Part 2) #108291
[Security Solutions][Detection Engine] Migrates exception lists to saved object references (Part 2) #108291
Conversation
Pinging @elastic/security-detections-response (Team:Detections and Resp) |
I don't know how feasible this is to do with SIEM rules, so leave this up to you, but it might be worth adding an end-to-end test that actually migrates the rules and validates that the references are built correctly. You can see some examples here: https://github.com/elastic/kibana/blob/master/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/migrations.ts |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the thorough tests! Left some questions about the migration and agree with @gmmorris's comment about adding functional tests if feasible.
@elasticmachine merge upstream |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks for the thorough tests!
…assanabad/kibana into add-savedobject-migrations-part-2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM; left some notes/nits
|
||
expect(migration7150(alert, migrationContext)).toEqual({ | ||
...alert, | ||
references: [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I think the references object can be removed here, the migrated alert should be equal to alert
, right? Would that make it clearer? Wondering if it's just here because of copypasta.
💚 Build SucceededMetrics [docs]
History
To update your PR or re-run it, just comment with: |
…ved object references (Part 2) (elastic#108291) ## Summary This is part 2 to addressing the issue seen here: elastic#101975 (Part 1 elastic#107064) This adds the alerting migration scripts and unit tests for exception list containers on Kibana startup for `7.15.0` We only migrate if we find these conditions and cases: - `exceptionLists` are an `array` and not `null`, `undefined`, or malformed data. - The exceptionList item is an `object` and its `id` is a `string` and not `null`, `undefined`, or malformed data - The existing references do not already have an exceptionItem reference already found within it. We migrate on the common use case - The saved object references do not exist but we have exceptionList items with the id's to create the saved object references, 👍 so we migrate. - The alert contains no exception list items, in which case we have nothing to migrate We do these additional (mis-use) cases and steps as well. These should _NOT_ be common things that happen but we safe guard for them here: - If the migration is run twice we are idempotent and do _NOT_ add duplicates list items or remove items. - If the migration was partially successful but re-run a second time, we only add what is missing. Again no duplicates or removed items should occur. - If the `exceptionLists` contains invalid data shape or not enough information to migrate, we filter it out and ignore it - If the saved object references already exists and contains a different or foreign value, we will retain the foreign reference(s) and still migrate. ## Manual testing There are unit tests but for any manual testing or verification you can do the following: Create a few alerts through the `security_solution` application with exception lists <img width="1775" alt="Screen Shot 2021-08-11 at 5 42 31 PM" src="https://user-images.githubusercontent.com/1151048/129117377-61b17fcf-ad01-4405-bbfe-42d97a6f7654.png"> Use the dev tools to de-migrate as well as to test end to end like so: ```json # First get an "_id" with an exceptions list like below. Mine I found was: "alert:38482620-ef1b-11eb-ad71-7de7959be71c": GET .kibana/_search { "query": { "terms": { "alert.alertTypeId": [ "siem.signals" ] } }, "size": 10000 } ``` With Kibana running downgrade and remove the references as a test: ```json # Set saved object array references as empty arrays and set our migration version to be 7.14.0 POST .kibana/_update/alert:38482620-ef1b-11eb-ad71-7de7959be71c { "script" : { "source": """ ctx._source.migrationVersion.alert = "7.14.0"; ctx._source.references = [] """, "lang": "painless" } } # Double check the references is empty and the version is 7.14.0 GET .kibana/_doc/alert:38482620-ef1b-11eb-ad71-7de7959be71c ``` Reload the alert in the `security_solution` and notice you get these errors until you restart Kibana to cause a migration moving forward ```sh server log [17:35:16.914] [error][plugins][securitySolution] The saved object references were not found for our exception list when we were expecting to find it. Kibana migrations might not have run correctly or someone might have removed the saved object references manually. Returning the last known good exception list id which might not work. exceptionItem with its id being returned is: {"list_id":"endpoint_list","namespace_type":"agnostic","id":"endpoint_list","type":"endpoint"} server log [17:35:16.914] [error][plugins][securitySolution] Cannot get a saved object reference using an index which is larger than the saved object references. Index is:1 which is larger than the savedObjectReferences:[] server log [17:35:16.915] [error][plugins][securitySolution] The saved object references were not found for our exception list when we were expecting to find it. Kibana migrations might not have run correctly or someone might have removed the saved object references manually. Returning the last known good exception list id which might not work. exceptionItem with its id being returned is: {"list_id":"cd152d0d-3590-4a45-a478-eed04da7936b","namespace_type":"single","id":"50e3bd70-ef1b-11eb-ad71-7de7959be71c","type":"detection"} server log [17:35:16.940] [error][plugins][securitySolution] The saved object references were not found for our exception list when we were expecting to find it. Kibana migrations might not have run correctly or someone might have removed the saved object references manually. Returning the last known good exception list id which might not work. exceptionItem with its id being returned is: {"list_id":"endpoint_list","namespace_type":"agnostic","id":"endpoint_list","type":"endpoint"} server log [17:35:16.940] [error][plugins][securitySolution] Cannot get a saved object reference using an index which is larger than the saved object references. Index is:1 which is larger than the savedObjectReferences:[] server log [17:35:16.940] [error][plugins][securitySolution] The saved object references were not found for our exception list when we were expecting to find it. Kibana migrations might not have run correctly or someone might have removed the saved object references manually. Returning the last known good exception list id which might not work. exceptionItem with its id being returned is: {"list_id":"cd152d0d-3590-4a45-a478-eed04da7936b","namespace_type":"single","id":"50e3bd70-ef1b-11eb-ad71-7de7959be71c","type":"detection"} ``` Restart Kibana and you should no longer have errors in the Kibana console. If you do this query in dev tools ```json # Check that the `migrationVersion` is `7.15.0` and that we have a `references` array filled out with the correct structure GET .kibana/_doc/alert:38482620-ef1b-11eb-ad71-7de7959be71c ``` You should notice that you now have a `references` array filled out: ```json "references" : [ { "name" : "param:exceptionsList_0", "id" : "endpoint_list", "type" : "exception-list-agnostic" }, { "name" : "param:exceptionsList_1", "id" : "50e3bd70-ef1b-11eb-ad71-7de7959be71c", "type" : "exception-list" } ], "migrationVersion" : { "alert" : "7.15.0" } ``` For testing [idempotentence](https://en.wikipedia.org/wiki/Idempotence) Run just this to downgrade and restart Kibana and you should notice on a GET that we do not have anything extra in the references array: ```json # Set our migration version to be 7.14.0 only POST .kibana/_update/alert:38482620-ef1b-11eb-ad71-7de7959be71c { "script" : { "source": """ ctx._source.migrationVersion.alert = "7.14.0"; """, "lang": "painless" } } # Double check the `references` is still there, and we do not get errors or changes to `references` after we restart Kibana GET .kibana/_doc/alert:38482620-ef1b-11eb-ad71-7de7959be71c ``` For testing foreign keys: ```json # Set saved object array references to foreign keys and set our migration version to be 7.14.0 POST .kibana/_update/alert:38482620-ef1b-11eb-ad71-7de7959be71c { "script" : { "source": """ ctx._source.migrationVersion.alert = "7.14.0"; ctx._source.references = [["name" : "foreign", "id" : "123", "type" : "some-type"]]; """, "lang": "painless" } } ``` Restart, ensure no errors in Kibana console and do a get call to ensure we have the foreign mixed with valid values: ```json GET .kibana/_doc/alert:38482620-ef1b-11eb-ad71-7de7959be71c ``` Should return this data: ```json "type" : "alert", "references" : [ { "name" : "foreign", "id" : "123", "type" : "some-type" }, { "name" : "param:exceptionsList_0", "id" : "endpoint_list", "type" : "exception-list-agnostic" }, { "name" : "param:exceptionsList_1", "id" : "50e3bd70-ef1b-11eb-ad71-7de7959be71c", "type" : "exception-list" } ] ``` ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
💚 Backport successful
This backport PR will be merged automatically after passing CI. |
…ved object references (Part 2) (#108291) (#108991) ## Summary This is part 2 to addressing the issue seen here: #101975 (Part 1 #107064) This adds the alerting migration scripts and unit tests for exception list containers on Kibana startup for `7.15.0` We only migrate if we find these conditions and cases: - `exceptionLists` are an `array` and not `null`, `undefined`, or malformed data. - The exceptionList item is an `object` and its `id` is a `string` and not `null`, `undefined`, or malformed data - The existing references do not already have an exceptionItem reference already found within it. We migrate on the common use case - The saved object references do not exist but we have exceptionList items with the id's to create the saved object references, 👍 so we migrate. - The alert contains no exception list items, in which case we have nothing to migrate We do these additional (mis-use) cases and steps as well. These should _NOT_ be common things that happen but we safe guard for them here: - If the migration is run twice we are idempotent and do _NOT_ add duplicates list items or remove items. - If the migration was partially successful but re-run a second time, we only add what is missing. Again no duplicates or removed items should occur. - If the `exceptionLists` contains invalid data shape or not enough information to migrate, we filter it out and ignore it - If the saved object references already exists and contains a different or foreign value, we will retain the foreign reference(s) and still migrate. ## Manual testing There are unit tests but for any manual testing or verification you can do the following: Create a few alerts through the `security_solution` application with exception lists <img width="1775" alt="Screen Shot 2021-08-11 at 5 42 31 PM" src="https://user-images.githubusercontent.com/1151048/129117377-61b17fcf-ad01-4405-bbfe-42d97a6f7654.png"> Use the dev tools to de-migrate as well as to test end to end like so: ```json # First get an "_id" with an exceptions list like below. Mine I found was: "alert:38482620-ef1b-11eb-ad71-7de7959be71c": GET .kibana/_search { "query": { "terms": { "alert.alertTypeId": [ "siem.signals" ] } }, "size": 10000 } ``` With Kibana running downgrade and remove the references as a test: ```json # Set saved object array references as empty arrays and set our migration version to be 7.14.0 POST .kibana/_update/alert:38482620-ef1b-11eb-ad71-7de7959be71c { "script" : { "source": """ ctx._source.migrationVersion.alert = "7.14.0"; ctx._source.references = [] """, "lang": "painless" } } # Double check the references is empty and the version is 7.14.0 GET .kibana/_doc/alert:38482620-ef1b-11eb-ad71-7de7959be71c ``` Reload the alert in the `security_solution` and notice you get these errors until you restart Kibana to cause a migration moving forward ```sh server log [17:35:16.914] [error][plugins][securitySolution] The saved object references were not found for our exception list when we were expecting to find it. Kibana migrations might not have run correctly or someone might have removed the saved object references manually. Returning the last known good exception list id which might not work. exceptionItem with its id being returned is: {"list_id":"endpoint_list","namespace_type":"agnostic","id":"endpoint_list","type":"endpoint"} server log [17:35:16.914] [error][plugins][securitySolution] Cannot get a saved object reference using an index which is larger than the saved object references. Index is:1 which is larger than the savedObjectReferences:[] server log [17:35:16.915] [error][plugins][securitySolution] The saved object references were not found for our exception list when we were expecting to find it. Kibana migrations might not have run correctly or someone might have removed the saved object references manually. Returning the last known good exception list id which might not work. exceptionItem with its id being returned is: {"list_id":"cd152d0d-3590-4a45-a478-eed04da7936b","namespace_type":"single","id":"50e3bd70-ef1b-11eb-ad71-7de7959be71c","type":"detection"} server log [17:35:16.940] [error][plugins][securitySolution] The saved object references were not found for our exception list when we were expecting to find it. Kibana migrations might not have run correctly or someone might have removed the saved object references manually. Returning the last known good exception list id which might not work. exceptionItem with its id being returned is: {"list_id":"endpoint_list","namespace_type":"agnostic","id":"endpoint_list","type":"endpoint"} server log [17:35:16.940] [error][plugins][securitySolution] Cannot get a saved object reference using an index which is larger than the saved object references. Index is:1 which is larger than the savedObjectReferences:[] server log [17:35:16.940] [error][plugins][securitySolution] The saved object references were not found for our exception list when we were expecting to find it. Kibana migrations might not have run correctly or someone might have removed the saved object references manually. Returning the last known good exception list id which might not work. exceptionItem with its id being returned is: {"list_id":"cd152d0d-3590-4a45-a478-eed04da7936b","namespace_type":"single","id":"50e3bd70-ef1b-11eb-ad71-7de7959be71c","type":"detection"} ``` Restart Kibana and you should no longer have errors in the Kibana console. If you do this query in dev tools ```json # Check that the `migrationVersion` is `7.15.0` and that we have a `references` array filled out with the correct structure GET .kibana/_doc/alert:38482620-ef1b-11eb-ad71-7de7959be71c ``` You should notice that you now have a `references` array filled out: ```json "references" : [ { "name" : "param:exceptionsList_0", "id" : "endpoint_list", "type" : "exception-list-agnostic" }, { "name" : "param:exceptionsList_1", "id" : "50e3bd70-ef1b-11eb-ad71-7de7959be71c", "type" : "exception-list" } ], "migrationVersion" : { "alert" : "7.15.0" } ``` For testing [idempotentence](https://en.wikipedia.org/wiki/Idempotence) Run just this to downgrade and restart Kibana and you should notice on a GET that we do not have anything extra in the references array: ```json # Set our migration version to be 7.14.0 only POST .kibana/_update/alert:38482620-ef1b-11eb-ad71-7de7959be71c { "script" : { "source": """ ctx._source.migrationVersion.alert = "7.14.0"; """, "lang": "painless" } } # Double check the `references` is still there, and we do not get errors or changes to `references` after we restart Kibana GET .kibana/_doc/alert:38482620-ef1b-11eb-ad71-7de7959be71c ``` For testing foreign keys: ```json # Set saved object array references to foreign keys and set our migration version to be 7.14.0 POST .kibana/_update/alert:38482620-ef1b-11eb-ad71-7de7959be71c { "script" : { "source": """ ctx._source.migrationVersion.alert = "7.14.0"; ctx._source.references = [["name" : "foreign", "id" : "123", "type" : "some-type"]]; """, "lang": "painless" } } ``` Restart, ensure no errors in Kibana console and do a get call to ensure we have the foreign mixed with valid values: ```json GET .kibana/_doc/alert:38482620-ef1b-11eb-ad71-7de7959be71c ``` Should return this data: ```json "type" : "alert", "references" : [ { "name" : "foreign", "id" : "123", "type" : "some-type" }, { "name" : "param:exceptionsList_0", "id" : "endpoint_list", "type" : "exception-list-agnostic" }, { "name" : "param:exceptionsList_1", "id" : "50e3bd70-ef1b-11eb-ad71-7de7959be71c", "type" : "exception-list" } ] ``` ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios Co-authored-by: Frank Hassanabad <frank.hassanabad@elastic.co>
Summary
This is part 2 to addressing the issue seen here: #101975 (Part 1 #107064)
This adds the alerting migration scripts and unit tests for exception list containers on Kibana startup for
7.15.0
We only migrate if we find these conditions and cases:
exceptionLists
are anarray
and notnull
,undefined
, or malformed data.object
and itsid
is astring
and notnull
,undefined
, or malformed dataWe migrate on the common use case
We do these additional (mis-use) cases and steps as well. These should NOT be common things that happen but we safe guard for them here:
exceptionLists
contains invalid data shape or not enough information to migrate, we filter it out and ignore itManual testing
There are unit tests but for any manual testing or verification you can do the following:
Create a few alerts through the
security_solution
application with exception listsUse the dev tools to de-migrate as well as to test end to end like so:
With Kibana running downgrade and remove the references as a test:
Reload the alert in the
security_solution
and notice you get these errors until you restart Kibana to cause a migration moving forwardRestart Kibana and you should no longer have errors in the Kibana console.
If you do this query in dev tools
You should notice that you now have a
references
array filled out:For testing idempotentence
Run just this to downgrade and restart Kibana and you should notice on a GET that we do not have anything extra in the references array:
For testing foreign keys:
Restart, ensure no errors in Kibana console and do a get call to ensure we have the foreign mixed with valid values:
GET .kibana/_doc/alert:38482620-ef1b-11eb-ad71-7de7959be71c
Should return this data:
Checklist