[Cloud Security] [Findings] Adding grouping component #169884

opauloh · 2023-10-25T21:27:57Z

Summary

This PR introduces a new Grouping feature to the Findings page as described in #168542. It uses the Grouping component from the @kbn/securitysolution-grouping package abstracts all common code related to tables across our solutions.

Changes included

@kbn/securitysolution-grouping:

Updated grouping component logic to behave in toggle mode when maxGroupingLeves = 1
Added an alternative label to the grouping component when when maxGroupingLeves = 1 ("Select grouping" instead of "Select up to 1 groupings")

Findings page

Added group by component
Added default group by options: None (default), Resource, Rule name, Cloud account, Kubernetes
Reusing the latest findings table for rendering the table visualizations when expanding a group with a filter (added nonPersistedFilter to combine the group by filter with the Url Params filtering)

Dashboard

Changed redirect link from the findings resources page to the findings page.

Out of scope (not included)

Removing the code for the group by resource pages that are no longer used will be done in a separate ticket alongside code optimizations due to refactoring.
Case insensitive sort of the results as it will need to be addressed in a separate effort.
Custom rendering for each of the group by default views (to be addressed in separate efforts)

Screenshot

Recordings

Screen.Recording.2023-11-15.at.12.05.31.AM.mov

Screen.Recording.2023-11-15.at.12.06.52.AM.mov

…fix'

packages/kbn-securitysolution-grouping/src/components/group_selector/index.tsx

maxcold · 2023-11-02T11:05:32Z

packages/kbn-securitysolution-grouping/src/components/group_selector/index.tsx

+        return false;
+      }
+      // Disable all non selected options when the maxGroupingLevels is reached
+      return groupsSelected.length === maxGroupingLevels && (key ? !isGroupSelected(key) : true);


should we keep the behaviour of disabling currently selected option when the max level is 1? Right now to remove grouping, you can either switch to none or "unselect" the option. If we change the behavior to toggle when max level is 1, maybe it makes sense to disable this "unselect" behaviour

great point, I'll adjust it in the code and bring it to discussion with the code owners

thats fine, can you please just add some unit tests? We never get to line 53 in the current tests

I added the unit tests related to the changes introduced in this PR, thanks a lot for reviewing it @stephmilovic!

packages/kbn-securitysolution-grouping/src/hooks/use_get_group_selector.tsx

x-pack/plugins/cloud_security_posture/public/common/types.ts

...s/cloud_security_posture/public/components/cloud_security_data_table/additional_controls.tsx

maxcold · 2023-11-02T11:19:22Z

I was confused by having Rows per page here at first until I realised that it acutally changes the number of groups per page. Maybe it makes sense to change the wording to "Groups per page"? It will be incosistent with Alerts, but maybe this change makes sense for them as well

...d_security_posture/public/pages/configurations/latest_findings/latest_findings_container.tsx

...s/cloud_security_posture/public/components/cloud_security_data_table/additional_controls.tsx

...d_security_posture/public/pages/configurations/latest_findings/latest_findings_container.tsx

JordanSh · 2023-11-06T10:41:45Z

...d_security_posture/public/pages/configurations/latest_findings/latest_findings_container.tsx

+    additionalFilters: [query],
+    groupByField: selectedGroup,
+    uniqueValue,
+    from: 'now-1y',


its an unlikely scenario but why this time limit? isnt showing the relevant results the job of the transform?

yes, currently our dashboard shows data without any filter, so to ensure consistency in the eventual case that the transform stops running I kept the time limit high. But I see now that this is going to change with this PR, so I will update the query to consider the retention policy instead, and keep in sync with the same filter used by the Dashboard.

...d_security_posture/public/pages/configurations/latest_findings/latest_findings_container.tsx

.../cloud_security_posture/public/pages/configurations/latest_findings/use_grouped_findings.tsx

JordanSh · 2023-11-06T11:23:35Z

Does this table use virtualization? it seems like the items disappear when scrolling, and you dont have to scroll a lot in order to get to the not-render zones

Screen.Recording.2023-11-06.at.13.18.47.mov

I would like to suggest to either extend the amount of items we do render, or to test if we can completely remove virtualization from the table for now. we only render a maximum amount of 500 items, 2 tables cannot be opened at once so i believe react can easily handle 500 rows of text.

opauloh · 2023-11-11T02:15:08Z

Does this table use virtualization? it seems like the items disappear when scrolling, and you dont have to scroll a lot in order to get to the not-render zones

Screen.Recording.2023-11-06.at.13.18.47.mov
I would like to suggest to either extend the amount of items we do render, or to test if we can completely remove virtualization from the table for now. we only render a maximum amount of 500 items, 2 tables cannot be opened at once so i believe react can easily handle 500 rows of text.

Virtualization was implemented on purpose since users can load up to 10k records using the "load more button" when going to the last page, and then without virtualization, the table starts to freeze.

