Disable fields added from unknown saved object types (#70951)

rudolf · rudolf · commit 000552ddc01f · 2020-07-09T13:46:13.000+02:00
* 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()
diff --git 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 <code>dynamic</code> mapping parameter. Saved Object fields should always inherit the <code>dynamic: 'strict'</code> paramater. If you are unsure of the shape of your data use <code>type: 'object', enabled: false</code> 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
@@ -0,0 +1,15 @@
+<!-- Do not edit this file. It is automatically generated by API Documenter. -->
+
+[Home](./index.md) &gt; [kibana-plugin-core-server](./kibana-plugin-core-server.md) &gt; [SavedObjectsComplexFieldMapping](./kibana-plugin-core-server.savedobjectscomplexfieldmapping.md) &gt; [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`<!-- -->.
+
+<b>Signature:</b>
+
+```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
@@ -0,0 +1,11 @@
+<!-- Do not edit this file. It is automatically generated by API Documenter. -->
+
+[Home](./index.md) &gt; [kibana-plugin-core-server](./kibana-plugin-core-server.md) &gt; [SavedObjectsComplexFieldMapping](./kibana-plugin-core-server.savedobjectscomplexfieldmapping.md) &gt; [enabled](./kibana-plugin-core-server.savedobjectscomplexfieldmapping.enabled.md)
+
+## SavedObjectsComplexFieldMapping.enabled property
+
+<b>Signature:</b>
+
+```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
@@ -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.
-
 <b>Signature:</b>
 
 ```typescript
@@ -19,6 +17,8 @@ export interface SavedObjectsComplexFieldMapping
 |  Property | Type | Description |
 |  --- | --- | --- |
 |  [doc\_values](./kibana-plugin-core-server.savedobjectscomplexfieldmapping.doc_values.md) | <code>boolean</code> |  |
+|  [dynamic](./kibana-plugin-core-server.savedobjectscomplexfieldmapping.dynamic.md) | <code>false &#124; 'strict'</code> | The dynamic property of the mapping, either <code>false</code> or <code>'strict'</code>. If unspecified <code>dynamic: 'strict'</code> will be inherited from the top-level index mappings.<!-- -->Note: To limit the number of mapping fields Saved Object types should \*never\* use <code>dynamic: true</code>. |
+|  [enabled](./kibana-plugin-core-server.savedobjectscomplexfieldmapping.enabled.md) | <code>boolean</code> |  |
 |  [properties](./kibana-plugin-core-server.savedobjectscomplexfieldmapping.properties.md) | <code>SavedObjectsMappingProperties</code> |  |
 |  [type](./kibana-plugin-core-server.savedobjectscomplexfieldmapping.type.md) | <code>string</code> |  |
 
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
diff --git 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) | <code>boolean</code> |  |
-|  [enabled](./kibana-plugin-core-server.savedobjectscorefieldmapping.enabled.md) | <code>boolean</code> |  |
 |  [fields](./kibana-plugin-core-server.savedobjectscorefieldmapping.fields.md) | <code>{</code><br/><code>        [subfield: string]: {</code><br/><code>            type: string;</code><br/><code>            ignore_above?: number;</code><br/><code>        };</code><br/><code>    }</code> |  |
 |  [index](./kibana-plugin-core-server.savedobjectscorefieldmapping.index.md) | <code>boolean</code> |  |
 |  [null\_value](./kibana-plugin-core-server.savedobjectscorefieldmapping.null_value.md) | <code>number &#124; boolean &#124; string</code> |  |
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
@@ -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.
 
 <b>Signature:</b>
 
diff --git 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) | <code>false &#124; 'strict'</code> | The dynamic property of the mapping. either <code>false</code> or 'strict'. Defaults to <code>false</code> |
+|  [dynamic](./kibana-plugin-core-server.savedobjectstypemappingdefinition.dynamic.md) | <code>false &#124; 'strict'</code> | The dynamic property of the mapping, either <code>false</code> or <code>'strict'</code>. If unspecified <code>dynamic: 'strict'</code> will be inherited from the top-level index mappings. |
 |  [properties](./kibana-plugin-core-server.savedobjectstypemappingdefinition.properties.md) | <code>SavedObjectsMappingProperties</code> | 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
@@ -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
@@ -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
@@ -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
diff --git a/src/core/server/server.api.md b/src/core/server/server.api.md
