[Cloud Security] Misconfiguration preview & Refactor CSP Plugin to include new package PHASE 3 by animehart · Pull Request #191317 · elastic/kibana

animehart · 2024-08-26T21:08:15Z

The previous #190105 was way too big and made it hard to review without missing any bugs or potential bugs, Thus we decided we are going to make series of smaller PR to make things more manageable

We will be splitting it into 4 PR
Phase 1: Creating empty packages for csp and csp-common
Phase 2: Move Types from CSP plugin to the Package + Deleting duplicates in the CSP plugin where possible
Phase 3: Move Functions, Utils or Helpers, Hooks to Package
Phase 4: Misconfiguration Preview feature (with Cypress test and other required test)

This is Phase 3 of the Process,
This also includes moving rule versions type

This PR is the continuation of this PR #190933

NOTE:
Merge phase 2 first before this

…port statement

…fix'

…m:animehart/kibana into misconfiguration-preview-refactor-phase-2

…orts to be more specific

… csp package

…fix'

…be directly from csp common package

…m:animehart/kibana into misconfiguration-preview-refactor-phase-2

…fix'

…nt for types or interface to be directly from csp package now

…m:animehart/kibana into misconfiguration-preview-refactor-phase-2

…ad of just import

… latest rule versions import on CSP plugin etc

animehart · 2024-08-26T21:08:24Z

/ci

…rsion to be directly from common package

…ules are from rules/latest in common package instead of rules.ts, furthermore deleted unused file that was created during Phase 2

animehart · 2024-08-26T22:33:48Z

/ci

animehart · 2024-08-26T23:39:52Z

/ci

elasticmachine · 2024-08-27T07:30:40Z

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

…checked on Phase 4 instead

opauloh · 2024-08-27T23:19:58Z

+export const extractErrorMessage = (e: unknown, defaultMessage = 'Unknown Error'): string => {
+  if (e instanceof Error) return e.message;
+  if (typeof e === 'string') return e;
+
+  return defaultMessage; // TODO: i18n
+};


Let's fix this todo:

const defaultErrorMessage = i18n.translate('xpack.csp.common.utils.helpers.unknownError', { defaultMessage: 'Unknown Error', }); export const extractErrorMessage = (e: unknown, fallbackMessage ?: string): string => { if (e instanceof Error) return e.message; if (typeof e === 'string') return e; return fallbackMessage ?? defaultErrorMessage; };

opauloh

looks good overall, I only have some questions and nitpicks

opauloh · 2024-08-27T23:30:26Z


 export const useGetCspBenchmarkRulesStatesApi = () => {
-  const { http } = useKibana().services;
+  const { http } = useKibana<CoreStart & CspClientPluginStartDeps>().services;


http is part of CoreStart so that's only what we need here:

const { http } = useKibana<CoreStart>().services;

opauloh · 2024-08-27T23:31:42Z

  options?: UseQueryOptions<CspSetupStatus, unknown, CspSetupStatus>
 ) => {
-  const { http } = useKibana().services;
+  const { http } = useKibana<CoreStart & CspClientPluginStartDeps>().services;


Suggested change

const { http } = useKibana<CoreStart & CspClientPluginStartDeps>().services;

const { http } = useKibana<CoreStart>().services;

opauloh · 2024-08-27T23:36:33Z

  UpdatePackagePolicy,
 } from '@kbn/fleet-plugin/common';
+import type { BenchmarkRuleSelectParams } from '@kbn/cloud-security-posture-common/schema/rules/latest';
+import type { BenchmarkRuleSelectParams as BenchmarkRuleSelectParamsV1 } from '@kbn/cloud-security-posture-common/schema/rules/v4';


shouldn't be BenchmarkRuleSelectParamsV4?

opauloh · 2024-08-27T23:36:56Z

+// export const extractErrorMessage = (e: unknown, defaultMessage = 'Unknown Error'): string => {
+//   if (e instanceof Error) return e.message;
+//   if (typeof e === 'string') return e;

