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] "Data view" selector is shown in "Edit filter" view on the Rule Editing page (#174026) #174922

Merged

Conversation

e40pud
Copy link
Contributor

@e40pud e40pud commented Jan 16, 2024

Summary

Addresses #174026

These changes fix the issue with filter editing on the rule's editing page when using index patterns instead of data view.

Steps to reproduce:

  1. Create a custom query rule and add a filter
  2. Save the rule
  3. Edit the rule
  4. Edit the filter

Current behaviour:
Right now when user tries to edit the filter the data view picking UI appears even though index patterns were not modified.

Screenshot 2024-01-16 at 15 14 23

Expected behaviour:
Data view picking UI should not be present and previously set field and value options should be shown in the filter editing dialog.

Screenshot 2024-01-16 at 15 16 07

Cause:
The behaviour for the filter editing on rule’s editing page changed in 8.11 with these changes #166318. We convert DataViewBase object without ID set to a DataView object with auto-generated ID. This happens each time we try to edit the rule and leads to a different ID which is saved in filter.meta.index. Unified search internally checks those IDs to verify whether the filter belongs to provided data view.

Solution:
To solve this issue, we set the data view id explicitly on creating an in-memory data view that represents index patterns and update filter.meta.index to use the same ID.

Known issue:
This does not resolve the issue for existing filters. In this case, user will need to update their filters manually.
(This was fixed by updating filter.meta.index field on rule editing)

Flaky test runner
ESS 50 times
Serverless 50 times

@e40pud e40pud added release_note:fix Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) Team:Detection Engine Security Solution Detection Engine Area labels Jan 16, 2024
@e40pud e40pud self-assigned this Jan 16, 2024
@e40pud e40pud requested review from a team as code owners January 16, 2024 14:16
@e40pud e40pud requested a review from vitaliidm January 16, 2024 14:16
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-solution (Team: SecuritySolution)

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-detection-engine (Team:Detection Engine)

@e40pud e40pud requested a review from yctercero January 16, 2024 14:17
@e40pud e40pud requested a review from marshallmain January 16, 2024 16:44
@e40pud
Copy link
Contributor Author

e40pud commented Jan 18, 2024

@elasticmachine merge upstream

Copy link
Contributor

@vitaliidm vitaliidm left a comment

Choose a reason for hiding this comment

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

@e40pud , thanks for fixing this issue
I tested create/edit page and it looks good.

Have 2 observations:

  1. Can you please run added cypress test in flaky test runner, so we can be sure test is not flaky?
  2. Noticed on rule details page, filters do not show warning.
Screen.Recording.2024-01-22.at.11.03.31.mov

Is this known issue? If not, can in be fixed in this PR or separate ticket would more appropriate?

@e40pud
Copy link
Contributor Author

e40pud commented Jan 23, 2024

@vitaliidm thanks for the review and testing this functionality.

  1. I added fluky test runners (for ESS and Serverless) in description
  2. I checked and to fix how we show filters on details page we would need to do changes outside of security solution and it just adds complexity to this PR. I created a separate ticket for that issue [Security Solution] Rule details page: Filter does not show warning state #175306, so we can track and fix it in future.

@e40pud e40pud requested a review from vitaliidm January 23, 2024 13:53
@kibana-ci
Copy link
Collaborator

💚 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 11.1MB 11.1MB +200.0B

History

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

cc @e40pud

