diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 6957dd1..3a4d552 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -3,7 +3,8 @@ Status: design, no implementation yet. Scope: upgrade a Stalwart Mail Server in place from **0.15.5** to the current latest release (**0.16.14** as of 2026-08-19) with no data loss, a working -rollback at every step, and an automated post-migration validation pass. +a recovery point the operator provides, and an automated post-migration +validation pass. ## 1. Why this isn't a thin wrapper @@ -50,8 +51,9 @@ smoke test, not a full migration. **Goals** - Zero data loss for mail, calendar, and contact content (the one thing Stalwart itself guarantees is untouched — everything else is on us). -- Every phase has a defined, tested undo. Nothing destructive happens until - a verified backup exists. +- Nothing destructive happens until the operator has confirmed a recovery + point exists. This tool does not implement the undo (see the non-goals + and §4.8); it refuses to start without being told one is in place. - Fully automated happy path; the operator answers a preflight confirmation once, then watches (or walks away and checks the report). - Resumable: if the process dies mid-migration (crash, SSH drop, OOM), a @@ -65,6 +67,11 @@ smoke test, not a full migration. engine. **Non-goals** +- **Not a recovery tool.** Restoring a failed migration is the operator's + own snapshot or backup, by whatever method they already trust — ZFS/LVM/ + btrfs snapshots, VM or volume snapshots, or a restorable backup. This + tool does not take one, verify one, or restore from one. §4.8 explains + why that turned out to be the right split. - Not a general Stalwart config management tool (no drift detection, no day-2 ops beyond the migration window). - Not a replacement for routine backups — it *produces* a migration-time @@ -86,7 +93,9 @@ smoke test, not a full migration. │ dry-run) │ │ in depth) │ │ config │ │ (apply plan) │ │ smoke) │ │ + counts) │ └─────────────┘ └───────────┘ └────────────┘ └───────────────┘ └────────────┘ └────────────┘ │ │ │ │ │ │ - └─────────────────┴────────────────┴─── on failure ──┴─────────────────┴──▶ ROLLBACK + └─────────────────┴────────────────┴─── on failure ──┴─────────────────┴──▶ STOP + REPORT + (operator restores + their own snapshot) ``` Each box is a **phase**; each phase is a sequence of idempotent, checkpointed @@ -167,14 +176,15 @@ different at each layer: above the threshold by default (time cost), but available as `--full-content-backup` regardless of size. 4. **Binary preservation**: old binary is moved aside (`stalwart.v0155`), - never deleted, so rollback doesn't depend on re-downloading anything. + never deleted, so putting the machine back by hand doesn't depend on + re-downloading a specific old release under pressure. Every backup artifact is checksummed and the checksum recorded in the checkpoint file. Before moving past this phase, the tool **verifies** the filesystem backup by opening it read-only with the *old* binary in a throwaway temp directory and confirming it reports the expected version and -a sane account count — catching a corrupt or partial copy before it's relied -on, not after a failed rollback. +a sane account count — catching a corrupt or partial copy while the +pre-migration instance is still up, rather than after it isn't. ### 4.3 Stage @@ -206,8 +216,7 @@ fire-and-forget: `STALWART_RECOVERY_ADMIN` credential (random, never the operator's real password, never logged). 3. Poll the recovery HTTP endpoint until healthy or a timeout elapses; on - timeout, capture logs and fail into the rollback path rather than - hanging indefinitely. + timeout, capture logs and stop rather than hanging indefinitely. 4. Run `stalwart-cli apply --file export.json`, then the generated best-effort settings plan from §4.3, capturing full output. 5. Verify the apply reported success for every object (the tool parses the @@ -232,6 +241,48 @@ against an already-migrated store. the management API, and poll the task queue until it completes rather than firing and moving on. +**Status: implemented** (`internal/cutover`), but nothing calls it yet — see +§8. Notes on how it turned out: + +- It refuses to run at all unless the operator has confirmed a recovery + point exists (§4.8). That's an acknowledgement, not a check — this tool + can't verify someone else's snapshot — but it makes the irreversibility + of this phase impossible to walk into unasked. +- The unit is rewritten in place, not generated from a template: an + operator's unit carries hardening options, limits and dependencies this + tool has no business having an opinion about, and regenerating it would + silently drop them. It repoints `ExecStart` (preserving systemd's `-@:+!` + prefix characters and every argument after the executable), updates + `--config` if asked, and strips recovery-mode `Environment=` lines. It + refuses on a unit with no `ExecStart`, and on an `Environment=` line that + mixes a recovery variable with others — a line it only partly understands + is one it must not edit. +- The original unit is preserved and recorded as the `service-unit` + artifact *before* the rewrite, so an operator restoring by hand isn't + reconstructing a unit file from memory. +- Docker deployments are refused: cutting over a container means pulling a + new image and recreating the container, not swapping a binary and + rewriting a unit. +- Quota recalculation is the one step allowed to fail without failing the + phase. Stale counters are an accounting problem; a failed cutover is one + an operator has to respond to by restoring a machine that is otherwise + migrated and serving mail correctly. Calling for that over a counter + would be the worse outcome, so it warns and points at the WebUI's Tasks + panel. + +The quota call itself is grounded in Stalwart's `x:Task` schema reference +(`docs/ref/object/task/`), not guessed: `x:Task/set` creating one +`AccountMaintenance` variant per account with `maintenanceType: +"recalculateQuota"`, exactly as the WebUI's own "Recalculate disk quotas" +fans out. The upgrade guide only documents the WebUI path, so two details +remain unconfirmed against a live server and are called out in +`internal/stalwartapi/task.go`: whether the schema's "read-only" annotation +on `accountId`/`maintenanceType` means "immutable after creation" (it has +to, or the variant couldn't be created), and whether a finished task simply +leaves the queue (`TaskStatus` documents Pending/Retry/Failed with no +success state). That uncertainty is the reason this step warns rather than +fails. + ### 4.6 Patch-bump fast path For an already-0.16.x install moving to a newer 0.16.x patch (the common @@ -244,9 +295,9 @@ code path, so it doesn't rot independently. ### 4.7 Post-migration validation -Runs automatically after cutover; failure here triggers rollback (§4.8) -unless `--no-auto-rollback` was passed, in which case it just reports and -exits non-zero. +Runs automatically after cutover; failure here stops the run, reports +loudly, and exits non-zero, leaving the operator to decide what to restore +(§4.8). - **Version check**: reported server version matches the target exactly. - **Auth check**: WebUI login succeeds over the *configured hostname* via @@ -279,61 +330,64 @@ exits non-zero. numbers are non-zero/sane where preflight showed non-zero usage. Output is a single structured report (JSON + human summary): pass/fail per -check, with enough detail to hand to the operator or to a rollback decision. +check, with enough detail to hand to the operator deciding whether to +restore. -### 4.8 Rollback +### 4.8 Recovery from a failed migration — out of scope -Two triggers: automatic (validation failure, unless disabled) or manual -(`stalwart-migrate rollback `, usable any time up to a -"rollback window closed" checkpoint the operator explicitly confirms once -they're satisfied — see §6). +**This tool does not undo a migration.** Recovery is the operator's own +snapshot or backup, taken by whatever method they already trust and know +how to restore: ZFS/LVM/btrfs snapshots, a VM or volume snapshot, or a +restorable backup. This tool does not take one, verify one, or restore from +one. Cutover refuses to start until the operator confirms one exists (§4.5). -Procedure, checkpoint-resumable like everything else: -1. Stop the new service (or recovery-mode process, if failure happened - there). -2. Restore the filesystem/DB backup from §4.2 to the original path - (`.v0155-backup` → ``), or restore the targeted SQL - dump for external databases. -3. Restore the old systemd unit / Compose config. -4. Restart the preserved old binary. -5. Re-run a reduced version of the §4.7 validation suite against the - *restored* instance (version check, protocol reachability, directory - counts) to confirm rollback actually worked rather than assuming it did. -6. Report clearly that the instance is back on 0.15.5 and the new-version - artifacts (staged binary, export.json, apply-plan) are preserved - untouched for a retry after the underlying issue is fixed. +This replaced a working, tested rollback implementation, and the reasoning +is worth recording because the deleted code looked good: -Rollback never deletes anything from the failed attempt — a second forward -attempt reuses the existing backup and dumps rather than re-capturing -(faster retry, and one fewer chance for the retry's own backup step to -fail). +- **Restoring bytes correctly is not the hard part; restoring everything + else is.** The implementation copied file contents and permissions and + verified every restored file against a manifest — and did not preserve + ownership. Run as root, it produced a byte-perfect, checksum-verified, + root-owned data directory that Stalwart, running as its own user, could + not open. It would have reported success. A filesystem snapshot has no + such failure mode, because it never lost the metadata in the first place. +- **The external-database path was worse.** `pg_dump` without `--clean` + emits `CREATE TABLE` + `COPY`; replaying that into a database whose + tables still exist fails outright, and `ON_ERROR_STOP=1` — added so a + half-applied restore couldn't be reported as success — turned that into + a hard failure. The two SQL paths were asymmetric and only one was + plausibly correct. +- **It was never exercised against anything real.** Every test drove fake + `systemctl`, `psql` and `stalwart` binaries. That's sound for logic and + ordering, and it is not evidence about production. +- **Snapshots are already in the operator's runbook.** They are atomic, + metadata-preserving, cheap with copy-on-write, and cover the whole system + — binary, unit file, config, data — rather than the subset one tool + thought to capture. -**Status: implemented** (`internal/rollback`, plus `internal/service` for -the systemd/Docker control it needs) for the manual trigger. Departures from -the procedure above, and what it still doesn't cover: +What this tool keeps doing, so a manual restore is as easy as possible: -- Only the manual trigger exists. The automatic one fires on validation - failure during a real cutover, and there is no real cutover to fail yet. -- The procedure gains a step 0 this design didn't call out: the - backup is re-verified against its manifest *before* the service is - stopped. Finding a corrupt backup is survivable while the failed - instance is still up, and unsurvivable once the data directory has been - moved aside. -- FoundationDB is refused rather than attempted: §4.2's backup step only - *starts* an `fdbbackup` job, and restoring one means `fdbrestore` - against a quiesced cluster. Refusing up front beats a rollback that - reports success without restoring anything. -- Step 3 (restore the old unit/Compose config) is wired but inert: it - restores a preserved service definition if the run recorded one, and - nothing records one yet because cutover — the phase that would rewrite - it — doesn't exist. It reports as an explicit skip, not a silent pass. -- For an external SQL store, step 2 replays the critical-table dump in - place. Unlike the filesystem path, the current contents are *not* - preserved first; the plan the command prints says so before it acts. +- The old binary is preserved next to the new one (§4.2), never deleted. +- The original service definition is preserved as a `service-unit` artifact + before cutover rewrites it, so the operator doesn't reconstruct a unit + file from memory. +- Every artifact path and checksum stays in the checkpoint, and `status + ` prints exactly which steps completed and which failed. -`stalwart-migrate run` without `--dry-run` still refuses, but the reason -has narrowed: what's missing now is §4.5 cutover itself, not the ability to -undo it. +**The mail-delivery gap is accepted.** Restoring any pre-migration recovery +point discards mail delivered since it was taken. This was equally true of +the rollback implementation, is inherent to restoring a point in time, and +is not something this tool can solve. Plan the migration window +accordingly. + +**Two consequences worth being explicit about.** First, the confirmation +cutover requires is an assertion, not a check — an unverifiable promise is +weaker than a guarantee, and the value is only that nobody migrates a +production mail server having never been asked the question. Second, there +is no longer an automatic response to a failed migration: a failure stops +the run and reports, and a human decides what to restore. Both are +deliberate trades for not shipping a recovery path that has never been +tested against a real server. ### 4.9 Dry run @@ -363,7 +417,7 @@ post-migration boot check, just pointed somewhere disposable: alternative exists. 4. The verified backup copy is cloned again into the sandbox directory (never reusing the same directory recovery mode is about to mutate as the - one rollback would restore from). + one a manual restore would use). 5. **Recovery-mode migration** (§4.4) runs for real against the sandbox: the actual target binary, actual `STALWART_RECOVERY_MODE=1` boot, actual `stalwart-cli apply`. @@ -419,37 +473,39 @@ is the same problem. stalwart-migrate preflight [--config PATH] ... # read-only, prints the report stalwart-migrate run --dry-run [--target-binary PATH] ... # implemented — see §4.9 [--keep-artifacts] - (without --dry-run: refused today — see §4.8's status note) + (without --dry-run: refused today — see §8) stalwart-migrate status [run-id] # implemented -stalwart-migrate rollback [--yes] # implemented — see §4.8 -stalwart-migrate confirm # not yet implemented stalwart-migrate report [--json] # not yet implemented ``` -`run` is the only command that mutates anything on a *successful* path, and -it always starts with preflight. `rollback` mutates too, by design — it's -the one command that stops a running mail server and overwrites a live data -directory — so it prints the plan it resolved from the run's checkpoint and -refuses to act without `--yes`. Once rollback exists, `confirm` will be a separate, explicit step -so backups aren't pruned just because validation passed automatically — the -operator gets a beat to actually use the migrated server before disk space -is reclaimed. Default retention if never confirmed: configurable TTL, warns -loudly, never auto-deletes silently. Flags shown here are the design intent; -run `stalwart-migrate -h` for the actual current flag set. +`run` is the only command that mutates anything, and it always starts with +preflight. Nothing in this tool restores a failed migration (§4.8), so there +is no `rollback` command, and no `confirm` step to close a rollback window +that no longer exists. + +The migration-time artifacts a run leaves behind — the preserved old binary, +the settings and principals dumps, the preserved service definition, and +(for the dry-run path) the filesystem copy — are never pruned automatically. +They're small next to the data directory, they're what a manual restore +reaches for first, and deleting them on a schedule to reclaim disk would be +the tool making a call that isn't its to make. Flags shown here are the +design intent; run `stalwart-migrate -h` for the actual current +flag set. ## 7. Project layout (Go, matches this workspace's other CLI tools) ``` stalwart-migrator/ - cmd/stalwart-migrate/ main.go, preflight.go, run.go, status.go, rollback.go — CLI entry + wiring + cmd/stalwart-migrate/ main.go, preflight.go, run.go, status.go — CLI entry + wiring internal/plan/ version-boundary → ordered phase list (§4.6) [done] internal/checkpoint/ run-id, state.json read/write, resume logic (§5) [done] internal/preflight/ §4.1 checks [done] internal/backup/ §4.2 — fs/db snapshot, settings dump+convert, Vandelay export [done] internal/recovery/ §4.4 — recovery-mode process supervision + apply [done] + internal/cutover/ §4.5 — binary swap, unit rewrite, restart, quota rebuild [done, unwired] internal/validate/ §4.7 — boot-check + content-integrity done; DKIM/TLS + mail-flow not yet [partial] - internal/rollback/ §4.8 [not started] - internal/stalwartapi/ Ping + AccountSnapshot, incl. per-mailbox counts via impersonation (§8) [done] + internal/service/ systemd/Docker start+stop, used by §4.5 [done] + internal/stalwartapi/ Ping, AccountSnapshot (per-mailbox counts via impersonation), quota tasks [done] internal/config/ tool's own config (paths, thresholds, credentials handling) [not started] docs/ this file + phase-specific notes as they get built out ``` @@ -458,7 +514,11 @@ There's no separate `internal/stage` package: the `convert` half of `migrate_v016.py` lives in `internal/backup` next to `dump` (same script, same invocation pattern), and the dry-run sandbox-cloning logic that stands in for the rest of §4.3 currently lives directly in `cmd/run.go` rather than -its own package, pending a real cutover phase to generalize it against. +its own package. Now that `internal/cutover` exists, that's the code a real +staging phase would be generalized out of. + +There's no `internal/rollback` either, and that's a deliberate removal +rather than a gap — see §4.8. `internal/stalwartapi` is deliberately the only thing that speaks JMAP/HTTP to Stalwart — every other package depends on it, not on `net/http` directly, @@ -466,9 +526,7 @@ so auth handling and retry/backoff live in one place. `internal/service` is the same idea for the other external surface: it is the only thing that shells out to `systemctl` or `docker`, so the commands that can take mail delivery down sit in one auditable file rather than in each phase that -happens to need them. Rollback needs it today and cutover will need exactly -the same operations, which is why it's its own package rather than living -inside `internal/rollback`. `preflight.DeploymentKind` is a type alias for +happens to need them. `preflight.DeploymentKind` is a type alias for `service.Kind`, so detection and control can't drift apart. ## 8. Open questions for the next pass @@ -535,28 +593,43 @@ inside `internal/rollback`. `preflight.DeploymentKind` is a type alias for implemented. With this done, recovery, backup, dry-run, and account/ mailbox snapshotting all work end-to-end, and dry-run's comparison is now the closest thing to §4.7's actual no-data-loss guarantee this tool has — - the remaining major gap is `internal/rollback` and real cutover (below). -- **Rollback: done. Real cutover: still missing.** `internal/rollback` and - `internal/service` are implemented and tested (§4.8), so `stalwart-migrate - rollback ` can now undo a run: it re-verifies the backup, stops the - service, moves the failed attempt aside without deleting it, restores the - data directory (re-verifying every restored file against the manifest) or - replays the SQL dump, reinstalls the preserved binary, restarts, and runs a - reduced validation suite against the *restored* instance rather than - assuming it worked. `run` without `--dry-run` still refuses, but now for - the narrower reason that §4.5 cutover itself isn't built - the thing that - would swap the binary, rewrite the unit, and switch the service over. Doing - rollback first was deliberate: this tool should never be able to commit to - a change it can't undo. -- **`confirm` still has no implementation**, so nothing can set - `RollbackWindowClosed` - rollback honours the flag and refuses when it's - set, but only a hand-edited state.json can currently set it. Closing the - window is the point of no return for the backups this restores from, so it - should land together with the retention/TTL policy §6 describes, not - before it. -- **Cutover must preserve the service definition it rewrites**, recording it - as a `service-unit` artifact; rollback's restore step already reads that - contract and reports an explicit skip until something writes one. + the remaining major gap is §4.3 staging and the production pipeline + (below). +- **Cutover is built; nothing wires it into a production run yet.** + `internal/cutover` (§4.5) and `internal/service` are implemented and + tested. `run` without `--dry-run` still refuses, for one remaining + reason: **§4.3 stage doesn't exist**, and neither does the production + pipeline that would run preflight → backup → stage → recovery-mode → + cutover → validate against real paths instead of a sandbox. What stage + still needs: downloading and verifying the target binary into a staging + path (`preflight.ResolveRelease` and `backup.DownloadFile` between them + already have the pieces), running the convert step against real paths + rather than the dry-run's patched sandbox ones, and the best-effort + settings apply-plan, which is its own open question below. +- **Nothing has ever run against a real Stalwart.** Every test in this + repo drives fake `systemctl`, `psql` and `stalwart` binaries and + httptest servers. That's sound for logic and ordering and is not + evidence about production. One smoke test on a throwaway VM - real + 0.15.5, real systemd unit, a few accounts with mail - would settle the + quota wire format, systemd drop-in handling, and cutover's unit rewrite + at once. It should happen before §4.3 is wired, not after. +- **Quota recalculation is grounded but unproven.** The `x:Task` wire + format comes from Stalwart's schema reference rather than a live server; + §4.5 lists exactly which two details are inferred. A smoke test against a + real 0.16 instance would settle both, and would let this step be promoted + from "warns on failure" to a hard check. +- **Cutover doesn't handle Docker.** It refuses container deployments + outright, since cutting one over means pulling an image and recreating + the container rather than swapping a binary and rewriting a unit. +- **Cutover ignores systemd drop-ins.** It rewrites only the main unit + file, so an `ExecStart` or `Environment` override in + `/etc/systemd/system/stalwart.service.d/*.conf` is invisible to it - + including a recovery-mode variable set there, which is exactly the + footgun the rewrite exists to prevent. Drop-ins are common enough that + this needs handling before a production run, at minimum by detecting + them and refusing. +- **Nothing prevents concurrent runs.** Two invocations against the same + run-id would both proceed; there's no lock file or equivalent. - **Dry-run's un-stopped backup snapshot** (§4.9 step 2): a dry-run still backs up a live, in-use store unless the operator stops it manually first. `internal/service` now makes doing this properly possible - dry-run just diff --git a/README.md b/README.md index 8238dca..5d3afb9 100644 --- a/README.md +++ b/README.md @@ -1,17 +1,20 @@ # stalwart-migrator In-place upgrade tool for Stalwart Mail Server, 0.15.5 → latest: no data -loss, a checkpoint at every step so a failure can be undone, and automated -validation that the server still works afterwards. +loss, a checkpoint at every step so an interrupted run resumes instead of +restarting, and automated validation that the server still works afterwards. + +**Recovery from a failed migration is your own snapshot or backup — this +tool does not undo a migration.** See [Recovery is your +job](#recovery-is-your-job) before using it on anything you care about. Go, standard library only — no external dependencies. ## Status -Partially implemented. Roughly 8,400 lines of tested code across backup, -preflight, checkpointing, validation, recovery and rollback. What's missing -now is the real cutover phase — the step that actually swaps the binary and -switches the live service over. +Partially implemented. Roughly 8,800 lines of tested code. Every phase +except staging now exists as a package, including cutover, but nothing wires +them into a production run yet, so `run` still refuses. | Command | State | |---|---| @@ -19,35 +22,28 @@ switches the live service over. | `stalwart-migrate run --dry-run` | **Works** — preflight, real backup, sandboxed trial conversion | | `stalwart-migrate run` | **Refuses on purpose** — see below | | `stalwart-migrate status ` | **Works** | -| `stalwart-migrate rollback ` | **Works** — prints its plan; acts only with `--yes` | -| `stalwart-migrate confirm ` | Not implemented | | `stalwart-migrate report ` | Not implemented | -**`run` without `--dry-run` deliberately refuses to proceed.** Rollback and -service control now exist, so the reason has narrowed: what's still missing -is cutover itself (ARCHITECTURE.md §4.5) — installing the new binary, -rewriting the service definition, and starting the migrated instance for -real. `run` stops rather than going partway. That refusal is the correct +**`run` without `--dry-run` deliberately refuses to proceed.** Cutover +(ARCHITECTURE.md §4.5) is implemented and tested, but nothing calls it: the +staging phase (§4.3) and the production pipeline that would run preflight → +backup → stage → recovery-mode → cutover → validate against real paths don't +exist yet. `run` stops rather than going partway. That refusal is the correct behaviour today, not a bug. -Rollback was built before cutover on purpose: this tool should never be able -to commit to a change it can't undo. `stalwart-migrate rollback ` -resolves what it would do from the run's own checkpoint, prints that plan, -and touches nothing without `--yes`. - Package state: | Package | Lines | Tests | |---|---|---| -| `internal/rollback` | 1807 | yes | -| `internal/backup` | 1805 | yes | +| `internal/backup` | 1806 | yes | | `internal/preflight` | 1329 | yes | +| `internal/stalwartapi` | 1276 | yes | +| `internal/cutover` | 1186 | yes | | `internal/validate` | 792 | yes | -| `internal/stalwartapi` | 716 | yes | | `internal/recovery` | 702 | yes | -| `internal/checkpoint` | 559 | yes | +| `internal/checkpoint` | 556 | yes | | `internal/service` | 467 | yes | -| `internal/plan` | 196 | yes | +| `internal/plan` | 195 | yes | | `internal/config` | stub | — | ## Why not a shell script @@ -55,8 +51,9 @@ Package state: Stalwart's 0.15 → 0.16 boundary is not a drop-in binary swap: settings move, and the data directory has to be migrated rather than merely copied. The failure mode that matters is a half-migrated mail store with no way back — -which is why backup verification, checkpointing, and rollback are the design -centre rather than conveniences bolted on afterwards. +which is why backup verification and checkpointing are the design centre +rather than conveniences bolted on afterwards — and why the tool refuses to +cut over until you confirm you have a way back. [`ARCHITECTURE.md`](ARCHITECTURE.md) covers this in full: §1 on why a thin wrapper is insufficient, §4 on the migration phases, §5 on the checkpoint @@ -100,38 +97,60 @@ you care about. Where the plan crosses the 0.15/0.16 boundary it clones that verified backup into a disposable sandbox and converts the copy, leaving the original untouched. -## Rolling back +## Recovery is your job -`rollback` is the one command that stops a running mail server and -overwrites a live data directory, so it never acts on its own reading of the -situation without showing you that reading first: +**This tool does not undo a migration.** There is no `rollback` command. +Recovery from a failed migration is your own snapshot or backup, taken by +whatever method you already trust and know how to restore — a ZFS, LVM or +btrfs snapshot, a VM or volume snapshot, or a restorable backup. Choosing +that method, taking it, and verifying you can actually restore from it is +out of scope for this tool: it does not take one, does not check that one +exists, and cannot restore from one. -```sh -stalwart-migrate rollback # prints the plan, touches nothing -stalwart-migrate rollback --yes # performs it -``` +Cutover refuses to start until you confirm a recovery point exists. That +confirmation is an acknowledgement, not a check — nothing here can verify +your snapshot. Its only purpose is that nobody migrates a production mail +server having never been asked the question. -Everything in the plan — which backup, which manifest, which preserved -binary — comes from that run's own checkpoint, so rolling back days later -doesn't depend on remembering any of it. What the checkpoint deliberately -doesn't store (database credentials for an external SQL backend) is what the -flags are for. +**Take the snapshot with the service stopped** if you want a clean one. A +snapshot of a running Stalwart is crash-consistent rather than clean; RocksDB +will usually recover from its WAL, but "usually" is doing real work in that +sentence. -The order matters and is not negotiable: the backup is re-verified against -its manifest **before** the service is stopped, because a corrupt backup is -survivable while the failed instance is still up and unsurvivable once its -data directory has been moved aside. Nothing from the failed attempt is -deleted — the half-migrated data directory and the displaced binary are -moved to `.failed-` names. Afterwards a reduced validation suite runs -against the *restored* instance (version, reachability, directory counts) -rather than assuming the restore worked; pass `--admin-url` to get the last -two, which are skipped without it. +### Restoring from a snapshot loses mail delivered since -Every step is checkpointed, so a rollback interrupted partway — which is -exactly when a machine is most likely to be rebooted out from under it — -resumes where it stopped instead of restarting a destructive sequence from -the top. Re-running a completed rollback is inert. +Reverting to any pre-migration recovery point discards mail delivered +between taking it and restoring it. This is inherent to restoring a point in +time and this tool cannot solve it — plan your migration window with that in +mind, and consider holding inbound mail at a secondary MX for the duration +if the gap matters to you. -FoundationDB installs are refused rather than attempted: the backup phase -only *starts* an `fdbbackup` job, and restoring one needs `fdbrestore` -against a quiesced cluster. +### What the tool does to make a manual restore easier + +- **The old binary is preserved**, never deleted, next to the new one as + `.v` — so putting things back doesn't depend on + re-downloading a specific old release under pressure. +- **The original service definition is preserved** as `.pre-` + before cutover rewrites it, so you aren't reconstructing a unit file from + memory. +- **The settings and principals dumps** taken during backup stay on disk. +- **Every artifact path and checksum is in the checkpoint**, and + `stalwart-migrate status ` prints exactly which steps completed + and which failed — which is the first thing you want when deciding what to + restore. + +None of this is a substitute for the snapshot. It's what makes the twenty +minutes after restoring one less unpleasant. + +### Why it works this way + +An earlier version of this tool implemented rollback itself: it restored the +filesystem backup, verified every restored file against a manifest, replayed +SQL dumps, reinstalled the old binary, and re-validated the result. It was +tested and it looked good. It was removed, because restoring bytes correctly +is not the hard part — it copied contents and permissions but not +*ownership*, so run as root it would have produced a byte-perfect, +checksum-verified, root-owned data directory that Stalwart, running as its +own user, could not open, and it would have reported success. A filesystem +snapshot has no such failure mode, because it never lost the metadata to +begin with. ARCHITECTURE.md §4.8 records the full reasoning. diff --git a/cmd/stalwart-migrate/main.go b/cmd/stalwart-migrate/main.go index c83d719..cc8671a 100644 --- a/cmd/stalwart-migrate/main.go +++ b/cmd/stalwart-migrate/main.go @@ -1,7 +1,8 @@ // Command stalwart-migrate drives an in-place Stalwart Mail Server upgrade // (0.15.5 -> latest) through preflight checks, a defense-in-depth backup, -// a checkpointed migration, and post-migration validation, with rollback -// available at every step. See ARCHITECTURE.md for the full design. +// a checkpointed migration, and post-migration validation. Recovery from a +// failed migration is the operator's own snapshot or backup and is out of +// scope for this tool - see ARCHITECTURE.md §4.8. package main import ( @@ -23,10 +24,6 @@ func main() { err = runRun(os.Args[2:]) case "status": err = runStatus(os.Args[2:]) - case "rollback": - err = runRollback(os.Args[2:]) - case "confirm": - err = fmt.Errorf("not implemented yet: see internal/checkpoint") case "report": err = fmt.Errorf("not implemented yet: see internal/validate") default: @@ -47,7 +44,5 @@ commands: preflight run read-only checks and print the migration plan run --dry-run: simulate and validate against a sandbox (real cutover isn't implemented yet) status show the state of an in-progress or completed run - rollback restore the pre-migration backup for a given run (prints the plan; --yes to act) - confirm close the rollback window for a completed run report print the validation report for a run`) } diff --git a/cmd/stalwart-migrate/rollback.go b/cmd/stalwart-migrate/rollback.go deleted file mode 100644 index 038a06b..0000000 --- a/cmd/stalwart-migrate/rollback.go +++ /dev/null @@ -1,159 +0,0 @@ -package main - -import ( - "context" - "flag" - "fmt" - "net/http" - "os" - "strings" - "time" - - "github.com/LINUXexpert-org/stalwart-migrator/internal/backup" - "github.com/LINUXexpert-org/stalwart-migrator/internal/checkpoint" - "github.com/LINUXexpert-org/stalwart-migrator/internal/rollback" - "github.com/LINUXexpert-org/stalwart-migrator/internal/service" -) - -// splitRunID pulls the run-id out of args wherever it appears, so -// `rollback --data-dir X` works as naturally as -// `rollback --data-dir X `. Go's flag package stops parsing at the -// first positional argument, which would otherwise make the obvious -// invocation order silently drop every flag after the run-id - on a command -// whose flags decide what gets overwritten, that is not a failure mode -// worth living with. Tokens consumed as a flag's value are skipped by -// asking the FlagSet itself which flags take one, rather than by -// maintaining a second list of them here. -func splitRunID(fs *flag.FlagSet, args []string) (runID string, rest []string) { - rest = make([]string, 0, len(args)) - for i := 0; i < len(args); i++ { - arg := args[i] - if arg == "--" { - rest = append(rest, args[i:]...) - break - } - if strings.HasPrefix(arg, "-") { - rest = append(rest, arg) - name := strings.TrimLeft(arg, "-") - if !strings.Contains(arg, "=") && takesValue(fs, name) && i+1 < len(args) { - i++ - rest = append(rest, args[i]) - } - continue - } - if runID == "" { - runID = arg - continue - } - rest = append(rest, arg) // a second positional: let Parse report it - } - return runID, rest -} - -func takesValue(fs *flag.FlagSet, name string) bool { - f := fs.Lookup(name) - if f == nil { - return false - } - boolFlag, ok := f.Value.(interface{ IsBoolFlag() bool }) - return !ok || !boolFlag.IsBoolFlag() -} - -// runRollback implements `stalwart-migrate rollback `: put the -// instance back the way it was before the named run touched it -// (ARCHITECTURE.md §4.8). -// -// This is the only command that stops a running mail server and overwrites -// a live data directory, so it always prints the resolved plan first and -// refuses to act without --yes. Everything the plan needs comes from the -// run's own checkpoint - which backup, which manifest, which preserved -// binary - so an operator rolling back days after the fact doesn't have to -// remember any of it. The flags below exist to supply what the checkpoint -// deliberately doesn't store (database credentials) or to override a -// detection that was wrong. -func runRollback(args []string) error { - fs := flag.NewFlagSet("rollback", flag.ExitOnError) - stateDir := fs.String("state-dir", checkpoint.DefaultBaseDir, "directory runs are checkpointed in") - dataDir := fs.String("data-dir", "/var/lib/stalwart", "stalwart data directory to restore into (embedded backends)") - binaryPath := fs.String("binary", "/usr/local/bin/stalwart", "path the preserved old binary is reinstalled to") - serviceUnitPath := fs.String("service-unit", "", "path a preserved systemd unit or Compose file is restored to (only used if the run recorded one)") - - deployment := fs.String("deployment", "", `override how the service is controlled: "systemd" or "docker" (default: whatever preflight detected for this run)`) - unitName := fs.String("unit", "stalwart", "systemd unit name") - containerName := fs.String("container", "stalwart", "docker container name") - stopTimeout := fs.Duration("stop-timeout", 60*time.Second, "how long to wait for the service to actually stop") - startTimeout := fs.Duration("start-timeout", 60*time.Second, "how long to wait for the restored service to come back") - - adminURL := fs.String("admin-url", "", "base URL for the restored instance's admin/JMAP API, for post-rollback verification") - adminUser := fs.String("admin-user", "", "admin username") - adminPassword := fs.String("admin-password", os.Getenv("STALWART_MIGRATE_ADMIN_PASSWORD"), - "admin password (or set STALWART_MIGRATE_ADMIN_PASSWORD)") - verifyTimeout := fs.Duration("verify-timeout", 60*time.Second, "how long to wait for the restored instance to answer") - - dbHost := fs.String("db-host", "", "external database host (postgresql/mysql backends)") - dbPort := fs.String("db-port", "", "external database port") - dbName := fs.String("db-name", "", "external database name") - dbUser := fs.String("db-user", "", "external database user") - dbPassword := fs.String("db-password", os.Getenv("STALWART_MIGRATE_DB_PASSWORD"), - "external database password (or set STALWART_MIGRATE_DB_PASSWORD)") - sqlDump := fs.String("sql-dump", "", "override the dump file to replay (default: the one this run recorded)") - - yes := fs.Bool("yes", false, "actually perform the rollback; without it, the plan is printed and nothing is touched") - - runID, rest := splitRunID(fs, args) - if err := fs.Parse(rest); err != nil { - return err - } - if runID == "" || fs.NArg() != 0 { - return fmt.Errorf("usage: stalwart-migrate rollback [flags]") - } - - store := checkpoint.NewStore(*stateDir) - rs, err := store.Load(runID) - if err != nil { - return fmt.Errorf("load run %s: %w", runID, err) - } - - opts := rollback.Options{ - Deployment: service.Options{ - Kind: service.Kind(*deployment), UnitName: *unitName, ContainerName: *containerName, - }, - StopTimeout: *stopTimeout, StartTimeout: *startTimeout, - DataDir: *dataDir, - BinaryPath: *binaryPath, - ServiceUnitPath: *serviceUnitPath, - SQL: backup.SQLOptions{ - Host: *dbHost, Port: *dbPort, Database: *dbName, User: *dbUser, Password: *dbPassword, OutPath: *sqlDump, - }, - AdminURL: *adminURL, AdminUser: *adminUser, AdminPassword: *adminPassword, - HTTPClient: &http.Client{}, VerifyTimeout: *verifyTimeout, - } - - plan, err := rollback.BuildPlan(rs, opts) - if err != nil { - return err - } - fmt.Print(plan.String()) - - if !*yes { - fmt.Println("\nnothing has been touched. Re-run with --yes to perform this rollback.") - return nil - } - if *adminURL == "" { - fmt.Println("\nnote: without --admin-url, the reachability and directory-count checks after the restart are skipped - " + - "the rollback will report success on the restore mechanics alone.") - } - - fmt.Println("\n--- rollback ---") - report, err := rollback.Run(context.Background(), store, rs, opts) - fmt.Print(report.String()) - if err != nil { - return fmt.Errorf("rollback did not complete: %w", err) - } - - fmt.Printf("\nROLLBACK COMPLETE for run %s: the instance is back on %s. "+ - "The failed attempt's data and binary are preserved under .failed-%s names, and this run's backups are untouched, "+ - "so a retry after the underlying issue is fixed doesn't have to re-capture anything.\n", - rs.RunID, rs.SourceVersion, rs.RunID) - return nil -} diff --git a/cmd/stalwart-migrate/run.go b/cmd/stalwart-migrate/run.go index df96a51..8c673f5 100644 --- a/cmd/stalwart-migrate/run.go +++ b/cmd/stalwart-migrate/run.go @@ -17,10 +17,11 @@ import ( ) // runRun implements `stalwart-migrate run`. Only --dry-run is available -// today: the cutover phase itself (§4.5) isn't built. internal/rollback and -// internal/service now exist, so the missing piece is no longer "this can't -// be undone" but "there's nothing here to undo" - see ARCHITECTURE.md §8. -// `run` without --dry-run refuses rather than doing a migration partway. +// today. internal/cutover exists, so the gap is no longer a phase but the +// pipeline around it: §4.3 staging, and the wiring that would drive +// preflight -> backup -> stage -> recovery-mode -> cutover -> validate +// against real paths rather than a sandbox. See ARCHITECTURE.md §8. `run` +// without --dry-run refuses rather than doing a migration partway. // // --dry-run runs preflight and a real backup (see the caveat printed below // about why backup still touches the live data directory), then - if the @@ -68,10 +69,11 @@ func runRun(args []string) (err error) { if !*dryRun { return fmt.Errorf( - "real (non-dry-run) migrations aren't available yet: the cutover phase - installing the new binary, rewriting the " + - "service definition, and starting the migrated instance for real (ARCHITECTURE.md §4.5) - isn't implemented. " + - "Rollback is, so a future cutover will be undoable; there just isn't one to undo yet. Run with --dry-run to " + - "validate the migration mechanics against a disposable sandbox copy of your data - nothing in production is touched", + "real (non-dry-run) migrations aren't available yet: cutover is implemented (ARCHITECTURE.md §4.5), but nothing wires it " + + "into a production run - the staging phase (§4.3) and the real pipeline don't exist yet, so this command has no path " + + "that touches production. Run with --dry-run to validate the migration mechanics against a disposable sandbox copy " + + "of your data. Note that when a real run does land, recovery from a failed migration will be your own snapshot or " + + "backup - this tool does not undo a migration (§4.8)", ) } if *adminURL == "" { diff --git a/cmd/stalwart-migrate/status.go b/cmd/stalwart-migrate/status.go index 1124cec..f055edb 100644 --- a/cmd/stalwart-migrate/status.go +++ b/cmd/stalwart-migrate/status.go @@ -3,6 +3,7 @@ package main import ( "flag" "fmt" + "strings" "github.com/LINUXexpert-org/stalwart-migrator/internal/checkpoint" ) @@ -11,9 +12,9 @@ func runStatus(args []string) error { fs := flag.NewFlagSet("status", flag.ExitOnError) stateDir := fs.String("state-dir", checkpoint.DefaultBaseDir, "directory runs are checkpointed in") - // Same treatment as `rollback`: without this, `status - // --state-dir X` would look up the run in the default directory and - // report it missing, since flag parsing stops at the run-id. + // Go's flag package stops parsing at the first positional argument, so + // without this `status --state-dir X` would look the run up in + // the default directory and report it missing. runID, rest := splitRunID(fs, args) if err := fs.Parse(rest); err != nil { return err @@ -49,7 +50,6 @@ func runStatus(args []string) error { fmt.Printf("source: %s\n", rs.SourceVersion) fmt.Printf("target: %s\n", rs.TargetVersion) fmt.Printf("topology: deployment=%s store=%s\n", rs.Topology.DeploymentKind, rs.Topology.StoreBackend) - fmt.Printf("rollback window closed: %v\n", rs.RollbackWindowClosed) fmt.Println("steps:") for _, step := range rs.Steps { tag := string(step.Status) @@ -67,3 +67,45 @@ func runStatus(args []string) error { } return nil } + +// splitRunID pulls the run-id out of args wherever it appears, so +// `status --state-dir X` works as naturally as +// `status --state-dir X `. Go's flag package stops parsing at the +// first positional argument, which would otherwise make the obvious +// invocation order silently drop every flag after the run-id. Tokens +// consumed as a flag's value are skipped by asking the FlagSet itself which +// flags take one, rather than by maintaining a second list of them here. +func splitRunID(fs *flag.FlagSet, args []string) (runID string, rest []string) { + rest = make([]string, 0, len(args)) + for i := 0; i < len(args); i++ { + arg := args[i] + if arg == "--" { + rest = append(rest, args[i:]...) + break + } + if strings.HasPrefix(arg, "-") { + rest = append(rest, arg) + name := strings.TrimLeft(arg, "-") + if !strings.Contains(arg, "=") && takesValue(fs, name) && i+1 < len(args) { + i++ + rest = append(rest, args[i]) + } + continue + } + if runID == "" { + runID = arg + continue + } + rest = append(rest, arg) // a second positional: let Parse report it + } + return runID, rest +} + +func takesValue(fs *flag.FlagSet, name string) bool { + f := fs.Lookup(name) + if f == nil { + return false + } + boolFlag, ok := f.Value.(interface{ IsBoolFlag() bool }) + return !ok || !boolFlag.IsBoolFlag() +} diff --git a/internal/backup/binary.go b/internal/backup/binary.go index 8129012..a0570c2 100644 --- a/internal/backup/binary.go +++ b/internal/backup/binary.go @@ -6,12 +6,13 @@ import ( ) // PreserveBinary moves the currently-installed binary aside to -// ".v" so rollback can restart the exact old -// binary without re-downloading anything, and cutover can install the new -// one at the original path. It never deletes the old binary, and it's -// idempotent: if a prior attempt at this run already preserved it, calling -// this again just returns the existing preserved path rather than erroring -// on a missing source file. +// ".v" so cutover can install the new one at the +// original path while the exact old binary stays on disk - an operator +// putting the machine back by hand needs it, and re-downloading a specific +// old release is not something to be doing under pressure. It never deletes +// the old binary, and it's idempotent: if a prior attempt at this run +// already preserved it, calling this again just returns the existing +// preserved path rather than erroring on a missing source file. func PreserveBinary(binaryPath, sourceVersion string) (preservedPath string, err error) { if sourceVersion == "" { return "", fmt.Errorf("backup: cannot preserve %s without a source version to suffix it with", binaryPath) diff --git a/internal/checkpoint/doc.go b/internal/checkpoint/doc.go index 239c28e..25da663 100644 --- a/internal/checkpoint/doc.go +++ b/internal/checkpoint/doc.go @@ -1,3 +1,3 @@ -// Package checkpoint implements run-id and state.json persistence, resume logic, and rollback-window tracking. +// Package checkpoint implements run-id and state.json persistence and resume logic. // See ARCHITECTURE.md §5 for the design. package checkpoint diff --git a/internal/checkpoint/types.go b/internal/checkpoint/types.go index 973bc49..d51f5ef 100644 --- a/internal/checkpoint/types.go +++ b/internal/checkpoint/types.go @@ -14,7 +14,6 @@ const ( PhaseRecovery Phase = "recovery" PhaseCutover Phase = "cutover" PhaseValidate Phase = "validate" - PhaseRollback Phase = "rollback" ) // StepStatus is the lifecycle state of one checkpointed step. @@ -82,9 +81,8 @@ type PreflightSnapshot struct { } // Topology records how this Stalwart instance is deployed, as detected -// during preflight, so later phases (cutover, rollback) know whether -// they're managing a systemd unit or a container and what backend they're -// dealing with. +// during preflight, so the cutover phase knows whether it's managing a +// systemd unit or a container and what backend it's dealing with. type Topology struct { DeploymentKind string `json:"deployment_kind,omitempty"` // "systemd", "docker", "unknown" ClusterNodes []string `json:"cluster_nodes,omitempty"` @@ -94,19 +92,18 @@ type Topology struct { } // RunState is the full persisted state of one migration run: everything -// needed to resume it after a crash, decide whether to roll back, or -// report on it later. See ARCHITECTURE.md §5. +// needed to resume it after a crash, or report on it later. See +// ARCHITECTURE.md §5. type RunState struct { - RunID string `json:"run_id"` - SourceVersion string `json:"source_version"` - TargetVersion string `json:"target_version"` - CreatedAt time.Time `json:"created_at"` - UpdatedAt time.Time `json:"updated_at"` - Topology Topology `json:"topology,omitempty"` - Steps []StepRecord `json:"steps"` - Artifacts map[string]Artifact `json:"artifacts,omitempty"` - PreflightSnapshot *PreflightSnapshot `json:"preflight_snapshot,omitempty"` - RollbackWindowClosed bool `json:"rollback_window_closed"` + RunID string `json:"run_id"` + SourceVersion string `json:"source_version"` + TargetVersion string `json:"target_version"` + CreatedAt time.Time `json:"created_at"` + UpdatedAt time.Time `json:"updated_at"` + Topology Topology `json:"topology,omitempty"` + Steps []StepRecord `json:"steps"` + Artifacts map[string]Artifact `json:"artifacts,omitempty"` + PreflightSnapshot *PreflightSnapshot `json:"preflight_snapshot,omitempty"` } // RecordArtifact stores a content-addressed record of a file this run diff --git a/internal/cutover/cutover.go b/internal/cutover/cutover.go new file mode 100644 index 0000000..d158538 --- /dev/null +++ b/internal/cutover/cutover.go @@ -0,0 +1,497 @@ +package cutover + +import ( + "context" + "crypto/sha256" + "encoding/hex" + "fmt" + "io" + "net/http" + "os" + "path/filepath" + "strings" + "time" + + "github.com/LINUXexpert-org/stalwart-migrator/internal/checkpoint" + "github.com/LINUXexpert-org/stalwart-migrator/internal/preflight" + "github.com/LINUXexpert-org/stalwart-migrator/internal/service" + "github.com/LINUXexpert-org/stalwart-migrator/internal/stalwartapi" +) + +// Artifact names this phase records. ArtifactServiceUnit is the preserved +// copy of the service definition as it was before this phase rewrote it - +// recovery is out of scope for this tool (see ARCHITECTURE.md §4.8), but +// the operator putting a machine back by hand should not also have to +// reconstruct their unit file from memory. +const ( + ArtifactNewBinary = "new-binary" + ArtifactServiceUnit = "service-unit" +) + +// Options configures the cutover phase - ARCHITECTURE.md §4.5. +type Options struct { + // StagedBinaryPath is the already-downloaded target-version binary. + // Cutover verifies its version before installing it and never + // downloads anything itself. + StagedBinaryPath string + // BinaryPath is where the staged binary is installed - the path the + // service definition runs. backup's preserve-binary step has already + // moved the old binary aside, so this path is normally empty by now. + BinaryPath string + + // ServiceUnitPath is the systemd unit to rewrite. ConfigPath, if set, + // becomes the unit's --config argument. + ServiceUnitPath string + ConfigPath string + + // RecoveryPointConfirmed is the operator asserting that a recovery + // point exists for this machine. This tool does not take one, verify + // one, or restore from one - see ARCHITECTURE.md §4.8 - so this is an + // acknowledgement, not a check, and BuildPlan refuses without it. An + // unverifiable assertion is weaker than a guarantee; making it explicit + // at least means nobody migrates a production mail server having never + // been asked the question. + RecoveryPointConfirmed bool + + Deployment service.Options + Controller service.Controller + + StartTimeout time.Duration // waiting for the service to report running; default 60s + HealthTimeout time.Duration // waiting for it to answer JMAP; default 120s + + AdminURL string + AdminUser string + AdminPassword string + HTTPClient *http.Client + + // RecalculateQuotas schedules the post-migration quota rebuild + // (ARCHITECTURE.md §4.5's last step). It's needed when crossing the + // 0.15/0.16 boundary, where Stalwart's own upgrade guide says quotas + // were reset to zero and have to be rebuilt; a patch bump doesn't + // touch them. TenantIDs additionally rebuilds tenant-level counters, + // which only multi-tenant installs have. + RecalculateQuotas bool + TenantIDs []string + QuotaTimeout time.Duration // default 30m; large installs legitimately take a while +} + +// Plan is what a cutover would do, resolved before anything is touched. +type Plan struct { + RunID string + TargetVersion string + Target string // what the service controller acts on + + StagedBinaryPath string + BinaryPath string + ServiceUnitPath string + ConfigPath string + RecalculateQuotas bool +} + +func (p Plan) String() string { + var b strings.Builder + fmt.Fprintf(&b, "cutover plan for run %s:\n", p.RunID) + fmt.Fprintf(&b, " 1. confirm %s really is %s, then install it as %s\n", p.StagedBinaryPath, p.TargetVersion, p.BinaryPath) + fmt.Fprintf(&b, " 2. preserve %s, then point its ExecStart at the new binary and strip any recovery-mode env vars\n", p.ServiceUnitPath) + fmt.Fprintf(&b, " 3. reload the service definition and start %s\n", p.Target) + fmt.Fprint(&b, " 4. wait for it to answer an authenticated JMAP session request\n") + if p.RecalculateQuotas { + fmt.Fprint(&b, " 5. schedule per-account quota recalculation and wait for the task queue to drain\n") + } else { + fmt.Fprint(&b, " 5. skip quota recalculation - not needed for this upgrade path\n") + } + return b.String() +} + +// BuildPlan resolves the cutover and refuses up front for anything it +// shouldn't attempt. The most important refusal is the first one: a cutover +// is only allowed to proceed if this run could still be rolled back, which +// is what makes "never commit to a change you can't undo" a property of the +// code rather than a claim in a design document. +func BuildPlan(rs *checkpoint.RunState, opts Options) (Plan, error) { + p := Plan{ + RunID: rs.RunID, TargetVersion: rs.TargetVersion, + StagedBinaryPath: opts.StagedBinaryPath, BinaryPath: opts.BinaryPath, + ServiceUnitPath: opts.ServiceUnitPath, ConfigPath: opts.ConfigPath, + RecalculateQuotas: opts.RecalculateQuotas, + } + + if !opts.RecoveryPointConfirmed { + return p, fmt.Errorf( + "cutover: no recovery point has been confirmed for this run. This phase migrates a live mail server in place and this " + + "tool has no way to undo that - recovery is the operator's own snapshot or backup (ARCHITECTURE.md §4.8). Take one, " + + "verify you can actually restore from it, and re-run confirming that you have") + } + + kind := opts.Deployment.Kind + if kind == "" { + kind = service.Kind(rs.Topology.DeploymentKind) + } + if kind == service.Docker { + return p, fmt.Errorf( + "cutover: this run's deployment is a Docker container, where cutting over means pulling a new image and recreating the " + + "container rather than swapping a binary and rewriting a unit. This tool doesn't automate that - do it by hand") + } + deployment := opts.Deployment + deployment.Kind = kind + controller := opts.Controller + if controller == nil { + var err error + controller, err = service.New(deployment) + if err != nil { + return p, err + } + } + p.Target = controller.Target() + + if opts.StagedBinaryPath == "" { + return p, fmt.Errorf("cutover: no staged target binary was given - cutover installs an already-downloaded binary, it doesn't fetch one") + } + if _, err := os.Stat(opts.StagedBinaryPath); err != nil { + return p, fmt.Errorf("cutover: staged binary %s: %w", opts.StagedBinaryPath, err) + } + if opts.BinaryPath == "" { + return p, fmt.Errorf("cutover: no path to install the new binary to was given") + } + if opts.ServiceUnitPath == "" { + return p, fmt.Errorf("cutover: no service definition path was given - without one this phase can't point the service at the new binary") + } + if _, err := os.Stat(opts.ServiceUnitPath); err != nil { + return p, fmt.Errorf("cutover: service definition %s: %w", opts.ServiceUnitPath, err) + } + return p, nil +} + +// Run executes ARCHITECTURE.md §4.5. Every step is checkpointed, and the +// order is chosen so that the irreversible-looking parts happen only after +// the reversible checks pass: the staged binary's version is confirmed +// before it's installed, and the service definition is preserved before +// it's rewritten. +// +// Quota recalculation is the one step allowed to fail without failing the +// cutover, and that's deliberate. Stale quota counters are an accounting +// problem, while a failed cutover is one an operator has to respond to by +// restoring a machine that is otherwise migrated and serving mail +// correctly. Calling for that over a counter would be the worse outcome, so +// this step warns loudly and tells the operator how to finish it by hand. +func Run(ctx context.Context, store *checkpoint.Store, rs *checkpoint.RunState, opts Options) (Report, error) { + var report Report + + plan, err := BuildPlan(rs, opts) + if err != nil { + report.Results = append(report.Results, CheckResult{Name: "plan", Status: StatusFail, Detail: err.Error()}) + return report, err + } + + controller := opts.Controller + if controller == nil { + deployment := opts.Deployment + if deployment.Kind == "" { + deployment.Kind = service.Kind(rs.Topology.DeploymentKind) + } + controller, err = service.New(deployment) + if err != nil { + report.Results = append(report.Results, CheckResult{Name: "plan", Status: StatusFail, Detail: err.Error()}) + return report, err + } + } + + step := func(name string, fn func() (checkpoint.StepOutcome, error)) error { + outcome, err := store.RunStep(rs, checkpoint.PhaseCutover, name, fn) + if err != nil { + report.Results = append(report.Results, CheckResult{Name: name, Status: StatusFail, Detail: err.Error()}) + return err + } + status := Status(outcome.Verdict) + if status == "" { + status = StatusOK + } + report.Results = append(report.Results, CheckResult{Name: name, Status: status, Detail: outcome.Detail}) + return nil + } + + if err := step("verify-staged-binary", func() (checkpoint.StepOutcome, error) { + got, err := preflight.DetectVersion(ctx, plan.StagedBinaryPath) + if err != nil { + return checkpoint.StepOutcome{}, fmt.Errorf("couldn't read the staged binary's version: %w", err) + } + if rs.TargetVersion != "" && got != rs.TargetVersion { + return checkpoint.StepOutcome{}, fmt.Errorf( + "staged binary %s reports version %s, but this run targets %s - installing it would migrate to a version nobody planned for", + plan.StagedBinaryPath, got, rs.TargetVersion) + } + return checkpoint.StepOutcome{Detail: fmt.Sprintf("staged binary %s reports %s, matching this run's target", plan.StagedBinaryPath, got), Extra: got}, nil + }); err != nil { + return report, err + } + + if err := step("install-binary", func() (checkpoint.StepOutcome, error) { + sum, size, err := installBinary(plan.StagedBinaryPath, plan.BinaryPath) + if err != nil { + return checkpoint.StepOutcome{}, err + } + rs.RecordArtifact(ArtifactNewBinary, checkpoint.Artifact{Path: plan.BinaryPath, SHA256: sum, SizeBytes: size}) + return checkpoint.StepOutcome{Detail: fmt.Sprintf("installed %s as %s (%d bytes)", plan.StagedBinaryPath, plan.BinaryPath, size)}, nil + }); err != nil { + return report, err + } + + if err := step("update-service-definition", func() (checkpoint.StepOutcome, error) { + preserved, err := preserveUnit(plan.ServiceUnitPath, rs.RunID) + if err != nil { + return checkpoint.StepOutcome{}, err + } + sum, size, err := hashFile(preserved) + if err != nil { + return checkpoint.StepOutcome{}, err + } + // Recorded before the rewrite, so a crash between preserving and + // rewriting still leaves the original findable. + rs.RecordArtifact(ArtifactServiceUnit, checkpoint.Artifact{Path: preserved, SHA256: sum, SizeBytes: size}) + + original, err := os.ReadFile(preserved) + if err != nil { + return checkpoint.StepOutcome{}, fmt.Errorf("cutover: read preserved unit %s: %w", preserved, err) + } + rewritten, err := RewriteUnit(string(original), plan.BinaryPath, plan.ConfigPath) + if err != nil { + return checkpoint.StepOutcome{}, err + } + if err := writeFileAtomic(plan.ServiceUnitPath, []byte(rewritten), 0o644); err != nil { + return checkpoint.StepOutcome{}, err + } + return checkpoint.StepOutcome{ + Detail: fmt.Sprintf("pointed %s at %s; the original is preserved at %s", plan.ServiceUnitPath, plan.BinaryPath, preserved), + Extra: preserved, + }, nil + }); err != nil { + return report, err + } + + if err := step("reload-service-definition", func() (checkpoint.StepOutcome, error) { + if err := controller.ReloadConfig(ctx); err != nil { + return checkpoint.StepOutcome{}, err + } + return checkpoint.StepOutcome{Detail: "service manager re-read the updated definition"}, nil + }); err != nil { + return report, err + } + + startTimeout := opts.StartTimeout + if startTimeout <= 0 { + startTimeout = 60 * time.Second + } + if err := step("start-service", func() (checkpoint.StepOutcome, error) { + if err := controller.Start(ctx); err != nil { + return checkpoint.StepOutcome{}, err + } + if err := service.WaitFor(ctx, controller, true, startTimeout); err != nil { + return checkpoint.StepOutcome{}, err + } + return checkpoint.StepOutcome{Detail: fmt.Sprintf("%s is running the migrated instance", controller.Target())}, nil + }); err != nil { + return report, err + } + + healthTimeout := opts.HealthTimeout + if healthTimeout <= 0 { + healthTimeout = 120 * time.Second + } + if err := step("wait-healthy", func() (checkpoint.StepOutcome, error) { + if opts.AdminURL == "" { + return checkpoint.StepOutcome{ + Verdict: string(StatusSkipped), + Detail: "no admin URL configured - the service was started, but nothing confirmed it actually answers", + }, nil + } + client := newClient(opts) + if err := client.WaitForPing(ctx, healthTimeout); err != nil { + return checkpoint.StepOutcome{}, fmt.Errorf("the migrated service started but never answered at %s within %s: %w", opts.AdminURL, healthTimeout, err) + } + return checkpoint.StepOutcome{Detail: fmt.Sprintf("migrated instance answered an authenticated JMAP session request at %s", opts.AdminURL)}, nil + }); err != nil { + return report, err + } + + // From here on, failures are warnings: the service is up and serving + // mail, and rolling that back over a quota counter would be worse than + // leaving the counter stale. + quotaOutcome, quotaErr := store.RunStep(rs, checkpoint.PhaseCutover, "recalculate-quotas", func() (checkpoint.StepOutcome, error) { + return recalculateQuotas(ctx, opts) + }) + switch { + case quotaErr != nil: + report.Results = append(report.Results, CheckResult{ + Name: "recalculate-quotas", Status: StatusWarn, + Detail: fmt.Sprintf("%v - the migration is complete and serving mail; finish this from the WebUI's Tasks panel "+ + "(\"Recalculate disk quotas\"), and note that until it runs, per-account usage counters read low", quotaErr), + }) + default: + status := Status(quotaOutcome.Verdict) + if status == "" { + status = StatusOK + } + report.Results = append(report.Results, CheckResult{Name: "recalculate-quotas", Status: status, Detail: quotaOutcome.Detail}) + } + + return report, nil +} + +func newClient(opts Options) *stalwartapi.Client { + return &stalwartapi.Client{ + BaseURL: opts.AdminURL, Username: opts.AdminUser, Password: opts.AdminPassword, HTTPClient: opts.HTTPClient, + } +} + +func recalculateQuotas(ctx context.Context, opts Options) (checkpoint.StepOutcome, error) { + if !opts.RecalculateQuotas { + return checkpoint.StepOutcome{ + Verdict: string(StatusSkipped), + Detail: "not needed for this upgrade path - quotas are only reset by the 0.15/0.16 schema migration", + }, nil + } + if opts.AdminURL == "" { + return checkpoint.StepOutcome{ + Verdict: string(StatusSkipped), + Detail: "no admin URL configured - quota recalculation has to be triggered from the WebUI's Tasks panel by hand", + }, nil + } + + timeout := opts.QuotaTimeout + if timeout <= 0 { + timeout = 30 * time.Minute + } + client := newClient(opts) + + accountIDs, err := client.AccountIDs(ctx) + if err != nil { + return checkpoint.StepOutcome{}, fmt.Errorf("couldn't enumerate accounts to recalculate: %w", err) + } + if len(accountIDs) == 0 { + return checkpoint.StepOutcome{Verdict: string(StatusSkipped), Detail: "the instance reports no accounts, so there are no quotas to rebuild"}, nil + } + + taskIDs, err := client.CreateQuotaRecalculationTasks(ctx, accountIDs) + if err != nil { + return checkpoint.StepOutcome{}, err + } + failures, err := client.WaitForTasks(ctx, taskIDs, timeout) + if err != nil { + return checkpoint.StepOutcome{}, err + } + if len(failures) > 0 { + return checkpoint.StepOutcome{}, fmt.Errorf("%d of %d account quota task(s) failed: %v", len(failures), len(taskIDs), failures) + } + detail := fmt.Sprintf("rebuilt disk quotas for %d account(s)", len(accountIDs)) + + // Tenant totals aggregate the per-account numbers, so they can only run + // once every account task above has finished - which is why this is + // sequenced after the wait rather than scheduled alongside it. + if len(opts.TenantIDs) > 0 { + tenantTasks, err := client.CreateTenantQuotaRecalculationTasks(ctx, opts.TenantIDs) + if err != nil { + return checkpoint.StepOutcome{}, fmt.Errorf("%s, but scheduling tenant recalculation failed: %w", detail, err) + } + tenantFailures, err := client.WaitForTasks(ctx, tenantTasks, timeout) + if err != nil { + return checkpoint.StepOutcome{}, fmt.Errorf("%s, but waiting on tenant recalculation failed: %w", detail, err) + } + if len(tenantFailures) > 0 { + return checkpoint.StepOutcome{}, fmt.Errorf("%s, but %d tenant quota task(s) failed: %v", detail, len(tenantFailures), tenantFailures) + } + detail += fmt.Sprintf(" and %d tenant(s)", len(opts.TenantIDs)) + } + return checkpoint.StepOutcome{Detail: detail}, nil +} + +// installBinary copies the staged binary into place through a temp file in +// the same directory, so the service definition never points at a +// half-written executable. It's idempotent: a retry that finds the right +// bytes already installed reports them rather than copying again. +func installBinary(stagedPath, binaryPath string) (sha256Hex string, size int64, err error) { + stagedSum, stagedSize, err := hashFile(stagedPath) + if err != nil { + return "", 0, err + } + if existingSum, existingSize, err := hashFile(binaryPath); err == nil && existingSum == stagedSum { + return existingSum, existingSize, nil + } + + data, err := os.ReadFile(stagedPath) + if err != nil { + return "", 0, fmt.Errorf("cutover: read staged binary %s: %w", stagedPath, err) + } + if err := writeFileAtomic(binaryPath, data, 0o755); err != nil { + return "", 0, err + } + return stagedSum, stagedSize, nil +} + +// preserveUnit copies the current service definition to +// ".pre-" so an operator restoring by hand has the original, +// and returns that path. +// Idempotent: a retry finds the copy already there and keeps it, since the +// file at path may by then be this phase's own rewrite. +func preserveUnit(unitPath, runID string) (preservedPath string, err error) { + preservedPath = fmt.Sprintf("%s.pre-%s", unitPath, runID) + if _, err := os.Stat(preservedPath); err == nil { + return preservedPath, nil + } else if !os.IsNotExist(err) { + return "", fmt.Errorf("cutover: stat %s: %w", preservedPath, err) + } + data, err := os.ReadFile(unitPath) + if err != nil { + return "", fmt.Errorf("cutover: read service definition %s: %w", unitPath, err) + } + perm := os.FileMode(0o644) + if info, err := os.Stat(unitPath); err == nil { + perm = info.Mode().Perm() + } + if err := writeFileAtomic(preservedPath, data, perm); err != nil { + return "", err + } + return preservedPath, nil +} + +func writeFileAtomic(path string, data []byte, perm os.FileMode) error { + tmp, err := os.CreateTemp(filepath.Dir(path), filepath.Base(path)+".tmp-*") + if err != nil { + return fmt.Errorf("cutover: create temp file next to %s: %w", path, err) + } + tmpPath := tmp.Name() + defer os.Remove(tmpPath) // no-op once the rename succeeds + + if _, err := tmp.Write(data); err != nil { + tmp.Close() + return fmt.Errorf("cutover: write %s: %w", tmpPath, err) + } + if err := tmp.Chmod(perm); err != nil { + tmp.Close() + return fmt.Errorf("cutover: chmod %s: %w", tmpPath, err) + } + if err := tmp.Sync(); err != nil { + tmp.Close() + return fmt.Errorf("cutover: sync %s: %w", tmpPath, err) + } + if err := tmp.Close(); err != nil { + return fmt.Errorf("cutover: close %s: %w", tmpPath, err) + } + if err := os.Rename(tmpPath, path); err != nil { + return fmt.Errorf("cutover: move %s into place at %s: %w", tmpPath, path, err) + } + return nil +} + +func hashFile(path string) (sha256Hex string, size int64, err error) { + f, err := os.Open(path) + if err != nil { + return "", 0, fmt.Errorf("cutover: hash %s: %w", path, err) + } + defer f.Close() + h := sha256.New() + n, err := io.Copy(h, f) + if err != nil { + return "", 0, fmt.Errorf("cutover: hash %s: %w", path, err) + } + return hex.EncodeToString(h.Sum(nil)), n, nil +} diff --git a/internal/cutover/cutover_test.go b/internal/cutover/cutover_test.go new file mode 100644 index 0000000..3694bc7 --- /dev/null +++ b/internal/cutover/cutover_test.go @@ -0,0 +1,387 @@ +package cutover + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/LINUXexpert-org/stalwart-migrator/internal/checkpoint" + "github.com/LINUXexpert-org/stalwart-migrator/internal/service" +) + +// fakeController stands in for systemd, recording call order. +type fakeController struct { + calls []string + active bool + startErr error +} + +func (f *fakeController) Stop(context.Context) error { + f.calls = append(f.calls, "stop") + f.active = false + return nil +} + +func (f *fakeController) Start(context.Context) error { + f.calls = append(f.calls, "start") + if f.startErr != nil { + return f.startErr + } + f.active = true + return nil +} + +func (f *fakeController) Active(context.Context) (bool, error) { return f.active, nil } + +func (f *fakeController) ReloadConfig(context.Context) error { + f.calls = append(f.calls, "reload") + return nil +} + +func (f *fakeController) Target() string { return "test service" } + +func readFile(t *testing.T, path string) string { + t.Helper() + data, err := os.ReadFile(path) + if os.IsNotExist(err) { + return "" + } + if err != nil { + t.Fatal(err) + } + return string(data) +} + +// migratedRun builds a checkpoint for a run that backed up successfully and +// is ready to cut over: the state this phase is actually invoked against. +func migratedRun(t *testing.T) (store *checkpoint.Store, rs *checkpoint.RunState, opts Options) { + t.Helper() + root := t.TempDir() + + store = checkpoint.NewStore(filepath.Join(root, "runs")) + rs, err := store.Create("0.15.5", "0.16.14") + if err != nil { + t.Fatal(err) + } + rs.Topology = checkpoint.Topology{DeploymentKind: "systemd", StoreBackend: "rocksdb"} + + staged := filepath.Join(root, "staged-stalwart") + if err := os.WriteFile(staged, []byte("#!/bin/sh\necho 'stalwart 0.16.14'\n"), 0o755); err != nil { + t.Fatal(err) + } + unitPath := filepath.Join(root, "stalwart.service") + if err := os.WriteFile(unitPath, []byte(realisticUnit), 0o644); err != nil { + t.Fatal(err) + } + + return store, rs, Options{ + StagedBinaryPath: staged, + BinaryPath: filepath.Join(root, "bin-stalwart"), + ServiceUnitPath: unitPath, + RecoveryPointConfirmed: true, + Controller: &fakeController{}, + } +} + +// This tool can't undo a cutover, so the least it can do is refuse to +// perform one without the operator having been asked the question. +func TestBuildPlanRefusesWithoutAConfirmedRecoveryPoint(t *testing.T) { + _, rs, opts := migratedRun(t) + opts.RecoveryPointConfirmed = false + + _, err := BuildPlan(rs, opts) + if err == nil { + t.Fatal("BuildPlan: want refusal when no recovery point has been confirmed, got nil") + } + if !strings.Contains(err.Error(), "no way to undo") { + t.Errorf("error %q should be plain that this is irreversible for the tool", err) + } +} + +func TestBuildPlanRefusesDockerDeployments(t *testing.T) { + _, rs, opts := migratedRun(t) + rs.Topology.DeploymentKind = string(service.Docker) + opts.Controller = nil + + _, err := BuildPlan(rs, opts) + if err == nil { + t.Fatal("BuildPlan: want refusal for a container deployment, got nil") + } + if !strings.Contains(err.Error(), "recreating the container") { + t.Errorf("error %q should explain what cutting over a container would actually involve", err) + } +} + +func TestBuildPlanRefusesAMissingStagedBinaryOrUnit(t *testing.T) { + for _, tc := range []struct{ name, field string }{{"staged binary", "staged"}, {"service unit", "unit"}} { + t.Run(tc.name, func(t *testing.T) { + _, rs, opts := migratedRun(t) + if tc.field == "staged" { + opts.StagedBinaryPath = filepath.Join(t.TempDir(), "absent") + } else { + opts.ServiceUnitPath = filepath.Join(t.TempDir(), "absent.service") + } + if _, err := BuildPlan(rs, opts); err == nil { + t.Fatalf("BuildPlan: want refusal for a missing %s, got nil", tc.name) + } + }) + } +} + +func TestRunInstallsRewritesAndStarts(t *testing.T) { + store, rs, opts := migratedRun(t) + ctl := opts.Controller.(*fakeController) + + report, err := Run(context.Background(), store, rs, opts) + if err != nil { + t.Fatalf("Run: %v\n%s", err, report) + } + + if got := readFile(t, opts.BinaryPath); !strings.Contains(got, "0.16.14") { + t.Errorf("installed binary = %q, want the staged one", got) + } + if info, err := os.Stat(opts.BinaryPath); err != nil || info.Mode().Perm() != 0o755 { + t.Errorf("installed binary mode = %v (err %v), want 0755 - a non-executable binary won't start", info.Mode().Perm(), err) + } + unit := readFile(t, opts.ServiceUnitPath) + if !strings.Contains(unit, "ExecStart="+opts.BinaryPath) { + t.Errorf("unit not repointed at the new binary:\n%s", unit) + } + if got, want := strings.Join(ctl.calls, ","), "reload,start"; got != want { + t.Errorf("controller calls = %q, want %q - the definition must be reloaded before the start", got, want) + } + if report.Blocking() { + t.Errorf("report should be clean:\n%s", report) + } +} + +// Recovery is the operator's own snapshot, but a snapshot revert doesn't +// help someone who only wants their unit file back - so this phase has to +// leave the original where they can find it. +func TestRunPreservesTheOriginalServiceDefinition(t *testing.T) { + store, rs, opts := migratedRun(t) + if _, err := Run(context.Background(), store, rs, opts); err != nil { + t.Fatal(err) + } + + art, found := rs.Artifacts[ArtifactServiceUnit] + if !found { + t.Fatalf("no %q artifact recorded; an operator restoring by hand would have to reconstruct the unit from memory", ArtifactServiceUnit) + } + preserved := readFile(t, art.Path) + if !strings.Contains(preserved, "ExecStart=/usr/local/bin/stalwart --config /etc/stalwart/config.toml") { + t.Errorf("preserved unit = %q, want the definition as it was before the rewrite", preserved) + } + if art.SHA256 == "" { + t.Error("preserved unit artifact has no checksum") + } +} + +// Installing a binary that isn't the version the run planned for would +// migrate to a version nobody chose. +func TestRunRefusesAStagedBinaryOfTheWrongVersion(t *testing.T) { + store, rs, opts := migratedRun(t) + if err := os.WriteFile(opts.StagedBinaryPath, []byte("#!/bin/sh\necho 'stalwart 0.16.9'\n"), 0o755); err != nil { + t.Fatal(err) + } + + report, err := Run(context.Background(), store, rs, opts) + if err == nil { + t.Fatal("Run: want failure for a staged binary of the wrong version, got nil") + } + if _, statErr := os.Stat(opts.BinaryPath); !os.IsNotExist(statErr) { + t.Error("the wrong-version binary was installed anyway") + } + if got := readFile(t, opts.ServiceUnitPath); !strings.Contains(got, "/usr/local/bin/stalwart") { + t.Error("the service definition was rewritten despite the refusal") + } + if !report.Blocking() { + t.Error("report should be blocking") + } +} + +func TestRunResumesWithoutRedoingCompletedSteps(t *testing.T) { + store, rs, opts := migratedRun(t) + if _, err := Run(context.Background(), store, rs, opts); err != nil { + t.Fatal(err) + } + + second := &fakeController{active: true} + opts.Controller = second + reloaded, err := store.Load(rs.RunID) + if err != nil { + t.Fatal(err) + } + if _, err := Run(context.Background(), store, reloaded, opts); err != nil { + t.Fatal(err) + } + if len(second.calls) != 0 { + t.Errorf("second invocation called the controller %v, want nothing - every step was already done", second.calls) + } +} + +func TestRunFailsWhenTheMigratedServiceNeverAnswers(t *testing.T) { + store, rs, opts := migratedRun(t) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusBadGateway) + })) + defer srv.Close() + opts.AdminURL = srv.URL + opts.HealthTimeout = 300 * time.Millisecond + + report, err := Run(context.Background(), store, rs, opts) + if err == nil { + t.Fatal("Run: want failure when the started service never answers, got nil") + } + if !strings.Contains(err.Error(), "never answered") { + t.Errorf("error %q should distinguish 'started but not answering' from 'failed to start'", err) + } + if !report.Blocking() { + t.Error("report should be blocking") + } +} + +// Rolling back a migration that completed successfully, because a counter +// didn't get rebuilt, would be worse than a stale counter. +func TestRunWarnsRatherThanFailsWhenQuotaRecalculationFails(t *testing.T) { + store, rs, opts := migratedRun(t) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodGet { + json.NewEncoder(w).Encode(map[string]any{"apiUrl": "/api"}) + return + } + w.WriteHeader(http.StatusInternalServerError) // the management API is unhappy + })) + defer srv.Close() + opts.AdminURL = srv.URL + opts.RecalculateQuotas = true + + report, err := Run(context.Background(), store, rs, opts) + if err != nil { + t.Fatalf("a failed quota rebuild must not fail the cutover: %v\n%s", err, report) + } + if report.Blocking() { + t.Errorf("report should not be blocking:\n%s", report) + } + var warned bool + for _, res := range report.Results { + if res.Name == "recalculate-quotas" { + warned = res.Status == StatusWarn + if !strings.Contains(res.Detail, "Tasks panel") { + t.Errorf("warning %q should tell the operator how to finish it by hand", res.Detail) + } + } + } + if !warned { + t.Errorf("quota failure should be a warning, not silence:\n%s", report) + } +} + +func TestRunSkipsQuotaRecalculationOnThePatchFastPath(t *testing.T) { + store, rs, opts := migratedRun(t) + opts.RecalculateQuotas = false + + report, err := Run(context.Background(), store, rs, opts) + if err != nil { + t.Fatal(err) + } + for _, res := range report.Results { + if res.Name == "recalculate-quotas" { + if res.Status != StatusSkipped { + t.Errorf("recalculate-quotas = %s, want skip", res.Status) + } + if !strings.Contains(res.Detail, "0.15/0.16") { + t.Errorf("skip detail %q should say why it isn't needed", res.Detail) + } + } + } +} + +// quotaServer answers the three calls quota recalculation makes: enumerate +// accounts, schedule one task per account, then poll until the queue +// drains. Tasks are removed once fetched, modelling a queue whose entries +// are consumed when they run. +func quotaServer(t *testing.T, accountIDs []string) (*httptest.Server, *[]map[string]any) { + t.Helper() + var scheduled []map[string]any + queued := map[string]bool{} + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodGet { + json.NewEncoder(w).Encode(map[string]any{"apiUrl": "/api"}) + return + } + var body map[string]any + json.NewDecoder(r.Body).Decode(&body) + call := body["methodCalls"].([]any)[0].([]any) + name := call[0].(string) + args := call[1].(map[string]any) + + switch name { + case "x:Account/query": + json.NewEncoder(w).Encode(map[string]any{"methodResponses": []any{ + []any{"x:Account/query", map[string]any{"ids": accountIDs}, "q"}, + }}) + case "x:Task/set": + created := map[string]any{} + for creationID, obj := range args["create"].(map[string]any) { + scheduled = append(scheduled, obj.(map[string]any)) + id := "task-" + creationID + queued[id] = true + created[creationID] = map[string]any{"id": id} + } + json.NewEncoder(w).Encode(map[string]any{"methodResponses": []any{ + []any{"x:Task/set", map[string]any{"created": created}, "s"}, + }}) + case "x:Task/get": + // Report every task as gone: it ran and left the queue. + for _, raw := range args["ids"].([]any) { + delete(queued, raw.(string)) + } + json.NewEncoder(w).Encode(map[string]any{"methodResponses": []any{ + []any{"x:Task/get", map[string]any{"list": []any{}}, "g"}, + }}) + default: + t.Errorf("unexpected method call %s", name) + } + })) + t.Cleanup(srv.Close) + return srv, &scheduled +} + +func TestRunSchedulesOneQuotaTaskPerAccountAndWaits(t *testing.T) { + store, rs, opts := migratedRun(t) + srv, scheduled := quotaServer(t, []string{"a1", "a2", "a3"}) + opts.AdminURL = srv.URL + opts.AdminUser = "admin" + opts.RecalculateQuotas = true + + report, err := Run(context.Background(), store, rs, opts) + if err != nil { + t.Fatalf("Run: %v\n%s", err, report) + } + if len(*scheduled) != 3 { + t.Fatalf("scheduled %d task(s), want one per account", len(*scheduled)) + } + for _, obj := range *scheduled { + if obj["maintenanceType"] != "recalculateQuota" { + t.Errorf("maintenanceType = %v, want recalculateQuota", obj["maintenanceType"]) + } + } + for _, res := range report.Results { + if res.Name == "recalculate-quotas" { + if res.Status != StatusOK { + t.Errorf("recalculate-quotas = %s: %s", res.Status, res.Detail) + } + if !strings.Contains(res.Detail, "3 account(s)") { + t.Errorf("detail %q should say how many accounts were rebuilt", res.Detail) + } + } + } +} diff --git a/internal/cutover/doc.go b/internal/cutover/doc.go new file mode 100644 index 0000000..9a50627 --- /dev/null +++ b/internal/cutover/doc.go @@ -0,0 +1,3 @@ +// Package cutover implements switching the live service onto the migrated instance: binary swap, service definition, restart, quota recalculation. +// See ARCHITECTURE.md §4.5 for the design. +package cutover diff --git a/internal/rollback/report.go b/internal/cutover/report.go similarity index 63% rename from internal/rollback/report.go rename to internal/cutover/report.go index 5c196e4..b7f69dc 100644 --- a/internal/rollback/report.go +++ b/internal/cutover/report.go @@ -1,4 +1,4 @@ -package rollback +package cutover import ( "fmt" @@ -9,6 +9,7 @@ type Status string const ( StatusOK Status = "ok" + StatusWarn Status = "warn" StatusSkipped Status = "skip" StatusFail Status = "fail" ) @@ -23,9 +24,9 @@ type Report struct { Results []CheckResult } -// Blocking reports whether anything failed. A rollback report that isn't -// clean means the instance is in an unknown state - never quietly "rolled -// back". +// Blocking reports whether anything failed outright. A warning does not +// block: the one step allowed to warn is quota recalculation, which leaves +// counters stale rather than mail unreachable (see Run). func (r Report) Blocking() bool { for _, res := range r.Results { if res.Status == StatusFail { @@ -38,7 +39,7 @@ func (r Report) Blocking() bool { func (r Report) String() string { var b strings.Builder for _, res := range r.Results { - fmt.Fprintf(&b, "[%-4s] %-22s %s\n", strings.ToUpper(string(res.Status)), res.Name, res.Detail) + fmt.Fprintf(&b, "[%-4s] %-24s %s\n", strings.ToUpper(string(res.Status)), res.Name, res.Detail) } return b.String() } diff --git a/internal/cutover/unit.go b/internal/cutover/unit.go new file mode 100644 index 0000000..1b14357 --- /dev/null +++ b/internal/cutover/unit.go @@ -0,0 +1,128 @@ +package cutover + +import ( + "fmt" + "strings" +) + +// recoveryEnvVars are the two variables that must never survive into the +// live service definition. ARCHITECTURE.md §4.5 calls leaving +// STALWART_RECOVERY_MODE=1 set a documented footgun, and it is: the service +// would recovery-boot on every restart from then on, quietly, forever. +// +// This tool never puts them in a unit itself - internal/recovery runs +// recovery mode as a supervised child process, not through systemd - so +// finding them here means an operator followed the manual upgrade guide by +// hand at some point. That's exactly the case worth catching. +var recoveryEnvVars = []string{"STALWART_RECOVERY_MODE", "STALWART_RECOVERY_ADMIN"} + +// RewriteUnit points a systemd unit's ExecStart at a new binary (and, if +// configPath is non-empty, a new --config path) and strips any recovery-mode +// environment lines, returning the rewritten file. +// +// It rewrites in place rather than generating a unit from a template: the +// operator's unit is theirs, and it may carry hardening options, resource +// limits, dependencies and overrides this tool has no business having an +// opinion about. Replacing it with something generated would silently drop +// all of that. +// +// It refuses rather than guesses in two cases: a unit with no ExecStart at +// all, and an Environment line that mixes a recovery variable with other +// variables. Both mean the file isn't shaped the way this rewrite assumes, +// and editing it anyway risks producing a unit that starts something other +// than what the operator intended. +func RewriteUnit(unit, binaryPath, configPath string) (string, error) { + lines := strings.Split(unit, "\n") + out := make([]string, 0, len(lines)) + execStarts := 0 + + for _, line := range lines { + trimmed := strings.TrimSpace(line) + + if strings.HasPrefix(trimmed, "ExecStart=") { + rewritten, err := rewriteExecStart(line, binaryPath, configPath) + if err != nil { + return "", err + } + execStarts++ + out = append(out, rewritten) + continue + } + + if strings.HasPrefix(trimmed, "Environment=") || strings.HasPrefix(trimmed, "Environment ") { + mentions, only := classifyEnvironmentLine(trimmed) + if mentions && !only { + return "", fmt.Errorf( + "cutover: the unit's %q line sets a recovery-mode variable alongside others, and this tool won't edit a line it "+ + "only partly understands - remove the STALWART_RECOVERY_* assignment by hand and re-run", trimmed) + } + if mentions { + continue // the whole line is recovery-only: drop it + } + } + + out = append(out, line) + } + + if execStarts == 0 { + return "", fmt.Errorf("cutover: the service definition has no ExecStart= line, so there's nothing to point at the new binary - is this the right unit file?") + } + return strings.Join(out, "\n"), nil +} + +// rewriteExecStart replaces the executable in an ExecStart line, preserving +// every argument after it (and any leading whitespace or systemd prefix +// characters like "-" or "@"), then updates --config if asked to. +func rewriteExecStart(line, binaryPath, configPath string) (string, error) { + indent := line[:len(line)-len(strings.TrimLeft(line, " \t"))] + value := strings.TrimSpace(line)[len("ExecStart="):] + + // systemd allows prefix characters on the executable ("-", "@", ":", + // "+", "!"). Preserve whatever is there rather than dropping semantics + // the operator chose deliberately. + prefix := "" + for len(value) > 0 && strings.ContainsRune("-@:+!", rune(value[0])) { + prefix += string(value[0]) + value = value[1:] + } + + fields := strings.Fields(value) + if len(fields) == 0 { + return "", fmt.Errorf("cutover: the unit's ExecStart= line names no executable") + } + fields[0] = binaryPath + + if configPath != "" { + replaced := false + for i := 0; i < len(fields)-1; i++ { + if fields[i] == "--config" || fields[i] == "-c" { + fields[i+1] = configPath + replaced = true + } + } + if !replaced { + fields = append(fields, "--config", configPath) + } + } + return indent + "ExecStart=" + prefix + strings.Join(fields, " "), nil +} + +// classifyEnvironmentLine reports whether an Environment= line mentions a +// recovery variable at all, and whether that's all it sets. +func classifyEnvironmentLine(trimmed string) (mentions, only bool) { + value := trimmed[strings.Index(trimmed, "=")+1:] + assignments := strings.Fields(value) + if len(assignments) == 0 { + return false, false + } + recoveryCount := 0 + for _, a := range assignments { + a = strings.Trim(a, `"'`) + for _, name := range recoveryEnvVars { + if strings.HasPrefix(a, name+"=") { + recoveryCount++ + } + } + } + return recoveryCount > 0, recoveryCount == len(assignments) +} diff --git a/internal/cutover/unit_test.go b/internal/cutover/unit_test.go new file mode 100644 index 0000000..35e9487 --- /dev/null +++ b/internal/cutover/unit_test.go @@ -0,0 +1,127 @@ +package cutover + +import ( + "strings" + "testing" +) + +const realisticUnit = `[Unit] +Description=Stalwart Mail Server +After=network.target + +[Service] +Type=simple +User=stalwart +ExecStart=/usr/local/bin/stalwart --config /etc/stalwart/config.toml +Restart=on-failure +LimitNOFILE=65536 +ProtectSystem=strict +ReadWritePaths=/var/lib/stalwart + +[Install] +WantedBy=multi-user.target +` + +func TestRewriteUnitRepointsExecStart(t *testing.T) { + got, err := RewriteUnit(realisticUnit, "/usr/local/bin/stalwart", "/etc/stalwart/config.json") + if err != nil { + t.Fatal(err) + } + if !strings.Contains(got, "ExecStart=/usr/local/bin/stalwart --config /etc/stalwart/config.json") { + t.Errorf("ExecStart not repointed:\n%s", got) + } +} + +// The operator's unit is theirs: hardening options, limits and paths this +// tool has no opinion about must survive untouched. +func TestRewriteUnitPreservesEverythingElse(t *testing.T) { + got, err := RewriteUnit(realisticUnit, "/opt/stalwart/bin/stalwart", "") + if err != nil { + t.Fatal(err) + } + for _, want := range []string{ + "Description=Stalwart Mail Server", "User=stalwart", "Restart=on-failure", + "LimitNOFILE=65536", "ProtectSystem=strict", "ReadWritePaths=/var/lib/stalwart", + "WantedBy=multi-user.target", + } { + if !strings.Contains(got, want) { + t.Errorf("rewrite dropped %q:\n%s", want, got) + } + } + if !strings.Contains(got, "ExecStart=/opt/stalwart/bin/stalwart --config /etc/stalwart/config.toml") { + t.Errorf("existing --config should be preserved when no new one is given:\n%s", got) + } +} + +func TestRewriteUnitAddsConfigWhenTheUnitHasNone(t *testing.T) { + got, err := RewriteUnit("[Service]\nExecStart=/usr/local/bin/stalwart\n", "/usr/local/bin/stalwart", "/etc/stalwart/config.json") + if err != nil { + t.Fatal(err) + } + if !strings.Contains(got, "ExecStart=/usr/local/bin/stalwart --config /etc/stalwart/config.json") { + t.Errorf("--config not added:\n%s", got) + } +} + +func TestRewriteUnitKeepsSystemdExecPrefixes(t *testing.T) { + got, err := RewriteUnit("[Service]\nExecStart=-@/old/stalwart --config /c\n", "/new/stalwart", "") + if err != nil { + t.Fatal(err) + } + if !strings.Contains(got, "ExecStart=-@/new/stalwart --config /c") { + t.Errorf("systemd exec prefix characters were dropped, changing what the unit means:\n%s", got) + } +} + +// Leaving STALWART_RECOVERY_MODE=1 in the unit is the documented footgun +// from §4.5: the service would recovery-boot on every restart, forever. +func TestRewriteUnitStripsRecoveryEnvironmentLines(t *testing.T) { + unit := `[Service] +Environment=STALWART_RECOVERY_MODE=1 +Environment="STALWART_RECOVERY_ADMIN=admin:hunter2" +Environment=RUST_LOG=info +ExecStart=/usr/local/bin/stalwart +` + got, err := RewriteUnit(unit, "/usr/local/bin/stalwart", "") + if err != nil { + t.Fatal(err) + } + for _, gone := range []string{"STALWART_RECOVERY_MODE", "STALWART_RECOVERY_ADMIN"} { + if strings.Contains(got, gone) { + t.Errorf("%s survived the rewrite - the service would recovery-boot on every restart:\n%s", gone, got) + } + } + if !strings.Contains(got, "Environment=RUST_LOG=info") { + t.Errorf("unrelated Environment line was dropped:\n%s", got) + } +} + +// A line this tool only partly understands is one it must not edit. +func TestRewriteUnitRefusesAMixedEnvironmentLine(t *testing.T) { + unit := "[Service]\nEnvironment=RUST_LOG=info STALWART_RECOVERY_MODE=1\nExecStart=/usr/local/bin/stalwart\n" + _, err := RewriteUnit(unit, "/usr/local/bin/stalwart", "") + if err == nil { + t.Fatal("want refusal for an Environment line mixing recovery and other variables, got nil") + } + if !strings.Contains(err.Error(), "by hand") { + t.Errorf("error %q should tell the operator what to do about it", err) + } +} + +func TestRewriteUnitRefusesAUnitWithNoExecStart(t *testing.T) { + _, err := RewriteUnit("[Unit]\nDescription=Something else entirely\n", "/usr/local/bin/stalwart", "") + if err == nil { + t.Fatal("want refusal for a unit with no ExecStart, got nil") + } + if !strings.Contains(err.Error(), "right unit file") { + t.Errorf("error %q should question whether this is the right file", err) + } +} + +func TestRewriteUnitHandlesMultipleExecStartLines(t *testing.T) { + unit := "[Service]\nExecStart=\nExecStart=/old/stalwart --config /c\n" + got, err := RewriteUnit(unit, "/new/stalwart", "") + if err == nil { + t.Fatalf("an empty ExecStart= names no executable and should be refused, got:\n%s", got) + } +} diff --git a/internal/plan/plan.go b/internal/plan/plan.go index bc84441..bd9d18a 100644 --- a/internal/plan/plan.go +++ b/internal/plan/plan.go @@ -50,8 +50,7 @@ func cmp(a, b int) int { // PhaseName identifies one phase in an ordered migration plan - the phase // packages this names are internal/preflight, internal/backup, -// internal/recovery, internal/rollback (invoked on failure, not part of the -// forward list), and internal/validate. +// internal/recovery, internal/cutover and internal/validate. type PhaseName string const ( diff --git a/internal/preflight/deployment.go b/internal/preflight/deployment.go index b474675..cd6e07f 100644 --- a/internal/preflight/deployment.go +++ b/internal/preflight/deployment.go @@ -9,7 +9,7 @@ import ( ) // DeploymentKind is how a Stalwart instance appears to be run, which -// determines how cutover and rollback restart it. It's an alias for +// determines how cutover restarts it. It's an alias for // service.Kind rather than a parallel type: detection here and control // there have to agree on the same vocabulary, and one definition can't // drift from itself. diff --git a/internal/rollback/doc.go b/internal/rollback/doc.go deleted file mode 100644 index 99f3baa..0000000 --- a/internal/rollback/doc.go +++ /dev/null @@ -1,3 +0,0 @@ -// Package rollback implements restoring the pre-migration backup and old binary on failure. -// See ARCHITECTURE.md §4.8 for the design. -package rollback diff --git a/internal/rollback/exectest_test.go b/internal/rollback/exectest_test.go deleted file mode 100644 index db65fdd..0000000 --- a/internal/rollback/exectest_test.go +++ /dev/null @@ -1,92 +0,0 @@ -package rollback - -import ( - "context" - "fmt" - "os" - "path/filepath" - "testing" -) - -// withFakeExecutable puts a fake executable named `name` at the front of -// PATH for the duration of the test, so code that shells out to a -// real-world tool (psql, mysql, the stalwart binary's --version) can be -// exercised without that tool being installed. t.Setenv restores PATH -// automatically and marks the test non-parallel. -func withFakeExecutable(t *testing.T, name, script string) (dir string) { - t.Helper() - dir = t.TempDir() - path := filepath.Join(dir, name) - if err := os.WriteFile(path, []byte(script), 0o755); err != nil { - t.Fatal(err) - } - t.Setenv("PATH", dir+string(os.PathListSeparator)+os.Getenv("PATH")) - return dir -} - -func argsFile(t *testing.T, dir string) string { - t.Helper() - return filepath.Join(dir, "invoked-args.log") -} - -func readFile(t *testing.T, path string) string { - t.Helper() - data, err := os.ReadFile(path) - if os.IsNotExist(err) { - return "" - } - if err != nil { - t.Fatal(err) - } - return string(data) -} - -func fakeScriptLoggingArgs(logPath string, extraBody string) string { - return fmt.Sprintf("#!/bin/sh\necho \"$@\" >> %q\n%s\n", logPath, extraBody) -} - -// fakeController stands in for a real systemd unit or Docker container, so -// the orchestration in Run can be tested without either. It records the -// order it was called in - which is the thing that actually matters here: -// restoring a data directory while the service is still running would be a -// silent corruption bug, and call order is how that's caught. -type fakeController struct { - calls []string - active bool - stopErr error - startErr error - - // onStop runs after a successful Stop, letting a test assert on the - // state of the world at the exact moment the service went down. - onStop func() -} - -func (f *fakeController) Stop(context.Context) error { - f.calls = append(f.calls, "stop") - if f.stopErr != nil { - return f.stopErr - } - f.active = false - if f.onStop != nil { - f.onStop() - } - return nil -} - -func (f *fakeController) Start(context.Context) error { - f.calls = append(f.calls, "start") - if f.startErr != nil { - return f.startErr - } - f.active = true - return nil -} - -func (f *fakeController) Active(context.Context) (bool, error) { return f.active, nil } - -func (f *fakeController) ReloadConfig(context.Context) error { - f.calls = append(f.calls, "reload") - return nil -} - -func (f *fakeController) Target() string { return "test service" } diff --git a/internal/rollback/restore.go b/internal/rollback/restore.go deleted file mode 100644 index 09492f2..0000000 --- a/internal/rollback/restore.go +++ /dev/null @@ -1,223 +0,0 @@ -package rollback - -import ( - "context" - "crypto/sha256" - "encoding/hex" - "fmt" - "io" - "os" - "os/exec" - "path/filepath" - "strings" - - "github.com/LINUXexpert-org/stalwart-migrator/internal/backup" -) - -// PreserveFailedState moves a path aside to ".failed-" instead -// of deleting it, so a rollback never destroys the failed attempt's own -// state (ARCHITECTURE.md §4.8) - if the rollback itself turns out to have -// been the wrong call, or the failure needs diagnosing afterward, the -// half-migrated data is still there under a name that says which run -// produced it. -// -// It's idempotent for a resumed rollback: if the preserved path already -// exists, a previous attempt already did this, and the function reports -// that rather than clobbering the earlier rescue with whatever is at path -// now. -func PreserveFailedState(path, runID string) (preservedPath string, moved bool, err error) { - preservedPath = fmt.Sprintf("%s.failed-%s", path, runID) - - if _, statErr := os.Stat(preservedPath); statErr == nil { - return preservedPath, false, nil - } else if !os.IsNotExist(statErr) { - return "", false, fmt.Errorf("rollback: stat %s: %w", preservedPath, statErr) - } - - if _, statErr := os.Stat(path); os.IsNotExist(statErr) { - return preservedPath, false, nil // nothing there to preserve - } else if statErr != nil { - return "", false, fmt.Errorf("rollback: stat %s: %w", path, statErr) - } - - if err := os.Rename(path, preservedPath); err != nil { - return "", false, fmt.Errorf("rollback: move %s aside to %s: %w", path, preservedPath, err) - } - return preservedPath, true, nil -} - -// RestoreDataDir copies a verified backup back over the original data -// directory and then re-verifies what it wrote against the same manifest. -// The second verification is the point: a restore that silently truncated -// or corrupted a file would otherwise be indistinguishable from a good one -// until Stalwart failed to open its store, long after this tool reported -// success. -// -// The caller must have moved any existing dataDir aside first (see -// PreserveFailedState) - CopyDataDir clears its destination, and clearing a -// live data directory is not a decision this function should be making on -// its own. -func RestoreDataDir(backupDir, dataDir string, m *backup.Manifest) error { - if _, err := backup.CopyDataDir(backupDir, dataDir); err != nil { - return fmt.Errorf("rollback: restore %s from %s: %w", dataDir, backupDir, err) - } - if err := backup.VerifyDataDirBackup(dataDir, m); err != nil { - return fmt.Errorf("rollback: the restored data directory doesn't match the backup manifest: %w", err) - } - return nil -} - -// RestoreBinary puts the preserved old binary back at binaryPath, moving -// whatever is there now aside first (never deleting it - the new-version -// binary is what a retry after the underlying issue is fixed will want). -// It verifies the preserved binary's checksum against the one recorded when -// it was preserved before installing it, so a rollback can't restore a -// binary that was corrupted or swapped since backup ran. -func RestoreBinary(preservedPath, binaryPath, wantSHA256, runID string) (displaced string, err error) { - sum, _, err := hashFile(preservedPath) - if err != nil { - return "", err - } - if wantSHA256 != "" && sum != wantSHA256 { - return "", fmt.Errorf( - "rollback: preserved binary %s has sha256 %s but the checkpoint recorded %s when it was preserved - refusing to install a binary that changed since then", - preservedPath, sum, wantSHA256) - } - - displaced, moved, err := PreserveFailedState(binaryPath, runID) - if err != nil { - return "", err - } - if !moved { - displaced = "" - } - - if err := os.Rename(preservedPath, binaryPath); err != nil { - return "", fmt.Errorf("rollback: restore %s to %s: %w", preservedPath, binaryPath, err) - } - return displaced, nil -} - -// RestoreFile copies src over dst (used for a preserved systemd unit or -// Compose file), preserving dst's own mode if it exists and falling back to -// src's otherwise. It writes to a temp file in the destination directory -// and renames it into place, so a service definition is never left -// half-written - the same reason checkpoint.Store.Save does it. -func RestoreFile(src, dst string) error { - perm := os.FileMode(0o644) - if info, err := os.Stat(dst); err == nil { - perm = info.Mode().Perm() - } else if info, err := os.Stat(src); err == nil { - perm = info.Mode().Perm() - } - - data, err := os.ReadFile(src) - if err != nil { - return fmt.Errorf("rollback: read %s: %w", src, err) - } - tmp, err := os.CreateTemp(filepath.Dir(dst), filepath.Base(dst)+".tmp-*") - if err != nil { - return fmt.Errorf("rollback: create temp file next to %s: %w", dst, err) - } - tmpPath := tmp.Name() - defer os.Remove(tmpPath) // no-op once the rename below succeeds - - if _, err := tmp.Write(data); err != nil { - tmp.Close() - return fmt.Errorf("rollback: write %s: %w", tmpPath, err) - } - if err := tmp.Chmod(perm); err != nil { - tmp.Close() - return fmt.Errorf("rollback: chmod %s: %w", tmpPath, err) - } - if err := tmp.Sync(); err != nil { - tmp.Close() - return fmt.Errorf("rollback: sync %s: %w", tmpPath, err) - } - if err := tmp.Close(); err != nil { - return fmt.Errorf("rollback: close %s: %w", tmpPath, err) - } - if err := os.Rename(tmpPath, dst); err != nil { - return fmt.Errorf("rollback: move %s into place at %s: %w", tmpPath, dst, err) - } - return nil -} - -// BuildPsqlArgs returns psql's argv for restoring the critical-table dump -// backup.RunPgDump produced, without the leading "psql". ON_ERROR_STOP=1 is -// not optional here: psql's default is to report an error and carry on, -// which would let a restore that only half-applied exit zero and be -// reported as a successful rollback. -func BuildPsqlArgs(o backup.SQLOptions) []string { - args := []string{"-v", "ON_ERROR_STOP=1", "-U", o.User, "-d", o.Database} - if o.Host != "" { - args = append(args, "-h", o.Host) - } - if o.Port != "" { - args = append(args, "-p", o.Port) - } - return append(args, "-f", o.OutPath) -} - -// BuildMySQLRestoreArgs returns mysql's argv for the same restore. mysql -// reads the dump from stdin rather than taking a file flag, so RunMySQLRestore -// redirects it. -func BuildMySQLRestoreArgs(o backup.SQLOptions) []string { - args := []string{"-u", o.User} - if o.Host != "" { - args = append(args, "-h", o.Host) - } - if o.Port != "" { - args = append(args, "-P", o.Port) - } - return append(args, o.Database) -} - -// RunPsqlRestore replays a pg_dump file, passing the password via -// PGPASSWORD exactly as backup.RunPgDump does rather than on the command -// line where `ps` could read it. -func RunPsqlRestore(ctx context.Context, o backup.SQLOptions) error { - cmd := exec.CommandContext(ctx, "psql", BuildPsqlArgs(o)...) - cmd.Env = append(os.Environ(), "PGPASSWORD="+o.Password) - out, err := cmd.CombinedOutput() - if err != nil { - return fmt.Errorf("rollback: psql restore failed: %w (output: %s)", err, strings.TrimSpace(string(out))) - } - return nil -} - -// RunMySQLRestore replays a mysqldump file from stdin, with the password -// passed via MYSQL_PWD. -func RunMySQLRestore(ctx context.Context, o backup.SQLOptions) error { - f, err := os.Open(o.OutPath) - if err != nil { - return fmt.Errorf("rollback: open dump %s: %w", o.OutPath, err) - } - defer f.Close() - - cmd := exec.CommandContext(ctx, "mysql", BuildMySQLRestoreArgs(o)...) - cmd.Env = append(os.Environ(), "MYSQL_PWD="+o.Password) - cmd.Stdin = f - var stderr strings.Builder - cmd.Stderr = &stderr - if err := cmd.Run(); err != nil { - return fmt.Errorf("rollback: mysql restore failed: %w (stderr: %s)", err, strings.TrimSpace(stderr.String())) - } - return nil -} - -// hashFile returns a file's SHA256 and size, for checking a preserved -// artifact against what the checkpoint recorded for it. -func hashFile(path string) (sha256Hex string, size int64, err error) { - f, err := os.Open(path) - if err != nil { - return "", 0, fmt.Errorf("rollback: hash %s: %w", path, err) - } - defer f.Close() - h := sha256.New() - n, err := io.Copy(h, f) - if err != nil { - return "", 0, fmt.Errorf("rollback: hash %s: %w", path, err) - } - return hex.EncodeToString(h.Sum(nil)), n, nil -} diff --git a/internal/rollback/restore_test.go b/internal/rollback/restore_test.go deleted file mode 100644 index cbba04a..0000000 --- a/internal/rollback/restore_test.go +++ /dev/null @@ -1,300 +0,0 @@ -package rollback - -import ( - "context" - "os" - "path/filepath" - "strings" - "testing" - - "github.com/LINUXexpert-org/stalwart-migrator/internal/backup" -) - -// writeTree creates a small directory tree and returns its path, standing -// in for a Stalwart data directory. -func writeTree(t *testing.T, root string, files map[string]string) string { - t.Helper() - for rel, content := range files { - path := filepath.Join(root, rel) - if err := os.MkdirAll(filepath.Dir(path), 0o750); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(path, []byte(content), 0o640); err != nil { - t.Fatal(err) - } - } - return root -} - -func TestPreserveFailedStateMovesRatherThanDeletes(t *testing.T) { - dir := t.TempDir() - dataDir := writeTree(t, filepath.Join(dir, "data"), map[string]string{"blobs/a": "half-migrated"}) - - preserved, moved, err := PreserveFailedState(dataDir, "run-1") - if err != nil { - t.Fatal(err) - } - if !moved { - t.Error("moved = false, want true") - } - if want := dataDir + ".failed-run-1"; preserved != want { - t.Errorf("preserved path = %q, want %q", preserved, want) - } - if _, err := os.Stat(dataDir); !os.IsNotExist(err) { - t.Errorf("original path still exists after being moved aside: %v", err) - } - if got := readFile(t, filepath.Join(preserved, "blobs/a")); got != "half-migrated" { - t.Errorf("preserved content = %q, want the failed attempt's data intact", got) - } -} - -// A resumed rollback re-runs steps that were interrupted. If this clobbered -// the earlier rescue with whatever is at the original path the second time -// around, the failed attempt's state - the thing §4.8 promises never to -// delete - would be lost precisely when a rollback got interrupted. -func TestPreserveFailedStateDoesNotClobberAnEarlierRescue(t *testing.T) { - dir := t.TempDir() - dataDir := filepath.Join(dir, "data") - writeTree(t, dataDir, map[string]string{"a": "first"}) - if _, _, err := PreserveFailedState(dataDir, "run-1"); err != nil { - t.Fatal(err) - } - writeTree(t, dataDir, map[string]string{"a": "second"}) - - preserved, moved, err := PreserveFailedState(dataDir, "run-1") - if err != nil { - t.Fatal(err) - } - if moved { - t.Error("moved = true on the second call, want false") - } - if got := readFile(t, filepath.Join(preserved, "a")); got != "first" { - t.Errorf("preserved content = %q, want %q - the first rescue must survive", got, "first") - } -} - -func TestPreserveFailedStateIsFineWhenThereIsNothingToPreserve(t *testing.T) { - preserved, moved, err := PreserveFailedState(filepath.Join(t.TempDir(), "absent"), "run-1") - if err != nil { - t.Fatalf("want no error when the path doesn't exist, got %v", err) - } - if moved { - t.Error("moved = true, want false") - } - if preserved == "" { - t.Error("preserved path should still be reported") - } -} - -func TestRestoreDataDirRestoresAndReverifies(t *testing.T) { - dir := t.TempDir() - src := writeTree(t, filepath.Join(dir, "data"), map[string]string{ - "config": "settings", "blobs/one": "hello", "blobs/two": "world", - }) - backupDir := filepath.Join(dir, "backup") - manifest, err := backup.CopyDataDir(src, backupDir) - if err != nil { - t.Fatal(err) - } - if err := os.RemoveAll(src); err != nil { - t.Fatal(err) - } - - if err := RestoreDataDir(backupDir, src, manifest); err != nil { - t.Fatalf("RestoreDataDir: %v", err) - } - for rel, want := range map[string]string{"config": "settings", "blobs/one": "hello", "blobs/two": "world"} { - if got := readFile(t, filepath.Join(src, rel)); got != want { - t.Errorf("restored %s = %q, want %q", rel, got, want) - } - } -} - -// A restore that put back corrupt bytes and reported success would be worse -// than one that failed: the operator would believe the rollback worked and -// only find out when Stalwart couldn't open its store. -func TestRestoreDataDirFailsWhenTheBackupNoLongerMatchesItsManifest(t *testing.T) { - dir := t.TempDir() - src := writeTree(t, filepath.Join(dir, "data"), map[string]string{"blobs/one": "hello"}) - backupDir := filepath.Join(dir, "backup") - manifest, err := backup.CopyDataDir(src, backupDir) - if err != nil { - t.Fatal(err) - } - if err := os.WriteFile(filepath.Join(backupDir, "blobs/one"), []byte("corrupted"), 0o640); err != nil { - t.Fatal(err) - } - if err := os.RemoveAll(src); err != nil { - t.Fatal(err) - } - - err = RestoreDataDir(backupDir, src, manifest) - if err == nil { - t.Fatal("RestoreDataDir: want error for a backup that no longer matches its manifest, got nil") - } - if !strings.Contains(err.Error(), "doesn't match the backup manifest") { - t.Errorf("error %q should say the restored directory didn't match the manifest", err) - } -} - -func TestRestoreBinaryReinstallsOldAndPreservesCurrent(t *testing.T) { - dir := t.TempDir() - binaryPath := filepath.Join(dir, "stalwart") - preserved := binaryPath + ".v0.15.5" - if err := os.WriteFile(preserved, []byte("old binary"), 0o755); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(binaryPath, []byte("new binary"), 0o755); err != nil { - t.Fatal(err) - } - sum, _, err := hashFile(preserved) - if err != nil { - t.Fatal(err) - } - - displaced, err := RestoreBinary(preserved, binaryPath, sum, "run-1") - if err != nil { - t.Fatalf("RestoreBinary: %v", err) - } - if got := readFile(t, binaryPath); got != "old binary" { - t.Errorf("binary at %s = %q, want the old one back", binaryPath, got) - } - if got := readFile(t, displaced); got != "new binary" { - t.Errorf("displaced binary = %q, want the new one preserved for a retry", got) - } -} - -func TestRestoreBinaryRefusesAChangedBinary(t *testing.T) { - dir := t.TempDir() - binaryPath := filepath.Join(dir, "stalwart") - preserved := binaryPath + ".v0.15.5" - if err := os.WriteFile(preserved, []byte("swapped out from under us"), 0o755); err != nil { - t.Fatal(err) - } - - _, err := RestoreBinary(preserved, binaryPath, "0000000000000000000000000000000000000000000000000000000000000000", "run-1") - if err == nil { - t.Fatal("RestoreBinary: want error when the preserved binary's checksum doesn't match, got nil") - } - if _, statErr := os.Stat(binaryPath); !os.IsNotExist(statErr) { - t.Error("a binary failing its checksum must not be installed") - } -} - -func TestRestoreFileKeepsDestinationPermissions(t *testing.T) { - dir := t.TempDir() - src := filepath.Join(dir, "stalwart.service.preserved") - dst := filepath.Join(dir, "stalwart.service") - if err := os.WriteFile(src, []byte("[Service]\nExecStart=/usr/local/bin/stalwart\n"), 0o600); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(dst, []byte("[Service]\nExecStart=/usr/local/bin/stalwart-new\n"), 0o644); err != nil { - t.Fatal(err) - } - - if err := RestoreFile(src, dst); err != nil { - t.Fatalf("RestoreFile: %v", err) - } - if got := readFile(t, dst); !strings.Contains(got, "/usr/local/bin/stalwart\n") { - t.Errorf("restored unit = %q, want the preserved one", got) - } - info, err := os.Stat(dst) - if err != nil { - t.Fatal(err) - } - if info.Mode().Perm() != 0o644 { - t.Errorf("restored unit mode = %v, want 0644 (the destination's own mode)", info.Mode().Perm()) - } - // The temp file it writes through must not be left behind next to a - // service definition directory. - entries, err := os.ReadDir(dir) - if err != nil { - t.Fatal(err) - } - if len(entries) != 2 { - t.Errorf("directory has %d entries, want 2 - a temp file was left behind", len(entries)) - } -} - -// psql's default is to report an error and keep going, which would let a -// half-applied restore exit zero and be reported as a successful rollback. -func TestBuildPsqlArgsStopsOnError(t *testing.T) { - args := BuildPsqlArgs(backup.SQLOptions{User: "stalwart", Database: "mail", Host: "db", Port: "5432", OutPath: "/tmp/dump.sql"}) - joined := strings.Join(args, " ") - if !strings.Contains(joined, "-v ON_ERROR_STOP=1") { - t.Errorf("psql args %q must set ON_ERROR_STOP=1", joined) - } - for _, want := range []string{"-U stalwart", "-d mail", "-h db", "-p 5432", "-f /tmp/dump.sql"} { - if !strings.Contains(joined, want) { - t.Errorf("psql args %q missing %q", joined, want) - } - } -} - -func TestBuildMySQLRestoreArgsOmitsUnsetConnectionFields(t *testing.T) { - args := BuildMySQLRestoreArgs(backup.SQLOptions{User: "stalwart", Database: "mail"}) - if got, want := strings.Join(args, " "), "-u stalwart mail"; got != want { - t.Errorf("mysql args = %q, want %q", got, want) - } -} - -func TestRunPsqlRestorePassesPasswordViaEnvironmentNotArgv(t *testing.T) { - dir := t.TempDir() - log := argsFile(t, dir) - withFakeExecutable(t, "psql", fakeScriptLoggingArgs(log, "echo \"PGPASSWORD=$PGPASSWORD\" >> "+log)) - - err := RunPsqlRestore(context.Background(), backup.SQLOptions{ - User: "stalwart", Database: "mail", Password: "hunter2", OutPath: filepath.Join(dir, "dump.sql"), - }) - if err != nil { - t.Fatal(err) - } - logged := readFile(t, log) - if !strings.Contains(logged, "PGPASSWORD=hunter2") { - t.Errorf("psql invocation %q should receive the password via PGPASSWORD", logged) - } - argv := strings.SplitN(logged, "\n", 2)[0] - if strings.Contains(argv, "hunter2") { - t.Errorf("psql argv %q contains the password - anything running `ps` could read it", argv) - } -} - -func TestRunPsqlRestoreSurfacesFailureOutput(t *testing.T) { - dir := t.TempDir() - withFakeExecutable(t, "psql", "#!/bin/sh\necho 'ERROR: relation \"s\" already exists' >&2\nexit 1\n") - err := RunPsqlRestore(context.Background(), backup.SQLOptions{User: "u", Database: "d", OutPath: filepath.Join(dir, "dump.sql")}) - if err == nil { - t.Fatal("want error when psql fails, got nil") - } - if !strings.Contains(err.Error(), "already exists") { - t.Errorf("error %q should carry psql's own output", err) - } -} - -func TestRunMySQLRestoreFeedsTheDumpOnStdin(t *testing.T) { - dir := t.TempDir() - log := argsFile(t, dir) - dump := filepath.Join(dir, "dump.sql") - if err := os.WriteFile(dump, []byte("INSERT INTO s VALUES (1);\n"), 0o640); err != nil { - t.Fatal(err) - } - withFakeExecutable(t, "mysql", fakeScriptLoggingArgs(log, "cat >> "+log)) - - err := RunMySQLRestore(context.Background(), backup.SQLOptions{ - User: "stalwart", Database: "mail", Password: "hunter2", OutPath: dump, - }) - if err != nil { - t.Fatal(err) - } - if got := readFile(t, log); !strings.Contains(got, "INSERT INTO s VALUES (1);") { - t.Errorf("mysql stdin = %q, want the dump's contents", got) - } -} - -func TestRunMySQLRestoreFailsOnMissingDump(t *testing.T) { - withFakeExecutable(t, "mysql", "#!/bin/sh\nexit 0\n") - err := RunMySQLRestore(context.Background(), backup.SQLOptions{User: "u", Database: "d", OutPath: filepath.Join(t.TempDir(), "absent.sql")}) - if err == nil { - t.Fatal("want error when the dump file doesn't exist, got nil") - } -} diff --git a/internal/rollback/rollback.go b/internal/rollback/rollback.go deleted file mode 100644 index 8e1dbdf..0000000 --- a/internal/rollback/rollback.go +++ /dev/null @@ -1,460 +0,0 @@ -package rollback - -import ( - "context" - "fmt" - "net/http" - "strings" - "time" - - "github.com/LINUXexpert-org/stalwart-migrator/internal/backup" - "github.com/LINUXexpert-org/stalwart-migrator/internal/checkpoint" - "github.com/LINUXexpert-org/stalwart-migrator/internal/service" -) - -// Artifact names this phase reads out of the checkpoint. backup.Run writes -// the first three as it captures them; ArtifactServiceUnit is the contract -// a future cutover phase has to honour when it rewrites a systemd unit or -// Compose file, so this phase can put the original back. -const ( - ArtifactOldBinary = "old-binary" - ArtifactFSBackup = "fs-backup" - ArtifactSQLDump = "sql-dump" - ArtifactServiceUnit = "service-unit" -) - -// Options configures one rollback. Most paths default to what the run's own -// checkpoint recorded, so an operator rolling back days later doesn't have -// to remember - and can't mistype - where the backup went. -type Options struct { - // Deployment names the service to stop and start. Controller overrides - // it outright when a caller already has one (cutover will, and the - // tests do). - Deployment service.Options - Controller service.Controller - - StopTimeout time.Duration // how long to wait for the service to actually stop; default 60s - StartTimeout time.Duration // how long to wait for it to come back; default 60s - - // DataDir is where an embedded-backend store is restored to. Required - // for rocksdb/sqlite runs. - DataDir string - // BackupDir and ManifestPath default to the fs-snapshot step's recorded - // artifact and manifest. - BackupDir string - ManifestPath string - - // SQL configures an external-database restore. OutPath defaults to the - // recorded sql-dump artifact; the connection fields must be supplied, - // since the checkpoint deliberately never stores database credentials. - SQL backup.SQLOptions - - // BinaryPath is where the preserved old binary goes back to. - BinaryPath string - // ServiceUnitPath is where a preserved systemd unit or Compose file - // goes back to. Both this and an ArtifactServiceUnit record are needed - // for that step to do anything. - ServiceUnitPath string - - AdminURL string - AdminUser string - AdminPassword string - HTTPClient *http.Client - VerifyTimeout time.Duration -} - -// Plan is what a rollback would do, resolved from the run's checkpoint -// before anything is touched. Building it can fail; executing it is what -// takes mail delivery down, so every reason to refuse is found here first. -type Plan struct { - RunID string - SourceVersion string // the version this rollback restores the instance to - Method string // "filesystem", "postgresql", or "mysql" - - Target string // what the service controller acts on - - BackupDir string - ManifestPath string - DataDir string - SQLDumpPath string - SQLDatabase string - - PreservedBinary string // preserved old binary to reinstall, "" if none was preserved - BinaryPath string - - ServiceUnitSource string // preserved unit/compose file, "" if none - ServiceUnitDest string -} - -// String renders the plan as the confirmation an operator should read -// before agreeing to it - this phase overwrites a live data directory, so -// "what exactly is about to happen" has to be answerable without reading -// the source. -func (p Plan) String() string { - var b strings.Builder - fmt.Fprintf(&b, "rollback plan for run %s:\n", p.RunID) - fmt.Fprintf(&b, " 1. stop %s\n", p.Target) - switch p.Method { - case "filesystem": - fmt.Fprintf(&b, " 2. move %s aside to %s.failed-%s (nothing is deleted)\n", p.DataDir, p.DataDir, p.RunID) - fmt.Fprintf(&b, " 3. restore %s from the verified backup at %s\n", p.DataDir, p.BackupDir) - default: - fmt.Fprintf(&b, " 2. replay the %s critical-table dump at %s into database %q\n", p.Method, p.SQLDumpPath, p.SQLDatabase) - fmt.Fprintf(&b, " (this overwrites those tables in place - unlike the filesystem path, the current contents are NOT preserved)\n") - } - if p.PreservedBinary != "" { - fmt.Fprintf(&b, " 4. reinstall %s as %s (the current binary is moved aside, not deleted)\n", p.PreservedBinary, p.BinaryPath) - } else { - fmt.Fprintf(&b, " 4. leave the binary alone - this run never preserved one\n") - } - if p.ServiceUnitSource != "" { - fmt.Fprintf(&b, " 5. restore the service definition %s to %s\n", p.ServiceUnitSource, p.ServiceUnitDest) - } else { - fmt.Fprintf(&b, " 5. leave the service definition alone - this run never preserved one\n") - } - version := p.SourceVersion - if version == "" { - version = "the version this run started from" - } - fmt.Fprintf(&b, " 6. start %s and verify it came back on %s\n", p.Target, version) - return b.String() -} - -// BuildPlan resolves what a rollback of this run would do, refusing up -// front for anything it can't undo. Every refusal here happens before the -// service is stopped, which is the whole point of separating this from Run: -// discovering "there's no backup to restore" after taking mail delivery -// down would be the worst possible time to find out. -func BuildPlan(rs *checkpoint.RunState, opts Options) (Plan, error) { - p := Plan{ - RunID: rs.RunID, SourceVersion: rs.SourceVersion, - BinaryPath: opts.BinaryPath, ServiceUnitDest: opts.ServiceUnitPath, - SQLDatabase: opts.SQL.Database, - } - - if rs.RollbackWindowClosed { - return p, fmt.Errorf( - "rollback: run %s had its rollback window closed by `confirm` - the operator declared the migration good, "+ - "and the backups this would restore from may since have been pruned. Restore manually if you're sure", rs.RunID) - } - - controller := opts.Controller - if controller == nil { - var err error - controller, err = service.New(deploymentFor(rs, opts)) - if err != nil { - return p, err - } - } - p.Target = controller.Target() - - backends := strings.ToLower(rs.Topology.StoreBackend) - switch { - case strings.Contains(backends, "rocksdb") || strings.Contains(backends, "sqlite"): - p.Method = "filesystem" - art, found := rs.Artifacts[ArtifactFSBackup] - if !found && opts.BackupDir == "" { - return p, fmt.Errorf("rollback: run %s recorded no %s artifact - there is no filesystem backup to restore, so this run cannot be rolled back by this tool", rs.RunID, ArtifactFSBackup) - } - p.BackupDir = opts.BackupDir - if p.BackupDir == "" { - p.BackupDir = art.Path - } - p.ManifestPath = opts.ManifestPath - if p.ManifestPath == "" { - p.ManifestPath = rs.Outcome(checkpoint.PhaseBackup, "fs-snapshot").Extra - } - if p.ManifestPath == "" { - return p, fmt.Errorf("rollback: run %s recorded no backup manifest path - without it the backup can't be verified before it's restored; pass one explicitly if you have it", rs.RunID) - } - if opts.DataDir == "" { - return p, fmt.Errorf("rollback: a data directory to restore into is required for a %s backend", rs.Topology.StoreBackend) - } - p.DataDir = opts.DataDir - - case strings.Contains(backends, "postgresql"), strings.Contains(backends, "mysql"): - p.Method = "postgresql" - if strings.Contains(backends, "mysql") { - p.Method = "mysql" - } - art, found := rs.Artifacts[ArtifactSQLDump] - if !found && opts.SQL.OutPath == "" { - return p, fmt.Errorf("rollback: run %s recorded no %s artifact - there is no database dump to restore", rs.RunID, ArtifactSQLDump) - } - p.SQLDumpPath = opts.SQL.OutPath - if p.SQLDumpPath == "" { - p.SQLDumpPath = art.Path - } - if opts.SQL.Database == "" || opts.SQL.User == "" { - return p, fmt.Errorf("rollback: database name and user are required to restore a %s backend - the checkpoint deliberately doesn't store database credentials", p.Method) - } - - case strings.Contains(backends, "foundationdb"): - return p, fmt.Errorf( - "rollback: run %s uses a FoundationDB backend, whose backup step only *starts* an fdbbackup job - restoring it means "+ - "`fdbrestore` against a quiesced cluster, which this tool doesn't automate. Roll back manually and don't rely on this command", rs.RunID) - - default: - return p, fmt.Errorf( - "rollback: run %s recorded no recognized store backend (topology.store_backend=%q), so there's no way to know what to restore", - rs.RunID, rs.Topology.StoreBackend) - } - - if art, found := rs.Artifacts[ArtifactOldBinary]; found { - if opts.BinaryPath == "" { - return p, fmt.Errorf("rollback: run %s preserved the old binary at %s, but no path to reinstall it to was given", rs.RunID, art.Path) - } - p.PreservedBinary = art.Path - } - if art, found := rs.Artifacts[ArtifactServiceUnit]; found && opts.ServiceUnitPath != "" { - p.ServiceUnitSource = art.Path - } - return p, nil -} - -func deploymentFor(rs *checkpoint.RunState, opts Options) service.Options { - d := opts.Deployment - if d.Kind == "" { - d.Kind = service.Kind(rs.Topology.DeploymentKind) - } - return d -} - -// Run executes ARCHITECTURE.md §4.8: stop the service, put the verified -// pre-migration state back, restart the old binary, and confirm the -// restored instance actually works rather than assuming it does. Every step -// is checkpointed under PhaseRollback, so a rollback interrupted partway - -// which is exactly when a machine is most likely to be rebooted out from -// under it - resumes where it stopped instead of restarting a destructive -// sequence from the top. -// -// Nothing from the failed attempt is deleted: the half-migrated data -// directory and the new binary are moved aside under ".failed-" -// names, so a retry after the underlying issue is fixed still has both the -// evidence and the artifacts it needs. -func Run(ctx context.Context, store *checkpoint.Store, rs *checkpoint.RunState, opts Options) (Report, error) { - var report Report - - plan, err := BuildPlan(rs, opts) - if err != nil { - report.Results = append(report.Results, CheckResult{Name: "plan", Status: StatusFail, Detail: err.Error()}) - return report, err - } - - controller := opts.Controller - if controller == nil { - controller, err = service.New(deploymentFor(rs, opts)) - if err != nil { - report.Results = append(report.Results, CheckResult{Name: "plan", Status: StatusFail, Detail: err.Error()}) - return report, err - } - } - - step := func(name string, fn func() (checkpoint.StepOutcome, error)) error { - outcome, err := store.RunStep(rs, checkpoint.PhaseRollback, name, fn) - if err != nil { - report.Results = append(report.Results, CheckResult{Name: name, Status: StatusFail, Detail: err.Error()}) - return err - } - status := Status(outcome.Verdict) - if status == "" { - status = StatusOK - } - report.Results = append(report.Results, CheckResult{Name: name, Status: status, Detail: outcome.Detail}) - return nil - } - - var manifest *backup.Manifest - - // Verify the backup before stopping anything. Discovering that the - // backup is corrupt is survivable while the failed-but-running instance - // is still up; discovering it after the data directory has been moved - // aside is not. - if err := step("verify-backup", func() (checkpoint.StepOutcome, error) { - if plan.Method != "filesystem" { - art, found := rs.Artifacts[ArtifactSQLDump] - if !found { - return checkpoint.StepOutcome{Verdict: string(StatusSkipped), Detail: fmt.Sprintf("dump at %s was supplied by hand, with no recorded checksum to check it against", plan.SQLDumpPath)}, nil - } - sum, size, err := hashFile(plan.SQLDumpPath) - if err != nil { - return checkpoint.StepOutcome{}, err - } - if sum != art.SHA256 { - return checkpoint.StepOutcome{}, fmt.Errorf( - "the dump at %s has sha256 %s but the checkpoint recorded %s when it was taken - refusing to restore a dump that changed since the backup", - plan.SQLDumpPath, sum, art.SHA256) - } - return checkpoint.StepOutcome{Detail: fmt.Sprintf("dump at %s (%d bytes) still matches the checksum recorded at backup time", plan.SQLDumpPath, size)}, nil - } - m, err := backup.ReadManifest(plan.ManifestPath) - if err != nil { - return checkpoint.StepOutcome{}, err - } - if err := backup.VerifyDataDirBackup(plan.BackupDir, m); err != nil { - return checkpoint.StepOutcome{}, err - } - return checkpoint.StepOutcome{ - Detail: fmt.Sprintf("re-hashed %d file(s) in %s, all still match the manifest recorded at backup time", len(m.Files), plan.BackupDir), - Extra: plan.ManifestPath, - }, nil - }); err != nil { - return report, err - } - - // A resumed run skips the step above, so the manifest is loaded here - // rather than inside it - the restore below needs it either way. - if plan.Method == "filesystem" { - manifest, err = backup.ReadManifest(plan.ManifestPath) - if err != nil { - report.Results = append(report.Results, CheckResult{Name: "restore-data", Status: StatusFail, Detail: err.Error()}) - return report, err - } - } - - stopTimeout := opts.StopTimeout - if stopTimeout <= 0 { - stopTimeout = 60 * time.Second - } - if err := step("stop-service", func() (checkpoint.StepOutcome, error) { - if err := controller.Stop(ctx); err != nil { - return checkpoint.StepOutcome{}, err - } - if err := service.WaitFor(ctx, controller, false, stopTimeout); err != nil { - return checkpoint.StepOutcome{}, err - } - return checkpoint.StepOutcome{Detail: fmt.Sprintf("%s is stopped", controller.Target())}, nil - }); err != nil { - return report, err - } - - if err := step("preserve-failed-state", func() (checkpoint.StepOutcome, error) { - if plan.Method != "filesystem" { - return checkpoint.StepOutcome{ - Verdict: string(StatusSkipped), - Detail: "an external SQL store's current contents can't be moved aside the way a data directory can - the restore below " + - "overwrites those tables in place. Take your own dump first if the failed attempt's state matters", - }, nil - } - preserved, moved, err := PreserveFailedState(plan.DataDir, rs.RunID) - if err != nil { - return checkpoint.StepOutcome{}, err - } - if !moved { - return checkpoint.StepOutcome{Detail: fmt.Sprintf("nothing to move aside (%s was already preserved, or %s doesn't exist)", preserved, plan.DataDir), Extra: preserved}, nil - } - return checkpoint.StepOutcome{Detail: fmt.Sprintf("moved the failed attempt's data directory to %s - it is not deleted", preserved), Extra: preserved}, nil - }); err != nil { - return report, err - } - - if err := step("restore-data", func() (checkpoint.StepOutcome, error) { - if plan.Method != "filesystem" { - sqlOpts := opts.SQL - sqlOpts.OutPath = plan.SQLDumpPath - restore := RunPsqlRestore - if plan.Method == "mysql" { - restore = RunMySQLRestore - } - if err := restore(ctx, sqlOpts); err != nil { - return checkpoint.StepOutcome{}, err - } - return checkpoint.StepOutcome{Detail: fmt.Sprintf("replayed %s into database %s", plan.SQLDumpPath, sqlOpts.Database)}, nil - } - if err := RestoreDataDir(plan.BackupDir, plan.DataDir, manifest); err != nil { - return checkpoint.StepOutcome{}, err - } - return checkpoint.StepOutcome{ - Detail: fmt.Sprintf("restored %d file(s) to %s and re-verified every one against the backup manifest", len(manifest.Files), plan.DataDir), - }, nil - }); err != nil { - return report, err - } - - if err := step("restore-binary", func() (checkpoint.StepOutcome, error) { - if plan.PreservedBinary == "" { - return checkpoint.StepOutcome{ - Verdict: string(StatusSkipped), - Detail: "this run never preserved an old binary (a dry run, or one that failed before the backup phase) - nothing to reinstall", - }, nil - } - art := rs.Artifacts[ArtifactOldBinary] - displaced, err := RestoreBinary(plan.PreservedBinary, plan.BinaryPath, art.SHA256, rs.RunID) - if err != nil { - return checkpoint.StepOutcome{}, err - } - detail := fmt.Sprintf("reinstalled the %s binary at %s", rs.SourceVersion, plan.BinaryPath) - if displaced != "" { - detail += fmt.Sprintf("; the binary that was there is preserved at %s for a retry", displaced) - } - return checkpoint.StepOutcome{Detail: detail, Extra: displaced}, nil - }); err != nil { - return report, err - } - - if err := step("restore-service-config", func() (checkpoint.StepOutcome, error) { - if plan.ServiceUnitSource == "" { - return checkpoint.StepOutcome{ - Verdict: string(StatusSkipped), - Detail: fmt.Sprintf("no %q artifact recorded for this run - nothing rewrote the service definition, so there's nothing to put back "+ - "(cutover, once it exists, is what will record one)", ArtifactServiceUnit), - }, nil - } - if err := RestoreFile(plan.ServiceUnitSource, plan.ServiceUnitDest); err != nil { - return checkpoint.StepOutcome{}, err - } - if err := controller.ReloadConfig(ctx); err != nil { - return checkpoint.StepOutcome{}, err - } - return checkpoint.StepOutcome{Detail: fmt.Sprintf("restored %s from %s and reloaded the service definition", plan.ServiceUnitDest, plan.ServiceUnitSource)}, nil - }); err != nil { - return report, err - } - - startTimeout := opts.StartTimeout - if startTimeout <= 0 { - startTimeout = 60 * time.Second - } - if err := step("start-service", func() (checkpoint.StepOutcome, error) { - if err := controller.Start(ctx); err != nil { - return checkpoint.StepOutcome{}, err - } - if err := service.WaitFor(ctx, controller, true, startTimeout); err != nil { - return checkpoint.StepOutcome{}, err - } - return checkpoint.StepOutcome{Detail: fmt.Sprintf("%s is running again", controller.Target())}, nil - }); err != nil { - return report, err - } - - // The verification results are reported individually rather than - // collapsed into the single step outcome, so an operator sees which - // check failed, not just that one did. - var verifyResults []CheckResult - if err := step("verify-rollback", func() (checkpoint.StepOutcome, error) { - results, err := Verify(ctx, VerifyOptions{ - BinaryPath: plan.BinaryPath, ExpectVersion: rs.SourceVersion, - AdminURL: opts.AdminURL, AdminUser: opts.AdminUser, AdminPassword: opts.AdminPassword, - HTTPClient: opts.HTTPClient, Snapshot: rs.PreflightSnapshot, Timeout: opts.VerifyTimeout, - }) - verifyResults = results - if err != nil { - return checkpoint.StepOutcome{}, err - } - return checkpoint.StepOutcome{Detail: summarize(results)}, nil - }); err != nil { - report.Results = append(report.Results, verifyResults...) - return report, err - } - report.Results = append(report.Results, verifyResults...) - - return report, nil -} - -func summarize(results []CheckResult) string { - parts := make([]string, 0, len(results)) - for _, r := range results { - parts = append(parts, fmt.Sprintf("%s=%s", r.Name, r.Status)) - } - return strings.Join(parts, " ") -} diff --git a/internal/rollback/rollback_test.go b/internal/rollback/rollback_test.go deleted file mode 100644 index 16243b9..0000000 --- a/internal/rollback/rollback_test.go +++ /dev/null @@ -1,349 +0,0 @@ -package rollback - -import ( - "context" - "os" - "path/filepath" - "strings" - "testing" - - "github.com/LINUXexpert-org/stalwart-migrator/internal/backup" - "github.com/LINUXexpert-org/stalwart-migrator/internal/checkpoint" - "github.com/LINUXexpert-org/stalwart-migrator/internal/service" -) - -// fsRun builds a checkpoint that looks like a real embedded-backend run -// that got as far as taking (and recording) a verified filesystem backup: -// the state a rollback is actually invoked against. -func fsRun(t *testing.T) (store *checkpoint.Store, rs *checkpoint.RunState, dataDir, backupDir string) { - t.Helper() - root := t.TempDir() - dataDir = writeTree(t, filepath.Join(root, "data"), map[string]string{ - "config": "original settings", "blobs/one": "original mail", - }) - backupDir = filepath.Join(root, "backup") - - store = checkpoint.NewStore(filepath.Join(root, "runs")) - rs, err := store.Create("0.15.5", "0.16.14") - if err != nil { - t.Fatal(err) - } - rs.Topology = checkpoint.Topology{DeploymentKind: "systemd", StoreBackend: "rocksdb"} - - manifest, err := backup.CopyDataDir(dataDir, backupDir) - if err != nil { - t.Fatal(err) - } - manifestPath := filepath.Join(root, "backup.manifest.json") - if err := backup.WriteManifest(manifestPath, manifest); err != nil { - t.Fatal(err) - } - sum, err := manifest.Checksum() - if err != nil { - t.Fatal(err) - } - rs.RecordArtifact(ArtifactFSBackup, checkpoint.Artifact{Path: backupDir, SHA256: sum, SizeBytes: manifest.TotalBytes}) - if _, err := store.RunStep(rs, checkpoint.PhaseBackup, "fs-snapshot", func() (checkpoint.StepOutcome, error) { - return checkpoint.StepOutcome{Detail: "copied", Extra: manifestPath}, nil - }); err != nil { - t.Fatal(err) - } - - // Simulate the half-migrated state a failed cutover leaves behind. - if err := os.WriteFile(filepath.Join(dataDir, "config"), []byte("half-migrated settings"), 0o640); err != nil { - t.Fatal(err) - } - return store, rs, dataDir, backupDir -} - -func TestBuildPlanRefusesOnceTheRollbackWindowIsClosed(t *testing.T) { - _, rs, dataDir, _ := fsRun(t) - rs.RollbackWindowClosed = true - - _, err := BuildPlan(rs, Options{DataDir: dataDir, Controller: &fakeController{}}) - if err == nil { - t.Fatal("BuildPlan: want refusal once the operator has confirmed the migration, got nil") - } - if !strings.Contains(err.Error(), "rollback window closed") { - t.Errorf("error %q should say why it refuses", err) - } -} - -func TestBuildPlanRefusesWhatItCannotRestore(t *testing.T) { - for _, tc := range []struct { - name string - backend string - wantIn string - }{ - {"foundationdb", "foundationdb", "fdbrestore"}, - {"unrecognized", "", "no recognized store backend"}, - } { - t.Run(tc.name, func(t *testing.T) { - _, rs, dataDir, _ := fsRun(t) - rs.Topology.StoreBackend = tc.backend - _, err := BuildPlan(rs, Options{DataDir: dataDir, Controller: &fakeController{}}) - if err == nil { - t.Fatalf("BuildPlan for backend %q: want refusal, got nil", tc.backend) - } - if !strings.Contains(err.Error(), tc.wantIn) { - t.Errorf("error %q should mention %q", err, tc.wantIn) - } - }) - } -} - -func TestBuildPlanRefusesWhenNoBackupWasEverRecorded(t *testing.T) { - _, rs, dataDir, _ := fsRun(t) - delete(rs.Artifacts, ArtifactFSBackup) - - _, err := BuildPlan(rs, Options{DataDir: dataDir, Controller: &fakeController{}}) - if err == nil { - t.Fatal("BuildPlan: want refusal when there's no backup to restore, got nil") - } - if !strings.Contains(err.Error(), "cannot be rolled back") { - t.Errorf("error %q should say the run can't be rolled back by this tool", err) - } -} - -func TestBuildPlanRefusesAnUnknownDeploymentKind(t *testing.T) { - _, rs, dataDir, _ := fsRun(t) - rs.Topology.DeploymentKind = string(service.Unknown) - - _, err := BuildPlan(rs, Options{DataDir: dataDir}) - if err == nil { - t.Fatal("BuildPlan: want refusal when it doesn't know how to stop Stalwart, got nil") - } -} - -func TestBuildPlanFillsPathsFromTheCheckpoint(t *testing.T) { - _, rs, dataDir, backupDir := fsRun(t) - rs.RecordArtifact(ArtifactOldBinary, checkpoint.Artifact{Path: "/usr/local/bin/stalwart.v0.15.5", SHA256: "abc"}) - - plan, err := BuildPlan(rs, Options{ - DataDir: dataDir, BinaryPath: "/usr/local/bin/stalwart", Controller: &fakeController{}, - }) - if err != nil { - t.Fatalf("BuildPlan: %v", err) - } - if plan.Method != "filesystem" { - t.Errorf("Method = %q, want filesystem", plan.Method) - } - if plan.BackupDir != backupDir { - t.Errorf("BackupDir = %q, want the recorded artifact %q", plan.BackupDir, backupDir) - } - if plan.ManifestPath == "" { - t.Error("ManifestPath should come from the fs-snapshot step's recorded outcome") - } - if plan.PreservedBinary != "/usr/local/bin/stalwart.v0.15.5" { - t.Errorf("PreservedBinary = %q, want the recorded artifact", plan.PreservedBinary) - } - if !strings.Contains(plan.String(), "nothing is deleted") { - t.Errorf("plan text should tell the operator nothing is deleted:\n%s", plan) - } -} - -func TestRunRestoresTheDataDirectoryWhileTheServiceIsDown(t *testing.T) { - store, rs, dataDir, _ := fsRun(t) - var configAtStop string - ctl := &fakeController{active: true} - ctl.onStop = func() { configAtStop = readFile(t, filepath.Join(dataDir, "config")) } - - report, err := Run(context.Background(), store, rs, Options{DataDir: dataDir, Controller: ctl}) - if err != nil { - t.Fatalf("Run: %v\n%s", err, report) - } - - if got, want := strings.Join(ctl.calls, ","), "stop,start"; got != want { - t.Errorf("controller calls = %q, want %q", got, want) - } - if configAtStop != "half-migrated settings" { - t.Errorf("data was already touched when the service stopped (config=%q) - the restore must happen after the stop", configAtStop) - } - if got := readFile(t, filepath.Join(dataDir, "config")); got != "original settings" { - t.Errorf("restored config = %q, want the pre-migration contents", got) - } - if got := readFile(t, filepath.Join(dataDir, "blobs/one")); got != "original mail" { - t.Errorf("restored mail = %q, want the pre-migration contents", got) - } - if got := readFile(t, filepath.Join(dataDir+".failed-"+rs.RunID, "config")); got != "half-migrated settings" { - t.Errorf("failed attempt's data = %q, want it preserved rather than deleted", got) - } - if report.Blocking() { - t.Errorf("report should be clean:\n%s", report) - } -} - -// Discovering a corrupt backup is survivable while the failed instance is -// still up, and unsurvivable once its data directory has been moved aside. -func TestRunVerifiesTheBackupBeforeStoppingAnything(t *testing.T) { - store, rs, dataDir, backupDir := fsRun(t) - if err := os.WriteFile(filepath.Join(backupDir, "blobs/one"), []byte("corrupt"), 0o640); err != nil { - t.Fatal(err) - } - ctl := &fakeController{active: true} - - report, err := Run(context.Background(), store, rs, Options{DataDir: dataDir, Controller: ctl}) - if err == nil { - t.Fatal("Run: want failure for a corrupt backup, got nil") - } - if len(ctl.calls) != 0 { - t.Errorf("controller was called %v - the service must not be stopped when the backup can't be trusted", ctl.calls) - } - if got := readFile(t, filepath.Join(dataDir, "config")); got != "half-migrated settings" { - t.Errorf("data directory was modified (%q) despite the refusal", got) - } - if !report.Blocking() { - t.Error("report should be blocking") - } -} - -func TestRunResumesWithoutRedoingCompletedSteps(t *testing.T) { - store, rs, dataDir, _ := fsRun(t) - first := &fakeController{active: true} - if _, err := Run(context.Background(), store, rs, Options{DataDir: dataDir, Controller: first}); err != nil { - t.Fatal(err) - } - - // Re-invoking a completed rollback must be inert: stopping the service - // a second time, or re-restoring over a data directory that has been - // live since, would turn a no-op into an outage. - second := &fakeController{active: true} - reloaded, err := store.Load(rs.RunID) - if err != nil { - t.Fatal(err) - } - if _, err := Run(context.Background(), store, reloaded, Options{DataDir: dataDir, Controller: second}); err != nil { - t.Fatal(err) - } - if len(second.calls) != 0 { - t.Errorf("second invocation called the controller %v, want nothing - every step was already done", second.calls) - } -} - -func TestRunReinstallsThePreservedBinary(t *testing.T) { - store, rs, dataDir, _ := fsRun(t) - binDir := t.TempDir() - binaryPath := filepath.Join(binDir, "stalwart") - preserved := binaryPath + ".v0.15.5" - // Real scripts, not placeholder bytes: the verification step runs the - // restored binary's --version, so this also proves the instance really - // came back on the version the run started from. - if err := os.WriteFile(preserved, []byte("#!/bin/sh\necho 'stalwart 0.15.5'\n"), 0o755); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(binaryPath, []byte("#!/bin/sh\necho 'stalwart 0.16.14'\n"), 0o755); err != nil { - t.Fatal(err) - } - sum, _, err := hashFile(preserved) - if err != nil { - t.Fatal(err) - } - rs.RecordArtifact(ArtifactOldBinary, checkpoint.Artifact{Path: preserved, SHA256: sum}) - - report, err := Run(context.Background(), store, rs, Options{ - DataDir: dataDir, BinaryPath: binaryPath, Controller: &fakeController{active: true}, - }) - if err != nil { - t.Fatalf("Run: %v\n%s", err, report) - } - if got := readFile(t, binaryPath); !strings.Contains(got, "0.15.5") { - t.Errorf("binary = %q, want the preserved old one reinstalled", got) - } - if got := readFile(t, binaryPath+".failed-"+rs.RunID); !strings.Contains(got, "0.16.14") { - t.Errorf("displaced binary = %q, want the new one kept for a retry", got) - } -} - -func TestRunRestoresAPreservedServiceUnit(t *testing.T) { - store, rs, dataDir, _ := fsRun(t) - unitDir := t.TempDir() - unitPath := filepath.Join(unitDir, "stalwart.service") - preservedUnit := filepath.Join(unitDir, "stalwart.service.preserved") - if err := os.WriteFile(preservedUnit, []byte("ExecStart=/usr/local/bin/stalwart\n"), 0o644); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(unitPath, []byte("ExecStart=/usr/local/bin/stalwart-0.16\n"), 0o644); err != nil { - t.Fatal(err) - } - rs.RecordArtifact(ArtifactServiceUnit, checkpoint.Artifact{Path: preservedUnit}) - - ctl := &fakeController{active: true} - report, err := Run(context.Background(), store, rs, Options{ - DataDir: dataDir, ServiceUnitPath: unitPath, Controller: ctl, - }) - if err != nil { - t.Fatalf("Run: %v\n%s", err, report) - } - if got := readFile(t, unitPath); !strings.Contains(got, "/usr/local/bin/stalwart\n") { - t.Errorf("unit = %q, want the preserved definition restored", got) - } - if got, want := strings.Join(ctl.calls, ","), "stop,reload,start"; got != want { - t.Errorf("controller calls = %q, want %q - a restored unit has to be reloaded before the start", got, want) - } -} - -// Nothing writes a service-unit artifact yet (cutover, which would rewrite -// the unit in the first place, doesn't exist). That has to read as an -// explicit skip, not a silent success. -func TestRunSkipsServiceConfigRestoreWithAnExplanation(t *testing.T) { - store, rs, dataDir, _ := fsRun(t) - report, err := Run(context.Background(), store, rs, Options{DataDir: dataDir, Controller: &fakeController{active: true}}) - if err != nil { - t.Fatal(err) - } - var found bool - for _, res := range report.Results { - if res.Name == "restore-service-config" { - found = true - if res.Status != StatusSkipped { - t.Errorf("restore-service-config status = %q, want %q", res.Status, StatusSkipped) - } - if !strings.Contains(res.Detail, "cutover") { - t.Errorf("skip detail %q should say what would record one", res.Detail) - } - } - } - if !found { - t.Errorf("no restore-service-config result in report:\n%s", report) - } -} - -func TestRunFailsWhenTheServiceWontStop(t *testing.T) { - store, rs, dataDir, _ := fsRun(t) - ctl := &fakeController{active: true, stopErr: os.ErrPermission} - - report, err := Run(context.Background(), store, rs, Options{DataDir: dataDir, Controller: ctl}) - if err == nil { - t.Fatal("Run: want failure when the service can't be stopped, got nil") - } - if got := readFile(t, filepath.Join(dataDir, "config")); got != "half-migrated settings" { - t.Errorf("data directory was touched (%q) even though the service never stopped", got) - } - if !report.Blocking() { - t.Error("report should be blocking") - } -} - -func TestRunSurfacesVerificationFailures(t *testing.T) { - store, rs, dataDir, _ := fsRun(t) - binDir := withFakeExecutable(t, "stalwart", "#!/bin/sh\necho 'stalwart 0.16.14'\n") - - report, err := Run(context.Background(), store, rs, Options{ - DataDir: dataDir, BinaryPath: filepath.Join(binDir, "stalwart"), Controller: &fakeController{active: true}, - }) - if err == nil { - t.Fatal("Run: want failure when the restored instance isn't on the original version, got nil") - } - var sawVersionFailure bool - for _, res := range report.Results { - if res.Name == "version" && res.Status == StatusFail { - sawVersionFailure = true - if !strings.Contains(res.Detail, "0.15.5") { - t.Errorf("version failure %q should name the version it expected", res.Detail) - } - } - } - if !sawVersionFailure { - t.Errorf("individual verification results should be in the report, not collapsed into one line:\n%s", report) - } -} diff --git a/internal/rollback/verify.go b/internal/rollback/verify.go deleted file mode 100644 index c9c7992..0000000 --- a/internal/rollback/verify.go +++ /dev/null @@ -1,170 +0,0 @@ -package rollback - -import ( - "context" - "fmt" - "net/http" - "sort" - "strings" - "time" - - "github.com/LINUXexpert-org/stalwart-migrator/internal/checkpoint" - "github.com/LINUXexpert-org/stalwart-migrator/internal/preflight" - "github.com/LINUXexpert-org/stalwart-migrator/internal/stalwartapi" -) - -// VerifyOptions configures the reduced validation suite that runs against -// the *restored* instance - ARCHITECTURE.md §4.8 step 5. It's deliberately -// smaller than §4.7's post-migration suite: the question here is "is the -// old instance actually back", not "did a migration preserve everything", -// and every check has to be one that would still pass on a healthy 0.15.5 -// install. -type VerifyOptions struct { - BinaryPath string // checked with --version; skipped if empty - ExpectVersion string // the run's recorded source version - - AdminURL string - AdminUser string - AdminPassword string - HTTPClient *http.Client - - // Snapshot is the pre-migration snapshot preflight captured. When set, - // the restored instance's account count and domains are compared - // against it - the "directory counts" half of §4.8 step 5. Per-mailbox - // message counts are deliberately not re-checked here: the restore is a - // byte-for-byte copy already verified against its manifest, so a - // per-mailbox walk would cost a lot of time on a large install to - // re-answer a question the manifest verification already answered. - Snapshot *checkpoint.PreflightSnapshot - - // Timeout bounds how long the reachability check waits for the restored - // service to answer, since it was started moments earlier. - Timeout time.Duration -} - -// Verify runs the reduced suite and returns one CheckResult per check. -// It always returns every result, even after a failure, so an operator sees -// the whole picture of a bad rollback in one pass rather than one problem -// at a time. The error is non-nil if any check failed. -func Verify(ctx context.Context, o VerifyOptions) ([]CheckResult, error) { - var results []CheckResult - fail := func(name, format string, args ...any) { - results = append(results, CheckResult{Name: name, Status: StatusFail, Detail: fmt.Sprintf(format, args...)}) - } - ok := func(name, format string, args ...any) { - results = append(results, CheckResult{Name: name, Status: StatusOK, Detail: fmt.Sprintf(format, args...)}) - } - skip := func(name, detail string) { - results = append(results, CheckResult{Name: name, Status: StatusSkipped, Detail: detail}) - } - - switch { - case o.BinaryPath == "" || o.ExpectVersion == "": - skip("version", "no binary path or recorded source version to check against") - default: - got, err := preflight.DetectVersion(ctx, o.BinaryPath) - switch { - case err != nil: - fail("version", "couldn't read the restored binary's version: %v", err) - case got != o.ExpectVersion: - fail("version", "restored binary reports %s, but this run started from %s - the rollback did not put the original binary back", got, o.ExpectVersion) - default: - ok("version", "restored binary reports %s, matching the version this run started from", got) - } - } - - if o.AdminURL == "" { - skip("reachable", "no --admin-url configured - can't confirm the restored instance answers") - skip("directory-counts", "no --admin-url configured - can't compare the restored directory against the pre-migration snapshot") - return results, resultsError(results) - } - - client := &stalwartapi.Client{ - BaseURL: o.AdminURL, Username: o.AdminUser, Password: o.AdminPassword, HTTPClient: o.HTTPClient, - } - timeout := o.Timeout - if timeout <= 0 { - timeout = 60 * time.Second - } - if err := waitForPing(ctx, client, timeout); err != nil { - fail("reachable", "restored instance never answered at %s within %s: %v", o.AdminURL, timeout, err) - skip("directory-counts", "skipped because the restored instance isn't reachable") - return results, resultsError(results) - } - ok("reachable", "restored instance answered a JMAP session request at %s", o.AdminURL) - - if o.Snapshot == nil { - skip("directory-counts", "this run has no pre-migration snapshot to compare against") - return results, resultsError(results) - } - - snap, err := client.AccountSnapshot(ctx) - if err != nil { - fail("directory-counts", "couldn't read the restored instance's directory: %v", err) - return results, resultsError(results) - } - if problems := compareDirectory(o.Snapshot, snap); len(problems) > 0 { - fail("directory-counts", "restored directory doesn't match the pre-migration snapshot: %s", strings.Join(problems, "; ")) - } else { - ok("directory-counts", "restored instance has %d account(s) and %d domain(s), matching the pre-migration snapshot", - snap.AccountCount, len(snap.Domains)) - } - return results, resultsError(results) -} - -// compareDirectory reports every way the restored directory differs from -// the pre-migration snapshot. Unlike the post-migration comparison in -// internal/validate, account names are compared exactly: the v0.16 -// migration's bare-username-to-email rewrite is precisely what a rollback -// undoes, so a restored instance that still shows rewritten names has not -// been restored. -func compareDirectory(before *checkpoint.PreflightSnapshot, after *stalwartapi.Snapshot) []string { - var problems []string - if before.AccountCount != after.AccountCount { - problems = append(problems, fmt.Sprintf("%d account(s) before, %d after", before.AccountCount, after.AccountCount)) - } - beforeDomains := append([]string(nil), before.Domains...) - afterDomains := append([]string(nil), after.Domains...) - sort.Strings(beforeDomains) - sort.Strings(afterDomains) - if strings.Join(beforeDomains, ",") != strings.Join(afterDomains, ",") { - problems = append(problems, fmt.Sprintf("domains were [%s], now [%s]", - strings.Join(beforeDomains, " "), strings.Join(afterDomains, " "))) - } - return problems -} - -// waitForPing polls until the instance accepts an authenticated session -// request or timeout elapses. The service was started seconds ago, so the -// first attempt failing is expected rather than meaningful. -func waitForPing(ctx context.Context, client *stalwartapi.Client, timeout time.Duration) error { - deadline := time.Now().Add(timeout) - var lastErr error - for { - lastErr = client.Ping(ctx) - if lastErr == nil { - return nil - } - if !time.Now().Before(deadline) { - return lastErr - } - select { - case <-ctx.Done(): - return ctx.Err() - case <-time.After(500 * time.Millisecond): - } - } -} - -func resultsError(results []CheckResult) error { - var failed []string - for _, r := range results { - if r.Status == StatusFail { - failed = append(failed, r.Name) - } - } - if len(failed) == 0 { - return nil - } - return fmt.Errorf("rollback verification failed: %s", strings.Join(failed, ", ")) -} diff --git a/internal/rollback/verify_test.go b/internal/rollback/verify_test.go deleted file mode 100644 index 2f2f29f..0000000 --- a/internal/rollback/verify_test.go +++ /dev/null @@ -1,166 +0,0 @@ -package rollback - -import ( - "context" - "encoding/json" - "net/http" - "net/http/httptest" - "path/filepath" - "strings" - "testing" - "time" - - "github.com/LINUXexpert-org/stalwart-migrator/internal/checkpoint" -) - -// restoredInstance is a fake Stalwart answering the two things the reduced -// post-rollback suite asks of it: a JMAP session document (reachability) -// and the x:Account/* management calls behind the directory comparison. -// Per-account mailbox impersonation is refused, which is deliberate - the -// reduced suite must not depend on it, since a rollback's guarantee comes -// from the verified manifest, not from re-walking every mailbox. -func restoredInstance(t *testing.T, accounts []map[string]any) *httptest.Server { - t.Helper() - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.Method == http.MethodGet && r.URL.Path == "/.well-known/jmap" { - if user, _, _ := r.BasicAuth(); strings.Contains(user, "%") { - w.WriteHeader(http.StatusForbidden) // no impersonate grant - return - } - json.NewEncoder(w).Encode(map[string]any{"apiUrl": "/api"}) - return - } - var body map[string]any - json.NewDecoder(r.Body).Decode(&body) - calls := body["methodCalls"].([]any) - name := calls[0].([]any)[0].(string) - - switch name { - case "x:Account/query": - ids := make([]string, len(accounts)) - for i, a := range accounts { - ids[i] = a["id"].(string) - } - json.NewEncoder(w).Encode(map[string]any{"methodResponses": []any{ - []any{"x:Account/query", map[string]any{"ids": ids}, "q"}, - }}) - case "x:Account/get": - json.NewEncoder(w).Encode(map[string]any{"methodResponses": []any{ - []any{"x:Account/get", map[string]any{"list": accounts}, "g"}, - }}) - default: - t.Errorf("unexpected method call %s - the reduced suite should not be making it", name) - } - })) - t.Cleanup(srv.Close) - return srv -} - -func resultFor(t *testing.T, results []CheckResult, name string) CheckResult { - t.Helper() - for _, r := range results { - if r.Name == name { - return r - } - } - t.Fatalf("no %q result in %+v", name, results) - return CheckResult{} -} - -func TestVerifyPassesOnAProperlyRestoredInstance(t *testing.T) { - srv := restoredInstance(t, []map[string]any{ - {"id": "a1", "name": "alice@example.com", "domainId": "example.com"}, - {"id": "a2", "name": "bob@example.com", "domainId": "example.com"}, - }) - binDir := withFakeExecutable(t, "stalwart", "#!/bin/sh\necho 'stalwart 0.15.5'\n") - - results, err := Verify(context.Background(), VerifyOptions{ - BinaryPath: filepath.Join(binDir, "stalwart"), ExpectVersion: "0.15.5", - AdminURL: srv.URL, AdminUser: "admin", AdminPassword: "hunter2", - Snapshot: &checkpoint.PreflightSnapshot{AccountCount: 2, Domains: []string{"example.com"}}, - }) - if err != nil { - t.Fatalf("Verify: %v (%+v)", err, results) - } - for _, name := range []string{"version", "reachable", "directory-counts"} { - if got := resultFor(t, results, name); got.Status != StatusOK { - t.Errorf("%s = %s: %s", name, got.Status, got.Detail) - } - } -} - -func TestVerifyDetectsTheWrongVersionStillInstalled(t *testing.T) { - binDir := withFakeExecutable(t, "stalwart", "#!/bin/sh\necho 'stalwart 0.16.14'\n") - - results, err := Verify(context.Background(), VerifyOptions{ - BinaryPath: filepath.Join(binDir, "stalwart"), ExpectVersion: "0.15.5", - }) - if err == nil { - t.Fatal("Verify: want error when the new binary is still installed, got nil") - } - res := resultFor(t, results, "version") - if res.Status != StatusFail { - t.Errorf("version = %s, want fail", res.Status) - } - if !strings.Contains(res.Detail, "did not put the original binary back") { - t.Errorf("detail %q should say plainly what went wrong", res.Detail) - } -} - -func TestVerifyDetectsADirectoryThatDoesNotMatchTheSnapshot(t *testing.T) { - srv := restoredInstance(t, []map[string]any{ - {"id": "a1", "name": "alice@example.com", "domainId": "example.com"}, - }) - - results, err := Verify(context.Background(), VerifyOptions{ - AdminURL: srv.URL, AdminUser: "admin", AdminPassword: "hunter2", - Snapshot: &checkpoint.PreflightSnapshot{AccountCount: 2, Domains: []string{"example.com", "example.org"}}, - }) - if err == nil { - t.Fatal("Verify: want error when the restored directory is smaller than the snapshot, got nil") - } - res := resultFor(t, results, "directory-counts") - if res.Status != StatusFail { - t.Fatalf("directory-counts = %s, want fail", res.Status) - } - for _, want := range []string{"2 account(s) before, 1 after", "example.org"} { - if !strings.Contains(res.Detail, want) { - t.Errorf("detail %q should include %q", res.Detail, want) - } - } -} - -func TestVerifyReportsAnUnreachableInstance(t *testing.T) { - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(http.StatusServiceUnavailable) - })) - defer srv.Close() - - results, err := Verify(context.Background(), VerifyOptions{ - AdminURL: srv.URL, AdminUser: "admin", Timeout: 300 * time.Millisecond, - Snapshot: &checkpoint.PreflightSnapshot{AccountCount: 1}, - }) - if err == nil { - t.Fatal("Verify: want error when the restored instance never answers, got nil") - } - if got := resultFor(t, results, "reachable"); got.Status != StatusFail { - t.Errorf("reachable = %s, want fail", got.Status) - } - // Reporting "directory matches" against an instance that never answered - // would be worse than reporting nothing. - if got := resultFor(t, results, "directory-counts"); got.Status != StatusSkipped { - t.Errorf("directory-counts = %s, want skip when the instance is unreachable", got.Status) - } -} - -func TestVerifySkipsRatherThanInventsWhatItCannotCheck(t *testing.T) { - results, err := Verify(context.Background(), VerifyOptions{}) - if err != nil { - t.Fatalf("Verify with nothing to check should not fail: %v", err) - } - for _, name := range []string{"version", "reachable", "directory-counts"} { - if got := resultFor(t, results, name); got.Status != StatusSkipped { - t.Errorf("%s = %s, want skip", name, got.Status) - } - } -} diff --git a/internal/service/service.go b/internal/service/service.go index c344391..4d7bf41 100644 --- a/internal/service/service.go +++ b/internal/service/service.go @@ -11,8 +11,8 @@ import ( // Kind is how a Stalwart instance is run, and therefore how it has to be // stopped and started. Preflight detects it (preflight.DetectDeploymentKind // is an alias for this type's detector-side constants) and records it in -// the checkpoint's Topology, so a rollback days later controls the same -// thing the original run observed rather than re-guessing. +// the checkpoint's Topology, so a phase running later controls the same +// thing preflight observed rather than re-guessing. type Kind string const ( @@ -57,9 +57,9 @@ type Controller interface { // New returns a Controller for the given deployment. It refuses an Unknown // (or unrecognized) kind rather than guessing: picking the wrong mechanism -// here means a rollback that reports "service stopped" while the old -// instance is still running and holding the data directory open, which is -// exactly the kind of quiet wrongness this tool exists to avoid. +// here means a phase that reports "service stopped" while the instance is +// still running and holding the data directory open, which is exactly the +// kind of quiet wrongness this tool exists to avoid. func New(o Options) (Controller, error) { switch o.Kind { case Systemd: diff --git a/internal/service/service_test.go b/internal/service/service_test.go index c59e675..0a3cf11 100644 --- a/internal/service/service_test.go +++ b/internal/service/service_test.go @@ -76,7 +76,8 @@ func TestSystemdStopReportsCommandFailure(t *testing.T) { // systemctl exits non-zero for every non-active state, so Active has to // read its output rather than its exit status - otherwise "inactive", the -// answer a rollback most needs, would look like a failure to read state. +// answer a caller waiting for a clean stop most needs, would look like a +// failure to read the state at all. func TestSystemdActiveReadsOutputNotExitStatus(t *testing.T) { for _, tc := range []struct { state string diff --git a/internal/stalwartapi/management.go b/internal/stalwartapi/management.go index 594bec1..c8be687 100644 --- a/internal/stalwartapi/management.go +++ b/internal/stalwartapi/management.go @@ -121,13 +121,7 @@ type account struct { // exactly what's missing rather than silently treating an unreachable // account's mailboxes as having zero messages. func (c *Client) AccountSnapshot(ctx context.Context) (*Snapshot, error) { - queryResp, err := c.call(ctx, managementCapabilities, []any{ - []any{"x:Account/query", map[string]any{"filter": map[string]any{}}, "q"}, - }) - if err != nil { - return nil, fmt.Errorf("stalwartapi: Account/query: %w", err) - } - ids, err := accountQueryIDs(queryResp) + ids, err := c.AccountIDs(ctx) if err != nil { return nil, err } diff --git a/internal/stalwartapi/task.go b/internal/stalwartapi/task.go new file mode 100644 index 0000000..bf5f5c6 --- /dev/null +++ b/internal/stalwartapi/task.go @@ -0,0 +1,286 @@ +package stalwartapi + +import ( + "context" + "encoding/json" + "fmt" + "sort" + "strings" + "time" +) + +// Quota recalculation is the last step of ARCHITECTURE.md §4.5, and the one +// post-migration task Stalwart's own v0.16 upgrade guide is emphatic about: +// "Disk quotas were reset to zero during the wipe and need to be rebuilt +// from the actual mailbox contents." +// +// The guide only documents doing this through the WebUI's Tasks panel, so +// the wire format below comes from Stalwart's schema reference for the +// x:Task object (docs/ref/object/task/) rather than from the upgrade guide: +// x:Task/set with an "AccountMaintenance" variant whose maintenanceType is +// "recalculateQuota", one per account, exactly as the WebUI's own +// "Recalculate disk quotas" fans out into one subtask per user account. +// +// Two things about this remain unverified against a running server, and +// callers are expected to treat a failure here as non-fatal for that +// reason: +// +// - The schema reference annotates accountId and maintenanceType as +// read-only. Read-only in that reference means immutable after +// creation (they have to be settable at create time or the variant +// couldn't be created at all), but that reading hasn't been confirmed +// against a live instance. +// - TaskStatus documents Pending, Retry and Failed, with no state for +// "finished successfully" - so completion is inferred from a task +// disappearing from the queue, which is the natural reading of a work +// queue whose entries are consumed, not a documented guarantee. +const ( + taskTypeAccountMaintenance = "AccountMaintenance" + taskTypeTenantMaintenance = "TenantMaintenance" + maintenanceRecalculateQuota = "recalculateQuota" +) + +// TaskFailure is one task that reached the terminal Failed state. +type TaskFailure struct { + TaskID string + Reason string +} + +func (f TaskFailure) String() string { + if f.Reason == "" { + return f.TaskID + } + return f.TaskID + ": " + f.Reason +} + +// AccountIDs returns every account's id, without the per-mailbox walk +// AccountSnapshot does. Quota recalculation needs the ids and nothing else, +// and on a large install the mailbox walk is the expensive part. +func (c *Client) AccountIDs(ctx context.Context) ([]string, error) { + responses, err := c.call(ctx, managementCapabilities, []any{ + []any{"x:Account/query", map[string]any{"filter": map[string]any{}}, "q"}, + }) + if err != nil { + return nil, fmt.Errorf("stalwartapi: Account/query: %w", err) + } + return accountQueryIDs(responses) +} + +// CreateQuotaRecalculationTasks schedules one recalculateQuota maintenance +// task per account id, in a single x:Task/set call, and returns the ids of +// the tasks the server said it created. +// +// A creation the server rejects is returned as an error naming the account +// and the server's own reason, rather than being counted as scheduled - +// silently reporting "quotas recalculated" for an account whose task was +// never accepted is precisely the sort of thing this tool exists not to do. +func (c *Client) CreateQuotaRecalculationTasks(ctx context.Context, accountIDs []string) ([]string, error) { + if len(accountIDs) == 0 { + return nil, nil + } + create := make(map[string]any, len(accountIDs)) + creationIDForAccount := make(map[string]string, len(accountIDs)) + for i, accountID := range accountIDs { + creationID := fmt.Sprintf("q%d", i) + creationIDForAccount[creationID] = accountID + create[creationID] = map[string]any{ + "@type": taskTypeAccountMaintenance, + "accountId": accountID, + "maintenanceType": maintenanceRecalculateQuota, + "status": map[string]any{"@type": "Pending"}, + } + } + return c.setTasks(ctx, create, creationIDForAccount, "account") +} + +// CreateTenantQuotaRecalculationTasks does the same for tenant-level +// counters. Stalwart's upgrade guide is explicit that this runs *after* +// per-account recalculation has finished for every user, since it +// aggregates those per-account totals - so callers must wait on +// CreateQuotaRecalculationTasks before calling this, not run both at once. +func (c *Client) CreateTenantQuotaRecalculationTasks(ctx context.Context, tenantIDs []string) ([]string, error) { + if len(tenantIDs) == 0 { + return nil, nil + } + create := make(map[string]any, len(tenantIDs)) + creationIDForTenant := make(map[string]string, len(tenantIDs)) + for i, tenantID := range tenantIDs { + creationID := fmt.Sprintf("t%d", i) + creationIDForTenant[creationID] = tenantID + create[creationID] = map[string]any{ + "@type": taskTypeTenantMaintenance, + "tenantId": tenantID, + "maintenanceType": maintenanceRecalculateQuota, + "status": map[string]any{"@type": "Pending"}, + } + } + return c.setTasks(ctx, create, creationIDForTenant, "tenant") +} + +func (c *Client) setTasks(ctx context.Context, create map[string]any, subjectFor map[string]string, subjectKind string) ([]string, error) { + responses, err := c.call(ctx, managementCapabilities, []any{ + []any{"x:Task/set", map[string]any{"create": create}, "s"}, + }) + if err != nil { + return nil, fmt.Errorf("stalwartapi: Task/set: %w", err) + } + if len(responses) == 0 { + return nil, fmt.Errorf("stalwartapi: Task/set returned no method responses") + } + r := responses[0] + if r.Name == "error" { + return nil, fmt.Errorf("stalwartapi: Task/set error: %s", r.Args) + } + var result struct { + Created map[string]struct { + ID string `json:"id"` + } `json:"created"` + NotCreated map[string]json.RawMessage `json:"notCreated"` + } + if err := json.Unmarshal(r.Args, &result); err != nil { + return nil, fmt.Errorf("stalwartapi: parse Task/set response: %w", err) + } + + if len(result.NotCreated) > 0 { + rejected := make([]string, 0, len(result.NotCreated)) + for creationID, reason := range result.NotCreated { + rejected = append(rejected, fmt.Sprintf("%s %s: %s", subjectKind, subjectFor[creationID], reason)) + } + sort.Strings(rejected) + return nil, fmt.Errorf("stalwartapi: Task/set refused %d of %d quota recalculation task(s): %s", + len(result.NotCreated), len(create), strings.Join(rejected, "; ")) + } + + ids := make([]string, 0, len(result.Created)) + for _, created := range result.Created { + ids = append(ids, created.ID) + } + sort.Strings(ids) + if len(ids) != len(create) { + return ids, fmt.Errorf("stalwartapi: Task/set created %d task(s) but %d were requested, and none were reported as refused", + len(ids), len(create)) + } + return ids, nil +} + +// WaitForTasks polls x:Task/get until none of the given tasks are still in +// the queue, or timeout elapses. A task that has left the queue is treated +// as finished (see this file's opening comment on why that inference is +// necessary); one still present in the terminal Failed state is collected +// and reported rather than waited on forever. +// +// It returns the failures it observed. A non-nil error means the polling +// itself couldn't be completed - the queue couldn't be read, or the timeout +// elapsed while tasks were still pending - which is a different thing from +// "the tasks ran and some failed", and callers report them differently. +func (c *Client) WaitForTasks(ctx context.Context, taskIDs []string, timeout time.Duration) (failures []TaskFailure, err error) { + if len(taskIDs) == 0 { + return nil, nil + } + remaining := make(map[string]bool, len(taskIDs)) + for _, id := range taskIDs { + remaining[id] = true + } + seenFailure := map[string]bool{} + + deadline := time.Now().Add(timeout) + for { + pending := make([]string, 0, len(remaining)) + for id := range remaining { + pending = append(pending, id) + } + sort.Strings(pending) + + found, err := c.taskStatuses(ctx, pending) + if err != nil { + return failures, err + } + for id := range remaining { + status, stillQueued := found[id] + if !stillQueued { + delete(remaining, id) // consumed by the queue: finished + continue + } + if status.Type == "Failed" && !seenFailure[id] { + seenFailure[id] = true + failures = append(failures, TaskFailure{TaskID: id, Reason: status.FailureReason}) + delete(remaining, id) + } + } + if len(remaining) == 0 { + sort.Slice(failures, func(i, j int) bool { return failures[i].TaskID < failures[j].TaskID }) + return failures, nil + } + if !time.Now().Before(deadline) { + return failures, fmt.Errorf("stalwartapi: %d quota recalculation task(s) were still queued after %s - they may simply need longer on a large install; check the Tasks panel rather than assuming they failed", + len(remaining), timeout) + } + select { + case <-ctx.Done(): + return failures, ctx.Err() + case <-time.After(2 * time.Second): + } + } +} + +type taskStatus struct { + Type string + FailureReason string +} + +// taskStatuses fetches the given tasks, returning only those the server +// still knows about, keyed by id. +func (c *Client) taskStatuses(ctx context.Context, ids []string) (map[string]taskStatus, error) { + responses, err := c.call(ctx, managementCapabilities, []any{ + []any{"x:Task/get", map[string]any{"ids": ids, "properties": []string{"id", "status"}}, "g"}, + }) + if err != nil { + return nil, fmt.Errorf("stalwartapi: Task/get: %w", err) + } + if len(responses) == 0 { + return nil, fmt.Errorf("stalwartapi: Task/get returned no method responses") + } + r := responses[0] + if r.Name == "error" { + return nil, fmt.Errorf("stalwartapi: Task/get error: %s", r.Args) + } + var result struct { + List []struct { + ID string `json:"id"` + Status struct { + Type string `json:"@type"` + FailureReason string `json:"failureReason"` + } `json:"status"` + } `json:"list"` + } + if err := json.Unmarshal(r.Args, &result); err != nil { + return nil, fmt.Errorf("stalwartapi: parse Task/get response: %w", err) + } + statuses := make(map[string]taskStatus, len(result.List)) + for _, t := range result.List { + statuses[t.ID] = taskStatus{Type: t.Status.Type, FailureReason: t.Status.FailureReason} + } + return statuses, nil +} + +// WaitForPing polls until the instance accepts an authenticated session +// request or timeout elapses. Cutover uses it after starting the migrated +// service, which came up seconds earlier - so early failures are expected +// rather than meaningful. +func (c *Client) WaitForPing(ctx context.Context, timeout time.Duration) error { + deadline := time.Now().Add(timeout) + for { + err := c.Ping(ctx) + if err == nil { + return nil + } + if !time.Now().Before(deadline) { + return err + } + select { + case <-ctx.Done(): + return ctx.Err() + case <-time.After(500 * time.Millisecond): + } + } +} diff --git a/internal/stalwartapi/task_test.go b/internal/stalwartapi/task_test.go new file mode 100644 index 0000000..db2a6c4 --- /dev/null +++ b/internal/stalwartapi/task_test.go @@ -0,0 +1,280 @@ +package stalwartapi + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "sync" + "testing" + "time" +) + +// taskServer answers x:Task/set and x:Task/get. queue models Stalwart's own +// task queue: a task that has run is *removed* from it, which is how +// completion is detected (see task.go's opening comment). +type taskServer struct { + mu sync.Mutex + queue map[string]string // task id -> status @type + created []map[string]any // the create objects received, in request order + notFound bool // reject every creation +} + +func newTaskServer(t *testing.T) (*taskServer, *httptest.Server) { + t.Helper() + ts := &taskServer{queue: map[string]string{}} + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var body map[string]any + json.NewDecoder(r.Body).Decode(&body) + call := body["methodCalls"].([]any)[0].([]any) + name := call[0].(string) + args := call[1].(map[string]any) + + ts.mu.Lock() + defer ts.mu.Unlock() + + switch name { + case "x:Task/set": + create := args["create"].(map[string]any) + created := map[string]any{} + notCreated := map[string]any{} + for creationID, obj := range create { + ts.created = append(ts.created, obj.(map[string]any)) + if ts.notFound { + notCreated[creationID] = map[string]any{"type": "forbidden"} + continue + } + id := "task-" + creationID + ts.queue[id] = "Pending" + created[creationID] = map[string]any{"id": id} + } + json.NewEncoder(w).Encode(map[string]any{"methodResponses": []any{ + []any{"x:Task/set", map[string]any{"created": created, "notCreated": notCreated}, "s"}, + }}) + case "x:Task/get": + var list []any + for _, raw := range args["ids"].([]any) { + id := raw.(string) + status, stillQueued := ts.queue[id] + if !stillQueued { + continue // consumed: finished + } + entry := map[string]any{"id": id, "status": map[string]any{"@type": status}} + if status == "Failed" { + entry["status"].(map[string]any)["failureReason"] = "store unavailable" + } + list = append(list, entry) + } + json.NewEncoder(w).Encode(map[string]any{"methodResponses": []any{ + []any{"x:Task/get", map[string]any{"list": list}, "g"}, + }}) + case "x:Account/query": + json.NewEncoder(w).Encode(map[string]any{"methodResponses": []any{ + []any{"x:Account/query", map[string]any{"ids": []string{"a1", "a2", "a3"}}, "q"}, + }}) + default: + t.Errorf("unexpected method call %s", name) + } + })) + t.Cleanup(srv.Close) + return ts, srv +} + +func (ts *taskServer) finish(id string) { + ts.mu.Lock() + defer ts.mu.Unlock() + delete(ts.queue, id) +} + +func (ts *taskServer) fail(id string) { + ts.mu.Lock() + defer ts.mu.Unlock() + ts.queue[id] = "Failed" +} + +// The wire shape here comes from Stalwart's x:Task schema reference, so the +// test pins it: an AccountMaintenance variant with maintenanceType +// recalculateQuota, one per account, in a single Task/set call. +func TestCreateQuotaRecalculationTasksSendsOnePerAccount(t *testing.T) { + ts, srv := newTaskServer(t) + client := &Client{BaseURL: srv.URL, Username: "admin", Password: "hunter2"} + + ids, err := client.CreateQuotaRecalculationTasks(context.Background(), []string{"a1", "a2"}) + if err != nil { + t.Fatalf("CreateQuotaRecalculationTasks: %v", err) + } + if len(ids) != 2 { + t.Fatalf("created %d task ids, want 2: %v", len(ids), ids) + } + if len(ts.created) != 2 { + t.Fatalf("server received %d creations, want 2", len(ts.created)) + } + seen := map[string]bool{} + for _, obj := range ts.created { + if obj["@type"] != "AccountMaintenance" { + t.Errorf("@type = %v, want AccountMaintenance", obj["@type"]) + } + if obj["maintenanceType"] != "recalculateQuota" { + t.Errorf("maintenanceType = %v, want recalculateQuota", obj["maintenanceType"]) + } + status := obj["status"].(map[string]any) + if status["@type"] != "Pending" { + t.Errorf("status.@type = %v, want Pending", status["@type"]) + } + seen[obj["accountId"].(string)] = true + } + if !seen["a1"] || !seen["a2"] { + t.Errorf("accountIds sent = %v, want a1 and a2", seen) + } +} + +func TestCreateTenantQuotaRecalculationTasksUsesTenantVariant(t *testing.T) { + ts, srv := newTaskServer(t) + client := &Client{BaseURL: srv.URL, Username: "admin", Password: "hunter2"} + + if _, err := client.CreateTenantQuotaRecalculationTasks(context.Background(), []string{"t1"}); err != nil { + t.Fatal(err) + } + if got := ts.created[0]["@type"]; got != "TenantMaintenance" { + t.Errorf("@type = %v, want TenantMaintenance", got) + } + if got := ts.created[0]["tenantId"]; got != "t1" { + t.Errorf("tenantId = %v, want t1", got) + } +} + +// Reporting "quotas recalculated" for an account whose task the server +// refused would be exactly the silent partial success this tool exists to +// catch. +func TestCreateQuotaRecalculationTasksFailsOnRefusedCreations(t *testing.T) { + ts, srv := newTaskServer(t) + ts.notFound = true + client := &Client{BaseURL: srv.URL, Username: "admin", Password: "hunter2"} + + _, err := client.CreateQuotaRecalculationTasks(context.Background(), []string{"a1"}) + if err == nil { + t.Fatal("want error when the server refuses a creation, got nil") + } + if !strings.Contains(err.Error(), "account a1") { + t.Errorf("error %q should name the account whose task was refused", err) + } +} + +func TestCreateQuotaRecalculationTasksIsANoOpForNoAccounts(t *testing.T) { + _, srv := newTaskServer(t) + client := &Client{BaseURL: srv.URL, Username: "admin"} + ids, err := client.CreateQuotaRecalculationTasks(context.Background(), nil) + if err != nil || len(ids) != 0 { + t.Errorf("want no ids and no error for an empty account list, got %v, %v", ids, err) + } +} + +func TestWaitForTasksReturnsOnceTheQueueDrains(t *testing.T) { + ts, srv := newTaskServer(t) + client := &Client{BaseURL: srv.URL, Username: "admin", Password: "hunter2"} + ids, err := client.CreateQuotaRecalculationTasks(context.Background(), []string{"a1", "a2"}) + if err != nil { + t.Fatal(err) + } + + go func() { + time.Sleep(100 * time.Millisecond) + for _, id := range ids { + ts.finish(id) + } + }() + + failures, err := client.WaitForTasks(context.Background(), ids, 10*time.Second) + if err != nil { + t.Fatalf("WaitForTasks: %v", err) + } + if len(failures) != 0 { + t.Errorf("failures = %v, want none", failures) + } +} + +func TestWaitForTasksCollectsFailedTasksInsteadOfWaitingForever(t *testing.T) { + ts, srv := newTaskServer(t) + client := &Client{BaseURL: srv.URL, Username: "admin", Password: "hunter2"} + ids, err := client.CreateQuotaRecalculationTasks(context.Background(), []string{"a1", "a2"}) + if err != nil { + t.Fatal(err) + } + ts.fail(ids[0]) + ts.finish(ids[1]) + + failures, err := client.WaitForTasks(context.Background(), ids, 10*time.Second) + if err != nil { + t.Fatalf("a task reaching Failed is a result, not a polling error: %v", err) + } + if len(failures) != 1 || failures[0].TaskID != ids[0] { + t.Fatalf("failures = %v, want just %s", failures, ids[0]) + } + if !strings.Contains(failures[0].Reason, "store unavailable") { + t.Errorf("failure reason = %q, want the server's own reason", failures[0].Reason) + } +} + +// "Still running after the timeout" and "ran and failed" are different +// answers for an operator - one means wait longer, the other means +// something is wrong. +func TestWaitForTasksDistinguishesATimeoutFromAFailure(t *testing.T) { + _, srv := newTaskServer(t) + client := &Client{BaseURL: srv.URL, Username: "admin", Password: "hunter2"} + ids, err := client.CreateQuotaRecalculationTasks(context.Background(), []string{"a1"}) + if err != nil { + t.Fatal(err) + } + + failures, err := client.WaitForTasks(context.Background(), ids, 100*time.Millisecond) + if err == nil { + t.Fatal("want an error when tasks are still queued at the timeout, got nil") + } + if len(failures) != 0 { + t.Errorf("failures = %v, want none - a still-queued task hasn't failed", failures) + } + if !strings.Contains(err.Error(), "still queued") { + t.Errorf("error %q should say the tasks were still queued, not that they failed", err) + } +} + +func TestAccountIDsSkipsTheMailboxWalk(t *testing.T) { + _, srv := newTaskServer(t) + client := &Client{BaseURL: srv.URL, Username: "admin", Password: "hunter2"} + + ids, err := client.AccountIDs(context.Background()) + if err != nil { + t.Fatal(err) + } + if len(ids) != 3 { + t.Errorf("AccountIDs = %v, want 3 ids", ids) + } +} + +func TestWaitForPingReturnsOnceTheInstanceAnswers(t *testing.T) { + var mu sync.Mutex + up := false + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + mu.Lock() + defer mu.Unlock() + if !up { + w.WriteHeader(http.StatusServiceUnavailable) + return + } + json.NewEncoder(w).Encode(map[string]any{"apiUrl": "/api"}) + })) + defer srv.Close() + + go func() { + time.Sleep(100 * time.Millisecond) + mu.Lock() + up = true + mu.Unlock() + }() + + client := &Client{BaseURL: srv.URL, Username: "admin", Password: "hunter2"} + if err := client.WaitForPing(context.Background(), 10*time.Second); err != nil { + t.Fatalf("WaitForPing: %v", err) + } +}