From c00fed6db31ebff0f539128afe6c17f9f7e83b31 Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Thu, 17 Sep 2026 20:58:45 +0200 Subject: [PATCH] R-553: four decisions stop reading their own Hungarian words (sites 1-4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every Hungarian sentence is byte-identical; each decision now reads a signal set where the message is made. util.KindErrorf builds the same bytes fmt.Errorf did while carrying a sentinel for errors.Is. - Deploy status (api/router.go): deployStatusFor() by kind — stacks.ErrAlreadyDeployed (409), ErrRequiredField / ErrPathMissing / ErrNotEnoughMemory (400). The „kötelező" / „memória" / "does not exist" / "already deployed" text chain is gone. - Off-site failure class (backup/offbox.go): ErrOffsiteQuota replaces the „tárhelykeretet" match. The restic/ssh signatures stay text matches on purpose — that output is not ours and is not translated. - Alert placement (web/alerts.go): monitor.HealthReport carries WarningKinds parallel to Warnings; the "not on a separate drive" warning is inline by KIND. The hub report is untouched (builder.go copies Status/Issues/Warnings only) — pinned by a wire test. - Stale off-site note (web/handlers.go): settings LastWarningKind + backup.OffboxWarnNoAppsSelected. The text test survives ONLY for kind == "" (a box whose last run predates 0.251.0) and is removed when R-570 closes; slice 2 must not translate that producer before then. Tests (all red-proofed by restoring the pre-fix predicate — see the audit's redproofs.txt): TestR553_Deploy_DecisionSurvivesWordingChange, TestR553_DeployHandlerUsesTheKind, TestR553_DeployProducersCarryKindAndKeepTheirWords (through the real DeployStack), TestR553_OffsiteQuota_{Decision,HeadLine}SurvivesWordingChange, TestR553_OffboxRunRecordsTheKind, TestR553_StorageWarningsCarryKindsAndKeepTheirWords, TestR553_DiskWarningPlacementSurvivesWordingChange, TestR553_HubReportWarningsAreUnchangedOnTheWire, TestR553_StaleNote*, TestR553_WarningKindIsPersistedAndCopied. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0159rPz1ZhFKsS53msqPYxtS --- REUSE.md | 10 ++ .../internal/api/r553_deploy_status_test.go | 83 +++++++++++ controller/internal/api/router.go | 29 +++- controller/internal/backup/offbox.go | 27 +++- .../internal/backup/offsite_diag_test.go | 7 +- .../backup/r553_offsite_quota_test.go | 101 +++++++++++++ controller/internal/monitor/healthcheck.go | 69 +++++++-- .../monitor/r553_warning_kinds_test.go | 74 ++++++++++ controller/internal/settings/settings.go | 9 +- controller/internal/stacks/deploy.go | 16 +- controller/internal/stacks/deploy_errors.go | 21 +++ .../stacks/r553_deploy_error_kinds_test.go | 137 ++++++++++++++++++ controller/internal/util/errkind.go | 36 +++++ controller/internal/web/alerts.go | 12 +- controller/internal/web/handlers.go | 25 +++- controller/internal/web/offbox_handlers.go | 2 +- .../web/offbox_warning_display_test.go | 15 +- .../internal/web/r553_alert_placement_test.go | 105 ++++++++++++++ .../internal/web/r553_stale_note_test.go | 93 ++++++++++++ 19 files changed, 816 insertions(+), 55 deletions(-) create mode 100644 controller/internal/api/r553_deploy_status_test.go create mode 100644 controller/internal/backup/r553_offsite_quota_test.go create mode 100644 controller/internal/monitor/r553_warning_kinds_test.go create mode 100644 controller/internal/stacks/deploy_errors.go create mode 100644 controller/internal/stacks/r553_deploy_error_kinds_test.go create mode 100644 controller/internal/util/errkind.go create mode 100644 controller/internal/web/r553_alert_placement_test.go create mode 100644 controller/internal/web/r553_stale_note_test.go diff --git a/REUSE.md b/REUSE.md index 6654667..7c5f174 100644 --- a/REUSE.md +++ b/REUSE.md @@ -37,6 +37,16 @@ | `parseWWWAuthenticate` + `fetchAnonymousToken` | controller/internal/selfupdate/updater.go | Bearer-challenge parse + anonymous Docker v2 token | Any credential-free registry API access | realm comes FROM THE HEADER (never hardcode a token URL); denial = errAnonymousDenied, never "credentials missing" | | `Syncer.runGit` / `runGitInDir` | controller/internal/sync/sync.go | `(args...) error` | git CLI ops | Credentials masked in logs via `maskRepoURL` | +### Error kinds — never branch on a customer-facing sentence (R-553) + +| Symbol | File | Short signature | Use for | Gotchas | +|---|---|---|---|---| +| `util.KindErrorf` / `util.KindError` | controller/internal/util/errkind.go | `(kind error, format string, a ...interface{}) error` | ANY refusal a caller must tell apart: build the message exactly as `fmt.Errorf` would AND carry a sentinel for `errors.Is` | The message bytes are unchanged (pinned by tests); never `fmt.Errorf("%w: …")`, which would prepend the sentinel's own text to the customer's sentence | +| `stacks.ErrAlreadyDeployed` / `ErrRequiredField` / `ErrPathMissing` / `ErrNotEnoughMemory` | controller/internal/stacks/deploy_errors.go | sentinels | the API's deploy status code (`api.deployStatusFor`) | 409 / 400 / 400 / 400. Do NOT add a text signature beside them | +| `backup.ErrOffsiteQuota` | controller/internal/backup/offbox.go | sentinel | `ClassifyOffsiteFailure` telling a quota over-run apart | The other arms of that switch stay TEXT matches on purpose — they are restic's and ssh's own English output, which we neither write nor translate | +| `monitor.WarnKind*` + `HealthReport.addWarning` / `WarningKindAt` | controller/internal/monitor/healthcheck.go | `(text, kind string)` | a health warning whose PLACEMENT the dashboard decides | Internal only: `internal/report/builder.go` copies Status/Issues/Warnings, so kinds never reach the hub (pinned) | +| `settings.OffboxTarget.LastWarningKind` + `backup.OffboxWarnNoAppsSelected` | controller/internal/settings/settings.go | persisted string | the Távoli mentés page's stale-note substitution | Written and cleared with `LastWarning`; the text fallback in `offboxWarningDisplay` is LEGACY only (kind == "") and is removed when R-570 closes | + ### HTTP/JSON envelopes + flash messages | Symbol | File | Short signature | Use for | Gotchas | diff --git a/controller/internal/api/r553_deploy_status_test.go b/controller/internal/api/r553_deploy_status_test.go new file mode 100644 index 0000000..b88646f --- /dev/null +++ b/controller/internal/api/r553_deploy_status_test.go @@ -0,0 +1,83 @@ +package api + +import ( + "errors" + "net/http" + "os" + "strings" + "testing" + + "gitea.dooplex.hu/admin/felhom-controller/internal/stacks" + "gitea.dooplex.hu/admin/felhom-controller/internal/util" +) + +// R-553 — the deploy endpoint's status code must survive the day its Hungarian words are translated. +// +// Until v0.251.0 the handler read the refusal's TEXT: „kötelező", „memória", "does not exist", +// "already deployed". Localisation slice 2 translates exactly those words, and every empty required +// field would then answer 500 instead of 400 — a server error for something the customer can fix. +// +// The table below therefore passes messages that are ALREADY translated: the words are gone, only the +// kind remains. RED-PROOF (REPORT): restore the strings.Contains chain in deployStack and this fails +// on every translated row. +func TestR553_Deploy_DecisionSurvivesWordingChange(t *testing.T) { + cases := []struct { + name string + err error + want int + }{ + {"required field, Hungarian as shipped", util.KindErrorf(stacks.ErrRequiredField, + "a(z) %q (%s) mező kitöltése kötelező", "Jelszó", "PASSWORD"), http.StatusBadRequest}, + {"required field, TRANSLATED", util.KindErrorf(stacks.ErrRequiredField, + "the %q (%s) field is required", "Password", "PASSWORD"), http.StatusBadRequest}, + {"password field, TRANSLATED", util.KindErrorf(stacks.ErrRequiredField, + "fill in the %q field — use the Generate button", "Password"), http.StatusBadRequest}, + {"path missing, TRANSLATED", util.KindErrorf(stacks.ErrPathMissing, + "a mappa %q nem létezik (%q mező)", "/mnt/nope", "Adatok"), http.StatusBadRequest}, + {"not enough memory, Hungarian as shipped", util.KindError(stacks.ErrNotEnoughMemory, + "Nincs elég memória az alkalmazás telepítéséhez. Szükséges: 512 MB, Elérhető: 100 MB"), http.StatusBadRequest}, + {"not enough memory, TRANSLATED", util.KindError(stacks.ErrNotEnoughMemory, + "Not enough memory to install this app. Needed: 512 MB, available: 100 MB"), http.StatusBadRequest}, + {"already deployed, TRANSLATED", util.KindErrorf(stacks.ErrAlreadyDeployed, + "a(z) %q alkalmazás már telepítve van", "privatebin"), http.StatusConflict}, + {"anything else is a server error", errors.New("compose exploded"), http.StatusInternalServerError}, + {"a message that merely CONTAINS the old words is not a refusal kind", + errors.New("docker: kötelező memória does not exist"), http.StatusInternalServerError}, + } + for _, c := range cases { + if got := deployStatusFor(c.err); got != c.want { + t.Errorf("%s: deployStatusFor(%q) = %d, want %d", c.name, c.err.Error(), got, c.want) + } + } +} + +// The seam has to be WIRED, not merely present: the handler must call deployStatusFor, and no +// decision in the deploy handler may read the error's text again (seam-built-but-never-wired, four +// instances in this repo). +func TestR553_DeployHandlerUsesTheKind(t *testing.T) { + src, err := os.ReadFile("router.go") + if err != nil { + t.Fatal(err) + } + body := string(src) + i := strings.Index(body, "func (r *Router) deployStack(") + if i < 0 { + t.Fatal("deployStack handler not found — this test no longer reads what it thinks it reads") + } + handler := body[i:] + if j := strings.Index(handler[10:], "\nfunc "); j > 0 { + handler = handler[:j+10] + } + if !strings.Contains(handler, "deployStatusFor(err)") { + t.Error("the deploy handler no longer asks deployStatusFor for the status code") + } + // Comments may still NAME those words (this one does); a DECISION may not read them. + for _, line := range strings.Split(handler, "\n") { + if strings.HasPrefix(strings.TrimSpace(line), "//") { + continue + } + if strings.Contains(line, "strings.Contains(err.Error()") { + t.Errorf("the deploy handler decides by reading the error text again (R-553): %s", strings.TrimSpace(line)) + } + } +} diff --git a/controller/internal/api/router.go b/controller/internal/api/router.go index 9e878cf..bd4c738 100644 --- a/controller/internal/api/router.go +++ b/controller/internal/api/router.go @@ -398,6 +398,25 @@ func (r *Router) getDeployFields(w http.ResponseWriter, _ *http.Request, name st writeJSON(w, http.StatusOK, apiResponse{OK: true, Data: data}) } +// deployStatusFor maps a deploy refusal to its HTTP status by the refusal's KIND (R-553). +// +// It used to read the customer's Hungarian sentence — `strings.Contains(err.Error(), "kötelező")`, +// „memória", plus the English "does not exist" / "already deployed". Localisation slice 2 translates +// exactly those words, and on that day every validation refusal would have become a 500: the customer +// sees a server error for a field they simply left empty, and the API contract changes silently. +// The sentences are untouched; the kinds come from internal/stacks (deploy_errors.go). +// Pinned by TestR553_Deploy_DecisionSurvivesWordingChange. +func deployStatusFor(err error) int { + switch { + case errors.Is(err, stacks.ErrAlreadyDeployed): + return http.StatusConflict + case errors.Is(err, stacks.ErrRequiredField), errors.Is(err, stacks.ErrPathMissing), + errors.Is(err, stacks.ErrNotEnoughMemory): + return http.StatusBadRequest + } + return http.StatusInternalServerError +} + func (r *Router) deployStack(w http.ResponseWriter, req *http.Request, name string) { limitBody(w, req) r.logger.Printf("[INFO] [api] Deploy requested for stack: %s", name) @@ -471,14 +490,8 @@ func (r *Router) deployStack(w http.ResponseWriter, req *http.Request, name stri warning, err := r.stackMgr.DeployStack(deployReq) if err != nil { r.logger.Printf("[ERROR] [api] Deploy failed for %s: %v", name, err) - status := http.StatusInternalServerError - if strings.Contains(err.Error(), "already deployed") { - status = http.StatusConflict - } - if strings.Contains(err.Error(), "required field") || strings.Contains(err.Error(), "does not exist") || strings.Contains(err.Error(), "kötelező") || strings.Contains(err.Error(), "memória") { - status = http.StatusBadRequest - } - writeJSON(w, status, apiResponse{OK: false, Error: err.Error()}) + // R-553 — the status code comes from the refusal's KIND (deployStatusFor), never from its words. + writeJSON(w, deployStatusFor(err), apiResponse{OK: false, Error: err.Error()}) return } diff --git a/controller/internal/backup/offbox.go b/controller/internal/backup/offbox.go index 789e25f..49361f6 100644 --- a/controller/internal/backup/offbox.go +++ b/controller/internal/backup/offbox.go @@ -17,6 +17,7 @@ import ( "time" "gitea.dooplex.hu/admin/felhom-controller/internal/settings" + "gitea.dooplex.hu/admin/felhom-controller/internal/util" ) // Off-box (NAS) backup target — Part B. An ENCRYPTED restic repo reached over SFTP (no kernel mount; @@ -77,6 +78,15 @@ func (m *Manager) SetOffboxSSH(fn func(ctx context.Context, host, user string, p // the orphan card instead of the raw restic error. var ErrOffboxOrphaned = fmt.Errorf("offbox repo orphaned: exists but keyed under a previous, no-longer-available passphrase") +// OffboxWarnNoAppsSelected is the kind of the zero-selection notice (R-553): the run succeeded but +// nothing was selected to copy. The Távoli mentés page replaces that line once the household HAS +// selected apps — it used to find it by searching the sentence for „nincs mentésre jelölt alkalmazás". +const OffboxWarnNoAppsSelected = "no-apps-selected" + +// ErrOffsiteQuota marks a run refused because the repository is at or over its quota (R-553). The +// customer's Hungarian sentence is unchanged; this sentinel is what ClassifyOffsiteFailure reads. +var ErrOffsiteQuota = errors.New("offsite quota exceeded") + // ErrOffboxRunInFlight is returned to the MANUAL caller only, when the single-flight dropped the // request because a run was already going (R-234). It is not a failure of anything — the run in // flight is doing the work — but it IS a request that did nothing, and the page must say so instead @@ -181,10 +191,14 @@ func ClassifyOffsiteFailure(err error) OffsiteFailureClass { if errors.Is(err, ErrOffboxOrphaned) { return OffsiteFailOrphaned } + // R-553 — the quota refusal is OURS, so it is told apart by its sentinel, not by the Hungarian + // word „tárhelykeretet" it happens to contain today. The signatures below stay text matches on + // purpose: they are restic's and ssh's own English output, which we neither write nor translate. + if errors.Is(err, ErrOffsiteQuota) { + return OffsiteFailQuota + } s := strings.ToLower(err.Error()) switch { - case strings.Contains(s, "tárhelykeretet"): - return OffsiteFailQuota case strings.Contains(s, "produced no snapshots"): return OffsiteFailNoUnits case strings.Contains(s, "unable to open config file"), @@ -599,7 +613,7 @@ func (m *Manager) ApplyOffsiteTarget(ctx context.Context, tgt *settings.OffboxTa if cur := m.settings.GetOffboxTarget(); cur != nil { tgt.EscrowState = cur.EscrowState tgt.LastRun, tgt.LastStatus, tgt.LastError = cur.LastRun, cur.LastStatus, cur.LastError - tgt.LastDuration, tgt.LastWarning = cur.LastDuration, cur.LastWarning + tgt.LastDuration, tgt.LastWarning, tgt.LastWarningKind = cur.LastDuration, cur.LastWarning, cur.LastWarningKind // R-100: carry the staleness anchor across a hub re-apply, for the same reason as the rest of // this block — a re-apply is not a new tier. Dropping it would reset an established tier to // "never succeeded" every time the hub re-pushes the descriptor. @@ -918,7 +932,7 @@ func (m *Manager) runOffboxBackup(ctx context.Context, withProgress bool) error // run refuses. m.offboxPruneOnly(ctx, base, env) m.offboxRecordStats(ctx, base, env) // the prune may have brought the size back down — refresh - runErr = fmt.Errorf("A távoli mentés túllépte a tárhelykeretet (%d/%d GB) — törölj régi mentéseket vagy kérj nagyobb keretet.", usedGB, quota) + runErr = util.KindErrorf(ErrOffsiteQuota, "A távoli mentés túllépte a tárhelykeretet (%d/%d GB) — törölj régi mentéseket vagy kérj nagyobb keretet.", usedGB, quota) } else { // R-43/R-44 (v0.148.0) — THE COHERENCE PRE-PHASE. Refresh the DB/volume dumps and the recovery // units BEFORE capturing, so the snapshot restic is about to write is an internally coherent @@ -987,10 +1001,12 @@ func (m *Manager) runOffboxBackup(ctx context.Context, withProgress bool) error o.LastStatus = "error" o.LastError = "" o.LastWarning = "" + o.LastWarningKind = "" } else if runErr != nil { o.LastStatus = "error" o.LastError = runErr.Error() o.LastWarning = "" + o.LastWarningKind = "" } else { // R-203 — THE VERDICT. A run that could not capture a directory the app declares MANDATORY // is not a successful run. Until v0.197.0 it reported `ok` with a warning beside it, and a @@ -1044,6 +1060,7 @@ func (m *Manager) runOffboxBackup(ctx context.Context, withProgress bool) error o.StatsKnown = true // R-225: measured, even if the answer is zero o.EnlargedBlocked = blockedNames // replace each run (sorted); empty slice clears it var warns []string + warnKind := "" // Zero-toggle honesty (take-two obs.): a configured target with NOTHING selected reports // its emptiness instead of a bare success — the customer thinks offsite runs, but nothing // is covered until at least one app is toggled. @@ -1051,6 +1068,7 @@ func (m *Manager) runOffboxBackup(ctx context.Context, withProgress bool) error // must not be told "nothing is selected". if len(apps) == 0 && !runResult.sharesBackedUp { warns = append(warns, "Sikeres — nincs mentésre jelölt alkalmazás") + warnKind = OffboxWarnNoAppsSelected // R-553: the page reads this, not the sentence } // R-234 §7.4 — WHICH apps, WHY, and WHEN. The old sentence said only that N apps "had no // available backup and were left out", which names a problem with no next step and reads @@ -1093,6 +1111,7 @@ func (m *Manager) runOffboxBackup(ctx context.Context, withProgress bool) error warns = append(warns, qw) } o.LastWarning = strings.Join(warns, " ") + o.LastWarningKind = warnKind } }); perr != nil { m.logger.Printf("[WARN] [offbox] status persist (final) failed: %v", perr) diff --git a/controller/internal/backup/offsite_diag_test.go b/controller/internal/backup/offsite_diag_test.go index af82b82..fe2b510 100644 --- a/controller/internal/backup/offsite_diag_test.go +++ b/controller/internal/backup/offsite_diag_test.go @@ -7,6 +7,8 @@ import ( "time" "gitea.dooplex.hu/admin/felhom-controller/internal/settings" + + "gitea.dooplex.hu/admin/felhom-controller/internal/util" ) // the real demo-hp target shape — the values the sanitiser must remove literally @@ -31,7 +33,10 @@ func TestClassifyOffsiteFailure_EachCauseIsDistinct(t *testing.T) { err error want OffsiteFailureClass }{ - {"quota gate", fmt.Errorf("A távoli mentés túllépte a tárhelykeretet (51/50 GB) — törölj régi mentéseket vagy kérj nagyobb keretet."), OffsiteFailQuota}, + // R-553 (v0.251.0): the quota refusal is built with its KIND at the producer, exactly as the run + // builds it — the classifier no longer recognises this sentence by its Hungarian words, and that + // is the point (localisation slice 2 translates them). + {"quota gate", util.KindErrorf(ErrOffsiteQuota, "A távoli mentés túllépte a tárhelykeretet (51/50 GB) — törölj régi mentéseket vagy kérj nagyobb keretet."), OffsiteFailQuota}, {"orphaned repo", fmt.Errorf("probe: %w", ErrOffboxOrphaned), OffsiteFailOrphaned}, {"no repo", fmt.Errorf("restic: unable to open config file: Stat: file does not exist\nIs there a repository at the following location?"), OffsiteFailNoRepo}, {"no units", fmt.Errorf("off-box backup produced no snapshots: 3 app(s) toggled but no recovery unit was found on any connected drive (missing: a, b, c)"), OffsiteFailNoUnits}, diff --git a/controller/internal/backup/r553_offsite_quota_test.go b/controller/internal/backup/r553_offsite_quota_test.go new file mode 100644 index 0000000..fc7c048 --- /dev/null +++ b/controller/internal/backup/r553_offsite_quota_test.go @@ -0,0 +1,101 @@ +package backup + +import ( + "errors" + "os" + "strings" + "testing" + "time" + + "gitea.dooplex.hu/admin/felhom-controller/internal/settings" + "gitea.dooplex.hu/admin/felhom-controller/internal/util" +) + +// R-553 — the off-site failure classifier must keep telling a quota over-run apart after the sentence +// is translated. It used to look for the Hungarian word „tárhelykeretet" inside the error; slice 2 +// translates that sentence, and the customer would then be told the copy failed „ismeretlen okból" +// — unknown cause — for the one failure with a clear, actionable cause. +// +// RED-PROOF (REPORT): put `case strings.Contains(s, "tárhelykeretet")` back and delete the +// errors.Is arm → the translated row falls to OffsiteFailUnknown and this fails. +func TestR553_OffsiteQuota_DecisionSurvivesWordingChange(t *testing.T) { + cases := []struct { + name string + err error + want OffsiteFailureClass + }{ + {"quota refusal, Hungarian as shipped", util.KindErrorf(ErrOffsiteQuota, + "A távoli mentés túllépte a tárhelykeretet (%d/%d GB) — törölj régi mentéseket vagy kérj nagyobb keretet.", 51, 50), OffsiteFailQuota}, + {"quota refusal, TRANSLATED", util.KindErrorf(ErrOffsiteQuota, + "The remote backup is over its storage quota (%d/%d GB) — delete old backups or ask for more space.", 51, 50), OffsiteFailQuota}, + {"quota refusal wrapped by a caller", util.KindErrorf(ErrOffsiteQuota, "over quota"), OffsiteFailQuota}, + // Negative controls: output we do NOT write stays matched by its text, on purpose. + {"restic: no repository", errors.New("Fatal: unable to open config file: Stat: file does not exist"), OffsiteFailNoRepo}, + {"ssh: unreachable", errors.New("dial tcp 10.0.0.9:22: connect: connection refused"), OffsiteFailTransport}, + {"nothing to copy", errors.New("off-box backup produced no snapshots: 2 app(s) toggled"), OffsiteFailNoUnits}, + {"unknown stays unknown", errors.New("something else entirely"), OffsiteFailUnknown}, + {"no error", nil, ""}, + } + for _, c := range cases { + if got := ClassifyOffsiteFailure(c.err); got != c.want { + t.Errorf("%s: ClassifyOffsiteFailure = %q, want %q", c.name, got, c.want) + } + } +} + +// The consequence, not only the mechanism: the head line the customer reads on /backups/remote is the +// quota one for a TRANSLATED quota error. (offsiteFailureMessage is the only caller of the classifier.) +func TestR553_OffsiteQuota_HeadLineSurvivesWordingChange(t *testing.T) { + tgt := &settings.OffboxTarget{Host: "nas.local", User: "felhom", RepoPath: "/srv/repo"} + translated := util.KindErrorf(ErrOffsiteQuota, "The remote backup is over its storage quota (51/50 GB).") + msg := offsiteFailureMessage(tgt, translated, 12*time.Second) + if !strings.HasPrefix(msg, "A távoli mentés nem fért el a tárhelykereten belül") { + t.Errorf("a translated quota failure is reported with the wrong cause line: %q", msg) + } + unknown := errors.New("The remote backup is over its storage quota (51/50 GB).") + if m := offsiteFailureMessage(tgt, unknown, time.Second); strings.HasPrefix(m, "A távoli mentés nem fért el") { + t.Errorf("an error WITHOUT the kind must not be guessed into the quota class from its words: %q", m) + } +} + +// The producer keeps its Hungarian sentence byte-for-byte while carrying the kind. +func TestR553_QuotaProducerKeepsItsWords(t *testing.T) { + err := util.KindErrorf(ErrOffsiteQuota, + "A távoli mentés túllépte a tárhelykeretet (%d/%d GB) — törölj régi mentéseket vagy kérj nagyobb keretet.", 51, 50) + want := "A távoli mentés túllépte a tárhelykeretet (51/50 GB) — törölj régi mentéseket vagy kérj nagyobb keretet." + if err.Error() != want { + t.Errorf("message CHANGED:\n got %q\nwant %q", err.Error(), want) + } + if !errors.Is(err, ErrOffsiteQuota) { + t.Error("the quota refusal carries no kind") + } +} + +// The zero-selection run must RECORD its kind beside the sentence, or the page falls back to reading +// the words for ever. A real off-site run needs restic and an SSH target, so this is pinned at the +// source — the defect it guards is a producer written the old way, and the display half is covered by +// TestR553_StaleNoteDecisionSurvivesWordingChange in internal/web. +func TestR553_OffboxRunRecordsTheKind(t *testing.T) { + src, err := os.ReadFile("offbox.go") + if err != nil { + t.Fatal(err) + } + body := string(src) + i := strings.Index(body, `warns = append(warns, "Sikeres — nincs mentésre jelölt alkalmazás")`) + if i < 0 { + t.Fatal("the zero-selection sentence is gone from the run — this test no longer reads what it thinks it reads") + } + if !strings.Contains(body[i:i+400], "warnKind = OffboxWarnNoAppsSelected") { + t.Error("the zero-selection run records its sentence but not its KIND — the Távoli mentés page " + + "is left reading Hungarian words, which localisation slice 2 will change (R-553)") + } + if !strings.Contains(body, "o.LastWarningKind = warnKind") { + t.Error("the recorded kind is never persisted, so the page sees nothing after a restart") + } + for _, clear := range []string{`o.LastWarning = "" + o.LastWarningKind = ""`} { + if !strings.Contains(body, clear) { + t.Error("a run that clears LastWarning must clear its kind too, or a stale kind outlives its text") + } + } +} diff --git a/controller/internal/monitor/healthcheck.go b/controller/internal/monitor/healthcheck.go index 9795700..e82eb98 100644 --- a/controller/internal/monitor/healthcheck.go +++ b/controller/internal/monitor/healthcheck.go @@ -16,11 +16,42 @@ import ( // HealthReport contains the results of a system health check. type HealthReport struct { - Status string // "ok", "warn", "fail" - Issues []string // critical problems - Warnings []string // non-critical warnings - Info []string // informational items - Timestamp time.Time + Status string // "ok", "warn", "fail" + Issues []string // critical problems + Warnings []string // non-critical warnings + // WarningKinds is parallel to Warnings: one entry per warning, "" when the warning has no kind. + // R-553 — it exists so the dashboard can decide WHERE a warning is shown without reading the + // warning's Hungarian words (internal/web/alerts.go used to match „meghajtó"/„adattároló", which + // localisation slice 2 translates). It is INTERNAL: internal/report/builder.go copies Status, + // Issues and Warnings only, so the hub report is unchanged — pinned by + // TestR553_HubReportWarningsAreUnchangedOnTheWire. + WarningKinds []string + Info []string // informational items + Timestamp time.Time +} + +// Warning kinds (R-553). A kind names WHAT the warning is about; the text stays the only thing shown. +const ( + WarnKindStorageNotSeparate = "storage-not-separate" // app data sits on the system drive + WarnKindStorageDisconnected = "storage-disconnected" // a registered drive is unplugged + WarnKindStorageUnavailable = "storage-unavailable" // the path cannot be read + WarnKindStorageUsageHigh = "storage-usage-high" // a data drive is filling up +) + +// addWarning appends a warning together with its kind, so the two slices cannot drift apart. Every +// warning goes through here; `WarningKindAt` reads them back. +func (r *HealthReport) addWarning(text, kind string) { + r.Warnings = append(r.Warnings, text) + r.WarningKinds = append(r.WarningKinds, kind) +} + +// WarningKindAt returns the kind of Warnings[i], or "" when there is none (older callers, a report +// built by hand in a test, or a warning that simply has no kind). +func (r *HealthReport) WarningKindAt(i int) string { + if r == nil || i < 0 || i >= len(r.WarningKinds) { + return "" + } + return r.WarningKinds[i] } // RunHealthCheck runs system checks and returns a diagnostic report. @@ -61,7 +92,7 @@ func RunHealthCheck(cfg *config.Config, cpuCollector *system.CPUCollector, stora logger.Printf("[DEBUG] [monitor] SSD disk: CRITICAL (%.0f%% >= %d%%)", sysInfo.DiskPercent, cfg.Monitoring.Thresholds.DiskCritPercent) } } else if sysInfo.DiskPercent >= float64(cfg.Monitoring.Thresholds.DiskWarnPercent) { - report.Warnings = append(report.Warnings, fmt.Sprintf("SSD disk usage high: %.0f%%", sysInfo.DiskPercent)) + report.addWarning(fmt.Sprintf("SSD disk usage high: %.0f%%", sysInfo.DiskPercent), "") if logger != nil { logger.Printf("[WARN] [monitor] Disk (SSD) threshold breached: %.0f%% (limit: %d%%)", sysInfo.DiskPercent, cfg.Monitoring.Thresholds.DiskWarnPercent) } @@ -84,7 +115,7 @@ func RunHealthCheck(cfg *config.Config, cpuCollector *system.CPUCollector, stora logger.Printf("[WARN] [monitor] Disk (HDD) threshold breached: %.0f%% (limit: %d%%)", sysInfo.HDDPercent, cfg.Monitoring.Thresholds.DiskCritPercent) } } else if sysInfo.HDDPercent >= float64(cfg.Monitoring.Thresholds.DiskWarnPercent) { - report.Warnings = append(report.Warnings, fmt.Sprintf("HDD disk usage high: %.0f%%", sysInfo.HDDPercent)) + report.addWarning(fmt.Sprintf("HDD disk usage high: %.0f%%", sysInfo.HDDPercent), "") if logger != nil { logger.Printf("[WARN] [monitor] Disk (HDD) threshold breached: %.0f%% (limit: %d%%)", sysInfo.HDDPercent, cfg.Monitoring.Thresholds.DiskWarnPercent) } @@ -94,7 +125,7 @@ func RunHealthCheck(cfg *config.Config, cpuCollector *system.CPUCollector, stora // 2. Memory usage if sysInfo.MemPercent > 0 { if sysInfo.MemPercent >= float64(cfg.Monitoring.Thresholds.MemoryWarnPercent) { - report.Warnings = append(report.Warnings, fmt.Sprintf("Memory usage high: %.0f%%", sysInfo.MemPercent)) + report.addWarning(fmt.Sprintf("Memory usage high: %.0f%%", sysInfo.MemPercent), "") if logger != nil { logger.Printf("[WARN] [monitor] Memory threshold breached: %.0f%% (limit: %d%%)", sysInfo.MemPercent, cfg.Monitoring.Thresholds.MemoryWarnPercent) } @@ -112,7 +143,7 @@ func RunHealthCheck(cfg *config.Config, cpuCollector *system.CPUCollector, stora // 3. CPU usage if sysInfo.CPUPercent > 0 { if sysInfo.CPUPercent >= float64(cfg.Monitoring.Thresholds.CPUWarnPercent) { - report.Warnings = append(report.Warnings, fmt.Sprintf("CPU usage high: %.0f%%", sysInfo.CPUPercent)) + report.addWarning(fmt.Sprintf("CPU usage high: %.0f%%", sysInfo.CPUPercent), "") if logger != nil { logger.Printf("[WARN] [monitor] CPU threshold breached: %.0f%% (limit: %d%%)", sysInfo.CPUPercent, cfg.Monitoring.Thresholds.CPUWarnPercent) } @@ -130,7 +161,7 @@ func RunHealthCheck(cfg *config.Config, cpuCollector *system.CPUCollector, stora // 4. Temperature if sysInfo.TemperatureCelsius > 0 { if sysInfo.TemperatureCelsius >= float64(cfg.Monitoring.Thresholds.TemperatureWarnCelsius) { - report.Warnings = append(report.Warnings, fmt.Sprintf("Temperature high: %.0f°C (%s)", sysInfo.TemperatureCelsius, sysInfo.TemperatureSource)) + report.addWarning(fmt.Sprintf("Temperature high: %.0f°C (%s)", sysInfo.TemperatureCelsius, sysInfo.TemperatureSource), "") if logger != nil { logger.Printf("[WARN] [monitor] Temperature threshold breached: %.0f°C (limit: %d°C)", sysInfo.TemperatureCelsius, cfg.Monitoring.Thresholds.TemperatureWarnCelsius) } @@ -177,9 +208,11 @@ func RunHealthCheck(cfg *config.Config, cpuCollector *system.CPUCollector, stora } // 7. Storage paths - storageIssues, storageWarnings := checkStoragePaths(storagePaths) + storageIssues, storageWarnings, storageKinds := checkStoragePaths(storagePaths) report.Issues = append(report.Issues, storageIssues...) - report.Warnings = append(report.Warnings, storageWarnings...) + for i, w := range storageWarnings { + report.addWarning(w, storageKinds[i]) + } // Determine status if len(report.Issues) > 0 { @@ -309,7 +342,10 @@ func checkProtectedContainers(protected []string) []string { return missing } -func checkStoragePaths(paths []settings.StoragePath) (issues, warnings []string) { +// checkStoragePaths returns the storage issues and, beside each warning, its KIND (R-553) — the +// dashboard places the "not on a separate drive" warning inline under the storage bars, and it must +// find it by kind rather than by the words the sentence happens to contain today. +func checkStoragePaths(paths []settings.StoragePath) (issues, warnings, kinds []string) { for _, sp := range paths { // Skip decommissioned paths — no longer in active use if sp.Decommissioned { @@ -318,13 +354,13 @@ func checkStoragePaths(paths []settings.StoragePath) (issues, warnings []string) // Skip disconnected paths — handled by the storage watchdog if sp.Disconnected { - warnings = append(warnings, fmt.Sprintf("Meghajtó leválasztva: %s (%s)", sp.Label, sp.Path)) + warnings, kinds = append(warnings, fmt.Sprintf("Meghajtó leválasztva: %s (%s)", sp.Label, sp.Path)), append(kinds, WarnKindStorageDisconnected) continue } // Path accessible? if _, err := os.Stat(sp.Path); err != nil { - warnings = append(warnings, fmt.Sprintf("Adattároló nem elérhető: %s", sp.Path)) + warnings, kinds = append(warnings, fmt.Sprintf("Adattároló nem elérhető: %s", sp.Path)), append(kinds, WarnKindStorageUnavailable) continue } @@ -332,6 +368,7 @@ func checkStoragePaths(paths []settings.StoragePath) (issues, warnings []string) if !system.IsMountPoint(sp.Path) { warnings = append(warnings, fmt.Sprintf( "Az adattároló (%s) nem külön meghajtón van — az adatok a rendszermeghajtóra íródnak", sp.Path)) + kinds = append(kinds, WarnKindStorageNotSeparate) } // Disk usage @@ -339,7 +376,7 @@ func checkStoragePaths(paths []settings.StoragePath) (issues, warnings []string) if di.UsedPercent >= 95 { issues = append(issues, fmt.Sprintf("Adattároló majdnem megtelt: %s (%.0f%%)", sp.Path, di.UsedPercent)) } else if di.UsedPercent >= 90 { - warnings = append(warnings, fmt.Sprintf("Adattároló használat magas: %s (%.0f%%)", sp.Path, di.UsedPercent)) + warnings, kinds = append(warnings, fmt.Sprintf("Adattároló használat magas: %s (%.0f%%)", sp.Path, di.UsedPercent)), append(kinds, WarnKindStorageUsageHigh) } } } diff --git a/controller/internal/monitor/r553_warning_kinds_test.go b/controller/internal/monitor/r553_warning_kinds_test.go new file mode 100644 index 0000000..17f2fd9 --- /dev/null +++ b/controller/internal/monitor/r553_warning_kinds_test.go @@ -0,0 +1,74 @@ +package monitor + +import ( + "os" + "path/filepath" + "testing" + + "gitea.dooplex.hu/admin/felhom-controller/internal/settings" +) + +// R-553 — the storage warnings carry a KIND, and their sentences are byte-for-byte unchanged. +// +// checkStoragePaths is the real producer the health check folds into the report. The kinds are what +// the dashboard reads to decide WHERE a warning is shown; the sentence is what the household reads. +// +// RED-PROOF (REPORT): drop the `kinds = append(…, WarnKindStorageNotSeparate)` line and the +// placement test in internal/web fails with the warning back in the top banner. +func TestR553_StorageWarningsCarryKindsAndKeepTheirWords(t *testing.T) { + dir := t.TempDir() + present := filepath.Join(dir, "data") + if err := os.MkdirAll(present, 0o755); err != nil { + t.Fatal(err) + } + paths := []settings.StoragePath{ + {Path: filepath.Join(dir, "gone"), Label: "Leválasztott", Disconnected: true}, + {Path: filepath.Join(dir, "nem-letezik")}, + {Path: present}, + {Path: filepath.Join(dir, "kihagyott"), Decommissioned: true}, + } + _, warnings, kinds := checkStoragePaths(paths) + if len(warnings) != len(kinds) { + t.Fatalf("a warning without its kind (or the reverse): %d warnings, %d kinds", len(warnings), len(kinds)) + } + want := []struct{ text, kind string }{ + {"Meghajtó leválasztva: Leválasztott (" + filepath.Join(dir, "gone") + ")", WarnKindStorageDisconnected}, + {"Adattároló nem elérhető: " + filepath.Join(dir, "nem-letezik"), WarnKindStorageUnavailable}, + {"Az adattároló (" + present + ") nem külön meghajtón van — az adatok a rendszermeghajtóra íródnak", WarnKindStorageNotSeparate}, + } + if len(warnings) != len(want) { + t.Fatalf("warnings = %q, want %d of them", warnings, len(want)) + } + for i, w := range want { + if warnings[i] != w.text { + t.Errorf("warning %d CHANGED:\n got %q\nwant %q", i, warnings[i], w.text) + } + if kinds[i] != w.kind { + t.Errorf("warning %d has kind %q, want %q", i, kinds[i], w.kind) + } + } + // The decommissioned path is skipped entirely — it must not appear with an empty kind either. + for i, w := range warnings { + if kinds[i] == "" { + t.Errorf("storage warning %q carries no kind — the dashboard would fall back to placing it in the banner", w) + } + } +} + +// A warning and its kind can never drift apart, because every warning goes through addWarning. +func TestR553_WarningKindsStayParallel(t *testing.T) { + r := &HealthReport{} + r.addWarning("CPU usage high: 91%", "") + r.addWarning("Az adattároló (/mnt/x) nem külön meghajtón van", WarnKindStorageNotSeparate) + if len(r.Warnings) != 2 || len(r.WarningKinds) != 2 { + t.Fatalf("lengths drifted: %d warnings, %d kinds", len(r.Warnings), len(r.WarningKinds)) + } + if r.WarningKindAt(0) != "" || r.WarningKindAt(1) != WarnKindStorageNotSeparate { + t.Errorf("kinds read back wrong: %q, %q", r.WarningKindAt(0), r.WarningKindAt(1)) + } + // A report built by hand (an older caller, a test fixture) answers "" instead of panicking. + old := &HealthReport{Warnings: []string{"x"}} + if old.WarningKindAt(0) != "" || old.WarningKindAt(9) != "" { + t.Error("a report with no kinds must answer \"\", never index out of range") + } +} diff --git a/controller/internal/settings/settings.go b/controller/internal/settings/settings.go index 7a1863b..3009557 100644 --- a/controller/internal/settings/settings.go +++ b/controller/internal/settings/settings.go @@ -34,10 +34,10 @@ type Settings struct { // separate enabled flag — an empty token matches nothing). LauncherSharePasswordHash is an // OPTIONAL bcrypt hash for a per-share password, ALWAYS SEPARATE from the admin PasswordHash above. // The token is a secret and must never be logged. - LauncherShareToken string `json:"launcher_share_token,omitempty"` + LauncherShareToken string `json:"launcher_share_token,omitempty"` // Language (v0.247.0, i18n) — the household's dashboard language: "hu" | "en". Empty means never // chosen and reads as Hungarian (GetLanguage). Reported to the hub so its e-mails can follow. - Language string `json:"language,omitempty"` + Language string `json:"language,omitempty"` LauncherSharePasswordHash string `json:"launcher_share_password_hash,omitempty"` // FileBrowser admin login (R-513, v0.243.0). Every box used to accept admin/admin. The controller @@ -368,6 +368,11 @@ type OffboxTarget struct { // LastWarning is a customer-visible notice set on an otherwise-OK run when SOME toggled apps had // no discoverable recovery unit (partial run). Empty on a fully-successful or failed run. LastWarning string `json:"last_warning,omitempty"` + // LastWarningKind names WHAT LastWarning is about, so the page can react to it without reading its + // Hungarian words (R-553). Today one kind exists: OffboxWarnNoAppsSelected, the zero-selection + // notice the Távoli mentés page replaces once apps HAVE been selected. Written by the run that + // writes LastWarning, cleared with it. + LastWarningKind string `json:"last_warning_kind,omitempty"` // EnlargedBlocked (3a) lists the apps whose ENLARGED (mandatory-userdata) offsite push was refused // by the pre-push quota gate on the last run — their unit-only push still succeeded. Replaced each // OK run (sorted; empty clears). Drives the per-app "config+DB only" note on /backups/remote and diff --git a/controller/internal/stacks/deploy.go b/controller/internal/stacks/deploy.go index 09bc97f..c99bfe6 100644 --- a/controller/internal/stacks/deploy.go +++ b/controller/internal/stacks/deploy.go @@ -4,7 +4,6 @@ import ( "crypto/rand" "encoding/base64" "encoding/hex" - "errors" "fmt" "log" "math/big" @@ -17,6 +16,7 @@ import ( "gitea.dooplex.hu/admin/felhom-controller/internal/appbackup" "gitea.dooplex.hu/admin/felhom-controller/internal/crypto" "gitea.dooplex.hu/admin/felhom-controller/internal/system" + "gitea.dooplex.hu/admin/felhom-controller/internal/util" "gopkg.in/yaml.v3" ) @@ -202,7 +202,7 @@ func (m *Manager) DeployStack(req DeployRequest) (string, error) { } if sPtr.Deployed { m.mu.Unlock() - return "", fmt.Errorf("stack %q is already deployed; use update instead", req.StackName) + return "", util.KindErrorf(ErrAlreadyDeployed, "stack %q is already deployed; use update instead", req.StackName) } sPtr.Deploying = true sPtr.DeployError = "" @@ -242,7 +242,7 @@ func (m *Manager) DeployStack(req DeployRequest) (string, error) { refusal, deployWarning := m.memoryVerdict(ParseMemoryMB(meta.Resources.MemRequest), ParseMemoryMB(meta.Resources.MemLimit), 0, 0) if refusal != "" { clearDeploying() - return "", errors.New(refusal) + return "", util.KindError(ErrNotEnoughMemory, refusal) } // Debug: log received values (redact passwords/secrets) @@ -308,7 +308,7 @@ func (m *Manager) DeployStack(req DeployRequest) (string, error) { value = userVal } else { clearDeploying() - return "", fmt.Errorf("a(z) %q mező kitöltése kötelező — használja a Generálás gombot vagy írjon be egy jelszót", field.Label) + return "", util.KindErrorf(ErrRequiredField, "a(z) %q mező kitöltése kötelező — használja a Generálás gombot vagy írjon be egy jelszót", field.Label) } default: @@ -323,14 +323,14 @@ func (m *Manager) DeployStack(req DeployRequest) (string, error) { // Validate required fields if field.Required && value == "" { clearDeploying() - return "", fmt.Errorf("a(z) %q (%s) mező kitöltése kötelező", field.Label, field.EnvVar) + return "", util.KindErrorf(ErrRequiredField, "a(z) %q (%s) mező kitöltése kötelező", field.Label, field.EnvVar) } // Validate path fields exist on the host filesystem if field.Type == "path" && value != "" { if _, err := os.Stat(value); os.IsNotExist(err) { clearDeploying() - return "", fmt.Errorf("path %q does not exist for field %q", value, field.Label) + return "", util.KindErrorf(ErrPathMissing, "path %q does not exist for field %q", value, field.Label) } } @@ -408,7 +408,9 @@ func (m *Manager) DeployStack(req DeployRequest) (string, error) { // // Same shape as SetMigrationDoneHook, deliberately: the policy (which event, what wording) lives in // the caller, and this package only reports what happened. -func (m *Manager) SetDeployDoneHook(fn func(name string, ok bool, detail string)) { m.deployDoneHook = fn } +func (m *Manager) SetDeployDoneHook(fn func(name string, ok bool, detail string)) { + m.deployDoneHook = fn +} func (m *Manager) runComposeDeploy(name, stackDir string, env map[string]string, appCfg *AppConfig) { start := time.Now() diff --git a/controller/internal/stacks/deploy_errors.go b/controller/internal/stacks/deploy_errors.go new file mode 100644 index 0000000..4525909 --- /dev/null +++ b/controller/internal/stacks/deploy_errors.go @@ -0,0 +1,21 @@ +package stacks + +import "errors" + +// R-553 — the deploy path's refusals carry a KIND, so the API can pick its status code without +// reading the customer's Hungarian sentence. The sentences themselves are unchanged (a test pins +// each one byte-for-byte); `internal/util.KindErrorf` builds the message exactly as fmt.Errorf did. +// +// Before this, internal/api/router.go matched „kötelező", „memória", "does not exist" and +// "already deployed" in the error text — so localisation slice 2 would have turned every validation +// refusal into a 500. +var ( + // ErrAlreadyDeployed — the stack is already deployed (API: 409). + ErrAlreadyDeployed = errors.New("stack already deployed") + // ErrRequiredField — a required (or password) field was left empty (API: 400). + ErrRequiredField = errors.New("required field empty") + // ErrPathMissing — a path field names something that is not on the filesystem (API: 400). + ErrPathMissing = errors.New("path field does not exist") + // ErrNotEnoughMemory — the memory verdict refused the deploy (API: 400). + ErrNotEnoughMemory = errors.New("not enough memory") +) diff --git a/controller/internal/stacks/r553_deploy_error_kinds_test.go b/controller/internal/stacks/r553_deploy_error_kinds_test.go new file mode 100644 index 0000000..83bc352 --- /dev/null +++ b/controller/internal/stacks/r553_deploy_error_kinds_test.go @@ -0,0 +1,137 @@ +package stacks + +import ( + "errors" + "io" + "log" + "os" + "path/filepath" + "strings" + "testing" + + "gitea.dooplex.hu/admin/felhom-controller/internal/config" +) + +// r553Manager builds a real Manager over a temp stacks dir holding one app fixture, so DeployStack's +// own validation runs — no stub, no modelled predicate. +func r553Manager(t *testing.T, felhomYml string) *Manager { + t.Helper() + dir := t.TempDir() + cfg := &config.Config{} + cfg.Paths.StacksDir = filepath.Join(dir, "stacks") + cfg.Paths.SystemDataPath = filepath.Join(dir, "system") + cfg.Stacks.ComposeCommand = "docker compose" // no detection; nothing is executed in this test + appDir := filepath.Join(cfg.Paths.StacksDir, "r553app") + if err := os.MkdirAll(appDir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(appDir, "docker-compose.yml"), []byte("services:\n app:\n image: busybox\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(appDir, ".felhom.yml"), []byte(felhomYml), 0o644); err != nil { + t.Fatal(err) + } + m, err := NewManager(cfg, log.New(io.Discard, "", 0)) + if err != nil { + t.Fatal(err) + } + if err := m.ScanStacks(); err != nil { + t.Fatal(err) + } + if _, ok := m.GetStack("r553app"); !ok { + t.Fatal("fixture invalid: the app was not scanned, so DeployStack would refuse for the wrong reason") + } + return m +} + +// R-553 — the deploy refusals carry their KIND, and their Hungarian sentences are byte-for-byte what +// they were before the kinds existed (a localisation-adjacent change may not move one byte of copy). +// +// RED-PROOF (REPORT): drop the kind from either producer in deploy.go (back to plain fmt.Errorf) and +// the errors.Is assertion fails, while the message assertion still passes — which is exactly the +// silent state this row exists to prevent. +func TestR553_DeployProducersCarryKindAndKeepTheirWords(t *testing.T) { + cases := []struct { + name string + yml string + values map[string]string + kind error + wantMsg string + }{ + { + name: "required field left empty", + yml: "display_name: R553\ndeploy_fields:\n - env_var: DATA_DIR\n label: Adatmappa\n type: text\n required: true\n", + values: map[string]string{}, + kind: ErrRequiredField, + wantMsg: `a(z) "Adatmappa" (DATA_DIR) mező kitöltése kötelező`, + }, + { + name: "password field left empty", + yml: "display_name: R553\ndeploy_fields:\n - env_var: ADMIN_PW\n label: Jelszó\n type: password\n", + values: map[string]string{}, + kind: ErrRequiredField, + wantMsg: `a(z) "Jelszó" mező kitöltése kötelező — használja a Generálás gombot vagy írjon be egy jelszót`, + }, + { + name: "path field naming something that is not there", + yml: "display_name: R553\ndeploy_fields:\n - env_var: MEDIA\n label: Média\n type: path\n", + values: map[string]string{"MEDIA": "/definitely/not/here/r553"}, + kind: ErrPathMissing, + wantMsg: `path "/definitely/not/here/r553" does not exist for field "Média"`, + }, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + m := r553Manager(t, c.yml) + _, err := m.DeployStack(DeployRequest{StackName: "r553app", Values: c.values}) + if err == nil { + t.Fatal("deploy was admitted — the fixture no longer exercises the refusal") + } + if !errors.Is(err, c.kind) { + t.Errorf("refusal carries no kind: %v (want errors.Is … %v)", err, c.kind) + } + if err.Error() != c.wantMsg { + t.Errorf("the customer's sentence CHANGED:\n got %q\nwant %q", err.Error(), c.wantMsg) + } + }) + } +} + +// The already-deployed refusal, on the same Manager: its words are unchanged and it carries its kind +// (the API answers 409 from that kind). +func TestR553_AlreadyDeployedCarriesKindAndKeepsItsWords(t *testing.T) { + m := r553Manager(t, "display_name: R553\n") + m.mu.Lock() + m.stacks["r553app"].Deployed = true + m.mu.Unlock() + _, err := m.DeployStack(DeployRequest{StackName: "r553app"}) + if err == nil { + t.Fatal("a deployed stack was admitted for a second deploy") + } + if !errors.Is(err, ErrAlreadyDeployed) { + t.Errorf("refusal carries no kind: %v", err) + } + if want := `stack "r553app" is already deployed; use update instead`; err.Error() != want { + t.Errorf("message CHANGED:\n got %q\nwant %q", err.Error(), want) + } +} + +// The memory refusal is a string built by memoryVerdict from the HOST's real memory, so a unit test +// cannot make it fire. Its kind is therefore pinned where it is attached — at the one line that turns +// that string into an error. Source-level on purpose, like TestDeployAcceptance_DoesNotClaimTheAppIsInstalled: +// the defect this guards is a call written the old way, not a wrong value. The status mapping itself +// is covered by TestR553_Deploy_DecisionSurvivesWordingChange with a translated memory message. +func TestR553_MemoryRefusalIsWrappedWithItsKind(t *testing.T) { + src, err := os.ReadFile("deploy.go") + if err != nil { + t.Fatal(err) + } + body := string(src) + if !strings.Contains(body, `util.KindError(ErrNotEnoughMemory, refusal)`) { + t.Error("the memory refusal no longer carries ErrNotEnoughMemory — the API would answer 500 " + + "for a refusal the customer can act on, and translating the sentence would hide it completely") + } + if strings.Contains(body, `errors.New(refusal)`) { + t.Error("the memory refusal is back to a bare errors.New: its kind is gone (R-553)") + } +} diff --git a/controller/internal/util/errkind.go b/controller/internal/util/errkind.go new file mode 100644 index 0000000..f466e8c --- /dev/null +++ b/controller/internal/util/errkind.go @@ -0,0 +1,36 @@ +package util + +import "fmt" + +// R-553 — a decision must never be made by reading a customer-facing Hungarian word. +// +// Five places in this product branched on their own copy (`strings.Contains(err.Error(), "kötelező")` +// and friends). The day that copy is translated — localisation slice 2 — every one of them silently +// takes the other branch: an install error becomes a 500, a quota failure reads as "unknown cause". +// +// KindErrorf is the fix in one line: it produces the EXACT same message bytes as `fmt.Errorf` while +// carrying a sentinel that `errors.Is` can test. The message is display; the sentinel is the decision. +// +// return util.KindErrorf(ErrRequiredField, "a(z) %q mező kitöltése kötelező", label) +// ... +// if errors.Is(err, stacks.ErrRequiredField) { status = http.StatusBadRequest } +// +// Deliberately NOT `fmt.Errorf("%w: …")`: that prepends the sentinel's own text and would change the +// sentence the customer reads, which a localisation-adjacent change may never do. +func KindErrorf(kind error, format string, a ...interface{}) error { + return &kindError{kind: kind, msg: fmt.Sprintf(format, a...)} +} + +// KindError wraps an existing message (a string built elsewhere) with its kind, bytes unchanged. +func KindError(kind error, msg string) error { return &kindError{kind: kind, msg: msg} } + +type kindError struct { + kind error + msg string +} + +func (e *kindError) Error() string { return e.msg } + +// Unwrap is what makes errors.Is(err, kind) true. It deliberately returns the KIND, not a cause +// chain: the message is a leaf, and the kind is the only thing a caller may branch on. +func (e *kindError) Unwrap() error { return e.kind } diff --git a/controller/internal/web/alerts.go b/controller/internal/web/alerts.go index 169d19c..a9889f1 100644 --- a/controller/internal/web/alerts.go +++ b/controller/internal/web/alerts.go @@ -4,7 +4,6 @@ import ( "fmt" "hash/crc32" "log" - "strings" "sync" "time" @@ -196,7 +195,7 @@ func (am *AlertManager) Refresh(report *monitor.HealthReport, cfg *config.Config } // From health check warnings - for _, w := range report.Warnings { + for i, w := range report.Warnings { alert := Alert{ ID: "health-" + simpleHash(w), Level: "warning", @@ -204,8 +203,13 @@ func (am *AlertManager) Refresh(report *monitor.HealthReport, cfg *config.Config Link: "/monitoring", LinkText: "Rendszermonitor", } - // Disk-related warnings rendered inline under storage bars, not in top banner - if strings.Contains(w, "meghajtón") || strings.Contains(w, "adattároló") || strings.Contains(w, "meghajtó") { + // R-553 — WHERE this warning is shown is decided by its KIND, not by the Hungarian words it + // contains. The old test was `strings.Contains(w, "meghajtón"/"adattároló"/"meghajtó")`, which + // matched exactly one of today's warnings (the not-on-a-separate-drive one, the only lower-case + // „meghajtón"); the day that sentence is translated the warning would jump from its quiet place + // under the storage bars to the red banner on EVERY page. Pinned by + // TestR553_DiskWarningPlacementSurvivesWordingChange. + if report.WarningKindAt(i) == monitor.WarnKindStorageNotSeparate { alert.ID = "disk-not-separate" alert.PageOnly = []string{"dashboard", "monitoring"} alert.Inline = true diff --git a/controller/internal/web/handlers.go b/controller/internal/web/handlers.go index 90b388d..57fd438 100644 --- a/controller/internal/web/handlers.go +++ b/controller/internal/web/handlers.go @@ -897,7 +897,7 @@ func (s *Server) backupsOffboxData(data map[string]interface{}) { data["OffboxToggledCount"] = offboxToggled // Part E (v0.126.0): the LastWarning DISPLAY pick — never a state mutation. if offboxTgt != nil { - data["OffboxWarningDisplay"] = offboxWarningDisplay(offboxTgt.LastWarning, offboxToggled) + data["OffboxWarningDisplay"] = offboxWarningDisplay(offboxTgt.LastWarning, offboxTgt.LastWarningKind, offboxToggled) } else { data["OffboxWarningDisplay"] = "" } @@ -913,9 +913,9 @@ func (s *Server) backupsOffboxData(data map[string]interface{}) { data["OffboxBlockedSet"] = blocked } -// offboxStaleWarningMarker is the substring the zero-toggled offbox run writes into -// LastWarning (backup/offbox.go); offboxSelectionChangedLine replaces it once the -// selection has moved on. +// offboxStaleWarningMarker is the substring the zero-toggled offbox run wrote into LastWarning before +// v0.251.0, when the run started recording a KIND beside it (R-553). It is kept ONLY to read boxes +// upgraded with that older text already persisted — see offboxWarningDisplay. const ( offboxStaleWarningMarker = "nincs mentésre jelölt alkalmazás" offboxSelectionChangedLine = "A kijelölés módosult az utolsó futás óta — a következő távoli mentés már tartalmazza." @@ -927,8 +927,21 @@ const ( // replace it with the honest "selection changed, the next run covers it" note. // Pure display logic: the persisted LastWarning is never touched, and every other // warning (quota, partial failure) passes through verbatim. -func offboxWarningDisplay(lastWarning string, toggledCount int) string { - if toggledCount >= 1 && strings.Contains(lastWarning, offboxStaleWarningMarker) { +// R-553 — the decision is made on the KIND the run recorded, not on the words of the sentence. +// The one text test that remains runs ONLY when there is no kind, i.e. on a box whose last off-site +// run happened before v0.251.0 and whose persisted warning therefore predates kinds. +// +// R-553 legacy: remove after every fleet box has completed one off-site run on ≥ 0.251.0 (row R-570). +// Localisation slice 2 (R-557) must not translate the producer at backup/offbox.go until that row is +// closed — translating it while this fallback is still needed would strand exactly those boxes. +func offboxWarningDisplay(lastWarning, kind string, toggledCount int) string { + if toggledCount < 1 { + return lastWarning + } + if kind == backup.OffboxWarnNoAppsSelected { + return offboxSelectionChangedLine + } + if kind == "" && strings.Contains(lastWarning, offboxStaleWarningMarker) { return offboxSelectionChangedLine } return lastWarning diff --git a/controller/internal/web/offbox_handlers.go b/controller/internal/web/offbox_handlers.go index 6cc97bd..b136476 100644 --- a/controller/internal/web/offbox_handlers.go +++ b/controller/internal/web/offbox_handlers.go @@ -100,7 +100,7 @@ func (s *Server) offboxConfigHandler(w http.ResponseWriter, r *http.Request) { // succeeded" on a routine settings save. Pinned by TestOffboxEdit_PreservesLastSuccess. tgt.LastSuccess = prev.LastSuccess tgt.LastDuration, tgt.RepoSizeHuman, tgt.SnapshotCount = prev.LastDuration, prev.RepoSizeHuman, prev.SnapshotCount - tgt.LastWarning = prev.LastWarning + tgt.LastWarning, tgt.LastWarningKind = prev.LastWarning, prev.LastWarningKind tgt.EscrowState = prev.EscrowState tgt.RepoSizeBytes = prev.RepoSizeBytes tgt.StatsKnown = prev.StatsKnown // R-225: preserved with the numbers it qualifies diff --git a/controller/internal/web/offbox_warning_display_test.go b/controller/internal/web/offbox_warning_display_test.go index 424ff98..ae9cb25 100644 --- a/controller/internal/web/offbox_warning_display_test.go +++ b/controller/internal/web/offbox_warning_display_test.go @@ -12,25 +12,28 @@ import ( // jelölt alkalmazás" run-result for the selection-changed note once ≥1 app is toggled. // COMPANION red-proof (recorded in REPORT): make offboxWarningDisplay return lastWarning // unconditionally → TestOffboxWarningDisplay_Pick's 1-enabled case FAILS. +// +// R-553 (v0.251.0): these calls pass an EMPTY kind on purpose — they are now the LEGACY path, a box +// whose last off-site run predates kinds. The kind path is TestR553_StaleNoteDecisionSurvivesWordingChange. const staleZeroToggleWarning = "Sikeres — nincs mentésre jelölt alkalmazás volt a futáskor." func TestOffboxWarningDisplay_Pick(t *testing.T) { // warning + ≥1 enabled → the replacement line - if got := offboxWarningDisplay(staleZeroToggleWarning, 1); got != offboxSelectionChangedLine { + if got := offboxWarningDisplay(staleZeroToggleWarning, "", 1); got != offboxSelectionChangedLine { t.Errorf("stale warning + 1 toggled: got %q, want the selection-changed line", got) } // warning + 0 enabled → the original line, verbatim (the v0.123.0 honesty stays) - if got := offboxWarningDisplay(staleZeroToggleWarning, 0); got != staleZeroToggleWarning { + if got := offboxWarningDisplay(staleZeroToggleWarning, "", 0); got != staleZeroToggleWarning { t.Errorf("stale warning + 0 toggled: got %q, want the original warning unchanged", got) } // any OTHER warning passes through regardless of toggles (quota, partial failure) quota := "A tároló a keret 84%-át használja (42/50 GB)." - if got := offboxWarningDisplay(quota, 3); got != quota { + if got := offboxWarningDisplay(quota, "", 3); got != quota { t.Errorf("non-stale warning must pass through verbatim, got %q", got) } // no warning → no line - if got := offboxWarningDisplay("", 2); got != "" { + if got := offboxWarningDisplay("", "", 2); got != "" { t.Errorf("empty warning must stay empty, got %q", got) } } @@ -42,7 +45,7 @@ func TestOffboxWarningDisplay_RemotePageRender(t *testing.T) { Enabled: true, Host: "nas.local", LastStatus: "ok", EscrowState: "escrowed", LastWarning: staleZeroToggleWarning, } - data["OffboxWarningDisplay"] = offboxWarningDisplay(staleZeroToggleWarning, 1) + data["OffboxWarningDisplay"] = offboxWarningDisplay(staleZeroToggleWarning, "", 1) html := renderBackupPage(t, "backups_remote", data) if !strings.Contains(html, offboxSelectionChangedLine) { t.Error("selection-changed note missing with 1 app toggled") @@ -59,7 +62,7 @@ func TestOffboxWarningDisplay_RemotePageRender(t *testing.T) { Enabled: true, Host: "nas.local", LastStatus: "ok", EscrowState: "escrowed", LastWarning: staleZeroToggleWarning, } - data["OffboxWarningDisplay"] = offboxWarningDisplay(staleZeroToggleWarning, 0) + data["OffboxWarningDisplay"] = offboxWarningDisplay(staleZeroToggleWarning, "", 0) html = renderBackupPage(t, "backups_remote", data) if !strings.Contains(html, staleZeroToggleWarning) { t.Error("0 toggled: the original run warning must render unchanged") diff --git a/controller/internal/web/r553_alert_placement_test.go b/controller/internal/web/r553_alert_placement_test.go new file mode 100644 index 0000000..77fc834 --- /dev/null +++ b/controller/internal/web/r553_alert_placement_test.go @@ -0,0 +1,105 @@ +package web + +import ( + "encoding/json" + "io" + "log" + "reflect" + "strings" + "testing" + + "gitea.dooplex.hu/admin/felhom-controller/internal/config" + "gitea.dooplex.hu/admin/felhom-controller/internal/monitor" + "gitea.dooplex.hu/admin/felhom-controller/internal/report" +) + +// R-553 — the „your data is not on a separate drive" warning is shown quietly, inline under the +// storage bars on two pages. Which warning that is used to be decided by looking for „meghajtó" in +// the sentence; slice 2 translates it, and from that day the warning appears in the RED BANNER on +// every page of the dashboard instead — a household that changed nothing suddenly reads an alarm. +// +// RED-PROOF (REPORT): put the strings.Contains(w, "meghajtón"/"adattároló"/"meghajtó") test back in +// alerts.go and the TRANSLATED row below fails (Inline=false, no PageOnly). +func TestR553_DiskWarningPlacementSurvivesWordingChange(t *testing.T) { + cases := []struct { + name string + text, kind string + wantInline bool + }{ + {"as shipped", "Az adattároló (/mnt/hdd) nem külön meghajtón van — az adatok a rendszermeghajtóra íródnak", + monitor.WarnKindStorageNotSeparate, true}, + {"TRANSLATED", "The data storage (/mnt/hdd) is not on a separate drive — the data is written to the system drive", + monitor.WarnKindStorageNotSeparate, true}, + {"a CPU warning stays in the banner", "CPU usage high: 91%", "", false}, + {"a disconnected drive stays in the banner (its own alert covers it)", + "Meghajtó leválasztva: Külső (/mnt/usb)", monitor.WarnKindStorageDisconnected, false}, + {"a warning that merely CONTAINS the old words is not moved", + "A meghajtó ellenőrzése nem futott le", "", false}, + } + for _, c := range cases { + am := NewAlertManager(log.New(io.Discard, "", 0)) + hr := &monitor.HealthReport{Status: "warn", Warnings: []string{c.text}, WarningKinds: []string{c.kind}} + cfg := &config.Config{} + cfg.Hub.Enabled = false + am.Refresh(hr, cfg, nil, false, "") + var found *Alert + for i, a := range am.GetAlerts() { + if a.Message == c.text { + found = &am.GetAlerts()[i] + } + } + if found == nil { + t.Fatalf("%s: the warning did not reach the alerts at all", c.name) + } + if found.Inline != c.wantInline { + t.Errorf("%s: Inline = %v, want %v (message %q)", c.name, found.Inline, c.wantInline, c.text) + } + wantPages := 0 + if c.wantInline { + wantPages = 2 + } + if len(found.PageOnly) != wantPages { + t.Errorf("%s: PageOnly = %v, want %d page(s)", c.name, found.PageOnly, wantPages) + } + if c.wantInline && (found.ID != "disk-not-separate" || + strings.Join(found.PageOnly, ",") != "dashboard,monitoring") { + t.Errorf("%s: id/pages changed: %q %v", c.name, found.ID, found.PageOnly) + } + } +} + +// THE WIRE IS SACRED — the kinds are an internal field and must not reach the hub. The report's +// health block is its own struct; this pins that it carries exactly three fields, their JSON names, +// and that a health report WITH kinds marshals to the same bytes as one without. +// +// RED-PROOF (REPORT): add `WarningKinds []string \`json:"warning_kinds"\`` to report.HealthReport and +// both halves fail. +func TestR553_HubReportWarningsAreUnchangedOnTheWire(t *testing.T) { + rt := reflect.TypeOf(report.HealthReport{}) + var names []string + for i := 0; i < rt.NumField(); i++ { + names = append(names, rt.Field(i).Tag.Get("json")) + } + if got, want := strings.Join(names, ","), "status,issues,warnings"; got != want { + t.Errorf("the hub's health block changed shape: %q, want %q", got, want) + } + // The same warnings, one report built the way builder.go builds it (Status/Issues/Warnings only). + src := &monitor.HealthReport{ + Status: "warn", + Issues: []string{}, + Warnings: []string{"CPU usage high: 91%", "Az adattároló (/mnt/hdd) nem külön meghajtón van"}, + WarningKinds: []string{"", monitor.WarnKindStorageNotSeparate}, + } + wire := report.HealthReport{Status: src.Status, Issues: src.Issues, Warnings: src.Warnings} + b, err := json.Marshal(wire) + if err != nil { + t.Fatal(err) + } + const want = `{"status":"warn","issues":[],"warnings":["CPU usage high: 91%","Az adattároló (/mnt/hdd) nem külön meghajtón van"]}` + if string(b) != want { + t.Errorf("the hub report's health bytes CHANGED:\n got %s\nwant %s", b, want) + } + if strings.Contains(string(b), "kind") { + t.Error("a warning KIND reached the wire — the hub contract gained a field nobody agreed to") + } +} diff --git a/controller/internal/web/r553_stale_note_test.go b/controller/internal/web/r553_stale_note_test.go new file mode 100644 index 0000000..7ca0e17 --- /dev/null +++ b/controller/internal/web/r553_stale_note_test.go @@ -0,0 +1,93 @@ +package web + +import ( + "io" + "log" + "path/filepath" + "strings" + "testing" + + "gitea.dooplex.hu/admin/felhom-controller/internal/backup" + "gitea.dooplex.hu/admin/felhom-controller/internal/settings" +) + +// R-553 — the Távoli mentés page replaces a stale „nothing was selected" run note once apps HAVE been +// selected. It used to find that note by searching the sentence for „nincs mentésre jelölt alkalmazás"; +// slice 2 translates the sentence, and the stale note would then come back and stay — telling a +// household that has selected apps that nothing is covered. +// +// RED-PROOF (REPORT): make offboxWarningDisplay decide by the marker again (drop the kind arm) and +// the TRANSLATED row fails: the stale sentence is shown verbatim. +func TestR553_StaleNoteDecisionSurvivesWordingChange(t *testing.T) { + const hu = "Sikeres — nincs mentésre jelölt alkalmazás" + const en = "Success — no app is selected for backup" + cases := []struct { + name, warning, kind string + toggled int + want string + }{ + {"as shipped, apps now selected", hu, backup.OffboxWarnNoAppsSelected, 1, offboxSelectionChangedLine}, + {"TRANSLATED, apps now selected", en, backup.OffboxWarnNoAppsSelected, 2, offboxSelectionChangedLine}, + {"still nothing selected → the honest note stays", hu, backup.OffboxWarnNoAppsSelected, 0, hu}, + {"a quota note passes through", "A tároló a keret 84%-át használja (42/50 GB).", "", 3, + "A tároló a keret 84%-át használja (42/50 GB)."}, + // LEGACY: a box upgraded to v0.251.0 still carries the OLD text with no kind until its next + // off-site run rewrites it. Until then the text test is what it has. + {"legacy persisted text, no kind", hu, "", 1, offboxSelectionChangedLine}, + {"legacy: any other text without a kind is shown verbatim", "Valami más történt.", "", 1, "Valami más történt."}, + } + for _, c := range cases { + if got := offboxWarningDisplay(c.warning, c.kind, c.toggled); got != c.want { + t.Errorf("%s:\n got %q\nwant %q", c.name, got, c.want) + } + } +} + +// The consequence on the page itself: with a TRANSLATED warning that carries the kind, the rendered +// Távoli mentés page shows the selection-changed note and not the stale sentence. +func TestR553_StaleNoteOnTheRenderedPage(t *testing.T) { + const en = "Success — no app is selected for backup" + data := appRowSplitData() + data["Offbox"] = &settings.OffboxTarget{ + Enabled: true, Host: "nas.local", LastStatus: "ok", EscrowState: "escrowed", + LastWarning: en, LastWarningKind: backup.OffboxWarnNoAppsSelected, + } + data["OffboxWarningDisplay"] = offboxWarningDisplay(en, backup.OffboxWarnNoAppsSelected, 1) + html := renderBackupPage(t, "backups_remote", data) + if !strings.Contains(html, offboxSelectionChangedLine) { + t.Error("the selection-changed note is missing for a translated warning that carries its kind") + } + if strings.Contains(html, en) { + t.Error("the stale note is still on the page beside its replacement") + } +} + +// The kind has to be PERSISTED beside the text and survive an edit of the target, or the page falls +// back to the legacy text test on the next render (settings.json is the only carrier between runs). +func TestR553_WarningKindIsPersistedAndCopied(t *testing.T) { + tgt := settings.OffboxTarget{ + Enabled: true, Host: "nas.local", User: "felhom", RepoPath: "/srv/repo", + LastWarning: "Sikeres — nincs mentésre jelölt alkalmazás", LastWarningKind: backup.OffboxWarnNoAppsSelected, + } + dir := t.TempDir() + load := func() *settings.Settings { + s, err := settings.Load(filepath.Join(dir, "settings.json"), log.New(io.Discard, "", 0)) + if err != nil { + t.Fatal(err) + } + return s + } + if err := load().SetOffboxTarget(&tgt); err != nil { + t.Fatal(err) + } + reloaded := load().GetOffboxTarget() + if reloaded == nil { + t.Fatal("the target did not survive the reload") + } + if reloaded.LastWarningKind != backup.OffboxWarnNoAppsSelected { + t.Errorf("the warning kind was not persisted: %q", reloaded.LastWarningKind) + } + if reloaded.LastWarning != tgt.LastWarning { + t.Errorf("the warning text changed across the reload: %q", reloaded.LastWarning) + } +}