[Cloud Security] Misconfiguration preview & Refactor CSP Plugin to include new package PHASE 2 #190933

animehart · 2024-08-20T22:32:45Z

Summary

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 2 of the Process

…port statement

animehart · 2024-08-21T00:08:23Z

/ci

…fix'

animehart · 2024-08-21T00:23:03Z

/ci

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

animehart · 2024-08-21T06:33:20Z

/ci

elasticmachine · 2024-08-21T08:36:53Z

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

…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

maxcold

The main question we should decide on is what to do with our versioned types and schemas.
Also pls check all imports and add import type when importing types

maxcold · 2024-08-26T11:47:55Z

x-pack/packages/kbn-cloud-security-posture-common/schema/csp_finding.ts

 import type { EcsDataStream, EcsEvent } from '@elastic/ecs';
-import { CspBenchmarkRuleMetadata } from '../types/latest';
+import { CspBenchmarkRuleMetadata } from './rules';

 export interface CspFinding {


This file doesn't contain schema anymore, only CspFinding interface , maybe make sense to move the code to types.ts?

maxcold · 2024-08-26T12:00:58Z

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

@@ -6,44 +6,17 @@
 */

 import { schema, TypeOf } from '@kbn/config-schema';
-import { CSPM_POLICY_TEMPLATE, KSPM_POLICY_TEMPLATE } from '../../constants';
+
+import { cspBenchmarkRuleMetadataSchema } from '@kbn/cloud-security-posture-common/schema';


I still think we need to move the whole common/types with all the versions into our common package. Otherwise the versioning won't work for us. Imagine you need to make a breaking change to the schema. If you do it in the cspBenchmarkRuleMetadataSchema in the package you would break the versioning as it is imported here in the v3. To do it properly you need to introduce the new version of the schema, the same way as we migrated cspBenchmarkRuleMetadataSchema between v2 and v3.
I'm not sure if we can reduce the number of changes caused by moving the whole versioned types and schemas, but without it we are kind of breaking the versioning

again to not make the change too large, maybe it's ok to proceed with this PR but then create a new PR right away moving the versioned types and schemas to our shared package so that we don't risk

maxcold · 2024-08-26T12:36:33Z

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

@@ -6,15 +6,11 @@
 */

 import { schema, TypeOf } from '@kbn/config-schema';
+import { CspBenchmarkRulesStates } from '@kbn/cloud-security-posture-common';
+import { ruleStateAttributes, rulesStates } from '@kbn/cloud-security-posture-common/schema';


same problem with versioning here. We lose information about the version when importing from our shared package without versioning. Again to make a breaking change you would need to version it somehow on the package level, that's why it's better to move the whole versioned types and schemas to our common package

maxcold · 2024-08-26T12:38:24Z

...ns/cloud_security_posture/public/pages/configurations/latest_findings/use_latest_findings.ts

 import {
  CDR_MISCONFIGURATIONS_INDEX_PATTERN,
  LATEST_FINDINGS_RETENTION_POLICY,
-} from '../../../../common/constants';
-import { MAX_FINDINGS_TO_LOAD } from '../../../common/constants';
+  CspBenchmarkRulesStates,


import type

maxcold · 2024-08-26T12:38:48Z

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

@@ -6,6 +6,7 @@
 */

 import { Truthy } from 'lodash';
+import { BaseCspSetupStatus } from '@kbn/cloud-security-posture-common';


import type

maxcold · 2024-08-26T12:38:57Z

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

@@ -5,7 +5,7 @@
 * 2.0.
 */
 import { QueryDslQueryContainer } from '@kbn/data-views-plugin/common/types';
-import { CspBenchmarkRulesStates } from '../types/latest';
+import { CspBenchmarkRulesStates } from '@kbn/cloud-security-posture-common';


import type

maxcold · 2024-08-26T12:40:02Z

...ugins/cloud_security_posture/public/pages/compliance_dashboard/compliance_dashboard.test.tsx

@@ -8,6 +8,7 @@
 import React from 'react';

 import { coreMock } from '@kbn/core/public/mocks';
+import { BaseCspSetupStatus, CspStatusCode } from '@kbn/cloud-security-posture-common';


import type

maxcold · 2024-08-26T12:40:10Z

...ugins/cloud_security_posture/public/pages/compliance_dashboard/compliance_dashboard.test.tsx

-  ComplianceDashboardDataV2,
-  CspStatusCode,
-} from '../../../common/types_old';
+import { ComplianceDashboardDataV2 } from '../../../common/types_old';


maxcold · 2024-08-26T12:40:25Z

x-pack/plugins/cloud_security_posture/public/pages/configurations/__mocks__/findings.ts

@@ -5,7 +5,7 @@
 * 2.0.
 */

-import { CspFinding } from '../../../../common/schemas/csp_finding';
+import { CspFinding } from '@kbn/cloud-security-posture-common';


