Allow SOC wrappers to be excluded #40275
Conversation
|
Here is the POC we talked about which allows consumers to choose which SOC wrappers to exclude. No tests yet, see example usage here: https://github.com/elastic/kibana/pull/40275/files#diff-1edef6db6e7b5243c656d0d63ea99d32 If we're all on board with this approach, then I'll add tests and get this ready for a proper review. Looking forward to your feedback! |
💚 Build Succeeded |
| const { getScopedSavedObjectsClient } = savedObjects; | ||
|
|
||
| const savedObjectsClient = getScopedSavedObjectsClient(request, { | ||
| excludedWrappers: ['spaces'], |
There was a problem hiding this comment.
Sorry, I haven't worked with SavedObjectClient so far. Why we need this functionality?
There was a problem hiding this comment.
There are very limited use-cases for excluding wrappers.
We are trying to use this for the Copy to Space feature. Specifically, we will be introducing an API endpoint within the Spaces plugin which needs to be able to reach across spaces. By removing the spaces wrapper from that specific endpoint's instance, we can control which space (or, namespace) the SavedObjectsClient is targeting for each request.
Yeah, that's a valid concern, which we briefly talked about on our call last week. The wrappers have all been written with the assumption that there may or may not be other wrappers defined. For example, the security wrapper can work with the spaces wrapper, but it doesn't assume that the spaces wrapper will in fact be there. The reverse also applies. The ordering of the wrappers is somewhat "magical" in that it's not immediately clear why the wrappers are ordered in any particular way. That hasn't changed with this PR, but the ability to omit wrappers does add to the complexity. |
|
I personally prefer this approach over the others that we've attempted in #38444. It feels like a cleaner implementation than what we have in that other PR. I agree with everyone that it's not perfect though. Any strong objections from anyone to moving forward with this? |
No objections from me, I also prefer this approach to the others that we've explored. |
|
no objections |
💚 Build Succeeded |
legrego
added
release_note:skip
|
Pinging @elastic/kibana-security |
💚 Build Succeeded |
|
@restrry or @rudolf would either of you be interested in reviewing this? |
💚 Build Succeeded |
|
@legrego They're both out today, but will be back on Monday |
|
Thanks for the heads-up @joshdover! |
💔 Build Failed |
|
retest |
💚 Build Succeeded |
* allow SOC wrappers to be excluded * remove example route * add tests * more testing
Summary
Currently, the saved objects client wrappers come as a transparent package that the consumers has no control over. This is very useful for most scenarios, where the consumer does not care how the client is augmented.
However, there are special cases (such as #37286) where bypassing specific wrappers is beneficial. In the case of #37286, excluding the
spaceswrapper will allow us to implement an API endpoint which can work across multiple spaces, while still honoring the constraints placed by thesecuritywrapper.This PR adds an optional parameter when requesting a SOC instance which allows the consumer to specify which wrappers are to be excluded. See example here:
https://github.com/elastic/kibana/blob/816a3f457bf3e2debae3b8981ee1393029308a4f/x-pack/legacy/plugins/spaces/server/routes/api/v1/example.ts#L17:L19