From de0b1ee4db92bab0433533fd6dbfb86c6268a8b8 Mon Sep 17 00:00:00 2001 From: root Date: Fri, 21 Aug 2026 02:00:07 +0200 Subject: [PATCH] Fix backups Audit/Details for k8s-format archives + broken local access Two separate bugs found while testing the Audit/Details UI against a real k8s-format backup (myapps-k8s-backup-*): 1. Format-hardcoded checks. audit_backup()'s Internal Structure and Volume Count checks assumed the legacy Docker-era archive layout (volumes/*.tar.gz, compose-files/) and would fail every k8s-format backup (per-app dirs with manifests.yaml/db-dump.sql.gz/pvc-data.tar.gz) even when it's perfectly healthy - same root cause as the myapps-backup-*/myapps-k8s-backup-* prefix bug fixed earlier, just in the audit checks instead of the listing/delete glob. cloud_backup.py's r2_audit_backup() had the identical unpatched filename regex, and app.py's /api/backups/details route had its own, which outright 400'd any k8s-format filename before even looking at it. 2. A bigger, separate bug this surfaced: get_local_backups(), get_vm_backups(), and _resolve_archive_path() all branch on RUNNING_ON_MAIN_SERVER to decide between direct filesystem access and SSH-to-self - and from inside the management-platform pod that flag's hostname check can be wrong for the pod's own ephemeral hostname depending on how it's evaluated, sending these down the SSH branch using a topology (separate warm-standby-on-the-VM SSH path) that doesn't apply to a pod that already has /root hostPath-mounted in. Fixed by trying direct filesystem access first wherever the archive would already be locally reachable, falling back to the existing SSH paths otherwise - this preserves the original warm-standby-on-VM failover design (a real second deployment of this platform on the VM host, kept reachable via SSH when the main server is down) completely unchanged; it only adds the fast local path for the case where the caller already has direct filesystem access. Verified against both a real k8s-format backup (odoo, single-app) and a real legacy-format backup on disk - both audit correctly now, with format-appropriate checks and labels. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_017avLHFqkiti3g62Anq9sVA --- platform/app.py | 42 ++++--- platform/modules/backups.py | 181 +++++++++++++++++++++++-------- platform/modules/cloud_backup.py | 6 +- 3 files changed, 166 insertions(+), 63 deletions(-) diff --git a/platform/app.py b/platform/app.py index 87c2d2c..efba6ec 100644 --- a/platform/app.py +++ b/platform/app.py @@ -461,7 +461,9 @@ def api_backup_details(): backup_file = data.get('backup_file', '').strip() source = data.get('source', 'local').strip() - if not re.match(r'^myapps-backup-\d{8}_\d{6}\.tar\.gz$', backup_file): + # Accepts both the legacy Docker-era lineage (myapps-backup-*) and the + # k8s-native one (myapps-k8s-backup-*, from backup-k8s-apps.sh). + if not re.match(r'^myapps-(k8s-)?backup-\d{8}_\d{6}\.tar\.gz$', backup_file): return jsonify({'error': 'Invalid filename'}), 400 if source == 'local': @@ -484,16 +486,21 @@ def api_backup_details(): } # ── size + mtime ────────────────────────────────────────────── + # Try direct stat first regardless of RUNNING_ON_MAIN_SERVER: the + # management-platform pod has /root hostPath-mounted from the main + # server, so /root/backups is already right here even though the + # hostname check that flag relies on never matches inside a pod (it + # used to always take the SSH branch, using a key/topology this + # in-pod deployment doesn't have, and silently come back empty). if source == 'local': - if RUNNING_ON_MAIN_SERVER: + if os.path.exists(archive_path): # direct stat on this machine - if os.path.exists(archive_path): - st = os.stat(archive_path) - result['size_bytes'] = st.st_size - result['size_human'] = _human_bytes(st.st_size) - mtime = datetime.fromtimestamp(st.st_mtime, tz=timezone.utc) - result['created_at'] = mtime.strftime('%Y-%m-%d %H:%M:%S UTC') - result['age_days'] = (datetime.now(tz=timezone.utc) - mtime).days + st = os.stat(archive_path) + result['size_bytes'] = st.st_size + result['size_human'] = _human_bytes(st.st_size) + mtime = datetime.fromtimestamp(st.st_mtime, tz=timezone.utc) + result['created_at'] = mtime.strftime('%Y-%m-%d %H:%M:%S UTC') + result['age_days'] = (datetime.now(tz=timezone.utc) - mtime).days if os.path.exists(sha_path): try: with open(sha_path) as f: @@ -517,15 +524,14 @@ def api_backup_details(): result['sha256'] = sha_out.strip() elif source == 'vm': - if not RUNNING_ON_MAIN_SERVER: - # we ARE the VM — direct stat - if os.path.exists(archive_path): - st = os.stat(archive_path) - result['size_bytes'] = st.st_size - result['size_human'] = _human_bytes(st.st_size) - mtime = datetime.fromtimestamp(st.st_mtime, tz=timezone.utc) - result['created_at'] = mtime.strftime('%Y-%m-%d %H:%M:%S UTC') - result['age_days'] = (datetime.now(tz=timezone.utc) - mtime).days + if os.path.exists(archive_path): + # direct stat when reachable (VM's own local disk, or a mount) + st = os.stat(archive_path) + result['size_bytes'] = st.st_size + result['size_human'] = _human_bytes(st.st_size) + mtime = datetime.fromtimestamp(st.st_mtime, tz=timezone.utc) + result['created_at'] = mtime.strftime('%Y-%m-%d %H:%M:%S UTC') + result['age_days'] = (datetime.now(tz=timezone.utc) - mtime).days if os.path.exists(sha_path): try: with open(sha_path) as f: diff --git a/platform/modules/backups.py b/platform/modules/backups.py index 9be1c73..33c48d0 100644 --- a/platform/modules/backups.py +++ b/platform/modules/backups.py @@ -55,10 +55,34 @@ def _ssh_main(remote_cmd, timeout=30): # BACKUPS # ──────────────────────────────────────────────────────────────── +def _list_local_dir(local_dir): + """Direct filesystem listing, newest first, both lineages.""" + files = ( + glob.glob(f'{local_dir}/myapps-backup-*.tar.gz') + + glob.glob(f'{local_dir}/myapps-k8s-backup-*.tar.gz') + ) + files.sort(key=os.path.getmtime, reverse=True) + return [os.path.basename(f) for f in files[:20]] + + def get_local_backups(): # myapps-backup-* is the legacy Docker-era lineage; myapps-k8s-backup-* # is the k8s-native one (backup-k8s-apps.sh). Listed together, sorted # by mtime, so history from both stays visible in the UI. + # + # Try direct filesystem access first: the management-platform pod has + # /root hostPath-mounted from the main server (see k8s/management- + # platform-deployment.yaml), so /root/backups is already right here. + # RUNNING_ON_MAIN_SERVER's hostname check never matches inside a pod + # (its hostname is the pod name, not the host's), so this used to + # always fall through to the SSH-to-self branch below — which also + # doesn't actually work in this topology (wrong key for this deploy + # model) — silently returning an empty list. Falling back to SSH only + # when there's genuinely no local mount keeps this working for any + # future non-mounted deployment too. + if os.path.isdir('/root/backups'): + return _list_local_dir('/root/backups') + stdout, _ = _ssh_main( "ls -t /root/backups/myapps-backup-*.tar.gz " "/root/backups/myapps-k8s-backup-*.tar.gz 2>/dev/null | head -20" @@ -73,34 +97,33 @@ def get_local_backups(): def get_vm_backups(): + # Same fix as get_local_backups(): try direct access first (covers the + # case where this process's filesystem actually is the VM's), fall + # back to SSH to the VM host otherwise — which is the real case for + # the management-platform pod, since only /root is mounted in, not + # anything from the separate VM backup host. + local_dir = '/backups/cloudproject' + if os.path.isdir(local_dir): + return _list_local_dir(local_dir) + vm_backups = [] - if RUNNING_ON_MAIN_SERVER: - try: - cmd = ( - f"ssh -i {VM_KEY} -p {VM_PORT} " - f"-o StrictHostKeyChecking=no -o ConnectTimeout=10 " - f"-o BatchMode=yes " - f"{VM_USER}@{VM_HOST} " - f"'ls -t /backups/cloudproject/myapps-backup-*.tar.gz " - f"/backups/cloudproject/myapps-k8s-backup-*.tar.gz 2>/dev/null | head -20'" - ) - stdout, _ = _run(cmd, timeout=25) - if stdout: - for line in stdout.split('\n'): - line = line.strip() - if line and '.tar.gz' in line: - vm_backups.append(os.path.basename(line)) - except Exception as e: - print(f"[backups] VM backup fetch error: {e}") - else: - backup_dir = '/backups/cloudproject' - if os.path.exists(backup_dir): - files = ( - glob.glob(f'{backup_dir}/myapps-backup-*.tar.gz') - + glob.glob(f'{backup_dir}/myapps-k8s-backup-*.tar.gz') - ) - files.sort(key=os.path.getmtime, reverse=True) - vm_backups = [os.path.basename(f) for f in files[:20]] + try: + cmd = ( + f"ssh -i {VM_KEY} -p {VM_PORT} " + f"-o StrictHostKeyChecking=no -o ConnectTimeout=10 " + f"-o BatchMode=yes " + f"{VM_USER}@{VM_HOST} " + f"'ls -t /backups/cloudproject/myapps-backup-*.tar.gz " + f"/backups/cloudproject/myapps-k8s-backup-*.tar.gz 2>/dev/null | head -20'" + ) + stdout, _ = _run(cmd, timeout=25) + if stdout: + for line in stdout.split('\n'): + line = line.strip() + if line and '.tar.gz' in line: + vm_backups.append(os.path.basename(line)) + except Exception as e: + print(f"[backups] VM backup fetch error: {e}") return vm_backups @@ -115,16 +138,19 @@ def get_vm_backups(): # test, easy to extend without pushing complexity back up. def _resolve_archive_path(backup_file, source, add): - """Figure out the local path to the archive, pulling it from the main - server first if needed. Returns the path, or None if it could not be - resolved (in which case an error result dict has already been added - via `add` and the caller should bail out).""" + """Figure out the local path to the archive, pulling it over SSH only + if it's genuinely not reachable on this filesystem. Returns the path, + or None if it could not be resolved (in which case an error result + dict has already been added via `add` and the caller should bail out). + + Tries direct access first regardless of RUNNING_ON_MAIN_SERVER: the + management-platform pod has /root hostPath-mounted from the main + server, so /root/backups is already right here even though the + hostname check that flag relies on never matches inside a pod.""" if source == 'local': archive_path = f"/root/backups/{backup_file}" - else: - archive_path = f"/backups/cloudproject/{backup_file}" - - if not RUNNING_ON_MAIN_SERVER and source == 'local': + if os.path.exists(archive_path): + return archive_path tmp_path = f"/tmp/audit_{backup_file}" if not os.path.exists(tmp_path): pull_cmd = ( @@ -137,9 +163,24 @@ def _resolve_archive_path(backup_file, source, add): if not os.path.exists(tmp_path): add('File Access', 'fail', f'Could not pull from main server: {err}') return None - archive_path = tmp_path - - return archive_path + return tmp_path + else: + archive_path = f"/backups/cloudproject/{backup_file}" + if os.path.exists(archive_path): + return archive_path + tmp_path = f"/tmp/audit_vm_{backup_file}" + if not os.path.exists(tmp_path): + pull_cmd = ( + f"scp -i {VM_KEY} -P {VM_PORT} " + f"-o StrictHostKeyChecking=no -o ConnectTimeout=15 " + f"{VM_USER}@{VM_HOST}:/backups/cloudproject/{backup_file} " + f"{tmp_path}" + ) + _out, err = _run(pull_cmd, timeout=120) + if not os.path.exists(tmp_path): + add('File Access', 'fail', f'Could not pull from VM: {err}') + return None + return tmp_path def _check_file_size(archive_path, add): @@ -216,13 +257,54 @@ def _list_archive_members(archive_path): return [] -def _check_internal_structure(members, add): +# The two backup lineages have completely different internal layouts: +# - legacy (myapps-backup-*, from backup-myapps.sh): volumes/*.tar.gz, +# compose-files/, backup-info.txt at the archive root. +# - k8s-native (myapps-k8s-backup-*, from backup-k8s-apps.sh): per-app +# subdirs (n8n/, odoo/, mautic/, nextcloud/, frappe/) each with +# manifests.yaml, secret.yaml, db-dump.sql.gz, pvc-data.tar.gz, plus +# backup-info.txt at the archive root. +# Auditing a k8s-format archive against the legacy layout would always +# fail it ("volumes/ missing") even when it's a perfectly good backup — +# same root cause as the myapps-backup-*/myapps-k8s-backup-* prefix bug +# already fixed elsewhere in this file, just in the audit checks instead +# of the listing/delete glob. +_K8S_APP_NAMES = ('n8n', 'odoo', 'mautic', 'nextcloud', 'frappe') + + +def _is_k8s_format(backup_file): + return os.path.basename(backup_file).startswith('myapps-k8s-backup-') + + +def _check_internal_structure(members, backup_file, add): if not members: add('Internal Structure', 'warn', 'Could not inspect archive members') return - has_volumes = any('volumes/' in m for m in members) has_info = any('backup-info.txt' in m for m in members) + + if _is_k8s_format(backup_file): + app_dirs = sorted({ + parts[1] for m in members + if len(parts := m.split('/')) > 1 and parts[1] in _K8S_APP_NAMES + }) + has_app_data = any( + m.endswith(('manifests.yaml', 'db-dump.sql.gz', 'pvc-data.tar.gz')) + for m in members + ) + issues = [] + if not has_app_data: + issues.append('no per-app manifests/db-dump/pvc-data found') + if not has_info: + issues.append('backup-info.txt missing') + if issues: + add('Internal Structure', 'fail', ' · '.join(issues)) + return + add('Internal Structure', 'pass', + f'backup-info.txt ✓ apps: {", ".join(app_dirs) or "?"}') + return + + has_volumes = any('volumes/' in m for m in members) has_compose = any('compose-files/' in m for m in members) vol_count = len([m for m in members if '/volumes/' in m and m.endswith('.tar.gz')]) @@ -302,7 +384,20 @@ def _check_executable_scripts(archive_path, add): add('Executable Scripts', 'pass', 'No unexpected executable scripts found') -def _check_volume_count(members, add): +def _check_volume_count(members, backup_file, add): + if _is_k8s_format(backup_file): + pvc_archives = [m for m in members if m.endswith('/pvc-data.tar.gz')] + v = len(pvc_archives) + if v == 0: + add('App Data Archives', 'fail', 'No pvc-data.tar.gz found in backup') + elif v < 5: + add('App Data Archives', 'warn', + f'{v} app(s) backed up — fine if this was a --apps filtered run, ' + f'otherwise expected 5 for a full backup') + else: + add('App Data Archives', 'pass', f'{v} app data archives present') + return + vol_archives = [m for m in members if 'volumes/' in m and m.endswith('.tar.gz')] v = len(vol_archives) if v == 0: @@ -379,10 +474,10 @@ def audit_backup(backup_file, source='local'): _check_archive_integrity(archive_path, add) members = _list_archive_members(archive_path) - _check_internal_structure(members, add) + _check_internal_structure(members, backup_file, add) _check_security_scan(members, add) _check_executable_scripts(archive_path, add) - _check_volume_count(members, add) + _check_volume_count(members, backup_file, add) score = _score_checks(checks) has_fails = any(c['status'] == 'fail' for c in checks) diff --git a/platform/modules/cloud_backup.py b/platform/modules/cloud_backup.py index d886104..47586af 100644 --- a/platform/modules/cloud_backup.py +++ b/platform/modules/cloud_backup.py @@ -345,10 +345,12 @@ def r2_audit_backup(key: str) -> dict: else: report["checks"].append(f"✅ File size looks healthy ({_human_size(size)})") - # Check 5: Filename format + # Check 5: Filename format — accepts both the legacy Docker-era + # lineage (myapps-backup-*) and the k8s-native one + # (myapps-k8s-backup-*, from backup-k8s-apps.sh). import re filename = key.replace("backups/", "") - if re.match(r"myapps-backup-\d{8}_\d{6}\.tar\.gz", filename): + if re.match(r"myapps-(k8s-)?backup-\d{8}_\d{6}\.tar\.gz", filename): report["checks"].append("✅ Filename format is valid") else: report["warnings"].append(f"⚠️ Unexpected filename format: {filename}")