chore(install-dynamic-plugins): consume installer from npm via root yarn workspace#4908
Conversation
|
Skipping CI for Draft Pull Request. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4908 +/- ##
==========================================
- Coverage 55.82% 55.25% -0.58%
==========================================
Files 121 109 -12
Lines 2350 2132 -218
Branches 562 537 -25
==========================================
- Hits 1312 1178 -134
+ Misses 1033 953 -80
+ Partials 5 1 -4
Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
|
The container image build workflow finished with status: |
|
The container image build workflow finished with status: |
|
The container image build workflow finished with status: |
…gins yarn workspace Pivots redhat-developer#4908 from `RUN npm install …` to a plain yarn dependency in dynamic-plugins/package.json. The existing `yarn install --immutable` at line 151 of this Containerfile pulls the installer as part of the same yarn run that already brings in the rest of the dynamic-plugins deps, so the bin lands at /opt/app-root/src/dynamic-plugins/node_modules/.bin/install-dynamic-plugins and the final stage just writes a shim pointing there. Why yarn and not npm: scripts/local-hermeto-build.sh:213 already prefetches yarn deps for ./dynamic-plugins via cachi2/hermeto. No infra change is needed in this repo or in the midstream Konflux pipeline — the hermetic build "just works". The earlier npm-install approach required wiring npm into hermeto's fetch-deps, which is real work spanning two repos. Validated locally: yarn install of the unbundled cli-module variant (via a tarball of the fast-path build) produces .bin/install-dynamic-plugins and the smoke invocation on an empty dynamic-plugins.yaml exits 0. dynamic-plugins/yarn.lock is intentionally NOT regenerated in this commit — that has to happen against the published @red-hat-developer-hub/cli-module-install-dynamic-plugins@0.1.0 once redhat-developer/rhdh-plugins ships it. Marking the PR as Draft until then. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The container image build workflow finished with status: |
…nbundled cli-module Replaces the dual build (esbuild bundle + backstage-cli) with plain `backstage-cli package build` — the standard Backstage cli-module pattern. The bundle and the keytar gymnastics only existed to satisfy RHDH's init-container `COPY` of a single self-contained `.cjs`; that consumption model is moving to `npm install` (redhat-developer/rhdh#4908), so the single-file requirement is going away. What's left in the package: - `src/installer.ts`: install pipeline + `main(args, programName)`. - `src/index.ts`: `createCliModule(...)` default export, registers the `install` command. Discovered by `backstage-cli` when this package is a dependency of a host project. - `src/command.ts`: thin loader that calls `installer.main`. - `bin/install-dynamic-plugins`: fast-path shim that loads `dist/installer.cjs.js` directly and runs `main(process.argv.slice(2))`, bypassing `@backstage/cli-node`'s `runCliModule` dispatch — saves ~80 ms of cold start for direct/`npx`/init-container invocations. The cli-module discovery path still goes through `runCliModule` and pays the dispatch cost where it belongs. Removed: - `esbuild.config.mjs` (the custom bundle config) - `src/cli.ts` (the esbuild entry) - `dist/install-dynamic-plugins.cjs` from `files` (no longer produced) - `esbuild` devDependency 166/166 tests pass; tsc/lint/prettier/api-reports clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…undle The README and the createCliModule docstring still referenced the self-contained bundle that the package no longer ships: - `npm run build` description and committed-bundle CI-check section. - `node install-dynamic-plugins.cjs "$1"` wrapper line in "How RHDH consumes it" — replaced with a pointer to redhat-developer/rhdh#4908. - `src/index.ts` describing the bin path as the bundle. - Source layout was missing `index.ts` / `command.ts` / `installer.ts`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The container image build workflow finished with status: |
…install-dynamic-plugins (#3246) * feat(install-dynamic-plugins): import package from redhat-developer/rhdh Migrates scripts/install-dynamic-plugins/ from redhat-developer/rhdh#4574 into this repo as @red-hat-developer-hub/install-dynamic-plugins so it can be published to npm and consumed by the RHDH init-container without curl-by-SHA. Runtime contract (CLI args, env vars, plugin-hash format, on-disk layout, tar/OCI security guards) preserved verbatim. Build remains a single self-contained .cjs via esbuild. Tests migrated from vitest to jest to align with the repo's backstage-cli pipeline (14 suites / 166 tests pass). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(install-dynamic-plugins): address self-review feedback - Add bin/install-dynamic-plugins shim and have package.json bin point at it (matches the convention used by extensions-cli, translations-cli, and rhdh-repo-tools). Split src/cli.ts as the esbuild entry so the bundle no longer needs the require.main guard or a shebang banner. - Stop committing dist/install-dynamic-plugins.cjs; the release pipeline rebuilds via the customBuild path, and a new prepack script makes yarn npm publish self-healing for local runs. - Drop the .js suffix from relative imports across src/ so the package matches the rest of the repo and the jest moduleNameMapper workaround is no longer needed. - Consolidate the tsconfigs: the inner package extends the workspace tsconfig and only declares what differs. - Add why-it's-intentional comments to the two eslint-disable lines (PullPolicy const+type pair, tar.x filter inside a sequential loop). - README now leads with the npm/npx usage path; the RHDH init-container section is below. tar/yaml stay in dependencies (not devDependencies as the review suggested) — @backstage/no-undeclared-imports flagged the source imports, and the repo convention treats bundling as opaque. 166/166 tests pass, tsc/lint/prettier clean, bin shim and bundle both exit 0 on the empty-config smoke run. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(install-dynamic-plugins): unblock CI api-reports step The CI step "check api reports and generate API reference" runs `backstage-repo-tools api-reports --ci` before the build, and that tool requires the bin file to introspect the CLI. The previous shim did a plain `require('../dist/install-dynamic-plugins.cjs')`, which failed under CI because dist/ is no longer committed and the build hasn't run yet. - Switch the bin shim to the local-vs-installed pattern used by every other CLI in the repo (extensions-cli, translations-cli, rhdh-repo-tools): when `src/` exists (monorepo), load TS directly via `@backstage/cli/config/nodeTransform`; otherwise require the built bundle (npm-installed scenario). - Add `--help` / `-h` handling to main() so the api-reports tool can introspect the CLI usage without creating a stray `--help/` directory. - Commit the generated `cli-report.md`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(install-dynamic-plugins): address SonarCloud findings - Use String.raw for strings containing backslash literals so the source reads with one '\' instead of '\\\\' (catalog-index.ts log message, extra-catalog-index.test.ts subdirectory fixtures, skopeo.test.ts shell escape). - Switch the OCI regex builder to a joined string array — eliminates the nested template literals SonarCloud was flagging on oci-key.ts (and reads much better). - Object.prototype.hasOwnProperty.call -> Object.hasOwn in merger.test.ts (ES2022, available since Node 16.9). - String#replace(/'/g, ...) -> String#replaceAll("'", ...) in skopeo.test.ts (ES2021). - Hoist test helpers (stageLayer, fakeImageCache) out of their describe blocks so they aren't re-defined on every test. - Drop the redundant parseMaxEntrySize(undefined) call in types.test.ts — the parameter already defaults to process.env.MAX_ENTRY_SIZE. 166/166 tests still pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(install-dynamic-plugins): parse argv with cleye Switches the hand-rolled `process.argv` + USAGE-string handling in main() to `cleye` — the same parser every `@backstage/cli-module-*` package uses (already in our transitive deps). Aligns with the Backstage CLI convention requested during PR review. Existing surface preserved: - positional `<dynamic-plugins-root>` (required, exit 1 if absent) - `--help` / `-h` prints usage and exits 0 - normal run still exits with the installer's status code Bundle grew from 226 kB -> 267 kB (cleye + type-flag minified). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(install-dynamic-plugins): adopt cli-module convention while keeping the bundled bin Aligns with the @backstage/cli-module-* convention without losing the self-contained bundled artifact: - Rename to @red-hat-developer-hub/cli-module-install-dynamic-plugins, backstage.role: cli-module. - src/installer.ts holds the install pipeline and main() (formerly src/index.ts). - New src/index.ts default-exports `createCliModule(...)` registering an `install` command whose loader is src/command.ts. Exposes the package through backstage-cli discovery — `backstage-cli install <dir>` works when the package is a dependency. - src/cli.ts (the esbuild entry) keeps invoking installer.main() directly, so the bundled .cjs stays self-contained: no @backstage/cli-node and no keytar gymnastics in the bin path. - Build is dual now — `backstage-cli package build && node esbuild.config.mjs`. backstage-cli emits dist/index.cjs.js (the cli-module export) and the unbundled supporting modules; esbuild emits dist/install-dynamic-plugins.cjs (the standalone bin). Both are published. - bin shim's installed branch now requires the bundled .cjs explicitly rather than going through `main` — that keeps direct/npx/init-container invocations at ~60 ms cold start instead of paying the cli-module dispatch cost. - main() now takes optional `args` and `programName` so the cli-module loader can pass the command's argv slice and have `--help` print the real invocation (`install-dynamic-plugins install …`). - @backstage/cli-node added as a runtime dependency. It is only loaded by the cli-module discovery path; the bundled bin never imports it. 166/166 tests pass; tsc/lint/prettier/api-reports clean. Bundle size unchanged at ~267 KB. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(install-dynamic-plugins): drop custom esbuild bundle, ship unbundled cli-module Replaces the dual build (esbuild bundle + backstage-cli) with plain `backstage-cli package build` — the standard Backstage cli-module pattern. The bundle and the keytar gymnastics only existed to satisfy RHDH's init-container `COPY` of a single self-contained `.cjs`; that consumption model is moving to `npm install` (redhat-developer/rhdh#4908), so the single-file requirement is going away. What's left in the package: - `src/installer.ts`: install pipeline + `main(args, programName)`. - `src/index.ts`: `createCliModule(...)` default export, registers the `install` command. Discovered by `backstage-cli` when this package is a dependency of a host project. - `src/command.ts`: thin loader that calls `installer.main`. - `bin/install-dynamic-plugins`: fast-path shim that loads `dist/installer.cjs.js` directly and runs `main(process.argv.slice(2))`, bypassing `@backstage/cli-node`'s `runCliModule` dispatch — saves ~80 ms of cold start for direct/`npx`/init-container invocations. The cli-module discovery path still goes through `runCliModule` and pays the dispatch cost where it belongs. Removed: - `esbuild.config.mjs` (the custom bundle config) - `src/cli.ts` (the esbuild entry) - `dist/install-dynamic-plugins.cjs` from `files` (no longer produced) - `esbuild` devDependency 166/166 tests pass; tsc/lint/prettier/api-reports clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(install-dynamic-plugins): drop stale references to the esbuild bundle The README and the createCliModule docstring still referenced the self-contained bundle that the package no longer ships: - `npm run build` description and committed-bundle CI-check section. - `node install-dynamic-plugins.cjs "$1"` wrapper line in "How RHDH consumes it" — replaced with a pointer to redhat-developer/rhdh#4908. - `src/index.ts` describing the bin path as the bundle. - Source layout was missing `index.ts` / `command.ts` / `installer.ts`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(install-dynamic-plugins): address PR review — align with upstream cli-module contract Applies the four findings from @schultzp2020's review: - package.json reshaped to match the upstream `@backstage/cli-module-*` contract: `main`/`types` point at `src/index.ts` for local dev, `publishConfig` overrides with `dist/index.cjs.js`/`dist/index.d.ts`, `prepack` and `postpack` wire up `backstage-cli package prepack/postpack` so the published artifact has the correct entry points, and `files` is `["bin", "dist"]` (the old `dist/**/*.js` glob excluded `.d.ts`). - bin wrapper now uses `@backstage/cli-node/config/nodeTransform.cjs` (the cli-node variant) instead of `@backstage/cli/config/nodeTransform.cjs`. `@backstage/cli` is only a devDependency — the previous import would have broken `npx`/installed-package invocations. - bin wrapper now goes through `runCliModule(...)` like every other `@backstage/cli-module-*` package. The earlier fast path bypassed it to save ~80 ms of cold start, but per @schultzp2020 that cost is one-time per init-container run (not per-plugin), and going through the standard dispatch gives us `--version`/`--help` and future runCliModule improvements for free. - installer.ts: added an `isPlainObject` guard for the parsed main config, mirroring the existing guard on include files. A YAML scalar or array in `dynamic-plugins.yaml` now throws a clear `InstallException` instead of a confusing downstream `TypeError`. Also drops the stale `install-dynamic-plugins.sh` wrapper from the package — it pointed at the long-removed esbuild bundle and was no longer listed in `files`. 166/166 tests pass; tsc/lint/prettier/api-reports clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The container image build workflow finished with status: |
Replaces the COPY of scripts/install-dynamic-plugins/{install-dynamic-plugins.cjs,
install-dynamic-plugins.sh} with an `npm install` of
@red-hat-developer-hub/cli-module-install-dynamic-plugins (built and published
out of redhat-developer/rhdh-plugins).
This unblocks the cli-module structure on the rhdh-plugins side — it lets
that package use the standard `backstage-cli package build` (unbundled,
multi-file dist) instead of a custom esbuild bundle with a keytar stub. See
the conversation context: redhat-developer/rhdh-plugins#3254
Backward compatibility is preserved by writing a tiny
`/opt/app-root/src/install-dynamic-plugins.sh` shim that delegates to the
npm-installed bin, so the Helm chart and Operator init-container spec
continue to invoke `./install-dynamic-plugins.sh /dynamic-plugins-root`
unchanged.
DRAFT — DO NOT MERGE: blocked on
redhat-developer/rhdh-plugins#3254 (or the unbundled successor) being
merged and published to npm. Opened for review of the consumption pattern
and to back the cold-start benchmark posted in Slack.
Trade-off summary (cold-start benchmark on empty config):
- Current (bundled .cjs, 231 KB single file): ~89 ms warm cache (median)
- Proposed (npm install, 25 MB node_modules): ~180 ms warm cache (median)
The ~90 ms gap is the module-resolution overhead of unbundled Node — paid
once per pod start. Image build time also gets +`npm install` of ~25 MB
(one extra layer), offset by deleting ~7000 lines of vendored installer
script from this repo in a follow-up.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… step - Install into /opt/dynamic-plugins-installer (its own dir, no package.json) instead of /opt/app-root/src so npm cannot honor the yarn workspace and perturb the production tree that `yarn workspaces focus` built at line 208. - Delegate the shim to `node_modules/.bin/install-dynamic-plugins` (the symlink npm creates from the package's bin field) instead of reaching into the package's internal layout. - Add `--no-save --omit=dev` so npm doesn't write a package-lock.json into the installer dir and doesn't fetch devDependencies. - Pin the installer to an exact version (0.1.0) so image builds are reproducible. - Add a build-time smoke check (`install-dynamic-plugins --help`) so a missing or renamed CLI entrypoint fails the image build instead of the init container at pod start. The hermetic-build concern (npm reaching the public registry when this Containerfile runs under Konflux with networking disabled) is acknowledged separately in the PR description — it's the real gating work and is not addressed by this commit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The unbundled cli-module variant ships a fast-path bin that calls the installer directly (bypassing @backstage/cli-node's runCliModule dispatch), so the published binary takes the dynamic-plugins-root as a positional without a subcommand prefix — matching the original CLI surface. Verified locally: $ /opt/dynamic-plugins-installer/node_modules/.bin/install-dynamic-plugins /dynamic-plugins-root exits 0 on an empty config. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…gins yarn workspace Pivots redhat-developer#4908 from `RUN npm install …` to a plain yarn dependency in dynamic-plugins/package.json. The existing `yarn install --immutable` at line 151 of this Containerfile pulls the installer as part of the same yarn run that already brings in the rest of the dynamic-plugins deps, so the bin lands at /opt/app-root/src/dynamic-plugins/node_modules/.bin/install-dynamic-plugins and the final stage just writes a shim pointing there. Why yarn and not npm: scripts/local-hermeto-build.sh:213 already prefetches yarn deps for ./dynamic-plugins via cachi2/hermeto. No infra change is needed in this repo or in the midstream Konflux pipeline — the hermetic build "just works". The earlier npm-install approach required wiring npm into hermeto's fetch-deps, which is real work spanning two repos. Validated locally: yarn install of the unbundled cli-module variant (via a tarball of the fast-path build) produces .bin/install-dynamic-plugins and the smoke invocation on an empty dynamic-plugins.yaml exits 0. dynamic-plugins/yarn.lock is intentionally NOT regenerated in this commit — that has to happen against the published @red-hat-developer-hub/cli-module-install-dynamic-plugins@0.1.0 once redhat-developer/rhdh-plugins ships it. Marking the PR as Draft until then. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The rhdh-plugins side reverted the bin shim to the standard `runCliModule` dispatch (redhat-developer/rhdh-plugins#3246 review), so the installed bin now expects an `install` subcommand. Updating the wrapper to match and to forward `"$@"` instead of `"$1"` so any extra positional argument the Helm chart or Operator passes is preserved. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
….lock redhat-developer/rhdh-plugins#3246 merged and the changesets release published @red-hat-developer-hub/cli-module-install-dynamic-plugins as 0.2.0 (the changesets minor bump rolled past 0.1.0 because of the existing `0.0.0` workspace version). Updates the dynamic-plugins/package.json pin to 0.2.0 and regenerates dynamic-plugins/yarn.lock so `yarn install --immutable` (line 151 of the Containerfile) and the hermeto yarn prefetch at scripts/local-hermeto-build.sh:213 both resolve the package. The Build Image GitHub Actions job should now go green. Verified locally: yarn install resolves the package, the bin symlink lands at dynamic-plugins/node_modules/.bin/install-dynamic-plugins, and invoking it with `install <dir>` exits 0 on an empty config (going through the standard @backstage/cli-node runCliModule dispatch like the final Containerfile shim does). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…n npm The dynamic-plugins installer is consumed from @red-hat-developer-hub/cli-module-install-dynamic-plugins via the yarn dependency declared in dynamic-plugins/package.json, so the vendored copy under scripts/install-dynamic-plugins/ is no longer used by the Containerfile and can be removed. Deletes: - scripts/install-dynamic-plugins/ (46 files, ~7000 lines) Cleans up the references that pointed at it: - .gitattributes: drop linguist-generated marker for the bundled .cjs - .dockerignore: drop the dist/ exceptions that kept the bundle in the build context - codecov.yml: drop the install-dynamic-plugins flag (used to track Python coverage from the pre-TS era) - .github/workflows/pr.yaml: drop the vitest + bundle-up-to-date checks - .github/workflows/coverage-baseline.yml: drop the pytest baseline - docs/dynamic-plugins/installing-plugins.md: link to the rhdh-plugins source instead of the vendored path - docs/coverage/e2e-rhdh.md: drop the vitest row from the coverage table - .rulesync/rules/ci-e2e-testing.md + the derived files under .claude/, .cursor/, .opencode/: update the installer link Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
c0fb0ed to
bfdfe8f
Compare
|
The container image build workflow finished with status: |
|
The container image build workflow finished with status: |
The Containerfile's build stage at line 175 wipes everything under
dynamic-plugins/ except dist/, which deleted dynamic-plugins/node_modules/.bin/
along with everything else. The final stage then failed at runtime with
exit code 127 because the bin wasn't there:
/bin/sh: line 1: /opt/app-root/src/dynamic-plugins/node_modules/.bin/install-dynamic-plugins:
No such file or directory
Moves @red-hat-developer-hub/cli-module-install-dynamic-plugins from
dynamic-plugins/package.json to the repo root package.json. The bin then
survives the `yarn workspaces focus --all --production` at line 208 and
lands at /opt/app-root/src/node_modules/.bin/ where the final stage's shim
can reach it. Hermeto already prefetches yarn deps for the repo root
(scripts/local-hermeto-build.sh:213, `{"type": "yarn", "path": "."}`), so
no infra change.
Also corrects the shim path: the published package uses the string form of
`bin` ("bin/install-dynamic-plugins"), and yarn names the symlink after
the package's base name (after the scope), so the actual bin lands at
node_modules/.bin/cli-module-install-dynamic-plugins, not
node_modules/.bin/install-dynamic-plugins.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… era The dynamic-plugins installer was migrated to TypeScript in redhat-developer#4574 and the vendored TypeScript copy was removed earlier in this PR, so the pytest test infrastructure no longer has anything live to test. - Deletes python/requirements-dev.in and python/requirements-dev.txt (pytest, pytest-cov, pytest-mock — and their transitive deps). - Drops the .github/workflows/pr.yaml "Install Python dependencies" step that installed all three requirements files; no subsequent step in the PR workflow uses Python. - Cleans the stale comment in codecov.yml referencing the removed install-dynamic-plugins pytest flag. python/requirements.txt and python/requirements-build.txt stay — they are consumed by the TechDocs venv build in the Containerfile (line 237) for mkdocs / mkdocs-techdocs-core / plantuml-markdown, which is unrelated. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The container image build workflow finished with status: |
|
The container image build workflow finished with status: |
`.claude/scheduled_tasks.lock` is a Claude Code runtime artifact (per-session pid lock); it got accidentally included by `git add -A` in the previous commit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
3a0e4c1 to
e936008
Compare
|
The container image build workflow finished with status: |
… numbers, drop orphaned comments
- Build-time smoke check now invokes `install --help`, not just `--help`.
A top-level `--help` only proves the bin resolves; the shim hardcodes
the `install` subcommand, so if a future release of the package
renames/removes it, the failure should land at image build time, not at
pod start. cleye prints the subcommand help and exits 0 before
validating the required `<dynamic-plugins-root>` positional, so this is
a strict upgrade with no behavioural change for the current published
package.
- Replace hardcoded line-number references in the Containerfile comment
(already drifted: line 175 → actual 173) with the step banners
`=== YARN WORKSPACES FOCUS ===` and `=== DELETE DYNAMIC PLUGINS/* ===`,
which survive edits to the surrounding RUN steps. Same treatment for
scripts/local-hermeto-build.sh: point at the
`{"type": "yarn", "path": "."}` fetch-deps entry instead of a line
number.
- Drop the orphaned `.dockerignore` comment that explained the now-removed
`!scripts/install-dynamic-plugins/dist` re-include lines and was reading
as if it described `**/node_modules`.
- Drop the orphaned `.gitattributes` section header
`# Generated bundles — collapsed in GitHub diffs, …` left behind when
the `dist/install-dynamic-plugins.cjs linguist-generated=true` entry
was removed.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The container image build workflow finished with status: |
The sherif rule in `yarn run monorepo:check` rejects `dependencies` on the private root package.json — the rationale being that dependencies vs devDependencies is a no-op for a private package and creates confusion. CI fails on that check today. Moves `@red-hat-developer-hub/cli-module-install-dynamic-plugins` from the root package.json to `packages/backend/package.json`, keeping the load-bearing behaviour: backend is `private: true` and gets included by `yarn workspaces focus --all --production`, so the bin still hoists to `/opt/app-root/src/node_modules/.bin/cli-module-install-dynamic-plugins` where the Containerfile shim picks it up. Semantically this is the right home anyway — the backend is the runtime that the init-container runs alongside. Containerfile comment updated to point at the new declaration site. Verified locally: - yarn run monorepo:check → No issues found - node_modules/.bin/cli-module-install-dynamic-plugins install --help exits 0 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The container image build workflow finished with status: |
|
gashcrumb
left a comment
There was a problem hiding this comment.
Gave the image a test via rhdh-local worked nicely for me, nice to have this command as a standalone package too.
1059aaa
into
redhat-developer:main




Ready for review
Switches the dynamic-plugins installer from the vendored
scripts/install-dynamic-plugins/(carry-over from the Python era) to the npm package@red-hat-developer-hub/cli-module-install-dynamic-plugins, and deletes the vendored copy in the same PR now that the package is published.The matching rhdh-plugins PR (#3246) is merged and
@red-hat-developer-hub/cli-module-install-dynamic-plugins@0.2.0is on the public npm registry. The rootyarn.lockresolves against the published version.What this PR does
build/containerfiles/Containerfile— drops theCOPYofscripts/install-dynamic-plugins/{install-dynamic-plugins.cjs,install-dynamic-plugins.sh}and writes a small shim at/opt/app-root/src/install-dynamic-plugins.shthat delegates to the yarn-installed bin viainstall \"\$@\". Helm chart and Operator init-container spec keep invoking./install-dynamic-plugins.sh /dynamic-plugins-rootunchanged.packages/backend/package.json+yarn.lock— declares@red-hat-developer-hub/cli-module-install-dynamic-plugins@0.2.0as a runtime dependency of the backend workspace, not of thedynamic-plugins/workspace. The Containerfile's build stage wipesdynamic-plugins/node_modules/before the final stage runs (the=== DELETE DYNAMIC PLUGINS/* ===step keeps onlydist/), so the dep has to live somewhere the=== YARN WORKSPACES FOCUS ===production install will hoist into/opt/app-root/src/node_modules/. Backend isprivate: trueand is included in the production focus install, so the bin hoists correctly. (The dep originally sat on the rootpackage.json, butyarn run monorepo:check(sherif) rejectsdependencieson a private root; backend is the natural alternative — it's the runtime the init-container runs alongside.)bin(\"bin/install-dynamic-plugins\"), so yarn names the symlink after the package's unscoped base name. The shim points at/opt/app-root/src/node_modules/.bin/cli-module-install-dynamic-plugins, not…/install-dynamic-plugins.scripts/install-dynamic-plugins/(46 files, ~7000 lines)..gitattributes,.dockerignore,codecov.yml, the two GitHub Actions workflows that tested the vendored copy (pr.yaml,coverage-baseline.yml),docs/dynamic-plugins/installing-plugins.md,docs/coverage/e2e-rhdh.md, and the rulesync-managed AI rule files. Also drops the orphan pytest test infrastructure (python/requirements-dev.{in,txt}and theInstall Python dependenciesstep inpr.yaml) — TechDocs still usespython/requirements{,-build}.txtfor the mkdocs venv, those stay.Why yarn (and not
npm install)scripts/local-hermeto-build.shalready prefetches yarn deps for the repo root via cachi2/hermeto (the{\"type\": \"yarn\", \"path\": \".\"}entry in the fetch-deps call). Putting the installer as a yarn dep there reuses that existing prefetch — no infra change required in this repo or in the midstream Konflux pipeline.An earlier revision of this PR used
RUN npm install ...directly, which hit a real hermetic-build blocker: hermeto doesn't prefetch npm today, and Konflux runs with networking disabled. Wiring npm into hermeto would have spanned two repos (this one + the midstream). The yarn approach sidesteps that entirely.Why we want to do this
Consuming the package from npm means:
npm installthe same package instead of vendoring their own copies.@backstage/cli-module-*convention: plainbackstage-cli package build, no custom esbuild config, no keytar workaround.Trade-offs (cold-start benchmark)
hyperfine, 50 runs warm cache, emptydynamic-plugins.yaml:.cjs(pre-PRmain)npm install(cli-module dispatch, this PR's path)~117 ms gap per pod start. On a 20-plugin install where
skopeopulls dominate (~60 s of total work) that's 0.2% — invisible in practice. Image build picks up ~25 MB from the new layer (one-time, cached in OCI registry).Containerfile change details
/opt/app-root/src/node_modules/.bin/cli-module-install-dynamic-pluginsvia the existing yarn install +=== YARN WORKSPACES FOCUS ===production step.install --helpsmoke check so a missing bin or a renamedinstallsubcommand fails the image build instead of the init-container at pod start. cleye prints the subcommand usage and exits 0 before validating the required<dynamic-plugins-root>positional, so this check covers both "the bin resolved" and "the verb the shim invokes still exists"./opt/app-root/src/install-dynamic-plugins.shpath forwards\"\$@\"(not just\"\$1\") so any future positional argument the Helm chart or Operator passes is preserved.Cleanup details
.gitattributes— drops thelinguist-generatedmarker for the vendored bundle and the surrounding orphan section header..dockerignore— drops thedist/exceptions that kept the bundle in the build context, and the orphan comment that explained them.codecov.yml— drops theinstall-dynamic-pluginsflag (defined paths pointed at the long-removed Python files) and the comment that referenced it..github/workflows/pr.yaml— drops the vitest test step + theinstall-dynamic-pluginscodecov upload + the bundle-up-to-date verification step + theInstall Python dependenciesstep that only existed to feed the pytest tests of the Python era..github/workflows/coverage-baseline.yml— drops the pytest baseline that was still referencing the Python tests removed in feat(install-dynamic-plugins): port from Python to TypeScript/Node.js #4574.python/requirements-dev.{in,txt}— deleted; only containedpytest,pytest-cov,pytest-mockfor the removed Python install script.docs/dynamic-plugins/installing-plugins.md— installer link points at the rhdh-plugins source.docs/coverage/e2e-rhdh.md— drops the vitest row from the coverage source table..rulesync/rules/ci-e2e-testing.md+ derived files under.claude/,.cursor/,.opencode/— installer link points at the rhdh-plugins source.🤖 Generated with Claude Code