From d45da0742dfd7c9d2985c6d6d17140a513a9140b Mon Sep 17 00:00:00 2001 From: VAIO73 <50487331+Vaso73@users.noreply.github.com> Date: Sun, 27 Sep 2026 20:22:55 +0200 Subject: [PATCH 1/3] test(oci): run Glances port contract in isolation --- oci/tests/test_glances_ports.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/oci/tests/test_glances_ports.py b/oci/tests/test_glances_ports.py index 00a8ff33..a89edbbb 100644 --- a/oci/tests/test_glances_ports.py +++ b/oci/tests/test_glances_ports.py @@ -1,15 +1,18 @@ """Glances' Web UI is the only listener used by the native ``-w`` profile.""" from pathlib import Path +import sys import unittest +ROOT = Path(__file__).resolve().parents[1] +sys.path.insert(0, str(ROOT / "src")) + from proxmenux_oci.catalog import Catalog class GlancesPortContractTests(unittest.TestCase): def test_web_profile_exposes_only_the_web_ui_port(self): - root = Path(__file__).resolve().parents[1] - template = Catalog(root).compose("glances") + template = Catalog(ROOT).compose("glances") ports = template["container_contract"]["ports"] self.assertEqual([item["container_port"] for item in ports], [61208]) From ec391246a17133d2cef72912cd3b65271a664b23 Mon Sep 17 00:00:00 2001 From: VAIO73 <50487331+Vaso73@users.noreply.github.com> Date: Sun, 27 Sep 2026 21:14:55 +0200 Subject: [PATCH 2/3] fix(oci): clean up owned host firewall rules --- lang/de.json | 2 ++ lang/es.json | 2 ++ lang/fr.json | 2 ++ lang/it.json | 2 ++ lang/pt.json | 2 ++ lang/sk.json | 2 ++ lang/sv.json | 2 ++ oci/catalog/overlays/netdata.json | 4 +++ oci/remote/install_oci.sh | 14 +++++--- oci/remote/oci_remove.py | 38 ++++++++++++++++++++++ oci/tests/test_host_monitor_firewall.py | 43 +++++++++++++++++++++++++ 11 files changed, 109 insertions(+), 4 deletions(-) diff --git a/lang/de.json b/lang/de.json index 5c976121..905703cb 100644 --- a/lang/de.json +++ b/lang/de.json @@ -2595,6 +2595,8 @@ "Host directory:": "Hostverzeichnis:", "Host firewall": "Host-Firewall", "Host firewall already allows:": "Die Host-Firewall erlaubt bereits:", + "Host firewall rule removed:": "Host-Firewallregel entfernt:", + "The managed host firewall rule could not be removed and was left unchanged.": "Die verwaltete Host-Firewallregel konnte nicht entfernt werden und blieb unverändert.", "Host firewall rule added:": "Host-Firewall-Regel hinzugefügt:", "Host fstab CIFS Mounts:": "Host fstab CIFS-Mounts:", "Host fstab CIFS mounts (not registered with pvesm):": "Host fstab CIFS-Mounts (nicht bei pvesm registriert):", diff --git a/lang/es.json b/lang/es.json index 16ea17e8..9000f850 100644 --- a/lang/es.json +++ b/lang/es.json @@ -2595,6 +2595,8 @@ "Host directory:": "Directorio del host:", "Host firewall": "Cortafuegos del host", "Host firewall already allows:": "El cortafuegos del host ya permite:", + "Host firewall rule removed:": "Regla del cortafuegos del host eliminada:", + "The managed host firewall rule could not be removed and was left unchanged.": "No se pudo eliminar la regla administrada del cortafuegos del host y se dejó sin cambios.", "Host firewall rule added:": "Regla añadida al cortafuegos del host:", "Host fstab CIFS Mounts:": "Montajes CIFS del host fstab:", "Host fstab CIFS mounts (not registered with pvesm):": "Montajes CIFS del host fstab (no registrados con pvesm):", diff --git a/lang/fr.json b/lang/fr.json index f3e1ebda..ea85009d 100644 --- a/lang/fr.json +++ b/lang/fr.json @@ -2595,6 +2595,8 @@ "Host directory:": "Répertoire hôte :", "Host firewall": "Pare-feu hôte", "Host firewall already allows:": "Le pare-feu hôte permet déjà :", + "Host firewall rule removed:": "Règle du pare-feu hôte supprimée :", + "The managed host firewall rule could not be removed and was left unchanged.": "La règle gérée du pare-feu hôte n’a pas pu être supprimée et est restée inchangée.", "Host firewall rule added:": "Règle de pare-feu hôte ajoutée :", "Host fstab CIFS Mounts:": "Hôte des montages fstab CIFS :", "Host fstab CIFS mounts (not registered with pvesm):": "Hôte des montages fstab CIFS (non enregistrés auprès de pvesm) :", diff --git a/lang/it.json b/lang/it.json index f2742cd5..9d184d70 100644 --- a/lang/it.json +++ b/lang/it.json @@ -2595,6 +2595,8 @@ "Host directory:": "Directory host:", "Host firewall": "firewall host", "Host firewall already allows:": "il firewall host consente già:", + "Host firewall rule removed:": "Regola del firewall host rimossa:", + "The managed host firewall rule could not be removed and was left unchanged.": "Non è stato possibile rimuovere la regola gestita del firewall host ed è rimasta invariata.", "Host firewall rule added:": "Aggiunta regola del firewall host:", "Host fstab CIFS Mounts:": "Montaggi CIFS host fstab:", "Host fstab CIFS mounts (not registered with pvesm):": "Montaggi CIFS host fstab (non registrato con pvesm):", diff --git a/lang/pt.json b/lang/pt.json index c554db6b..e0e5f962 100644 --- a/lang/pt.json +++ b/lang/pt.json @@ -2595,6 +2595,8 @@ "Host directory:": "Diretório de host:", "Host firewall": "Firewall de host", "Host firewall already allows:": "O firewall do host já permite:", + "Host firewall rule removed:": "Regra do firewall do host removida:", + "The managed host firewall rule could not be removed and was left unchanged.": "Não foi possível remover a regra gerida do firewall do host, que permaneceu inalterada.", "Host firewall rule added:": "regra de firewall do host adicionada:", "Host fstab CIFS Mounts:": "Host fstab montagens CIFS:", "Host fstab CIFS mounts (not registered with pvesm):": "Host fstab montagens CIFS (não registradas no pvesm):", diff --git a/lang/sk.json b/lang/sk.json index f725dc76..81f70d0f 100644 --- a/lang/sk.json +++ b/lang/sk.json @@ -2595,6 +2595,8 @@ "Host directory:": "Priečinok hosta:", "Host firewall": "Firewall hostiteľa", "Host firewall already allows:": "Firewall hostiteľa už povoľuje:", + "Host firewall rule removed:": "Pravidlo firewallu hostiteľa odstránené:", + "The managed host firewall rule could not be removed and was left unchanged.": "Spravované pravidlo firewallu hostiteľa sa nepodarilo odstrániť a zostalo bez zmeny.", "Host firewall rule added:": "Pravidlo firewallu hostiteľa bolo pridané:", "Host fstab CIFS Mounts:": "CIFS mounty v host fstab:", "Host fstab CIFS mounts (not registered with pvesm):": "CIFS mounty v host fstab (nie sú registrované cez pvesm):", diff --git a/lang/sv.json b/lang/sv.json index e2b414e6..65b18fd4 100644 --- a/lang/sv.json +++ b/lang/sv.json @@ -2595,6 +2595,8 @@ "Host directory:": "Värdkatalog:", "Host firewall": "Värdbrandvägg", "Host firewall already allows:": "Värdbrandväggen tillåter redan:", + "Host firewall rule removed:": "Värdbrandväggsregel borttagen:", + "The managed host firewall rule could not be removed and was left unchanged.": "Den hanterade värdbrandväggsregeln kunde inte tas bort och lämnades oförändrad.", "Host firewall rule added:": "Värdbrandväggsregel har lagts till:", "Host fstab CIFS Mounts:": "Värdens CIFS-monteringar i fstab:", "Host fstab CIFS mounts (not registered with pvesm):": "Värdens CIFS-monteringar i fstab (inte registrerade med pvesm):", diff --git a/oci/catalog/overlays/netdata.json b/oci/catalog/overlays/netdata.json index 24ef5646..92c9248f 100644 --- a/oci/catalog/overlays/netdata.json +++ b/oci/catalog/overlays/netdata.json @@ -81,6 +81,10 @@ }, "installer_profile": { "host_monitor": "netdata", + "host_monitor_firewall": { + "protocol": "tcp", + "web_port": 19999 + }, "host_monitor_mounts": [ { "source": "/proc", diff --git a/oci/remote/install_oci.sh b/oci/remote/install_oci.sh index 1b1f3cc6..7eda3725 100755 --- a/oci/remote/install_oci.sh +++ b/oci/remote/install_oci.sh @@ -259,7 +259,9 @@ apply_host_monitor_firewall() { fi local node comment rules existing node=$(hostname) - comment="ProxMenux OCI host monitor CT ${VMID}" + # This UUID makes removal safe even after a VMID is reused. Only a rule + # carrying this exact marker belongs to this installation. + comment="ProxMenux OCI firewall ${INSTANCE_ID}" rules=$(pvesh get "/nodes/${node}/firewall/rules" --output-format json) \ || die "$(translate "Could not read the host firewall rules")" existing=$(jq -r --arg source "$HOST_FIREWALL_SOURCE" --arg port "$HOST_FIREWALL_PORT" ' @@ -270,9 +272,13 @@ apply_host_monitor_firewall() { msg_ok "$(translate "Host firewall already allows:") TCP ${HOST_FIREWALL_PORT} $(translate "from") ${HOST_FIREWALL_SOURCE}" return 0 fi - pvesh create "/nodes/${node}/firewall/rules" --type in --action ACCEPT --proto tcp \ - --dport "$HOST_FIREWALL_PORT" --source "$HOST_FIREWALL_SOURCE" --comment "$comment" \ - || die "$(translate "Could not add the confirmed host firewall rule")" + if ! pvesh create "/nodes/${node}/firewall/rules" --type in --action ACCEPT --proto tcp \ + --dport "$HOST_FIREWALL_PORT" --source "$HOST_FIREWALL_SOURCE" --enable 1 --comment "$comment"; then + # The CT and its saved contract are already complete. Do not destroy a + # usable installation merely because the optional network exposure failed. + msg_warn "$(translate "Could not add the confirmed host firewall rule")" + return 0 + fi msg_ok "$(translate "Host firewall rule added:") TCP ${HOST_FIREWALL_PORT} $(translate "from") ${HOST_FIREWALL_SOURCE}" } diff --git a/oci/remote/oci_remove.py b/oci/remote/oci_remove.py index 7fd83d40..33fa9f30 100644 --- a/oci/remote/oci_remove.py +++ b/oci/remote/oci_remove.py @@ -104,6 +104,43 @@ def release_bridge(bridge): subprocess.run(['pvesh', 'delete', f'/nodes/{node}/network/{bridge}'], check=False, capture_output=True) +def remove_owned_host_firewall(record): + """Remove only the narrowly scoped rule created by this installation. + + A matching port alone is never evidence of ownership: administrators and + other applications may legitimately use it. Older CT-number comments are + deliberately left alone as well. + """ + plan = record.get('deployment', {}).get('host_firewall') or {} + installation_id = record.get('installation_id', '') + if not isinstance(plan, dict) or not re.fullmatch(r'[0-9a-f-]{36}', installation_id): + return + source, port = plan.get('source'), plan.get('port') + if not isinstance(source, str) or not isinstance(port, int): + return + comment = f'ProxMenux OCI firewall {installation_id}' + node = socket.gethostname().split('.', 1)[0] + try: + result = subprocess.run(['pvesh', 'get', f'/nodes/{node}/firewall/rules', '--output-format', 'json'], + check=True, capture_output=True, text=True) + rules = json.loads(result.stdout) + matches = [rule for rule in rules if rule.get('comment') == comment + and str(rule.get('dport')) == str(port) + and rule.get('source') == source + and str(rule.get('proto', '')).lower() == 'tcp' + and str(rule.get('type', '')).lower() == 'in' + and str(rule.get('action', '')).upper() == 'ACCEPT'] + if len(matches) != 1 or not isinstance(matches[0].get('pos'), int): + return + subprocess.run(['pvesh', 'delete', f"/nodes/{node}/firewall/rules/{matches[0]['pos']}"], + check=True, capture_output=True) + msg_ok(f"{translate('Host firewall rule removed:')} TCP {port} {translate('from')} {source}") + except (OSError, ValueError, subprocess.CalledProcessError, json.JSONDecodeError): + # Removal already destroyed the CT. A firewall API failure must not + # turn that successful lifecycle operation into a failed one. + msg_warn(translate('The managed host firewall rule could not be removed and was left unchanged.')) + + def remove(root, vmid): primary_id, primary, members = members_of(root, vmid) for member in members: @@ -132,6 +169,7 @@ def remove(root, vmid): msg_ok(f"{translate('Private network of the application released:')} {bridge}") elif bridge: msg_warn(f"{translate('The private network is still used by another container and is kept:')} {bridge}") + remove_owned_host_firewall(primary) lifecycle = Path(f'/etc/pve/priv/proxmenux-stack-{primary_id}.json') if lifecycle.exists() and not lifecycle.is_symlink(): lifecycle.unlink() diff --git a/oci/tests/test_host_monitor_firewall.py b/oci/tests/test_host_monitor_firewall.py index 7b5f84c5..9dfada4f 100644 --- a/oci/tests/test_host_monitor_firewall.py +++ b/oci/tests/test_host_monitor_firewall.py @@ -4,13 +4,17 @@ from pathlib import Path import sys import unittest from unittest.mock import patch +from subprocess import CompletedProcess +import json ROOT = Path(__file__).resolve().parents[1] sys.path.insert(0, str(ROOT / "src")) +sys.path.insert(0, str(ROOT / "remote")) from proxmenux_oci.installer import (InstallError, confirm_host_monitor_firewall, host_monitor_firewall_plan) +import oci_remove class ConfirmUI: @@ -55,13 +59,52 @@ class HostMonitorFirewallPlanTests(unittest.TestCase): class HostMonitorFirewallInstallerContractTests(unittest.TestCase): + def test_netdata_declares_its_host_web_port_for_the_firewall_plan(self): + catalog = json.loads((ROOT / "catalog" / "overlays" / "netdata.json").read_text(encoding="utf-8")) + profile = catalog["proxmox"]["installer_profile"] + self.assertEqual(profile["host_monitor"], "netdata") + self.assertEqual(profile["host_monitor_firewall"], + {"protocol": "tcp", "web_port": 19999}) + def test_remote_installer_revalidates_and_uses_proxmox_rule_api(self): source = (ROOT / "remote" / "install_oci.sh").read_text(encoding="utf-8") self.assertIn("validate_host_monitor_firewall", source) self.assertIn("The host-monitor firewall subnet changed; no firewall rule was added", source) self.assertIn('pvesh create "/nodes/${node}/firewall/rules"', source) self.assertIn("--dport \"$HOST_FIREWALL_PORT\" --source \"$HOST_FIREWALL_SOURCE\"", source) + self.assertIn('--enable 1 --comment "$comment"', source) self.assertIn("only the original, separately confirmed installation", source) + self.assertIn('comment="ProxMenux OCI firewall ${INSTANCE_ID}"', source) + self.assertIn('if ! pvesh create "/nodes/${node}/firewall/rules"', source) + + def test_removal_only_targets_a_uniquely_owned_firewall_rule(self): + source = (ROOT / "remote" / "oci_remove.py").read_text(encoding="utf-8") + self.assertIn("def remove_owned_host_firewall(record):", source) + self.assertIn("ProxMenux OCI firewall {installation_id}", source) + self.assertIn("len(matches) != 1", source) + self.assertIn("firewall/rules/{matches[0]['pos']}", source) + + def test_removal_deletes_only_the_exact_rule_owned_by_the_installation(self): + record = {"installation_id": "123e4567-e89b-12d3-a456-426614174000", + "deployment": {"host_firewall": {"source": "192.0.2.0/24", "port": 61208}}} + rules = [{"pos": 7, "comment": "ProxMenux OCI firewall 123e4567-e89b-12d3-a456-426614174000", + "type": "in", "action": "ACCEPT", "proto": "tcp", "source": "192.0.2.0/24", + "dport": "61208"}, + {"pos": 8, "comment": "manual rule", "type": "in", "action": "ACCEPT", + "proto": "tcp", "source": "192.0.2.0/24", "dport": "61208"}] + with patch("oci_remove.subprocess.run", side_effect=[ + CompletedProcess([], 0, json.dumps(rules), ""), CompletedProcess([], 0, "", "")]) as run: + oci_remove.remove_owned_host_firewall(record) + self.assertIn("/firewall/rules/7", run.call_args_list[1].args[0][-1]) + + def test_removal_keeps_an_unowned_matching_rule(self): + record = {"installation_id": "123e4567-e89b-12d3-a456-426614174000", + "deployment": {"host_firewall": {"source": "192.0.2.0/24", "port": 61208}}} + rules = [{"pos": 8, "comment": "manual rule", "type": "in", "action": "ACCEPT", + "proto": "tcp", "source": "192.0.2.0/24", "dport": "61208"}] + with patch("oci_remove.subprocess.run", return_value=CompletedProcess([], 0, json.dumps(rules), "")) as run: + oci_remove.remove_owned_host_firewall(record) + run.assert_called_once() def test_oci_menu_wrapper_does_not_hide_engine_errors_with_the_main_menu(self): wrapper = (ROOT.parent / "scripts" / "oci" / "oci_manager_apps.sh").read_text(encoding="utf-8") From f5627afe27deea35b5aa8e3912bb3d6e517cceb7 Mon Sep 17 00:00:00 2001 From: VAIO73 <50487331+Vaso73@users.noreply.github.com> Date: Sun, 27 Sep 2026 22:04:52 +0200 Subject: [PATCH 3/3] test(i18n): stub OCI firewall cleanup in wording test --- .github/scripts/tests/test_oci_recovery_inventory_wording.py | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/scripts/tests/test_oci_recovery_inventory_wording.py b/.github/scripts/tests/test_oci_recovery_inventory_wording.py index dd61ecac..122f5ead 100644 --- a/.github/scripts/tests/test_oci_recovery_inventory_wording.py +++ b/.github/scripts/tests/test_oci_recovery_inventory_wording.py @@ -74,6 +74,7 @@ class InventoryMessages(unittest.TestCase): 'guest_config': lambda vmid: b'description: owned', 'host_directories': lambda root, members: ['/bind/saved'], 'private_bridge': lambda primary: None, + 'remove_owned_host_firewall': lambda primary: None, 'run': lambda *args: events.append(('run', args)), 'subprocess': SimpleNamespace(run=lambda *args, **kwargs: None), 'Path': Path, 'shutil': SimpleNamespace(rmtree=lambda path: None),