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.

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

feat(config): use enum values for configMigration #30000

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions docs/usage/configuration-options.md
Original file line number Diff line number Diff line change
Expand Up @@ -554,6 +554,12 @@ If enabled, all issues created by Renovate are set as confidential, even in a pu
If enabled, Renovate raises a pull request when it needs to migrate the Renovate config file.
Renovate only performs `configMigration` on `.json` and `.json5` files.

You can select which behavior you want from Renovate:

- `auto` (default): Currently does nothing (same as `disabled`), but new behavior is in development
- `enabled`: Renovate creates a migration PR when needed
- `disabled`: Renovate never creates a migration PR

We're adding new features to Renovate bot often.
Often you can keep using your Renovate config and use the new features right away.
But sometimes you need to update your Renovate configuration.
Expand Down
2 changes: 1 addition & 1 deletion docs/usage/upgrade-best-practices.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ The [`config:best-practices` preset](./presets-config.md#configbest-practices) h

```json
{
"configMigration": true,
"configMigration": "enabled",
"extends": [
"config:recommended",
"docker:pinDigests",
Expand Down
39 changes: 39 additions & 0 deletions lib/config/migrations/custom/config-migration-migration.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import { ConfigMigrationMigration } from './config-migration-migration';

describe('config/migrations/custom/config-migration-migration', () => {
it('should migrate true to enabled', () => {
expect(ConfigMigrationMigration).toMigrate(
{
// @ts-expect-error invalid options
configMigration: true,
},
{
configMigration: 'enabled',
},
);
});

it('should migrate false to disabled', () => {
expect(ConfigMigrationMigration).toMigrate(
{
// @ts-expect-error invalid options
configMigration: false,
},
{
configMigration: 'disabled',
},
);
});

it('should not migrate', () => {
expect(ConfigMigrationMigration).toMigrate(
{
configMigration: 'auto',
},
{
configMigration: 'auto',
},
false,
);
});
});
14 changes: 14 additions & 0 deletions lib/config/migrations/custom/config-migration-migration.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { AbstractMigration } from '../base/abstract-migration';

export class ConfigMigrationMigration extends AbstractMigration {
override readonly propertyName = 'configMigration';

override run(value: unknown): void {
if (value === true) {
this.rewrite('enabled');
}
if (value === false) {
this.rewrite('disabled');
}
}
}
2 changes: 2 additions & 0 deletions lib/config/migrations/migrations-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { BranchNameMigration } from './custom/branch-name-migration';
import { BranchPrefixMigration } from './custom/branch-prefix-migration';
import { CompatibilityMigration } from './custom/compatibility-migration';
import { ComposerIgnorePlatformReqsMigration } from './custom/composer-ignore-platform-reqs-migration';
import { ConfigMigrationMigration } from './custom/config-migration-migration';
import { CustomManagersMigration } from './custom/custom-managers-migration';
import { DatasourceMigration } from './custom/datasource-migration';
import { DepTypesMigration } from './custom/dep-types-migration';
Expand Down Expand Up @@ -156,6 +157,7 @@ export class MigrationsService {
FetchReleaseNotesMigration,
MatchManagersMigration,
CustomManagersMigration,
ConfigMigrationMigration,
];

static run(originalConfig: RenovateConfig): RenovateConfig {
Expand Down
5 changes: 3 additions & 2 deletions lib/config/options/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -226,8 +226,9 @@ const options: RenovateOptions[] = [
name: 'configMigration',
description: 'Enable this to get config migration PRs when needed.',
stage: 'repository',
type: 'boolean',
default: false,
type: 'string',
allowedValues: ['auto', 'disabled', 'enabled'],
default: 'auto',
experimental: true,
experimentalDescription:
'Config migration PRs are still being improved, in particular to reduce the amount of reordering and whitespace changes.',
Expand Down
2 changes: 1 addition & 1 deletion lib/config/presets/internal/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import type { Preset } from '../types';

export const presets: Record<string, Preset> = {
'best-practices': {
configMigration: true,
configMigration: 'enabled',
description:
'Preset with best practices from the Renovate maintainers. Recommended for advanced users, who want to follow our best practices.',
extends: [
Expand Down
2 changes: 1 addition & 1 deletion lib/config/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -557,7 +557,7 @@ export interface PackageRuleInputConfig extends Record<string, unknown> {
}

export interface ConfigMigration {
configMigration?: boolean;
configMigration?: 'auto' | 'disabled' | 'enabled';
}

export interface MigratedConfig {
Expand Down
28 changes: 17 additions & 11 deletions lib/workers/global/config/parse/cli.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,19 +33,19 @@ describe('workers/global/config/parse/cli', () => {
});

it('supports boolean no value', () => {
argv.push('--config-migration');
expect(cli.getConfig(argv)).toEqual({ configMigration: true });
argv.push('--draft-p-r');
expect(cli.getConfig(argv)).toEqual({ draftPR: true });
argv = argv.slice(0, -1);
});

it('supports boolean space true', () => {
argv.push('--config-migration');
argv.push('--draft-p-r');
argv.push('true');
expect(cli.getConfig(argv)).toEqual({ configMigration: true });
expect(cli.getConfig(argv)).toEqual({ draftPR: true });
});

it('throws exception for invalid boolean value', () => {
argv.push('--config-migration');
argv.push('--draft-p-r');
argv.push('badvalue');
expect(() => cli.getConfig(argv)).toThrow(
Error(
Expand All @@ -55,19 +55,19 @@ describe('workers/global/config/parse/cli', () => {
});

it('supports boolean space false', () => {
argv.push('--config-migration');
argv.push('--draft-p-r');
argv.push('false');
expect(cli.getConfig(argv)).toEqual({ configMigration: false });
expect(cli.getConfig(argv)).toEqual({ draftPR: false });
});

it('supports boolean equals true', () => {
argv.push('--config-migration=true');
expect(cli.getConfig(argv)).toEqual({ configMigration: true });
argv.push('--draft-p-r=true');
expect(cli.getConfig(argv)).toEqual({ draftPR: true });
});

it('supports boolean equals false', () => {
argv.push('--config-migration=false');
expect(cli.getConfig(argv)).toEqual({ configMigration: false });
argv.push('--draft-p-r=false');
expect(cli.getConfig(argv)).toEqual({ draftPR: false });
});

it('supports list single', () => {
Expand Down Expand Up @@ -136,6 +136,12 @@ describe('workers/global/config/parse/cli', () => {
${'--recreate-when=auto'} | ${{ recreateWhen: 'auto' }}
${'--recreate-when=always'} | ${{ recreateWhen: 'always' }}
${'--recreate-when=never'} | ${{ recreateWhen: 'never' }}
${'--config-migration=false'} | ${{ configMigration: 'disabled' }}
${'--config-migration=true'} | ${{ configMigration: 'enabled' }}
${'--config-migration'} | ${{ configMigration: 'enabled' }}
${'--config-migration=auto'} | ${{ configMigration: 'auto' }}
${'--config-migration=enabled'} | ${{ configMigration: 'enabled' }}
${'--config-migration=disabled'} | ${{ configMigration: 'disabled' }}
`('"$arg" -> $config', ({ arg, config }) => {
argv.push(arg);
expect(cli.getConfig(argv)).toMatchObject(config);
Expand Down
5 changes: 4 additions & 1 deletion lib/workers/global/config/parse/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,10 @@ export function getConfig(input: string[]): AllConfig {
.replace('--include-forks', '--fork-processing=enabled')
.replace('--recreate-closed=false', '--recreate-when=auto')
.replace('--recreate-closed=true', '--recreate-when=always')
.replace('--recreate-closed', '--recreate-when=always'),
.replace('--recreate-closed', '--recreate-when=always')
.replace('--config-migration=true', '--config-migration=enabled')
.replace('--config-migration=false', '--config-migration=disabled')
.replace(/^--config-migration$/, '--config-migration=enabled'),
)
.filter((a) => !a.startsWith('--git-fs'));
const options = getOptions();
Expand Down
27 changes: 16 additions & 11 deletions lib/workers/global/config/parse/env.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,20 +10,20 @@ describe('workers/global/config/parse/env', () => {
});

it('supports boolean true', async () => {
const envParam: NodeJS.ProcessEnv = { RENOVATE_CONFIG_MIGRATION: 'true' };
expect((await env.getConfig(envParam)).configMigration).toBeTrue();
const envParam: NodeJS.ProcessEnv = { RENOVATE_DRAFT_P_R: 'true' };
expect((await env.getConfig(envParam)).draftPR).toBeTrue();
});

it('supports boolean false', async () => {
const envParam: NodeJS.ProcessEnv = {
RENOVATE_CONFIG_MIGRATION: 'false',
RENOVATE_DRAFT_P_R: 'false',
};
expect((await env.getConfig(envParam)).configMigration).toBeFalse();
expect((await env.getConfig(envParam)).draftPR).toBeFalse();
});

it('throws exception for invalid boolean value', async () => {
const envParam: NodeJS.ProcessEnv = {
RENOVATE_CONFIG_MIGRATION: 'badvalue',
RENOVATE_DRAFT_P_R: 'badvalue',
};
await expect(env.getConfig(envParam)).rejects.toThrow(
Error(
Expand Down Expand Up @@ -86,12 +86,17 @@ describe('workers/global/config/parse/env', () => {
});

test.each`
envArg | config
${{ RENOVATE_RECREATE_CLOSED: 'true' }} | ${{ recreateWhen: 'always' }}
${{ RENOVATE_RECREATE_CLOSED: 'false' }} | ${{ recreateWhen: 'auto' }}
${{ RENOVATE_RECREATE_WHEN: 'auto' }} | ${{ recreateWhen: 'auto' }}
${{ RENOVATE_RECREATE_WHEN: 'always' }} | ${{ recreateWhen: 'always' }}
${{ RENOVATE_RECREATE_WHEN: 'never' }} | ${{ recreateWhen: 'never' }}
envArg | config
${{ RENOVATE_RECREATE_CLOSED: 'true' }} | ${{ recreateWhen: 'always' }}
${{ RENOVATE_RECREATE_CLOSED: 'false' }} | ${{ recreateWhen: 'auto' }}
${{ RENOVATE_RECREATE_WHEN: 'auto' }} | ${{ recreateWhen: 'auto' }}
${{ RENOVATE_RECREATE_WHEN: 'always' }} | ${{ recreateWhen: 'always' }}
${{ RENOVATE_RECREATE_WHEN: 'never' }} | ${{ recreateWhen: 'never' }}
${{ RENOVATE_CONFIG_MIGRATION: 'true' }} | ${{ configMigration: 'enabled' }}
${{ RENOVATE_CONFIG_MIGRATION: 'false' }} | ${{ configMigration: 'disabled' }}
${{ RENOVATE_CONFIG_MIGRATION: 'enabled' }} | ${{ configMigration: 'enabled' }}
${{ RENOVATE_CONFIG_MIGRATION: 'disabled' }} | ${{ configMigration: 'disabled' }}
${{ RENOVATE_CONFIG_MIGRATION: 'auto' }} | ${{ configMigration: 'auto' }}
`('"$envArg" -> $config', async ({ envArg, config }) => {
expect(await env.getConfig(envArg)).toMatchObject(config);
});
Expand Down
13 changes: 13 additions & 0 deletions lib/workers/global/config/parse/env.ts
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,19 @@ export async function getConfig(
config[option.name] = 'optional';
}
}
if (option.name === 'configMigration') {
if ((config[option.name] as string) === 'true') {
logger.warn(
'env config configMigration property has been changed to enabled',
);
config[option.name] = 'enabled';
} else if ((config[option.name] as string) === 'false') {
logger.warn(
'env config configMigration property has been changed to disabled',
);
config[option.name] = 'disabled';
}
}
}
}
}
Expand Down
8 changes: 4 additions & 4 deletions lib/workers/repository/config-migration/index.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type { Indent } from 'detect-indent';
import { Fixtures } from '../../../../test/fixtures';
import { mockedFunction, partial } from '../../../../test/util';
import { RenovateConfig, mockedFunction, partial } from '../../../../test/util';
import { getConfig } from '../../../config/defaults';
import { checkConfigMigrationBranch } from './branch';
import { MigratedDataFactory } from './branch/migrated-data';
Expand All @@ -16,8 +16,8 @@ const filename = 'renovate.json';
const branchName = 'renovate/config-migration';
const config = {
...getConfig(),
configMigration: true,
};
configMigration: 'enabled',
} satisfies RenovateConfig;

describe('workers/repository/config-migration/index', () => {
beforeEach(() => {
Expand All @@ -36,7 +36,7 @@ describe('workers/repository/config-migration/index', () => {
});

it('does nothing when config migration is disabled', async () => {
await configMigration({ ...config, configMigration: false }, []);
await configMigration({ ...config, configMigration: 'disabled' }, []);
expect(MigratedDataFactory.getAsync).toHaveBeenCalledTimes(0);
expect(checkConfigMigrationBranch).toHaveBeenCalledTimes(0);
expect(ensureConfigMigrationPr).toHaveBeenCalledTimes(0);
Expand Down
2 changes: 1 addition & 1 deletion lib/workers/repository/config-migration/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ export async function configMigration(
config: RenovateConfig,
branchList: string[],
): Promise<void> {
if (config.configMigration) {
if (config.configMigration === 'enabled') {
if (config.mode === 'silent') {
logger.debug(
'Config migration issues are not created, updated or closed when mode=silent',
Expand Down
1 change: 0 additions & 1 deletion lib/workers/repository/config-migration/pr/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ describe('workers/repository/config-migration/pr/index', () => {

config = {
...getConfig(),
configMigration: true,
defaultBranch: 'main',
description: [],
};
Expand Down