diff --git a/CHANGELOG.md b/CHANGELOG.md index a7e0ceb..4f710d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,73 @@ ## Changelog +### v0.147.0 — feedback slice 1: pressing a button says something (2026-07-19) + +Green: `go build ./... && go vet ./... && go test ./...` all pass (23 packages); +`template_id_gate` + `emoji_gate` + `native_confirm_gate` + `offbox_rename_gate` + +`app_row_dedup_gate` + `mojibake_gate` all PASS. (`docker_run_volume_path_gate` fails on +`internal/appexport/estimate.go:179` — **pre-existing on HEAD, untouched by this release**; verified +by stashing this work and re-running.) + +**The systemic complaint, twice in one evening: you press a button and nothing happens.** No +progress, no ETA, no named result. This slice fixes the three worst offenders using the two patterns +already in the codebase (the deploy 3-step panel and the storage-init status poll). It deliberately +does **not** introduce a feedback framework — that is a ROADMAP item ("unified async-job feedback"), +because three targeted cards are worth shipping tonight and a framework is not. + +- **4a — a verification restore now names its result.** The completion flash said the app had been + restored „ellenőrző mappába a meghajtón" — *which* folder, on *which* drive, was invisible, so the + customer could not go and look at the thing they had just asked for. It now carries the **full + path**. The restore page gained a **„Meglévő ellenőrző másolatok"** listing (app · size · date · + path) — until now nothing anywhere showed what these restores had accumulated, so they piled up + and the only way to find them was SSH — each with a double-confirmed **„Másolat törlése"**. + - That delete is the **only** delete this release adds, so it names a STACK, never a path: the + Manager resolves the name inside a `backups/offsite-restore` root it computed itself and refuses + anything landing outside. Red-proofed — neutralise the name guard and `stack: ""` resolves to the + offsite-restore ROOT and takes every copy with it. Every refusal is asserted as a **non-effect** + (the neighbouring copy and the live data are still on disk afterwards). + - `backups/offsite-restore` was open-coded in three places; it now has one home + (`offsiteRestoreRootFor`), and a test pins the path in the flash to the path in the listing so + the customer can never be told about a directory the page cannot show or remove. + +- **4b — Megosztás enable shows what it is waiting for.** Enabling sharing ran `ReconcileSamba()` + **synchronously inside the POST handler**. On a box whose golden had not baked `felhom-samba` that + is `compose up -d` pulling ~100MB from a private registry: minutes of an apparently-hung form post, + then „Beállítás mentve." whether or not anything had come up. Now detached + polled, with a card + that distinguishes **„képfájl letöltése"** (image genuinely absent — the multi-minute case) from + **„indítás"** (already baked — seconds). The distinction is decided *before* the work starts, + because afterwards the image is always present and the card could never truthfully say „letöltés". + - Success is **probed, not inferred**: `compose up -d` exits 0 on a crash-loop, so the terminal + state is container liveness. `nil` from reconcile also covers "deliberately deployed nothing + because there is no household password yet", which now gets its own message instead of a card + spinning forever. + - The **password** form starts the same job — with `UserSet` false reconcile deploys nothing, so on + a fresh box *that*, not the enable toggle, is where the pull actually happens. + +- **4c — „Távoli mentés most" streams real progress.** restic was already reporting bytes and + percentages; the runner seam used `CombinedOutput()` and threw them away. The manual run now passes + `--json`, scans stdout line-by-line, and the page shows **total bytes, percent and the app + currently being pushed**. Before the scan finishes it says „a mentendő adatok felmérése…" rather + than pinning a bar at 0%, which reads as stuck. + - **Manual only.** The nightly run stays silent and its output format is untouched — pinned by a + test that fails if the scheduled path ever passes `--json` or publishes progress. + - The poll now **arms unconditionally**. It used to start only if the page already rendered „Fut…", + which loses a race the manual trigger always runs: the POST redirects and the page renders before + the detached goroutine writes `LastStatus=running`, so the poll never armed and the customer + watched a static page during the very run they had just started. + - Red-proofed twice, both confirmed: break the parser → the percent assertion fails; drop the + wiring → the `--json` assertion fails. The `--json` stream is tail-bounded (40 lines) so a large + backup does not buffer megabytes of status spam for error diagnosis. + +- **Golden/controller infra-image drift closed at the source (supports the agent-side change).** + `infra.Images()` derives the list from the existing pins, `--print-infra-images` prints it, and the + golden bake now asks the controller binary it is about to bake instead of carrying its own copy. + The copy had already drifted: `felhom-samba` was never added to it, so the golden baked 3 of 4 — + which is *why* enabling Megosztás pulled at runtime. A test parses the const block out of the + source and fails if a pin is added without reaching `Images()`. + +**Live-validated** on demo guest 9201 through the real UI. **No floor change** — Viktor decides floor +timing. + ### v0.146.0 — nav polish: styled scrollbars + collapsible sidebar groups (2026-07-18) UI-only; no behavioural or backup/restore surface touched. Green: diff --git a/controller/cmd/controller/main.go b/controller/cmd/controller/main.go index ad1173f..e96373f 100644 --- a/controller/cmd/controller/main.go +++ b/controller/cmd/controller/main.go @@ -30,6 +30,7 @@ import ( cf "gitea.dooplex.hu/admin/felhom-controller/internal/cloudflare" "gitea.dooplex.hu/admin/felhom-controller/internal/config" "gitea.dooplex.hu/admin/felhom-controller/internal/crypto" + "gitea.dooplex.hu/admin/felhom-controller/internal/infra" "gitea.dooplex.hu/admin/felhom-controller/internal/integrations" "gitea.dooplex.hu/admin/felhom-controller/internal/mailrelay" "gitea.dooplex.hu/admin/felhom-controller/internal/metrics" @@ -69,6 +70,7 @@ func main() { configPath := flag.String("config", "/opt/docker/felhom-controller/controller.yaml", "Path to configuration file") showVersion := flag.Bool("version", false, "Show version and exit") printResetCode := flag.Bool("print-reset-code", false, "Customer-claim escape hatch (v0.122.0, F-4): print a fresh one-time local claim/reset code to stdout, then exit. Root-gated by reachability (docker exec). Same gate consumes it.") + printInfraImages := flag.Bool("print-infra-images", false, "Print every controller-managed infra image (one per line) and exit. Read by the golden bake (felhom-agent configs/build-golden.sh) so the appliance image pre-pulls exactly what THIS controller version will request.") flag.Parse() if *showVersion { @@ -76,6 +78,15 @@ func main() { os.Exit(0) } + // Config-free by design: the golden bake runs this against a bare `docker run` of the controller + // image, where no controller.yaml, data dir or settings exist yet. It must never touch either. + if *printInfraImages { + for _, img := range infra.Images() { + fmt.Println(img) + } + os.Exit(0) + } + if *printResetCode { cfg, err := config.LoadPermissive(*configPath) if err != nil { diff --git a/controller/internal/backup/backup.go b/controller/internal/backup/backup.go index 8a9c373..c2e440b 100644 --- a/controller/internal/backup/backup.go +++ b/controller/internal/backup/backup.go @@ -35,6 +35,11 @@ type Manager struct { // offbox (Part B): the restic-SFTP exec seam (nil → real restic) + the failure→operator-alert hook. offboxRunner offboxRunner offboxNotify func(dur time.Duration, snapshots int, err error) + // offboxStreamRunner + offboxProgress: the MANUAL run's live progress (v0.147.0, 4c). The stream + // seam scans restic's `--json` stdout line-by-line; the state is what the page polls. Both are + // inert on the nightly path — the sink is installed only for the duration of a manual run. + offboxStreamRunner offboxStreamRunner + offboxProgress offboxProgressState // offboxOrphanEvent (v0.142.0), if set, pushes a hub event on offsite-repo continuity transitions // ("offbox_repo_orphaned" / "offbox_repo_reset"); renamedTo names the move-aside path (reset only). // Wired in main.go to the notifier. Nil-safe. diff --git a/controller/internal/backup/offbox.go b/controller/internal/backup/offbox.go index a64564f..7a40bfa 100644 --- a/controller/internal/backup/offbox.go +++ b/controller/internal/backup/offbox.go @@ -572,6 +572,22 @@ func (m *Manager) ensureOffboxRepo(ctx context.Context, base, env []string) erro // tars) to the SFTP repo, then prunes per the retention policy. Single-flight + migration-guarded. A // failure (incl. a fail-fast dead-NAS error) records status + alerts the operator. Returns the first error. func (m *Manager) RunOffboxBackup(ctx context.Context) error { + return m.runOffboxBackup(ctx, false) +} + +// RunOffboxBackupWithProgress is the MANUAL („Távoli mentés most") entry point: identical work, but +// with the live progress sink installed so the page can show total bytes, percent and current app +// (v0.147.0, 4c). The nightly scheduled run keeps calling RunOffboxBackup and stays silent — nobody +// is watching a progress bar at 03:00, and a sink left installed would publish stale percentages +// into a page that never asked for them. +func (m *Manager) RunOffboxBackupWithProgress(ctx context.Context) error { + return m.runOffboxBackup(ctx, true) +} + +func (m *Manager) runOffboxBackup(ctx context.Context, withProgress bool) error { + if withProgress { + defer m.beginManualProgress()() + } if !m.OffboxConfigured() { return fmt.Errorf("off-box backup not configured") } @@ -890,7 +906,9 @@ func (m *Manager) runOffboxInternal(ctx context.Context, apps, base, env []strin } args := append([]string{"backup", "--tag", "felhom-offbox", "--tag", stack, src}, extra...) bctx, cancel := context.WithTimeout(ctx, offboxBackupTimeout) - out, berr := m.resticStep(bctx, env, base, "backup:"+stack, args...) + // Streaming twin (4c): identical to resticStep unless a MANUAL run installed a progress sink, + // in which case it adds --json and feeds the parsed status lines to the page's poll. + out, berr := m.resticBackupStep(bctx, env, base, "backup:"+stack, stack, args...) cancel() if berr != nil { m.logger.Printf("[ERROR] [offbox] backup %s failed: %v: %s", stack, berr, truncate(out)) diff --git a/controller/internal/backup/offbox_progress.go b/controller/internal/backup/offbox_progress.go new file mode 100644 index 0000000..983d3c1 --- /dev/null +++ b/controller/internal/backup/offbox_progress.go @@ -0,0 +1,257 @@ +package backup + +import ( + "bufio" + "context" + "encoding/json" + "io" + "os" + "os/exec" + "sync" + "time" +) + +// Offsite backup progress (v0.147.0, feedback slice 4c). +// +// THE PROBLEM: „Távoli mentés most" started a background restic run and redirected with „A távoli +// mentés elindult". After that the page polled a status field whose only values were running / ok / +// error. For a first offsite push of tens of gigabytes over SFTP that is 20+ minutes of a spinner +// with no total, no percentage and no indication of WHICH app is being pushed — indistinguishable +// from a hang. +// +// restic already reports all of it: `backup --json` writes newline-delimited status objects to +// stdout. We only had to stop throwing them away — the existing runner seam uses CombinedOutput(), +// which buffers everything until exit. +// +// SCOPE: the MANUAL trigger only. The nightly scheduled run stays silent (nobody is watching a +// progress bar at 03:00, and a sink left installed would keep publishing stale percentages into a +// page that never asked). The sink is installed for the duration of a manual run and cleared after. + +// OffboxProgress is a snapshot of an in-flight manual offsite backup. +type OffboxProgress struct { + Active bool `json:"active"` + CurrentApp string `json:"current_app"` + Percent float64 `json:"percent"` // 0..100 + BytesDone int64 `json:"bytes_done"` + TotalBytes int64 `json:"total_bytes"` + DoneHuman string `json:"done_human"` + TotalHuman string `json:"total_human"` +} + +// resticStatusLine is the subset of restic's `--json` status object we consume. restic emits several +// message_types (status, summary, error, verbose_status); anything that is not "status" is ignored +// here rather than treated as garbage, because restic adds new types between versions and an unknown +// type must never break the run. +type resticStatusLine struct { + MessageType string `json:"message_type"` + PercentDone float64 `json:"percent_done"` // 0..1 + TotalBytes int64 `json:"total_bytes"` + BytesDone int64 `json:"bytes_done"` +} + +// parseResticStatus parses ONE line of restic --json output into a progress delta. +// +// Kept as a pure function precisely so it can be tested without restic, a network, or a repo — the +// parser is the part that silently rots when restic changes its output, and a progress bar that +// quietly stops moving is worse than no progress bar at all. +func parseResticStatus(line string) (pct float64, done, total int64, ok bool) { + var s resticStatusLine + if err := json.Unmarshal([]byte(line), &s); err != nil { + return 0, 0, 0, false + } + if s.MessageType != "status" { + return 0, 0, 0, false + } + pct = s.PercentDone * 100 + if pct < 0 { + pct = 0 + } + if pct > 100 { + pct = 100 + } + return pct, s.BytesDone, s.TotalBytes, true +} + +// offboxProgressState is the published snapshot, guarded independently of the Manager mutex so a +// poll never blocks behind the running backup. +type offboxProgressState struct { + mu sync.Mutex + cur OffboxProgress + live bool +} + +func (p *offboxProgressState) begin() { + p.mu.Lock() + p.cur = OffboxProgress{Active: true} + p.live = true + p.mu.Unlock() +} + +func (p *offboxProgressState) end() { + p.mu.Lock() + p.cur = OffboxProgress{} + p.live = false + p.mu.Unlock() +} + +func (p *offboxProgressState) setApp(app string) { + p.mu.Lock() + if p.live { + // A new app resets the byte counters: restic's percentages are per-invocation, and carrying + // the previous app's 100% into the next app's start would show a bar that jumps backwards. + p.cur.CurrentApp = app + p.cur.Percent, p.cur.BytesDone, p.cur.TotalBytes = 0, 0, 0 + p.cur.DoneHuman, p.cur.TotalHuman = "", "" + } + p.mu.Unlock() +} + +func (p *offboxProgressState) update(pct float64, done, total int64) { + p.mu.Lock() + if p.live { + p.cur.Percent, p.cur.BytesDone, p.cur.TotalBytes = pct, done, total + p.cur.DoneHuman, p.cur.TotalHuman = humanizeBytes(done), humanizeBytes(total) + } + p.mu.Unlock() +} + +func (p *offboxProgressState) snapshot() OffboxProgress { + p.mu.Lock() + defer p.mu.Unlock() + return p.cur +} + +// OffboxProgressSnapshot is the poll surface for the „Távoli mentés" page. +func (m *Manager) OffboxProgressSnapshot() OffboxProgress { return m.offboxProgress.snapshot() } + +// offboxStreamRunner is the streaming restic-exec seam: like offboxRunner, but calls onLine for each +// stdout line AS IT ARRIVES instead of only returning the buffered output at exit. Tests inject a +// fake that emits canned `--json` status lines, so the whole progress path is exercised without +// restic, a network or a repo. +type offboxStreamRunner func(ctx context.Context, env []string, onLine func(string), args ...string) ([]byte, error) + +// SetOffboxStreamRunner installs the streaming seam (nil → the real streaming exec). +func (m *Manager) SetOffboxStreamRunner(r offboxStreamRunner) { m.offboxStreamRunner = r } + +func (m *Manager) streamRunner() offboxStreamRunner { + if m.offboxStreamRunner != nil { + return m.offboxStreamRunner + } + return defaultOffboxStreamRunner +} + +// defaultOffboxStreamRunner runs restic with stdout scanned line-by-line. stderr is captured whole +// (restic's --json progress goes to stdout; errors go to stderr) and appended to the returned output +// so callers keep the same error-diagnosis material CombinedOutput gave them. +func defaultOffboxStreamRunner(ctx context.Context, env []string, onLine func(string), args ...string) ([]byte, error) { + cmd := exec.CommandContext(ctx, "restic", args...) + cmd.Env = append(os.Environ(), env...) + + stdout, err := cmd.StdoutPipe() + if err != nil { + return nil, err + } + var stderr syncBuf + cmd.Stderr = &stderr + + if err := cmd.Start(); err != nil { + return nil, err + } + + var tail lineTail + scanner := bufio.NewScanner(stdout) + // restic status lines are small, but a --json summary listing many paths can exceed the 64KB + // default; a scanner that dies mid-run would silently freeze the progress bar. + scanner.Buffer(make([]byte, 0, 64*1024), 4*1024*1024) + for scanner.Scan() { + line := scanner.Text() + tail.add(line) + if onLine != nil { + onLine(line) + } + } + _, _ = io.Copy(io.Discard, stdout) + + werr := cmd.Wait() + // Keep only the tail of stdout: the full --json stream of a large backup is megabytes of status + // spam, and every caller uses this output for error diagnosis (and lock-pattern matching) only. + out := append(tail.bytes(), stderr.bytes()...) + return out, werr +} + +// lineTail keeps the last N lines seen, so error diagnosis has context without buffering the whole +// --json stream. +type lineTail struct { + lines []string +} + +func (t *lineTail) add(s string) { + const keep = 40 + t.lines = append(t.lines, s) + if len(t.lines) > keep { + t.lines = t.lines[len(t.lines)-keep:] + } +} + +func (t *lineTail) bytes() []byte { + var b []byte + for _, l := range t.lines { + b = append(b, l...) + b = append(b, '\n') + } + return b +} + +type syncBuf struct { + mu sync.Mutex + b []byte +} + +func (s *syncBuf) Write(p []byte) (int, error) { + s.mu.Lock() + s.b = append(s.b, p...) + s.mu.Unlock() + return len(p), nil +} + +func (s *syncBuf) bytes() []byte { + s.mu.Lock() + defer s.mu.Unlock() + return append([]byte{}, s.b...) +} + +// resticBackupStep is resticStep's streaming twin, used ONLY by the app-backup leg when a manual run +// has a progress sink installed. It keeps resticStep's crash-lock self-heal semantics by delegating +// the retry path to resticStep (a retry after an unlock is rare and does not need progress). +func (m *Manager) resticBackupStep(ctx context.Context, env, base []string, label, app string, args ...string) ([]byte, error) { + if !m.offboxProgress.snapshot().Active { + return m.resticStep(ctx, env, base, label, args...) // nightly / no watcher: unchanged path + } + m.offboxProgress.setApp(app) + full := append(append([]string{}, base...), args...) + // --json turns on the machine-readable progress stream. It is added ONLY here, so the nightly + // run's output format (and everything that greps it) is untouched. + full = append(full, "--json") + out, err := m.streamRunner()(ctx, env, func(line string) { + if pct, done, total, ok := parseResticStatus(line); ok { + m.offboxProgress.update(pct, done, total) + } + }, full...) + if err == nil || !offboxLockRe.Match(out) { + return out, err + } + // Lock collision: fall back to the non-streaming step, which owns the unlock --remove-all + // self-heal. Progress stalls for that one retry; correctness beats a moving bar. + m.logger.Printf("[WARN] [offbox] %s hit a lock during a manual run — retrying via the self-healing step", label) + return m.resticStep(ctx, env, base, label, args...) +} + +// beginManualProgress installs the progress sink for a manual run and returns the cleanup func. +func (m *Manager) beginManualProgress() func() { + m.offboxProgress.begin() + started := time.Now() + return func() { + m.logger.Printf("[INFO] [offbox] manual run progress reporting ended after %s", time.Since(started).Round(time.Second)) + m.offboxProgress.end() + } +} diff --git a/controller/internal/backup/offbox_progress_test.go b/controller/internal/backup/offbox_progress_test.go new file mode 100644 index 0000000..a9d0528 --- /dev/null +++ b/controller/internal/backup/offbox_progress_test.go @@ -0,0 +1,228 @@ +package backup + +import ( + "context" + "strings" + "testing" +) + +// v0.147.0 slice 4c — live progress for a MANUAL offsite run. +// +// These tests exercise the WHOLE path a real run takes: a fake restic emits canned `--json` status +// lines through the streaming seam, and the assertion is on what the poll surface +// (OffboxProgressSnapshot) reports — not on the parser in isolation. A parser that works but is +// never wired to the snapshot would leave the customer looking at the same silent spinner, which is +// the bug being fixed. +// +// RED-PROOF (run manually, both confirmed to fail): +// 1. break the parser — flip `s.MessageType != "status"` to `== "status"` in parseResticStatus: +// TestManualRunReportsParsedProgress fails ("percent = 0, want 42"). +// 2. drop the wiring — make resticBackupStep always delegate to resticStep: +// the same test fails (no --json, no stream, no snapshot). + +// jsonStatus is one restic --json status line. +func jsonStatus(pct float64, done, total int64) string { + return `{"message_type":"status","percent_done":` + ftoa(pct) + `,"total_bytes":` + itoa(total) + `,"bytes_done":` + itoa(done) + `}` +} + +func ftoa(f float64) string { + // small helper — avoids strconv import noise in the canned lines + switch f { + case 0: + return "0" + case 0.42: + return "0.42" + case 1: + return "1" + } + return "0.5" +} + +func itoa(i int64) string { + if i == 0 { + return "0" + } + var b []byte + neg := i < 0 + if neg { + i = -i + } + for i > 0 { + b = append([]byte{byte('0' + i%10)}, b...) + i /= 10 + } + if neg { + return "-" + string(b) + } + return string(b) +} + +// TestManualRunReportsParsedProgress is the headline: a manual run driven by a fake restic that emits +// --json status lines must make OffboxProgressSnapshot report the parsed percentage, byte counts and +// the app currently being pushed. +func TestManualRunReportsParsedProgress(t *testing.T) { + e := newSharesOffboxEnv(t, "immich") + if err := e.sett.SetSMBEnabled(false); err != nil { // keep this test to the app leg only + t.Fatal(err) + } + + var seen []OffboxProgress + var sawJSONFlag bool + e.m.SetOffboxStreamRunner(func(ctx context.Context, env []string, onLine func(string), args ...string) ([]byte, error) { + for _, a := range args { + if a == "--json" { + sawJSONFlag = true + } + } + // Emit a scan phase (total not yet known), then real progress, sampling the published + // snapshot after each line exactly as the polling page would. + onLine(jsonStatus(0, 0, 0)) + seen = append(seen, e.m.OffboxProgressSnapshot()) + onLine(`{"message_type":"verbose_status","action":"unchanged"}`) // must be ignored, not fatal + onLine(jsonStatus(0.42, 4200, 10000)) + seen = append(seen, e.m.OffboxProgressSnapshot()) + return []byte(`{"message_type":"summary","snapshot_id":"abc"}`), nil + }) + // The non-streaming seam still serves every other restic call (cat config, forget, snapshots…). + e.m.SetOffboxRunner(func(ctx context.Context, env []string, args ...string) ([]byte, error) { + if contains(args, "snapshots") { + return []byte(`[]`), nil + } + return []byte(""), nil + }) + + if err := e.m.RunOffboxBackupWithProgress(context.Background()); err != nil { + t.Fatalf("manual run: %v", err) + } + + if !sawJSONFlag { + t.Fatal("the manual backup leg did not pass --json to restic — nothing could ever be parsed") + } + if len(seen) != 2 { + t.Fatalf("expected 2 sampled snapshots, got %d", len(seen)) + } + + // While restic is still scanning, total is unknown: report 0 rather than inventing a percentage. + if seen[0].TotalBytes != 0 || seen[0].Percent != 0 { + t.Errorf("scan phase: got %+v, want zeroed counters", seen[0]) + } + if seen[0].CurrentApp != "immich" { + t.Errorf("scan phase: current_app = %q, want %q", seen[0].CurrentApp, "immich") + } + + got := seen[1] + if got.Percent != 42 { + t.Errorf("percent = %v, want 42", got.Percent) + } + if got.BytesDone != 4200 || got.TotalBytes != 10000 { + t.Errorf("bytes = %d/%d, want 4200/10000", got.BytesDone, got.TotalBytes) + } + if got.CurrentApp != "immich" { + t.Errorf("current_app = %q, want %q", got.CurrentApp, "immich") + } + if got.TotalHuman == "" || got.DoneHuman == "" { + t.Errorf("humanized byte strings not populated: %+v", got) + } + if !got.Active { + t.Error("progress reported inactive during a manual run") + } + + // After the run the sink must be torn down, or the page would keep rendering a stale bar. + if after := e.m.OffboxProgressSnapshot(); after.Active || after.Percent != 0 { + t.Errorf("progress still published after the run: %+v", after) + } +} + +// TestNightlyRunStaysSilent pins the scope decision: the scheduled run must neither install the sink +// nor pass --json, so its output format (and everything that greps it) is untouched. +func TestNightlyRunStaysSilent(t *testing.T) { + e := newSharesOffboxEnv(t, "immich") + if err := e.sett.SetSMBEnabled(false); err != nil { + t.Fatal(err) + } + e.m.SetOffboxStreamRunner(func(ctx context.Context, env []string, onLine func(string), args ...string) ([]byte, error) { + t.Fatal("the nightly run used the streaming runner — progress must be manual-only") + return nil, nil + }) + var backupArgs []string + e.m.SetOffboxRunner(func(ctx context.Context, env []string, args ...string) ([]byte, error) { + if contains(args, "backup") { + backupArgs = append([]string{}, args...) + } + if contains(args, "snapshots") { + return []byte(`[]`), nil + } + return []byte(""), nil + }) + + if err := e.m.RunOffboxBackup(context.Background()); err != nil { + t.Fatalf("nightly run: %v", err) + } + if backupArgs == nil { + t.Fatal("no backup call was made") + } + if contains(backupArgs, "--json") { + t.Errorf("the nightly run passed --json: %v", backupArgs) + } + if p := e.m.OffboxProgressSnapshot(); p.Active { + t.Errorf("the nightly run published progress: %+v", p) + } +} + +// TestParseResticStatusIgnoresNonStatus covers the lines restic actually interleaves with status +// output. An unknown message_type must be ignored, never mistaken for progress — restic adds new +// types between versions. +func TestParseResticStatusIgnoresNonStatus(t *testing.T) { + for _, line := range []string{ + `{"message_type":"summary","total_bytes_processed":123}`, + `{"message_type":"verbose_status","action":"new"}`, + `{"message_type":"error","error":{"message":"boom"}}`, + `not json at all`, + ``, + `{}`, + } { + if _, _, _, ok := parseResticStatus(line); ok { + t.Errorf("parsed a non-status line as progress: %q", line) + } + } + pct, done, total, ok := parseResticStatus(jsonStatus(0.42, 4200, 10000)) + if !ok { + t.Fatal("a real status line was not parsed") + } + if pct != 42 || done != 4200 || total != 10000 { + t.Errorf("got %v %d %d, want 42 4200 10000", pct, done, total) + } +} + +// TestParseResticStatusClampsPercent — restic has been seen to report percent_done slightly above 1 +// near completion. A bar wider than its track is a visible bug. +func TestParseResticStatusClampsPercent(t *testing.T) { + pct, _, _, ok := parseResticStatus(`{"message_type":"status","percent_done":1.04}`) + if !ok || pct != 100 { + t.Errorf("percent = %v (ok=%v), want clamped to 100", pct, ok) + } + pct, _, _, ok = parseResticStatus(`{"message_type":"status","percent_done":-0.2}`) + if !ok || pct != 0 { + t.Errorf("percent = %v (ok=%v), want clamped to 0", pct, ok) + } +} + +// TestLineTailKeepsOnlyTheTail — the --json stream of a large backup is megabytes of status spam, and +// every caller uses this output for error diagnosis and lock-pattern matching. Buffering all of it +// would be a memory leak proportional to backup size. +func TestLineTailKeepsOnlyTheTail(t *testing.T) { + var tl lineTail + for i := 0; i < 500; i++ { + tl.add("line-" + itoa(int64(i))) + } + out := string(tl.bytes()) + if strings.Contains(out, "line-0\n") { + t.Error("the oldest line survived — the tail is unbounded") + } + if !strings.Contains(out, "line-499") { + t.Error("the newest line was dropped") + } + if n := strings.Count(out, "\n"); n > 64 { + t.Errorf("tail kept %d lines, want a small bounded number", n) + } +} diff --git a/controller/internal/backup/offbox_restore.go b/controller/internal/backup/offbox_restore.go index 6f259cd..1908521 100644 --- a/controller/internal/backup/offbox_restore.go +++ b/controller/internal/backup/offbox_restore.go @@ -128,9 +128,10 @@ func (m *Manager) offboxSnapshotSize(ctx context.Context, id string) (int64, err // probe). NEVER cfg.Paths.DataDir (the rootfs — the F-A1 filler). App's HDD drive first; else the first // schedulable storage path; else a Hungarian refusal. func (m *Manager) offboxRestoreScratchDir(stack string) (scratch, nsRoot string, err error) { + // offsiteRestoreRootFor is THE place `backups/offsite-restore` is spelled (offbox_verify_copies.go) + // — the listing/delete surface must resolve byte-identical paths to the ones written here. scratchFor := func(root string) (string, string) { - nr := m.namespaceRoot(root) - return filepath.Join(nr, "backups", "offsite-restore", stack), nr + return filepath.Join(m.offsiteRestoreRootFor(root), stack), m.namespaceRoot(root) } isNet := func(path string) bool { return m.settings != nil && m.settings.IsNetworkStoragePath(path) } // (1) the app's own drive — preferred, but ONLY if it is not NETWORK storage (F-3afix-1). restic diff --git a/controller/internal/backup/offbox_verify_copies.go b/controller/internal/backup/offbox_verify_copies.go new file mode 100644 index 0000000..910a1a1 --- /dev/null +++ b/controller/internal/backup/offbox_verify_copies.go @@ -0,0 +1,145 @@ +package backup + +import ( + "fmt" + "os" + "path/filepath" + "sort" + "strings" + "time" +) + +// Verification copies — the listing/delete surface for `/backups/offsite-restore/`. +// +// WHY THIS EXISTS (v0.147.0, feedback slice 4a): an offsite verification restore wrote its result to +// a path the customer was never told, and nothing anywhere listed what had accumulated. Pressing +// „Ellenőrző visszaállítás" produced a flash saying it had been restored "to a verification folder +// on the drive" — which folder, on which drive, and how much space it was now using were all +// invisible. So copies piled up and the only way to find them was SSH. +// +// The path segments were already open-coded in three places; offsiteRestoreRootFor() is now the one +// place `backups/offsite-restore` is spelled, and offboxRestoreScratchDir() builds on it. + +// OffsiteRestoreCopy is one verification copy on disk. +type OffsiteRestoreCopy struct { + Stack string `json:"stack"` // app slug, or SharesPseudoStack for the shares copy + Path string `json:"path"` // absolute path — the thing the customer could not see + Size int64 `json:"size"` // bytes + SizeHuman string `json:"size_human"` // pre-humanized for the template + Created time.Time `json:"created"` // dir mtime; restic writes the tree once, so this is the restore time +} + +// offsiteRestoreRootFor returns `/backups/offsite-restore` for a drive path. THE single place +// these segments are written. +func (m *Manager) offsiteRestoreRootFor(drivePath string) string { + return filepath.Join(m.namespaceRoot(drivePath), "backups", "offsite-restore") +} + +// offsiteRestoreDriveRoots returns every drive path a verification copy could live under, in the same +// preference order offboxRestoreScratchDir uses to CHOOSE one — so listing can never miss a copy the +// restore path was capable of creating. Deduplicated, order preserved. +func (m *Manager) offsiteRestoreDriveRoots() []string { + seen := map[string]bool{} + var roots []string + add := func(p string) { + p = strings.TrimSpace(p) + if p == "" || seen[p] { + return + } + seen[p] = true + roots = append(roots, p) + } + // App HDDs first (offboxRestoreScratchDir's rule 1), then every schedulable path (rules 2 and 3). + if m.stackProvider != nil { + for _, s := range m.stackProvider.ListDeployedStacks() { + add(m.stackProvider.GetStackHDDPath(s.Name)) + } + } + if m.settings != nil { + for _, sp := range m.settings.GetSchedulableStoragePaths() { + add(sp.Path) + } + } + return roots +} + +// ListOffsiteRestoreCopies enumerates every verification copy across every candidate drive, newest +// first. Missing directories are not an error — "none yet" is the normal state. +func (m *Manager) ListOffsiteRestoreCopies() []OffsiteRestoreCopy { + sizer := m.offboxSize() + var out []OffsiteRestoreCopy + seen := map[string]bool{} + for _, drive := range m.offsiteRestoreDriveRoots() { + root := m.offsiteRestoreRootFor(drive) + entries, err := os.ReadDir(root) + if err != nil { + continue // no copies on this drive (or the drive is not mounted) — not an error + } + for _, e := range entries { + if !e.IsDir() { + continue + } + p := filepath.Join(root, e.Name()) + if seen[p] { + continue // two stacks can resolve to the same drive; list each path once + } + seen[p] = true + c := OffsiteRestoreCopy{Stack: e.Name(), Path: p} + if fi, err := e.Info(); err == nil { + c.Created = fi.ModTime() + } + c.Size = sizer(p) + c.SizeHuman = humanizeBytes(c.Size) + out = append(out, c) + } + } + sort.Slice(out, func(i, j int) bool { return out[i].Created.After(out[j].Created) }) + return out +} + +// DeleteOffsiteRestoreCopy removes ONE verification copy. +// +// This is the only delete path v0.147.0 adds, so it is guarded twice over. The stack name must pass +// isSafeStackName (no separators, no traversal), and the resolved path must sit STRICTLY INSIDE a +// `backups/offsite-restore` root that this Manager itself computed — a path that merely looks right +// is refused. Both checks are on the RESOLVED path, not the input, so a symlinked scratch cannot +// walk the delete out of the sandbox. +func (m *Manager) DeleteOffsiteRestoreCopy(stack string) error { + if !isSafeStackName(stack) { + return fmt.Errorf("érvénytelen alkalmazásnév") + } + for _, drive := range m.offsiteRestoreDriveRoots() { + root := m.offsiteRestoreRootFor(drive) + target := filepath.Join(root, stack) + + fi, err := os.Stat(target) + if err != nil || !fi.IsDir() { + continue + } + // Prefix safety: only ever remove strictly inside `backups/offsite-restore/`. Same shape as + // the F5 stale-primary prune (backup.go) — refuse loudly rather than best-effort skip, since + // reaching here with an out-of-sandbox path means a helper above is wrong. + cleanTarget := filepath.Clean(target) + cleanRoot := filepath.Clean(root) + string(filepath.Separator) + if !strings.HasPrefix(cleanTarget+string(filepath.Separator), cleanRoot) { + m.logger.Printf("[WARN] [offbox] refusing to delete verification copy outside %s: %s", root, cleanTarget) + return fmt.Errorf("a törlés útvonala kívül esik az ellenőrző mappán") + } + if err := os.RemoveAll(cleanTarget); err != nil { + return fmt.Errorf("a másolat törlése nem sikerült: %w", err) + } + m.logger.Printf("[INFO] [offbox] deleted verification copy: %s", cleanTarget) + return nil + } + return fmt.Errorf("nincs ilyen ellenőrző másolat") +} + +// OffsiteRestoreScratchPath exposes WHERE a verification restore for stack would land, so the UI can +// name the full path in the completion message instead of saying "a verification folder somewhere". +func (m *Manager) OffsiteRestoreScratchPath(stack string) string { + scratch, _, err := m.offboxRestoreScratchDir(stack) + if err != nil { + return "" + } + return scratch +} diff --git a/controller/internal/backup/offbox_verify_copies_test.go b/controller/internal/backup/offbox_verify_copies_test.go new file mode 100644 index 0000000..07424dd --- /dev/null +++ b/controller/internal/backup/offbox_verify_copies_test.go @@ -0,0 +1,181 @@ +package backup + +import ( + "os" + "path/filepath" + "testing" +) + +// v0.147.0 slice 4a — the verification-copy listing/delete surface. +// +// DeleteOffsiteRestoreCopy is the ONLY delete this slice adds, so these tests are about what it must +// REFUSE as much as what it must do. Every refusal is asserted as a NON-EFFECT: the neighbouring copy +// and the customer's live data must still be on disk afterwards. A guard that returns an error but +// deletes anyway passes a naive test and loses data. +// +// RED-PROOF (run manually, confirmed): neutralise the isSafeStackName check in +// DeleteOffsiteRestoreCopy and TestDeleteVerifyCopyRefusesUnsafeNames fails hard — `stack: ""` +// resolves to the offsite-restore ROOT and os.RemoveAll takes every verification copy with it. That +// is the failure this guard exists to prevent, and it is data loss, not a bad error message. +// +// The HasPrefix containment check inside DeleteOffsiteRestoreCopy could NOT be red-proofed +// independently: with isSafeStackName in front of it, no input this API accepts can reach it with an +// escaping path, so removing it leaves every test green (and the variable unused). It is deliberate +// defence-in-depth against a future caller or a refactor that loosens the name check — kept, but +// honestly labelled here as unproven-by-test rather than pretending to a red-proof it does not have. + +// verifyCopyEnv wires a manager with one drive and materialised verification copies. +type verifyCopyEnv struct { + m *Manager + drive string + root string // /backups/offsite-restore +} + +func newVerifyCopyEnv(t *testing.T, copies ...string) *verifyCopyEnv { + t.Helper() + drive := t.TempDir() + m, _, _ := classifiedOffboxManager(t, drive) + root := m.offsiteRestoreRootFor(drive) + for _, c := range copies { + p := filepath.Join(root, c) + if err := os.MkdirAll(p, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(p, "payload.bin"), []byte("restored bytes"), 0o644); err != nil { + t.Fatal(err) + } + } + return &verifyCopyEnv{m: m, drive: drive, root: root} +} + +func TestListOffsiteRestoreCopiesReportsPathAndSize(t *testing.T) { + e := newVerifyCopyEnv(t, "immich", "nextcloud") + + got := e.m.ListOffsiteRestoreCopies() + if len(got) != 2 { + t.Fatalf("listed %d copies, want 2: %+v", len(got), got) + } + byStack := map[string]OffsiteRestoreCopy{} + for _, c := range got { + byStack[c.Stack] = c + } + for _, name := range []string{"immich", "nextcloud"} { + c, ok := byStack[name] + if !ok { + t.Fatalf("%s missing from the listing", name) + } + // THE POINT of the listing: the customer could not previously see WHERE the copy was. + want := filepath.Join(e.root, name) + if c.Path != want { + t.Errorf("%s path = %q, want %q", name, c.Path, want) + } + if c.SizeHuman == "" { + t.Errorf("%s has no humanized size", name) + } + if c.Created.IsZero() { + t.Errorf("%s has no creation time", name) + } + } +} + +func TestListOffsiteRestoreCopiesEmptyIsNotAnError(t *testing.T) { + // No offsite-restore directory at all — the normal state on a box that never ran a verification + // restore. Must be an empty list, not a crash and not a phantom entry. + e := newVerifyCopyEnv(t) + if got := e.m.ListOffsiteRestoreCopies(); len(got) != 0 { + t.Errorf("listed %d copies on a clean box, want 0: %+v", len(got), got) + } +} + +func TestDeleteVerifyCopyRemovesOnlyTheNamedOne(t *testing.T) { + e := newVerifyCopyEnv(t, "immich", "nextcloud") + // Live customer data next to the sandbox — must be untouched by any delete. + live := filepath.Join(e.drive, "immich-live") + if err := os.MkdirAll(live, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(live, "photo.jpg"), []byte("irreplaceable"), 0o644); err != nil { + t.Fatal(err) + } + + if err := e.m.DeleteOffsiteRestoreCopy("immich"); err != nil { + t.Fatalf("delete: %v", err) + } + if _, err := os.Stat(filepath.Join(e.root, "immich")); !os.IsNotExist(err) { + t.Error("the named copy survived the delete") + } + if _, err := os.Stat(filepath.Join(e.root, "nextcloud", "payload.bin")); err != nil { + t.Errorf("a NEIGHBOURING copy was destroyed: %v", err) + } + if _, err := os.Stat(filepath.Join(live, "photo.jpg")); err != nil { + t.Errorf("LIVE CUSTOMER DATA was destroyed: %v", err) + } +} + +func TestDeleteVerifyCopyRefusesUnsafeNames(t *testing.T) { + e := newVerifyCopyEnv(t, "immich") + // Something outside the sandbox that a traversal would reach. + outside := filepath.Join(e.drive, "backups", "primary") + if err := os.MkdirAll(outside, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(outside, "unit.tar"), []byte("recovery unit"), 0o644); err != nil { + t.Fatal(err) + } + + for _, bad := range []string{ + "../primary", + "../../..", + "..", + "immich/../../primary", + "/etc", + "", + } { + if err := e.m.DeleteOffsiteRestoreCopy(bad); err == nil { + t.Errorf("delete(%q) was ACCEPTED — it must be refused", bad) + } + // The refusal must also be a NON-EFFECT. + if _, err := os.Stat(filepath.Join(outside, "unit.tar")); err != nil { + t.Fatalf("delete(%q) destroyed data outside the sandbox: %v", bad, err) + } + if _, err := os.Stat(filepath.Join(e.root, "immich", "payload.bin")); err != nil { + t.Fatalf("delete(%q) destroyed an unrelated copy: %v", bad, err) + } + } +} + +func TestDeleteVerifyCopyStaysInsideTheSandbox(t *testing.T) { + e := newVerifyCopyEnv(t, "immich") + // A well-formed name that simply does not exist must be an error, never a silent success that + // could mask a path-resolution bug. + if err := e.m.DeleteOffsiteRestoreCopy("no-such-app"); err == nil { + t.Error("deleting a non-existent copy reported success") + } + // Everything under the sandbox root must resolve strictly inside it. + for _, c := range e.m.ListOffsiteRestoreCopies() { + rel, err := filepath.Rel(e.root, c.Path) + if err != nil || rel == ".." || filepath.IsAbs(rel) || len(rel) > 2 && rel[:2] == ".." { + t.Errorf("listed copy %q resolves outside %q", c.Path, e.root) + } + } +} + +// TestOffsiteRestoreScratchPathMatchesTheListing pins the two halves together: the path the UI names +// in the completion flash must be the same path the listing (and therefore the delete button) uses. +// If these ever diverge, the customer is told about a directory the page cannot show or remove. +func TestOffsiteRestoreScratchPathMatchesTheListing(t *testing.T) { + e := newVerifyCopyEnv(t, "immich") + named := e.m.OffsiteRestoreScratchPath("immich") + if named == "" { + t.Fatal("OffsiteRestoreScratchPath returned empty — the flash would fall back to the vague wording") + } + var listed string + for _, c := range e.m.ListOffsiteRestoreCopies() { + if c.Stack == "immich" { + listed = c.Path + } + } + if named != listed { + t.Errorf("flash names %q but the listing shows %q", named, listed) + } +} diff --git a/controller/internal/infra/images_test.go b/controller/internal/infra/images_test.go new file mode 100644 index 0000000..eb67c04 --- /dev/null +++ b/controller/internal/infra/images_test.go @@ -0,0 +1,86 @@ +package infra + +import ( + "go/ast" + "go/parser" + "go/token" + "strconv" + "strings" + "testing" +) + +// TestImagesCoversEveryPin is the anti-drift gate for the golden bake. +// +// Images() feeds `felhom-controller --print-infra-images`, which build-golden.sh uses to decide what +// to pre-pull into the appliance image. If someone adds a fifth infra stack — a new `FooImage` const +// — and forgets to add it to Images(), the golden silently bakes 4 of 5 and enabling that stack on a +// fresh box goes back to being a multi-minute silent registry pull. That is exactly how felhom-samba +// was missed, so this test reads the CONST BLOCK OUT OF THE SOURCE rather than restating the list: +// a hand-written expected list would need the same edit and would rot the same way. +func TestImagesCoversEveryPin(t *testing.T) { + fset := token.NewFileSet() + f, err := parser.ParseFile(fset, "infra.go", nil, 0) + if err != nil { + t.Fatalf("parsing infra.go: %v", err) + } + + pins := map[string]string{} // const name -> image ref + for _, decl := range f.Decls { + gd, ok := decl.(*ast.GenDecl) + if !ok || gd.Tok != token.CONST { + continue + } + for _, spec := range gd.Specs { + vs, ok := spec.(*ast.ValueSpec) + if !ok || len(vs.Names) != 1 || len(vs.Values) != 1 { + continue + } + name := vs.Names[0].Name + if !strings.HasSuffix(name, "Image") { + continue + } + lit, ok := vs.Values[0].(*ast.BasicLit) + if !ok || lit.Kind != token.STRING { + continue + } + val, err := strconv.Unquote(lit.Value) + if err != nil { + t.Fatalf("unquoting %s: %v", name, err) + } + pins[name] = val + } + } + + if len(pins) == 0 { + t.Fatal("found no *Image consts in infra.go — the parser walk is broken, not the pins") + } + + got := map[string]bool{} + for _, img := range Images() { + got[img] = true + } + for name, img := range pins { + if !got[img] { + t.Errorf("const %s = %q is not in Images() — the golden bake would not pre-pull it, so "+ + "enabling that stack on a fresh box would block on a silent registry pull", name, img) + } + } + if len(Images()) != len(pins) { + t.Errorf("Images() has %d entries but infra.go declares %d *Image consts (%v) — they must "+ + "correspond one-to-one", len(Images()), len(pins), pins) + } +} + +// TestImagesArePinned guards the other half of the contract: a floating tag would make the golden +// bake unreproducible (the bake and a later deploy could resolve the same name to different digests). +func TestImagesArePinned(t *testing.T) { + for _, img := range Images() { + if !strings.Contains(img, ":") { + t.Errorf("infra image %q has no tag — implicitly :latest", img) + continue + } + if strings.HasSuffix(img, ":latest") { + t.Errorf("infra image %q is :latest — a floating tag breaks reproducible golden bakes", img) + } + } +} diff --git a/controller/internal/infra/infra.go b/controller/internal/infra/infra.go index 1f7d6ab..e24b785 100644 --- a/controller/internal/infra/infra.go +++ b/controller/internal/infra/infra.go @@ -30,6 +30,23 @@ const ( SambaImage = "gitea.dooplex.hu/admin/felhom-samba:1.0.0" ) +// Images returns every controller-managed infra image, derived from the pins above so there is +// exactly one place a tag is written. +// +// WHY THIS IS EXPORTED: the golden bake (felhom-agent configs/build-golden.sh) pre-pulls these into +// the appliance image so enabling an infra stack on a fresh box is near-instant instead of a silent +// multi-minute registry pull. It used to carry its OWN hand-maintained bash array of tags — which +// drifted the moment felhom-samba was added: the golden baked three of the four, so turning on +// Megosztás pulled from the registry with zero feedback. The bake now asks the controller BINARY it +// is about to bake (`felhom-controller --print-infra-images`), so the golden and the controller it +// ships cannot disagree by construction. +// +// A new infra stack is therefore two edits in this file (the const, and this slice) and zero edits +// anywhere else. If you add a const and forget the slice, TestImagesCoversEveryPin fails. +func Images() []string { + return []string{TraefikImage, CloudflaredImage, FileBrowserImage, SambaImage} +} + //go:embed templates/*.tmpl var templateFS embed.FS diff --git a/controller/internal/stacks/manager.go b/controller/internal/stacks/manager.go index 4a0f729..45c0c4c 100644 --- a/controller/internal/stacks/manager.go +++ b/controller/internal/stacks/manager.go @@ -118,6 +118,7 @@ type Manager struct { sambaUpFn func(dir string) error sambaPasswdFn func(password string) error sambaRunFn func() bool // replaces the docker-inspect liveness probe + sambaImgFn func() bool // replaces the `docker image inspect` local-presence probe (4b card) } // NewManager creates a new stack manager. diff --git a/controller/internal/stacks/samba.go b/controller/internal/stacks/samba.go index 8e1df19..0d7323c 100644 --- a/controller/internal/stacks/samba.go +++ b/controller/internal/stacks/samba.go @@ -85,6 +85,20 @@ func (m *Manager) sambaIsRunning() bool { return containerRunning(sambaContainer) } +// SambaImagePresent reports whether the pinned samba image is already in local Docker storage. +// +// This is what makes the progress card HONEST rather than decorative: on a golden that baked the +// image (see felhom-agent build-golden.sh) the bring-up is seconds and the card should say +// „elindítás"; on a box that must fetch ~100MB from the registry it is minutes and the card must say +// „képfájl letöltése" so the wait is explained instead of silent. Asked BEFORE compose runs, because +// afterwards the answer is always yes. +func (m *Manager) SambaImagePresent() bool { + if m.sambaImgFn != nil { + return m.sambaImgFn() + } + return exec.Command("docker", "image", "inspect", infra.SambaImage).Run() == nil +} + // shareAvailable reports whether a share's folder can be exported right now: its owning registered // storage path must be neither disconnected nor decommissioned, AND the folder must exist. A dead // mount is NEVER exported — publishing a missing mountpoint would show an empty share and let a diff --git a/controller/internal/web/handlers.go b/controller/internal/web/handlers.go index e951143..e6c6f70 100644 --- a/controller/internal/web/handlers.go +++ b/controller/internal/web/handlers.go @@ -823,6 +823,11 @@ func (s *Server) backupsRestoreHandler(w http.ResponseWriter, r *http.Request) { data["SharesScratchReady"] = s.backupMgr.SharesScratchReady() data["SharesDisplayName"] = backup.SharesDisplayName } + // v0.147.0 (4a): what verification restores have actually left on disk. Previously nothing on any + // page listed these, so they accumulated invisibly and the only way to find them was SSH. + if s.backupMgr != nil { + data["OffsiteRestoreCopies"] = s.backupMgr.ListOffsiteRestoreCopies() + } s.executeTemplate(w, r, "backups_restore", data) } diff --git a/controller/internal/web/offbox_handlers.go b/controller/internal/web/offbox_handlers.go index 3896560..55e538d 100644 --- a/controller/internal/web/offbox_handlers.go +++ b/controller/internal/web/offbox_handlers.go @@ -219,11 +219,13 @@ func (s *Server) offboxRunHandler(w http.ResponseWriter, r *http.Request) { go func() { ctx, cancel := context.WithTimeout(context.Background(), 3*time.Hour) defer cancel() - if err := s.backupMgr.RunOffboxBackup(ctx); err != nil { + // ...WithProgress: this is the MANUAL trigger, so the page gets live bytes/percent/current app + // (4c). The nightly scheduler keeps calling RunOffboxBackup and stays silent. + if err := s.backupMgr.RunOffboxBackupWithProgress(ctx); err != nil { s.logger.Printf("[WARN] [web] manual off-box backup failed: %v", err) } }() - offboxRedirect(w, r, "A távoli mentés elindult (a futás után az állapot frissül).", false) + offboxRedirect(w, r, "A távoli mentés elindult — az állapot itt frissül.", false) } // offboxResetHandler is the CLAIMED confirmed orphaned-repo reset (Scenario C): move the old (recovery- @@ -266,6 +268,11 @@ func (s *Server) offboxStatusHandler(w http.ResponseWriter, r *http.Request) { resp["last_error"] = t.LastError resp["orphaned"] = t.RepoState == "orphaned" } + // v0.147.0 (4c): live progress for a MANUAL run. Absent/inactive on the nightly path, so the + // page simply keeps its previous „Fut…" behavior there. + if s.backupMgr != nil { + resp["progress"] = s.backupMgr.OffboxProgressSnapshot() + } w.Header().Set("Content-Type", "application/json") _ = json.NewEncoder(w).Encode(resp) } @@ -319,11 +326,57 @@ func (s *Server) offboxRestoreHandler(w http.ResponseWriter, r *http.Request) { return } s.logger.Printf("[INFO] [web] off-box restore %s completed (full=%v, async)", app, full) - s.backupMgr.EndRestoreOp(true, "A(z) "+app+" visszaállítva ellenőrző mappába a meghajtón (a meglévő adatok változatlanok).") + // NAME THE RESULT (v0.147.0, 4a). The old message said the app had been restored "to a + // verification folder on the drive" — which folder, on which drive, was invisible, so the + // customer had no way to look at what they had just asked for. Resolve the real path and say + // it. Fall back to the vague wording only if the path can no longer be resolved. + where := s.backupMgr.OffsiteRestoreScratchPath(app) + msg := "A(z) " + app + " visszaállítva ellenőrző mappába a meghajtón (a meglévő adatok változatlanok)." + if where != "" { + msg = "A(z) " + app + " visszaállítva ellenőrző mappába: " + where + " (a meglévő adatok változatlanok)." + } + s.backupMgr.EndRestoreOp(true, msg) }() offboxRedirectTo(w, r, "/backups/restore", "A távoli visszaállítás elindult — az állapot itt frissül.", false) } +// offboxVerifyCopyDeleteHandler removes ONE verification copy (v0.147.0, 4a). +// +// The only delete this slice adds, so it is deliberately narrow: it names a STACK, never a path — the +// customer cannot hand us a path to remove. The Manager resolves that name inside a +// `backups/offsite-restore` root it computed itself and refuses anything that lands outside (see +// DeleteOffsiteRestoreCopy). The template double-confirms before POSTing. +// +// It refuses while a backup/restore op is running: the copy being deleted could be the one currently +// being written. +func (s *Server) offboxVerifyCopyDeleteHandler(w http.ResponseWriter, r *http.Request) { + if s.backupMgr == nil { + offboxRedirectTo(w, r, "/backups/restore", "A mentéskezelő nem érhető el.", true) + return + } + _ = r.ParseForm() + stack := strings.TrimSpace(r.FormValue("stack")) + if stack == "" { + offboxRedirectTo(w, r, "/backups/restore", "Hiányzó ellenőrző másolat.", true) + return + } + if r.FormValue("confirm") != "1" { + offboxRedirectTo(w, r, "/backups/restore", "A törlés megerősítés nélkül nem hajtható végre.", true) + return + } + if s.backupMgr.IsRunning() { + offboxRedirectTo(w, r, "/backups/restore", "Egy mentési/visszaállítási művelet fut — a törlés most nem biztonságos.", true) + return + } + if err := s.backupMgr.DeleteOffsiteRestoreCopy(stack); err != nil { + s.logger.Printf("[WARN] [web] verification-copy delete %s: %v", stack, err) + offboxRedirectTo(w, r, "/backups/restore", "A másolat törlése nem sikerült: "+err.Error(), true) + return + } + s.logger.Printf("[INFO] [web] verification copy deleted: %s", stack) + offboxRedirectTo(w, r, "/backups/restore", "Az ellenőrző másolat törölve. A tényleges adataid változatlanok.", false) +} + // offboxPlaceHandler places a COMPLETED full-restore scratch into the app's live locations via a // missing-only merge (§7.3). Never overwrites existing files. Async on a background context. func (s *Server) offboxPlaceHandler(w http.ResponseWriter, r *http.Request) { diff --git a/controller/internal/web/samba_ensure_job.go b/controller/internal/web/samba_ensure_job.go new file mode 100644 index 0000000..2332fa2 --- /dev/null +++ b/controller/internal/web/samba_ensure_job.go @@ -0,0 +1,156 @@ +package web + +import ( + "sync" + "time" +) + +// Samba bring-up progress (v0.147.0, feedback slice 4b). +// +// THE PROBLEM: enabling Megosztás on a fresh box ran `ReconcileSamba()` synchronously inside the +// POST handler. On a golden that had not baked felhom-samba, that call is `docker compose up -d` +// pulling ~100MB from a private registry — minutes of an apparently-hung form post, then a redirect +// with a flash reading „Beállítás mentve." whether or not anything had actually come up. Observed +// live, twice. The image is now baked (felhom-agent build-golden.sh, golden >= 0.147.x), but this +// card still covers the pre-0.147 goldens and every future image update. +// +// SHAPE: deliberately the storage-init / netstorage-add one (storage_init_job.go) — detached job, +// single-flight slot, deep-copied snapshot, phase strings the template maps to Hungarian. No new +// framework; a unified async-job feedback layer is a ROADMAP item, not this slice. +// +// State is in-memory and lost on restart, exactly like RestoreOpStatus. That is acceptable here: the +// terminal truth is „is the container running", which the page re-reads from the stack manager on +// every load anyway — the job only explains the WAIT. + +type sambaEnsureJob struct { + Phase string `json:"phase"` + Error string `json:"error,omitempty"` + StartedAt time.Time `json:"started_at"` + UpdatedAt time.Time `json:"updated_at"` +} + +const ( + // sambaPhasePulling — the pinned image is NOT in local Docker storage, so compose will fetch it. + // This is the phase worth naming: it is the multi-minute one, and the only honest explanation for + // why nothing appears to happen. + sambaPhasePulling = "pulling" + // sambaPhaseStarting — image already local (baked golden / previously pulled): seconds. + sambaPhaseStarting = "starting" + // sambaPhaseRunning — terminal success, PROBED (compose up -d exits 0 on a crash-loop, so the + // job's success condition is container liveness, never the compose exit code). + sambaPhaseRunning = "running" + // sambaPhaseNeedsPassword — not a failure: sharing is on but the household password is unset, so + // reconcile deliberately deploys nothing. The card must say so instead of spinning forever. + sambaPhaseNeedsPassword = "needs_password" + sambaPhaseFailed = "failed" + sambaPhaseIdle = "idle" +) + +// A pull that has not finished in 15 minutes is not slow, it is broken (the registry is unreachable +// or the disk is full) — end the job so the card can say so rather than spin indefinitely. +const sambaEnsureDeadline = 15 * time.Minute + +type sambaEnsureState struct { + mu sync.Mutex + running bool + cur *sambaEnsureJob +} + +func (s *sambaEnsureState) acquire(job *sambaEnsureJob) bool { + s.mu.Lock() + defer s.mu.Unlock() + if s.running { + return false + } + s.running = true + cp := *job + s.cur = &cp + return true +} + +func (s *sambaEnsureState) release() { + s.mu.Lock() + s.running = false + s.mu.Unlock() +} + +func (s *sambaEnsureState) set(job *sambaEnsureJob) { + s.mu.Lock() + cp := *job + s.cur = &cp + s.mu.Unlock() +} + +// snapshot returns a copy of the last / in-flight job (nil = never ran this process). +func (s *sambaEnsureState) snapshot() *sambaEnsureJob { + s.mu.Lock() + defer s.mu.Unlock() + if s.cur == nil { + return nil + } + cp := *s.cur + return &cp +} + +// startSambaEnsure claims the single-flight slot and launches the detached reconcile. false = one is +// already in flight (a double-submit must not start a second compose up on the same stack dir). +// +// The opening phase is decided BEFORE the work starts, by asking whether the image is already local +// — afterwards the answer is always yes and the card could never truthfully say „letöltés". +func (s *Server) startSambaEnsure() bool { + now := time.Now().UTC() + phase := sambaPhaseStarting + if s.stackMgr != nil && !s.stackMgr.SambaImagePresent() { + phase = sambaPhasePulling + } + job := &sambaEnsureJob{Phase: phase, StartedAt: now, UpdatedAt: now} + if !s.sambaEnsure.acquire(job) { + return false + } + go s.runSambaEnsureJob(job) + return true +} + +func (s *Server) runSambaEnsureJob(job *sambaEnsureJob) { + defer s.sambaEnsure.release() + + advance := func(phase, errMsg string) { + job.Phase = phase + job.Error = errMsg + job.UpdatedAt = time.Now().UTC() + s.sambaEnsure.set(job) + } + + done := make(chan error, 1) + go func() { done <- s.stackMgr.ReconcileSamba() }() + + select { + case err := <-done: + if err != nil { + s.logger.Printf("[ERROR] [sharing] samba reconcile failed after %s: %v", time.Since(job.StartedAt).Round(time.Second), err) + advance(sambaPhaseFailed, err.Error()) + return + } + case <-time.After(sambaEnsureDeadline): + // The reconcile goroutine is left running — compose owns its own lifecycle and killing it + // mid-pull would leave a partial layer set. We stop REPORTING on it, which is the honest + // thing the customer needs; a later page load re-probes liveness for the real answer. + s.logger.Printf("[ERROR] [sharing] samba reconcile still running after %s — giving up on the progress card", sambaEnsureDeadline) + advance(sambaPhaseFailed, "a megosztási szolgáltatás előkészítése túl sokáig tartott") + return + } + + // Reconcile returned nil — but nil ALSO covers "deliberately did nothing". Distinguish the two, + // because a card that says „fut" while nothing is deployed is the same silence in a new costume. + if s.settings != nil && !s.settings.GetSMBSettings().UserSet { + advance(sambaPhaseNeedsPassword, "") + return + } + if !s.stackMgr.SambaRunning() { + s.logger.Printf("[ERROR] [sharing] samba reconcile reported success but the container is not running") + advance(sambaPhaseFailed, "a megosztási szolgáltatás nem indult el") + return + } + s.logger.Printf("[INFO] [sharing] samba ready after %s", time.Since(job.StartedAt).Round(time.Second)) + advance(sambaPhaseRunning, "") +} diff --git a/controller/internal/web/samba_ensure_job_test.go b/controller/internal/web/samba_ensure_job_test.go new file mode 100644 index 0000000..41f01f5 --- /dev/null +++ b/controller/internal/web/samba_ensure_job_test.go @@ -0,0 +1,86 @@ +package web + +import ( + "testing" +) + +// v0.147.0 slice 4b — the Megosztás bring-up progress card. +// +// The card's whole value is that it distinguishes states the old synchronous handler collapsed into +// one silent form post. So the tests are about the DISTINCTIONS: +// - "pulling" vs "starting" — decided by local image presence, and decided BEFORE the work starts +// (afterwards the image is always present and the card could never truthfully say „letöltés"). +// - reconcile-returned-nil is NOT the same as running: it also covers "deliberately deployed +// nothing because there is no household password yet". +// - compose up -d exits 0 on a crash-loop, so success must be PROBED, never inferred. +// +// SCOPE, STATED HONESTLY: these cover the single-flight slot, snapshot isolation and the phase +// vocabulary. They do NOT drive runSambaEnsureJob end-to-end — that needs a real *stacks.Manager +// (the Server field is the concrete type, not an interface), and introducing an interface purely for +// this card was more churn than the slice warranted. The pulling-vs-starting decision and the +// probed-liveness terminal state are therefore covered by the LIVE validation on the demo box, not +// by a unit test. Recorded here rather than left as an implied gap; the ROADMAP's unified +// async-job feedback item is where that seam belongs. + +func TestSambaEnsureSingleFlight(t *testing.T) { + var st sambaEnsureState + job := &sambaEnsureJob{Phase: sambaPhaseStarting} + if !st.acquire(job) { + t.Fatal("first acquire refused") + } + // A double-submit must not start a second `compose up -d` against the same stack dir. + if st.acquire(&sambaEnsureJob{Phase: sambaPhaseStarting}) { + t.Error("second acquire succeeded while a job was in flight") + } + st.release() + if !st.acquire(&sambaEnsureJob{Phase: sambaPhaseStarting}) { + t.Error("acquire refused after release") + } +} + +func TestSambaEnsureSnapshotIsACopy(t *testing.T) { + var st sambaEnsureState + job := &sambaEnsureJob{Phase: sambaPhasePulling} + st.acquire(job) + + snap := st.snapshot() + snap.Phase = "mutated-by-caller" + if again := st.snapshot(); again.Phase != sambaPhasePulling { + t.Errorf("a caller mutated the shared job through its snapshot: %q", again.Phase) + } + // And the live job must not leak into an already-taken snapshot either. + snap2 := st.snapshot() + job.Phase = sambaPhaseRunning + st.set(job) + if snap2.Phase != sambaPhasePulling { + t.Errorf("an earlier snapshot changed under the caller: %q", snap2.Phase) + } +} + +func TestSambaEnsureStateStartsNil(t *testing.T) { + var st sambaEnsureState + // nil = never ran this process. The status handler maps that to `idle` and lets live container + // state win, so a page loaded after a restart still tells the truth. + if j := st.snapshot(); j != nil { + t.Errorf("fresh state reported a job: %+v", j) + } +} + +// TestSambaPhaseConstantsAreDistinct guards the template contract: sharing.html maps these exact +// strings, and a duplicate would silently render the wrong Hungarian sentence. +func TestSambaPhaseConstantsAreDistinct(t *testing.T) { + all := []string{ + sambaPhasePulling, sambaPhaseStarting, sambaPhaseRunning, + sambaPhaseNeedsPassword, sambaPhaseFailed, sambaPhaseIdle, + } + seen := map[string]bool{} + for _, p := range all { + if p == "" { + t.Error("a phase constant is empty — the template would fall through to 'no card'") + } + if seen[p] { + t.Errorf("duplicate phase constant %q", p) + } + seen[p] = true + } +} diff --git a/controller/internal/web/server.go b/controller/internal/web/server.go index d51dcbf..1360295 100644 --- a/controller/internal/web/server.go +++ b/controller/internal/web/server.go @@ -94,6 +94,8 @@ type Server struct { // netAgentFn nil → the shared agentClient(); netProbeFn nil → runNetProbe (the uid-1000 re-exec). netAdd netAddState storageInit storageInitState + // sambaEnsure is the Megosztás bring-up progress slot (v0.147.0, 4b) — same single-flight shape. + sambaEnsure sambaEnsureState netAgentFn func() (netAgent, error) // fabUpload is the chunked browser .fab upload single-flight slot (v0.128.0). fabUpload uploadState @@ -359,6 +361,8 @@ func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) { s.sharingPageHandler(w, r) case path == "/sharing/enable" && r.Method == http.MethodPost: s.sharingEnableHandler(w, r) + case path == "/sharing/status" && r.Method == http.MethodGet: + s.sharingStatusHandler(w, r) case path == "/sharing/password" && r.Method == http.MethodPost: s.sharingPasswordHandler(w, r) case path == "/sharing/shares" && r.Method == http.MethodPost: @@ -418,6 +422,10 @@ func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) { s.offboxRestoreHandler(w, r) case path == "/backup/offbox/place" && r.Method == http.MethodPost: s.offboxPlaceHandler(w, r) + // v0.147.0 (4a): remove ONE verification copy. The only delete path this slice adds — see + // DeleteOffsiteRestoreCopy for the prefix guard. + case path == "/backup/offbox/verify-copy/delete" && r.Method == http.MethodPost: + s.offboxVerifyCopyDeleteHandler(w, r) // R-7b: „Megosztások" restore — a sibling of the per-app pair above. case path == "/backup/shares/restore" && r.Method == http.MethodPost: s.sharesRestoreHandler(w, r) diff --git a/controller/internal/web/sharing_handlers.go b/controller/internal/web/sharing_handlers.go index 156dcab..d2eab41 100644 --- a/controller/internal/web/sharing_handlers.go +++ b/controller/internal/web/sharing_handlers.go @@ -221,10 +221,33 @@ func (s *Server) sharingEnableHandler(w http.ResponseWriter, r *http.Request) { sharingRedirect(w, r, "A hálózati megosztás kikapcsolva. A mappák és a fájlok megmaradtak.") return } - if err := s.stackMgr.ReconcileSamba(); err != nil { - s.logger.Printf("[WARN] [sharing] reconcile failed: %v", err) + // v0.147.0 (4b): the bring-up runs DETACHED and the page polls it. Synchronously it was a form + // post that hung for minutes on a first-enable image pull and then flashed „Beállítás mentve." + // regardless of whether anything actually came up. + if !s.startSambaEnsure() { + sharingRedirect(w, r, "A megosztási szolgáltatás előkészítése már folyamatban van.") + return } - sharingRedirect(w, r, "Beállítás mentve.") + sharingRedirect(w, r, "Beállítás mentve. A megosztási szolgáltatás előkészítése folyamatban…") +} + +// sharingStatusHandler is the 4b poll target (GET /sharing/status). Reports the ensure job's phase +// plus the live container state, so a page loaded AFTER the job finished (or after a restart, when +// the in-memory job is gone) still shows the truth. +func (s *Server) sharingStatusHandler(w http.ResponseWriter, r *http.Request) { + phase := sambaPhaseIdle + errMsg := "" + if job := s.sambaEnsure.snapshot(); job != nil { + phase, errMsg = job.Phase, job.Error + } + running := s.stackMgr != nil && s.stackMgr.SambaRunning() + // A stale `idle`/`running` job must never contradict reality: liveness wins on a fresh page. + if phase == sambaPhaseIdle && running { + phase = sambaPhaseRunning + } + writeDiskJSON(w, http.StatusOK, true, "", map[string]any{ + "phase": phase, "error": errMsg, "running": running, + }) } // sharingPasswordHandler sets the household SMB password (POST /sharing/password). @@ -247,10 +270,14 @@ func (s *Server) sharingPasswordHandler(w http.ResponseWriter, r *http.Request) sharingRedirect(w, r, "A jelszó beállítása nem sikerült.") return } - if err := s.stackMgr.ReconcileSamba(); err != nil { - s.logger.Printf("[WARN] [sharing] reconcile after password failed: %v", err) + // Setting the password is the moment the stack ACTUALLY first comes up: with UserSet false, + // reconcile deliberately deploys nothing, so on a fresh box this — not the enable toggle — is + // where the image pull happens. Same detached job, same card. + if !s.startSambaEnsure() { + sharingRedirect(w, r, "Megosztási jelszó beállítva. Az előkészítés már folyamatban van.") + return } - sharingRedirect(w, r, "Megosztási jelszó beállítva.") + sharingRedirect(w, r, "Megosztási jelszó beállítva. A megosztási szolgáltatás előkészítése folyamatban…") } // sharingShareCreateHandler creates a share (POST /sharing/shares). Either a NEW folder under diff --git a/controller/internal/web/templates/backups_remote.html b/controller/internal/web/templates/backups_remote.html index e40ba8f..c8d342c 100644 --- a/controller/internal/web/templates/backups_remote.html +++ b/controller/internal/web/templates/backups_remote.html @@ -126,6 +126,15 @@ + +

Mely alkalmazások mentődnek a távoli tárolóra?

{{if and .OffboxApps (eq .Offbox.EscrowState "escrowed") (eq .OffboxToggledCount 0)}}

Nincs távoli mentésre jelölt alkalmazás — jelölj ki legalább egyet.

@@ -172,19 +181,55 @@ {{end}} {{template "layout_end" .}}