Ability to filter out items onExport or onImport

[ymao1]

In order to support removing legacy multi-tenancy, we have enabled rules and connectors to be exported and imported using the Saved Objects Management UI. However, security solutions have requested that their rules be excluded from this export (in favor of their solution specific import/export). There is currently no mechanism for doing this exclusion.

Possible options:

1. Exclude onExport - update the onExport implementation to allow export transforms to remove items from the export array. Currently, this throws an error: Invalid transform performed on objects to export. Note that this might lead to a confusing UX experience if we were unable to warn the user somehow that items have been filtered out and are not exportable.
2. Exclude onImport - update the onImport implementation to allow import transforms to be performed before saved objects are created and filter out items. It would be up to the onImport implementer to create the appropriate warning message warning users about items that have been filtered out and are not importable.
3. Another option??

References:
#50266, specific comment

[elasticmachine]

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

[mshustov]

It would be up to the onImport implementer to create the appropriate warning message warning users about items that have been filtered out and are not importable.

It's better from the UX point of view but makes export filed unnecessarily large.
Maybe we can take the better parts from both: allow plugins to exclude SO during export operation and provide a user-facing message that some SO hasn't been exported?

[pgayvallet]

However, security solutions have requested that their rules be excluded from this export (in favor of their solution specific import/export)

Could we get more context on why the decision was made to favor a solution-specific export/import ? what limitation of the SO management export leaded to that?

The whole point of the SO import/export improvements we're planning to do before 8.0 is to allow all solutions to be able to leverage the 'built-in' import/export instead of developing their own. It's also a very important goal for SO import/export to be able to handle all types of SOs.

cc @kobelb

EDIT: from the discussion here it seems caused by the fact that some prepackaged rules are using 'hardcoded' static IDs.

[pgayvallet]

Also, note that the interactions between this issue and #100043 may be complex.

If we allow export hooks to be invoked for the whole object tree recursively, and if, at the same time, we allow the export hooks to filter out objects, we'll be in a situation were we will need to dynamically update the references of the objects when some nested reference got omitted.

E.g

• we got an object foo:1 with references to bar:1 and bar:2
• the bar type got an export hook which would exclude bar:2 from the export (for any arbitrary reason).

During the export:

• we process foo:1 (no export hook)
• we then process its references, bar:1 and bar:2
• the export hook for the bar type removes bar:2 from the export
• we will be exporting foo:1 with a reference to an object that is not present in the export.

What should we be doing in that situation? Dynamically updates the foo:1 references to remove bar:2 from it? Keep an invalid reference? Throw an error?

[ymao1]

However, security solutions have requested that their rules be excluded from this export (in favor of their solution specific import/export)

Could we get more context on why the decision was made to favor a solution-specific export/import ? what limitation of the SO management export leaded to that?

EDIT: from the discussion here it seems caused by the fact that some prepackaged rules are using 'hardcoded' static IDs.

@pgayvallet Thanks for looking into this! Agree that ideally all import/export of rules should be done from the SO management UI. With the way that stack monitoring rules are currently auto-created and pre-packaged detection rules are currently loaded, there is a concern about unnecessarily duplicating those rule types by allowing them to be imported/exported.

There are currently some issues open to try to address this:

• Using static ids for detection rules: [Security Solution][Detections] Investigate updating _id to be static rule_id when migrating rules to rule registry #99741
• Not auto-creating stack monitoring rules: [Stack Monitoring] Change out of the box alerts to be opt-in, rather than auto-created #100133

If those issues are addressed, then this enhancement might not be necessary.

[mikecote]

Since the last comment from @ymao1, it sounds like we can't get a guarantee of the issues being resolved in time for 8.0, which brings us to a situation where we have to explore excluding some rules (Stack Monitoring rules mostly, still working with Security solution for SIEM rules).

What should we be doing in that situation? Dynamically updates the foo:1 references to remove bar:2 from it? Keep an invalid reference? Throw an error?

