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] Add Host/User flyout in One Discover. #199279

Merged

Conversation

logeekal
Copy link
Contributor

@logeekal logeekal commented Nov 7, 2024

Summary

Handles #191998

Follow up work:

This PR add below entity flyouts for below entities in One Discover:

  • host.name
  • user.name
  • source.ip
  • destination.ip

In this PR we re-use the security solution code by making use of below model based on discover-shared plugin.

flowchart TD
  discoverShared["Discover Shared"]
  securitySolution["Security Solution"]
  discover["Discover"]


  securitySolution -- "registers Features" --> discoverShared
  discover -- "consume Features" --> discoverShared

Loading

How to Test

Note

This PR adds security-root-profile in One discover which is currently in experimental mode. All changes below can only be tested when profile is activated. Profile can activated by adding below lines in config/kibana.dev.yml

 discover.experimental.enabledProfiles:
    - security-root-profile
  1. As mentioned above, adding above experimental flag in kibana.dev.yml.
  2. Spin up Security Serverless project and add some alert Data.
  3. Navigate to Discover and add columns host.name and user.name in table. Now host and user flyouts should be available on clicking host.name, user.name, source.ip & destination.ip.
  4. Flyout should work without any error.
  5. Below things are not working and will be tackled in followup PR :
    • Security Hover actions
    • Actions such as Add to Timeline or Add to Case

Checklist

Delete any items that are not applicable to this PR.

incremental change

incremental save

entity flyout working

[CI] Auto-commit changed files from 'node scripts/notice'

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --fix'

fix: quick checks

Merge discover getRenderAppWrapper

fix: incremental change

fix: merge mistakes

fix: types and tests

fix: undo unnecessary changes

tests: cell renderer

fix: small quirks

fix: types

fix: getCellRenderers

fix: test comment

fix: formatting
@logeekal logeekal force-pushed the feat/entity_flyout_discover_context_final branch from 2a300b9 to ac84386 Compare November 8, 2024 16:37
@logeekal logeekal changed the title Feat/entity flyout discover context final [Security Solution] Add Host/User flyout in One Discover. Nov 8, 2024
@logeekal logeekal added backport:skip This commit does not require backporting Team:Threat Hunting:Investigations Security Solution Investigations Team release_note:skip Skip the PR/issue when compiling release notes labels Nov 8, 2024
@logeekal
Copy link
Contributor Author

logeekal commented Nov 8, 2024

/ci

@logeekal
Copy link
Contributor Author

/ci

@logeekal
Copy link
Contributor Author

/ci

string,
(props: DataGridCellValueElementProps) => ReactElement
>;
export type CustomCellRenderer = Record<string, FunctionComponent<DataGridCellValueElementProps>>;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@elastic/kibana-data-discovery

Changed this to FunctionComponent because previous type does not accept memoized components. FunctionComponent is more inclusive.

Please let me know if you would keep this a function instead.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't really see an issue with this, although we do already memoize cell renderers here, so it's likely not necessary to memoize again:

: memo(UnifiedDataTableRenderCellValue);

If we make this change, it might even make sense to go all the way and just accept ComponentType, but not really a strong opinion.

@logeekal logeekal requested a review from semd November 11, 2024 15:14
@logeekal logeekal marked this pull request as ready for review November 11, 2024 15:17
@logeekal logeekal requested review from a team as code owners November 11, 2024 15:17
@elasticmachine
Copy link
Contributor

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

Copy link
Contributor

@jloleysens jloleysens left a comment

Choose a reason for hiding this comment

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

kibana.jsonc LGTM

Copy link
Contributor

@semd semd left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks Jatin for doing this and taking the time to discuss and address each comment.
Great work! 💯

Copy link
Contributor

@davismcphee davismcphee left a comment

Choose a reason for hiding this comment

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

@logeekal It looks like discoverShared might have been partially removed from Discover and now there are some related type errors. Other than that this LGTM from the Data Discovery side and will be good to go once fixed.

Copy link
Contributor

@davismcphee davismcphee left a comment

Choose a reason for hiding this comment

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

Thanks for fixing the types, and this now LGTM on the Data Discovery side 👍 Congrats on merging the first Security extension point implementations @logeekal!

@logeekal
Copy link
Contributor Author

