[ZDT migration] support root field addition (#179595)

## Summary Fix #179258 Change the version compatibility and mapping generation (`checkVersionCompatibility` and `generateAdditiveMappingDiff`) of the ZDT migration algorithm to support the scenario were root fields are added (adding new fields to our base mappings)
elastic · Apr 2, 2024 · f89c3e0 · f89c3e0
1 parent da69703
commit f89c3e0
Show file tree

Hide file tree

Showing 11 changed files with 514 additions and 27 deletions.
diff --git a/...ed-objects/core-saved-objects-migration-server-internal/src/core/build_active_mappings.ts b/...ed-objects/core-saved-objects-migration-server-internal/src/core/build_active_mappings.ts
@@ -40,6 +40,10 @@ export function buildActiveMappings(
  * @returns {IndexMapping}
  */
 export function getBaseMappings(): IndexMapping {
+  // Important: the ZDT algorithm won't trigger a reindex on documents
+  // when changes on root field mappings are detected, meaning that adding
+  // a non-indexed root field and then later switching it to indexed is
+  // not support atm and would require changes to the ZDT algo.
   return {
     dynamic: 'strict',
     properties: {

diff --git a/...ed-objects/core-saved-objects-migration-server-internal/src/core/compare_mappings.test.ts b/...ed-objects/core-saved-objects-migration-server-internal/src/core/compare_mappings.test.ts
@@ -118,4 +118,32 @@ describe('getUpdatedRootFields', () => {
 
     expect(updatedFields).toEqual(['namespace', 'references']);
   });
+
+  it('ignores fields not being present on the base mapping for the diff', () => {
+    const updatedFields = getUpdatedRootFields({
+      properties: {
+        ...getBaseMappings().properties,
+        someUnknownField: {
+          type: 'text',
+        },
+      },
+    });
+
+    expect(updatedFields).toEqual([]);
+  });
+
+  it('ignores fields not being present on the base mapping even with nested props', () => {
+    const updatedFields = getUpdatedRootFields({
+      properties: {
+        ...getBaseMappings().properties,
+        someTypeProps: {
+          properties: {
+            foo: { type: 'text' },
+          },
+        },
+      },
+    });
+
+    expect(updatedFields).toEqual([]);
+  });
 });
diff --git a/...ed-objects/core-saved-objects-migration-server-internal/src/zdt/model/stages/init.test.ts b/...ed-objects/core-saved-objects-migration-server-internal/src/zdt/model/stages/init.test.ts
@@ -345,7 +345,7 @@ describe('Stage: init', () => {
         expect(generateAdditiveMappingDiffMock).toHaveBeenCalledTimes(1);
         expect(generateAdditiveMappingDiffMock).toHaveBeenCalledWith({
           types: ['foo', 'bar'].map((type) => context.typeRegistry.getType(type)),
-          meta: fetchIndexResponse[currentIndex].mappings._meta,
+          mapping: fetchIndexResponse[currentIndex].mappings,
           deletedTypes: context.deletedTypes,
         });
       });

diff --git a/...e/saved-objects/core-saved-objects-migration-server-internal/src/zdt/model/stages/init.ts b/...e/saved-objects/core-saved-objects-migration-server-internal/src/zdt/model/stages/init.ts
@@ -164,7 +164,7 @@ export const init: ModelStage<
     case 'greater':
       const additiveMappingChanges = generateAdditiveMappingDiff({
         types,
-        meta: currentMappings._meta ?? {},
+        mapping: currentMappings,
         deletedTypes: context.deletedTypes,
       });
       return {

diff --git a/...objects-migration-server-internal/src/zdt/utils/check_version_compatibility.test.mocks.ts b/...objects-migration-server-internal/src/zdt/utils/check_version_compatibility.test.mocks.ts
@@ -19,3 +19,13 @@ jest.doMock('@kbn/core-saved-objects-base-server-internal', () => {
     getVirtualVersionMap: getVirtualVersionMapMock,
   };
 });
+
+export const getUpdatedRootFieldsMock = jest.fn();
+
+jest.doMock('../../core/compare_mappings', () => {
+  const actual = jest.requireActual('../../core/compare_mappings');
+  return {
+    ...actual,
+    getUpdatedRootFields: getUpdatedRootFieldsMock,
+  };
+});
diff --git a/...saved-objects-migration-server-internal/src/zdt/utils/check_version_compatibility.test.ts b/...saved-objects-migration-server-internal/src/zdt/utils/check_version_compatibility.test.ts
@@ -10,6 +10,7 @@ import {
   compareVirtualVersionsMock,
   getVirtualVersionMapMock,
   getVirtualVersionsFromMappingsMock,
+  getUpdatedRootFieldsMock,
 } from './check_version_compatibility.test.mocks';
 import type { SavedObjectsType } from '@kbn/core-saved-objects-server';
 import type {
@@ -30,6 +31,7 @@ describe('checkVersionCompatibility', () => {
     compareVirtualVersionsMock.mockReset().mockReturnValue({});
     getVirtualVersionMapMock.mockReset().mockReturnValue({});
     getVirtualVersionsFromMappingsMock.mockReset().mockReturnValue({ status: 'equal' });
+    getUpdatedRootFieldsMock.mockReset().mockReturnValue([]);
 
     types = [createType({ name: 'foo' }), createType({ name: 'bar' })];
 
@@ -88,24 +90,112 @@ describe('checkVersionCompatibility', () => {
     });
   });
 
-  it('returns the result of the compareModelVersions call', () => {
-    const expected: CompareModelVersionResult = {
-      status: 'lesser',
-      details: {
-        greater: [],
-        lesser: [],
-        equal: [],
-      },
-    };
-    compareVirtualVersionsMock.mockReturnValue(expected);
-
-    const result = checkVersionCompatibility({
+  it('calls getUpdatedRootFields with the correct parameters', () => {
+    checkVersionCompatibility({
       types,
       mappings,
       source: 'mappingVersions',
       deletedTypes,
     });
 
-    expect(result).toEqual(expected);
+    expect(getUpdatedRootFieldsMock).toHaveBeenCalledTimes(1);
+    expect(getUpdatedRootFieldsMock).toHaveBeenCalledWith(mappings);
+  });
+
+  describe('without updated root fields', () => {
+    it('returns the result of the compareModelVersions call', () => {
+      const expected: CompareModelVersionResult = {
+        status: 'lesser',
+        details: { greater: [], lesser: [], equal: [] },
+      };
+      compareVirtualVersionsMock.mockReturnValue(expected);
+
+      const result = checkVersionCompatibility({
+        types,
+        mappings,
+        source: 'mappingVersions',
+        deletedTypes,
+      });
+
+      expect(result).toEqual({
+        status: expected.status,
+        versionDetails: expected.details,
+        updatedRootFields: [],
+      });
+    });
+  });
+
+  describe('with updated root fields', () => {
+    beforeEach(() => {
+      getUpdatedRootFieldsMock.mockReturnValue(['rootA']);
+    });
+
+    it('returns the correct status for `greater` version status check', () => {
+      const expected: CompareModelVersionResult = {
+        status: 'greater',
+        details: { greater: [], lesser: [], equal: [] },
+      };
+      compareVirtualVersionsMock.mockReturnValue(expected);
+
+      const result = checkVersionCompatibility({
+        types,
+        mappings,
+        source: 'mappingVersions',
+        deletedTypes,
+      });
+
+      expect(result.status).toEqual('greater');
+    });
+
+    it('returns the correct status for `lesser` version status check', () => {
+      const expected: CompareModelVersionResult = {
+        status: 'lesser',
+        details: { greater: [], lesser: [], equal: [] },
+      };
+      compareVirtualVersionsMock.mockReturnValue(expected);
+
+      const result = checkVersionCompatibility({
+        types,
+        mappings,
+        source: 'mappingVersions',
+        deletedTypes,
+      });
+
+      expect(result.status).toEqual('conflict');
+    });
+
+    it('returns the correct status for `equal` version status check', () => {
+      const expected: CompareModelVersionResult = {
+        status: 'equal',
+        details: { greater: [], lesser: [], equal: [] },
+      };
+      compareVirtualVersionsMock.mockReturnValue(expected);
+
+      const result = checkVersionCompatibility({
+        types,
+        mappings,
+        source: 'mappingVersions',
+        deletedTypes,
+      });
+
+      expect(result.status).toEqual('greater');
+    });
+
+    it('returns the correct status for `conflict` version status check', () => {
+      const expected: CompareModelVersionResult = {
+        status: 'conflict',
+        details: { greater: [], lesser: [], equal: [] },
+      };
+      compareVirtualVersionsMock.mockReturnValue(expected);
+
+      const result = checkVersionCompatibility({
+        types,
+        mappings,
+        source: 'mappingVersions',
+        deletedTypes,
+      });
+
+      expect(result.status).toEqual('conflict');
+    });
   });
 });
diff --git a/...core-saved-objects-migration-server-internal/src/zdt/utils/check_version_compatibility.ts b/...core-saved-objects-migration-server-internal/src/zdt/utils/check_version_compatibility.ts
@@ -12,8 +12,10 @@ import {
   compareVirtualVersions,
   getVirtualVersionMap,
   type IndexMapping,
-  type CompareModelVersionResult,
+  type CompareModelVersionStatus,
+  type CompareModelVersionDetails,
 } from '@kbn/core-saved-objects-base-server-internal';
+import { getUpdatedRootFields } from '../../core/compare_mappings';
 
 interface CheckVersionCompatibilityOpts {
   mappings: IndexMapping;
@@ -22,12 +24,20 @@ interface CheckVersionCompatibilityOpts {
   deletedTypes: string[];
 }
 
+type CheckVersionCompatibilityStatus = 'greater' | 'lesser' | 'equal' | 'conflict';
+
+interface CheckVersionCompatibilityResult {
+  status: CheckVersionCompatibilityStatus;
+  versionDetails: CompareModelVersionDetails;
+  updatedRootFields: string[];
+}
+
 export const checkVersionCompatibility = ({
   mappings,
   types,
   source,
   deletedTypes,
-}: CheckVersionCompatibilityOpts): CompareModelVersionResult => {
+}: CheckVersionCompatibilityOpts): CheckVersionCompatibilityResult => {
   const appVersions = getVirtualVersionMap(types);
   const indexVersions = getVirtualVersionsFromMappings({
     mappings,
@@ -37,5 +47,34 @@ export const checkVersionCompatibility = ({
   if (!indexVersions) {
     throw new Error(`Cannot check version: ${source} not present in the mapping meta`);
   }
-  return compareVirtualVersions({ appVersions, indexVersions, deletedTypes });
+
+  const updatedRootFields = getUpdatedRootFields(mappings);
+  const modelVersionStatus = compareVirtualVersions({ appVersions, indexVersions, deletedTypes });
+  const status = getCompatibilityStatus(modelVersionStatus.status, updatedRootFields.length > 0);
+
+  return {
+    status,
+    updatedRootFields,
+    versionDetails: modelVersionStatus.details,
+  };
+};
+
+const getCompatibilityStatus = (
+  versionStatus: CompareModelVersionStatus,
+  hasUpdatedRootFields: boolean
+): CheckVersionCompatibilityStatus => {
+  if (!hasUpdatedRootFields) {
+    return versionStatus;
+  }
+  switch (versionStatus) {
+    case 'lesser':
+      // lower model versions but additional root mappings => conflict
+      return 'conflict';
+    case 'equal':
+      // no change on model versions but additional root mappings => greater
+      return 'greater';
+    default:
+      // greater and conflict are not impacted
+      return versionStatus;
+  }
 };
diff --git a/...ects-migration-server-internal/src/zdt/utils/generate_additive_mapping_diff.test.mocks.ts b/...ects-migration-server-internal/src/zdt/utils/generate_additive_mapping_diff.test.mocks.ts
@@ -0,0 +1,27 @@
+/*
+ * 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 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 or the Server
+ * Side Public License, v 1.
+ */
+
+export const getBaseMappingsMock = jest.fn();
+
+jest.doMock('../../core/build_active_mappings', () => {
+  const actual = jest.requireActual('../../core/build_active_mappings');
+  return {
+    ...actual,
+    getBaseMappings: getBaseMappingsMock,
+  };
+});
+
+export const getUpdatedRootFieldsMock = jest.fn();
+
+jest.doMock('../../core/compare_mappings', () => {
+  const actual = jest.requireActual('../../core/compare_mappings');
+  return {
+    ...actual,
+    getUpdatedRootFields: getUpdatedRootFieldsMock,
+  };
+});