fix(kasmvnc): optimize KasmVNC deployment script

[djarbz]

I took the opportunity to majorly refactor the run.sh script.

Basically a rewrite from the ground up, and I found a few things that I don't think were tested for non-Ubuntu distros.

I updated the Kasm Version reference to avoid confusion in the script.

modules/kasmvnc/main.tf

Lines 42 to 46 in a8cc861

	script = templatefile("${path.module}/run.sh", {
	PORT : var.port,
	DESKTOP_ENVIRONMENT : var.desktop_environment,
	KASM_VERSION : var.kasm_version
	})

Removed the extra shebang and added an early exit signal in case of failure.

modules/kasmvnc/run.sh

Lines 1 to 4 in a8cc861

	#!/usr/bin/env bash
	# Exit on error, undefined variables, and pipe failures
	set -euo pipefail

Optimize the download function to set the command and then one block of logic to perform the download and handle a failure.

modules/kasmvnc/run.sh

Lines 16 to 37 in a8cc861

	# Function to download a file using wget, curl, or busybox as a fallback
	download_file() {
	local url="$1"
	local output="$2"
	local download_tool
	if command -v curl &> /dev/null; then
	download_tool="curl -fsSL"
	elif command -v wget &> /dev/null; then
	download_tool="wget -q -O-"
	elif command -v busybox &> /dev/null; then
	download_tool="busybox wget -O-"
	else
	echo "ERROR: No download tool available (curl, wget, or busybox required)"
	exit 1
	fi
	$download_tool "$url" > "$output" || {
	echo "ERROR: Failed to download $url"
	exit 1
	}
	}

Created a cross platform function to add a user to a group based on which command is available.

modules/kasmvnc/run.sh

Lines 39 to 52 in a8cc861

	# Add user to group using available commands
	add_user_to_group() {
	local user="$1"
	local group="$2"
	if command -v usermod &> /dev/null; then
	sudo usermod -aG "$group" "$user"
	elif command -v adduser &> /dev/null; then
	sudo adduser "$user" "$group"
	else
	echo "ERROR: At least one of 'adduser'(Debian) 'usermod'(RHEL) is required"
	exit 1
	fi
	}

Refactored the install_deb function to dynamically update the (stale) package cache and wait for a lock.

modules/kasmvnc/run.sh

Lines 54 to 69 in a8cc861

	# Function to install kasmvncserver for debian-based distros
	install_deb() {
	local url=$1
	download_file "$url" /tmp/kasmvncserver.deb
	# Define the directory to check
	CACHE_DIR="/var/lib/apt/lists/partial"
	# Check if the directory exists and was modified in the last 60 minutes
	if [ ! -d "$CACHE_DIR" ] || ! find "$CACHE_DIR" -mmin -60 -print -quit &>/dev/null; then
	echo "Stale Package Cache, updating..."
	# Update package cache with a 300-second timeout for dpkg lock
	sudo apt-get -o DPkg::Lock::Timeout=300 -qq update
	fi
	DEBIAN_FRONTEND=noninteractive sudo apt-get -o DPkg::Lock::Timeout=300 install --yes -qq --no-install-recommends --no-install-suggests /tmp/kasmvncserver.deb
	add_user_to_group "$USER" ssl-cert
	rm /tmp/kasmvncserver.deb
	}

Check and fail early if sudo or /etc/os-release are not available.

Source /etc/os-release since it is formatted as a .env file.
Also, adjust the distro and version for Oracle.
Note we are using distro_version to make it more readable vs just version.

modules/kasmvnc/run.sh

Lines 100 to 108 in a8cc861

	source /etc/os-release
	distro="$ID"
	distro_version="$VERSION_ID"
	codename="$VERSION_CODENAME"
	arch="$(uname -m)"
	if [[ "$ID" == "ol" ]]; then
	distro="oracle"
	distro_version="$${distro_version%%.*}"
	fi

Cleaned up the ARCH mappings to make it more readable.

modules/kasmvnc/run.sh

Lines 115 to 127 in a8cc861

	# Map arch to package arch
	case "$arch" in
	x86_64)
	[[ "$distro" =~ ^(ubuntu|debian|kali)$ ]] && arch="amd64" || arch="x86_64"
	;;
	aarch64|arm64)
	[[ "$distro" =~ ^(ubuntu|debian|kali)$ ]] && arch="arm64" || arch="aarch64"
	;;
	*)
	echo "ERROR: Unsupported architecture: $arch"
	exit 1
	;;
	esac

