From 9bdd23a460b31443fa8cf4c656600c659aa268e5 Mon Sep 17 00:00:00 2001 From: Josh Dover Date: Fri, 28 Feb 2020 13:31:59 -0700 Subject: [PATCH] Do not write UUID file during optimize process (#58899) --- src/core/server/uuid/resolve_uuid.test.ts | 230 +++++++++++++----- src/core/server/uuid/resolve_uuid.ts | 41 +++- src/core/server/uuid/uuid_service.test.ts | 36 ++- src/core/server/uuid/uuid_service.ts | 10 +- src/dev/build/build_distributables.js | 2 + src/dev/build/tasks/index.js | 1 + src/dev/build/tasks/uuid_verification_task.js | 38 +++ 7 files changed, 278 insertions(+), 80 deletions(-) create mode 100644 src/dev/build/tasks/uuid_verification_task.js diff --git a/src/core/server/uuid/resolve_uuid.test.ts b/src/core/server/uuid/resolve_uuid.test.ts index d1332daa0205..efc90c07c1fa 100644 --- a/src/core/server/uuid/resolve_uuid.test.ts +++ b/src/core/server/uuid/resolve_uuid.test.ts @@ -19,7 +19,7 @@ import { join } from 'path'; import { readFile, writeFile } from './fs'; -import { resolveInstanceUuid } from './resolve_uuid'; +import { resolveInstanceUuid, UUID_7_6_0_BUG } from './resolve_uuid'; import { configServiceMock } from '../config/config_service.mock'; import { loggingServiceMock } from '../logging/logging_service.mock'; import { BehaviorSubject } from 'rxjs'; @@ -97,58 +97,96 @@ describe('resolveInstanceUuid', () => { }); describe('when file is present and config property is set', () => { - it('writes to file and returns the config uuid if they mismatch', async () => { - const uuid = await resolveInstanceUuid(configService, logger); - expect(uuid).toEqual(DEFAULT_CONFIG_UUID); - expect(writeFile).toHaveBeenCalledWith( - join('data-folder', 'uuid'), - DEFAULT_CONFIG_UUID, - expect.any(Object) - ); - expect(logger.debug).toHaveBeenCalledTimes(1); - expect(logger.debug.mock.calls[0]).toMatchInlineSnapshot(` - Array [ - "Updating Kibana instance UUID to: CONFIG_UUID (was: FILE_UUID)", - ] - `); + describe('when they mismatch', () => { + describe('when syncToFile is true', () => { + it('writes to file and returns the config uuid', async () => { + const uuid = await resolveInstanceUuid({ configService, logger, syncToFile: true }); + expect(uuid).toEqual(DEFAULT_CONFIG_UUID); + expect(writeFile).toHaveBeenCalledWith( + join('data-folder', 'uuid'), + DEFAULT_CONFIG_UUID, + expect.any(Object) + ); + expect(logger.debug).toHaveBeenCalledTimes(1); + expect(logger.debug.mock.calls[0]).toMatchInlineSnapshot(` + Array [ + "Updating Kibana instance UUID to: CONFIG_UUID (was: FILE_UUID)", + ] + `); + }); + }); + + describe('when syncTofile is false', () => { + it('does not write to file and returns the config uuid', async () => { + const uuid = await resolveInstanceUuid({ configService, logger, syncToFile: false }); + expect(uuid).toEqual(DEFAULT_CONFIG_UUID); + expect(writeFile).not.toHaveBeenCalled(); + expect(logger.debug).toHaveBeenCalledTimes(1); + expect(logger.debug.mock.calls[0]).toMatchInlineSnapshot(` + Array [ + "Updating Kibana instance UUID to: CONFIG_UUID (was: FILE_UUID)", + ] + `); + }); + }); }); - it('does not write to file if they match', async () => { - mockReadFile({ uuid: DEFAULT_CONFIG_UUID }); - const uuid = await resolveInstanceUuid(configService, logger); - expect(uuid).toEqual(DEFAULT_CONFIG_UUID); - expect(writeFile).not.toHaveBeenCalled(); - expect(logger.debug).toHaveBeenCalledTimes(1); - expect(logger.debug.mock.calls[0]).toMatchInlineSnapshot(` - Array [ - "Kibana instance UUID: CONFIG_UUID", - ] - `); + + describe('when they match', () => { + it('does not write to file', async () => { + mockReadFile({ uuid: DEFAULT_CONFIG_UUID }); + const uuid = await resolveInstanceUuid({ configService, logger, syncToFile: true }); + expect(uuid).toEqual(DEFAULT_CONFIG_UUID); + expect(writeFile).not.toHaveBeenCalled(); + expect(logger.debug).toHaveBeenCalledTimes(1); + expect(logger.debug.mock.calls[0]).toMatchInlineSnapshot(` + Array [ + "Kibana instance UUID: CONFIG_UUID", + ] + `); + }); }); }); describe('when file is not present and config property is set', () => { - it('writes the uuid to file and returns the config uuid', async () => { - mockReadFile({ error: fileNotFoundError }); - const uuid = await resolveInstanceUuid(configService, logger); - expect(uuid).toEqual(DEFAULT_CONFIG_UUID); - expect(writeFile).toHaveBeenCalledWith( - join('data-folder', 'uuid'), - DEFAULT_CONFIG_UUID, - expect.any(Object) - ); - expect(logger.debug).toHaveBeenCalledTimes(1); - expect(logger.debug.mock.calls[0]).toMatchInlineSnapshot(` - Array [ - "Setting new Kibana instance UUID: CONFIG_UUID", - ] - `); + describe('when syncToFile is true', () => { + it('writes the uuid to file and returns the config uuid', async () => { + mockReadFile({ error: fileNotFoundError }); + const uuid = await resolveInstanceUuid({ configService, logger, syncToFile: true }); + expect(uuid).toEqual(DEFAULT_CONFIG_UUID); + expect(writeFile).toHaveBeenCalledWith( + join('data-folder', 'uuid'), + DEFAULT_CONFIG_UUID, + expect.any(Object) + ); + expect(logger.debug).toHaveBeenCalledTimes(1); + expect(logger.debug.mock.calls[0]).toMatchInlineSnapshot(` + Array [ + "Setting new Kibana instance UUID: CONFIG_UUID", + ] + `); + }); + }); + + describe('when syncToFile is false', () => { + it('does not write the uuid to file and returns the config uuid', async () => { + mockReadFile({ error: fileNotFoundError }); + const uuid = await resolveInstanceUuid({ configService, logger, syncToFile: false }); + expect(uuid).toEqual(DEFAULT_CONFIG_UUID); + expect(writeFile).not.toHaveBeenCalled(); + expect(logger.debug).toHaveBeenCalledTimes(1); + expect(logger.debug.mock.calls[0]).toMatchInlineSnapshot(` + Array [ + "Setting new Kibana instance UUID: CONFIG_UUID", + ] + `); + }); }); }); describe('when file is present and config property is not set', () => { it('does not write to file and returns the file uuid', async () => { configService = getConfigService(undefined); - const uuid = await resolveInstanceUuid(configService, logger); + const uuid = await resolveInstanceUuid({ configService, logger, syncToFile: true }); expect(uuid).toEqual(DEFAULT_FILE_UUID); expect(writeFile).not.toHaveBeenCalled(); expect(logger.debug).toHaveBeenCalledTimes(1); @@ -160,23 +198,95 @@ describe('resolveInstanceUuid', () => { }); }); + describe('when file is present with 7.6.0 UUID', () => { + describe('when config property is not set', () => { + it('writes new uuid to file and returns new uuid', async () => { + mockReadFile({ uuid: UUID_7_6_0_BUG }); + configService = getConfigService(undefined); + const uuid = await resolveInstanceUuid({ configService, logger, syncToFile: true }); + expect(uuid).not.toEqual(UUID_7_6_0_BUG); + expect(uuid).toEqual('NEW_UUID'); + expect(writeFile).toHaveBeenCalledWith( + join('data-folder', 'uuid'), + 'NEW_UUID', + expect.any(Object) + ); + expect(logger.debug).toHaveBeenCalledTimes(2); + expect(logger.debug.mock.calls).toMatchInlineSnapshot(` + Array [ + Array [ + "UUID from 7.6.0 bug detected, ignoring file UUID", + ], + Array [ + "Setting new Kibana instance UUID: NEW_UUID", + ], + ] + `); + }); + }); + + describe('when config property is set', () => { + it('writes config uuid to file and returns config uuid', async () => { + mockReadFile({ uuid: UUID_7_6_0_BUG }); + configService = getConfigService(DEFAULT_CONFIG_UUID); + const uuid = await resolveInstanceUuid({ configService, logger, syncToFile: true }); + expect(uuid).not.toEqual(UUID_7_6_0_BUG); + expect(uuid).toEqual(DEFAULT_CONFIG_UUID); + expect(writeFile).toHaveBeenCalledWith( + join('data-folder', 'uuid'), + DEFAULT_CONFIG_UUID, + expect.any(Object) + ); + expect(logger.debug).toHaveBeenCalledTimes(2); + expect(logger.debug.mock.calls).toMatchInlineSnapshot(` + Array [ + Array [ + "UUID from 7.6.0 bug detected, ignoring file UUID", + ], + Array [ + "Setting new Kibana instance UUID: CONFIG_UUID", + ], + ] + `); + }); + }); + }); + describe('when file is not present and config property is not set', () => { - it('generates a new uuid and write it to file', async () => { - configService = getConfigService(undefined); - mockReadFile({ error: fileNotFoundError }); - const uuid = await resolveInstanceUuid(configService, logger); - expect(uuid).toEqual('NEW_UUID'); - expect(writeFile).toHaveBeenCalledWith( - join('data-folder', 'uuid'), - 'NEW_UUID', - expect.any(Object) - ); - expect(logger.debug).toHaveBeenCalledTimes(1); - expect(logger.debug.mock.calls[0]).toMatchInlineSnapshot(` - Array [ - "Setting new Kibana instance UUID: NEW_UUID", - ] - `); + describe('when syncToFile is true', () => { + it('generates a new uuid and write it to file', async () => { + configService = getConfigService(undefined); + mockReadFile({ error: fileNotFoundError }); + const uuid = await resolveInstanceUuid({ configService, logger, syncToFile: true }); + expect(uuid).toEqual('NEW_UUID'); + expect(writeFile).toHaveBeenCalledWith( + join('data-folder', 'uuid'), + 'NEW_UUID', + expect.any(Object) + ); + expect(logger.debug).toHaveBeenCalledTimes(1); + expect(logger.debug.mock.calls[0]).toMatchInlineSnapshot(` + Array [ + "Setting new Kibana instance UUID: NEW_UUID", + ] + `); + }); + }); + + describe('when syncToFile is false', () => { + it('generates a new uuid and does not write it to file', async () => { + configService = getConfigService(undefined); + mockReadFile({ error: fileNotFoundError }); + const uuid = await resolveInstanceUuid({ configService, logger, syncToFile: false }); + expect(uuid).toEqual('NEW_UUID'); + expect(writeFile).not.toHaveBeenCalled(); + expect(logger.debug).toHaveBeenCalledTimes(1); + expect(logger.debug.mock.calls[0]).toMatchInlineSnapshot(` + Array [ + "Setting new Kibana instance UUID: NEW_UUID", + ] + `); + }); }); }); @@ -184,7 +294,7 @@ describe('resolveInstanceUuid', () => { it('throws an explicit error for file read errors', async () => { mockReadFile({ error: permissionError }); await expect( - resolveInstanceUuid(configService, logger) + resolveInstanceUuid({ configService, logger, syncToFile: true }) ).rejects.toThrowErrorMatchingInlineSnapshot( `"Unable to read Kibana UUID file, please check the uuid.server configuration value in kibana.yml and ensure Kibana has sufficient permissions to read / write to this file. Error was: EACCES"` ); @@ -192,7 +302,7 @@ describe('resolveInstanceUuid', () => { it('throws an explicit error for file write errors', async () => { mockWriteFile(isDirectoryError); await expect( - resolveInstanceUuid(configService, logger) + resolveInstanceUuid({ configService, logger, syncToFile: true }) ).rejects.toThrowErrorMatchingInlineSnapshot( `"Unable to write Kibana UUID file, please check the uuid.server configuration value in kibana.yml and ensure Kibana has sufficient permissions to read / write to this file. Error was: EISDIR"` ); diff --git a/src/core/server/uuid/resolve_uuid.ts b/src/core/server/uuid/resolve_uuid.ts index 3f5bdc738739..516357e10d3f 100644 --- a/src/core/server/uuid/resolve_uuid.ts +++ b/src/core/server/uuid/resolve_uuid.ts @@ -28,11 +28,21 @@ import { Logger } from '../logging'; const FILE_ENCODING = 'utf8'; const FILE_NAME = 'uuid'; +/** + * This UUID was inadvertantly shipped in the 7.6.0 distributable and should be deleted if found. + * See https://github.com/elastic/kibana/issues/57673 for more info. + */ +export const UUID_7_6_0_BUG = `ce42b997-a913-4d58-be46-bb1937feedd6`; -export async function resolveInstanceUuid( - configService: IConfigService, - logger: Logger -): Promise { +export async function resolveInstanceUuid({ + configService, + syncToFile, + logger, +}: { + configService: IConfigService; + syncToFile: boolean; + logger: Logger; +}): Promise { const [pathConfig, serverConfig] = await Promise.all([ configService .atPath(pathConfigDef.path) @@ -46,7 +56,7 @@ export async function resolveInstanceUuid( const uuidFilePath = join(pathConfig.data, FILE_NAME); - const uuidFromFile = await readUuidFromFile(uuidFilePath); + const uuidFromFile = await readUuidFromFile(uuidFilePath, logger); const uuidFromConfig = serverConfig.uuid; if (uuidFromConfig) { @@ -61,7 +71,7 @@ export async function resolveInstanceUuid( } else { logger.debug(`Updating Kibana instance UUID to: ${uuidFromConfig} (was: ${uuidFromFile})`); } - await writeUuidToFile(uuidFilePath, uuidFromConfig); + await writeUuidToFile(uuidFilePath, uuidFromConfig, syncToFile); return uuidFromConfig; } } @@ -69,7 +79,7 @@ export async function resolveInstanceUuid( const newUuid = uuid.v4(); // no uuid either in config or file, we need to generate and write it. logger.debug(`Setting new Kibana instance UUID: ${newUuid}`); - await writeUuidToFile(uuidFilePath, newUuid); + await writeUuidToFile(uuidFilePath, newUuid, syncToFile); return newUuid; } @@ -77,10 +87,17 @@ export async function resolveInstanceUuid( return uuidFromFile; } -async function readUuidFromFile(filepath: string): Promise { +async function readUuidFromFile(filepath: string, logger: Logger): Promise { try { const content = await readFile(filepath); - return content.toString(FILE_ENCODING); + const decoded = content.toString(FILE_ENCODING); + + if (decoded === UUID_7_6_0_BUG) { + logger.debug(`UUID from 7.6.0 bug detected, ignoring file UUID`); + return undefined; + } else { + return decoded; + } } catch (e) { if (e.code === 'ENOENT') { // non-existent uuid file is ok, we will create it. @@ -94,7 +111,11 @@ async function readUuidFromFile(filepath: string): Promise { } } -async function writeUuidToFile(filepath: string, uuidValue: string) { +async function writeUuidToFile(filepath: string, uuidValue: string, syncToFile: boolean) { + if (!syncToFile) { + return; + } + try { return await writeFile(filepath, uuidValue, { encoding: FILE_ENCODING }); } catch (e) { diff --git a/src/core/server/uuid/uuid_service.test.ts b/src/core/server/uuid/uuid_service.test.ts index 315df7af8aa1..a61061ff8426 100644 --- a/src/core/server/uuid/uuid_service.test.ts +++ b/src/core/server/uuid/uuid_service.test.ts @@ -23,6 +23,8 @@ import { CoreContext } from '../core_context'; import { loggingServiceMock } from '../logging/logging_service.mock'; import { mockCoreContext } from '../core_context.mock'; +import { Env } from '../config'; +import { getEnvOptions } from '../config/__mocks__/env'; jest.mock('./resolve_uuid', () => ({ resolveInstanceUuid: jest.fn().mockResolvedValue('SOME_UUID'), @@ -31,26 +33,44 @@ jest.mock('./resolve_uuid', () => ({ describe('UuidService', () => { let logger: ReturnType; let coreContext: CoreContext; - let service: UuidService; beforeEach(() => { jest.clearAllMocks(); logger = loggingServiceMock.create(); coreContext = mockCoreContext.create({ logger }); - service = new UuidService(coreContext); }); describe('#setup()', () => { - it('calls manageInstanceUuid with core configuration service', async () => { + it('calls resolveInstanceUuid with core configuration service', async () => { + const service = new UuidService(coreContext); await service.setup(); expect(resolveInstanceUuid).toHaveBeenCalledTimes(1); - expect(resolveInstanceUuid).toHaveBeenCalledWith( - coreContext.configService, - logger.get('uuid') - ); + expect(resolveInstanceUuid).toHaveBeenCalledWith({ + configService: coreContext.configService, + syncToFile: true, + logger: logger.get('uuid'), + }); }); - it('returns the uuid resolved from manageInstanceUuid', async () => { + describe('when cliArgs.optimize is true', () => { + it('calls resolveInstanceUuid with syncToFile: false', async () => { + coreContext = mockCoreContext.create({ + logger, + env: Env.createDefault(getEnvOptions({ cliArgs: { optimize: true } })), + }); + const service = new UuidService(coreContext); + await service.setup(); + expect(resolveInstanceUuid).toHaveBeenCalledTimes(1); + expect(resolveInstanceUuid).toHaveBeenCalledWith({ + configService: coreContext.configService, + syncToFile: false, + logger: logger.get('uuid'), + }); + }); + }); + + it('returns the uuid resolved from resolveInstanceUuid', async () => { + const service = new UuidService(coreContext); const setup = await service.setup(); expect(setup.getInstanceUuid()).toEqual('SOME_UUID'); }); diff --git a/src/core/server/uuid/uuid_service.ts b/src/core/server/uuid/uuid_service.ts index 10104fa70493..62ed4a19edf5 100644 --- a/src/core/server/uuid/uuid_service.ts +++ b/src/core/server/uuid/uuid_service.ts @@ -20,7 +20,7 @@ import { resolveInstanceUuid } from './resolve_uuid'; import { CoreContext } from '../core_context'; import { Logger } from '../logging'; -import { IConfigService } from '../config'; +import { IConfigService, CliArgs } from '../config'; /** * APIs to access the application's instance uuid. @@ -38,15 +38,21 @@ export interface UuidServiceSetup { export class UuidService { private readonly log: Logger; private readonly configService: IConfigService; + private readonly cliArgs: CliArgs; private uuid: string = ''; constructor(core: CoreContext) { this.log = core.logger.get('uuid'); this.configService = core.configService; + this.cliArgs = core.env.cliArgs; } public async setup() { - this.uuid = await resolveInstanceUuid(this.configService, this.log); + this.uuid = await resolveInstanceUuid({ + configService: this.configService, + syncToFile: !this.cliArgs.optimize, + logger: this.log, + }); return { getInstanceUuid: () => this.uuid, diff --git a/src/dev/build/build_distributables.js b/src/dev/build/build_distributables.js index c4f9d7f56554..6c2efeebc60c 100644 --- a/src/dev/build/build_distributables.js +++ b/src/dev/build/build_distributables.js @@ -54,6 +54,7 @@ import { VerifyExistingNodeBuildsTask, PathLengthTask, WriteShaSumsTask, + UuidVerificationTask, } from './tasks'; export async function buildDistributables(options) { @@ -136,6 +137,7 @@ export async function buildDistributables(options) { await run(CleanNodeBuildsTask); await run(PathLengthTask); + await run(UuidVerificationTask); /** * package platform-specific builds into archives diff --git a/src/dev/build/tasks/index.js b/src/dev/build/tasks/index.js index 56e813111279..8105fa8a7d5d 100644 --- a/src/dev/build/tasks/index.js +++ b/src/dev/build/tasks/index.js @@ -38,3 +38,4 @@ export * from './verify_env_task'; export * from './write_sha_sums_task'; export * from './path_length_task'; export * from './build_kibana_platform_plugins'; +export * from './uuid_verification_task'; diff --git a/src/dev/build/tasks/uuid_verification_task.js b/src/dev/build/tasks/uuid_verification_task.js new file mode 100644 index 000000000000..32c9e73dba98 --- /dev/null +++ b/src/dev/build/tasks/uuid_verification_task.js @@ -0,0 +1,38 @@ +/* + * 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 { read } from '../lib'; + +export const UuidVerificationTask = { + description: 'Verify that no UUID file is baked into the build', + + async run(config, log, build) { + const uuidFilePath = build.resolvePath('data', 'uuid'); + await read(uuidFilePath).then( + function success() { + throw new Error(`UUID file should not exist at [${uuidFilePath}]`); + }, + function error(err) { + if (err.code !== 'ENOENT') { + throw err; + } + } + ); + }, +};