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

Add skuba migrate command to upgrade projects to Node.js 20 #1382

Merged
merged 16 commits into from
Feb 15, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
22 changes: 22 additions & 0 deletions .changeset/real-oranges-rescue.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
---
'skuba': minor
AaronMoat marked this conversation as resolved.
Show resolved Hide resolved
AaronMoat marked this conversation as resolved.
Show resolved Hide resolved
---

format: Automatically upgrade projects to Node.js 20

`skuba format` will now attempt to automatically upgrade projects to Node.js 20.
It will look in the project root for Dockerfiles, `.nvmrc`, and Serverless files running Node.js 18 and try to
upgrade them to Node.js 20.

skuba might not be able to upgrade all projects, so please check your project for any files that skuba missed. It's
possible that skuba will modify a file incorrectly, in which case please
[open an issue](https://github.com/seek-oss/skuba/issues/new).

If you cannot upgrade to Node.js 20, after upgrading skuba, you can run `skuba format` with the environment variable
`SKIP_NODE_20_PATCH=true` to prevent skuba from upgrading to Node.js 20.
This is required on the first run of `skuba format` after upgrading skuba only.

Please read the [Node.js 20 release notes](https://nodejs.org/en/blog/announcements/v20-release-announce) alongside the
skuba release notes.

A future version of skuba will drop support for Node.js 18.
2 changes: 1 addition & 1 deletion docs/cli/run.md
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,6 @@ Execution should pause on the breakpoint until we hit `F5` or the `▶️` butto
[`tsconfig-paths`]: https://github.com/dividab/tsconfig-paths
[express]: https://expressjs.com/
[fastify]: https://www.fastify.io/
[http server]: https://nodejs.org/docs/latest-v18.x/api/http.html#class-httpserver
[http server]: https://nodejs.org/docs/latest-v20.x/api/http.html#class-httpserver
[koa]: https://koajs.com/
[node.js options]: https://nodejs.org/en/docs/guides/debugging-getting-started/#command-line-options
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@
"eslint": "^8.11.0",
"eslint-config-skuba": "3.1.0",
"execa": "^5.0.0",
"fast-glob": "^3.3.2",
"fdir": "^6.0.0",
"fs-extra": "^11.0.0",
"function-arguments": "^1.0.9",
Expand Down
3 changes: 3 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion scripts/package.ts
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ const main = async () => {
'CONTRIBUTING.md',
path.join('dist-docs', 'CONTRIBUTING.md'),
),
// `fs.promises.cp` is still experimental in Node.js 18.
// `fs.promises.cp` is still experimental in Node.js 20.
copy('site', 'dist-docs'),
copy('docs', path.join('dist-docs', 'docs')),
]);
Expand Down
8 changes: 8 additions & 0 deletions src/cli/__snapshots__/format.int.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ Patch skipped: Upgrade Node.js Distroless Docker image to -debian12 variant - no

Patch skipped: Add keepAliveTimeout to server listener - no listener file found

Patch skipped: Upgrade Node.js to 20 - unable to find any Node.js <20 usage
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wonder if we should mock the patches, this might get annoying


skuba update complete.

Processed skuba lints in <random>s.
Expand Down Expand Up @@ -83,6 +85,8 @@ Patch skipped: Upgrade Node.js Distroless Docker image to -debian12 variant - no

Patch skipped: Add keepAliveTimeout to server listener - no listener file found

Patch skipped: Upgrade Node.js to 20 - unable to find any Node.js <20 usage

skuba update complete.

Processed skuba lints in <random>s.
Expand Down Expand Up @@ -143,6 +147,8 @@ Patch skipped: Upgrade Node.js Distroless Docker image to -debian12 variant - no

Patch skipped: Add keepAliveTimeout to server listener - no listener file found

Patch skipped: Upgrade Node.js to 20 - unable to find any Node.js <20 usage

skuba update complete.

Processed skuba lints in <random>s.
Expand Down Expand Up @@ -176,6 +182,8 @@ Patch skipped: Upgrade Node.js Distroless Docker image to -debian12 variant - no

