From 4ed0c75b29127d551fffdba91d6486e316bd6a3a Mon Sep 17 00:00:00 2001 From: Oleg Krivtsov Date: Sun, 19 Dec 2021 20:44:42 +0700 Subject: [PATCH 1/6] Implement fix --- lib/workers/global/config/parse/file.spec.ts | 27 ++++++++++++++++++++ lib/workers/global/config/parse/file.ts | 6 +++++ 2 files changed, 33 insertions(+) diff --git a/lib/workers/global/config/parse/file.spec.ts b/lib/workers/global/config/parse/file.spec.ts index a0826752160183..4778842d74abd8 100644 --- a/lib/workers/global/config/parse/file.spec.ts +++ b/lib/workers/global/config/parse/file.spec.ts @@ -92,6 +92,33 @@ describe('workers/global/config/parse/file', () => { expect(mockProcessExit).toHaveBeenCalledWith(1); }); + it('fatal error and exit if default config.js does not exist', async () => { + const mockProcessExit = jest + .spyOn(process, 'exit') + .mockImplementation(() => undefined as never); + + await file.getConfig({}); + + expect(mockProcessExit).toHaveBeenCalledWith(1); + }); + + it('fatal error and exit if config.js contains unresolved env var', async () => { + const mockProcessExit = jest + .spyOn(process, 'exit') + .mockImplementation(() => undefined as never); + + const configFile = upath.resolve( + __dirname, + './__fixtures__/config-ref-error.js-invalid' + ); + const tmpConfigFile = upath.resolve(tmp.path, 'config-ref-error.js'); + fs.copyFileSync(configFile, tmpConfigFile); + + await file.getConfig({ RENOVATE_CONFIG_FILE: tmpConfigFile }); + + expect(mockProcessExit).toHaveBeenCalledWith(1); + }); + it.each([ ['invalid config file type', './file.txt'], ['missing config file type', './file'], diff --git a/lib/workers/global/config/parse/file.ts b/lib/workers/global/config/parse/file.ts index 90bd2aa809038e..cca43716b62bc2 100644 --- a/lib/workers/global/config/parse/file.ts +++ b/lib/workers/global/config/parse/file.ts @@ -45,6 +45,12 @@ export async function getConfig(env: NodeJS.ProcessEnv): Promise { if (err instanceof SyntaxError || err instanceof TypeError) { logger.fatal(`Could not parse config file \n ${err.stack}`); process.exit(1); + } else if (err instanceof ReferenceError) { + logger.fatal(err, `Config file parsing error: ${err.message}`); + process.exit(1); + } else if (err.code === 'MODULE_NOT_FOUND') { + logger.fatal(`Config file ${configFile} must present`); + process.exit(1); } else if (err.message === 'Unsupported file type') { logger.fatal(err.message); process.exit(1); From 4a0c5cbba42c30912bbe7599541efbef7546ed55 Mon Sep 17 00:00:00 2001 From: Oleg Krivtsov Date: Sun, 19 Dec 2021 21:24:06 +0700 Subject: [PATCH 2/6] Fix unit test --- lib/workers/global/config/parse/file.spec.ts | 24 ++++++++------------ lib/workers/global/config/parse/file.ts | 2 +- 2 files changed, 10 insertions(+), 16 deletions(-) diff --git a/lib/workers/global/config/parse/file.spec.ts b/lib/workers/global/config/parse/file.spec.ts index 4778842d74abd8..7a005f21540301 100644 --- a/lib/workers/global/config/parse/file.spec.ts +++ b/lib/workers/global/config/parse/file.spec.ts @@ -46,10 +46,6 @@ describe('workers/global/config/parse/file', () => { expect(res.rangeStrategy).toBe('bump'); }); - it('parse and returns empty config if there is no RENOVATE_CONFIG_FILE in env', async () => { - expect(await file.getConfig({})).toBeDefined(); - }); - it.each([ [ 'config.js', @@ -92,16 +88,6 @@ describe('workers/global/config/parse/file', () => { expect(mockProcessExit).toHaveBeenCalledWith(1); }); - it('fatal error and exit if default config.js does not exist', async () => { - const mockProcessExit = jest - .spyOn(process, 'exit') - .mockImplementation(() => undefined as never); - - await file.getConfig({}); - - expect(mockProcessExit).toHaveBeenCalledWith(1); - }); - it('fatal error and exit if config.js contains unresolved env var', async () => { const mockProcessExit = jest .spyOn(process, 'exit') @@ -111,11 +97,19 @@ describe('workers/global/config/parse/file', () => { __dirname, './__fixtures__/config-ref-error.js-invalid' ); - const tmpConfigFile = upath.resolve(tmp.path, 'config-ref-error.js'); + const tmpDir = tmp.path; + if (!fs.existsSync(tmpDir)) { + fs.mkdirSync(tmpDir); + } + const tmpConfigFile = upath.resolve(tmpDir, 'config-ref-error.js'); + console.log(tmpConfigFile); fs.copyFileSync(configFile, tmpConfigFile); await file.getConfig({ RENOVATE_CONFIG_FILE: tmpConfigFile }); + expect(logger.fatal).toHaveBeenCalledWith( + `Config file parsing error: CI_API_V4_URL is not defined` + ); expect(mockProcessExit).toHaveBeenCalledWith(1); }); diff --git a/lib/workers/global/config/parse/file.ts b/lib/workers/global/config/parse/file.ts index cca43716b62bc2..d7c3f348f569cd 100644 --- a/lib/workers/global/config/parse/file.ts +++ b/lib/workers/global/config/parse/file.ts @@ -46,7 +46,7 @@ export async function getConfig(env: NodeJS.ProcessEnv): Promise { logger.fatal(`Could not parse config file \n ${err.stack}`); process.exit(1); } else if (err instanceof ReferenceError) { - logger.fatal(err, `Config file parsing error: ${err.message}`); + logger.fatal(`Config file parsing error: ${err.message}`); process.exit(1); } else if (err.code === 'MODULE_NOT_FOUND') { logger.fatal(`Config file ${configFile} must present`); From bbf42fc21fe09164e97ba03d1b17ca1af5d7fb5c Mon Sep 17 00:00:00 2001 From: Oleg Krivtsov Date: Mon, 20 Dec 2021 11:25:49 +0700 Subject: [PATCH 3/6] Changes after code review --- lib/workers/global/config/parse/file.spec.ts | 7 +++++-- lib/workers/global/config/parse/file.ts | 14 ++++++++++---- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/lib/workers/global/config/parse/file.spec.ts b/lib/workers/global/config/parse/file.spec.ts index 7a005f21540301..3228b3e2cd73df 100644 --- a/lib/workers/global/config/parse/file.spec.ts +++ b/lib/workers/global/config/parse/file.spec.ts @@ -46,6 +46,10 @@ describe('workers/global/config/parse/file', () => { expect(res.rangeStrategy).toBe('bump'); }); + it('parse and returns empty config if there is no RENOVATE_CONFIG_FILE in env', async () => { + expect(await file.getConfig({})).toBeDefined(); + }); + it.each([ [ 'config.js', @@ -102,13 +106,12 @@ describe('workers/global/config/parse/file', () => { fs.mkdirSync(tmpDir); } const tmpConfigFile = upath.resolve(tmpDir, 'config-ref-error.js'); - console.log(tmpConfigFile); fs.copyFileSync(configFile, tmpConfigFile); await file.getConfig({ RENOVATE_CONFIG_FILE: tmpConfigFile }); expect(logger.fatal).toHaveBeenCalledWith( - `Config file parsing error: CI_API_V4_URL is not defined` + `Error parsing config file due to unresolved variable(s): CI_API_V4_URL is not defined` ); expect(mockProcessExit).toHaveBeenCalledWith(1); }); diff --git a/lib/workers/global/config/parse/file.ts b/lib/workers/global/config/parse/file.ts index d7c3f348f569cd..6635c7dbfc3b39 100644 --- a/lib/workers/global/config/parse/file.ts +++ b/lib/workers/global/config/parse/file.ts @@ -1,3 +1,4 @@ +import * as fs from 'fs'; import is from '@sindresorhus/is'; import { load } from 'js-yaml'; import JSON5 from 'json5'; @@ -36,6 +37,12 @@ export async function getConfig(env: NodeJS.ProcessEnv): Promise { if (!upath.isAbsolute(configFile)) { configFile = `${process.cwd()}/${configFile}`; } + + if (env.RENOVATE_CONFIG_FILE && !fs.existsSync(configFile)) { + logger.fatal(`Custom config file ${configFile} must exist`); + process.exit(1); + } + logger.debug('Checking for config file in ' + configFile); let config: AllConfig = {}; try { @@ -46,10 +53,9 @@ export async function getConfig(env: NodeJS.ProcessEnv): Promise { logger.fatal(`Could not parse config file \n ${err.stack}`); process.exit(1); } else if (err instanceof ReferenceError) { - logger.fatal(`Config file parsing error: ${err.message}`); - process.exit(1); - } else if (err.code === 'MODULE_NOT_FOUND') { - logger.fatal(`Config file ${configFile} must present`); + logger.fatal( + `Error parsing config file due to unresolved variable(s): ${err.message}` + ); process.exit(1); } else if (err.message === 'Unsupported file type') { logger.fatal(err.message); From 601034e5b5cb81a298e63b776b45132585f4fec5 Mon Sep 17 00:00:00 2001 From: Oleg Krivtsov Date: Mon, 20 Dec 2021 11:44:53 +0700 Subject: [PATCH 4/6] Add file --- .../config/parse/__fixtures__/config-ref-error.js-invalid | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 lib/workers/global/config/parse/__fixtures__/config-ref-error.js-invalid diff --git a/lib/workers/global/config/parse/__fixtures__/config-ref-error.js-invalid b/lib/workers/global/config/parse/__fixtures__/config-ref-error.js-invalid new file mode 100644 index 00000000000000..156a4ee7474d7f --- /dev/null +++ b/lib/workers/global/config/parse/__fixtures__/config-ref-error.js-invalid @@ -0,0 +1,4 @@ +// @ts-ignore +module.exports = { + endpoint: CI_API_V4_URL +}; From 41e438dc6150b5c8557eebced4d1992e49647b54 Mon Sep 17 00:00:00 2001 From: Oleg Krivtsov Date: Mon, 20 Dec 2021 15:52:42 +0700 Subject: [PATCH 5/6] Apply suggestions from code review Co-authored-by: Michael Kriese --- lib/workers/global/config/parse/file.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/workers/global/config/parse/file.ts b/lib/workers/global/config/parse/file.ts index 6635c7dbfc3b39..24ca308da39fa4 100644 --- a/lib/workers/global/config/parse/file.ts +++ b/lib/workers/global/config/parse/file.ts @@ -39,7 +39,7 @@ export async function getConfig(env: NodeJS.ProcessEnv): Promise { } if (env.RENOVATE_CONFIG_FILE && !fs.existsSync(configFile)) { - logger.fatal(`Custom config file ${configFile} must exist`); + logger.fatal({ configFile }, `Custom config file specified in RENOVATE_CONFIG_FILE must exist`); process.exit(1); } From 5a179d4e7ce9dffd92ff3df16af18cd17b2df385 Mon Sep 17 00:00:00 2001 From: Oleg Krivtsov Date: Mon, 20 Dec 2021 16:32:41 +0700 Subject: [PATCH 6/6] Changes after code review --- lib/workers/global/config/parse/file.spec.ts | 8 ++++---- lib/workers/global/config/parse/file.ts | 9 ++++++--- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/lib/workers/global/config/parse/file.spec.ts b/lib/workers/global/config/parse/file.spec.ts index 3228b3e2cd73df..f69792d891a069 100644 --- a/lib/workers/global/config/parse/file.spec.ts +++ b/lib/workers/global/config/parse/file.spec.ts @@ -1,4 +1,5 @@ import fs from 'fs'; +import fsExtra from 'fs-extra'; import { DirectoryResult, dir } from 'tmp-promise'; import upath from 'upath'; import { logger } from '../../../../logger'; @@ -102,11 +103,10 @@ describe('workers/global/config/parse/file', () => { './__fixtures__/config-ref-error.js-invalid' ); const tmpDir = tmp.path; - if (!fs.existsSync(tmpDir)) { - fs.mkdirSync(tmpDir); - } + await fsExtra.ensureDir(tmpDir); + const tmpConfigFile = upath.resolve(tmpDir, 'config-ref-error.js'); - fs.copyFileSync(configFile, tmpConfigFile); + await fsExtra.copy(configFile, tmpConfigFile); await file.getConfig({ RENOVATE_CONFIG_FILE: tmpConfigFile }); diff --git a/lib/workers/global/config/parse/file.ts b/lib/workers/global/config/parse/file.ts index 24ca308da39fa4..fa2076c3c1be4b 100644 --- a/lib/workers/global/config/parse/file.ts +++ b/lib/workers/global/config/parse/file.ts @@ -1,5 +1,5 @@ -import * as fs from 'fs'; import is from '@sindresorhus/is'; +import * as fs from 'fs-extra'; import { load } from 'js-yaml'; import JSON5 from 'json5'; import upath from 'upath'; @@ -38,8 +38,11 @@ export async function getConfig(env: NodeJS.ProcessEnv): Promise { configFile = `${process.cwd()}/${configFile}`; } - if (env.RENOVATE_CONFIG_FILE && !fs.existsSync(configFile)) { - logger.fatal({ configFile }, `Custom config file specified in RENOVATE_CONFIG_FILE must exist`); + if (env.RENOVATE_CONFIG_FILE && !(await fs.pathExists(configFile))) { + logger.fatal( + { configFile }, + `Custom config file specified in RENOVATE_CONFIG_FILE must exist` + ); process.exit(1); }