A registered drive that is unplugged rendered through the same path as any
other candidate — a "needs care" badge, "free of" with no numbers on either
side, an empty meter. To a first-time installer that reads as two broken disks
the scan turned up, with nothing tying the card back to a drive they registered
and later unplugged. Say "not connected", name the path, and draw no meter: a
meter with nothing in it is a claim about free space nobody measured. The same
locations are withheld from the dropdowns, since the wizard cannot stat a
directory on a drive that is absent.
Both dropdowns now end in "Custom path…", for a NAS mount or an LVM volume the
disk heuristics never rank as a candidate. Validation goes through
validateStep(3) rather than a disabled button: the apply side already refuses a
relative or system path, but its refusal is to fall back to the system disk,
and that is indistinguishable from having chosen the system disk on purpose.
A typed path is not a registered location, so setup_apply registers it via
storageAdd — which is what keeps the empty-directory admission rule and the
fitness checks in play — named after its basename, so it reads as "nas" rather
than "location-3" in the placement menus.
libreportal-storage: accept the name the listing prints. remove matched id and
path only, so `remove location-3` failed against a row displayed as
location-3. Root-owned helper changed, so footprint_version 10 -> 11.
Expose window.setupWizard: the instance was local to a promise in the
orchestrator and unreachable from the console or a test.
lp-storage-custom-test drives the step in a browser. Two holes it found in the
tests themselves, both the shape it exists to catch — a check whose failure
mode is to not run:
- It counted the cards that say "not connected" and asserted over those.
Turn the feature off and the count is zero, every() over an empty list is
true, and the block passed having checked nothing. The expectation now
comes from the feed.
- Both browser tests exited 0 whenever the page returned nothing. Under sudo,
where chromium will not start, they reported PASS having asserted nothing.
They now probe with `lp-shot --url` and curl: if the WebUI answers HTTP the
browser is the only thing that can have broken, and that is a failure.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
611 lines
58 KiB
Markdown
611 lines
58 KiB
Markdown
# LibrePortal — Storage Locations (per-app data placement)
|
||
|
||
**Status:** Phases 0-5 **built** (incl. the setup-wizard Storage step); the Disks WebUI page is not. · **Audience:** us, future-self · **Scope:** register more than one filesystem root for live app data, choose one per app, move an app between them, and resolve the right one on restore/migrate · **Origin:** "add different locations to set up LibrePortal on, with control per app" (2026-08-24)
|
||
|
||
---
|
||
|
||
## 0. The one idea
|
||
|
||
Today LibrePortal has **three** relocatable roots (`--system-dir` / `--containers-dir` / `--backups-dir`), each chosen once at install and fixed afterwards. This adds a fourth degree of freedom, on top of the existing containers root:
|
||
|
||
> **The containers root becomes a *list*. Every app declares which entry in that list holds its data. Everything else — install, compose, backup, restore, migrate — resolves the app's directory through one function instead of one variable.**
|
||
|
||
Nextcloud's 4 TB of photos go on the spinning disk. Vaultwarden and the control plane stay on the NVMe. Jellyfin's library lives on the external HDD that isn't always plugged in — and when it isn't, LibrePortal refuses to start Jellyfin rather than silently rebuilding it empty on the bare mountpoint.
|
||
|
||
## 1. Non-goals
|
||
|
||
- ❌ Relocating the **system** root (configs/db/logs) per-app. It stays one place, chosen at install. Same for the WebUI's own container dir.
|
||
- ❌ A general volume manager. We don't format, partition or mount anything — the drive must already be mounted; we validate and use it.
|
||
- ⚠️ **One exception, added deliberately:** on explicit request we append a single marked `/etc/fstab` entry so a registered drive comes back after a reboot (§6.3). Telling a non-expert "add this line to fstab yourself" is a wall, and the most likely outcome is a reboot where nothing starts.
|
||
- ❌ Striping/tiering/RAID-alikes. One app's data lives on exactly one location. No splitting an app across two.
|
||
- ❌ Per-*volume* placement inside an app (`./data` here, `./db` there). Location granularity is the app directory. Revisit only if a real need shows up.
|
||
- ❌ Merging the backup-location and storage-location **registries**. They differ in trust, lifecycle and ownership, so they stay separate — but they may freely share a *drive*, which is a supported and expected setup (§6.2).
|
||
|
||
## 2. What already works in our favour
|
||
|
||
The current code is closer to this than it looks:
|
||
|
||
- **Compose volumes are relative.** Every shipped template uses `./data:/config`, and `dockerComposeUp` does `cd $containers_dir$app_name && docker compose …`. Change the `cd` target and the app comes up unchanged, with the same compose project name (derived from the directory *basename*, which doesn't change). The only absolute host paths in any template are `/etc/localtime`, `/dev/net/tun`, `/etc/ssl/certs` and the docker socket — none of them ours.
|
||
- **An app dir is self-describing.** `<app>/<app>.config` + `docker-compose.yml` + `.libreportal-manifest.json` is everything needed to identify and rebuild it. Discovery by scanning is viable, so we never have to trust a stale index.
|
||
- **The three-root split already exists** end-to-end: flag parsing, `libreportalValidatePaths`, the nesting/protected-path refusals, the baked `__CONTAINERS_DIR__` placeholders, and `paths.sh` as the single source of truth. We're generalising a design that's already there, not inventing one.
|
||
- **Backup locations are the exact template** for the registry UX: `configs/backup/locations/<idx>/location.config`, `locationAdd`/`locationRemove`, `sourceBackupLocations`, a `libreportal backup location …` CLI, a WebUI *Locations* page, and `backupLocationLocalGuard` — which already implements the FAT/exFAT warning and the `REQUIRE_MOUNT` refusal we need verbatim.
|
||
- **`instance create` proves the model.** "An instance is just another app" — a cloned dir with its own slug and `CFG_<SLUG>_*` namespace. Per-app placement inherits multi-instance support for free.
|
||
|
||
## 3. The central constraint — root must not be told where to `chown`
|
||
|
||
This decides the whole design, so it leads.
|
||
|
||
The manager (`libreportal`) runs with a **scoped sudoers allowlist**: it may run the root-owned helpers in `/usr/local/lib/libreportal/` and a fixed system-binary set, and nothing else. No `sudo chown`, no `sudo tee`, no `sudo bash`. The helpers have the three roots **baked in at install** by `sed` (`__CONTAINERS_DIR__` …) precisely so that:
|
||
|
||
> the manager cannot redirect a root `chown` by editing a config file.
|
||
|
||
`libreportal-ownership` even re-checks its baked roots against a dangerous-path list as defence in depth. A naïve "storage locations live in a manager-writable config, and the helpers read it" **reopens that hole completely** — `chown -R dockerinstall /etc` is a full escalation.
|
||
|
||
**The fix: a root-owned registry with an empty-directory admission rule.**
|
||
|
||
1. The truth lives at `/usr/local/lib/libreportal/storage.roots`, **root:root 0644**. One record per line: `id<TAB>path<TAB>dev<TAB>fs_uuid`. Manager reads it; only root writes it. Every helper resolves an app dir through it instead of through a single baked constant.
|
||
2. Adding a location goes through a new helper, `libreportal-storage add <path>`, which is in the sudoers allowlist — so the manager *can* call it, but it accepts a path only if **all** of these hold:
|
||
- absolute, and `realpath -e` returns the input unchanged (no symlink component, no `..`)
|
||
- not in the protected set (`/ /etc /usr /bin /sbin /lib* /boot /proc /sys /dev /run /var /tmp /root /home`), and not inside any user's home unless the install was made with `--allow-home`
|
||
- does not nest — in either direction — with the system/containers/backups roots or any already-registered location
|
||
- **is an empty directory** (tolerating only `lost+found`) **or already carries a `.libreportal-storage` marker** — the adopt case, §3.1
|
||
|
||
The emptiness rule is what makes this safe: **root only ever chowns a directory that contains nothing**, so acceptance can't hand away anything that already existed. Everything created underneath afterwards is ours by construction.
|
||
3. On acceptance the helper writes a root-owned `.libreportal-storage` marker (location id + install id + created-at), `chown`s the root to the container owner, `chmod 0751`, and appends the record. `remove` refuses while any app dir still lives there.
|
||
|
||
Consequence worth stating plainly: **`storage add` is a genuine privilege boundary crossing, not a config edit.** The WebUI can drive it (through the task system → CLI → helper, same as every other mutating action), but the *helper*, not the WebUI, is the gate.
|
||
|
||
### 3.1 — Why "empty **or** marked" is still safe (and why the marker earns its keep)
|
||
|
||
Requiring strict emptiness would break the most valuable case there is: plugging in a drive that **already holds LibrePortal app data** from another install and adopting it. So the rule relaxes to *empty or carrying our marker* — and that relaxation costs nothing, because:
|
||
|
||
> writing the marker into a directory requires already being able to write that directory.
|
||
|
||
The manager can only plant a marker somewhere it can already write, and chowning a directory it already controls grants it nothing. There is no path where the marker gets the manager access it didn't have. (The fixed-path helpers can't be tricked into writing one elsewhere — none of them takes a caller-supplied destination.)
|
||
|
||
That single file then does **three** jobs, which is the main reason to like it:
|
||
|
||
| Job | How |
|
||
|---|---|
|
||
| Admission | "empty or marked" — §3 |
|
||
| **Mount detection** | the marker lives *on the drive*. Not mounted ⇒ bare mountpoint ⇒ no marker ⇒ location unavailable. No `findmnt`, no `fs_uuid` bookkeeping, and it works identically for USB disks, network mounts, and LUKS volumes that haven't been unlocked (§10.1) |
|
||
| Provenance on migrate | it carries the install id and location id the snapshot's manifest names, so "is this the same `bigdisk` the app came from?" is a file read (§9) |
|
||
|
||
**Residual wrinkle, not solved by elegance:** validate-then-`chown` is a TOCTOU window, and bash is a poor language for race-free path handling. The practical closure is to additionally require that the **parent** directory is not manager-writable — true for `/mnt`, `/srv`, `/media`, false for a path inside the manager's home. That's a real restriction on where locations may live, not a free lunch, and it should be stated in the docs rather than hidden.
|
||
|
||
## 4. The resolution layer — one function, ~200 call sites
|
||
|
||
The mechanical bulk of the work. Three new primitives in `scripts/source/paths.sh` (and mirrored inline in `init.sh`, per the existing keep-in-sync note):
|
||
|
||
```bash
|
||
storageRoots # every enabled root, primary first
|
||
appDir <slug> # the app's directory — memoised
|
||
pathIsContainerData <p> # is this path under ANY container root?
|
||
```
|
||
|
||
- **`appDir`** builds a `slug → dir` map once per process by scanning each enabled root for `*/<slug>.config`, memoises it in an associative array, and falls back to the primary root for a slug that doesn't exist yet (fresh install). Discovery-first, config-second: if the map and `CFG_<APP>_STORAGE` disagree, **the directory on disk wins** and we warn. That's what makes the system self-healing after a hand-move or a half-finished migration.
|
||
|
||
It is also **the single place the availability check belongs**. Every one of the ~200 sites has to call it, so a location whose drive is missing fails *once*, centrally, instead of needing a guard sprinkled at each caller. `appDir` returns non-zero **and** prints an unusable sentinel path, so the many callers that won't check `$?` still fail loudly on a path that cannot exist, rather than writing into a bare mountpoint. See §10.1.
|
||
- **`pathIsContainerData`** replaces the `[[ "$p" == "$containers_dir"* ]]` idiom that decides manager-vs-container-user elevation. It appears in ~10 files (`create_folder.sh`, `create_touch.sh`, `copy_file.sh`, `copy_files.sh`, `copy_folder.sh`, `copy_folders.sh`, `move_file.sh`, `runCfgOp`, `tags_manager_update.sh`, `webui_atomic_write.sh`). **Every one is a silent-corruption bug if missed** — a file under an unrecognised root gets written as the manager, lands with the wrong owner, and the container fails to read it at a moment far removed from the cause.
|
||
|
||
Scale of the sweep (measured, not estimated):
|
||
|
||
| Pattern | Count | Action |
|
||
|---|---|---|
|
||
| `$containers_dir$app`-shaped, app-scoped | ~186 across ~90 files | → `$(appDir "$app")` |
|
||
| `$containers_dir/libreportal/…` (the WebUI's own tree) | 63 | → `webuiDir` helper, **pinned to the primary root** |
|
||
| `$containers_dir/{traefik,prometheus,grafana,adguard,…}` — one app reaching into another by literal name | ~29 | → `appDir <name>`; those apps stay pinned in phase 1 (§10.3) |
|
||
| root-helper `$CONTAINERS_DIR/$app` | `libreportal-ownership`, `libreportal-appcfg` | → registry lookup inside the helper |
|
||
| the `== "$containers_dir"*` elevation test | ~10 | → `pathIsContainerData` |
|
||
|
||
`sourceScanFiles app_configs` must also loop the roots. Keep the existing `-maxdepth 3` + prune rules exactly as they are — and remember `*.config` is a reserved extension anywhere under an app dir.
|
||
|
||
**Finding the sites we missed.** A 90-file mechanical diff is exactly where a silent regression hides, and review alone won't catch a `$containers_dir$app` that survived. Make the runtime find them instead: once the sweep lands, **stop defining `containers_dir` as a usable path** and point it at a sentinel (`/nonexistent-libreportal-unconverted/`). Legitimate primary-root users have moved to `primaryRoot`/`webuiDir` by then, so every survivor now fails immediately, loudly, and *harmlessly* — on a path that cannot exist — with a sentinel string that greps straight out of the logs. Silent wrongness becomes a stack trace. Then a `scripts/dev/` linter keeps the pattern from coming back, the way `lp-task-names` guards task titles.
|
||
|
||
## 5. Per-app configuration
|
||
|
||
One new key in every app template, defaulting to the primary location:
|
||
|
||
```
|
||
# STORAGE = which storage location holds this app's data (see Storage on /admin)
|
||
CFG_BOOKSTACK_STORAGE=default
|
||
```
|
||
|
||
- The value is a **location name**, not a path. Names are what survive a migrate to a host whose disks are laid out differently; paths aren't. `default` always resolves to the install-time containers root.
|
||
- It renders as a dropdown on the app's config page for free — the WebUI config renderer already builds selects from the `[a:A|b:B]` comment convention; the option list is generated from the registry into the config comment on regen.
|
||
- Changing it in the config editor does **not** move data. It records intent; the move is `libreportal app move` (§8). The config page shows the current *resolved* directory next to the field, and flags a mismatch.
|
||
- `configBackfillMissingKeys` carries the new key into already-installed apps on the next update, so nothing needs a reinstall.
|
||
- **New compose tag** `#LIBREPORTAL|APP_DIR_TAG|APP_DIR_DATA`, filled in `dockerConfigSetupFileWithData` alongside the existing `CONTAINERS_DIR_TAG`, for the rare template that genuinely needs its own absolute host path. Most apps need nothing — relative volumes already do the right thing.
|
||
|
||
### 5.1 — Why the value is a name and not `CFG_<APP>_STORAGE_PATH`
|
||
|
||
A path reads better in isolation — `grep STORAGE *.config` would tell you where everything lives without consulting anything. It loses on the two things that actually happen:
|
||
|
||
- **Mount paths move.** External drives arrive at `/media/<user>/<label>` or `/run/media/…`, and those change on relabel, on a different desktop session, on a distro that mounts differently. With a name, re-register the location's path once and every app on it follows. With a path, every app config has to be rewritten — N edits, each able to fail halfway, and a half-finished pass leaves some apps agreeing with `appDir` and some not.
|
||
- **Migrate is a lookup, not a string match.** This feature's motivating case is "the restored config names a location this host doesn't have — ask the user." A name makes that a clean miss against the registry. A path makes it a comparison against a string that never meant anything on this host, and it can't distinguish "different disk layout" from "typo".
|
||
|
||
Debuggability doesn't require the path be *stored*, only *retrievable*: `libreportal storage list`, `libreportal app info <app>`, and the resolved directory shown beside the field on the config page (above) all print it. The split that keeps this honest — **the config stores the name because it is intent; the manifest (§9) stores name *and* path *and* `fs_uuid` because it is a record of fact.**
|
||
|
||
Note this deliberately diverges from the backup-location template, which offers `PATH_MODE=auto|custom` + an explicit `PATH`. There, restic runs as an ordinary user and can write anywhere it has permission, so a custom path is genuinely useful. Here, an app directory needs ownership that only root can establish, and root only touches registered roots (§3) — so a path outside the registry isn't discouraged, it's **unusable**. There is nothing for an override to override.
|
||
|
||
### 5.2 — The resolved path rides in the comment
|
||
|
||
The name is the value; the **path is written into the field's own comment**, so the file answers "where does this actually live?" without any tooling — the 2am-recovery case that makes a path attractive in the first place:
|
||
|
||
```
|
||
CFG_NEXTCLOUD_STORAGE=bigdisk # Storage Location - Currently at /mnt/bigdisk/apps/nextcloud [default:Primary (…)|bigdisk:Big disk]
|
||
```
|
||
|
||
Rules that keep this from becoming a liability:
|
||
|
||
- **Written only when it changes** — on install, on `app move`, and on a regen that finds it stale. Never unconditionally on every regen: the app `.config` is user-editable and lives in the container-owned tree, so a no-op rewrite is both file churn and an avoidable `runFileOp` on every pass. Compare, then write only on a difference.
|
||
- **It is a breadcrumb, not a source of truth.** `appDir` resolves by discovery; nothing ever *reads* this comment to decide anything. A stale one is cosmetic — and the regen self-heals it.
|
||
- **The WebUI panel doesn't rely on it.** `webui_generate_configs.sh` composes the field's description from `appDir` at generation time, so the editor shows the live path even if the on-disk comment hasn't caught up.
|
||
- The dropdown option list (`[default:…|bigdisk:…]`) is regenerated from the registry in the same pass, which is what makes the selector reflect locations added since the app was installed.
|
||
|
||
## 6. Fitness checks — what a location must pass before it can hold data
|
||
|
||
Admission (§3) answers *"is it safe for root to accept this path?"*. That is a **security** question and it lives in the root helper. It does not answer *"will app data actually work here?"* — a separate, larger question that runs in the manager, needs no privilege, and therefore can run **speculatively on a candidate the user hasn't chosen yet**, which is what lets the wizard (§7) show a verdict per disk before anything is committed.
|
||
|
||
Split them deliberately: security checks refuse, fitness checks grade. And **a fitness check only refuses when the location cannot work at all** — never merely because it's inconvenient or needs care. A removable drive is a supported configuration, not a mistake to be prevented (§6.1).
|
||
|
||
| # | Check | On failure | Why it matters |
|
||
|---|---|---|---|
|
||
| 1 | **Filesystem type** — `findmnt -no FSTYPE` | **refuse** on `vfat`/`exfat`/`ntfs`/`ntfs3`/`msdos`/`fuseblk` | no POSIX ownership ⇒ a rootless app dir is broken from the first write. Stricter than backups, where `backupLocationLocalGuard` only warns |
|
||
| 2 | **Mount options** — `findmnt -no OPTIONS` | **refuse** `ro` and `noexec`; note `nosuid`/`nodev` | some apps execute out of their data dir; a read-only remount is the silent killer |
|
||
| 3 | **Real ownership probe** | **refuse** | the decisive test, and the only one that catches NFS `root_squash` — which reports a perfectly respectable `nfs4` at check 1 and then can't `chown`. Create a temp dir, chown it to the container user, `stat` it back, remove it |
|
||
| 4 | **Sub-UID probe** | **refuse** | rootless docker maps container users into the 100000+ range. Chown the probe to a high uid; some FUSE and network mounts simply can't hold it, and the failure otherwise surfaces much later as an app that won't start |
|
||
| 5 | **Write / fsync / read-back** | **refuse** | catches a full disk, a flaky USB bridge, a silently-degraded mount |
|
||
| 6 | **Persistence across reboot** — `findmnt --fstab`, else a systemd `.mount` | **loud, durable warning — never refuse** | hand-mount a disk, register it, install Nextcloud, reboot, and the apps there won't start. That's worth saying clearly and repeatedly; it isn't worth blocking (§6.1). We don't write `fstab` (§1), but we say exactly what to add |
|
||
| 7 | **Removable / hot-plug** — `lsblk -o RM,HOTPLUG` | **loud, durable warning** + marker discipline | an external drive is a supported setup. The user gets told plainly what happens when it's absent — once at registration, and standing on the location afterwards |
|
||
| 8 | **Distinct device** — compare `st_dev` with the primary root | warn | same disk ⇒ the location buys nothing. Usually a misunderstanding, not an error |
|
||
| 9 | **Free space — per *device*, not per location** | refuse under a floor, warn under a fraction | locations can share a filesystem with each other and with a backup repo (§6.2), so they draw on one pool. Sum every LibrePortal consumer on the device |
|
||
| 10 | **Encryption at rest** — is it a LUKS/dm-crypt mapping? | informational | privacy-first product; worth surfacing, never worth blocking |
|
||
| 11 | **Shared device with a backup location** — `st_dev` vs every registered backup location | **warn, never refuse** | shared fate: one disk failure takes the data and its only snapshots (§6.2) |
|
||
|
||
Two more things this wants:
|
||
|
||
- **`libreportal storage check [<id>]`** runs the whole table against a registered location, and against an arbitrary path in *dry-run* mode (`--candidate`) so the wizard, the CLI and the WebUI all share one implementation. It fits the existing `scripts/checks/requirements/check_*.sh` convention.
|
||
- **It must repeat, not just gate at add time.** A location that passed in March can be full, remounted read-only, unplugged, or backed by a dying disk in June. Put `storage check` on the existing crontab alongside the other periodic checks and surface degradation as a dashboard warning — the same way a failing backup surfaces today.
|
||
|
||
### 6.1 — Why a non-persistent or removable drive warns instead of refusing
|
||
|
||
Blocking these was tempting and would have been wrong. "This drive isn't in `fstab`" and "this drive is removable" both describe a **supported setup** — the media library on the USB disk is one of the reasons to want this feature at all — not a broken one. A refusal there fails the honest user on their first attempt to do something reasonable, and teaches them to look for an override flag rather than to read the warning.
|
||
|
||
The reason we can afford it: **the dangerous moment is start-up, not registration, and start-up is already gated.** The marker file (§3.1) means a location whose drive is absent simply isn't available; `appDir` fails and `dockerComposeUp` refuses (§10.1). An app on an unmounted drive doesn't quietly rebuild itself empty — it doesn't start. Given that gate, refusing at registration is belt-and-braces that costs a legitimate use case and buys nothing the runtime doesn't already enforce.
|
||
|
||
So the obligation moves from *prevent* to *inform*, and "loud" has to mean **durable**, not a toast that scrolls away:
|
||
|
||
- an explicit warning on the wizard/CLI card at registration, stating the consequence in plain words — *"apps stored here will not start until this drive is mounted"* — plus the exact `fstab` line (or `systemd.mount` unit) that would make it permanent, ready to copy
|
||
- a **standing badge** on the device's row in the Disks view (§7.1) and in `storage list`, for as long as the condition holds. Six months later, when an app won't start, the reason should be visible without archaeology
|
||
- the resulting start-up refusal must name the cause: *"nextcloud is on location `bigdisk`, which is not mounted"* — never a generic failure
|
||
- `storage check` re-reports it on its periodic run, so a drive that *was* in `fstab` and silently stopped being one gets caught
|
||
|
||
A one-time acknowledgement at registration ("I understand apps here won't start unless this drive is mounted") is probably worth it too — it makes the choice deliberate without making it hard. That's a UX call, not an architectural one.
|
||
|
||
### 6.2 — Sharing a drive with a backup location
|
||
|
||
**Supported, deliberately.** One big disk holding both the app data and its snapshots is a completely reasonable home setup, and it's what most people will actually have. The registries stay separate (§1) — that's about trust and lifecycle, not hardware — but nothing stops the two from landing on the same device, and two facts make it cheap:
|
||
|
||
- **Ownership already matches.** `reconcile()` chowns both `CONTAINERS_DIR` and `BACKUPS_DIR` to the same container user, so a shared drive has one owner and no permission negotiation.
|
||
- **The existing nesting rule is already the right rule.** Sibling directories on one filesystem — `/mnt/bigdisk/apps` and `/mnt/bigdisk/backups` — don't nest, so §3 admits them today with no change.
|
||
|
||
**The one hard rule: same drive yes, nested never.** A storage location that *contains* a backup repo (or the reverse) is a recursive-inclusion trap — the backup engine walking a tree that holds its own repository. §3's nesting refusal already covers it in both directions and must not be relaxed for the shared-drive case. What does need work is the error: pointing storage at `/mnt/bigdisk` when `/mnt/bigdisk/backups` already exists fails the empty-directory rule, and the message must say *"use a subdirectory such as `/mnt/bigdisk/apps`"* rather than a bare "not empty" — that's the single most likely first attempt.
|
||
|
||
**The honest warning: shared fate.** One disk failure loses the data *and* the only copy of its backups. That deserves saying plainly and permanently, with the same durable-not-a-toast discipline as §6.1 — a standing badge on both the storage and backup location, and a line in the backup summary along the lines of *"3 apps have no copy off this device."*
|
||
|
||
Say it accurately, though, rather than moralising: a same-drive backup still protects against **accidental deletion, a bad update, a botched migration, and ransomware** (with append-only enabled). What it does not survive is the disk dying. Both halves are true and the user should get both — the failure it doesn't cover is specific and nameable, not a general "this is wrong."
|
||
|
||
Worth noting the compounding case explicitly, because it's the one people don't picture: if the shared drive is also *removable* (§6.1), then unplugging it takes the apps **and** the restore path away at the same moment. Neither warning implies the other; a location that is both should say both.
|
||
|
||
**Two things this changes in the checks:**
|
||
|
||
- **Free space becomes per-device, not per-location** (check 9). Two locations on one filesystem draw from one pool, so a growing restic repo can starve the apps sharing the disk — a real new failure mode where before there were two independent budgets. Account for every LibrePortal consumer on the device, keep a reserve, and warn on the *device*, not the location.
|
||
- **Add a shared-device check** — `st_dev` of a candidate storage location against every registered backup location, and vice versa. Never refuse; emit the shared-fate warning above.
|
||
|
||
**Two things it enables**, both nearly free:
|
||
|
||
- the picker on either side can **offer known drives** as suggestions, with the shared-fate note inline, instead of making the user type a path twice
|
||
- the storage marker (§3.1) is a strictly better mount test than the backup subsystem's `findmnt` probe — it catches a mountpoint shadowed by a stale directory, which `findmnt` reports as mounted. A backup location on a drive that also hosts a storage location can borrow it.
|
||
|
||
**Consequence for naming.** Sharing makes the collision worse, not better: the user now genuinely sees "bigdisk" in two places meaning two things. That's what the **Disks view** (§7.1) answers — one row per device, with *app data* and *backups* as roles on it rather than as competing top-level nouns. It's also where this section's shared-fate badge and per-device space accounting naturally live.
|
||
|
||
### 6.3 — Writing `/etc/fstab`, and why that is allowed here
|
||
|
||
§1 originally ruled this out, and reversing that deserves an argument rather
|
||
than a shrug.
|
||
|
||
The case for: the persistence warning (§6, check 6) is useless to the audience
|
||
this product is for. "Add `UUID=… /mnt/disk ext4 defaults,nofail 0 2` to
|
||
/etc/fstab" assumes SSH, root, an editor, and knowing what fstab is. Someone who
|
||
registers a drive, doesn't act on the warning, and reboots gets apps that refuse
|
||
to start — the exact failure we were trying to prevent, arrived at by a longer
|
||
road. LibrePortal already writes sysctl drop-ins, `modules-load.d`, systemd
|
||
units, sudoers and firewall rules; fstab is not a different category of file.
|
||
|
||
The case against is real and specific: **a bad fstab entry can leave a machine
|
||
unbootable**, needing rescue media. That is worse than any other failure mode in
|
||
this product.
|
||
|
||
What makes it defensible is `nofail`, plus `x-systemd.device-timeout=10s`.
|
||
Together they mean a missing device can never block boot — which is precisely
|
||
the failure the objection is about. Without that pair this would stay a non-goal.
|
||
|
||
The rules, all enforced in the root helper (`libreportal-storage fstab-add`):
|
||
|
||
| Rule | Why |
|
||
|---|---|
|
||
| `nofail,x-systemd.device-timeout=10s` always | a missing drive can't block boot or strand the box at a systemd timeout |
|
||
| `UUID=`, never `/dev/sdX` | device names reorder between boots; a stale one mounts the wrong disk or nothing |
|
||
| append inside a marked block, never rewrite | anything the user or another tool manages is untouched |
|
||
| refuse if the target or UUID is already described | we don't get to be the second opinion on someone else's mount |
|
||
| refuse the root filesystem outright | never our business |
|
||
| must currently be a real mount | we describe reality, we don't invent it |
|
||
| timestamped backup to `/etc/fstab.libreportal-*.bak` | recoverable |
|
||
| `findmnt --verify` before install; discard on failure | a file that doesn't parse never reaches `/etc` |
|
||
| opt-in only | nothing calls it unless a person ticked the box |
|
||
|
||
Verified against a real filesystem: the entry is added and verifies, the
|
||
persistence warning then disappears on the next scan, and a duplicate, the root
|
||
filesystem, a non-mountpoint and a relative path are each refused with the
|
||
reason.
|
||
|
||
## 7. Surfaces — first-run wizard, config panels, CLI
|
||
|
||
**First-run wizard.** The setup wizard (`core/setup/js/setup-wizard.js`) currently runs `Experience → Identity → Domains → Recommended → Metrics`, and already has the pattern for a step that isn't always shown: Metrics is advanced-only, and `_effectiveTotalSteps()` makes the count dynamic. A **Storage** step slots in **before Recommended** — locations must exist before apps get placed on them — and follows the same conditional rule, with a better trigger:
|
||
|
||
> **Only show it if the box actually has somewhere else to put things.** One disk, no candidates ⇒ the step never appears, and the 90 % case is unchanged.
|
||
|
||
Candidate detection is `lsblk` + `findmnt`, filtering the noise aggressively — `loop`/`squashfs`/snap mounts, `/boot`, `/boot/efi`, swap, `tmpfs`, `overlay`, anything under `/proc`, `/sys`, `/run`, and the filesystem already holding the primary root. On this dev box that filter has to drop ~15 snap loop devices to find the real answer, so it isn't optional. Each surviving candidate renders as a card: device, size, free, filesystem, mountpoint, and its §6 verdict — green / warned / refused. The three behave differently on purpose:
|
||
|
||
- **green** — selectable, no friction
|
||
- **warned** — fully selectable, with the consequence stated on the card and carried forward as a standing badge (§6.1). A USB drive lands here. It is *not* second-class; it's the reason the feature exists
|
||
- **refused** — shown **greyed with the reason**, never hidden. "Why isn't my drive listed?" is a support burden we don't need, and the reason ("exFAT can't store file ownership") is usually actionable
|
||
|
||
Refusals are reserved for locations that genuinely cannot work (checks 1–5). Nothing is blocked for merely needing care.
|
||
|
||
Registering from the wizard goes through the task system → CLI → `libreportal-storage` helper, exactly like every other mutating action. No chicken-and-egg: by the time the wizard runs, the manager is de-sudo'd but the helper is already in its scoped allowlist.
|
||
|
||
Nice follow-on, nearly free: the **Recommended** step comes next, so once a big disk is registered the bulky picks (Nextcloud, Jellyfin, Immich) can default onto it in the same payload — the wizard already posts one JSON blob to `setupApplyConfig`.
|
||
|
||
For unattended installs the wizard isn't there, so keep an `init.sh --storage-dir=` flag that pre-registers extra locations at install time (§13.5 resolved: build both — the flag is trivial once the helper exists).
|
||
|
||
**Config panels.** Two distinct editors, both generated:
|
||
|
||
- **Per-location config** — `configs/storage/locations/<id>/location.config` holding `CFG_STORAGE_LOC_<id>_*` (`NAME`, `ENABLED`, `NOTES`, `REQUIRE_MOUNT`…). `webui_generate_configs.sh` already has a bespoke loop for exactly this shape for backup locations (`configs/backup/locations/*/location.config`, which the flat-file scan can't reach at that depth) — the storage loop is a near-copy, and the WebUI *Storage* page renders from config metadata like every other page. Note the **path is not among these fields**: it's registry data, root-owned, and changing it is `storage add`/`remove`, not a config edit.
|
||
- **Per-app field** — the `CFG_<APP>_STORAGE` dropdown described in §5, on the app's existing config page.
|
||
|
||
**CLI.** `libreportal storage {list,add,remove,check,scan}` mirroring `libreportal backup location …` — `scan` being the candidate detection the wizard uses, so a headless user gets the same view. Plus `libreportal storage disks`, the CLI rendering of §7.1.
|
||
|
||
### 7.1 — The Disks view
|
||
|
||
**One row per filesystem, showing what LibrePortal does with it.** This is the answer to the naming collision (§13.1) and to shared drives (§6.2): the user stops holding "storage location" and "backup location" as two competing top-level nouns, and instead sees their actual hardware with *roles* on it.
|
||
|
||
**It extends a page that already exists.** `/admin/system/storage` is currently the "Docker disk breakdown" (`system-page.js:9`), and `webui_system_metrics.sh:80` already builds a `disks` array from `df -PB1` with tmpfs/devtmpfs/squashfs/overlay/aufs excluded. The Disks view is that array enriched and joined, not a new page bolted on. The dashboard's existing gauge stays as a summary that links here — which is also how §10.5 gets fixed properly rather than by adding a second misleading number.
|
||
|
||
**Three sources, unioned:**
|
||
|
||
| Source | Contributes |
|
||
|---|---|
|
||
| `lsblk -J -e7 -o NAME,UUID,PARTUUID,MODEL,TRAN,ROTA,RM,HOTPLUG,FSTYPE,SIZE,FSAVAIL,MOUNTPOINT` | hardware identity — model, USB vs NVMe, spinning vs solid, removable. `-e7` drops the loop devices (~15 snap mounts on a desktop box) |
|
||
| `df` — already generated | live capacity per mounted filesystem |
|
||
| both registries | which roles LibrePortal has claimed, and on what |
|
||
|
||
**The union matters more than the enrichment.** A registered drive that is currently unplugged does not appear in `lsblk` at all — and that is precisely the moment the user opens this page. So rows come from *the registry first*, attached hardware second:
|
||
|
||
> **A registered location whose device is missing must still render**, marked *not attached*, with the apps or backup repos that are stranded on it named. A row vanishing when the disk is pulled is the one failure this page cannot have.
|
||
|
||
**Identity is the `fs_uuid`**, not `/dev/sdb1` — device names reorder across reboots and would scramble the table. Fall back to `PARTUUID`, then the mountpoint, and say so in the row when identity is weak.
|
||
|
||
**Per row:** model and transport, size, filesystem, mountpoint, device-wide free space, mount state, and the **roles** — *System*, *App data (N apps)*, *Backups (N locations)*, or *Unused*. Then the standing badges from §6/§6.1/§6.2: transient (not in `fstab`), removable, shared fate, low space, unsupported filesystem. This is the natural home for all of them, because every one is a property of the device rather than of either registry.
|
||
|
||
**Actions on the row** are what make it a view rather than a report — register as app storage, add a backup location here, move apps here, unregister, re-run checks. That collapses "type the same path into two different pages" into one place, and the picker suggestions from §6.2 become unnecessary because the disk is already the thing you're looking at.
|
||
|
||
**Generator:** `scripts/webui/data/generators/system/webui_disks.sh` → `data/system/disks.json`, following the existing generator convention. Unused candidates are included (that's the §7 wizard's `storage scan`, same implementation) so the page answers "what else could I use?" as well as "what am I using?".
|
||
|
||
**Two honest limitations**, worth writing down rather than discovering later:
|
||
|
||
- **ZFS datasets report distinct `st_dev` values but share one pool**, so grouping by device over-reports available space — each dataset appears to have the pool's free space to itself. Btrfs subvolumes share a device and are fine. Detect ZFS and group by pool, or state the caveat in the row; don't silently under-count.
|
||
- **This stays read-only about the system.** No formatting, partitioning, mounting, or `fstab` writing (§1). The page can show the exact line to add and let the user copy it; it does not add it.
|
||
|
||
## 8. Moving an installed app
|
||
|
||
`libreportal app move <app> <location>` — a task like any other, with progress rows in the WebUI.
|
||
|
||
1. Resolve source and destination; refuse if they're the same, if the app isn't installed, or if the destination isn't mounted/writable.
|
||
2. **Space check** with headroom: `du -sb` of the source vs `df` of the destination, refuse under 110 %.
|
||
3. Compose **down** (data must be quiescent — a live copy of a running Postgres is a corrupt copy).
|
||
4. **Pre-move snapshot** to the first enabled backup location, reusing `migratePreBackupDestination`. Skippable with `--no-pre-backup`, on by default.
|
||
5. `runOwnership app-move <app> <dest-id>` — the copy runs as root inside the helper because app data contains rootless sub-UID files (postgres at uid 231141 etc.) that the manager can neither read nor recreate.
|
||
- same filesystem → `mv` (atomic, instant)
|
||
- across filesystems → `cp -a --reflink=auto` → verify → **only then** remove the source. Never `mv` semantics that could half-delete.
|
||
6. Update `CFG_<APP>_STORAGE`, re-run `dockerComposeUpdateAndStartApp` (the `cd` target moves with `appDir`), bring it up, and health-check.
|
||
7. On any failure before step 6 the source is untouched; after step 6, roll back by pointing the config at the source and restarting.
|
||
|
||
## 9. Restore and migrate — the interesting case
|
||
|
||
This is the scenario that motivated the feature: *"we migrate another install, it checks the path in the config against our current locations, and if there's no match the user picks an existing one or sets up a new one."*
|
||
|
||
**Add to `.libreportal-manifest.json`** (written into the app dir, so it rides inside the snapshot):
|
||
|
||
```json
|
||
"storage": { "location": "bigdisk", "path": "/mnt/bigdisk/apps/nextcloud", "fs_uuid": "…" }
|
||
```
|
||
|
||
**On restore/migrate**, resolve in this order: exact location-name match → a location whose `fs_uuid` matches → **ask**. The "ask" is a real WebUI step in the migrate wizard — *"This app came from location `bigdisk` (`/mnt/bigdisk/apps`), which this host doesn't have. Restore it to: [ default ▾ ] or [ + add a location ]"* — with a CLI equivalent (`--storage=<name>`) for unattended runs.
|
||
|
||
**The blocker that must be fixed first.** Restore currently does:
|
||
|
||
```bash
|
||
engineRestoreSnapshot "$idx" "$id" "/" "$containers_dir$app"
|
||
```
|
||
|
||
— restore to `/` with an include filter built from the **local** containers root. Restic reproduces the snapshot's absolute paths, so this only works when source and destination paths are byte-identical. That means:
|
||
|
||
- relocating on restore is impossible today, and
|
||
- **this is already broken** for the shipped three-root feature: migrating from a host installed with `--containers-dir=/mnt/ssd/apps` onto a default host matches no include path and restores *nothing*, quietly.
|
||
|
||
The fix is needed for this feature and is worth doing on its own: restore into `$restore_dir` staging with the snapshot's own path prefix, then move the tree into `appDir`. `resticRestoreAppLatest`, `restore_app_start.sh`, `migrate_apply.sh` and `migrate_preflight.sh` all take the source path from the snapshot/manifest rather than from the local `containers_dir`.
|
||
|
||
Also worth noting: an app's restic snapshot paths change when it moves. Snapshot *history* stays intact and restorable (each snapshot knows its own path), but path-based filters in the snapshot browser need to accept either.
|
||
|
||
**Built — and then found not to work, which is the part worth remembering.**
|
||
`storageSnapshotSourcePath` / `storageRestoreAppTo` landed as described above,
|
||
but the resolver asked for a snapshot with
|
||
|
||
```bash
|
||
engineSnapshotsJson "$idx" "$snapshot_id"
|
||
```
|
||
|
||
and that function's second parameter is an app **tag** filter. So it ran
|
||
`restic snapshots --tag app=<snapshot-id>`, matched nothing, and returned 1 on
|
||
every call. `storageRestoreAppTo` has a fallback for engines that cannot report
|
||
paths (borg genuinely cannot), so it took that fallback *always* — restoring in
|
||
place, which is exactly the blocker above, reinstated in the code written to
|
||
remove it.
|
||
|
||
It survived because nothing failed: the fallback is a legitimate branch, and the
|
||
restore preflight's only symptom was printing every app's size as `?`. Fixed by
|
||
`engineSnapshotPaths`, an engine call that takes a snapshot id because that is
|
||
what it is for. The lesson generalises — **a check with a fallback needs a test
|
||
that the check itself fires**, not just that the command succeeds
|
||
(`scripts/dev/lp-preflight-test`).
|
||
|
||
**What that actually cost, measured.** A restore onto a host whose containers
|
||
root differs from the backup's is the entire point of "rebuild my server", and
|
||
until this was fixed it restored *nothing* — silently, reporting success. The
|
||
in-place fallback builds restic's `--include` from the LOCAL root, so on a
|
||
relocated install the filter matched no path in the snapshot and zero bytes
|
||
landed.
|
||
|
||
Verified end to end on a purpose-built install (`scripts/dev/lp-install-matrix`
|
||
case 4: system on one disk, apps and backups on another), restoring a real
|
||
13-app repository taken from a default-layout host:
|
||
|
||
```
|
||
This snapshot was taken at '/libreportal-containers/bookstack';
|
||
restoring to '/mnt/lptest2/libreportal-containers/bookstack'.
|
||
✓ Restored bookstack to /mnt/lptest2/libreportal-containers/bookstack
|
||
```
|
||
|
||
Every app takes the stage-and-move branch there, and it crosses devices
|
||
(staging on disk 1, destination on disk 2), so `app-adopt` exercises its
|
||
copy-then-remove path rather than the same-device `mv`.
|
||
|
||
## 10. Risks, in the order they'll bite
|
||
|
||
**10.1 — An unplugged drive is the top data-integrity risk.** `crontab_boot_app_reconcile.sh` brings **every** installed app up at boot. If a location's drive isn't mounted yet — or at all — docker cheerfully creates the bind-mount directories on the bare mountpoint and the app boots **empty**. Plug the drive back in and there are now two divergent copies, with the good one hidden underneath the mount. Mitigation is mandatory, not optional:
|
||
|
||
The mitigation is **one test, not a family of them**: `.libreportal-storage` lives on the drive, so *marker present* ⇒ mounted, and *marker absent where the registry says a location is* ⇒ not mounted, refuse. That subsumes `findmnt`, `REQUIRE_MOUNT` and `fs_uuid` comparison into a single file read, and it behaves correctly for cases those don't cover — a LUKS volume that hasn't been unlocked, an NFS/SMB mount that dropped, a mountpoint shadowed by a stale directory.
|
||
|
||
Where it's enforced:
|
||
|
||
- **`appDir` fails** on an unavailable location (§4) — the central gate, hit by every caller by construction
|
||
- **`dockerComposeUp` refuses** to start such an app — the gate that actually protects data, since it also covers manual and task-triggered starts, not just boot
|
||
- boot reconcile skips those apps with a loud notice and a dashboard warning rather than failing silently
|
||
- keep `fs_uuid` in the registry anyway, but as *diagnostics* ("this is a different disk than the one registered"), not as the liveness test
|
||
|
||
**This gate is load-bearing.** Because registration deliberately does not block removable or non-persistent drives (§6.1), it is the *only* thing standing between an unmounted disk and an app rebuilding itself empty on the bare mountpoint. Weakening it — a "just start anyway" escape hatch, a caller that resolves a path without going through `appDir` — re-arms the exact failure §6.1 assumes is impossible. Anything that relaxes it has to revisit that decision too.
|
||
|
||
**10.2 — Missing an elevation-test site** (§4) produces wrong-owner files that fail much later. The sentinel-plus-linter approach in §4 is the mitigation: make the survivors fail loudly during the sweep, then make the pattern un-reintroducible.
|
||
|
||
**10.3 — Cross-app literal paths.** Traefik, Prometheus, Grafana, AdGuard, Gluetun, CrowdSec, Headscale are reached into by name from other apps' code. They're also small and infrastructural, so pinning them to the primary location costs a user nothing real.
|
||
|
||
Express it with **no new key at all.** "Pinned" is not a fact about the app's storage, it's a statement about whether the field is editable — and the config format already has a place for that: the comment. `**ADVANCED**` is parsed out of a field's comment today (`webui_generate_configs.sh:326`) to drive the editor's Advanced reveal. Add `**READONLY**` beside it, parsed the same way, and these templates simply ship:
|
||
|
||
```
|
||
CFG_TRAEFIK_STORAGE=default # Storage Location - Fixed: other apps reference Traefik by path **READONLY**
|
||
```
|
||
|
||
The value stays a normal `CFG_<APP>_STORAGE`, so `appDir` and the migrate path need no special case; only the renderer treats it differently. Relaxing an app later is deleting one token from a comment plus converting its call sites. And `**READONLY**` immediately earns its keep elsewhere: derived fields already warn in prose that nothing enforces — `crowdsec.config:72` literally says *"editing this value does not re-register the bouncer"* next to an editable input.
|
||
|
||
The WebUI's own dir stays pinned permanently and *structurally* (`webuiDir`), not by any config marker.
|
||
|
||
**10.4 — Filesystem capability.** Superseded by the fitness checks (§6), which is where this risk got its answer: checks 1–5 turn "hope the filesystem is suitable" into a refusal with a stated reason, and check 3 (a real chown probe) is what catches the cases type-sniffing alone misses.
|
||
|
||
**10.5 — `du`/`df` reporting.** The dashboard's disk gauge reads `/` only, which becomes actively misleading once apps are spread across disks. Answered by the Disks view (§7.1): the gauge stays as a summary that links there, rather than being duplicated into a second number that's wrong in a different way.
|
||
|
||
**10.6 — Uninstall/teardown.** `init.sh uninstall` prints and removes a fixed set of paths. It must enumerate the registry, and default to **leaving** external locations alone (with an explicit `--remove-storage` to wipe them) — an external drive is not ours to erase.
|
||
|
||
**10.7 — Footprint version.** New root helper + changed baked helpers ⇒ **bump `footprint_version`** (currently 5), or existing installs update the manager-owned code and keep root helpers that don't know about the registry.
|
||
|
||
## 10.8 — Postmortem: the index that became a fork bomb
|
||
|
||
Recorded because the mistake was subtle and the blast radius was the whole
|
||
machine.
|
||
|
||
`storageIndexSet` cached the app→root map at `configs/storage/app_locations`. The
|
||
file's stated requirements are "manager-owned" and "not on a removable disk", and
|
||
`configs/` satisfies both — which is exactly why it looked like the right home.
|
||
It carries a third property the file violated: **`sourceScanFiles` sources what it
|
||
finds under `configs/`, and sourcing means executing.**
|
||
|
||
The index is a TSV of `<slug><TAB><root>`. bash reads that as a command and its
|
||
argument. It stayed harmless while no slug matched a real executable — and armed
|
||
the instant a row existed for the app named `libreportal`, because that IS the CLI
|
||
on `PATH`. Sourcing ran `libreportal /libreportal-containers`, which re-entered the
|
||
scan, which sourced the file again: one process pair per level, from every CLI
|
||
invocation on the box including the task processor's own poll. 514 `start.sh`
|
||
processes within 100 seconds of boot; the kernel killed plasmashell, kwin and sddm
|
||
as collateral.
|
||
|
||
Two fixes, deliberately at both ends:
|
||
|
||
- `scan_files.sh` now requires a `.category` marker before sourcing anything in a
|
||
`configs/` subdirectory — the contract `commandReloadConfigs` already enforced,
|
||
and one all five real categories already satisfied.
|
||
- the index moved to `$system_dir/storage/app_locations`, with a one-shot
|
||
migration. A machine-written data file has no business in the one tree whose
|
||
contract is "everything here is executed", guard or no guard.
|
||
|
||
`scripts/dev/lp-configs-guard-test` pins both: a file of the exact detonating
|
||
shape placed in an unmarked `configs/` subdirectory must not execute, while a
|
||
marked category must still load.
|
||
|
||
**The generalisable rule:** before putting a file anywhere in this repo, ask what
|
||
the directory's *contract* is, not just who owns it. `configs/**` is executed.
|
||
`containers/<app>/**.config` is executed (see the note in `scan_files.sh`). Both
|
||
are load-bearing conventions that a plain data file silently violates.
|
||
|
||
## 11. What doesn't get a clean answer
|
||
|
||
Most of §10 collapses into a small number of good primitives — the marker file, `appDir`, the emptiness rule, a comment marker. Four things don't, and pretending otherwise would set us up to be surprised later.
|
||
|
||
- **The TOCTOU window in `libreportal-storage add`.** Validate-then-`chown` can't be made atomic in bash. The closure is a *restriction* — the parent directory must not be manager-writable — which rules out locations inside the manager's home and has to be documented as a rule users will occasionally hit and dislike.
|
||
- **Two registries that look like one.** Storage locations and backup locations will sit next to each other in the UI, both pointing at drives, with different lifecycles and different trust. No amount of engineering fixes that; it's a naming and information-architecture problem (§13.1) and the honest options are a genuinely different word or a single combined "Disks" page that owns both roles.
|
||
- **The sweep is big no matter how good the target API is.** ~90 files. Elegance makes the *result* better and the pattern un-reintroducible; it does not make the diff safe. Only the sentinel, the linter, and a full install → backup → restore → migrate soak on a real box do that — which is why phase 0 ships alone.
|
||
- **Moving a large app is just slow.** Cross-filesystem `cp -a` of a multi-terabyte Nextcloud is hours, needs root for sub-UID data, and the app is down throughout. There is no clever version. The deliverable is honesty about it: size estimate and duration warning up front, real progress in the task row, resumable on interruption, and never a delete of the source until the copy verifies.
|
||
|
||
## 12. Phasing
|
||
|
||
Each phase is independently shippable and independently verifiable. Phase 0 carries almost all of the regression risk and none of the user-visible value — land and soak it alone.
|
||
|
||
| Phase | Deliverable | Visible? |
|
||
|---|---|---|
|
||
| **0** ✅ | `appDir` / `storageRoots` / `pathIsContainerData` / `webuiDir`; ~260 call sites swept. `scripts/dev/lp-storage-test`. | No |
|
||
| **1** ✅ | Root-owned registry + `libreportal-storage` + fitness checks + `libreportal storage {list,add,remove,check,scan,apps,disks}`. | CLI only |
|
||
| **2** ✅ | `CFG_<APP>_STORAGE` in 37 templates + the `**READONLY**` marker + resolved path in the comment (§5.2) + per-location config panel emit. | Yes |
|
||
| **3** ✅ | Setup-wizard **Storage** step (§7), with a details modal and the opt-in `/etc/fstab` offer (§6.3). `--storage-dir=` for unattended installs is still open. | Yes |
|
||
| **4** ✅ | `libreportal app move` — stop, snapshot, copy as root, verify, then delete the source. | Yes |
|
||
| **5** ◐ | Manifest `storage` block and the staged restore + path rewrite are done (§9's latent bug is fixed). The "unknown location" **prompt** is not — an unresolvable location currently falls back to this host's default rather than asking. | Yes |
|
||
| **6** ◐ | The Disks **data layer** is built and `libreportal storage disks` renders it (including not-attached rows). The WebUI page at `/admin/system/storage` and the periodic `storage check` cron are **not**. | Yes |
|
||
|
||
## 12.5 — Testing this, and what it found
|
||
|
||
Multi-disk support has a specific failure shape: **anything that resolves a root
|
||
at runtime works on a default install and points at the wrong disk on a
|
||
relocated one.** Paths are baked into the root-owned helpers, the systemd unit
|
||
and the CLI wrapper at install time, so a code path that reads them from
|
||
somewhere else is invisible until the roots actually differ. Testing only "all
|
||
default" or only "all moved" misses it, because in both of those the wrong
|
||
answer often happens to be the right one.
|
||
|
||
So the matrix is the point, not the individual runs:
|
||
|
||
| | system | apps | backups |
|
||
|---|---|---|---|
|
||
| 1 | `/` | `/` | `/` |
|
||
| 2 | `/` | disk 2 | `/` |
|
||
| 3 | disk 1 | `/` | `/` |
|
||
| 4 | disk 1 | disk 2 | disk 2 |
|
||
|
||
scripts/dev/lp-testdisk up 2 30G # loopback ext4: real superblock,
|
||
# own st_dev, own free space, disposable
|
||
sudo scripts/dev/lp-install-matrix all
|
||
|
||
Each case checks the roots landed on the intended **device** (not merely the
|
||
intended path), the owners, that every helper was baked with no `__PLACEHOLDER__`
|
||
left, and that the WebUI answers.
|
||
|
||
What the matrix turned up that review had not:
|
||
|
||
- **`lp-shot` hardcoded `/libreportal-containers`** for both the compose file it
|
||
reads the port from and the `.auth.json` it signs a session with. On a
|
||
relocated install it fell back to a default port and a missing auth file —
|
||
which looks exactly like a WebUI that failed to boot.
|
||
- **The `@reboot` boot-reconcile resolved the default roots.** `paths.sh` takes
|
||
them from the environment, and a crontab entry is started by neither the CLI
|
||
wrapper nor the unit. That job brings every app up; pointed at the wrong root
|
||
it does not fail, docker just creates the missing bind-mount directories and
|
||
every app comes back empty (§10.1). Fixed by recovering them from the unit.
|
||
- **`libreportal-relocate` never rewrote the crontab**, whose entry embeds an
|
||
absolute path under the system dir — so after moving that root the job silently
|
||
did not exist.
|
||
- **The setup wizard called the app-data drive "System disk"** when it was not,
|
||
showing the data drive's size under the system disk's name.
|
||
- **The installer said nothing at all** when the only other drive was unmounted.
|
||
|
||
Case 4 is also the honest test of §9: restoring a repository taken on a
|
||
default-layout host onto it means *every* app takes the stage-and-move branch,
|
||
and staging (disk 1) and destination (disk 2) are different devices, so
|
||
`app-adopt` exercises its copy path rather than `mv`.
|
||
|
||
## 12.6 — The Storage step, after watching someone read it
|
||
|
||
Two things about the wizard's Storage step were wrong in a way only a fresh
|
||
pair of eyes catches.
|
||
|
||
**A registered drive that is unplugged.** `storage list` reports it as
|
||
`unmounted`, and the step rendered that through the same path as every other
|
||
candidate: a yellow "needs care" badge, "free of" with no numbers either side,
|
||
and an empty meter. To someone installing for the first time that reads as *the
|
||
scan found two broken disks* — there is nothing on the card connecting it to a
|
||
drive they registered earlier and have since unplugged. It now says **not
|
||
connected**, names the path, and draws no meter, because a meter with nothing in
|
||
it is a claim about free space that was never measured.
|
||
|
||
The same locations are also withheld from the two dropdowns. The wizard cannot
|
||
stat a directory on a drive that is absent, so it cannot promise an app placed
|
||
there would have anywhere to write.
|
||
|
||
**Custom paths.** The dropdowns offered only what the scan turned up, which is
|
||
wrong for a NAS mount, an LVM volume, or anything else the disk heuristics do
|
||
not rank as a candidate. Both now end in *Custom path…*, revealing a text box.
|
||
|
||
The validation lives in `validateStep(3)` rather than in a disabled button. The
|
||
apply side already refuses a relative or system path — but its refusal is to
|
||
fall back to the system disk, and *that is indistinguishable from having chosen
|
||
the system disk on purpose*. This is the failure shape this project keeps
|
||
having to fix (§10, and the compose-guard and `app-data-remove` bugs before it),
|
||
so the step blocks and says which rule the path broke.
|
||
|
||
A path typed here is not a registered location, so `setup_apply.sh` registers it
|
||
through `storageAdd` before writing `CFG_STORAGE_DEFAULT`. Going through
|
||
`storageAdd` rather than writing the path straight into the config is what keeps
|
||
the empty-directory admission rule and the fitness checks in play — the
|
||
alternative silently adopts a directory full of someone else's data. It is named
|
||
after its own basename, so it reads as `nas` in the placement menus rather than
|
||
`location-3`.
|
||
|
||
`libreportal storage remove` also learned to accept the name the listing prints.
|
||
It matched on id and path only, so `remove location-3` failed against a row the
|
||
table displayed as `location-3`.
|
||
|
||
### What the test found about the tests
|
||
|
||
`lp-storage-custom-test` drives the step in a real browser. Two things it caught
|
||
about itself are worth recording, because both are the same shape as the bugs it
|
||
exists to prevent — a check whose failure mode is to not run.
|
||
|
||
It first counted "how many cards say *not connected*" and asserted things about
|
||
those. Disabling the feature makes that count zero, `every()` over an empty list
|
||
is true, and the whole block passed while asserting nothing. The expected count
|
||
now comes from the feed (`storageCandidates` in state `unmounted`), so removing
|
||
the rendering is a failure rather than an empty set.
|
||
|
||
And both browser tests skipped — exit 0 — whenever the page returned nothing.
|
||
Run under `sudo`, where chromium refuses to start, they reported PASS having
|
||
checked nothing at all. They now probe the WebUI with `lp-shot --url` and curl:
|
||
if it answers HTTP then the browser is the only thing that can have broken, and
|
||
that is a failure, not a skip.
|
||
|
||
## 13. Open questions
|
||
|
||
1. ~~**Naming.** "Storage location" vs "backup location" in the same UI~~ — **resolved (2026-08-24):** build the Disks view (§7.1). The device becomes the organising concept and the two registries become *roles* on it, so the user never has to hold the distinction to understand their own hardware. Registries stay separate underneath.
|
||
2. ~~**Can a storage location double as a backup location?**~~ — **resolved (2026-08-24):** yes, sharing a drive is supported. Registries stay separate, sibling directories only (never nested), with a durable shared-fate warning and per-device space accounting. See §6.2.
|
||
3. **Does the `libreportal` app itself ever get to move?** Currently pinned. If the primary root fills up, that's a reinstall — acceptable, or worth solving?
|
||
4. **Per-instance vs per-type placement.** Instances inherit the model for free, but should `instance create` offer a location up front (likely yes — "work Nextcloud on the big disk, family on the SSD" is a good demo)?
|
||
5. ~~**`--storage-dir=` install flags**~~ — **resolved:** build it. The wizard covers the interactive case, but unattended installs have no wizard, and the flag is a few lines once the helper exists (§7).
|
||
6. ~~**How hard should check 6 (reboot persistence) refuse?**~~ — **resolved (2026-08-24):** it doesn't. Warn loudly and durably, never block; removable drives are a supported setup, and start-up is already gated by the marker test. No `--allow-transient` flag — there's nothing left for it to unlock. See §6.1.
|
||
7. **Docs promise.** `docs/guide/install-and-use.md` currently states the roots are *"chosen at install and fixed afterward … part of the security model."* That stays true of the three roots; the guide needs a paragraph explaining that additional **containers** locations are addable at runtime, and why the empty-directory rule keeps that honest.
|