diff --git a/CHANGELOG.md b/CHANGELOG.md index beb403c..d9009df 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,9 +8,19 @@ Versions follow [Semantic Versioning](https://semver.org/). ## [Unreleased] -### Added +### Fixed - VM IP lookup now accepts the native QEMU Guest Agent list payload from `qm agent ... network-get-interfaces` as well as the earlier wrapped shape, preventing parser errors on hosts that return a bare JSON array. +- Snapshot name validation now matches Proxmox `pve-configid` rules before calling `pct` / `qm`; + numeric-only and single-letter names are rejected locally with a clearer message. +- SPICE `.vv` files now prefer the actual SPICE bind address/port from Proxmox instead of always + using the host LAN IP, preventing mismatched connection targets on loopback-bound SPICE setups. +- `spice_info()` no longer claims success launching `virt-viewer` from a non-graphical shell; it + falls back to the `.vv` file with a clear desktop-session hint. +- VM console guidance now tells the user to follow the escape hint printed by Proxmox instead of + hardcoding a potentially wrong key sequence. + +### Added - `--filter STATUS` flag: filter `--list` / `--json` output to `running`, `stopped`, or `paused` instances; invalid values exit 1 with a clear error message. - `--timeout SECS` flag: set a timeout (default 60 s) for stop operations; on exit code 124 diff --git a/proxmox-manager.sh b/proxmox-manager.sh index eef973c..c181f89 100755 --- a/proxmox-manager.sh +++ b/proxmox-manager.sh @@ -192,12 +192,13 @@ validate_vmid() { } # validate_snapshot_name NAME — reject names Proxmox would refuse. -# Valid: starts with alphanumeric, only [a-zA-Z0-9_-], max 40 chars. +# Proxmox uses the pve-configid format for snapshot names: +# start with a letter, then one or more of [a-zA-Z0-9_-], max 40 chars. validate_snapshot_name() { local sn="$1" - if [[ ! "$sn" =~ ^[a-zA-Z0-9][a-zA-Z0-9_-]{0,39}$ ]]; then + if [[ ! "$sn" =~ ^[a-zA-Z][a-zA-Z0-9_-]{1,39}$ ]]; then err "Invalid snapshot name '$sn'." - note "Name must start with a letter or digit, contain only [a-zA-Z0-9_-], and be at most 40 characters." + note "Name must start with a letter, contain only [a-zA-Z0-9_-], and be at most 40 characters." return 1 fi return 0 @@ -1080,7 +1081,7 @@ open_console() { err "VM $id is not running. Start it first." return fi - note "Press Ctrl+] to exit the VM terminal." + note "If the terminal opens, use the escape hint shown by Proxmox to exit." if qm terminal "$id" 2>/dev/null; then : else @@ -1292,18 +1293,46 @@ snapshots_menu() { # SPICE # ============================================================================= -spice_info() { - local id="$1" name="$2" - local host port - host="$(hostname -I 2>/dev/null | awk '{print $1}')" - port="$(qm monitor "$id" <<<"info spice" 2>/dev/null | - awk '/port/ {for(i=1;i<=NF;i++) if($i ~ /^[0-9]+$/){print $i; exit}}')" - [[ -z "$port" ]] && port="$(grep -E "(spice).*port" "/var/log/qemu-server/${id}.log" 2>/dev/null | - tail -1 | sed -n 's/.*port=\([0-9]\+\).*/\1/p' || true)" - local id_int +_spice_has_gui_session() { + [[ -n "${DISPLAY:-}" ]] || [[ -n "${WAYLAND_DISPLAY:-}" && -n "${XDG_RUNTIME_DIR:-}" ]] +} + +_spice_endpoint() { + local id="$1" + local monitor endpoint host='' port='' cfg_spice cfg_addr id_int + + monitor="$(qm monitor "$id" <<<'info spice' 2>/dev/null || true)" + endpoint="$(sed -n 's/^[[:space:]]*address:[[:space:]]*\([^[:space:]]*\).*/\1/p' <<<"$monitor" | head -1)" + if [[ -n "$endpoint" ]]; then + if [[ "$endpoint" == \[*\]:* ]]; then + host="${endpoint%%]:*}" + host="${host#[}" + port="${endpoint##*:}" + elif [[ "$endpoint" == *:* ]]; then + host="${endpoint%:*}" + port="${endpoint##*:}" + fi + fi + + cfg_spice="$(qm config "$id" 2>/dev/null | sed -n 's/^spice: .*port=\([0-9]\+\).*/\1/p' | head -1)" + cfg_addr="$(qm config "$id" 2>/dev/null | sed -n 's/^spice: .*addr=\([^,]*\).*/\1/p' | head -1)" + [[ -z "$port" && -n "$cfg_spice" ]] && port="$cfg_spice" + [[ -z "$host" && -n "$cfg_addr" ]] && host="$cfg_addr" + + [[ -z "$host" ]] && host="${PROXMOX_MANAGER_SPICE_ADDR:-$(hostname -I 2>/dev/null | awk '{print $1}')}" id_int=$((10#$id)) [[ -z "$port" ]] && port="$((61000 + id_int))" + printf '%s\t%s\n' "$host" "$port" +} + +spice_info() { + local id="$1" name="$2" + local host port endpoint + endpoint="$(_spice_endpoint "$id")" + host="${endpoint%%$'\t'*}" + port="${endpoint##*$'\t'}" + printf ' %bSPICE:%b spice://%s:%s\n' "${BOLD}${CYAN_BRIGHT}" "${NC}" "$host" "$port" umask 077 local vv @@ -1321,12 +1350,16 @@ title=VM ${id} (${name}) delete-this-file=1 fullscreen=0 EOF - if have virt-viewer; then + if have virt-viewer && _spice_has_gui_session; then virt-viewer "$vv" & ok "Launching virt-viewer for VM ${id}..." else ok "SPICE connection file: ${vv}" - note "Install virt-viewer with: apt install virt-viewer" + if have virt-viewer; then + note "No graphical session detected in this shell. Open the .vv file from a desktop session." + else + note "Install virt-viewer with: apt install virt-viewer" + fi fi } diff --git a/tests/mock-bin/hostname b/tests/mock-bin/hostname new file mode 100755 index 0000000..b05b485 --- /dev/null +++ b/tests/mock-bin/hostname @@ -0,0 +1,7 @@ +#!/usr/bin/env bash +set -euo pipefail +if [[ "${1:-}" == "-I" ]]; then + echo "192.168.178.222 10.0.0.1" +else + echo "mock-host" +fi diff --git a/tests/mock-bin/qm b/tests/mock-bin/qm index 4deacd6..c9deece 100755 --- a/tests/mock-bin/qm +++ b/tests/mock-bin/qm @@ -19,8 +19,29 @@ EOL EOL fi ;; + monitor) + cat <<'EOL' +qm> info spice +qm> Entering QEMU Monitor for VM 200 - type 'help' for help +Server: + address: 127.0.0.1:61000 [tls] + migrated: false + auth: spice + compiled: 0.15.2 + mouse-mode: client +Channels: none +EOL + ;; config) case "${1:-}" in + 200) + cat <<'EOL' +agent: enabled=1 +name: vm-one +spice: port=61000,addr=127.0.0.1 +vga: qxl +EOL + ;; 201) echo "name: vm-fallback" ;; *) echo "name: vm-${1:-unknown}" ;; esac diff --git a/tests/mock-bin/virt-viewer b/tests/mock-bin/virt-viewer new file mode 100755 index 0000000..f687d97 --- /dev/null +++ b/tests/mock-bin/virt-viewer @@ -0,0 +1,4 @@ +#!/usr/bin/env bash +set -euo pipefail +: "${HERMES_VIRT_VIEWER_MARKER:=/tmp/hermes-virt-viewer-called}" +printf '%s\n' "$*" >> "$HERMES_VIRT_VIEWER_MARKER" diff --git a/tests/run.sh b/tests/run.sh index 4cc5a26..8096cc3 100755 --- a/tests/run.sh +++ b/tests/run.sh @@ -153,6 +153,8 @@ _snap_test "my-snap_2" 0 "with hyphen and underscore" _snap_test "$(printf 'a%.0s' {1..40})" 0 "exactly 40 chars (max valid)" # Invalid names — expect exit 1 +_snap_test "1" 1 "numeric-only name" +_snap_test "a" 1 "single-letter name" _snap_test "_snap" 1 "starts with underscore" _snap_test "-bad" 1 "starts with hyphen" _snap_test "snap name" 1 "contains space" @@ -178,6 +180,33 @@ else _fail "ip_info: CT did not return expected IPv4 address" fi +# --------------------------------------------------------------------------- +# Unit tests: spice_info() +# --------------------------------------------------------------------------- +rm -f /tmp/hermes-virt-viewer-called +unset DISPLAY WAYLAND_DISPLAY XDG_RUNTIME_DIR +_spice_exit=0 +_spice_out="$(spice_info 200 vm-one 2>&1)" || _spice_exit=$? +_spice_vv="$(printf '%s\n' "$_spice_out" | sed -n 's/.*SPICE connection file: //p' | tail -1)" +if [[ "$_spice_exit" == "0" ]] && printf '%s\n' "$_spice_out" | grep -q 'spice://127.0.0.1:61000'; then + _pass "spice_info: URI uses monitor/config host and port" +else + _fail "spice_info: URI did not use expected host/port" +fi + +if [[ -n "$_spice_vv" ]] && [[ -f "$_spice_vv" ]] && grep -q '^host=127.0.0.1$' "$_spice_vv"; then + _pass "spice_info: .vv file uses actual SPICE bind host" +else + _fail "spice_info: .vv file did not use expected host" +fi + +if [[ -n "$_spice_vv" ]]; then rm -f "$_spice_vv"; fi +if [[ ! -e /tmp/hermes-virt-viewer-called ]]; then + _pass "spice_info: does not auto-launch virt-viewer without GUI session" +else + _fail "spice_info: auto-launched virt-viewer without GUI session" +fi + # --------------------------------------------------------------------------- # Tests: --filter flag # ---------------------------------------------------------------------------