fix(coder/kasmvnc): use the release asset names KasmVNC publishes by mthemis-provenir · Pull Request #1217 · coder/registry

mthemis-provenir · 2026-10-28T23:11:49Z

Summary

run.sh builds KasmVNC release asset URLs from hardcoded patterns, and three of
the four no longer match what upstream publishes. Any workspace pinning
kasm_version >= 1.5.1 (and Alpine >= 1.4.0, Fedora >= 1.5.0) 404s during
the download step, so the module installs nothing.

What changed upstream

KasmVNC	Change
1.4.0	Alpine: `kasmvnc.alpine_321_x86_64.tgz` → `kasmvncserver_alpine_321_1.4.0_x86_64.apk`
1.5.0	Fedora: `kasmvncserver_fedora_forty_…` → `kasmvncserver_fedora_42_…`
1.5.1	deb: `kasmvncserver_noble_…` → `kasmvncserver_ubuntu_noble_…` (kasmtech/KasmVNC@`6b72886`)

Two more mismatches on the same path:

Alpine's VERSION_ID carries the patch component (3.21.0), so the previous
dot-stripping composed alpine_3210 instead of alpine_321.
openSUSE Leap reports VERSION_ID=15.6 (assets are opensuse_15) and
ID=opensuse-leap, so it never matched the opensuse case at all and exited
with "Unsupported distribution".

Changes

run.sh — version-gated names for deb (>= 1.5.1), Fedora rpm
(>= 1.5.0, keeping the release-name form for older versions), and Alpine
(>= 1.4.0 selects .apk and apk add --allow-untrusted, older versions keep
the tarball). rpm versions are truncated to the major version, openSUSE's ID
is normalised, and Fedora's VERSION_ID stays numeric while the release name
lives in its own variable. Gates use a small version_ge X.Y.Z comparator
rather than sort -V/dpkg --compare-versions, because the same block runs on
Alpine (busybox only) and on the deb/rpm hosts (neither guaranteed). A
kasm_version that isn't X.Y.Z now fails with a clear message instead of
404ing at download time.
main.test.ts — asserts the composed asset name for 11 platform/version
pairs by extracting the naming block from run.sh (install functions stubbed,
nothing downloaded). Previously the module's tests only ran terraform apply,
which is why all three renames shipped silently.
test/test.ts — runTerraformApply/runTerraformInit run
TERRAFORM_BINARY_NAME (falling back to TF_ACC_TERRAFORM_PATH, then
terraform) so the suite can run against OpenTofu. Separate commit; happy to
split it into its own PR if you'd rather.

Verification

Composed 40 (platform, version) names and probed them against the real release
URLs: every one 302 except builds that were never published (fedora_42 before
1.5.0, alpine_322 at 1.4.0, tarballs for Alpine 3.21/3.22).
bun test registry/coder/modules/kasmvnc: 16 pass under terraform and under
OpenTofu 1.13.2 with terraform removed from PATH. Against the pre-patch
run.sh the new cases fail with the actual bug, e.g. expected
kasmvncserver_ubuntu_noble_1.5.1_amd64.deb, received
kasmvncserver_noble_1.5.1_amd64.deb.
Locally green: check:shellcheck, fmt:ci (prettier), validate_set_u_order.sh,
check:validate, crate-ci/typos.

KasmVNC renamed its release assets in three separate releases since this module's packaging logic was written, so a workspace pinning any recent version fails to install: * 1.4.0 replaced the Alpine tarball with an apk package; 1.3.2 was the last release with kasmvnc.alpine_<ver>_<arch>.tgz. Alpine's VERSION_ID carries the patch component (3.21.0), so stripping dots built "alpine_3210" even for the tarball names. * 1.5.0 renamed Fedora assets from the release name (fedora_forty) to the major version (fedora_42). * 1.5.1 added the distro name to deb assets: kasmvncserver_noble_... became kasmvncserver_ubuntu_noble_.... See kasmtech/KasmVNC@6b72886, which aligned the deb output directory with the <os>_<codename> convention the upload job parses to derive the published name. rpm assets always carry <distro>_<major version>, which openSUSE Leap broke twice: it reports VERSION_ID=15.6 (assets are opensuse_15) and ID=opensuse-leap, so it previously fell through to "Unsupported distribution". The version gates use a small X.Y.Z comparator instead of `sort -V` or `dpkg --compare-versions`, because the same logic has to run on Alpine, where only busybox is available, and on the deb and rpm hosts, where neither tool is guaranteed. Every KasmVNC tag is X.Y.Z, and a non-conforming kasm_version now fails with a clear message instead of 404ing at download time.

