[Security Solution][Detection Engine] move lists to data stream #162508

vitaliidm · 2023-07-25T16:30:38Z

Summary

addresses https://github.com/elastic/security-team/issues/7198
moves list/items indices to data stream
- adds @timestamp mapping to indices mappings
- migrate to data stream if indices already exist(for customers < 8.11) or create data stream(for customers 8.11+ or serverless)
- adds DLM to index templates
- replaces update/delete queries with update_by_query/delete_by_query which supported in data streams
- fixes existing issues with update/patch APIs for lists/items
  - update/patch for lists didn't save version parameter in ES
  - update and patch APIs for lists/items were identical, i.e. for both routes was called the same update method w/o any changes

Technical detail on moving API to (update/delete)_by_query

update_by_query, delete_by_query do not support refresh=wait_for, only false/true values. Which might break some of the use cases on UI(when list is removed, we refetch all lists. Deleted list will be returned for some time. Default refresh time is 1s). So, we retry refetching deleted/updated document before finishing request, to return reindexed document

update_by_query does not support OCC as update API.
Which is supported in both list/list item updates through _version parameter.
_version is base64 encoded "_seq_no", "_primary_term" props used for OCC

So, to keep it without breaking changes: implemented check for version conflict within update method

Checklist

Delete any items that are not applicable to this PR.

Unit or functional tests were updated or added to match the most common scenarios

This reverts commit 245ba57.

# Conflicts: # x-pack/plugins/lists/server/routes/create_list_index_route.ts # x-pack/plugins/lists/server/routes/delete_list_index_route.ts # x-pack/plugins/lists/server/routes/read_list_index_route.ts

vitaliidm · 2023-08-04T18:25:27Z

there is still one failing test: #163197
Doesn't seem to be related to functionality in this PR and does not fail locally(on this PR) or on main, only in scope of all tests on CI

dasansol92 · 2023-08-07T09:22:18Z

We need to ensure that this still works with artifact-packager task which reads from .lists and packages Endpoint Exceptions, Trusted Apps, etc for the Endpoint.

We have our automated Defend Workflows Endpoint cypress tests which stand up real Endpoints and create artifacts, so if those pass, the basic create, read, and edit should work.

fyi @dasansol92 - could you also review this PR?

@kevinlog The x-pack/test/security_solution_endpoint/apps/integrations/artifact_entries_list.ts FTR test should ensure the artifact-packager task is still running as we expect.

dasansol92 · 2023-08-07T11:57:00Z

hey @vitaliidm , are these changes affecting also the exceptions lists/items types or only the regular lists/items? Thanks!

vitaliidm · 2023-08-07T12:06:33Z

hey @vitaliidm , are these changes affecting also the exceptions lists/items types or only the regular lists/items? Thanks!

hey @dasansol92 , only .lists-*, .items-* indices, which moved to data stream in this PR. They can be used in exceptions, but it should not affect exceptions workflows

e40pud

Great work! I left a few comments, let me know what you think about those.

e40pud · 2023-08-17T11:36:44Z

x-pack/plugins/lists/server/routes/list/patch_list_route.ts

