chore(install-dynamic-plugins): consume installer via yarn through dynamic-plugins workspace#4908
Open
gustavolira wants to merge 4 commits into
Conversation
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>
|
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.35% -0.48%
==========================================
Files 121 118 -3
Lines 2350 2325 -25
Branches 562 535 -27
==========================================
- Hits 1312 1287 -25
- Misses 1032 1033 +1
+ Partials 6 5 -1
Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
… 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>
Contributor
|
The container image build workflow finished with status: |
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>
gustavolira
changed the title
Contributor
|
The container image build workflow finished with status: |
Contributor
|
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>
|
Contributor
|
The container image build workflow finished with status: |
gustavolira
changed the title
gustavolira
added a commit
to gustavolira/rhdh-plugins
that referenced
this pull request
Jun 4, 2026
…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>
6 tasks
gustavolira
added a commit
to gustavolira/rhdh-plugins
that referenced
this pull request
Jun 4, 2026
…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>
|
@gustavolira: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ready for review — merge gated on rhdh-plugins publish
The Containerfile change is finalized and the diff is small enough to review now. Cannot merge yet, though: the matching rhdh-plugins PR (#3246) needs to land first and publish
@red-hat-developer-hub/cli-module-install-dynamic-plugins@0.1.0on npm. Once that's published, the sequencing in § Sequencing plays out.Known:
Build Imageis failing — and that's expectedThe
Build ImageGitHub Actions check is red becausedynamic-plugins/yarn.lockdoesn't yet have an entry for@red-hat-developer-hub/cli-module-install-dynamic-plugins— the package isn't published, soyarn install --immutablecan't resolve it.This unblocks itself the moment rhdh-plugins#3246 merges and publishes:
After that commit,
Build Imageshould go green.What this PR does
build/containerfiles/Containerfileswaps theCOPYofscripts/install-dynamic-plugins/{install-dynamic-plugins.cjs,install-dynamic-plugins.sh}for a yarn dependency declared indynamic-plugins/package.json. The existingyarn install --immutableat 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. The final stage writes a small shim at/opt/app-root/src/install-dynamic-plugins.shthat delegates to that bin — Helm chart and Operator init-container spec keep invoking./install-dynamic-plugins.sh /dynamic-plugins-rootunchanged.Why yarn (and not
npm install)scripts/local-hermeto-build.sh:213already prefetches yarn deps for./dynamic-pluginsvia cachi2/hermeto. Adding 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
scripts/install-dynamic-plugins/is vendored in this repo today as a ~7000-line carry-over from the Python era. Consuming the package from npm means:npm installthe same package).The matching rhdh-plugins PR (#3246) ships the package the way Backstage cli-modules normally do: plain
backstage-cli package build, no custom esbuild config, no keytar stub. So this PR doesn't need a "fix it later" follow-up on that side.Trade-offs (cold-start benchmark)
hyperfine, 50 runs warm cache, emptydynamic-plugins.yaml:.cjs(currentmain)npm install(cli-module dispatch)npm install(fast-path bin, what this PR consumes)~42 ms gap per pod start vs the current bundled approach. On a 20-plugin install where
skopeopulls dominate (~60 s of total work) that's 0.07% — 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/dynamic-plugins/node_modules/.bin/install-dynamic-pluginsvia the existingyarn install --immutablestep at line 151.install-dynamic-plugins --helpsmoke check so a missing or renamed CLI entrypoint fails the image build instead of the init-container at pod start./opt/app-root/src/install-dynamic-plugins.shpath delegates to npm's.binsymlink so the Helm chart and Operator init-container spec keep working unchanged.Sequencing
@red-hat-developer-hub/cli-module-install-dynamic-plugins@0.1.0.yarn installindynamic-plugins/, commit the updatedyarn.lockto this branch —Build Imagegoes green.Follow-ups
scripts/install-dynamic-plugins/from this repo once the npm path is in place.🤖 Generated with Claude Code