refactor: use hosted tool cache installer#63
Conversation
imjasonh
left a comment
There was a problem hiding this comment.
blocking merge until I can take a closer look 👀
imjasonh
left a comment
There was a problem hiding this comment.
This change seems like a reasonable translation of the bash version to a Node version, which is great.
I'm slightly worried about what the change will mean for the maintenance burden of the repo; what used to just be inline yamlbash is now JS which needs to be compiled. One benefit is that we get actual unit tests for the code (❤️).
It seems like we'll also install ko faster since it can rely on the tool cache, is that so? Is there any measurement around how much faster that makes things? It's already pretty simple.
I'm not opposed to the rewrite in general, I'm just not super excited about having to maintain this as JS, but maybe I'll get over it.
More than anything, thanks for taking the time to open this PR! It's certainly a lot easier to consider making this change while reviewing code than just thinking about it in the abstract. And having the code already exist lowers the cost (actually, pre-pays the cost) and demonstrates the benefit.
|
|
||
| - uses: pnpm/action-setup@d41d4278a1c93e6b2d6962d5448c0169f6f66cd3 # v4.1.0 | ||
| with: | ||
| version: 11.5.2 |
There was a problem hiding this comment.
One of my least favorite things about GitHub Actions (and the list is long!) is that there's no automatic mechanism for bumping these values, without either a.) writing your own custom script to do it, or b.) having an agent do it, and both kinda suck.
This isn't a reason not to do this on its own, but it's something I'd love to resolve if we go this way.
|
|
||
| - uses: actions/setup-node@a0853c24544627f65ddf259abe73b1d18a591444 # v6.0.0 | ||
| with: | ||
| node-version: '24' |
| - package-ecosystem: gomod | ||
| directory: "/" | ||
| directory: '/' | ||
| schedule: | ||
| interval: "daily" | ||
| interval: 'daily' | ||
| open-pull-requests-limit: 10 | ||
| groups: | ||
| all: | ||
| update-types: | ||
| - "patch" | ||
| - 'patch' |
There was a problem hiding this comment.
What little Go is in this repo doesn't even have external deps, so we can probably just remove this.
(Not necessarily in this PR, just making a note of it)
| import * as exec from '@actions/exec'; | ||
| import * as github from '@actions/github'; | ||
| import * as tc from '@actions/tool-cache'; | ||
| import * as semver from 'semver'; |
There was a problem hiding this comment.
One thing that kinda sucks is that this repo didn't used to depend on anything (besides bash,curl,jq,etc) to install ko, and now it depends on some JS code that needs to be bumped and kept secure.
This isn't a deal breaker, just sort of ...a shame.
I'm happy to see that it doesn't need anything besides @actions/* and semver, that's nice at least, and I'd probably be more strongly opposed if it depended on a bunch of random things just to install a tool.
|
|
||
| export function getKoDownloadUrl( | ||
| tag: string, | ||
| cacheVersion: string, |
There was a problem hiding this comment.
This can just be version right?
There was a problem hiding this comment.
Pull request overview
This PR refactors setup-ko from a composite action that installs into /usr/local/bin into a Node 24 JavaScript action that installs released ko binaries via the GitHub Actions hosted tool cache (while preserving version: tip via go install).
Changes:
- Replaces the composite
action.ymlimplementation with anode24entrypoint (dist/setup/index.js) and a TypeScript source implementation undersrc/. - Adds an installer implementation that resolves versions (including
latest-releasevia GitHub API), downloads/extracts/cachesko, and configuresKO_DOCKER_REPO+ GHCR login. - Introduces Node/TypeScript tooling (tsconfig, Jest, ESLint, Prettier, pnpm lockfile) and updates CI workflows to run format/lint/tests/build and verify generated
dist.
Reviewed changes
Copilot reviewed 10 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
action.yml |
Switches the action runtime to node24 and adds a token input. |
src/setup-ko.ts |
Adds the JS action entrypoint that calls the installer and reports failures. |
src/installer.ts |
Implements version resolution, hosted tool-cache installation, and GHCR/KO_DOCKER_REPO setup logic. |
src/installer.test.ts |
Adds unit tests for platform/arch mapping and URL/tag normalization helpers. |
package.json |
Introduces the Node toolchain, scripts, and dependencies for building/testing/bundling the action. |
pnpm-lock.yaml |
Locks Node dependencies for reproducible installs. |
tsconfig.json |
Adds TypeScript compiler configuration for the action source. |
jest.config.js |
Configures Jest + ts-jest for TypeScript tests and coverage. |
.eslintrc.js |
Adds ESLint configuration for TypeScript/Jest code quality checks. |
.prettierrc.js |
Adds Prettier configuration for formatting consistency. |
.gitignore |
Adds standard Node/TypeScript build and tooling ignores. |
.github/workflows/ci.yaml |
Adds a build job (format/lint/test/build + dist diff check) and gates integration tests on it. |
.github/workflows/use-action.yaml |
Updates integration workflow steps to reflect the new install mechanism (no /usr/local/bin cleanup). |
.github/dependabot.yml |
Adds npm ecosystem updates alongside existing update configurations. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
52ce865 to
4b28bcd
Compare
|
Thanks for the review. I’ve updated the newly added action setup revision and npm dependencies to their latest versions. I also verified that the action runs successfully in CI on 27269391004, and that the existing 'Use Action' workflow still passes on 27269412405. |
|
Regarding the performance concern, I agree that for a single job under normal network conditions, the time saved may only be a few seconds, since this action only downloads an archive of around 10 MB. The main value is that using a setup action makes the installation path more reliable and consistent across CI environments. It can help tolerate transient network issues, avoid relying on curl or other commands being available, support setting up specific or multiple ko versions, and avoid requiring sudo for installation. These benefits also accumulate across jobs and releases. Reducing repeated release downloads is useful for both users and the upstream release infrastructure. This is similar in spirit to |
This refactors
setup-kointo a Node 24 action that installs releasedkobinaries through the GitHub Actions hosted tool cache.The old composite action installed directly into
/usr/local/bin, which required sudo-aware behavior and did not reuse the runner tool cache that setup actions normally use.This gives us:
koversionssudo,jq, orcurl | tarinstall pathv0.18.1and0.18.1user inputsdist/setup/index.jsbundleWhat changed under the hood
action.ymlnow runs withnode24and points atdist/setup/index.js.@actions/tool-cache, extracted, cached, and added toPATH.latest-releaseis resolved through the GitHub API.version: tipstill usesgo install github.com/google/ko@main.use-sudowas removed because the action no longer writes to/usr/local/bin.distbundle before running the action integration checks.How this was verified
Existing action workflow:
ubuntu-latestandmacos-latest.ubuntu-latest-armis not covered by this run because this repository does not currently have access to that runner.New checks added for this refactor:
pnpm run format-check: verifies the TypeScript and workflow files follow the checked-in Prettier configuration.pnpm run lint: verifies the TypeScript action source follows the ESLint rules used for the project.pnpm test: verifies the version, platform, architecture, and download URL mapping logic.Fixes #58 #59