Skip to content

Commit

Permalink
feat(npm): migrate and deprecate unpublishSafe (#7464)
Browse files Browse the repository at this point in the history
The existing npm-specific `unpublishSafe` setting will be removed and migrated instead to use `stabilityDays` (3). The `renovate/unpublish-safe` status check is also deprecated and the existing `renovate/stability-days` will be used instead.

Closes #5265

BREAKING CHANGE: The status check `renovate/unpublish-safe` will be replaced with `renovate/stability-days`. Please migrate any branch protection rules if you were relying on `renovate/unpublish-safe`.
  • Loading branch information
zharinov committed Dec 11, 2020
1 parent 663117b commit 6f7b4af
Show file tree
Hide file tree
Showing 24 changed files with 135 additions and 287 deletions.
15 changes: 3 additions & 12 deletions docs/usage/configuration-options.md
Original file line number Diff line number Diff line change
Expand Up @@ -1522,8 +1522,9 @@ Note that this limit is enforced on a per-repository basis.

If you configure `prCreation=not-pending`, then Renovate will wait until tests are non-pending (all pass or at least one fails) before creating PRs.
However there are cases where PRs may remain in pending state forever, e.g. absence of tests or status checks that are configure to pending indefinitely.
Therefore we configure an upper limit - default 24 hours - for how long we wait until creating a PR.
Note also this is the same length of time as for Renovate's own `unpublishSafe` status check for npm.
Therefore we configure an upper limit for how long we wait until creating a PR.

Note: if the option `stabilityDays` is non-zero then Renovate will disable the `prNotPendingHours` functionality.

## prPriority

Expand Down Expand Up @@ -2022,16 +2023,6 @@ Please see the above link for valid timezone names.

If enabled emoji shortcodes (`:warning:`) are replaced with their unicode equivalents (`⚠️`)

## unpublishSafe

It is not known by many that npm package authors and collaborators can _delete_ an npm version if it is less than 24 hours old.
e.g. version 1.0.0 might exist, then version 1.1.0 is released, and then version 1.1.0 might get deleted hours later.
This means that version 1.1.0 essentially "disappears" and 1.0.0 returns to being the "latest".
If you have installed 1.1.0 during that time then your build is essentially broken.

Enabling `unpublishSafe` will add a `renovate/unpublish-safe` status check with value pending to every branch to warn you about this possibility.
It can be handy when used with the `prCreation` = `not-pending` configuration option - that way you won't get the PR raised until after a patch is 24 hours old or more.

## updateLockFiles

## updateNotScheduled
Expand Down
8 changes: 0 additions & 8 deletions lib/config/definitions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1078,7 +1078,6 @@ const options: RenovateOptions[] = [
stage: 'package',
type: 'object',
default: {
unpublishSafe: false,
recreateClosed: true,
rebaseWhen: 'behind-base-branch',
groupName: 'Pin Dependencies',
Expand Down Expand Up @@ -1153,12 +1152,6 @@ const options: RenovateOptions[] = [
type: 'string',
default: 'rebase',
},
{
name: 'unpublishSafe',
description: 'Set a status check for unpublish-safe upgrades',
type: 'boolean',
default: false,
},
{
name: 'stabilityDays',
description:
Expand All @@ -1177,7 +1170,6 @@ const options: RenovateOptions[] = [
name: 'prNotPendingHours',
description: 'Timeout in hours for when prCreation=not-pending',
type: 'integer',
// Must be at least 24 hours to give time for the unpublishSafe check to "complete".
default: 25,
},
{
Expand Down
86 changes: 85 additions & 1 deletion lib/config/migration.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ describe('config/migration', () => {
pathRules: [
{
paths: ['node/**'],
extends: ['node'],
extends: 'node',
},
],
},
Expand Down Expand Up @@ -435,6 +435,7 @@ describe('config/migration', () => {
expect(res.isMigrated).toBe(false);
expect(res.migratedConfig).toMatchObject({ semanticCommits: 'disabled' });
});

it('it migrates preset strings to array', () => {
let config: RenovateConfig;
let res: MigratedConfig;
Expand All @@ -456,5 +457,88 @@ describe('config/migration', () => {
extends: ['foo', 'config:js-app', 'bar'],
});
});

it('it migrates unpublishSafe', () => {
let config: RenovateConfig;
let res: MigratedConfig;

config = { unpublishSafe: true };
res = configMigration.migrateConfig(config);
expect(res.isMigrated).toBe(true);
expect(res.migratedConfig).toMatchObject({
extends: ['npm:unpublishSafe'],
});

config = { unpublishSafe: true, extends: 'foo' } as never;
res = configMigration.migrateConfig(config);
expect(res.isMigrated).toBe(true);
expect(res.migratedConfig).toMatchObject({
extends: ['foo', 'npm:unpublishSafe'],
});

config = { unpublishSafe: true, extends: [] };
res = configMigration.migrateConfig(config);
expect(res.isMigrated).toBe(true);
expect(res.migratedConfig).toMatchObject({
extends: ['npm:unpublishSafe'],
});

config = { unpublishSafe: true, extends: ['foo', 'bar'] };
res = configMigration.migrateConfig(config);
expect(res.isMigrated).toBe(true);
expect(res.migratedConfig).toMatchObject({
extends: ['foo', 'bar', 'npm:unpublishSafe'],
});

config = {
unpublishSafe: true,
extends: ['foo', ':unpublishSafe', 'bar'],
};
res = configMigration.migrateConfig(config);
expect(res.isMigrated).toBe(true);
expect(res.migratedConfig).toMatchObject({
extends: ['foo', ':unpublishSafe', 'bar', 'npm:unpublishSafe'],
});

config = {
unpublishSafe: true,
extends: ['foo', 'default:unpublishSafe', 'bar'],
};
res = configMigration.migrateConfig(config);
expect(res.isMigrated).toBe(true);
expect(res.migratedConfig).toMatchObject({
extends: ['foo', 'default:unpublishSafe', 'bar', 'npm:unpublishSafe'],
});

config = {
unpublishSafe: false,
extends: ['foo', 'bar'],
};
res = configMigration.migrateConfig(config);
expect(res.isMigrated).toBe(true);
expect(res.migratedConfig).toMatchObject({
extends: ['foo', 'bar'],
});

config = {
unpublishSafe: true,
extends: ['foo', 'bar'],
};
res = configMigration.migrateConfig(config);
expect(res.isMigrated).toBe(true);
expect(res.migratedConfig).toMatchObject({
extends: ['foo', 'bar', 'npm:unpublishSafe'],
});

config = {
unpublishSafe: true,
extends: [':unpublishSafeDisabled'],
};
res = configMigration.migrateConfig(config);
expect(res.isMigrated).toBe(true);
expect(res.migratedConfig).toMatchObject({
extends: [':unpublishSafeDisabled', 'npm:unpublishSafe'],
});
});
});
});
13 changes: 13 additions & 0 deletions lib/config/migration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,18 @@ export function migrateConfig(
presets[i] = preset;
}
}
} else if (key === 'unpublishSafe') {
isMigrated = true;
if (val === true) {
migratedConfig.extends = migratedConfig.extends || [];
if (is.string(migratedConfig.extends)) {
migratedConfig.extends = [migratedConfig.extends];
}
if (!migratedConfig.extends.includes('npm:unpublishSafe')) {
migratedConfig.extends.push('npm:unpublishSafe');
}
}
delete migratedConfig.unpublishSafe;
} else if (key === 'versionScheme') {
isMigrated = true;
migratedConfig.versioning = val;
Expand Down Expand Up @@ -535,6 +547,7 @@ export function migrateConfig(
delete migratedConfig.endpoints;
isMigrated = true;
}

return { isMigrated, migratedConfig };
} catch (err) /* istanbul ignore next */ {
logger.debug({ config }, 'migrateConfig() error');
Expand Down

Large diffs are not rendered by default.

1 change: 0 additions & 1 deletion lib/config/presets/__snapshots__/index.spec.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,6 @@ Object {
"separateMajorMinor": true,
"separateMinorPatch": false,
"timezone": "Asia/Taipei",
"unpublishSafe": false,
}
`;

Expand Down
1 change: 1 addition & 0 deletions lib/config/presets/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ export function parsePreset(input: string): ParsedPreset {
'group',
'helpers',
'monorepo',
'npm',
'packages',
'preview',
'schedule',
Expand Down
10 changes: 0 additions & 10 deletions lib/config/presets/internal/default.ts
Original file line number Diff line number Diff line change
Expand Up @@ -210,16 +210,6 @@ export const presets: Record<string, Preset> = {
'Rebase existing PRs any time the base branch has been updated',
rebaseWhen: 'behind-base-branch',
},
unpublishSafe: {
description:
'Set a status check to warn when upgrades < 24 hours old might get unpublished',
unpublishSafe: true,
},
unpublishSafeDisabled: {
description:
"Create branches/PRs for dependency upgrades as soon as they're available",
unpublishSafe: false,
},
prImmediately: {
description: 'Raise PRs immediately (after branch is created)',
prCreation: 'immediate',
Expand Down
2 changes: 2 additions & 0 deletions lib/config/presets/internal/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import * as dockerPreset from './docker';
import * as groupPreset from './group';
import * as helpersPreset from './helpers';
import * as monorepoPreset from './monorepo';
import * as npm from './npm';
import * as packagesPreset from './packages';
import * as previewPreset from './preview';
import * as schedulePreset from './schedule';
Expand All @@ -17,6 +18,7 @@ export const groups: Record<string, Record<string, Preset>> = {
group: groupPreset.presets,
helpers: helpersPreset.presets,
monorepo: monorepoPreset.presets,
npm: npm.presets,
packages: packagesPreset.presets,
preview: previewPreset.presets,
schedule: schedulePreset.presets,
Expand Down
11 changes: 11 additions & 0 deletions lib/config/presets/internal/npm.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { Preset } from '../common';

export const presets: Record<string, Preset> = {
unpublishSafe: {
description:
'Set a status check pending for 3 days from release timestamp to guard against npm unpublishing',
npm: {
stabilityDays: 3,
},
},
};
1 change: 0 additions & 1 deletion lib/datasource/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ export function isGetPkgReleasesConfig(
}

export interface Release {
canBeUnpublished?: boolean;
changelogUrl?: string;
checksumUrl?: string;
downloadUrl?: string;
Expand Down

0 comments on commit 6f7b4af

Please sign in to comment.