I think we can try instead to set additional settings on the virtualization options such as increasing overscanRowCount to improve the experience when scrolling to fewer items and also adding a style for rows that are being loaded while scrolling so we don't show just a blank while scrolling (like on this example), WDYT?

packages/kbn-securitysolution-grouping/src/containers/query/types.ts

opauloh · 2023-11-16T20:58:23Z

...ity_posture/public/common/hooks/use_cloud_posture_data_table/use_cloud_posture_data_table.ts

+/*
+  Hook for managing common table state and methods for the Cloud Posture DataTable
+*/
+export const useCloudPostureDataTable = ({


This hook was created from the use_cloud_posture_table and it will replace it as we aim to remove old code in future PRs.

maxcold · 2023-11-23T12:07:56Z

When a data grid has a small amount of rows, it still takes up a lot of space. I guess this is know, just wanted to check if there is a plan to improve it later on

maxcold · 2023-11-23T12:16:11Z

Errors in the search query are not recoverable as it seems. I'm not 100% sure that it's specific to your PR or not, but on 8.11/8.12 ESS errors are recoverable. Here is what I mean:
Serverless in this branch. It's not possible to recover from the error rather then updating the url. I'm searching for @timestamp : 2023-11-23T10:53:48.410Z

While in 8.12 ESS it looks like this. I can change the search to recover

maxcold

Looks good, couple of nit comments and a couple of things to check:

let's check if the non-recoverable error is specific to this pr or is it serverless-specific. If it's introduced in this PR, let's fix it
I'd add some unit tests to the shared library, at least to cover changes introduced by this PR

maxcold · 2023-11-23T11:51:18Z

packages/kbn-securitysolution-grouping/src/hooks/use_get_group_selector.tsx

-          setSelectedGroups(['none']);
-        } else {
-          setSelectedGroups(groups);
+      // Simulate a toggle behavior when maxGroupingLevels is 1


I think kbn-securitysolution-grouping lib would benefit from having some unit tests. As it didn't have any, I wouldn't block this PR on that, but I'd vote for adding some of the tests which are testing your changes to the lib

I think kbn-securitysolution-grouping lib would benefit from having some unit tests. As it didn't have any...

@maxcold Why do you think there are not unit tests? We do have tests that should have been added to :

packages/kbn-securitysolution-grouping/src/components/group_selector/index.test.tsx

packages/kbn-securitysolution-grouping/src/hooks/use_get_group_selector.test.tsx

Please add unit tests

@stephmilovic totally my bad, I have no idea where I looked tbh, but I remember checking the codebase in my code editor and not finding the unit test files. Some kind of a blackout from my side I guess :) for sure we need to cover new cases with unit tests

maxcold · 2023-11-23T11:55:17Z

x-pack/plugins/cloud_security_posture/public/common/hooks/use_cloud_posture_data_table/utils.ts

+  size: pageSize,
+});
+
+export const useBaseEsQuery = ({


should we move it to own hook file? not really a util

maxcold · 2023-11-23T12:22:36Z

...cloud_security_posture/public/components/cloud_security_grouping/cloud_security_grouping.tsx

+}: CloudSecurityGroupingProps) => {
+  return (
+    <div
+      data-test-subj="cloudSecurityGrouping"


should we move it to TEST_SUBJECTS constant?

I agree, I'm going to address those neats in the next PR

stephmilovic · 2023-11-27T15:29:50Z

packages/kbn-securitysolution-grouping/src/hooks/use_get_group_selector.tsx

-          setSelectedGroups(groups);
+      // Simulate a toggle behavior when maxGroupingLevels is 1
+      if (maxGroupingLevels === 1) {
+        setSelectedGroups([groupSelection]);


this looks to be the only untested condition here

test added 🙌

stephmilovic · 2023-11-27T15:36:43Z

I was confused by having Rows per page here at first until I realised that it acutally changes the number of groups per page. Maybe it makes sense to change the wording to "Groups per page"? It will be incosistent with Alerts, but maybe this change makes sense for them as well

Agreed, this especially becomes confusing because when you open up an alerts table:

We can do in follow up

stephmilovic

Before merging, please add a few unit tests for the code changes in kbn-securitysolution-grouping. LGTM other than that. Thanks for the improvements

opauloh · 2023-11-27T23:43:55Z

I was confused by having Rows per page here at first until I realised that it acutally changes the number of groups per page. Maybe it makes sense to change the wording to "Groups per page"? It will be incosistent with Alerts, but maybe this change makes sense for them as well

Agreed, this especially becomes confusing because when you open up an alerts table:

We can do in follow up

Great point, I'll add that in a separate PR

kibana-ci · 2023-11-28T00:01:08Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 6ff86ab
Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-169884-6ff86abf3b85
Security Deployment

Failed CI Steps

Serverless Investigations - Security Solution Cypress Tests #5

Test Failures

[job] [logs] Serverless Investigations - Security Solution Cypress Tests #5 / Detection response view Open in timeline "after each" hook for "opens timeline with correct query count for hosts by alert severity table" "after each" hook for "opens timeline with correct query count for hosts by alert severity table"
[job] [logs] Serverless Investigations - Security Solution Cypress Tests #5 / Detection response view Open in timeline opens timeline with correct query count for hosts by alert severity table opens timeline with correct query count for hosts by alert severity table
[job] [logs] Serverless Investigations - Security Solution Cypress Tests #5 / Detection response view Redirection to AlertPage should redirect to alert page with host and status as the filters should redirect to alert page with host and status as the filters
[job] [logs] Serverless Investigations - Security Solution Cypress Tests #5 / Detection response view Redirection to AlertPage should redirect to alert page with host, status and severity as the filters should redirect to alert page with host, status and severity as the filters
[job] [logs] Serverless Investigations - Security Solution Cypress Tests #5 / Detection response view Redirection to AlertPage should redirect to alert page with rule name & status as filters should redirect to alert page with rule name & status as filters
[job] [logs] Serverless Investigations - Security Solution Cypress Tests #5 / Detection response view Redirection to AlertPage should redirect to alert page with user and status as the filters should redirect to alert page with user and status as the filters
[job] [logs] Serverless Investigations - Security Solution Cypress Tests #5 / Detection response view Redirection to AlertPage should redirect to alert page with user, status and severity as the filters should redirect to alert page with user, status and severity as the filters

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`cloudSecurityPosture`	384	425	+41

Async chunks

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

id	before	after	diff
`cloudSecurityPosture`	401.5KB	427.6KB	+26.1KB
`securitySolution`	12.8MB	12.8MB	+338.0B
total			+26.4KB

Canvas Sharable Runtime

The Canvas "shareable runtime" is an bundle produced to enable running Canvas workpads outside of Kibana. This bundle is included in third-party webpages that embed canvas and therefor should be as slim as possible.

id	before	after	diff
`module count`	-	5657	+5657
`total size`	-	5.9MB	+5.9MB

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id	before	after	diff
`@kbn/securitysolution-grouping`	7	8	+1

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`cloudSecurityPosture`	15.0KB	15.2KB	+245.0B

Unknown metric groups

miscellaneous assets size

id	before	after	diff
`cloudSecurityPosture`	213.1KB	915.0KB	⚠️ +701.9KB

History

💛 Build #177807 was flaky dca18ba
💔 Build #177777 failed 3d866de
💔 Build #177509 failed 2c35de7
💔 Build #177174 failed 70e9539
💚 Build #176597 succeeded 95f8cf4

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

…72256) ## Summary This PR adds custom rendering for each of the default Grouping visualizations: - #168543 - #169043 - #169044 - #169045 **It also adds:** - Fix error handling (follow up from [this comment](#169884 (comment))) - Change the Findings page to have the Misconfiguration tab in the first position. - Added `size` property to the `ComplianceScoreBar` component - Custom message for groups that don't have value (ex. No Cloud accounts) - Changed the sort order of grouping components to be based on the compliance score - Added compliance score for custom renderers ### Screenshot Resource <img width="1492" alt="image" src="https://github.com/elastic/kibana/assets/19270322/596f8bdb-abcc-4325-8512-23c919c727a9"> Rule name <img width="1489" alt="image" src="https://github.com/elastic/kibana/assets/19270322/787138e3-b3b2-4e15-811a-84c583831469"> Cloud account <img width="1490" alt="image" src="https://github.com/elastic/kibana/assets/19270322/9a48145d-dba5-4eda-bd7d-a97ed8f78a2d"> <img width="1492" alt="image" src="https://github.com/elastic/kibana/assets/19270322/399d0be0-4bc0-4090-ac20-e4b016cc4be5"> Kubernetes <img width="1499" alt="image" src="https://github.com/elastic/kibana/assets/19270322/3745498a-969a-4769-b4ae-3c932511a5a9"> Custom field: <img width="1488" alt="image" src="https://github.com/elastic/kibana/assets/19270322/8c75535d-2248-4cf9-b1cb-9b0d318114e9"> --------- Co-authored-by: kibanamachine <[email protected]>

opauloh and others added 15 commits October 13, 2023 16:29

wip: grouping

8bbb01a

group selector fix

c40e2fc

WIP: group selector on findings page

0ac7e9e

adding more group by fields

bb7f308

Merge branch 'main' into grouping/foundation

1bacb6a

add non url additional filters

c9f251e

Merge branch 'main' into grouping/foundation

9432b87

adding custom title

4fd1510

additional filters

fd4e071

datatable group component

40a7797

clean up code

bd80f34

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

befa489

…fix'

add more information to disabled

2f761d5

code clean up

ec0dbdf

hiding take action button

d55081c

opauloh requested review from Omolola-Akinleye and JordanSh October 26, 2023 05:39

maxcold added the ci:project-deploy-security Create a Security Serverless Project label Nov 2, 2023