Files
felhom-controller/controller/internal/backup/r358_scratch_marker_test.go
T
admin b8af72764d
gates / gates (push) Successful in 11s
R-353/R-357/R-358/R-360: the restore tells the truth (v0.226.0)
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 baseline
f8c9390. 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 baseline e5eee50.

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.
2026-08-30 19:31:31 +02:00

249 lines
8.9 KiB
Go

package backup
import (
"bytes"
"go/ast"
"go/parser"
"go/token"
"log"
"os"
"path/filepath"
"strings"
"testing"
"gitea.dooplex.hu/admin/felhom-controller/internal/settings"
)
// ── R-358 — a failed download was offered as a good one ──────────────────────────────────────────
//
// `OffboxFullScratchReady` used to answer "the directory exists and is non-empty". A restic run that
// dies part-way leaves exactly that. So the product showed „Teljes visszaállítás indítása" over a
// part-copy and pressing it reported success — observed on demo-hp 2026-08-21.
//
// A non-empty directory is evidence that SOMETHING was written, never that everything was. The marker
// carries the only fact that distinguishes them: did the run FINISH, and was it the full one.
// newR358Manager gives a manager whose scratch resolves into a t.TempDir().
func newR358Manager(t *testing.T) (*Manager, string) {
t.Helper()
m, sett := newOffboxManager(t)
drive := t.TempDir()
if err := sett.AddStoragePath(settings.StoragePath{Path: drive, Label: "drive", Schedulable: true}); err != nil {
t.Fatal(err)
}
m.SetStackProvider(&offbox3aProvider{
hdd: map[string]string{"kimai": drive}, binds: map[string][]ClassifiedBind{}, has: map[string]bool{},
})
scratch, _, err := m.offboxRestoreScratchDir("kimai")
if err != nil {
t.Fatal(err)
}
if err := os.MkdirAll(scratch, 0o755); err != nil {
t.Fatal(err)
}
return m, scratch
}
// writeScratchPayload plants the files a part-way restic run leaves behind.
func writeScratchPayload(t *testing.T, scratch string) {
t.Helper()
if err := os.MkdirAll(filepath.Join(scratch, "backups", "primary", "kimai"), 0o755); err != nil {
t.Fatal(err)
}
if err := os.WriteFile(filepath.Join(scratch, "backups", "primary", "kimai", "half.tar"), []byte("partial"), 0o644); err != nil {
t.Fatal(err)
}
}
func TestR358_FailedRestoreLeavesNoUsableScratch(t *testing.T) {
m, scratch := newR358Manager(t)
writeScratchPayload(t, scratch) // files present, run never finished → no marker
if m.OffboxFullScratchReady("kimai") {
t.Fatal("a part-copy was reported READY — this is the defect: the customer is offered " +
"„Teljes visszaállítás indítása" + " over a download that never finished")
}
}
func TestR358_UnitOnlyScratchIsNotFullReady(t *testing.T) {
m, scratch := newR358Manager(t)
writeScratchPayload(t, scratch)
if err := m.writeScratchMarker(scratch, "snap-1", false); err != nil {
t.Fatal(err)
}
if m.OffboxFullScratchReady("kimai") {
t.Fatal("a UNIT-ONLY scratch was reported ready for a full restore — both modes write the " +
"same directory, so only the marker's `full` field separates them")
}
}
func TestR358_StaleMarkerIsClearedBeforeTheRun(t *testing.T) {
m, scratch := newR358Manager(t)
if err := m.writeScratchMarker(scratch, "snap-OLD", true); err != nil {
t.Fatal(err)
}
if !m.OffboxFullScratchReady("kimai") {
t.Fatal("fixture wrong: a valid full marker should read ready")
}
// What the next run does before restic touches anything.
m.clearScratchMarker(scratch)
writeScratchPayload(t, scratch) // ...and then that run dies part-way
if m.OffboxFullScratchReady("kimai") {
t.Fatal("a marker from a PREVIOUS run certified a later part-copy — the stale certificate " +
"is the whole reason the clear happens before restic, not after")
}
}
func TestR358_UnreadableMarkerFailsClosed(t *testing.T) {
var buf bytes.Buffer
m, scratch := newR358Manager(t)
m.logger = log.New(&buf, "", 0)
writeScratchPayload(t, scratch)
if err := os.WriteFile(filepath.Join(scratch, scratchMarkerName), []byte("{not json"), 0o600); err != nil {
t.Fatal(err)
}
if m.OffboxFullScratchReady("kimai") {
t.Fatal("an unparseable marker was treated as a completion certificate")
}
if !strings.Contains(buf.String(), "WARN") {
t.Errorf("a scratch refused for an unreadable marker must say so — silence makes a refusal "+
"indistinguishable from a missing download; log was %q", buf.String())
}
}
func TestR358_WrongSchemaFailsClosed(t *testing.T) {
m, scratch := newR358Manager(t)
writeScratchPayload(t, scratch)
if err := os.WriteFile(filepath.Join(scratch, scratchMarkerName),
[]byte(`{"schema":99,"snapshot_id":"s","full":true,"finished_at":"2026-08-30T00:00:00Z"}`), 0o600); err != nil {
t.Fatal(err)
}
if m.OffboxFullScratchReady("kimai") {
t.Fatal("a marker with an unrecognised schema was accepted — a format we cannot read is not a certificate")
}
}
func TestR358_CompletedFullScratchStillReady(t *testing.T) {
// The happy path is unchanged: a finished full download is still offered.
m, scratch := newR358Manager(t)
writeScratchPayload(t, scratch)
if err := m.writeScratchMarker(scratch, "snap-1", true); err != nil {
t.Fatal(err)
}
if !m.OffboxFullScratchReady("kimai") {
t.Fatal("a COMPLETED full restore is no longer offered — the fix broke the thing it protects")
}
}
func TestR358_MarkerIsWrittenAt0600AndAtomically(t *testing.T) {
m, scratch := newR358Manager(t)
if err := m.writeScratchMarker(scratch, "snap-1", true); err != nil {
t.Fatal(err)
}
fi, err := os.Stat(filepath.Join(scratch, scratchMarkerName))
if err != nil {
t.Fatal(err)
}
if fi.Mode().Perm() != 0o600 {
t.Errorf("marker mode = %v, want 0600", fi.Mode().Perm())
}
// The tmp file must not survive: a leftover .tmp beside the marker is a torn write that a later
// reader could mistake for the real thing.
if _, err := os.Stat(filepath.Join(scratch, scratchMarkerName+".tmp")); !os.IsNotExist(err) {
t.Error("the temporary marker file was left behind")
}
}
// TestR358_MarkerIsNeverPlaced pins the assumption the whole design rests on: placement is driven by
// the SNAPSHOT's own path list, not by a directory walk, so a file that exists only locally cannot be
// copied into the customer's live data. Stated as a test rather than trusted as a comment — the spec
// asked for exactly this, and "a comment asserting an invariant needs a test pinning it" is a standing
// rule earned nine times over in this project.
func TestR358_MarkerIsNeverPlaced(t *testing.T) {
const stack = "kimai"
oldNs := "/mnt/old"
scratch := t.TempDir()
liveNs := t.TempDir()
snapPaths := []string{
oldNs + "/backups/primary/" + stack,
oldNs + "/appdata/" + stack,
}
placements, err := mapOffsiteRestorePaths(snapPaths, stack, scratch, liveNs)
if err != nil {
t.Fatalf("mapOffsiteRestorePaths: %v", err)
}
if len(placements) == 0 {
t.Fatal("fixture produced no placements — the test would prove nothing")
}
for _, pl := range placements {
if strings.Contains(pl.src, scratchMarkerName) || strings.Contains(pl.dst, scratchMarkerName) {
t.Fatalf("the completion marker entered a placement (src=%q dst=%q) — it would be copied "+
"into the customer's live data", pl.src, pl.dst)
}
}
}
// TestR358_MarkerIsClearedBeforeResticAndWrittenAfter walks the AST of RestoreOffboxScratch.
//
// It exists because `resticStep` is not a seam — a test cannot run the real download without restic,
// so the ORDER of the three calls cannot be proven by execution here. Order is the entire safety
// property: a marker written before restic certifies a download that has not happened, and a clear
// that runs after it leaves a stale certificate covering a fresh part-copy. A substring search would
// not do: a commented-out call satisfies strings.Contains, which a sibling test in this project
// records paying for.
func TestR358_MarkerIsClearedBeforeResticAndWrittenAfter(t *testing.T) {
fset := token.NewFileSet()
f, err := parser.ParseFile(fset, "offbox_restore.go", nil, 0)
if err != nil {
t.Fatalf("parse offbox_restore.go: %v", err)
}
var body *ast.BlockStmt
for _, d := range f.Decls {
if fn, ok := d.(*ast.FuncDecl); ok && fn.Name.Name == "RestoreOffboxScratch" && fn.Body != nil {
body = fn.Body
}
}
if body == nil {
t.Fatal("RestoreOffboxScratch not found")
}
var order []string
ast.Inspect(body, func(n ast.Node) bool {
call, ok := n.(*ast.CallExpr)
if !ok {
return true
}
if sel, ok := call.Fun.(*ast.SelectorExpr); ok {
switch sel.Sel.Name {
case "clearScratchMarker", "resticStep", "writeScratchMarker":
order = append(order, sel.Sel.Name)
}
}
return true
})
idx := func(name string) int {
for i, n := range order {
if n == name {
return i
}
}
return -1
}
clear, restic, write := idx("clearScratchMarker"), idx("resticStep"), idx("writeScratchMarker")
if clear < 0 || restic < 0 || write < 0 {
t.Fatalf("RestoreOffboxScratch does not call all three (order seen: %v) — the marker is not wired", order)
}
if !(clear < restic) {
t.Errorf("the stale marker is cleared AFTER restic runs (order %v) — a previous run's "+
"certificate would cover this run's part-copy", order)
}
if !(restic < write) {
t.Errorf("the marker is written BEFORE restic returns (order %v) — that certifies a download "+
"that has not happened, which is the defect with an extra step", order)
}
}