-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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] Only apply field aliases to legacy .siem-signals indices #115290
[Security Solution] Only apply field aliases to legacy .siem-signals indices #115290
Conversation
@elasticmachine merge upstream |
@@ -170,7 +200,7 @@ const addFieldAliasesToIndices = async ({ | |||
// index: string; | |||
// aadIndexAliasName: string; | |||
// }) => { | |||
// const { body: indices } = await esClient.indices.getAlias({ name: index }); | |||
// const { body: indices } = await esClient.indices.getAlias({ index: `${index}-*`, name: index }); |
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.
When we do use this code again (most likely for 8.0, so very soon) we'll want to limit this query the same way the getBootstrapIndexExists
check is limited.
@@ -41,7 +41,10 @@ export const readIndexRoute = (router: SecuritySolutionPluginRouter, config: Con | |||
const { ruleRegistryEnabled } = parseExperimentalConfigValue(config.enableExperimental); | |||
|
|||
const index = siemClient.getSignalsIndex(); | |||
const indexExists = await getIndexExists(esClient, index); | |||
const indexExists = await getBootstrapIndexExists( | |||
context.core.elasticsearch.client.asInternalUser, |
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.
getBootstrapIndexExists
requires either view_index_metadata
or manage
privileges for the concrete backing indices, which is not technically a required permission for detection engine users. To avoid potential permissions issues the internal user is used here, with the theory being that any user who has the Kibana application privilege to use this Security Solution API at all is authorized to know if the .siem-signals
index exists - so this isn't a data leakage.
Pinging @elastic/security-detections-response (Team:Detections and Resp) |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
💔 Build Failed
Failed CI StepsTest FailuresKibana Pipeline / general / X-Pack API Integration Tests.x-pack/test/api_integration/apis/security_solution/network_dns·ts.apis SecuritySolution Endpoints Network DNS With packetbeat Make sure that we get Dns data and sorting by uniqueDomains ascendingStandard Out
Stack Trace
Kibana Pipeline / general / displays the data provider action menu when Enter is pressed.timeline data providers displays the data provider action menu when Enter is pressedStack Trace
Metrics [docs]History
To update your PR or re-run it, just comment with: |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
💚 Build Succeeded
Metrics [docs]History
To update your PR or re-run it, just comment with: |
Reviewed and tested locally / received expected results. |
…indices (elastic#115290) * Only apply field aliases to legacy .siem-signals indices * Fix unit test mocks * Add new function for special index existence check * Actually add new function for special index existence check * Undo getIndexVersion change * Add basic integration tests for field alias logic * Add back create_index to test list * Add missing markdown to readme * Revert change to delete_index_route Co-authored-by: Kibana Machine <[email protected]>
…indices (elastic#115290) * Only apply field aliases to legacy .siem-signals indices * Fix unit test mocks * Add new function for special index existence check * Actually add new function for special index existence check * Undo getIndexVersion change * Add basic integration tests for field alias logic * Add back create_index to test list * Add missing markdown to readme * Revert change to delete_index_route Co-authored-by: Kibana Machine <[email protected]> # Conflicts: # x-pack/plugins/security_solution/server/lib/detection_engine/routes/index/create_index_route.ts # x-pack/plugins/security_solution/server/lib/detection_engine/routes/index/read_index_route.ts # x-pack/test/detection_engine_api_integration/security_and_spaces/tests/create_index.ts
…indices (#115290) (#116838) * Only apply field aliases to legacy .siem-signals indices * Fix unit test mocks * Add new function for special index existence check * Actually add new function for special index existence check * Undo getIndexVersion change * Add basic integration tests for field alias logic * Add back create_index to test list * Add missing markdown to readme * Revert change to delete_index_route Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Kibana Machine <[email protected]>
…ignals indices (#115290) (#116841) * [Security Solution] Only apply field aliases to legacy .siem-signals indices (#115290) * Only apply field aliases to legacy .siem-signals indices * Fix unit test mocks * Add new function for special index existence check * Actually add new function for special index existence check * Undo getIndexVersion change * Add basic integration tests for field alias logic * Add back create_index to test list * Add missing markdown to readme * Revert change to delete_index_route Co-authored-by: Kibana Machine <[email protected]> # Conflicts: # x-pack/plugins/security_solution/server/lib/detection_engine/routes/index/create_index_route.ts # x-pack/plugins/security_solution/server/lib/detection_engine/routes/index/read_index_route.ts # x-pack/test/detection_engine_api_integration/security_and_spaces/tests/create_index.ts * Remove extra esClient definition * Adjust for old ES client Co-authored-by: Kibana Machine <[email protected]>
…oes not exist" FTR test (#186789) ## Summary - addresses #179208 by removing skipped test I tracked skipped test to this PR: #115290 Test was added already skipped https://github.com/elastic/kibana/pull/115290/files#diff-16cebcbaef99c1aab50640a5bee66351bcbfd7575361d97eee4d2ca6753f5a27R38-R41 In the tested route itself, when index does not exist, it returns 200: https://github.com/elastic/kibana/pull/115290/files#diff-a4e27aaa05560a7737e153e53fe4bdaf839056180347c338e8d0842ab39f1240R79-R84 So, test from the very beginning was not testing valid use case. After talking to PR author @marshallmain , we agreed to remove that test
Summary
Closes #112600
Filtering Alerts as Data indices from legacy siem signals indices
This PR fixes an issue where the read and create index routes in the detection were treating the new alerts-as-data indices as legacy .siem-signals indices, since they both have
.siem-signals
as an index alias. This was causing problems because the backwards compatibility mappings intended for legacy.siem-signals
indices are not compatible with the new alerts-as-data indices (the legacy compatibility mappings definekibana.alert.*
fields as field aliases, so when added to alerts-as-data index mappings that definekibana.alert.*
fields as real fields it causes a mapping conflict).To fix this issue, we verify that the concrete index names we apply compatibility mappings to start with
.siem-signals-<space id>-
, ensuring that they are legacy indices. Alerts as data concrete index names start with.internal.alerts
instead.To verify that this issue is fixed, create an index that has
.siem-signals-default
as an alias, e.g.then visit the Detection Alerts page to trigger the calls to
read_index_route
andcreate_index_route
. Prior to this PR, you would see the first call toread_index_route
return that the signals index exists, but is outdated. Then a call to create_index_route would be made and fail because it attempts to add the backwards compatibility mappings to this test index, but the mappings are only valid for .siem-signals indices.With this PR, the call to
read_index_route
in the above scenario correctly reports that the .siem-signals index doesn't exist. The call tocreate_index_route
creates the .siem-signals index and does not attempt to apply the compatibility mappings to the test index that does not need them.Improve performance of backwards compatibility mapping updates
This PR also improves the logic used to add field aliases to existing indices. Previously, the backwards compatibility mappings were added to all existing siem signals indices individually on every call to create_index_route, which is expensive and unnecessary. The new logic only adds the mappings to indices that have not already been updated, and also updates the mappings in batches of (up to) 20 to reduce the cost of round trip times to ES and other overhead of using many small requests. In local testing (both local ES and Kibana), this resulted in a 3x speedup with ~40 siem signals indices being updated. This speedup does not include any of the benefits from only updating the indices that need to be updated. The speedup should be greater in environments where ES and Kibana are running in separate environments and round trip times are higher.