From 40479ec0f02f3f6d11767cf4e0be617baff66dcd Mon Sep 17 00:00:00 2001 From: Rudolf Meijering Date: Thu, 9 Jul 2020 15:55:23 +0200 Subject: [PATCH] Disable fields added from unknown saved object types (#70951) (#71225) * Allow disabled: false SO field mappings * Disable fields for unknown SO types * Update everyone else's docs ;) * Address review comments * Add unit tests for disableUnknownTypeMappingFields() --- .../core/server/kibana-plugin-core-server.md | 2 +- ...savedobjectscomplexfieldmapping.dynamic.md | 15 ++++ ...savedobjectscomplexfieldmapping.enabled.md | 11 +++ ...-server.savedobjectscomplexfieldmapping.md | 4 +- ...er.savedobjectscorefieldmapping.enabled.md | 11 --- ...ore-server.savedobjectscorefieldmapping.md | 1 - ...vedobjectstypemappingdefinition.dynamic.md | 2 +- ...erver.savedobjectstypemappingdefinition.md | 2 +- .../server/saved_objects/mappings/types.ts | 20 +++-- .../migrations/core/index_migrator.test.ts | 65 +++++++++++++- .../migrations/core/migration_context.test.ts | 88 +++++++++++++++++++ .../migrations/core/migration_context.ts | 70 +++++++++++++-- src/core/server/server.api.md | 5 +- 13 files changed, 258 insertions(+), 38 deletions(-) create mode 100644 docs/development/core/server/kibana-plugin-core-server.savedobjectscomplexfieldmapping.dynamic.md create mode 100644 docs/development/core/server/kibana-plugin-core-server.savedobjectscomplexfieldmapping.enabled.md delete mode 100644 docs/development/core/server/kibana-plugin-core-server.savedobjectscorefieldmapping.enabled.md create mode 100644 src/core/server/saved_objects/migrations/core/migration_context.test.ts diff --git a/docs/development/core/server/kibana-plugin-core-server.md b/docs/development/core/server/kibana-plugin-core-server.md index 7ebd0531619fd..8d4c0c915437e 100644 --- a/docs/development/core/server/kibana-plugin-core-server.md +++ b/docs/development/core/server/kibana-plugin-core-server.md @@ -154,7 +154,7 @@ The plugin integrates with the core system via lifecycle events: `setup` | [SavedObjectsBulkUpdateResponse](./kibana-plugin-core-server.savedobjectsbulkupdateresponse.md) | | | [SavedObjectsClientProviderOptions](./kibana-plugin-core-server.savedobjectsclientprovideroptions.md) | Options to control the creation of the Saved Objects Client. | | [SavedObjectsClientWrapperOptions](./kibana-plugin-core-server.savedobjectsclientwrapperoptions.md) | Options passed to each SavedObjectsClientWrapperFactory to aid in creating the wrapper instance. | -| [SavedObjectsComplexFieldMapping](./kibana-plugin-core-server.savedobjectscomplexfieldmapping.md) | See [SavedObjectsFieldMapping](./kibana-plugin-core-server.savedobjectsfieldmapping.md) for documentation.Note: this type intentially doesn't include a type definition for defining the dynamic mapping parameter. Saved Object fields should always inherit the dynamic: 'strict' paramater. If you are unsure of the shape of your data use type: 'object', enabled: false instead. | +| [SavedObjectsComplexFieldMapping](./kibana-plugin-core-server.savedobjectscomplexfieldmapping.md) | See [SavedObjectsFieldMapping](./kibana-plugin-core-server.savedobjectsfieldmapping.md) for documentation. | | [SavedObjectsCoreFieldMapping](./kibana-plugin-core-server.savedobjectscorefieldmapping.md) | See [SavedObjectsFieldMapping](./kibana-plugin-core-server.savedobjectsfieldmapping.md) for documentation. | | [SavedObjectsCreateOptions](./kibana-plugin-core-server.savedobjectscreateoptions.md) | | | [SavedObjectsDeleteByNamespaceOptions](./kibana-plugin-core-server.savedobjectsdeletebynamespaceoptions.md) | | diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectscomplexfieldmapping.dynamic.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectscomplexfieldmapping.dynamic.md new file mode 100644 index 0000000000000..b01da3c62fda6 --- /dev/null +++ b/docs/development/core/server/kibana-plugin-core-server.savedobjectscomplexfieldmapping.dynamic.md @@ -0,0 +1,15 @@ + + +[Home](./index.md) > [kibana-plugin-core-server](./kibana-plugin-core-server.md) > [SavedObjectsComplexFieldMapping](./kibana-plugin-core-server.savedobjectscomplexfieldmapping.md) > [dynamic](./kibana-plugin-core-server.savedobjectscomplexfieldmapping.dynamic.md) + +## SavedObjectsComplexFieldMapping.dynamic property + +The dynamic property of the mapping, either `false` or `'strict'`. If unspecified `dynamic: 'strict'` will be inherited from the top-level index mappings. + +Note: To limit the number of mapping fields Saved Object types should \*never\* use `dynamic: true`. + +Signature: + +```typescript +dynamic?: false | 'strict'; +``` diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectscomplexfieldmapping.enabled.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectscomplexfieldmapping.enabled.md new file mode 100644 index 0000000000000..08513aa2a849b --- /dev/null +++ b/docs/development/core/server/kibana-plugin-core-server.savedobjectscomplexfieldmapping.enabled.md @@ -0,0 +1,11 @@ + + +[Home](./index.md) > [kibana-plugin-core-server](./kibana-plugin-core-server.md) > [SavedObjectsComplexFieldMapping](./kibana-plugin-core-server.savedobjectscomplexfieldmapping.md) > [enabled](./kibana-plugin-core-server.savedobjectscomplexfieldmapping.enabled.md) + +## SavedObjectsComplexFieldMapping.enabled property + +Signature: + +```typescript +enabled?: boolean; +``` diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectscomplexfieldmapping.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectscomplexfieldmapping.md index cb81686b424ec..fc262cad54f18 100644 --- a/docs/development/core/server/kibana-plugin-core-server.savedobjectscomplexfieldmapping.md +++ b/docs/development/core/server/kibana-plugin-core-server.savedobjectscomplexfieldmapping.md @@ -6,8 +6,6 @@ See [SavedObjectsFieldMapping](./kibana-plugin-core-server.savedobjectsfieldmapping.md) for documentation. -Note: this type intentially doesn't include a type definition for defining the `dynamic` mapping parameter. Saved Object fields should always inherit the `dynamic: 'strict'` paramater. If you are unsure of the shape of your data use `type: 'object', enabled: false` instead. - Signature: ```typescript @@ -19,6 +17,8 @@ export interface SavedObjectsComplexFieldMapping | Property | Type | Description | | --- | --- | --- | | [doc\_values](./kibana-plugin-core-server.savedobjectscomplexfieldmapping.doc_values.md) | boolean | | +| [dynamic](./kibana-plugin-core-server.savedobjectscomplexfieldmapping.dynamic.md) | false | 'strict' | The dynamic property of the mapping, either false or 'strict'. If unspecified dynamic: 'strict' will be inherited from the top-level index mappings.Note: To limit the number of mapping fields Saved Object types should \*never\* use dynamic: true. | +| [enabled](./kibana-plugin-core-server.savedobjectscomplexfieldmapping.enabled.md) | boolean | | | [properties](./kibana-plugin-core-server.savedobjectscomplexfieldmapping.properties.md) | SavedObjectsMappingProperties | | | [type](./kibana-plugin-core-server.savedobjectscomplexfieldmapping.type.md) | string | | diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectscorefieldmapping.enabled.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectscorefieldmapping.enabled.md deleted file mode 100644 index c0b556e99ebc3..0000000000000 --- a/docs/development/core/server/kibana-plugin-core-server.savedobjectscorefieldmapping.enabled.md +++ /dev/null @@ -1,11 +0,0 @@ - - -[Home](./index.md) > [kibana-plugin-core-server](./kibana-plugin-core-server.md) > [SavedObjectsCoreFieldMapping](./kibana-plugin-core-server.savedobjectscorefieldmapping.md) > [enabled](./kibana-plugin-core-server.savedobjectscorefieldmapping.enabled.md) - -## SavedObjectsCoreFieldMapping.enabled property - -Signature: - -```typescript -enabled?: boolean; -``` diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectscorefieldmapping.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectscorefieldmapping.md index b9e726eac799d..e9b9c2bcf51b5 100644 --- a/docs/development/core/server/kibana-plugin-core-server.savedobjectscorefieldmapping.md +++ b/docs/development/core/server/kibana-plugin-core-server.savedobjectscorefieldmapping.md @@ -17,7 +17,6 @@ export interface SavedObjectsCoreFieldMapping | Property | Type | Description | | --- | --- | --- | | [doc\_values](./kibana-plugin-core-server.savedobjectscorefieldmapping.doc_values.md) | boolean | | -| [enabled](./kibana-plugin-core-server.savedobjectscorefieldmapping.enabled.md) | boolean | | | [fields](./kibana-plugin-core-server.savedobjectscorefieldmapping.fields.md) | {
[subfield: string]: {
type: string;
ignore_above?: number;
};
} | | | [index](./kibana-plugin-core-server.savedobjectscorefieldmapping.index.md) | boolean | | | [null\_value](./kibana-plugin-core-server.savedobjectscorefieldmapping.null_value.md) | number | boolean | string | | diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectstypemappingdefinition.dynamic.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectstypemappingdefinition.dynamic.md index 74efa75768f9c..70775760ac77d 100644 --- a/docs/development/core/server/kibana-plugin-core-server.savedobjectstypemappingdefinition.dynamic.md +++ b/docs/development/core/server/kibana-plugin-core-server.savedobjectstypemappingdefinition.dynamic.md @@ -4,7 +4,7 @@ ## SavedObjectsTypeMappingDefinition.dynamic property -The dynamic property of the mapping. either `false` or 'strict'. Defaults to `false` +The dynamic property of the mapping, either `false` or `'strict'`. If unspecified `dynamic: 'strict'` will be inherited from the top-level index mappings. Signature: diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectstypemappingdefinition.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectstypemappingdefinition.md index 77ded4389c0a0..3d3b73880fa7f 100644 --- a/docs/development/core/server/kibana-plugin-core-server.savedobjectstypemappingdefinition.md +++ b/docs/development/core/server/kibana-plugin-core-server.savedobjectstypemappingdefinition.md @@ -41,6 +41,6 @@ const typeDefinition: SavedObjectsTypeMappingDefinition = { | Property | Type | Description | | --- | --- | --- | -| [dynamic](./kibana-plugin-core-server.savedobjectstypemappingdefinition.dynamic.md) | false | 'strict' | The dynamic property of the mapping. either false or 'strict'. Defaults to false | +| [dynamic](./kibana-plugin-core-server.savedobjectstypemappingdefinition.dynamic.md) | false | 'strict' | The dynamic property of the mapping, either false or 'strict'. If unspecified dynamic: 'strict' will be inherited from the top-level index mappings. | | [properties](./kibana-plugin-core-server.savedobjectstypemappingdefinition.properties.md) | SavedObjectsMappingProperties | The underlying properties of the type mapping | diff --git a/src/core/server/saved_objects/mappings/types.ts b/src/core/server/saved_objects/mappings/types.ts index 7521e4a4bee86..7a7955ee745e8 100644 --- a/src/core/server/saved_objects/mappings/types.ts +++ b/src/core/server/saved_objects/mappings/types.ts @@ -45,7 +45,9 @@ * @public */ export interface SavedObjectsTypeMappingDefinition { - /** The dynamic property of the mapping. either `false` or 'strict'. Defaults to `false` */ + /** The dynamic property of the mapping, either `false` or `'strict'`. If + * unspecified `dynamic: 'strict'` will be inherited from the top-level + * index mappings. */ dynamic?: false | 'strict'; /** The underlying properties of the type mapping */ properties: SavedObjectsMappingProperties; @@ -134,7 +136,6 @@ export interface SavedObjectsCoreFieldMapping { null_value?: number | boolean | string; index?: boolean; doc_values?: boolean; - enabled?: boolean; fields?: { [subfield: string]: { type: string; @@ -146,14 +147,19 @@ export interface SavedObjectsCoreFieldMapping { /** * See {@link SavedObjectsFieldMapping} for documentation. * - * Note: this type intentially doesn't include a type definition for defining - * the `dynamic` mapping parameter. Saved Object fields should always inherit - * the `dynamic: 'strict'` paramater. If you are unsure of the shape of your - * data use `type: 'object', enabled: false` instead. - * * @public */ export interface SavedObjectsComplexFieldMapping { + /** + * The dynamic property of the mapping, either `false` or `'strict'`. If + * unspecified `dynamic: 'strict'` will be inherited from the top-level + * index mappings. + * + * Note: To limit the number of mapping fields Saved Object types should + * *never* use `dynamic: true`. + */ + dynamic?: false | 'strict'; + enabled?: boolean; doc_values?: boolean; type?: string; properties: SavedObjectsMappingProperties; diff --git a/src/core/server/saved_objects/migrations/core/index_migrator.test.ts b/src/core/server/saved_objects/migrations/core/index_migrator.test.ts index 86c79cbfb5824..f8b203bf66d6a 100644 --- a/src/core/server/saved_objects/migrations/core/index_migrator.test.ts +++ b/src/core/server/saved_objects/migrations/core/index_migrator.test.ts @@ -151,7 +151,7 @@ describe('IndexMigrator', () => { ); }); - test('retains mappings from the previous index', async () => { + test('retains unknown core field mappings from the previous index', async () => { const { callCluster } = testOpts; testOpts.mappingProperties = { foo: { type: 'text' } }; @@ -162,7 +162,7 @@ describe('IndexMigrator', () => { aliases: {}, mappings: { properties: { - author: { type: 'text' }, + unknown_core_field: { type: 'text' }, }, }, }, @@ -187,7 +187,66 @@ describe('IndexMigrator', () => { }, }, properties: { - author: { type: 'text' }, + unknown_core_field: { type: 'text' }, + foo: { type: 'text' }, + migrationVersion: { dynamic: 'true', type: 'object' }, + namespace: { type: 'keyword' }, + namespaces: { type: 'keyword' }, + type: { type: 'keyword' }, + updated_at: { type: 'date' }, + references: { + type: 'nested', + properties: { + name: { type: 'keyword' }, + type: { type: 'keyword' }, + id: { type: 'keyword' }, + }, + }, + }, + }, + settings: { number_of_shards: 1, auto_expand_replicas: '0-1' }, + }, + index: '.kibana_2', + }); + }); + + test('disables complex field mappings from unknown types in the previous index', async () => { + const { callCluster } = testOpts; + + testOpts.mappingProperties = { foo: { type: 'text' } }; + + withIndex(callCluster, { + index: { + '.kibana_1': { + aliases: {}, + mappings: { + properties: { + unknown_complex_field: { properties: { description: { type: 'text' } } }, + }, + }, + }, + }, + }); + + await new IndexMigrator(testOpts).migrate(); + + expect(callCluster).toHaveBeenCalledWith('indices.create', { + body: { + mappings: { + dynamic: 'strict', + _meta: { + migrationMappingPropertyHashes: { + foo: '625b32086eb1d1203564cf85062dd22e', + migrationVersion: '4a1746014a75ade3a714e1db5763276f', + namespace: '2f4316de49999235636386fe51dc06c1', + namespaces: '2f4316de49999235636386fe51dc06c1', + references: '7997cf5a56cc02bdc9c93361bde732b0', + type: '2f4316de49999235636386fe51dc06c1', + updated_at: '00da57df13e94e9d98437d13ace4bfe0', + }, + }, + properties: { + unknown_complex_field: { dynamic: false, properties: {} }, foo: { type: 'text' }, migrationVersion: { dynamic: 'true', type: 'object' }, namespace: { type: 'keyword' }, diff --git a/src/core/server/saved_objects/migrations/core/migration_context.test.ts b/src/core/server/saved_objects/migrations/core/migration_context.test.ts new file mode 100644 index 0000000000000..34d8d94d5ddab --- /dev/null +++ b/src/core/server/saved_objects/migrations/core/migration_context.test.ts @@ -0,0 +1,88 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { disableUnknownTypeMappingFields } from './migration_context'; + +describe('disableUnknownTypeMappingFields', () => { + const sourceMappings = { + _meta: { + migrationMappingPropertyHashes: { + unknown_type: 'md5hash', + unknown_core_field: 'md5hash', + known_type: 'oldmd5hash', + }, + }, + properties: { + unknown_type: { + properties: { + unused_field: { type: 'text' }, + }, + }, + unknown_core_field: { type: 'keyword' }, + known_type: { + properties: { + field_1: { type: 'text' }, + old_field: { type: 'boolean' }, + }, + }, + }, + }; + const activeMappings = { + _meta: { + migrationMappingPropertyHashes: { + known_type: 'md5hash', + }, + }, + properties: { + known_type: { + properties: { + new_field: { type: 'binary' }, + field_1: { type: 'keyword' }, + }, + }, + }, + }; + const targetMappings = disableUnknownTypeMappingFields(activeMappings, sourceMappings); + + it('disables complex field mappings from unknown types in the source mappings', () => { + expect(targetMappings.properties.unknown_type).toEqual({ dynamic: false, properties: {} }); + }); + + it('retains unknown core field mappings from the source mappings', () => { + expect(targetMappings.properties.unknown_core_field).toEqual({ type: 'keyword' }); + }); + + it('overrides source mappings with known types from active mappings', () => { + expect(targetMappings.properties.known_type).toEqual({ + properties: { + new_field: { type: 'binary' }, + field_1: { type: 'keyword' }, // was type text in source mappings + // old_field was present in source but ommited in active mappings + }, + }); + }); + + it('retains the active mappings _meta ignoring any _meta fields in the source mappings', () => { + expect(targetMappings._meta).toEqual({ + migrationMappingPropertyHashes: { + known_type: 'md5hash', + }, + }); + }); +}); diff --git a/src/core/server/saved_objects/migrations/core/migration_context.ts b/src/core/server/saved_objects/migrations/core/migration_context.ts index 3a6145f5d9623..adf1851a1aa75 100644 --- a/src/core/server/saved_objects/migrations/core/migration_context.ts +++ b/src/core/server/saved_objects/migrations/core/migration_context.ts @@ -26,7 +26,11 @@ import { Logger } from 'src/core/server/logging'; import { SavedObjectsSerializer } from '../../serialization'; -import { SavedObjectsTypeMappingDefinitions } from '../../mappings'; +import { + SavedObjectsTypeMappingDefinitions, + SavedObjectsMappingProperties, + IndexMapping, +} from '../../mappings'; import { buildActiveMappings } from './build_active_mappings'; import { CallCluster } from './call_cluster'; import { VersionedTransformer } from './document_migrator'; @@ -107,20 +111,68 @@ function createSourceContext(source: FullIndexInfo, alias: string) { function createDestContext( source: FullIndexInfo, alias: string, - mappingProperties: SavedObjectsTypeMappingDefinitions + typeMappingDefinitions: SavedObjectsTypeMappingDefinitions ): FullIndexInfo { - const activeMappings = buildActiveMappings(mappingProperties); + const targetMappings = disableUnknownTypeMappingFields( + buildActiveMappings(typeMappingDefinitions), + source.mappings + ); return { aliases: {}, exists: false, indexName: nextIndexName(source.indexName, alias), - mappings: { - ...activeMappings, - properties: { - ...source.mappings.properties, - ...activeMappings.properties, - }, + mappings: targetMappings, + }; +} + +/** + * Merges the active mappings and the source mappings while disabling the + * fields of any unknown Saved Object types present in the source index's + * mappings. + * + * Since the Saved Objects index has `dynamic: strict` defined at the + * top-level, only Saved Object types for which a mapping exists can be + * inserted into the index. To ensure that we can continue to store Saved + * Object documents belonging to a disabled plugin we define a mapping for all + * the unknown Saved Object types that were present in the source index's + * mappings. To limit the field count as much as possible, these unkwnown + * type's mappings are set to `dynamic: false`. + * + * (Since we're using the source index mappings instead of looking at actual + * document types in the inedx, we potentially add more "unknown types" than + * what would be necessary to support migrating all the data over to the + * target index.) + * + * @param activeMappings The mappings compiled from all the Saved Object types + * known to this Kibana node. + * @param sourceMappings The mappings of index used as the migration source. + * @returns The mappings that should be applied to the target index. + */ +export function disableUnknownTypeMappingFields( + activeMappings: IndexMapping, + sourceMappings: IndexMapping +): IndexMapping { + const targetTypes = Object.keys(activeMappings.properties); + + const disabledTypesProperties = Object.keys(sourceMappings.properties) + .filter((sourceType) => { + const isObjectType = 'properties' in sourceMappings.properties[sourceType]; + // Only Object/Nested datatypes can be excluded from the field count by + // using `dynamic: false`. + return !targetTypes.includes(sourceType) && isObjectType; + }) + .reduce((disabledTypesAcc, sourceType) => { + disabledTypesAcc[sourceType] = { dynamic: false, properties: {} }; + return disabledTypesAcc; + }, {} as SavedObjectsMappingProperties); + + return { + ...activeMappings, + properties: { + ...sourceMappings.properties, + ...disabledTypesProperties, + ...activeMappings.properties, }, }; } diff --git a/src/core/server/server.api.md b/src/core/server/server.api.md index 107edf11bb6f4..efeafc9e68d35 100644 --- a/src/core/server/server.api.md +++ b/src/core/server/server.api.md @@ -2022,6 +2022,9 @@ export interface SavedObjectsClientWrapperOptions { export interface SavedObjectsComplexFieldMapping { // (undocumented) doc_values?: boolean; + dynamic?: false | 'strict'; + // (undocumented) + enabled?: boolean; // (undocumented) properties: SavedObjectsMappingProperties; // (undocumented) @@ -2033,8 +2036,6 @@ export interface SavedObjectsCoreFieldMapping { // (undocumented) doc_values?: boolean; // (undocumented) - enabled?: boolean; - // (undocumented) fields?: { [subfield: string]: { type: string;