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

Jump to bottom

feat(config): validate top-level options #28383

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

feat(config): validate top-level options #28383

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bc80c8c
feat(config): validate top-level options
rarkins Apr 13, 2024
6b73d14
DRY
rarkins Apr 13, 2024
f151414
drop dequal
rarkins Apr 13, 2024
0a5dfc8
Merge branch 'main' into feat/28300-enforce-top-level-options
rarkins May 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions lib/config/defaults.ts
View file
Open in desktop
@@ -1,4 +1,4 @@
import { getOptions } from './options';
import { getOptions, isTopLevelOnlyOption } from './options';
import type { AllConfig, RenovateOptions } from './types';

// Use functions instead of direct values to avoid introducing global references.
Expand All @@ -23,7 +23,7 @@ export function getConfig(): AllConfig {
const options = getOptions();
const config: AllConfig = {};
options.forEach((option) => {
if (!option.parents) {
if (!option.parents || isTopLevelOnlyOption(option)) {
config[option.name] = getDefault(option);
}
});
Expand Down
30 changes: 30 additions & 0 deletions lib/config/options/index.ts
View file
Open in desktop
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We need to check that each of these changes is correct and won't have unintended side effects.

Expand Up @@ -173,6 +173,7 @@ const options: RenovateOptions[] = [
experimentalDescription:
'Config migration PRs are still being improved, in particular to reduce the amount of reordering and whitespace changes.',
experimentalIssues: [16359],
parents: ['.'],
},
{
name: 'productLinks',
Expand Down Expand Up @@ -232,6 +233,7 @@ const options: RenovateOptions[] = [
subType: 'string',
allowString: true,
cli: false,
parents: ['.'],
},
{
name: 'migratePresets',
Expand Down Expand Up @@ -364,6 +366,7 @@ const options: RenovateOptions[] = [
'If enabled, Renovate logs the fully resolved config for each repository, plus the fully resolved presets.',
type: 'boolean',
default: false,
parents: ['.'],
},
{
name: 'binarySource',
Expand Down Expand Up @@ -434,6 +437,7 @@ const options: RenovateOptions[] = [
experimentalIssues: [23286],
default: {},
mergeable: true,
parents: ['.'],
},
{
name: 'dockerChildPrefix',
Expand Down Expand Up @@ -543,6 +547,7 @@ const options: RenovateOptions[] = [
type: 'string',
allowedValues: ['auto', 'enabled', 'disabled'],
default: 'auto',
parents: ['.'],
},
{
name: 'includeMirrors',
Expand Down Expand Up @@ -649,6 +654,7 @@ const options: RenovateOptions[] = [
'Whether to create a "Dependency Dashboard" issue in the repository.',
type: 'boolean',
default: false,
parents: ['.'],
},
{
name: 'dependencyDashboardApproval',
Expand All @@ -663,12 +669,14 @@ const options: RenovateOptions[] = [
'Set to `true` to let Renovate close the Dependency Dashboard issue if there are no more updates.',
type: 'boolean',
default: false,
parents: ['.'],
},
{
name: 'dependencyDashboardTitle',
description: 'Title for the Dependency Dashboard issue.',
type: 'string',
default: `Dependency Dashboard`,
parents: ['.'],
},
{
name: 'dependencyDashboardHeader',
Expand All @@ -677,12 +685,14 @@ const options: RenovateOptions[] = [
type: 'string',
default:
'This issue lists Renovate updates and detected dependencies. Read the [Dependency Dashboard](https://docs.renovatebot.com/key-concepts/dashboard/) docs to learn more.',
parents: ['.'],
},
{
name: 'dependencyDashboardFooter',
description:
'Any text added here will be placed last in the Dependency Dashboard issue body, with a divider separator before it.',
type: 'string',
parents: ['.'],
},
{
name: 'dependencyDashboardLabels',
Expand All @@ -691,6 +701,7 @@ const options: RenovateOptions[] = [
type: 'array',
subType: 'string',
default: null,
parents: ['.'],
},
{
name: 'dependencyDashboardOSVVulnerabilitySummary',
Expand All @@ -700,13 +711,15 @@ const options: RenovateOptions[] = [
allowedValues: ['none', 'all', 'unresolved'],
default: 'none',
experimental: true,
parents: ['.'],
},
{
name: 'configWarningReuseIssue',
description:
'Set this to `false` to make Renovate create a new issue for each config warning, instead of reopening or reusing an existing issue.',
type: 'boolean',
default: true,
parents: ['.'],
},

// encryption
Expand Down Expand Up @@ -986,6 +999,7 @@ const options: RenovateOptions[] = [
subType: 'string',
stage: 'package',
cli: false,
parents: ['.'],
},
{
name: 'useBaseBranchConfig',
Expand All @@ -994,6 +1008,7 @@ const options: RenovateOptions[] = [
type: 'string',
allowedValues: ['merge', 'none'],
default: 'none',
parents: ['.'],
},
{
name: 'gitAuthor',
Expand Down Expand Up @@ -1033,6 +1048,7 @@ const options: RenovateOptions[] = [
subType: 'string',
mergeable: false,
stage: 'repository',
parents: ['.'],
},
{
name: 'includePaths',
Expand Down Expand Up @@ -1932,20 +1948,23 @@ const options: RenovateOptions[] = [
'Rate limit PRs to maximum x created per hour. 0 means no limit.',
type: 'integer',
default: 2,
parents: ['.'],
},
{
name: 'prConcurrentLimit',
description:
'Limit to a maximum of x concurrent branches/PRs. 0 means no limit.',
type: 'integer',
default: 10,
parents: ['.'],
},
{
name: 'branchConcurrentLimit',
description:
'Limit to a maximum of x concurrent branches. 0 means no limit, `null` (default) inherits value from `prConcurrentLimit`.',
type: 'integer',
default: null, // inherit prConcurrentLimit
parents: ['.'],
},
{
name: 'prPriority',
Expand Down Expand Up @@ -2044,6 +2063,7 @@ const options: RenovateOptions[] = [
cli: false,
env: false,
supportedPlatforms: ['github'],
parents: ['.'],
},
{
name: 'osvVulnerabilityAlerts',
Expand All @@ -2052,6 +2072,7 @@ const options: RenovateOptions[] = [
default: false,
experimental: true,
experimentalIssues: [20542],
parents: ['.'],
},
{
name: 'pruneBranchAfterAutomerge',
Expand Down Expand Up @@ -2198,6 +2219,7 @@ const options: RenovateOptions[] = [
additionalProperties: {
type: 'string',
},
parents: ['.'],
},
{
name: 'lockFileMaintenance',
Expand Down Expand Up @@ -2427,6 +2449,7 @@ const options: RenovateOptions[] = [
stage: 'repository',
cli: true,
mergeable: true,
parents: ['.'],
},
{
name: 'hostType',
Expand Down Expand Up @@ -2718,6 +2741,7 @@ const options: RenovateOptions[] = [
stage: 'package',
cli: true,
mergeable: true,
parents: ['.'],
},
{
name: 'customType',
Expand Down Expand Up @@ -2936,6 +2960,7 @@ const options: RenovateOptions[] = [
experimental: true,
globalOnly: true,
default: [],
parents: ['.'],
},
{
name: 'maxRetryAfter',
Expand All @@ -2956,6 +2981,7 @@ const options: RenovateOptions[] = [
stage: 'repository',
cli: false,
env: false,
parents: ['.'],
},
{
name: 'matchMessage',
Expand Down Expand Up @@ -2987,6 +3013,10 @@ export function getOptions(): RenovateOptions[] {
return options;
}

export function isTopLevelOnlyOption(option: RenovateOptions): boolean {
return option.parents?.length === 1 && option.parents[0] === '.';
}

function loadManagerOptions(): void {
const allManagers = new Map([...getManagers(), ...getCustomManagers()]);
for (const [name, config] of allManagers.entries()) {
Expand Down
1 change: 1 addition & 0 deletions lib/config/types.ts
View file
Open in desktop
Expand Up @@ -387,6 +387,7 @@ export interface ValidationMessage {
}

export type AllowedParents =
| '.' // Root
| 'customManagers'
| 'customDatasources'
| 'hostRules'
Expand Down
6 changes: 5 additions & 1 deletion lib/config/validation.ts
View file
Open in desktop
Expand Up @@ -263,9 +263,13 @@ export async function validateConfig(
!optionParents[key].includes(parentName as AllowedParents)
) {
// TODO: types (#22198)
const message = `${key} should only be configured within one of "${optionParents[
let message = `${key} should only be configured within one of "${optionParents[
key
]?.join(' or ')}" objects. Was found in ${parentName}`;
message = message.replace(
'within one of "." objects',
'at the top level',
);
warnings.push({
topic: `${parentPath ? `${parentPath}.` : ''}${key}`,
message,
Expand Down
6 changes: 3 additions & 3 deletions test/documentation.spec.ts
View file
Open in desktop
@@ -1,6 +1,6 @@
import fs from 'fs-extra';
import { glob } from 'glob';
import { getOptions } from '../lib/config/options';
import { getOptions, isTopLevelOnlyOption } from '../lib/config/options';
import { regEx } from '../lib/util/regex';

const options = getOptions();
Expand Down Expand Up @@ -31,7 +31,7 @@ describe('documentation', () => {

const expectedOptions = options
.filter((option) => !option.globalOnly)
.filter((option) => !option.parents)
.filter((option) => !option.parents || isTopLevelOnlyOption(option))
.filter((option) => !option.autogenerated)
.map((option) => option.name)
.sort();
Expand All @@ -51,7 +51,7 @@ describe('documentation', () => {
const expectedSubOptions = options
.filter((option) => option.stage !== 'global')
.filter((option) => !option.globalOnly)
.filter((option) => option.parents)
.filter((option) => option.parents && !isTopLevelOnlyOption(option))
.map((option) => option.name)
.sort();
expectedSubOptions.sort();
Expand Down
8 changes: 7 additions & 1 deletion tools/docs/schema.ts
View file
Open in desktop
Expand Up @@ -75,7 +75,7 @@ function createSingleConfig(option: RenovateOptions): Record<string, unknown> {

function createSchemaForParentConfigs(): void {
for (const option of options) {
if (!option.parents) {
if (!option.parents || option.parents.includes('.')) {
properties[option.name] = createSingleConfig(option);
}
}
Expand All @@ -85,6 +85,9 @@ function addChildrenArrayInParents(): void {
for (const option of options) {
if (option.parents) {
for (const parent of option.parents) {
if (parent === '.') {
continue;
}
properties[parent].items = {
allOf: [
{
Expand Down Expand Up @@ -120,6 +123,9 @@ function createSchemaForChildConfigs(): void {
for (const option of options) {
if (option.parents) {
for (const parent of option.parents) {
if (parent === '.') {
continue;
}
properties[parent].items.allOf[0].properties[option.name] =
createSingleConfig(option);
}
Expand Down