main.test.ts only ran terraform apply, so run.sh's URL construction was never exercised and all three renames shipped unnoticed: the module kept passing while every install 404'd. These cases extract the naming block from run.sh (un-escaping the Terraform template) and assert the composed file name for each platform, with the install functions stubbed so nothing is downloaded. Every expectation is a name that exists on the KasmVNC releases page. If run.sh is restructured so the block cannot be found, the test fails loudly instead of silently covering nothing.

The module tests only need a Terraform-compatible CLI, so let the caller pick one: TERRAFORM_BINARY_NAME (the name CDK Terrain and Atlantis use, so it takes a command name or a path), falling back to TF_ACC_TERRAFORM_PATH (the path-valued variable terraform-plugin-testing uses), then plain terraform. Verified against OpenTofu v1.13.2 with terraform removed from PATH.

Copilot

🟡 Changes recommended

Runtime references to unset KASM_VERSION abort fresh Debian, Fedora, and Alpine installations under set -u, while the test harness masks the regression.

4 open issues

What changed in this PR

This PR updates the KasmVNC workspace module to match upstream release asset names and adds regression tests for package-name selection.

Changes:

Adds version-gated deb, Fedora RPM, and Alpine package naming, plus openSUSE and OS-version normalization.
Adds package-name tests across platforms and checks malformed KasmVNC versions.
Lets shared test helpers select Terraform or OpenTofu through environment variables.

File	Description
`test/test.ts`	Makes the Terraform-compatible test executable configurable.
`registry/coder/modules/kasmvnc/run.sh`	Updates release asset selection and adds Alpine APK installation.
`registry/coder/modules/kasmvnc/main.test.ts`	Adds platform/version naming cases and invalid-version coverage.

_{Review effort: Balanced}

KASM_VERSION is only a Terraform template input (main.tf), so the three version gates that compared against a runtime $KASM_VERSION referenced a shell variable that is never assigned. Under `set -u` that aborts a fresh install on deb, Fedora and Alpine alike: "KASM_VERSION: unbound variable". The naming tests missed it because the harness exported KASM_VERSION into the subprocess, which is not what templatefile does; the rendered script has no such variable. The harness now substitutes ${KASM_VERSION} the way templatefile does before unescaping shell expansions, leaves the variable unset, and fails the case if the extracted block exits non-zero — so a placeholder/runtime mixup shows up as a test failure instead of a broken install. Also pin the legacy Alpine case to the 3.20 tarball, which v1.3.2 actually published (alpine_321 returns 404), so every expected asset in the matrix exists upstream.

Installation behaviour changed, so bump the patch version and the README reference with .github/scripts/version-bump.sh patch.

…e block Substituting the placeholder is only half of it: if the developer's shell has KASM_VERSION set, the extracted block would find it and the unbound-variable regression would stay hidden. Remove it from the child environment so the test only passes when run.sh keeps the value at plan time.

mthemis-provenir · 2026-11-02T11:17:17Z

Thanks @matifali, they should be fixed now.

matifali · 2026-11-02T11:29:08Z

@Edd88-pixel can you review and test this please? Thanks

matifali · 2026-11-02T11:29:43Z

Thanks for your contribution. We will get it reviewed and merged soon.

Edd88-pixel

LGTM, I tested it and everything works well. Nice work! 🙂

github-actions · 2026-11-03T09:21:56Z

Module Scorecard Check

`coder/kasmvnc`: 55 → 59

✅ Score improvement: 55 → 59 (+4).

Theme	Before	After
Presentation & Onboarding	6 / 17	6 / 17
Integration	—	—
Credential Hygiene	18 / 20	16 / 16
Restricted-Environment	5 / 20	5 / 20
Engineering Quality	8 / 10	10 / 10
Overall	55 / 100	59 / 100

Full scorecard for this PR

Presentation & Onboarding	Credential Hygiene	Restricted-Environment Readiness	Engineering Quality	Overall
6 / 17	16 / 16	5 / 20	10 / 10	59 / 100

Drilldown

Track: Utility (VNC remote-desktop server; not an AI agent or code-editor IDE)

Presentation & Onboarding — 6 / 17

Criterion	Max	Score	Notes
Configuration-mode examples	12	half	Only one minimal example is shown (xfce + subdomain). Many options exist (`desktop_environment`, `share`, `port`, `subdomain=false` path-sharing mode) but no examples cover alternate modes like `share="public"`, path-based access (`subdomain=false`), or other desktop environments.
Visual preview	5	0	README contains only an icon reference (`icon: ../../../../.icons/kasmvnc.svg`); no embedded image, GIF, or video of the running desktop.

Credential Hygiene — 16 / 16 (Non-hardcoded auth path N/A, 4 pts excluded)

