fix(vm): reject failed ISO downloads and repair image sources

This commit is contained in:
martino
2026-10-16 15:42:58 +02:00
parent 7f21cae196
commit f07dbfe10f
5 changed files with 513 additions and 30 deletions
@@ -0,0 +1,263 @@
"""The VM wizard must not accept failed ISO downloads as installation media.
Run its real functions against a loopback HTTP fixture and stub only external
Proxmox/UI services. No product script is sourced, no guest operation runs,
and no public network or real ISO storage is accessed.
"""
from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer
import os
from pathlib import Path
import re
import shlex
import shutil
import subprocess
import tempfile
import threading
import unittest
ROOT = Path(__file__).resolve().parents[3]
CREATOR = ROOT / "scripts/vm/vm_creator.sh"
PAYLOAD = b"fixture installation media\x00\x01\n"
REAL_WGET = shutil.which("wget")
class MediaHandler(BaseHTTPRequestHandler):
def do_GET(self):
if self.path == "/missing.iso":
self.send_response(404)
self.end_headers()
return
if self.path == "/partial.iso":
self.send_response(200)
self.send_header("Content-Length", "1024")
self.end_headers()
self.wfile.write(b"partial")
self.close_connection = True
return
body = b"" if self.path == "/empty.iso" else PAYLOAD
self.send_response(200)
self.send_header("Content-Length", str(len(body)))
self.send_header("Content-Disposition", 'attachment; filename="remote-name.iso"')
self.end_headers()
self.wfile.write(body)
def log_message(self, format, *args):
pass
class VmIsoDownloadTests(unittest.TestCase):
@classmethod
def setUpClass(cls):
if REAL_WGET is None:
raise RuntimeError("These offline tests require wget, as the VM wizard does")
cls.server = ThreadingHTTPServer(("127.0.0.1", 0), MediaHandler)
cls.thread = threading.Thread(target=cls.server.serve_forever, daemon=True)
cls.thread.start()
cls.http = f"http://127.0.0.1:{cls.server.server_port}"
@classmethod
def tearDownClass(cls):
cls.server.shutdown()
cls.server.server_close()
cls.thread.join()
def setUp(self):
self.temp = tempfile.TemporaryDirectory(prefix="vm-iso-download-")
self.addCleanup(self.temp.cleanup)
self.directory = Path(self.temp.name)
self.iso = self.directory / "installation media.iso"
self.qm_log = self.directory / "qm.log"
self.wget_log = self.directory / "wget.log"
self.bin = self.directory / "bin"
self.bin.mkdir()
# Route only the optional fixed VirtIO/SourceForge endpoints to our
# fixture. Wget still performs the actual HTTP transfer and file I/O.
assert REAL_WGET is not None
wrapper = self.bin / "wget"
wrapper.write_text(
"#!/usr/bin/env bash\n"
'printf "%s\\n" "$*" >> "$WGET_LOG"\n'
'args=("$@")\n'
'for i in "${!args[@]}"; do\n'
' case "${args[$i]}" in\n'
' https://fedorapeople.org/*) args[$i]="$VIRTIO_FIXTURE_URL";;\n'
' https://sourceforge.net/*) args[$i]="$HTTP_FIXTURE/success.iso";;\n'
' esac\n'
' if [[ "${args[$i]}" == http://* || "${args[$i]}" == https://* ]]; then\n'
' [[ "${args[$i]}" == "$HTTP_FIXTURE"/* ]] || exit 98\n'
' fi\n'
'done\n'
'for i in "${!args[@]}"; do\n'
' if [[ "${args[$i]}" == -O ]]; then\n'
' case "${args[$((i+1))]}" in "$FIXTURE_DIR"/*) ;; *) exit 97;; esac\n'
' fi\n'
'done\n'
f"exec {shlex.quote(REAL_WGET)} --no-config --no-proxy --tries=1 --timeout=1 "
'"${args[@]}"\n'
)
wrapper.chmod(0o755)
def run_wizard(self, route="/success.iso", windows=False, overwrite=True, url=None):
source = CREATOR.read_text()
functions = "\n\n".join(re.findall(r"^function \w+\(\) \{\n.*?^\}", source, re.M | re.S))
self.assertIn("function create_vm()", functions)
prelude = r'''
translate() { printf '%s' "$1"; }
msg_ok() { printf 'OK:%s\n' "$*"; }
msg_error() { printf 'ERROR:%s\n' "$*"; }
msg_warn() { printf 'WARN:%s\n' "$*"; }
msg_info() { :; }
msg_success() { :; }
qm() { printf '%q ' "$@" >> "$QM_LOG"; printf '\n' >> "$QM_LOG"; }
pvesm() { printf 'Name Type Status\nfixture dir active\n'; }
select_storage_target() { printf fixture; }
# No next menu, start, or guest-facing process is launched.
bash() { :; }
whiptail() {
if [[ " $* " == *' --yesno '* ]]; then
return "$OVERWRITE_STATUS"
fi
if [[ ! -f "$DIALOG_ONCE" ]]; then
: > "$DIALOG_ONCE"
printf '1' >&2
return 0
fi
return 1
}
IMPORT_DISKS=(); PASSTHROUGH_DISKS=(); VIRTUAL_DISKS=(); CONTROLLER_NVME_PCIS=()
VMID=99999; HN=fixture; CORE_COUNT=1; RAM_SIZE=512; BRG=fixture
MAC='02:00:00:00:00:01'; VLAN=''; MTU=''; BIOS_TYPE=''; CPU_TYPE=''
MACHINE=''; SERIAL_PORT=''; START_VM=no; WIZARD_ADD_GPU=no
TAB=''; GN=''; CL=''
create_vm
'''
env = dict(os.environ, PATH=str(self.bin) + os.pathsep + os.environ["PATH"],
ISO_PATH=str(self.iso), ISO_URL=url if url is not None else self.http + route,
ISO_VOLID="local:iso/installation media.iso", ISO_DIR=str(self.directory),
OS_TYPE="2" if windows else "3",
QM_LOG=str(self.qm_log), WGET_LOG=str(self.wget_log),
DIALOG_ONCE=str(self.directory / "dialog-once"), FIXTURE_DIR=str(self.directory),
OVERWRITE_STATUS="0" if overwrite else "1", HTTP_FIXTURE=self.http,
VIRTIO_FIXTURE_URL=self.http + route)
return subprocess.run(["bash", "--noprofile", "--norc", "-c", functions + prelude],
env=env, stdin=subprocess.DEVNULL, capture_output=True,
text=True, timeout=15)
def qm_calls(self):
return self.qm_log.read_text() if self.qm_log.exists() else ""
def test_http_failure_stops_before_creating_a_guest(self):
result = self.run_wizard("/missing.iso")
self.assertNotEqual(result.returncode, 0, result.stdout + result.stderr)
self.assertEqual(self.qm_calls(), "")
self.assertNotIn("OK:ISO image downloaded", result.stdout)
self.assertIn("ERROR:Failed to download ISO image", result.stdout)
def test_partial_transfer_is_not_published_as_cached_media(self):
result = self.run_wizard("/partial.iso")
self.assertNotEqual(result.returncode, 0, result.stdout + result.stderr)
self.assertEqual(self.qm_calls(), "")
self.assertFalse(self.iso.exists(), "A failed transfer must not become reusable media")
def test_an_empty_cached_download_is_retried(self):
self.iso.write_bytes(b"")
result = self.run_wizard()
self.assertEqual(result.returncode, 0, result.stdout + result.stderr)
self.assertEqual(self.iso.read_bytes(), PAYLOAD)
self.assertIn("create ", self.qm_calls())
def test_virtio_download_uses_the_configured_iso_directory(self):
self.iso.write_bytes(PAYLOAD)
result = self.run_wizard(windows=True)
self.assertEqual(result.returncode, 0, result.stdout + result.stderr)
driver = self.directory / "virtio-win.iso"
self.assertTrue(driver.is_file(), result.stdout + result.stderr)
self.assertEqual(driver.read_bytes(), PAYLOAD)
self.assertIn("-ide3 ", self.qm_calls())
def test_failed_virtio_fetch_is_not_selected_for_attachment(self):
self.iso.write_bytes(PAYLOAD)
result = self.run_wizard("/missing.iso", windows=True)
self.assertEqual(result.returncode, 0, result.stdout + result.stderr)
self.assertNotIn("-ide3 ", self.qm_calls())
self.assertNotIn("OK:VirtIO driver ISO downloaded successfully.", result.stdout)
self.assertIn("ERROR:Failed to download VirtIO driver ISO.", result.stdout)
self.assertFalse((self.directory / "virtio-win.iso").exists())
def test_successful_virtio_replacement_preserves_existing_permissions(self):
self.iso.write_bytes(PAYLOAD)
driver = self.directory / "virtio-win.iso"
driver.write_bytes(b"previous driver")
driver.chmod(0o640)
result = self.run_wizard(windows=True)
self.assertEqual(result.returncode, 0, result.stdout + result.stderr)
self.assertEqual(driver.read_bytes(), PAYLOAD)
self.assertEqual(driver.stat().st_mode & 0o777, 0o640)
def test_empty_http_success_is_rejected(self):
result = self.run_wizard("/empty.iso")
self.assertNotEqual(result.returncode, 0, result.stdout + result.stderr)
self.assertEqual(self.qm_calls(), "")
self.assertFalse(self.iso.exists())
self.assertEqual({p.name for p in self.directory.iterdir()}, {"bin", "wget.log"})
def test_existing_nonempty_media_is_reused_without_fetching(self):
self.iso.write_bytes(b"existing installation media")
result = self.run_wizard("/missing.iso")
self.assertEqual(result.returncode, 0, result.stdout + result.stderr)
self.assertEqual(self.iso.read_bytes(), b"existing installation media")
self.assertFalse(self.wget_log.exists())
self.assertIn("-ide2 ", self.qm_calls())
def test_sourceforge_download_keeps_the_selected_destination_name(self):
result = self.run_wizard(url="https://sourceforge.net/projects/fixture/iso/download")
self.assertEqual(result.returncode, 0, result.stdout + result.stderr)
self.assertEqual(self.iso.read_bytes(), PAYLOAD)
self.assertFalse((self.directory / "remote-name.iso").exists())
self.assertEqual({p.name for p in self.directory.iterdir()},
{"bin", "wget.log", "qm.log", self.iso.name})
def test_failed_virtio_replacement_preserves_previous_media(self):
self.iso.write_bytes(PAYLOAD)
driver = self.directory / "virtio-win.iso"
driver.write_bytes(b"previous driver")
driver.chmod(0o640)
result = self.run_wizard("/partial.iso", windows=True)
self.assertEqual(result.returncode, 0, result.stdout + result.stderr)
self.assertEqual(driver.read_bytes(), b"previous driver")
self.assertEqual(driver.stat().st_mode & 0o777, 0o640)
self.assertNotIn("-ide3 ", self.qm_calls())
self.assertNotIn("OK:VirtIO driver ISO downloaded successfully.", result.stdout)
def test_declining_virtio_replacement_reuses_previous_media(self):
self.iso.write_bytes(PAYLOAD)
driver = self.directory / "virtio-win.iso"
driver.write_bytes(b"previous driver")
result = self.run_wizard("/missing.iso", windows=True, overwrite=False)
self.assertEqual(result.returncode, 0, result.stdout + result.stderr)
self.assertEqual(driver.read_bytes(), b"previous driver")
self.assertFalse(self.wget_log.exists())
self.assertIn("-ide3 ", self.qm_calls())
def test_empty_cached_virtio_media_is_retried_not_attached(self):
self.iso.write_bytes(PAYLOAD)
driver = self.directory / "virtio-win.iso"
driver.write_bytes(b"")
result = self.run_wizard("/missing.iso", windows=True, overwrite=False)
self.assertEqual(result.returncode, 0, result.stdout + result.stderr)
self.assertTrue(self.wget_log.exists())
self.assertNotIn("-ide3 ", self.qm_calls())
self.assertIn("ERROR:Failed to download VirtIO driver ISO.", result.stdout)
def test_a_directory_cannot_be_published_as_media(self):
self.iso.mkdir()
result = self.run_wizard()
self.assertNotEqual(result.returncode, 0, result.stdout + result.stderr)
self.assertEqual(self.qm_calls(), "")
self.assertTrue(self.iso.is_dir())
self.assertEqual(list(self.iso.iterdir()), [])
if __name__ == "__main__":
unittest.main()
@@ -0,0 +1,217 @@
"""Offline VM source regressions; execute bounded real functions, never selectors.
Fixtures are reduced official metadata excerpts captured 2026-10-16. Network
commands are shell stubs: no ISO downloads, utils sourcing, or host changes.
"""
import os
from pathlib import Path
import re
import subprocess
import tempfile
import unittest
ROOT = Path(__file__).resolve().parents[3]
LINUX = ROOT / "scripts/vm/select_linux_iso.sh"
NAS = ROOT / "scripts/vm/select_nas_iso.sh"
FIELDS = ("ISO_NAME", "ISO_TYPE", "ISO_URL", "ISO_FILE", "ISO_PATH", "HN")
def extract_function(path, name):
"""Only top-level function definitions with an unindented closing brace."""
match = re.search(
rf"^function {re.escape(name)}\(\) \{{\n.*?^\}}(?=\n|$)",
path.read_text(), re.MULTILINE | re.DOTALL,
)
if match is None:
raise AssertionError(f"Missing bounded function: {name}")
return match.group(0)
class IsoSourceTests(unittest.TestCase):
def run_selector(self, path, function, *, menu_name="", metadata="",
metadata_url="", fetch_status=0, has_curl=True):
with tempfile.TemporaryDirectory(prefix="vm-iso-sources-") as directory:
temp = Path(directory)
fixture = temp / "metadata.html"
fixture.write_text(metadata)
trace = temp / "fetches.txt"
trace.touch()
iso_dir = temp / "iso"
iso_dir.mkdir()
env = os.environ.copy()
env.update(ISO_DIR=str(iso_dir), MENU_NAME=menu_name,
METADATA_FILE=str(fixture), METADATA_URL=metadata_url,
FETCH_STATUS=str(fetch_status), FETCH_TRACE=str(trace))
helpers = "\n".join(
extract_function(NAS, name)
for name in ("_has_curl", "_latest_version_from_lines")
)
script = helpers + r'''
translate() { printf '%s' "$1"; }
curl() {
local url="${@: -1}"
printf '%s\n' "$url" >> "$FETCH_TRACE"
[[ "$url" == "$METADATA_URL" ]] || return 97
[[ "$FETCH_STATUS" == 0 ]] || return "$FETCH_STATUS"
cat "$METADATA_FILE"
}
wget() { return 97; }
dialog() {
local arg previous="" name
for arg in "$@"; do
name="${arg%%│*}"
name="${name%"${name##*[![:space:]]}"}"
if [[ "$name" == "$MENU_NAME" ]]; then
printf '%s' "$previous" >&2
return 0
fi
previous="$arg"
done
return 1
}
'''
if not has_curl:
script += "\n_has_curl() { return 1; }\n"
script += extract_function(path, function)
script += f"\n{function} || exit $?\n"
script += 'printf \'%s\\0\' ' + " ".join(
f'"${{{field}:-}}"' for field in FIELDS
)
result = subprocess.run(
["bash", "--noprofile", "--norc", "-c", script],
env=env, capture_output=True, timeout=10,
)
self.assertEqual(result.returncode, 0, result.stderr.decode())
values = result.stdout.decode().split("\0")
self.assertEqual(len(values), len(FIELDS) + 1, result.stdout)
selected = dict(zip(FIELDS, values))
self.assertEqual(selected["ISO_PATH"], str(iso_dir / selected["ISO_FILE"]))
return selected, trace.read_text().splitlines()
def test_debian13_dvd_uses_fixed_official_archive(self):
selected, calls = self.run_selector(
LINUX, "select_linux_iso_official", menu_name="Debian 13",
)
self.assertEqual(selected["ISO_URL"],
"https://cdimage.debian.org/cdimage/archive/13.6.0/amd64/iso-dvd/"
"debian-13.6.0-amd64-DVD-1.iso")
self.assertEqual(selected["ISO_TYPE"], "Desktop")
self.assertEqual(selected["ISO_FILE"], "debian-13.6.0-amd64-DVD-1.iso")
self.assertEqual(calls, [])
def test_debian13_netinst_uses_fixed_official_archive(self):
selected, calls = self.run_selector(
LINUX, "select_linux_iso_official", menu_name="Debian 13 Netinst",
)
self.assertEqual(selected["ISO_URL"],
"https://cdimage.debian.org/cdimage/archive/13.6.0/amd64/iso-cd/"
"debian-13.6.0-amd64-netinst.iso")
self.assertEqual(selected["ISO_TYPE"], "CLI")
self.assertEqual(selected["ISO_FILE"], "debian-13.6.0-amd64-netinst.iso")
self.assertEqual(calls, [])
def test_omv_metadata_unavailable_uses_verified_fallback(self):
url = "https://sourceforge.net/projects/openmediavault/files/iso/"
for has_curl, fetch_status, metadata in (
(False, 0, ""), (True, 22, ""),
(True, 0, "<html>No release links</html>"),
):
with self.subTest(has_curl=has_curl, fetch_status=fetch_status):
selected, calls = self.run_selector(
NAS, "resolve_omv_iso", metadata_url=url, has_curl=has_curl,
fetch_status=fetch_status, metadata=metadata,
)
self.assertEqual(selected["ISO_URL"],
url + "8.3.1/openmediavault_8.3.1-amd64.iso/download")
self.assertEqual(selected["ISO_NAME"], "OpenMediaVault 8.3.1")
self.assertEqual(selected["ISO_FILE"], "openmediavault_8.3.1-amd64.iso")
self.assertEqual(calls, [url] if has_curl else [])
def test_rockstor_resolves_official_landing_without_directory_listing(self):
# Exact installer anchor excerpt from https://rockstor.com/dls/index.html.
url = "https://rockstor.com/dls/index.html"
metadata = (
'<a href="https://rockstor.com/downloads/installer/leap/15.6/x86_64/'
'Rockstor-Leap15.6-generic.x86_64-5.1.0-0.install.iso">'
'Rockstor-Leap15.6-generic.x86_64-5.1.0-0.install.iso</a> <br/>'
)
selected, calls = self.run_selector(
NAS, "resolve_rockstor_iso", metadata_url=url, metadata=metadata,
)
self.assertEqual(selected["ISO_URL"],
"https://rockstor.com/downloads/installer/leap/15.6/x86_64/"
"Rockstor-Leap15.6-generic.x86_64-5.1.0-0.install.iso")
self.assertEqual(selected["ISO_FILE"],
"Rockstor-Leap15.6-generic.x86_64-5.1.0-0.install.iso")
self.assertEqual(selected["HN"], "Rockstor")
self.assertEqual(calls, [url])
def test_rockstor_metadata_unavailable_uses_verified_fallback(self):
url = "https://rockstor.com/dls/index.html"
for has_curl, fetch_status, metadata in (
(False, 0, ""), (True, 22, ""),
(True, 0, "<html>No installer links</html>"),
):
with self.subTest(has_curl=has_curl, fetch_status=fetch_status):
selected, calls = self.run_selector(
NAS, "resolve_rockstor_iso", metadata_url=url, has_curl=has_curl,
fetch_status=fetch_status, metadata=metadata,
)
self.assertEqual(selected["ISO_URL"],
"https://rockstor.com/downloads/installer/leap/15.6/x86_64/"
"Rockstor-Leap15.6-generic.x86_64-5.1.0-0.install.iso")
self.assertEqual(selected["ISO_NAME"], "Rockstor")
self.assertEqual(selected["ISO_FILE"],
"Rockstor-Leap15.6-generic.x86_64-5.1.0-0.install.iso")
self.assertEqual(calls, [url] if has_curl else [])
def test_omv_current_metadata_resolution_is_preserved(self):
# Reduced release links from the official SourceForge ISO index.
url = "https://sourceforge.net/projects/openmediavault/files/iso/"
metadata = (
'<a href="/projects/openmediavault/files/iso/8.3.1/" '
'title="Click to enter 8.3.1">8.3.1</a>\n'
'<a href="/projects/openmediavault/files/iso/7.4.17/" '
'title="Click to enter 7.4.17">7.4.17</a>'
)
selected, calls = self.run_selector(
NAS, "resolve_omv_iso", metadata_url=url, metadata=metadata,
)
self.assertEqual(selected["ISO_URL"],
url + "8.3.1/openmediavault_8.3.1-amd64.iso/download")
self.assertEqual(calls, [url])
def test_omv_detected_release_takes_precedence_over_fallback(self):
url = "https://sourceforge.net/projects/openmediavault/files/iso/"
# Synthetic ordering case: must execute the real version parser.
metadata = ('<a href="/projects/openmediavault/files/iso/8.10.0/">new</a>'
'<a href="/projects/openmediavault/files/iso/8.9.0/">old</a>')
selected, calls = self.run_selector(
NAS, "resolve_omv_iso", metadata_url=url, metadata=metadata,
)
self.assertEqual(selected["ISO_URL"],
url + "8.10.0/openmediavault_8.10.0-amd64.iso/download")
self.assertEqual(calls, [url])
def test_rockstor_retains_generic_x86_64_leap156_edition(self):
url = "https://rockstor.com/dls/index.html"
# Synthetic ordering / edition cases; not claims of published images.
metadata = "\n".join((
"Rockstor-Leap15.6-generic.x86_64-5.10.0-0.install.iso",
"Rockstor-Leap15.6-generic.x86_64-5.9.0-0.install.iso",
"Rockstor-Leap15.6-generic.aarch64-9.0.0-0.install.iso",
"Rockstor-Leap16.0-generic.x86_64-9.0.0-0.install.iso",
"Rockstor-Leap15.5-generic.x86_64-9.0.0-0.install.iso",
))
selected, calls = self.run_selector(
NAS, "resolve_rockstor_iso", metadata_url=url, metadata=metadata,
)
self.assertEqual(selected["ISO_URL"],
"https://rockstor.com/downloads/installer/leap/15.6/x86_64/"
"Rockstor-Leap15.6-generic.x86_64-5.10.0-0.install.iso")
self.assertEqual(calls, [url])
if __name__ == "__main__":
unittest.main()