For installation, set a base URL that is the same for all distros.
Cleaned up the cases and merged ones that could be merged.
Reduced checking for specific distro versions, we want this to be compatible with future versions without modifying the script each time. If KASM doesn't support the version, then it will fail during the attempt to download.

modules/kasmvnc/run.sh

Lines 129 to 154 in a8cc861

	# Check if vncserver is installed, and install if not
	if ! check_installed; then
	base_url="https://github.com/kasmtech/KasmVNC/releases/download/v${KASM_VERSION}"
	echo "Installing KASM version: ${KASM_VERSION}"
	case $distro in
	ubuntu | debian | kali)
	bin_name="kasmvncserver_$${codename}_${KASM_VERSION}_$${arch}.deb"
	install_deb "$base_url/$bin_name"
	;;
	oracle | fedora | opensuse)
	bin_name="kasmvncserver_$${distro}_$${distro_version}_${KASM_VERSION}_$${arch}.rpm"
	install_rpm "$base_url/$bin_name"
	;;
	alpine)
	bin_name="kasmvnc.alpine_$${distro_version//./}_$${arch}.tgz"
	install_alpine "$base_url/$bin_name"
	;;
	*)
	echo "Unsupported distribution: $distro"
	exit 1
	;;
	esac
	else
	echo "vncserver already installed. Skipping installation."
	fi

Finally, added a check to verify that the VNC server was actually running.

modules/kasmvnc/run.sh

Lines 177 to 182 in a8cc861

	# Wait for server to start
	sleep 5
	if ! pgrep -f vncserver > /dev/null; then
	echo "ERROR: Failed to start KasmVNC server. Check logs at /tmp/kasmvncserver.log"
	exit 1
	fi

Also closes #327

[matifali]

The refactor LGTM. And yes we did not tested enough for all non ubuntu distros.

[djarbz]

@matifali I have pushed a few changes and just tested locally.
We now only need sudo if we are installing kasm.
We are running KasmVNC as the coder user.

Still need to test with non-Ubuntu images, but I don't have any readily available.
Will also need to test if this works for @bpmct and his issue with official KasmWeb images.

[matifali]

@djarbz I just tested with kasmweb/postman:1.16.0,, and VNC starts, although I do not see a postman icon on the Desktop.

[djarbz]

@djarbz I just tested with kasmweb/postman:1.16.0,, and VNC starts, although I do not see a postman icon on the Desktop.

Is it available in the application menu?
The $HOME directory might be overridden or somehow masked, or maybe it never existed on the desktop?

[djarbz]

@matifali Can you share your template?
I just tried with a minimal template and I get stuck on the dashboard loading.

[mafredri]

Thanks for putting so much effort into this refactor, much appreciated. Left some suggestions and questions.

[mafredri]

Thanks for making all the amends and thinking of alternative solutions, this is starting to look good, just a few more issues remain.

[mafredri]

We probably need both install_rpm and install_dnf. I just tried the opensuse/leap image and it has neither dnf nor yum, only zypper and rpm.

I'm not sure if this is a script meant for installing the kasmvnc built package or something else, but this at least gives hints for the expectation per distro: https://github.com/kasmtech/KasmVNC/blob/3a8517d7dc461eaccc7ed8e3d3b155e233426fc8/builder/scripts/common.sh#L22-L29

[djarbz]

Perhaps in the install_rpm function, we have a switch case to select based on what tooling is available?
Similar to the download function?

Priority:

1. dnf
2. zypper
3. yum
4. rpm

[mafredri]

Doesn't look like arm64 case in aarch64 | arm64) is ever applicable:

❯ for container in ubuntu debian kalilinux/kali-rolling alpine fedora opensuse/leap container-registry.oracle.com/os/oraclelinux:9-slim; do for plat in linux/arm64 linux/amd64; do docker run -it --rm --platform $plat $container /bin/sh -c 'echo $(grep ^NAME= /etc/os-release): $(uname -m)'; done; done 2>/dev/null
NAME="Ubuntu": aarch64
NAME="Ubuntu": x86_64
NAME="Debian GNU/Linux": aarch64
NAME="Debian GNU/Linux": x86_64
NAME="Kali GNU/Linux": aarch64
NAME="Kali GNU/Linux": x86_64
NAME="Alpine Linux": aarch64
NAME="Alpine Linux": x86_64
NAME="Fedora Linux": aarch64
NAME="Fedora Linux": x86_64
NAME="openSUSE Leap": aarch64
NAME="openSUSE Leap": x86_64
NAME="Oracle Linux Server": aarch64
NAME="Oracle Linux Server": x86_64

