From d0093f48b30d53aa1db3da0c4c545435be029731 Mon Sep 17 00:00:00 2001
From: Kevin Adams <kevin.adams@ksatechnologies.com>
Date: Mon, 8 Jun 2026 21:54:12 -0400
Subject: [PATCH] fix: safe multipath.conf management via BEGIN/END tagged
 block

On install: if /etc/multipath.conf already exists, append a tagged
block rather than skipping silently (old) or overwriting (dangerous).
On upgrade: replace only the tagged block, leaving the rest untouched.
On remove/purge: strip the tagged block; rest of the file preserved.

This means admins who added their own stanzas during manual testing
(e.g. before the package existed) won't lose config on install or
upgrade. Closes the silent "file already exists" footgun.

Also fixes pre-existing SC2015 shellcheck warnings in postrm.

Refs #256

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---
 .../DEBIAN-multipath/multipath.conf.example   | 16 +++++--
 packaging/DEBIAN-multipath/postinst           | 47 +++++++++++++++----
 packaging/DEBIAN-multipath/postrm             | 14 +++++-
 3 files changed, 63 insertions(+), 14 deletions(-)

diff --git a/packaging/DEBIAN-multipath/multipath.conf.example b/packaging/DEBIAN-multipath/multipath.conf.example
index 320131d..95fa788 100644
--- a/packaging/DEBIAN-multipath/multipath.conf.example
+++ b/packaging/DEBIAN-multipath/multipath.conf.example
@@ -1,12 +1,19 @@
-# /etc/multipath.conf — installed by truenas-proxmox-multipath
-# This file is only created if /etc/multipath.conf does not already exist.
-# If you have an existing multipath.conf, merge the 'devices' section below
-# into it manually.
+# /etc/multipath.conf — reference for truenas-proxmox-multipath
+#
+# postinst installs this automatically if /etc/multipath.conf does not exist.
+# If you already have a multipath.conf, postinst appends the tagged block below
+# instead of overwriting your file.
+#
+# The BEGIN/END markers let postinst update the stanza on upgrade and postrm
+# strip it on remove without touching the rest of your config.
 
 defaults {
     find_multipaths  yes
 }
 
+# BEGIN truenas-proxmox-multipath
+# Managed by truenas-proxmox-multipath. Do not edit between these markers.
+# To customise, remove the package tags and maintain the block yourself.
 devices {
     device {
         vendor                "TrueNAS"
@@ -18,3 +25,4 @@ devices {
         no_path_retry         5
     }
 }
+# END truenas-proxmox-multipath
diff --git a/packaging/DEBIAN-multipath/postinst b/packaging/DEBIAN-multipath/postinst
index ea8c35b..14ecb5a 100644
--- a/packaging/DEBIAN-multipath/postinst
+++ b/packaging/DEBIAN-multipath/postinst
@@ -17,17 +17,48 @@ install_plugin() {
     cp "${INSTALL_DIR}/TrueNASMultipath.pm" "$PLUGIN_DST"
 }
 
+BLOCK_BEGIN="# BEGIN truenas-proxmox-multipath"
+BLOCK_END="# END truenas-proxmox-multipath"
+
+# Appends or replaces the tagged TrueNAS device stanza in multipath.conf.
+# Safe to call on upgrade — idempotent via BEGIN/END markers.
 install_multipath_conf() {
-    if [ -f "$MULTIPATH_CONF" ]; then
-        log "${MULTIPATH_CONF} already exists — not overwriting"
-        log "Ensure it contains a 'devices' section for TrueNAS iSCSI Disk"
-        log "See ${INSTALL_DIR}/multipath.conf.example for reference"
-        return 0
+    local stanza
+    stanza=$(cat <<STANZA
+${BLOCK_BEGIN}
+# Managed by truenas-proxmox-multipath. Do not edit between these markers.
+# To customise, remove the package tags and maintain the block yourself.
+devices {
+    device {
+        vendor                "TrueNAS"
+        product               "iSCSI Disk"
+        path_grouping_policy  multibus
+        path_checker          tur
+        failback              immediate
+        rr_weight             uniform
+        no_path_retry         5
+    }
+}
+${BLOCK_END}
+STANZA
+)
+
+    if [ ! -f "$MULTIPATH_CONF" ]; then
+        log "Creating ${MULTIPATH_CONF} with defaults + TrueNAS device stanza"
+        printf 'defaults {\n    find_multipaths  yes\n}\n\n%s\n' "$stanza" > "$MULTIPATH_CONF"
+    elif grep -qF "$BLOCK_BEGIN" "$MULTIPATH_CONF"; then
+        log "Updating existing TrueNAS stanza in ${MULTIPATH_CONF}"
+        # Replace the block between markers (inclusive) with the new stanza
+        perl -i -0777 -pe \
+            "s|\Q${BLOCK_BEGIN}\E.*?\Q${BLOCK_END}\E|${stanza}|s" \
+            "$MULTIPATH_CONF"
+    else
+        log "Appending TrueNAS device stanza to existing ${MULTIPATH_CONF}"
+        printf '\n%s\n' "$stanza" >> "$MULTIPATH_CONF"
     fi
-    log "Installing ${MULTIPATH_CONF}"
-    cp "${INSTALL_DIR}/multipath.conf.example" "$MULTIPATH_CONF"
+
     systemctl enable multipathd 2>/dev/null || true
-    systemctl start multipathd  2>/dev/null || true
+    systemctl start  multipathd 2>/dev/null || true
     log "multipathd enabled and started"
 }
 
diff --git a/packaging/DEBIAN-multipath/postrm b/packaging/DEBIAN-multipath/postrm
index ea065b1..564e08d 100644
--- a/packaging/DEBIAN-multipath/postrm
+++ b/packaging/DEBIAN-multipath/postrm
@@ -3,6 +3,9 @@
 set -e
 
 PLUGIN_DST="/usr/share/perl5/PVE/Storage/Custom/TrueNASMultipath.pm"
+MULTIPATH_CONF="/etc/multipath.conf"
+BLOCK_BEGIN="# BEGIN truenas-proxmox-multipath"
+BLOCK_END="# END truenas-proxmox-multipath"
 LOG_FILE="/var/log/truenas-proxmox-install.log"
 
 log() {
@@ -16,9 +19,16 @@ case "$1" in
             rm -f "$PLUGIN_DST"
         fi
 
+        if [ -f "$MULTIPATH_CONF" ] && grep -qF "$BLOCK_BEGIN" "$MULTIPATH_CONF"; then
+            log "Removing TrueNAS stanza from ${MULTIPATH_CONF}"
+            perl -i -0777 -pe \
+                "s|\n?\Q${BLOCK_BEGIN}\E.*?\Q${BLOCK_END}\E\n?||s" \
+                "$MULTIPATH_CONF"
+        fi
+
         log "Restarting Proxmox VE services ..."
-        pvedaemon restart 2>/dev/null && log "pvedaemon restarted" || true
-        pveproxy restart  2>/dev/null && log "pveproxy restarted"  || true
+        if pvedaemon restart 2>/dev/null; then log "pvedaemon restarted"; fi
+        if pveproxy  restart 2>/dev/null; then log "pveproxy restarted";  fi
         log "Done."
         ;;
     upgrade|failed-upgrade|abort-install|abort-upgrade|disappear)