+        // needs to be migrated to data stream if index exists
+        if (!dataStreamExists) {
+          const indexExists = await lists.getListIndexExists();
+          if (indexExists) {


what if it does not exist? or this is not possible?

this is going to be handled down the function execution, few lines below this one

const list = await lists.patchList({ _version, description, id, meta, name, version }); if (list == null) { return siemResponse.error({ body: `list id: "${id}" not found`, ``

e40pud · 2023-08-17T11:37:06Z

x-pack/plugins/lists/server/routes/list/update_list_route.ts

+        // needs to be migrated to data stream if index exists
+        if (!dataStreamExists) {
+          const indexExists = await lists.getListIndexExists();
+          if (indexExists) {


same as my previous question

same as https://github.com/elastic/kibana/pull/162508/files/c85531e2424f32db708a93db06719010d63af87c#r1297165741

e40pud · 2023-08-17T11:49:43Z

x-pack/plugins/lists/server/routes/list_index/create_list_index_route.ts

+
+        await removeLegacyTemplatesIfExist(lists);
+
+        if (listDataStreamExists && listItemDataStreamExists) {


I see that it was implemented this way earlier, but just curios whether we should do this check before setting templates above? since we know that this is an invalid request and we gonna return 409 error maybe we should not do any modifications and throw an exception earlier

I don't know the reason it was done like this before, but I can guess, if there are any updates to templates, that would the only way to update them - by calling that API. Otherwise, if no updates were made to templates, that action won't have any effect

e40pud · 2023-08-17T11:52:10Z

x-pack/plugins/lists/server/routes/list_index/delete_list_index_route.ts

+        const listItemDataStreamExists = await lists.getListItemDataStreamExists();
+
+        // data streams exists, it means indices were migrated
+        if (listDataStreamExists) {


Is it possible that listItemDataStreamExists = true and listDataStreamExists = false? In case of indices we check that both exist listIndexExists and listItemIndexExists

I think, this could only happen, if you use patch/update listItem API, before loading Security Solution page(which performs migration to DS). Very unlikely, because, you have to use direct API call, since these APIs are not used in UI.
And those migration in patch/update was added recently, after delete method was migrated.

Thanks for noticing, I have updated the code, so it will account for this

e40pud · 2023-08-17T12:02:30Z

x-pack/plugins/lists/server/routes/list_index/read_list_index_route.ts


-        if (listIndexExists || listItemIndexExists) {
+        if (listDataStreamExists || listItemDataStreamExists) {


These IF statements are not going to work as initially intended.

First if (listDataStreamExists || listItemDataStreamExists) { will handle three out of four possible cases:

listDataStreamExists = true && listItemDataStreamExists = true

listDataStreamExists = false && listItemDataStreamExists = true

listDataStreamExists = true && listItemDataStreamExists = false

Then, second if (!listDataStreamExists && listItemDataStreamExists) { statement will handle the last possible case:

listDataStreamExists = false && listItemDataStreamExists = false

This means, that last two statements (if (!listItemDataStreamExists && listDataStreamExists) { and final else case) will never be called.

Yes, you are right - I think, probably, its initial implementation meant to have the firs statement as AND, in that case the rest of statements would make sense.
I will change it AND, thanks for catching this

e40pud · 2023-08-17T12:04:44Z

x-pack/plugins/lists/server/routes/list_item/patch_list_item_route.ts

+        // needs to be migrated to data stream if index exists
+        if (!dataStreamExists) {
+          const indexExists = await lists.getListItemIndexExists();
+          if (indexExists) {


same as one of the previous questions about possibility of indexExists = false

similar to same questions, it's going to be handled below these lines

const listItem = await lists.patchListItem({ _version, id, meta, value, }); if (listItem == null) { return siemResponse.error({ body: `list item id: "${id}" not found`, statusCode: 404, }); } else {

e40pud · 2023-08-17T12:04:57Z

x-pack/plugins/lists/server/routes/list_item/update_list_item_route.ts

+        // needs to be migrated to data stream if index exists
+        if (!dataStreamExists) {
+          const indexExists = await lists.getListItemIndexExists();
+          if (indexExists) {


https://github.com/elastic/kibana/pull/162508/files/c85531e2424f32db708a93db06719010d63af87c#r1297169997

…iidm/kibana into alerts_8_10/move-lists-to-ds

…-fix'

e40pud

LGTM!

dasansol92 · 2023-08-23T10:15:03Z

For changes affecting exceptions items/lists, would be nice having this test passing in CI when it's enabled: #164473
Thanks!

kibana-ci · 2023-08-23T11:41:35Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 61c13e8

Failed CI Steps

Defend Workflows Endpoint Cypress Tests #2

Test Failures

[job] [logs] Defend Workflows Endpoint Cypress Tests #2 / Automated Response Actions From alerts "after all" hook for "should have generated endpoint and rule" "after all" hook for "should have generated endpoint and rule"
[job] [logs] Defend Workflows Endpoint Cypress Tests #2 / Automated Response Actions From alerts "before all" hook for "should have generated endpoint and rule" "before all" hook for "should have generated endpoint and rule"

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`lists`	261	262	+1
`securitySolution`	4444	4451	+7
total			+8

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`@kbn/securitysolution-es-utils`	62	76	+14
`@kbn/securitysolution-io-ts-list-types`	515	519	+4
`lists`	94	96	+2
total			+20

Async chunks

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

id	before	after	diff
`lists`	145.3KB	145.4KB	+81.0B
`securitySolution`	15.7MB	15.7MB	+89.0B
total			+170.0B

Unknown metric groups

API count

id	before	after	diff
`@kbn/securitysolution-es-utils`	68	87	+19
`@kbn/securitysolution-io-ts-list-types`	528	532	+4
`lists`	210	222	+12
total			+35

Unreferenced deprecated APIs

id	before	after	diff
`lists`	0	4	+4

History

💔 Build #152262 failed 561b3e6
💛 Build #151557 was flaky 65fc067
💔 Build #151478 failed ee45572
💛 Build #150469 was flaky c85531e
💚 Build #148835 succeeded fc8cb43

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

cc @vitaliidm

vitaliidm and others added 14 commits May 5, 2023 11:42

add test element

245ba57

Merge branch 'elastic:main' into main

c3d1806

Revert "add test element"

cea3e3b

This reverts commit 245ba57.

Merge remote-tracking branch 'upstream/main'

8a240c5

Merge branch 'elastic:main' into main

d1b32ab

Merge branch 'elastic:main' into main

1b8bd2b

Merge remote-tracking branch 'upstream/main'

711fe3e

Merge branch 'main' of ssh://github.com/vitaliidm/kibana

5272556

Merge remote-tracking branch 'upstream/main'

9589ab4

Merge remote-tracking branch 'upstream/main'

fcd9618

Merge remote-tracking branch 'upstream/main'

5b12314

Merge remote-tracking branch 'upstream/main'

0b09ae1

Merge remote-tracking branch 'upstream/main'

82db206

[Security Solution][Detection Engine] move lists to data stream

8eb1e39

vitaliidm self-assigned this Jul 25, 2023

vitaliidm added 8.10 candidate Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Detection Engine Security Solution Detection Engine Area labels Jul 25, 2023

vitaliidm added 11 commits July 26, 2023 11:39

update apis

b80c1b4

update schemas

ed9ddee

add migration

5b7092b

add delete stream

a1754e3

Merge remote-tracking branch 'upstream/main'

d19ce2c

Merge branch 'main' of https://github.com/vitaliidm/kibana

0870df1

Merge remote-tracking branch 'upstream/main'

f298651

Merge branch 'main' of ssh://github.com/vitaliidm/kibana

659f27b

Merge branch 'main' into alerts_8_10/move-lists-to-ds

c05770f

# Conflicts: # x-pack/plugins/lists/server/routes/create_list_index_route.ts # x-pack/plugins/lists/server/routes/delete_list_index_route.ts # x-pack/plugins/lists/server/routes/read_list_index_route.ts

move routes to separate folder

7960d47

add delete list

5719cfd

import

d664868

vitaliidm mentioned this pull request Aug 4, 2023

(Accessibility) [Stack Monitoring] Failing/Flaky test "a11y tests for self monitoring home page" #163197

Closed

vitaliidm requested a review from marshallmain August 4, 2023 18:24

Merge branch 'main' into alerts_8_10/move-lists-to-ds

79ffe2d

vitaliidm added 8.11 candidate and removed 8.10 candidate v8.10.0 labels Aug 8, 2023

vitaliidm added 2 commits August 9, 2023 16:50

Merge branch 'main' into alerts_8_10/move-lists-to-ds

fc8cb43

Merge branch 'main' into alerts_8_10/move-lists-to-ds

c85531e

e40pud reviewed Aug 17, 2023

View reviewed changes

vitaliidm and others added 5 commits August 17, 2023 16:27

CR: Update read_list_index_route.ts

ee45572

Merge branch 'main' into alerts_8_10/move-lists-to-ds

6ff6bb8

CR: delete logic rehaul

d54dbaf

Merge branch 'alerts_8_10/move-lists-to-ds' of ssh://github.com/vital…

3f8cc90

…iidm/kibana into alerts_8_10/move-lists-to-ds

[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…

65fc067

…-fix'

vitaliidm requested a review from e40pud August 22, 2023 09:38

vitaliidm added 2 commits August 22, 2023 10:38

Merge branch 'main' into alerts_8_10/move-lists-to-ds

561b3e6

Merge branch 'main' into alerts_8_10/move-lists-to-ds

b7f2179

e40pud approved these changes Aug 23, 2023

View reviewed changes

Merge branch 'main' into alerts_8_10/move-lists-to-ds

61c13e8

vitaliidm merged commit 505d826 into elastic:main Aug 23, 2023

kibanamachine added the v8.11.0 label Aug 23, 2023

vitaliidm mentioned this pull request Aug 30, 2023

[ResponseOps] use Data Streams for AAD indices in serverless #160572

Merged

3 tasks

vitaliidm deleted the alerts_8_10/move-lists-to-ds branch March 4, 2024 17:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security Solution][Detection Engine] move lists to data stream #162508

[Security Solution][Detection Engine] move lists to data stream #162508

vitaliidm commented Jul 25, 2023 •

edited

Loading

vitaliidm commented Aug 4, 2023 •

edited

Loading

dasansol92 commented Aug 7, 2023

dasansol92 commented Aug 7, 2023

vitaliidm commented Aug 7, 2023 •

edited

Loading

e40pud left a comment

e40pud Aug 17, 2023 •

edited

Loading

vitaliidm Aug 17, 2023

e40pud Aug 17, 2023

vitaliidm Aug 17, 2023

e40pud Aug 17, 2023

vitaliidm Aug 17, 2023

e40pud Aug 17, 2023

vitaliidm Aug 22, 2023

e40pud Aug 17, 2023

vitaliidm Aug 17, 2023

e40pud Aug 17, 2023

vitaliidm Aug 17, 2023

e40pud Aug 17, 2023

vitaliidm Aug 17, 2023

e40pud left a comment

dasansol92 commented Aug 23, 2023

kibana-ci commented Aug 23, 2023

API count

Unreferenced deprecated APIs


		await removeLegacyTemplatesIfExist(lists);

		if (listDataStreamExists && listItemDataStreamExists) {


		if (listIndexExists \|\| listItemIndexExists) {
		if (listDataStreamExists \|\| listItemDataStreamExists) {

[Security Solution][Detection Engine] move lists to data stream #162508

[Security Solution][Detection Engine] move lists to data stream #162508

Conversation

vitaliidm commented Jul 25, 2023 • edited Loading

Summary

Checklist

vitaliidm commented Aug 4, 2023 • edited Loading

dasansol92 commented Aug 7, 2023

dasansol92 commented Aug 7, 2023

vitaliidm commented Aug 7, 2023 • edited Loading

e40pud left a comment

Choose a reason for hiding this comment

e40pud Aug 17, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

e40pud left a comment

Choose a reason for hiding this comment

dasansol92 commented Aug 23, 2023

kibana-ci commented Aug 23, 2023

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

API count

Unreferenced deprecated APIs

History

vitaliidm commented Jul 25, 2023 •

edited

Loading

vitaliidm commented Aug 4, 2023 •

edited

Loading

vitaliidm commented Aug 7, 2023 •

edited

Loading

e40pud Aug 17, 2023 •

edited

Loading