From 110e043915db7be8c7f008a90dd36763103bd9cc Mon Sep 17 00:00:00 2001 From: Joe Portner Date: Thu, 2 Jun 2022 16:18:27 -0400 Subject: [PATCH] Refactor new code into separate transform function Also added tests! --- ...reate_or_upgrade_saved_config.test.mock.ts | 8 ++ .../create_or_upgrade_saved_config.test.ts | 35 +++++- .../create_or_upgrade_saved_config.ts | 37 ++----- .../create_or_upgrade_saved_config/index.ts | 1 + .../server/ui_settings/saved_objects/index.ts | 1 + .../saved_objects/transforms.test.ts | 101 ++++++++++++++++++ .../ui_settings/saved_objects/transforms.ts | 67 ++++++++++++ 7 files changed, 218 insertions(+), 32 deletions(-) create mode 100644 src/core/server/ui_settings/saved_objects/transforms.test.ts create mode 100644 src/core/server/ui_settings/saved_objects/transforms.ts diff --git a/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.test.mock.ts b/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.test.mock.ts index 5f6b1d07358ac2..0573737943983e 100644 --- a/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.test.mock.ts +++ b/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.test.mock.ts @@ -6,8 +6,16 @@ * Side Public License, v 1. */ +import type { transformDefaultIndex } from '../saved_objects'; import type { getUpgradeableConfig } from './get_upgradeable_config'; +export const mockTransformDefaultIndex = jest.fn() as jest.MockedFunction< + typeof transformDefaultIndex +>; +jest.mock('../saved_objects', () => ({ + transformDefaultIndex: mockTransformDefaultIndex, +})); + export const mockGetUpgradeableConfig = jest.fn() as jest.MockedFunction< typeof getUpgradeableConfig >; diff --git a/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.test.ts b/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.test.ts index a896fdfa05823a..b95be43c0b0a0d 100644 --- a/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.test.ts +++ b/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.test.ts @@ -6,7 +6,10 @@ * Side Public License, v 1. */ -import { mockGetUpgradeableConfig } from './create_or_upgrade_saved_config.test.mock'; +import { + mockTransformDefaultIndex, + mockGetUpgradeableConfig, +} from './create_or_upgrade_saved_config.test.mock'; import { SavedObjectsErrorHelpers } from '../../saved_objects'; import { savedObjectsClientMock } from '../../saved_objects/service/saved_objects_client.mock'; import { loggingSystemMock } from '../../logging/logging_system.mock'; @@ -106,6 +109,36 @@ describe('uiSettings/createOrUpgradeSavedConfig', function () { ); }); + it('should prefer transformed attributes when merging', async () => { + const { run, savedObjectsClient } = setup(); + mockGetUpgradeableConfig.mockResolvedValue({ + id: prevVersion, + attributes: { + buildNum: buildNum - 100, + defaultIndex: 'some-index', + }, + }); + mockTransformDefaultIndex.mockResolvedValue({ + defaultIndex: 'another-index', + isDefaultIndexMigrated: true, + }); + + await run(); + + expect(mockGetUpgradeableConfig).toHaveBeenCalledTimes(1); + expect(mockTransformDefaultIndex).toHaveBeenCalledTimes(1); + expect(savedObjectsClient.create).toHaveBeenCalledTimes(1); + expect(savedObjectsClient.create).toHaveBeenCalledWith( + 'config', + { + buildNum, + defaultIndex: 'another-index', + isDefaultIndexMigrated: true, + }, + { id: version } + ); + }); + it('should log a message for upgrades', async () => { const { logger, run } = setup(); diff --git a/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.ts b/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.ts index c214459e5aa8d0..e370da94879674 100644 --- a/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.ts +++ b/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.ts @@ -13,6 +13,7 @@ import { SavedObjectsClientContract } from '../../saved_objects/types'; import { SavedObjectsErrorHelpers } from '../../saved_objects'; import { getUpgradeableConfig } from './get_upgradeable_config'; +import { transformDefaultIndex } from '../saved_objects'; interface ConfigLogMeta extends LogMeta { kibana: { @@ -39,41 +40,15 @@ export async function createOrUpgradeSavedConfig( version, }); - let defaultIndex: string | undefined; - let isDefaultIndexMigrated = false; - if ( - upgradeableConfig?.attributes.defaultIndex && - !upgradeableConfig.attributes.isDefaultIndexMigrated - ) { - defaultIndex = upgradeableConfig.attributes.defaultIndex; // Keep the existing defaultIndex attribute if the data view is not found - try { - // The defaultIndex for this config object was created prior to 8.3, and it might refer to a data view ID that is no longer valid. - // We should try to resolve the data view and change the defaultIndex to the new ID, if necessary. - const resolvedIndex = await savedObjectsClient.resolve('index-pattern', defaultIndex); - if (resolvedIndex.outcome === 'aliasMatch' || resolvedIndex.outcome === 'conflict') { - // This resolved to an aliasMatch or conflict outcome; that means we should change the defaultIndex to the data view's new ID. - // Note, the alias_target_id field is guaranteed to exist iff the resolve outcome is aliasMatch or conflict. - defaultIndex = resolvedIndex.alias_target_id!; - isDefaultIndexMigrated = true; - } - } catch (err) { - // If the defaultIndex is not found at all, it will throw a Not Found error and we should mark the defaultIndex attribute as upgraded. - if (SavedObjectsErrorHelpers.isNotFoundError(err)) { - isDefaultIndexMigrated = true; - } - // For any other error, leave it unchanged so we can try to upgrade this attribute again in the future. - } - } - // default to the attributes of the upgradeableConfig if available const attributes = defaults( { buildNum, - ...(isDefaultIndexMigrated && { - // We migrated the defaultIndex attribute for this config, make sure these two fields take precedence over any in the old config - defaultIndex, - isDefaultIndexMigrated, - }), + // If there is an upgradeableConfig, attempt to transform the defaultIndex attribute to ensure it is set properly in 8.3.1+ + ...(await transformDefaultIndex({ + savedObjectsClient, + configAttributes: upgradeableConfig?.attributes, + })), }, upgradeableConfig?.attributes ); diff --git a/src/core/server/ui_settings/create_or_upgrade_saved_config/index.ts b/src/core/server/ui_settings/create_or_upgrade_saved_config/index.ts index 3559c8555803ae..5ad02b1668fc66 100644 --- a/src/core/server/ui_settings/create_or_upgrade_saved_config/index.ts +++ b/src/core/server/ui_settings/create_or_upgrade_saved_config/index.ts @@ -7,3 +7,4 @@ */ export { createOrUpgradeSavedConfig } from './create_or_upgrade_saved_config'; +export type { UpgradeableConfigType } from './get_upgradeable_config'; diff --git a/src/core/server/ui_settings/saved_objects/index.ts b/src/core/server/ui_settings/saved_objects/index.ts index eb8e7cfcc2a1b2..369dfe3358aa8a 100644 --- a/src/core/server/ui_settings/saved_objects/index.ts +++ b/src/core/server/ui_settings/saved_objects/index.ts @@ -7,3 +7,4 @@ */ export { uiSettingsType } from './ui_settings'; +export { transformDefaultIndex } from './transforms'; diff --git a/src/core/server/ui_settings/saved_objects/transforms.test.ts b/src/core/server/ui_settings/saved_objects/transforms.test.ts new file mode 100644 index 00000000000000..d4c4b9a96f89a1 --- /dev/null +++ b/src/core/server/ui_settings/saved_objects/transforms.test.ts @@ -0,0 +1,101 @@ +/* + * 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. + */ + +import { savedObjectsClientMock } from '../../mocks'; +import { SavedObjectsErrorHelpers } from '../../saved_objects'; +import { SavedObject } from '../../types'; +import { transformDefaultIndex } from './transforms'; + +describe('#transformDefaultIndex', () => { + const savedObjectsClient = savedObjectsClientMock.create(); + + beforeEach(() => { + jest.resetAllMocks(); + }); + + it('should return early if configAttributes is undefined', async () => { + const result = await transformDefaultIndex({ savedObjectsClient, configAttributes: undefined }); + + expect(savedObjectsClient.resolve).not.toHaveBeenCalled(); + expect(result).toEqual({ isDefaultIndexMigrated: true }); + }); + + it('should return early if the defaultIndex attribute is undefined', async () => { + const result = await transformDefaultIndex({ + savedObjectsClient, + configAttributes: { defaultIndex: undefined }, + }); + + expect(savedObjectsClient.resolve).not.toHaveBeenCalled(); + expect(result).toEqual({ isDefaultIndexMigrated: true }); + }); + + describe('should resolve the data view for the defaultIndex and return the result according to the outcome', () => { + it('outcome: exactMatch', async () => { + savedObjectsClient.resolve.mockResolvedValue({ + outcome: 'exactMatch', + alias_target_id: 'another-index', // This wouldn't realistically be set if the outcome is exactMatch, but we're including it in this test to assert that the returned defaultIndex will be 'some-index' + saved_object: {} as SavedObject, // Doesn't matter + }); + const result = await transformDefaultIndex({ + savedObjectsClient, + configAttributes: { defaultIndex: 'some-index' }, + }); + + expect(savedObjectsClient.resolve).toHaveBeenCalledTimes(1); + expect(savedObjectsClient.resolve).toHaveBeenCalledWith('index-pattern', 'some-index'); + expect(result).toEqual({ isDefaultIndexMigrated: true, defaultIndex: 'some-index' }); + }); + + for (const outcome of ['aliasMatch' as const, 'conflict' as const]) { + it(`outcome: ${outcome}`, async () => { + savedObjectsClient.resolve.mockResolvedValue({ + outcome, + alias_target_id: 'another-index', + saved_object: {} as SavedObject, // Doesn't matter + }); + const result = await transformDefaultIndex({ + savedObjectsClient, + configAttributes: { defaultIndex: 'some-index' }, + }); + + expect(savedObjectsClient.resolve).toHaveBeenCalledTimes(1); + expect(savedObjectsClient.resolve).toHaveBeenCalledWith('index-pattern', 'some-index'); + expect(result).toEqual({ isDefaultIndexMigrated: true, defaultIndex: 'another-index' }); + }); + } + + it('returns the expected result if resolve fails with a Not Found error', async () => { + savedObjectsClient.resolve.mockRejectedValue( + SavedObjectsErrorHelpers.createGenericNotFoundError('Oh no!') + ); + const result = await transformDefaultIndex({ + savedObjectsClient, + configAttributes: { defaultIndex: 'some-index' }, + }); + + expect(savedObjectsClient.resolve).toHaveBeenCalledTimes(1); + expect(savedObjectsClient.resolve).toHaveBeenCalledWith('index-pattern', 'some-index'); + expect(result).toEqual({ isDefaultIndexMigrated: true, defaultIndex: 'some-index' }); + }); + + it('returns the expected result if resolve fails with another error', async () => { + savedObjectsClient.resolve.mockRejectedValue( + SavedObjectsErrorHelpers.createIndexAliasNotFoundError('Oh no!') + ); + const result = await transformDefaultIndex({ + savedObjectsClient, + configAttributes: { defaultIndex: 'some-index' }, + }); + + expect(savedObjectsClient.resolve).toHaveBeenCalledTimes(1); + expect(savedObjectsClient.resolve).toHaveBeenCalledWith('index-pattern', 'some-index'); + expect(result).toEqual({ isDefaultIndexMigrated: false, defaultIndex: 'some-index' }); + }); + }); +}); diff --git a/src/core/server/ui_settings/saved_objects/transforms.ts b/src/core/server/ui_settings/saved_objects/transforms.ts new file mode 100644 index 00000000000000..fa717112f48df6 --- /dev/null +++ b/src/core/server/ui_settings/saved_objects/transforms.ts @@ -0,0 +1,67 @@ +/* + * 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. + */ + +import { SavedObjectsErrorHelpers } from '../../saved_objects'; +import type { SavedObjectsClientContract } from '../../types'; +import type { UpgradeableConfigType } from '../create_or_upgrade_saved_config'; + +interface Params { + savedObjectsClient: SavedObjectsClientContract; + configAttributes: + | Pick + | undefined; +} + +/** The resulting parameters that should be used when upgrading the config object. */ +interface ReturnType { + isDefaultIndexMigrated: boolean; + defaultIndex?: string; +} + +/** + * This is applied during `createOrUpgradeSavedConfig` to optionally transform the `defaultIndex` attribute of a config saved object. The + * `defaultIndex` attribute points to a data view ID, but those saved object IDs were regenerated in the 8.0 upgrade. That resulted in a bug + * where the `defaultIndex` would be broken in custom spaces. + * + * We are fixing this bug after the fact, and we can't retroactively change a saved object that's already been migrated, so we use this + * transformation instead to ensure that the `defaultIndex` attributeis not broken. + * + * Note that what used to be called "index patterns" prior to 8.0 have been renamed to "data views", but the object type cannot be changed, + * so that type remains `index-pattern`. + */ +export async function transformDefaultIndex(params: Params): Promise { + const { savedObjectsClient, configAttributes } = params; + if (!configAttributes || !configAttributes.defaultIndex) { + // If configAttributes is undefined (there's no config object being upgraded), set isDefaultIndexMigrated to true and return. + // If configAttributes is defined but the defaultIndex attribute is not set, set isDefaultIndexMigrated to true and return. + return { isDefaultIndexMigrated: true }; + } + + let defaultIndex = configAttributes.defaultIndex!; // Retain the existing defaultIndex attribute in case we run into a resolve error + let isDefaultIndexMigrated: boolean; + try { + // The defaultIndex for this config object was created prior to 8.3, and it might refer to a data view ID that is no longer valid. + // We should try to resolve the data view and change the defaultIndex to the new ID, if necessary. + const resolvedDataView = await savedObjectsClient.resolve('index-pattern', defaultIndex); + if (resolvedDataView.outcome === 'aliasMatch' || resolvedDataView.outcome === 'conflict') { + // This resolved to an aliasMatch or conflict outcome; that means we should change the defaultIndex to the data view's new ID. + // Note, the alias_target_id field is guaranteed to exist iff the resolve outcome is aliasMatch or conflict. + defaultIndex = resolvedDataView.alias_target_id!; + } + isDefaultIndexMigrated = true; // Regardless of the resolve outcome, we now consider this defaultIndex attribute to be migrated + } catch (err) { + // If the defaultIndex is not found at all, it will throw a Not Found error and we should mark the defaultIndex attribute as upgraded. + if (SavedObjectsErrorHelpers.isNotFoundError(err)) { + isDefaultIndexMigrated = true; + } else { + // For any other error, explicitly set isDefaultIndexMigrated to false so we can try this upgrade again in the future. + isDefaultIndexMigrated = false; + } + } + return { isDefaultIndexMigrated, defaultIndex }; +}