[Saved Objects] Fix document migrator to prevent losing namespaces on import

[dokmic]

Summary

Resolves #164454.

Checklist

• Unit or functional tests were updated or added to match the most common scenarios

For maintainers

• This was checked for breaking API changes and was labeled appropriately

[rudolf]

Based on Pierre's comment that create works but import doesn't I didn't expect that the problem was in the internal transform. Although he tested create not bulk_create, it seemed to me like there must be some difference in the payload we pass to bulkCreate from /api/saved_objects/_import vs /api/saved_objects/_bulk_create

https://github.com/elastic/kibana/blob/main/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/import/lib/create_saved_objects.ts#L123-L127
https://github.com/elastic/kibana/blob/main/packages/core/saved-objects/core-saved-objects-server-internal/src/routes/bulk_create.ts#L80-L83

I think it would also be useful to add an API integration test against the _import API for this case.

[dokmic]

@rudolf The problem is that we set typeMigrationVersion in case it's undefined to '':

kibana/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/import/lib/collect_saved_objects.ts

Line 72 in c288c8b

...(!obj.migrationVersion && !obj.typeMigrationVersion ? { typeMigrationVersion: '' } : {}),

Before typeMigrationVersion, we were using {}, which was for the same purpose — explicitly triggering all the migrations.

In other words, if we pass "typeMigrationVersion": "" in a (bulk) create payload, we get the same issue. But adding a test for the import endpoint makes total sense 👍

[dokmic]

After some investigation of the nature of the conversion transforms, I found out that there is some inconsistency in conversion transforms. If we import objects with types that have convertToMultiNamespaceTypeVersion, and there is a type migration with a version higher than in convertToMultiNamespaceTypeVersion, then the conversion transforms will never be applied.

After discussing with @jeramysoucy and digging into the commit history, I found the reason for deferring the conversion transforms. The idea is that the newly created objects or imported should not have legacy-url-alias, but the ones that migrated from the previous version should have that. This is why migrateAndConvert is called during the re-indexing only. That should probably not be called on Kibana restart, but it's not critical as long as we handle namespace migrations correctly.

After a few attempts at fixing this by changing the flow to apply conversion transform on all the input documents (newly created or imported), that creates unnecessary legacy-url-alias objects. Ignoring these objects makes using those conversion transforms redundant, as we already handle namespace conversion upon object creation. Changing the latter requires more work and additional changes in the test suite.

The best approach would be not to fix this inconsistency at this point, as the convertToMultiNamespaceTypeVersion parameter is already deprecated, and we can safely remove the conversion transform along with the parameter.

With regard to an additional test covering the _import endpoint covering the difference with the bulk create, it cannot be tested with the already existing saved object types as the setup will be too brittle and fail whenever the type introduces another migration. Also, migrator tests are already covering this behavior, so it doesn't make much sense to add this now. However, I have added a test covering the resolution of this particular bug.

cc @pgayvallet @rudolf

[elasticmachine]

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

[kibana-ci]

💚 Build Succeeded

• Buildkite Build
• Commit: 3e31044

Metrics [docs]

Unknown metric groups

References to deprecated APIs

id	before	after	diff
@kbn/core-saved-objects-migration-server-internal	27	28	+1

History

• 💚 Build #154038 succeeded ad5ce9dc34d186c7327913ce143c1f242bad0730
• 💔 Build #153562 failed 5cfa68244e35327a6897dab7eb43cdd6447b086f

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

[gsoldevila]

LGTM!

I tested the scenario described in https://github.com/elastic/sdh-kibana/issues/4101.

Without the fix, the Sample Data Logs dataset gets corrupted upon restart (3 saved objects are moved to the default space).
With the fix, all SO preserve their namespace upon restart.

[kibanamachine]

💚 All backports created successfully

Status	Branch	Result
✅	8.10

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation