Skip to content

Conversation

@netsgnut
Copy link
Contributor

Description

This PR makes the following changes to the coder/modules/kasmvnc:

  • Change the installation check from checking vncserver to kasmvncserver.
  • Bump the default KasmVNC installation version to 1.4.0.

In images where there is already TightVNC installed, the current installation check will erroneously report that KasmVNC is already installed. By checking kasmvncserver instead, it ensures KasmVNC is installed.

Tested on Debian, Kali and Alpine-based images.

Type of Change

  • New module
  • New template
  • Bug fix
  • Feature/enhancement
  • Documentation
  • Other

Module Information

Path: registry/coder/modules/kasmvnc
New version: v1.2.5
Breaking change: [ ] Yes [X] No

Testing & Validation

  • Tests pass (bun test)
  • Code formatted (bun fmt)
  • Changes tested locally

Related Issues

None

@netsgnut
Copy link
Contributor Author

I revisited this code just now, and now that I think about it, apart from using the existence of kasmvncserver to check if KasmVNC is installed, the invocation of vncpasswd and vncserver should be changed to kasmvncpasswd and kasmvncserver respectively. The rationale is that in images where multiple VNC services exist, and even KasmVNC is not the one currently aliased to vnc{passwd,server}, it should still work as intended.

This is addressed with commit 78a878b.

@DevelopmentCats
Copy link
Contributor

I revisited this code just now, and now that I think about it, apart from using the existence of kasmvncserver to check if KasmVNC is installed, the invocation of vncpasswd and vncserver should be changed to kasmvncpasswd and kasmvncserver respectively. The rationale is that in images where multiple VNC services exist, and even KasmVNC is not the one currently aliased to vnc{passwd,server}, it should still work as intended.

This is addressed with commit 78a878b.

Ill go ahead and test this out now :) thanks for the update

@DevelopmentCats
Copy link
Contributor

Everything looks good to me!

@DevelopmentCats DevelopmentCats enabled auto-merge (squash) October 24, 2025 17:05
@DevelopmentCats
Copy link
Contributor

@netsgnut can you pull the latest branch changes, and then we can commit this.

For some reason its not letting me trigger the merge like it would normally

@netsgnut
Copy link
Contributor Author

@DevelopmentCats Sure! I have merged main onto this branch.

@DevelopmentCats DevelopmentCats merged commit a327e79 into coder:main Oct 24, 2025
4 checks passed
@netsgnut netsgnut deleted the fix/kasmvnc/patch-1 branch October 24, 2025 17:48
@netsgnut
Copy link
Contributor Author

Thanks!

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.

2 participants