Add npm trusted-publishing support + ci.yaml publish workflow#34
Conversation
Distribute the volcano CLI via npm as a thin wrapper (@kong/volcano-cli): - package.json with bin shim, files allowlist, postinstall, public access - bin/volcano.js launcher execs the platform binary, forwarding args/exit code - scripts/npm/download.js downloads + SHA256-verifies the binary from the matching GitHub Release; self-heals on first run if postinstall is skipped - .github/workflows/ci.yaml publishes to npm via OIDC trusted publishing (id-token: write, no NPM_TOKEN) on stable release publish - README + .gitignore updates
There was a problem hiding this comment.
Pull request overview
This PR adds an npm-distributed wrapper package (@kong/volcano-cli) that downloads a matching GitHub Release binary (verified via SHA256SUMS) and introduces a GitHub Actions workflow to publish that package to npm using trusted publishing (OIDC).
Changes:
- Introduces an npm package (
package.json) + launcher shim (bin/volcano.js) to run the platform-specific Release binary. - Adds Node scripts to download and SHA256-verify Release assets (
scripts/npm/*) with a non-fatal postinstall hook. - Adds an npm publish workflow triggered by stable GitHub Releases (
.github/workflows/ci.yaml) and updates docs/ignore rules.
Reviewed changes
Copilot reviewed 5 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
scripts/npm/install.js |
Adds non-fatal postinstall hook to download the CLI binary (or defer to first run). |
scripts/npm/download.js |
Implements platform resolution, release URL construction, download, and SHA256 verification. |
bin/volcano.js |
Adds launcher shim that downloads on demand and execs the native binary with argument/stdio passthrough. |
package.json |
Defines @kong/volcano-cli package metadata, published files allowlist, scripts, and publish config. |
.github/workflows/ci.yaml |
Adds OIDC-based npm publish workflow triggered by stable release: published. |
README.md |
Documents npm installation flow and environment variable to skip postinstall download. |
.gitignore |
Ignores downloaded platform binaries and node_modules/. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…write, pin npm - download.js: select http/https client by URL scheme (supports http mirrors), reject unsupported schemes with a clear error - download.js: remove partial temp file on download failure; clear destination before rename so force re-download / Windows overwrite works - ci.yaml: pin npm to the 11 line instead of npm@latest for reproducibility
- Rename .github/workflows/ci.yaml -> publish-npm.yml (parallels publish-cli.yml; avoids confusion with the existing ci.yml Go checks) - bin/volcano.js: handle unsupported platforms (e.g. win32-arm64, which passes the package.json os/cpu gate) with a clean error instead of an unhandled promise rejection; add a last-resort .catch() on main() - publish-npm.yml: scope id-token: write to the publish job (least privilege); widen release-asset verification retries (~90s) to ride out the window where the release event can fire before all assets finish uploading
- download.js: hash the fully-written temp file instead of a mid-stream data listener; verifies exactly what landed on disk and avoids flowing-mode subtleties (#5) - publish-npm.yml: verify assets with a ranged GET (first byte) instead of a HEAD request, confirming they are actually downloadable like postinstall does; GitHub honors Range so only 1 byte is fetched (#6)
swkeever
left a comment
There was a problem hiding this comment.
I found one issue in the automatic npm publish path.
marckong
left a comment
There was a problem hiding this comment.
Review
Careful, well-documented change making the repo npm publish-compatible via trusted publishing (OIDC). I traced asset naming and SHA256SUMS format against publish-cli.yml / install-volcano.sh and they line up (parseChecksum, TARGETS, and the v<version> tag mapping all match), and node --check passes on all three JS files.
Edge cases I'd normally hunt for are already handled: win32-arm64 caught by resolveTarget() throwing, non-fatal postinstall with first-run self-heal, signal re-raising, partial-download cleanup, and the 0.0.0 publish guard. No blocking bugs found.
Overall correctness: correct. Existing code/tests are unaffected and the patch is free of blocking issues. Just the one already-documented operational caveat: the one-time npm Trusted Publisher setup (Workflow filename: publish-npm.yml) must be configured or publishing will fail — expected, not a code defect.
Two optional non-blocking notes inline below.
Fold npm publishing into publish-cli.yml as a stable-only publish-npm job instead of a standalone release-triggered workflow: - npm's trusted publisher must be the workflow that directly runs npm publish; dispatched/reusable workflows are not supported, and a release: published trigger would not fire for GITHUB_TOKEN-created releases (swkeever) - runs in the tag-push release run, needs publish-github-release so assets are already uploaded (no race); id-token: write scoped to the job - continue-on-error during bootstrap so releases stay green until an npm admin does the manual first publish + configures OIDC (workflow file: publish-cli.yml) - delete standalone .github/workflows/publish-npm.yml Address marckong review (P3): skip download for placeholder version 0.0.0 so maintainers running npm install in a source checkout see a quiet skip instead of a failed-download warning (install.js), and the shim errors cleanly without hitting a nonexistent v0.0.0 release (download.js).
|
Thanks both — all review feedback addressed: @swkeever (blocking): @marckong (2 × P3): (1) postinstall now skips the placeholder Rollout note: the |
swkeever
left a comment
There was a problem hiding this comment.
Found one workflow input issue.
package-manager-cache is not a v4 input (added in v5), so it was silently ignored. v4 only caches when 'cache' is set, which it is not here, so behavior is unchanged; dropping the dead line.
marckong
left a comment
There was a problem hiding this comment.
Summary
Adds npm distribution via a thin wrapper package (@kong/volcano-cli) published through npm trusted publishing (OIDC) from a new stable-only publish-npm job in publish-cli.yml. The launcher shim + postinstall download and SHA-256-verify the platform binary from the matching GitHub release, with a self-healing first-run fallback.
Review
In solid shape — job wiring matches the resolve-release/publish-github-release outputs, checksum parsing matches the shasum -a 256 manifest generated by the release job, and edge cases (dev version 0.0.0, --ignore-scripts, Windows arm64 passing the os/cpu gate) all fail cleanly. One minor nit:
- [P3]
package-manager-cache(line 396) is not a validactions/setup-node@v4input — it was introduced in@v5, so on@v4it's silently ignored (v4 doesn't cache by default anyway). Drop the line, or bump tosetup-node@v5if disabling a default cache was the intent.
Overall: correct — no blocking issues; existing jobs and release behavior are unaffected.
Summary
Makes the repo
npm publish-compatible via npm trusted publishing (OIDC) — no long-livedNPM_TOKEN. The CLI is distributed as a thin npm wrapper (@kong/volcano-cli) around the existing cosign-signed GitHub Release binaries, and it is published from a stable-onlypublish-npmjob insidepublish-cli.yml.Changes
package.json—@kong/volcano-cliwithbin: volcano → bin/volcano.js,filesallowlist (published tarball is ~9 kB — no Go source or 10 MB binary),postinstall,publishConfig.access=public, andrepository.urlmatching the GitHub repo (required for trusted publishing).bin/volcano.js— launcher that execs the platform binary, forwarding args, stdio, and exit code; downloads on first run if the binary is missing. Unsupported platforms (e.g. Windows arm64) and the0.0.0dev version fail with a clean error instead of an unhandled rejection.scripts/npm/download.js— maps platform → release target, downloads from the matchingv<version>release, and verifies against that release'sSHA256SUMS(pure Node, no cosign at install time). Skips the placeholder0.0.0version.scripts/npm/install.js— postinstall hook (non-fatal sonpm installstill succeeds offline / with--ignore-scripts); quietly skips for0.0.0source checkouts..github/workflows/publish-cli.yml— newpublish-npmjob (needs: [resolve-release, publish-github-release], stable-only) that sets the version from the release tag, verifies the release assets exist, and runsnpm publishvia OIDC (id-token: writescoped to the job,npm@11). Publishing lives here (not a separate workflow) because npm's trusted publisher must be the workflow that directly runsnpm publish— dispatched/reusable workflows are unsupported, and arelease: publishedtrigger would not fire forGITHUB_TOKEN-created releases.README.md/.gitignore— npm install docs; ignore downloaded binaries +node_modules.Rollout
@kong/volcano-cli(npm has no "pending publisher"; the package must exist before OIDC can be attached). Use a real version matching an existing GitHub Release, e.g.npm version 0.0.4 && npm publish --access public.Kong, repositoryvolcano-cli, workflow file namepublish-cli.yml(the file that runsnpm publish).Publish to npmstep iscontinue-on-error: trueso releases stay green while it failsENEEDAUTH. Removecontinue-on-erroronce OIDC is live so real failures surface; after that, stable releases publish automatically with no further changes.Verification
node --checkon all JS;publish-cli.ymlparses as valid YAML with thepublish-npmjob wired correctlynpm install → runagainst the realv0.0.4release (downloaded, checksum-verified, ranvolcano --version)0.0.0dev version; postinstall skips0.0.0quietlyNotes
vX.Y.Zreleases publish to npm; nightlies/prereleases are skipped.SHA256SUMSover TLS rather than running cosign (which the shell installer does). This keeps the package dependency-free at install; npm provenance still attests the wrapper.