fix(monitor): write Proxmox's notification settings only while /etc/pve is mounted

- Nothing is written under /etc/pve unless it is the mounted cluster
  filesystem; a Monitor that starts while pve-cluster is being restarted
  waits for it and registers its webhook afterwards.
- The Monitor service is ordered after pve-cluster.service.
- The settings are written only when they have to change. The copies
  taken before a change are kept in
  /usr/local/share/proxmenux/backups/pve-notifications, and the ones left
  in /etc/pve by earlier versions are moved there.
- Uninstalling the Monitor removes its target and matcher from Proxmox.

Reported in #427.
This commit is contained in:
MacRimi
2026-10-08 17:14:36 +02:00
parent 053b6c81f8
commit 07713d5365
10 changed files with 355 additions and 83 deletions
+146 -74
View File
@@ -895,6 +895,20 @@ def _pve_webhook_url() -> str:
_PVE_WEBHOOK_URL = _pve_webhook_url()
_PVE_NOT_MOUNTED = 'The Proxmox cluster filesystem (/etc/pve) is not mounted'
def _pve_cluster_fs_mounted() -> bool:
"""Whether /etc/pve is the Proxmox cluster filesystem.
While its service is stopped or restarting, /etc/pve is an empty
directory of the root disk. A file written there is not a configuration
file: it keeps the service from mounting the filesystem again.
"""
import os
return os.path.ismount(os.path.dirname(_PVE_NOTIFICATIONS_CFG))
def _pve_read_file(path):
"""Read file, return (content, error). Content is '' if missing."""
try:
@@ -909,26 +923,47 @@ def _pve_read_file(path):
_PVE_BACKUPS_KEPT = 3
# Copies of Proxmox's notification settings, taken before changing them. They
# are kept with ProxMenux, not next to the files: /etc/pve belongs to Proxmox
# and is replicated to every node of a cluster.
_PVE_BACKUP_DIR = '/usr/local/share/proxmenux/backups/pve-notifications'
_PVE_BACKUP_MARK = '.proxmenux_backup_'
def _pve_backup_prefix(path):
"""Name the copies of a file are kept under. The private file carries
its own name, as both are called notifications.cfg."""
import os
name = os.path.basename(path)
return ('priv-' if path == _PVE_PRIV_CFG else '') + name + _PVE_BACKUP_MARK
def _pve_backups(path):
"""The copies kept of a file, newest first."""
import os
prefix = _pve_backup_prefix(path)
try:
names = os.listdir(_PVE_BACKUP_DIR)
except OSError:
return []
found = [os.path.join(_PVE_BACKUP_DIR, name) for name in names if name.startswith(prefix)]
return sorted(found, key=os.path.getmtime, reverse=True)
def _pve_backup_file(path):
"""Create timestamped backup if file exists. Never fails fatally.
"""Keep a timestamped copy of the file if it exists. Never fails fatally.
The backups live next to the file, inside the cluster filesystem, whose
size is limited: a copy identical to the newest one is not written again
and only the newest few are kept. Restore uses the newest.
A copy identical to the newest one is not written again and only the
newest few are kept. Restore uses the newest.
"""
import os, shutil
from datetime import datetime
try:
if not os.path.exists(path):
return
directory, name = os.path.split(path)
prefix = f"{name}.proxmenux_backup_"
existing = sorted(
(os.path.join(directory, f) for f in os.listdir(directory) if f.startswith(prefix)),
key=os.path.getmtime, reverse=True,
)
os.makedirs(_PVE_BACKUP_DIR, mode=0o700, exist_ok=True)
os.chmod(_PVE_BACKUP_DIR, 0o700)
existing = _pve_backups(path)
with open(path, 'rb') as f:
current = f.read()
newest_matches = False
@@ -940,16 +975,48 @@ def _pve_backup_file(path):
pass
if not newest_matches:
ts = datetime.now().strftime('%Y%m%d_%H%M%S')
shutil.copy2(path, os.path.join(directory, prefix + ts))
existing = sorted(
(os.path.join(directory, f) for f in os.listdir(directory) if f.startswith(prefix)),
key=os.path.getmtime, reverse=True,
)
for stale in existing[_PVE_BACKUPS_KEPT:]:
try:
os.remove(stale)
except OSError:
pass
destination = os.path.join(_PVE_BACKUP_DIR, _pve_backup_prefix(path) + ts)
shutil.copy2(path, destination)
# The private file holds the secrets of every notification target.
os.chmod(destination, 0o600)
_pve_prune_backups(path)
except Exception:
pass
def _pve_prune_backups(path):
"""Keep only the newest copies of a file."""
import os
for stale in _pve_backups(path)[_PVE_BACKUPS_KEPT:]:
try:
os.remove(stale)
except OSError:
pass
def _pve_collect_legacy_backups(path):
"""Earlier versions left the copies next to the file, inside /etc/pve.
The newest ones are moved with the rest and none is left there."""
import os, shutil
try:
directory, name = os.path.split(path)
legacy = sorted(
(os.path.join(directory, f) for f in os.listdir(directory) if f.startswith(name + _PVE_BACKUP_MARK)),
key=os.path.getmtime, reverse=True,
)
if not legacy:
return
os.makedirs(_PVE_BACKUP_DIR, mode=0o700, exist_ok=True)
os.chmod(_PVE_BACKUP_DIR, 0o700)
for position, source in enumerate(legacy):
if position < _PVE_BACKUPS_KEPT:
stamp = os.path.basename(source)[len(name + _PVE_BACKUP_MARK):]
destination = os.path.join(_PVE_BACKUP_DIR, _pve_backup_prefix(path) + stamp)
if not os.path.exists(destination):
shutil.copy2(source, destination)
os.chmod(destination, 0o600)
os.remove(source)
_pve_prune_backups(path)
except Exception:
pass
@@ -1091,6 +1158,11 @@ def setup_pve_webhook_core() -> dict:
}
try:
if not _pve_cluster_fs_mounted():
result['error'] = _PVE_NOT_MOUNTED
result['waiting'] = True
return result
# ── Step 1: Ensure webhook secret exists (for our own internal use) ──
secret = notification_manager.get_webhook_secret()
if not secret:
@@ -1109,11 +1181,10 @@ def setup_pve_webhook_core() -> dict:
if err:
priv_text = None
# ── Step 4: Create backups before ANY modification ──
_pve_backup_file(_PVE_NOTIFICATIONS_CFG)
if priv_text is not None:
_pve_backup_file(_PVE_PRIV_CFG)
# ── Step 4: Backups are taken below, only of a file about to change ──
_pve_collect_legacy_backups(_PVE_NOTIFICATIONS_CFG)
_pve_collect_legacy_backups(_PVE_PRIV_CFG)
# ── Step 5: Remove any previous proxmenux blocks from BOTH files ──
cleaned_cfg = _pve_remove_our_blocks(cfg_text, _PVE_OUR_HEADERS)
@@ -1161,10 +1232,12 @@ def setup_pve_webhook_core() -> dict:
new_cfg = cleaned_cfg + endpoint_block + '\n' + matcher_block
# ── Step 8: Write main config ──
# ── Step 8: Write main config, when it is not already as wanted ──
try:
with open(_PVE_NOTIFICATIONS_CFG, 'w') as f:
f.write(new_cfg)
if new_cfg != cfg_text:
_pve_backup_file(_PVE_NOTIFICATIONS_CFG)
with open(_PVE_NOTIFICATIONS_CFG, 'w') as f:
f.write(new_cfg)
except PermissionError:
result['error'] = f'Permission denied writing {_PVE_NOTIFICATIONS_CFG}'
result['fallback_commands'] = _build_webhook_fallback()
@@ -1214,8 +1287,11 @@ def setup_pve_webhook_core() -> dict:
new_priv = priv_block
try:
with open(_PVE_PRIV_CFG, 'w') as f:
f.write(new_priv)
if new_priv != priv_text:
if priv_text is not None:
_pve_backup_file(_PVE_PRIV_CFG)
with open(_PVE_PRIV_CFG, 'w') as f:
f.write(new_priv)
except PermissionError:
result['error'] = f'Permission denied writing {_PVE_PRIV_CFG}'
result['fallback_commands'] = _build_webhook_fallback()
@@ -1238,6 +1314,17 @@ def setup_pve_webhook_core() -> dict:
return result
def setup_pve_webhook_when_mounted(timeout=900, interval=5) -> dict:
"""Configure the webhook once /etc/pve is mounted. For the Monitor that
starts while the cluster service is still starting or being restarted."""
deadline = time.monotonic() + timeout
while not _pve_cluster_fs_mounted():
if time.monotonic() >= deadline:
return {'configured': False, 'error': _PVE_NOT_MOUNTED, 'waiting': True}
time.sleep(interval)
return setup_pve_webhook_core()
@notification_bp.route('/api/notifications/proxmox/setup-webhook', methods=['POST'])
@require_auth
def setup_proxmox_webhook():
@@ -1254,6 +1341,10 @@ def cleanup_pve_webhook_core() -> dict:
result = {'cleaned': False, 'error': None}
try:
if not _pve_cluster_fs_mounted():
result['error'] = _PVE_NOT_MOUNTED
return result
# Read both files
cfg_text, err = _pve_read_file(_PVE_NOTIFICATIONS_CFG)
if err:
@@ -1336,10 +1427,6 @@ def read_pve_notification_cfg():
'priv_cfg': '/etc/pve/priv/notifications.cfg',
}
# Also look for any backups we created
backup_dir = '/etc/pve'
priv_backup_dir = '/etc/pve/priv'
result = {}
for key, path in files.items():
try:
@@ -1357,24 +1444,18 @@ def read_pve_notification_cfg():
except Exception as e:
result[key] = {'path': path, 'content': None, 'size': 0, 'error': str(e)}
# Find backups
# The copies kept of both files
backups = []
for d in [backup_dir, priv_backup_dir]:
for fpath in _pve_backups(_PVE_NOTIFICATIONS_CFG) + _pve_backups(_PVE_PRIV_CFG):
try:
for fname in sorted(os.listdir(d)):
if 'proxmenux_backup' in fname:
fpath = os.path.join(d, fname)
try:
with open(fpath, 'r') as f:
backups.append({
'path': fpath,
'content': f.read(),
'size': os.path.getsize(fpath),
})
except Exception:
backups.append({'path': fpath, 'content': None, 'error': 'read_failed'})
with open(fpath, 'r') as f:
backups.append({
'path': fpath,
'content': f.read(),
'size': os.path.getsize(fpath),
})
except Exception:
pass
backups.append({'path': fpath, 'content': None, 'error': 'read_failed'})
result['backups'] = backups
return jsonify(result), 200
@@ -1391,35 +1472,26 @@ def restore_pve_notification_cfg():
import os
import shutil
files_to_restore = {
'/etc/pve': '/etc/pve/notifications.cfg',
'/etc/pve/priv': '/etc/pve/priv/notifications.cfg',
}
restored = []
errors = []
for search_dir, target_path in files_to_restore.items():
try:
# Pick the most recent backup by mtime, not lexicographic name.
# An attacker (or accidental rename) with a write primitive
# could craft `notifications.cfg.proxmenux_backup_99999999_999999`
# and have it sort first, hijacking the restore. mtime tracks
# the actual file age so renamed/touched files don't fool us.
# Audit Tier 3.1 — restore-cfg sort lexicográfico.
candidates = [
f for f in os.listdir(search_dir)
if 'proxmenux_backup' in f and f.startswith('notifications.cfg')
]
if not _pve_cluster_fs_mounted():
return jsonify({'restored': [], 'errors': [{'target': '/etc/pve', 'error': _PVE_NOT_MOUNTED}],
'success': False}), 200
for target_path in (_PVE_NOTIFICATIONS_CFG, _PVE_PRIV_CFG):
try:
_pve_collect_legacy_backups(target_path)
# The newest by modification time: a name can be crafted to sort
# first, a file's age cannot.
candidates = _pve_backups(target_path)
if candidates:
candidates.sort(
key=lambda f: os.path.getmtime(os.path.join(search_dir, f)),
reverse=True,
)
backup_path = os.path.join(search_dir, candidates[0])
shutil.copy2(backup_path, target_path)
restored.append({'target': target_path, 'from_backup': backup_path})
# Content only: the cluster filesystem sets its own permissions.
with open(candidates[0], 'rb') as source:
content = source.read()
with open(target_path, 'wb') as target:
target.write(content)
restored.append({'target': target_path, 'from_backup': candidates[0]})
else:
errors.append({'target': target_path, 'error': 'no_backup_found'})
except Exception as e:
+17
View File
@@ -1141,6 +1141,11 @@ class NotificationManager:
wh_result = setup_pve_webhook_core()
if wh_result.get('configured'):
print("[NotificationManager] PVE webhook configured OK.")
elif wh_result.get('waiting'):
# /etc/pve is not mounted yet: nothing is written until it is.
print("[NotificationManager] PVE webhook: waiting for /etc/pve to be mounted.")
threading.Thread(target=self._configure_pve_webhook_when_mounted, daemon=True,
name='pve-webhook-setup').start()
elif wh_result.get('error'):
print(f"[NotificationManager] PVE webhook warning: {wh_result['error']}")
except ImportError:
@@ -2285,6 +2290,18 @@ class NotificationManager:
'auth_fail',
)
def _configure_pve_webhook_when_mounted(self):
"""Configure the PVE webhook once the cluster filesystem is back."""
try:
from flask_notification_routes import setup_pve_webhook_when_mounted
result = setup_pve_webhook_when_mounted()
if result.get('configured'):
print("[NotificationManager] PVE webhook configured OK.")
elif result.get('error'):
print(f"[NotificationManager] PVE webhook warning: {result['error']}")
except Exception as e:
print(f"[NotificationManager] PVE webhook setup error: {e}")
def _reset_cooldowns_on_start(self):
"""Clear DB rows in notification_last_sent for the curated set of
event types listed in `_EVENT_TYPES_RESET_ON_START`.
@@ -0,0 +1,170 @@
"""Proxmox's notification files are only written while /etc/pve is the
mounted cluster filesystem, and only when their content has to change."""
import ast
import os
import tempfile
import unittest
from pathlib import Path
from unittest import mock
SCRIPTS = Path(__file__).resolve().parents[1]
SOURCE = SCRIPTS / 'flask_notification_routes.py'
NAMES = {'_pve_cluster_fs_mounted', '_pve_read_file', '_pve_backup_file', '_pve_prune_backups', '_pve_backup_prefix',
'_pve_backups', '_pve_collect_legacy_backups', 'restore_pve_notification_cfg', '_pve_remove_our_blocks', '_pve_webhook_url',
'_build_webhook_fallback', 'setup_pve_webhook_core', 'cleanup_pve_webhook_core', 'setup_pve_webhook_when_mounted'}
ASSIGNED = {'_PVE_ENDPOINT_ID', '_PVE_MATCHER_ID', '_PVE_OUR_HEADERS', '_PVE_NOT_MOUNTED', '_PVE_BACKUPS_KEPT',
'_PVE_HEADER_RE', 'WEBHOOK_LOOPBACK_PORT', '_PVE_BACKUP_MARK'}
class FakeManager:
def __init__(self):
self.secret = 'c2VjcmV0LXRva2Vu'
def get_webhook_secret(self):
return self.secret
def _save_setting(self, _key, value):
self.secret = value
def load(directory):
"""The functions under test, pointed at a directory standing for /etc/pve."""
body = []
for node in ast.parse(SOURCE.read_text()).body:
if isinstance(node, ast.FunctionDef) and node.name in NAMES:
node.decorator_list = []
body.append(node)
elif isinstance(node, ast.Assign) and any(getattr(t, 'id', None) in ASSIGNED for t in node.targets):
body.append(node)
elif isinstance(node, ast.Import) and any(a.asname == '_re_pve_cfg' for a in node.names):
body.append(node)
import time
scope = {'notification_manager': FakeManager(), 'time': time, 'jsonify': lambda value: value,
'_PVE_NOTIFICATIONS_CFG': str(directory / 'notifications.cfg'),
'_PVE_PRIV_CFG': str(directory / 'priv' / 'notifications.cfg'),
'_PVE_BACKUP_DIR': str(directory.parent / 'proxmenux' / 'backups' / 'pve-notifications')}
exec(compile(ast.fix_missing_locations(ast.Module(body=body, type_ignores=[])), str(SOURCE), 'exec'), scope)
return scope
class PveNotificationWriteTests(unittest.TestCase):
def setUp(self):
tmp = tempfile.TemporaryDirectory()
self.addCleanup(tmp.cleanup)
self.pve = Path(tmp.name) / 'pve'
self.pve.mkdir()
self.scope = load(self.pve)
self.kept = Path(self.scope['_PVE_BACKUP_DIR'])
self.mounted = True
patcher = mock.patch('os.path.ismount', side_effect=lambda path: self.mounted and path == str(self.pve))
patcher.start()
self.addCleanup(patcher.stop)
def files(self):
return sorted(str(p.relative_to(self.pve)) for p in self.pve.rglob('*') if p.is_file())
def test_nothing_is_written_while_the_cluster_filesystem_is_down(self):
# The bare directory of the root disk: empty, and it has to stay so.
self.mounted = False
result = self.scope['setup_pve_webhook_core']()
self.assertFalse(result['configured'])
self.assertTrue(result['waiting'])
self.assertEqual(self.scope['cleanup_pve_webhook_core']()['error'], self.scope['_PVE_NOT_MOUNTED'])
self.assertEqual(list(self.pve.iterdir()), [])
def test_the_webhook_is_written_once_and_not_again(self):
(self.pve / 'priv').mkdir()
existing = 'sendmail: mail-to-root\n\tcomment Send mails to root@pam\n\tmailto-user root@pam\n'
(self.pve / 'notifications.cfg').write_text(existing)
(self.pve / 'priv' / 'notifications.cfg').write_text('')
self.assertTrue(self.scope['setup_pve_webhook_core']()['configured'])
written = (self.pve / 'notifications.cfg').read_text()
self.assertTrue(written.startswith(existing))
self.assertIn('webhook: proxmenux-webhook\n', written)
self.assertIn('matcher: proxmenux-default\n', written)
first = {name: (self.pve / name).stat().st_mtime_ns for name in self.files()}
# The copies taken before the change are kept with ProxMenux, not in /etc/pve.
self.assertEqual(sorted(first), ['notifications.cfg', 'priv/notifications.cfg'])
copies = sorted(p.name.split('.proxmenux_backup_')[0] for p in self.kept.iterdir())
self.assertEqual(copies, ['notifications.cfg', 'priv-notifications.cfg'])
self.assertEqual({oct(p.stat().st_mode & 0o777) for p in self.kept.iterdir()}, {'0o600'})
self.assertEqual(oct(self.kept.stat().st_mode & 0o777), '0o700')
# A second start finds both files as wanted: no write, no new backup.
os.utime(self.pve / 'notifications.cfg', ns=(1, 1))
os.utime(self.pve / 'priv' / 'notifications.cfg', ns=(1, 1))
self.assertTrue(self.scope['setup_pve_webhook_core']()['configured'])
self.assertEqual(self.files(), sorted(first))
self.assertEqual((self.pve / 'notifications.cfg').stat().st_mtime_ns, 1)
self.assertEqual((self.pve / 'priv' / 'notifications.cfg').stat().st_mtime_ns, 1)
def test_copies_left_in_etc_pve_by_an_earlier_version_are_collected_without_a_write(self):
(self.pve / 'priv').mkdir()
(self.pve / 'priv' / 'notifications.cfg').write_text('')
(self.pve / 'notifications.cfg').write_text('')
self.scope['setup_pve_webhook_core']()
for item in self.kept.iterdir():
item.unlink()
for index in range(40):
for folder in (self.pve, self.pve / 'priv'):
stale = folder / f'notifications.cfg.proxmenux_backup_20260101_{index:06d}'
stale.write_text(f'old {index}')
os.utime(stale, ns=(index * 10 ** 9, index * 10 ** 9))
current = (self.pve / 'notifications.cfg').read_text()
self.assertTrue(self.scope['setup_pve_webhook_core']()['configured'])
self.assertEqual(self.files(), ['notifications.cfg', 'priv/notifications.cfg'])
self.assertEqual((self.pve / 'notifications.cfg').read_text(), current)
kept = sorted(p.name for p in self.kept.iterdir())
self.assertEqual(kept, [f'notifications.cfg.proxmenux_backup_20260101_{i:06d}' for i in (37, 38, 39)]
+ [f'priv-notifications.cfg.proxmenux_backup_20260101_{i:06d}' for i in (37, 38, 39)])
self.assertEqual((self.kept / kept[2]).read_text(), 'old 39')
def test_restore_takes_the_newest_copy_of_each_file(self):
(self.pve / 'priv').mkdir()
(self.pve / 'notifications.cfg').write_text('before\n')
(self.pve / 'priv' / 'notifications.cfg').write_text('private before\n')
self.scope['setup_pve_webhook_core']()
answer, _status = self.scope['restore_pve_notification_cfg']()
self.assertTrue(answer['success'])
self.assertEqual((self.pve / 'notifications.cfg').read_text(), 'before\n')
self.assertEqual((self.pve / 'priv' / 'notifications.cfg').read_text(), 'private before\n')
self.mounted = False
answer, _status = self.scope['restore_pve_notification_cfg']()
self.assertFalse(answer['success'])
def test_uninstalling_the_monitor_removes_its_target_from_proxmox(self):
menu = (SCRIPTS.parents[1] / 'scripts/menus/config_menu.sh').read_text()
body = menu[menu.index('remove_monitor_notification_target() {'):menu.index('uninstall_proxmenux_monitor() {')]
self.assertIn('mountpoint -q /etc/pve || return 0', body)
# The rule first: Proxmox does not delete a target that a rule still uses.
self.assertLess(body.index('matchers/proxmenux-default'), body.index('endpoints/webhook/proxmenux-webhook'))
self.assertIn('/etc/pve/priv/notifications.cfg.proxmenux_backup_*', body)
uninstall = menu[menu.index('uninstall_proxmenux_monitor() {'):menu.index('check_monitor_status() {')]
self.assertIn('remove_monitor_notification_target', uninstall)
def test_a_start_during_a_restart_waits_for_the_mount(self):
self.mounted = False
(self.pve / 'priv').mkdir()
def back(_seconds):
self.mounted = True
with mock.patch('time.sleep', side_effect=back):
result = self.scope['setup_pve_webhook_when_mounted'](timeout=60, interval=1)
self.assertTrue(result['configured'])
self.assertIn('webhook: proxmenux-webhook', (self.pve / 'notifications.cfg').read_text())
def test_the_wait_ends_without_writing(self):
self.mounted = False
result = self.scope['setup_pve_webhook_when_mounted'](timeout=0, interval=0)
self.assertTrue(result['waiting'])
self.assertEqual(list(self.pve.iterdir()), [])
def test_the_monitor_starts_after_the_cluster_service(self):
root = SCRIPTS.parents[1]
for name in ('install_proxmenux.sh', 'install_proxmenux_beta.sh', 'scripts/menus/config_menu.sh'):
text = (root / name).read_text()
self.assertIn('After=network.target pve-cluster.service', text, name)
self.assertNotIn('After=network.target\n\n[Service]\nType=simple\nUser=root\nWorkingDirectory=$MONITOR', text, name)
if __name__ == '__main__':
unittest.main()