import type

maxcold · 2024-08-26T12:40:41Z

...rity_posture/public/pages/configurations/findings_flyout/findings_detection_rule_counter.tsx

@@ -7,7 +7,7 @@

 import type { HttpSetup } from '@kbn/core/public';
 import React from 'react';
-import { CspFinding } from '../../../../common/schemas/csp_finding';
+import { CspFinding } from '@kbn/cloud-security-posture-common';


import type

…ad of just import

maxcold

Approving in case we decide to move the versioned types and schemas in a separate PR

kibana-ci · 2024-08-26T18:09:59Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 6b8adb6

Failed CI Steps

FTR Configs #73

Test Failures

[job] [logs] FTR Configs #73 / Cloud Security Posture Test adding Cloud Security Posture Integrations CNVM CNVM AWS Hyperlink on PostInstallation Modal should have the correct URL

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`cloudSecurityPosture`	703	705	+2

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`	0	19	+19
`@kbn/cloud-security-posture-common`	0	45	+45
total			+64

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	481.1KB	-53.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`	16.9KB	17.0KB	+93.0B

Unknown metric groups

API count

id	before	after	diff
`@kbn/cloud-security-posture`	0	19	+19
`@kbn/cloud-security-posture-common`	0	45	+45
total			+64

History

💚 Build #229818 succeeded ceafb0b
💚 Build #229658 succeeded feae686
💚 Build #229626 succeeded 447c47b
💛 Build #229622 was flaky 0c1f750
💔 Build #229620 failed 3398ecf
💔 Build #229614 failed 9f2814e

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

machadoum

Security Solution changes LGTM!

…clude new package PHASE 3 (#191317) 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 --------- Co-authored-by: kibanamachine <[email protected]>

animehart added 2 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'

animehart added 2 commits August 20, 2024 23:32

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

animehart marked this pull request as ready for review August 21, 2024 08:35

animehart requested a review from a team as a code owner August 21, 2024 08:35

animehart added the Team:Cloud Security Cloud Security team related label Aug 21, 2024

animehart added v8.16.0 release_note:skip Skip the PR/issue when compiling release notes labels Aug 21, 2024

animehart added 5 commits August 23, 2024 10:21

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

animehart requested a review from a team as a code owner August 23, 2024 22:56

kibanamachine and others added 10 commits August 23, 2024 23:07

[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

maxcold requested changes Aug 26, 2024

View reviewed changes

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

ceafb0b

…ad of just import

maxcold approved these changes Aug 26, 2024

View reviewed changes

fix conflict

6b8adb6

animehart mentioned this pull request Aug 26, 2024

[Cloud Security]Misconfiguration preview & Refactor CSP Plugin to include new package #190105

Closed

animehart mentioned this pull request Aug 27, 2024

[Cloud Security] Misconfiguration preview & Refactor CSP Plugin to include new package PHASE 3 #191317

Merged

machadoum approved these changes Aug 27, 2024

View reviewed changes

animehart merged commit 6d3721a into elastic:main Aug 27, 2024
40 checks passed

kibanamachine added the backport:skip This commit does not require backporting label Aug 27, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Cloud Security] Misconfiguration preview & Refactor CSP Plugin to include new package PHASE 2 #190933

[Cloud Security] Misconfiguration preview & Refactor CSP Plugin to include new package PHASE 2 #190933

animehart commented Aug 20, 2024 •

edited

Loading

animehart commented Aug 21, 2024

animehart commented Aug 21, 2024

animehart commented Aug 21, 2024

elasticmachine commented Aug 21, 2024

maxcold left a comment

maxcold Aug 26, 2024

maxcold Aug 26, 2024

maxcold Aug 26, 2024

maxcold Aug 26, 2024

maxcold Aug 26, 2024

maxcold Aug 26, 2024

maxcold Aug 26, 2024

maxcold Aug 26, 2024

maxcold Aug 26, 2024

maxcold Aug 26, 2024

maxcold Aug 26, 2024

maxcold left a comment

kibana-ci commented Aug 26, 2024

API count

machadoum left a comment

[Cloud Security] Misconfiguration preview & Refactor CSP Plugin to include new package PHASE 2 #190933

[Cloud Security] Misconfiguration preview & Refactor CSP Plugin to include new package PHASE 2 #190933

Conversation

animehart commented Aug 20, 2024 • edited Loading

Summary

animehart commented Aug 21, 2024

animehart commented Aug 21, 2024

animehart commented Aug 21, 2024

elasticmachine commented Aug 21, 2024

maxcold left a 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

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

maxcold left a comment

Choose a reason for hiding this comment

kibana-ci commented Aug 26, 2024

💛 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

machadoum left a comment

Choose a reason for hiding this comment

animehart commented Aug 20, 2024 •

edited

Loading