diff --git a/README.md b/README.md index a2de0eb..2459ad8 100644 --- a/README.md +++ b/README.md @@ -94,3 +94,17 @@ Maintenance evacuation leaves HA-managed guests under Proxmox HA control. Remaining shared-storage guests are routed to online, non-maintenance nodes with the required storage, while local-storage guests are gracefully shut down. HA node-affinity preferences are honored when an eligible node exists. + +## Development checks + +Run the local validation suite with Bash 4.3 or newer: + +```bash +./tests/run.sh +``` + +The suite checks Bash syntax and Git whitespace, runs ShellCheck when it is +installed, and exercises Git update states, LXC input/storage selection, +maintenance evacuation routing, HA affinity parsing, and VirtIO filename +validation. Tests use temporary files and mocked Proxmox output; they do not +download installers or change a Proxmox host. diff --git a/defaults.inc b/defaults.inc index f27ad12..3cc0123 100755 --- a/defaults.inc +++ b/defaults.inc @@ -3,7 +3,7 @@ action="${1:-}" FOLDER='/opt/idssys/ta-proxmenu' -VERS='2026.7.25-44' +VERS='2026.7.25-45' noupdate=' ' diff --git a/inc/deploy-iso-nfs-lxc.sh b/inc/deploy-iso-nfs-lxc.sh index 71d914f..00bee4f 100644 --- a/inc/deploy-iso-nfs-lxc.sh +++ b/inc/deploy-iso-nfs-lxc.sh @@ -9,6 +9,10 @@ TAPM_ISO_NFS_VALID_CTID() { [[ "${1:-}" =~ ^[1-9][0-9]{2,8}$ ]] } +TAPM_ISO_NFS_VALID_HOSTNAME() { + [[ "${1:-}" =~ ^[A-Za-z0-9][A-Za-z0-9.-]{0,62}$ ]] +} + TAPM_ISO_NFS_VALID_IPV4_CIDR() { local value="${1:-}" local address prefix octet @@ -126,7 +130,7 @@ TAPM_DEPLOY_ISO_NFS_LXC() { fi TAPM_ISO_NFS_PROMPT hostname "Container hostname" "PVE-Shared-Storage" - [[ "$hostname" =~ ^[A-Za-z0-9][A-Za-z0-9.-]{0,62}$ ]] || + TAPM_ISO_NFS_VALID_HOSTNAME "$hostname" || { TAPM_ISO_NFS_FAIL "The hostname is invalid."; return 1; } TAPM_ISO_NFS_PROMPT address_cidr "Static IPv4 address with prefix (example: 10.20.30.10/24)" TAPM_ISO_NFS_VALID_IPV4_CIDR "$address_cidr" || diff --git a/inc/deploy-proxmox-keepalived.sh b/inc/deploy-proxmox-keepalived.sh index 389d0be..65d427f 100644 --- a/inc/deploy-proxmox-keepalived.sh +++ b/inc/deploy-proxmox-keepalived.sh @@ -345,6 +345,7 @@ remote_exec() { if [[ "$node" == "$LOCAL_NODE" ]]; then bash -lc "$cmd" else + # shellcheck disable=SC2029 # cmd is intentionally expanded into the remote command. ssh "${SSH_OPTS[@]}" "root@${node}" "$cmd" fi } diff --git a/inc/virtio-helpers.inc b/inc/virtio-helpers.inc new file mode 100644 index 0000000..ce04a1b --- /dev/null +++ b/inc/virtio-helpers.inc @@ -0,0 +1,25 @@ +#!/usr/bin/env bash +# Pure VirtIO filename helpers shared by TA-ProxMenu and its tests. + +TAPM_VIRTIO_FILENAME_FROM_URL() { + local url="${1%%\?*}" + local filename="${url##*/}" + + [[ "$filename" =~ ^virtio-win(-0\.1\.[0-9]+)?\.iso$ ]] || return 1 + printf '%s\n' "$filename" +} + +TAPM_VIRTIO_LABELED_FILENAME() { + local source_filename="$1" + local label="$2" + local version='' + + [[ "$label" =~ ^[a-z0-9-]+$ ]] || return 1 + if [[ "$source_filename" =~ ^virtio-win-(0\.1\.[0-9]+)\.iso$ ]]; then + version="${BASH_REMATCH[1]}" + elif [[ "$source_filename" != 'virtio-win.iso' ]]; then + return 1 + fi + + printf 'virtio-win-%s%s.iso\n' "$label" "${version:+-${version}}" +} diff --git a/proxmenu-scripts.sh b/proxmenu-scripts.sh index db1ad20..4907e3f 100755 --- a/proxmenu-scripts.sh +++ b/proxmenu-scripts.sh @@ -7,6 +7,7 @@ source /opt/idssys/defaults/default.inc source /opt/idssys/ta-proxmenu/defaults.inc source /opt/idssys/ta-proxmenu/inc/git-update.inc source /opt/idssys/ta-proxmenu/inc/deploy-iso-nfs-lxc.sh +source /opt/idssys/ta-proxmenu/inc/virtio-helpers.inc ACTION_REQUESTED=0 [[ -n "${action:-}" ]] && ACTION_REQUESTED=1 @@ -791,29 +792,6 @@ declare -a VIRTIO_STORAGE_IDS=() declare -a VIRTIO_STORAGE_TYPES=() declare -a VIRTIO_STORAGE_DIRS=() -TAPM_VIRTIO_FILENAME_FROM_URL() { - local url="${1%%\?*}" - local filename="${url##*/}" - - [[ "$filename" =~ ^virtio-win(-0\.1\.[0-9]+)?\.iso$ ]] || return 1 - printf '%s\n' "$filename" -} - -TAPM_VIRTIO_LABELED_FILENAME() { - local source_filename="$1" - local label="$2" - local version='' - - [[ "$label" =~ ^[a-z0-9-]+$ ]] || return 1 - if [[ "$source_filename" =~ ^virtio-win-(0\.1\.[0-9]+)\.iso$ ]]; then - version="${BASH_REMATCH[1]}" - elif [[ "$source_filename" != 'virtio-win.iso' ]]; then - return 1 - fi - - printf 'virtio-win-%s%s.iso\n' "$label" "${version:+-${version}}" -} - TAPM_REFRESH_ISO_STORAGES() { local storage local type @@ -1398,7 +1376,7 @@ RESTART_SERVICE_GROUP() { choice="y" else echo -en "${idsCL[LightCyan]}Restart ${description} on this host (Y/n)?${idsCL[Default]} " - read -n 1 choice + read -r -n 1 choice echo fi [[ "$choice" =~ ^[Nn]$ ]] && return @@ -1443,7 +1421,7 @@ RESTART_CLUSTER_FILESYSTEM() { fi echo -en "\n${idsCL[LightCyan]}Restart pve-cluster on this host (y/N)?${idsCL[Default]} " - read -n 1 choice + read -r -n 1 choice echo [[ "$choice" =~ ^[Yy]$ ]] || return @@ -2019,7 +1997,7 @@ SWITCH_SCRIPT_BRANCH() { fi echo -en "\n${idsCL[LightCyan]}Switch TA-ProxMenu from '${current_branch}' to '${target_branch}' (y/N)?${idsCL[Default]} " - read -n 1 choice + read -r -n 1 choice echo [[ "$choice" =~ ^[Yy]$ ]] || return @@ -2181,7 +2159,7 @@ UTILITIES_MENU() { continue fi echo -en "\n${idsCL[LightCyan]}Install the update for the current branch (y/N)?${idsCL[Default]} " - read -n 1 choice + read -r -n 1 choice echo if [[ "$choice" =~ ^[Yy]$ ]] && /opt/idssys/ta-proxmenu/run.sh update; then @@ -2229,7 +2207,13 @@ if (( ACTION_REQUESTED == 1 )); then glances) INSTALL_GLANCES;; acronis) INSTALL_ACRONIS;; post-install|post_install) PROXMENUX_POST_INSTALL;; - proxmenux) [ ! -f /usr/local/bin/menu ] && INSTALL_PROXMENUX || /usr/local/bin/menu;; + proxmenux) + if [[ ! -f /usr/local/bin/menu ]]; then + INSTALL_PROXMENUX + else + /usr/local/bin/menu + fi + ;; virtio) VIRTIO_MENU;; sentinelone|s1) INSTALL_S1;; screenconnect) INSTALL_SCREENCONNECT;; diff --git a/tests/run.sh b/tests/run.sh new file mode 100755 index 0000000..ce2aa4e --- /dev/null +++ b/tests/run.sh @@ -0,0 +1,83 @@ +#!/usr/bin/env bash +set -u -o pipefail + +TEST_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +declare -a shell_files=() +declare -a test_files=() +failures=0 +required_command='' + +if (( BASH_VERSINFO[0] < 4 || + (BASH_VERSINFO[0] == 4 && BASH_VERSINFO[1] < 3) )); then + printf 'TA-ProxMenu tests require Bash 4.3 or newer; found %s.\n' \ + "$BASH_VERSION" >&2 + exit 1 +fi + +for required_command in git python3; do + if ! command -v "$required_command" >/dev/null 2>&1; then + printf 'TA-ProxMenu tests require %s.\n' "$required_command" >&2 + exit 1 + fi +done + +while IFS= read -r -d '' file; do + shell_files+=("$file") +done < <( + find "$TEST_ROOT" -type f \( -name '*.sh' -o -name '*.inc' \) \ + -not -path '*/.git/*' -print0 | + sort -z +) + +printf 'Bash syntax\n' +for file in "${shell_files[@]}"; do + if ! bash -n "$file"; then + (( failures += 1 )) + fi +done +(( failures == 0 )) && printf ' PASS: %d files\n' "${#shell_files[@]}" + +printf '\nGit whitespace\n' +if git -C "$TEST_ROOT" diff --check && + git -C "$TEST_ROOT" diff --cached --check; then + printf ' PASS\n' +else + (( failures += 1 )) +fi + +printf '\nShellCheck\n' +if command -v shellcheck >/dev/null 2>&1; then + # These rules cannot follow TAPM's runtime-sourced defaults/colors or + # functions invoked indirectly through traps and menu dispatch. + if shellcheck -x -e SC1091,SC2034,SC2154,SC2317 "${shell_files[@]}"; then + printf ' PASS\n' + else + (( failures += 1 )) + fi +else + printf ' SKIP: shellcheck is not installed\n' +fi + +while IFS= read -r -d '' file; do + test_files+=("$file") +done < <( + find "${TEST_ROOT}/tests" -maxdepth 1 -type f -name 'test-*.sh' \ + -print0 | sort -z +) + +printf '\nBehavior tests\n' +for file in "${test_files[@]}"; do + printf ' %-30s ' "${file##*/}" + if bash "$file"; then + : + else + (( failures += 1 )) + fi +done + +printf '\n' +if (( failures > 0 )); then + printf 'FAILED: %d check group(s) failed.\n' "$failures" >&2 + exit 1 +fi +printf 'All TA-ProxMenu checks passed.\n' diff --git a/tests/test-evacuation.sh b/tests/test-evacuation.sh new file mode 100755 index 0000000..e999216 --- /dev/null +++ b/tests/test-evacuation.sh @@ -0,0 +1,70 @@ +#!/usr/bin/env bash +set -u -o pipefail + +TEST_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +source "${TEST_ROOT}/tests/testlib.sh" +source "${TEST_ROOT}/inc/evacuate-proxmox-node.sh" + +set_routing_fixture() { + MIGRATION_NODES=(node2 node3) + NODE_STORAGE_CACHE=() + NODE_STORAGE_CACHE[node2]='shared-a,shared-b' + NODE_STORAGE_CACHE[node3]='shared-a,shared-c' + PREFERRED_TARGET=node2 +} + +set_routing_fixture +assert_success "preferred destination is eligible" \ + choose_destination shared-a '' 0 +assert_equal node2 "$CHOSEN_DESTINATION" "preferred destination selected" + +set_routing_fixture +assert_success "affinity fallback is available" \ + choose_destination shared-a node3 1 +assert_equal node3 "$CHOSEN_DESTINATION" "affinity destination selected" +assert_equal "routed to satisfy HA node-affinity preference" "$CHOSEN_NOTE" \ + "affinity routing explanation" + +set_routing_fixture +assert_success "storage fallback is available" \ + choose_destination shared-c '' 0 +assert_equal node3 "$CHOSEN_DESTINATION" "storage-compatible destination selected" + +set_routing_fixture +assert_failure "strict affinity blocks two noncompliant destinations" \ + choose_destination shared-a node4 1 + +set_routing_fixture +MIGRATION_NODES=(node2) +assert_success "sole eligible node overrides affinity" \ + choose_destination shared-a node4 1 +assert_equal node2 "$CHOSEN_DESTINATION" "sole eligible destination selected" +assert_equal "only eligible node; HA node-affinity preference overridden" \ + "$CHOSEN_NOTE" "sole-node override explanation" + +TEST_RULES_FILE="$(mktemp /tmp/tapm-ha-rules.XXXXXX)" +cleanup_evacuation_tests() { + if [[ "$TEST_RULES_FILE" == /tmp/tapm-ha-rules.* ]]; then + rm -f -- "$TEST_RULES_FILE" + fi +} +trap cleanup_evacuation_tests EXIT + +printf '%s\n' \ + 'node-affinity: preferred-nodes' \ + ' resources vm:210,ct:211' \ + ' nodes node3:20,node2:10' \ + ' strict 1' >"$TEST_RULES_FILE" +HA_RULES_FILE="$TEST_RULES_FILE" + +assert_equal $'preferred-nodes\x1f1\x1fnode3,node2' \ + "$(get_guest_node_affinity qemu 210)" \ + "QEMU affinity parsing" +assert_equal $'preferred-nodes\x1f1\x1fnode3,node2' \ + "$(get_guest_node_affinity lxc 211)" \ + "LXC affinity parsing" +assert_equal $'none\x1f0\x1f' \ + "$(get_guest_node_affinity qemu 999)" \ + "guest without affinity" + +finish_tests diff --git a/tests/test-git-update.sh b/tests/test-git-update.sh new file mode 100755 index 0000000..19437c3 --- /dev/null +++ b/tests/test-git-update.sh @@ -0,0 +1,79 @@ +#!/usr/bin/env bash +set -u -o pipefail + +TEST_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +source "${TEST_ROOT}/tests/testlib.sh" +source "${TEST_ROOT}/inc/git-update.inc" + +TEST_TEMP_DIR="$(mktemp -d /tmp/tapm-git-tests.XXXXXX)" +TEST_REPOSITORY="${TEST_TEMP_DIR}/repository" + +cleanup_git_tests() { + if [[ "$TEST_TEMP_DIR" == /tmp/tapm-git-tests.* && -d "$TEST_TEMP_DIR" ]]; then + rm -rf -- "$TEST_TEMP_DIR" + fi +} +trap cleanup_git_tests EXIT + +git init -q -b main "$TEST_REPOSITORY" +git -C "$TEST_REPOSITORY" config user.name 'TA-ProxMenu Tests' +git -C "$TEST_REPOSITORY" config user.email 'tapm-tests@example.invalid' + +printf 'first\n' >"${TEST_REPOSITORY}/fixture.txt" +git -C "$TEST_REPOSITORY" add fixture.txt +git -C "$TEST_REPOSITORY" commit -q -m first +commit_a="$(git -C "$TEST_REPOSITORY" rev-parse HEAD)" + +printf 'second\n' >>"${TEST_REPOSITORY}/fixture.txt" +git -C "$TEST_REPOSITORY" commit -q -am second +commit_b="$(git -C "$TEST_REPOSITORY" rev-parse HEAD)" + +git -C "$TEST_REPOSITORY" switch -q --detach "$commit_a" +printf 'alternate\n' >"${TEST_REPOSITORY}/alternate.txt" +git -C "$TEST_REPOSITORY" add alternate.txt +git -C "$TEST_REPOSITORY" commit -q -m alternate +commit_c="$(git -C "$TEST_REPOSITORY" rev-parse HEAD)" + +git -C "$TEST_REPOSITORY" update-ref refs/tapm/a "$commit_a" +git -C "$TEST_REPOSITORY" update-ref refs/tapm/b "$commit_b" +git -C "$TEST_REPOSITORY" update-ref refs/tapm/c "$commit_c" + +assert_equal current \ + "$(TAPM_GIT_RELATION "$TEST_REPOSITORY" refs/tapm/a refs/tapm/a)" \ + "equal commits" +assert_equal behind \ + "$(TAPM_GIT_RELATION "$TEST_REPOSITORY" refs/tapm/a refs/tapm/b)" \ + "local commit behind remote" +assert_equal ahead \ + "$(TAPM_GIT_RELATION "$TEST_REPOSITORY" refs/tapm/b refs/tapm/a)" \ + "local commit ahead of remote" +assert_equal diverged \ + "$(TAPM_GIT_RELATION "$TEST_REPOSITORY" refs/tapm/b refs/tapm/c)" \ + "diverged commits" + +git -C "$TEST_REPOSITORY" switch -q -C main "$commit_a" +git -C "$TEST_REPOSITORY" update-ref refs/remotes/origin/main "$commit_a" +assert_equal current \ + "$(TAPM_GIT_BRANCH_STATE "$TEST_REPOSITORY" main)" \ + "clean current branch" + +printf 'untracked\n' >"${TEST_REPOSITORY}/untracked.txt" +assert_equal dirty \ + "$(TAPM_GIT_BRANCH_STATE "$TEST_REPOSITORY" main)" \ + "dirty working tree" +rm -f -- "${TEST_REPOSITORY}/untracked.txt" + +git -C "$TEST_REPOSITORY" switch -q -c feature +assert_equal wrong-branch \ + "$(TAPM_GIT_BRANCH_STATE "$TEST_REPOSITORY" main)" \ + "wrong checked-out branch" + +git -C "$TEST_REPOSITORY" switch -q main +git -C "$TEST_REPOSITORY" update-ref refs/remotes/origin/main "$commit_b" +assert_success "fast-forward merge" \ + TAPM_GIT_FAST_FORWARD "$TEST_REPOSITORY" main +assert_equal "$commit_b" \ + "$(git -C "$TEST_REPOSITORY" rev-parse HEAD)" \ + "fast-forward destination" + +finish_tests diff --git a/tests/test-iso-nfs.sh b/tests/test-iso-nfs.sh new file mode 100755 index 0000000..b62a31f --- /dev/null +++ b/tests/test-iso-nfs.sh @@ -0,0 +1,58 @@ +#!/usr/bin/env bash +set -u -o pipefail + +TEST_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +source "${TEST_ROOT}/tests/testlib.sh" +source "${TEST_ROOT}/inc/deploy-iso-nfs-lxc.sh" + +assert_success "valid storage ID" TAPM_ISO_NFS_VALID_ID PVE-Shared-Storage +assert_failure "storage ID cannot start with a number" TAPM_ISO_NFS_VALID_ID 1-storage +assert_failure "storage ID rejects spaces" TAPM_ISO_NFS_VALID_ID 'shared storage' + +assert_success "valid CTID" TAPM_ISO_NFS_VALID_CTID 210 +assert_failure "reserved two-digit CTID" TAPM_ISO_NFS_VALID_CTID 99 +assert_failure "CTID rejects text" TAPM_ISO_NFS_VALID_CTID ct210 + +assert_success "valid hostname" TAPM_ISO_NFS_VALID_HOSTNAME PVE-Shared-Storage +assert_success "valid FQDN hostname" TAPM_ISO_NFS_VALID_HOSTNAME iso-nfs.example.net +assert_failure "hostname rejects spaces" TAPM_ISO_NFS_VALID_HOSTNAME 'ISO Storage' + +assert_success "valid IPv4 CIDR" TAPM_ISO_NFS_VALID_IPV4_CIDR 10.10.2.45/16 +assert_failure "IPv4 octet out of range" TAPM_ISO_NFS_VALID_IPV4_CIDR 10.10.2.256/24 +assert_failure "IPv4 prefix out of range" TAPM_ISO_NFS_VALID_IPV4_CIDR 10.10.2.45/33 +assert_failure "IPv4 prefix required" TAPM_ISO_NFS_VALID_IPV4_CIDR 10.10.2.45 + +pvesm() { + [[ "${1:-}" == status ]] || return 1 + printf '%s\n' \ + 'Name Type Status Total Used Available %' \ + 'local-lvm lvmthin active 100 10 90 10%' \ + 'iSCSI-Datastore1 lvm active 200 20 180 10%' \ + 'iSCSI-Datastore1-2 lvm active 200 20 180 10%' \ + 'offline-store dir inactive 100 0 100 0%' +} + +SELECT_MENU() { + local title="$1" + + assert_equal "Root filesystem storage" "$title" "storage menu title" + assert_equal \ + 'iSCSI-Datastore1-2 (lvm) — default' \ + "${labels[0]}" \ + "default storage placed first" + assert_equal 3 "${#labels[@]}" "only active storages offered" + MENU_SELECTION="${values[0]}" +} + +test_default_storage_selection() { + local selected_storage='' + + TAPM_ISO_NFS_SELECT_STORAGE selected_storage \ + "Root filesystem storage" "iSCSI-Datastore1-2" || return 1 + assert_equal iSCSI-Datastore1-2 "$selected_storage" \ + "pressing Enter retains default storage" +} + +assert_success "default storage menu selection" test_default_storage_selection + +finish_tests diff --git a/tests/test-virtio.sh b/tests/test-virtio.sh new file mode 100755 index 0000000..b753d89 --- /dev/null +++ b/tests/test-virtio.sh @@ -0,0 +1,34 @@ +#!/usr/bin/env bash +set -u -o pipefail + +TEST_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +source "${TEST_ROOT}/tests/testlib.sh" +source "${TEST_ROOT}/inc/virtio-helpers.inc" + +assert_equal virtio-win.iso \ + "$(TAPM_VIRTIO_FILENAME_FROM_URL \ + 'https://fedorapeople.org/stable/virtio-win.iso?download=1')" \ + "stable source filename" +assert_equal virtio-win-0.1.285.iso \ + "$(TAPM_VIRTIO_FILENAME_FROM_URL \ + 'https://fedorapeople.org/archive/virtio-win-0.1.285.iso')" \ + "versioned source filename" +assert_failure "unexpected source filename rejected" \ + TAPM_VIRTIO_FILENAME_FROM_URL \ + 'https://example.invalid/virtio-win-latest.iso' + +assert_equal virtio-win-latest-0.1.285.iso \ + "$(TAPM_VIRTIO_LABELED_FILENAME virtio-win-0.1.285.iso latest)" \ + "stable local filename" +assert_equal virtio-win-server-2016-0.1.240.iso \ + "$(TAPM_VIRTIO_LABELED_FILENAME \ + virtio-win-0.1.240.iso server-2016)" \ + "Server 2016 local filename" +assert_equal virtio-win-server-2008r2-0.1.172.iso \ + "$(TAPM_VIRTIO_LABELED_FILENAME \ + virtio-win-0.1.172.iso server-2008r2)" \ + "Server 2008 R2 local filename" +assert_failure "unsafe local label rejected" \ + TAPM_VIRTIO_LABELED_FILENAME virtio-win-0.1.285.iso '../latest' + +finish_tests diff --git a/tests/testlib.sh b/tests/testlib.sh new file mode 100644 index 0000000..a339cf8 --- /dev/null +++ b/tests/testlib.sh @@ -0,0 +1,48 @@ +#!/usr/bin/env bash + +TEST_ASSERTIONS=0 +TEST_FAILURES=0 + +test_fail() { + printf 'FAIL: %s\n' "$1" >&2 + (( TEST_FAILURES += 1 )) +} + +assert_equal() { + local expected="$1" + local actual="$2" + local label="$3" + + (( TEST_ASSERTIONS += 1 )) + if [[ "$actual" != "$expected" ]]; then + test_fail "${label}: expected '${expected}', received '${actual}'" + fi +} + +assert_success() { + local label="$1" + shift + + (( TEST_ASSERTIONS += 1 )) + "$@" || test_fail "${label}: expected success" +} + +assert_failure() { + local label="$1" + shift + + (( TEST_ASSERTIONS += 1 )) + if "$@"; then + test_fail "${label}: expected failure" + fi +} + +finish_tests() { + if (( TEST_FAILURES > 0 )); then + printf '%d assertion(s), %d failure(s)\n' \ + "$TEST_ASSERTIONS" "$TEST_FAILURES" >&2 + return 1 + fi + + printf '%d assertion(s) passed\n' "$TEST_ASSERTIONS" +}