[Cloud Security Posture][Benchmark Improvements] Rules Page Improvements phase 1 + Empty Benchmark #173345

animehart · 2023-12-14T02:53:40Z

Summary

This PR

updates the Rules Page looks to accomodate the changes we made to Benchmark page
adds Empty page (when user doesn't have any CSP integration) for Benchmark page
updates the NoFindings components message when users don't have any CSP integration installed

Summarize your PR. If it involves visual changes include a screenshot or gif.

animehart · 2023-12-14T07:57:39Z

/ci

animehart · 2023-12-14T23:04:17Z

/ci

…into rule-page-phase1-clean

elasticmachine · 2023-12-19T11:56:00Z

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

JordanSh · 2023-12-19T12:14:17Z

x-pack/plugins/cloud_security_posture/common/utils/helpers.ts

+    default:
+      return null;


is this needed? we will already get undefined as default if the matching strings won't be passed

JordanSh · 2023-12-19T12:14:32Z

x-pack/plugins/cloud_security_posture/common/utils/helpers.ts

+    default:
+      return null;


same comment here

JordanSh · 2023-12-19T12:17:51Z

x-pack/plugins/cloud_security_posture/common/utils/helpers.ts

+export const getBenchmarkFilterQuery = (
+  id: BenchmarkId,
+  version?: string,
+  selectParams?: BenchmarkRuleSelectParams
+): string => {
+  const baseQuery = `${CSP_BENCHMARK_RULE_SAVED_OBJECT_TYPE}.attributes.metadata.benchmark.id:${id} AND ${CSP_BENCHMARK_RULE_SAVED_OBJECT_TYPE}.attributes.metadata.benchmark.version:"v${version}"`;
+  const sectionQuery = ` AND ${CSP_BENCHMARK_RULE_SAVED_OBJECT_TYPE}.attributes.metadata.section: "${selectParams?.section}"`;
+  const ruleNumberQuery = ` AND ${CSP_BENCHMARK_RULE_SAVED_OBJECT_TYPE}.attributes.metadata.benchmark.rule_number: "${selectParams?.ruleNumber}"`;
+  if (!selectParams?.section && !selectParams?.ruleNumber) return baseQuery;
+  else if (selectParams.section && selectParams.ruleNumber)
+    return baseQuery + sectionQuery + ruleNumberQuery;
+  else if (selectParams.section) return baseQuery + sectionQuery;
+  else return baseQuery + ruleNumberQuery;
 };


Suggested change

export const getBenchmarkFilterQuery = (

id: BenchmarkId,

version?: string,

selectParams?: BenchmarkRuleSelectParams

): string => {

const baseQuery = `${CSP_BENCHMARK_RULE_SAVED_OBJECT_TYPE}.attributes.metadata.benchmark.id:${id} AND ${CSP_BENCHMARK_RULE_SAVED_OBJECT_TYPE}.attributes.metadata.benchmark.version:"v${version}"`;

const sectionQuery = ` AND ${CSP_BENCHMARK_RULE_SAVED_OBJECT_TYPE}.attributes.metadata.section: "${selectParams?.section}"`;

const ruleNumberQuery = ` AND ${CSP_BENCHMARK_RULE_SAVED_OBJECT_TYPE}.attributes.metadata.benchmark.rule_number: "${selectParams?.ruleNumber}"`;

if (!selectParams?.section && !selectParams?.ruleNumber) return baseQuery;

else if (selectParams.section && selectParams.ruleNumber)

return baseQuery + sectionQuery + ruleNumberQuery;

else if (selectParams.section) return baseQuery + sectionQuery;

else return baseQuery + ruleNumberQuery;

};

export const getBenchmarkFilterQuery = (

id: BenchmarkId,

version?: string,

selectParams?: BenchmarkRuleSelectParams

): string => {

const baseQuery = `${CSP_BENCHMARK_RULE_SAVED_OBJECT_TYPE}.attributes.metadata.benchmark.id:${id} AND ${CSP_BENCHMARK_RULE_SAVED_OBJECT_TYPE}.attributes.metadata.benchmark.version:"v${version}"`;

const sectionQuery = selectParams?.section ? ` AND ${CSP_BENCHMARK_RULE_SAVED_OBJECT_TYPE}.attributes.metadata.section: "${selectParams.section}"` : '';

const ruleNumberQuery = selectParams?.ruleNumber ? ` AND ${CSP_BENCHMARK_RULE_SAVED_OBJECT_TYPE}.attributes.metadata.benchmark.rule_number: "${selectParams.ruleNumber}"` : '';

return baseQuery + sectionQuery + ruleNumberQuery;

};

This should do the same thing but keep it easier to read

JordanSh · 2023-12-19T12:26:03Z

x-pack/plugins/cloud_security_posture/public/components/no_findings_states.tsx

-                </EuiLink>
-              ),
-            }}
+            defaultMessage="Maintain the confidentiality, integrity, and availability of your data in the cloud by continuously identifying and remediating potential configuration risks, like publicly accessible s3 buckets, using Elastic Security posture management for Cloud & Kubernetes"


