Skip to content

Commit

Permalink
[8.x] Don't migrate partial documents (#198703) (#199131)
Browse files Browse the repository at this point in the history
# Backport

This will backport the following commits from `main` to `8.x`:
- [Don't migrate partial documents
(#198703)](#198703)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Rudolf
Meijering","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-11-06T10:21:17Z","message":"Don't
migrate partial documents (#198703)\n\n## Summary\r\n\r\nFixes a
regression by adding back the functionality that was
originally\r\nintroduced in
#162404: skipping\r\nmigrations
(transform and forwardCompatibilitySchema) when the fields\r\noption is
specified\r\n\r\nIn working on this we uncovered some problem scenarios
we previously\r\nhadn't thought through (will create separate issues for
these):\r\n1. Even if we can't reliably run the transform part of
migrations on a\r\npartial document we should still run the
`forwardCompatibilitySchema` so\r\nthat unknown fields from future
versions are dropped.\r\nHowever, we realised most teams missed the
detail in
our\r\n[docs](https://github.com/elastic/kibana/blob/main/docs/developer/architecture/core/saved-objects-service.asciidoc?plain=1#L429-L457)\r\nthat
call out that forwardCompatibilitySchemas shouldn't assert on
data\r\nbut only drop unknown fields. This means if we would run
the\r\nforwardCompatibilitySchema today it would most often fail on
partial\r\ndocuments.\r\n3. When `fields` contain an encrypted field but
not all AAD fields,\r\nencryption would fail and log an error
(potentially triggering\r\nserverless
alerts).","sha":"36f6d6fa02fc4a35b7076d127c4f8ce92d31d71f","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","Team:Core","release_note:skip","v9.0.0","backport:prev-major"],"title":"Don't
migrate partial
documents","number":198703,"url":"https://github.com/elastic/kibana/pull/198703","mergeCommit":{"message":"Don't
migrate partial documents (#198703)\n\n## Summary\r\n\r\nFixes a
regression by adding back the functionality that was
originally\r\nintroduced in
#162404: skipping\r\nmigrations
(transform and forwardCompatibilitySchema) when the fields\r\noption is
specified\r\n\r\nIn working on this we uncovered some problem scenarios
we previously\r\nhadn't thought through (will create separate issues for
these):\r\n1. Even if we can't reliably run the transform part of
migrations on a\r\npartial document we should still run the
`forwardCompatibilitySchema` so\r\nthat unknown fields from future
versions are dropped.\r\nHowever, we realised most teams missed the
detail in
our\r\n[docs](https://github.com/elastic/kibana/blob/main/docs/developer/architecture/core/saved-objects-service.asciidoc?plain=1#L429-L457)\r\nthat
call out that forwardCompatibilitySchemas shouldn't assert on
data\r\nbut only drop unknown fields. This means if we would run
the\r\nforwardCompatibilitySchema today it would most often fail on
partial\r\ndocuments.\r\n3. When `fields` contain an encrypted field but
not all AAD fields,\r\nencryption would fail and log an error
(potentially triggering\r\nserverless
alerts).","sha":"36f6d6fa02fc4a35b7076d127c4f8ce92d31d71f"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/198703","number":198703,"mergeCommit":{"message":"Don't
migrate partial documents (#198703)\n\n## Summary\r\n\r\nFixes a
regression by adding back the functionality that was
originally\r\nintroduced in
#162404: skipping\r\nmigrations
(transform and forwardCompatibilitySchema) when the fields\r\noption is
specified\r\n\r\nIn working on this we uncovered some problem scenarios
we previously\r\nhadn't thought through (will create separate issues for
these):\r\n1. Even if we can't reliably run the transform part of
migrations on a\r\npartial document we should still run the
`forwardCompatibilitySchema` so\r\nthat unknown fields from future
versions are dropped.\r\nHowever, we realised most teams missed the
detail in
our\r\n[docs](https://github.com/elastic/kibana/blob/main/docs/developer/architecture/core/saved-objects-service.asciidoc?plain=1#L429-L457)\r\nthat
call out that forwardCompatibilitySchemas shouldn't assert on
data\r\nbut only drop unknown fields. This means if we would run
the\r\nforwardCompatibilitySchema today it would most often fail on
partial\r\ndocuments.\r\n3. When `fields` contain an encrypted field but
not all AAD fields,\r\nencryption would fail and log an error
(potentially triggering\r\nserverless
alerts).","sha":"36f6d6fa02fc4a35b7076d127c4f8ce92d31d71f"}}]}]
BACKPORT-->

Co-authored-by: Rudolf Meijering <[email protected]>
  • Loading branch information
kibanamachine and rudolf authored Nov 6, 2024
1 parent 4cf3ee8 commit 8cf6d9c
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
CheckAuthorizationResult,
SavedObjectsRawDocSource,
GetFindRedactTypeMapParams,
SavedObjectUnsanitizedDoc,
} from '@kbn/core-saved-objects-server';
import {
DEFAULT_NAMESPACE_STRING,
Expand Down Expand Up @@ -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[];
Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit 8cf6d9c

Please sign in to comment.