Move Functions from CSP to Shared packages #212663
Conversation
|
/ci |
|
/ci |
|
/ci |
|
/ci |
|
/ci |
|
/ci |
|
/ci |
|
/ci |
|
/ci |
|
/ci |
|
/ci |
|
/ci |
|
/ci |
|
/ci |
animehart
added
v9.0.0
v9.1.0
release_note:skip
|
Pinging @elastic/kibana-cloud-security-posture (Team:Cloud Security) |
There was a problem hiding this comment.
Should we group these under utils? A lot of the code in the shared package will be utils
I prefer if all the related code in the shared package is grouped by similar directories. All the types and functions related rules should be in a directory called rules
rules
- types.ts
- create_detection_rules.ts
- detection_rules.ts
There was a problem hiding this comment.
🤔 This needs discussion, but IMO
I think the types should stay co-located with the code unless they are reused by more than 1 module (file), then it may be better to locate them at a high level in the file structure.
Even if the type is exported out of the library, if only one module in the library is using it, it should be colocated.
If we have a type in the library for shared usage with other libraries and no share package module uses it, it could be in this type of types file.
There was a problem hiding this comment.
so in this case, we should put RuleCreateProps type in create_detection_rule.ts file as its only being used there ? or should we can just do your first suggestion where we put all those thing into 1 sub folder (rules in this case) ?
There was a problem hiding this comment.
so in this case, we should put RuleCreateProps type in create_detection_rule.ts file as its only being used there ?
Yes
| import { Truthy } from 'lodash'; | ||
|
|
||
| export const truthy = <T>(value: T): value is Truthy<T> => !!value; |
There was a problem hiding this comment.
I noticed that we were importing only the Truthy type from Lodash without actually using any Lodash functions. I think this is a bit of an odd pattern because Lodash is primarily a utility library, so importing only the type instead of simply using the truthy function directly from lodash doesn't seem to make much sense.
If the case is that we had to customize the lodash's truthy function because it doesn't work for us out of the box, I think it's better to create our own Truthy type as well (based on lodash):
| import { Truthy } from 'lodash'; | |
| export const truthy = <T>(value: T): value is Truthy<T> => !!value; | |
| type Truthy<T> = T extends null | undefined | false | "" | 0 | 0n ? never : T; | |
| export const truthy = <T>(value: T): value is Truthy<T> => !!value; |
|
|
||
| export const CLOUD_SECURITY_POSTURE_PACKAGE_NAME = 'cloud_security_posture'; | ||
|
|
||
| export const FINDINGS_INDEX_NAME = 'logs-cloud_security_posture.findings'; | ||
| export const FINDINGS_INDEX_PATTERN = 'logs-cloud_security_posture.findings-default*'; | ||
| // export const FINDINGS_INDEX_PATTERN = 'logs-cloud_security_posture.findings-default*'; |
There was a problem hiding this comment.
is there any reason to keep the comments?
There was a problem hiding this comment.
nope
I missed this
Thanks !
| @@ -168,7 +170,7 @@ const getFlyoutDescriptionList = (finding: CspFinding): EuiDescriptionListProps[ | |||
| ].filter(truthy); | |||
|
|
|||
| const FindingsTab = ({ tab, finding }: { finding: CspFinding; tab: FindingsTab }) => { | |||
| const { application } = useKibana().services; | |||
| const { application } = useKibana<CoreStart & CspClientPluginStartDeps>().services; | |||
There was a problem hiding this comment.
We have been using this combined type in a quite few places now, I think it's time to add a dedicate type?
i.e
export type CspUseKibanaServices = CoreStart & CspClientPluginStartDepsThere was a problem hiding this comment.
hmmm I remembered we did that but we revert to combined type maybe I remembered it wrongly ?
There's a reason why we did this in the first place
💚 Build Succeeded
Metrics [docs]Module Count
Public APIs missing comments
Async chunks
Public APIs missing exports
History
|
Summary
As a part of Expandable Findings flyout, we will need to move some Constants, Types, Functions, Components into Security Solution plugin or Shared package
This PR is phase 2 for Findings (Misconfiguration flyout) which include moving functions into shared package or security solution plugin