diff --git a/CHANGELOG.md b/CHANGELOG.md index cba8af5..f711601 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,104 @@ +## v0.235.0 — freeze the version, keep the fixes flowing (2026-09-06, update arc slice 3) + +**OPERATOR RULING, 2026-09-06 — Option 1.** R-447 was `BLOCKED` because R-438 established that +`RestartStack`'s use of `up -d` to pick up template changes was **chosen** and written down in its own +comment; reversing a chosen behaviour is a decision, not a bug fix. The decision is made and this +release implements it. + +**The rule, in one sentence: while the catalog is offering the same version you are running, its fixes +flow to you; the moment it moves to a newer version, you are frozen at what you have until you choose +to update.** + +**Nothing was added to any of the thirteen `compose up -d` call sites.** They are made safe by +removing the reason, not by gating them — the most important of them are REPAIRS (the boot reconciler, +the drive-return gate, the app-stop guard), and a repair path that refuses to repair leaves a +customer's app down, which is worse than the problem. + +### The pin (`app.yaml.pinned_images`) + +`AppConfig.PinnedImages`, service → image ref. **It is not `InstalledImages`.** That field is an +OBSERVATION ("what is running"); this one is a DECISION ("what should run"), written only by an act +entitled to move a version. Letting an observation feed a decision would make a bad reading become a +bad deployment — the category error `desired_state` exists to avoid (R-166). **Absent means UNPINNED, +and unpinned means the app behaves exactly as it did before this release.** + +**Four writers** (`internal/stacks/pin.go`): the deploy path; `UpdateStack`; the restore +(`stackAdapter.RecreateStackDefinitionFromUnit`); and the one-time adoption pass. Each also stores the +exact definition the pin came from as **`applied-compose.yml`** in the stack dir — a name +`Syncer.copyTemplates` does not copy, written atomically. + +**`UpdateStack` advances the pin and re-renders BEFORE the pull, and the ordering is load-bearing:** +`pull` and `up -d` act on the file on disk, so the catalog's definition has to BE that file first. +A pin set afterwards would pull the frozen version and report success. **A failed pin write REFUSES +the update** — the opposite of `recordInstalledImages`, because this field is intent. + +### The render (`internal/sync/sync.go`) + +One new nil-safe seam, `SetRenderPlanFn`, in the same shape as `rescanFn`/`postSyncHook`. The syncer +never reads `app.yaml`. `copyTemplates` now applies a table rather than copying: + +| app state | result | +|---|---| +| not deployed / protected / no seam | catalog verbatim — today's behaviour | +| deployed, **unpinned** | catalog verbatim + one DEBUG | +| deployed, pinned, catalog images **equal** | catalog verbatim — **fixes flow, self-healing works** | +| deployed, pinned, catalog images **differ** | the **stored definition** — frozen WHOLE | +| pinned, differ, nothing stored | catalog verbatim + one WARN | +| mid-deploy | the compose file is left alone this cycle | + +**`.felhom.yml` is always copied verbatim** — it holds no image, and it carries `catalog_since`, which +the badge needs. That asymmetry is a known limitation, filed as **R-458**. + +**The frozen branch writes a WHOLE file, never a substitution of refs into a newer template:** +`wger 2.6` needs a full DB configuration the older template cannot supply, so a new template around an +old image is a third state nobody chose. + +**And this is NOT "skip deployed apps"** — that was option B, rejected, because it also stops +health-check fixes, memory limits and new deploy fields, and destroys the self-healing measured live +in `SPIKE-app-update-2026-09-01` §3. + +### Adoption, and the startup ordering + +`Manager.AdoptPins` runs once at boot, immediately after `BackfillInstalledImages`, and pins every +deployed app to what it is already running. It reads and writes **files only** — no container is +started, stopped or touched. It skips, loudly, when the observation is incomplete (reusing +`observationCoversTemplate`, not a second rule) or when the app is running something the current +template no longer offers and no stored definition exists. Those apps keep pre-v0.235.0 behaviour. + +**`syncer.Start()` moved to after adoption.** It fires an immediate sync; at its old position that +first sync ran while every app was still unpinned and would have copied the catalog over a deployed +app once per boot — precisely the behaviour this release removes. + +### The badge had to change or slice 2 would have inverted silently + +`Stack.TemplateImages` is read from the app's LIVE compose file, which is now the **rendered** one. On +a frozen app that file names the OLD version, so `compareInstalledToTemplate` would have found +installed == template and answered **„Naprakész" on exactly the apps that are behind** — with every +test still green, because the two fields have the same type. The comparison now reads a new +`Stack.CatalogImages`, taken from the syncer's git clone. **No readable catalog entry renders +nothing.** No Hungarian string, badge state or partial changed. + +### Tests + ++16 (1729 → 1745). 28 packages green, 0 FAIL. New: `internal/sync/render_test.go`, +`internal/stacks/pin_test.go`, Group G in `internal/web/updatebadge_test.go`. + +**Three companion red-proofs, each run, observed failing, and reverted (2026-09-06):** + +| # | mutation | observed failure | +|---|---|---| +| 1 | the frozen branch returns the catalog template | `TestGroupB` — *"a pinned app must NOT receive the catalog's new version"* | +| 2 | adoption's completeness guard removed | `TestGroupE/incomplete_observation` — *"pinned 1, want 0 — only 1 of 2 services was observed"* | +| 3 | the badge reads `TemplateImages` again | `TestGroupG` — *"THE FEATURE IS INVERTED"*, plus „Naprakész" with no catalog entry | + +**Wiring:** `TestGroupH` walks the AST of `cmd/controller/main.go` for `SetRenderPlanFn` and +`AdoptPins` **and asserts their order** against the backfill and `syncer.Start()` — a +`strings.Contains` would match a commented-out call, and the render is inert without the seam. + +**One hole was found by a test rather than by review:** the syncer trusted the applied path handed to +it and would have written an empty compose file over a live app. It now re-reads and falls back to the +catalog. `TestRenderTable_EmptyStoredDefinitionIsTreatedAsAbsent`. + ## v0.234.0 — the label now appears on an app nobody has touched (2026-09-03, update arc slice 1b) **Found by the operator on demo-felhom the morning after v0.233.0, and it is a real gap, not a diff --git a/CONTEXT.md b/CONTEXT.md index 00b0bbd..430393f 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -7,7 +7,33 @@ > > Ask Claude Code: "Please update CONTEXT.md with what we did today" -Last updated: 2026-09-03 (v0.234.0 — the installed-images backfill, so the badge appears on an app nobody touched) +Last updated: 2026-09-06 (v0.235.0 — the version freeze: slice 3, on the operator's ruling) + +> **2026-09-06 — v0.235.0. THE RULING, AND THE TRAP IT SET.** +> +> **1. OPERATOR RULING: freeze the version, keep the fixes flowing.** R-447 sat `BLOCKED` because +> R-438 established that `RestartStack`'s `up -d` was a CHOSEN behaviour with its reason in its own +> comment. Option 1 was taken: an app's version is frozen to what the customer has and only a +> deliberate Update moves it, while health-check fixes, memory limits and self-healing keep arriving +> on the 15-minute cycle. **Both halves of the old behaviour were examined; only the version change +> was unwanted.** +> +> **2. THE MECHANISM IS A RENDER, NOT A GATE — and that distinction is the whole design.** Nothing was +> added to any of the thirteen `compose up -d` call sites. Most of them are REPAIRS (boot reconciler, +> drive-return gate, app-stop guard), and a repair that refuses to repair leaves a customer's app +> down. They are made safe by removing the reason: the file they act on no longer changes version. +> +> **3. `pinned_images` IS INTENT; `installed_images` IS AN OBSERVATION. NEVER FEED ONE FROM THE +> OTHER.** Letting a reading become a deployment is the R-166 category error one field over. They will +> normally agree; when they disagree that is a signal. +> +> **4. THE TRAP THIS RELEASE SET FOR ITSELF, and it would have shipped silently.** `Stack.TemplateImages` +> is read from the app's LIVE compose file — which is now the RENDERED one. On a frozen app that file +> names the OLD version, so the update badge would have found installed == template and answered +> **„Naprakész" on exactly the apps that are behind**, with every test still green, because the new +> field has the same type and shape. The badge now reads `Stack.CatalogImages`, from the syncer's own +> clone. **A feature that silently inverts a previous feature is the failure mode to look for whenever +> a file changes meaning.** > **2026-09-03 — v0.234.0. ONE GAP CLOSED, ONE TEST DEFECT OF MY OWN.** > diff --git a/REUSE.md b/REUSE.md index 7034e19..31a0be9 100644 --- a/REUSE.md +++ b/REUSE.md @@ -120,6 +120,9 @@ | `Manager.DeleteStack` / `RemoveStack` | controller/internal/stacks/delete.go | `(name, removeHDDData[, backupPaths])` | THE guarded removal paths | Orphan/protected/deploying/running checks + ProtectedHDDPaths filter before any RemoveAll | | `resolveContainerState` / `aggregateState` | controller/internal/stacks/manager.go | `(dockerState, dockerStatus)` / `([]ContainerInfo)` | State classification | `.State` says "running" even when unhealthy — `.Status` parse is the fix | | `Manager.recordInstalledImages` (v0.233.0) | controller/internal/stacks/installed.go | `(name, stackDir string, env []string)` | writing down what each compose SERVICE is ACTUALLY running, into `app.yaml.installed_images` | Called after a successful compose up from `StartStack`/`RestartStack`/`UpdateStack`/`runComposeDeploy`. **Reads the CONTAINER, never `docker-compose.yml`** — that file is the value the syncer has already moved (spike §3: 25 minutes of disagreement). **A failed write NEVER refuses the action** — the deliberate OPPOSITE of `SetDesiredState`: intent refused, observation logged at ERROR. **NOT from `StartStackServices`** (the R-47 DB-only window would overwrite a complete record with a partial one). Skips the write when ref+digest are unchanged, and carries `at` forward so it means "running since". Its OWN seam (`installedExecFn`) with a **context + 30 s timeout** — the two existing exec helpers have neither | +| `Manager.SetPin` / `AdoptPins` / `RenderPlanFor` / `AppliedComposePath` (v0.235.0) | controller/internal/stacks/pin.go | `SetPin(name, stackDir, pin, composeSrc) error` | THE version freeze — `app.yaml.pinned_images` + the stored `applied-compose.yml` | **`PinnedImages` is INTENT, `InstalledImages` is an OBSERVATION — never feed one from the other** (the R-166 category error, one field over). Four writers only: deploy, `UpdateStack` (via `advancePinToCatalog`, which advances the pin and re-renders BEFORE the pull, and REFUSES the update if the pin cannot be written), the restore adapter (this is what closes R-441), and `AdoptPins`. `AdoptPins` reuses `observationCoversTemplate` — do NOT write a second completeness rule — and skips loudly rather than inventing a pin. Absent pin = pre-v0.235.0 behaviour | +| `Syncer.SetRenderPlanFn` + `renderSource` (v0.235.0) | controller/internal/sync/sync.go | `func(appName string) stacks.RenderPlan` | the catalog render table | **NIL-SAFE: no seam = copy verbatim = the old product.** Catalog images == pin → verbatim (fixes flow + self-healing, both deliberately kept); differ → the WHOLE stored definition, **never a ref substitution into a newer template** (`wger 2.6`). `.felhom.yml` always verbatim (R-458). The syncer must NEVER read app.yaml. Re-reads the applied file before writing it — a test caught it writing an empty compose over a live app | +| `Stack.CatalogImages` vs `Stack.TemplateImages` (v0.235.0) | controller/internal/stacks/manager.go | both `map[string]string` | badge input vs "what the next `up -d` gives this app" | **THE TRAP: same type, same shape, opposite meaning after the freeze.** `TemplateImages` reads the LIVE (possibly frozen) compose file; `CatalogImages` reads the syncer's clone. `web.compareInstalledToTemplate` MUST use `CatalogImages` or it answers „Naprakész" on exactly the apps that are behind, with every test green. Red-proved | | `Manager.BackfillInstalledImages` (v0.234.0) | controller/internal/stacks/installed.go | `() int` | seeding `installed_images` for apps that have NO record — call ONCE at startup | Beside `BackfillDesiredState` in `cmd/controller/main.go`, after it and BEFORE the boot reconciler (pinned by an AST-walking test that asserts the ORDER). **READS only** — starts nothing, writes no compose file. **Never overwrites an existing record** (an app that has one is not even observed). **REFUSES a partial observation** (`observationCoversTemplate`): `web.compareInstalledToTemplate` reads a service-count mismatch as BEHIND, so seeding a degraded app from what is visible renders „Frissítés elérhető" over an app that is current. The bring-up paths may write a partial because they follow a SUCCESSFUL `up -d` where a gap is real news; a backfill meets any state and must be stricter | | `stacks.ParseComposeImages` (v0.233.0) | controller/internal/stacks/installed.go | `(composePath string) (map[string]string, error)` | compose SERVICE name -> the image the FILE pins; feeds `Stack.TemplateImages` and the update badge | yaml.v3 `services:` MAP parse, never a line scan (same reason as `DBServiceNames`). An error means CANNOT-TELL — `ScanStacks` leaves `TemplateImages` nil and the badge renders NOTHING, never "current" | | `web.updateBadge` / `updateBadgeAt` / `Metadata.CatalogSince` + `CatalogSinceAge` (v0.233.0) | controller/internal/web/updatebadge.go, controller/internal/stacks/metadata.go | `(stacks.Stack) *MetaBadge` | THE "is this app current?" label — „Naprakész" / „Frissítés elérhető — N napja" | The SECOND `*MetaBadge` user the type was built for: existing `meta_badge` partial, **no new markup or CSS**. **NO RECORD RENDERS NOTHING — absent means UNKNOWN, never current** (R-166 applied to an observation; red-proved). **No version number reaches the customer** and **no registry is queried**. `catalog_since` is tolerant in the `lifecycle` style — absent/empty/malformed/**future** all degrade to a badge with no age + one WARN. LIMITATION: for the 23 floating pins the ref can match while the image has moved, so those read „Naprakész" when they may not be | diff --git a/controller/README.md b/controller/README.md index 97449d5..972f6f5 100644 --- a/controller/README.md +++ b/controller/README.md @@ -532,6 +532,29 @@ the current template pins and returns a `*MetaBadge` rendered by the existing `m Reasoning and the seven-slice plan: `felhom.eu/documentation/architecture/09-update-architecture.md`. +#### Freeze the version, keep the fixes flowing (v0.235.0 — update arc slice 3) + +**Operator ruling, 2026-09-06.** An app's version is frozen to what the customer has; only a +deliberate Update moves it. Everything else in a template — health checks, memory limits, new deploy +fields — still arrives on the 15-minute cycle, and a broken definition still repairs itself. + +- **`app.yaml` gains `pinned_images`** (service → ref): what the app is SUPPOSED to run. **Not** + `installed_images`, which is an observation. Written only by the deploy path, `UpdateStack`, a + restore, and the one-time `AdoptPins`. **Absent = unpinned = pre-v0.235.0 behaviour.** +- **`applied-compose.yml`** in the stack dir stores the exact definition the pin came from. The + syncer copies only `docker-compose.yml` and `.felhom.yml`, so that name is safe. +- **`Syncer` renders instead of copying**, via the nil-safe `SetRenderPlanFn` seam. Catalog images + equal the pin → copy verbatim (fixes flow, self-healing works). They differ → write the stored + definition, **whole** — never a substitution of refs into a newer template (`wger 2.6`). +- **`.felhom.yml` always flows**, even to a frozen app: it holds no image and carries `catalog_since`. + Known limitation, R-458. +- **Nothing was added to the thirteen `compose up -d` call sites.** Most are repairs; a repair that + refuses to repair leaves an app down. +- **The badge reads `Stack.CatalogImages`, never `TemplateImages`.** After the freeze the live compose + file is the frozen one, so comparing against it would answer „Naprakész" on apps that are behind. + +Reasoning: `felhom.eu/documentation/architecture/09-update-architecture.md` §3, §5. + #### App Info Pages Each app can define rich metadata in `.felhom.yml`: diff --git a/controller/cmd/controller/main.go b/controller/cmd/controller/main.go index 59b02fa..f21db71 100644 --- a/controller/cmd/controller/main.go +++ b/controller/cmd/controller/main.go @@ -389,7 +389,12 @@ func main() { syncer := catalogsync.New(cfg, logger, stackMgr.ScanStacks, func(updated []string) { stackMgr.InjectMissingFields(updated) }) - syncer.Start() + // v0.235.0 — the render seam. Without this line the syncer copies the catalog verbatim, i.e. + // the pre-v0.235.0 product; the freeze is INERT unless it is wired here, which is why a test + // walks this file's AST for the call rather than trusting the package to be correct alone. + syncer.SetRenderPlanFn(stackMgr.RenderPlanFor) + // Start() is deliberately NOT called here — see the comment beside the call further down, after + // the pin adoption pass. defer syncer.Stop() // --- Graceful shutdown context --- @@ -447,6 +452,21 @@ func main() { // reason: a just-restarted app is observed in its settled state. stackMgr.BackfillInstalledImages() + // --- v0.235.0: pin adoption (complete AND matching observations only) --- + // Give every deployed app a pin for what it is already running, so the render has something to + // obey. Runs immediately after the backfill so an app that pass has just observed can be pinned + // in the same boot. It reads and writes FILES only — no container is started, stopped or touched. + // An app it cannot pin confidently is left UNPINNED and keeps pre-v0.235.0 behaviour, loudly. + stackMgr.AdoptPins() + + // --- Start the catalog syncer, AFTER adoption --- + // ORDERING IS LOAD-BEARING. Start() fires an immediate sync in a goroutine. Started at its + // original place — before adoption — that first sync would run while every app was still + // unpinned, copy the catalog verbatim over a deployed app, and hand the next restart a version + // change: precisely the behaviour this release removes, once per boot. Adoption first means the + // very first sync of a boot already obeys the pins. + syncer.Start() + // --- R-52: boot desired-state reconciliation --- // A deployed app that missed its boot start used to stay down until a human noticed (F5: immich // and calibre-web sat Exited for ~18 h while ten siblings came back). One bounded start-once @@ -2595,7 +2615,27 @@ func (a *stackAdapter) RecreateStackDefinitionFromUnit(name, composeSrcDir strin return fmt.Errorf("restoring %s from unit: %w", fname, err) } } - return a.mgr.PersistUnitRedeployConfig(name, fullEnv) + if err := a.mgr.PersistUnitRedeployConfig(name, fullEnv); err != nil { + return err + } + + // v0.235.0 — PIN TO WHAT THE UNIT CAPTURED. THIS IS WHAT CLOSES R-441. + // + // Measured before this line existed: this function writes the recovery unit's compose — carrying + // the OLD image pin — into the live stack dir, and `Syncer.copyIfChanged` then overwrote it from + // the catalog on the next 15-minute tick. So a restore's image-level recovery had a <=15-minute + // half-life, and the next `compose up -d` from any of the thirteen call sites re-applied the + // catalog's version. Pinning here makes the render obey the restored definition instead. + // + // Pinned from the file just written, so the pin and the stored definition cannot disagree. + // A failure is loud and does NOT fail the restore: an unpinned app is exactly today's behaviour, + // and refusing a completed restore over a bookkeeping write would be the worse trade. + if pin, data, err := stacks.PinFromCompose(stacks.ComposePathIn(stackDir)); err != nil { + log.Printf("[WARN] [stacks] pin %s: cannot pin from the restored definition: %v", name, err) + } else if err := a.mgr.SetPin(name, stackDir, pin, data); err != nil { + log.Printf("[ERROR] [stacks] pin %s: %v", name, err) + } + return nil } // StartStackServices brings up only the named compose services (the DB-only replay window, R-47). diff --git a/controller/internal/stacks/deploy.go b/controller/internal/stacks/deploy.go index 47f938c..4f5ac0f 100644 --- a/controller/internal/stacks/deploy.go +++ b/controller/internal/stacks/deploy.go @@ -138,6 +138,21 @@ type AppConfig struct { // WRITTEN BY: Manager.recordInstalledImages ONLY, from StartStack / RestartStack / UpdateStack // and the deploy path. READ BY: web.updateBadge (v0.233.0). Nothing takes a DECISION from it. InstalledImages map[string]InstalledImage `yaml:"installed_images,omitempty" json:"installed_images,omitempty"` + // PinnedImages is what this app is SUPPOSED to run, per compose service — the customer's + // INTENT, and the input the catalog render obeys (v0.235.0, operator ruling 2026-09-06: + // "freeze the version, keep the fixes flowing"). + // + // IT IS NOT InstalledImages. That field is an OBSERVATION ("what is running"), written by + // looking at containers. This one is a DECISION ("what should run"), written only by an act + // that is entitled to move a version: a deploy, a deliberate update, a restore, or the + // one-time adoption pass. Letting an observation feed a decision would make a bad reading + // become a bad deployment — the same category error the desired_state field exists to avoid + // (R-166). They will normally agree; when they disagree that is a signal, not a bug to + // paper over. + // + // ABSENT MEANS UNPINNED, and unpinned means the app behaves exactly as it did before + // v0.235.0. It never means "pin to whatever the catalog says now". + PinnedImages map[string]string `yaml:"pinned_images,omitempty" json:"pinned_images,omitempty"` } // InstalledImage is one compose service's observed image. See AppConfig.InstalledImages. @@ -482,6 +497,16 @@ func (m *Manager) runComposeDeploy(name, stackDir string, env map[string]string, deployEnv := m.stackEnv(stackDir) m.recordInstalledImages(name, stackDir, deployEnv) + // Pin what we just deployed FROM (v0.235.0). The stack dir's compose file IS what the deploy + // used, so it is both the pin's source and the definition stored beside it. A failure here is + // logged and never fails the deploy — the app is up, and an unpinned app simply keeps + // pre-v0.235.0 behaviour. + if pin, data, err := PinFromCompose(ComposePathIn(stackDir)); err != nil { + m.logger.Printf("[WARN] [stacks] pin %s: cannot pin from the deployed compose file: %v", name, err) + } else if err := m.SetPin(name, stackDir, pin, data); err != nil { + m.logger.Printf("[ERROR] [stacks] pin %s: %v", name, err) + } + // Post-deploy container state check (async, non-blocking) m.logPostStartStatus(name, stackDir, deployEnv) diff --git a/controller/internal/stacks/manager.go b/controller/internal/stacks/manager.go index 155d4d4..2d862c0 100644 --- a/controller/internal/stacks/manager.go +++ b/controller/internal/stacks/manager.go @@ -150,13 +150,26 @@ type Stack struct { // forgetting costs at most one threshold window, whereas persisting could carry a stale // "this app is crash-looping" verdict across the restart that fixed it. RestartingSince time.Time `json:"restarting_since,omitempty"` - // TemplateImages is what the stack's CURRENT docker-compose.yml pins, per compose service — - // i.e. what the catalog says this app should be running right now. Refreshed by ScanStacks for - // deployed, non-protected apps only; nil for everything else and nil when the file cannot be - // parsed. Nil means CANNOT-TELL and never means "matches": web.updateBadge renders nothing. - // Not persisted — it is a read of a file the syncer owns, and re-reading is cheaper than a - // second copy that can go stale. + // TemplateImages is what the stack's LIVE docker-compose.yml pins, per compose service. + // + // ⚠ SINCE v0.235.0 THIS IS NOT "WHAT THE CATALOG OFFERS". The live file is RENDERED: for a + // pinned app whose version the catalog has moved past, it is the app's own frozen definition. + // So TemplateImages answers "what will the next `compose up -d` bring this app to" — which is + // exactly what a debugger wants and exactly the WRONG input for the update badge, because a + // frozen app's live file names the OLD version and the comparison would read „Naprakész". + // THE BADGE USES CatalogImages. See web.compareInstalledToTemplate. + // + // Refreshed by ScanStacks for deployed, non-protected apps only; nil otherwise and nil when the + // file cannot be parsed. Not persisted. TemplateImages map[string]string `json:"template_images,omitempty"` + // CatalogImages is what the CATALOG currently offers for this app, read from the syncer's git + // clone (`/catalog-cache/templates//docker-compose.yml`) rather than from the + // stack dir. Added in v0.235.0 because the render made the live file unusable for the question + // "is this app behind?". + // + // Nil means CANNOT-TELL — the cache is missing, unreadable, or the app is not in the catalog — + // and the badge then renders NOTHING. Absent is unknown; it is never „Naprakész". + CatalogImages map[string]string `json:"catalog_images,omitempty"` } // Manager handles all docker compose stack operations. @@ -527,6 +540,19 @@ func (m *Manager) ScanStacks() error { } } + // What the CATALOG offers — the badge's input, and deliberately a different file from the + // one above (v0.235.0). A missing catalog entry is silent at INFO: an orphaned app has no + // catalog template by definition, and warning once per app per scan would be noise. + var catImages map[string]string + if deployed && !m.cfg.IsProtectedStack(name) { + catPath := m.CatalogTemplatePath(name, "docker-compose.yml") + if imgs, cerr := ParseComposeImages(catPath); cerr == nil { + catImages = imgs + } else if m.isDebug() { + m.logger.Printf("[DEBUG] [stacks] ScanStacks: no readable catalog template for %s (%v) — the update badge will render nothing", name, cerr) + } + } + if existing, ok := m.stacks[name]; ok { existing.ComposePath = composePath existing.Meta = meta @@ -537,6 +563,7 @@ func (m *Manager) ScanStacks() error { existing.Deployed = deployed existing.AppConfig = appCfg existing.TemplateImages = tplImages + existing.CatalogImages = catImages } } else { m.stacks[name] = &Stack{ @@ -548,6 +575,7 @@ func (m *Manager) ScanStacks() error { Protected: m.cfg.IsProtectedStack(name), AppConfig: appCfg, TemplateImages: tplImages, + CatalogImages: catImages, } } } @@ -1205,6 +1233,24 @@ func (m *Manager) UpdateStack(name string) error { m.logger.Printf("[INFO] [stacks] Updating stack: %s", name) start := time.Now() dir := filepath.Dir(stack.ComposePath) + + // v0.235.0 — ADVANCE THE PIN FIRST, AND RE-RENDER BEFORE THE PULL. + // + // This is the ONE act entitled to move a version; the freeze exists so that nothing else can. + // The ordering is load-bearing, not stylistic: `compose pull` and `up -d` act on the file on + // disk, so the catalog's current definition has to BE that file before either runs. Setting the + // pin afterwards would pull the frozen version and change nothing, while reporting success — and + // a button that lies is worse than a button that refuses. + // + // A FAILED PIN WRITE REFUSES THE UPDATE, deliberately the opposite of recordInstalledImages. + // That field is an observation and a failed write is a bookkeeping gap; this one is INTENT, and + // an update whose intent could not be recorded leaves the box running a version it has no record + // of choosing — the exact ambiguity R-166 closed for desired_state, one field over. + if err := m.advancePinToCatalog(name, dir); err != nil { + m.logger.Printf("[ERROR] [stacks] Stack %s update refused: %v", name, err) + return fmt.Errorf("updating stack %s: %w", name, err) + } + env := m.stackEnv(dir) if m.isDebug() { diff --git a/controller/internal/stacks/pin.go b/controller/internal/stacks/pin.go new file mode 100644 index 0000000..984874e --- /dev/null +++ b/controller/internal/stacks/pin.go @@ -0,0 +1,358 @@ +package stacks + +import ( + "fmt" + "os" + "path/filepath" + "sort" + "strings" +) + +// AppliedComposeFile is the stored copy of the docker-compose.yml an app was last brought up FROM. +// +// WHY IT LIVES IN THE STACK DIRECTORY, and why this exact name: `Syncer.copyTemplates` copies +// EXACTLY `docker-compose.yml` and `.felhom.yml` and nothing else, so any other name in that +// directory is safe from the catalog. Keeping it beside the app means it travels through every path +// that already moves a stack dir, and anyone debugging the box can see it without a tool. +// +// It is the FROZEN definition: when the catalog has moved past an app's pin, this whole file is +// written to docker-compose.yml. Never a substitution of pinned refs into a newer template — a +// template's env and volumes can belong to its version (`wger 2.6` needs a full DB config the older +// template cannot supply), and an old image under a new template is a third broken state nobody +// chose. +const AppliedComposeFile = "applied-compose.yml" + +// AppliedComposePath is the single definition of where the stored definition lives. +func AppliedComposePath(stackDir string) string { + return filepath.Join(stackDir, AppliedComposeFile) +} + +// StoreAppliedDefinition writes the compose definition this app is pinned to, atomically. +// +// Atomic tmp+rename for the same reason SaveAppConfig is: the syncer may read this file at any +// moment, and a half-written compose file rendered over a live app is a broken app. +func StoreAppliedDefinition(stackDir string, data []byte) error { + if len(data) == 0 { + // NEVER store an empty definition. An empty applied file would later be rendered over the + // live compose file and take the app down — see the render table's "unreadable or empty" + // row, which treats it as absent precisely so this cannot happen. + return fmt.Errorf("refusing to store an empty applied definition for %s", filepath.Base(stackDir)) + } + path := AppliedComposePath(stackDir) + tmp := path + ".tmp" + if err := os.WriteFile(tmp, data, 0o644); err != nil { + return fmt.Errorf("writing %s: %w", tmp, err) + } + if err := os.Rename(tmp, path); err != nil { + _ = os.Remove(tmp) + return fmt.Errorf("renaming %s to %s: %w", tmp, path, err) + } + return nil +} + +// LoadAppliedDefinition returns the stored definition, or an error when there is none or it is +// unusable. An empty file is an ERROR, not empty content — see the render table. +func LoadAppliedDefinition(stackDir string) ([]byte, error) { + data, err := os.ReadFile(AppliedComposePath(stackDir)) + if err != nil { + return nil, err + } + if len(strings.TrimSpace(string(data))) == 0 { + return nil, fmt.Errorf("stored applied definition for %s is empty", filepath.Base(stackDir)) + } + return data, nil +} + +// SetPin records what an app is SUPPOSED to run and stores the definition that pin came from. +// +// PIN FIRST, THEN STORE, and the degradation is deliberate: if the pin lands and the store fails, +// the app is pinned with no stored definition, and the render table's own row for that case copies +// the catalog verbatim and WARNs. That is today's behaviour plus a loud line — the same outcome as +// being unpinned, never a silent freeze onto a definition we do not have. +// +// `composeSrc` MUST be the exact bytes the pin was derived from. Passing a different file is how a +// stack ends up frozen onto something it never ran. +func (m *Manager) SetPin(name, stackDir string, pin map[string]string, composeSrc []byte) error { + if len(pin) == 0 { + return fmt.Errorf("refusing to pin %s to an empty image set", name) + } + cfg := LoadAppConfig(stackDir) + if cfg == nil { + // No app.yaml means nothing is deployed here. Not an error — the same no-op + // SetDesiredState makes for the same reason. + m.logger.Printf("[DEBUG] [stacks] pin %s: no app.yaml — nothing deployed here, nothing to pin", name) + return nil + } + + if !samePin(cfg.PinnedImages, pin) { + cfg.PinnedImages = pin + meta := LoadMetadata(stackDir) + if err := SaveAppConfig(stackDir, cfg, m.encKey, SensitiveEnvVars(&meta)); err != nil { + return fmt.Errorf("recording pin for %s: %w", name, err) + } + m.logger.Printf("[INFO] [stacks] pin %s: %s", name, summarisePin(pin)) + + m.mu.Lock() + if st, ok := m.stacks[name]; ok && st.AppConfig != nil { + st.AppConfig.PinnedImages = pin + } + m.mu.Unlock() + } + + if err := StoreAppliedDefinition(stackDir, composeSrc); err != nil { + // Loud, and NOT fatal to the pin — see the comment above. + m.logger.Printf("[ERROR] [stacks] pin %s: the pin is recorded but its definition could not be stored (the app is unaffected; the catalog will be copied verbatim until this is fixed): %v", name, err) + } + return nil +} + +// samePin compares two pins by content so an unchanged pin does not rewrite app.yaml — that file +// holds encrypted secrets and rewriting it for no new information is pure risk (the SetDesiredState +// rule). +func samePin(a, b map[string]string) bool { + if len(a) != len(b) { + return false + } + for k, va := range a { + if vb, ok := b[k]; !ok || va != vb { + return false + } + } + return true +} + +// summarisePin renders a pin for ONE log line. Image refs only — this file never logs anything out +// of app.yaml's env map, which holds encrypted secrets. +func summarisePin(pin map[string]string) string { + svcs := make([]string, 0, len(pin)) + for svc := range pin { + svcs = append(svcs, svc) + } + sort.Strings(svcs) + parts := make([]string, 0, len(svcs)) + for _, svc := range svcs { + parts = append(parts, svc+"="+pin[svc]) + } + return strings.Join(parts, ", ") +} + +// ComposePathIn resolves a stack directory's compose file, honouring the .yml/.yaml pair exactly as +// ScanStacks does. One rule, so a caller cannot pin from a file the scanner would not have read. +func ComposePathIn(stackDir string) string { + p := filepath.Join(stackDir, "docker-compose.yml") + if _, err := os.Stat(p); err == nil { + return p + } + alt := filepath.Join(stackDir, "docker-compose.yaml") + if _, err := os.Stat(alt); err == nil { + return alt + } + return p // the canonical name; the caller's read will report the real error +} + +// PinFromCompose reads a compose file and returns both the pin it implies and its exact bytes, so a +// caller cannot accidentally pin from one file and store another. +func PinFromCompose(composePath string) (map[string]string, []byte, error) { + images, err := ParseComposeImages(composePath) + if err != nil { + return nil, nil, err + } + if len(images) == 0 { + return nil, nil, fmt.Errorf("%s declares no images", composePath) + } + data, err := os.ReadFile(composePath) + if err != nil { + return nil, nil, err + } + return images, data, nil +} + +// --- what the syncer is told --- + +// RenderPlan is the per-app answer the stack manager gives the catalog syncer. +// +// It carries FACTS, not a decision: the render table lives in the syncer, which is the thing doing +// the writing. The syncer must never read app.yaml itself — that file is the manager's and carries +// encrypted values — so everything it needs to apply the table comes through here. +type RenderPlan struct { + Deployed bool + Deploying bool + Protected bool + Pinned map[string]string // service -> image ref; nil/empty means UNPINNED + AppliedPath string // the stored definition; "" when none is stored +} + +// RenderPlanFor answers for one app by name. Unknown apps come back as an empty plan, which the +// syncer reads as "not deployed" — i.e. copy verbatim, exactly the pre-v0.235.0 behaviour. +func (m *Manager) RenderPlanFor(name string) RenderPlan { + s, ok := m.GetStack(name) + if !ok { + return RenderPlan{} + } + plan := RenderPlan{ + Deployed: s.Deployed, + Deploying: s.Deploying, + Protected: s.Protected, + } + if s.AppConfig != nil && len(s.AppConfig.PinnedImages) > 0 { + plan.Pinned = s.AppConfig.PinnedImages + } + stackDir := filepath.Dir(s.ComposePath) + if _, err := LoadAppliedDefinition(stackDir); err == nil { + plan.AppliedPath = AppliedComposePath(stackDir) + } + return plan +} + +// --- adoption --- + +// AdoptPins gives a pin to every deployed app that has none, and stores the definition it is +// running. Call ONCE at startup, immediately AFTER BackfillInstalledImages so an app the backfill +// has just observed can be pinned in the same boot. +// +// ── IT NEVER GUESSES, AND THAT IS MOST OF THE CODE ─────────────────────────────────────────── +// +// Two skips, each with a reason that is not interchangeable: +// +// 1. The observation is INCOMPLETE (stopped, crash-looping, mid-anything). We do not know what the +// app runs, so we cannot say what it should run. Reuses observationCoversTemplate — the SAME +// completeness rule the backfill uses, deliberately not a second one. +// 2. The observation is complete but DIFFERS from the current template. The app is already running +// something the catalog no longer offers, and we have no stored definition for it. Pinning here +// would be right, but the FREEZE would then render a definition we do not possess — and the only +// way to manufacture one is to substitute the running refs into the newer template, which is +// exactly the third-broken-state this design refuses (see AppliedComposeFile). +// +// In both cases the app keeps behaving exactly as it did before v0.235.0, loudly. Absent means +// unknown; unknown is never resolved by inventing an answer. +// +// It reads and writes FILES only. It starts, stops and touches no container. +func (m *Manager) AdoptPins() int { + pinned, alreadyPinned, skippedIncomplete, skippedMismatch := 0, 0, 0, 0 + + for _, s := range m.GetStacks() { + if !s.Deployed || s.Protected || s.Deploying { + continue + } + if s.AppConfig != nil && len(s.AppConfig.PinnedImages) > 0 { + alreadyPinned++ + continue + } + stackDir := filepath.Dir(s.ComposePath) + + tpl, err := ParseComposeImages(s.ComposePath) + if err != nil || len(tpl) == 0 { + m.logger.Printf("[WARN] [stacks] pin adoption: %s left UNPINNED — its compose file declares no readable images (%v). It keeps pre-v0.235.0 behaviour.", s.Name, err) + skippedIncomplete++ + continue + } + + var observed map[string]InstalledImage + if s.AppConfig != nil { + observed = s.AppConfig.InstalledImages + } + if !observationCoversTemplate(observed, tpl) { + m.logger.Printf("[WARN] [stacks] pin adoption: %s left UNPINNED — no complete record of what it is running (%d of %d service(s) observed). It keeps pre-v0.235.0 behaviour.", + s.Name, len(observed), len(tpl)) + skippedIncomplete++ + continue + } + + if diff := pinMismatch(observed, tpl); diff != "" { + m.logger.Printf("[WARN] [stacks] pin adoption: %s left UNPINNED — it is running something the current template no longer offers, and there is no stored definition for it: %s. It keeps pre-v0.235.0 behaviour.", + s.Name, diff) + skippedMismatch++ + continue + } + + _, data, err := PinFromCompose(s.ComposePath) + if err != nil { + m.logger.Printf("[WARN] [stacks] pin adoption: %s: %v", s.Name, err) + skippedIncomplete++ + continue + } + if err := m.SetPin(s.Name, stackDir, tpl, data); err != nil { + m.logger.Printf("[ERROR] [stacks] pin adoption: %s: %v", s.Name, err) + continue + } + pinned++ + } + + // A POSITIVE OBSERVABLE EITHER WAY (standing rule 3): "0 pinned" and "the pass never ran" must + // not look the same in a log. + m.logger.Printf("[INFO] [stacks] pin adoption: %d pinned, %d already pinned, %d left unpinned (%d not completely observed, %d running something the template no longer offers)", + pinned, alreadyPinned, skippedIncomplete+skippedMismatch, skippedIncomplete, skippedMismatch) + return pinned +} + +// pinMismatch returns a human description of the first service whose RUNNING reference differs from +// what the template pins, or "" when they agree everywhere. Deterministic order so two runs produce +// the same line. +func pinMismatch(observed map[string]InstalledImage, tpl map[string]string) string { + svcs := make([]string, 0, len(tpl)) + for svc := range tpl { + svcs = append(svcs, svc) + } + sort.Strings(svcs) + var diffs []string + for _, svc := range svcs { + got, ok := observed[svc] + if !ok { + continue // completeness was already checked + } + if got.Ref != tpl[svc] { + diffs = append(diffs, fmt.Sprintf("%s runs %s, template offers %s", svc, got.Ref, tpl[svc])) + } + } + return strings.Join(diffs, "; ") +} + +// CatalogTemplatePath is where the syncer's git clone keeps one app's template. It is the ONLY +// definition of that layout outside the syncer, and the badge and the update path both use it. +func (m *Manager) CatalogTemplatePath(appName, filename string) string { + return filepath.Join(m.cfg.Paths.DataDir, "catalog-cache", "templates", appName, filename) +} + +// advancePinToCatalog moves a PINNED app onto the catalog's current definition: it writes the +// catalog template over the live compose file, records the new pin, and stores that definition as +// the applied one. Called by UpdateStack BEFORE the pull — see the comment at that call site. +// +// AN UNPINNED APP IS LEFT ALONE AND THIS RETURNS NIL. It behaves exactly as it did before v0.235.0: +// the syncer has already copied the catalog verbatim into its stack dir, so `pull` + `up -d` do +// today's job with no help from here. +func (m *Manager) advancePinToCatalog(name, stackDir string) error { + cfg := LoadAppConfig(stackDir) + if cfg == nil || len(cfg.PinnedImages) == 0 { + return nil // unpinned — today's behaviour, unchanged + } + + src := m.CatalogTemplatePath(name, "docker-compose.yml") + pin, data, err := PinFromCompose(src) + if err != nil { + // REFUSE rather than silently update to the frozen definition (which would be a no-op + // reported as success). Names the cause so the operator is not left guessing. + return fmt.Errorf("cannot read the catalog's current definition for %s (%s): %w", name, src, err) + } + + live := ComposePathIn(stackDir) + if err := StoreAppliedDefinition(stackDir, data); err != nil { + return fmt.Errorf("storing the new applied definition for %s: %w", name, err) + } + if err := os.WriteFile(live, data, 0o644); err != nil { + return fmt.Errorf("rendering the catalog definition for %s: %w", name, err) + } + + cfg.PinnedImages = pin + meta := LoadMetadata(stackDir) + if err := SaveAppConfig(stackDir, cfg, m.encKey, SensitiveEnvVars(&meta)); err != nil { + return fmt.Errorf("recording the advanced pin for %s: %w", name, err) + } + m.mu.Lock() + if st, ok := m.stacks[name]; ok && st.AppConfig != nil { + st.AppConfig.PinnedImages = pin + } + m.mu.Unlock() + + m.logger.Printf("[INFO] [stacks] update %s: pin advanced to the catalog's current definition (%s)", name, summarisePin(pin)) + return nil +} diff --git a/controller/internal/stacks/pin_test.go b/controller/internal/stacks/pin_test.go new file mode 100644 index 0000000..0d03a01 --- /dev/null +++ b/controller/internal/stacks/pin_test.go @@ -0,0 +1,334 @@ +package stacks + +import ( + "context" + "go/ast" + "go/parser" + "go/token" + "io" + "log" + "os" + "path/filepath" + "strings" + "testing" + + "gitea.dooplex.hu/admin/felhom-controller/internal/config" + "gopkg.in/yaml.v3" +) + +// Slice 3 (v0.235.0) — the pin, the stored definition, and adoption. + +const pinTplOld = "services:\n web:\n image: nextcloud:31.0.14-apache\n" +const pinTplNew = "services:\n web:\n image: nextcloud:34.0.1-apache\n" + +// newPinManager builds a Manager with one deployed stack and a catalog cache. +func newPinManager(t *testing.T, liveCompose, catalogCompose, appYAML string) (*Manager, string) { + t.Helper() + root := t.TempDir() + cfg := &config.Config{} + cfg.Paths.StacksDir = filepath.Join(root, "stacks") + cfg.Paths.DataDir = filepath.Join(root, "data") + stackDir := filepath.Join(cfg.Paths.StacksDir, "nextcloud") + catDir := filepath.Join(cfg.Paths.DataDir, "catalog-cache", "templates", "nextcloud") + for _, d := range []string{stackDir, catDir} { + if err := os.MkdirAll(d, 0o755); err != nil { + t.Fatal(err) + } + } + mustWrite(t, filepath.Join(stackDir, "docker-compose.yml"), liveCompose) + if catalogCompose != "" { + mustWrite(t, filepath.Join(catDir, "docker-compose.yml"), catalogCompose) + } + mustWrite(t, filepath.Join(stackDir, "app.yaml"), appYAML) + + m := &Manager{ + cfg: cfg, logger: log.New(io.Discard, "", 0), composeCmd: "docker compose", + encKey: []byte("0123456789abcdef0123456789abcdef"), + stacks: map[string]*Stack{}, + } + m.stacks["nextcloud"] = &Stack{ + Name: "nextcloud", Deployed: true, + ComposePath: filepath.Join(stackDir, "docker-compose.yml"), + AppConfig: LoadAppConfig(stackDir), + } + return m, stackDir +} + +func mustWrite(t *testing.T, path, body string) { + t.Helper() + if err := os.WriteFile(path, []byte(body), 0o644); err != nil { + t.Fatal(err) + } +} + +func readPin(t *testing.T, dir string) *AppConfig { + t.Helper() + b, err := os.ReadFile(filepath.Join(dir, "app.yaml")) + if err != nil { + t.Fatal(err) + } + cfg := &AppConfig{} + if err := yaml.Unmarshal(b, cfg); err != nil { + t.Fatal(err) + } + return cfg +} + +// --- GROUP D: the Update button advances the pin, BEFORE the pull --- + +// TestGroupD_UpdateAdvancesThePinAndRendersTheNewDefinition. +// +// The ordering is the assertion that matters: `compose pull` and `up -d` act on the file on disk, so +// the catalog's definition has to BE that file before either runs. A pin set afterwards would pull +// the frozen version and report success — a button that lies. +func TestGroupD_UpdateAdvancesThePinAndRendersTheNewDefinition(t *testing.T) { + m, stackDir := newPinManager(t, pinTplOld, pinTplNew, + "deployed: true\nenv: {}\npinned_images:\n web: nextcloud:31.0.14-apache\n") + mustWrite(t, AppliedComposePath(stackDir), pinTplOld) + + if err := m.advancePinToCatalog("nextcloud", stackDir); err != nil { + t.Fatal(err) + } + if got := readPin(t, stackDir).PinnedImages["web"]; got != "nextcloud:34.0.1-apache" { + t.Fatalf("pin = %q, want the catalog's current ref", got) + } + live, _ := os.ReadFile(filepath.Join(stackDir, "docker-compose.yml")) + if !strings.Contains(string(live), "34.0.1-apache") { + t.Fatalf("the LIVE compose file must carry the new definition BEFORE the pull:\n%s", live) + } + applied, _ := os.ReadFile(AppliedComposePath(stackDir)) + if !strings.Contains(string(applied), "34.0.1-apache") { + t.Fatalf("the new definition must be stored as the applied one:\n%s", applied) + } +} + +// TestGroupD_UpdateRefusesWhenTheCatalogCannotBeRead — a no-op reported as success is worse than a +// refusal. An UNPINNED app is untouched and returns nil: that is pre-v0.235.0 behaviour. +func TestGroupD_UpdateRefusesWhenTheCatalogCannotBeRead(t *testing.T) { + m, stackDir := newPinManager(t, pinTplOld, "", // no catalog template at all + "deployed: true\nenv: {}\npinned_images:\n web: nextcloud:31.0.14-apache\n") + err := m.advancePinToCatalog("nextcloud", stackDir) + if err == nil { + t.Fatal("a pinned app whose catalog definition cannot be read must REFUSE, not silently no-op") + } + if !strings.Contains(err.Error(), "catalog") { + t.Errorf("the refusal must name the cause, got: %v", err) + } + + m2, dir2 := newPinManager(t, pinTplOld, "", "deployed: true\nenv: {}\n") // unpinned + if err := m2.advancePinToCatalog("nextcloud", dir2); err != nil { + t.Fatalf("an UNPINNED app must be left alone and succeed: %v", err) + } +} + +// --- GROUP E: adoption never guesses --- + +// TestGroupE_AdoptionSkipsWhatItCannotPinConfidently. +// +// COMPANION RED-PROOF 2 (run 2026-09-06): delete the observationCoversTemplate guard from AdoptPins +// so it pins from whatever it observed. The "incomplete observation" sub-test then fails with a pin +// written from a partial reading. Reverted. +func TestGroupE_AdoptionSkipsWhatItCannotPinConfidently(t *testing.T) { + twoSvc := "services:\n web:\n image: nextcloud:31.0.14-apache\n db:\n image: postgres:16-alpine\n" + + t.Run("incomplete observation", func(t *testing.T) { + m, stackDir := newPinManager(t, twoSvc, twoSvc, `deployed: true +env: {} +installed_images: + web: + ref: nextcloud:31.0.14-apache + digest: sha256:a + at: "2026-09-01T00:00:00Z" +`) + m.stacks["nextcloud"].AppConfig = LoadAppConfig(stackDir) + if n := m.AdoptPins(); n != 0 { + t.Fatalf("pinned %d, want 0 — only 1 of 2 services was observed", n) + } + if got := readPin(t, stackDir).PinnedImages; len(got) != 0 { + t.Fatalf("no pin may be synthesised from a partial observation, got %+v", got) + } + }) + + t.Run("complete but running something the template no longer offers", func(t *testing.T) { + m, stackDir := newPinManager(t, pinTplNew, pinTplNew, `deployed: true +env: {} +installed_images: + web: + ref: nextcloud:31.0.14-apache + digest: sha256:a + at: "2026-09-01T00:00:00Z" +`) + m.stacks["nextcloud"].AppConfig = LoadAppConfig(stackDir) + if n := m.AdoptPins(); n != 0 { + t.Fatalf("pinned %d, want 0 — we have no stored definition for what it runs", n) + } + if got := readPin(t, stackDir).PinnedImages; len(got) != 0 { + t.Fatalf("no pin may be invented here, got %+v", got) + } + if _, err := os.Stat(AppliedComposePath(stackDir)); err == nil { + t.Fatal("no applied definition may be manufactured by substituting refs into a newer template") + } + }) + + t.Run("complete and matching — pinned, with the definition stored", func(t *testing.T) { + m, stackDir := newPinManager(t, pinTplOld, pinTplOld, `deployed: true +env: {} +installed_images: + web: + ref: nextcloud:31.0.14-apache + digest: sha256:a + at: "2026-09-01T00:00:00Z" +`) + m.stacks["nextcloud"].AppConfig = LoadAppConfig(stackDir) + if n := m.AdoptPins(); n != 1 { + t.Fatalf("pinned %d, want 1", n) + } + if got := readPin(t, stackDir).PinnedImages["web"]; got != "nextcloud:31.0.14-apache" { + t.Fatalf("pin = %q", got) + } + stored, err := LoadAppliedDefinition(stackDir) + if err != nil || !strings.Contains(string(stored), "31.0.14-apache") { + t.Fatalf("the running definition must be stored: %v %s", err, stored) + } + // Idempotent: a second pass must not re-pin. + if n := m.AdoptPins(); n != 0 { + t.Errorf("a second adoption pass pinned %d, want 0", n) + } + }) +} + +// TestGroupE_AdoptionTouchesNoContainer — it reads and writes files only. +func TestGroupE_AdoptionTouchesNoContainer(t *testing.T) { + m, stackDir := newPinManager(t, pinTplOld, pinTplOld, `deployed: true +env: {} +installed_images: + web: + ref: nextcloud:31.0.14-apache + digest: sha256:a + at: "2026-09-01T00:00:00Z" +`) + m.stacks["nextcloud"].AppConfig = LoadAppConfig(stackDir) + m.installedExecFn = func(context.Context, string, []string, string, ...string) (string, error) { + t.Fatal("adoption must not run any docker command") + return "", nil + } + m.execFn = func(string, ...string) (string, error) { + t.Fatal("adoption must not run any docker command") + return "", nil + } + m.AdoptPins() +} + +// --- GROUP F: a restore's pin survives, and RenderPlanFor reports it --- + +// TestGroupF_RestorePinIsReportedToTheSyncer is the unit half of R-441. The live half is the +// measurement in the report; this pins the contract the syncer relies on. +func TestGroupF_RestorePinIsReportedToTheSyncer(t *testing.T) { + m, stackDir := newPinManager(t, pinTplOld, pinTplNew, "deployed: true\nenv: {}\n") + + // What RecreateStackDefinitionFromUnit does: the unit's compose is already in the stack dir. + pin, data, err := PinFromCompose(ComposePathIn(stackDir)) + if err != nil { + t.Fatal(err) + } + if err := m.SetPin("nextcloud", stackDir, pin, data); err != nil { + t.Fatal(err) + } + + plan := m.RenderPlanFor("nextcloud") + if !plan.Deployed || len(plan.Pinned) == 0 { + t.Fatalf("the syncer must be told the app is deployed and pinned: %+v", plan) + } + if plan.Pinned["web"] != "nextcloud:31.0.14-apache" { + t.Errorf("pin = %q, want the unit's captured image", plan.Pinned["web"]) + } + if plan.AppliedPath == "" { + t.Fatal("the stored definition must be reported, or the syncer cannot freeze and the catalog wins in 15 minutes (R-441)") + } +} + +// TestSetPin_EmptyPinAndMissingAppYAMLAreRefusedOrNoOps. +func TestSetPin_EmptyPinAndMissingAppYAMLAreRefusedOrNoOps(t *testing.T) { + m, stackDir := newPinManager(t, pinTplOld, pinTplOld, "deployed: true\nenv: {}\n") + if err := m.SetPin("nextcloud", stackDir, map[string]string{}, []byte(pinTplOld)); err == nil { + t.Error("an empty pin must be refused — it would read as UNPINNED and silently unfreeze the app") + } + if err := StoreAppliedDefinition(stackDir, nil); err == nil { + t.Error("an empty applied definition must be refused") + } +} + +// TestSetPin_UnchangedPinDoesNotRewriteAppYAML — app.yaml holds encrypted secrets. +func TestSetPin_UnchangedPinDoesNotRewriteAppYAML(t *testing.T) { + m, stackDir := newPinManager(t, pinTplOld, pinTplOld, + "deployed: true\nenv: {}\npinned_images:\n web: nextcloud:31.0.14-apache\n") + p := filepath.Join(stackDir, "app.yaml") + st0, _ := os.Stat(p) + if err := m.SetPin("nextcloud", stackDir, map[string]string{"web": "nextcloud:31.0.14-apache"}, []byte(pinTplOld)); err != nil { + t.Fatal(err) + } + st1, _ := os.Stat(p) + if !st0.ModTime().Equal(st1.ModTime()) || st0.Size() != st1.Size() { + t.Error("an unchanged pin must not rewrite app.yaml") + } +} + +// --- GROUP H: the wiring --- + +// TestGroupH_RenderSeamAndAdoptionAreWiredAtStartup. +// +// The render is INERT unless main.go passes the function, and adoption is inert unless it is called. +// An AST walk, NOT a strings.Contains: a commented-out call still contains the string, which is the +// exact shape of the seam-built-but-never-wired class this project has shipped four times. +// +// It also asserts the ORDER, which is load-bearing: syncer.Start() fires an immediate sync, and if +// that runs before adoption every app is still unpinned, so the first sync of every boot would copy +// the catalog verbatim over a deployed app — the behaviour this release removes, once per boot. +func TestGroupH_RenderSeamAndAdoptionAreWiredAtStartup(t *testing.T) { + src := filepath.Join("..", "..", "cmd", "controller", "main.go") + fset := token.NewFileSet() + f, err := parser.ParseFile(fset, src, nil, 0) + if err != nil { + t.Skipf("cmd/controller is gitignored in some checkouts: %v", err) + } + var seamLine, adoptLine, startLine, backfillLine int + ast.Inspect(f, func(n ast.Node) bool { + call, ok := n.(*ast.CallExpr) + if !ok { + return true + } + sel, ok := call.Fun.(*ast.SelectorExpr) + if !ok { + return true + } + line := fset.Position(call.Pos()).Line + switch sel.Sel.Name { + case "SetRenderPlanFn": + seamLine = line + case "AdoptPins": + adoptLine = line + case "BackfillInstalledImages": + backfillLine = line + case "Start": + if id, ok := sel.X.(*ast.Ident); ok && id.Name == "syncer" { + startLine = line + } + } + return true + }) + if seamLine == 0 { + t.Fatal("SetRenderPlanFn is never called from cmd/controller — the render is INERT and the syncer copies verbatim") + } + if adoptLine == 0 { + t.Fatal("AdoptPins is never called from cmd/controller — nothing would ever be pinned") + } + if backfillLine != 0 && adoptLine < backfillLine { + t.Errorf("adoption (line %d) must run AFTER the installed-images backfill (line %d) — it needs that observation", adoptLine, backfillLine) + } + if startLine == 0 { + t.Fatal("syncer.Start() is never called") + } + if startLine < adoptLine { + t.Errorf("syncer.Start() (line %d) must come AFTER AdoptPins (line %d): the initial sync would otherwise run while every app is unpinned and overwrite a deployed app's version once per boot", startLine, adoptLine) + } +} diff --git a/controller/internal/sync/render_test.go b/controller/internal/sync/render_test.go new file mode 100644 index 0000000..18c055f --- /dev/null +++ b/controller/internal/sync/render_test.go @@ -0,0 +1,267 @@ +package sync + +import ( + "io" + "log" + "os" + "path/filepath" + "strings" + "testing" + + "gitea.dooplex.hu/admin/felhom-controller/internal/config" + "gitea.dooplex.hu/admin/felhom-controller/internal/stacks" +) + +// Slice 3 (v0.235.0) — "freeze the version, keep the fixes flowing" (operator ruling, 2026-09-06). +// +// Every assertion reads the rendered docker-compose.yml BACK OFF DISK. "copyTemplates returned nil" +// is hollow: the whole feature is which bytes end up in that file. + +const ( + tplOld = `services: + web: + image: nextcloud:31.0.14-apache + healthcheck: + test: ["CMD", "curl", "-f", "http://127.0.0.1:80"] +` + // Same version, a FIX to the healthcheck — Scenario A's input. + tplOldFixed = `services: + web: + image: nextcloud:31.0.14-apache + healthcheck: + test: ["CMD", "curl", "-fsS", "http://127.0.0.1:80/status.php"] +` + // A new VERSION, and a template shaped for it — Scenario B's input. + tplNew = `services: + web: + image: nextcloud:34.0.1-apache + environment: + - NEXTCLOUD_TRUSTED_DOMAINS=example +` +) + +// renderFixture builds a syncer over a temp root with one catalog template and one stack dir. +func renderFixture(t *testing.T, catalogCompose string) (*Syncer, string, string) { + t.Helper() + root := t.TempDir() + cfg := &config.Config{} + cfg.Paths.DataDir = filepath.Join(root, "data") + cfg.Paths.StacksDir = filepath.Join(root, "stacks") + catDir := filepath.Join(cfg.Paths.DataDir, "catalog-cache", "templates", "nextcloud") + stackDir := filepath.Join(cfg.Paths.StacksDir, "nextcloud") + for _, d := range []string{catDir, stackDir} { + if err := os.MkdirAll(d, 0o755); err != nil { + t.Fatal(err) + } + } + write(t, filepath.Join(catDir, "docker-compose.yml"), catalogCompose) + write(t, filepath.Join(catDir, ".felhom.yml"), "display_name: Nextcloud\ncatalog_since: \"2026-07-18\"\n") + s := New(cfg, log.New(io.Discard, "", 0), func() error { return nil }, nil) + return s, stackDir, catDir +} + +func write(t *testing.T, path, body string) { + t.Helper() + if err := os.WriteFile(path, []byte(body), 0o644); err != nil { + t.Fatal(err) + } +} + +func readFile(t *testing.T, path string) string { + t.Helper() + b, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + return string(b) +} + +// pinnedPlan is the seam's answer for a deployed, pinned app with a stored definition. +func pinnedPlan(stackDir string, pin map[string]string, applied bool) func(string) stacks.RenderPlan { + return func(string) stacks.RenderPlan { + p := stacks.RenderPlan{Deployed: true, Pinned: pin} + if applied { + p.AppliedPath = stacks.AppliedComposePath(stackDir) + } + return p + } +} + +// --- GROUP A: the catalog still offers the pinned version → FIXES FLOW --- + +// TestGroupA_FixFlowsToAPinnedMatchingApp is the half the operator explicitly chose to KEEP. +// Freezing everything would have been far simpler and would have broken this. +func TestGroupA_FixFlowsToAPinnedMatchingApp(t *testing.T) { + s, stackDir, _ := renderFixture(t, tplOldFixed) + live := filepath.Join(stackDir, "docker-compose.yml") + write(t, live, tplOld) + write(t, stacks.AppliedComposePath(stackDir), tplOld) + s.SetRenderPlanFn(pinnedPlan(stackDir, map[string]string{"web": "nextcloud:31.0.14-apache"}, true)) + + if _, _, err := s.copyTemplates(); err != nil { + t.Fatal(err) + } + got := readFile(t, live) + if !strings.Contains(got, "status.php") { + t.Fatalf("the healthcheck FIX must reach a pinned app whose version the catalog still offers.\n%s", got) + } + if !strings.Contains(got, "31.0.14-apache") { + t.Errorf("the version must not have moved: %s", got) + } +} + +// --- GROUP B: the catalog moved → the app FREEZES, WHOLE --- + +// TestGroupB_CatalogMoveFreezesTheAppWhole. +// +// COMPANION RED-PROOF 1 (run 2026-09-06): make renderSource return the catalog template on the +// moved branch (i.e. the "substitute the refs" shortcut, or simply forgetting the branch). This test +// then fails on the version assertion. Reverted. +func TestGroupB_CatalogMoveFreezesTheAppWhole(t *testing.T) { + s, stackDir, _ := renderFixture(t, tplNew) + live := filepath.Join(stackDir, "docker-compose.yml") + write(t, live, tplOld) + write(t, stacks.AppliedComposePath(stackDir), tplOld) + s.SetRenderPlanFn(pinnedPlan(stackDir, map[string]string{"web": "nextcloud:31.0.14-apache"}, true)) + + if _, _, err := s.copyTemplates(); err != nil { + t.Fatal(err) + } + got := readFile(t, live) + if strings.Contains(got, "34.0.1-apache") { + t.Fatalf("a pinned app must NOT receive the catalog's new version:\n%s", got) + } + if !strings.Contains(got, "31.0.14-apache") { + t.Fatalf("the frozen version must still be named:\n%s", got) + } + // THE WHOLE definition, never a substitution. The new template's env belongs to the new + // version; an old image under a new template is a third state nobody chose (`wger 2.6`). + if strings.Contains(got, "NEXTCLOUD_TRUSTED_DOMAINS") { + t.Fatalf("the NEW template's body leaked into a frozen app — the whole stored definition must be used:\n%s", got) + } + if !strings.Contains(got, "healthcheck") { + t.Errorf("the stored definition should have been written verbatim:\n%s", got) + } + // `.felhom.yml` is ALWAYS copied — it carries catalog_since, which the badge needs. + if !strings.Contains(readFile(t, filepath.Join(stackDir, ".felhom.yml")), "catalog_since") { + t.Error(".felhom.yml must flow even to a frozen app") + } +} + +// --- GROUP C: self-healing, in BOTH branches --- + +// TestGroupC_SelfHealingSurvivesInBothBranches. A hand-broken compose file repairing itself within +// 15 minutes was MEASURED in SPIKE-app-update-2026-09-01 §3, and is a property this task must keep. +func TestGroupC_SelfHealingSurvivesInBothBranches(t *testing.T) { + t.Run("catalog still offers the pin — heals to the catalog", func(t *testing.T) { + s, stackDir, _ := renderFixture(t, tplOld) + live := filepath.Join(stackDir, "docker-compose.yml") + write(t, live, "services:\n web:\n image: alpine:3.20 # hand-broken\n") + write(t, stacks.AppliedComposePath(stackDir), tplOld) + s.SetRenderPlanFn(pinnedPlan(stackDir, map[string]string{"web": "nextcloud:31.0.14-apache"}, true)) + if _, _, err := s.copyTemplates(); err != nil { + t.Fatal(err) + } + if got := readFile(t, live); !strings.Contains(got, "31.0.14-apache") || strings.Contains(got, "alpine") { + t.Fatalf("a corrupted file must heal from the catalog:\n%s", got) + } + }) + t.Run("catalog has moved — heals to the STORED definition", func(t *testing.T) { + s, stackDir, _ := renderFixture(t, tplNew) + live := filepath.Join(stackDir, "docker-compose.yml") + write(t, live, "services:\n web:\n image: alpine:3.20 # hand-broken\n") + write(t, stacks.AppliedComposePath(stackDir), tplOld) + s.SetRenderPlanFn(pinnedPlan(stackDir, map[string]string{"web": "nextcloud:31.0.14-apache"}, true)) + if _, _, err := s.copyTemplates(); err != nil { + t.Fatal(err) + } + got := readFile(t, live) + if strings.Contains(got, "alpine") { + t.Fatalf("a corrupted file must heal even while frozen:\n%s", got) + } + if !strings.Contains(got, "31.0.14-apache") || strings.Contains(got, "34.0.1") { + t.Fatalf("it must heal to the STORED definition, not the catalog's new one:\n%s", got) + } + }) +} + +// --- the render table's remaining rows --- + +func TestRenderTable_UnpinnedAndUndeployedAndMissingStore(t *testing.T) { + cases := []struct { + name string + plan stacks.RenderPlan + applied bool + wantNew bool // does the catalog's NEW version land in the live file? + wantSkip bool + }{ + {name: "not deployed", plan: stacks.RenderPlan{}, wantNew: true}, + {name: "protected", plan: stacks.RenderPlan{Deployed: true, Protected: true}, wantNew: true}, + {name: "deployed but UNPINNED", plan: stacks.RenderPlan{Deployed: true}, wantNew: true}, + {name: "mid-deploy — skipped", plan: stacks.RenderPlan{Deployed: true, Deploying: true, + Pinned: map[string]string{"web": "nextcloud:31.0.14-apache"}}, wantSkip: true}, + {name: "pinned, moved, NO stored definition", plan: stacks.RenderPlan{Deployed: true, + Pinned: map[string]string{"web": "nextcloud:31.0.14-apache"}}, wantNew: true}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + s, stackDir, _ := renderFixture(t, tplNew) + live := filepath.Join(stackDir, "docker-compose.yml") + write(t, live, tplOld) + plan := c.plan + if c.applied { + plan.AppliedPath = stacks.AppliedComposePath(stackDir) + } + s.SetRenderPlanFn(func(string) stacks.RenderPlan { return plan }) + if _, _, err := s.copyTemplates(); err != nil { + t.Fatal(err) + } + got := readFile(t, live) + switch { + case c.wantSkip: + if got != tplOld { + t.Fatalf("a mid-deploy app's compose file must be left ALONE:\n%s", got) + } + case c.wantNew: + if !strings.Contains(got, "34.0.1-apache") { + t.Fatalf("want the catalog copied verbatim (pre-v0.235.0 behaviour):\n%s", got) + } + } + }) + } +} + +// TestRenderTable_NilSeamIsExactlyTheOldBehaviour — the nil case is the safety net: a wiring mistake +// must degrade to the previous product, not to a broken one. +func TestRenderTable_NilSeamIsExactlyTheOldBehaviour(t *testing.T) { + s, stackDir, _ := renderFixture(t, tplNew) + live := filepath.Join(stackDir, "docker-compose.yml") + write(t, live, tplOld) + if _, _, err := s.copyTemplates(); err != nil { + t.Fatal(err) + } + if !strings.Contains(readFile(t, live), "34.0.1-apache") { + t.Fatal("with no seam the syncer must copy verbatim, exactly as before v0.235.0") + } +} + +// TestRenderTable_EmptyStoredDefinitionIsTreatedAsAbsent — never write an empty compose file over a +// live app. An empty applied file is "absent", which copies the catalog and WARNs. +func TestRenderTable_EmptyStoredDefinitionIsTreatedAsAbsent(t *testing.T) { + s, stackDir, _ := renderFixture(t, tplNew) + live := filepath.Join(stackDir, "docker-compose.yml") + write(t, live, tplOld) + write(t, stacks.AppliedComposePath(stackDir), " \n") + s.SetRenderPlanFn(func(string) stacks.RenderPlan { + // The manager's own RenderPlanFor would report "" here; this asserts the syncer is not + // relying on that alone — an empty file must never be rendered even if a path arrives. + return stacks.RenderPlan{Deployed: true, Pinned: map[string]string{"web": "nextcloud:31.0.14-apache"}, + AppliedPath: stacks.AppliedComposePath(stackDir)} + }) + if _, _, err := s.copyTemplates(); err != nil { + t.Fatal(err) + } + if got := readFile(t, live); strings.TrimSpace(got) == "" { + t.Fatal("an empty compose file was written over a live app") + } +} diff --git a/controller/internal/sync/sync.go b/controller/internal/sync/sync.go index 2120e98..0e19274 100644 --- a/controller/internal/sync/sync.go +++ b/controller/internal/sync/sync.go @@ -17,6 +17,11 @@ import ( "time" "gitea.dooplex.hu/admin/felhom-controller/internal/config" + // stacks is imported for its PURE helpers only — RenderPlan and ParseComposeImages. The syncer + // must not reach for the Manager through it: everything it needs about an app arrives through + // renderPlanFn. Importing the package rather than re-writing a compose parser is deliberate; + // a second image parser is exactly the duplication REUSE.md exists to prevent. + "gitea.dooplex.hu/admin/felhom-controller/internal/stacks" ) // gitCmdTimeout bounds each individual git subprocess (campaign finding F3, 2026-07-06): @@ -33,6 +38,15 @@ type Syncer struct { cacheDir string // local git clone rescanFn func() error postSyncHook func(updated []string) // called after sync with names of updated stacks + // renderPlanFn answers, for one app: is it deployed, is it pinned, and where is its stored + // applied definition (v0.235.0). It is the ONLY way this package learns any of that: the syncer + // must not import the stack manager and must NEVER read app.yaml itself — that file is the + // manager's and carries encrypted values. + // + // NIL-SAFE BY DESIGN: a nil seam means "copy verbatim", i.e. byte-for-byte the pre-v0.235.0 + // behaviour. Every existing test that constructs a Syncer without one keeps passing, and a + // wiring mistake degrades to the old product rather than to a broken one. + renderPlanFn func(appName string) stacks.RenderPlan mu sync.Mutex lastSync time.Time lastErr error @@ -71,6 +85,11 @@ func New(cfg *config.Config, logger *log.Logger, rescanFn func() error, postSync } } +// SetRenderPlanFn injects the per-app render plan (v0.235.0). Exported and separate from New for the +// same reason SetSambaRunProbe is: New's signature is called from tests in several packages, and the +// seam has to be optional so a Syncer without one keeps the pre-v0.235.0 behaviour exactly. +func (s *Syncer) SetRenderPlanFn(fn func(appName string) stacks.RenderPlan) { s.renderPlanFn = fn } + // isDebug returns true if the logging level is set to "debug". func (s *Syncer) isDebug() bool { return s.cfg.Logging.Level == "debug" } @@ -356,6 +375,16 @@ func (s *Syncer) copyTemplates() (newApps []string, updated []string, err error) continue } + // v0.235.0 — the RENDER. `.felhom.yml` is always copied verbatim (it holds no image); + // only the compose file can be frozen. See renderSource for the whole table. + if filename == "docker-compose.yml" { + frozenSrc, skip := s.renderSource(appName, src) + if skip { + continue + } + src = frozenSrc + } + changed, err := copyIfChanged(src, dst) if err != nil { s.logger.Printf("[WARN] [sync] Failed to copy catalog file %s/%s: %v", appName, filename, err) @@ -382,6 +411,103 @@ func (s *Syncer) copyTemplates() (newApps []string, updated []string, err error) return newApps, updated, nil } +// renderSource decides WHICH file becomes this app's live docker-compose.yml, and is the whole of +// the v0.235.0 ruling: "freeze the version, keep the fixes flowing" (operator, 2026-09-06). +// +// It returns the path to copy FROM, and whether to skip the app this cycle. +// +// ── THE TABLE, COMPLETE ────────────────────────────────────────────────────────────────────── +// +// not deployed / protected / no seam → the catalog template (today's behaviour) +// deployed, UNPINNED → the catalog template (today's behaviour) + one DEBUG +// deployed, pinned, catalog images == → the catalog template — FIXES FLOW, SELF-HEALING WORKS +// deployed, pinned, catalog images != → the STORED definition — the app is frozen WHOLE +// deployed, pinned, differ, none stored → the catalog template + one WARN +// mid-deploy → skip the compose file this cycle +// +// ── WHY THE FROZEN BRANCH WRITES A WHOLE FILE AND NEVER A SUBSTITUTION ─────────────────────── +// +// The obvious-looking alternative — take the new template and put the old image refs back — creates +// a third state nobody chose: `wger 2.6` needs a full DB configuration the older template cannot +// supply, so a new template around an old image is broken in a way neither version is. When the +// catalog has moved, the WHOLE stored definition is used. +// +// ── AND WHY THIS IS NOT SIMPLY "SKIP DEPLOYED APPS" ────────────────────────────────────────── +// +// That was option B and it was rejected: it also stops health-check fixes, memory limits and new +// deploy fields reaching a deployed app, and it destroys the self-healing measured in +// SPIKE-app-update-2026-09-01 §3 — a hand-broken compose file repaired itself within 15 minutes. +// Both halves were worth keeping; only the version change was not. +func (s *Syncer) renderSource(appName, catalogSrc string) (src string, skip bool) { + if s.renderPlanFn == nil { + return catalogSrc, false // pre-v0.235.0 behaviour, byte for byte + } + plan := s.renderPlanFn(appName) + + if !plan.Deployed || plan.Protected { + return catalogSrc, false + } + if plan.Deploying { + // Do not race an in-flight deploy for its own compose file. + if s.isDebug() { + s.logger.Printf("[DEBUG] [sync] %s: mid-deploy, compose file left alone this cycle", appName) + } + return "", true + } + if len(plan.Pinned) == 0 { + if s.isDebug() { + s.logger.Printf("[DEBUG] [sync] %s: deployed but UNPINNED — catalog copied verbatim (pre-v0.235.0 behaviour)", appName) + } + return catalogSrc, false + } + + catalogImages, err := stacks.ParseComposeImages(catalogSrc) + if err != nil { + // CANNOT TELL whether the catalog has moved. Leave the app's file alone rather than guess in + // either direction — an unreadable catalog template must not be able to unfreeze an app. + s.logger.Printf("[WARN] [sync] %s: cannot read the catalog template's images (%v) — compose file left alone this cycle", appName, err) + return "", true + } + + if samePin(plan.Pinned, catalogImages) { + // The catalog still offers what this app runs: everything else in the template is a FIX and + // is delivered, exactly as before v0.235.0. This branch is why the feature is not a freeze. + return catalogSrc, false + } + + if plan.AppliedPath == "" { + // We cannot freeze what we do not have, and we must not invent it. + s.logger.Printf("[WARN] [sync] %s: the catalog has moved past this app's pinned version, but no stored definition exists — copying the catalog verbatim (pre-v0.235.0 behaviour). The app will take the new version on its next start.", appName) + return catalogSrc, false + } + // DEFENCE IN DEPTH, and this line was added because a test demanded it: the manager's + // RenderPlanFor already refuses to hand over a path whose file is missing or empty, but the + // syncer is what WRITES, and writing an empty compose file over a live app takes that app down. + // The cost of re-reading a small file once per app per cycle is nothing next to that. + if data, err := os.ReadFile(plan.AppliedPath); err != nil || len(strings.TrimSpace(string(data))) == 0 { + s.logger.Printf("[WARN] [sync] %s: the stored definition is missing or empty (%v) — copying the catalog verbatim rather than writing an empty compose file", appName, err) + return catalogSrc, false + } + if s.isDebug() { + s.logger.Printf("[DEBUG] [sync] %s: catalog has moved past the pin — rendering the stored applied definition", appName) + } + return plan.AppliedPath, false +} + +// samePin compares a pin against a template's images. Local to the syncer so this package needs +// nothing from the manager beyond the plan it is handed. +func samePin(pinned, catalog map[string]string) bool { + if len(pinned) != len(catalog) { + return false + } + for svc, ref := range pinned { + if got, ok := catalog[svc]; !ok || got != ref { + return false + } + } + return true +} + // logFileHashes logs the source and destination file hashes for debugging. func (s *Syncer) logFileHashes(appName, filename, src, dst string) { srcData, err := os.ReadFile(src) diff --git a/controller/internal/web/updatebadge.go b/controller/internal/web/updatebadge.go index ad71915..01cceaf 100644 --- a/controller/internal/web/updatebadge.go +++ b/controller/internal/web/updatebadge.go @@ -29,7 +29,14 @@ const ( // // NO REGISTRY QUERY, deliberately: a customer's box must not depend on reaching eight upstream // registries to render a page. The comparison is therefore reference-to-reference — what the -// container was created from, against what the compose file now pins. +// container was created from, against what the CATALOG currently offers. +// +// ⚠ IT COMPARES AGAINST Stack.CatalogImages, NEVER Stack.TemplateImages, AND v0.235.0 IS WHY. +// Since the freeze, a pinned app's LIVE docker-compose.yml is rendered from its own stored +// definition once the catalog moves past it — so the live file names the OLD version, installed +// would equal template, and this function would answer „Naprakész" on precisely the apps that are +// behind. It would invert the feature silently, with every test still green, because the two fields +// have the same type and shape. CatalogImages is read from the syncer's git clone instead. // // KNOWN LIMITATION, stated rather than hidden (see 09-update-architecture.md and the register row): // 23 of the catalog's 66 distinct pins FLOAT (postgres:16-alpine, mariadb:11.6, …). For those the @@ -46,15 +53,15 @@ func compareInstalledToTemplate(s stacks.Stack) updateState { if s.AppConfig == nil || len(s.AppConfig.InstalledImages) == 0 { return updateUnknown // legacy app.yaml — no record was ever written } - if len(s.TemplateImages) == 0 { - return updateUnknown // the compose file could not be read or pins nothing + if len(s.CatalogImages) == 0 { + return updateUnknown // no readable catalog template — cannot tell, so say nothing } - if len(s.AppConfig.InstalledImages) != len(s.TemplateImages) { + if len(s.AppConfig.InstalledImages) != len(s.CatalogImages) { // A service was added or removed by the template. That IS a change the customer's running // stack has not taken up. return updateBehind } - for svc, want := range s.TemplateImages { + for svc, want := range s.CatalogImages { got, ok := s.AppConfig.InstalledImages[svc] if !ok || got.Ref != want { return updateBehind diff --git a/controller/internal/web/updatebadge_test.go b/controller/internal/web/updatebadge_test.go index 2439e47..454ee4a 100644 --- a/controller/internal/web/updatebadge_test.go +++ b/controller/internal/web/updatebadge_test.go @@ -15,18 +15,35 @@ import ( var badgeNow = time.Date(2026, 9, 2, 12, 0, 0, 0, time.UTC) -// ubStack builds a deployed app whose record and template pins are stated explicitly. -func ubStack(installed map[string]stacks.InstalledImage, template map[string]string, since string) stacks.Stack { +// ubStack builds a deployed app whose record and CATALOG images are stated explicitly. +// +// Since v0.235.0 the badge compares against `CatalogImages` — what the catalog OFFERS — and never +// against `TemplateImages`, which after the freeze is the app's own (possibly frozen) live file. +// Both are set to the same map here because that is the un-frozen case; `ubFrozenStack` is the one +// where they deliberately differ. +func ubStack(installed map[string]stacks.InstalledImage, catalog map[string]string, since string) stacks.Stack { return stacks.Stack{ Name: "bookstack", Deployed: true, State: stacks.StateRunning, Meta: stacks.Metadata{DisplayName: "BookStack", Slug: "bookstack", CatalogSince: since}, AppConfig: &stacks.AppConfig{Deployed: true, InstalledImages: installed}, - TemplateImages: template, + TemplateImages: catalog, + CatalogImages: catalog, } } +// ubFrozenStack models a v0.235.0 FROZEN app: it runs an old version, its LIVE compose file has been +// rendered from its own stored definition and therefore also names the old version, and the CATALOG +// has moved on. This is the shape that silently inverts the badge if the comparison reads the wrong +// field — see TestGroupG. +func ubFrozenStack(running, catalogRef, since string) stacks.Stack { + st := ubStack(map[string]stacks.InstalledImage{"web": rec(running)}, map[string]string{"web": catalogRef}, since) + st.AppConfig.PinnedImages = map[string]string{"web": running} + st.TemplateImages = map[string]string{"web": running} // the frozen live file + return st +} + func rec(ref string) stacks.InstalledImage { return stacks.InstalledImage{Ref: ref, Digest: "sha256:x", At: "2026-09-01T00:00:00Z"} } @@ -277,3 +294,50 @@ func TestGroupF_CatalogSinceTolerance(t *testing.T) { } } } + +// --- GROUP G (v0.235.0): the badge must read the CATALOG, not the rendered file --- + +// TestGroupG_FrozenAppStillReadsBehind is the test that stops slice 3 from silently inverting slice 2. +// +// After the freeze, a pinned app whose version the catalog has moved past has its LIVE +// docker-compose.yml rendered from its own stored definition — so that file names the OLD version. +// A comparison against it finds installed == template and answers „Naprakész" on precisely the apps +// that are behind. The two fields have the same type and shape, so nothing but this test catches it. +// +// COMPANION RED-PROOF 3 (run 2026-09-06): point compareInstalledToTemplate back at +// s.TemplateImages. This test then fails with „Naprakész" on a frozen app. Reverted. +func TestGroupG_FrozenAppStillReadsBehind(t *testing.T) { + since := time.Now().UTC().AddDate(0, 0, -46).Format("2006-01-02") + frozen := ubFrozenStack("nextcloud:31.0.14-apache", "nextcloud:34.0.1-apache", since) + + b := updateBadgeAt(frozen, time.Now().UTC()) + if b == nil { + t.Fatal("a frozen, behind app must carry a badge") + } + if b.Label == "Naprakész" { + t.Fatal("THE FEATURE IS INVERTED: the comparison read the frozen live file instead of the catalog") + } + if !strings.HasPrefix(b.Label, "Frissítés elérhető") || b.Class != "tag-warn" { + t.Fatalf("label = %q class = %q", b.Label, b.Class) + } + + // And it renders that way on the real page, not just in the pure function. + html := renderBackupPage(t, "stacks", ubStacksData(frozen)) + if !strings.Contains(html, "Frissítés elérhető") { + t.Error("the frozen app must be badged as behind on the app list") + } + if strings.Contains(html, "Naprakész") { + t.Error("a frozen, behind app must never render Naprakész") + } +} + +// TestGroupG_NoCatalogEntryRendersNothing — an orphaned app, or a box whose catalog cache is missing, +// cannot be judged. Absent is unknown; it is never „Naprakész". +func TestGroupG_NoCatalogEntryRendersNothing(t *testing.T) { + st := ubStack(map[string]stacks.InstalledImage{"web": rec("nginx:1.27")}, + map[string]string{"web": "nginx:1.27"}, "2026-07-18") + st.CatalogImages = nil // the catalog cache could not be read + if b := updateBadgeAt(st, badgeNow); b != nil { + t.Fatalf("no readable catalog template must render NOTHING, got %q", b.Label) + } +}