b8af72764d
gates / gates (push) Successful in 11s
Four defects on the restore surface, all proven on demo-hp during the 2026-08-21 backup-truth drill, all still in shipped code. They share one acceptance idea: a restore surface must state what it actually did, and must refuse what it cannot do. VERSION NOTE. The task specifying this targeted v0.224.0 against baselinef8c9390. Both were consumed earlier the same day by R-330 (0.224.0) and R-331 (0.225.0). Drift re-confirmed against live Gitea before the first edit, operator authorised proceeding, every symbol the spec named re-verified present at the real baselinee5eee50. R-353 -- a restore that gave back nothing still said it worked. RestoreFromRecoveryUnit returned only error, so the surface printed "<app> visszaallitva (<snapshot>)." -- equally true of a run that returned an entire dataset and one that returned nothing. The count already existed and was discarded one line deep: restoreDockerVolumesFrom always returned it, the wrapper threw it away. Now (UnitRestoreResult, error), carrying replayed counts AND what the manifest LISTED, because zero-replayed has two causes that are opposite news. Three cases, three sentences, and EVERY one is a claim about the BACKUP, never about the app -- this path has no SafetyDump discriminator, and 07-backup-architecture 6.3 records that an absent dump says nothing about the app (R-361 destroyed canonical .sql files for four months). R-357 -- the destructive restore had no free-space gate. offbox_reconstitute.go contained ZERO references to offboxFree; all three existing gates guard non-destructive paths. The gate now sits before mapOffsiteRestorePaths, writeSafetyDump and StopStack, so a refusal costs nothing. Position IS the fix, which is why the test asserts StopStack was never called. No headroom multiplier (matches PlaceOffsiteRestore; the x1.1 elsewhere predicts a download). Fail closed on either probe <= 0 -- otherwise `free < need` with need==0 is FALSE and an unmeasurable scratch sails through: a gate present and inert. R-358 -- a failed download was offered as a good one. The gate answered "the directory exists and is non-empty", which is exactly what a part-way restic run leaves. Now a completion marker written 0600 atomically AFTER restic returns nil, with any stale one cleared BEFORE it starts; both orders pinned by an AST test because resticStep is not a seam. Both handlers refuse server-side: the wizard flags control a button, and a hidden button is not a guard. SCENARIO F ANSWERED, and worse than the question assumed: a unit-only scratch IS reachable through the real flow, by the most ordinary route. "Ellenorzo visszaallitas" (mode=unit, advertised non-destructive) writes the SAME directory -- offboxRestoreScratchDir ignores `full` and --include limits what restic extracts, never where -- so a customer who ran the SAFE restore was then offered the destructive one over a unit-only copy. Filed R-396; the marker closes it. R-360 -- the delete refused only while a BACKUP ran. IsRunning() is FALSE for the whole of a verification restore; the five sibling handlers all use restoreOpBlocked(). Its doc comment claimed it already did this, which is why nobody looked -- corrected in place. Red-proofs, each printing the pre-fix behaviour, in CHANGELOG and REPORT. The first R-357 red-proof exposed a hollow test OF MY OWN and it is recorded rather than quietly fixed: the fixture refused earlier at the placement stat pre-pass, so `stops == 0` passed against the pre-fix code. Fixture corrected, assertions reordered so a removed gate reports the outage rather than "no error returned". Green gate clean: 28 packages, rc 0. All 12 controller gates OK.
224 lines
9.1 KiB
Go
224 lines
9.1 KiB
Go
package web
|
|
|
|
import (
|
|
"io"
|
|
"log"
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"os"
|
|
"path/filepath"
|
|
"strings"
|
|
"testing"
|
|
|
|
"gitea.dooplex.hu/admin/felhom-controller/internal/backup"
|
|
"gitea.dooplex.hu/admin/felhom-controller/internal/config"
|
|
"gitea.dooplex.hu/admin/felhom-controller/internal/settings"
|
|
)
|
|
|
|
// ── R-358 / R-360 at the HANDLERS ────────────────────────────────────────────────────────────────
|
|
//
|
|
// Both defects are server-side. R-358's part-copy was hidden by a template flag, and a hidden button
|
|
// is not a guard — these POST directly, which is what a curious customer, a stale tab or a double
|
|
// submit does anyway. R-360's delete refused only while a BACKUP ran, so it went through during a
|
|
// restore; that one asserts the CONSEQUENCE (the directory still exists), never the branch.
|
|
|
|
func newR358Server(t *testing.T) (*Server, *backup.Manager, string) {
|
|
t.Helper()
|
|
tmp := t.TempDir()
|
|
lg := log.New(io.Discard, "", 0)
|
|
drive := filepath.Join(tmp, "drive")
|
|
if err := os.MkdirAll(drive, 0o755); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
sett, err := settings.Load(filepath.Join(tmp, "settings.json"), lg)
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if err := sett.AddStoragePath(settings.StoragePath{Path: drive, Label: "drive", Schedulable: true}); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if err := sett.SetOffboxTarget(&settings.OffboxTarget{
|
|
Enabled: true, Host: "nas.local", Port: 22, User: "u", RepoPath: "/srv/repo",
|
|
Schedule: "daily", EscrowState: "escrowed",
|
|
}); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
cfg := &config.Config{}
|
|
cfg.Paths.DataDir = tmp
|
|
m := backup.NewManager(cfg, sett, lg)
|
|
if err := m.WriteOffboxSecrets("KEY", "nas.local ssh-ed25519 AAAA"); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
m.SetStackProvider(&r353Provider{hdd: drive})
|
|
s := &Server{cfg: cfg, backupMgr: m, settings: sett, logger: lg}
|
|
return s, m, drive
|
|
}
|
|
|
|
// plantIncompleteScratch writes the exact shape a failed restic download leaves: files, no marker.
|
|
func plantIncompleteScratch(t *testing.T, m *backup.Manager, app string) string {
|
|
t.Helper()
|
|
scratch := m.OffsiteRestoreScratchPath(app)
|
|
if scratch == "" {
|
|
t.Fatal("could not resolve the scratch path")
|
|
}
|
|
if err := os.MkdirAll(filepath.Join(scratch, "backups", "primary", app), 0o755); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if err := os.WriteFile(filepath.Join(scratch, "backups", "primary", app, "half.tar"), []byte("partial"), 0o644); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
return scratch
|
|
}
|
|
|
|
func TestR358_PlaceHandlerRefusesIncompleteScratch(t *testing.T) {
|
|
s, m, _ := newR358Server(t)
|
|
plantIncompleteScratch(t, m, "kimai")
|
|
|
|
req := httptest.NewRequest(http.MethodPost, "/backup/offbox/place", strings.NewReader("app=kimai"))
|
|
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
|
|
w := httptest.NewRecorder()
|
|
s.offboxPlaceHandler(w, req)
|
|
|
|
loc := w.Header().Get("Location")
|
|
if !strings.Contains(loc, "nem+teljes") && !strings.Contains(loc, "nem%20teljes") {
|
|
t.Fatalf("a direct POST over a part-copy was NOT refused server-side; redirect was %q", loc)
|
|
}
|
|
if m.RestoreStatus().Running {
|
|
t.Fatal("the place operation actually STARTED over an incomplete scratch")
|
|
}
|
|
}
|
|
|
|
func TestR358_ReconstituteHandlerRefusesIncompleteScratch(t *testing.T) {
|
|
// The destructive one. On 2026-08-21 „Teljes visszaállítás indítása" was offered over exactly this
|
|
// state and reported ok=true.
|
|
s, m, _ := newR358Server(t)
|
|
plantIncompleteScratch(t, m, "kimai")
|
|
|
|
req := httptest.NewRequest(http.MethodPost, "/backup/offbox/reconstitute",
|
|
strings.NewReader("app=kimai&confirm=1"))
|
|
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
|
|
w := httptest.NewRecorder()
|
|
s.offboxReconstituteHandler(w, req)
|
|
|
|
loc := w.Header().Get("Location")
|
|
if !strings.Contains(loc, "nem+teljes") && !strings.Contains(loc, "nem%20teljes") {
|
|
t.Fatalf("the DESTRUCTIVE restore was not refused over a part-copy; redirect was %q", loc)
|
|
}
|
|
if m.RestoreStatus().Running {
|
|
t.Fatal("the destructive restore actually STARTED over an incomplete scratch")
|
|
}
|
|
}
|
|
|
|
// TestR360_VerifyCopyDeleteRefusedDuringRestore — Scenario G, asserting the CONSEQUENCE.
|
|
//
|
|
// The state is the one observed live on 2026-08-21 22:35 and it is the whole reason the bug existed:
|
|
// RestoreStatus().Running is TRUE while IsRunning() is FALSE. The old guard read only the second.
|
|
func TestR360_VerifyCopyDeleteRefusedDuringRestore(t *testing.T) {
|
|
s, m, drive := newR358Server(t)
|
|
|
|
// A real verification copy on disk, at the path DeleteOffsiteRestoreCopy resolves.
|
|
copyDir := filepath.Join(drive, "backups", "offsite-restore", "kimai")
|
|
if err := os.MkdirAll(copyDir, 0o755); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if err := os.WriteFile(filepath.Join(copyDir, "payload.txt"), []byte("the copy a restore is writing into"), 0o644); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
|
|
// The live state: a restore op in flight, no BACKUP running.
|
|
m.BeginRestoreOp("offbox-restore", "kimai")
|
|
if !m.RestoreStatus().Running {
|
|
t.Fatal("fixture wrong: no restore op is in flight")
|
|
}
|
|
if m.IsRunning() {
|
|
t.Fatal("fixture wrong: IsRunning() must be FALSE — that divergence IS the defect")
|
|
}
|
|
|
|
req := httptest.NewRequest(http.MethodPost, "/backup/offbox/verify-copy/delete",
|
|
strings.NewReader("stack=kimai&confirm=1"))
|
|
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
|
|
w := httptest.NewRecorder()
|
|
s.offboxVerifyCopyDeleteHandler(w, req)
|
|
|
|
// THE ASSERTION THAT MATTERS: the copy the restore is writing into is still there.
|
|
if _, err := os.Stat(copyDir); os.IsNotExist(err) {
|
|
t.Fatal("THE VERIFICATION COPY WAS DELETED while a restore was writing into it — this is " +
|
|
"the 2026-08-21 behaviour, and the customer can do it from the UI")
|
|
}
|
|
if _, err := os.Stat(filepath.Join(copyDir, "payload.txt")); err != nil {
|
|
t.Fatalf("the copy's contents did not survive the delete attempt: %v", err)
|
|
}
|
|
if loc := w.Header().Get("Location"); !strings.Contains(loc, "flash_error") {
|
|
t.Errorf("the refusal must reach the customer as an error flash; redirect was %q", loc)
|
|
}
|
|
}
|
|
|
|
func TestR360_VerifyCopyDeleteStillWorksWhenIdle(t *testing.T) {
|
|
// Scenario H for this handler: with nothing in flight the delete must still work. A guard that
|
|
// refuses always is not a fix, it is a removed feature.
|
|
s, _, drive := newR358Server(t)
|
|
copyDir := filepath.Join(drive, "backups", "offsite-restore", "kimai")
|
|
if err := os.MkdirAll(copyDir, 0o755); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
|
|
req := httptest.NewRequest(http.MethodPost, "/backup/offbox/verify-copy/delete",
|
|
strings.NewReader("stack=kimai&confirm=1"))
|
|
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
|
|
w := httptest.NewRecorder()
|
|
s.offboxVerifyCopyDeleteHandler(w, req)
|
|
|
|
if _, err := os.Stat(copyDir); !os.IsNotExist(err) {
|
|
t.Fatalf("an idle delete no longer removes the copy (redirect %q)", w.Header().Get("Location"))
|
|
}
|
|
}
|
|
|
|
// TestR358_UnitOnlyScratchClosesTheFullRestoreCard — Scenario F at the FLOW level.
|
|
//
|
|
// THE ANSWER TO THE SPEC'S OPEN QUESTION, and it is worse than the question assumed. The task asked
|
|
// whether the real UI flow can reach a state where a unit-only scratch makes the full-restore action
|
|
// appear. It can, and by the MOST ORDINARY route available:
|
|
//
|
|
// - „Ellenőrző visszaállítás" (`mode=unit`, the default, advertised as non-destructive) calls
|
|
// RestoreOffboxScratch(ctx, app, full=false);
|
|
// - both modes write the SAME directory — offboxRestoreScratchDir ignores `full`, and `--include`
|
|
// limits WHAT restic extracts, never WHERE;
|
|
// - the wizard sets ScratchReady from OffboxFullScratchReady, which pre-fix answered
|
|
// "directory exists and is non-empty";
|
|
// - deriveWizardStep then sets PlaceEnabled AND RestoreEnabled from that one flag.
|
|
//
|
|
// So a customer who ran the SAFE verification restore was then offered „Teljes visszaállítás
|
|
// indítása" over a unit-only copy. Filed as a register row; the fix closes it because the marker
|
|
// records full=false.
|
|
func TestR358_UnitOnlyScratchClosesTheFullRestoreCard(t *testing.T) {
|
|
s, m, _ := newR358Server(t)
|
|
scratch := m.OffsiteRestoreScratchPath("kimai")
|
|
if err := os.MkdirAll(scratch, 0o755); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
// What a completed `mode=unit` verification restore leaves behind.
|
|
if err := os.MkdirAll(filepath.Join(scratch, "mnt", "old", "backups", "primary", "kimai"), 0o755); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if err := m.WriteScratchMarkerForTest(scratch, "snap-1", false); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
|
|
ready := s.backupMgr.OffboxFullScratchReady("kimai")
|
|
if ready {
|
|
t.Fatal("a unit-only verification restore still unlocks the full-restore card — the customer " +
|
|
"is offered a destructive restore over a copy that holds only the recovery unit")
|
|
}
|
|
view := deriveWizardStep(restoreWizardInput{App: "kimai", ScratchReady: ready})
|
|
if view.RestoreEnabled || view.PlaceEnabled {
|
|
t.Fatalf("the wizard still offers place/restore over a unit-only scratch: %+v", view)
|
|
}
|
|
if !view.PrepareEnabled {
|
|
t.Fatal("the customer is left with no way forward — PrepareEnabled must be true so they can " +
|
|
"run the real full download")
|
|
}
|
|
if !view.VerifyEnabled {
|
|
t.Fatal("the verification restore must stay available")
|
|
}
|
|
}
|