Criterion	Max	Score	Notes
Secrets marked sensitive	16	full	Module has no secret/API-key inputs; README example contains no inline credentials (VNC password is generated internally, not exposed).
Non-hardcoded auth path	4	N/A	No external auth/credential flow exists by construction — the module only starts a local VNC server behind the Coder tunnel, so this concern doesn't apply.

Restricted-Environment Readiness — 5 / 20

Criterion	Max	Score	Notes
Mirrorable artifact source	5	0	`base_url="https://github.com/kasmtech/KasmVNC/releases/download/v${KASM_VERSION}"` in `run.sh` is hardcoded; no module variable overrides this download URL.
Bring-your-own binary	10	half	`check_installed()` skips install if `kasmvncserver` is already on PATH, enabling a baked-image flow, but this behavior is undocumented in the README — only discoverable in source.
Egress transparency	3	0	No dedicated network/offline section; the only download endpoint (GitHub releases) appears solely in script source, not documented.
Runs without sudo	2	0	Script hard-fails without sudo: `"ERROR: sudo NOPASSWD access required!"` is raised when installing via apt/rpm/apk — core install functionality requires root with no working fallback.

Engineering Quality — 10 / 10

Criterion	Max	Score	Notes
Input quality	6	full	All variables have descriptions; `desktop_environment` and `share` have `validation` blocks with clear error messages; sensible defaults (`port=6800`, `subdomain=true`, `share="owner"`).
Test coverage	4	full	`main.test.ts` has extensive `.tftest`-style business-logic coverage via `testRequiredVariables`, `runTerraformApply` across all desktop environments, and a large matrix of package-naming logic tests (11+ distro/version cases) plus version-format validation tests.

Overall — 59 / 100

Raw 37 / 63 → round(37 / 63 × 100) = 59

Tip

You can run this locally by telling your agent: "review this module against .github/scorecard/SCORECARD.md".

Scored against SCORECARD.md with solstice-1. Language-model scores are advisory.

mthemis-provenir added 3 commits October 28, 2026 23:02

matifali requested a balanced review from Copilot October 31, 2026 19:23

Copilot started reviewing on behalf of matifali October 31, 2026 19:24

View session

matifali self-requested a review October 31, 2026 19:24

Copilot AI reviewed Oct 31, 2026

View reviewed changes

Comment thread registry/coder/modules/kasmvnc/run.sh Resolved

Comment thread registry/coder/modules/kasmvnc/main.test.ts Resolved

Comment thread registry/coder/modules/kasmvnc/main.test.ts Resolved

Comment thread registry/coder/modules/kasmvnc/run.sh Resolved

mthemis-provenir added 3 commits November 2, 2026 10:58

chore(coder/kasmvnc): bump module version to 1.3.1

71281ac

Installation behaviour changed, so bump the patch version and the README reference with .github/scripts/version-bump.sh patch.

matifali requested a review from Edd88-pixel November 2, 2026 11:28

Edd88-pixel self-assigned this Nov 2, 2026

Edd88-pixel approved these changes Nov 3, 2026

View reviewed changes

matifali approved these changes Nov 3, 2026

View reviewed changes

matifali enabled auto-merge (squash) November 3, 2026 14:28

Navigation Menu

Repository navigation

fix(coder/kasmvnc): use the release asset names KasmVNC publishes - #1217

fix(coder/kasmvnc): use the release asset names KasmVNC publishes#1217
mthemis-provenir wants to merge 6 commits into
coder:mainfrom
mthemis-provenir:fix/kasmvnc-release-asset-names

mthemis-provenir commented Oct 28, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mthemis-provenir commented Nov 2, 2026

Uh oh!

matifali commented Nov 2, 2026

Uh oh!

matifali commented Nov 2, 2026

Uh oh!

Edd88-pixel left a comment

Uh oh!

github-actions Bot commented Nov 3, 2026

Presentation & Onboarding — 6 / 17

Credential Hygiene — 16 / 16 (Non-hardcoded auth path N/A, 4 pts excluded)

Restricted-Environment Readiness — 5 / 20

Engineering Quality — 10 / 10

Overall — 59 / 100

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

mthemis-provenir commented Oct 28, 2026

Summary

What changed upstream

Changes

Verification

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mthemis-provenir commented Nov 2, 2026

Uh oh!

matifali commented Nov 2, 2026

Uh oh!

matifali commented Nov 2, 2026

Uh oh!

Edd88-pixel left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Nov 3, 2026

Module Scorecard Check

coder/kasmvnc: 55 → 59

Presentation & Onboarding — 6 / 17

Credential Hygiene — 16 / 16 (Non-hardcoded auth path N/A, 4 pts excluded)

Restricted-Environment Readiness — 5 / 20

Engineering Quality — 10 / 10

Overall — 59 / 100

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

`coder/kasmvnc`: 55 → 59