-
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
[Meta] Saved Object Tagging #74571
Comments
Pinging @elastic/kibana-platform (Team:Platform) |
@alexfrancoeur About tags and spaces: I wonder if we could simplify things here by having tags by default in all spaces |
@ruflin if you're up for an in person chat, I'd like to pick your brain about this over zoom. As a quick thought, I think we could possibly make a distinction between system generated saved objects and user generated saved objects and what their default experiences are. If we were to prioritize something like read only / system generated assets (#70461) then I can see the default experience to |
One thing that needs discussion is how we are going to handle sharing tags across spaces since there are a few unique attributes about tags that are different than other SO types. In the description above, we list these cons for space-specific tags:
The current share-to-space workflow has built-in handling for object collisions where two spaces contain objects with the same document ID. Depending on how we implement tags, we may have collisions on other fields than the For instance, if we use a schema like this (this is pseudo-code of sorts): interface Tag {
_id: string; // UUID
attributes: {
label: string; // user-visible tag label, should be unique per space
color: string; // the tag's color in the UI
};
namespaces: string[];
} interface TagAssignment {
_id: string; // UUID
attributes: {
tag_id: string; // UUID of the tag
object_id: string; // UUID of the object that is being tagged
};
references: string[]; // includes the two UUIDs above
} A few notes about this schema:
The share-to-space issue arises when a user does something like:
In this case, the default space's "prod" tag should also be shared since it is a referenced object. However, there is already a "prod" tag in the Marketing space. Because the current system only handles It seems to me we need to have explicit logic for this situation or else we are likely to introduce a confusing UX. For instance, if we didn't do anything here and simply shared the prod tag in the Marketing space (so now there are two "prod" tags in Marketing) the user would be presented with two tags of the same label in the tag assignment UI. I believe we're going to need a "merge tags" UI flow for when this situation is encountered. I believe the only decisions the user would need to make are:
It's also worth pointing out that tags are not the only SO type with a uniqueness constraint. Another one that comes to mind is Index Patterns. It is possible to have two index patterns in the same space with the same pattern, however it is quite confusing to the user and not functionally useful as far as I understand. Maybe the @elastic/kibana-app-arch can confirm here, but I suspect having a merge flow for Index Patterns would also be useful. If so, it may make sense to solve this problem more holistically by adding a new generic feature to SavedObjects for uniqueness constraints. |
This isn't quite accurate.
When I wrote users can run into “tag collisions” where similar tags are created in different spaces and shared, I didn't mean an ID collision. I meant a situation where there are two tags with different IDs that appear to be the same, because they have the same label. So, exactly what you described in more detail above 🙂
Currently, when you export an object, only its outbound references are included. I think that's behavior that we want to keep in general, though we could make a special exception for just this I did introduce new
Agree. Additional comments regarding your proposal: If we implement a separate |
Thanks for clarifying here. I believe this doesn't change what we need to do here, as Just to confirm, what does share-to-space do when it encounters an ID collision? Does it always generate a new ID?
Yep, we're on the same page here. My intention was simply to explain this situation more thoroughly for those not in the technical weeds 😄
I don't think this will ultimately be necessary and is probably a premature optimization. I was only trying to think through any other potential benefits of having a separate object but it sounds like this isn't one. I believe we can get around version conflicts with scripted updates or application-side logic for retries. I think the RBAC / feature control justification is good enough reason for separate objects. I'll update the parent comment with your info.
Definitely something we will need to consider. The easy way to solve this is to make tags a hidden type that can only be managed via the dedicated tags UI, however that has other downsides as well (not being included in the global SavedObjects UI & API, etc.). |
I agree!
Share-to-space doesn't check for collisions based on Copy-to-space and Import both have the top-level option to check for conflicts or to generate new IDs.
Unless I'm misunderstanding, I do think this will be necessary. When you 1. export/import, 2. copy, or 3. share a saved object, we want to include tags, according to the Flow Diagrams in the issue description. So for instance when you export a Dashboard, we'll need to:
|
Couldn't the saved-objects themselves have a direct reference to the tag saved-objects? For example, Dashboards would directly reference the tag saved-objects? |
That's probably a better option than what I suggested, though my understanding is that currently we don't have any such "circular references" so we'd need to make some small tweaks to export/copy/share to account for that. |
Would tags need to have an explicit reference to what they tag? We can always just query for which objects have a specific tag, when that's required... |
I believe this would either require that each object that wants tagging to add explicit support for it OR that we have a mapping field that applies to all SavedObjects, which has some licensing implications. Ideally, we build tagging in a way that can be turned on for any SavedObject with little to no work and also be disabled entirely if admins choose to do so.
I believe this would require more work to support the "export everything with this tag" use case which I think is something we would want. @alexfrancoeur do you imagine this use case being a requirement in the MVP or near future? |
I was initially thinking that tags would be just another saved-object, and use the normal saved-object references for this relationship. However, that potentially has repercussions with the referential integrity checking which will be implemented as part of Sharing saved-objects in multiple spaces.
After further thought, it would also potentially make implementing the following UI more complicated as well, since each tag explicitly denotes which saved-object it's attached to. |
I apologize, my response was confusing there. I meant that we do not need to optimize for the "version conflicts when there are simultaenous edits" problem I was talking about previously since there are other workarounds. For import/export we will definitely need to add support for including inbound-references. Either way we structure the schema (TagAssignment or including the tag references in the objects themselves), we will need to do the reverse lookup to support both export use cases (export all objects with tag X; and export object Y with all associated tags). |
As useful as a feature as I think this is, I believe that the primary goal for MVP should be organization.
Either approach will work, and at some point, I think this functionality will be necessary. We'll be fine tuning MVP requirements next week and can update this issue accordingly. |
'Quick' thoughts about the technical implementation options:I apologize in advance for the structuring and the ordering that may be a little chaotic, but there is a lot to talk about here. Lexicon:
Regarding the
|
const rootObjects = await fetchObjectsToExport({ | |
types, | |
objects, | |
search, | |
savedObjectsClient, | |
exportSizeLimit, | |
namespace, | |
}); | |
let exportedObjects: Array<SavedObject<unknown>> = []; | |
let missingReferences: SavedObjectsExportResultDetails['missingReferences'] = []; | |
if (includeReferencesDeep) { | |
const fetchResult = await fetchNestedDependencies(rootObjects, savedObjectsClient, namespace); | |
exportedObjects = sortObjects(fetchResult.objects); | |
missingReferences = fetchResult.missingRefs; | |
} else { | |
exportedObjects = sortObjects(rootObjects); | |
} |
With tag assignment model
- If we are using the
TagAssignment
join, we will need to add APIs to wrap the export logic. One option would be to move thefetchObjectsToExport
+fetchNestedDependencies
part of the export logic to the SO client public API. That way, we could use a tag SOC wrapper to add the additional assignments + tag objects to the list of exported object before streaming it to the client.
As discussed with @joshdover on slack, I feel like adding a 'include inbound references' option to the export may not be is not a good option, because:
- We only want inbound references that are
TagAssignment
(don't think this will even be used in any other use case) - Inbound references are not enough, as we also need to add the
Tag
(which is an outbound reference fromTagAssignment
). So we basically need 'inbound references with outbound references from them, but only one level deep please', which seem like a very specific need. This is why I would rather do something specific to Tagging types instead of trying to implements a generic solution for a single specific use case (but once again, I'm all ears for any counter argument)
With direct references model
- If the SO-Tag relation a direct relation via the SO's references, I think it would work without modifications (as long as 'include references deep' is enabled, the tags will also be exported)
Using the import API
In both cases, I think that as long as the export API is properly adapted, The import API should not need any modification at all, as the assignments and/or the tags will be present in the exported dump.
Tag / Assignment API design
CRUD on Tag
type
Probably no real problematic here. The SOT plugin will expose an API to perform crud operations on the Tag
type on the server-side, with probably a client-side counterpart. This API will just use the SO client under the hood.
Changing tag assignment on a SO
This is where the challenging part regarding the API design is. I see two main options here, integrating that in the SOC API, or having it as a totally dissociated one.
Dissociated API
Adding or removing tags would be done using an API exposed by the savedObjectTagging
plugin.
For example, when creating a new object, the consumer would call SOC.create
, then savedObjectTagging.addTags
.
Pros:
- Slightly better isolation between the core SO API and the SOT one.
Cons: - (way) worse developer experience
- consumers will have to manually check when the SOT is not available (oss) or disabled (licensed) to not perform the call
- the isolation is far from perfect, as there are still some use cases that will require to alter the SOR API. (For example
SOR.find
will still need to have a newtags
option defined)
The cons are significants here, which is why I think the other option is preferable:
Integration in the SOC/SOR API
This would integrate the SO tagging feature into core's SOR.
For example, when creating a new object, the consumer would just call SOC.create({ tags: myTags, ...otherOptions})
, and the repository would create the TagAssignment
(or add the references depending on the chosen implementation) when/after creating the object.
We will have the same challenges the spaces plugin did regarding integration into the SOR:
Technically, I was thinking that we could get most of the work done using a new client wrapper provided by the SOT plugin (please invalidate and/or comment on any of the following, this is one of the most impactful part of the design)
Some examples: (note: this is based on the TagAssignment
approach)
-
SOC.create
:- we add a
tags: string[]
to theSavedObjectsCreateOptions
- the SOR/SOC implementation of
create
actually don't care about this new option - the newly introduced SOT client wrapper would first delegate to the client to perform the default action (create the object), then, if successful, would create the
TagAssignment
to link the object to its tags.
- we add a
-
SOC.delete
-
the SOR/SOC implementation of
delete
actually don't care about this new option -
the SOT client wrapper would delegate to the client, then, on success, also delete the associated
TagAssignment
s
Important notes on delete
:
- The user performing the SO deletion does not necessarily have write permissions on the
TagAssignment
SO type, but we probably still want to delete the assignments during the deletion (as it is functionally just a join/relation) . Would the SOT client wrapper be able to act with higher privileges here to do that, and would is be acceptable in term of security? @elastic/kibana-security WDYT? - If the SOT plugin is disabled during the deletion of an object with tags, the assignments are not doing to be removed. I'm actually unsure of the importance of this point (seems similar but less impactful than what caused spaces to have the implementation in core's SOR instead of their wrapper)? If this is blocker, I guess we will be kinda forced to have this logic in the original SOR instead (which probably invalidates the SOT client wrapper on its whole, forcing to have the impl in the SOR instead, as
spaces
did)
We would also add APIs to add/remove tags from a SO to the SOR (addTags(so, tags)
and removeTags(so, tags)
), Similar to addToNamespaces
and deleteFromNamespaces
that were added to the SOR for the multi-namespace feature. Note that a notable difference with the namespaces feature is that, I think, we could just have the SOR/SOC implementation be a no-op, and have the SOT client wrapper handle this logic. This still means that using the SOR directly will not properly handle tags (but I think this is the same for the other wrappers?)
@legrego @jportner @kobelb You probably are the ones with the most experience regarding SO client wrapper. I would need your opinion on that ^
Pros:
- Better developer experience. This just 'feels right' imho
- If we can keep most of the logic in the SOT client wrapper, we still got a good isolation
Cons:
- Depends on the open questions.
- Anything else?
oss - xpack separation / isolation
This is also a tricky part. the tagging feature is going to be under a basic (or more) license, meaning that the actual implementation needs to be (as much as possible) in xpack. However, both the SO management section and all oss apps that are going to use tags needs to still access the feature from OSS APIs, one way or the other.
Server-side
Already discussed in previous section, Integration in the SOC/SOR API
. I don't think there is more to cover here.
Client-side
The most commonly used approach for this problematic is the 'shim'/'bridge' plugin.
We would have a soTaggingOss
plugin in OSS, that would be a facade of the actual soTagging
one (the xpack one). Consumers of the SO tagging feature from OSS would use this 'oss' plugin. It would also provide an API to know if the tagging feature is actually enabled or not (to show, or not, the tag selection in SO creation/edition pages for example).
The soTagging
plugin, when present, would call a registration API of the soTaggingOss
one to provide the actual implementation.
Regarding the UI components the plugin needs to expose to consumers (such as the tag selector one to be used for the various SO creation pages, or the tag representation), I'm unsure what the best approach is:
- The components should either be in OSS (and re-exported from the xpack one), and would requires to wire manually to the corresponding APIs what would be exposed on both plugins, or
- Preconfigured components could be exposed via both plugins contracts
@elastic/kibana-app-arch You are probably the ones with the most knowledge of that specific oss/xpack split subject, WDYT?
In short
I think I would go with the TagAssignment
approach and try to put as much as possible into the new SOT SO client wrapper, but I would really like to have everybody's opinion here.
Hey, gang. We've been working on design concepts for this issue, and I'd like to share our current progress. You may find links to our Figma mockups and clickable prototypes below. The prototypes are set up as a choose-your-own-adventure, where you can select the area of interest and see how the addition of tagging will impact that area of Kibana. Additionally, I've posted some screenshots that spotlight some of the key points of the experience. If you have any questions or feedback, don't hesitate to let me know (either here or via Figma comments). If anyone feels that we should get together for a quick meeting to review and discuss further, I'd be happy to throw something on the calendar. Thanks! Assign Tags & Create InlineCanvasDashboardDiscoverSaved SearchSaved QueryGraphMachine LearningMapsVisualizeStack ManagementTagsSaved Objects |
Thanks for your detailed analysis, @pgayvallet.
Do you mind elaborating on this? I get that we want a different subset of users to be able to manage tags vs actually tag a saved-object. Is there some other complication here?
Assuming we use saved-object references and the restrictions put in place by them, yup!
The biggest noted downside seems to be the delete operation. If we use a All of the other common queries seem easier to me with having the SO directly reference the tags.
I was originally thinking that if a user is able to edit a saved-object, it should be able to edit the tags assigned to the saved-object. For example, if you can edit a Dashboard you can add/remove tags. Does this allude to a requirement that I overlooked where we want a discrete privilege for whether or not a user can tag a saved-object?
If we're using the method where the SO references the tags directly, we can use the |
I don't think there is. I lack knowledge of how we actually manage non-so-based permissions, but.I thought that directly using SO permissions was the easier way. But you probably know way better than me on that subject, so please tell me what you think.
That's actually right. What I like the most about the assignment approach is the fact that we might be able to delegates most of the logic in the SOT SO client wrapper, which, I think, would be harder with the direct reference approach. But I might be wrong here, and would like more opinions on that specific part.
Ideally, I would like a better developer experience that having to find the id of the tag you want to search for. SO tagging is 'more' than just a plain relation between the object and its tag(s) and should have a better integration imho.
The conversion from |
Have more to add, but some initial thoughts: Regarding tag permissions, these two statements may be resolved by the same question:
What do we want the SO tagging permissions to look like?
From a UX perspective, (2) is probably the most straightfoward approach. I could see an argument for only wanting some users to have access to tags, but this seems like a contrived use case to me. @alexfrancoeur any thoughts? If (2) is deemed the appropriate UX, then I believe that makes the argument for TagAssignments much less potent. The remaining reason to do it that way is to be able to encapsulate most of the tagging functionality inside the SOT. This still feels like a good reason, but maybe not the best reason depending on what we're optimizing for.
We can probably craft the UX to make this always the case (+ server-side validation).
I believe we could circumvent this by using the internal user in this case. However, I do wonder if this could be seen as a 'permissions leak' that could confuse users. If we went this route, I think that at the very least, we'd need to show in the UI something like "this will remove tags from X other objects".
This sounds like a classic relational cascading delete problem. If we did support cascading deletes on SO references in general, that could solve this problem. But it faces the same permission leaks as above.
In the preconfigured case, would the OSS components just render nothing if the x-pack plugin is not enabled?
This doesn't feel like a blocker to me and there are plausible workarounds (detect orphaned tag assigments at startup and delete them). I'd also like to consider the option of not allowing this plugin to be disabled. It adds significant complexity on top of an already complicated feature. Instead, I think we could get away with exposing a config option to disable the UI for adding tags, but I don't think we need to support x-pack distributions completely disabling this plugin. Spaces could not do this since Spaces fundamentally changes many parts of Kibana. But I think tags are more isolated so that we could hide that they exist without turning off the fundamental logic that makes them work. |
Our SO relations are parent->child (the parent got the relation to its children when in a relational DB, it's usually the opposite, at least in 1-n joins). Cascade delete would work when deleting a dashboard, to delete the references viz for example. With the tag assignment model, the references to the SO and the tag obj on the assignments. I don't think cascade delete would help us when we delete the 'referenced' SO. (With direct references, this issue doesn't exist)
I was thinking a an API that exposes the whole optional API. The global idea would be: // API definition, type is in the oss plugin
type SavedObjectTaggingApi {
listTags(): Tag[];
getTagSelectionWidget({initialValue, onChange, ...}): TagSelectionWidget // React.FC<TagSelectionWidgetProps>
// [...]
}
// xpack plugin contract, re-using the definition type from the oss plugin.
type SavedObjectPluginStart = SavedObjectTaggingApi;
// oss contract
SavedObjectOssPluginStart {
isTaggingEnabled(): boolean;
// facade to the xpack API. is undefined if `isTaggingEnabled` returns false
getTaggingApi(): SavedObjectTaggingApi?
}
That would make sense to me that way consumers would just check if tagging is enabled before retrieving the API from the OSS bridge. |
I'll just add that, in my opinion, if this feature was meant for OSS, and given the pros and cons, we would have gone for the new But, once again, this is only my personal opinion. |
Even if tags need to abide by all of the rules and access patterns that references currently do? |
Correct me if I'm wrong, but as tags are still distinct objects, the security rules and access patterns would still apply anyway, won't they? I mean, accessing the tags associated with an object is still a distinct SOC call, so access control is already in place, isn't it? |
That's correct. However, the fact that a saved-object references another saved-object has implications on the ability to share saved-objects in multiple spaces per #27004:
|
To be honest, I'm not 100% sure of the expected behavior when sharing to space. I guess we'll want to also share the associated tags, as I don't see multi-spaces objects referencing single-space tags (even if technically, that is possible and would mean some tags would only be visible in some spaces of the object), but maybe @alexfrancoeur can confirm that. If that was the case though, I agree that it would be some additional change to the integrity check of the sharing features and is a con. However, as (if?) this integrity check is going to be done at the repository level, adding additional checks for tags seems not that impacting, as the logic is very similar (but I have to agree that it would work out of the box by using references) |
I'd expect the tags to be shared as well. We would run the risk of introducing seemingly duplicate tags within the space (different IDs, same name), but we that's hopefully something we can make clear in the tag management interface. We could try to be smart and assign to existing tags if they share the same name in the target space, but that might be even more confusing. Getting back to the question of adding I've gone back and forth on this so many times. I'm not conceptually opposed to adding a That said, it might be easier to see what this generalized solution looks like once we've proved out the tagging feature a bit more and solicited feedback. I worry about rushing into the generalized solution now to find out that we created the wrong abstractions, but on a now larger scale. If we're concerned about the additional class of bugs/complexity that come with re-using references, then we could start with adding the |
After a sync discussion during yesterday's platform team weekly with @elastic/kibana-platform, @kobelb @legrego and @alexfrancoeur, we came to the following conclusions:
Which is why we acted that:
|
Update of #79096: Tagging is now wired to Now for the bad news: after that, I spent some time looking on how to wire tagging to the update / create modals to allow selecting tags when creating a dashboard or a visualization. In short: I missed it in my initial lookup, but my fears about the The call chain is a pain to follow, but basically kibana/src/plugins/saved_objects/public/saved_object/helpers/save_saved_object.ts Lines 81 to 84 in 78e0bdf
kibana/src/plugins/saved_objects/public/saved_object/helpers/build_saved_object.ts Line 91 in 15c0644
kibana/src/plugins/saved_objects/public/saved_object/helpers/serialize_saved_object.ts Lines 23 to 27 in 40d2cfc
kibana/src/plugins/visualizations/public/saved_visualizations/_saved_vis.ts Lines 76 to 78 in 19bda1f
(Note that using a new Of course, both Adapt the base
|
Out of the options you've presented, I think I prefer the "Adapt the base SavedObject class to add tag support" option. I don't like that X-Pack concerns are leaking into OSS though. Building upon this option, do you think we could generalize this approach and add "saved-object decorations"? This way instead of having OSS explicitly aware of tags, they could be aware that there are various "saved-object decorations", and the only implementation of these at the moment is |
As discussed on slack, the decoration pattern seems like a viable way to improve SoC. It requires some preliminary work to allow the savedObjects plugin to pass down the this decorator registry to all |
@pgayvallet here is a quick take on validation errors for tag naming. Special charactersThe before and after versions are displayed below. The Max 50 charactersAs for the max characters, let's set a Preventing duplicatesIf we decide to show a warning or error for duplicates (presumably within the given Space), then an For reference, here are the EUI form validation docs: https://elastic.github.io/eui/#/forms/form-validation |
@alexfrancoeur, et al: Let me know if it would be desirable to have a slimmed-down, MVP-only set of mockups (rather than the full vision) for these initial development efforts. I'd be happy to throw it together. |
Thanks for the offer @MichaelMarcialis! After our discussion during the platform team sync I think we're in a good place. It's good to have the full vision for future work, I just think we needed to prioritize for MVP. |
@pgayvallet @ryankeairns @myasonik @MichaelMarcialis @joshdover I think we're good to close out this MVP, what do you think? We already have #81980 opened to address future requirements (most of which have been addressed). The only other feature that is mentioned in this MVP is share to space from tag management, which I think we've decided we'll keep in SO management for now. Maybe we can open an issue to track separately. |
Closing. Created
|
Summary
There are numerous deployments of the Elastic Stack across our community that have hundreds or thousands of saved objects with no good way to manage the content. This has been a longstanding request for quite some time, with past attempts that were put on hold. With the addition of Ingest Manager and Elastic Packages, it will become even more important to tag these system generated saved objects (#70461). With tagging, we aim to provide a simplistic way to organize, manage, import and export saved objects within Kibana. It’s possible that entities outside of saved objects would also require tags but for the MVP, we should aim to reduce scope as much as possible.
We will use this issue to track the requirements and status of the initial implementation of saved object tagging.
Feature requirements
Implementation Breakdown (WIP)
MVP
Future iterations
Questions
Flow diagrams
Screenshots / Prior art
All updated mockups can be found in this comment below: #74571 (comment)
There was some older work done on tagging from design and a recent space time project / POC (#71650, #16484 (comment)).
Personas and user stories
Impacted parties or dependencies
Assumptions and open questions
_meta
fields to unifying content across products. This type of standardization would enable "tagging" of Kibana saved objects with Elasticsearch assets like ingest pipelines, API keys, ILM/SLM policies, etc. If it makes sense for saved objects to adopt a similar approach, then we'll have to take tags into consideration as well.Spaces
There are two high-level approaches to tag management with regards to spaces. We are leaning towards tags being multi-[name]space.
cc: @arisonl @legrego @jportner @kobelb @ryankeairns @VijayDoshi @peterschretlen @AlonaNadler @joshdover @jinmu03 @mostlyjason @ruflin @rayafratkina
The text was updated successfully, but these errors were encountered: