diff --git a/lib/config/validation.js b/lib/config/validation.js new file mode 100644 index 0000000000..7902f36f52 --- /dev/null +++ b/lib/config/validation.js @@ -0,0 +1,84 @@ +const options = require('./definitions').getOptions(); + +const optionTypes = {}; +options.forEach(option => { + optionTypes[option.name] = [option.type]; +}); + +module.exports = { + validateConfig, +}; + +function validateConfig(config) { + let errors = []; + + function isIgnored(key) { + const ignoredNodes = ['api', 'depType']; + return ignoredNodes.indexOf(key) !== -1; + } + + function isAFunction(value) { + const getType = {}; + return value && getType.toString.call(value) === '[object Function]'; + } + + function isObject(obj) { + return Object.prototype.toString.call(obj) === '[object Object]'; + } + + for (const key of Object.keys(config)) { + const val = config[key]; + if ( + !isIgnored(key) && // We need to ignore some reserved keys + !isAFunction(val) // Ignore all functions + ) { + if (!optionTypes[key]) { + errors.push({ + depName: 'Configuration Error', + message: `Invalid configuration option: \`${key}\``, + }); + } else { + const type = optionTypes[key].toString(); + if (type === 'boolean') { + if (val !== true && val !== false) { + errors.push({ + depName: 'Configuration Error', + message: `Configuration option \`${key}\` should be boolean`, + }); + } + } else if (type === 'list') { + if (!Array.isArray(val)) { + errors.push({ + depName: 'Configuration Error', + message: `Configuration option \`${key}\` should be a list (Array)`, + }); + } + } else if (type === 'string') { + if (!(typeof val === 'string' || val instanceof String)) { + errors.push({ + depName: 'Configuration Error', + message: `Configuration option \`${key}\` should be a string`, + }); + } + } else if (type === 'integer') { + if (val !== parseInt(val, 10)) { + errors.push({ + depName: 'Configuration Error', + message: `Configuration option \`${key}\` should be an integer`, + }); + } + } else if (type === 'json') { + if (isObject(val)) { + errors = errors.concat(module.exports.validateConfig(val)); + } else { + errors.push({ + depName: 'Configuration Error', + message: `Configuration option \`${key}\` should be a json object`, + }); + } + } + } + } + } + return errors; +} diff --git a/lib/workers/global/index.js b/lib/workers/global/index.js index f87a55d244..9118166c5b 100644 --- a/lib/workers/global/index.js +++ b/lib/workers/global/index.js @@ -2,6 +2,7 @@ const logger = require('../../logger'); const configParser = require('../../config'); const repositoryWorker = require('../repository'); const versions = require('./versions'); +const configValidation = require('../../config/validation'); module.exports = { start, @@ -11,6 +12,10 @@ module.exports = { async function start() { try { const config = await configParser.parseConfigs(process.env, process.argv); + const configErrors = configValidation.validateConfig(config); + if (configErrors.length) { + logger.error({ configErrors }, 'Found config errors'); + } config.logger = logger; config.versions = versions.detectVersions(config); // Iterate through repositories sequentially diff --git a/lib/workers/package-file/index.js b/lib/workers/package-file/index.js index 80a30cdc08..267d488a70 100644 --- a/lib/workers/package-file/index.js +++ b/lib/workers/package-file/index.js @@ -1,6 +1,7 @@ const path = require('path'); const configParser = require('../../config'); const depTypeWorker = require('../dep-type'); +const configValidation = require('../../config/validation'); let logger = require('../../logger'); @@ -11,6 +12,7 @@ module.exports = { async function renovatePackageFile(packageFileConfig) { let config = Object.assign({}, packageFileConfig); + let upgrades = []; logger = config.logger; logger.info(`Processing package file`); // If onboarding, use the package.json in onboarding branch unless a custom base branch was defined @@ -33,6 +35,23 @@ async function renovatePackageFile(packageFileConfig) { { config: packageContent.renovate }, 'package.json>renovate config' ); + const errors = configValidation.validateConfig(packageContent.renovate); + if (errors.length) { + logger.warn( + { errors }, + 'Found package.json>renovate configuration errors' + ); + /* TODO #556 + errors.forEach(error => { + upgrades.push( + Object.assign({}, error, { + depName: `${config.packageFile}(renovate)`, + type: 'error', + }) + ); + }); + */ + } // package.json>renovate config takes precedence over existing config config = configParser.mergeChildConfig(config, packageContent.renovate); } else { @@ -41,14 +60,13 @@ async function renovatePackageFile(packageFileConfig) { // Now check if config is disabled if (config.enabled === false) { logger.info('packageFile is disabled'); - return []; + return upgrades; } const depTypeConfigs = config.depTypes.map(depType => module.exports.getDepTypeConfig(config, depType) ); logger.trace({ config: depTypeConfigs }, `depTypeConfigs`); - let upgrades = []; for (const depTypeConfig of depTypeConfigs) { upgrades = upgrades.concat( await depTypeWorker.renovateDepType(packageContent, depTypeConfig) diff --git a/lib/workers/repository/apis.js b/lib/workers/repository/apis.js index 581705d778..97afea74b5 100644 --- a/lib/workers/repository/apis.js +++ b/lib/workers/repository/apis.js @@ -1,6 +1,7 @@ const ini = require('ini'); const jsonValidator = require('json-dup-key-validator'); const configParser = require('../../config'); +const configValidation = require('../../config/validation'); // API const githubApi = require('../../api/github'); const gitlabApi = require('../../api/gitlab'); @@ -119,6 +120,16 @@ async function mergeRenovateJson(config, branchName) { } renovateJson = JSON.parse(renovateJsonContent); config.logger.debug({ config: renovateJson }, 'renovate.json config'); + const renovateJsonErrors = configValidation.validateConfig(renovateJson); + if (renovateJsonErrors.length) { + config.logger.warn({ renovateJsonErrors }, 'Found renovate.json errors'); + /* TODO #556 + renovateJsonErrors.forEach(error => { + config.errors.push( + Object.assign({}, error, { depName: 'renovate.json' }) + ); + }); */ + } returnConfig = configParser.mergeChildConfig(returnConfig, renovateJson); returnConfig.renovateJsonPresent = true; } catch (err) { diff --git a/test/config/__snapshots__/validation.spec.js.snap b/test/config/__snapshots__/validation.spec.js.snap new file mode 100644 index 0000000000..e893793b75 --- /dev/null +++ b/test/config/__snapshots__/validation.spec.js.snap @@ -0,0 +1,39 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`config/validation validateConfig(config) errors for all types 1`] = ` +Array [ + Object { + "depName": "Configuration Error", + "message": "Configuration option \`enabled\` should be boolean", + }, + Object { + "depName": "Configuration Error", + "message": "Configuration option \`schedule\` should be a list (Array)", + }, + Object { + "depName": "Configuration Error", + "message": "Configuration option \`semanticPrefix\` should be a string", + }, + Object { + "depName": "Configuration Error", + "message": "Configuration option \`githubAppId\` should be an integer", + }, + Object { + "depName": "Configuration Error", + "message": "Configuration option \`lockFileMaintenance\` should be a json object", + }, +] +`; + +exports[`config/validation validateConfig(config) returns nested errors 1`] = ` +Array [ + Object { + "depName": "Configuration Error", + "message": "Invalid configuration option: \`foo\`", + }, + Object { + "depName": "Configuration Error", + "message": "Invalid configuration option: \`bar\`", + }, +] +`; diff --git a/test/config/validation.spec.js b/test/config/validation.spec.js new file mode 100644 index 0000000000..03b9854ed7 --- /dev/null +++ b/test/config/validation.spec.js @@ -0,0 +1,30 @@ +const configValidation = require('../../lib/config/validation.js'); + +describe('config/validation', () => { + describe('validateConfig(config)', () => { + it('returns nested errors', () => { + const config = { + foo: 1, + prBody: 'some-body', + lockFileMaintenance: { + bar: 2, + }, + }; + const errors = configValidation.validateConfig(config); + expect(errors).toHaveLength(2); + expect(errors).toMatchSnapshot(); + }); + it('errors for all types', () => { + const config = { + enabled: 1, + schedule: 'after 5pm', + semanticPrefix: 7, + githubAppId: 'none', + lockFileMaintenance: false, + }; + const errors = configValidation.validateConfig(config); + expect(errors).toHaveLength(5); + expect(errors).toMatchSnapshot(); + }); + }); +}); diff --git a/test/workers/global/index.spec.js b/test/workers/global/index.spec.js index 96e753228b..551f8a41b9 100644 --- a/test/workers/global/index.spec.js +++ b/test/workers/global/index.spec.js @@ -11,6 +11,13 @@ describe('lib/workers/global', () => { configParser.getRepositoryConfig = jest.fn(); repositoryWorker.renovateRepository = jest.fn(); }); + it('handles config errors', async () => { + configParser.parseConfigs.mockReturnValueOnce({ + repositories: [], + foo: 1, + }); + await globalWorker.start(); + }); it('handles zero repos', async () => { configParser.parseConfigs.mockReturnValueOnce({ repositories: [], @@ -19,7 +26,7 @@ describe('lib/workers/global', () => { }); it('processes repositories', async () => { configParser.parseConfigs.mockReturnValueOnce({ - foo: 1, + enabled: true, repositories: ['a', 'b'], }); await globalWorker.start(); diff --git a/test/workers/package-file/__snapshots__/index.spec.js.snap b/test/workers/package-file/__snapshots__/index.spec.js.snap new file mode 100644 index 0000000000..c1631b6198 --- /dev/null +++ b/test/workers/package-file/__snapshots__/index.spec.js.snap @@ -0,0 +1,3 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`packageFileWorker renovatePackageFile(config) handles renovate config errors 1`] = `Array []`; diff --git a/test/workers/package-file/index.spec.js b/test/workers/package-file/index.spec.js index 3aa0e1761a..2141b83bf7 100644 --- a/test/workers/package-file/index.spec.js +++ b/test/workers/package-file/index.spec.js @@ -22,6 +22,12 @@ describe('packageFileWorker', () => { logger, }); }); + it('handles renovate config errors', async () => { + config.enabled = false; + config.api.getFileJson.mockReturnValueOnce({ renovate: { foo: 1 } }); + const res = await packageFileWorker.renovatePackageFile(config); + expect(res).toMatchSnapshot(); + }); it('handles null', async () => { const allUpgrades = await packageFileWorker.renovatePackageFile(config); expect(allUpgrades).toHaveLength(1); diff --git a/test/workers/repository/__snapshots__/apis.spec.js.snap b/test/workers/repository/__snapshots__/apis.spec.js.snap index 37bbfa14db..c9858fc3fe 100644 --- a/test/workers/repository/__snapshots__/apis.spec.js.snap +++ b/test/workers/repository/__snapshots__/apis.spec.js.snap @@ -52,7 +52,7 @@ exports[`workers/repository/apis mergeRenovateJson(config) returns error in conf Array [ Object { "depName": "renovate.json", - "message": "Syntax error: expecting String near { foo: 1 }", + "message": "Syntax error: expecting String near { enabled:", }, ] `; @@ -61,7 +61,9 @@ exports[`workers/repository/apis mergeRenovateJson(config) returns error plus ex Array [ Object { "depName": "renovate.json", - "message": "Syntax error: duplicated keys \\"foo\\" near \\"foo\\": 2 }", + "message": "Syntax error: duplicated keys \\"enabled\\" near \\": false }", }, ] `; + +exports[`workers/repository/apis mergeRenovateJson(config) returns error plus extended config if unknown keys 1`] = `Array []`; diff --git a/test/workers/repository/apis.spec.js b/test/workers/repository/apis.spec.js index 932562b4d7..4ece3f4b6d 100644 --- a/test/workers/repository/apis.spec.js +++ b/test/workers/repository/apis.spec.js @@ -129,29 +129,41 @@ describe('workers/repository/apis', () => { expect(await apis.mergeRenovateJson(config)).toEqual(config); }); it('returns extended config if renovate.json found', async () => { - config.api.getFileContent.mockReturnValueOnce('{ "foo": 1 }'); + config.api.getFileContent.mockReturnValueOnce('{ "enabled": true }'); const returnConfig = await apis.mergeRenovateJson(config); - expect(returnConfig.foo).toBe(1); + expect(returnConfig.enabled).toBe(true); expect(returnConfig.renovateJsonPresent).toBe(true); expect(returnConfig.errors).toHaveLength(0); }); - it('returns error plus extended config if duplicate keys', async () => { - config.api.getFileContent.mockReturnValueOnce('{ "foo": 1, "foo": 2 }'); + it('returns error plus extended config if unknown keys', async () => { + config.api.getFileContent.mockReturnValueOnce( + '{ "enabled": true, "foo": false }' + ); const returnConfig = await apis.mergeRenovateJson(config); - expect(returnConfig.foo).toBe(2); + expect(returnConfig.enabled).toBe(true); + expect(returnConfig.renovateJsonPresent).toBe(true); + expect(returnConfig.errors).toHaveLength(0); // TODO: Update to 1 later + expect(returnConfig.errors).toMatchSnapshot(); + }); + it('returns error plus extended config if duplicate keys', async () => { + config.api.getFileContent.mockReturnValueOnce( + '{ "enabled": true, "enabled": false }' + ); + const returnConfig = await apis.mergeRenovateJson(config); + expect(returnConfig.enabled).toBe(false); expect(returnConfig.renovateJsonPresent).toBe(true); expect(returnConfig.errors).toHaveLength(1); expect(returnConfig.errors).toMatchSnapshot(); }); it('returns error in config if renovate.json cannot be parsed', async () => { - config.api.getFileContent.mockReturnValueOnce('{ foo: 1 }'); + config.api.getFileContent.mockReturnValueOnce('{ enabled: true }'); const returnConfig = await apis.mergeRenovateJson(config); - expect(returnConfig.foo).toBeUndefined(); + expect(returnConfig.enabled).toBeUndefined(); expect(returnConfig.renovateJsonPresent).toBeUndefined(); expect(returnConfig.errors).toMatchSnapshot(); }); it('returns error in JSON.parse', async () => { - config.api.getFileContent.mockReturnValueOnce('{ foo: 1 }'); + config.api.getFileContent.mockReturnValueOnce('{ enabled: true }'); jsonValidator.validate = jest.fn(); jsonValidator.validate.mockReturnValueOnce(false); jsonValidator.validate.mockReturnValueOnce(false);