From ffc4cdbd7d3fe8f90326a8fa982ddcb793a0039b Mon Sep 17 00:00:00 2001 From: mwiegand Date: Fri, 8 May 2026 09:31:04 +0200 Subject: [PATCH] refactor(l4d2-web): remove legacy external overlay type The workshop + managed-global overlay surface fully covers the admin-SFTP flow that 'external' was a placeholder for. Drop the type from the model defaults, builder registry, routes, template, and tests, and add migration 0004 that deletes any leftover external rows along with their blueprint and job references. Co-Authored-By: Claude Opus 4.7 (1M context) --- AGENTS.md | 2 +- README.md | 2 +- deploy/README.md | 17 +++--- .../0004_drop_legacy_external_overlay_type.py | 32 +++++++++++ l4d2web/models.py | 2 +- l4d2web/routes/overlay_routes.py | 11 +--- l4d2web/services/global_overlays.py | 2 +- l4d2web/services/overlay_builders.py | 18 ------- l4d2web/templates/overlays.html | 3 -- l4d2web/tests/test_global_overlays.py | 6 +-- l4d2web/tests/test_l4d2_facade.py | 2 +- l4d2web/tests/test_overlay_builders.py | 28 ++-------- l4d2web/tests/test_overlays.py | 53 +++++-------------- l4d2web/tests/test_workshop_overlay_models.py | 15 +++--- 14 files changed, 73 insertions(+), 120 deletions(-) create mode 100644 l4d2web/alembic/versions/0004_drop_legacy_external_overlay_type.py diff --git a/AGENTS.md b/AGENTS.md index de11cc4..b29f4b8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -41,7 +41,7 @@ Do not invent architecture outside these plans unless explicitly requested. - `logs --lines --follow/--no-follow` - Runtime paths are rooted at `LEFT4ME_ROOT`, defaulting to `/var/lib/left4me`. - Deployment/config management owns global units under `/usr/local/lib/systemd/system` and privileged helpers under `/usr/local/libexec/left4me`. -- Overlays are external directories (no overlay content management here). +- Overlay directories are populated by the web app (workshop downloads, managed-global refresh). The host library only mounts them. - Fail-fast subprocess behavior; pass raw stderr; propagate return code. - No lock manager, no rollback, no preflight runtime checks. - Delete missing instance/runtime dirs must succeed (no-op). diff --git a/README.md b/README.md index 91ea925..22f48a9 100644 --- a/README.md +++ b/README.md @@ -27,7 +27,7 @@ Implementation plans remain the source of truth for architecture and task sequen - `logs --lines --follow/--no-follow` - The web app calls host operations through `l4d2ctl`, not direct `l4d2host` imports. - Deployment uses `/var/lib/left4me` for runtime state, `/opt/left4me` for repository contents and the virtualenv, `/etc/left4me` for environment files, and global units under `/usr/local/lib/systemd/system`. -- Overlay handling is directory-based and externally populated. +- Overlay handling is directory-based; the web app populates each overlay (workshop downloads, managed-global refresh). - No lock manager, no rollback, no preflight checks in host library. - CLI propagates subprocess failures via stderr and return code. - `delete` on missing instance is no-op success. diff --git a/deploy/README.md b/deploy/README.md index b4a6831..6e3fa48 100644 --- a/deploy/README.md +++ b/deploy/README.md @@ -12,7 +12,7 @@ The deployment uses these paths: - `/opt/left4me`: deployed repository contents. - `/var/lib/left4me/left4me.db`: SQLite database used by the web app. - `/var/lib/left4me/installation`: shared L4D2 installation. -- `/var/lib/left4me/overlays`: overlay directories. External (admin-managed) overlays still live at any relative path under here; new overlays created through the web UI use `${overlay_id}` as their path. +- `/var/lib/left4me/overlays`: overlay directories. Each overlay lives at `${overlay_id}` under here. - `/var/lib/left4me/workshop_cache`: deduplicated cache of `.vpk` files downloaded for workshop overlays. One file per Steam item, named `{steam_id}.vpk`. Workshop overlays symlink into this tree. - `/var/lib/left4me/global_overlay_cache`: cache of non-Steam map archives and extracted `.vpk` files used by managed global map overlays. - `/var/lib/left4me/instances`: rendered instance specifications and per-instance state. @@ -60,13 +60,7 @@ Use a strong one-time password and rotate it after first login if needed. ## Overlay References -Overlay references are relative paths below `${LEFT4ME_ROOT}/overlays`. With the default deployment root, they resolve under `/var/lib/left4me/overlays`. - -Valid examples: - -- `standard` -- `competitive/base` -- `users/42/custom` +Overlay references are relative paths below `${LEFT4ME_ROOT}/overlays`. With the default deployment root, they resolve under `/var/lib/left4me/overlays`. New overlays use `${overlay_id}` as their path; the digit-only form is the only one created by the web app. Invalid references are rejected: @@ -75,6 +69,9 @@ Invalid references are rejected: - Empty path components such as `competitive//base`. - Symlink escapes that resolve outside `${LEFT4ME_ROOT}/overlays`. -Overlay content for `external` (admin-managed) overlays is populated outside the host library — typically via SFTP. The web app does not write into them. +The web app currently supports two overlay surfaces: -`workshop` overlays are populated by the web app: it downloads `.vpk` files from the public Steam Web API into `${LEFT4ME_ROOT}/workshop_cache/{steam_id}.vpk` and creates absolute symlinks under `${LEFT4ME_ROOT}/overlays/{overlay_id}/left4dead2/addons/{steam_id}.vpk`. Both the cache and the overlay directory are owned by the `left4me` runtime user; if the web service ever runs as a different uid, ensure it shares a group with the host process and that both trees are group-readable. +- `workshop` overlays (user-owned) — populated by downloading `.vpk` files from the public Steam Web API into `${LEFT4ME_ROOT}/workshop_cache/{steam_id}.vpk` and creating absolute symlinks under `${LEFT4ME_ROOT}/overlays/{overlay_id}/left4dead2/addons/{steam_id}.vpk`. +- Managed global overlays (`l4d2center_maps`, `cedapug_maps`, system-wide) — populated by the daily `left4me-refresh-global-overlays` job, which downloads archives into `${LEFT4ME_ROOT}/global_overlay_cache/` and symlinks them into the overlay directory. + +Both the caches and the overlay directories are owned by the `left4me` runtime user; if the web service ever runs as a different uid, ensure it shares a group with the host process and that both trees are group-readable. diff --git a/l4d2web/alembic/versions/0004_drop_legacy_external_overlay_type.py b/l4d2web/alembic/versions/0004_drop_legacy_external_overlay_type.py new file mode 100644 index 0000000..b04fcfa --- /dev/null +++ b/l4d2web/alembic/versions/0004_drop_legacy_external_overlay_type.py @@ -0,0 +1,32 @@ +"""drop legacy external overlay type + +Revision ID: 0004_drop_legacy_external_overlay_type +Revises: 0003_global_map_overlays +Create Date: 2026-05-08 +""" +from typing import Sequence, Union + +from alembic import op + + +revision: str = "0004_drop_legacy_external_overlay_type" +down_revision: Union[str, Sequence[str], None] = "0003_global_map_overlays" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + op.execute( + "DELETE FROM jobs " + "WHERE overlay_id IN (SELECT id FROM overlays WHERE type = 'external')" + ) + op.execute( + "DELETE FROM blueprint_overlays " + "WHERE overlay_id IN (SELECT id FROM overlays WHERE type = 'external')" + ) + op.execute("DELETE FROM overlays WHERE type = 'external'") + + +def downgrade() -> None: + # data is gone; intentional one-way migration + pass diff --git a/l4d2web/models.py b/l4d2web/models.py index 9c06361..c1b4ff8 100644 --- a/l4d2web/models.py +++ b/l4d2web/models.py @@ -57,7 +57,7 @@ class Overlay(Base): id: Mapped[int] = mapped_column(Integer, primary_key=True, autoincrement=True) name: Mapped[str] = mapped_column(String(128), nullable=False) path: Mapped[str] = mapped_column(String(512), nullable=False) - type: Mapped[str] = mapped_column(String(16), nullable=False, default="external") + type: Mapped[str] = mapped_column(String(16), nullable=False, default="workshop") user_id: Mapped[int | None] = mapped_column(ForeignKey("users.id"), nullable=True) created_at: Mapped[datetime] = mapped_column(DateTime, default=now_utc, nullable=False) updated_at: Mapped[datetime] = mapped_column(DateTime, default=now_utc, nullable=False) diff --git a/l4d2web/routes/overlay_routes.py b/l4d2web/routes/overlay_routes.py index 9c2b646..c036f32 100644 --- a/l4d2web/routes/overlay_routes.py +++ b/l4d2web/routes/overlay_routes.py @@ -29,8 +29,6 @@ def _can_edit_overlay(overlay: Overlay, user) -> bool: return False if user.admin: return True - if overlay.type == "external": - return False if overlay.type == "workshop": return overlay.user_id == user.id return False @@ -54,18 +52,13 @@ def create_overlay() -> Response: assert user is not None name = request.form.get("name", "").strip() - overlay_type = request.form.get("type", "external").strip().lower() + overlay_type = request.form.get("type", "workshop").strip().lower() if not name: return Response("missing fields", status=400) if not is_creatable_overlay_type(overlay_type, admin=user.admin): return Response(f"unknown overlay type: {overlay_type}", status=400) - if overlay_type == "external": - if not user.admin: - return Response("admin only", status=403) - scope_user_id: int | None = None - else: # workshop - scope_user_id = user.id + scope_user_id: int | None = user.id with session_scope() as db: if _name_already_taken(db, name, scope_user_id): diff --git a/l4d2web/services/global_overlays.py b/l4d2web/services/global_overlays.py index 8e0fb84..eb04422 100644 --- a/l4d2web/services/global_overlays.py +++ b/l4d2web/services/global_overlays.py @@ -37,7 +37,7 @@ GLOBAL_OVERLAYS = ( MANAGED_GLOBAL_OVERLAY_TYPES = {overlay.overlay_type for overlay in GLOBAL_OVERLAYS} USER_CREATABLE_TYPES = {"workshop"} -ADMIN_CREATABLE_TYPES = {"external", "workshop"} +ADMIN_CREATABLE_TYPES = {"workshop"} def is_creatable_overlay_type(overlay_type: str, *, admin: bool) -> bool: diff --git a/l4d2web/services/overlay_builders.py b/l4d2web/services/overlay_builders.py index 93ee1e3..ed1cb73 100644 --- a/l4d2web/services/overlay_builders.py +++ b/l4d2web/services/overlay_builders.py @@ -40,23 +40,6 @@ def _overlay_root(overlay: Overlay) -> Path: return get_left4me_root() / "overlays" / overlay.path -class ExternalBuilder: - """No-op builder for admin-managed overlays. Ensures the overlay directory - exists; everything inside it is the admin's responsibility (SFTP, etc.).""" - - def build( - self, - overlay: Overlay, - *, - on_stdout: LogSink, - on_stderr: LogSink, - should_cancel: CancelCheck, - ) -> None: - root = _overlay_root(overlay) - root.mkdir(parents=True, exist_ok=True) - on_stdout(f"external overlay {overlay.name!r} ready at {root}") - - class WorkshopBuilder: """Diff-apply symlinks under `left4dead2/addons/` against the overlay's current `WorkshopItem` associations. Cached items get an absolute symlink @@ -280,7 +263,6 @@ def _is_under(path: Path, root: Path) -> bool: BUILDERS: dict[str, OverlayBuilder] = { - "external": ExternalBuilder(), "workshop": WorkshopBuilder(), "l4d2center_maps": GlobalMapOverlayBuilder(), "cedapug_maps": GlobalMapOverlayBuilder(), diff --git a/l4d2web/templates/overlays.html b/l4d2web/templates/overlays.html index 6cb449c..fd6f012 100644 --- a/l4d2web/templates/overlays.html +++ b/l4d2web/templates/overlays.html @@ -37,9 +37,6 @@
Type - {% if g.user.admin %} - - {% endif %}

