[Saved objects] Update forward compatibility schema to only return known values w/o converting them (#216081)

package.json

@@ -1360,6 +1360,7 @@
     "@elastic/synthetics": "^1.18.0",
     "@emotion/babel-preset-css-prop": "^11.11.0",
     "@emotion/jest": "^11.11.0",
+    "@fast-check/jest": "^2.1.0",
     "@frsource/cypress-plugin-visual-regression-diff": "^3.3.10",
     "@jest/console": "^29.7.0",
     "@jest/reporters": "^29.7.0",

renovate.json

@@ -671,6 +671,7 @@
         "getos",
         "joi-to-json",
         "@apidevtools/json-schema-ref-parser",
+        "@fast-check/jest",
         "json5",
         "load-json-file",
         "mock-fs",

src/core/packages/saved-objects/base-server-internal/src/model_version/backward_conversion_schema.test.ts

@@ -88,7 +88,7 @@ describe('convertModelVersionBackwardConversionSchema', () => {
       const output = converted(doc);
 
       expect(validateSpy).toHaveBeenCalledTimes(1);
-      expect(validateSpy).toHaveBeenCalledWith({ foo: 'bar' }, {});
+      expect(validateSpy).toHaveBeenCalledWith({ foo: 'bar' });
       expect(output).toEqual(doc);
     });
 
@@ -128,5 +128,28 @@ describe('convertModelVersionBackwardConversionSchema', () => {
         `"[hello]: definition for this key is missing"`
       );
     });
+
+    it('returns the known subset of keys with their original values', () => {
+      const conversionSchema = schema.object(
+        {
+          durations: schema.arrayOf(schema.duration()),
+          byteSize: schema.byteSize(),
+        },
+        { unknowns: 'ignore' }
+      );
+
+      const doc = createDoc({
+        attributes: { durations: ['1m', '4d'], byteSize: '1gb', excluded: true },
+      });
+      const converted = convertModelVersionBackwardConversionSchema(conversionSchema);
+
+      expect(converted(doc)).toEqual({
+        ...doc,
+        attributes: {
+          durations: ['1m', '4d'],
+          byteSize: '1gb',
+        },
+      });
+    });
   });
 });

src/core/packages/saved-objects/base-server-internal/src/model_version/backward_conversion_schema.ts

@@ -12,6 +12,7 @@ import type {
   SavedObjectUnsanitizedDoc,
   SavedObjectModelVersionForwardCompatibilitySchema,
 } from '@kbn/core-saved-objects-server';