-  return defaultMessage; // TODO: i18n
-};
+//   return defaultMessage; // TODO: i18n
+// };


missed commented code

opauloh · 2024-08-27T23:42:15Z

 import { i18n } from '@kbn/i18n';
-import { extractErrorMessage } from '../../../common/utils/helpers';
+import { extractErrorMessage } from './helpers';



shouldn't this file be in kbn-cloud-security-posture instead? A toast notification It's mainly a frontend function, not meant to be shared with the server.

…isId to be directly from csp package

…m:animehart/kibana into misconfiguration-preview-refactor-phase-3

maxcold

lgtm, one small comment on the duplicated types file

maxcold · 2024-08-28T10:22:20Z

+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+export type BenchmarksCisId = 'cis_k8s' | 'cis_azure' | 'cis_aws' | 'cis_eks' | 'cis_gcp';


let's either put it under ./types.ts or another way around split types.ts into meaningful files, eg. types/status.ts and types/finding.ts

maxcold · 2024-08-28T10:29:59Z

/ci

animehart · 2024-08-28T13:57:53Z

/ci

animehart · 2024-08-28T14:20:23Z

/ci

animehart · 2024-08-28T15:12:55Z

/ci

animehart · 2024-08-28T16:49:08Z

/ci

kibana-ci · 2024-08-28T17:41:11Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 2395a7d
Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-191317-2395a7dde5dc
Security Deployment

Failed CI Steps

FTR Configs #35

Test Failures

[job] [logs] FTR Configs #35 / Cloud Security Posture Test adding Cloud Security Posture Integrations CSPM AWS CIS_AWS Organization Manual Direct Access CIS_AWS Organization Manual Direct Access Workflow

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`cloudSecurityPosture`	706	709	+3

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`@kbn/cloud-security-posture`	19	22	+3
`@kbn/cloud-security-posture-common`	45	49	+4
total			+7

Async chunks

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

