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] [Exceptions] Fix Exception Auto-populate from Alert actions #159908

Merged

Conversation

WafaaNasr
Copy link
Contributor

@WafaaNasr WafaaNasr commented Jun 19, 2023

WafaaNasr added 20 commits June 5, 2023 15:02
@WafaaNasr WafaaNasr added release_note:fix ci:cloud-deploy Create or update a Cloud deployment Team:Detection Engine Security Solution Detection Engine Area labels Jun 19, 2023
@WafaaNasr WafaaNasr self-assigned this Jun 19, 2023
@WafaaNasr WafaaNasr marked this pull request as ready for review June 19, 2023 16:56
@WafaaNasr WafaaNasr requested a review from a team as a code owner June 19, 2023 16:56
@yctercero
Copy link
Contributor

@elasticmachine merge upstream

@@ -419,6 +419,7 @@ export const EventFiltersForm: React.FC<ArtifactFormComponentProps & { allowSele
comments: exception?.comments ?? [],
os_types: exception?.os_types ?? [OperatingSystem.WINDOWS],
tags: exception?.tags ?? [],
meta: exception.meta,
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this change related to the bug fix or something else you found?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is related to the Event Filters component
The component was throwing the below issue, when we updated useEffect with the deps
image

Copy link
Contributor

@yctercero yctercero left a comment

Choose a reason for hiding this comment

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

Pulled down and tested the fix - it all seems to be working as expected now 👍🏽 We also tested together during an earlier meeting today. Didn't see values resetting to initial state like before.

I do agree with others that it's not our ideal fix, but you had noted this from the beginning and we'd chatted about whether to do a more in depth fix to revisit how we're setting initial state. We're in that post feature freeze time where we want to minimize the surface area we touch on fixes to avoid even more possible side effects. I think there's already things you've identified that could be done to improve the stability and maintainability of the component (and created an issue to track). Hoping we can do this work in 8.10.

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Security Solution Tests #4 / Alerts generated by building block rules Alerts should be visible on the Rule Detail page and not visible on the Overview page

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
lists 260 261 +1

Async chunks

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

id before after diff
lists 143.2KB 144.0KB +844.0B
securitySolution 11.0MB 11.0MB +33.0B
total +877.0B
Unknown metric groups

ESLint disabled line counts

id before after diff
enterpriseSearch 14 16 +2
lists 18 17 -1
securitySolution 413 417 +4
total +5

Total ESLint disabled count

id before after diff
enterpriseSearch 15 17 +2
lists 23 22 -1
securitySolution 492 496 +4
total +5

History

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

cc @WafaaNasr

@WafaaNasr WafaaNasr merged commit fdd709b into elastic:main Jun 28, 2023
@WafaaNasr WafaaNasr deleted the fix-endpointexception-autopopulateAlertActions branch June 28, 2023 11:14
@kibanamachine kibanamachine added v8.10.0 backport:skip This commit does not require backporting labels Jun 28, 2023
@WafaaNasr WafaaNasr restored the fix-endpointexception-autopopulateAlertActions branch June 28, 2023 11:15
@WafaaNasr
Copy link
Contributor Author

💚 All backports created successfully

Status Branch Result
8.9

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

Questions ?

Please refer to the Backport tool documentation

WafaaNasr added a commit to WafaaNasr/kibana that referenced this pull request Jun 28, 2023
…rt actions (elastic#159908)

## Summary

- Addresses  elastic#159784

---------

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
(cherry picked from commit fdd709b)
WafaaNasr added a commit that referenced this pull request Jun 28, 2023
…om Alert actions (#159908) (#160731)

# Backport

This will backport the following commits from `main` to `8.9`:
- [[Security Solution] [Exceptions] Fix Exception Auto-populate from
Alert actions (#159908)](#159908)

<!--- Backport version: 8.9.7 -->

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

<!--BACKPORT [{"author":{"name":"Wafaa
Nasr","email":"wafaa.nasr@elastic.co"},"sourceCommit":{"committedDate":"2023-06-28T11:14:19Z","message":"[Security
Solution] [Exceptions] Fix Exception Auto-populate from Alert actions
(#159908)\n\n## Summary\r\n\r\n- Addresses
https://github.com/elastic/kibana/issues/159784\r\n\r\n---------\r\n\r\nCo-authored-by:
Kibana Machine
<42973632+kibanamachine@users.noreply.github.com>","sha":"fdd709b02579b05124585b10fa6535d0a90480e2","branchLabelMapping":{"^v8.10.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","backport:skip","8.9
candidate","Team:Detection
Engine","v8.10.0"],"number":159908,"url":"https://github.com/elastic/kibana/pull/159908","mergeCommit":{"message":"[Security
Solution] [Exceptions] Fix Exception Auto-populate from Alert actions
(#159908)\n\n## Summary\r\n\r\n- Addresses
https://github.com/elastic/kibana/issues/159784\r\n\r\n---------\r\n\r\nCo-authored-by:
Kibana Machine
<42973632+kibanamachine@users.noreply.github.com>","sha":"fdd709b02579b05124585b10fa6535d0a90480e2"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.10.0","labelRegex":"^v8.10.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/159908","number":159908,"mergeCommit":{"message":"[Security
Solution] [Exceptions] Fix Exception Auto-populate from Alert actions
(#159908)\n\n## Summary\r\n\r\n- Addresses
https://github.com/elastic/kibana/issues/159784\r\n\r\n---------\r\n\r\nCo-authored-by:
Kibana Machine
<42973632+kibanamachine@users.noreply.github.com>","sha":"fdd709b02579b05124585b10fa6535d0a90480e2"}}]}]
BACKPORT-->
@@ -89,6 +89,7 @@ export const ADD_RULE_EXCEPTION_FROM_ALERT_COMMENT = (alertId: string) =>
'xpack.securitySolution.ruleExceptions.addExceptionFlyout.addRuleExceptionFromAlertComment',
{
values: { alertId },
defaultMessage: 'Exception conditions are pre-filled with relevant data from {alertId}.',
defaultMessage:
'Exception conditions are pre-filled with relevant data from alert with "id" {alertId}.',
Copy link
Contributor

Choose a reason for hiding this comment

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

@WafaaNasr @yctercero what's the field that stores the alert ID value? Is it the _id field? If it is, here's the revision I suggest:

    'Exception conditions are pre-filled with relevant data from an alert with the alert id (_id): {alertId}.',

If you think including the field name is too much or would be confusing, you can remove it, for ex:

    'Exception conditions are pre-filled with relevant data from an alert with the alert id: {alertId}.',

Copy link
Contributor

Choose a reason for hiding this comment

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

One more note: In the first option, I included the field that stores the alert value in case users need to know what field value pair to query when searching for the alert.

WafaaNasr added a commit that referenced this pull request Jul 5, 2023
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jul 5, 2023
kibanamachine added a commit that referenced this pull request Jul 5, 2023
…t` text (#161092) (#161242)

# Backport

This will backport the following commits from `main` to `8.9`:
- [[Security Solution] [Exceptions] Amend `Rule Exception's Comment`
text (#161092)](#161092)

<!--- Backport version: 8.9.7 -->

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

<!--BACKPORT [{"author":{"name":"Wafaa
Nasr","email":"wafaa.nasr@elastic.co"},"sourceCommit":{"committedDate":"2023-07-05T10:44:03Z","message":"[Security
Solution] [Exceptions] Amend `Rule Exception's Comment` text
(#161092)\n\n## Summary\r\n\r\n- Addresses Docs team
comment\r\nhttps://github.com//pull/159908#discussion_r1247964658","sha":"16528cf28994b30642262a018c89c5217c28f884","branchLabelMapping":{"^v8.10.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","backport:prev-minor","v8.9.0","Team:Detection
Engine","v8.10.0"],"number":161092,"url":"https://github.com/elastic/kibana/pull/161092","mergeCommit":{"message":"[Security
Solution] [Exceptions] Amend `Rule Exception's Comment` text
(#161092)\n\n## Summary\r\n\r\n- Addresses Docs team
comment\r\nhttps://github.com//pull/159908#discussion_r1247964658","sha":"16528cf28994b30642262a018c89c5217c28f884"}},"sourceBranch":"main","suggestedTargetBranches":["8.9"],"targetPullRequestStates":[{"branch":"8.9","label":"v8.9.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.10.0","labelRegex":"^v8.10.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/161092","number":161092,"mergeCommit":{"message":"[Security
Solution] [Exceptions] Amend `Rule Exception's Comment` text
(#161092)\n\n## Summary\r\n\r\n- Addresses Docs team
comment\r\nhttps://github.com//pull/159908#discussion_r1247964658","sha":"16528cf28994b30642262a018c89c5217c28f884"}}]}]
BACKPORT-->

Co-authored-by: Wafaa Nasr <wafaa.nasr@elastic.co>
@WafaaNasr WafaaNasr deleted the fix-endpointexception-autopopulateAlertActions branch February 6, 2024 10:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.9 candidate backport:skip This commit does not require backporting release_note:fix Team:Detection Engine Security Solution Detection Engine Area v8.9.0 v8.10.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants