feat(web): delete a template, not just back it up
POST /host/delete-template: same two-step confirm gate, CSRF check, and known-template containment as /host/backup, offloaded to a thread the same way (rmtree on a multi-GB template dir is not instant). No backup-required safety rail -- same confirm-gate as every other destructive action, nothing more; the confirm summary spells out that clones can no longer be made from the template until one is restored. Also cleaned up the per-backup Delete button it sits next to: name+meta grouped into one shrinkable unit so the row wraps as a block instead of the delete button drifting away from a name that wrapped, and each backup is now its own bordered line instead of a run-on flex list.
This commit is contained in:
@@ -48,7 +48,7 @@ from starlette import status
|
||||
|
||||
from ci_orchestrator.operations.audit import AuditEntry, append_audit, read_audit
|
||||
from ci_orchestrator.operations.cache import purge_cargo_cache
|
||||
from ci_orchestrator.operations.host import backup_template
|
||||
from ci_orchestrator.operations.host import backup_template, delete_template
|
||||
from ci_orchestrator.operations.inventory import delete_artifact, delete_backup, list_templates
|
||||
from ci_orchestrator.operations.services import ALLOWED_ACTIONS, control_service, find_service
|
||||
from ci_orchestrator.operations.vms import RemovalResult, cleanup_orphans, remove_vm
|
||||
@@ -715,6 +715,92 @@ async def host_backup(
|
||||
)
|
||||
|
||||
|
||||
# ─────────────────────────────────────────────────────── POST /host/delete-template
|
||||
|
||||
|
||||
async def host_delete_template(
|
||||
request: Request,
|
||||
user: str = Depends(require_user),
|
||||
_csrf: None = Depends(require_csrf),
|
||||
) -> HTMLResponse:
|
||||
"""Permanently remove a template directory, behind the confirm gate.
|
||||
|
||||
Same containment discipline as :func:`host_backup`: the submitted name
|
||||
must be one of the templates this host actually discovers. No backup is
|
||||
required first -- unlike a backup or an artifact, there is no built-in
|
||||
undo here beyond restoring one manually, so the confirm summary spells
|
||||
that out.
|
||||
"""
|
||||
config: Config = request.app.state.config
|
||||
action_label = "Delete template"
|
||||
|
||||
form = await request.form()
|
||||
template = _form_str(form, "template")
|
||||
if not template:
|
||||
raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail="template is required")
|
||||
|
||||
known_templates = {t.name for t in list_templates(config)}
|
||||
if template not in known_templates:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_400_BAD_REQUEST,
|
||||
detail="template is not a recognized template name",
|
||||
)
|
||||
execute = _form_str(form, _STAGE_FIELD) == _STAGE_EXECUTE
|
||||
|
||||
try:
|
||||
# rmtree on a multi-GB template dir is not instant; offloaded like
|
||||
# host_backup's compression step so a slow removal never blocks the
|
||||
# event loop (every other request, all the dashboard polls, would
|
||||
# otherwise freeze for the duration).
|
||||
result = await asyncio.to_thread(delete_template, config, template, dry_run=not execute)
|
||||
except Exception as exc: # never 500 on an operational failure
|
||||
detail = f"error: {exc}"
|
||||
_audit(
|
||||
config, user=user, action="host.delete_template", target=template, result="error", detail=detail
|
||||
)
|
||||
return _result_partial(request, action_label=action_label, ok=False, detail=detail)
|
||||
|
||||
if execute:
|
||||
detail = f"deleted template '{template}'" if result.ok else (result.error or "unknown error")
|
||||
_audit(
|
||||
config,
|
||||
user=user,
|
||||
action="host.delete_template",
|
||||
target=template,
|
||||
result="ok" if result.ok else "error",
|
||||
detail=detail,
|
||||
)
|
||||
return _result_partial(request, action_label=action_label, ok=result.ok, detail=detail)
|
||||
|
||||
if not result.ok:
|
||||
detail = result.error or "unknown error"
|
||||
_audit(
|
||||
config,
|
||||
user=user,
|
||||
action="host.delete_template",
|
||||
target=template,
|
||||
result="error",
|
||||
detail=detail,
|
||||
)
|
||||
return _result_partial(request, action_label=action_label, ok=False, detail=detail)
|
||||
|
||||
detail = (
|
||||
f"would permanently delete the template directory for '{template}' -- "
|
||||
"clones can no longer be made from it until restored from a backup"
|
||||
)
|
||||
_audit(
|
||||
config, user=user, action="host.delete_template", target=template, result="dry-run", detail=detail
|
||||
)
|
||||
return _confirm_partial(
|
||||
request,
|
||||
action_label=action_label,
|
||||
target=template,
|
||||
summary=detail,
|
||||
action_url="/host/delete-template",
|
||||
extra_fields={"template": template},
|
||||
)
|
||||
|
||||
|
||||
# ───────────────────────────────────────────────────── POST /services/{id}/{action}
|
||||
|
||||
#: The ``ManagedService.id`` whose stop/restart confirm step warns about
|
||||
@@ -843,6 +929,12 @@ def register_action_routes(app: FastAPI) -> None:
|
||||
response_class=HTMLResponse,
|
||||
)
|
||||
app.add_api_route("/host/backup", host_backup, methods=["POST"], response_class=HTMLResponse)
|
||||
app.add_api_route(
|
||||
"/host/delete-template",
|
||||
host_delete_template,
|
||||
methods=["POST"],
|
||||
response_class=HTMLResponse,
|
||||
)
|
||||
app.add_api_route(
|
||||
"/services/{service_id}/{action}",
|
||||
services_control,
|
||||
@@ -860,6 +952,7 @@ __all__ = [
|
||||
"encode_artifact_id",
|
||||
"encode_vmx_id",
|
||||
"host_backup",
|
||||
"host_delete_template",
|
||||
"register_action_routes",
|
||||
"services_control",
|
||||
"vms_cleanup",
|
||||
|
||||
@@ -40,8 +40,10 @@
|
||||
<ul class="backups-list">
|
||||
{% for b in bks %}
|
||||
<li>
|
||||
<code title="{{ b.name }}">{{ b.name }}</code>
|
||||
<span class="text-dim">{{ b.size_bytes | humanbytes }} · {{ b.mtime | relative_time }}</span>
|
||||
<span class="backup-info">
|
||||
<code title="{{ b.name }}">{{ b.name }}</code>
|
||||
<span class="text-dim">{{ b.size_bytes | humanbytes }} · {{ b.mtime | relative_time }}</span>
|
||||
</span>
|
||||
<form method="post" action="/backups/{{ b.name | artifact_id }}/delete" class="backup-del"
|
||||
aria-label="Delete backup {{ b.name }}"
|
||||
hx-post="/backups/{{ b.name | artifact_id }}/delete" hx-target="#action-slot" hx-swap="innerHTML">
|
||||
@@ -59,14 +61,24 @@
|
||||
{% endif %}
|
||||
</td>
|
||||
<td>
|
||||
<form method="post" action="/host/backup" aria-label="Backup template {{ tmpl.name }}"
|
||||
hx-post="/host/backup" hx-target="#action-slot" hx-swap="innerHTML show:top">
|
||||
<input type="hidden" name="csrf" value="{{ csrf_token }}">
|
||||
<input type="hidden" name="template" value="{{ tmpl.name }}">
|
||||
<button type="submit" class="btn btn-sm" aria-label="Backup template {{ tmpl.name }}">
|
||||
<svg class="icon" aria-hidden="true"><use href="#i-box"></use></svg>Backup
|
||||
</button>
|
||||
</form>
|
||||
<div class="actions-row">
|
||||
<form method="post" action="/host/backup" aria-label="Backup template {{ tmpl.name }}"
|
||||
hx-post="/host/backup" hx-target="#action-slot" hx-swap="innerHTML show:top">
|
||||
<input type="hidden" name="csrf" value="{{ csrf_token }}">
|
||||
<input type="hidden" name="template" value="{{ tmpl.name }}">
|
||||
<button type="submit" class="btn btn-sm" aria-label="Backup template {{ tmpl.name }}">
|
||||
<svg class="icon" aria-hidden="true"><use href="#i-box"></use></svg>Backup
|
||||
</button>
|
||||
</form>
|
||||
<form method="post" action="/host/delete-template" aria-label="Delete template {{ tmpl.name }}"
|
||||
hx-post="/host/delete-template" hx-target="#action-slot" hx-swap="innerHTML show:top">
|
||||
<input type="hidden" name="csrf" value="{{ csrf_token }}">
|
||||
<input type="hidden" name="template" value="{{ tmpl.name }}">
|
||||
<button type="submit" class="btn btn-danger btn-sm" aria-label="Delete template {{ tmpl.name }}">
|
||||
<svg class="icon" aria-hidden="true"><use href="#i-trash"></use></svg>Delete
|
||||
</button>
|
||||
</form>
|
||||
</div>
|
||||
</td>
|
||||
</tr>
|
||||
{% endfor %}
|
||||
|
||||
@@ -22,7 +22,9 @@ from ci_orchestrator.operations.host import (
|
||||
BackupResult,
|
||||
DiskStatus,
|
||||
RunnerStatus,
|
||||
TemplateDeleteResult,
|
||||
backup_template,
|
||||
delete_template,
|
||||
disk_status,
|
||||
runner_status,
|
||||
)
|
||||
@@ -276,3 +278,66 @@ def test_backup_template_never_calls_real_vmrun_or_7z(monkeypatch: pytest.Monkey
|
||||
backup_template(config, "WinBuild2025")
|
||||
|
||||
assert called == [True]
|
||||
|
||||
|
||||
# ── delete_template ─────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
def test_delete_template_missing_template_dir_errors(tmp_path: Path) -> None:
|
||||
templates = tmp_path / "templates"
|
||||
templates.mkdir()
|
||||
config = _make_config(templates)
|
||||
|
||||
result = delete_template(config, "NoSuchTemplate")
|
||||
|
||||
assert result == TemplateDeleteResult(
|
||||
template="NoSuchTemplate", ok=False, error=result.error
|
||||
)
|
||||
assert result.error is not None
|
||||
assert "NoSuchTemplate" in result.error or "not found" in result.error.lower()
|
||||
|
||||
|
||||
def test_delete_template_dry_run_does_not_remove_the_directory(tmp_path: Path) -> None:
|
||||
templates = tmp_path / "templates"
|
||||
tmpl_dir = templates / "WinBuild2025"
|
||||
tmpl_dir.mkdir(parents=True)
|
||||
(tmpl_dir / "WinBuild2025.vmx").write_text("config.version = \"8\"")
|
||||
config = _make_config(templates)
|
||||
|
||||
result = delete_template(config, "WinBuild2025", dry_run=True)
|
||||
|
||||
assert result == TemplateDeleteResult(template="WinBuild2025", ok=True, error=None)
|
||||
assert tmpl_dir.is_dir()
|
||||
|
||||
|
||||
def test_delete_template_removes_the_whole_directory(tmp_path: Path) -> None:
|
||||
templates = tmp_path / "templates"
|
||||
tmpl_dir = templates / "WinBuild2025"
|
||||
tmpl_dir.mkdir(parents=True)
|
||||
(tmpl_dir / "WinBuild2025.vmx").write_text("config.version = \"8\"")
|
||||
config = _make_config(templates)
|
||||
|
||||
result = delete_template(config, "WinBuild2025")
|
||||
|
||||
assert result == TemplateDeleteResult(template="WinBuild2025", ok=True, error=None)
|
||||
assert not tmpl_dir.exists()
|
||||
|
||||
|
||||
def test_delete_template_removal_failure_reported(
|
||||
monkeypatch: pytest.MonkeyPatch, tmp_path: Path
|
||||
) -> None:
|
||||
templates = tmp_path / "templates"
|
||||
tmpl_dir = templates / "WinBuild2025"
|
||||
tmpl_dir.mkdir(parents=True)
|
||||
config = _make_config(templates)
|
||||
|
||||
def _boom(_path: Path) -> None:
|
||||
raise OSError("device busy")
|
||||
|
||||
monkeypatch.setattr(shutil, "rmtree", _boom)
|
||||
|
||||
result = delete_template(config, "WinBuild2025")
|
||||
|
||||
assert result.ok is False
|
||||
assert result.error is not None
|
||||
assert tmpl_dir.is_dir()
|
||||
|
||||
@@ -918,6 +918,88 @@ def test_host_backup_execute_exception_never_500s(
|
||||
assert entry.result == "error"
|
||||
|
||||
|
||||
# ── POST /host/delete-template ─────────────────────────────────────────────
|
||||
|
||||
|
||||
def test_host_delete_template_missing_template_field_is_400(tmp_path: Path) -> None:
|
||||
config = _make_config(tmp_path)
|
||||
app = create_app(config)
|
||||
client = TestClient(app)
|
||||
csrf = _get_csrf(client)
|
||||
|
||||
resp = client.post("/host/delete-template", data={"csrf": csrf})
|
||||
|
||||
assert resp.status_code == 400
|
||||
|
||||
|
||||
def test_host_delete_template_unknown_template_name_is_400_not_audited(tmp_path: Path) -> None:
|
||||
"""Same B6 containment discipline as ``/host/backup``: an arbitrary name
|
||||
is rejected before ``delete_template`` is ever called."""
|
||||
config = _make_config(tmp_path)
|
||||
app = create_app(config)
|
||||
client = TestClient(app)
|
||||
csrf = _get_csrf(client)
|
||||
|
||||
resp = client.post(
|
||||
"/host/delete-template", data={"csrf": csrf, "template": "NoSuchTemplate"}
|
||||
)
|
||||
|
||||
assert resp.status_code == 400
|
||||
assert not config.web.audit_log.exists()
|
||||
|
||||
|
||||
def test_host_delete_template_preview_then_execute_full_cycle(tmp_path: Path) -> None:
|
||||
template_dir = tmp_path / "templates" / "LinuxBuild2404"
|
||||
template_dir.mkdir(parents=True)
|
||||
(template_dir / "LinuxBuild2404.vmx").write_text("x", encoding="utf-8")
|
||||
|
||||
config = _make_config(tmp_path)
|
||||
app = create_app(config)
|
||||
client = TestClient(app)
|
||||
csrf = _get_csrf(client)
|
||||
|
||||
preview = client.post(
|
||||
"/host/delete-template", data={"csrf": csrf, "template": "LinuxBuild2404"}
|
||||
)
|
||||
assert preview.status_code == 200
|
||||
assert "Confirm" in preview.text
|
||||
assert "permanently delete" in preview.text
|
||||
assert _last_audit_entry(config.web.audit_log).result == "dry-run"
|
||||
assert template_dir.is_dir() # nothing removed on the preview stage
|
||||
|
||||
execute = client.post(
|
||||
"/host/delete-template",
|
||||
data={"csrf": csrf, "stage": "execute", "template": "LinuxBuild2404"},
|
||||
)
|
||||
assert execute.status_code == 200
|
||||
entry = _last_audit_entry(config.web.audit_log)
|
||||
assert entry.result == "ok"
|
||||
assert entry.action == "host.delete_template"
|
||||
assert not template_dir.exists()
|
||||
|
||||
|
||||
def test_host_delete_template_execute_exception_never_500s(
|
||||
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
def _boom(*args: Any, **kwargs: Any) -> Any:
|
||||
raise RuntimeError("device busy")
|
||||
|
||||
monkeypatch.setattr(routes_module, "delete_template", _boom)
|
||||
(tmp_path / "templates" / "X").mkdir(parents=True)
|
||||
config = _make_config(tmp_path)
|
||||
app = create_app(config)
|
||||
client = TestClient(app)
|
||||
csrf = _get_csrf(client)
|
||||
|
||||
resp = client.post(
|
||||
"/host/delete-template", data={"csrf": csrf, "stage": "execute", "template": "X"}
|
||||
)
|
||||
|
||||
assert resp.status_code == 200
|
||||
entry = _last_audit_entry(config.web.audit_log)
|
||||
assert entry.result == "error"
|
||||
|
||||
|
||||
# ── web/security.py: direct unit coverage of the CSRF primitives ──────────
|
||||
|
||||
|
||||
|
||||
@@ -65,10 +65,10 @@ def test_host_route_renders_snapshot(tmp_path: Path, monkeypatch: pytest.MonkeyP
|
||||
resp = client.get("/host")
|
||||
|
||||
assert resp.status_code == 200
|
||||
# Stage E: CPU%/RAM-used moved to the status-header tiles; the Host card
|
||||
# keeps uptime, load average, core count and total memory.
|
||||
# Stage F folded the retired tiles' CPU%/RAM readouts back into this card,
|
||||
# alongside the hostname/kernel/CPU-model rows it gained at the same time.
|
||||
assert "Host" in resp.text
|
||||
assert "16.0" in resp.text # total memory GB
|
||||
assert "16" in resp.text # total memory GB
|
||||
|
||||
|
||||
def test_host_route_degrades_on_probe_failure(
|
||||
@@ -289,6 +289,20 @@ def test_templates_partial_offers_per_row_backup_action(
|
||||
assert "Backup" in resp.text
|
||||
|
||||
|
||||
def test_templates_partial_offers_per_row_delete_action(
|
||||
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
monkeypatch.setattr(app_module, "list_templates", lambda _config: [_TEMPLATE_ROW])
|
||||
app = create_app(_make_config(tmp_path))
|
||||
client = TestClient(app)
|
||||
|
||||
resp = client.get("/templates")
|
||||
|
||||
assert resp.status_code == 200
|
||||
assert 'action="/host/delete-template"' in resp.text
|
||||
assert "Delete" in resp.text
|
||||
|
||||
|
||||
def test_templates_partial_no_backup_action_when_no_templates(
|
||||
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
|
||||
Reference in New Issue
Block a user