let's do this in a different PR. as we talked about it in the last sync, product & design still need to provide a final version for this text

JordanSh · 2023-12-19T12:38:36Z

x-pack/plugins/cloud_security_posture/public/pages/benchmarks/benchmarks.tsx

-        }
-      />
+      {showConfigurationInstallPrompt ? (
+        <NoFindingsStates posturetype={'all'} />


Suggested change

<NoFindingsStates posturetype={'all'} />

<NoFindingsStates postureType={'all'} />

let's change the prop to use camel casing.
also, what does 'all' does? i did a quick look for it and couldn't find a use of it

originally back then, 'all' means either cspm or kspm (it doesnt matter). In this case it doesn't really matter if we use 'all', 'cspm' or 'kspm' as we wanted to render the case where both cspm and kspm are both not-installed.
Should I just use 'cspm' or 'kspm' in this case then ?

so there's no difference at all between cspm kspm and all? maybe just change it to be either cloud or vulnerabilities?

no i mean, just for this case (where we want to use NoFindingsState component for benchmark empty page), it doesn't matter whether we use cspm/kspm/all.

JordanSh · 2023-12-19T13:48:42Z

x-pack/plugins/cloud_security_posture/public/pages/rules/rules_table_header.tsx

  return (
    <EuiFlexGroup>
-      <EuiFlexItem>
+      <EuiFlexItem grow={4}>


A few fixes needed for this bar:

there is an empty space left on the right of the screen

the buttons are not equal in size

in the figma they appear as grouped buttons:

Figma:

Actual:

meaning they share borders. try to see if we can achieve that without too many hacks. if not, let add some space between them, like around 4px. it looks weird without any space and without combined borders

It also seems like they behave a bit differently to what is described in the designs, missing multi selects options and showing the number of selected values. if we dont have a specific ticket for that please open a new one and add it to the epic

Showing number of selected values might require custom component as such in my opinion is better to be done on separate PR
Also regarding the multi select options it is listed on a different ticket #171220 (I'll work on this on a different PR later)
We can probably just add the requirement for the custom component on the ticket above as well

yes a different pr for sure. just as long as we know we still missing those. for this pr just update the css

JordanSh · 2023-12-19T13:51:19Z

x-pack/plugins/cloud_security_posture/public/pages/rules/rules_table_header.tsx

+        <EuiFlexGroup gutterSize="none">
+          <EuiFlexItem
+            css={css`
+              max-width: 200px;


lets make sure the sizes are equal

JordanSh · 2023-12-19T14:01:30Z

x-pack/plugins/cloud_security_posture/server/routes/benchmark_rules/find/v2.ts

+  options: FindCspBenchmarkRuleRequest
+): Promise<FindCspBenchmarkRuleResponse> => {
+  if (!options.benchmarkId) {
+    throw new Error('Please provide BenchmarkId');


Suggested change

throw new Error('Please provide BenchmarkId');

throw new Error('Please provide benchmarkId');

JordanSh · 2023-12-19T14:02:02Z

x-pack/plugins/cloud_security_posture/server/routes/benchmark_rules/find/v2.ts

+    throw new Error('Please provide BenchmarkId');
+  }
+
+  const benchmarkId = options.benchmarkId ? options.benchmarkId : '';


this will never happen since we throw an error if its missing

JordanSh · 2023-12-19T14:04:02Z

x-pack/test/api_integration/apis/cloud_security_posture/rules/v1.ts

@@ -0,0 +1,237 @@
+/*


all test files should be have a suffix test.ts

i thought thats only for Jest test ? this one is FTR no ?

MB, missed the location of the file

JordanSh

I approve the changes but i checked out the PR, installed an integration and didn't see any rules. can you double check this and let me know if its something that happens to you as well? try starting from a clean env

JordanSh · 2023-12-21T12:24:44Z

x-pack/plugins/cloud_security_posture/common/types/rules/v4.ts

+  ruleNumber?: string;
+}
+
+// export type PageUrlParams = Record<'benchmarkId' | 'benchmarkVersion', BenchmarksCisId | string>;


can this be removed?

woops, forgot to remove this

JordanSh · 2023-12-21T12:33:47Z

x-pack/plugins/cloud_security_posture/public/pages/benchmarks/benchmarks_table.tsx

+  benchmarkVersion,
+  benchmarkId,
+}: {
+  benchmarkName: BenchmarksCisId;


IDK if this is considered nit picky but we named the prop benchmarkName and type it as BenchmarkCisId, and later call getBenchmarkCisName passing benchmarkName as an argument.

its nothing crucial for the functionality of the code but i think it will be more readable if we change benchmarkName to benchmarkCisId.

the type would make more sense and calling getBenchmarkCisName(benchmarkCisId) is clearer that it generates a name out of the cis id.

actually I think its better to just call it benchmarkId instead of benchmarkName or benchmarkCisId, considering the field itself is called id on the object

JordanSh · 2023-12-21T13:00:30Z

x-pack/plugins/cloud_security_posture/public/pages/benchmarks/benchmarks_table.tsx

+        benchmarkName={benchmarkId || ''}
+        benchmarkId={benchmarkId || ''}


if those are the same we dont need both

…into rule-page-phase1-clean

opauloh · 2023-12-22T23:34:53Z

x-pack/plugins/cloud_security_posture/public/pages/benchmarks/benchmarks_table.tsx

+    render: (benchmarkId: Benchmark['id'], benchmark: Benchmark) => (
+      <BenchmarkButtonLink
+        benchmarkId={benchmarkId || ''}
+        benchmarkVersion={benchmark.version || ''}
+      />
+    ),


could this render work with benchmark only?

render: (benchmark: Benchmark) => ( <BenchmarkButtonLink benchmarkId={benchmark.id || ''} benchmarkVersion={benchmark.version || ''} /> )

JordanSh · 2023-12-24T13:55:44Z

x-pack/plugins/cloud_security_posture/public/pages/rules/index.tsx

-                })}
-              />
+              <EuiFlexGroup gutterSize="s">
+                <EuiFlexItem grow={false} style={{ marginBottom: `6px` }}>


really minor but in JSS you dont have to use pixels as they are the default value if you just pass numbers.

meaning this will work the same:

Suggested change

<EuiFlexItem grow={false} style={{ marginBottom: `6px` }}>

<EuiFlexItem grow={false} style={{ marginBottom: 6 }}>

kibana-ci · 2023-12-26T10:35:16Z

💚 Build Succeeded

Buildkite Build
Commit: 30d797b

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`cloudSecurityPosture`	434	432	-2

Async chunks

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

id	before	after	diff
`cloudSecurityPosture`	452.0KB	452.0KB	-48.0B

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.6KB	15.6KB	-39.0B

History

💚 Build #184833 succeeded 308e048
💛 Build #184648 was flaky a33b704
💔 Build #184422 failed 6877d64
💔 Build #184392 failed 92f1850
💚 Build #184148 succeeded a82383a
💔 Build #184128 failed 3f1047c

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

cc @animehart

maxcold · 2023-12-28T16:30:34Z

x-pack/plugins/cloud_security_posture/common/types/rules/v4.ts

+
+const DEFAULT_BENCHMARK_RULES_PER_PAGE = 25;
+
+export const findCspBenchmarkRuleRequestSchema = schema.object({


@animehart was there any breaking change to the interfaces so we needed to introduce a new version?

first commit for rules page and empty benchmark page

2073cda

This was linked to issues Dec 14, 2023

[Cloud Security] Benchmarks page empty state - Phase I #171120

Closed

[Cloud Security] Rules table changes - Phase I #171122

Closed

animehart added 2 commits December 13, 2023 23:50

added initial versioning

e3bffb6

Merge branch 'main' into rule-page-phase1-clean

134b596

CI Fixes, updates, some clean up

9ea963b

clean up

4f0da28

animehart marked this pull request as ready for review December 17, 2023 04:41

animehart requested a review from a team as a code owner December 17, 2023 04:41

animehart added 5 commits December 18, 2023 00:16

Merge branch 'main' into rule-page-phase1-clean

8773e03

small updates

26c20a8

Merge branch 'rule-page-phase1-clean' of github.com:animehart/kibana …

3f1047c

…into rule-page-phase1-clean

ci fix, removed dupe file

f5685ae

removed unused translation

a82383a

JordanSh added release_note:skip Skip the PR/issue when compiling release notes Team:Cloud Security Cloud Security team related labels Dec 19, 2023

JordanSh assigned animehart Dec 19, 2023

JordanSh self-requested a review December 19, 2023 11:56

JordanSh requested changes Dec 19, 2023

View reviewed changes

animehart added 4 commits December 19, 2023 12:01

pr comments

92f1850

failure fix

6877d64

change to use camelcase

2777599

Merge branch 'main' into rule-page-phase1-clean

a33b704

animehart requested a review from JordanSh December 20, 2023 17:43

JordanSh requested changes Dec 21, 2023

View reviewed changes

animehart added 2 commits December 21, 2023 08:43

pr comments and small fix

c1083c8

Merge branch 'rule-page-phase1-clean' of github.com:animehart/kibana …

308e048

…into rule-page-phase1-clean

animehart requested a review from JordanSh December 21, 2023 17:06

opauloh reviewed Dec 22, 2023

View reviewed changes

JordanSh approved these changes Dec 24, 2023

View reviewed changes

animehart added 2 commits December 26, 2023 01:31

comments

37c6f68

Merge branch 'main' into rule-page-phase1-clean

30d797b

animehart merged commit aa2a0f3 into elastic:main Dec 26, 2023
19 checks passed

kibanamachine added v8.13.0 backport:skip This commit does not require backporting labels Dec 26, 2023

maxcold reviewed Dec 28, 2023

View reviewed changes

maxcold mentioned this pull request Dec 28, 2023

[Cloud Security] update score trendline to support muting rules #173987

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Cloud Security Posture][Benchmark Improvements] Rules Page Improvements phase 1 + Empty Benchmark #173345

[Cloud Security Posture][Benchmark Improvements] Rules Page Improvements phase 1 + Empty Benchmark #173345

animehart commented Dec 14, 2023 •

edited

Loading

animehart commented Dec 14, 2023

animehart commented Dec 14, 2023

elasticmachine commented Dec 19, 2023

JordanSh Dec 19, 2023

JordanSh Dec 19, 2023

JordanSh Dec 19, 2023

JordanSh Dec 19, 2023

JordanSh Dec 19, 2023

animehart Dec 19, 2023

JordanSh Dec 20, 2023

animehart Dec 20, 2023

JordanSh Dec 19, 2023

animehart Dec 19, 2023 •

edited

Loading

JordanSh Dec 20, 2023

JordanSh Dec 19, 2023

JordanSh Dec 19, 2023

JordanSh Dec 19, 2023

JordanSh Dec 19, 2023

animehart Dec 19, 2023

JordanSh Dec 20, 2023 •

edited

Loading

JordanSh left a comment •

edited

Loading

JordanSh Dec 21, 2023

animehart Dec 21, 2023

JordanSh Dec 21, 2023 •

edited

Loading

animehart Dec 21, 2023

JordanSh Dec 21, 2023

opauloh Dec 22, 2023

animehart Dec 26, 2023

JordanSh Dec 24, 2023

kibana-ci commented Dec 26, 2023

maxcold Dec 28, 2023

	<NoFindingsStates posturetype={'all'} />
	<NoFindingsStates postureType={'all'} />

	throw new Error('Please provide BenchmarkId');
	throw new Error('Please provide benchmarkId');

		benchmarkName={benchmarkId \|\| ''}
		benchmarkId={benchmarkId \|\| ''}

	<EuiFlexItem grow={false} style={{ marginBottom: `6px` }}>
	<EuiFlexItem grow={false} style={{ marginBottom: 6 }}>


		const DEFAULT_BENCHMARK_RULES_PER_PAGE = 25;

		export const findCspBenchmarkRuleRequestSchema = schema.object({

[Cloud Security Posture][Benchmark Improvements] Rules Page Improvements phase 1 + Empty Benchmark #173345

[Cloud Security Posture][Benchmark Improvements] Rules Page Improvements phase 1 + Empty Benchmark #173345

Conversation

animehart commented Dec 14, 2023 • edited Loading

Summary

animehart commented Dec 14, 2023

animehart commented Dec 14, 2023

elasticmachine commented Dec 19, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

animehart Dec 19, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JordanSh Dec 20, 2023 • edited Loading

Choose a reason for hiding this comment

JordanSh left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JordanSh Dec 21, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kibana-ci commented Dec 26, 2023

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Page load bundle

History

Choose a reason for hiding this comment

animehart commented Dec 14, 2023 •

edited

Loading

animehart Dec 19, 2023 •

edited

Loading

JordanSh Dec 20, 2023 •

edited

Loading

JordanSh left a comment •

edited

Loading

JordanSh Dec 21, 2023 •

edited

Loading