So we can essentially use the same logic as above for x86_64, default=aarch64, for debian based, use arm64 instead (verified from kasm releases that all others are aarch64).

[mafredri]

I just realized we're unconditionally overwriting the users kasmvnc configuration here.

We should switch back to writing this to /etc since it will still apply to the user. That'll allow the user to provide their own settings in dotfiles.

KasmVNC is configured via YAML based configurations. The server level configuration is at /etc/kasmvnc/kasmvnc.yaml. Edits to this file apply to all users. Individual users can override server global configurations by specifying them in their configuration file at ~/.vnc/kasmvnc.yaml.

(emphasis mine.)

[djarbz]

Regarding arch mapping.

Is this what you are thinking?

case "$arch" in
  x86_64)
    if [[ "$distro" =~ ^(ubuntu|debian|kali)$ ]]; then
      arch="amd64"
    fi
    ;;
  aarch64)
    if [[ "$distro" =~ ^(ubuntu|debian|kali)$ ]]; then
      arch="arm64"
    fi
    ;;
  arm64)
    :  # This is effectively a noop
    ;;
  *)
    echo "ERROR: Unsupported architecture: $arch"
    exit 1
    ;;
esac

[djarbz]

My concern with the /etc kasmvnc.yaml is the requirement for sudo and when using the official KasmWeb images, they have some settings preconfigured.

We could create a check for if sudo is available or not and handle differently.
Pseudo-code

if sudo
  if /etc/kasmvnc/kasmvnc.yaml exists
    Merge yaml with our config taking precedence?
  else
    write our config
else
  Merge yaml with our config taking precedence?

I'm thinking we need to enforce our config as it is what is needed for KasmVNC to work with coder.

[mafredri]

Regarding arch mapping.

Yes, except in my testing the noop for arm64 isn't necessary at all.

My concern with the /etc kasmvnc.yaml is the requirement for sudo and when using the official KasmWeb images, they have some settings preconfigured.

In what situations can KasmVNC be installed if sudo isn't available? I suppose KasmWeb images are the only case since it's preinstalled?

I think explicit overwrite of config in /etc is fine, they're using our module for a reason. Down the line we may want to think about introducing more KasmVNC settings and that means we again want to overwrite the yaml again. Merging yaml seems like a cumbersome process requiring additional tools for manipulation.

If we fallback to writing the config in user home, we should check it's presence and only write if it's missing. If we detect an existing config we can print a warning that if it doesn't work, consider deleting the file and restarting the workspace.

[djarbz]

Yes, except in my testing the noop for arm64 isn't necessary at all.
I'm just thinking of future, don't want to fail out if it is reported as such?

If we use a KasmWeb image, we do not have sudo access.
This means that we cannot write to the /etc config file and must write to the home directory.
I think we should default to /etc if sudo is available, but fallback to /home with a warning message in case the user is trying to figure out why their /home config might not be taking effect.

[mafredri]

If we use a KasmWeb image, we do not have sudo access.

Yeah, makes sense.

warning message in case the user is trying to figure out why their /home config might not be taking effect

Do you mean overwriting + warning or not overwriting + warning here? I'm still of the opinion that overwrite is not acceptable in user home due to the sheer amount of configuration options that are available. Wouldn't want to get in the way of what they want to do or to destroy all their work: https://www.kasmweb.com/kasmvnc/docs/latest/configuration.html

[djarbz]

I have added a commit to handle this with /etc as default and a warning if we need to fallback to /home for the config.

[mafredri]

Awesome work, thanks! I had one last question but short of validating the script on all platforms, I have nothing more to add, approved!

[mafredri]

Do we need sudo mkdir -p /etc/kasmvnc here like for the user config?

[djarbz]

I have not run into the need for it.
I can add it as a catchall just in case if you want?

[mafredri]

Looks like package-lock was accidentally included in this PR, could you remove it?