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] Update CellActions field type to be FieldSpec #157243

Closed

Conversation

machadoum
Copy link
Member

@machadoum machadoum commented May 10, 2023

issue: #150347

Context

Some Actions need to access FieldSpec data, which is not present on the CellActions API (subTypeand isMapped). So we are updating the CellActions field property to be compatible with FieldSpec.

Summary

This PR is the first step to fix #150347.

  • Updates the CellActions field type to FieldSpec
  • Adds minimal possible changes to make Security Solution compatible with the new type.
    • Adding the mandatory properties searchable and aggregatble to every CellActions call.
  • Fix SecurityCelActions calls to provide the right Kibana and ES field types
    • Previously we were mixing esTypes and kbnTypes. For example, if the esType is a keyword the kbnType has to be a string.
      Here you can check all possible ES and KBN types and here you can see the mapping between esType and kbnType

Next steps

  1. Add subType property to all CellActions calls so we can fix CellActions component should hide ShowTopN action for nested fields #150347
    We need to refactor the Security Solution to get FieldSpec from the DataView instead of using BrowserFields. Ideally, we have to do that for every CellAction call so the actions have access to the subType property.

  2. Add isMapped to all CellActions calls so we can fix [Security Solution] Disable filter cellActions for unmapped fields on Alerts table #154714

Checklist

