Skip to content

refactor: use hosted tool cache installer#63

Open
Mmx233 wants to merge 6 commits into
ko-build:mainfrom
Mmx233:refactor/hostedtoolcache
Open

refactor: use hosted tool cache installer#63
Mmx233 wants to merge 6 commits into
ko-build:mainfrom
Mmx233:refactor/hostedtoolcache

Conversation

@Mmx233

@Mmx233 Mmx233 commented Jun 9, 2026

Copy link
Copy Markdown

This refactors setup-ko into a Node 24 action that installs released ko binaries 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:

  • hosted tool cache installs for released ko versions
  • no sudo, jq, or curl | tar install path
  • support for both v0.18.1 and 0.18.1 user inputs
  • stable cache keys based on normalized semver versions
  • a standard JavaScript action entrypoint with a checked-in dist/setup/index.js bundle

What changed under the hood

  • action.yml now runs with node24 and points at dist/setup/index.js.
  • Released versions are downloaded with @actions/tool-cache, extracted, cached, and added to PATH.
  • latest-release is resolved through the GitHub API.
  • version: tip still uses go install github.com/google/ko@main.
  • use-sudo was removed because the action no longer writes to /usr/local/bin.
  • The workflow now checks formatting, linting, tests, and the generated dist bundle before running the action integration checks.

How this was verified

Existing action workflow:

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

@imjasonh imjasonh left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

blocking merge until I can take a closer look 👀

@imjasonh imjasonh left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread .github/workflows/ci.yaml

- uses: pnpm/action-setup@d41d4278a1c93e6b2d6962d5448c0169f6f66cd3 # v4.1.0
with:
version: 11.5.2

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread .github/workflows/ci.yaml

- uses: actions/setup-node@a0853c24544627f65ddf259abe73b1d18a591444 # v6.0.0
with:
node-version: '24'

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this one too

Comment thread .github/dependabot.yml
Comment on lines 4 to +12
- package-ecosystem: gomod
directory: "/"
directory: '/'
schedule:
interval: "daily"
interval: 'daily'
open-pull-requests-limit: 10
groups:
all:
update-types:
- "patch"
- 'patch'

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Comment thread src/installer.ts Outdated
import * as exec from '@actions/exec';
import * as github from '@actions/github';
import * as tc from '@actions/tool-cache';
import * as semver from 'semver';

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread src/installer.ts Outdated

export function getKoDownloadUrl(
tag: string,
cacheVersion: string,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can just be version right?

Comment thread package.json

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.yml implementation with a node24 entrypoint (dist/setup/index.js) and a TypeScript source implementation under src/.
  • Adds an installer implementation that resolves versions (including latest-release via GitHub API), downloads/extracts/caches ko, and configures KO_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.

Comment thread src/installer.ts
Comment thread src/installer.ts
Comment thread .github/workflows/ci.yaml Outdated
@Mmx233 Mmx233 marked this pull request as draft June 10, 2026 06:54
@Mmx233 Mmx233 force-pushed the refactor/hostedtoolcache branch from 52ce865 to 4b28bcd Compare June 10, 2026 10:19
@Mmx233 Mmx233 marked this pull request as ready for review June 10, 2026 10:28
@Mmx233

Mmx233 commented Jun 10, 2026

Copy link
Copy Markdown
Author

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.

@Mmx233

Mmx233 commented Jun 10, 2026

Copy link
Copy Markdown
Author

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 pnpm/action-setup, where the common case may be simple, but a dedicated setup action provides more reliable handling for CI edge cases than ad-hoc shell commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fails on self-hosted runners without sudo access due to hardcoded install path

3 participants