Skip to content

feat: add version-file input to read kubectl version from .tool-versions#261

Open
acouvreur wants to merge 1 commit into
Azure:mainfrom
acouvreur:feature/version-file
Open

feat: add version-file input to read kubectl version from .tool-versions#261
acouvreur wants to merge 1 commit into
Azure:mainfrom
acouvreur:feature/version-file

Conversation

@acouvreur
Copy link
Copy Markdown

Closes #259

Summary

Adds a new version-file input that reads the kubectl version from a .tool-versions file (asdf/mise compatible format).

Changes

  • action.yml – new optional version-file input
  • src/helpers.tsparseToolVersionsFile() parses a .tool-versions file and returns the kubectl version, skipping comments and blank lines
  • src/run.tsrun() checks version-file first; if provided the raw version is resolved via resolveKubectlVersion() (supports both 1.27 and 1.27.15 forms)
  • src/run.test.ts – unit tests for parseToolVersionsFile() and the new run() code path

Usage

- uses: Azure/setup-kubectl@v5
  with:
    version-file: .tool-versions   # reads kubectl entry from this file

Example .tool-versions:

kubectl 1.27.15
node 20.0.0

Implements Azure#259

- Add `version-file` input to action.yml that accepts a path to a
  .tool-versions file (asdf/mise compatible format)
- Add `parseToolVersionsFile()` helper that reads kubectl version
  from a .tool-versions file, skipping comments and blank lines
- Update `run()` to check `version-file` before `version`; when
  provided, the resolved version goes through `resolveKubectlVersion()`
  supporting both full (1.27.15) and major.minor (1.27) forms
- Add unit tests for `parseToolVersionsFile()` and the new run() path
@acouvreur acouvreur requested a review from a team as a code owner May 13, 2026 19:08
@Tatsinnit Tatsinnit requested a review from Copilot May 13, 2026 19:13
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds support for selecting the kubectl version from an asdf/mise-style .tool-versions file, with the new input taking precedence over the existing version input.

Changes:

  • Adds optional version-file input to action metadata.
  • Parses .tool-versions files for a kubectl entry.
  • Updates run() and unit tests for the new input path.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
action.yml Adds the version-file input and makes version optional with a default.
src/helpers.ts Adds .tool-versions parsing logic.
src/run.ts Reads version-file before falling back to version.
src/run.test.ts Adds and updates tests for version resolution and file parsing.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/run.ts

if (versionFile) {
const rawVersion = parseToolVersionsFile(versionFile)
version = await resolveKubectlVersion(rawVersion)
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@Tatsinnit do you want me to address this issue ?

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.

Thank you so much for the contributions @acouvreur here are some thoughts, please: #261 (review)

Copy link
Copy Markdown
Member

@Tatsinnit Tatsinnit left a comment

Choose a reason for hiding this comment

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

❤️ 💡 Thank you so much @acouvreur for this contribution, even though this is new feature addition. I really appreciate this contribution. ❤️

There are few takes which we should think as team (including OP) for brainstorm, therefore sharing the whole thought process here. and gently fyi to @davidgamero & @bosesuneha for their thoughts as maintainers as well - most of the observation is detailed below, revolves around the documentation update and some key observations.

Suggested key observations:

  • Address Copilot's review: assert in tests that toolCache.find/downloadTool is called with the resolved/normalized v1.27.15 when .tool-versions says kubectl 1.27 (and the bare 1.27.15 case).
  • Add a test for kubectl latest inside .tool-versions: decide whether to support it (route through getStableKubectlVersion) or reject it explicitly.
  • Add a regex sanity check on the parsed version before handing it to the resolver.
  • Document precedence (version-file overrides version) in the action.yml description and README.
  • Consider warning (core.warning) when both version-file and a non-default version are provided.

Behavior changes vs. current

  1. version input becomes optional (was required: true, now required: false with default: 'latest').

    • Existing workflows that omitted version previously errored at the action layer; now they silently resolve to latest. Mild behavior drift, but Copilot review's main concern is not here.
    • Internally run.ts still calls core.getInput('version', {required: true}): so the required-ness moved from action metadata to runtime. If a user passes version-file: '' and no version, the runtime required: true still throws. Slightly inconsistent UX (action.yml says optional, runtime says required when no file).
  2. Precedence ordering: version-file wins over version. If a workflow sets both (e.g., matrix overrides), the explicit version is silently ignored. Worth documenting more loudly — silent-ignore is a footgun.

  3. Resolution path divergence (Copilot reviewer flagged this):

    • Old version path: latestgetStableKubectlVersion(), else → resolveKubectlVersion() (handles 1.27 → latest patch).
    • New version-file path: only calls resolveKubectlVersion(rawVersion). It does not handle a literal latest token inside .tool-versions (asdf/mise allow kubectl latest). It will try to download vlatest and fail. Tests don't cover this.
  4. v prefix handling: .tool-versions convention is bare versions (1.27.15), but resolveKubectlVersion is what normalizes to v1.27.15. Need to confirm it handles both 1.27.15 and v1.27.15 entries equally, currently relies on existing helper, it is OK but worth a test.

  5. First non-comment kubectl line wins — duplicate kubectl entries silently ignored. Fine, but undocumented.

Security aspects

  1. Path traversal / arbitrary file read (low risk, worth noting): parseToolVersionsFile does fs.readFileSync(filePath, 'utf8') with no validation, no workspace-root containment. A malicious workflow author could already read any file the runner can read, so this isn't a new privilege: but it does turn an arbitrary file's first whitespace-split token after kubectl into a string that flows into a download URL. Threat model: workflow author is trusted, so acceptable, but should be acknowledged.

  2. Untrusted content → download URL injection: The parsed version string is interpolated into getkubectlDownloadURL. If a .tool-versions file is fetched from a PR (e.g., pull_request_target flow checking out PR code), an attacker-controlled value could become part of the URL. Currently resolveKubectlVersion likely validates against the k8s release tag list, which mitigates — but the new code path should be explicitly tested for malformed/malicious tokens like ../, ..%2F, shell metacharacters, very long strings, embedded newlines (split is /\s+/ so newlines split, good). Recommend adding a strict regex validation ^v?\d+\.\d+(\.\d+)?$ before passing to resolver.

  3. No fs.existsSync check / error messaging: A missing file throws an ENOENT: fine, but error doesn't tell users the file wasn't found vs. malformed. Minor.

  4. Symlink following: readFileSync follows symlinks. A .tool-versions symlinked to passwd would be parsed; parser would simply not find kubectl and throw. Low impact.

  5. No size limit: readFileSync of an attacker-supplied huge file could OOM the runner. Negligible in practice for GitHub Actions, but a maxBytes or stream-based read would be defensive.

  6. Supply-chain neutral: No new network endpoints, no new dependencies, no new code execution surfaces. The download flow is unchanged.

Altogather: it seems low-risk, additive feature. No new attack surface of consequence, but the new code path skips the latest branch and lacks input validation/tests at the boundary; those are the concrete improvements either we can do before or as post PR action.

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.

Feature Request: Add version-file input

3 participants