diff --git a/docs/extend/saved-objects-service.md b/docs/extend/saved-objects-service.md index 6d1c1597a214..79b5846b54ab 100644 --- a/docs/extend/saved-objects-service.md +++ b/docs/extend/saved-objects-service.md @@ -350,12 +350,24 @@ Used to execute an arbitrary transformation function. *Usage example:* ```ts -let change: SavedObjectsModelUnsafeTransformChange = { +// Please define your transform function on a separate const. +// Use explicit types for the generic arguments, as shown below. +// This will reduce the chances of introducing bugs. +const transformFn: SavedObjectModelUnsafeTransformFn = ( + doc: SavedObjectModelTransformationDoc +) => { + const attributes: AfterType = { + ...doc.attributes, + someAddedField: 'defaultValue', + }; + + return { document: { ...doc, attributes } }; +}; + +// this is how you would specify a change in the changes: [] +const change: SavedObjectsModelUnsafeTransformChange = { type: 'unsafe_transform', - transformFn: (document) => { - document.attributes.someAddedField = 'defaultValue'; - return { document }; - }, + transformFn: (typeSafeGuard) => typeSafeGuard(transformFn), }; ``` @@ -1111,7 +1123,3 @@ Which is why, when using this option, the API consumer needs to make sure that * #### Using `bulkUpdate` for fields with large `json` blobs [_using_bulkupdate_for_fields_with_large_json_blobs] The savedObjects `bulkUpdate` API will update documents client-side and then reindex the updated documents. These update operations are done in-memory, and cause memory constraint issues when updating many objects with large `json` blobs stored in some fields. As such, we recommend against using `bulkUpdate` for savedObjects that: - use arrays (as these tend to be large objects) - store large `json` blobs in some fields - - - - diff --git a/examples/eso_model_version_example/server/plugin.ts b/examples/eso_model_version_example/server/plugin.ts index e82de510cff1..0c5cd09643dc 100644 --- a/examples/eso_model_version_example/server/plugin.ts +++ b/examples/eso_model_version_example/server/plugin.ts @@ -104,9 +104,10 @@ export class EsoModelVersionExample changes: [ { type: 'unsafe_transform', - transformFn: (document) => { - return { document }; - }, + transformFn: (typeSafeGuard) => + typeSafeGuard((document) => { + return { document }; + }), }, ], schemas: { diff --git a/src/core/packages/saved-objects/base-server-internal/src/model_version/build_transform_fn.test.ts b/src/core/packages/saved-objects/base-server-internal/src/model_version/build_transform_fn.test.ts index 856ad60a34ea..5f20b43fb850 100644 --- a/src/core/packages/saved-objects/base-server-internal/src/model_version/build_transform_fn.test.ts +++ b/src/core/packages/saved-objects/base-server-internal/src/model_version/build_transform_fn.test.ts @@ -185,7 +185,7 @@ describe('buildModelVersionTransformFn', () => { const changes: SavedObjectsModelChange[] = [ { type: 'unsafe_transform', - transformFn, + transformFn: (typeSafeGuard) => typeSafeGuard(transformFn), }, ]; @@ -203,11 +203,20 @@ describe('buildModelVersionTransformFn', () => { const changes: SavedObjectsModelChange[] = [ { type: 'unsafe_transform', - transformFn: (document, ctx) => { - delete document.attributes.oldProp; - document.attributes.newProp = 'newValue'; - return { document }; - }, + transformFn: (typeSafeGuard) => + typeSafeGuard( + ( + document: SavedObjectModelTransformationDoc<{ + oldProp?: string; + newProp: string; + }>, + ctx + ) => { + delete document.attributes.oldProp; + document.attributes.newProp = 'newValue'; + return { document }; + } + ), }, ]; @@ -305,10 +314,17 @@ describe('buildModelVersionTransformFn', () => { }, { type: 'unsafe_transform', - transformFn: (document) => { - document.attributes.unsafeNewProp = 'unsafeNewValue'; - return { document }; - }, + transformFn: (typeSafeGuard) => + typeSafeGuard( + ( + document: SavedObjectModelTransformationDoc<{ + unsafeNewProp: string; + }> + ) => { + document.attributes.unsafeNewProp = 'unsafeNewValue'; + return { document }; + } + ), }, ]; diff --git a/src/core/packages/saved-objects/base-server-internal/src/model_version/build_transform_fn.ts b/src/core/packages/saved-objects/base-server-internal/src/model_version/build_transform_fn.ts index 1e68f9a8fd57..0dd669be8801 100644 --- a/src/core/packages/saved-objects/base-server-internal/src/model_version/build_transform_fn.ts +++ b/src/core/packages/saved-objects/base-server-internal/src/model_version/build_transform_fn.ts @@ -14,6 +14,10 @@ import type { SavedObjectsModelUnsafeTransformChange, SavedObjectsModelDataBackfillChange, SavedObjectsModelDataRemovalChange, + SavedObjectModelUnsafeTransformFn, + SavedObjectModelTransformationDoc, + SavedObjectModelTransformationContext, + SavedObjectModelTransformationResult, } from '@kbn/core-saved-objects-server'; /** @@ -59,11 +63,25 @@ export const dataBackfillChangeToTransformFn = ( }; }; +// we must force 'any' type on the generic arguments of the received function +// otherwise they are 'unknown' and they cannot be cast to the PreviousAttributes and NewAttributes +// generic arguments needed by the typeSafeGuard functions +type TypeSafeGuardUnsafeTransformFn = ( + fn: SavedObjectModelUnsafeTransformFn +) => SavedObjectModelUnsafeTransformFn; + +const typeSafeGuard: TypeSafeGuardUnsafeTransformFn = ( + fn: SavedObjectModelUnsafeTransformFn +): SavedObjectModelTransformationFn => { + return ( + document: SavedObjectModelTransformationDoc, + context: SavedObjectModelTransformationContext + ): SavedObjectModelTransformationResult => fn(document, context); +}; + export const unsafeTransformChangeToTransformFn = ( change: SavedObjectsModelUnsafeTransformChange -): SavedObjectModelTransformationFn => { - return change.transformFn; -}; +): SavedObjectModelTransformationFn => change.transformFn(typeSafeGuard); const mergeTransformFunctions = ( transformFns: SavedObjectModelTransformationFn[] diff --git a/src/core/packages/saved-objects/server/docs/model_versions.md b/src/core/packages/saved-objects/server/docs/model_versions.md index f3d67d6d299d..95b5baffd455 100644 --- a/src/core/packages/saved-objects/server/docs/model_versions.md +++ b/src/core/packages/saved-objects/server/docs/model_versions.md @@ -35,8 +35,8 @@ ## Introduction -The modelVersion API is a new way to define transformations (*"migrations"*) for your savedObject types, and will -replace the "old" migration API after Kibana version `8.10.0` (where it will no longer be possible to register +The modelVersion API is a new way to define transformations (*"migrations"*) for your savedObject types, and will +replace the "old" migration API after Kibana version `8.10.0` (where it will no longer be possible to register migrations using the old system). The main purpose of this API is to address two problems of the old migration system regarding managed ("serverless") deployments: @@ -56,7 +56,7 @@ migrations was the stack version. You couldn't for example, add 2 consecutive mi It was fine for on-prem distributions, given there is no way to upgrade Kibana to something else than a "fixed" stack version. -For our managed offering however, where we're planning on decoupling deployments and upgrades from stack versions +For our managed offering however, where we're planning on decoupling deployments and upgrades from stack versions (deploying more often, so more than once per stack release), it would have been an issue, as it wouldn't have been possible to add a new migration in-between 2 stack versions. @@ -66,8 +66,8 @@ We needed a way to decouple SO versioning from the stack versioning to support t ### 2. The current migrations API is unsafe for the zero-downtime and backward-compatible requirements -On traditional deployments (on-prem/non-managed cloud), upgrading Kibana is done with downtime. -The upgrade process requires shutting down all the nodes of the prior version before deploying the new one. +On traditional deployments (on-prem/non-managed cloud), upgrading Kibana is done with downtime. +The upgrade process requires shutting down all the nodes of the prior version before deploying the new one. That way, there is always a single version of Kibana running at a given time, which avoids all risks of data incompatibility between version (e.g the new version introduces a migration that changes the shape of the document in a way that breaks compatibility with the previous version) @@ -75,9 +75,9 @@ with the previous version) For serverless however, the same process can't be used, as we need to be able to upgrade Kibana without interruption of service. Which means that the old and new version of Kibana will have to cohabitate for a time. -This leads to a lot of constraints regarding what can, or cannot, be done with data transformations (migrations) during an upgrade. +This leads to a lot of constraints regarding what can, or cannot, be done with data transformations (migrations) during an upgrade. And, unsurprisingly, the existing migration API (which allows to register any kind of *(doc) => doc* transformations) was way too permissive and -unsafe given our backward compatibility requirements. +unsafe given our backward compatibility requirements. ## Defining model versions @@ -87,7 +87,7 @@ When registering a SO type, a new [modelVersions](https://github.com/elastic/kib property is available. This attribute is a map of [SavedObjectsModelVersion](https://github.com/elastic/kibana/blob/f0eb5d695745f1f3a19ae6392618d1826ce29ce2/src/core/packages/saved-objects/server/src/model_version/model_version.ts#L13-L21) which is the top-level type/container to define model versions. -This map follows a similar `{ [version number] => version definition }` format as the old migration map, however +This map follows a similar `{ [version number] => version definition }` format as the old migration map, however a given SO type's model version is now identified by a single integer. The first version must be numbered as version 1, incrementing by one for each new version. @@ -157,7 +157,7 @@ const myType: SavedObjectsType = { }; ``` -**Note:** Having multiple changes of the same type for a given version is supported by design +**Note:** Having multiple changes of the same type for a given version is supported by design to allow merging different sources (to prepare for an eventual higher-level API) *This definition would be perfectly valid:* @@ -188,9 +188,9 @@ It's currently composed of two main properties: [link to the TS doc for `changes`](https://github.com/elastic/kibana/blob/f0eb5d695745f1f3a19ae6392618d1826ce29ce2/src/core/packages/saved-objects/server/src/model_version/model_version.ts#L22-L73) -Describes the list of changes applied during this version. +Describes the list of changes applied during this version. -**Important:** This is the part that replaces the old migration system, and allows defining when a version adds new mapping, +**Important:** This is the part that replaces the old migration system, and allows defining when a version adds new mapping, mutates the documents, or other type-related changes. The current types of changes are: @@ -249,12 +249,12 @@ let change: SavedObjectsModelDataBackfillChange = { }; ``` -**note:** *Even if no check is performed to ensure it, this type of model change should only be used to +**note:** *Even if no check is performed to ensure it, this type of model change should only be used to backfill newly introduced fields.* #### - data_removal -Used to remove data (unset fields) from all documents of the type. +Used to remove data (unset fields) from all documents of the type. *Usage example:* @@ -276,12 +276,24 @@ Used to execute an arbitrary transformation function. *Usage example:* ```ts -let change: SavedObjectsModelUnsafeTransformChange = { +// Please define your transform function on a separate const. +// Use explicit types for the generic arguments, as shown below. +// This will reduce the chances of introducing bugs. +const transformFn: SavedObjectModelUnsafeTransformFn = ( + doc: SavedObjectModelTransformationDoc +) => { + const attributes: AfterType = { + ...doc.attributes, + someAddedField: 'defaultValue', + }; + + return { document: { ...doc, attributes } }; +}; + +// this is how you would specify a change in the changes: [] +const change: SavedObjectsModelUnsafeTransformChange = { type: 'unsafe_transform', - transformFn: (document) => { - document.attributes.someAddedField = 'defaultValue'; - return { document }; - }, + transformFn: (typeSafeGuard) => typeSafeGuard(transformFn), }; ``` @@ -306,7 +318,7 @@ This is a new concept introduced by model versions. This schema is used for inte When retrieving a savedObject document from an index, if the version of the document is higher than the latest version known of the Kibana instance, the document will go through the `forwardCompatibility` schema of the associated model version. -**Important:** These conversion mechanism shouldn't assert the data itself, and only strip unknown fields to convert the document to +**Important:** These conversion mechanism shouldn't assert the data itself, and only strip unknown fields to convert the document to the **shape** of the document at the given version. Basically, this schema should keep all the known fields of a given version, and remove all the unknown fields, without throwing. @@ -352,7 +364,7 @@ definition, now directly included in the model version definition. As a refresher the `create` schema is a `@kbn/config-schema` object-type schema, and is used to validate the properties of the document during `create` and `bulkCreate` operations. -**note:** *Implementing this schema is optional, but still recommended, as otherwise there will be no validating when +**note:** *Implementing this schema is optional, but still recommended, as otherwise there will be no validating when importing objects* ## Use-case examples @@ -402,7 +414,7 @@ const myType: SavedObjectsType = { From here, say we want to introduce a new `dolly` field that is not indexed, and that we don't need to populate with a default value. -To achieve that, we need to introduce a new model version, with the only thing to do will be to define the +To achieve that, we need to introduce a new model version, with the only thing to do will be to define the associated schemas to include this new field. The added model version would look like: @@ -425,7 +437,7 @@ let modelVersion2: SavedObjectsModelVersion = { }; ``` -The full type definition after the addition of the new model version: +The full type definition after the addition of the new model version: ```ts const myType: SavedObjectsType = { @@ -470,7 +482,7 @@ const myType: SavedObjectsType = { ### Adding an indexed field without default value This scenario is fairly close to the previous one. The difference being that working with an indexed field means -adding a `mappings_addition` change and to also update the root mappings accordingly. +adding a `mappings_addition` change and to also update the root mappings accordingly. To reuse the previous example, let's say the `dolly` field we want to add would need to be indexed instead. @@ -479,7 +491,7 @@ In that case, the new version needs to do the following: - update the root `mappings` accordingly - add the updated schemas as we did for the previous example -The new version definition would look like: +The new version definition would look like: ```ts let modelVersion2: SavedObjectsModelVersion = { @@ -721,7 +733,7 @@ From here, say we want to remove the `removed` field, as our application doesn't The first thing to understand here is the impact toward backward compatibility: Say that Kibana version `X` was still using this field, and that we stopped utilizing the field in version `X+1`. -We can't remove the data in version `X+1`, as we need to be able to rollback to the prior version at **any time**. +We can't remove the data in version `X+1`, as we need to be able to rollback to the prior version at **any time**. If we were to delete the data of this `removed` field during the upgrade to version `X+1`, and if then, for any reason, we'd need to rollback to version `X`, it would cause a data loss, as version `X` was still using this field, but it would no longer present in our document after the rollback. @@ -815,11 +827,11 @@ let modelVersion3: SavedObjectsModelVersion = { ], schemas: { forwardCompatibility: schema.object( - { kept: schema.string() }, + { kept: schema.string() }, { unknowns: 'ignore' } ), create: schema.object( - { kept: schema.string() }, + { kept: schema.string() }, ) }, }; @@ -896,32 +908,32 @@ with model version and their associated transformations. ### Tooling for unit tests -For unit tests, the package exposes utilities to easily test the impact of transforming documents +For unit tests, the package exposes utilities to easily test the impact of transforming documents from a model version to another one, either upward or backward. #### Model version test migrator -The `createModelVersionTestMigrator` helper allows to create a test migrator that can be used to +The `createModelVersionTestMigrator` helper allows to create a test migrator that can be used to test model version changes between versions, by transforming documents the same way the migration algorithm would during an upgrade. **Example:** ```ts -import { - createModelVersionTestMigrator, - type ModelVersionTestMigrator +import { + createModelVersionTestMigrator, + type ModelVersionTestMigrator } from '@kbn/core-test-helpers-model-versions'; const mySoTypeDefinition = someSoType(); describe('mySoTypeDefinition model version transformations', () => { let migrator: ModelVersionTestMigrator; - + beforeEach(() => { migrator = createModelVersionTestMigrator({ type: mySoTypeDefinition }); }); - + describe('Model version 2', () => { it('properly backfill the expected fields when converting from v1 to v2', () => { const obj = createSomeSavedObject(); @@ -955,7 +967,7 @@ describe('mySoTypeDefinition model version transformations', () => { During integration tests, we can boot a real Elasticsearch cluster, allowing us to manipulate SO documents in a way almost similar to how it would be done on production runtime. With integration tests, we can even simulate the cohabitation of two Kibana instances with different model versions -to assert the behavior of their interactions. +to assert the behavior of their interactions. #### Model version test bed @@ -966,7 +978,7 @@ and to initiate the migration between the two versions we're testing. **Example:** ```ts -import { +import { createModelVersionTestBed, type ModelVersionTestKit } from '@kbn/core-test-helpers-model-versions'; @@ -1032,7 +1044,7 @@ test bed currently has some limitations: ## Limitations and edge cases in serverless environments The serverless environment, and the fact that upgrade in such environments are performed in a way -where, at some point, the old and new version of the application are living in cohabitation, leads +where, at some point, the old and new version of the application are living in cohabitation, leads to some particularities regarding the way the SO APIs works, and to some limitations / edge case that we need to document diff --git a/src/core/packages/saved-objects/server/src/model_version/model_change.test.ts b/src/core/packages/saved-objects/server/src/model_version/model_change.test.ts new file mode 100644 index 000000000000..7b8a43ff631c --- /dev/null +++ b/src/core/packages/saved-objects/server/src/model_version/model_change.test.ts @@ -0,0 +1,96 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +import { SavedObjectDoc } from '../serialization'; +import { SavedObjectsModelUnsafeTransformChange } from './model_change'; +import { + SavedObjectModelTransformationContext, + SavedObjectModelTransformationDoc, + SavedObjectModelUnsafeTransformFn, +} from './transformations'; + +interface BeforeType { + a: boolean; +} + +interface AfterType extends BeforeType { + aString: string; +} + +describe('test', () => { + const testDoc: SavedObjectDoc = { + id: 'someType:docId', + type: 'someType', + attributes: { + a: false, + }, + }; + const testContext: SavedObjectModelTransformationContext = { + log: { + debug: jest.fn(), + info: jest.fn(), + warn: jest.fn(), + error: jest.fn(), + }, + modelVersion: 1, + namespaceType: 'agnostic', + }; + + it('TS fails if users try to define untyped transform functions', () => { + const untypedTransformFn: SavedObjectModelUnsafeTransformFn = (doc) => { + const attributes: AfterType = { + // @ts-expect-error + ...doc.attributes, + // @ts-expect-error + aString: doc.attributes.a ? 'true' : 'false', + }; + + return { document: { ...doc, attributes } }; + }; + + expect(untypedTransformFn(testDoc, testContext)).toMatchInlineSnapshot(` + Object { + "document": Object { + "attributes": Object { + "a": false, + "aString": "false", + }, + "id": "someType:docId", + "type": "someType", + }, + } + `); + }); + + it('allows defining transform changes', () => { + const transformFn: SavedObjectModelUnsafeTransformFn = ( + doc: SavedObjectModelTransformationDoc + ) => { + const attributes: AfterType = { + ...doc.attributes, + aString: doc.attributes.a ? 'true' : 'false', + }; + + return { document: { ...doc, attributes } }; + }; + + // this is how you would specify a change in the changes: [] + const change: SavedObjectsModelUnsafeTransformChange = { + type: 'unsafe_transform', + transformFn: (typeSafeGuard) => typeSafeGuard(transformFn), + }; + + expect(change).toMatchInlineSnapshot(` + Object { + "transformFn": [Function], + "type": "unsafe_transform", + } + `); + }); +}); diff --git a/src/core/packages/saved-objects/server/src/model_version/model_change.ts b/src/core/packages/saved-objects/server/src/model_version/model_change.ts index a8b2d62e3509..e3f3d52638e1 100644 --- a/src/core/packages/saved-objects/server/src/model_version/model_change.ts +++ b/src/core/packages/saved-objects/server/src/model_version/model_change.ts @@ -10,6 +10,7 @@ import type { SavedObjectsMappingProperties } from '../mapping_definition'; import type { SavedObjectModelDataBackfillFn, + SavedObjectModelTransformationFn, SavedObjectModelUnsafeTransformFn, } from './transformations'; @@ -143,15 +144,26 @@ export interface SavedObjectsModelDataRemovalChange { /** * A {@link SavedObjectsModelChange | model change} executing an arbitrary transformation function. - * + * Please define your transform function on a separate const. + * Use explicit types for the generic arguments, as shown below. + * This will reduce the chances of introducing bugs. * @example * ```ts - * let change: SavedObjectsModelUnsafeTransformChange = { + * const transformFn: SavedObjectModelUnsafeTransformFn = ( + * doc: SavedObjectModelTransformationDoc + * ) => { + * const attributes: AfterType = { + * ...doc.attributes, + * someAddedField: 'defaultValue', + * }; + * + * return { document: { ...doc, attributes } }; + * }; + * + * // this is how you would specify a change in the changes: [] + * const change: SavedObjectsModelUnsafeTransformChange = { * type: 'unsafe_transform', - * transformFn: (document) => { - * document.attributes.someAddedField = 'defaultValue'; - * return { document }; - * }, + * transformFn: (typeSafeGuard) => typeSafeGuard(transformFn), * }; * ``` * @@ -160,13 +172,14 @@ export interface SavedObjectsModelDataRemovalChange { * Those should only be used when there's no other way to cover one's migration needs. * Please reach out to the Core team if you think you need to use this, as you theoretically shouldn't. */ -export interface SavedObjectsModelUnsafeTransformChange< - PreviousAttributes = any, - NewAttributes = any -> { +export interface SavedObjectsModelUnsafeTransformChange { type: 'unsafe_transform'; /** * The transform function to execute. */ - transformFn: SavedObjectModelUnsafeTransformFn; + transformFn: ( + typeSafeGuard: ( + fn: SavedObjectModelUnsafeTransformFn + ) => SavedObjectModelTransformationFn + ) => SavedObjectModelTransformationFn; } diff --git a/src/core/server/integration_tests/saved_objects/migrations/kibana_migrator_test_kit.fixtures.ts b/src/core/server/integration_tests/saved_objects/migrations/kibana_migrator_test_kit.fixtures.ts index c7c688f4c60f..63f6d9688624 100644 --- a/src/core/server/integration_tests/saved_objects/migrations/kibana_migrator_test_kit.fixtures.ts +++ b/src/core/server/integration_tests/saved_objects/migrations/kibana_migrator_test_kit.fixtures.ts @@ -8,7 +8,10 @@ */ import type { SavedObjectsBulkCreateObject } from '@kbn/core-saved-objects-api-server'; -import type { SavedObjectsType } from '@kbn/core-saved-objects-server'; +import type { + SavedObjectModelUnsafeTransformFn, + SavedObjectsType, +} from '@kbn/core-saved-objects-server'; import type { IndexTypesMap } from '@kbn/core-saved-objects-base-server-internal'; import type { ElasticsearchClientWrapperFactory } from './elasticsearch_client_wrapper'; import { @@ -45,6 +48,18 @@ const defaultType: SavedObjectsType = { export const REMOVED_TYPES = ['deprecated', 'server']; +interface ComplexTypeV0 { + name: string; + value: number; + firstHalf: boolean; +} + +interface ComplexTypeV1 { + name: string; + value: number; + firstHalf: boolean; +} + export const baselineTypes: Array> = [ { ...defaultType, @@ -123,8 +138,18 @@ export const getCompatibleBaselineTypes = (removedTypes: string[]) => } }); -export const getReindexingBaselineTypes = (removedTypes: string[]) => - getUpToDateBaselineTypes(removedTypes).map((type) => { +export const getReindexingBaselineTypes = (removedTypes: string[]) => { + const transformComplex: SavedObjectModelUnsafeTransformFn = ( + doc + ) => { + if (doc.attributes.value % 100 === 0) { + throw new Error( + `Cannot convert 'complex' objects with values that are multiple of 100 ${doc.id}` + ); + } + return { document: doc }; + }; + return getUpToDateBaselineTypes(removedTypes).map((type) => { // introduce an incompatible change if (type.name === 'complex') { return { @@ -152,14 +177,7 @@ export const getReindexingBaselineTypes = (removedTypes: string[]) => }, { type: 'unsafe_transform', - transformFn: (doc) => { - if (doc.attributes.value % 100 === 0) { - throw new Error( - `Cannot convert 'complex' objects with values that are multiple of 100 ${doc.id}` - ); - } - return { document: doc }; - }, + transformFn: (typeSafeGuard) => typeSafeGuard(transformComplex), }, ], }, @@ -192,6 +210,7 @@ export const getReindexingBaselineTypes = (removedTypes: string[]) => return type; } }); +}; export interface GetBaselineDocumentsParams { documentsPerType?: number; diff --git a/src/core/server/integration_tests/saved_objects/migrations/zdt_1/conversion_failures.test.ts b/src/core/server/integration_tests/saved_objects/migrations/zdt_1/conversion_failures.test.ts index 4509cd02ff4e..4e58eece0419 100644 --- a/src/core/server/integration_tests/saved_objects/migrations/zdt_1/conversion_failures.test.ts +++ b/src/core/server/integration_tests/saved_objects/migrations/zdt_1/conversion_failures.test.ts @@ -107,9 +107,10 @@ describe('ZDT upgrades - encountering conversion failures', () => { changes: [ { type: 'unsafe_transform', - transformFn: (doc) => { - throw new Error(`error from ${doc.id}`); - }, + transformFn: (typeSafeGuard) => + typeSafeGuard((doc) => { + throw new Error(`error from ${doc.id}`); + }), }, ], }, @@ -122,12 +123,13 @@ describe('ZDT upgrades - encountering conversion failures', () => { changes: [ { type: 'unsafe_transform', - transformFn: (doc) => { - if (doc.id === 'b-0') { - throw new Error(`error from ${doc.id}`); - } - return { document: doc }; - }, + transformFn: (typeSafeGuard) => + typeSafeGuard((doc) => { + if (doc.id === 'b-0') { + throw new Error(`error from ${doc.id}`); + } + return { document: doc }; + }), }, ], }, diff --git a/src/core/test-helpers/model-versions/src/model_version_tester.test.ts b/src/core/test-helpers/model-versions/src/model_version_tester.test.ts index 9dac1c85842a..e65fadc34991 100644 --- a/src/core/test-helpers/model-versions/src/model_version_tester.test.ts +++ b/src/core/test-helpers/model-versions/src/model_version_tester.test.ts @@ -8,7 +8,11 @@ */ import { schema } from '@kbn/config-schema'; -import type { SavedObjectsType, SavedObject } from '@kbn/core-saved-objects-server'; +import type { + SavedObjectsType, + SavedObject, + SavedObjectModelUnsafeTransformFn, +} from '@kbn/core-saved-objects-server'; import { createModelVersionTestMigrator } from './model_version_tester'; const createObject = (parts: Partial): SavedObject => { @@ -22,6 +26,26 @@ const createObject = (parts: Partial): SavedObject => { }; describe('modelVersionTester', () => { + interface V3 { + someExistingField: string; + } + + interface V4 extends V3 { + fieldUnsafelyAddedInV4: string; + } + + const testTypeUnsafeTransform: SavedObjectModelUnsafeTransformFn = (doc) => { + const transformedDoc = { + ...doc, + attributes: { + ...doc.attributes, + fieldUnsafelyAddedInV4: '4', + }, + }; + + return { document: transformedDoc }; + }; + const testType: SavedObjectsType = { name: 'test-type', hidden: false, @@ -90,14 +114,7 @@ describe('modelVersionTester', () => { changes: [ { type: 'unsafe_transform', - transformFn: (doc) => { - doc.attributes = { - ...doc.attributes, - fieldUnsafelyAddedInV4: '4', - }; - - return { document: doc }; - }, + transformFn: (typeSafeGuard) => typeSafeGuard(testTypeUnsafeTransform), }, ], schemas: { diff --git a/x-pack/platform/plugins/shared/encrypted_saved_objects/server/create_model_version.test.ts b/x-pack/platform/plugins/shared/encrypted_saved_objects/server/create_model_version.test.ts index 89d9cb3e617b..87f228df29db 100644 --- a/x-pack/platform/plugins/shared/encrypted_saved_objects/server/create_model_version.test.ts +++ b/x-pack/platform/plugins/shared/encrypted_saved_objects/server/create_model_version.test.ts @@ -9,6 +9,10 @@ import { logger } from 'elastic-apm-node'; import type { SavedObjectModelTransformationContext, + SavedObjectModelTransformationDoc, + SavedObjectModelTransformationFn, + SavedObjectModelTransformationResult, + SavedObjectModelUnsafeTransformFn, SavedObjectsModelUnsafeTransformChange, } from '@kbn/core-saved-objects-server'; @@ -17,6 +21,15 @@ import type { EncryptedSavedObjectTypeRegistration } from './crypto'; import { EncryptionError, EncryptionErrorOperation } from './crypto'; import { encryptedSavedObjectsServiceMock } from './crypto/index.mock'; +const dummyTypeSafeGuard = ( + fn: SavedObjectModelUnsafeTransformFn +): SavedObjectModelTransformationFn => { + return ( + document: SavedObjectModelTransformationDoc, + ctx: SavedObjectModelTransformationContext + ): SavedObjectModelTransformationResult => fn(document, ctx); +}; + describe('create ESO model version', () => { afterEach(() => { jest.clearAllMocks(); @@ -47,9 +60,10 @@ describe('create ESO model version', () => { changes: [ { type: 'unsafe_transform', - transformFn: (document) => { - return { document }; - }, + transformFn: (typeSafeGuard) => + typeSafeGuard((document) => { + return { document }; + }), }, ], }, @@ -99,10 +113,11 @@ describe('create ESO model version', () => { changes: [ { type: 'unsafe_transform', - transformFn: (document) => { - document.attributes.three = '3'; - return { document }; - }, + transformFn: (typeSafeGuard) => + typeSafeGuard((document: SavedObjectModelTransformationDoc<{ three: string }>) => { + document.attributes.three = '3'; + return { document }; + }), }, { type: 'data_removal', @@ -110,10 +125,11 @@ describe('create ESO model version', () => { }, { type: 'unsafe_transform', - transformFn: (document) => { - document.attributes.two = '2'; - return { document: { ...document, new_prop_1: 'new prop 1' } }; - }, + transformFn: (typeSafeGuard) => + typeSafeGuard((document: SavedObjectModelTransformationDoc<{ two: string }>) => { + document.attributes.two = '2'; + return { document: { ...document, new_prop_1: 'new prop 1' } }; + }), }, { type: 'data_backfill', @@ -123,10 +139,11 @@ describe('create ESO model version', () => { }, { type: 'unsafe_transform', - transformFn: (document) => { - document.attributes.four = '4'; - return { document: { ...document, new_prop_2: 'new prop 2' } }; - }, + transformFn: (typeSafeGuard) => + typeSafeGuard((document: SavedObjectModelTransformationDoc<{ four: string }>) => { + document.attributes.four = '4'; + return { document: { ...document, new_prop_2: 'new prop 2' } }; + }), }, ], }, @@ -157,7 +174,7 @@ describe('create ESO model version', () => { ) as SavedObjectsModelUnsafeTransformChange[]; expect(unsafeTransforms.length === 1); - const result = unsafeTransforms[0].transformFn( + const result = unsafeTransforms[0].transformFn(dummyTypeSafeGuard)( { id: '123', type: 'known-type-1', @@ -204,9 +221,10 @@ describe('create ESO model version', () => { changes: [ { type: 'unsafe_transform', - transformFn: (document) => { - return { document }; - }, + transformFn: (typeSafeGuard) => + typeSafeGuard((document) => { + return { document }; + }), }, ], }, @@ -228,7 +246,7 @@ describe('create ESO model version', () => { ) as SavedObjectsModelUnsafeTransformChange[]; expect(unsafeTransforms.length === 1); expect(() => { - unsafeTransforms[0].transformFn( + unsafeTransforms[0].transformFn(dummyTypeSafeGuard)( { id: '123', type: 'known-type-1', @@ -265,9 +283,10 @@ describe('create ESO model version', () => { changes: [ { type: 'unsafe_transform', - transformFn: (document) => { - return { document }; - }, + transformFn: (typeSafeGuard) => + typeSafeGuard((document) => { + return { document }; + }), }, ], }, @@ -289,7 +308,7 @@ describe('create ESO model version', () => { ) as SavedObjectsModelUnsafeTransformChange[]; expect(unsafeTransforms.length === 1); expect(() => { - unsafeTransforms[0].transformFn( + unsafeTransforms[0].transformFn(dummyTypeSafeGuard)( { id: '123', type: 'known-type-1', @@ -327,9 +346,10 @@ describe('create ESO model version', () => { changes: [ { type: 'unsafe_transform', - transformFn: (document) => { - return { document }; - }, + transformFn: (typeSafeGuard) => + typeSafeGuard((document) => { + return { document }; + }), }, ], }, @@ -363,7 +383,7 @@ describe('create ESO model version', () => { (change) => change.type === 'unsafe_transform' ) as SavedObjectsModelUnsafeTransformChange[]; expect(unsafeTransforms.length === 1); - unsafeTransforms[0].transformFn( + unsafeTransforms[0].transformFn(dummyTypeSafeGuard)( { id: '123', type: 'known-type-1', @@ -406,9 +426,10 @@ describe('create ESO model version', () => { changes: [ { type: 'unsafe_transform', - transformFn: (document) => { - throw new Error('transform failed!'); - }, + transformFn: (typeSafeGuard) => + typeSafeGuard((document) => { + throw new Error('transform failed!'); + }), }, ], }, @@ -427,7 +448,7 @@ describe('create ESO model version', () => { ) as SavedObjectsModelUnsafeTransformChange[]; expect(unsafeTransforms.length === 1); expect(() => { - unsafeTransforms[0].transformFn( + unsafeTransforms[0].transformFn(dummyTypeSafeGuard)( { id: '123', type: 'known-type-1', @@ -464,9 +485,10 @@ describe('create ESO model version', () => { changes: [ { type: 'unsafe_transform', - transformFn: (document) => { - throw new Error('transform failed!'); - }, + transformFn: (typeSafeGuard) => + typeSafeGuard((document) => { + throw new Error('transform failed!'); + }), }, ], }, @@ -486,7 +508,7 @@ describe('create ESO model version', () => { ) as SavedObjectsModelUnsafeTransformChange[]; expect(unsafeTransforms.length === 1); expect(() => { - unsafeTransforms[0].transformFn( + unsafeTransforms[0].transformFn(dummyTypeSafeGuard)( { id: '123', type: 'known-type-1', @@ -523,9 +545,10 @@ describe('create ESO model version', () => { changes: [ { type: 'unsafe_transform', - transformFn: (document) => { - return { document }; - }, + transformFn: (typeSafeGuard) => + typeSafeGuard((document) => { + return { document }; + }), }, ], }, @@ -547,7 +570,7 @@ describe('create ESO model version', () => { ) as SavedObjectsModelUnsafeTransformChange[]; expect(unsafeTransforms.length === 1); expect(() => { - unsafeTransforms[0].transformFn( + unsafeTransforms[0].transformFn(dummyTypeSafeGuard)( { id: '123', type: 'known-type-1', @@ -591,9 +614,10 @@ describe('create ESO model version', () => { changes: [ { type: 'unsafe_transform', - transformFn: (document) => { - return { document }; - }, + transformFn: (typeSafeGuard) => + typeSafeGuard((document) => { + return { document }; + }), }, ], }, @@ -616,7 +640,7 @@ describe('create ESO model version', () => { ) as SavedObjectsModelUnsafeTransformChange[]; expect(unsafeTransforms.length === 1); expect(() => { - unsafeTransforms[0].transformFn( + unsafeTransforms[0].transformFn(dummyTypeSafeGuard)( { id: '123', type: 'known-type-1', @@ -667,16 +691,25 @@ describe('create ESO model version', () => { changes: [ { type: 'unsafe_transform', - transformFn: (document) => { - // modify an encrypted field - document.attributes.firstAttr = `~~${document.attributes.firstAttr}~~`; - // encrypt a non encrypted field if it's there - if (document.attributes.nonEncryptedAttr) { - document.attributes.encryptedAttr = document.attributes.nonEncryptedAttr; - delete document.attributes.nonEncryptedAttr; - } - return { document }; - }, + transformFn: (typeSafeGuard) => + typeSafeGuard( + ( + document: SavedObjectModelTransformationDoc<{ + firstAttr: string; + nonEncryptedAttr?: string; + encryptedAttr: string; + }> + ) => { + // modify an encrypted field + document.attributes.firstAttr = `~~${document.attributes.firstAttr}~~`; + // encrypt a non encrypted field if it's there + if (document.attributes.nonEncryptedAttr) { + document.attributes.encryptedAttr = document.attributes.nonEncryptedAttr; + delete document.attributes.nonEncryptedAttr; + } + return { document }; + } + ), }, ], }, @@ -703,7 +736,7 @@ describe('create ESO model version', () => { (change) => change.type === 'unsafe_transform' ) as SavedObjectsModelUnsafeTransformChange[]; expect(unsafeTransforms.length === 1); - const result = unsafeTransforms[0].transformFn( + const result = unsafeTransforms[0].transformFn(dummyTypeSafeGuard)( { id: '123', type: 'known-type-1', diff --git a/x-pack/platform/plugins/shared/encrypted_saved_objects/server/create_model_version.ts b/x-pack/platform/plugins/shared/encrypted_saved_objects/server/create_model_version.ts index c8e2a2eea761..38d772970173 100644 --- a/x-pack/platform/plugins/shared/encrypted_saved_objects/server/create_model_version.ts +++ b/x-pack/platform/plugins/shared/encrypted_saved_objects/server/create_model_version.ts @@ -73,7 +73,12 @@ export const getCreateEsoModelVersion = incomingChanges ); - return { ...modelVersion, changes: [{ type: 'unsafe_transform', transformFn }] }; + return { + ...modelVersion, + changes: [ + { type: 'unsafe_transform', transformFn: (typeSafeGuard) => typeSafeGuard(transformFn) }, + ], + }; }; function createMergedTransformFn( diff --git a/x-pack/platform/plugins/shared/entity_manager/server/saved_objects/entity_definition.ts b/x-pack/platform/plugins/shared/entity_manager/server/saved_objects/entity_definition.ts index f95e236e93f6..2c38f94bdaad 100644 --- a/x-pack/platform/plugins/shared/entity_manager/server/saved_objects/entity_definition.ts +++ b/x-pack/platform/plugins/shared/entity_manager/server/saved_objects/entity_definition.ts @@ -119,7 +119,7 @@ export const entityDefinition: SavedObjectsType = { changes: [ { type: 'unsafe_transform', - transformFn: removeOptionalIdentityFields, + transformFn: (typeSafeGuard) => typeSafeGuard(removeOptionalIdentityFields), }, ], }, diff --git a/x-pack/platform/test/encrypted_saved_objects_api_integration/plugins/api_consumer_plugin/server/index.ts b/x-pack/platform/test/encrypted_saved_objects_api_integration/plugins/api_consumer_plugin/server/index.ts index 19781d61bbc5..06f48f21ab3b 100644 --- a/x-pack/platform/test/encrypted_saved_objects_api_integration/plugins/api_consumer_plugin/server/index.ts +++ b/x-pack/platform/test/encrypted_saved_objects_api_integration/plugins/api_consumer_plugin/server/index.ts @@ -13,6 +13,7 @@ import type { SavedObjectsNamespaceType, SavedObjectUnsanitizedDoc, } from '@kbn/core/server'; +import type { SavedObjectModelTransformationDoc } from '@kbn/core-saved-objects-server'; import type { EncryptedSavedObjectsPluginSetup, EncryptedSavedObjectsPluginStart, @@ -337,13 +338,22 @@ function defineModelVersionWithMigration(core: CoreSetup, deps: Pl changes: [ { type: 'unsafe_transform', - transformFn: (document) => { - const { - attributes: { nonEncryptedAttribute }, - } = document; - document.attributes.nonEncryptedAttribute = `${nonEncryptedAttribute}-migrated`; - return { document }; - }, + transformFn: (typeSafeGuard) => + typeSafeGuard( + // ideally, we should use generic types for the whole function, defining it on a separate const + ( + document: SavedObjectModelTransformationDoc<{ + additionalEncryptedAttribute: string; + nonEncryptedAttribute: string; + }> + ) => { + const { + attributes: { nonEncryptedAttribute }, + } = document; + document.attributes.nonEncryptedAttribute = `${nonEncryptedAttribute}-migrated`; + return { document }; + } + ), }, ], }, @@ -356,11 +366,20 @@ function defineModelVersionWithMigration(core: CoreSetup, deps: Pl changes: [ { type: 'unsafe_transform', - transformFn: (document) => { - // clone and modify the non encrypted field - document.attributes.additionalEncryptedAttribute = `${document.attributes.nonEncryptedAttribute}-encrypted`; - return { document }; - }, + transformFn: (typeSafeGuard) => + typeSafeGuard( + // ideally, we should use generic types for the whole function, defining it on a separate const + ( + document: SavedObjectModelTransformationDoc<{ + additionalEncryptedAttribute: string; + nonEncryptedAttribute: string; + }> + ) => { + // clone and modify the non encrypted field + document.attributes.additionalEncryptedAttribute = `${document.attributes.nonEncryptedAttribute}-encrypted`; + return { document }; + } + ), }, ], }, diff --git a/x-pack/platform/test/encrypted_saved_objects_api_integration/plugins/api_consumer_plugin/tsconfig.json b/x-pack/platform/test/encrypted_saved_objects_api_integration/plugins/api_consumer_plugin/tsconfig.json index 9a45ef613419..6bad1eadbcb1 100644 --- a/x-pack/platform/test/encrypted_saved_objects_api_integration/plugins/api_consumer_plugin/tsconfig.json +++ b/x-pack/platform/test/encrypted_saved_objects_api_integration/plugins/api_consumer_plugin/tsconfig.json @@ -16,5 +16,6 @@ "@kbn/std", "@kbn/encrypted-saved-objects-plugin", "@kbn/spaces-plugin", + "@kbn/core-saved-objects-server", ] } diff --git a/x-pack/solutions/observability/plugins/infra/server/saved_objects/inventory_view/inventory_view_saved_object.ts b/x-pack/solutions/observability/plugins/infra/server/saved_objects/inventory_view/inventory_view_saved_object.ts index 3a4864ab3fb0..b5803def38c4 100644 --- a/x-pack/solutions/observability/plugins/infra/server/saved_objects/inventory_view/inventory_view_saved_object.ts +++ b/x-pack/solutions/observability/plugins/infra/server/saved_objects/inventory_view/inventory_view_saved_object.ts @@ -5,10 +5,15 @@ * 2.0. */ +import type { TypeOf } from '@kbn/config-schema'; import { schema } from '@kbn/config-schema'; import { fold } from 'fp-ts/Either'; import { pipe } from 'fp-ts/pipeable'; import type { SavedObject, SavedObjectsType } from '@kbn/core/server'; +import type { + SavedObjectModelTransformationDoc, + SavedObjectModelUnsafeTransformFn, +} from '@kbn/core-saved-objects-server'; import { inventoryViewSavedObjectRT } from './types'; export const inventoryViewSavedObjectName = 'inventory-view'; @@ -32,6 +37,37 @@ const schemaV2 = schema.object( { unknowns: 'allow' } ); +type V1 = TypeOf; +type V2 = TypeOf; + +const inventoryV2Transform: SavedObjectModelUnsafeTransformFn = (doc) => { + // steps property did exist, even though it wasn't present in the schema + const asV2 = doc as SavedObjectModelTransformationDoc; + + if (typeof asV2.attributes.legend?.steps === 'undefined') { + return { document: asV2 }; + } else { + let steps = asV2.attributes.legend?.steps; + if (steps > 18) { + steps = 18; + } else if (steps < 2) { + steps = 2; + } + + const document: SavedObjectModelTransformationDoc = { + ...asV2, + attributes: { + ...asV2.attributes, + legend: { + ...asV2.attributes.legend, + steps, + }, + }, + }; + return { document }; + } +}; + export const inventoryViewSavedObjectType: SavedObjectsType = { name: inventoryViewSavedObjectName, hidden: false, @@ -58,14 +94,7 @@ export const inventoryViewSavedObjectType: SavedObjectsType = { changes: [ { type: 'unsafe_transform', - transformFn: (document) => { - if (document.attributes.legend?.steps > 18) { - document.attributes.legend.steps = 18; - } else if (document.attributes.legend?.steps < 2) { - document.attributes.legend.steps = 2; - } - return { document }; - }, + transformFn: (typeSafeGuard) => typeSafeGuard(inventoryV2Transform), }, ], schemas: { diff --git a/x-pack/solutions/observability/plugins/infra/tsconfig.json b/x-pack/solutions/observability/plugins/infra/tsconfig.json index 2313b8b9fb4b..8175565596ea 100644 --- a/x-pack/solutions/observability/plugins/infra/tsconfig.json +++ b/x-pack/solutions/observability/plugins/infra/tsconfig.json @@ -119,7 +119,8 @@ "@kbn/core-chrome-browser", "@kbn/presentation-containers", "@kbn/object-utils", - "@kbn/coloring" + "@kbn/coloring", + "@kbn/core-saved-objects-server" ], "exclude": ["target/**/*"] } diff --git a/x-pack/solutions/observability/plugins/synthetics/server/saved_objects/migrations/private_locations/model_version_1.ts b/x-pack/solutions/observability/plugins/synthetics/server/saved_objects/migrations/private_locations/model_version_1.ts index 2b7d5be016bf..0b591c36d9e9 100644 --- a/x-pack/solutions/observability/plugins/synthetics/server/saved_objects/migrations/private_locations/model_version_1.ts +++ b/x-pack/solutions/observability/plugins/synthetics/server/saved_objects/migrations/private_locations/model_version_1.ts @@ -58,7 +58,7 @@ export const modelVersion1: SavedObjectsModelVersion = { changes: [ { type: 'unsafe_transform', - transformFn: transformGeoProperty, + transformFn: (typeSafeGuard) => typeSafeGuard(transformGeoProperty), }, ], };