From a44458ae0c84db901953710a1e5f38350aca17bb Mon Sep 17 00:00:00 2001 From: Maarten Date: Mon, 2 Mar 2026 16:35:37 +0100 Subject: [PATCH] Fix proxmox-setup: heredoc detection, dry_run_guard, set -e traps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three bugs that caused the script to exit silently or behave wrong: 1. SSH detection used a double-quoted string with nested Python/awk containing $ and " characters that bash tried to expand locally. Switched to a quoted heredoc (<<'DETECT') so the entire remote script is passed literally. 2. dry_run_guard had return values swapped: returned 0 (success) when NOT in dry-run, causing apply functions to skip real work. Now returns 0 when DRY_RUN=true so "if dry_run_guard; then return; fi" correctly short-circuits. 3. Multiple "[[ test ]] && action" patterns outside if-conditions return 1 when the test is false, triggering set -e to abort. Converted all to "if [[ test ]]; then action; fi" form. Also fixed Python any() pattern: "any(...) and print('yes')" never prints "no" — replaced with print("yes" if any(...) else "no"). Co-Authored-By: Claude Opus 4.6 --- hetzner/proxmox-setup | 165 ++++++++++++++++++++++-------------------- 1 file changed, 88 insertions(+), 77 deletions(-) diff --git a/hetzner/proxmox-setup b/hetzner/proxmox-setup index b52d800..7401227 100755 --- a/hetzner/proxmox-setup +++ b/hetzner/proxmox-setup @@ -126,9 +126,9 @@ detail() { [[ "$VERBOSE" != true ]] && return; echo -e "${DIM} $*${RES dry_run_guard() { if [[ "$DRY_RUN" == true ]]; then info "(dry-run) $*" - return 1 + return 0 fi - return 0 + return 1 } # Status indicator for the dashboard @@ -370,65 +370,76 @@ detect_all() { fi success "SSH connection to ${SSH_HOST}" - # Gather everything in a single SSH call for speed + # Gather everything in a single SSH call for speed. + # Uses a quoted heredoc (<<'DETECT') so nothing is expanded locally -- + # all $, ", etc. are passed literally to the remote shell. local detection_blob - detection_blob=$(remote_output " - echo '---HOSTNAME---' - hostname - echo '---KERNEL---' - uname -r - echo '---PVE_VERSION---' - pvesh get /version --output-format json 2>/dev/null | python3 -c 'import sys,json; print(json.load(sys.stdin)[\"version\"])' 2>/dev/null || echo 'unknown' - echo '---PUBLIC_IP---' - hostname -I | awk '{print \$1}' - echo '---BLOCK_DEVICES---' - lsblk -d -o NAME,SIZE,MODEL,ROTA,TYPE 2>/dev/null - echo '---ZPOOLS---' - zpool list -H 2>/dev/null || true - echo '---PVE_STORAGE---' - pvesm status 2>/dev/null || true - echo '---ENTERPRISE_REPOS---' - grep -rl '^deb.*enterprise' /etc/apt/sources.list.d/*.list 2>/dev/null || echo 'none' - echo '---NOSUB_REPO---' - test -f /etc/apt/sources.list.d/pve-no-subscription.list && echo 'yes' || echo 'no' - echo '---NAG_PATCHED---' - grep -q 'void({' /usr/share/javascript/proxmox-widget-toolkit/proxmoxlib.js 2>/dev/null && echo 'yes' || echo 'no' - echo '---UPGRADABLE---' - apt list --upgradable 2>/dev/null | grep -c upgradable || echo '0' - echo '---VMBR1---' - ip -br addr show vmbr1 2>/dev/null || echo 'none' - echo '---IP_FORWARD---' - cat /proc/sys/net/ipv4/ip_forward 2>/dev/null || echo '0' - echo '---NAT_ACTIVE---' - iptables -t nat -L POSTROUTING -n 2>/dev/null | grep -q MASQUERADE && echo 'yes' || echo 'no' - echo '---WG_INSTALLED---' - command -v wg >/dev/null 2>&1 && echo 'yes' || echo 'no' - echo '---WG_ACTIVE---' - wg show wg0 2>/dev/null | head -1 || echo 'none' - echo '---WG_PEERS---' - wg show wg0 peers 2>/dev/null | wc -l || echo '0' - echo '---WG_HANDSHAKE---' - wg show wg0 latest-handshakes 2>/dev/null | head -1 || echo 'none' - echo '---NFT_FILE---' - test -f /etc/nftables-hetzner.conf && echo 'yes' || echo 'no' - echo '---NFT_RULES---' - nft list ruleset 2>/dev/null | grep -q 'nft-drop' && echo 'yes' || echo 'no' - echo '---API_ROLE---' - pveum role list --output-format json 2>/dev/null | python3 -c 'import sys,json; any(r[\"roleid\"]==\"IncusOSDeployer\" for r in json.load(sys.stdin)) and print(\"yes\")' 2>/dev/null || echo 'no' - echo '---API_POOL---' - pveum pool list --output-format json 2>/dev/null | python3 -c 'import sys,json; any(p[\"poolid\"]==\"IncusLab\" for p in json.load(sys.stdin)) and print(\"yes\")' 2>/dev/null || echo 'no' - echo '---API_TOKEN---' - pveum user token list automation@pve --output-format json 2>/dev/null | python3 -c 'import sys,json; any(t[\"tokenid\"]==\"deploy\" for t in json.load(sys.stdin)) and print(\"yes\")' 2>/dev/null || echo 'no' - echo '---AVAILABLE_DISKS---' - used_disks=\$(zpool status 2>/dev/null | grep -oP '/dev/\\S+' | sed 's|/dev/||' || true) - lsblk -dno NAME,SIZE,TYPE | while read name size type; do - [ \"\$type\" != 'disk' ] && continue - echo \"\$used_disks\" | grep -qw \"\$name\" && continue - lsblk -no MOUNTPOINT /dev/\$name 2>/dev/null | grep -q '/' && continue - echo \"\$name \$size\" - done - echo '---END---' - " || true) + detection_blob=$(ssh -o ConnectTimeout=10 -o BatchMode=yes "$SSH_HOST" bash <<'DETECT' 2>/dev/null || true +echo '---HOSTNAME---' +hostname +echo '---KERNEL---' +uname -r +echo '---PVE_VERSION---' +pvesh get /version --output-format json 2>/dev/null \ + | python3 -c 'import sys,json; print(json.load(sys.stdin)["version"])' 2>/dev/null \ + || echo 'unknown' +echo '---PUBLIC_IP---' +hostname -I | awk '{print $1}' +echo '---BLOCK_DEVICES---' +lsblk -d -o NAME,SIZE,MODEL,ROTA,TYPE 2>/dev/null +echo '---ZPOOLS---' +zpool list -H 2>/dev/null || true +echo '---PVE_STORAGE---' +pvesm status 2>/dev/null || true +echo '---ENTERPRISE_REPOS---' +grep -rl '^deb.*enterprise' /etc/apt/sources.list.d/*.list 2>/dev/null || echo 'none' +echo '---NOSUB_REPO---' +test -f /etc/apt/sources.list.d/pve-no-subscription.list && echo 'yes' || echo 'no' +echo '---NAG_PATCHED---' +grep -q 'void({' /usr/share/javascript/proxmox-widget-toolkit/proxmoxlib.js 2>/dev/null && echo 'yes' || echo 'no' +echo '---UPGRADABLE---' +apt list --upgradable 2>/dev/null | grep -c upgradable || true +echo '---VMBR1---' +ip -br addr show vmbr1 2>/dev/null || echo 'none' +echo '---IP_FORWARD---' +cat /proc/sys/net/ipv4/ip_forward 2>/dev/null || echo '0' +echo '---NAT_ACTIVE---' +iptables -t nat -L POSTROUTING -n 2>/dev/null | grep -q MASQUERADE && echo 'yes' || echo 'no' +echo '---WG_INSTALLED---' +command -v wg >/dev/null 2>&1 && echo 'yes' || echo 'no' +echo '---WG_ACTIVE---' +wg show wg0 2>/dev/null | head -1 || echo 'none' +echo '---WG_PEERS---' +wg show wg0 peers 2>/dev/null | wc -l || echo '0' +echo '---WG_HANDSHAKE---' +wg show wg0 latest-handshakes 2>/dev/null | head -1 || echo 'none' +echo '---NFT_FILE---' +test -f /etc/nftables-hetzner.conf && echo 'yes' || echo 'no' +echo '---NFT_RULES---' +nft list ruleset 2>/dev/null | grep -q 'nft-drop' && echo 'yes' || echo 'no' +echo '---API_ROLE---' +pveum role list --output-format json 2>/dev/null \ + | python3 -c 'import sys,json; print("yes" if any(r["roleid"]=="IncusOSDeployer" for r in json.load(sys.stdin)) else "no")' 2>/dev/null \ + || echo 'no' +echo '---API_POOL---' +pveum pool list --output-format json 2>/dev/null \ + | python3 -c 'import sys,json; print("yes" if any(p["poolid"]=="IncusLab" for p in json.load(sys.stdin)) else "no")' 2>/dev/null \ + || echo 'no' +echo '---API_TOKEN---' +pveum user token list automation@pve --output-format json 2>/dev/null \ + | python3 -c 'import sys,json; print("yes" if any(t["tokenid"]=="deploy" for t in json.load(sys.stdin)) else "no")' 2>/dev/null \ + || echo 'no' +echo '---AVAILABLE_DISKS---' +used_disks=$(zpool status 2>/dev/null | grep -oP '/dev/\S+' | sed 's|/dev/||' || true) +lsblk -dno NAME,SIZE,TYPE | while read name size type; do + [ "$type" != 'disk' ] && continue + echo "$used_disks" | grep -qw "$name" && continue + lsblk -no MOUNTPOINT /dev/$name 2>/dev/null | grep -q '/' && continue + echo "$name $size" +done +echo '---END---' +DETECT +) # Parse the blob into variables local section="" @@ -451,7 +462,7 @@ ${line}" fi done <<< "$detection_blob" # Process last section - [[ -n "$section" ]] && _parse_section "$section" "$section_lines" + if [[ -n "$section" ]]; then _parse_section "$section" "$section_lines"; fi # Determine composite states _compute_states @@ -502,13 +513,13 @@ _parse_section() { NFT_FILE) DETECT_NFT_FILE="$content" ;; NFT_RULES) DETECT_NFT_RULES="$content" ;; API_ROLE) - [[ "$content" == "yes" ]] && DETECT_API_ROLE="yes" + if [[ "$content" == "yes" ]]; then DETECT_API_ROLE="yes"; fi ;; API_POOL) - [[ "$content" == "yes" ]] && DETECT_API_POOL="yes" + if [[ "$content" == "yes" ]]; then DETECT_API_POOL="yes"; fi ;; API_TOKEN) - [[ "$content" == "yes" ]] && DETECT_API_TOKEN="yes" + if [[ "$content" == "yes" ]]; then DETECT_API_TOKEN="yes"; fi ;; AVAILABLE_DISKS) DETECT_AVAILABLE_DISKS="$content" @@ -638,10 +649,10 @@ show_status() { local effective_wg="$STATE_WIREGUARD" local effective_fw="$STATE_FIREWALL" - [[ "$SKIP_REPOS" == true ]] && effective_repos="skipped" && effective_update="skipped" - [[ "$SKIP_STORAGE" == true ]] && effective_storage="skipped" - [[ "$SKIP_WIREGUARD" == true ]] && effective_wg="skipped" - [[ "$SKIP_FIREWALL" == true ]] && effective_fw="skipped" + if [[ "$SKIP_REPOS" == true ]]; then effective_repos="skipped"; effective_update="skipped"; fi + if [[ "$SKIP_STORAGE" == true ]]; then effective_storage="skipped"; fi + if [[ "$SKIP_WIREGUARD" == true ]]; then effective_wg="skipped"; fi + if [[ "$SKIP_FIREWALL" == true ]]; then effective_fw="skipped"; fi printf " %-20s %b" "Repositories" "$(status_icon "$effective_repos")" case "$effective_repos" in @@ -723,13 +734,13 @@ show_status() { # Count pending items local pending=0 - [[ "$effective_repos" == "needed" || "$effective_repos" == "partial" ]] && pending=$((pending + 1)) - [[ "$effective_update" == "needed" ]] && pending=$((pending + 1)) - [[ "$STATE_BRIDGE" == "needed" || "$STATE_BRIDGE" == "partial" ]] && pending=$((pending + 1)) - [[ "$effective_storage" == "needed" ]] && pending=$((pending + 1)) - [[ "$effective_wg" == "needed" ]] && pending=$((pending + 1)) - [[ "$effective_fw" == "needed" ]] && pending=$((pending + 1)) - [[ "$STATE_API" == "needed" || "$STATE_API" == "partial" ]] && pending=$((pending + 1)) + if [[ "$effective_repos" == "needed" || "$effective_repos" == "partial" ]]; then pending=$((pending + 1)); fi + if [[ "$effective_update" == "needed" ]]; then pending=$((pending + 1)); fi + if [[ "$STATE_BRIDGE" == "needed" || "$STATE_BRIDGE" == "partial" ]]; then pending=$((pending + 1)); fi + if [[ "$effective_storage" == "needed" ]]; then pending=$((pending + 1)); fi + if [[ "$effective_wg" == "needed" ]]; then pending=$((pending + 1)); fi + if [[ "$effective_fw" == "needed" ]]; then pending=$((pending + 1)); fi + if [[ "$STATE_API" == "needed" || "$STATE_API" == "partial" ]]; then pending=$((pending + 1)); fi if [[ "$pending" -eq 0 ]]; then success "All configured -- nothing to do" @@ -922,7 +933,7 @@ apply_storage() { info "ZFS pool already registered with Proxmox" info "${DETECT_AVAILABLE_DISK_COUNT} additional unused disk(s) available:" echo "$DETECT_AVAILABLE_DISKS" | while IFS= read -r line; do - [[ -n "$line" ]] && echo " /dev/$line" + if [[ -n "$line" ]]; then echo " /dev/$line"; fi done if ! confirm "Create an additional storage pool?"; then return 0 @@ -953,7 +964,7 @@ apply_storage() { info "Available disks:" echo "$DETECT_AVAILABLE_DISKS" | while IFS= read -r line; do - [[ -n "$line" ]] && echo " /dev/$line" + if [[ -n "$line" ]]; then echo " /dev/$line"; fi done local disk_count="$DETECT_AVAILABLE_DISK_COUNT"