[Cloud Security] Alerts Datagrids for Contextual Flyout by animehart · Pull Request #199573 · elastic/kibana

animehart · 2024-11-09T09:43:15Z

Summary

This PR is for Alerts Datagrid component in Contextual Flyout

This PR is for Alerts Datagrid in Contextual Flyout for User name and Host name

animehart · 2024-11-13T19:16:10Z

/ci

animehart · 2024-11-13T20:42:02Z

/ci

animehart · 2024-11-13T22:49:35Z

/ci

elasticmachine · 2024-11-13T22:51:13Z

Pinging @elastic/kibana-cloud-security-posture (Team:Cloud Security)

…f github.com:animehart/kibana into contextual-flyout-alerts-datagrid-from-preview-mark1

maxcold · 2024-11-14T08:41:19Z

...plugins/security_solution/public/cloud_security_posture/components/alerts/alerts_preview.tsx


  const totalAlertsCount = alertStats.reduce((total, item) => total + item.count, 0);

+  const { data } = useMisconfigurationPreview({


I don't understand why we need all this logic (getting misconfigs, vulnerabilities, riskScore) in the alert preview. From what I see we open the Alerts data grid in the left section of the flyout, this section doesn't have information about risk, misconfigs and vulnerabilities, then why all this data is needed?

We need this to pass it to the openLeftPanel, if we don't have this then when user opens the left panel, it doesn't know if it also has to render Misconfiguration/Vulnerabilities/Risk Score tabs

this logic is also being used on the 2 other preview component

Follow up on our discussion in DM:

First problem that we want to address is to encapsulate this logic in one place because right now it's copied over in 6 different places. We can start with one hook per piece of business logic (alerts, misconfigs, vulnerabilities, risk score) exposing and consuming only the pieces needed to a particular component. another possibility is to share this state through context

Another question, probably post 8.17 FF is to check if it's a good idea to have all these props passed to openLeftPanel . We do similar things in 3 places: load data we don't need in the component only to pass it to openLeftPanel. Maybe it should be self aware and get the information required to show the tabs by itself instead of getting them from the props

Here is the followup PR point number 1 above

maxcold · 2024-11-14T08:42:20Z

.../security_solution/public/cloud_security_posture/components/csp_details/insights_tab_csp.tsx

+        ) : activeInsightsId === CspInsightLeftPanelSubTab.VULNERABILITIES ? (
          <VulnerabilitiesFindingsDetailsTable queryName={name} />
+        ) : (
+          // <div>{'ALERTS HERE'}</div> //AlertsDetailsTable


maxcold · 2024-11-14T08:47:31Z

x-pack/plugins/security_solution/public/flyout/entity_details/user_right/index.tsx


  const hasMisconfigurationFindings = passedFindings > 0 || failedFindings > 0;

+  const { signalIndexName } = useSignalIndex();


you have this logic duplicated in multiple places, we need to abstract it

maxcold · 2024-11-14T08:50:07Z

x-pack/plugins/security_solution/public/cloud_security_posture/components/entity_insight.tsx

+        <MisconfigurationsPreview
+          name={name}
+          fieldName={fieldName}
+          hasNonClosedAlerts={alertsCount > 0}


It feels conceptually incorrect that the Misconfiguration and Vulnerability preview should know about non-closed alerts. Why do we need it here?

same reason as the checking existence of Misconfiguration and Vulnerability in Alerts Preview, we need to pass this information when opening left panel so it knows what tabs gets rendered

machadoum

EA team changes LGTM!

I left some minor comments. Could you please take a look?

machadoum · 2024-11-14T08:54:53Z

x-pack/plugins/security_solution/public/flyout/entity_details/host_right/index.tsx

+  const alertsOpenCount = alertsData?.open?.total || 0;
+
+  const alertsAcknowledgedCount = alertsData?.acknowledged?.total || 0;
+
+  const alertsCount = alertsOpenCount + alertsAcknowledgedCount;
+
+  const hasNonClosedAlerts = alertsCount;


nit: When a variable name starts with "has", it is expected to be a boolean.

Suggested change

const alertsOpenCount = alertsData?.open?.total || 0;

const alertsAcknowledgedCount = alertsData?.acknowledged?.total || 0;

const alertsCount = alertsOpenCount + alertsAcknowledgedCount;

const hasNonClosedAlerts = alertsCount;

const hasNonClosedAlerts = (alertsData?.acknowledged?.total || 0 + alertsData?.open?.total || 0) > 0;

machadoum · 2024-11-14T08:56:40Z

.../security_solution/public/cloud_security_posture/components/csp_details/insights_tab_csp.tsx

+        ) : activeInsightsId === CspInsightLeftPanelSubTab.VULNERABILITIES ? (
          <VulnerabilitiesFindingsDetailsTable queryName={name} />
+        ) : (
+          // <div>{'ALERTS HERE'}</div> //AlertsDetailsTable


You forgot some comments here.

elasticmachine · 2024-11-14T09:52:10Z

💚 Build Succeeded

Buildkite Build
Commit: 8cff4f7

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`securitySolution`	6201	6202	+1

Async chunks

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

id	before	after	diff
`securitySolution`	13.4MB	13.4MB	+7.3KB

History

💔 Build #251082 failed 15d5a57
💚 Build #251078 succeeded 0264507
💔 Build #251067 failed 41bdc3b
💛 Build #251044 was flaky 30ae6d8
💔 Build #251015 failed 2ea021a

maxcold · 2024-11-14T15:03:27Z

...plugins/security_solution/public/cloud_security_posture/components/alerts/alerts_preview.tsx

+
+  const { openLeftPanel } = useExpandableFlyoutApi();
+
+  const goToEntityInsightTab = useCallback(() => {


let's also move this logic into own component/utility . You have this goToEntityInsightTab in all three preview component, and it's only different by subTab and the tooltip. So it can be just a hook that depending on the passed type (alerts, misconfiguration, vulnerability) returns the link

maxcold · 2024-11-14T15:43:13Z

To not turn PR into huge refactoring PR, let's merge this one and do the refactoring of removing code duplication in a follow up. I tested the current logic, it works as expected

kibanamachine · 2024-11-14T19:07:44Z

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/11843860481

## Summary This PR is for Alerts Datagrid component in Contextual Flyout This PR is for Alerts Datagrid in Contextual Flyout for User name and Host name <img width="1480" alt="Screenshot 2024-11-14 at 9 08 26 AM" src="https://github.com/user-attachments/assets/46a254c8-b7f1-4b63-9637-2b1c281d5502"> --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> (cherry picked from commit ab965f7)

kibanamachine · 2024-11-14T19:12:20Z

💚 All backports created successfully

Status	Branch	Result
✅	8.x

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

Questions ?

Please refer to the Backport tool documentation

… (#200245) # Backport This will backport the following commits from `main` to `8.x`: - [[Cloud Security] Alerts Datagrids for Contextual Flyout (#199573)](#199573)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Rickyanto Ang <rickyangwyn@gmail.com>

## Summary This PR is for Alerts Datagrid component in Contextual Flyout This PR is for Alerts Datagrid in Contextual Flyout for User name and Host name <img width="1480" alt="Screenshot 2024-11-14 at 9 08 26 AM" src="https://github.com/user-attachments/assets/46a254c8-b7f1-4b63-9637-2b1c281d5502"> --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>

animehart added 13 commits October 21, 2024 09:43

initial Push

329020e

reversed array for dist bar

344a65f

Merge branch 'main' into contextual-flyout-alerts-preview-host

34fffd2

added test + updated logic to show entity insight component

c457662

updated logic to only exlude closed alerts

25e54ba

updated hooks used

d49d5ab

reverting logic to previous for preview component

136b04c

fix capital letter issue

3362998

initial push for alerts table

45b9338

clean up + implemented open flyout from table

61ec46d

update open flyout logic

59dceab

changed fields to use

41f1833

resolving conflicts

2ea021a

fix quick checks

30ae6d8

added navigate to alerts page

41bdc3b

animehart marked this pull request as ready for review November 13, 2024 22:49

animehart requested review from a team as code owners November 13, 2024 22:49

animehart requested a review from machadoum November 13, 2024 22:49

animehart added backport:prev-minor release_note:skip Skip the PR/issue when compiling release notes Team:Cloud Security Cloud Security team related labels Nov 13, 2024

animehart added v9.0.0 v8.17.0 labels Nov 13, 2024

more clean up + added unit test for helper

0264507

more improvements

5beeed8

animehart requested a review from a team as a code owner November 14, 2024 02:30

animehart requested a review from maxcold November 14, 2024 02:39

kibanamachine and others added 2 commits November 14, 2024 02:40

[CI] Auto-commit changed files from 'node scripts/notice'

15d5a57

reverting some changes

edfc433

animehart removed the request for review from a team November 14, 2024 05:35

kibanamachine and others added 5 commits November 14, 2024 05:48

[CI] Auto-commit changed files from 'node scripts/notice'

d011db8

update q name

68d5c11

Merge branch 'contextual-flyout-alerts-datagrid-from-preview-mark1' o…

66dc357

…f github.com:animehart/kibana into contextual-flyout-alerts-datagrid-from-preview-mark1

clean up

4e39671

reversed distribution bar array

8cff4f7

maxcold requested changes Nov 14, 2024

View reviewed changes

machadoum approved these changes Nov 14, 2024

View reviewed changes

maxcold reviewed Nov 14, 2024

View reviewed changes

maxcold approved these changes Nov 14, 2024

View reviewed changes

pr comments + replaced reason column with status

410cadc

animehart merged commit ab965f7 into elastic:main Nov 14, 2024

kibanamachine mentioned this pull request Nov 14, 2024

[8.x] [Cloud Security] Alerts Datagrids for Contextual Flyout (#199573) #200245

Merged


		const totalAlertsCount = alertStats.reduce((total, item) => total + item.count, 0);

		const { data } = useMisconfigurationPreview({


		const hasMisconfigurationFindings = passedFindings > 0 \|\| failedFindings > 0;

		const { signalIndexName } = useSignalIndex();


		const { openLeftPanel } = useExpandableFlyoutApi();

		const goToEntityInsightTab = useCallback(() => {

Conversation

animehart commented Nov 9, 2024 • edited by kibanamachine Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

animehart commented Nov 13, 2024

Uh oh!

animehart commented Nov 13, 2024

Uh oh!

animehart commented Nov 13, 2024

Uh oh!

elasticmachine commented Nov 13, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

animehart Nov 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

machadoum left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Nov 14, 2024

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

History

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maxcold commented Nov 14, 2024

Uh oh!

kibanamachine commented Nov 14, 2024

Uh oh!

kibanamachine commented Nov 14, 2024

💚 All backports created successfully

Questions ?

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

animehart commented Nov 9, 2024 •

edited by kibanamachine

Loading

animehart Nov 14, 2024 •

edited

Loading