Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Security Solution] Decouple TGrid layers and dependencies and move the components to security_solution plugin #143152

Closed
YulNaumenko opened this issue Oct 12, 2022 · 2 comments · Fixed by #140151
Assignees
Labels
Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Threat Hunting:Investigations Security Solution Investigations Team technical debt Improvement of the software architecture and operational architecture

Comments

@YulNaumenko
Copy link
Contributor

This issue includes the next scope:

  1. Move useTimelineEvents logic and merge the data fetch code with x-pack/plugins/security_solution/public/timelines/containers/index.tsx by the adding/moving on top with the cache mechanism similar to the timelines.
    Existing:

Screen Shot 2022-08-24 at 2 27 41 PM

Screen Shot 2022-08-24 at 2 27 21 PM

  1. Move Right toolbar to the security_solution plugin

Screen Shot 2022-08-15 at 4 31 32 PM

This component completely belongs to the `StatefulEventsViewerComponent` context and should live there.
  1. The change from step 2 will require to adjust the layers of the data representation, which means EventRenderedView should belongs to the security_solution and exposed as a reusable component to the StatefulEventsViewerComponent.
  2. GraphOverlay (SessionView and Analyzer) should be moved to own folder and used by StatefulEventsViewerComponent as a sharable component.
@YulNaumenko YulNaumenko added technical debt Improvement of the software architecture and operational architecture Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Threat Hunting:Investigations Security Solution Investigations Team labels Oct 12, 2022
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-solution (Team: SecuritySolution)

@andrew-goldstein
Copy link
Contributor

New and recent (October, 2022) guidance from the Kibana Core team suggests splitting functionality exposed by Kibana plugins into separate Presentational and Container Components, (a once-popular / recommended pattern that is less so in a post-Hooks world) where:

  • The Kibana plugin exports (only) Container components (and stateful services), which are connected to a state store, (Redux in the case of Timeline)
  • The plugin uses stateless Presentational components, organized as Kibana packages, where each Presentational component is split into three packages:

In Shared UX, we create three packages for any component or subject matter (for example):

@kbn/shared-ux-[component]
@kbn/shared-ux-[component]-types**
@kbn/shared-ux-[component]-mocks

We also created a standard folder structure:
shared_ux/[family]/[component]

/impl - component and services
/types - simple index.d.ts
/mocks - Storybook and Jest

Strictly separating Presentational and Container components may lead to prop drilling, and (per the blog linked above), it's a pattern that has lost mindshare with the advent of Hooks. That said, many Timeline components were developed during the pre-Hooks (Component) era of React, and later migrated to Hooks. As a result, these components are already separated into Presentational and Container components. This (happens to) align with the latest (October 2022) guidance from the Kibana Core team.

In the existing code, Container components following the naming convention Stateful<Thing>. For example, StatefulBody gathers state from Redux via useSelector, and passes that state down to presentational components, that are not connected to Redux.

This comment:

  • Notes the recently updated guidance from the Core team
  • Describes a potential alignment between the latest guidance and the existing code
  • Is intended to facilitate (at a later date) a team discussion ahead of moving code from the security_solution plugin to the timelines plugin, (and to other Kibana packages)

