From aac631fe3479e5e067fd4968aa0579277cfa1f3e Mon Sep 17 00:00:00 2001 From: Stan Lewis Date: Fri, 8 May 2026 11:01:51 -0400 Subject: [PATCH] fix(cli): stage dynamic plugins via npm pack Replace recursive copy of each dist-dynamic directory when running plugin package with the same contents npm would publish: npm pack into a scratch directory, extract the tarball with tar --strip-components=1, and keep the flattened layout expected by the OCI / --export-to flow. This matches publish semantics from dist-dynamic package.json (files, bundleDependencies, etc.), omits node_modules/.bin entries that referenced absolute paths on the builder machine and triggered "link outside of the archive" warnings during dynamic plugin install from OCI images (RHDHBUGS-1968). Run npm/tar through spawn bash with shell: false and stdio redirected to an anonymous log fd so chatter does not clutter the CLI; Task.forCommand is not used (no env for pack placeholders) nor run() (shell: true can still surface npm output). On staging failure the log contents are printed, then discarded. This change also adjusts the e2e test assertions which where asserting on the old recursive copy behavior to now compare using an independent `npm pack` step Assisted-By: Cursor Desktop rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED --- CHANGELOG.md | 6 + README.md | 12 ++ .../community-plugins-build-package.test.ts | 13 +- e2e-tests/rhdh-plugins-build-package.test.ts | 13 +- e2e-tests/support/plugin-export-build.ts | 40 ++++++ package.json | 2 +- .../package-dynamic-plugins/command.ts | 129 ++++++++++++++++-- 7 files changed, 194 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dcae17e..2dd5c04 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,12 @@ All notable changes to `@red-hat-developer-hub/cli` are documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## 1.11.0 - 2026-05-08 + +### Fixed + +- **`plugin package`:** each `dist-dynamic` plugin is staged with **`npm pack`** and **`tar`** (strip the `package/` root) instead of a recursive filesystem copy. This matches npm publish contents, omits `node_modules/.bin` entries that could point outside the image (see [RHDHBUGS-1968](https://redhat.atlassian.net/browse/RHDHBUGS-1968)), and avoids spurious “link outside of the archive” warnings when dynamic plugins are installed from OCI. **Requires `bash`, `npm` (7+ for `--pack-destination`), and `tar` on `PATH`** (for example Git Bash on Windows). + ## 1.10.6 - 2026-04-28 ### Fixed diff --git a/README.md b/README.md index d9a0136..93acedc 100644 --- a/README.md +++ b/README.md @@ -17,6 +17,18 @@ This new CLI aims to offer more flexibility and ease of use compared to the prev > | `npx @janus-idp/cli package package-dynamic-plugins` | `npx @red-hat-developer-hub/cli plugin package` | +## `plugin package` requirements + +The `plugin package` command stages each `dist-dynamic` plugin with `npm pack` and `tar` (via a short bash script). The following must be available on your `PATH`: + +- **bash** — runs the pack/extract script +- **npm** (7 or newer) — `npm pack --pack-destination` requires npm 7+ +- **tar** — extracts the packed tarball into the staging directory + +On Windows, use Git Bash or WSL so these tools are available. + +When you build an OCI image with `--tag` (instead of exporting to a directory with `--export-to`), a container build tool must also be on `PATH`. **podman** is the default; you can select **docker** or **buildah** with `--container-tool` (for example `--container-tool docker`). Directory-only exports with `--export-to` do not need a container tool. + ## Development ### Contributing diff --git a/e2e-tests/community-plugins-build-package.test.ts b/e2e-tests/community-plugins-build-package.test.ts index 33dac10..9bd8227 100644 --- a/e2e-tests/community-plugins-build-package.test.ts +++ b/e2e-tests/community-plugins-build-package.test.ts @@ -10,6 +10,7 @@ import { logSection, parseDynamicPluginAnnotation, runCommand, + topLevelEntriesAfterNpmPackStaging, } from './support/plugin-export-build'; // you can use COMMUNITY_PLUGINS_REPO_ARCHIVE env variable to specify a path existing local archive of the community plugins repository @@ -169,11 +170,13 @@ describe('export and package backstage-community plugin', () => { `ls -lah ${path.join(getFullPluginPath(), 'dist-dynamic')}`, ); - const filesInImage = fs.readdirSync(path.join(imageContentDir, key)); - const filesInDerivedPackage = fs.readdirSync( - path.join(getFullPluginPath(), 'dist-dynamic'), - ); - expect(filesInImage.length).toEqual(filesInDerivedPackage.length); + const distDynamicPath = path.join(getFullPluginPath(), 'dist-dynamic'); + const expectedTopLevel = + await topLevelEntriesAfterNpmPackStaging(distDynamicPath); + const filesInImage = fs + .readdirSync(path.join(imageContentDir, key)) + .sort(); + expect(filesInImage).toEqual(expectedTopLevel); const indexJson = JSON.parse( fs.readFileSync(path.join(imageContentDir, 'index.json'), 'utf-8'), diff --git a/e2e-tests/rhdh-plugins-build-package.test.ts b/e2e-tests/rhdh-plugins-build-package.test.ts index e28beb2..84ef47b 100644 --- a/e2e-tests/rhdh-plugins-build-package.test.ts +++ b/e2e-tests/rhdh-plugins-build-package.test.ts @@ -10,6 +10,7 @@ import { logSection, parseDynamicPluginAnnotation, runCommand, + topLevelEntriesAfterNpmPackStaging, } from './support/plugin-export-build'; // you can use RHDH_PLUGINS_REPO_ARCHIVE env variable to specify a path to an existing local archive of the rhdh-plugins repository @@ -169,11 +170,13 @@ describe('export and package rhdh-plugins scorecard workspace plugin', () => { `ls -lah ${path.join(getFullPluginPath(), 'dist-dynamic')}`, ); - const filesInImage = fs.readdirSync(path.join(imageContentDir, key)); - const filesInDerivedPackage = fs.readdirSync( - path.join(getFullPluginPath(), 'dist-dynamic'), - ); - expect(filesInImage.length).toEqual(filesInDerivedPackage.length); + const distDynamicPath = path.join(getFullPluginPath(), 'dist-dynamic'); + const expectedTopLevel = + await topLevelEntriesAfterNpmPackStaging(distDynamicPath); + const filesInImage = fs + .readdirSync(path.join(imageContentDir, key)) + .sort(); + expect(filesInImage).toEqual(expectedTopLevel); const indexJson = JSON.parse( fs.readFileSync(path.join(imageContentDir, 'index.json'), 'utf-8'), diff --git a/e2e-tests/support/plugin-export-build.ts b/e2e-tests/support/plugin-export-build.ts index 1903da3..e1de336 100644 --- a/e2e-tests/support/plugin-export-build.ts +++ b/e2e-tests/support/plugin-export-build.ts @@ -1,5 +1,6 @@ import fs from 'fs-extra'; import { exec as execCallback } from 'node:child_process'; +import os from 'node:os'; import path from 'node:path'; import { promisify } from 'node:util'; import type { ReadEntry } from 'tar'; @@ -79,6 +80,45 @@ export async function runCommand( } } +/** + * Top-level directory names using the same steps as `plugin package`: + * `npm pack --pack-destination …`, then `tar -xzf … --strip-components=1`. + * Names are sorted for stable comparison with staged OCI contents. + */ +export async function topLevelEntriesAfterNpmPackStaging( + distDynamicDir: string, +): Promise { + const work = fs.mkdtempSync(path.join(os.tmpdir(), 'rhdh-e2e-npm-pack-')); + const packDest = fs.mkdtempSync( + path.join(os.tmpdir(), 'rhdh-e2e-npm-pack-out-'), + ); + try { + await runCommand( + `npm pack --pack-destination "${packDest}" --foreground-scripts=false`, + { cwd: distDynamicDir }, + ); + const tgzs = fs + .readdirSync(packDest) + .filter(f => f.endsWith('.tgz')) + .sort(); + if (tgzs.length !== 1) { + throw new Error( + `expected exactly one .tgz in ${packDest}, got: ${tgzs.join(', ')}`, + ); + } + const extractInto = path.join(work, 'extracted'); + fs.mkdirSync(extractInto, { recursive: true }); + await runCommand( + `tar -xzf "${path.join(packDest, tgzs[0])}" -C "${extractInto}" --strip-components=1`, + { cwd: distDynamicDir }, + ); + return fs.readdirSync(extractInto).sort(); + } finally { + await fs.remove(work).catch(() => undefined); + await fs.remove(packDest).catch(() => undefined); + } +} + export async function parseDynamicPluginAnnotation( imageAnnotations: Record, ): Promise { diff --git a/package.json b/package.json index ab63863..a0535c3 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "@red-hat-developer-hub/cli", "description": "CLI for developing Backstage plugins and apps", - "version": "1.11.0", + "version": "1.11.1", "publishConfig": { "access": "public" }, diff --git a/src/commands/package-dynamic-plugins/command.ts b/src/commands/package-dynamic-plugins/command.ts index 829d480..e39b580 100644 --- a/src/commands/package-dynamic-plugins/command.ts +++ b/src/commands/package-dynamic-plugins/command.ts @@ -1,15 +1,19 @@ import { PackageRoles } from '@backstage/cli-node'; +import { spawn } from 'node:child_process'; +import { closeSync, openSync } from 'node:fs'; + import chalk from 'chalk'; import { OptionValues } from 'commander'; import fs from 'fs-extra'; import { PackageJson } from 'type-fest'; import YAML from 'yaml'; -import os from 'os'; -import path from 'path'; +import os from 'node:os'; +import path from 'node:path'; import { paths } from '../../lib/paths'; +import { waitForExit } from '../../lib/run'; import { Task } from '../../lib/tasks'; export async function command(opts: OptionValues): Promise { @@ -146,7 +150,7 @@ export async function command(opts: OptionValues): Promise { const pluginRegistryMetadata = []; const pluginConfigs: Record = {}; try { - // copy the dist-dynamic output folder for each plugin to some temp directory and generate the metadata entry for each plugin + // Stage each dist-dynamic tree via npm pack + tar (see RHDHBUGS-1968) and metadata for the registry for (const pluginPkg of packages) { const { packageDirectory, packageFilePath } = pluginPkg; const distDynamicDirectory = path.join(packageDirectory, 'dist-dynamic'); @@ -158,13 +162,14 @@ export async function command(opts: OptionValues): Promise { .replace(/^@/, '') .replace(/\//, '-'); const targetDirectory = path.join(tmpDir, packageName); - Task.log(`Copying '${distDynamicDirectory}' to '${targetDirectory}`); + Task.log( + `Packing '${distDynamicDirectory}' into staging directory '${targetDirectory}' (npm pack + tar)`, + ); try { - // Copy the exported package to the staging area and ensure symlinks - // are copied as normal folders - fs.cpSync(distDynamicDirectory, targetDirectory, { - recursive: true, - dereference: true, + await stageDistDynamicViaNpmPack({ + distDynamicDirectory, + targetDirectory, + packScratchParent: tmpDir, }); const { name, @@ -211,7 +216,7 @@ export async function command(opts: OptionValues): Promise { } } catch (err) { Task.log( - `Encountered an error copying static assets for plugin ${packageFilePath}, the plugin will not be packaged. The error was ${err}`, + `Encountered an error staging plugin ${packageFilePath} via npm pack, the plugin will not be packaged. The error was ${err}`, ); } } @@ -308,6 +313,110 @@ COPY . . return; } +type StageDistDynamicViaNpmPackOptions = { + distDynamicDirectory: string; + targetDirectory: string; + /** Directory under which a unique `npm-pack-*` scratch dir is created. */ + packScratchParent: string; +}; + +/** + * Stages `dist-dynamic` like a published tarball: `npm pack` to a scratch + * directory, then `tar -xzf … --strip-components=1` into the target. Omits + * `node_modules/.bin` and other paths npm does not ship (RHDHBUGS-1968). + * + * Sends npm/tar stdout and stderr to a temp log file (path is printed only on + * failure), not to the terminal, by wiring those streams to fd(s) opened on + * that file in the `spawn` options. + * + * Uses `spawn` with `shell: false`; pack paths are inlined with `bashSingleQuoted`. + * Does not pass a custom `env` (child inherits PATH so user-managed toolchains + * resolve — see adjacent NOSONAR). `Task.forCommand` lacks custom env support; `run()` + * uses `shell: true` on `spawn` and can replay npm output to the CLI. + * + * Requires `bash`, `npm` 7+ for `--pack-destination`, and `tar` on `PATH` + * (Linux, macOS, Git Bash, or WSL on Windows). + */ +async function stageDistDynamicViaNpmPack( + options: StageDistDynamicViaNpmPackOptions, +): Promise { + const { distDynamicDirectory, targetDirectory, packScratchParent } = options; + const packdir = fs.mkdtempSync(path.join(packScratchParent, 'npm-pack-')); + const packLogPath = path.join( + packScratchParent, + `npm-pack-output-${process.pid}-${Date.now()}.log`, + ); + + try { + fs.rmSync(targetDirectory, { recursive: true, force: true }); + fs.mkdirSync(targetDirectory, { recursive: true }); + + const logFd = openSync(packLogPath, 'w'); + try { + // Controlled bash -lc script (paths from bashSingleQuoted); not user-defined. + // child inherits PATH so npm/bash/tar resolve (nvm, fnm, Homebrew). + const child = spawn( + 'bash', // NOSONAR typescript:S4036 + ['-lc', npmPackExtractScript(packdir, targetDirectory)], + { + cwd: distDynamicDirectory, + stdio: ['ignore', logFd, logFd], + shell: false, + }, + ); + await waitForExit(child, 'npm pack / tar'); + } finally { + closeSync(logFd); + } + + await fs.remove(packLogPath).catch(() => undefined); + } catch (err) { + if (await fs.pathExists(packLogPath)) { + const logContents = await fs + .readFile(packLogPath, 'utf8') + .catch(() => ''); + const logHeader = chalk.yellow(`npm pack / tar output (${packLogPath}):`); + process.stderr.write(`${logHeader}\n\n${logContents}\n`); + await fs.remove(packLogPath).catch(() => undefined); + } + throw err; + } finally { + fs.rmSync(packdir, { recursive: true, force: true }); + } +} + +/** + * Single-quote a string for safe embedding in `bash -lc` (POSIX single-quoted + * literal, with `'` escaped as `'\''`). + */ +function bashSingleQuoted(value: string): string { + /** Bash: end `'...'`, emit a literal `'`, resume quoted segment — `'\''`. */ + const escapedForBash = String.raw`'\''`; + const escapedValue = value.replaceAll("'", escapedForBash); + return `'${escapedValue}'`; +} + +/** + * `npm pack` into `packdir`, then extract into `extractToDir` with the same + * layout as today’s staging tree (`package/` stripped). Fails if `packdir` does + * not contain exactly one `.tgz` after pack. Paths are bash-quoted in the script + * instead of passed via extra `env` entries. + */ +function npmPackExtractScript(packdir: string, extractToDir: string): string { + const qPack = bashSingleQuoted(packdir); + const qExtract = bashSingleQuoted(extractToDir); + return `set -euo pipefail +npm pack --pack-destination ${qPack} --foreground-scripts=false +shopt -s nullglob +tgzs=(${qPack}/*.tgz) +if (( \${#tgzs[@]} != 1 )); then + echo "expected exactly one .tgz in ${packdir}, got: \${tgzs[*]}" >&2 + exit 1 +fi +tar -xzf "\${tgzs[0]}" -C ${qExtract} --strip-components=1 +`; +} + /** * Scan the monorepo "plugins" directory and find all plugins that are eligible * to be exported as dynamic plugins