For our use case, we would only need to exclude certain rules on export. The chances are low that we would be in a situation where rules are part of another saved-object's references. This makes me think maybe throwing an error in that edge case isn't so bad. This path also avoids the core team taking too much technical debt until stack monitoring rules are exportable. @ymao1 does my thought process make sense?

[pgayvallet]

Having just implemented #100769, I can confirm my concerns explained in #99680 (comment). The way we currently collect missing references during the export will not play nice with root objects being removed.

Now, for this specific issue, I think we could just change the algorithm to just gather all the objects, and check for references integrity at the end instead of doing it incrementally as done currently.

However, what bothers me here is more the functional than the technical.

For our use case, we would only need to exclude certain rules on export. The chances are low that we would be in a situation where rules are part of another saved-object's references

Is it safe to assume that those rules will always be 'root' objects during the export (objects explicitly selected in the UI)?

Overall, my main concern is the user experience. Having some objects silently evicted from the export when the user explicitly selected them in the UI seems terrible in term of UX, so I would do everything possible to resurface the info as much as possible.

Option 1

The ugly (and fastest) option would be to allow export hooks to filter out objects, and just do that silently without propagating the info to the user.

Option 2

The slightly better option would be to allow export hooks to filter out objects, and then reflect that in the export summary (that is appended at the end of the .ndjson export file)

kibana/src/core/server/saved_objects/export/types.ts

Lines 63 to 75 in b2d36b8

	export interface SavedObjectsExportResultDetails {
	/** number of successfully exported objects */
	exportedCount: number;
	/** number of missing references */
	missingRefCount: number;
	/** missing references details */
	missingReferences: Array<{
	/** the missing reference id. */
	id: string;
	/** the missing reference type. */
	type: string;
	}>;
	}

We could just add it to missingReferences, or introduce a new field for that. We would then add a warning in the UI by parsing the export details as already done

kibana/src/plugins/saved_objects_management/public/management_section/objects_table/saved_objects_table.tsx

Lines 360 to 361 in 9dae1ef

	const exportDetails = await extractExportDetails(blob);
	this.showExportSuccessMessage(exportDetails);

Now, this is still not ideal, as when exporting via the UI, the user will not have this information until after the export has been performed, and he may just miss the warning about the fact that some objects were not included. Also, when using the HTTP API directly (e.g via any automation) those warnings would very easily be totally ignored (seems unlikely that any automated export parses the file to check the result details).

Option 3

I would really like to have something that is visible by the end user in the UI before the export is effectively generated. An option I see would be to try to know in advance if an object is exportable or not.

• Introduce a new exportable filter to the SO type definition

type SavedObjectsTypeManagementDefinition  = {
   // other props
  /** allow to specify per-object exportability when the type is importableAndExportable:true */
  exportable?: (obj: SavedObject) => boolean
}

• In SOM, propagate this info per object to the client via SavedObjectMetadata

Now the tricky part is: we currently have two way to export objects: by id and by type.

• By id

When exporting by ID, the user manually selects objects in the SOM table. In that case, as we would know the exportability status of each objects, we could prompt a warning when the user selects to export, stating (and listing) that some selected objects are not exportable, and then ask him if he wants to perform the export without those objects or abort.

Note that those exportability checks would be performed on the client side. When using the HTTP API directly, we have two options: Either throw a Bad Request informing that some of the specified objects are not exportable, or still perform the export and just list the non-exported objects in the export result details.

• By type

When exporting by type, we don't know if the user is going to export objects that are exportable(object) === false, as all objects are not loaded in the UI. We simply can't do that on the client-side.

One possible option would be to perform a preflight check against the server (using a distinct, new SOM endpoint) when exporting by type from the UI, to see if any objects are not exportable, and if so, inform the user that some objects of the selected types are not going to be exported (we can even list them), and ask him if he still wants to proceed (using the same confirm dialog we would be using for by id exports).

When using the UI, the look and feel would be the same as when exporting by ID (same prompt, same effective user workflow).

