Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not write UUID file during optimize process #58899

Merged
merged 6 commits into from
Feb 28, 2020
Merged

Do not write UUID file during optimize process #58899

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1054bd4
Do not write UUID file during optimize process
joshdover Feb 28, 2020
0efd204
Add UUID verification task
joshdover Feb 28, 2020
cd28676
Ignore 7.6.0 UUID if found
joshdover Feb 28, 2020
7bc0225
Use build.resolvePath
joshdover Feb 28, 2020
d0bc4d1
Merge remote-tracking branch 'upstream/master' into np/fix-uuid-on-op…
joshdover Feb 28, 2020
51f1b8f
Test nits
joshdover Feb 28, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
230 changes: 170 additions & 60 deletions src/core/server/uuid/resolve_uuid.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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.toHaveBeenCalledWith();
joshdover marked this conversation as resolved.
Show resolved Hide resolved
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);
Expand All @@ -160,39 +198,111 @@ describe('resolveInstanceUuid', () => {
});
});

describe('when file is present with 7.6.0 UUID', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is the right thing to do to fix the monitoring data for HA Kibana deployments, but to be clear this means all users who install 7.6.0 and then upgrade will get new instance UUIDs for the same instances after upgrading. Seems like an acceptable tradeoff to me?

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.toHaveBeenCalledWith();
joshdover marked this conversation as resolved.
Show resolved Hide resolved
expect(logger.debug).toHaveBeenCalledTimes(1);
expect(logger.debug.mock.calls[0]).toMatchInlineSnapshot(`
Array [
"Setting new Kibana instance UUID: NEW_UUID",
]
`);
});
});
});

describe('when file access error occurs', () => {
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"`
);
});
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"`
);
Expand Down
41 changes: 31 additions & 10 deletions src/core/server/uuid/resolve_uuid.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<string> {
export async function resolveInstanceUuid({
configService,
syncToFile,
logger,
}: {
configService: IConfigService;
syncToFile: boolean;
logger: Logger;
}): Promise<string> {
const [pathConfig, serverConfig] = await Promise.all([
configService
.atPath<PathConfigType>(pathConfigDef.path)
Expand All @@ -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) {
Expand All @@ -61,26 +71,33 @@ 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;
}
}
if (uuidFromFile === undefined) {
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;
}

logger.debug(`Resuming persistent Kibana instance UUID: ${uuidFromFile}`);
return uuidFromFile;
}

async function readUuidFromFile(filepath: string): Promise<string | undefined> {
async function readUuidFromFile(filepath: string, logger: Logger): Promise<string | undefined> {
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.
Expand All @@ -94,7 +111,11 @@ async function readUuidFromFile(filepath: string): Promise<string | undefined> {
}
}

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) {
Expand Down
Loading