+import { pickValuesBasedOnStructure } from '../utils';
 
 function isObjectType(
   schema: SavedObjectModelVersionForwardCompatibilitySchema
@@ -26,10 +27,18 @@ export const convertModelVersionBackwardConversionSchema = (
 ): ConvertedSchema => {
   if (isObjectType(schema)) {
     return (doc) => {
-      const attrs = schema.validate(doc.attributes, {});
+      const originalAttrs = doc.attributes as object;
+      // Get the validated object, with possible stripping of unknown keys
+      const validatedAttrs = schema.validate(doc.attributes);
+      // Use the validated attrs object to pick values from the original attrs.
+      //
+      // If we reversed this, validation conversion would be returned in the
+      // converted attrs, for example: { duration: '1m' } => { duration: moment.Duration }
+      // which this "conversion" wants to avoid.
+      const convertedAttrs = pickValuesBasedOnStructure(validatedAttrs, originalAttrs);
       return {
         ...doc,
-        attributes: attrs,
+        attributes: convertedAttrs,
       };
     };
   } else {

src/core/packages/saved-objects/base-server-internal/src/utils/index.ts

@@ -14,3 +14,4 @@ export {
   getFieldListMapFromMappingDefinitions,
   type FieldListMap,
 } from './get_field_list';
+export { pickValuesBasedOnStructure } from './pick_values_based_on_structure';

src/core/packages/saved-objects/base-server-internal/src/utils/pick_values_based_on_structure.test.ts

@@ -0,0 +1,134 @@
+/*
+ * 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 deepMerge from 'deepmerge';
+import { fc } from '@fast-check/jest';
+import { pickValuesBasedOnStructure, getFlattenedKeys } from './pick_values_based_on_structure';
+import { isPlainObject } from 'lodash';
+
+describe('getFlattenedKeys', () => {
+  test('empty object', () => {
+    expect(getFlattenedKeys({}).sort()).toEqual([]);
+  });
+
+  test('simple object', () => {
+    expect(getFlattenedKeys({ a: 1, b: 2, c: 3 }).sort()).toEqual(['["a"]', '["b"]', '["c"]']);
+  });
+
+  test('simple object 2', () => {
+    const obj = {
+      a: {},
+    };
+    expect(getFlattenedKeys(obj).sort()).toEqual(['["a"]']);
+  });
+
+  test('simple object 3', () => {
+    const obj = {
+      a: [[]],
+    };
+    expect(getFlattenedKeys(obj).sort()).toEqual(['["a"][0]']);
+  });
+
+  test('complex object 1', () => {
+    const obj = {
+      a: 1,
+      b: { c: 2 },
+      d: [{ e: 1 }, { e: 2 }],
+    };
+
+    expect(getFlattenedKeys(obj).sort()).toEqual([
+      '["a"]',
+      '["b"]["c"]',
+      '["d"][0]["e"]',
+      '["d"][1]["e"]',
+    ]);
+  });
+});
+
+describe('pickValuesBasedOnStructure', () => {
+  test('picks the values of the target', () => {
+    const a = { v1: 'a', v2: [{ c: 1, d: 1 }] };
+    const b = { v1: 'b', v2: [{ c: 2, d: 2 }, { e: 4 }], another: 'value', anArray: [1, 2, 3] };
+
+    expect(pickValuesBasedOnStructure(a, b)).toEqual({ v1: 'b', v2: [{ c: 2, d: 2 }] });
+  });
+
+  test('special case: empty arrays and objects', () => {
+    const a = { v1: 'a', v2: [], v3: {} };
+    const b = {
+      v1: 'b',
+      v2: [{ c: 2, d: 2 }, { e: 4 }],
+      v3: { a: 1 },
+      another: 'value',
+      anArray: [1, 2, 3],
+    };
+
+    expect(pickValuesBasedOnStructure(a, b)).toEqual({ v1: 'b', v2: [], v3: {} });
+  });
+
+  test('can extract structure map when present in target', () => {
+    /**
+     * The `keys` `Arbitrary` represents words with possible numbers like `loremv123`
+     * that might be used in saved objects as property names.
+     *
+     * We genereate 10 of them to promote key collisions. Something like: ['loremV123', 'ipsum.doleres'...]
+     */
+    const arbKeys = fc
+      .array(
+        fc.tuple(
+          fc.lorem({ mode: 'words', maxCount: 1 }),
+          fc.option(fc.nat({ max: 1_000 })),
+          fc.option(fc.constant('.'))
+        ),
+        {
+          minLength: 1,
+          maxLength: 10,
+        }
+      )
+      .chain((pairs) => {
+        const ks = pairs.map(([word, num, period]) => {
+          let result = word;
+          if (num != null) result += `v${num}`; // some keys might have a number
+          if (period != null) result += `${period}${word}`; // some keys might have a period
+          return result;
+        });
+        return fc.constantFrom(ks);
+      });
+
+    const arbObjects = arbKeys.chain((ks) =>
+      fc.tuple(
+        fc.object({ key: fc.constantFrom(...ks), depthSize: 6, maxKeys: 100 }),
+        fc.object({ key: fc.constantFrom(...ks), depthSize: 6, maxKeys: 100 })
+      )
+    );
+
+    fc.assert(
+      fc.property(arbObjects, ([objA, objB]) => {
+        const target = deepMerge(objB, objA, { arrayMerge }) as object;
+        const result = pickValuesBasedOnStructure(objA, target);
+        expect(result).toEqual(objA);
+      }),
+      { verbose: true, numRuns: 1_000 }
+    );
+  });
+});
+
+const arrayMerge = (targetArray: unknown[], sourceArray: unknown[]) => {
+  const dest = targetArray.slice();
+  sourceArray.forEach((srcItem, idx) => {
+    if (
+      (Array.isArray(dest[idx]) && Array.isArray(srcItem)) ||
+      (isPlainObject(dest[idx]) && isPlainObject(srcItem))
+    ) {
+      dest[idx] = deepMerge(dest[idx] as object, srcItem as object, { arrayMerge });
+    } else {
+      dest[idx] = srcItem;
+    }
+  });
+  return dest;
+};