When the user is directly using the HTTP API, we won't be performing this preflight check. In that case, as throwing an error when trying to export a type than got some non-exportable objects is not really an option, I guess our only choice is to proceed with the export, and use the export result detail to list the objects that where not exported. Note that given that, I think that for consistency, we should do the same when exporting by ID (see last paragraph of the previous bullet point)

Overall

I think that functionally, option 3 is strictly better than option 2, being itself strictly better than option 1. Now, the same goes with the development cost, 3>2>1, so it all depends on the effort we want to spend on this. Option 1 is imho non-acceptable as the info about the excluded objects is not surfacing anywhere for the user. So it seems that it's either 2. or 3.

Option 3. is still more work than 2., so if we think this only need that as a temporary measure, 2. is probably fine. If we foresee that we may want it long-term, it is probably worth it to implement 3.

WDYT?

[ymao1]

@pgayvallet Thank you for the detailed breakdown. As I understand it, the separate import/export capability that security solutions developed was to fill a gap for a feature that the alerting framework did not offer at the time. Now that we do, I believe the long term goal is to align the capabilities and eventually just provide the centralized way to import/export. Does that sound correct @mikecote @spong?

Given that, I think option #2 makes the most sense. I like the idea of adding a missingExports section to the export summary. I think that would align well with the missingReferences section. We can also provide detailed docs on why certain rules might be missing from the export.

[mikecote]

Now that we do, I believe the long term goal is to align the capabilities and eventually just provide the centralized way to import/export. Does that sound correct @mikecote @spong?

That is correct for the long term. I wouldn't want to go too far out on implementing this capability if it can be removed at a future time.

[pgayvallet]

I wouldn't want to go too far out on implementing this capability if it can be removed at a future time.

Ok. In that case, I agree that option 2. seems like the best compromise.

[pgayvallet]

After a sync discussion with @joshdover, we decided to go with option 3. without implementing the UI part (we would only display a post-export warning). The reasoning is that we may need this feature in the future for other use cases (such as read-only saved objects), so implementing the isExportable API seems more future-proof and more resilient if we decided to go further later.

[pgayvallet]

I'm currently working on a PR to implement this new isExportable API, and coming to some implementation questions.

The current isExportable API looks like:

export type IsObjectExportablePredicate<Attributes = unknown> = (
  obj: SavedObject<Attributes>
) => boolean | Promise<boolean>;

I have a problem of priority between checking the exportability status and applying the export transform functions.

isExportable is deterministic, meaning that for a given SO, the resulting value should always be the same. However, the export transform hook may alter an object's attribute, meaning that

isExportable(obj) ?== isExportable(initialObject(exportHook([obj])))

So, theoretically, I would need to check for the exportability status after applying the export hooks.

However, the export hook is also capable of returning additional objects, and ideally, those should not be included if the 'source' object got excluded. The problem is, the SavedObjectsExportTransform takes a list of SO as argument, not a single one, meaning that we can't really post-filter the added objects if we execute the export predicate after the export transform and an 'initial' object got filtered out.

So, our two options are

1. Apply the export predicate before the export hooks

pros:

• If an object is non-exportable, we will just not be invoking the export hook with it, making sure that any potential additional object that would have been added is automatically excluded from the export.

cons:

• if a given object is transformed by the export hook in a way that isExportable(objBeforeHook) !== isExportable(objAfterHook), we would not be using the correct value for the exportability status

2. Apply the export predicate after the export hooks

pros:

• we're executing the predicate against the (transformed) object that will effectively be exported

cons:

• if an object got filtered, and if it's associated export hook was returning additional objects, we won't be able to automatically exclude the additional objects, and would have to rely on the fact that the additional objects will also manually be specified as 'non exportable' by their exportable predicate, which, given the stateless nature of these predicate, seems unlikely

Overall, I feel like isExportable(objBeforeHook) !== isExportable(objAfterHook) seems like a very unlikely scenario, as least from our currently identified needs of the feature, so I think option 1. is the way to go. I will just properly document that when both a isExportable filter and an onExport hook is provided, isExportable(objBeforeHook) must returns the same value as isExportable(objAfterHook)