kibana
kibana copied to clipboard
[Cloud Security] POC sharing Cloud Security APIs
Summary:
As part of [CDR] Review epic Integrate context into alert flyout, we need to export our APIs for fetching the latest findings filtered by the entity host.name
(which requires changes in Cloudbeat) and user.name
(which is not currently tracked). For the POC, we can start by exporting the status API and the latest findings API. Additionally, we can filter the findings by another field to ensure functionality.
Definition of Done:
- [ ] Verify that the status API and latest findings API are successfully shared between Kibana plugins
- [ ] Test the functionality by filtering findings with another field
- [ ] Obtain approval from stakeholders to ensure the task meets acceptance criteria before closing this ticket
- [ ] Optionally, share the table component to display the data in another plugin, if this proves to be complex, it can be done as another poc ticket
Pinging @elastic/kibana-cloud-security-posture (Team:Cloud Security)
@JordanSh Can you elaborate more on the need for exposing our APIs? I was thinking if we could use the existing Security Data View to query this date, but not sure how the flyout works exactly and if that's feasible
@JordanSh Can you elaborate more on the need for exposing our APIs? I was thinking if we could use the existing Security Data View to query this date, but not sure how the flyout works exactly and if that's feasible
The idea was to use our latest findings APIs instead of manually querying the data view. Since the Alerts detail flyout suppose to display the latest findings table, filtered by the shown entity, my assumption was that we would have the exact same fetching for both the alerts findings table, and our findings table.
Meaning we could reuse our own APIs which are already tested and integrate with our table component (which we would like to explore the option of sharing it as well). This will save us the trouble of maintaining and testing duplicate code.
The main problem I anticipate is cyclic dependencies, though there are ways around it. I thought that making a POC and understanding the pros and cons for this approach is needed
ticket refactor looks great @maxcold. R Regarding cyclic dependencies, of course I trust you to do your own research, but if you want you can also take a look at this PR which i ended up closing because i managed to avoid a solution which revolved around cyclic deps, but it might be valuable for your research
How do we encapsulate our REST APIs to be used in different plugins? Can we create a service (maybe as a separate package) to be used in both security_soltuion and cloud_security_posture plugin
To answer this requirement I created a POC draft PR https://github.com/elastic/kibana/pull/187686 showcasing different approaches to share code between plugins.
Options
Option 1: Import directly from cloud_security_posture
into security_solution
As the code from cloud_security_posture
is alread imported in security_solution
to render CSP pages (findings, dashboards, benchmark rules) we can continue with just importing the code directly from our plugin. The code of entity (user, host) flyout and document (alert, event) flyout is a part sucurity_solution
too so there is no risk of cyclic dependencies atm.
In the POC the example of this approach can be found in how the useCspmStatsApi
is imported
Pros:
- simplicity
- no specific changes required
Cons:
- questionable separation of concerns, no clear boundaries between shared code and specific plugin code
- potential problems with cyclic dependencies in case flyouts move out of security_solution.
- not following best practices recommended by the platform team in the Package docs
Option 2: Create two packages for shared code between plugins
Following the Internal Dependency Management docs and the approach taken be elastic-assistant
plugin (AI Assistant, one of the most recent plugins), we could create two packages in x-pack/packages
. One to share the code that can only be used on the client side (see shared-browser
in the docs) and another one for the code that can be used in both server side and client side (see shared-common
in the docs). This should allow us to not worry about cyclic dependencies in the future and separate concerns better.
In the POC the example of this approach can be found in two packages I created @kbn/cloud-security-posture
and @kbn/cloud-security-posture-common
Pros:
- better separation of concerns
- more robust and scalable solution for future requirements
- with the move from shared plugins to shared packages explained in the documentation, we potentially have less work to do in the future
Cons:
- more setup
- a bit more complex process of reusing the code between plugins. We need to think about shared code and split it between two packages, eg. we can't import from the
shared-browser
code into our server code, eg. in routes. - nothing will stop us from sharing the code following the first option (directly from the plugin) which could lead to a mix of approaches. We need the whole team onboard with the approach with packages and remember about it during CRs
Option 3: build contextual flyout components as a part of security_solution
Technically we can just build new components that are required for the contextual flyout work inside security_solution
Pros:
- local imports inside
security_solution
Cons:
- we won't be able to reuse these components in our plugin or any other plugin that security solution imports from, due to cyclic deps. That's the issue that @JordanSh hit with the PLI block which was built as a part of Security Solution
Option 4: have cloud_security_solution
as a part of security_solution
Adding this option for the documentation purposes, as I don't consider it as a viable option for the following reasons:
- it has the downsides of Option 3 of potentially hitting cyclic deps issue if sharing components with other plugins
- afaik the Security Team is concerned with how large
security_solution
plugin became and consider moving things out, not in
cc @JordanSh @kfirpeled @opauloh
The next step for me is to investigate what approach we could take in building data grids in the flyouts. Options to consider:
-
CloudSecurityDataTable
used on the Findings page in both Miconfigurations and Vulnerabilities -
UnifiedDataTable
shared component.CloudSecurityDataTable
from option 1 is a wrapper around it - Build the features around pure EUI components, either
EuiDataGrid
,EuiBasicTable
orEuiInMemoryTable
For transparency, worth noting that we chatted with @opauloh who worked on building CloudSecurityDataTable
and his suggestion was not to try to reuse this component and go with the option 3 as it's simpler and we don't have any functionality from our Findings data grids in the current mocks for contextual flyout. The data grids in the mocks consist of predefined set of columns and only support pagination and sorting.
For reference, here is the PR where CloudSecurityDataTable
was introduced:
- https://github.com/elastic/kibana/pull/167587
in the description it contains the matrix of different features covered by the component. The Misconfigurations page prio to CloudSecurityDataTable
was using EuiBasicTable
and the Vulnerabilities page was using EuiDataGrid
Thanks for sharing the proposal and suggesting the next steps
Maybe worth to add few more options to discuss, like whether cloud_security_posture should be merged into the security_solution or be a separate plugin.
Option 2 seems to be in its first steps, do you think it is fully ready for us to adopt? or we will be early adopters of it?
Maybe worth to add few more options to discuss, like whether cloud_security_posture should be merged into the security_solution or be a separate plugin.
I added two more options to the comment. I don't consider them viable, but I agree it worth mentioning them in case I'm missing smth or at least for the documentation purposes
Option 2 seems to be in its first steps, do you think it is fully ready for us to adopt? or we will be early adopters of it?
I don't know the full history of it, but I think the documentation is just old and maybe out of date. The initial PR with the docs https://github.com/elastic/kibana/commit/249b1648b05588506d2cf6628f8d25e627776db5 is 2 years old, and there are a lot of packages that follow this approach, to we won't be early adopters. Don't know about the next steps in their plan though, maybe it's not currently a prio
I played a bit with the CloudSecurityDataTable
and was able to have a data grid filtered by host.name
on the extended flyout (not a surprise tbh) but after working with it for a little bit, I tend to agree with @opauloh that it is probably not a good way forward. By default the UnifiedDataTable
and as a result CloudSecurityDataTable
has quite a lot of features that we don't need to the table on the flyouts. It felt like hacking around to cut the features out of it, and I'm pretty sure I would hit some blockers to cut them further to get to the simple table we want. And in general I don't feel like sharing this very opinionated component between or Findings page and the Contextual flyout is smth we want tbh. I don't see us adding features that appear on both at the same time. So it will be constantly adding more and more complexity to the shared component to cut features out.
Therefore for the AC
Optionally, share the table component to display the data in another plugin, if this proves to be complex, it can be done as another poc ticket
I would propose not to spend much time now and make a decision:
- if we agree that sharing data grid between Findings and flyouts is not feaseable, we can close it and just implement simple table based on the EUI component. We still might reuse some smaller parts used on Findings, eg. failed/passed renderer or utility hooks to query data
- if we still think it worth investing time into doing a proper POC of building a shared component that would allow rendering a complex and simple version of the data grid, I suggest we do it after implementing what is required for the normal flyout. we don't have a data grid there, and it's better to move forward with our widgets before spending time on components specific to the expanded flyout
@JordanSh @kfirpeled
Thanks @maxcold for the detailed explanation. I agree that sharing the table component doesn’t make sense given the differences between the two tables. Initially, I was hoping to save development and future maintenance time by reusing the component, but it’s clear that if they are completely different, trying to share the component could end up achieving the opposite. Focusing on separate implementations seems like the more efficient approach.
As for the options regarding our API sharing, I’m personally in favor of option #2. I do think that this is a significant decision. I suggest we bring it up in our next team huddle. We can present how it works, gather feedback, discuss any potential drawbacks, and make a collective decision.
@JordanSh thanks for reviewing the outcomes! I added the topic of sharing the code between plugins to the agenda of our huddle
To save us some time, we can start with option 3, and later refactor it into different packages to be able to re-use it, correct?
Thanks for all of the detailed explorations and notes here @JordanSh and @maxcold! Regarding Option 1 above:
Option 1: Import directly from cloud_security_posture into security_solution Cons: ... potential problems with cyclic dependencies in case flyouts move out of security_solution.
We're currently investigating migrating the security solution flyout into it's own package, and migrating the shared code from the flyout to it's own shared package as well. Depending on your requirements, it may be simpler for you to just implement the table directly in the security solution flyout package as a csp sub-directory. Now this work is currently only in the POC phase, and our timelines for the work may not align, but I did want to start the discussion regarding this planned work in case it may. @logeekal is handling the POC from our side.
As an aside, after being a part of a number of table refactors and seeing a number of them happening here, I agree that the best solution for you will most likely be option 3, building a simpler table around the existing EuiDataGrid
or EuiInMemoryTable
tables. The benefit of using the UnifiedDataTable
really comes into play if you do need the breadth of functionality that's provided by default there, but if it's not necessary at this time, no need to prematurely optimize.
@kfirpeled @michaelolo24
I think we already have some code in our plugin that we would like to reuse in the flyout instead of copying it over to the security solution. Eg. the logic of loading our data, or some data grid cell renderers. Tbh I don't see why we shouldn't start with Option 2 which I guess we all agree is a proper way forward. Creating our package is not a large amount of work, I already did it in my POC.
From the time perspective, the fastest and simplest would be Option 1, to import everything we need from the cloud_security_solution
plugin
Agree, let's start with your suggestion @maxcold.
@michaelolo24 where we can track the migration progress to the referred package that will contain the flyout?
Another question I have to you @maxcold , how do you suggest sharing code between packages? Is it by sharing the react hooks? or sharing services through kibana plugins mechanism? I'm not sure I fully understand all the implications and how it works under the hood. Or how each method affects the bundle size.
how do you suggest sharing code between packages?
@kfirpeled I was thinking about just sharing hooks and components from a package. I didn't look into sharing services through Kibana plugins mechanism. In the end, I was just looking at elastic-assistant
as the latest example of a feature present across security solution. They as well have elasti-assitant
plugin and kbn-elastic-agent
to share client side code and kbn-elastic-agent-common
to share universal (client+server) code. If you have a specific idea/example in mind regarding sharing through registered services, pls share, I can try to investigate a bit more
@kfirpeled
where we can track the migration progress to the referred package that will contain the flyout?
You can track the progress of migration of flyout components in a common package this PR and the work will continue as part of this epic issue.
Currently, it is a browser-package
with the intention of having components used in Security solution
. For now the name (@kbn/securitysolution-common
) is generic by intention and we can make specific packages ( for eg, @kbn/securitysolution-flyout
) if the need arises.
Please do let me know if there are more questions.
We agreed on creating kbn-cloud-security-posture
packages as a way to share components and APIs with other plugins