From dcab56734949a94cac16d708b515d64f28c50d29 Mon Sep 17 00:00:00 2001 From: RahulGautamSingh Date: Mon, 3 Jun 2024 21:22:46 +0545 Subject: [PATCH] feat(config/validation): add validation for negative numbers (#29178) --- .../__snapshots__/validation.spec.ts.snap | 2 +- lib/config/options/index.ts | 1 + lib/config/types.ts | 5 ++ lib/config/validation.spec.ts | 56 ++++++++++++++++- lib/config/validation.ts | 60 ++++++++++++------- 5 files changed, 99 insertions(+), 25 deletions(-) diff --git a/lib/config/__snapshots__/validation.spec.ts.snap b/lib/config/__snapshots__/validation.spec.ts.snap index c2c5c75f5c3859..3887cf94919695 100644 --- a/lib/config/__snapshots__/validation.spec.ts.snap +++ b/lib/config/__snapshots__/validation.spec.ts.snap @@ -38,7 +38,7 @@ exports[`config/validation validateConfig(config) catches invalid templates 1`] exports[`config/validation validateConfig(config) errors for all types 1`] = ` [ { - "message": "Configuration option \`azureWorkItemId\` should be an integer. Found: false (boolean)", + "message": "Configuration option \`azureWorkItemId\` should be an integer. Found: false (boolean).", "topic": "Configuration Error", }, { diff --git a/lib/config/options/index.ts b/lib/config/options/index.ts index a2eaddf718783a..2c7894a0e28948 100644 --- a/lib/config/options/index.ts +++ b/lib/config/options/index.ts @@ -2065,6 +2065,7 @@ const options: RenovateOptions[] = [ description: 'Set sorting priority for PR creation. PRs with higher priority are created first, negative priority last.', type: 'integer', + allowNegative: true, default: 0, parents: ['packageRules'], cli: false, diff --git a/lib/config/types.ts b/lib/config/types.ts index 6333f5b51e63cc..cbf76ea436c896 100644 --- a/lib/config/types.ts +++ b/lib/config/types.ts @@ -454,6 +454,11 @@ export interface RenovateOptionBase { * For internal use only: add it to any config option that supports regex or glob matching */ patternMatch?: boolean; + + /** + * For internal use only: add it to any config option of type integer that supports negative integers + */ + allowNegative?: boolean; } export interface RenovateArrayOption< diff --git a/lib/config/validation.spec.ts b/lib/config/validation.spec.ts index 39e6d11cea7ad1..ee8e4d5a5fa040 100644 --- a/lib/config/validation.spec.ts +++ b/lib/config/validation.spec.ts @@ -1308,6 +1308,41 @@ describe('config/validation', () => { expect(errors).toHaveLength(1); expect(warnings).toHaveLength(0); }); + + it('catches when negative number is used for integer type', async () => { + const config = { + azureWorkItemId: -2, + }; + const { errors } = await configValidation.validateConfig('repo', config); + expect(errors).toMatchObject([ + { + message: + 'Configuration option `azureWorkItemId` should be a positive integer. Found negative value instead.', + topic: 'Configuration Error', + }, + ]); + }); + + it('validates prPriority', async () => { + const config = { + packageRules: [ + { + matchDepNames: ['somedep'], + prPriority: -2, + }, + { + matchDepNames: ['some-other-dep'], + prPriority: 2, + }, + ], + }; + const { errors, warnings } = await configValidation.validateConfig( + 'repo', + config, + ); + expect(errors).toBeEmptyArray(); + expect(warnings).toBeEmptyArray(); + }); }); describe('validateConfig() -> globaOnly options', () => { @@ -1680,13 +1715,30 @@ describe('config/validation', () => { ); expect(warnings).toMatchObject([ { - message: 'Configuration option `secrets` should be a JSON object.', topic: 'Configuration Error', + message: + 'Configuration option `cacheTtlOverride.someField` should be an integer. Found: false (boolean).', }, { + message: 'Configuration option `secrets` should be a JSON object.', topic: 'Configuration Error', + }, + ]); + }); + + it('warns if negative number is used for integer type', async () => { + const config = { + prCommitsPerRunLimit: -2, + }; + const { warnings } = await configValidation.validateConfig( + 'global', + config, + ); + expect(warnings).toMatchObject([ + { message: - 'Invalid `cacheTtlOverride.someField` configuration: value must be an integer.', + 'Configuration option `prCommitsPerRunLimit` should be a positive integer. Found negative value instead.', + topic: 'Configuration Error', }, ]); }); diff --git a/lib/config/validation.ts b/lib/config/validation.ts index cbc87d6da6f5cf..15b87c3b144022 100644 --- a/lib/config/validation.ts +++ b/lib/config/validation.ts @@ -46,6 +46,7 @@ let optionParents: Record; let optionGlobals: Set; let optionInherits: Set; let optionRegexOrGlob: Set; +let optionAllowsNegativeIntegers: Set; const managerList = getManagerList(); @@ -89,6 +90,33 @@ function validatePlainObject(val: Record): true | string { return true; } +function validateNumber( + key: string, + val: unknown, + currentPath?: string, + subKey?: string, +): ValidationMessage[] { + const errors: ValidationMessage[] = []; + const path = `${currentPath}${subKey ? '.' + subKey : ''}`; + if (is.number(val)) { + if (val < 0 && !optionAllowsNegativeIntegers.has(key)) { + errors.push({ + topic: 'Configuration Error', + message: `Configuration option \`${path}\` should be a positive integer. Found negative value instead.`, + }); + } + } else { + errors.push({ + topic: 'Configuration Error', + message: `Configuration option \`${path}\` should be an integer. Found: ${JSON.stringify( + val, + )} (${typeof val}).`, + }); + } + + return errors; +} + function getUnsupportedEnabledManagers(enabledManagers: string[]): string[] { return enabledManagers.filter( (manager) => !allManagersList.includes(manager.replace('custom.', '')), @@ -126,6 +154,7 @@ function initOptions(): void { optionTypes = {}; optionRegexOrGlob = new Set(); optionGlobals = new Set(); + optionAllowsNegativeIntegers = new Set(); for (const option of options) { optionTypes[option.name] = option.type; @@ -145,6 +174,10 @@ function initOptions(): void { if (option.globalOnly) { optionGlobals.add(option.name); } + + if (option.allowNegative) { + optionAllowsNegativeIntegers.add(option.name); + } } optionsInitialized = true; @@ -334,14 +367,7 @@ export async function validateConfig( }); } } else if (type === 'integer') { - if (!is.number(val)) { - errors.push({ - topic: 'Configuration Error', - message: `Configuration option \`${currentPath}\` should be an integer. Found: ${JSON.stringify( - val, - )} (${typeof val})`, - }); - } + errors.push(...validateNumber(key, val, currentPath)); } else if (type === 'array' && val) { if (is.array(val)) { for (const [subIndex, subval] of val.entries()) { @@ -964,14 +990,7 @@ async function validateGlobalConfig( }); } } else if (type === 'integer') { - if (!is.number(val)) { - warnings.push({ - topic: 'Configuration Error', - message: `Configuration option \`${currentPath}\` should be an integer. Found: ${JSON.stringify( - val, - )} (${typeof val}).`, - }); - } + warnings.push(...validateNumber(key, val, currentPath)); } else if (type === 'boolean') { if (val !== true && val !== false) { warnings.push({ @@ -1037,12 +1056,9 @@ async function validateGlobalConfig( } } else if (key === 'cacheTtlOverride') { for (const [subKey, subValue] of Object.entries(val)) { - if (!is.number(subValue)) { - warnings.push({ - topic: 'Configuration Error', - message: `Invalid \`${currentPath}.${subKey}\` configuration: value must be an integer.`, - }); - } + warnings.push( + ...validateNumber(key, subValue, currentPath, subKey), + ); } } else { const res = validatePlainObject(val);