fix: apply review feedback on Docker storage and security documentation

- Fix storage_ip derivation to use default route first, not Swarm RemoteManagers
- Make iSCSI container entrypoint idempotent (check before create for target/LUN/bind)
- Pin erichough/nfs-server from 'latest' to '2.2.1' for reproducibility
- Replace curl|sh Docker install with apt repo in Dockerfile.test
- Add docker.sock security warnings in docker-compose.test.yml and CI workflow
- Update stale 'storage VM' references in comments and .env.test.example"

Co-authored-by: GoodOlClint <151449+GoodOlClint@users.noreply.github.com>
Agent-Logs-Url: https://github.com/GoodOlClint/PSProxmoxVE/sessions/534c0a63-2988-496c-b0ae-e291fcfa050b

.github/workflows/integration-tests.yml

@@ -115,6 +115,10 @@ jobs:
         password: ${{ secrets.GITHUB_TOKEN }}
       volumes:
         - /opt/pve-isos:/opt/pve-isos
+        # WARNING: The Docker socket is mounted to allow storage containers to be
+        # managed from within the job container. This grants root-equivalent access
+        # to the runner's Docker daemon. Only use on dedicated, isolated self-hosted
+        # runners — never on shared runners.
         - /var/run/docker.sock:/var/run/docker.sock
 
     steps:
@@ -186,6 +190,10 @@ jobs:
         password: ${{ secrets.GITHUB_TOKEN }}
       volumes:
         - /opt/pve-isos:/opt/pve-isos
+        # WARNING: The Docker socket is mounted to allow storage containers to be
+        # stopped from within the job container. This grants root-equivalent access
+        # to the runner's Docker daemon. Only use on dedicated, isolated self-hosted
+        # runners — never on shared runners.
         - /var/run/docker.sock:/var/run/docker.sock
 
     steps:

tests/.env.test.example

@@ -5,10 +5,10 @@
 # will be provisioned. Integration tests run against the nested VMs,
 # not against this host directly.
 #
-# Provisioned VMs:
+# Provisioned resources:
 #   - 2x PVE 9 nodes (pve9a, pve9b) for cluster testing
 #   - 2x PVE 8 nodes (pve8a, pve8b) for cluster testing
-#   - 1x Ubuntu storage VM (iSCSI target + NFS server)
+#   - Shared storage provided by Docker containers on the runner host (iSCSI + NFS)
 
 # ── Required: parent PVE for provisioning ─────────────────────────────
 PVE_ENDPOINT=https://pve.example.com:8006

tests/Dockerfile.test

