EMT-339: policy response schema, views and tests #65962
Conversation
nnamdifrankie
added
v8.0.0
release_note:skip
|
@elasticmachine merge upstream |
paul-tavares
left a comment
paul-tavares
left a comment
There was a problem hiding this comment.
Some initial feedback
| */ | ||
| export interface HostPolicyResponseActionDetails { | ||
| export interface HostPolicyResponseAppliedAction { | ||
| name: string; |
There was a problem hiding this comment.
It would be nice if we could have a permissive list of possible actions that we could use here and everywhere else where we do string[]. Some like this:
type HostPolicyActionName =
| 'download_model'
| 'ingest_events_config'
| 'workflow'
| 'configure_elasticsearch_connection'
| 'configure_kernel'
| 'configure_logging'
| 'configure_malware'
| 'connect_kernel'
| 'detect_file_open_events'
| 'detect_file_write_events'
| 'detect_image_load_events'
| 'detect_process_events'
| 'download_global_artifacts'
| 'load_config'
| 'load_malware_model'
| 'read_elasticsearch_config'
| 'read_events_config'
| 'read_kernel_config'
| 'read_logging_config'
| 'read_malware_config'
| string;Then on this line (and other areas below):
| name: string; | |
| name: HostPolicyActionName; |
Tried it quickly on my IDE and seems to work ok even with custom unknown strings:
| interface HostPolicyResponseConfigurationStatus { | ||
| status: HostPolicyResponseActionStatus; | ||
| concerned_actions: Array<keyof HostPolicyResponseActions>; | ||
| concerned_actions: string[]; |
There was a problem hiding this comment.
If you implement the suggestion above, this would then be:
| concerned_actions: string[]; | |
| concerned_actions: HostPolicyActionName[] |
| id: '17d4b81d-9940-4b64-9de5-3e03ef1fb5cf', | ||
| actions: { | ||
| download_model: { | ||
| actions: [ |
There was a problem hiding this comment.
you probably added this only because @parkiino has not yet merged her change that integrates with API, correct (and to suppress ts errors)?
There was a problem hiding this comment.
Yes just to get the test to pass.
| export const policyResponseActions: ( | ||
| state: Immutable<HostState> | ||
| ) => undefined | Partial<HostPolicyResponseActions> = createSelector( | ||
| ) => undefined | ImmutableArray<HostPolicyResponseAppliedAction> = createSelector( |
There was a problem hiding this comment.
Just use Immutable<> instead of ImmutableArray<>. That type will take care of using the appropriate Immutable type in its implementation. I saw Rob provide this feedback recently when I saw him providing the same feedback - the reason being: if Typescript introduces a builtin Immutable Generic, then we will be in a better place to remove our implementation.
| }: { | ||
| actions: Immutable<Array<keyof HostPolicyResponseActions>>; | ||
| actionStatus: Partial<HostPolicyResponseActions>; | ||
| actions: ImmutableArray<string>; |
There was a problem hiding this comment.
Use Immutable<> (same below in several places). Also, would this change based on my prior suggestion to type up known action names?
…e/kibana into EMT-339_update_schema_views
|
@elasticmachine merge upstream |
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
|
Closed for #66264 |
4 participants
Summary
Checklist