The path is generated automatically.

diff --git a/l4d2web/tests/test_global_overlays.py b/l4d2web/tests/test_global_overlays.py index 011c89b..ae10701 100644 --- a/l4d2web/tests/test_global_overlays.py +++ b/l4d2web/tests/test_global_overlays.py @@ -43,7 +43,7 @@ def test_ensure_global_overlays_repairs_existing_rows(tmp_path, monkeypatch): init_db() with session_scope() as session: - overlay = Overlay(name="cedapug-maps", path="legacy", type="external", user_id=None) + overlay = Overlay(name="cedapug-maps", path="legacy", type="cedapug_maps", user_id=None) session.add(overlay) session.flush() session.add( @@ -160,8 +160,8 @@ def test_enqueue_refresh_global_overlays_creates_system_job(tmp_path, monkeypatc def test_is_creatable_overlay_type_policy(): assert is_creatable_overlay_type("workshop", admin=False) is True - assert is_creatable_overlay_type("external", admin=False) is False - assert is_creatable_overlay_type("external", admin=True) is True assert is_creatable_overlay_type("workshop", admin=True) is True + assert is_creatable_overlay_type("external", admin=False) is False + assert is_creatable_overlay_type("external", admin=True) is False assert is_creatable_overlay_type("l4d2center_maps", admin=True) is False assert is_creatable_overlay_type("cedapug_maps", admin=True) is False diff --git a/l4d2web/tests/test_l4d2_facade.py b/l4d2web/tests/test_l4d2_facade.py index aa9e903..93a14dc 100644 --- a/l4d2web/tests/test_l4d2_facade.py +++ b/l4d2web/tests/test_l4d2_facade.py @@ -32,7 +32,7 @@ def server_with_blueprint(tmp_path, monkeypatch): session.add(user) session.flush() - overlay = Overlay(name="Standard Overlay", path="standard", type="external", user_id=None) + overlay = Overlay(name="Standard Overlay", path="standard", type="workshop", user_id=user.id) session.add(overlay) session.flush() diff --git a/l4d2web/tests/test_overlay_builders.py b/l4d2web/tests/test_overlay_builders.py index aab4be5..a017ad0 100644 --- a/l4d2web/tests/test_overlay_builders.py +++ b/l4d2web/tests/test_overlay_builders.py @@ -1,4 +1,4 @@ -"""Tests for overlay builders (registry, ExternalBuilder, WorkshopBuilder).""" +"""Tests for overlay builders (registry, WorkshopBuilder).""" from __future__ import annotations import os @@ -61,9 +61,9 @@ def _capture_logs(): return out, err, out.append, err.append -def test_registry_has_external_and_workshop() -> None: - assert "external" in overlay_builders.BUILDERS +def test_registry_has_workshop() -> None: assert "workshop" in overlay_builders.BUILDERS + assert "external" not in overlay_builders.BUILDERS def test_registry_unknown_type_raises_keyerror() -> None: @@ -71,28 +71,6 @@ def test_registry_unknown_type_raises_keyerror() -> None: overlay_builders.BUILDERS["nope"] -def test_external_builder_is_idempotent_noop_with_log(env: Path) -> None: - _, overlay_id = _create_user_and_overlay("ext", "external") - out, err, on_stdout, on_stderr = _capture_logs() - - with session_scope() as s: - overlay = s.query(Overlay).filter_by(id=overlay_id).one() - overlay_builders.BUILDERS["external"].build( - overlay, on_stdout=on_stdout, on_stderr=on_stderr, should_cancel=lambda: False - ) - - assert (env / "overlays" / "7").is_dir() - assert any("external overlay" in line for line in out), out - # Existing files in the overlay dir are not touched on subsequent build. - (env / "overlays" / "7" / "untouched.txt").write_text("data") - with session_scope() as s: - overlay = s.query(Overlay).filter_by(id=overlay_id).one() - overlay_builders.BUILDERS["external"].build( - overlay, on_stdout=on_stdout, on_stderr=on_stderr, should_cancel=lambda: False - ) - assert (env / "overlays" / "7" / "untouched.txt").read_text() == "data" - - def test_workshop_builder_creates_absolute_symlinks(env: Path) -> None: _, overlay_id = _create_user_and_overlay("ws", "workshop") cache_root = env / "workshop_cache" diff --git a/l4d2web/tests/test_overlays.py b/l4d2web/tests/test_overlays.py index e4f2c4f..90aa50e 100644 --- a/l4d2web/tests/test_overlays.py +++ b/l4d2web/tests/test_overlays.py @@ -38,8 +38,10 @@ def user_client_with_overlay(tmp_path, monkeypatch): with session_scope() as session: user = User(username="alice", password_digest=hash_password("secret"), admin=False) session.add(user) - # System external overlay (no user_id), pre-existing. - session.add(Overlay(name="standard", path="standard", type="external", user_id=None)) + # System overlay (managed-global, no user_id), pre-existing. + session.add( + Overlay(name="standard", path="standard", type="l4d2center_maps", user_id=None) + ) session.flush() user_id = user.id @@ -79,10 +81,10 @@ def test_admin_can_view_overlay_edit_controls(admin_client) -> None: assert 'action="/overlays"' in text -def test_admin_can_create_external_overlay(admin_client) -> None: +def test_admin_can_create_workshop_overlay_via_route(admin_client) -> None: response = admin_client.post( "/overlays", - data={"name": "standard", "type": "external"}, + data={"name": "standard", "type": "workshop"}, headers={"X-CSRF-Token": "test-token"}, ) assert response.status_code == 302 @@ -106,15 +108,6 @@ def test_overlay_ref_rejects_unsafe_components(overlay_ref: str) -> None: validate_overlay_ref(overlay_ref) -def test_non_admin_cannot_create_external_overlay(user_client_with_overlay) -> None: - response = user_client_with_overlay.post( - "/overlays", - data={"name": "bad", "type": "external"}, - headers={"X-CSRF-Token": "test-token"}, - ) - assert response.status_code == 400 - - def test_user_can_create_workshop_overlay(user_client_with_overlay) -> None: response = user_client_with_overlay.post( "/overlays", @@ -182,7 +175,7 @@ def test_two_users_can_have_workshop_overlay_with_same_name(tmp_path, monkeypatc def test_admin_can_update_and_delete_overlay(admin_client) -> None: create = admin_client.post( "/overlays", - data={"name": "standard", "type": "external"}, + data={"name": "standard", "type": "workshop"}, headers={"X-CSRF-Token": "test-token"}, ) assert create.status_code == 302 @@ -265,7 +258,7 @@ def test_update_overlay_rejects_duplicate_name(admin_client) -> None: for name in ("standard", "competitive"): response = admin_client.post( "/overlays", - data={"name": name, "type": "external"}, + data={"name": name, "type": "workshop"}, headers={"X-CSRF-Token": "test-token"}, ) assert response.status_code == 302 @@ -286,7 +279,7 @@ def test_update_overlay_rejects_duplicate_name(admin_client) -> None: def test_overlay_detail_page_lists_using_blueprints(admin_client) -> None: create = admin_client.post( "/overlays", - data={"name": "shared", "type": "external"}, + data={"name": "shared", "type": "workshop"}, headers={"X-CSRF-Token": "test-token"}, ) assert create.status_code == 302 @@ -366,8 +359,8 @@ def test_overlay_detail_page_404_when_missing(admin_client) -> None: assert response.status_code == 404 -def test_overlay_detail_hides_edit_for_non_admin_external(user_client_with_overlay) -> None: - # The seeded "standard" external overlay (id=1, user_id=NULL) is admin-only edit. +def test_overlay_detail_hides_edit_for_non_admin_managed_global(user_client_with_overlay) -> None: + # The seeded "standard" managed-global overlay (id=1, user_id=NULL) is read-only for non-admins. response = user_client_with_overlay.get("/overlays/1") text = response.get_data(as_text=True) @@ -377,26 +370,6 @@ def test_overlay_detail_hides_edit_for_non_admin_external(user_client_with_overl assert "delete-overlay-modal" not in text -def test_non_admin_cannot_view_other_users_private_non_workshop_overlay(user_client_with_overlay) -> None: - with session_scope() as session: - other = User(username="mallory", password_digest=hash_password("secret"), admin=False) - session.add(other) - session.flush() - overlay = Overlay( - name="private-external", - path="private-external", - type="external", - user_id=other.id, - ) - session.add(overlay) - session.flush() - overlay_id = overlay.id - - response = user_client_with_overlay.get(f"/overlays/{overlay_id}") - - assert response.status_code == 403 - - def test_managed_global_overlay_detail_shows_source_url(admin_client) -> None: overlay_id = _create_managed_global_overlay() @@ -410,7 +383,7 @@ def test_managed_global_overlay_detail_shows_source_url(admin_client) -> None: def test_overlay_update_redirects_to_detail(admin_client) -> None: create = admin_client.post( "/overlays", - data={"name": "standard", "type": "external"}, + data={"name": "standard", "type": "workshop"}, headers={"X-CSRF-Token": "test-token"}, ) assert create.status_code == 302 @@ -429,7 +402,7 @@ def test_overlay_update_redirects_to_detail(admin_client) -> None: def test_delete_overlay_rejects_in_use_overlay(admin_client) -> None: create = admin_client.post( "/overlays", - data={"name": "standard", "type": "external"}, + data={"name": "standard", "type": "workshop"}, headers={"X-CSRF-Token": "test-token"}, ) assert create.status_code == 302 diff --git a/l4d2web/tests/test_workshop_overlay_models.py b/l4d2web/tests/test_workshop_overlay_models.py index 37f0870..9489ae5 100644 --- a/l4d2web/tests/test_workshop_overlay_models.py +++ b/l4d2web/tests/test_workshop_overlay_models.py @@ -34,18 +34,18 @@ def test_overlay_has_type_and_user_id(db) -> None: s.add(Overlay(name="standard", path="standard")) s.flush() row = s.query(Overlay).filter_by(name="standard").one() - assert row.type == "external" + assert row.type == "workshop" assert row.user_id is None -def test_two_externals_with_same_name_are_rejected(db) -> None: +def test_two_system_overlays_with_same_name_are_rejected(db) -> None: with session_scope() as s: - s.add(Overlay(name="shared", path="shared", type="external", user_id=None)) + s.add(Overlay(name="shared", path="shared", type="l4d2center_maps", user_id=None)) s.flush() with pytest.raises(IntegrityError): with session_scope() as s: - s.add(Overlay(name="shared", path="other", type="external", user_id=None)) + s.add(Overlay(name="shared", path="other", type="cedapug_maps", user_id=None)) s.flush() @@ -161,9 +161,10 @@ def test_job_has_overlay_id_column(db) -> None: def test_overlay_id_does_not_reuse_after_delete(db) -> None: """SQLite AUTOINCREMENT must guarantee deleted IDs are never reused.""" + user_id = _make_user("alice") with session_scope() as s: - s.add(Overlay(name="first", path="1", type="external", user_id=None)) - s.add(Overlay(name="second", path="2", type="external", user_id=None)) + s.add(Overlay(name="first", path="1", type="workshop", user_id=user_id)) + s.add(Overlay(name="second", path="2", type="workshop", user_id=user_id)) s.flush() ids_before = sorted(o.id for o in s.query(Overlay).all()) last_id = ids_before[-1] @@ -174,7 +175,7 @@ def test_overlay_id_does_not_reuse_after_delete(db) -> None: s.flush() with session_scope() as s: - s.add(Overlay(name="third", path="3", type="external", user_id=None)) + s.add(Overlay(name="third", path="3", type="workshop", user_id=user_id)) s.flush() new_id = s.query(Overlay).filter_by(name="third").one().id