fix(PROJ-66): Build-Fehler in printHelp() + -force überschreibt keine Hardlinks
BUG-1 (QA): Backticks in der restore-Hilfezeile schlossen das Raw-String- Literal von printHelp() vorzeitig, Build brach komplett. Backticks durch einfache Anführungszeichen ersetzt. BUG-2 (QA): -force bei restore scheiterte an bereits vorhandenen Hardlink-Zieldateien (os.Link: file exists) — für den Normalfall (Restore gegen einen PROJ-65-Tenant-Hardlink-Bestand) war -force damit funktionslos. copyTreePreservingHardlinks() entfernt jetzt eine vorhandene Zieldatei vor os.Link.
This commit is contained in:
sysops
@@ -202,6 +202,14 @@ func copyTreePreservingHardlinks(src, dst string) (linked, copied int, err error
|
||||
stat, ok := info.Sys().(*syscall.Stat_t)
|
||||
if ok {
|
||||
if existing, dup := seen[stat.Ino]; dup {
|
||||
// os.Link fails with EEXIST if target is already occupied — happens
|
||||
// on a -force restore where the destination isn't empty (BUG-2,
|
||||
// PROJ-66 QA). Remove any stale file first; harmless when the
|
||||
// destination is empty already (os.Remove on a non-existent path
|
||||
// is ignored below).
|
||||
if rmErr := os.Remove(target); rmErr != nil && !os.IsNotExist(rmErr) {
|
||||
return fmt.Errorf("hardlink %s -> %s: remove stale target: %w", existing, target, rmErr)
|
||||
}
|
||||
if err := os.Link(existing, target); err != nil {
|
||||
return fmt.Errorf("hardlink %s -> %s: %w", existing, target, err)
|
||||
}
|
||||
|
||||
@@ -307,7 +307,7 @@ Commands:
|
||||
ocr-reprocess OCR für Anhänge nachholen (alle oder pro Mandant/Status)
|
||||
index-pending Ungeindexte Mails nachindexieren (cron-fähig, PROJ-58 batch_mode)
|
||||
backup Store, Keyfile, PostgreSQL und Config konsistent sichern (PROJ-66)
|
||||
restore Backup aus `archivmail backup` zurückspielen (PROJ-66)
|
||||
restore Backup aus 'archivmail backup' zurückspielen (PROJ-66)
|
||||
update Auf neueste Version aktualisieren (führt update.sh aus)
|
||||
status Healthcheck für DB, Manticore und Storage
|
||||
version Version anzeigen
|
||||
|
||||
+1
-1
@@ -81,7 +81,7 @@
|
||||
| PROJ-63 | Defensive Tenant-Scope-Härtung der Tenant-Verwaltungs-Endpunkte (FUND-2) | Deployed | [PROJ-63](PROJ-63-harden-tenant-admin-scope.md) | 2026-06-25 |
|
||||
| PROJ-64 | Session-Invalidation bei Passwort-Change + Datei-Permissions-Härtung (Security-Audit) | Deployed | [PROJ-64](PROJ-64-session-invalidation-file-permissions.md) | 2026-07-03 |
|
||||
| PROJ-65 | Physische Tenant-Trennung im Storage-Layer | Deployed | [PROJ-65](PROJ-65-physische-tenant-trennung.md) | 2026-07-04 |
|
||||
| PROJ-66 | Backup-Strategie für Store, Keyfile, PostgreSQL (Produktiv + Teilproduktiv) | In Review | [PROJ-66](PROJ-66-backup-strategie.md) | 2026-07-04 |
|
||||
| PROJ-66 | Backup-Strategie für Store, Keyfile, PostgreSQL (Produktiv + Teilproduktiv) | In Progress | [PROJ-66](PROJ-66-backup-strategie.md) | 2026-07-04 |
|
||||
|
||||
<!-- Add features above this line -->
|
||||
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
# PROJ-66: Backup-Strategie für archivmail (Produktiv + Teilproduktiv)
|
||||
|
||||
**Status:** In Review
|
||||
**Status:** In Review (BUG-1/BUG-2 aus QA-Runde 1 gefixt, Re-Test steht aus)
|
||||
**Erstellt:** 2026-07-04
|
||||
|
||||
## Problem / Ausgangslage
|
||||
@@ -522,3 +522,101 @@ Speicherplatz-Mehrverbrauch).
|
||||
blockierend bei nicht-leerem Store.
|
||||
- `-dest`-Ziel für einen produktiven Cron-Eintrag muss noch vom Nutzer
|
||||
festgelegt werden, bevor die auskommentierte Cron-Zeile aktiviert wird.
|
||||
|
||||
## QA Test Results (2026-07-04, Testserver 132)
|
||||
|
||||
**Getestet gegen Commit f3a7dea (lokal, nicht gepusht/deployt).** Build und
|
||||
Funktionstests auf 132 in isoliertem Testbereich (synthetischer Store mit
|
||||
PROJ-65-Hardlinks unter `/tmp/qa66-*`, eigene Test-DB `archivmail_qa66_test`).
|
||||
Produktion (60.595 Store-Dateien, DB `archivmail`) nachweislich unberührt —
|
||||
kein Zugriff auf echte Store-Dateien oder die echte DB, nach dem Test verifiziert.
|
||||
|
||||
**Gesamtergebnis: QA NICHT BESTANDEN** — 1 Critical (build-breaking) + 1 High
|
||||
(-force funktionslos). Nach Fix beider Bugs erneut testen.
|
||||
|
||||
| # | Testpunkt | Ergebnis |
|
||||
|---|-----------|----------|
|
||||
| 1 | `go build ./cmd/archivmail/` | **FAIL** (BUG-1, build-breaking) |
|
||||
| 2 | Backup-Lauf gegen Test-Config | PASS (nur mit QA-Workaround-Patch) |
|
||||
| 3 | postgres.dump/store/keyfile/config.yml vorhanden + Store-Hardlinks (Inode-Gleichheit, link count 2) | PASS |
|
||||
| 4 | Restore-Roundtrip auf isoliertes Ziel: Store + Hardlinks + pg_restore (DB-Zeilen korrekt), Keyfile 0600 | PASS |
|
||||
| 5 | `-force`-Schutz: ohne `-force` blockiert (5a) / mit `-force` läuft durch (5b) | 5a PASS / **5b FAIL** (BUG-2) |
|
||||
| 6 | Rotation `-keep 2` über 3 Läufe → nur 2 Verzeichnisse | PASS |
|
||||
| 7 | Ungültiges/nicht beschreibbares `-dest` → sauberer Abbruch, keine Rotation guter Backups | PASS |
|
||||
| 8 | `backup` ohne `-dest` → Fehlerabbruch | PASS |
|
||||
|
||||
Zusätzlich verifiziert: pg_dump-Fehlerfall (falsche DB) lässt partiellen
|
||||
Backup-Ordner stehen und rotiert **keine** guten alten Backups weg (Lehre aus
|
||||
PROJ-58) — PASS.
|
||||
|
||||
### BUG-1 (Severity: CRITICAL, Priorität: sofort) — Binary kompiliert nicht
|
||||
|
||||
`cmd/archivmail/cmd_import.go:310` bricht den Build. Die Hilfetext-Zeile für
|
||||
`restore` wurde im Commit f3a7dea in den mit Backticks begrenzten
|
||||
Raw-String-Literal von `printHelp()` (`fmt.Printf` mit Backtick-String, geschlossen
|
||||
mit `` `, AppVersion) ``) eingefügt, enthält aber selbst Backticks um
|
||||
`` `archivmail backup` ``. Diese beenden das Raw-String-Literal vorzeitig:
|
||||
|
||||
```
|
||||
cmd/archivmail/cmd_import.go:310:30: syntax error: unexpected name archivmail in argument list; possibly missing comma or )
|
||||
```
|
||||
|
||||
Repro: `CGO_ENABLED=0 go build -buildvcs=false -o /tmp/x ./cmd/archivmail/`
|
||||
→ scheitert. **Das gesamte Backend (nicht nur backup/restore) ist nicht
|
||||
baubar/deploybar, solange dieser Fehler besteht.** Für die restlichen QA-Punkte
|
||||
wurde in einem Wegwerf-Build-Verzeichnis die Zeile behelfsweise auf einfache
|
||||
Anführungszeichen geändert (kein Fix am Repo/Commit — Bug besteht unverändert).
|
||||
Fix-Vorschlag (Backend Developer): Backticks in Zeile 310 durch einfache
|
||||
Anführungszeichen ersetzen oder die Zeile ohne Inline-Code-Markup formulieren.
|
||||
|
||||
### BUG-2 (Severity: HIGH, Priorität: hoch) — `restore -force` schlägt bei nicht-leerem Store mit Hardlinks fehl
|
||||
|
||||
`-force` soll laut Spec/AC einen bereits befüllten Store überschreiben. Tatsächlich
|
||||
bricht der Restore ab, sobald der Store Hardlinks enthält (PROJ-65-Tenant-Struktur —
|
||||
der Normalfall):
|
||||
|
||||
```
|
||||
restore: store restore failed: hardlink /…/ab/abcdef123.bin -> /…/tenant_1/abcdef123.bin:
|
||||
link …: file exists
|
||||
```
|
||||
|
||||
Ursache: `copyTreePreservingHardlinks()` (`cmd_backup.go`) ruft `os.Link()` für die
|
||||
zweite Inode-Referenz auf, ohne dass das Zielverzeichnis vorher geleert wird. Bei
|
||||
`-force` überschreibt `copyFile()` reguläre Dateien zwar via `O_TRUNC`, aber `os.Link()`
|
||||
scheitert an einer bereits existierenden Zieldatei (`EEXIST`). Damit ist `-force`
|
||||
für genau den Anwendungsfall funktionslos, für den es gedacht ist (Restore über
|
||||
einen vorhandenen, mit Tenant-Hardlinks befüllten Store). Repro: einmal restoren,
|
||||
dann erneut mit `-force` auf dasselbe Ziel → Fehler.
|
||||
Fix-Vorschlag (Backend Developer): bei `-force` das Zielverzeichnis vor dem
|
||||
Restore leeren, oder in `copyTreePreservingHardlinks()` vor `os.Link()`/`copyFile()`
|
||||
ein vorhandenes Ziel entfernen (`os.Remove(target)`, `ErrNotExist` ignorieren).
|
||||
Hinweis: `-force` ohne Hardlinks (nur reguläre Dateien) läuft dank `O_TRUNC` durch —
|
||||
der Fehler tritt nur bei der zweiten+ Inode-Referenz auf.
|
||||
|
||||
### Nicht getestet / Hinweise
|
||||
- Nur die CLI-Kommandos wurden getestet, nicht die offenen ACs (Cron/Timer AC 1,
|
||||
physische Trennung AC 3, Verschlüsselung AC 4, Restore-Verifikation via echtem
|
||||
`reconcile` AC 6, Alerting AC 7, Runbook AC 8, Bitwarden AC 10, PBS AC 11) —
|
||||
diese sind laut Implementation Notes bewusst noch offen.
|
||||
- `pg_restore --clean --if-exists` wurde nur gegen eine leere Test-DB geprüft;
|
||||
Idempotenz über eine bereits befüllte DB nicht separat getestet.
|
||||
- Testartefakte (`/tmp/qa66-*`, Test-DB `archivmail_qa66_test`, Wegwerf-Build
|
||||
`/root/archivmail-qa66`) nach Testende entfernt und Entfernung verifiziert.
|
||||
|
||||
## Fixes nach QA-Runde 1 (2026-07-04)
|
||||
|
||||
- **BUG-1 (Critical, build-breaking):** `cmd/archivmail/cmd_import.go:310`
|
||||
enthielt Backticks um `` `archivmail backup` `` innerhalb des Raw-String-
|
||||
Literals von `printHelp()`, was den String vorzeitig schloss
|
||||
(`syntax error: unexpected name archivmail in argument list`). Fix:
|
||||
Backticks durch einfache Anführungszeichen ersetzt.
|
||||
- **BUG-2 (High):** `-force` bei `restore` scheiterte an bereits vorhandenen
|
||||
Hardlink-Zieldateien (`os.Link: file exists`) — für den Normalfall (Restore
|
||||
gegen einen PROJ-65-Tenant-Hardlink-Bestand) war `-force` damit
|
||||
funktionslos. Fix in `copyTreePreservingHardlinks()`: vor `os.Link` wird
|
||||
eine bereits vorhandene Zieldatei entfernt (`os.Remove`, `os.IsNotExist`
|
||||
wird ignoriert — im Normalfall ist das Ziel leer und der Remove ist ein
|
||||
No-Op).
|
||||
|
||||
Re-Test von Punkt 1 (Build) und Punkt 5b (`-force` gegen befüllten Store)
|
||||
steht aus.
|
||||
|
||||
Reference in New Issue
Block a user