feat(operations): backup-to-template grouping fix + artifact/backup delete
_template_of_backup only stripped one "_"-chunk off the archive name, so real Template_<name>_%Y%m%d_%H%M%S.7z files (date *and* time) never matched their template -- backups silently failed to show up grouped under it. Replaced with a regex that strips the full optional date+time suffix and tolerates underscores in template names. Also adds delete_artifact/delete_backup (mirrored, containment-checked single-file deletion) and backups_by_template (groups list_backups() output by parsed template name, newest-first) for the web dashboard's per-item delete actions and the Templates panel's per-row backup list.
This commit is contained in:
@@ -17,6 +17,7 @@ outside ``root``.
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
import os
|
import os
|
||||||
|
import re
|
||||||
import time
|
import time
|
||||||
from dataclasses import dataclass
|
from dataclasses import dataclass
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
@@ -280,9 +281,134 @@ def list_artifacts(config: Config) -> list[FileEntry]:
|
|||||||
return list_dir(config.paths.artifacts, root=config.paths.root, pattern="*")
|
return list_dir(config.paths.artifacts, root=config.paths.root, pattern="*")
|
||||||
|
|
||||||
|
|
||||||
|
@dataclass(frozen=True)
|
||||||
|
class ArtifactDeleteResult:
|
||||||
|
"""Outcome of a (real or dry-run) single-artifact deletion."""
|
||||||
|
|
||||||
|
target: Path | None
|
||||||
|
ok: bool
|
||||||
|
error: str | None
|
||||||
|
|
||||||
|
|
||||||
|
def delete_artifact(config: Config, name: str, *, dry_run: bool) -> ArtifactDeleteResult:
|
||||||
|
"""Delete one artifact file directly under ``config.paths.artifacts``.
|
||||||
|
|
||||||
|
SECURITY: ``name`` is only ever a filename the dashboard already listed,
|
||||||
|
but this still resolves the candidate and refuses anything whose parent
|
||||||
|
is not exactly the artifacts directory (a ``..`` segment, a nested
|
||||||
|
subdir, or a symlink pointing elsewhere) and anything that is not a
|
||||||
|
regular file -- the same containment discipline
|
||||||
|
:func:`list_dir`/``web/routes.py`` apply. ``dry_run`` validates and
|
||||||
|
reports the target without unlinking. Never raises.
|
||||||
|
"""
|
||||||
|
artifacts_dir = config.paths.artifacts
|
||||||
|
try:
|
||||||
|
resolved_dir = artifacts_dir.resolve()
|
||||||
|
resolved = (artifacts_dir / name).resolve()
|
||||||
|
except OSError as exc:
|
||||||
|
return ArtifactDeleteResult(target=None, ok=False, error=f"cannot resolve path: {exc}")
|
||||||
|
|
||||||
|
if resolved.parent != resolved_dir:
|
||||||
|
return ArtifactDeleteResult(
|
||||||
|
target=resolved, ok=False, error="artifact is not contained in the artifacts directory"
|
||||||
|
)
|
||||||
|
if not resolved.is_file():
|
||||||
|
return ArtifactDeleteResult(target=resolved, ok=False, error="artifact not found")
|
||||||
|
|
||||||
|
if dry_run:
|
||||||
|
return ArtifactDeleteResult(target=resolved, ok=True, error=None)
|
||||||
|
|
||||||
|
try:
|
||||||
|
resolved.unlink()
|
||||||
|
except OSError as exc:
|
||||||
|
return ArtifactDeleteResult(target=resolved, ok=False, error=str(exc))
|
||||||
|
return ArtifactDeleteResult(target=resolved, ok=True, error=None)
|
||||||
|
|
||||||
|
|
||||||
|
#: ``Template_<name>_%Y%m%d[_%H%M%S].7z`` -- the timestamp both
|
||||||
|
#: ``operations/host.py::backup_template`` and the ``template backup`` CLI
|
||||||
|
#: write is ``_%Y%m%d_%H%M%S`` (a date *and* a time chunk, e.g.
|
||||||
|
#: ``_20260722_145001``), so ``<name>`` is everything before that trailing
|
||||||
|
#: ``_\d{8}(_\d{6})?``. A greedy ``.+`` lets template names that themselves
|
||||||
|
#: contain underscores (e.g. ``Win_Build_2025``) survive intact; the optional
|
||||||
|
#: time group tolerates the older date-only shape too.
|
||||||
|
_BACKUP_NAME_RE = re.compile(r"^Template_(?P<name>.+)_\d{8}(?:_\d{6})?\.7z$")
|
||||||
|
|
||||||
|
|
||||||
|
def _template_of_backup(filename: str) -> str | None:
|
||||||
|
"""Parse the template name out of a ``Template_<name>_<timestamp>.7z`` archive.
|
||||||
|
|
||||||
|
Mirrors the naming ``operations/host.py::backup_template`` writes. Returns
|
||||||
|
``None`` for any filename that does not match the ``Template_..._....7z``
|
||||||
|
shape (see :data:`_BACKUP_NAME_RE`).
|
||||||
|
"""
|
||||||
|
match = _BACKUP_NAME_RE.match(filename)
|
||||||
|
return match.group("name") if match else None
|
||||||
|
|
||||||
|
|
||||||
|
def backups_by_template(config: Config) -> dict[str, list[FileEntry]]:
|
||||||
|
"""Group every backup archive by the template it belongs to, newest-first.
|
||||||
|
|
||||||
|
Built on :func:`list_backups` (so it inherits the same containment guard
|
||||||
|
and newest-first ordering); each value list preserves that ordering.
|
||||||
|
Archives whose name does not parse to a template are skipped. Never
|
||||||
|
raises -- an unreadable backups dir degrades to an empty mapping.
|
||||||
|
"""
|
||||||
|
out: dict[str, list[FileEntry]] = {}
|
||||||
|
for entry in list_backups(config):
|
||||||
|
name = _template_of_backup(entry.name)
|
||||||
|
if name is None:
|
||||||
|
continue
|
||||||
|
out.setdefault(name, []).append(entry)
|
||||||
|
return out
|
||||||
|
|
||||||
|
|
||||||
|
def delete_backup(config: Config, name: str, *, dry_run: bool) -> ArtifactDeleteResult:
|
||||||
|
"""Delete one template backup archive under ``<config.paths.root>/backups``.
|
||||||
|
|
||||||
|
SECURITY: mirrors :func:`delete_artifact`'s containment discipline -- the
|
||||||
|
candidate must resolve to a direct child of the backups directory (no
|
||||||
|
``..``, no nested subdir, no symlink escaping it) and be a regular file.
|
||||||
|
It must additionally parse as a real ``Template_..._....7z`` archive (see
|
||||||
|
:data:`_BACKUP_NAME_RE`) so this can never be used to unlink an arbitrary
|
||||||
|
file that merely happens to sit in the backups directory. ``dry_run``
|
||||||
|
validates and reports the target without unlinking. Never raises.
|
||||||
|
"""
|
||||||
|
backups_dir = config.paths.root / "backups"
|
||||||
|
if _template_of_backup(name) is None:
|
||||||
|
return ArtifactDeleteResult(
|
||||||
|
target=None, ok=False, error="not a template backup archive name"
|
||||||
|
)
|
||||||
|
try:
|
||||||
|
resolved_dir = backups_dir.resolve()
|
||||||
|
resolved = (backups_dir / name).resolve()
|
||||||
|
except OSError as exc:
|
||||||
|
return ArtifactDeleteResult(target=None, ok=False, error=f"cannot resolve path: {exc}")
|
||||||
|
|
||||||
|
if resolved.parent != resolved_dir:
|
||||||
|
return ArtifactDeleteResult(
|
||||||
|
target=resolved, ok=False, error="backup is not contained in the backups directory"
|
||||||
|
)
|
||||||
|
if not resolved.is_file():
|
||||||
|
return ArtifactDeleteResult(target=resolved, ok=False, error="backup not found")
|
||||||
|
|
||||||
|
if dry_run:
|
||||||
|
return ArtifactDeleteResult(target=resolved, ok=True, error=None)
|
||||||
|
|
||||||
|
try:
|
||||||
|
resolved.unlink()
|
||||||
|
except OSError as exc:
|
||||||
|
return ArtifactDeleteResult(target=resolved, ok=False, error=str(exc))
|
||||||
|
return ArtifactDeleteResult(target=resolved, ok=True, error=None)
|
||||||
|
|
||||||
|
|
||||||
__all__ = [
|
__all__ = [
|
||||||
|
"ArtifactDeleteResult",
|
||||||
"FileEntry",
|
"FileEntry",
|
||||||
"TemplateInfo",
|
"TemplateInfo",
|
||||||
|
"backups_by_template",
|
||||||
|
"delete_artifact",
|
||||||
|
"delete_backup",
|
||||||
"list_artifacts",
|
"list_artifacts",
|
||||||
"list_backups",
|
"list_backups",
|
||||||
"list_dir",
|
"list_dir",
|
||||||
|
|||||||
@@ -162,6 +162,27 @@ def test_list_templates_last_backup_picks_newest_matching_archive(tmp_path: Path
|
|||||||
assert info.last_backup.name == "Template_LinuxBuild2404_20260601_000000.7z"
|
assert info.last_backup.name == "Template_LinuxBuild2404_20260601_000000.7z"
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
("filename", "expected"),
|
||||||
|
[
|
||||||
|
# Real archive shape: ``Template_<name>_%Y%m%d_%H%M%S.7z`` -- both the
|
||||||
|
# date and the time chunk must be stripped (regression: only the time
|
||||||
|
# chunk was, leaving "LinuxBuild2404_20260722").
|
||||||
|
("Template_LinuxBuild2404_20260722_145001.7z", "LinuxBuild2404"),
|
||||||
|
("Template_WinBuild2025_20260101_000000.7z", "WinBuild2025"),
|
||||||
|
# Template names containing underscores survive intact.
|
||||||
|
("Template_Win_Build_2025_20260722_145001.7z", "Win_Build_2025"),
|
||||||
|
# Date-only shape (no time chunk) is still tolerated.
|
||||||
|
("Template_LinuxBuild2404_20260722.7z", "LinuxBuild2404"),
|
||||||
|
# Non-matching names parse to None.
|
||||||
|
("not-a-template-archive.7z", None),
|
||||||
|
("Template_no_timestamp.txt", None),
|
||||||
|
],
|
||||||
|
)
|
||||||
|
def test_template_of_backup_strips_full_timestamp(filename: str, expected: str | None) -> None:
|
||||||
|
assert inventory_module._template_of_backup(filename) == expected
|
||||||
|
|
||||||
|
|
||||||
def test_list_templates_last_backup_none_when_no_backups_dir(tmp_path: Path) -> None:
|
def test_list_templates_last_backup_none_when_no_backups_dir(tmp_path: Path) -> None:
|
||||||
(tmp_path / "templates" / "LinuxBuild2404").mkdir(parents=True)
|
(tmp_path / "templates" / "LinuxBuild2404").mkdir(parents=True)
|
||||||
config = _make_config(tmp_path)
|
config = _make_config(tmp_path)
|
||||||
|
|||||||
Reference in New Issue
Block a user