docs(roadmap): fitness checks, first-run wizard step, config panels
Adds the three surfaces the plan was missing, and the check suite that gates them. §6 splits validation in two, which is the point: admission (§3) is a security question answered by the root helper and it refuses; fitness is "will app data actually work here", needs no privilege, and therefore can be run speculatively against a disk the user hasn't chosen — which is what lets the wizard grade candidates before anything is committed. Ten checks, of which two are worth calling out: a real chown probe, because NFS root_squash reports a healthy nfs4 and then can't chown; and reboot persistence, because hand-mount → register → install Nextcloud → reboot is §10.1's disaster on day one. Checks recur on the crontab rather than only gating at add time — a location that passed in March can be full, read-only or unplugged by June. §7 adds the setup-wizard Storage step. It follows the Metrics precedent for a conditional step but with a better trigger: only appear if the box has somewhere else to put things, so the one-disk case is untouched. Candidate detection has to filter hard — this dev box has ~15 snap loop devices to drop before the real answer shows. Refused candidates render greyed with the reason rather than vanishing. §5.2 puts the resolved path in the field's comment for hand-recovery, written only on change (install, move, or a regen that finds it stale) so it isn't churn on a user-editable file in the container-owned tree. It's a breadcrumb: nothing reads it to decide anything, and the WebUI panel composes its description from appDir so the editor is right even when the comment is stale. Phasing regrouped to 7 phases; §10.4 folded into §6; open question 5 resolved (build the flag — unattended installs have no wizard). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
librelad
parent
786d8ebfe8
commit
bb56b1db4b
@ -18,7 +18,7 @@ Nextcloud's 4 TB of photos go on the spinning disk. Vaultwarden and the control
|
||||
- ❌ A general volume manager. We don't format, partition, mount, or write `/etc/fstab`. The drive must already be mounted; we validate and use it.
|
||||
- ❌ 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.
|
||||
- ❌ Making backup locations and storage locations the same registry. They overlap on disk but differ in trust, lifecycle and ownership (see §9, §11.2).
|
||||
- ❌ Making backup locations and storage locations the same registry. They overlap on disk but differ in trust, lifecycle and ownership (see §11, §13.2).
|
||||
|
||||
## 2. What already works in our favour
|
||||
|
||||
@ -67,8 +67,8 @@ 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 (§8.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 (§7) |
|
||||
| **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.
|
||||
|
||||
@ -84,7 +84,7 @@ 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 §8.1.
|
||||
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):
|
||||
@ -93,7 +93,7 @@ Scale of the sweep (measured, not estimated):
|
||||
|---|---|---|
|
||||
| `$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 (§8.3) |
|
||||
| `$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` |
|
||||
|
||||
@ -112,7 +112,7 @@ 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` (§6). The config page shows the current *resolved* directory next to the field, and flags a mismatch.
|
||||
- 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.
|
||||
|
||||
@ -123,11 +123,71 @@ A path reads better in isolation — `grep STORAGE *.config` would tell you wher
|
||||
- **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 (§7) stores name *and* path *and* `fs_uuid` because it is a record of fact.**
|
||||
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.
|
||||
|
||||
## 6. Moving an installed app
|
||||
### 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.
|
||||
|
||||
| # | 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` | **refuse by default**, override with `--allow-transient` | **the check most worth having.** Hand-mount a disk, register it, install Nextcloud, reboot → §10.1's disaster on day one. We don't write `fstab` (§1) but we must *notice*, and say exactly what to add |
|
||||
| 7 | **Removable / hot-plug** — `lsblk -o RM,HOTPLUG` | warn + force marker discipline | an external drive is legitimate, but the user should be told once, loudly, what happens when it's absent |
|
||||
| 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** | refuse under a floor, warn under a fraction | |
|
||||
| 10 | **Encryption at rest** — is it a LUKS/dm-crypt mapping? | informational | privacy-first product; worth surfacing, never worth blocking |
|
||||
|
||||
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.
|
||||
|
||||
## 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-with-reason / refused-with-reason. A refused candidate is shown **greyed with the reason**, never hidden; "why isn't my drive listed?" is a support burden we don't need.
|
||||
|
||||
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.
|
||||
|
||||
## 8. Moving an installed app
|
||||
|
||||
`libreportal app move <app> <location>` — a task like any other, with progress rows in the WebUI.
|
||||
|
||||
@ -141,7 +201,7 @@ Note this deliberately diverges from the backup-location template, which offers
|
||||
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.
|
||||
|
||||
## 7. Restore and migrate — the interesting case
|
||||
## 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."*
|
||||
|
||||
@ -168,9 +228,9 @@ The fix is needed for this feature and is worth doing on its own: restore into `
|
||||
|
||||
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.
|
||||
|
||||
## 8. Risks, in the order they'll bite
|
||||
## 10. Risks, in the order they'll bite
|
||||
|
||||
**8.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:
|
||||
**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.
|
||||
|
||||
@ -181,9 +241,9 @@ Where it's enforced:
|
||||
- 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
|
||||
|
||||
**8.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.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.
|
||||
|
||||
**8.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.
|
||||
**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:
|
||||
|
||||
@ -195,41 +255,43 @@ The value stays a normal `CFG_<APP>_STORAGE`, so `appDir` and the migrate path n
|
||||
|
||||
The WebUI's own dir stays pinned permanently and *structurally* (`webuiDir`), not by any config marker.
|
||||
|
||||
**8.4 — Filesystem capability.** exFAT/NTFS/vfat can't hold POSIX ownership, so a rootless app dir on one is broken from the start — worse than for backups, where it's only a warning. For storage locations this must be a **refusal**, not a notice. Also check `noexec`/`nosuid` mount flags (some apps execute out of their data dir) and that the filesystem supports the sub-UID range rootless docker maps into.
|
||||
**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.
|
||||
|
||||
**8.5 — `du`/`df` reporting.** The dashboard's disk gauge reads `/` only. With apps spread over disks it needs one gauge per location, or the number is actively misleading.
|
||||
**10.5 — `du`/`df` reporting.** The dashboard's disk gauge reads `/` only. With apps spread over disks it needs one gauge per location, or the number is actively misleading.
|
||||
|
||||
**8.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.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.
|
||||
|
||||
**8.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.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.
|
||||
|
||||
## 9. What doesn't get a clean answer
|
||||
## 11. What doesn't get a clean answer
|
||||
|
||||
Most of §8 collapses into a small number of good primitives — the marker file, `appDir`, the emptiness rule, a declared pin flag. Four things don't, and pretending otherwise would set us up to be surprised later.
|
||||
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 (§11.1) and the honest options are a genuinely different word or a single combined "Disks" page that owns both roles.
|
||||
- **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.
|
||||
|
||||
## 10. Phasing
|
||||
## 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`; sweep the ~200 call sites; helpers read a registry that contains exactly one root. Full install + backup + restore verified. | No |
|
||||
| **1** | Root-owned registry + `libreportal-storage` helper + `libreportal storage {list,add,remove,check}`. Locations can exist; nothing uses them yet. | CLI only |
|
||||
| **2** | `CFG_<APP>_STORAGE`, install-time placement, per-location disk gauges. | Yes |
|
||||
| **3** | `libreportal app move`. | Yes |
|
||||
| **4** | Manifest `storage` block, staged restore + path rewrite (fixes §7's latent bug), the "unknown location" prompt in migrate. | Yes |
|
||||
| **5** | WebUI **Storage** page mirroring *Backup → Locations*: add/remove, mount state, free space, apps per location, drag-to-move. | Yes |
|
||||
| **1** | Root-owned registry + `libreportal-storage` helper + the §6 fitness checks + `libreportal storage {list,add,remove,check,scan}`. Locations can exist; nothing uses them yet. | CLI only |
|
||||
| **2** | `CFG_<APP>_STORAGE` + the `**READONLY**` comment marker, resolved path in the comment (§5.2), install-time placement, per-location config panels and disk gauges. | Yes |
|
||||
| **3** | Setup-wizard **Storage** step (§7) + `--storage-dir=` for unattended installs. Depends on 1 and 2; deliberately after them, so first-run drives a path that already works headlessly. | Yes |
|
||||
| **4** | `libreportal app move`. | Yes |
|
||||
| **5** | Manifest `storage` block, staged restore + path rewrite (fixes §9's latent bug), the "unknown location" prompt in migrate. | Yes |
|
||||
| **6** | WebUI **Storage** page mirroring *Backup → Locations*: add/remove, mount state, free space, apps per location, drag-to-move. Periodic `storage check` + dashboard degradation warnings. | Yes |
|
||||
|
||||
## 11. Open questions
|
||||
## 13. Open questions
|
||||
|
||||
1. **Naming.** "Storage location" vs "backup location" in the same UI — is that confusing enough to want a different word (*volume*? *disk*? *pool*?) for one of them?
|
||||
2. **Can a storage location double as a backup location?** Same drive, different subdirectory, two registries. Recommend keeping them separate but having the picker offer known paths. Backing an app up onto the same drive its data lives on should at least warn.
|
||||
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** for pre-registering extra locations at install time, or is post-install `storage add` enough?
|
||||
6. **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.
|
||||
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?** A hard refuse is right for a fixed internal disk and arguably wrong for "I plug this in when I want Jellyfin". `--allow-transient` covers it, but the default matters — and defaulting to refuse means a user with a genuinely removable drive hits a wall on their first attempt.
|
||||
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.
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user