@@ -65,5 +65,12 @@ RUN apt-get update && apt-get install -y --no-install-recommends \
     && rm -rf /var/lib/apt/lists/*
 
 # Install Docker CLI (for managing storage containers on the host via mounted socket)
-RUN curl -fsSL https://get.docker.com | sh 2>/dev/null \
+RUN install -m 0755 -d /etc/apt/keyrings \
+    && curl -fsSL https://download.docker.com/linux/ubuntu/gpg \
+        | gpg --dearmor -o /etc/apt/keyrings/docker.gpg \
+    && chmod a+r /etc/apt/keyrings/docker.gpg \
+    && echo "deb [arch=amd64 signed-by=/etc/apt/keyrings/docker.gpg] https://download.docker.com/linux/ubuntu noble stable" \
+        > /etc/apt/sources.list.d/docker.list \
+    && apt-get update \
+    && apt-get install -y --no-install-recommends docker-ce-cli docker-compose-plugin \
     && rm -rf /var/lib/apt/lists/*

tests/docker-compose.test.yml

@@ -38,6 +38,8 @@ services:
     volumes:
       - ..:/repo
       - /opt/pve-isos:/opt/pve-isos
+      # WARNING: Mounting the Docker socket grants this container root-equivalent
+      # access to the host's Docker daemon. Only use on trusted, isolated machines.
       - /var/run/docker.sock:/var/run/docker.sock
     working_dir: /repo
     stdin_open: true

tests/infrastructure/docker-compose.storage.yml

@@ -30,15 +30,24 @@ services:
         fi
         tgtd --foreground &
         sleep 2
-        tgtadm --lld iscsi --op new --mode target --tid 1 -T $$ISCSI_IQN
+        # Create target if it does not already exist
-        tgtadm --lld iscsi --op new --mode logicalunit --tid 1 --lun 1 --backing-store /srv/iscsi/lun0.img
+        if ! tgtadm --lld iscsi --op show --mode target | grep -q "Target 1: $$ISCSI_IQN"; then
-        tgtadm --lld iscsi --op bind --mode target --tid 1 -I ALL
+          tgtadm --lld iscsi --op new --mode target --tid 1 -T $$ISCSI_IQN
+        fi
+        # Create logical unit (LUN 1) if it does not already exist
+        if ! tgtadm --lld iscsi --op show --mode logicalunit --tid 1 2>/dev/null | grep -qE "LUN:[[:space:]]*1($|[^0-9])"; then
+          tgtadm --lld iscsi --op new --mode logicalunit --tid 1 --lun 1 --backing-store /srv/iscsi/lun0.img
+        fi
+        # Bind target to all initiators if not already bound
+        if ! tgtadm --lld iscsi --op show --mode target --tid 1 2>/dev/null | grep -q "Initiator-address: ALL"; then
+          tgtadm --lld iscsi --op bind --mode target --tid 1 -I ALL
+        fi
         echo "iSCSI target ready: $$ISCSI_IQN (port 3260)"
         wait
     restart: unless-stopped
 
   nfs-server:
-    image: erichough/nfs-server:latest
+    image: erichough/nfs-server:2.2.1
     container_name: pvetest-nfs
     network_mode: host
     privileged: true

tests/infrastructure/scripts/run-integration.sh

@@ -5,10 +5,10 @@
 # Called by both the GitHub Actions workflow and the local dev container.
 #
 # Provisions two PVE nodes per version (a/b) for cluster testing, plus
-# an Ubuntu storage VM for iSCSI/NFS shared storage testing.
+# Docker containers on the runner host for iSCSI/NFS shared storage.
 #
 # Usage:
-#   run-integration.sh provision          Provision nested PVE VMs + storage VM
+#   run-integration.sh provision          Provision nested PVE VMs + start storage containers
 #   run-integration.sh test [8|9|all]     Run integration tests (default: all)
 #   run-integration.sh cleanup            Destroy provisioned VMs
 #   run-integration.sh all [8|9|all]      Full lifecycle: provision → test → cleanup
@@ -204,10 +204,12 @@ cmd_provision() {
     # Get the Docker host's real IP (not the container's). The storage containers
     # use host networking, so PVE nodes reach them via the host's IP.
     local storage_ip
-    storage_ip=$(docker info --format '{{range .Swarm.RemoteManagers}}{{.Addr}}{{end}}' 2>/dev/null | cut -d: -f1)
+    # Prefer the IP of the default route's interface on the Docker host.
+    storage_ip=$(docker run --rm --net=host alpine ip route get 1.1.1.1 2>/dev/null | awk '{for(i=1;i<=NF;i++) if($i=="src") print $(i+1)}')
     if [ -z "$storage_ip" ]; then
-        # Fallback: get IP of the default route's interface on the Docker host
+        # Fallback: use the local node address from Docker info (not RemoteManagers,
-        storage_ip=$(docker run --rm --net=host alpine ip route get 1.1.1.1 2>/dev/null | awk '{for(i=1;i<=NF;i++) if($i=="src") print $(i+1)}')
+        # which can return addresses of remote Swarm managers).
+        storage_ip=$(docker info --format '{{.Swarm.NodeAddr}}' 2>/dev/null | cut -d: -f1)
     fi
     if [ -z "$storage_ip" ]; then
         ci_error "Could not determine Docker host IP for storage services"
@@ -453,7 +455,7 @@ main() {
             echo "Usage: $(basename "$0") {provision|test|cleanup|all} [8|9|all]"
             echo ""
             echo "Subcommands:"
-            echo "  provision          Provision nested PVE VMs + storage VM"
+            echo "  provision          Provision nested PVE VMs + start storage containers"
             echo "  test [8|9|all]     Run integration tests (default: all versions)"
             echo "  cleanup            Destroy all provisioned VMs"
             echo "  all [8|9|all]      Full lifecycle: provision → test → cleanup"