f7c9894b789deb323c445ed102d2c92f885d8183
29 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
a0fc91a700 |
Integration tests: grep the whole occ output instead of grep -q, which stopped reading and failed the pipe under pipefail
CI / unit (8.2) (push) Has been cancelled
CI / unit (8.3) (push) Has been cancelled
CI / unit (8.4) (push) Has been cancelled
CI / psalm (push) Has been cancelled
CI / frontend (push) Has been cancelled
CI / integration (mariadb, 32) (push) Has been cancelled
CI / integration (mariadb, 33) (push) Has been cancelled
CI / integration (mariadb, 34) (push) Has been cancelled
CI / integration (mariadb, 35) (push) Has been cancelled
CI / integration (pgsql, 32) (push) Has been cancelled
CI / integration (pgsql, 33) (push) Has been cancelled
CI / integration (pgsql, 34) (push) Has been cancelled
CI / integration (pgsql, 35) (push) Has been cancelled
CI / integration (sqlite, 32) (push) Has been cancelled
CI / integration (sqlite, 33) (push) Has been cancelled
CI / integration (sqlite, 34) (push) Has been cancelled
CI / integration (sqlite, 35) (push) Has been cancelled
CI / integration-targets (push) Has been cancelled
CI / integration-user (32) (push) Has been cancelled
CI / integration-user (33) (push) Has been cancelled
CI / integration-user (34) (push) Has been cancelled
CI / integration-user (35) (push) Has been cancelled
CI / integration-full-restore (push) Has been cancelled
|
||
|
|
a11e6c7254 |
1.0.1: label for "Back up now", retention explained in the trash, occ times in the schedule's zone, single-file restore to a directory, leftovers of a stopped restore removed
CI / unit (8.2) (push) Has been cancelled
CI / unit (8.3) (push) Has been cancelled
CI / unit (8.4) (push) Has been cancelled
CI / psalm (push) Has been cancelled
CI / frontend (push) Has been cancelled
CI / integration (mariadb, 32) (push) Has been cancelled
CI / integration (mariadb, 33) (push) Has been cancelled
CI / integration (mariadb, 34) (push) Has been cancelled
CI / integration (mariadb, 35) (push) Has been cancelled
CI / integration (pgsql, 32) (push) Has been cancelled
CI / integration (pgsql, 33) (push) Has been cancelled
CI / integration (pgsql, 34) (push) Has been cancelled
CI / integration (pgsql, 35) (push) Has been cancelled
CI / integration (sqlite, 32) (push) Has been cancelled
CI / integration (sqlite, 33) (push) Has been cancelled
CI / integration (sqlite, 34) (push) Has been cancelled
CI / integration (sqlite, 35) (push) Has been cancelled
CI / integration-targets (push) Has been cancelled
CI / integration-user (32) (push) Has been cancelled
CI / integration-user (33) (push) Has been cancelled
CI / integration-user (34) (push) Has been cancelled
CI / integration-user (35) (push) Has been cancelled
CI / integration-full-restore (push) Has been cancelled
- Admin page: an optional label (at most 100 characters) for a backup started with "Back up now"; an (i) by "removed by retention policy" in the trash that explains what it means and until when the restore point can be brought back. backup:trash says the same below its list. - backup:status, backup:list, backup:trash and backup:browse show times in the zone of the daily schedule and name it, instead of the server's UTC. - backup:restore --to-directory of a file restored nothing: the path got a "/" appended. - A restore removes "<name>.ngb-restore-<hex>" temporaries that a stopped restore left in the folders it writes to. - Roadmap: the cron warning (#13), reading ahead from S3/SMB and --salvage move to 1.0.2. |
||
|
|
3eec2e9554 |
Full restore on SQLite: take the write lock first, so another writer is waited for instead of "database is locked"
CI / unit (8.2) (push) Has been cancelled
CI / unit (8.3) (push) Has been cancelled
CI / unit (8.4) (push) Has been cancelled
CI / psalm (push) Has been cancelled
CI / frontend (push) Has been cancelled
CI / integration (mariadb, 32) (push) Has been cancelled
CI / integration (mariadb, 33) (push) Has been cancelled
CI / integration (mariadb, 34) (push) Has been cancelled
CI / integration (mariadb, 35) (push) Has been cancelled
CI / integration (pgsql, 32) (push) Has been cancelled
CI / integration (pgsql, 33) (push) Has been cancelled
CI / integration (pgsql, 34) (push) Has been cancelled
CI / integration (pgsql, 35) (push) Has been cancelled
CI / integration (sqlite, 32) (push) Has been cancelled
CI / integration (sqlite, 33) (push) Has been cancelled
CI / integration (sqlite, 34) (push) Has been cancelled
CI / integration (sqlite, 35) (push) Has been cancelled
CI / integration-targets (push) Has been cancelled
CI / integration-user (32) (push) Has been cancelled
CI / integration-user (33) (push) Has been cancelled
CI / integration-user (34) (push) Has been cancelled
CI / integration-user (35) (push) Has been cancelled
CI / integration-full-restore (push) Has been cancelled
SQLite in WAL mode starts a transaction as a reader; a write from another process between our first read and our first write makes our first write fail at once, whatever the busy timeout. The SQLite full-restore test now runs a second writer alongside the restore. |
||
|
|
95e3dfcf6d |
User restore by user id, backup:user:list (#22)
CI / unit (8.2) (push) Has been cancelled
CI / unit (8.3) (push) Has been cancelled
CI / unit (8.4) (push) Has been cancelled
CI / psalm (push) Has been cancelled
CI / integration (mariadb, 32) (push) Has been cancelled
CI / integration (mariadb, 33) (push) Has been cancelled
CI / integration (mariadb, 34) (push) Has been cancelled
CI / integration (mariadb, 35) (push) Has been cancelled
CI / integration (pgsql, 32) (push) Has been cancelled
CI / integration (pgsql, 33) (push) Has been cancelled
CI / integration (pgsql, 34) (push) Has been cancelled
CI / integration (pgsql, 35) (push) Has been cancelled
CI / integration (sqlite, 32) (push) Has been cancelled
CI / integration (sqlite, 33) (push) Has been cancelled
CI / integration (sqlite, 34) (push) Has been cancelled
CI / integration (sqlite, 35) (push) Has been cancelled
CI / integration-targets (push) Has been cancelled
CI / integration-user (32) (push) Has been cancelled
CI / integration-user (33) (push) Has been cancelled
CI / integration-user (34) (push) Has been cancelled
CI / integration-user (35) (push) Has been cancelled
CI / integration-full-restore (push) Has been cancelled
backup:user:restore <location> <uid> only said "is not a recorded user export": the argument is the manifest path, which only backup:user:backup printed once. backup:user:list shows the exports again; a bare user id works when that user has exactly one export, otherwise the error lists them. The export is resolved before the replace confirmation. Fixes #22 |
||
|
|
384b2d2792 | Full-restore test: the emoji deck board sets archived as a boolean (PostgreSQL refuses 0) | ||
|
|
be1c628f07 | Full-restore test: install Mail and Deck with up to three tries (the app store is slow at times) | ||
|
|
a8355df12e | Full restore: tables the restore creates get this installation's charset and collation (utf8mb4 on MySQL/MariaDB); test with an emoji | ||
|
|
fd0f786f5f | Full restore: a run again after an interrupted restore keeps maintenance mode on when its check refuses; retry text about apps | ||
|
|
bdcb4f0cae | Full restore runs in maintenance mode, switched off again at the end (also after an error) | ||
|
|
827e46381e | Log a snapshot that cannot be adopted when a location is added; full-restore test prints what target:add and backup:list said when the snapshot is missing | ||
|
|
2fcd38f478 | Full-restore test: Mail and Deck checks only when the origin had them (not on a SQLite origin) | ||
|
|
85ea0acc50 | Full-restore test: restore onto another database (to-pgsql, to-mysql, to-sqlite), every crossing in CI | ||
|
|
ac57e31c93 | Full restore onto PostgreSQL: counters of restored tables go after their data when the backup records none (MySQL, SQLite); test MariaDB onto PostgreSQL | ||
|
|
c95f552cc6 | Read ahead over SFTP: one connection with 32 requests in flight by default, more with restore_sftp_connections; a Storage Box gets slower with parallel readers | ||
|
|
88b719879d | Release candidate 4 (app version 0.9.10): full restore over SFTP reads ahead; SFTP leg in the full-restore test | ||
|
|
8e5c8ccda4 | Full restore: skip the ID counters of tables left out with --skip-missing-tables; test a backup made by rc.1; version 0.9.9 | ||
|
|
4f6a2e5b70 |
Check MySQL foreign keys explicitly; keep index prefix lengths and foreign keys of created tables
Switching FOREIGN_KEY_CHECKS back on does not check the rows loaded meanwhile, and DDL would commit halfway, so on MySQL every foreign key is checked with a query for rows that point at nothing, before the commit. The recorded table structure now keeps index options (the prefix lengths MySQL needs for long text columns) and foreign keys; tables the restore creates get their foreign keys after the commit. The full-restore test also runs on MariaDB. |
||
|
|
d8ef50732f |
Full restore creates missing tables and suspends foreign keys; --install-apps; clearer messages
Backups now record each table's structure in the dump manifest. The full restore creates tables that do not exist on the new instance (left behind by apps that were removed or are switched off), lists all problems at once, and no longer stops on apps that are missing. Foreign keys are not checked while rows are loaded in name order, and checked again before the restore commits (PostgreSQL: dropped and added back in the transaction). --install-apps installs or enables the apps that were enabled at backup time; --skip-missing-tables leaves out tables of older backups whose structure is unknown. nextcloud-aio is never required. backup:target:add without a key points to backup:key:import, and says that connecting can take a while. Removing a location drops its cached index; a migration removes the cache of locations that are gone. The full-restore test now also runs on PostgreSQL with Mail (foreign keys) and a disabled app. |
||
|
|
bec070b1b5 | Full-restore test: show the output and the log when restore:full fails | ||
|
|
16e09f4fde |
Fix trash on SFTP and the disaster recovery test after the review fixes
CI / unit (8.2) (push) Has been cancelled
CI / unit (8.3) (push) Has been cancelled
CI / unit (8.4) (push) Has been cancelled
CI / psalm (push) Has been cancelled
CI / integration (mariadb, 32) (push) Has been cancelled
CI / integration (mariadb, 33) (push) Has been cancelled
CI / integration (mariadb, 34) (push) Has been cancelled
CI / integration (mariadb, 35) (push) Has been cancelled
CI / integration (pgsql, 32) (push) Has been cancelled
CI / integration (pgsql, 33) (push) Has been cancelled
CI / integration (pgsql, 34) (push) Has been cancelled
CI / integration (pgsql, 35) (push) Has been cancelled
CI / integration (sqlite, 32) (push) Has been cancelled
CI / integration (sqlite, 33) (push) Has been cancelled
CI / integration (sqlite, 34) (push) Has been cancelled
CI / integration (sqlite, 35) (push) Has been cancelled
CI / integration-targets (push) Has been cancelled
CI / integration-user (32) (push) Has been cancelled
CI / integration-user (33) (push) Has been cancelled
CI / integration-user (34) (push) Has been cancelled
CI / integration-user (35) (push) Has been cancelled
CI / integration-full-restore (push) Has been cancelled
forget() and untrash() uploaded a remote read stream directly, which the new size check could not judge on SFTP, so cleanup failed there. They now copy through a local buffer, and the size check only trusts local streams (php://temp, php://memory, plain files). The disaster recovery test calls backup:restore:full with --no-interaction: docker exec without a terminal still counts as interactive, so the new confirmation question read an empty answer and cancelled. |
||
|
|
45069994e9 |
Pre-beta review: safer destructive commands and hardening
CI / unit (8.2) (push) Has been cancelled
CI / unit (8.3) (push) Has been cancelled
CI / unit (8.4) (push) Has been cancelled
CI / psalm (push) Has been cancelled
CI / integration (mariadb, 32) (push) Has been cancelled
CI / integration (mariadb, 33) (push) Has been cancelled
CI / integration (mariadb, 34) (push) Has been cancelled
CI / integration (mariadb, 35) (push) Has been cancelled
CI / integration (pgsql, 32) (push) Has been cancelled
CI / integration (pgsql, 33) (push) Has been cancelled
CI / integration (pgsql, 34) (push) Has been cancelled
CI / integration (pgsql, 35) (push) Has been cancelled
CI / integration (sqlite, 32) (push) Has been cancelled
CI / integration (sqlite, 33) (push) Has been cancelled
CI / integration (sqlite, 34) (push) Has been cancelled
CI / integration (sqlite, 35) (push) Has been cancelled
CI / integration-targets (push) Has been cancelled
CI / integration-user (32) (push) Has been cancelled
CI / integration-user (33) (push) Has been cancelled
CI / integration-user (34) (push) Has been cancelled
CI / integration-user (35) (push) Has been cancelled
CI / integration-full-restore (push) Has been cancelled
- backup:restore:full refuses on an installation with more than one user and asks before it starts (--force skips both, also to rerun after a failure halfway), and warns when the snapshot's database dump was not one consistent transaction. - backup:user:restore --mode=replace asks before deleting an existing account; non-interactive use needs --force. - The recovery kit file is created owner-only and never over an existing file or symlink; backup:key:import shows the deletion delay it read from the kit. - Adding a location through the web API needs password confirmation, like removing one. - Key derivation parameters from a kit or repository config are bounded, and a stalled network stream no longer spins at full CPU. - Floating-point columns are dumped exactly instead of with 14 digits. |
||
|
|
ba7f70e0f8 |
Rename per-user backup/restore commands to backup:user:backup and backup:user:restore
CI / unit (8.2) (push) Has been cancelled
CI / unit (8.3) (push) Has been cancelled
CI / unit (8.4) (push) Has been cancelled
CI / psalm (push) Has been cancelled
CI / integration (mariadb, 32) (push) Has been cancelled
CI / integration (mariadb, 33) (push) Has been cancelled
CI / integration (mariadb, 34) (push) Has been cancelled
CI / integration (mariadb, 35) (push) Has been cancelled
CI / integration (pgsql, 32) (push) Has been cancelled
CI / integration (pgsql, 33) (push) Has been cancelled
CI / integration (pgsql, 34) (push) Has been cancelled
CI / integration (pgsql, 35) (push) Has been cancelled
CI / integration (sqlite, 32) (push) Has been cancelled
CI / integration (sqlite, 33) (push) Has been cancelled
CI / integration (sqlite, 34) (push) Has been cancelled
CI / integration (sqlite, 35) (push) Has been cancelled
CI / integration-targets (push) Has been cancelled
CI / integration-user (32) (push) Has been cancelled
CI / integration-user (33) (push) Has been cancelled
CI / integration-user (34) (push) Has been cancelled
CI / integration-user (35) (push) Has been cancelled
CI / integration-full-restore (push) Has been cancelled
|
||
|
|
c9f82a79e4 |
Phase 2: retention for per-user exports (occ backup:retention --user-last)
CI / unit (8.2) (push) Has been cancelled
CI / unit (8.3) (push) Has been cancelled
CI / unit (8.4) (push) Has been cancelled
CI / psalm (push) Has been cancelled
CI / integration (mariadb, 32) (push) Has been cancelled
CI / integration (mariadb, 33) (push) Has been cancelled
CI / integration (mariadb, 34) (push) Has been cancelled
CI / integration (mariadb, 35) (push) Has been cancelled
CI / integration (pgsql, 32) (push) Has been cancelled
CI / integration (pgsql, 33) (push) Has been cancelled
CI / integration (pgsql, 34) (push) Has been cancelled
CI / integration (pgsql, 35) (push) Has been cancelled
CI / integration (sqlite, 32) (push) Has been cancelled
CI / integration (sqlite, 33) (push) Has been cancelled
CI / integration (sqlite, 34) (push) Has been cancelled
CI / integration (sqlite, 35) (push) Has been cancelled
CI / integration-targets (push) Has been cancelled
CI / integration-user (push) Has been cancelled
CI / integration-full-restore (push) Has been cancelled
PruneService::userExportRetention()/setUserExportRetention(): a single, app-wide "keep the newest N" setting (default 3, same default as the snapshot policy's "last"), deliberately simpler than the full daily/weekly/monthly RetentionPolicy -- per-user exports are on-demand only (occ backup:backup:user, see the fold-out task about whether they ever join the automatic schedule), so there is no regular cadence yet to bucket by day/week/month. PruneService::applyUserExports(): groups a location's user exports by uid (read from each manifest's own 'time' field, the same field snapshot retention already reads), keeps the newest N per user, forgets the rest. No trash, unlike a snapshot: these are not yet the primary safety net the deletion delay protects. Runs under the same exclusive lock as snapshot retention. occ backup:retention gained --user-last=N; occ backup:prune now also applies it for every target it processes, reporting counts the same way it already does for snapshots and blob packs. Verified live against the dev rig (four real exports across four users, confirmed exactly the expected ones were kept/forgotten at two different --user-last values) and added coverage to tests/integration/user-roundtrip.sh (an account's manual export plus the automatic safety-export a replace-mode restore makes of it, pruned down to --user-last=2). |
||
|
|
932f60bc5e |
Phase 2: FTP, SMB and NFS storage backends
CI / unit (8.2) (push) Has been cancelled
CI / unit (8.3) (push) Has been cancelled
CI / unit (8.4) (push) Has been cancelled
CI / psalm (push) Has been cancelled
CI / integration (mariadb, 32) (push) Has been cancelled
CI / integration (mariadb, 33) (push) Has been cancelled
CI / integration (mariadb, 34) (push) Has been cancelled
CI / integration (mariadb, 35) (push) Has been cancelled
CI / integration (pgsql, 32) (push) Has been cancelled
CI / integration (pgsql, 33) (push) Has been cancelled
CI / integration (pgsql, 34) (push) Has been cancelled
CI / integration (pgsql, 35) (push) Has been cancelled
CI / integration (sqlite, 32) (push) Has been cancelled
CI / integration (sqlite, 33) (push) Has been cancelled
CI / integration (sqlite, 34) (push) Has been cancelled
CI / integration (sqlite, 35) (push) Has been cancelled
CI / integration-targets (push) Has been cancelled
CI / integration-user (push) Has been cancelled
CI / integration-full-restore (push) Has been cancelled
No ng_backup code changes needed: files_external's generic backend/auth adapter (the same
ExternalStorageFactory/StorageBackend already used for S3/SFTP) already covers FTP and SMB, and
NFS was never a files_external backend to begin with (PLAN.md 3.5) -- the "local" backend plus an
admin-managed OS-level mount already does the job. This commit is test coverage and documentation.
tests/integration/targets-matrix.sh now also runs the full round trip against:
- FTP (stilliard/pure-ftpd). A first attempt with delfer/alpine-ftp-server (vsftpd) failed with a
generic "create directory operation failed": files_external's FTP backend always builds
absolute paths, which that server's chroot rejects even via plain php ftp_mkdir() with an
absolute path (a relative path on the same server works fine) -- not a files_external or
ng_backup bug, just a server-specific quirk, confirmed by switching test servers.
- SMB (dperson/samba), via a small Nextcloud image built on the fly with the smbclient PECL
extension (libsmbclient-dev + pecl install), in its own temp directory as the build context
(never /tmp directly). The backend *lists* as available with just the smbclient CLI binary too
(files_external's shell-wrapped fallback), but that fallback's stream cannot seek, and ng_backup
reads blobs back by byte range while backing up -- confirmed live, it fails the very first
(small) backup with "Seek failed in packs/...". The native extension is genuinely required here,
not just preferred.
README: new "Storage backend experiences" entries for both findings (and a pointer from the
"no extra PHP extensions" intro line, since SMB is the one real exception), plus NFS guidance
(mount it yourself, point the local backend at the mount point).
PLAN.md: FTP/SMB marked done; SMB/NFS testing on the user's own OMV instance stays open (internal,
not something CI can cover).
Local review (Codex + CodeRabbit) found three issues, all fixed: the README's first-pass SMB claim
("CLI tool is not enough") was imprecise about *why* -- it lists fine without the extension, it
just doesn't work for ng_backup's read pattern; the cleanup trap could abort before removing the
network if an earlier docker rm failed, and didn't remove the built image/Dockerfile; and the
Docker build used all of /tmp as its context instead of a dedicated directory.
|
||
|
|
935c7b6a08 |
Phase 2: checksum audit (occ backup:verify)
CI / unit (8.2) (push) Has been cancelled
CI / unit (8.3) (push) Has been cancelled
CI / unit (8.4) (push) Has been cancelled
CI / psalm (push) Has been cancelled
CI / integration (mariadb, 32) (push) Has been cancelled
CI / integration (mariadb, 33) (push) Has been cancelled
CI / integration (mariadb, 34) (push) Has been cancelled
CI / integration (mariadb, 35) (push) Has been cancelled
CI / integration (pgsql, 32) (push) Has been cancelled
CI / integration (pgsql, 33) (push) Has been cancelled
CI / integration (pgsql, 34) (push) Has been cancelled
CI / integration (pgsql, 35) (push) Has been cancelled
CI / integration (sqlite, 32) (push) Has been cancelled
CI / integration (sqlite, 33) (push) Has been cancelled
CI / integration (sqlite, 34) (push) Has been cancelled
CI / integration (sqlite, 35) (push) Has been cancelled
CI / integration-targets (push) Has been cancelled
CI / integration-user (push) Has been cancelled
CI / integration-full-restore (push) Has been cancelled
Repository::verify(): every blob a snapshot's files and database dump reference must be in a freshly re-downloaded, re-authenticated index (never trusted from the local cache, same reasoning as prune()) and have its pack still present on the location -- shallow, cheap, no blob content read, catches a deleted pack, a tampered/rolled-back index, or a database manifest that went missing (reported as a failure, not silently skipped). With $deep, each blob is also downloaded and decrypted, same as loadBlob() already does on every real read (AEAD authentication tag, content hash against its blob id), done here deliberately for everything in the snapshot rather than waiting for it to come up during an actual restore. A blob referenced by more than one file is only checked once; pack presence is cached per pack, not re-checked per blob. occ backup:verify <target> <snapshot> [--deep], meant to run periodically (cron, a sample of recent snapshots), per PLAN.md's "Verificatie: periodiek checksums controleren, steekproef-restore". The lease covering the audit is refreshed roughly every half of its TTL while blobs are checked, rather than relying on a single long-lived lease like the other new phase-2 commands do. Verified against a real S3-backed 1.4 GiB / 603-blob snapshot (shallow: instant; --deep: ~28s, fully decrypted) in addition to the new unit tests and the roundtrip.sh addition (shallow and deep on a snapshot with real, varied file sizes, now covered on every CI matrix combination). Local review (Codex) on the first pass found three real issues, all fixed: a missing database manifest was silently skipped instead of reported; shallow verification trusted the (possibly stale or unauthenticated) local index cache instead of re-checking the authenticated index on the location; and the lease had no refresh, so a --deep audit of a very large snapshot could outrun its TTL and run unprotected against a concurrent prune. |
||
|
|
f18b4f0862 |
Phase 2: disaster recovery (occ backup:restore:full)
CI / unit (8.2) (push) Has been cancelled
CI / unit (8.3) (push) Has been cancelled
CI / unit (8.4) (push) Has been cancelled
CI / psalm (push) Has been cancelled
CI / integration (mariadb, 32) (push) Has been cancelled
CI / integration (mariadb, 33) (push) Has been cancelled
CI / integration (mariadb, 34) (push) Has been cancelled
CI / integration (mariadb, 35) (push) Has been cancelled
CI / integration (pgsql, 32) (push) Has been cancelled
CI / integration (pgsql, 33) (push) Has been cancelled
CI / integration (pgsql, 34) (push) Has been cancelled
CI / integration (pgsql, 35) (push) Has been cancelled
CI / integration (sqlite, 32) (push) Has been cancelled
CI / integration (sqlite, 33) (push) Has been cancelled
CI / integration (sqlite, 34) (push) Has been cancelled
CI / integration (sqlite, 35) (push) Has been cancelled
CI / integration-targets (push) Has been cancelled
CI / integration-user (push) Has been cancelled
CI / integration-full-restore (push) Has been cancelled
New commands, per PLAN.md 3.7: - occ backup:key:import: adopt the master key from a recovery kit on a fresh installation (instead of occ backup:key:init generating a new one), so occ backup:target:add opens the existing repository on the location rather than creating an empty one. - occ backup:restore:full: replace the database (consistent, one transaction), restore the data directory in place, and merge config.php -- refuses up front and lists what's missing when the Nextcloud major version or any app enabled at backup time doesn't match here (their tables must exist before the database restore can use them), rather than guessing or partially applying. Repository::restore() gained an optional $stripPrefix (write "data/<uid>/..." straight into the real data directory, not a "data/" subdirectory of it) -- restoring "data/" and "config/" this way needs no temporary copy of what can be the largest part of an instance. occ backup:target:add now backfills the local snapshot cache (ngb_snapshots, what occ backup:list actually reads) when it adopts an existing repository: without this, a freshly connected location showed no restore points at all, even though the repository itself had them. One unreadable snapshot is skipped rather than aborting the backfill or the target add itself. Three self-referential problems found and fixed by testing this against two real, separate Nextcloud instances sharing only the backup location (tests/integration/full-restore-roundtrip.sh covers this; the per-process nature of these bugs meant they were invisible from inside a single occ invocation, including the one used to write the fixes, which is exactly why a second, fresh process re-running occ backup:status etc. was necessary to catch them): - The full database dump includes ng_backup's own tables and appconfig rows; restoring them verbatim would overwrite the key/target that is actively performing the restore, on THIS installation, with the backed-up instance's. Both are now preserved: ng_backup's own tables (and, on PostgreSQL, their sequences -- excluding a table does not exclude its sequence) are excluded from what gets restored, and its appconfig rows are captured before the database restore and written back after, in their own transaction. - config.php's 'secret' is merged in from the backup (needed so already-encrypted content in the restored database, e.g. share tokens, stays decryptable), but Target::options, the master key, the wrapped key and the recovery kit code are all encrypted with that same instance secret (OCP's ICrypto, or Nextcloud AppConfig's own "sensitive" flag underneath), not with ng_backup's own key. Changing it without re-encrypting them first left all four permanently undecryptable the moment any later occ command ran. Fixed with two helpers (TargetService::reencryptOptionsAround(), KeyService::reencryptForSecretRotation()) that decrypt before and re-encrypt after whatever callback actually changes the secret; config.php is applied dead last in restoreFull(), after every other repository access. Not crash-safe (a process kill mid-rotation would leave a key missing, not just stale) -- disaster recovery is run once by hand, right after occ maintenance:install, a window too narrow yet to justify a two-phase write. - A subtler variant of the same bug: Nextcloud's own AppConfig additionally encrypts a "sensitive" value itself, tied to the same secret, underneath our own encryption. Overwriting an existing sensitive key makes it read the old value first to compare, which then also fails against the new secret. Deleting the key before setting the new value avoids that read entirely. Roadmap (PLAN.md 9): phase 3 (in-place full restore, user self-service) moved out to v1.3.0; phase 2 continues toward v1.2.0 with FTP/SMB, a verification/checksum-audit command, user-export retention, and deciding whether per-user export joins the automatic schedule. |
||
|
|
6cbc5d79d4 |
Phase 2: per-user backup and restore via user_migration
CI / unit (8.2) (push) Has been cancelled
CI / unit (8.3) (push) Has been cancelled
CI / unit (8.4) (push) Has been cancelled
CI / psalm (push) Has been cancelled
CI / integration (mariadb, 32) (push) Has been cancelled
CI / integration (mariadb, 33) (push) Has been cancelled
CI / integration (mariadb, 34) (push) Has been cancelled
CI / integration (mariadb, 35) (push) Has been cancelled
CI / integration (pgsql, 32) (push) Has been cancelled
CI / integration (pgsql, 33) (push) Has been cancelled
CI / integration (pgsql, 34) (push) Has been cancelled
CI / integration (pgsql, 35) (push) Has been cancelled
CI / integration (sqlite, 32) (push) Has been cancelled
CI / integration (sqlite, 33) (push) Has been cancelled
CI / integration (sqlite, 34) (push) Has been cancelled
CI / integration (sqlite, 35) (push) Has been cancelled
CI / integration-targets (push) Has been cancelled
CI / integration-user (push) Has been cancelled
Wires the existing RepositoryExportDestination/RepositoryImportSource (see lib/UserMigration/) into user_migration's own UserMigrationService, resolved on demand through a new UserMigrationServiceLocator since the app is an optional dependency (PLAN.md 3.8) -- without it, only this feature is unavailable, the full backup still works. New: UserBackupService::backupUser() (occ backup:backup:user) streams a user_migration export straight into the repository, under the same shared location lock as a regular backup. UserRestoreService::restoreUser() (occ backup:restore:user) never imports over a live account: --mode=replace safety-exports the current state first, then deletes and recreates the account from the backup under the same uid; --mode=as-backup restores into a new "<uid>-bak" account instead, since Nextcloud cannot rename a uid. The catalog now also tracks user export manifests (Repository::recordUserExport/ listUserExports/forgetUserExport/isRecordedUserExport), rollback-protected like a snapshot: listUserExports() verifies each one is still present on the location, and restore only ever reads a manifest the verified catalog actually recorded, never an arbitrary caller-supplied path. prune()'s existing raw users/ scan for blob retention is unchanged. A new short exclusive "catalog" lock (PruneService::catalogLockKey()) serializes the catalog write itself, so two concurrent per-user backups (or one alongside a full backup) can no longer race to write the same next generation and silently drop one of them -- the broader per-target shared lock only ever protected against a concurrent cleanup, not against other shared holders. Two real bugs found by testing this against a live Nextcloud instance, not by unit tests (RepositoryExportDestination/RepositoryImportSource implement OCP interfaces that plain PHPUnit can't load, see tests/Unit/Repository/UserExportTest.php's docblock; the new tests/integration/user-roundtrip.sh covers the real round trip instead): - RepositoryImportSource::copyToFolder() double-closed the stream it handed to putContent()/newFile() (both already close it themselves), which throws a TypeError on PHP 8 and broke every restored file with content. - A replace-mode restore failed on NotPermittedException: IRootFolder::getUserFolder() caches a Folder per uid for the process lifetime, so after delete+recreate of the same uid the import still got the deleted account's (now storage-less) folder. Fixed by dispatching FilesystemTornDownEvent, which Root already listens for to clear exactly that cache -- no internal OC\ classes needed. Local review (CodeRabbit + Codex) against this diff found three more real issues, all fixed: the catalog write race above, a failed as-backup import leaving a broken stub account behind (now deleted on failure), and an export stream leak on a read/backend failure in RepositoryExportDestination::copyFolder() (now closed in a finally). Deliberately left out: wiring per-user export into the automatic backup schedule (on-demand via occ only, for now) and retention/pruning of old user exports -- both open design questions, not yet decided. The lease covering a single export/import still has a fixed TTL (3600s) with no mid-call refresh, since UserMigrationService exposes no progress; a resumable, step-based per-user backup (like the main BackupService run) would remove that cap but is a bigger change than this pass. |
||
|
|
48ea6bb33d |
CI: round trip against real S3 and SFTP backends, not just a local path
CI / unit (8.2) (push) Has been cancelled
CI / unit (8.3) (push) Has been cancelled
CI / unit (8.4) (push) Has been cancelled
CI / integration (mariadb, 32) (push) Has been cancelled
CI / integration (mariadb, 33) (push) Has been cancelled
CI / integration (mariadb, 34) (push) Has been cancelled
CI / integration (mariadb, 35) (push) Has been cancelled
CI / integration (pgsql, 32) (push) Has been cancelled
CI / integration (pgsql, 33) (push) Has been cancelled
CI / integration (pgsql, 34) (push) Has been cancelled
CI / integration (pgsql, 35) (push) Has been cancelled
CI / integration (sqlite, 32) (push) Has been cancelled
CI / integration (sqlite, 33) (push) Has been cancelled
CI / integration (sqlite, 34) (push) Has been cancelled
CI / integration (sqlite, 35) (push) Has been cancelled
CI / integration-targets (push) Has been cancelled
Added tests/integration/targets-matrix.sh, which runs the full backup/restore round trip through files_external's AmazonS3 and SFTP backend classes (S3 via a disposable s3mock container, SFTP via atmoz/sftp), since the existing matrix only ever exercised the local backend. This caught a real bug: moving a snapshot to the trash read it from the location and streamed it straight back into a write on the same connection, which SFTP can't do reliably (phpseclib fails to interleave a read and a write on one connection). Backends now expose a native move() and the id-bound snapshot object is moved with that instead of a read+write copy; the path-bound database dump still goes through decrypt/re-encrypt, since its AEAD authentication is tied to its path. |
||
|
|
c54b2cebed |
M5: end-to-end test matrix and CI
- tests/integration/roundtrip.sh: key, kit code and confirmation, location, full and unchanged incremental backup, restore new-folder and replace (files identical, extra file in the trash), database dump and restore, retention, trash and restore from trash, no files_external errors in the log - tests/integration/matrix.sh: fresh Nextcloud per version and database (SQLite, MariaDB, PostgreSQL), removes only its own containers and their volumes; locally green on NC32 and NC35 with all three databases - GitHub Actions: unit tests on PHP 8.2-8.4, round trip on NC32-35 x SQLite/MariaDB/PostgreSQL Fixes found by the matrix: - files_external is no longer booted when it is disabled: only its classes are loaded and a private BackendService gets its backends from its Application class (booting registered its mount provider, which failed on installations where its tables were never created) - SQLite: the database file is excluded from the file backup (it is covered by the logical dump and copying the live file is inconsistent) - occ backup:key:confirm accepts --code and --confirm for scripts |