Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Discuss] Saved object references (global -> non-global) #107575

Open
jportner opened this issue Aug 3, 2021 · 6 comments
Open

[Discuss] Saved object references (global -> non-global) #107575

jportner opened this issue Aug 3, 2021 · 6 comments
Labels
discuss loe:medium Medium Level of Effort Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc

Comments

@jportner
Copy link
Contributor

jportner commented Aug 3, 2021

Background

Saved object references in Kibana are one-way. Given A -> B, where A has a reference to B:

  • A is the source and it holds the actual reference; it has what we call an outbound reference to B
  • B is the target; it has what we call an inbound reference from A

The fields stored in a reference are: type, id, and name.

As stated in #100489, in the 8.0 release, we plan to convert existing isolated object types (namespaceType: 'single') to become share-capable (namespaceType: 'multiple-isolated'). This has the effect of regenerating IDs for any objects of that type that do not exist in the Default space.

During the 8.0 upgrade process, if object B's ID changes, any inbound references should be updated automatically. However, the underlying "reference transform" algorithm actually needs to change object A to do this; as a result, it needs to know the space that B exists in. The algorithm assumes that A and B exist in the same space.

Problem

We are advising consumers to only use the references field to store links to other objects, and to make sure that happens before the 8.0 release (7.16 at the latest). However, we have identified at least two use cases where global object types (namespaceType: 'agnostic') have to store links to non-global, isolated objects. These global objects do not currently use the references field, they use type-level fields for such links, so they can store the space ID of the target object.

We need to support these "global to non-global" object links, and we need to have a migration path when non-global object IDs change in 8.0. I can think of two obvious solutions to this problem.

Solution 1

Since these object links seem to be edge cases, the easiest solution would be to tell consumers to continue using type-level object links for these situations. This is more responsibility for the consumer, which sounds reasonable.

To reuse the example of A and B above, consumers would need to write a custom migration function to update the ID for B in 8.0. They can "guess" the new object ID today via:

uuidv5(`${namespace}:${type}:${id}`)

The only issue with this approach is that object IDs are only converted in index migrations (which are triggered when Kibana is upgraded). However, consumer-defined migrations are applied during both index migrations and one-off document migrations (which are triggered when an object is created or re-created with an outdated migrationVersion -- this could happen because of an object import or an external migration).

So to make this approach work, we would need to allow consumers to identify whether the migration is an index migration or a document migration. We could expose this with a new field on the existing migrationContext. Then, a consumer could write a custom migration function that reacts to the migrationContext and only applies any changes during an index migration.

Unfortunately this adds complexity to the already-complex migration process. We do already have precedent where "conversion" and "reference" transforms are only applied during index migrations, and this situation appears to be an edge case, so maybe this is acceptable.

Solution 2

As mentioned above, the fields stored in a reference are: type, id, and name. We could add a fourth (optional) field, namespace. To reuse the example of A and B above, this optional reference field would override the default assumption that A and B exist in the same space.

Unfortunately this has a much bigger blast radius than Solution 1, and it may result in other consumers relying on this namespace override when we don't want to proliferate this type of usage.

@jportner jportner added discuss Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! labels Aug 3, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-core (Team:Core)

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-security (Team:Security)

@legrego
Copy link
Member

legrego commented Aug 3, 2021

Solution 1 seems more tolerable to me. Introducing a namespace to references gives the impression that an isolated object can reference another isolated object in another space, and I don't think this is a configuration we want to support. This would also further complicate the "namespace redaction" process that we undergo today within the security SOC wrapper, and could lead to even more situations where an object graph is only partially available.

Something else we had discussed offline is to push the complexity onto these (hopefully edge-case) consumers. Rather than defining an entirely new migration concept, we could augment the existing migration context so that migration authors can know when their functions are running as part of an Index migration, or as part of a one-off Document migration.

@jportner
Copy link
Contributor Author

jportner commented Aug 3, 2021

Introducing a namespace to references gives the impression that an isolated object can reference another isolated object in another space, and I don't think this is a configuration we want to support.

Agreed

This would also further complicate the "namespace redaction" process that we undergo today within the security SOC wrapper, and could lead to even more situations where an object graph is only partially available.

Great point, I hadn't considered that!

Something else we had discussed offline is to push the complexity onto these (hopefully edge-case) consumers. Rather than defining an entirely new migration concept, we could augment the existing migration context so that migration authors can know when their functions are running as part of an Index migration, or as part of a one-off Document migration.

Yes, this is what I tried to describe with: "So to make this approach work, we would need to allow consumers to identify whether the migration is an index migration or a document migration. Then, a consumer could write a custom migration function that only gets applied during an index migration."

I realize now that wasn't clear, I'll update the issue description.

@joshdover
Copy link
Contributor

From a Platform perspective, I agree Solution 1 makes the most sense. For the use case I brought up related to Fleet/EPM packages, I'm actually not sure it will be a workable solution. The reason being, that the epm-packages type actually doesn't currently store the namespace that these objects were created in, so I don't believe we can use this migration.

Instead, I think we'll need to use the resolve API inside a "custom migration" in the plugin's start contract that tries to locate all Spaces these objects may have been created in and/or copied to, delete the old objects, and then create a new object that is shared to all of these Spaces.

Not tracking the namespace is breaking several things in the Fleet UI, so this is a larger issue that needs to be addressed.

@exalate-issue-sync exalate-issue-sync bot added impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort labels Aug 4, 2021
@jportner
Copy link
Contributor Author

jportner commented Aug 5, 2021

From a Platform perspective, I agree Solution 1 makes the most sense. For the use case I brought up related to Fleet/EPM packages, I'm actually not sure it will be a workable solution. The reason being, that the epm-packages type actually doesn't currently store the namespace that these objects were created in, so I don't believe we can use this migration.

😔

Instead, I think we'll need to use the resolve API inside a "custom migration" in the plugin's start contract that tries to locate all Spaces these objects may have been created in and/or copied to, delete the old objects, and then create a new object that is shared to all of these Spaces.

Yeah that will be tough as each function that the SavedObjectsClient API exposes is only scoped to the current space. The exception to this is find, which can search across spaces.
So in this "custom migration" they would be better off using find/PIT, paging through the results, and deleting/recreating the objects as stated.
They can't use a simple create with overwrite: true because we prevent consumers from overwriting objects that are not in the current space.

@exalate-issue-sync exalate-issue-sync bot added loe:medium Medium Level of Effort and removed loe:small Small Level of Effort labels Sep 29, 2021
@legrego legrego removed EnableJiraSync impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. labels Aug 18, 2022
@legrego legrego removed the Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! label Jan 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss loe:medium Medium Level of Effort Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc
Projects
None yet
Development

No branches or pull requests

4 participants