src/core/packages/saved-objects/base-server-internal/src/utils/pick_values_based_on_structure.ts

@@ -0,0 +1,75 @@
+/*
+ * 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 { isPlainObject, get } from 'lodash';
+import { set } from '@kbn/safer-lodash-set';
+
+export function getFlattenedKeys(obj: object): string[] {
+  const result: string[] = [];
+
+  const keys: string[] = Object.keys(obj).map((k) => `["${k}"]`);
+  let current: object | unknown[];
+
+  while (keys.length) {
+    const key = keys.pop()!;
+    current = get(obj, key) as object | unknown[];
+    if (isPlainObject(current)) {
+      const innerKeys = Object.keys(current as object);
+      if (innerKeys.length < 1) {
+        result.push(`${key}`);
+      } else {
+        for (const innerKey of innerKeys) {
+          keys.unshift(`${key}["${innerKey}"]`);
+        }
+      }
+    } else if (Array.isArray(current)) {
+      const arr = current as unknown[];
+      if (arr.length < 1) {
+        result.push(key);
+      } else {
+        for (let i = 0; i < arr.length; i++) keys.unshift(`${key}[${i}]`);
+      }
+    } else {
+      result.push(key);
+    }
+  }
+  return result;
+}
+
+/**
+ * Given two objects, use the first object as a structural map to extract values
+ * from a second object, preserving the placement in the first object.
+ *
+ * @example
+ * ```ts
+ * const keySource = { a: 1, b: [{ a: 1 }, { a: 2 }] };
+ * const target = { a: 2, b: [{ a: 2, b: 3 }, { a: 3, b: 4 }] };
+ * pickValuesBasedOnStructure(keySource, target);
+ * // => { a: 2, b: [{ a: 2 }, { a: 3 }] }
+ * ```
+ *
+ * @note This is intended to specifically be used in the application of forward
+ *       compatibility schemas when loading a saved object from the database,
+ *       downgrading it and keeping only the known, validated subset of values.
+ */
+export function pickValuesBasedOnStructure(structuralSource: object, target: object): object {
+  const paths = getFlattenedKeys(structuralSource);
+  const result: object = {};
+  for (const path of paths) {
+    const value = get(target, path);
+    if (Array.isArray(value)) {
+      set(result, path, []);
+    } else if (isPlainObject(value)) {
+      set(result, path, {});
+    } else {
+      set(result, path, value);
+    }
+  }
+  return result;
+}

src/core/packages/saved-objects/base-server-internal/tsconfig.json

@@ -18,6 +18,7 @@
     "@kbn/core-saved-objects-utils-server",
     "@kbn/std",
     "@kbn/logging-mocks",