Patch skipped: Add keepAliveTimeout to server listener - no listener file found

Patch skipped: Upgrade Node.js to 20 - unable to find any Node.js <20 usage

skuba update complete.

Processed skuba lints in <random>s.
Expand Down
5 changes: 5 additions & 0 deletions src/cli/configure/upgrade/patches/7.3.1/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { tryPatchRenovateConfig } from '../../../patchRenovateConfig';
import { tryAddEmptyExports } from './addEmptyExports';
import { tryPatchDockerfile } from './patchDockerfile';
import { tryPatchServerListener } from './patchServerListener';
import { tryUpgradeToNode20 } from './upgradeToNode20';

export const patches: Patches = [
{
Expand All @@ -23,4 +24,8 @@ export const patches: Patches = [
apply: tryPatchServerListener,
description: 'Add keepAliveTimeout to server listener',
},
{
apply: tryUpgradeToNode20,
description: 'Upgrade Node.js to 20',
},
];
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ describe('patchServerListener', () => {
});

// Gantry ALB default idle timeout is 30 seconds
// https://nodejs.org/docs/latest-v18.x/api/http.html#serverkeepalivetimeout
// https://nodejs.org/docs/latest-v20.x/api/http.html#serverkeepalivetimeout
// Node default is 5 seconds
// https://docs.aws.amazon.com/elasticloadbalancing/latest/application/application-load-balancers.html#connection-idle-timeout
// AWS recommends setting an application timeout larger than the load balancer
Expand All @@ -72,7 +72,7 @@ describe('patchServerListener', () => {
"src/listen.ts": "const listener = app.listen(config.port);

// Gantry ALB default idle timeout is 30 seconds
// https://nodejs.org/docs/latest-v18.x/api/http.html#serverkeepalivetimeout
// https://nodejs.org/docs/latest-v20.x/api/http.html#serverkeepalivetimeout
// Node default is 5 seconds
// https://docs.aws.amazon.com/elasticloadbalancing/latest/application/application-load-balancers.html#connection-idle-timeout
// AWS recommends setting an application timeout larger than the load balancer
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ const SERVER_LISTENER_FILENAME = 'src/listen.ts';

const KEEP_ALIVE_CODE = `
// Gantry ALB default idle timeout is 30 seconds
// https://nodejs.org/docs/latest-v18.x/api/http.html#serverkeepalivetimeout
// https://nodejs.org/docs/latest-v20.x/api/http.html#serverkeepalivetimeout
// Node default is 5 seconds
// https://docs.aws.amazon.com/elasticloadbalancing/latest/application/application-load-balancers.html#connection-idle-timeout
// AWS recommends setting an application timeout larger than the load balancer
Expand Down
147 changes: 147 additions & 0 deletions src/cli/configure/upgrade/patches/7.3.1/upgradeToNode20.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
import memfs, { vol } from 'memfs';

import type { PatchReturnType } from '../..';

import { tryUpgradeToNode20 } from './upgradeToNode20';

jest.mock('fs-extra', () => memfs);
jest.mock('fast-glob', () => ({
glob: (pat: any, opts: any) =>
jest.requireActual('fast-glob').glob(pat, { ...opts, fs: memfs }),
}));

const volToJson = () => vol.toJSON(process.cwd(), undefined, true);

beforeEach(jest.clearAllMocks);
beforeEach(() => vol.reset());

const reason = 'unable to find any Node.js 20 usage';

describe('tryUpgradeToNode20', () => {
const scenarios: Array<{
filesBefore: Record<string, string>;
filesAfter?: Record<string, string>;
result: PatchReturnType;
scenario: string;
}> = [
{
scenario: 'an empty project',
filesBefore: {},
result: { result: 'skip', reason },
},
{
scenario: 'several files to patch',
filesBefore: {
'.nvmrc': 'v18.1.2',
Dockerfile: 'FROM node:18.1.2\nRUN echo "hello"',
'Dockerfile.dev-deps':
'FROM --platform=linux/amd64 node:18-slim AS dev-deps\nRUN echo "hello"',
'serverless.yml':
'provider:\n logRetentionInDays: 30\n runtime: nodejs18.x\n region: ap-southeast-2',
'serverless.melb.yaml':
'provider:\n logRetentionInDays: 7\n runtime: nodejs16.x\n region: ap-southeast-4',
},
filesAfter: {
'.nvmrc': '20',
Dockerfile: 'FROM node:20\nRUN echo "hello"',
'Dockerfile.dev-deps':
'FROM --platform=linux/amd64 node:20-slim AS dev-deps\nRUN echo "hello"',
'serverless.yml':
'provider:\n logRetentionInDays: 30\n runtime: nodejs20.x\n region: ap-southeast-2',
'serverless.melb.yaml':
'provider:\n logRetentionInDays: 7\n runtime: nodejs20.x\n region: ap-southeast-4',
},
result: { result: 'apply' },
},
{
scenario: 'various node formats',
filesBefore: {
'.nvmrc': '18.3.4',
'Dockerfile.1': 'FROM node:18.1.2\nRUN echo "hello"',
'Dockerfile.2': 'FROM node:18\nRUN echo "hello"',
'Dockerfile.3': 'FROM node:18-slim\nRUN echo "hello"',
'Dockerfile.4': 'FROM node:18.1.2-slim\nRUN echo "hello"',
'Dockerfile.5':
'FROM --platform=linux/amd64 node:18.1.2 AS dev-deps\nRUN echo "hello"',
'Dockerfile.6':
'FROM --platform=linux/amd64 node:18 AS dev-deps\nRUN echo "hello"',
'Dockerfile.7':
'FROM --platform=linux/amd64 node:18-slim AS dev-deps\nRUN echo "hello"',
'Dockerfile.8':
'FROM --platform=linux/amd64 node:18.1.2-slim AS dev-deps\nRUN echo "hello"',
'Dockerfile.9':
'FROM gcr.io/distroless/nodejs18-debian12\nRUN echo "hello"',
'Dockerfile.10':
'FROM --platform=linux/amd64 gcr.io/distroless/nodejs18-debian12 AS dev-deps\nRUN echo "hello"',
},
filesAfter: {
'.nvmrc': '20',
'Dockerfile.1': 'FROM node:20\nRUN echo "hello"',
'Dockerfile.2': 'FROM node:20\nRUN echo "hello"',
'Dockerfile.3': 'FROM node:20-slim\nRUN echo "hello"',
'Dockerfile.4': 'FROM node:20-slim\nRUN echo "hello"',
'Dockerfile.5':
'FROM --platform=linux/amd64 node:20 AS dev-deps\nRUN echo "hello"',
'Dockerfile.6':
'FROM --platform=linux/amd64 node:20 AS dev-deps\nRUN echo "hello"',
'Dockerfile.7':
'FROM --platform=linux/amd64 node:20-slim AS dev-deps\nRUN echo "hello"',
'Dockerfile.8':
'FROM --platform=linux/amd64 node:20-slim AS dev-deps\nRUN echo "hello"',
'Dockerfile.9':
'FROM gcr.io/distroless/nodejs20-debian12\nRUN echo "hello"',
'Dockerfile.10':
'FROM --platform=linux/amd64 gcr.io/distroless/nodejs20-debian12 AS dev-deps\nRUN echo "hello"',
},
result: { result: 'apply' },
},
{
scenario: 'already node 20',
filesBefore: {
'.nvmrc': '20',
Dockerfile: 'FROM node:20\nRUN echo "hello"',
'Dockerfile.dev-deps':
'FROM --platform=linux/amd64 node:20-slim AS dev-deps\nRUN echo "hello"',
'serverless.yml':
'provider:\n logRetentionInDays: 30\n runtime: nodejs20.x\n region: ap-southeast-2',
},
result: { result: 'skip', reason },
},
{
scenario: 'not detectable',
filesBefore: {
'.nvmrc': 'lts/*',
Dockerfile: 'FROM node:latest\nRUN echo "hello"',
},
result: { result: 'skip', reason },
},
];

describe('format mode', () => {
it.each(scenarios)(
'handles $scenario',
async ({ filesBefore, filesAfter, result: expected }) => {
vol.fromJSON(filesBefore, process.cwd());

const result = await tryUpgradeToNode20('format');

expect(result).toEqual(expected);
expect(volToJson()).toEqual(filesAfter ?? filesBefore);
},
);
});

describe('lint mode', () => {
it.each(scenarios)(
'handles $scenario',
async ({ filesBefore, result: expected }) => {
vol.fromJSON(filesBefore, process.cwd());

const result = await tryUpgradeToNode20('lint');

expect(result).toEqual(expected);
expect(volToJson()).toEqual(filesBefore); // no changes
},
);
});
});
103 changes: 103 additions & 0 deletions src/cli/configure/upgrade/patches/7.3.1/upgradeToNode20.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
import { inspect } from 'util';

import { glob } from 'fast-glob';
import fs from 'fs-extra';

import type { PatchFunction, PatchReturnType } from '../..';
import { log } from '../../../../../utils/logging';
import { createDestinationFileReader } from '../../../analysis/project';

type SubPatch = (
| { files: string; file?: never }
| { file: string; files?: never }
) & {
test: RegExp;
replace: string;
};

const subPatches: SubPatch[] = [
{ file: '.nvmrc', test: /^v?18.*/gm, replace: '20' },
{
files: 'Dockerfile*',
test: /^FROM(.*) node:18(\.[^- \n]+)?(-[^ \n]+)?( .+|)$/gm,
replace: 'FROM$1 node:20$3$4',
},
{
files: 'Dockerfile*',
test: /^FROM(.*) gcr.io\/distroless\/nodejs18-debian(.+)$/gm,
replace: 'FROM$1 gcr.io/distroless/nodejs20-debian$2',
},
{
files: 'serverless*.y*ml',
test: /nodejs(16|18).x/gm,
replace: 'nodejs20.x',
},
// TODO: CDK, CloudFormation?
];

const runSubPatch = async (
mode: 'format' | 'lint',
dir: string,
{ file, files, test, replace }: SubPatch,
): Promise<boolean> => {
const readFile = createDestinationFileReader(dir);
const paths = file ? [file] : await glob(files ?? [], { cwd: dir });

return (
await Promise.all(
paths.map(async (path) => {
const contents = await readFile(path);
if (!contents) {
return false;
}

const patched = contents.replaceAll(test, replace);
if (patched === contents) {
return false;
}

if (mode === 'format') {
await fs.promises.writeFile(path, patched);
}

return true;
}),
)
).some((result) => result);
};

const upgradeToNode20 = async (
mode: 'format' | 'lint',
dir: string,
): Promise<PatchReturnType> => {
const results = await Promise.all(
subPatches.map((subPatch) => runSubPatch(mode, dir, subPatch)),
);

return results.some((result) => result)
? { result: 'apply' }
: { result: 'skip', reason: 'unable to find any Node.js <20 usage' };
};

export const tryUpgradeToNode20: PatchFunction = async (
mode: 'format' | 'lint',
dir = process.cwd(),
) => {
if (process.env.SKIP_NODE_20_PATCH) {
log.warn(
'Skipping Node.js 20 patch due to SKIP_NODE_20_PATCH environment variable',
);

return {
result: 'apply', // because we don't want to try again
};
}

try {
return await upgradeToNode20(mode, dir);
} catch (err) {
log.warn('Failed to upgrade Node.js to 20');
log.subtle(inspect(err));
return { result: 'skip', reason: 'due to an error' };
}
};
2 changes: 1 addition & 1 deletion template/express-rest-api/src/listen.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ const listener = app.listen(config.port, () => {
});

// Gantry ALB default idle timeout is 30 seconds
// https://nodejs.org/docs/latest-v18.x/api/http.html#serverkeepalivetimeout
// https://nodejs.org/docs/latest-v20.x/api/http.html#serverkeepalivetimeout
// Node default is 5 seconds
// https://docs.aws.amazon.com/elasticloadbalancing/latest/application/application-load-balancers.html#connection-idle-timeout
// AWS recommends setting an application timeout larger than the load balancer
Expand Down
Loading
Loading