From d9ced2ff470110be3fa0b451d2a1a49eeeb4dfd2 Mon Sep 17 00:00:00 2001 From: "Christiane (Tina) Heiligers" Date: Fri, 17 Jan 2025 14:04:57 -0700 Subject: [PATCH 1/4] Adds assertion to ensure no types removed from previoously registered types --- .../check_registered_types.test.ts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/core/server/integration_tests/ci_checks/saved_objects/check_registered_types.test.ts b/src/core/server/integration_tests/ci_checks/saved_objects/check_registered_types.test.ts index 353ef8c85548d..95e8cea705b44 100644 --- a/src/core/server/integration_tests/ci_checks/saved_objects/check_registered_types.test.ts +++ b/src/core/server/integration_tests/ci_checks/saved_objects/check_registered_types.test.ts @@ -186,4 +186,21 @@ describe('checking migration metadata changes on all registered SO types', () => } `); }); + + // This test is meant to fail when any change is made in the number of registered types. + // as of 8.8, + // Just update the snapshot by running this test file via jest_integration with `-u` and push the update. + // The intent is to trigger a code review from the Core team to review the SO type changes. + it.skip('detecting no deregistered types', () => { + const allTypes = typeRegistry.getAllTypes(); + + const hashMap = allTypes.reduce((map, type) => { + map[type.name] = getMigrationHash(type); + return map; + }, {} as Record); + const hashedSOTypes = Object.keys(hashMap).sort(); + const registeredSOTypes = allTypes.map(({ name }) => name).sort(); + expect(hashedSOTypes.length).toBeGreaterThanOrEqual(registeredSOTypes.length); + expect(hashedSOTypes).toEqual(registeredSOTypes); // ensure that all types registered are hashed already or need to be added to the hash. If there's a type that's hashed and no longer registered, fail. + }); }); From bb9c1b0666b6b66a566bffc0e75ffcc9c4e26530 Mon Sep 17 00:00:00 2001 From: "Christiane (Tina) Heiligers" Date: Fri, 17 Jan 2025 16:22:42 -0700 Subject: [PATCH 2/4] Export min saved object type count from core internal saved objects package, moves registration test to dedicated subfolder --- .../src/core/unused_types.ts | 2 ++ .../saved-objects/server-internal/index.ts | 1 + .../server-internal/src/index.ts | 1 + .../server-internal/src/object_types/index.ts | 4 ++++ .../saved-objects/server/src/type_registry.ts | 1 - .../check_registered_types.test.ts | 21 ++++--------------- .../saved_objects/jest.integration.config.js | 7 ++++--- .../type_registrations.test.ts | 0 8 files changed, 16 insertions(+), 21 deletions(-) rename src/core/server/integration_tests/saved_objects/{migrations/group3 => registration}/type_registrations.test.ts (100%) diff --git a/src/core/packages/saved-objects/migration-server-internal/src/core/unused_types.ts b/src/core/packages/saved-objects/migration-server-internal/src/core/unused_types.ts index 48594416cbece..2d234c6f4d3d3 100644 --- a/src/core/packages/saved-objects/migration-server-internal/src/core/unused_types.ts +++ b/src/core/packages/saved-objects/migration-server-internal/src/core/unused_types.ts @@ -11,6 +11,8 @@ import type { QueryDslQueryContainer } from '@elastic/elasticsearch/lib/api/type /** * Types that are no longer registered and need to be removed + * As of 8.8, no new types are allowed to be removed. + * Removing saved object types is not backward compatible */ export const REMOVED_TYPES: string[] = [ 'apm-services-telemetry', diff --git a/src/core/packages/saved-objects/server-internal/index.ts b/src/core/packages/saved-objects/server-internal/index.ts index 7b67bfa6861b4..8f0a7280127da 100644 --- a/src/core/packages/saved-objects/server-internal/index.ts +++ b/src/core/packages/saved-objects/server-internal/index.ts @@ -11,6 +11,7 @@ export { MIGRATION_CLIENT_OPTIONS, SavedObjectsService, CoreSavedObjectsRouteHandlerContext, + MIN_SAVED_OBJECT_TYPES_COUNT, } from './src'; export type { InternalSavedObjectsServiceStart, diff --git a/src/core/packages/saved-objects/server-internal/src/index.ts b/src/core/packages/saved-objects/server-internal/src/index.ts index a964d30a0e66a..d594389b115d9 100644 --- a/src/core/packages/saved-objects/server-internal/src/index.ts +++ b/src/core/packages/saved-objects/server-internal/src/index.ts @@ -18,3 +18,4 @@ export type { InternalSavedObjectsRequestHandlerContext, InternalSavedObjectRouter, } from './internal_types'; +export { MIN_SAVED_OBJECT_TYPES_COUNT } from './object_types'; diff --git a/src/core/packages/saved-objects/server-internal/src/object_types/index.ts b/src/core/packages/saved-objects/server-internal/src/object_types/index.ts index 74ba99cf95498..d880cbc24e043 100644 --- a/src/core/packages/saved-objects/server-internal/src/object_types/index.ts +++ b/src/core/packages/saved-objects/server-internal/src/object_types/index.ts @@ -8,3 +8,7 @@ */ export { registerCoreObjectTypes } from './registration'; + +// set minimum number of registered saved objects to ensure no object types are removed after 8.8 +// declared in internal implementation exclicilty to prevent unintended changes. +export const MIN_SAVED_OBJECT_TYPES_COUNT = 127 as const; diff --git a/src/core/packages/saved-objects/server/src/type_registry.ts b/src/core/packages/saved-objects/server/src/type_registry.ts index a1128b683166b..d64ca1917be0d 100644 --- a/src/core/packages/saved-objects/server/src/type_registry.ts +++ b/src/core/packages/saved-objects/server/src/type_registry.ts @@ -8,7 +8,6 @@ */ import type { SavedObjectsType } from './saved_objects_type'; - /** * Registry holding information about all the registered {@link SavedObjectsType | saved object types}. */ diff --git a/src/core/server/integration_tests/ci_checks/saved_objects/check_registered_types.test.ts b/src/core/server/integration_tests/ci_checks/saved_objects/check_registered_types.test.ts index 95e8cea705b44..c81afed273db2 100644 --- a/src/core/server/integration_tests/ci_checks/saved_objects/check_registered_types.test.ts +++ b/src/core/server/integration_tests/ci_checks/saved_objects/check_registered_types.test.ts @@ -15,6 +15,7 @@ import { createRootWithCorePlugins, type TestElasticsearchUtils, } from '@kbn/core-test-helpers-kbn-server'; +import { MIN_SAVED_OBJECT_TYPES_COUNT } from '@kbn/core-saved-objects-server-internal'; describe('checking migration metadata changes on all registered SO types', () => { let esServer: TestElasticsearchUtils; @@ -46,6 +47,8 @@ describe('checking migration metadata changes on all registered SO types', () => // This test is meant to fail when any change is made in registered types that could potentially impact the SO migration. // Just update the snapshot by running this test file via jest_integration with `-u` and push the update. // The intent is to trigger a code review from the Core team to review the SO type changes. + // The number of types in the hashMap should never be reduced, it can only increase. + // Removing saved object types is forbidden after 8.8. it('detecting migration related changes in registered types', () => { const allTypes = typeRegistry.getAllTypes(); @@ -185,22 +188,6 @@ describe('checking migration metadata changes on all registered SO types', () => "workplace_search_telemetry": "52b32b47ee576f554ac77cb1d5896dfbcfe9a1fb", } `); - }); - - // This test is meant to fail when any change is made in the number of registered types. - // as of 8.8, - // Just update the snapshot by running this test file via jest_integration with `-u` and push the update. - // The intent is to trigger a code review from the Core team to review the SO type changes. - it.skip('detecting no deregistered types', () => { - const allTypes = typeRegistry.getAllTypes(); - - const hashMap = allTypes.reduce((map, type) => { - map[type.name] = getMigrationHash(type); - return map; - }, {} as Record); - const hashedSOTypes = Object.keys(hashMap).sort(); - const registeredSOTypes = allTypes.map(({ name }) => name).sort(); - expect(hashedSOTypes.length).toBeGreaterThanOrEqual(registeredSOTypes.length); - expect(hashedSOTypes).toEqual(registeredSOTypes); // ensure that all types registered are hashed already or need to be added to the hash. If there's a type that's hashed and no longer registered, fail. + expect(Object.keys(hashMap).length).toEqual(MIN_SAVED_OBJECT_TYPES_COUNT); }); }); diff --git a/src/core/server/integration_tests/saved_objects/jest.integration.config.js b/src/core/server/integration_tests/saved_objects/jest.integration.config.js index 842eeb6e9d19d..53567e050ef72 100644 --- a/src/core/server/integration_tests/saved_objects/jest.integration.config.js +++ b/src/core/server/integration_tests/saved_objects/jest.integration.config.js @@ -16,9 +16,10 @@ module.exports = { preset: '@kbn/test/jest_integration', rootDir: '../../../../..', roots: [ - '/src/core/server/integration_tests/saved_objects/routes', - '/src/core/server/integration_tests/saved_objects/service', - '/src/core/server/integration_tests/saved_objects/validation', + '/src/core/server/integration_tests/saved_objects/registration', + // '/src/core/server/integration_tests/saved_objects/routes', + // '/src/core/server/integration_tests/saved_objects/service', + // '/src/core/server/integration_tests/saved_objects/validation', ], // must override to match all test given there is no `integration_tests` subfolder testMatch: ['**/*.test.{js,mjs,ts,tsx}'], diff --git a/src/core/server/integration_tests/saved_objects/migrations/group3/type_registrations.test.ts b/src/core/server/integration_tests/saved_objects/registration/type_registrations.test.ts similarity index 100% rename from src/core/server/integration_tests/saved_objects/migrations/group3/type_registrations.test.ts rename to src/core/server/integration_tests/saved_objects/registration/type_registrations.test.ts From 8f6fa3c098c241e5f8a2fcf29d3c0799bcd6afc7 Mon Sep 17 00:00:00 2001 From: "Christiane (Tina) Heiligers" Date: Fri, 17 Jan 2025 16:43:57 -0700 Subject: [PATCH 3/4] Reinstate tests --- .../saved_objects/jest.integration.config.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/core/server/integration_tests/saved_objects/jest.integration.config.js b/src/core/server/integration_tests/saved_objects/jest.integration.config.js index 53567e050ef72..6f1903f5cf689 100644 --- a/src/core/server/integration_tests/saved_objects/jest.integration.config.js +++ b/src/core/server/integration_tests/saved_objects/jest.integration.config.js @@ -17,9 +17,9 @@ module.exports = { rootDir: '../../../../..', roots: [ '/src/core/server/integration_tests/saved_objects/registration', - // '/src/core/server/integration_tests/saved_objects/routes', - // '/src/core/server/integration_tests/saved_objects/service', - // '/src/core/server/integration_tests/saved_objects/validation', + '/src/core/server/integration_tests/saved_objects/routes', + '/src/core/server/integration_tests/saved_objects/service', + '/src/core/server/integration_tests/saved_objects/validation', ], // must override to match all test given there is no `integration_tests` subfolder testMatch: ['**/*.test.{js,mjs,ts,tsx}'], From 5dc727a05cc408145efab606ffa7943285769468 Mon Sep 17 00:00:00 2001 From: "Christiane (Tina) Heiligers" Date: Tue, 21 Jan 2025 09:51:19 -0700 Subject: [PATCH 4/4] fix name to reflect equality check --- src/core/packages/saved-objects/server-internal/index.ts | 2 +- src/core/packages/saved-objects/server-internal/src/index.ts | 2 +- .../saved-objects/server-internal/src/object_types/index.ts | 2 +- .../ci_checks/saved_objects/check_registered_types.test.ts | 4 ++-- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/core/packages/saved-objects/server-internal/index.ts b/src/core/packages/saved-objects/server-internal/index.ts index 8f0a7280127da..3a9c80749c72e 100644 --- a/src/core/packages/saved-objects/server-internal/index.ts +++ b/src/core/packages/saved-objects/server-internal/index.ts @@ -11,7 +11,7 @@ export { MIGRATION_CLIENT_OPTIONS, SavedObjectsService, CoreSavedObjectsRouteHandlerContext, - MIN_SAVED_OBJECT_TYPES_COUNT, + SAVED_OBJECT_TYPES_COUNT, } from './src'; export type { InternalSavedObjectsServiceStart, diff --git a/src/core/packages/saved-objects/server-internal/src/index.ts b/src/core/packages/saved-objects/server-internal/src/index.ts index d594389b115d9..ae34f8f945462 100644 --- a/src/core/packages/saved-objects/server-internal/src/index.ts +++ b/src/core/packages/saved-objects/server-internal/src/index.ts @@ -18,4 +18,4 @@ export type { InternalSavedObjectsRequestHandlerContext, InternalSavedObjectRouter, } from './internal_types'; -export { MIN_SAVED_OBJECT_TYPES_COUNT } from './object_types'; +export { SAVED_OBJECT_TYPES_COUNT } from './object_types'; diff --git a/src/core/packages/saved-objects/server-internal/src/object_types/index.ts b/src/core/packages/saved-objects/server-internal/src/object_types/index.ts index d880cbc24e043..6246c59b8e943 100644 --- a/src/core/packages/saved-objects/server-internal/src/object_types/index.ts +++ b/src/core/packages/saved-objects/server-internal/src/object_types/index.ts @@ -11,4 +11,4 @@ export { registerCoreObjectTypes } from './registration'; // set minimum number of registered saved objects to ensure no object types are removed after 8.8 // declared in internal implementation exclicilty to prevent unintended changes. -export const MIN_SAVED_OBJECT_TYPES_COUNT = 127 as const; +export const SAVED_OBJECT_TYPES_COUNT = 127 as const; diff --git a/src/core/server/integration_tests/ci_checks/saved_objects/check_registered_types.test.ts b/src/core/server/integration_tests/ci_checks/saved_objects/check_registered_types.test.ts index 58b0089167589..427fcc06c3c30 100644 --- a/src/core/server/integration_tests/ci_checks/saved_objects/check_registered_types.test.ts +++ b/src/core/server/integration_tests/ci_checks/saved_objects/check_registered_types.test.ts @@ -15,7 +15,7 @@ import { createRootWithCorePlugins, type TestElasticsearchUtils, } from '@kbn/core-test-helpers-kbn-server'; -import { MIN_SAVED_OBJECT_TYPES_COUNT } from '@kbn/core-saved-objects-server-internal'; +import { SAVED_OBJECT_TYPES_COUNT } from '@kbn/core-saved-objects-server-internal'; describe('checking migration metadata changes on all registered SO types', () => { let esServer: TestElasticsearchUtils; @@ -188,6 +188,6 @@ describe('checking migration metadata changes on all registered SO types', () => "workplace_search_telemetry": "52b32b47ee576f554ac77cb1d5896dfbcfe9a1fb", } `); - expect(Object.keys(hashMap).length).toEqual(MIN_SAVED_OBJECT_TYPES_COUNT); + expect(Object.keys(hashMap).length).toEqual(SAVED_OBJECT_TYPES_COUNT); }); });