id	before	after	diff
`cloudSecurityPosture`	481.1KB	480.7KB	-385.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`	17.0KB	17.6KB	+545.0B

Unknown metric groups

API count

id	before	after	diff
`@kbn/cloud-security-posture`	19	22	+3
`@kbn/cloud-security-posture-common`	45	49	+4
total			+7

History

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

animehart and others added 24 commits August 20, 2024 15:31

preliminary adding types to csp and csp-common packages

f97c940

moving types from csp plugin to csp packages + updating import and ex…

5989868

…port statement

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

6283bd4

…fix'

attempt on fixing bundle size leak

7fafa52

Merge branch 'misconfiguration-preview-refactor-phase-2' of github.co…

dad76fd

…m:animehart/kibana into misconfiguration-preview-refactor-phase-2

change schema import to be from schema folder + updating index.ts exp…

72f00b0

…orts to be more specific

forgot to add index.ts in last commit

783626e

removed re export on csp plugin + updated imports to be directly from…

a89617d

… csp package

fix ci failure: forgot to edit some file

6600a34

fix check types failure

19d7451

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

51b44b9

…fix'

fix failed FTR due to forgot updating imports on that file

e2b8f48

fix conflict merge

4496119

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

9f2814e

…fix'

remove re exports and update imports of csp_finding related types to …

113376a

…be directly from csp common package

Merge branch 'misconfiguration-preview-refactor-phase-2' of github.co…

c1a3b6a

…m:animehart/kibana into misconfiguration-preview-refactor-phase-2

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

3398ecf

…fix'

fix check types failure + removed re-export and update import stateme…

8c69b3e

…nt for types or interface to be directly from csp package now

Merge branch 'misconfiguration-preview-refactor-phase-2' of github.co…

0c1f750

…m:animehart/kibana into misconfiguration-preview-refactor-phase-2

updating CspStatusCode imports to be directly from csp common package

447c47b

Merge branch 'main' into misconfiguration-preview-refactor-phase-2

feae686

PR Comments: Updated type imports to explicitly use import type inste…

ceafb0b

…ad of just import

fix conflict

6b8adb6

added all Version of Rules into common csp package + updated usage of…

02bbdb8

… latest rule versions import on CSP plugin etc

animehart added 2 commits August 26, 2024 15:06

removed rules versions from csp plugin + updated imports for rule ve…

7cb59d3

…rsion to be directly from common package

Reverting and Updating some changes from Phase 2 so now imports for R…

c349bac

…ules are from rules/latest in common package instead of rules.ts, furthermore deleted unused file that was created during Phase 2

fix lint and check types ci failures

dff574d

animehart requested review from a team as code owners August 27, 2024 07:30

animehart and others added 5 commits August 27, 2024 01:13

FIX MERGE CONFLICTS

57dee10

fix check types failed, removed useless file

8925785

Merge branch 'main' into misconfiguration-preview-refactor-phase-3

6343043

removed a useMisConfigurationPreview hook to make sure that hooks is …

3078818

…checked on Phase 4 instead

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

8bdeb63

opauloh reviewed Aug 27, 2024

View reviewed changes

Comment thread x-pack/packages/kbn-cloud-security-posture-common/utils/helpers.ts Outdated

opauloh reviewed Aug 27, 2024

View reviewed changes

animehart added 2 commits August 27, 2024 23:43

PR Comments: Addressing PR Comments + Updated Imports for BenchmarksC…

050e23d

…isId to be directly from csp package

Merge branch 'misconfiguration-preview-refactor-phase-3' of github.co…

54bb14c

…m:animehart/kibana into misconfiguration-preview-refactor-phase-3

animehart requested a review from opauloh August 28, 2024 06:46

kibanamachine and others added 3 commits August 28, 2024 06:54

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

c4127c5

missed one PR comment for previous commit, adding it now

f5f3072

Merge branch 'misconfiguration-preview-refactor-phase-3' of github.co…

e5ae31a

…m:animehart/kibana into misconfiguration-preview-refactor-phase-3

maxcold approved these changes Aug 28, 2024

View reviewed changes

maxcold added the ci:project-deploy-security Create a Security Serverless Project label Aug 28, 2024

PR Comments, removed types file and re added types into types folder

b01e87c

Merge branch 'main' into misconfiguration-preview-refactor-phase-3

39ae84b

Merge branch 'main' into misconfiguration-preview-refactor-phase-3

2395a7d

animehart merged commit a78c69b into elastic:main Aug 28, 2024

kibanamachine added the backport:skip This PR does not require backporting label Aug 28, 2024

	const { http } = useKibana<CoreStart & CspClientPluginStartDeps>().services;
	const { http } = useKibana<CoreStart>().services;

Conversation

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

Uh oh!

Uh oh!

animehart commented Aug 26, 2024

Uh oh!

animehart commented Aug 26, 2024

Uh oh!

animehart commented Aug 26, 2024

Uh oh!

elasticmachine commented Aug 27, 2024

Uh oh!

opauloh Aug 27, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

opauloh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

opauloh Aug 27, 2024

Choose a reason for hiding this comment

Uh oh!

opauloh Aug 27, 2024

Choose a reason for hiding this comment

Uh oh!

opauloh Aug 27, 2024

Choose a reason for hiding this comment

Uh oh!

opauloh Aug 27, 2024

Choose a reason for hiding this comment

Uh oh!

opauloh Aug 27, 2024

Choose a reason for hiding this comment

Uh oh!

maxcold left a comment

Choose a reason for hiding this comment

Uh oh!

maxcold Aug 28, 2024

Choose a reason for hiding this comment

Uh oh!

maxcold commented Aug 28, 2024

Uh oh!

animehart commented Aug 28, 2024

Uh oh!

animehart commented Aug 28, 2024

Uh oh!

animehart commented Aug 28, 2024

Uh oh!

animehart commented Aug 28, 2024

Uh oh!

kibana-ci commented Aug 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

Page load bundle

API count

History

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

animehart commented Aug 26, 2024 •

edited

Loading

kibana-ci commented Aug 28, 2024 •

edited

Loading