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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017avLHFqkiti3g62Anq9sVA
This commit is contained in:
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user