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

Export hooks are not called when exporting referenced saved objects #100043

Closed
ymao1 opened this issue May 13, 2021 · 7 comments · Fixed by #100769
Closed

Export hooks are not called when exporting referenced saved objects #100043

ymao1 opened this issue May 13, 2021 · 7 comments · Fixed by #100769
Assignees
Labels
bug Fixes for quality problems that affect the customer experience Feature:Saved Objects Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc triage_needed

Comments

@ymao1
Copy link
Contributor

ymao1 commented May 13, 2021

I have two saved object types alerts and actions that implement the onExport hook to perform some simple transformations before export. The alerts SO can reference the actions SO. When exporting actions on their own, the onExport hook is called correctly. When exporting alerts with referenced actions, the alerts onExport hook is called bu the actions onExport hook is not called and the actions are not correctly transformed before export.

@ymao1 ymao1 added bug Fixes for quality problems that affect the customer experience Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc labels May 13, 2021
@elasticmachine
Copy link
Contributor

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

@lukeelmers
Copy link
Member

Took a quick look at this, and AFAICT it must have been a case we didn't consider in the initial implementation. The SO exporter tests check that the transforms are applied to a SO type, but not that they are applied to referenced objects.

@pgayvallet
Copy link
Contributor

I can confirm we just did not think of that in the initial implementation.

I need to take a closer look, I suspect it may be a little tricky to implement due to the fact that the export hooks can return objects with their own references, so we would need to recurse on the object tree after each call to the export hooks.

@joshdover joshdover removed the Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc label May 18, 2021
@botelastic botelastic bot added the needs-team Issues missing a team label label May 18, 2021
@joshdover joshdover added the Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc label May 18, 2021
@botelastic botelastic bot removed the needs-team Issues missing a team label label May 18, 2021
@pgayvallet
Copy link
Contributor

To cross-link, this feature may have some complex interactions with the capability to filter objects out of the export (#99680), see #99680 (comment).

I do think that we'll want to implement a solution for current issue anyway, and think about the interactions when trying to address #99680, as this one is more of a bug, and #99680 more of an enhancement.

@pgayvallet
Copy link
Contributor

So, after #100043 (comment), I digged a bit deeper.

We currently perform the operation in two steps

  1. execute the export hooks on the first level of saved objects
  2. (if includeReferences is true) recursively fetch the references of the objects until all references on all levels are resolved.

To properly invoke export hooks on the whole tree, we will need to

  1. execute the export hooks on the current (initial) level of saved objects
  2. (if includeReferences is true) non recursively fetch the references of this level
  3. keep track of the processed objects
  4. repeat 1. and 2. for objects that were not already processed until no more references are unresolved.

@pgayvallet
Copy link
Contributor

pgayvallet commented May 27, 2021

@ymao1 opened a PR to address this: #100769

Had a question though:

atm, the PR is fixing the issue by invoking the export hooks on all the referenced objects

E.g objA has a reference to objB => the export hooks will be execute for both objA and objB

however, I'm still not invoking export hooks on objects returned by the export hooks.

E.g if typeA has an export hook that will include some typeB docs, and typeB has an export hook registered, it will not invoke the export hooks for the typeB docs returned from the typeA hook.

Is that alright, or do you also need this? (this is only impacting if you're using the export hook to add additional objects to the export list, not if you're just mutating the objects properties before export)

@ymao1
Copy link
Contributor Author

ymao1 commented May 27, 2021

@pgayvallet Thanks for looking into this! For our current use case, what you've done is exactly what we need: having the export hooks for both objA and objB executed if objA contains a reference to objB. I tried out your PR with rules & connectors and it is working as expected!

I am trying to think of a case where we might, in the future, need to add additional objects to the export but am coming up empty (and it seems like it would be a strange user experience to export an object and then have it show up with new references when imported elsewhere?). That said, that doesn't mean there'll never be a use case for it, so maybe it would be good to open a separate issue for that and see if there are any requests for it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience Feature:Saved Objects Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc triage_needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants