Kibana: [Meta] Saved Object Tagging

[alexfrancoeur]

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 (https://github.com/elastic/kibana/issues/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

• Saved objects should be able to support one or multiple tags
• Tags are mutable - strings and colors can change, but the id of that tag is still persisted with the associated objects
• Tags should make it easy to group and sort on objects, whether that's in dashboards, Elastic packages with groupings of ML job configs, alerts, etc. or saved object management for quick export or copy to space
• Tags should be consistent across solutions and applications within Kibana with a reusable and backwards compatible service that developers can trust
• Tags should have a central area for management, with metadata, full CRUD capabilities and support for RBAC

Implementation Breakdown (WIP)

MVP

• [x] Base infrastucture - Platform
  
  
  
  • [x] SavedObjects schema
  
  
  • [x] Feature control integration
  
  
  • [x] Server-side APIs (if any)
  
  
• [ ] Tag Management UI - Platform
  
  
  
  • [x] Create a tag modal - Core UI
    
    
    
    
    
    • [x] Randomize color (copy / steal this from Spaces)
    
    
    
  
  
  • [x] Edit tag modal
  
  
  • [x] Table view
    
    
    
    
    
    • Includes: name, description, connections (tagged objects), shared spaces
    
    
    
  
  
  • [ ] Share & copy to space flyout
    
    
    
    
    
    • Not sure if this is going to be in 7.11, need to talk to Joe / Larry
    
    
    
    • Should be able to reuse existing UI with minimal changes
    
    
    
    • Defaults to including all tagged objects
    
    
    
  
  
• [x] Saved Objects UI - Platform
  
  
  
  • [x] Add tags column
  
  
  • [x] Filter by tags
  
  
• [x] Tag UI Service (components shared with plugins)
  
  
  
  • [x] Design Plugin APIs - Platform
  
  
  • [x] Assign a tag dropdown / autocomplete - Core UI
  
  
  • [x] Create a tag modal (reuse from above) - Core UI
  
  
• [x] Integrate with applications - Core UI
  
  
  
  • [x] Dashboard
  
  
  • [x] Visualize
    
    
    
    
    
    • Sync with app team on priority
    
    
    
  
  
  • For each we need:
    
    
    
    
    
    • Add tags to list view table (column + filtering)
    
    
    
    • Add tags to saving workflow
    
    
    
  
  
  • This will require that we expose APIs from OSS to X-Pack for enhancing with tags
  
  

Future iterations

• [ ] Merging tags
  
  
  
  • Not supporting this in the MVP means it will be possible to have two tags in the same Space with the same name
  
  
  • If we're not supporting copy & share to space in MVP, this is less of a problem.
  
  
• [ ] Exports
  
  
  
  • [ ] Export all objects with this tag
  
  
  • [ ] Include tag on exported objects (eg. I export a Dashboard and it includes its tags)
  
  
  • Depending on how we structure the schema & if/when we need to support both of these exports, we will also need to support including inbound references in exports. See https://github.com/elastic/kibana/issues/74571#issuecomment-692370096.
  
  
• [ ] Integrate with more applications
  
  
  
  • [ ] Canvas
  
  
  • [ ] Discover
  
  
  • [ ] Graph
  
  
  • [x] Maps - https://github.com/elastic/kibana/issues/82434
  
  
  • [ ] Machine Learning
    
    
    
    
    
    • [x] Depends on making ML jobs SavedObjects https://github.com/elastic/kibana/pull/77916
    
    
    
  
  
  • [ ] Alerts
    
    
    
    
    
    • esp import for pre-packaged alerts
    
    
    
    • they have an existing tag implementation
    
    
    
  
  
  • [ ] Solutions (cases, case comments have existing tags)
  
  
  • [ ] Scheduled reports (have existing tag feature)
  
  
  • [ ] Observability - saved views (Metrics, etc.)
  
  
  • [ ] Fleet Integrations (https://github.com/elastic/integrations/issues/327)
  
  
• [ ] Improvements to Copy-to-Space UI
  
  
  
  • [ ] Defer the conflict handling decision until after it occurs
  
  
• [ ] Global Search integration - https://github.com/elastic/kibana/issues/81846
  
  
  
  • [ ] Show an object's tags in the result list
  
  
  • [ ] Add tags to search results
  
  

Questions

• If we don't support export in MVP, will Ingest Manager be able to import tags as part of their packages? (Pierre)
• Look into feature controls UI for tag & tag assignment permissions (Josh)
• Check on bug where SO UI would export everything regardless of filters (Josh)

Flow diagrams

Screenshots / Prior art

All updated mockups can be found in this comment below: https://github.com/elastic/kibana/issues/74571#issuecomment-698521428

• Mockups
• Clickable prototype

There was some older work done on tagging from design and a recent space time project / POC (https://github.com/elastic/kibana/pull/71650, https://github.com/elastic/kibana/issues/16484#issuecomment-661866752).

Personas and user stories

Persona	User stories
Administrator	As an administrator I can create, edit, delete and filter on tags and control access to who can do the same
End user (full access)	As an end user, I can create, edit, delete and filter on tags
End user (write access for app, read only tags)	As an end user, I can apply pre-existing tags to saved objects I create and filter by them
End user (read only access)	As an end user, I can filter on pre-existing tags

Impacted parties or dependencies

Team / application name	Areas of impact
Kibana Analyze	This team will be affected most, their saved objects are in need of tagging. Saved searches, visualizations, dashboards, index patterns, saved queries, maps, canvas workpads, graph workspaces, and more.
Alerting	Alerting supports tags, but they’re just strings. We’d like to support adding alerts to Elastic packages and at the point we can, treating these as saved objects with tags makes sense
Ingest Manager	Ingest manager would like to use tags with their pre-packaged saved objects (#67258, #70461)
Machine Learning	Machine learning jobs have tags and will be converting to saved objects (#64172). Ideally, these would support tagging
Observability	Observability will be using alerts and pre-canned ML jobs, so we’ll need to support tagging here. As to whether or not other entities require tags, that’s probably out of scope for MVP
Security Solution	Security will be using “alerts” in their threat detection engine and pre-canned ML jobs, so we’ll need to support tagging here. As to whether or not other entities require tags, that’s probably out of scope for MVP

Assumptions and open questions

• The tagging MVP will be constrained to select saved objects
• RBAC needs to be discussed in more detail
• Ownership of implementation across other applications will need to be coordinated, planned and prioritized in cross-team roadmaps
• There is an open question around using Elasticsearch _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.

Tags are [name]space-agnostic; each tag that is created simultaneously exists in all spaces	Tags are multi-[name]space; each tag that is created exists in one or more spaces
Pros: simplifies basic operations for saved objects (share, import/export)	Pros: simplifies tag management (when an object is shared, the tag could be shared with it; “deleting” a tag in one space would have no effect on another space)
Cons: complicates tag management (e.g., admins in one space shouldn’t be able to delete a tag that is in use in another space), potential for information leakage when tags are created / doesn’t work with multi-tenant scenarios (e.g., one Kibana installation that has multiple tenants with their own space)	Cons: complicates end-user experience (users can run into “tag collisions” where similar tags are created in different spaces and shared; tags lose meaning if users can’t differentiate)

cc: @arisonl @legrego @jportner @kobelb @ryankeairns @VijayDoshi @peterschretlen @AlonaNadler @joshdover @jinmu03 @mostlyjason @ruflin @rayafratkina

[elasticmachine]

Pinging @elastic/kibana-platform (Team:Platform)

[ruflin]

@alexfrancoeur About tags and spaces: I wonder if we could simplify things here by having tags by default in all spaces * (upcoming feature, but no the same as space agnostic). If we have tags in spaces, it might become odd that there are space agnostic saved objects which have different tags inside different spaces? Happy to take this conversation to an other issue to not "pollute" this one here.

[alexfrancoeur]

@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 (https://github.com/elastic/kibana/issues/70461) then I can see the default experience to * spaces for read only tags shipped with packages. Given our security and saved object model today, it feels odd having user generated tags automatically be available in all spaces.

[joshdover]

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:

Cons: complicates end-user experience (users can run into “tag collisions” where similar tags are created in different spaces and shared; tags lose meaning if users can’t differentiate)

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 _id field which will create a confusing UX if we do not explicitly handle these collisions as well.

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:

• We don't use the tag label as the tag's _id so that tags can easily be renamed without having to update potentially 1000s of objects.
• The primary reason for two separate SO types is so that we can properly apply feature controls for the desired UX. For example, users with 'All' permissions to TagAssignments and only 'Read' permissions to Tags should be able to assign tags to new dashboards, but not have the ability to create new tags.

The share-to-space issue arises when a user does something like:

• In the default space, create a tag with the label "prod"
• In the Marketing space, create a tag with the label "prod" (this will be a different tag object with a different _id)
• User shares a dashboard tagged with the "prod" tag from the default space to the Marketing space.

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 _id collisions, this would not be detected in the current implementation and now the uniqueness constraint of tag labels within a space has been violated.

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:
1) Choose the color (only if the colors do not match between the conflicting tags); and
2) Confirm that this is what they want to do (maybe unnecessary?)

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.

[jportner]

The current share-to-space workflow has built-in handling for object collisions where two spaces contain objects with the same document ID.

This isn't quite accurate.

• _Copy-to-space_ and _import_ has built-in handling for object collisions for documents with the same id or originId (the latter of which is new concept introduced with the "Sharing saved objects" feature).
• _Share-to-space_ will not check for collisions based on a document's originId, and two multi-namespace (shareable) objects intrinsically cannot have the same id.

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 🙂

One thing I'm not sure of is if the Tag type would also need to include a reference to the TagAssignment type for import/export/share-to-space to work properly with references. If it does require bi-directional references like this, then this point is moot. @jportner do you have an answer on how this works?

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 TagAssignment object type.

I did introduce new rootSearchFields functionality to the saved object find operation that we should be able to use for this (e.g., _find all TagAssignment objects that have outbound references to Dashboard 'Foo'_). However, some exports may have tons of saved objects, and we'd need to run such an individual query for each one. Exports shouldn't be an extremely common occurrence, though, so it's probably OK if they are more expensive.

It seems to me we need to have explicit logic for this situation or else we are likely to introduce a confusing UX.
...
I believe we're going to need a "merge tags" UI flow for when this situation is encountered.

Agree.

---

Additional comments regarding your proposal:

If we implement a separate TagAssignment object, we will need to ensure we can always get rid of those when saved objects are deleted. As mentioned above, it also complicates document exports, and sharing will have the same issue.

[joshdover]

This isn't quite accurate.

* _Copy-to-space_ and _import_ has built-in handling for object collisions for documents with the same `id` or `originId` (the latter of which is new concept introduced with the "Sharing saved objects" feature).

* _Share-to-space_ will not check for collisions based on a document's `originId`, and two multi-namespace (shareable) objects intrinsically cannot have the same `id`.

Thanks for clarifying here. I believe this doesn't change what we need to do here, as originId won't be useful for this case. Please let me know if I'm wrong. Good information to have though.

Just to confirm, what does share-to-space do when it encounters an ID collision? Does it always generate a new ID?

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 🙂

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 😄

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 TagAssignment object type.

I did introduce new rootSearchFields functionality to the saved object find operation that we should be able to use for this (e.g., _find all TagAssignment objects that have outbound references to Dashboard 'Foo'_). However, some exports may have tons of saved objects, and we'd need to run such an individual query for each one. Exports shouldn't be an extremely common occurrence, though, so it's probably OK if they are more expensive.

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.

If we implement a separate TagAssignment object, we will need to ensure we can always get rid of those when saved objects are deleted. As mentioned above, it also complicates document exports, and sharing will have the same issue.

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

[jportner]

I believe this doesn't change what we need to do here, as originId won't be useful for this case. Please let me know if I'm wrong. Good information to have though.

I agree!

Just to confirm, what does share-to-space do when it encounters an ID collision? Does it always generate a new ID?

Share-to-space doesn't check for collisions based on originId. And multi-namespace saved objects _cannot_ share the same ID, so there can't be any collisions based on id.

Copy-to-space and Import both have the top-level option to check for conflicts or to generate new IDs.

I don't think this will ultimately be necessary and is probably a premature optimization.

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:

• Find all outbound references (direct and transitive)
• For each of those, find all inbound-referencing TagAssignment objects

[kobelb]

If we implement a separate TagAssignment object, we will need to ensure we can always get rid of those when saved objects are deleted. As mentioned above, it also complicates document exports, and sharing will have the same issue.

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

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?

[jportner]

Couldn't the saved-objects themselves have a direct reference to the tag saved-object? 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.

[kobelb]

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

[joshdover]

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?

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.

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

[kobelb]

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

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?

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.

[joshdover]

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:

• Find all outbound references (direct and transitive)
• For each of those, find all inbound-referencing TagAssignment objects

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

[alexfrancoeur]

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?

As useful as a feature as I think this is, I believe that the primary goal for MVP should be organization.

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

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.

[pgayvallet]

'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:

• SOC: SavedObjectClient
• SOR: SavedObjectRepository
• SOT: Saved Object Tagging Plugin

Regarding the TagAssignment approach versus direct references from SOs to Tags

I'm mixed on this one. using a n-n join table is definitely what I would be doing a relational database, but that's quite an uncommon pattern in a document-based DB (which, by itself, doesn't mean it is necessarily bad). Still, it seems to be the easiest way to plugin into our permissions system due to the tag vs assignment permissions.

Using TagAssignment as a 'join table' SO

My main concern if we go with two SO types, Tag and TagAssignment, is the interaction with spaces and permissions. Please correct the following statements if any, or all, are wrong:

• to work properly, a SO and its associated TagAssignment(s) should be in the same namespace(s). that means that all share/copy to space workflows would need to be adapted
  
  
  
  • when I add a tag to a SO from a space, I create an assignment that will be assigned to the current space (or to all the spaces the SO is accessible from?)
  
  
  • when I share a SO to another space(s), I will need to ALSO share the tag assignment (and the tag?) to the same space(s)
  
  
  • when I copy a SO to another space, I should also copy the tag assignment (and the tag?) to the other space
  
  
  • or should I copy the tag assignment and point it to the existing tag (that probably don't work in some case because of permissions, as the Tag would not be visible in the namespace the SO has been copied to)
  
  

Note that the issue about having to share/copy the Tag to the space(s) the SO is shared/copied to is also present in the other approach.

• if, instead, TagAssignment was to be namespace agnostic, we loose per-namespace feature control on the 'assign tag' action, which was, I believe, the main argument for this approach, so I think this is not an option. That also doesn't address the issue that the assignment, once copied or shared, might point to a Tag that is not visible/present in the destination space (unless tags are, also, namespace agnostic, which is probably not an option).

Using direct references from SO to Tag

This feels more natural to me, but has some downsides regarding feature control, and impacts of the main common queries we will be using when performing operations between SOs and their tags (see below for more details).

Regarding feature control, as we will not have the TagAssignment SO to use for tag assignment permission, we would have to manually manage that. Also, as these would be 'virtual' SO permissions, this would not be able to be handled under the hood by the security plugin, we will need to handle that in the tag SOC wrapper (that would use security for that).

Performing the most common queries / workflows

find SOs by criteria + tags

Even if that feature is not available in OSS, we will have to adapt the SOC.find API to have a additional tags option

SavedObjectClient.find({ 
  filter: 'some filter',
  ...someOtherArbitraryOptions,
  tags: ['tagId1', 'tagId2'] 
})

With tag assignment model

1. Fetch the results as it is currently done
2. Fetch all TagAssignment objects that are assigned to the fetched objects (could maybe be cached or something)
3. filter the results depending on their actual assignments to the requested tags

Note: 2. and 3. could be done in the SOT client wrapper, keeping the SOR implementation clean.

With direct references model

1. Adapt the SOR (getSearchDsl) to handle this new option

Note:

• as SO feature control would not be available with this approach, we probably also need to fetch the tags to see which one are available in current user context?
• as the implementation is in the SOR, searching by tag would work even if the SOT plugin is disabled. Not sure if this is a pro or a con

find / export all objects attached to given tag

With tag assignment model

1. fetch all assignment for given tag using SOR.find
2. fetch all SOs using SOR.bulkGet for the SO ids from the tag assignment
3. (for export, if includeReferencesDeep is enabled) fetch the references of the objects as already done

With direct references model

1. fetch all SO having a reference to the Tag SO

delete a tag by id

With tag assignment model

• delete the tag SO object
• delete all assignments SO objects referencing the deleted tag

Note: user would need write permission on BOTH the Tag and TagAssignment types to perform a tag deletion.

With direct references model

• delete the tag SO object
• remove the reference to the tag to all SO objects referencing it

Note: user would need write permission on the Tag type, and on ALL types that the tag has been applied to. Also, we will need to update all the objects referencing the tag. This seems to be a major con.

Using the export API

The export API is currently not extensible, as it's not directly exposed on the SO client. The SO wrapper enhancement pattern is not applied here (security and spaces wrapper are still used by the fact that the export scripts uses the SO client, but the 'which objects to export' is not extensible.

https://github.com/elastic/kibana/blob/cd86b81b82f3cf28edc47f56bab900bd43214d9c/src/core/server/saved_objects/export/get_sorted_objects_for_export.ts#L146-L163

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 the fetchObjectsToExport + 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 from TagAssignment). 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 new tags 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 the SavedObjectsCreateOptions
  • 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.

• 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 TagAssignments

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.

[MichaelMarcialis]

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.

• Mockups
• Clickable prototype

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 Inline

Canvas

Dashboard

Discover

Saved Search

Saved Query

Graph

Machine Learning

Maps

Visualize

Stack Management

Tags

Saved Objects

[kobelb]

Thanks for your detailed analysis, @pgayvallet.

Still, it seems to be the easiest way to plugin into our permissions system due to the tag vs assignment permissions.

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?

to work properly, a SO and its associated TagAssignment(s) should be in the same namespace(s). that means that all share/copy to space workflows would need to be adapted

Assuming we use saved-object references and the restrictions put in place by them, yup!

This feels more natural to me, but has some downsides regarding feature control, and impacts of the main common queries we will be using when performing operations between SOs and their tags (see below for more details).

The biggest noted downside seems to be the delete operation. If we use a TagAssignment saved-object, it'll be a _delete_by_query for the TagAssignment. Where-as if we have the SO reference the tag directly, it'll be a _update_by_query for the SO.

All of the other common queries seem easier to me with having the SO directly reference the tags.

Regarding feature control, as we will not have the TagAssignment SO to use for tag assignment permission, we would have to manually manage that. Also, as these would be 'virtual' SO permissions, this would not be able to be handled under the hood by the security plugin, we will need to handle that in the tag SOC wrapper (that would use security for that).

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?

Even if that feature is not available in OSS, we will have to adapt the SOC.find API to have a additional tags option

If we're using the method where the SO references the tags directly, we can use the hasReferences parameter to SavedObjectsClient::find

[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?

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.

All of the other common queries seem easier to me with having the SO directly reference the tags.

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.

If we're using the method where the SO references the tags directly, we can use the hasReferences parameter to SavedObjectsClient::find

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.

SOC.find({tag: 'my-tag-id'}) seems way better than SOC.find({hasReferences: [{type: 'tag', 'id': 'technicalId'}]})

The conversion from {tag: 'my-tag-id'} to {hasReferences: [{type: 'tag', 'id': 'technicalId'}]} could eventually be handled in the SOT client wrapper's find method though, leaving the SOR logic unchanged. It would still require to add the tag option to SavedObjectsFindOptions, which is in OSS.

[joshdover]

Have more to add, but some initial thoughts:

Regarding tag permissions, these two statements may be resolved by the same question:

if, instead, TagAssignment was to be namespace agnostic, we loose per-namespace feature control on the 'assign tag' action, which was, I believe, the main argument for this approach, so I think this is not an option

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?

What do we want the SO tagging permissions to look like?

1. Do we want admins to be able to grant access to assign tags for any object in a space?; or
2. Do we want users with write access for an SO type always to be able to assign tags to those objects?; or
3. Do we want both, that is, users must have write permissions on the object being tagged and write permissions for assigning tags in general?
   
   
   
   • This is the most flexible option, but could also be quite confusing.
   
   

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.

delete a tag by id

With tag assignment model

* delete the tag SO object

* delete all assignments SO objects referencing the deleted tag

Note: user would need write permission on BOTH the Tag and TagAssignment types to perform a tag deletion.

We can probably craft the UX to make this always the case (+ server-side validation).

With direct references model

* delete the tag SO object

* remove the reference to the tag to all SO objects referencing it

Note: user would need write permission on the Tag type, and on ALL types that the tag has been applied to. Also, we will need to update all the objects referencing the tag. This seems to be a major con.

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

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)

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.

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

In the preconfigured case, would the OSS components just render nothing if the x-pack plugin is not enabled?

* 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)

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.

[pgayvallet]

@joshdover

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

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)

In the preconfigured case, would the OSS components just render nothing if the x-pack plugin is not enabled?

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? 
}

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

That would make sense to me

that way consumers would just check if tagging is enabled before retrieving the API from the OSS bridge.

[pgayvallet]

Regarding tag assignments edition / creation when creating / updating SOs

Without changing the current SavedObject model

Reading / accessing the tags assigned to a SO

We would just add the proper API to the SOT server-side (and client-side) API to retrieve the tags from the object

SavedObjectTaggingStart {
  getAssignedTags(obj: SavedObject<unknown>): Tag[] // or id/type instead of SavedObject
  // ...
}

With direct references, SO returned by SOC.find or SOC.get (for example) would 'out of the box' have references pointing to the tags, so the method would just fetch the tags from the references.

With assignment model, the logic is not much harder, and need to fetch the assignments, and then the tags.

Note that most tag access operation from the UI would be done from the preconfigured UI component anyway, that would just requires the SO object as input (the tag selection widget, the tag 'pills'/'card' list widget...), and would use that under the hood.

Setting/updating tags to an object

With direct references, this would be done by simply updating the tag type references on the object before saving it. We could provide an utility API for that

SavedObjectTaggingStart {
  /* update the tags references on given object (without saving it) to given tagIds */
  updateTags(obj: SavedObject<unknown>, tagIds: string[]): Tag[] // or id/type instead of SavedObject
  // ...
}

Unsure of the need on the server-side (most of the tagging actions are from the UI atm). On the client-side, SOT widgets such as the TagSelectorDropDown could just do that under the hood by having a reference to the SO object (or a setter to its references field), which would allow to let the saving logic almost unchanged from the consumer side.

With the assignment model, it becomes harder, as there are no, well, direct references to the tag on the object. Without updating the SavedObject model to have a virtual tag field, we would be forced to perform two operations when creating an object from the UI:

• Save the saved object SOC.create/update
• Then, call the SOT API to set the tags on it.

This makes, imho, integration with existing apps SO creation/edition workflow harder, in addition of being a bad developer experience.

We could eventually add a saveObjectWithTags(so, tags) to the SOT client-side API, but there are various ways to save a SO from the client side(SO client API, SO loaders, custom endpoints), so I don't think we can go with something generic without causing changes on the consumers side.

By adding a tags: string[] 'virtual' field to SavedObject

We could add a new field to SOs that would be the list of the tag ids that are referencing this object.

• When retrieving a SO, the tags field would be populated with the correct ids.
• When saving a SO, the tags field would be used to create the references or the TagAssignments, depending on the chosen approach.

However, this introduce more complexity:

1. that requires to add a new tags 'virtual' field to the SO model, with all the underlying implications

2. there is the question of where will this logic be? In the SOR (bad isolation of an xpack feature in oss), in the SOT SO client wrapper (which would not be needed for the direct references approach)

Note that with the direct references approach, this solution can be misleading, as adding this field would results in duplication: the tags relations would be represented both in references and in tags. If for read operations, that shouldn't be an issue, write ones can be a little trickier: what would be the source of truth to update the object tag, as both fields represent them. Taking into account that this proposition is trying to solve an issue present only on the tag assignment model, I think we can just exclude this option if we are going with direct references.

Overall

If we go with direct references, we don't need to change the SavedObject model, and can probably achieve a correct dev exp by just adding the correct sugar APIs to the SOT plugin.

If we go with the tag assignment approach, I think we will need to implement the tag virtual field to SavedObject to avoid a tedious developer experience when manipulating tags on a object. However changing the SO model is never easy, as a lot of code is using it. We need to avoid exporting the tag field during export operations for example, make sure all consumers of the SO apis properly handles this field as 'not really directly persisted and so on, so it seems to be way more work and risks than the direct references option.

The more we discuss about it, the more I'm starting to think that direct references are way less impactful and way easier to implement for 90% of our use cases.

Remaining concerns about the direct reference approach:

• permissions for tag assignement

see second point of https://github.com/elastic/kibana/issues/74571#issuecomment-700325311

• messing with application owned SOs references

As discussed with @kobelb yesterday, one of my biggest concern regarding using references for tags is the fact that we will be adding new references to all taggable SO types.

I don't have enough knowledge of all our usages of references, but I scared of scenarios such as:

• Some code for a specific SO makes the assumption that all references are of type XXX, causing the introduction of new tag references to cause bugs or errors
• Some code for a specific SO makes the assumption that it can, in some update operations, replace the references field with a new array because all references were 'internal' to the update logic before the introduction of tags, causing the tags reference to be lost during such operation.

@elastic/kibana-app-arch @elastic/kibana-app (and anyone else with knowledge on SO references usage) Maybe you can help us here. Do you think these kind of scenarios could be present in our codebase, or are we just safe to introduce additional references for a functionality that is outside of the scope/logic of the SO type owner?

[alexfrancoeur]

Great thread and glad we're making some progress on the implementation details! This is exciting to see. Quick response to @joshdover ping below.

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?

From the descriptions you've listed out, I _believe_ (and may be wrong) my original line of thinking was more in line with (3). This is outlined in Personas and user stories in the above description. IE a user may have access to assign existing tags to saved objects, but not create new tags or edit existing ones.

I don't think we need separate permissions for assigning tags. With (2) are we thinking that by granting access for assigning tags, we'd also be granting full CRUD access to creating new and editing existing tags? If yes, I'm not saying we shouldn't take this approach, but want to think through it a bit more.

Separately, I also updated the description with some of the latest mocks and links.

[kobelb]

I believe that option (2) that @joshdover presented is compatible with the specified personas and user stories.

In my opinion, we should add a single new feature called "Tag Management".

When the user is granted the "All" privilege for tag management, they can create, edit, delete and filter on tags. These operations will be available from both the dedicated tag-management screens, and the inline saved-object tagging.

When the user is granted the "Read" privilege for tag management, they'll be able to access the dedicated tag-management screen in a read-only fashion.

When the user is granted the "All" privilege to features that support tagging, they will be able to tag those saved-objects with the existing tags. For example, if the user has "All" access to Dashboards, they will be able to tag Dashboards.

Personas, user stories and privileges

Persona	User stories	Privileges
Administrator	As an administrator I can create, edit, delete and filter on tags and control access to who can do the same	manage_security cluster privilege, Tag Management "All"
End user (full access)	As an end user, I can create, edit, delete and filter on tags	Tag Management "All"
End user (write access for app, read only tags)	As an end user, I can apply pre-existing tags to saved objects I create and filter by them	Dashboard, Discover, etc. "All", Tag Management "Read" (optional)
End user (read only access)	As an end user, I can filter on pre-existing tags	Dashboard, Discover, etc. "Read", Tag Management "Read" (optional)

[pgayvallet]

Note: could we use the assign term for the action of assigning (or removing assignments) of tags on objects? (as opposite to create and delete which are the actions of creating and deleting tag objects)

@kobelb's proposition should greatly ease the privileges implementation.

I think there is a mistake on your third persona though, you stated Tag Management "Read" (optional) to As an end user, I can apply pre-existing tags to saved objects I create. The Tag Management "Read" privilege would be mandatory for that part, right?

The only thing I would like to be sure here is: Do we want to be able to fully disable the tagging feature for some users (not being able to filter on tags, not being able to see tags assigned to objects I have read privilege on by applying None to the tagging feature, right?

Because in that case, we wouldn't be able to have distinct privilege for users allowed to filter/see tags, and users allowed to filter/see/assign tags?

Persona	User stories	Privileges
End user (full access)	As an end user, I can create, edit, delete and filter on tags	Tag Management "All"
End user (write access for app, read only tags)	As an end user, I can assign pre-existing tags to saved objects I create and filter by them	Dashboard, Discover, etc. "All", Tag Management "Read"
End user (read only access)	As an end user, I can see assigned tags and filter on pre-existing tags, but not assign tags to saved objects I create.	Dashboard, Discover, etc. "Read", Tag Management **???**
End user (read only access)	As an end user, I don't have permission to see tags assigned to the objects/features I have read privilege for, and I don't have permission to filter these objects by tag	Dashboard, Discover, etc. "Read", Tag Management "None" (?)

or would Tag Management "None" still allow to see assigned tags and filter by them? (that could make sense, but just want to be sure our '3 states' privilege system is alright here.)

As an example, I created the feature (as I understand it) in my POC, it would look like:

export const savedObjectsTaggingFeature: KibanaFeatureConfig = {
  id: 'savedObjectsTagging',
  name: i18n.translate('xpack.savedObjectsTagging.feature.featureName', {
    defaultMessage: 'Tag Management',
  }),
  category: DEFAULT_APP_CATEGORIES.management,
  order: 1800,
  icon: 'savedObjectsApp',
  app: [],
  management: {
    kibana: ['tags'],
  },
  privileges: {
    all: {
      savedObject: {
        all: ['tag'],
        read: [],
      },
      api: [],
      management: {
        kibana: ['tags'],
      },
      ui: ['view', 'create', 'delete', 'assign'],
    },
    read: {
      savedObject: {
        all: [],
        read: ['tag'],
      },
      management: {
        kibana: ['tags'],
      },
      api: [],
      ui: ['view', 'assign'],
    },
  },
};

[kobelb]

I think there is a mistake on your third persona though, you stated Tag Management "Read" (optional) to As an end user, I can apply pre-existing tags to saved objects I create. The Tag Management "Read" privilege would be mandatory for that part, right?

Not a mistake. This is following the precedent that we've done elsewhere with features like "Index Pattern Management". When the end-user gets "All"/"Read" access to the "Index Pattern Management" feature, they're able to use the "Index Pattern Management UI". However, users who only have access to Visualizations are able to enumerate all of the existing index-patterns to create their Visualizations.

The only thing I would like to be sure here is: Do we want to be able to fully disable the tagging feature for some users (not being able to filter on tags, not being able to see tags assigned to objects I have read privilege on by applying None to the tagging feature, right?

If we want this, the approach that I recommended is incompatible.

or would Tag Management "None" still allow to see assigned tags and filter by them? (that could make sense, but just want to be sure our '3 states' privilege system is alright here.)

Yup! This requires us to add the ability to read tag saved-objects to the other features which inherently allow end-users to consume tags.

[pgayvallet]

Not a mistake. This is following the precedent that we've done elsewhere with features like "Index Pattern Management". When the end-user gets "All"/"Read" access to the "Index Pattern Management" feature, they're able to use the "Index Pattern Management UI". However, users who only have access to Visualizations are able to enumerate all of the existing index-patterns to create their Visualizations.

@kobelb Great, if there is already precedence on this, then it's perfect

@alexfrancoeur Are you in good with that? (that matches the persona in https://github.com/elastic/kibana/issues/74571#issuecomment-701019279)

| | FULL | READ | NONE |
| --- | :---: | :---: | :---: |
| create tags | ✅ | :x: | :x: |
| delete tags | ✅ | :x: | :x: |
| edit existing tags | ✅ | :x: | :x: |
| access the tag management section in edition mode | ✅ | :x: | :x: |
| access the tag management section in read-only mode | N/A | ✅ | :x: |
| assign tags to objects I have write access to | ✅ | ✅ | :x: |
| see tags on objects I have read access to | ✅ | ✅ | ✅ |
| use tags to filter objects I have read access to | ✅ | ✅ | ✅ |

[alexfrancoeur]

I just thread through the back and forth on RBAC and the proposal from @kobelb @joshdover and @pgayvallet looks good to me. It looks like this supports the original requirements.

The only thing I would like to be sure here is: Do we want to be able to fully disable the tagging feature for some users (not being able to filter on tags, not being able to see tags assigned to objects I have read privilege on by applying None to the tagging feature, right?

I don't believe this is necessary

[pgayvallet]

So, I just opened a quick draft PR (it currently only adds the tags section to the management app), by trying to implement the direct references approach.

The first blocker I see is on how to implements the delete tag action.

In short:

https://github.com/elastic/kibana/blob/7c560fa936fb8c7954730d3c2a901b4f34dfa12d/x-pack/plugins/saved_objects_tagging/server/tags/tags_client.ts#L52-L56

Here, I would need to perform an _update_by_query operation on all saved objects referencing the Tag object. to remove the said reference.

I could use a SOR.find to find using hasReference followed by a SOR.bulkUpdate operation instead, but it got the limitations of theses operations in term of scaling, and is probably worse in term of performances than a _update_by_query. The only upside I can find (apart from working out of the box), is that we wouldn't use painless script to perform the update on the objects' references (but as other APIs such as deleteByNamespace already uses it, I guess it's not a real problem).

If we want to use _update_by_query, we'll have to add a public APIs to do so on the SO repository and client.
Taking into account that for my specific use case, I will need to use a painless script (seems there is no other way to remove an element from an array), I'm unsure how we could expose an API to do that without allowing other SOR consumers to do dangerous things (I'm pretty sure we want to avoid allowing to pass a script to any SO operation, right?

One option I see would be to add a deleteReferencesTo(type, id, options) API to the SOR. That would address my use case, while leaving the actual update_by_query operation internal to the SOR (also, it's generic enough to be added to the SOR/OSS without having to find an 'excuse' to do so)

@kobelb @legrego @joshdover what do you think about that? Do you see any other options?

[kobelb]

One option I see would be to add a deleteReferencesTo(type, id, options) API to the SOR. That would address my use case, while leaving the actual update_by_query operation internal to the SOR (also, it's generic enough to be added to the SOR/OSS without having to find an 'excuse' to do so)

I like this approach. It's similar to what we did with deleteByNamespace. Long-term, it'd be great for us to find a way to allow consumers of the SavedObjectsClient and SavedObjectsRepository to use the equivalent of _update_by_query and _delete_by_query while not invalidating our business rules and authorization model, but that feels incredibly complicated to me.

[legrego]

One option I see would be to add a deleteReferencesTo(type, id, options) API to the SOR. That would address my use case, while leaving the actual update_by_query operation internal to the SOR (also, it's generic enough to be added to the SOR/OSS without having to find an 'excuse' to do so)

I like this approach. It's similar to what we did with deleteByNamespace. Long-term, it'd be great for us to find a way to allow consumers of the SavedObjectsClient and SavedObjectsRepository to use the equivalent of _update_by_query and _delete_by_query while not invalidating our business rules and authorization model, but that feels incredibly complicated to me.

I'm also +1 on this approach, for all the reasons Brandon mentioned.

[MichaelMarcialis]

Hey, gang. Below is a link to a quick video update regarding design changes from the last round of feedback. These changes include:

• Default to having the "include relatives" and "include tags" checkboxes checked during the export/share/copy saved objects flows.
• Addition of a new tag assignment action for bulk saved objects.
• Moving the link to "manage tags" out of the tags EuiComboBox component for better accessibility.

Have a look when you get a moment and let me know if there are any questions or concerns.

https://www.loom.com/share/8be4b450f603414ab0d645e9c72c3303

[pgayvallet]

These changes look very nice, and the SO management page is way better that way ihmo.

Just want to point out that some changes seems out of scope of just the tagging feature, in particular the fact that many batch actions are added, such a batch sharing to space or batch copy to space, which is (I think?) something that is not even supported atm. The feature being already quite massive in term of impacts on the codebase, I would really like to know the separation between the UI changes directly impacting the tagging features from the ones being on a higher level UI improvement / cleanup pass (I have the same remark for the mockups that change some app's objects creation workflows from the current modals to flyout for example)

Another point, that is related to the technical implementation: In the export modal, include all relatives and include all tags are two distinct options. If that definitely make sense on a functional point of view, it asks a few question on the technical level:

• it will force to dissociate the tag type references from the others when exporting. The import/export being in OSS, that will force to have a few if ref.type === 'tag'-ish logic splits in OSS code for a feature being in xpack. It will also be the same if we want to dissociate tags from relations when copying or sharing to space.
• When exporting by selecting include relatives but not include tags, should we remove the references to tags from the exported objects?

[pgayvallet]

Status update

So, I'm progressing a bit on my initial implementation in https://github.com/elastic/kibana/pull/79096. We now have the administration section for tags, and are able to create / edit / delete / list tags, with those actions wired to your privilege system. I also developed the (initial version of the) <TagList> and <TagComboBox> components to allow displaying the tags of a object, and editing them.

Next steps

The next steps are:

• Wire the tagging display in the SO management section (just display the assigned tags in the SO table for now)
• Plug the tagging system to a sample app (I'm going with dashboard for that), to
  
  
  
  • display tags on the dashboard table
  
  
  • filter by tag on the dashboard table
  
  
  • allow to select tags when creating a new dashboard
  
  
  • allow to edit tags when saving an existing dashboard
  
  

It will be easier to start with the second point, to avoid having to manually inject tag assignments to our objects during manual testing.

This is were the tricky part begins though, as we will need to decide on the actual SO-Tag relation implementation to progress, which leads to:

Pending questions

I spent a lot of time thinking about our SO->Tag relation implementation, and I came to the following conclusions:

First, I have to say that the direct reference approach between SOs and tags is definitely the way to go. It was already heavily discussed in the current issue, But after pseudo-coding all the queries / api changes we need to perform to plug the tagging to existing pages / objects, It's way easier in almost all situation / queries, and don't have that many drawbacks compared to the assignment join/table/so.

Now, after looking at the changes we'll need to make for all the functionalities we want to implement, I'm really starting to question the decision of using the references array to represent the relation between an object and its tags, and I'm thinking of the option to add an additional tags field to the SO root/default mapping instead.

My reasoning is the following:

Functionally, we are not handling tags as the other relations

Most (if not all) of our functionalities dissociates tags from other referenced objects. For example, for all export / share operations (export, share to space, copy to space), we want distinct options to include the references and/or the tags of the object. Functionally, a tag is NOT a referenced / relative object of a given SO.

For instance, for the 'export' action, we will have two boolean flags: includeReferencesDeep (existing) and includeTags (new).

With tags as plain references, we would have to adapt the export script (that is in core/OSS) anyway, to dissociate the tags references from the others to gather what to export. As we will have to perform changes in the export mechanism anyway, it is probably just better to just have distinct fields for references and for tags. This also feels way more intuitive and natural when thinking about the implementation.

The same goes for all other actions, such as copy to space, that will need the same distinct options for references and tags.

We would have to perform changes in the SOR anyway

Even if tags were normal relations, we would have needed to perform some changes in the SOR.

First example coming to mind: To filter SOs by tag in the UI, I need to add a way for the find API to filter results by tags, as most, if not all, tables displaying SOs are directly using the _find API (and if not done directly on the client-side, the server-side is performing a SOC.find call anyway.

Of course, we could filter using hasRelation: myTag. However all our tables are using EUI Query language, meaning that when we'll be adding the tag filter on the searchbar, the query would look like (tag:my-tag-id) my term. To have just done the exercise with the dashboard table a few hours ago, converting that, on the client side, to hasRelation: [type: 'tag', id: 'my-tag-id'] is tedious, impacts applications code a lot (and in some cases I mean a LOT), and overall feels like a terrible developer experience. This is why I would have been adding a tag?: string | string[] parameter to the _find endpoint ad the associated SOR.find API. The SOR.find implementation (well, or the SOT client wrapper) would then have converted that option to hasRelation: [{type: 'tag', id}]

This is only an example. The create / edit workflows where the tags combobox must do tricks to load its initial list of selected tags from the object's relations, then, when the selection changes, updates the object's relations while preserving the ones that were not tags, is also quite horrible and feels plain wrong.

My point is, and taking into account that the tagging feature would have, anyway, impacted the SOR API, a lot of workflows are way easier and intuitive with a specific tag field to the SavedObject model and APIs.

Relations are supposed to reference objects that are used in the SO's attributes/data

Not the most significant argument, but the way references were designed was to point to objects that are directly used in a given SO's data/attributes. You just have to check the injectReferences and extractReferences methods from the SO loaders to confirm that. Using references to point to objects that are not directly related, such as tags, feels like, in a technical design point of view, borderline.

It feels safer for existing consumers of the SO apis

As already stated at the bottom of https://github.com/elastic/kibana/issues/74571#issuecomment-700513221, I'm a little scared that references manipulations from SO consumers might result in overriding / erasing the tag references. By using a specific field to store the tags, this risk just goes away.

Even for an update operation, as our SO update is actually a partial update, omitting, for any reason, to pass the existing tags when updating a SO will just keep the tags fields unchanged during the update.

Our implementation should not be impacted by licensing decisions

The main technical challenges in implementing the tagging system are caused by the fact that the feature is behind a license. That forces us to move as much as possible of the code/implementation to the xpack savedObjectTagging plugin. However, for the things that can't, technically, be in this plugin (or not without heavy, long and dangerous refactoring to the whole SO internal API), I think that we shouldn't do risky compromises just because of an idealist vision of the isolation between oss and licensed code. I'll add that we already have a precedence of that exact situation with spaces, and the namespace / namespaces fields that were already added to the SO base mapping (in addition to the associated spaces-specific APIs added to the SOR/SOC)

Adding this field doesn't mean the functionality is enabled for OSS

Note that the references approach has the exact same characteristics here, so that don't change anything, but just by the record:

By adding the tags field to the base SavedObject model, we are basically allowing OSS users to perform these operations using the HTTP API ONLY, not use UI:

• search SOs by tag
• add tags references to SOs

That's it. All the UI functionality will still be provided by the savedObjectTagging plugin that is licensed and in xpack, including:

• create tags (the Tag SO type is NOT available in OSS, meaning that the whole functionality is just a no-op in oss distribs)
• seeing tags associated with objects
• filter by tag on the UI
• pretty much everything specified in this issue

Downsides

Not solution is perfect, and this one comes with a few downsides too:

• We will need to add a tag-specific API to the SOR, removeTagReferencesTo. this one will replace the removeReferencesTo API I was planning to implement, as I need a way to perform an update_by_query operation on all SOs referencing a tag when I delete the tag to remove the tag 'reference' from the tags field of all objects that were assigned to this tag. Once again, we have precedence of this scenario with the multi-spaces feature.
• We need to add the new tags field to SavedObjectsUpdateOptions, SavedObjectsCreateOptions and the equivalent bulk operations, and implement the associated handling in the update / create (and bulks) methods. Same precedence.

So, what do you think?

I know I'm writing long and hard to read text blocks, and I apologize for that 😅. However this feature is really impactful and a lot of part of the Kibana codebase, and I really want to go with the correct design on the first short to avoid the risk to recode and breaks everything later.

@joshdover @kobelb @legrego Does that proposition feels acceptable and make sense to you? Do you see any (blocking or not) downsides I didn't think about?

[joshdover]

Functionally, we are not handling tags as the other relations

As we've iterated on the designs and talked through many of the UX considerations, this has become more and more clear. I agree that at this point there will be enough 'special cases' for tags in our existing flows that trying to shove it into the same model doesn't necessarily save us much work.

We would have to perform changes in the SOR anyway

👍

By adding the tags field to the base SavedObject model, we are basically allowing OSS users to perform these operations using the HTTP API ONLY, not use UI:

Would it be practical to have the OSS implementations be no-ops and have the real implementation provided by an SOC wrapper registered by the tagging plugin? Or does this have to be at the repository level for some reason? If not, I think this is fine, but just wanted to ask in case it was.

Slightly related, we should be sure that the HTTP API does not erase tag assignments if existing REST integrations do not provide a tags array on update calls.

We will need to add a tag-specific API to the SOR

We definitely have precedent here with Spaces, as mentioned. My long-term concern with this approach is that every new signficant SO feature will require changes at every layer of the service. I think at some point this will be very hard to maintain. While I don't think we are there yet, I can see some features on the horizon that may run into similar issues (#17888 comes to mind).

That said, a larger re-architecture may be needed if and when we start to run into problems scaling development of the SO service. It may be impossible to solve this problem without a larger refactoring anyways, so moving forward with this approach right now makes sense.

Does that proposition feels acceptable and make sense to you? Do you see any (blocking or not) downsides I didn't think about?

I'm having a hard time thinking of any signficant downsides here. The fact that the product requirements dictate that tags need be handled differently invalidates a lot of the benefits of the other approach & simplifies many of the tag-specific logic we'll need to do.

[kobelb]

I feel like I'm missing something. In what situations do we want tags to behave differently than our existing references? I get that the UI mockups have them being treated differently, but conceptually they seem to abide by the rules of our existing references in every situation I've thought of. Do we really think that tags are conceptually distinct enough that they should be a one-off, as opposed to a reason to build some generalized solution?

Functionally, we are not handling tags as the other relations

Based on the current UI mockups, I agree. If that wasn't the case, they seem functionally equivalent to me.

We would have to perform changes in the SOR anyway

You can find saved-objects by their tags with the existing implementation of SavedObjectsClient#find. It's somewhat clumsy, sure, but it's possible. If translating the "EUI Query" into the call to _find is impossible to address within EUI, we can add a dedicated API endpoint to do this translation.

Relations are supposed to reference objects that are used in the SO's attributes/data

Existing references behave this way, I don't see why this couldn't be adjusted though.

It feels safer for existing consumers of the SO apis

We are definitely changing expectations, so this is definitely a risk with existing code.

Our implementation should not be impacted by licensing decisions

Agreed with the general premise. However, if we can build some generalized solution that is useful elsewhere, there are benefits to allowing tags to be implemented within a plugin. The concept of namespace(s) made its way into OSS because we wanted to support users who downgraded from Basic to OSS. This was the last resort after we exhausted every other possible method of implementing this without causing a huge maintenance burden. It's a precedent, but I think it's a high-bar before we should consider this.

[alexfrancoeur]

feel like I'm missing something. In what situations do we want tags to behave differently than our existing references? I get that the UI mockups have them being treated differently, but conceptually they seem to abide by the rules of our existing references in every situation I've thought of.

Does it make sense to touch base on what parts of the UI mockups are causing us think that tags are being handled different? A fair bit of the mockups themselves feel future facing and not necessarily mapped to the MVP requirements defined above. I'm happy to bring clarity to priorities if needed. Maybe we can use the platform team sync tomorrow to discuss?

[kobelb]

Maybe we can use the platform team sync tomorrow to discuss?

👍

[pgayvallet]

@joshdover

Would it be practical to have the OSS implementations be no-ops and have the real implementation provided by an SOC wrapper registered by the tagging plugin?

For some actions such as export, share to space and so on, this would just not be possible at all, as the logic of these actions in not inside the SOC. Moving export to the SOC could be possible, however for actions that are not in OSS (such as share to space), I don't think there is an obvious solution.

For the SO find API, if we go with the new tags field, we can't have the SOR implementation be a no-op, as the SO search DSL needs to be able to generate the query for this field. If we go with the references approach instead, we could have the 'tag to reference' conversion in the client wrapper, however we would still need to add this tags option to the client/repository API, so the gain feels minimal.

My long-term concern with this approach is that every new significant SO feature will require changes at every layer of the service. I think at some point this will be very hard to maintain.

Overall, I agree. But this is a very complex subject, that would require a whole technical re-design to solve. If I do think this is something that we'll need to do, I don't see us doing it short, or even mid, term.

@kobelb

I feel like I'm missing something. In what situations do we want tags to behave differently than our existing references?

Well, In pretty much every functional use cases I enumerated, tags should not be considered as 'references', and are functionally distinct. All batch operations on SO now have two options: include relative objects and/or include tags.

Technically, they are also different by the fact that they are not referencing anything concrete regarding the referencing object's data or behavior. References were conceived (well, at least from what I understand from the code) to indicate that some arbitrary data inside an object's attributes is using another object, as mentioned in the part about what injectReferences and extractReferences does in a client-side SO loader

To return the question: In what situation do you see tags behaving similarly to our existing references?

Based on the current UI mockups, I agree. If that wasn't the case, they seem functionally equivalent to me.

These mockups are driving the functional need though. You could rephrase this sentence by 'based on the current functional needs'.

It's somewhat clumsy, sure, but it's possible. If translating the "EUI Query" into the call to _find is impossible to address within EUI, we can add a dedicated API endpoint to do this translation.

It is possible. However by doing that, we'll be adding a lot of complexity to all client-side consumers of the _find API that will need to perform additional(s) call(s) before calling _find. I don't say it's technically blocking, but it would be way harder to implement that way.

[kobelb]

Technically, they are also different by the fact that they are not referencing anything concrete regarding the referencing object's data or behavior. References were conceived (well, at least from what I understand from the code) to indicate that some arbitrary data inside an object's attributes is using another object, as mentioned in the part about what injectReferences and extractReferences does in a client-side SO loader

Agreed. In my mind, this is the largest difference. Part of me wonders whether or not we should just be changing the way that references are modeled, and remove the necessity to call injectReferences and extractReferences.

If we weren't worried about the extensible nature of this, I'd suggest creating a DashboardSavedObject class which would have methods like addEmbeddable and getEmbeddables that internally maintained the list of references. However, that approach wouldn't play nicely with tags being implemented by a plugin. However, we can still embrace object-oriented design and no longer treat saved-objects as simply a data-structure, and add methods that hide the internal state.

[joshdover]

I feel like I'm missing something. In what situations do we want tags to behave differently than our existing references?

I think we're likely to introduce bugs like "I saved the Viz and my tags on it disappeared" if we use the same references array that is currently used by SOs. The extractReferences functions today would not be able to actually extract the tags from the object's attributes. We could adapt these to take the existing SO (if not new) as part of extracting references so that we could preserve any references that start with tag:.

Given the complexity of our visualization & dashboard architecture, this could be non-trivial to do, but I'm not actually sure of that. I also don't think that "its a lot of work" is necessarily a good reason if it seems feasible to accomplish and provides a simpler architecture.

That said, I'm not sure what benefits we'd be getting out of using the regular references field if the behavior needs to be essentially different in most cases.

The concept of namespace(s) made its way into OSS because we wanted to support users who downgraded from Basic to OSS. This was the last resort after we exhausted every other possible method of implementing this without causing a huge maintenance burden. It's a precedent, but I think it's a high-bar before we should consider this.

This is good context to know. I hadn't thought through this particular usage pattern yet. I _believe_ either approach would still be compatible with downgrading to OSS, though you may lose tags after saving objects.

Agreed. In my mind, this is the largest difference. Part of me wonders whether or not we should just be changing the way that references are modeled, and remove the necessity to call injectReferences and extractReferences.

I'm not sure how we would do this without changing a lot of application code. Doesn't mean we shouldn't do it, but I think we should probably be discussing the ideal architecture vs the most practical one, what the differences are, and how much longer the ideal arch would take to build.

One caveat I see with a dedicated tags field is that any referential integrity checks that we add in the future will need to be duplicated or adapted somehow for tags. The real question though is how hard would it be to duplicate this (and other reference-specific business logic)? From @pgayvallet's analysis it seems that the effort for the service changes needed to support these reference-specific features on the tags field is less than the effort required by plugin developers to integrate with them. That _seems_ like a good trade-off to me.

[kobelb]

I'm not sure how we would do this without changing a lot of application code. Doesn't mean we shouldn't do it, but I think we should probably be discussing the ideal architecture vs the most practical one, what the differences are, and how much longer the ideal arch would take to build.

Agreed. When talking to @pgayvallet earlier, the concerns with using references in this manner was largely unsubstantiated and hypothetical. Has your additional work on this feature revealed true complications with using references in this manner that are pervasive within the consuming application code?

One caveat I see with a dedicated tags field is that any referential integrity checks that we add in the future will need to be duplicated or adapted somehow for tags. The real question though is how hard would it be to duplicate this (and other reference-specific business logic)? From @pgayvallet's analysis it seems that the effort for the service changes needed to support these reference-specific features on the tags field is less than the effort required by plugin developers to integrate with them. That seems like a good trade-off to me.

@jportner, you've recently worked on the import/export logic recently. I think this is one of the larger areas where references have a rather large impact. Do you have an opinion on this matter?

[pgayvallet]

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 tag field without questions. That was I meant by the 'Our implementation should not be impacted by licensing decisions`. I know having this feature in x-pack has technical implications, and that ideally, we would have a way to extend any OSS API arbitrary, but that is unfortunately not the case, at least right now, for a lot of reasons (including the usage of typescript, AFAIK it would be easier with plain JS regarding api/options extension). Taking that into account, and excluding the option of just rewrite all from scratch (at least now) I feel like the best option in term of API definition, integration in the existing codebase, developer and user experience, is to just go as we would have been if the feature was in OSS.

But, once again, this is only my personal opinion.

[kobelb]

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 tag field without questions.

Even if tags need to abide by all of the rules and access patterns that references currently do?

[pgayvallet]

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?

[kobelb]

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 https://github.com/elastic/kibana/issues/27004:

Before a saved-object can be shared to a space, all direct and transitive references to saved-objects must be shared. For example, dashboards have references to visualizations, which have a reference to an index-pattern. When a dashboard is shared to a space, all referenced visualizations and in-turn index-patterns must be shared before the dashboard is shared.

Additionally, when a saved-object is either created or updated and references new saved-objects, the references will be checked to ensure they exist in at least all spaces which the saved-object exists in. This will prevent saved-objects from being updated or created with broken references.

[pgayvallet]

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)

[legrego]

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

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 tags as a top-level property instead of using references:

I've gone back and forth on this so many times. I'm not conceptually opposed to adding a tags property, but I understand the desire to create a generalized references solution that we can build upon.

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 tags property at the top level, and eventually migrate convert them to proper "references" once we are comfortable doing so. We will have to introduce the concept of a core SO conversion as part of https://github.com/elastic/kibana/issues/54837, so the additional effort of moving tags from one location to the other would theoretically be reduced.

[pgayvallet]

After a sync discussion during yesterday's platform team weekly with @elastic/kibana-platform, @kobelb @legrego and @alexfrancoeur, we came to the following conclusions:

• The main pain points for the references approach was when functionalities were making a distinction between tags and other relations
• The references approach, even if requiring overall more changes on the client-side part of the code, is more robust and future-proof, especially regarding SO referencial integrity that is going to be implemented to share SO across spaces.

Which is why we acted that:

• We are going to implement the SO-Tag relation using the references array
• To get rid of the technical pain points:
  
  
  
  • Tags will not be editable from the inspect page of the SO management section (would still be able to do so using the references field editable in this page)
  
  
  • The export / share to space / copy to space actions will not make a distinction between tags and relations. The current include related objects option will just consider tags a related objects.
  
  

[pgayvallet]

Update of https://github.com/elastic/kibana/pull/79096: Tagging is now wired to dashboard and visualize listing views: When the x-pack savedObjectTagging is enabled, the listing pages for these two apps are displaying a tags column and a tag filter (which is properly wired to use tag references when searching from these pages).

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 SavedObjectLoader behavior regarding references is confirmed: all references are transient on the client side, meaning that they are all supposed to be injected to the SO's data when the SO loader loads the object using injectReferences, and then injected back into the raw saved object during save operations using extractReferences. Any references not collected by these methods is lost during a save/update operation.

The call chain is a pain to follow, but basically _serialize just injects the 'legacy' references (searchSourceFields, unresolvedIndexPatternReference) and then let injectReferences, which is provided by the child class, do the rest of the job. Any existing reference not handled/ by either of these methods will be erased during a create or update operation, which, of course, is the case for the tag references we are adding.

https://github.com/elastic/kibana/blob/78e0bdf3144e11d40092fbd3a70e546115771d86/src/plugins/saved_objects/public/saved_object/helpers/save_saved_object.ts#L81-L84

https://github.com/elastic/kibana/blob/15c0644c9756eb58e86cef7e7cd1ce8266bc7ed0/src/plugins/saved_objects/public/saved_object/helpers/build_saved_object.ts#L91

https://github.com/elastic/kibana/blob/40d2cfc4db64c6df4d1897b5d3e3787271e63783/src/plugins/saved_objects/public/saved_object/helpers/serialize_saved_object.ts#L23-L27

https://github.com/elastic/kibana/blob/19bda1fceeca564d5bae73b3a78276870e25f220/src/plugins/visualizations/public/saved_visualizations/_saved_vis.ts#L76-L78

https://github.com/elastic/kibana/blob/b7e87c2c8b316ac6d567363f1eedd4322bdbf11d/src/plugins/dashboard/public/saved_dashboards/saved_dashboard.ts#L47-L51

(Note that using a new tags field instead of references would have similar consequences on the changes to the loader)

Of course, both dashboard and visualize are using the SO loaders to perform save operations, so if we want to get this done, I see two (+1) options, which are both quite impactful:

Adapt the base SavedObject class to add tag support

We would add tag support directly to the client-side SavedObject class/api.

(This all starts there: src/plugins/saved_objects/public/saved_object/saved_object.ts)

We would have to:

• adapt the SavedObject (src/plugins/saved_objects/public/types.ts) interface to add the following methods (I'm afraid of directly accessible properties to avoid risks of conflicts when child classes inject their own attributes to the instance...)
  
  
  
  • getTags(): string[] // return the ids of the tags associated with this object
  
  
  • setTags(tagIds: string[]) // sets the tags associated with this object
  
  
• adapt initializeSavedObject (or underlying call to applyESResp) to load the tag references into a private field accessible via the previous getter/setters
  
  
  
  • adapt saveSavedObject (or underlying call to _serialize) to reinject the tag references after calling extractReferences
  
  

Note that we have the same issue than security had when implementing the spaces in core's SOR: We can't have the risk for tags to be erased from the references when saving an object when the SOT plugin is (temporarily) disabled, meaning that we can't have any abstraction / delegation here. the savedObject OSS plugin WILL have code/logic only used for a licensed feature, as the tag extraction/injection mechanism MUST always be enabled (we can't delegate/bridge it to the SOT OSS plugin).

Adapt the sub-classes of SavedObject to manually add tag support to each of them

For example, we would adapt the SavedVis class (src/plugins/visualizations/public/saved_visualizations/_saved_vis.ts) to add tag support. We would adapt their extractReferences and injectReferences to have the tag extraction / injection logic in the implementation instead of the base / abstract SavedObject class.

I think I still prefer the first option, as the 'licensing' problem remains the same (we also can't disable the inject/extract logic here), and this just looks like code duplication to me, which leads to more errors and more maintenance (especially when we'll plug tagging to other apps).

Just let the teams handle that

The last option, that @kobelb suggested during a sync discussion, would be to just let the owning teams perform the required changes for each individual application.

For that, we could basically just implement an assign tag action to the SO management table rows, which would allow us to properly test the feature, and then, after merging the PR (or using a feature branch), let the other teams perform the changes on their own. Note that It doesn't really change the SO loader root issue that they will just have to face instead of us, and I think it will just 'force' teams to implement the second option to avoid touching the root SO loader classes.

ping @kobelb @joshdover @spalger @legrego

cc @elastic/kibana-app @alexfrancoeur @ryankeairns

[kobelb]

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 tags?

[pgayvallet]

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 SavedObject instances. I created https://github.com/elastic/kibana/pull/80063 that is the first step.

[ryankeairns]

@pgayvallet here is a quick take on validation errors for tag naming.

Special characters

The before and after versions are displayed below. The EuiFormRow's helpText prop can be used to display the gray text, while the error prop can display the resulting red text. This does cause the form to shift down, which is unfortunate, however there is an "urgent" EUI issue that will ultimately provide a better solution.

Max 50 characters

As for the max characters, let's set a maxlength on the input and block further typing. That should prevent the need to ever display the max characters error. In the event that it does not fully prevent this from occurring, we can likewise output an additional validation error message using EuiFormRow's error prop.

Preventing duplicates

If we decide to show a warning or error for duplicates (presumably within the given Space), then an EuiCallOut placed atop the form would be the way to go. Use color: "danger" if we decide to go with an error instead of a warning.

For reference, here are the EUI form validation docs: https://elastic.github.io/eui/#/forms/form-validation

[MichaelMarcialis]

A fair bit of the mockups themselves feel future facing and not necessarily mapped to the MVP requirements defined above.

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

[alexfrancoeur]

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.