v0.235.0: freeze the version, keep the fixes flowing (operator ruling 2026-09-06)

Slice 3. 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. The operator ruled Option 1, and this implements it. The rule: while the catalog offers the same version you run, its fixes flow to you; the moment it moves to a newer version you are frozen until you update. NOTHING was added to any of the thirteen compose up -d call sites. Most 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. They are made safe by removing the reason. app.yaml gains pinned_images: what the app is SUPPOSED to run. It is NOT installed_images, which is an observation; letting a reading become a deployment is the R-166 category error one field over. Four writers, each also storing the exact definition as applied-compose.yml. UpdateStack advances the pin and re-renders BEFORE the pull, because pull and up -d act on the file on disk, and a pin set afterwards would pull the frozen version and report success. The syncer renders instead of copying, through one nil-safe seam. Catalog images equal the pin -> verbatim, so fixes and self-healing both survive; they differ -> the WHOLE stored definition, never a substitution of refs into a newer template (wger 2.6 needs a DB config the older template cannot supply). This is deliberately not 'skip deployed apps', which was option B and was rejected. AdoptPins runs once at boot after the backfill, files only, and skips loudly rather than inventing a pin. syncer.Start() moved to after it: the initial sync would otherwise run while every app was unpinned and overwrite a deployed app's version once per boot. THE BADGE HAD TO CHANGE OR SLICE 2 WOULD HAVE INVERTED SILENTLY. TemplateImages reads the LIVE compose file, which is now the frozen one, so the comparison would have answered Naprakesz on exactly the apps that are behind - with every test green, because the new field has the same type. It now reads CatalogImages. +16 tests (1729 -> 1745), 28 packages green. Three red-proofs run and reverted. A test also caught the syncer writing an empty compose file over a live app.
2026-09-06 09:45:34 +02:00
parent 998aa31958
commit 8a0e0a59ad
13 changed files with 1437 additions and 17 deletions
@@ -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")
+	}
+}
@@ -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)