Saved objects import - inconsistent handling of missing references

[legrego]

The Saved Objects Import API (and corresponding UI feature) attempts to identify missing references when importing objects. For example, the import will fail with a missing_references error when importing a visualization which references an index-pattern that doesn't exist.

This check currently works for direct references of index-patterns and search objects, but it does not handle references of any other saved object type, and it also doesn't handle transitive references.

For example, the API allows for dashboards to be imported which reference non-existent visualizations.

Similarly, the API allows for dashboards to be imported which reference a visualization which in turn references a non-existent index pattern.

Ideally, I think the missing references should handle transient references, and references to other saved object types besides index-pattern and search. I think this should also be an optional condition. The API should allow users to copy the object knowing full well that the reference is missing, if they choose to ignore this warning.

Blocks:

• Spaces - copy saved objects should allow copying without references #43985
• Copy to Space should allow associated index patterns to be changed  #59544

[elasticmachine]

Pinging @elastic/kibana-platform

[rudolf]

Looking at the code this looks like a intentional design decision https://github.com/elastic/kibana/blob/master/src/core/server/saved_objects/import/validate_references.ts#L24

@mikecote Was there a reason we don't try to resolve all references regardless of the saved object type?

[mikecote]

From what I recall, this was for scope reduction. I decided to get feature parity with the legacy import which only made sure index patterns existed. I think index patterns were more important to check as Kibana would crash severely when they're missing. Other types of saved object would just cause errors when loading the app (same state as they would be when exporting and including dependencies). The other portion would be having to design a UI / UX around other missing references (or maybe just ask if they still want to import object x?).

[rudolf]

I think this should also be an optional condition. The API should allow users to copy the object knowing full well that the reference is missing, if they choose to ignore this warning.

Can you expand on why you think this should be possible?

Allowing Saved Objects with missing references essentially removes some data integrity guarantees. Kibana breaks in awkward ways when references don't exist which creates hard to diagnose support cases / bugs or just a general bad experience for users. We'd need to do a lot to get to this point, but it would be really nice if a Saved Object library consumer could trust that their object references would always be intact. The only "excuse" for a missing reference would be a sysadmin fiddling with data in the .kibana index.

[legrego]

I think this should also be an optional condition. The API should allow users to copy the object knowing full well that the reference is missing, if they choose to ignore this warning.

Can you expand on why you think this should be possible?

Sure! This may be a bit contrived, but here's a scenario: a well-informed user might be in the process of trying to setup a new Space by copying specific saved objects over to it. For example, a dashboard with a number of visualizations, which reference an index pattern. Maybe this index pattern is overly complex for the new space, with scripted fields and custom formatters that the dashboard in question doesn't take advantage of. The user knows they'll have to re-associate the visualizations to the new index pattern at some point, but perhaps they aren't ready for that just yet.

Allowing Saved Objects with missing references essentially removes some data integrity guarantees. Kibana breaks in awkward ways when references don't exist which creates hard to diagnose support cases / bugs or just a general bad experience for users. We'd need to do a lot to get to this point, but it would be really nice if a Saved Object library consumer could trust that their object references would always be intact. The only "excuse" for a missing reference would be a sysadmin fiddling with data in the .kibana index.

I agree 100%, but Kibana's story around data integrity isn't consistent at this point. Most of Kibana doesn't enforce referential integrity today, which does lead to bugs and a poor user experience. I think it's more aggravating that certain operations enforce these integrity checks, while others don't.

For example, there is nothing stopping you from deleting an index pattern via the documented Saved Objects API, even if it's in use. Same goes for the Saved Objects Management UI today -- we allow this to happen without so much as a warning to the end-user. Visualizations and saved searches have a similar story.

I think in an ideal world, the default behavior would be to enforce these integrity checks, but allow consumers to override this behavior if they absolutely need to. Most users will benefit from these integrity checks, but power users will sometimes need Kibana to "get out of their way" in order to solve whatever problem they're having.

[legrego]

Not a high priority right now, but for reference, this blocks the following enhancements:

• Spaces - copy saved objects should allow copying without references #43985
• Copy to Space should allow associated index patterns to be changed  #59544

[jportner]

#62553 will partially resolve this issue.

This check currently works for direct references of index-patterns and search objects, but it does not handle references of any other saved object type, and it also doesn't handle transitive references.

I don't believe this is 100% true. Currently, a "missing_references" error blocks any inbound references from being created:

kibana/src/core/server/saved_objects/import/validate_references.ts

Lines 118 to 134 in 8a4daff

	// Filter out objects that reference objects within the import but are missing_references
	// For example: visualization referencing a search that is missing an index pattern needs to be filtered out
	filteredObjects = filteredObjects.filter((savedObject) => {
	let isBlocked = false;
	for (const reference of savedObject.references || []) {
	const referencedObjectError = errorMap[`${reference.type}:${reference.id}`];
	if (!referencedObjectError || referencedObjectError.error.type !== 'missing_references') {
	continue;
	}
	referencedObjectError.error.blocking.push({
	type: savedObject.type,
	id: savedObject.id,
	});
	isBlocked = true;
	}
	return !isBlocked;
	});

For the following scenario:

visualization 'foo' -> search 'bar' -> index-pattern 'baz'

If index-pattern 'baz' does not exist, then search 'bar' will result in a missing_references error, and visualization 'foo' will be blocked too.

The PR linked above changes import behavior to block any object from being created if a resolvable error is detected (that is: missing_references, conflict, or ambiguous_conflict). This as-is would worsen the existing issue for copy-to-space operations.

Proposal 1:
The PR linked above can add a flag to circumvent blocking behavior when a missing_references error is present, so that copy-to-space operations won't fail completely in these scenarios. In the scenario above, visualization 'foo' would be copied, but search 'bar' would not.

Proposal 2:
The PR linked above can add a new retry option to ignore a missing_references error, so that copy-to-space operations can proceed. In the scenario above, visualization 'foo' and search 'bar' would both be copied. (Up for debate: do we silently/automatically enable this type of retry for copy-to-space? Or do we default to skip that error? I'd prefer the former)

Proposal 3:
The PR linked above can add a flag to preemptively ignore any missing_references errors, so that copy-to-space operations can proceed. This would still surface the error so the user can be notified in the UI, but it would allow the object to be created anyway. In the scenario above, visualization 'foo' and search 'bar' would both be copied.

I am leaning towards Proposal 3.

Edit: Proposal 2 gets us closer to our end state of allowing users to actually resolve missing_references errors in the UI. It also allows users to abort the copy operation if one of these errors is detected. So I'll implement that.