+    "@kbn/safer-lodash-set",
   ],
   "exclude": [
     "target/**/*",

src/platform/packages/shared/kbn-test/jest-preset.js

@@ -111,7 +111,7 @@ module.exports = {
   transformIgnorePatterns: [
     // ignore all node_modules except monaco-editor, monaco-yaml which requires babel transforms to handle dynamic import()
     // since ESM modules are not natively supported in Jest yet (https://github.com/facebook/jest/issues/4842)
-    '[/\\\\]node_modules(?![\\/\\\\](byte-size|monaco-editor|monaco-yaml|monaco-languageserver-types|monaco-marker-data-provider|monaco-worker-manager|vscode-languageserver-types|d3-interpolate|d3-color|langchain|langsmith|@cfworker|gpt-tokenizer|flat|@langchain|eventsource-parser))[/\\\\].+\\.js$',
+    '[/\\\\]node_modules(?![\\/\\\\](byte-size|monaco-editor|monaco-yaml|monaco-languageserver-types|monaco-marker-data-provider|monaco-worker-manager|vscode-languageserver-types|d3-interpolate|d3-color|langchain|langsmith|@cfworker|gpt-tokenizer|flat|@langchain|eventsource-parser|fast-check|@fast-check/jest))[/\\\\].+\\.js$',
     'packages/kbn-pm/dist/index.js',
     '[/\\\\]node_modules(?![\\/\\\\](langchain|langsmith|@langchain))/dist/[/\\\\].+\\.js$',
     '[/\\\\]node_modules(?![\\/\\\\](langchain|langsmith|@langchain))/dist/util/[/\\\\].+\\.js$',

yarn.lock

@@ -2714,6 +2714,13 @@
   resolved "https://registry.yarnpkg.com/@faker-js/faker/-/faker-8.3.1.tgz#7753df0cb88d7649becf984a96dd1bd0a26f43e3"
   integrity sha512-FdgpFxY6V6rLZE9mmIBb9hM0xpfvQOSNOLnzolzKwsE1DH+gC7lEKV1p1IbR0lAYyvYd5a4u3qWJzowUkw1bIw==
 
+"@fast-check/jest@^2.1.0":
+  version "2.1.0"
+  resolved "https://registry.yarnpkg.com/@fast-check/jest/-/jest-2.1.0.tgz#67585ede285267dc9efc3ac3e05c41879887980a"
+  integrity sha512-9ZVvFnngR0EpfxWoGrq/e9ERrDv3qqE4RXVnn/mGv5Rn33LoDcIfg7xjqVREsyhGcwegEKeL0KJlrWzWiOBV8A==
+  dependencies:
+    fast-check "^3.0.0 || ^4.0.0"
+
 "@fastify/busboy@^2.0.0":
   version "2.0.0"
   resolved "https://registry.yarnpkg.com/@fastify/busboy/-/busboy-2.0.0.tgz#f22824caff3ae506b18207bad4126dbc6ccdb6b8"
@@ -18171,6 +18178,13 @@ fancy-log@^1.3.3:
     parse-node-version "^1.0.0"
     time-stamp "^1.0.0"
 
+"fast-check@^3.0.0 || ^4.0.0":
+  version "4.0.0"
+  resolved "https://registry.yarnpkg.com/fast-check/-/fast-check-4.0.0.tgz#a5aff60fb20728458c91d1ff98ca1b65d991abc5"
+  integrity sha512-aXLyLemZ7qhLNn2oq+YpjT2Xed21+i29WGAYuyrGbU4r8oinB3i4XR4e62O3NY6qmm5qHEDoc/7d+gMsri3AfA==
+  dependencies:
+    pure-rand "^7.0.0"
+
 fast-content-type-parse@^2.0.0:
   version "2.0.1"
   resolved "https://registry.yarnpkg.com/fast-content-type-parse/-/fast-content-type-parse-2.0.1.tgz#c236124534ee2cb427c8d8e5ba35a4856947847b"
@@ -26077,6 +26091,11 @@ pure-rand@^6.0.0:
   resolved "https://registry.yarnpkg.com/pure-rand/-/pure-rand-6.0.2.tgz#a9c2ddcae9b68d736a8163036f088a2781c8b306"
   integrity sha512-6Yg0ekpKICSjPswYOuC5sku/TSWaRYlA0qsXqJgM/d/4pLPHPuTxK7Nbf7jFKzAeedUhR8C7K9Uv63FBsSo8xQ==
 
+pure-rand@^7.0.0:
+  version "7.0.1"
+  resolved "https://registry.yarnpkg.com/pure-rand/-/pure-rand-7.0.1.tgz#6f53a5a9e3e4a47445822af96821ca509ed37566"
+  integrity sha512-oTUZM/NAZS8p7ANR3SHh30kXB+zK2r2BPcEn/awJIbOvq82WoMN4p62AWWp3Hhw50G0xMsw1mhIBLqHw64EcNQ==
+
 qs@6.13.0, qs@^6.11.0, qs@^6.7.0:
   version "6.13.0"
   resolved "https://registry.yarnpkg.com/qs/-/qs-6.13.0.tgz#6ca3bd58439f7e245655798997787b0d88a51906"