-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Saved Object Management - Copy to Space #37286
Comments
Pinging @elastic/kibana-security |
High-level tasks🔌 Design and implement extension point into Saved Objects UIConsiderationsIs this a candidate for the Embeddable/Actions API? Should we allow for bulk operations, like Export and Delete behave today, or should this be an action specified at the grid row level? Either way, we can support copying related saved objects. 💥 Design and implement "Copy to Space" actionOnce the extension point is in place, then the Spaces plugin can embed its own "Copy to Space" action in the Saved Objects Management UI. ConsiderationsWe will need to be somewhat intelligent about how we show this to the end-user. At an absolute minimum, we should only show a list of spaces the user has access to. question: When determining which spaces the user is able to copy to, should we be checking for the "Saved Objects Management" feature privilege, or should we be checking that the user has privileges for the underlying saved object types? For example, if a user wants to copy a dashboard (and related SOs) from Space A to Space B, is it sufficient to have the ✏️ Implement Spaces API endpoint to allow objects to be copiedConsiderationsThis endpoint will likely be constrained to the existing SO import/export API limit of 10,000 saved objects. It's unlikely to be a problem, but the endpoint needs to account for the fact that it could happen. This endpoint (and corresponding UI) will need to account for conflicts when copying into the target space(s). Currently, the import/export functionality requires an instance of the Saved Objects Client to function. This client is scoped to the current request, which binds it to the current user and space. While this is fine for the export operation, it won't work for the subsequent import operation. Here are a few options I've come up with so far:
|
I'm not too familiar with the Embeddable/Actions API to comment on this.
Ideally, both would be possible. I'm not sure if there's a reason why we don't allow "Export" to occur on a per-row basis though, there might be a reason why not to do so. @mikecote I know you've worked on this screen somewhat recently, any insights?
I'm rather torn on this, but I'm also leaning toward the management feature privilege as well. Especially because we're going to be using the management specific APIs to make this possible, and we could potentially restrict access to these APIs to only users with that feature. Let's start with this and relax the constraint if it is problematic/limiting. |
@stacey-gammon would this be a candidate for the Embeddable/Actions API? We're looking for Spaces to augment the UI of the Saved Objects Management screen, in one or both of the ways outlined in the screenshots here |
This is actually pretty interesting. I was thinking the other day that we actually don't need to couple the actions API and the embeddables API. So you could do this via embeddables and actions... but do you really need the component to be an "embeddable"? Not really... so we could actually separate the actions API from embeddables and have actions just take in any trigger context, so instead of: class MyAction extends Action {
execute({ embeddable, triggerContext } {...}
} you could do class MyAction extends Action {
execute(triggerContext: { } {...}
} and for most embeddable use cases, they would pass the embeddable as part of triggerContext class MyAction extends Action {
execute(triggerContext: { embeddable, anyExtraContext }) {...}
} Then, we just have a really generic way of having developers register triggers and actions and we don't need to add a new registry every time someone wants to expose part of their UI to be extendable via a context menu. I wasn't really planning on thinking this through till at least post PR 1 when an actual use case for it came up... but this actually feels like a good candidate. |
For the per-row export option, there isn't a reason we don't have it. The export APIs would support a single item if ever this was to be implemented. |
@stacey-gammon thanks for your input! I won't hold you to a timeline on landing these APIs, but I'll investigate using them if they're available when we work on this. If not, we can always go back in a subsequent PR and clean it up. @mikecote good to know, thanks! |
@kobelb I updated my comment above with additional thoughts around the API implementation. |
I agree that this is a less than ideal solution. However, it definitely seems the easiest. We currently limit the import payload size to 10mB, so theoretically we'd only need to consume 10mB of browser memory to facilitate this process. However, without any changes to the export/import APIs, we'd use the export size times the number of spaces of bandwidth to perform this operation. It depends on the network topology and the size of the saved objects for how large of an impact this has.
If I'm thinking about this properly, we should only need to do this for the import API itself. As the export API will only be dealing with a single space. The risk in not using the SOC wrappers at all as part of the import is that it'd be skipping over the encrypted saved objects wrapper. Even though on the export we're stripping the encrypted attributes, we don't want to be inserting unencrypted attributes if they do so happen to be in the import payload.
I do think we're going to need some facility in the future to let the routes themselves specify the namespace(s) directly instead of only inferring it from the route URL. If we wanted to add a "copy to namespace(s)" route, we couldn't infer both the destination and source namespaces from the basepath of the URL, as the namespaces need to be different. Currently, the SOC wrappers ordering is Security (which uses the spaces service directly to determine the current namespace and perform the authorization) and then the Spaces wrapper (which augments the options to include the namespace). This is largely because I kept insisting that we have to perform authorization first before doing anything else, which isn't a good reason in retrospect. If we were to change the ordering of the wrappers to be Spaces (which augments the options to include the namespace) and then Security (which can use the actual namespace argument to perform the authorization), we can then augment the spaces wrapper to allow the consumer to specify a namespace otherwise we infer one from the basepath. We'd potentially have to figure out how to make it explicitly obvious that we're specifying the default namespace, besides by omitting the namespace argument entirely, but this seems do-able. |
Thanks for your detailed followup!
I like this approach, and it doesn't feel like it'd be all that bad to implement. I'll see what this looks like, and we can decide if we like it or not |
If this starts to get more complicated than initially anticipated, we can potentially introduce the ability to copy an individual saved object and all of its dependencies to only one target space at a time. This would still be a lot better than what we have right now, without exacerbating the limitations of the current implementations. |
Currently, to "copy" objects from one space to another we instruct users to export the saved objects from the source space, change their space to their destination space, then import the saved objects. Additionally, the user must repeat the import for all destination spaces, which can be time consuming and painful.
Now that #34896 has merged and we can more easily export a saved object and all of it's references, we should add a first-class "copy to space" option to the saved object management screen.
The text was updated successfully, but these errors were encountered: