-
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 objects import/export should include spaces #91615
Comments
Since importing saved objects will add and potentially change existing saved objects in other spaces than the user's current space, users will have to have access to all spaces which the import will affect. However, the user experience can be quite frustrating when imports stop working because an existing saved object has been saved to a different space to which the current user doesn't have access. So it feels like import/export needs to be limited to "administrator" users who have access to all spaces. |
Pinging @elastic/kibana-core (Team:Core) |
Pinging @elastic/kibana-security (Team:Security) |
FWIW, today (as of Sharing Saved Objects phase 1.5), when we run into this situation ("unresolvable conflict") we simply regenerate the imported object's ID. However, once import/export is space-aware, we can instead show an appropriate conflict warning to the end user, then overwrite these objects by setting the "initialNamespaces" upon create. I feel like this would be a better user experience.
I worry that "non-global admins" (users who don't have all-spaces-admin-access) will chafe at the idea that they cannot export or import saved objects. Sometimes Kibana is used in "multi-tenant" scenarios where each tenant gets their own Kibana space. I think we could still support a space-aware import/export for non-global admins, even if it means they must skip importing an object when an unresolvable conflict occurs. Edit: forgot to mention, I think this is blocked until we can support bulk copy-to-space (#52098 and #90025). In the multi-tenant scenario we ostensibly have users that want to "stage" new spaces using import/export, but they will need an alternative when this is no longer possible. |
We already discussed about it a while ago with @kobelb (and probably others from the team), but I think there are two distinct scenarios here: The (new) admin backup/restore, where we do want to preserve all spaces information, and that should probably only be performed by admins And the 'classic' (current) import/export, that can, for example, be used to copy SOs from one environment to another one, and not necessarily to the same space, where stripping space information during export and relying on the current user's (or API call's) space make sense. Also note that
Even if we do enhance the API to preserve namespace information during export and use it during import, there is current no way (from the UI or the API) to export objects from multiple (distinct) spaces at the same time, which seems like pretty mandatory for the feature. That makes me think that it could even be two totally distinct features, from different administration sections. It would also means that this would be an enhancement, and not a breaking change, as the current SO management import/export would remain unchanged. For the 'backup/restore' feature, we may also want to only allow types and/or spaces selection, instead of allowing to filter and search for specific objects by searchableFields as the SOM is currently allowing via the searchBar. User would basically choose
For #82020, we may also want to allow administrators to remap the spaces during import, aka 'objects in space X in the backup file would be reimported in space Y' @kobelb @elastic/kibana-security wdyt? |
@elastic/kibana-security Friendly bump on this. We'd like to align on an approach in the next few weeks so that we can prioritize any necessary work on our end for the 8.0 release. |
👍
I like this proposal. I think we should limit this new feature to users who are "global admins", those who have privileges to access the global resource (e.g., "All Spaces"). If a user is not a global admin, each exported object's Just to be clear: the idea here is that the kibana-wide backup would include the actual spaces themselves, yes? Thoughts/questions that come to mind:
Overall I agree with the approach at a high level, I think we need to work out some of the details and what the UX would look like.
I think we need to tease out the details of this a bit more.
|
@pgayvallet is my understanding correct that you're proposing leaving the import/export API how it behaves today, where it only works for a single space, always excludes the namespaces on export and uses the current space on import. And to support the additional use-cases, we'd add a backup/restore API that would work across all Spaces and include the namespaces on export and respect them on import? If my understanding is correct, this leaves me with a couple questions:
I ask these questions, because I've been wondering whether we should just deprecate the existing import/export API and make a new backup/restore API that works for all situations. |
I'd love to explore this further. I'm not sure how much pushback we would get from the community (or support), but it's an exciting proposal.
IMO the new feature is an opportunity to use a new data structure, with first-class support for metadata. |
Yea, this is meant to be an administrator-only feature, to restricting to the highest permissions / role seems fine.
Good question. I think we want to, but retaining the space-specific information such as the permission matrix may be a pain to reimport, e.g to another instance where the defined roles are not the same. I'm afraid exporting the spaces themselves would also require to manage / include the associated roles, which is all but trivial, wdyt?
If we focus on backup/restore, I'd say erasing existing conflicting objects should be ok. The way I see it, we don't want to support object-level granularity as we do with the current import/export feature.
Same, in the scope of backup/restore, we should probably make the assumption that a backup respects the ref integrity, as any backup-ed object also did include its references, so this seems like an edge case.
The way I see it, we will have another page for this feature, however we will not do down to the object granularity: We will list the spaces, the number of object per type once the user selects a list of spaces, but we will never list the objects as we do in the current SOM page.
Definitely, but I just want to be sure we're all ok with the high-level idea before going down on the details.
YTBD. Even if I'm not sure to see a need to import multiple input spaces into the same output one, I'm sure some customers will.
Probably not. I think we do want this for 8.0 though. Seems like a strong requirement for #82020
That's it yea.
My thinking is:
My conclusion is:
ATM, I don't see any technical blocker to do so. Also (as Larry said already), TBH, the current NDJSON format is not that great. E.g one year ago we had to add an additional metadata entry at the end, that is not following the same structure, to store meta such as number of non found objects and so. Allowing us to use distinct formats would also allow us to challenge the existing one.
All, please read - this summarize all the previous points The question and its implications are complex, but to summarize (this also kinda summarize all the other discussions): In term of UX/UI, I strongly think that the current feature and backup/export would require distinct UIs to best address them. As already explained, selecting spaces and types during a backup is not the same workflow as picking objects as done in the current SOM app. Also, the required permissions for both features are not the same. In term of API, I also do think we should have distinct HTTP endpoint / associated service methods. the current In term of technical implementation, I feel like the current code was conceived (and patched multiple times) to only address the current feature. I'm afraid adapting it to support this additional multi-spaces export would be a pain. In term of exported data format (ndjson), the main (only?) reason for this format was the possibility to stream the results to the client 'in real time' during the export. Functionality that was never effectively used, and that is now impossible due to the addition of the additional metadata entry at the end of the stream. Given that, imho, the cons of the format are quite impacting (plain JSON is just way easier to manipulate), I do think that this would be a good time to challenge and/or revisit the format. However, I did assume that we wanted to preserve the current import/export API, it never came to my mind that we could develop a new API supporting both the current features and the new ones. I do think this is an option (and probably the best one), if (and only if):
wdyt all? |
Distinct management sections@pgayvallet, I'm interested in why you're pushing for distinct management sections and screens. My opinion isn't the only one that matters, but I don't hate the saved-object management screen. There are imperfections to it, like being able to edit the raw JSON of saved-objects, but I still see the utility in allowing a user to have a unified view of all of their saved-objects and allowing them to perform common functions across them. Use-casesI think we need to take a step-back and think through the ideal UX and API to enable all of the valid use-cases. The following are the use-cases that I've thought of, segmented to within a cluster and between different clusters. I've offered a bit of commentary on these use-cases. We don't have to support all of them initially, but we should make sure that the choices we make will work for all of the valid use-cases long-term. Within a single clusterPseudo-copyI don't think we should support this in 8.0+. It's a surprising legacy behavior that we should just get rid of ASAP. True copy between SpacesA large number of users will likely want to do this. We shouldn't restrict this functionality to users who have the ability to manage saved-objects in all spaces. We don't need to rely on the import/export or backup/restore APIs to perform this action, and we can add a dedicated copy endpoint(s). Backup/Restore one-to-many saved-objects within a SpaceA large number of users will likely want to do this as well, including users who only have access to a single Space. As long as the operation is restricted to a single Space, we can drop all space information on export, ignore space information on import, or include the space information and ensure we're importing the saved-object to the space it was exported from. Backup/Restore one-to-many saved-objects across SpacesFewer users will potentially want to do this, but I don't think we can safely assume that only users who can manage saved-objects in all spaces will want this functionality. If I happened to be the admin of 2 spaces out of 1000, I'd want to be able to see all of the saved-objects in those two spaces and selectively backup/restore them. Backup/Restore everything across all SpacesOnly allowing users who can manage saved-objects across all Spaces is the logical option here Between clustersCopy between clusters, ignoring the source SpaceUsers may have different spaces in different clusters, but still want to copy saved-objects between them. A long as the user has access to manage saved-objects in the source/target cluster and Spaces, they should be able to perform this operation. Copy between clusters, including the source SpacesEven if users only have access to a subset of spaces in the source space, they should be able to export all of these saved-objects into the target cluster and spaces, while maintaining the source space information. Changing NDJSON data-formatUsers can currently export saved-objects from the previous major version of Kibana: https://www.elastic.co/guide/en/kibana/current/managing-saved-objects.html#_compatibility_across_versions. It'd be beneficial to our users to continue supporting exports from 7.x in 8.x. |
Because (the way I see it) these are totally distinct user workflows (see #91615 (comment)), and the backup features is not based on a UI displaying a table listing all SOs, as we don't want this granularity for the feature. But once again, this was my gut feeling based on my assumptions, please challenge me on that. If we think that it makes more sense to just add a Note that adding the feature to SOM and allowing all users to export/import from/to all spaces they're allowed seems like a huge pain in term of security checks during the import, to ensure that all objects are imported to spaces where the user has correct permissions. This is another reason why only allowing administrator to use multi-space import/export feels very pragmatic to me.
Is that true though? What about objects with export/import hooks?
It's even mandatory for #82020, as this is how users will migrate from legacy multitenancy in 7.x to spaces in 8.x. However, just look back to the 'legacy' export format. We don't have any metadata attached to either the legacy or the current export format. Do you know how we are currently making the distinction between these two formats during import? Please have a look look at this beauty: Line 161 in 4584a8b
That's right, we're looking at the file extension. What will be the plan when we'll be forced to introduce breaking changes to the format (trust me, it will happen, sooner or later). We will export in I agree that we do need to preserve BWC, but we also DO need a better export format. As the legacy format will go away in However I agree that this is not needed by this specific feature. I'm just afraid that by adding functionalities one by one without looking at the big picture, we'll end up doing way more work than necessary and being forced to rewrite more things more times. E.g the recent issue about the need to exclude connectors from export based on a predicate. |
Me too :) I think we need to get design involved to help us figure out whether or not this is reasonable.
I think it'd be fine to only allow administrators to use multi-space import/export in the short-term. However, I think we should make sure that our UI and API design can evolve over time to facilitate users who don't have access to every single space. As users lean into spaces for solving their multi-tenancy concerns, fewer and fewer users will be global administrators.
You're right that we do need to make sure that the existing logic in export/import hooks applies to copy as well. It was my intent to say that we could have a dedicated HTTP API for doing so, it doesn't have to rely on the import/export or backup/restore HTTP APIs.
100%. |
Ok, I'm going to KISS and assume we'll go that way for now. So, let's focus a bit on the technicals now: From what I see: we're currently hard-removing the namespaces from the objects during the export process kibana/src/core/server/saved_objects/export/saved_objects_exporter.ts Lines 142 to 144 in 49078c8
This could easily be changed with an export-scoped option. However it seems to be only for multi-namespace types: kibana/src/core/types/saved_objects.ts Lines 87 to 88 in 4584a8b
We apparently aren't surfacing the Also, I tend to forget that you can't specify the initial namespace for single-namespace types when using kibana/x-pack/plugins/spaces/server/saved_objects/spaces_saved_objects_client.ts Lines 128 to 132 in 106afd4
@elastic/kibana-security Do you have any idea of the estimated cost to allow these API to allow the consumer/caller to specify the Note: we would also need to change the In short:
@elastic/kibana-security what's you take on that? |
I'd prefer to leverage the existing
It wouldn't be a lot of work to allow this. We would need to normalize the namespaces in the SOR ( Using |
I actually missed that, but you're right,
Can they? Reading the code of the spaces wrapper, I assumed otherwise, at least for single-ns types, as we're throwing if kibana/x-pack/plugins/spaces/server/saved_objects/spaces_saved_objects_client.ts Lines 109 to 114 in b2d36b8
Or did you mean they can already use
Using
Yea, well, if you ask me, this approach is perfectly fine imho. I even kinda like the fact that the API is the same for single and multi-namespace types, and that we just perform validation under the hood. |
Sorry for the confusion, that slipped my mind. You're right, consumers can't specify the kibana/x-pack/plugins/reporting/server/core.ts Lines 282 to 300 in f97aad3
Sounds like we are in agreement on the technicals, then. |
@MichaelMarcialis @thesmallestduck just a quick ping, in case you want to get involved. ATM, the plan is to just use the existing SO management section, and
Does that sound alright with you, on a functional/design POV? |
Hey, @pgayvallet! I'm coming into this conversation sideways, so I apologize in advance if I'm misinterpreting anything. Am I correct in assuming the spaces filter you mention (and possibly related table column) will be referencing saved objects that are shared across spaces? I assume the request for my involvement is in reference to my past design suggestions as part of the saved object tagging feature, as some of those suggestions included improvements to saved object management. If that's the case, based on what I'm reading here, I'm OK with the notion of updating the existing saved object management page as you've described. In fact, that approach is largely what my design suggestions for the saved object management page entailed (along with a handful of other UX quality-of-life improvements). Assuming these designs are still relevant to the conversation, please feel free to have a look and let me know if you'd also like me to put some time on the calendar to discuss further, if need be. Thanks. |
We should probably split this issue in two parts:
@legrego @jportner do you think part |
I started working on it, and I have a few remarks: Adding multi-space support to SO import/export effectively have the requirement to also support multiple spaces in the whole SOM ui (which we totally overlooked). E.g:
On a more global level, the whole Kibana app was conceived in a space-isolated manner. We're introducing a precedent here, by allowing to visualize and interact with objects from other spaces. I don't thing anything would be impossible here, but the whole 'make the SOM page multi-NS compatible' is something we just totally forgot to take into consideration, and that is going to require some work. I just want to be sure we're all on the same page and that we all agree that this must be done (the other option would be to just adapt the HTTP import/export APIs to support multi-NS without doing any change to the UI, but that feel wrong, doesn't it?) cc @kobelb (told ya) @lukeelmers |
We have this right now (provided by the Spaces plugin) but it 1. it is commented out until shareable objects are available in the SOM page, and 2. it only shows other spaces (not the current space). We could easily change it so it shows the current space, too.
Good point. I was under the impression that we wanted to remove "inspect objects" from the SOM page anyway? That still leaves navigation though. And we would have to take into account objects that exist in multiple spaces. So, yeah, new UX needed for that / non-trivial.
So, today the only other example of a page that has "cross-space-aware" saved objects in Kibana is ML. That said, we are talking about a management page here and I think the current behavior doesn't really make a whole lot of sense. Other management pages (such as role management and user management) also affect the rest of Kibana. Not to mention that if you delete an object that is shared, it gets deleted from all spaces that it exists in. A couple more questions come to mind:
|
Yea. I think just disabling the actions if the object is not present in the active space is acceptable until we take a closer look at what we may exactly want.
I thought a lot about this, and I'm not sure of the correct solution. Restricting to full admins would definitely be an option, and (probably) the easiest one. Another option would be to filter the spaces listed in the UI depending on the user's permissions in each of those spaces (probably via a new wdyt?
I was planning to keep the current behavior (only current space) by default anyway, to avoid introducing unnecessary disruption to the UI. I was thinking about just adding another filter defaulting to the current space: But maybe the only modes we want are 'active space' and 'all spaces', in which case, a toggle switch would probably make more sense, I'm unsure. What do you (and @kobelb) think? (Note that if we go with the select, we would need to add a |
I love how it's impossible to identify all the things that are going to be on your way when touching the SO import/export code before starting to try to implementing it. A few blockers: Export
SOM UI
|
I think restricting to full admins is the best thing to do, at least for now. Otherwise you may have a situation where you have permission to import to only a subset of the specified spaces, for example. How would we handle that? Just fail the whole import, I guess?
Well, we still want users to be able to export/import without So I think a switch may be a better solution. We can always come up with a more complex control later on if need be. |
That would be fine with me. In term of API, how can I leverage the security plugin to know the user is a 'full admin' ? Is this a specific role? Side question: should we only show the 'namespace' column in such situation? Technically that's a bit complex, and as we're going to need it for everyone once the shareable type are there, I'd say it's fine to have this new column even when not allowed to perform multi-namespace exports.
We do. I added a new I'm unsure what the default value should be though. I guess that, for (user-)BWC, it should be disabled by default?
The POC is currently using a select (and to be honest, I'm not sure how a toggle would play in an EUI's searchbar). Let's try with that once the technical prerequisites are complete, and we'll iterate from here. |
Good question. No, it's not a specific role. A "full admin" has I am assuming that we want a user who has read-only access in all spaces to be able to export all objects, too. I don't think we have a special term for these users, but let's call them "read-only admins" 😄 These privileges are defined here: kibana/x-pack/plugins/security/server/authorization/privileges/privileges.ts Lines 103 to 122 in ea06239
So you'll probably want to do a few things:
diff --git a/x-pack/plugins/security/server/authorization/privileges/privileges.ts b/x-pack/plugins/security/server/authorization/privileges/privileges.ts
index c38a5c9a44f..a9ee584de23 100644
--- a/x-pack/plugins/security/server/authorization/privileges/privileges.ts
+++ b/x-pack/plugins/security/server/authorization/privileges/privileges.ts
@@ -111,12 +111,18 @@ export function privilegesFactory(
actions.ui.get('management', 'kibana', 'spaces'),
actions.ui.get('catalogue', 'spaces'),
actions.ui.get('enterpriseSearch', 'all'),
+ actions.api.get('importAllSavedObjects'),
+ actions.ui.get('savedObjectsManagement', 'importAcrossSpaces'),
+ actions.api.get('exportAllSavedObjects'),
+ actions.ui.get('savedObjectsManagement', 'exportAcrossSpaces'),
...allActions,
],
read: [
actions.login,
actions.version,
actions.api.get('decryptedTelemetry'),
+ actions.api.get('exportAllSavedObjects'),
+ actions.ui.get('savedObjectsManagement', 'exportAcrossSpaces'),
...readActions,
],
},
Gotcha, that makes sense. So if the user has already changed the top select filter to display multiple spaces, that option is selected already in the Export flyout / cannot be disabled?
Yes I think so.
Sounds good. |
Thanks a lot for the pointers
That's the plan yes, even if not implemented yet in the PR.
We talked about this sync, and we agreed that distinct endpoints, owned by core, seems like the best compromise. |
So, just when I thought #109196 was getting close to be ready-ish, I discovered that the whole conflict resolving logic is currently only supporting a single namespace as the target, which is not sufficient for the cross-space import where you can import shareable types into multiple spaces, and need to check for conflicts accordingly. After discussing with @jportner, enhancing the behavior to support such scenarios is not an easy task, and, more importantly, it's functionally unclear how we should handle conflicts in such cases, and if we want to take shortcuts because the API is currently only limited to 'super users', or if we shouldn't, which would allow us to more easily open the feature to other users with proper space restriction. Given the sync discussion we had today with @kobelb @rudolf @legrego and @lukeelmers about how we should handle pseudo-copy in the 'new' import, it seems like we'll probably want to do the same for the cross-space import (which could actually, once we address the PR's current super-user restriction, be the start of this new, BWC, import). This is why the team decided to put #109196 on hold until we have a more precise vision of how we want to handle conflict resolution and pseudo-copy. |
Notes from our last meeting (1 Sep 2021): The problem with psuedo-copiesAccounting for all the conflict scenarios introduces significant complexity and tech debt to the saved objects codebase. But more than that, it introduces significant complexity to our users. Even if we have a UI that allows users to resolve conflicts it’s really hard for a user to understand why this conflict is occurring, and in many cases users won’t have the necessary context to resolve a conflict. E.g. faced with JSON of two To provide a better experience for our users we should prevent them from encountering these conflicts in the first place, e.g. remove the ability to create pseudo-copies of documents. Avoiding pseudo-copies on importTo avoid pseudo-copies on import we would need to introduce a new API which takes not only the documents, but also all the namespaces a document is shared to. Importing with this API can only do two types of operations, either we override existing documents or we make a true copy by regenerating their id (completely dissociating them from their source). These operations are simple enough, the difficult part is in how we handle the namespaces of the imported documents.
|
Linking back to my comment regarding a recent PR that fixed 3 import bugs and surfaced a new bug since it's related to the discussion here: #121046 (review)
|
removing my assignment as I'm no longer actively working on this one |
When users use the saved objects import/export UI or API they are able to export all objects from a specific space or import objects into a specific space, but the saved objects in the NDJSON file itself are stripped of all namespace information.
This causes several problems:
a
and importing it into spaceb
without changing theid
of the document.One way to remove "psuedo-copies" is to always generate new id's on import ([Breaking change] Drop support for copy/import conflicts ("pseudo copies") #81904). However, this will make it impossible for users to perform a backup restore with the import API. For instance if a user accidentally broke a dashboard they won't be able to re-import that dashboard to restore it to it's working state, instead the import will create a new dashboard.
The text was updated successfully, but these errors were encountered: