From b3a06270423604fdbbb36d85811bad7c34eb2624 Mon Sep 17 00:00:00 2001 From: blam Date: Tue, 30 Jan 2024 11:10:51 +0100 Subject: [PATCH 1/3] chore: move the messaging and fix windows verify builds Signed-off-by: blam --- .github/workflows/verify_windows.yml | 2 +- .../src/lib/templating/SecureTemplater.ts | 9 +++++ .../src/lib/templating/helpers.ts | 37 ++++++++++++++++++ .../scaffolder-backend/src/service/helpers.ts | 39 ------------------- .../scaffolder-backend/src/service/router.ts | 16 +------- 5 files changed, 48 insertions(+), 55 deletions(-) create mode 100644 plugins/scaffolder-backend/src/lib/templating/helpers.ts diff --git a/.github/workflows/verify_windows.yml b/.github/workflows/verify_windows.yml index 143b22c8b6..5b188770b4 100644 --- a/.github/workflows/verify_windows.yml +++ b/.github/workflows/verify_windows.yml @@ -21,7 +21,7 @@ jobs: env: CI: true - NODE_OPTIONS: --max-old-space-size=4096 + NODE_OPTIONS: ${{ matrix.node-version == '20.x' && '--max-old-space-size=4096 --no-node-snapshot' || '--max-old-space-size=4096' }} INTEGRATION_TEST_GITHUB_TOKEN: ${{ secrets.INTEGRATION_TEST_GITHUB_TOKEN }} INTEGRATION_TEST_GITLAB_TOKEN: ${{ secrets.INTEGRATION_TEST_GITLAB_TOKEN }} INTEGRATION_TEST_BITBUCKET_TOKEN: ${{ secrets.INTEGRATION_TEST_BITBUCKET_TOKEN }} diff --git a/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.ts b/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.ts index 607b1c6add..9c5fed3aa3 100644 --- a/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.ts +++ b/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.ts @@ -22,6 +22,7 @@ import { } from '@backstage/plugin-scaffolder-node'; import fs from 'fs-extra'; import { JsonValue } from '@backstage/types'; +import { getMajorNodeVersion, isNoNodeSnapshotOptionProvided } from './helpers'; // language=JavaScript const mkScript = (nunjucksSource: string) => ` @@ -128,6 +129,14 @@ export class SecureTemplater { nunjucksConfigs = {}, } = options; + const nodeVersion = getMajorNodeVersion(); + if (nodeVersion >= 20 && !isNoNodeSnapshotOptionProvided()) { + throw new Error( + `When using node v20+ Scaffolder requires that node be started with the --no-node-snapshot option. + Please make sure that you have NODE_OPTIONS=--no-node-snapshot in your environment.`, + ); + } + const isolate = new Isolate({ memoryLimit: 128 }); const context = await isolate.createContext(); const contextGlobal = context.global; diff --git a/plugins/scaffolder-backend/src/lib/templating/helpers.ts b/plugins/scaffolder-backend/src/lib/templating/helpers.ts new file mode 100644 index 0000000000..3dfa4589bf --- /dev/null +++ b/plugins/scaffolder-backend/src/lib/templating/helpers.ts @@ -0,0 +1,37 @@ +/* + * Copyright 2024 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +export function isNoNodeSnapshotOptionProvided(): boolean { + return ( + process.env.NODE_OPTIONS?.includes('--no-node-snapshot') || + process.argv.includes('--no-node-snapshot') + ); +} + +/** + * Gets the major version of the currently running Node.js process. + * + * @remarks + * This function extracts the major version from `process.versions.node` (a string representing the Node.js version), + * which includes the major, minor, and patch versions. It splits this string by the `.` character to get an array + * of these versions, and then parses the first element of this array (the major version) to a number. + * + * @returns {number} The major version of the currently running Node.js process. + */ +export function getMajorNodeVersion(): number { + const version = process.versions.node; + return parseInt(version.split('.')[0], 10); +} diff --git a/plugins/scaffolder-backend/src/service/helpers.ts b/plugins/scaffolder-backend/src/service/helpers.ts index 9e271a9d79..3412fec2d3 100644 --- a/plugins/scaffolder-backend/src/service/helpers.ts +++ b/plugins/scaffolder-backend/src/service/helpers.ts @@ -107,42 +107,3 @@ export async function findTemplate(options: { return template as TemplateEntityV1beta3; } - -/** - * Checks if the '--no-node-snapshot' option is included in the NODE_OPTIONS environment variable - * or not included in the command line arguments. - * - * @remarks - * This function checks whether the '--no-node-snapshot' option is part of the NODE_OPTIONS environment - * variable or is missing from the command line arguments. If either condition is met, the function returns `true`. - * This check is especially important when using the "isolated-vm" package with Node.js version 20.x or later. - * - * According to the "isolated-vm" documentation on GitHub (https://github.com/laverdet/isolated-vm), - * if you are using a version of Node.js 20.x or later and you don't pass the '--no-node-snapshot' option, - * it can cause the process to crash. This function helps prevent such crashes by ensuring that the option - * is correctly provided. - * - * @returns {boolean} Returns `true` if the '--no-node-snapshot' option is included in the NODE_OPTIONS - * environment variable or not included in the command line arguments. Otherwise, it returns `false`. - */ -export function isNoNodeSnapshotOptionProvided(): boolean { - return ( - process.env.NODE_OPTIONS?.includes('--no-node-snapshot') || - process.argv.includes('--no-node-snapshot') - ); -} - -/** - * Gets the major version of the currently running Node.js process. - * - * @remarks - * This function extracts the major version from `process.versions.node` (a string representing the Node.js version), - * which includes the major, minor, and patch versions. It splits this string by the `.` character to get an array - * of these versions, and then parses the first element of this array (the major version) to a number. - * - * @returns {number} The major version of the currently running Node.js process. - */ -export function getMajorNodeVersion(): number { - const version = process.versions.node; - return parseInt(version.split('.')[0], 10); -} diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index dec3dcf69a..c0322af283 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -61,13 +61,7 @@ import { } from '../scaffolder'; import { createDryRunner } from '../scaffolder/dryrun'; import { StorageTaskBroker } from '../scaffolder/tasks/StorageTaskBroker'; -import { - findTemplate, - getEntityBaseUrl, - getMajorNodeVersion, - getWorkingDirectory, - isNoNodeSnapshotOptionProvided, -} from './helpers'; +import { findTemplate, getEntityBaseUrl, getWorkingDirectory } from './helpers'; import { IdentityApi, IdentityApiGetIdentityRequest, @@ -266,14 +260,6 @@ export async function createRouter( const logger = parentLogger.child({ plugin: 'scaffolder' }); - const nodeVersion = getMajorNodeVersion(); - if (nodeVersion >= 20 && !isNoNodeSnapshotOptionProvided()) { - throw new Error( - 'When using node v20+ Scaffolder requires that node be started with the --no-node-snapshot option. Please restart ' + - 'Backstage providing the node --no-node-snapshot option.', - ); - } - const identity: IdentityApi = options.identity || buildDefaultIdentityClient(options); const workingDirectory = await getWorkingDirectory(config, logger); From f6792c6758d5562879cfa833bef2dba636d64aa8 Mon Sep 17 00:00:00 2001 From: blam Date: Tue, 30 Jan 2024 11:14:07 +0100 Subject: [PATCH 2/3] chore: updating messaging Signed-off-by: blam --- .changeset/funny-buttons-sip.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/funny-buttons-sip.md diff --git a/.changeset/funny-buttons-sip.md b/.changeset/funny-buttons-sip.md new file mode 100644 index 0000000000..c455b8a31b --- /dev/null +++ b/.changeset/funny-buttons-sip.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-scaffolder-backend': patch +--- + +Move the `NODE_OPTIONS` messaging for `--no-node-snapshot` to the `SecureTemplater` in order to get better messaging at runtime From 26ca6a43424ee621d4f521572f6052680b0859f6 Mon Sep 17 00:00:00 2001 From: Ben Lambert Date: Tue, 30 Jan 2024 11:34:28 +0100 Subject: [PATCH 3/3] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Fredrik Adelöw Signed-off-by: Ben Lambert Signed-off-by: blam --- .../scaffolder-backend/src/lib/templating/SecureTemplater.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.ts b/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.ts index 9c5fed3aa3..14a58e4975 100644 --- a/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.ts +++ b/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.ts @@ -132,7 +132,7 @@ export class SecureTemplater { const nodeVersion = getMajorNodeVersion(); if (nodeVersion >= 20 && !isNoNodeSnapshotOptionProvided()) { throw new Error( - `When using node v20+ Scaffolder requires that node be started with the --no-node-snapshot option. + `When using Node.js version 20 or newer, the scaffolder backend plugin requires that it be started with the --no-node-snapshot option. Please make sure that you have NODE_OPTIONS=--no-node-snapshot in your environment.`, ); }