Thanks for fixing the types, and this now LGTM on the Data Discovery side 👍 Congrats on merging the first Security extension point implementations @logeekal

Thanks. Finally :D . But this is also experimental 🤣

@elasticmachine
Copy link
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Jest Tests #18 / useActionStatus should post cancel and invoke callback on cancel upgrade - plural
  • [job] [logs] Jest Tests #18 / useActionStatus should report error on cancel upgrade failure

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
discover 981 983 +2
securitySolution 6277 6283 +6
total +8

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
discover 100 161 +61
discoverShared 15 23 +8
securitySolution 119 123 +4
total +73

Async chunks

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

id before after diff
discover 811.3KB 812.2KB +874.0B
securitySolution 13.4MB 14.6MB ⚠️ +1.2MB
total ⚠️ +1.2MB

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id before after diff
discover 24 29 +5
discoverShared 3 2 -1
securitySolution 33 34 +1
total +5

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
securitySolution 86.6KB 87.8KB +1.2KB
Unknown metric groups

API count

id before after diff
discover 148 209 +61
discoverShared 16 26 +10
securitySolution 187 191 +4
total +75

async chunk count

id before after diff
securitySolution 101 102 +1

History

@logeekal logeekal merged commit c80f91e into elastic:main Nov 29, 2024
8 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.17, 8.x

https://github.com/elastic/kibana/actions/runs/12085265704

@kibanamachine
Copy link
Contributor

💔 All backports failed

Status Branch Result
8.17 Backport failed because of merge conflicts
8.x Backport failed because of merge conflicts

You might need to backport the following PRs to 8.x:
- [Fleet] Change uninstall tokens space when changing agent policies spaces (#199536)

Manual backport

To create the backport manually run:

node scripts/backport --pr 199279

Questions ?

Please refer to the Backport tool documentation

@logeekal logeekal added backport:skip This commit does not require backporting and removed backport v8.17.0 labels Nov 29, 2024
logeekal added a commit that referenced this pull request Dec 10, 2024
…199818)

## Summary

Fixes elastic/security-team#11112

Follow up to 
- #199279

Adds functional test for Security Profiles in One Discover.


### Checklist

Delete any items that are not applicable to this PR.

- [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

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Dec 12, 2024
…9279)

## Summary

Handles elastic#191998

Follow up work:
  - elastic/security-team#11112
  - elastic#196667


This PR add below entity flyouts for below entities in One Discover:
- host.name
- user.name
- source.ip
- destination.ip


In this PR we re-use the security solution code by making use of below
model based on `discover-shared` plugin.

```mermaid
flowchart TD
  discoverShared["Discover Shared"]
  securitySolution["Security Solution"]
  discover["Discover"]


  securitySolution -- "registers Features" --> discoverShared
  discover -- "consume Features" --> discoverShared

```

## How to Test

>[!Note]
>This PR adds `security-root-profile` in One discover which is currently
in `experimental mode`. All changes below can only be tested when
profile is activated. Profile can activated by adding below lines in
`config/kibana.dev.yml`
> ```yaml
>  discover.experimental.enabledProfiles:
>     - security-root-profile
> ```
>

1. As mentioned above, adding above experimental flag in
`kibana.dev.yml`.
2. Spin up Security Serverless project and add some alert Data.
3. Navigate to Discover and add columns `host.name` and `user.name` in
table. Now `host` and `user` flyouts should be available on clicking
`host.name`, `user.name`, `source.ip` & `destination.ip`.
4. Flyout should work without any error.
5. Below things are not working and will be tackled in followup PR :
    - Security Hover actions
    - Actions such as `Add to Timeline` or `Add to Case` 



### Checklist

Delete any items that are not applicable to this PR.


- [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

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Dec 12, 2024
…lastic#199818)

## Summary

Fixes elastic/security-team#11112

Follow up to 
- elastic#199279

Adds functional test for Security Profiles in One Discover.


### Checklist

Delete any items that are not applicable to this PR.

- [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

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
@jughosta jughosta added the Project:OneDiscover Enrich Discover with contextual awareness label Jan 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Project:OneDiscover Enrich Discover with contextual awareness release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team Team:Threat Hunting:Investigations Security Solution Investigations Team v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.