@e40pud e40pud merged commit f0f6274 into elastic:main Jan 24, 2024
35 checks passed
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jan 24, 2024
…ew on the Rule Editing page (elastic#174026) (elastic#174922)

## Summary

Addresses elastic#174026

These changes fix the issue with filter editing on the rule's editing
page when using index patterns instead of data view.

**Steps to reproduce**:
1. Create a custom query rule and add a filter
2. Save the rule
3. Edit the rule
4. Edit the filter

**Current behaviour**:
Right now when user tries to edit the filter the data view picking UI
appears even though index patterns were not modified.

<img width="1261" alt="Screenshot 2024-01-16 at 15 14 23"
src="https://github.com/elastic/kibana/assets/2700761/b2d28b79-a7d8-482c-a2be-fa8e20cb9e25">

**Expected behaviour**:
Data view picking UI should not be present and previously set field and
value options should be shown in the filter editing dialog.

<img width="1252" alt="Screenshot 2024-01-16 at 15 16 07"
src="https://github.com/elastic/kibana/assets/2700761/bb99dd9f-aa6a-4003-b8c4-ccda344c4c5c">

**Cause**:
The behaviour for the filter editing on rule’s editing page changed in
`8.11` with these changes elastic#166318.
We convert `DataViewBase` object without ID set to a `DataView` object
with auto-generated ID. This happens each time we try to edit the rule
and leads to a different ID which is saved in `filter.meta.index`.
Unified search internally checks those IDs to verify whether the filter
belongs to provided data view.

**Solution**:
To solve this issue, we set the data view id explicitly on creating an
in-memory data view that represents index patterns and update
`filter.meta.index` to use the same ID.

~~**Known issue**:
This does not resolve the issue for existing filters. In this case, user
will need to update their filters manually.~~ (This was fixed by
updating `filter.meta.index` field on rule editing)

**Flaky test runner**
[ESS 50
times](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/4927)
[Serverless 50
times](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/4935)

---------

Co-authored-by: Kibana Machine <[email protected]>
(cherry picked from commit f0f6274)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.12

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Jan 25, 2024
… &quot;Edit filter&quot; view on the Rule Editing page (#174026) (#174922) (#175433)

# Backport

This will backport the following commits from `main` to `8.12`:
- [[Security Solution] &quot;Data view&quot; selector is shown in
&quot;Edit filter&quot; view on the Rule Editing page (#174026)
(#174922)](#174922)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Ievgen
Sorokopud","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-01-24T13:34:54Z","message":"[Security
Solution] \"Data view\" selector is shown in \"Edit filter\" view on the
Rule Editing page (#174026) (#174922)\n\n## Summary\r\n\r\nAddresses
https://github.com/elastic/kibana/issues/174026\r\n\r\nThese changes fix
the issue with filter editing on the rule's editing\r\npage when using
index patterns instead of data view.\r\n\r\n**Steps to
reproduce**:\r\n1. Create a custom query rule and add a filter\r\n2.
Save the rule\r\n3. Edit the rule\r\n4. Edit the filter\r\n\r\n**Current
behaviour**:\r\nRight now when user tries to edit the filter the data
view picking UI\r\nappears even though index patterns were not
modified.\r\n\r\n<img width=\"1261\" alt=\"Screenshot 2024-01-16 at 15
14
23\"\r\nsrc=\"https://github.com/elastic/kibana/assets/2700761/b2d28b79-a7d8-482c-a2be-fa8e20cb9e25\">\r\n\r\n**Expected
behaviour**:\r\nData view picking UI should not be present and
previously set field and\r\nvalue options should be shown in the filter
editing dialog.\r\n\r\n<img width=\"1252\" alt=\"Screenshot 2024-01-16
at 15 16
07\"\r\nsrc=\"https://github.com/elastic/kibana/assets/2700761/bb99dd9f-aa6a-4003-b8c4-ccda344c4c5c\">\r\n\r\n**Cause**:\r\nThe
behaviour for the filter editing on rule’s editing page changed
in\r\n`8.11` with these changes
https://github.com/elastic/kibana/pull/166318.\r\nWe convert
`DataViewBase` object without ID set to a `DataView` object\r\nwith
auto-generated ID. This happens each time we try to edit the rule\r\nand
leads to a different ID which is saved in
`filter.meta.index`.\r\nUnified search internally checks those IDs to
verify whether the filter\r\nbelongs to provided data
view.\r\n\r\n**Solution**:\r\nTo solve this issue, we set the data view
id explicitly on creating an\r\nin-memory data view that represents
index patterns and update\r\n`filter.meta.index` to use the same
ID.\r\n\r\n~~**Known issue**:\r\nThis does not resolve the issue for
existing filters. In this case, user\r\nwill need to update their
filters manually.~~ (This was fixed by\r\nupdating `filter.meta.index`
field on rule editing)\r\n\r\n**Flaky test runner**\r\n[ESS
50\r\ntimes](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/4927)\r\n[Serverless
50\r\ntimes](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/4935)\r\n\r\n---------\r\n\r\nCo-authored-by:
Kibana Machine
<[email protected]>","sha":"f0f6274b9563cc453a11a937fe66f3bfbe8311bf","branchLabelMapping":{"^v8.13.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:
SecuritySolution","backport:prev-minor","Team:Detection
Engine","v8.13.0"],"title":"[Security Solution] \"Data view\" selector
is shown in \"Edit filter\" view on the Rule Editing page
(#174026)","number":174922,"url":"https://github.com/elastic/kibana/pull/174922","mergeCommit":{"message":"[Security
Solution] \"Data view\" selector is shown in \"Edit filter\" view on the
Rule Editing page (#174026) (#174922)\n\n## Summary\r\n\r\nAddresses
https://github.com/elastic/kibana/issues/174026\r\n\r\nThese changes fix
the issue with filter editing on the rule's editing\r\npage when using
index patterns instead of data view.\r\n\r\n**Steps to
reproduce**:\r\n1. Create a custom query rule and add a filter\r\n2.
Save the rule\r\n3. Edit the rule\r\n4. Edit the filter\r\n\r\n**Current
behaviour**:\r\nRight now when user tries to edit the filter the data
view picking UI\r\nappears even though index patterns were not
modified.\r\n\r\n<img width=\"1261\" alt=\"Screenshot 2024-01-16 at 15
14
23\"\r\nsrc=\"https://github.com/elastic/kibana/assets/2700761/b2d28b79-a7d8-482c-a2be-fa8e20cb9e25\">\r\n\r\n**Expected
behaviour**:\r\nData view picking UI should not be present and
previously set field and\r\nvalue options should be shown in the filter
editing dialog.\r\n\r\n<img width=\"1252\" alt=\"Screenshot 2024-01-16
at 15 16
07\"\r\nsrc=\"https://github.com/elastic/kibana/assets/2700761/bb99dd9f-aa6a-4003-b8c4-ccda344c4c5c\">\r\n\r\n**Cause**:\r\nThe
behaviour for the filter editing on rule’s editing page changed
in\r\n`8.11` with these changes
https://github.com/elastic/kibana/pull/166318.\r\nWe convert
`DataViewBase` object without ID set to a `DataView` object\r\nwith
auto-generated ID. This happens each time we try to edit the rule\r\nand
leads to a different ID which is saved in
`filter.meta.index`.\r\nUnified search internally checks those IDs to
verify whether the filter\r\nbelongs to provided data
view.\r\n\r\n**Solution**:\r\nTo solve this issue, we set the data view
id explicitly on creating an\r\nin-memory data view that represents
index patterns and update\r\n`filter.meta.index` to use the same
ID.\r\n\r\n~~**Known issue**:\r\nThis does not resolve the issue for
existing filters. In this case, user\r\nwill need to update their
filters manually.~~ (This was fixed by\r\nupdating `filter.meta.index`
field on rule editing)\r\n\r\n**Flaky test runner**\r\n[ESS
50\r\ntimes](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/4927)\r\n[Serverless
50\r\ntimes](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/4935)\r\n\r\n---------\r\n\r\nCo-authored-by:
Kibana Machine
<[email protected]>","sha":"f0f6274b9563cc453a11a937fe66f3bfbe8311bf"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.13.0","branchLabelMappingKey":"^v8.13.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/174922","number":174922,"mergeCommit":{"message":"[Security
Solution] \"Data view\" selector is shown in \"Edit filter\" view on the
Rule Editing page (#174026) (#174922)\n\n## Summary\r\n\r\nAddresses
https://github.com/elastic/kibana/issues/174026\r\n\r\nThese changes fix
the issue with filter editing on the rule's editing\r\npage when using
index patterns instead of data view.\r\n\r\n**Steps to
reproduce**:\r\n1. Create a custom query rule and add a filter\r\n2.
Save the rule\r\n3. Edit the rule\r\n4. Edit the filter\r\n\r\n**Current
behaviour**:\r\nRight now when user tries to edit the filter the data
view picking UI\r\nappears even though index patterns were not
modified.\r\n\r\n<img width=\"1261\" alt=\"Screenshot 2024-01-16 at 15
14
23\"\r\nsrc=\"https://github.com/elastic/kibana/assets/2700761/b2d28b79-a7d8-482c-a2be-fa8e20cb9e25\">\r\n\r\n**Expected
behaviour**:\r\nData view picking UI should not be present and
previously set field and\r\nvalue options should be shown in the filter
editing dialog.\r\n\r\n<img width=\"1252\" alt=\"Screenshot 2024-01-16
at 15 16
07\"\r\nsrc=\"https://github.com/elastic/kibana/assets/2700761/bb99dd9f-aa6a-4003-b8c4-ccda344c4c5c\">\r\n\r\n**Cause**:\r\nThe
behaviour for the filter editing on rule’s editing page changed
in\r\n`8.11` with these changes
https://github.com/elastic/kibana/pull/166318.\r\nWe convert
`DataViewBase` object without ID set to a `DataView` object\r\nwith
auto-generated ID. This happens each time we try to edit the rule\r\nand
leads to a different ID which is saved in
`filter.meta.index`.\r\nUnified search internally checks those IDs to
verify whether the filter\r\nbelongs to provided data
view.\r\n\r\n**Solution**:\r\nTo solve this issue, we set the data view
id explicitly on creating an\r\nin-memory data view that represents
index patterns and update\r\n`filter.meta.index` to use the same
ID.\r\n\r\n~~**Known issue**:\r\nThis does not resolve the issue for
existing filters. In this case, user\r\nwill need to update their
filters manually.~~ (This was fixed by\r\nupdating `filter.meta.index`
field on rule editing)\r\n\r\n**Flaky test runner**\r\n[ESS
50\r\ntimes](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/4927)\r\n[Serverless
50\r\ntimes](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/4935)\r\n\r\n---------\r\n\r\nCo-authored-by:
Kibana Machine
<[email protected]>","sha":"f0f6274b9563cc453a11a937fe66f3bfbe8311bf"}}]}]
BACKPORT-->

---------

Co-authored-by: Ievgen Sorokopud <[email protected]>
e40pud added a commit that referenced this pull request Jan 26, 2024
## Summary

This is a followup to #174922.
While doing [backport](#175433) of
that PR, I found that tests are failing in 8.12 because of not using
state updating path for updating `filters` property of the component.
This led to stale version of `filters` property being passed to the
`SearchBar` component and thus the initial fix was not working.


[Here](532178b)
is the fix for 8.12 which was cherry-picked here.

### Checklist

- [x] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed
- [ESS 50
times](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/4955)
- [Serverless 50
times](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/4956)
CoenWarmer pushed a commit to CoenWarmer/kibana that referenced this pull request Feb 15, 2024
…ew on the Rule Editing page (elastic#174026) (elastic#174922)

## Summary

Addresses elastic#174026

These changes fix the issue with filter editing on the rule's editing
page when using index patterns instead of data view.

**Steps to reproduce**:
1. Create a custom query rule and add a filter
2. Save the rule
3. Edit the rule
4. Edit the filter

**Current behaviour**:
Right now when user tries to edit the filter the data view picking UI
appears even though index patterns were not modified.

<img width="1261" alt="Screenshot 2024-01-16 at 15 14 23"
src="https://github.com/elastic/kibana/assets/2700761/b2d28b79-a7d8-482c-a2be-fa8e20cb9e25">

**Expected behaviour**:
Data view picking UI should not be present and previously set field and
value options should be shown in the filter editing dialog.

<img width="1252" alt="Screenshot 2024-01-16 at 15 16 07"
src="https://github.com/elastic/kibana/assets/2700761/bb99dd9f-aa6a-4003-b8c4-ccda344c4c5c">

**Cause**:
The behaviour for the filter editing on rule’s editing page changed in
`8.11` with these changes elastic#166318.
We convert `DataViewBase` object without ID set to a `DataView` object
with auto-generated ID. This happens each time we try to edit the rule
and leads to a different ID which is saved in `filter.meta.index`.
Unified search internally checks those IDs to verify whether the filter
belongs to provided data view.

**Solution**:
To solve this issue, we set the data view id explicitly on creating an
in-memory data view that represents index patterns and update
`filter.meta.index` to use the same ID.

~~**Known issue**:
This does not resolve the issue for existing filters. In this case, user
will need to update their filters manually.~~ (This was fixed by
updating `filter.meta.index` field on rule editing)

**Flaky test runner**
[ESS 50
times](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/4927)
[Serverless 50
times](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/4935)

---------

Co-authored-by: Kibana Machine <[email protected]>
CoenWarmer pushed a commit to CoenWarmer/kibana that referenced this pull request Feb 15, 2024
## Summary

This is a followup to elastic#174922.
While doing [backport](elastic#175433) of
that PR, I found that tests are failing in 8.12 because of not using
state updating path for updating `filters` property of the component.
This led to stale version of `filters` property being passed to the
`SearchBar` component and thus the initial fix was not working.


[Here](elastic@532178b)
is the fix for 8.12 which was cherry-picked here.

### Checklist

- [x] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed
- [ESS 50
times](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/4955)
- [Serverless 50
times](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/4956)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) release_note:fix Team:Detection Engine Security Solution Detection Engine Area Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.12.1 v8.13.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants