chore(install-dynamic-plugins): consume installer from npm#4908
Open
gustavolira wants to merge 3 commits into
Open
chore(install-dynamic-plugins): consume installer from npm#4908gustavolira wants to merge 3 commits into
gustavolira wants to merge 3 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.25% -0.58%
==========================================
Files 121 109 -12
Lines 2350 2132 -218
Branches 562 536 -26
==========================================
- Hits 1312 1178 -134
+ Misses 1032 954 -78
+ Partials 6 0 -6
Continue to review full report in Codecov by Sentry.
🚀 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: |
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.
Summary
Swaps the
COPYofscripts/install-dynamic-plugins/{install-dynamic-plugins.cjs,install-dynamic-plugins.sh}for annpm installof@red-hat-developer-hub/cli-module-install-dynamic-plugins@0.1.0(built and published out of redhat-developer/rhdh-plugins#3254 / the unbundled successorfeat/install-dynamic-plugins-cli-module-unbundled).Lets the rhdh-plugins side use the standard
backstage-cli package build(the cli-module convention) and drops a couple of workarounds on its side: no custom esbuild bundling step, no keytar native-binary stub.Containerfile change
/opt/dynamic-plugins-installer/(its own dir, nopackage.json) so npm cannot reach into/opt/app-root/src— that path is the yarn workspace root andnpm installthere would honorworkspacesand reshuffle the production tree built byyarn workspaces focus.--no-save --omit=devso npm doesn't writepackage-lock.jsoninto the installer dir.0.1.0so image builds stay reproducible.install-dynamic-plugins --helpsmoke check so a missing or renamed CLI entrypoint fails the image build instead of the pod.install-dynamic-plugins.shshim at the original/opt/app-root/src/path delegates tonode_modules/.bin/install-dynamic-plugins(npm-managed symlink) so the Helm chart and Operator init-container spec keep working unchanged.Cold-start benchmark (final)
On an empty
dynamic-plugins.yaml, macOS, hyperfine, 50 runs each, warm cache:.cjs(currentmain)npm installcli-module + dispatchnpm installcli-module + fast-path bin (this PR's target)The fast-path bin (in the rhdh-plugins unbundled branch) bypasses
@backstage/cli-node'srunCliModuledispatch for direct invocation — saves ~75 ms of cold start. Net cost vs the current bundled approach is ~42 ms per pod start, dwarfed by the actualskopeo/npm packwork that follows.Hermetic build (Konflux / hermeto)
scripts/local-hermeto-build.sh:213calls hermetofetch-depswithrpm,yarn,yarn ./dynamic-plugins, andpip— nonpm. Downstream Konflux runs withenableNetwork: false, so as-is this PR'sRUN npm install ...will fail in the hermetic build.hermeto does support npm prefetch (needs
package-lock.jsonv2+). To land this PR end-to-end, the gating work is:package-lock.jsonsomewhere in this repo describing the installer + transitive deps{"type": "npm", "path": "<lockfile-dir>"}to the hermetofetch-depsinvocation inscripts/local-hermeto-build.shand the equivalent midstream config (seesync-midstream.sh)RUN npm installin this Containerfile needs to read from/cachi2/output/deps/npm/(likely via injected env vars fromgenerate-env)Happy to do that work in a follow-up commit on this PR (or split into a separate prep PR) — flagging now so reviewers can weigh the cost. If the hermeto-npm wiring is too costly, the alternative is to keep the
COPYpattern but pull the bundled.cjsfrom the published npm tarball duringsync-midstream.sh— that keeps the hermetic build trivial and gives us the upstream-published source of truth.Follow-ups
scripts/install-dynamic-plugins/from this repo once@red-hat-developer-hub/cli-module-install-dynamic-pluginsis published and consumed.npminto hermetofetch-deps(per "Hermetic build" above).🤖 Generated with Claude Code