From 0b349e2415170673d0a743873851ecd88c2b45ff Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Mon, 5 Oct 2026 21:52:26 +0200 Subject: [PATCH] R-607 + R-615: the catalog sync follows a changed repo_url and rescans when the catalog moved MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit R-615: a changed git.repo_url was inert (the clone's stored origin was fetched for ever). The sync now compares the configured URL with the clone's origin: another repository -> drop the cache and clone it; the same repository with new credentials -> set-url. R-607: a catalog move that changed no stack directory (a deployed, pinned app keeps its stored definition) skipped the rescan, so CatalogImages and the update badge stayed stale, and the sync said 'nincs változás'. The sync now notes the cache commit before and after, rescans when it moved, says so, and returns catalog_moved. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_0159rPz1ZhFKsS53msqPYxtS --- controller/internal/sync/r607_r615_test.go | 132 +++++++++++++++++++++ controller/internal/sync/sync.go | 85 ++++++++++++- 2 files changed, 213 insertions(+), 4 deletions(-) create mode 100644 controller/internal/sync/r607_r615_test.go diff --git a/controller/internal/sync/r607_r615_test.go b/controller/internal/sync/r607_r615_test.go new file mode 100644 index 0000000..0f4cf3b --- /dev/null +++ b/controller/internal/sync/r607_r615_test.go @@ -0,0 +1,132 @@ +package sync + +import ( + "log" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + "time" + + "gitea.dooplex.hu/admin/felhom-controller/internal/config" +) + +// Local git repositories only (file://) — no network. + +func gitT(t *testing.T, dir string, args ...string) string { + t.Helper() + cmd := exec.Command("git", append([]string{"-c", "user.email=t@example.invalid", "-c", "user.name=t", "-c", "init.defaultBranch=main", "-c", "commit.gpgsign=false"}, args...)...) + cmd.Dir = dir + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("git %v: %v\n%s", args, err, out) + } + return strings.TrimSpace(string(out)) +} + +// newCatalogRepo makes a work repo with one template and a marker file, on branch main. +func newCatalogRepo(t *testing.T, marker string) string { + t.Helper() + dir := t.TempDir() + gitT(t, dir, "init", "-q", "-b", "main") + app := filepath.Join(dir, "templates", "demo") + if err := os.MkdirAll(app, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(app, "docker-compose.yml"), []byte("services:\n web:\n image: busybox:1\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "MARKER"), []byte(marker), 0o644); err != nil { + t.Fatal(err) + } + gitT(t, dir, "add", "-A") + gitT(t, dir, "commit", "-q", "-m", "init") + return dir +} + +func newLocalSyncer(t *testing.T, repo string, rescans *int) *Syncer { + t.Helper() + root := t.TempDir() + cfg := &config.Config{} + cfg.Git.RepoURL = "file://" + repo + cfg.Git.Branch = "main" + cfg.Git.SyncInterval = "15m" + cfg.Paths.DataDir = filepath.Join(root, "data") + cfg.Paths.StacksDir = filepath.Join(root, "stacks") + if err := os.MkdirAll(cfg.Paths.StacksDir, 0o755); err != nil { + t.Fatal(err) + } + return New(cfg, log.New(os.Stderr, "", 0), func() error { *rescans++; return nil }, nil) +} + +func syncNow(t *testing.T, s *Syncer) SyncResult { + t.Helper() + s.mu.Lock() + s.lastSync = time.Now().Add(-time.Hour) // past the debounce + s.mu.Unlock() + res := s.TriggerSync() + if !res.OK { + t.Fatalf("sync failed: %s", res.Message) + } + return res +} + +// R-615: changing git.repo_url under an existing cache makes the next sync read the NEW repository. +func TestR615_ChangedRepoURLIsFollowed(t *testing.T) { + a := newCatalogRepo(t, "catalog-A") + b := newCatalogRepo(t, "catalog-B") + n := 0 + s := newLocalSyncer(t, a, &n) + syncNow(t, s) + readMarker := func() string { + bs, err := os.ReadFile(filepath.Join(s.cacheDir, "MARKER")) + if err != nil { + t.Fatal(err) + } + return string(bs) + } + if readMarker() != "catalog-A" { + t.Fatalf("setup: cache should hold catalog A") + } + s.cfg.Git.RepoURL = "file://" + b // the operator repoints the box + syncNow(t, s) + if got := readMarker(); got != "catalog-B" { + t.Fatalf("R-615: after changing repo_url the cache still reads %q — the old repository is still being followed", got) + } + if origin := gitT(t, s.cacheDir, "config", "--get", "remote.origin.url"); origin != "file://"+b { + t.Errorf("the cache's origin must be the configured repository, got %q", origin) + } +} + +// R-607: a catalog move that changes no stack directory still rescans (CatalogImages is read from the +// cache) and is not reported as „nincs változás". +func TestR607_CatalogMoveWithoutStackChangeRescansAndSaysSo(t *testing.T) { + a := newCatalogRepo(t, "v1") + n := 0 + s := newLocalSyncer(t, a, &n) + syncNow(t, s) + before := n + + // No change at all: honest „nincs változás", no rescan. + res := syncNow(t, s) + if res.CatalogMoved || !strings.Contains(res.Message, "nincs v") || n != before { + t.Fatalf("an unchanged catalog: moved=%v msg=%q rescans %d->%d", res.CatalogMoved, res.Message, before, n) + } + + // The catalog moves in a file no stack dir copies. + if err := os.WriteFile(filepath.Join(a, "MARKER"), []byte("v2"), 0o644); err != nil { + t.Fatal(err) + } + gitT(t, a, "commit", "-q", "-am", "move") + res = syncNow(t, s) + if !res.CatalogMoved { + t.Fatal("R-607: the sync did not notice the catalog moved") + } + if n != before+1 { + t.Fatalf("R-607: a catalog move must rescan (CatalogImages is read from the cache); rescans %d->%d", before, n) + } + if strings.Contains(res.Message, "nincs v") { + t.Fatalf("R-607: a moved catalog was reported as unchanged: %q", res.Message) + } +} diff --git a/controller/internal/sync/sync.go b/controller/internal/sync/sync.go index 74772a1..0457eea 100644 --- a/controller/internal/sync/sync.go +++ b/controller/internal/sync/sync.go @@ -69,6 +69,9 @@ type SyncResult struct { NewApps []string `json:"new_apps,omitempty"` Updated []string `json:"updated,omitempty"` Message string `json:"message"` + // CatalogMoved (R-607): the catalog cache's commit changed in this sync, whether or not any stack + // directory did. + CatalogMoved bool `json:"catalog_moved,omitempty"` } // New creates a new Syncer. rescanFn is called after a successful sync to trigger ScanStacks(). @@ -207,7 +210,11 @@ func (s *Syncer) doSync() SyncResult { s.logger.Printf("[INFO] [sync] Starting catalog sync") - // Step 1: Clone or pull + // Step 1: Clone or pull. R-607: note the cache's commit before and after — the stack-dir copies + // below are NOT the whole story: a deployed, pinned app keeps its stored definition (renderSource), + // so a catalog move can change nothing in the stack dirs and still change what the update badge + // must compare against (`CatalogImages`, read from the cache by the rescan). + headBefore := s.cacheHead() if err := s.gitCloneOrPull(); err != nil { s.logger.Printf("[ERROR] [sync] Catalog sync failed: %v", err) s.mu.Lock() @@ -230,9 +237,17 @@ func (s *Syncer) doSync() SyncResult { result.NewApps = newApps result.Updated = updated + headAfter := s.cacheHead() + catalogMoved := headAfter != "" && headAfter != headBefore + result.CatalogMoved = catalogMoved + if catalogMoved { + s.logger.Printf("[INFO] [sync] catalog moved %s -> %s (%d new, %d stack dir(s) changed)", shortHead(headBefore), shortHead(headAfter), len(newApps), len(updated)) + } - // Step 3: Trigger rescan if anything changed - if len(newApps) > 0 || len(updated) > 0 { + // Step 3: Trigger rescan if anything changed — the catalog itself included (R-607: before, a move + // that changed no stack dir left `CatalogImages` stale until the next timed scan, and the badge + // could read „Naprakész" on an app that was behind). + if len(newApps) > 0 || len(updated) > 0 || catalogMoved { if err := s.rescanFn(); err != nil { s.logger.Printf("[WARN] [sync] Rescan after sync failed: %v", err) } @@ -254,7 +269,11 @@ func (s *Syncer) doSync() SyncResult { if len(updated) > 0 { parts = append(parts, fmt.Sprintf("frissítve: %s", strings.Join(updated, ", "))) } - if len(parts) == 0 { + if len(parts) == 0 && catalogMoved { + // R-607: the catalog DID change; only the installed apps' own files did not (each keeps the + // version it runs until the household updates it). „nincs változás" was false here. + result.Message = "Sablonok frissítve — a katalógus új változata betöltve; a telepített alkalmazások fájljai nem változtak" + } else if len(parts) == 0 { result.Message = "Sablonok naprakészek — nincs változás" } else { result.Message = "Sablonok frissítve — " + strings.Join(parts, "; ") @@ -292,6 +311,26 @@ func (s *Syncer) gitCloneOrPull() error { // Remove stale git lock files left behind by interrupted operations s.removeGitLockFiles() + // R-615: the clone remembers the repository it was made from. A changed `git.repo_url` used to be + // INERT — every later fetch went to the stored origin and reported success. Compare and follow: + // a different repository (credentials aside) → drop the cache and clone the new one; the same + // repository with different credentials (a rotated token) → point origin at the new URL. + if cur, err := s.gitOutput(s.cacheDir, "config", "--get", "remote.origin.url"); err != nil { + s.logger.Printf("[WARN] [sync] cannot read the catalog cache's origin (%v) — fetching from it as before", err) + } else if want := s.buildRepoURL(); cur != want { + if stripURLCreds(cur) != stripURLCreds(want) { + s.logger.Printf("[WARN] [sync] git.repo_url changed (cache was cloned from %s, config says %s) — re-cloning the catalog cache from the configured repository (R-615)", maskRepoURL(cur), maskRepoURL(want)) + if err := os.RemoveAll(s.cacheDir); err != nil { + return fmt.Errorf("removing the catalog cache for a re-clone: %w", err) + } + return s.gitCloneOrPull() + } + s.logger.Printf("[INFO] [sync] catalog repository credentials changed — updating the cache's origin (R-615)") + if err := s.gitCmd(s.cacheDir, "remote", "set-url", "origin", want); err != nil { + return fmt.Errorf("git remote set-url: %w", err) + } + } + // Pull s.logger.Printf("[INFO] [sync] Pulling latest from %s (branch: %s)", s.cfg.Git.RepoURL, s.cfg.Git.Branch) if s.isDebug() { @@ -600,6 +639,44 @@ func copyIfChanged(src, dst string) (bool, error) { return true, nil } +// gitOutput runs one git command in dir and returns its trimmed stdout (R-615/R-607 reads). +func (s *Syncer) gitOutput(dir string, args ...string) (string, error) { + ctx, cancel := context.WithTimeout(context.Background(), gitCmdTimeout) + defer cancel() + cmd := exec.CommandContext(ctx, "git", args...) + cmd.Dir = dir + out, err := cmd.Output() + if err != nil { + return "", fmt.Errorf("git %s: %w", maskRepoURL(strings.Join(args, " ")), err) + } + return strings.TrimSpace(string(out)), nil +} + +// cacheHead is the catalog cache's current commit, "" when there is no clone yet or it cannot be read. +func (s *Syncer) cacheHead() string { + if _, err := os.Stat(filepath.Join(s.cacheDir, ".git")); err != nil { + return "" + } + h, err := s.gitOutput(s.cacheDir, "rev-parse", "HEAD") + if err != nil { + return "" + } + return h +} + +func shortHead(h string) string { + if h == "" { + return "(none)" + } + if len(h) > 12 { + return h[:12] + } + return h +} + +// stripURLCreds removes a `user:token@` part, so two URLs naming the same repository compare equal. +func stripURLCreds(u string) string { return reURLCreds.ReplaceAllString(u, "${1}") } + // runGit executes a git command with the given args under the standard per-command deadline. func (s *Syncer) runGit(args ...string) error { return s.gitCmd("", args...)