Don't migrate partial documents (#198703)

## Summary

Fixes a regression by adding back the functionality that was originally
introduced in https://github.com/elastic/kibana/pull/162404: skipping
migrations (transform and forwardCompatibilitySchema) when the fields
option is specified

In working on this we uncovered some problem scenarios we previously
hadn't thought through (will create separate issues for these):
1. Even if we can't reliably run the transform part of migrations on a
partial document we should still run the `forwardCompatibilitySchema` so
that unknown fields from future versions are dropped.
However, we realised most teams missed the detail in our
[docs](https://github.com/elastic/kibana/blob/main/docs/developer/architecture/core/saved-objects-service.asciidoc?plain=1#L429-L457)
that call out that forwardCompatibilitySchemas shouldn't assert on data
but only drop unknown fields. This means if we would run the
forwardCompatibilitySchema today it would most often fail on partial
documents.
3. When `fields` contain an encrypted field but not all AAD fields,
encryption would fail and log an error (potentially triggering
serverless alerts).
This commit is contained in:
Rudolf Meijering 2024-11-06 11:21:17 +01:00 committed by GitHub
parent bf51662b7b
commit 36f6d6fa02
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 49 additions and 6 deletions

View file

@ -344,6 +344,33 @@ describe('find', () => {
),
});
});
it('does not perform migrations when a partial document is requested by specifying no fields should be retrieved', async () => {
const noNamespaceSearchResults = generateIndexPatternSearchResults();
client.search.mockResolvedValueOnce(
elasticsearchClientMock.createSuccessTransportRequestPromise(noNamespaceSearchResults)
);
migrator.migrateDocument.mockImplementationOnce((doc) => ({ ...doc, migrated: true }));
await expect(
repository.find({
type,
fields: [], // don't return any fields
})
).resolves.not.toHaveProperty('saved_objects.0.migrated');
expect(migrator.migrateDocument).not.toHaveBeenCalled();
});
it('does not perform migrations when a partial document is requested by specifying some fields', async () => {
const noNamespaceSearchResults = generateIndexPatternSearchResults();
client.search.mockResolvedValueOnce(
elasticsearchClientMock.createSuccessTransportRequestPromise(noNamespaceSearchResults)
);
migrator.migrateDocument.mockImplementationOnce((doc) => ({ ...doc, migrated: true }));
await expect(repository.find({ type, fields: ['title'] })).resolves.not.toHaveProperty(
'saved_objects.0.migrated'
);
expect(migrator.migrateDocument).not.toHaveBeenCalled();
});
});
describe('search dsl', () => {

View file

@ -16,6 +16,7 @@ import {
CheckAuthorizationResult,
SavedObjectsRawDocSource,
GetFindRedactTypeMapParams,
SavedObjectUnsanitizedDoc,
} from '@kbn/core-saved-objects-server';
import {
DEFAULT_NAMESPACE_STRING,
@ -57,6 +58,7 @@ export const performFind = async <T = unknown, A = unknown>(
common: commonHelper,
serializer: serializerHelper,
migration: migrationHelper,
encryption: encryptionHelper,
} = helpers;
const { securityExtension, spacesExtension } = extensions;
let namespaces!: string[];
@ -266,13 +268,27 @@ export const performFind = async <T = unknown, A = unknown>(
const migratedDocuments: Array<SavedObjectsFindResult<T>> = [];
try {
for (const savedObject of savedObjects) {
let migrated: SavedObjectUnsanitizedDoc | undefined;
const { sort, score, ...rawObject } = savedObject;
const migrated = disableExtensions
? migrationHelper.migrateStorageDocument(rawObject)
: await migrationHelper.migrateAndDecryptStorageDocument({
document: rawObject,
typeMap: redactTypeMap,
});
if (fields !== undefined) {
// If the fields argument is set, don't migrate.
// This document may only contains a subset of it's fields meaning the migration
// (transform and forwardCompatibilitySchema) is not guaranteed to succeed. We
// still try to decrypt/redact the fields that are present in the document.
migrated = await encryptionHelper.optionallyDecryptAndRedactSingleResult(
savedObject,
redactTypeMap
);
} else if (disableExtensions) {
migrated = migrationHelper.migrateStorageDocument(rawObject);
} else {
migrated = await migrationHelper.migrateAndDecryptStorageDocument({
document: rawObject,
typeMap: redactTypeMap,
});
}
migratedDocuments.push({ ...migrated, sort, score } as SavedObjectsFindResult<T>);
}
} catch (error) {