Skip to content

Commit

Permalink
feat(config/validation): add validation for negative numbers (#29178)
Browse files Browse the repository at this point in the history
  • Loading branch information
RahulGautamSingh committed Jun 3, 2024
1 parent c89ae5c commit dcab567
Show file tree
Hide file tree
Showing 5 changed files with 99 additions and 25 deletions.
2 changes: 1 addition & 1 deletion lib/config/__snapshots__/validation.spec.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -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",
},
{
Expand Down
1 change: 1 addition & 0 deletions lib/config/options/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
5 changes: 5 additions & 0 deletions lib/config/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<
Expand Down
56 changes: 54 additions & 2 deletions lib/config/validation.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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',
},
]);
});
Expand Down
60 changes: 38 additions & 22 deletions lib/config/validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ let optionParents: Record<string, AllowedParents[]>;
let optionGlobals: Set<string>;
let optionInherits: Set<string>;
let optionRegexOrGlob: Set<string>;
let optionAllowsNegativeIntegers: Set<string>;

const managerList = getManagerList();

Expand Down Expand Up @@ -89,6 +90,33 @@ function validatePlainObject(val: Record<string, unknown>): 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.', '')),
Expand Down Expand Up @@ -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;
Expand All @@ -145,6 +174,10 @@ function initOptions(): void {
if (option.globalOnly) {
optionGlobals.add(option.name);
}

if (option.allowNegative) {
optionAllowsNegativeIntegers.add(option.name);
}
}

optionsInitialized = true;
Expand Down Expand Up @@ -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()) {
Expand Down Expand Up @@ -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({
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit dcab567

Please sign in to comment.