[Cloud Security] POC sharing Cloud Security APIs

[JordanSh]

Summary:

as a part of https://github.com/elastic/security-team/issues/9015 and https://github.com/elastic/security-team/issues/9137 we need to show components related to CSP in the context of security_solution plugin. Specifically, we have:

• Misconfiguration and Vulnerability posture preview per host.name and user.name
• Misconfiguration and Vulnerability basic data grid with findings per host.name and user.name

Specific data that we are concerned for these epics:

• status of the data streams/indexes and integrations. Basically, the information which is already covered by CSP's status/ API. We also might need to cover 3rd party integrations and data stream/index statuses there
• Current Misconfiguration and Vulnerability posture (number of failed/passed findings, number of vulnerabilities split by severity)
• Misconfiguration and Vulnerability finding documents for a basic data grid (with pagination, sorting, filtering)

In this issue API means any programmatic interface that can be shared: REST APIs over HTTP, services encapsulating data fetching, set of hooks or other components shared across plugins, etc.

The strategic questions we want to answer in this ticket:

1. how do we expose our data fetching APIs so that we reuse them across security_solution and cloud_security_posture plugins? The ideal state is that we reduce the number of places to update to a minimum. Situation to avoid: disconnected code path across multiple plugins doing things differently
2. how to avoid problems with cyclic dependencies so that we could freely use those APIs in any plugin, or at minimum in both security_solution and cloud_security_posture plugins. See related PR where this challenge surfaced and explored for PLI component

More specific questions to answer:

1. 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
2. Does it make sense to reuse the Data Grid from our Findings page in the flows of Security Solution? Is it feasible as on the Findings page we use the data grid with grouping which has many more features than we plan for the data grid on the flyouts?
3. If we don't want to reuse the e2e data grid component, what parts we can share between data grids, eg. Data View data fetching logic

Definition of Done:

• Verify that the status API and latest findings API can be successfully shared between Kibana plugins
• 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

[elasticmachine]

Pinging @elastic/kibana-cloud-security-posture (Team:Cloud Security)

[maxcold]

@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]

@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

[JordanSh]

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

[maxcold]

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

[maxcold]

The next step for me is to investigate what approach we could take in building data grids in the flyouts. Options to consider:

1. CloudSecurityDataTable used on the Findings page in both Miconfigurations and Vulnerabilities
2. UnifiedDataTable shared component. CloudSecurityDataTable from option 1 is a wrapper around it
3. Build the features around pure EUI components, either EuiDataGrid, EuiBasicTable or EuiInMemoryTable

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:

• [Cloud Security] CloudSecurityDataTable component #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

[kfirpeled]

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?

[maxcold]

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 249b164 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

[maxcold]

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:

1. 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
2. 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

[JordanSh]

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.

[maxcold]

@JordanSh thanks for reviewing the outcomes! I added the topic of sharing the code between plugins to the agenda of our huddle

[kfirpeled]

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?

[michaelolo24]

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.

[maxcold]

@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

[kfirpeled]

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.

[maxcold]

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

[logeekal]

@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.

[kfirpeled]

We agreed on creating kbn-cloud-security-posture packages as a way to share components and APIs with other plugins