@machadoum machadoum force-pushed the siem-explore-cell-actions-150347 branch from a3c227b to 3fea3f8 Compare May 10, 2023 11:35
@machadoum machadoum changed the title Siem explore cell actions 150347 [Security Solution] Update CellActions field type to be FieldSpec May 10, 2023
@machadoum machadoum force-pushed the siem-explore-cell-actions-150347 branch 4 times, most recently from c5a273b to b7dea3d Compare May 12, 2023 11:17
@machadoum machadoum self-assigned this May 12, 2023
@machadoum machadoum added release_note:skip Skip the PR/issue when compiling release notes Team:Threat Hunting Security Solution Threat Hunting Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Threat Hunting:Explore v8.9.0 technical debt Improvement of the software architecture and operational architecture Feature:Cell Actions Security Solution Cell Actions feature labels May 12, 2023
@@ -46,7 +47,7 @@ export const createShowTopNCellActionFactory = createCellActionFactory(
getDisplayNameTooltip: ({ field }) => SHOW_TOP(field.name),
isCompatible: async ({ field }) =>
fieldHasCellActions(field.name) &&
!UNSUPPORTED_FIELD_TYPES.includes(field.type) &&
!UNSUPPORTED_FIELD_TYPES.includes(head(field.esTypes) ?? '') &&
Copy link
Member Author

@machadoum machadoum May 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated the check to use esTypes and all CellActions calls to pass the rights types.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we always going to check only the first element in esTypes?

@machadoum machadoum force-pushed the siem-explore-cell-actions-150347 branch from 5843bab to b5924eb Compare May 15, 2023 09:01
@machadoum machadoum marked this pull request as ready for review May 15, 2023 09:22
@machadoum machadoum requested review from a team as code owners May 15, 2023 09:22
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-threat-hunting (Team:Threat Hunting)

@elasticmachine
Copy link
Contributor

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

@machadoum machadoum force-pushed the siem-explore-cell-actions-150347 branch from 730bbda to 5231efd Compare May 15, 2023 17:09
@kibana-ci
Copy link
Collaborator

kibana-ci commented May 15, 2023

💔 Build Failed

Failed CI Steps

Test Failures

  • [job] [logs] Security Solution Tests #1 / Detection response view Open in timeline opens timeline with correct query count for hosts by alert severity table
  • [job] [logs] Security Solution Tests #1 / Detection response view Open in timeline opens timeline with correct query count for hosts by alert severity table
  • [job] [logs] Security Solution Tests #1 / Detection response view Open in timeline opens timeline with correct query count for open alerts by rule table
  • [job] [logs] Security Solution Tests #1 / Detection response view Open in timeline opens timeline with correct query count for open alerts by rule table
  • [job] [logs] Security Solution Tests #1 / Detection response view Open in timeline opens timeline with correct query count for users by alert severity table
  • [job] [logs] Security Solution Tests #1 / Detection response view Open in timeline opens timeline with correct query count for users by alert severity table
  • [job] [logs] Jest Tests #9 / draggable_score renders correctly against snapshot
  • [job] [logs] Jest Tests #9 / draggable_score renders correctly against snapshot when the index is not included
  • [job] [logs] Jest Tests #9 / entity_draggable renders correctly against snapshot
  • [job] [logs] Security Solution Tests #3 / Hover actions Adds global filter - filter in
  • [job] [logs] Security Solution Tests #3 / Hover actions Adds global filter - filter in
  • [job] [logs] Security Solution Tests #3 / Hover actions Adds global filter - filter out
  • [job] [logs] Security Solution Tests #3 / Hover actions Adds global filter - filter out
  • [job] [logs] Security Solution Tests #3 / Hover actions Adds to timeline
  • [job] [logs] Security Solution Tests #3 / Hover actions Adds to timeline

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
securitySolution 3875 3876 +1

Async chunks

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

id before after diff
securitySolution 9.2MB 9.2MB +762.0B

History

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

cc @machadoum

@machadoum machadoum force-pushed the siem-explore-cell-actions-150347 branch from 5231efd to b5924eb Compare May 16, 2023 08:03
@machadoum
Copy link
Member Author

I am moving this PR to #157834

@machadoum machadoum closed this May 16, 2023
machadoum added a commit that referenced this pull request Jun 22, 2023
…7243 (#157834)

issue: #150347

## Context
Some Actions need to access `FieldSpec` data, which is not present on
the `CellActions` API (`subType`and `isMapped`). So we are updating the
`CellActions` `field` property to be compatible with `FieldSpec`.

## Summary

This PR is the first step to fix
#150347.
* Updates the `CellActions` to support an array of data objects, each
containing field (`FieldSpec`) and value
* Create a new `SecurityCellActions` component that accepts a field name
and loads `FieldSpec` from the Dataview.

## Examples
Before: 
```tsx
 <SecurityCellActions
      value={'admin'}
      field={{
        name: 'user.name',
        type: 'keyword',
        searchable: true,
        aggregatable: true,
        ...
      }} />
```
After:
```tsx
 <SecurityCellActions data={{ field: 'user.name', value: 'admin' }}/>
```
`SecurityCellActions` will load the spec from the Dataview and provide
it to `CellActons`.

`CellActons` now also support an of fields instead of only one field. It
will be useful when rendering cell actions for aggregated data like on
the Entity Analytic page. But for now, the actions are not supporting
multiple values, we need to rewrite them
#159480.

### Next steps
We must refactor the Security Solution to get `FieldSpec` from the
`DataView` instead of using BrowserFields. Ideally, we have to do that
for every `CellAction` call so the actions can access the `subType`
property.
- [x] ~Refactor the Security Solution CellActions calls to get
`FieldSpec` from the `DataView`~
- [x] Refactor data grid cell actions to get `FieldSpec` from the
`DataView`
- [ ] Rewrite actions to support multiple fields and use them on the
investigation in timeline (.andFilters)
- [ ] Fix #150347 using
`subType` from `fieldSpec`
- [ ] Fix #154714 using
`isMapped` from `fieldSpec`

### Extra information
*** Previously we were mixing `esTypes` and `kbnTypes`. For example, if
the `esType` is a keyword the `kbnType` has to be a `string`.

[Here](https://github.com/machadoum/kibana/blob/9799dbba27c5baf594357eae0bbfc79b4e7da77c/packages/kbn-field-types/src/types.ts#L22)
you can check all possible ES and KBN types and
[here](https://github.com/machadoum/kibana/blob/9799dbba27c5baf594357eae0bbfc79b4e7da77c/packages/kbn-field-types/src/kbn_field_types_factory.ts)
you can see the mapping between esType and kbnType


### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Cell Actions Security Solution Cell Actions feature release_note:skip Skip the PR/issue when compiling release notes Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Threat Hunting:Explore Team:Threat Hunting Security Solution Threat Hunting Team technical debt Improvement of the software architecture and operational architecture v8.9.0
Projects
None yet
4 participants