YulNaumenko added a commit that referenced this issue Dec 6, 2022
…security_solution (#140151)

Resolves [#143152](#143152)
### Observability changes
This changes is a result of removal some types from `timelines` plugin:
- cleaned up timelines plugin related types, 
- replaced `Pick<ActionProps,'data' | 'eventId' | 'ecsData' |
'setEventsDeleted' >` with the props which were actually used:
  
    ```
  data: TimelineNonEcsData[];
    ecsData: Ecs;
    eventId: string;
  ```
In this PR we still have references to `@kbn/timelines-plugin`, which
needs to be changed later.
Threat Hunting team are going to think about replacing
`TimelineNonEcsData` with the other type definition (maybe
`NonEcsData`?) and moving `Ecs` type to the non `timelines` related
plugin/package.

### Security Solution changes
Before the current PR changes the components dependencies around `TGrid`
looked like the image below:
<img width="848" alt="Screen Shot 2022-11-29 at 6 16 14 AM"
src="https://user-images.githubusercontent.com/55110838/204663019-664431fb-f360-4a11-b395-6fa54c35dd6d.png">
After decomposition the `timelines` plugin hosted TGrid HOC and moving
all the data tables related sub-components to `security_solution` plugin
the new components architecture got the next shape:
<img width="842" alt="Screen Shot 2022-11-29 at 6 14 41 AM"
src="https://user-images.githubusercontent.com/55110838/204663068-40897f18-1485-4b59-a71b-ce09e660f7db.png">
`security_solution` plugin changes includes the next things:
- Moved some data table and actions types to
`x-pack/plugins/security_solution/common/types`, which is widely used
across the related components.
- Due to the movement of the data table with the store to from
`timeline` plugin to `security_solution` many test files which had the
reference to `tGridReducer` now cleaned up from the unnecessary logic:
```
- import { tGridReducer } from '@kbn/timelines-plugin/public';
```

and `TableState` references was replaced with the next changes:
```
- import type { TableState } from '@kbn/timelines-plugin/public';
+ import type { TableState } from '../common/store/data_table/types';
```
- Replaced `tGridActions` with `dataTableActions` name.
- Moved `control_columns` to `security_solution` common plugin
components: `RowCheckBox`, `HeaderCheckBox` and
`transformControlColumns`:
`RowActionComponent` moved from `timelines` plugin to
`x-pack/plugins/security_solution/public/common/components/control_columns/row_action`
without changes.
`transformControlColumns` moved from timelines plugin to
`x-pack/plugins/security_solution/public/common/components/control_columns/transform_control_columns.tsx`.
Removed not used property `hasAlertsCrudPermissions`, added unit test.
<img width="1222" alt="Screen Shot 2022-11-29 at 8 59 42 PM"
src="https://user-images.githubusercontent.com/55110838/204711499-9f90fee2-3c2f-4ff6-af28-c324ab1840d8.png">

- Many translation changes as a result of the owner plugin change:
 ```
 - i18n.translate('xpack.timelines....', {
 + i18n.translate('xpack.securitySolution....', {
```
- Moved `useDraggableKeyboardWrapper` to security_solution, added reference to `useAddToTimeline`, by using timelines plugin with kibana services. Added unit tests. 
<img width="1112" alt="Screen Shot 2022-11-30 at 9 06 42 AM" src="https://user-images.githubusercontent.com/55110838/204862298-bcd50a52-dbf7-480b-bf13-8e48d6835746.png">

- Replaced the next references:
```
- type: 'x-pack/timelines/t-grid/UPDATE_COLUMN_WIDTH',
+ type: 'x-pack/security_solution/data-table/UPDATE_COLUMN_WIDTH',
```

```
- type: 'x-pack/timelines/t-grid/REMOVE_COLUMN',
+ type: 'x-pack/security_solution/data-table/REMOVE_COLUMN',
```
- moved TGrid store previously hosted in timeline plugin  to `security_solution` as `data_table` store:
<img width="1109" alt="Screen Shot 2022-11-29 at 9 24 08 PM" src="https://user-images.githubusercontent.com/55110838/204714668-257a9c50-d722-4a6d-9214-f3ef8a14d0d2.png">

- Migrated TGrid `BodyComponent` to `DataTableComponent`
`x-pack/plugins/security_solution/public/common/components/data_table/index.tsx`
Removed some unused properties: `hasAlertsCrudPermissions, appId, getRowRenderer, isEventViewer, tableView, totalSelectAllAlerts, trailingControlColumns`. Current DataTableComponent is a subset of the previous BodyComponent, which includes only table related functionality:
<img width="1028" alt="Screen Shot 2022-11-30 at 10 44 35 AM" src="https://user-images.githubusercontent.com/55110838/204882561-0950b9ce-5a9f-4bdb-b38f-6ff742fc3f92.png">

- Renamed `TimelineExpandedDetail` to `ExpandedDetail` to make the type more generic for usage.
- BulkActions related changes includes:
<img width="1288" alt="Screen Shot 2022-11-29 at 9 13 32 PM" src="https://user-images.githubusercontent.com/55110838/204713196-409f3d5e-f752-4fe9-9ae9-e752514cbf99.png">

`AlertBulkActionsComponent` moved from timelines plugin to `x-pack/plugins/security_solution/public/common/components/toolbar/bulk_actions/alert_bulk_actions.tsx`, just renaming changes.

Added `x-pack/plugins/security_solution/public/common/components/toolbar/bulk_actions/types.ts` to consolidate types
`useBulkActionItems` moved from timelines plugin to `x-pack/plugins/security_solution/public/common/components/toolbar/bulk_actions/use_bulk_action_items.tsx`. Changed links, renamed `AlertsStatus` to `AlertWorkflowStatus`, removed `in-progress` case handling.

`useUpdateAlertsStatus` moved from timelines plugin to `x-pack/plugins/security_solution/public/common/components/toolbar/bulk_actions/use_update_alerts.ts`. Cleaned up the code from handling Observability API.
- Updated `x-pack/plugins/security_solution/public/common/lib/kuery/index.ts` with the actual implementations of 
```
  convertKueryToDslFilter,
  convertKueryToElasticSearchQuery,
  convertToBuildEsQuery,
  escapeKuery,
  escapeQueryValue,
  combineQueries,
```
instead of referencing timelines plugin.
- Moved `EventRenderedView` component to security_solution common components. Later planning to make it as a package.
- `EventsViewer` component became the stateful component which is responsible for the data representation managing. Some part from TGridIntegratedComponent and BodyComponent was merged under its logic:
<img width="1052" alt="Screen Shot 2022-11-30 at 6 22 22 PM" src="https://user-images.githubusercontent.com/55110838/204950708-a8875acd-eb62-4df5-8ac4-613a0a571de6.png">
<img width="242" alt="Screen Shot 2022-11-30 at 6 24 06 PM" src="https://user-images.githubusercontent.com/55110838/204950819-bca194a4-4309-4cb4-a2ba-0176e9fe6c65.png">

- Moved header actions  to common components `x-pack/plugins/security_solution/public/common/components/header_actions`
- Renamed component `AlertCount` to `UnitCount`.
- Moved to `security_solution` configuration for `APM_USER_INTERACTIONS`
- changes `createStore` interface by using the direct reference to `dataTableReducer` instead of passing down it's value through the params.
### Timeline plugin changes
- cleaned up timeline plugin interface by removing:
```
getTGrid: <T extends TGridType = 'embedded'>(
    props: GetTGridProps<T>
  ) => ReactElement<GetTGridProps<T>>;
  // eslint-disable-next-line @typescript-eslint/no-explicit-any
getTGridReducer: () => any;
getUseDraggableKeyboardWrapper: () => (
    props: UseDraggableKeyboardWrapperProps
  ) => UseDraggableKeyboardWrapper;
```
- renamed embedded store
```
- setTGridEmbeddedStore: (store: Store) => void;
+ setTimelineEmbeddedStore: (store: Store) => void;
```
- removed dependency to triggers_actions_ui plugin
- removed duplicated components and types with `security_solution`: 
```
TruncatableText
SubtitleComponent
EventsCountComponent
PopoverRowItems
PagingControlComponent
FooterComponent
SortIndicator
SortNumber
RowRendererContainer
plainRowRenderer
getColumnRenderer
StatefulRowRenderer
getMappedNonEcsValue
InspectButtonComponent
TGridCellAction
useMountAppended
tgrid store
```

Co-authored-by: kibanamachine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Threat Hunting:Investigations Security Solution Investigations Team technical debt Improvement of the software architecture and operational architecture
Projects
None yet
3 participants