Traefik v3.7.17 (GHSA-xv5x-242w-7hf7); R-1347 a restore forgets the update-held flag; R-1348 a failed put-back at start-up mails; R-1339, R-1351, R-1323 truthful start-up lines and one lock over FileBrowser's compose. Commit only.

2026-11-02 23:14:16 +01:00
1 parent a90b62323f
commit c4b6844d34
@@ -109,7 +109,7 @@ func (m *Manager) ensureFileBrowserPublic() {
 	}
 	if m.fbNetOn(target, traefikNetwork) {
 		if err := m.fbNetDisconnect(target, traefikNetwork); err != nil {
-			m.logger.Printf("[WARN] [infra] filebrowser: could not leave %s — every app can still reach it (R-1281): %v", traefikNetwork, err)
+			m.reportFileBrowserNotLeft(target, err)
 		} else {
 			m.logger.Printf("[INFO] [infra] filebrowser: left %s — no app container reaches it any more (R-1281)", traefikNetwork)
 		}
@@ -117,6 +117,27 @@ func (m *Manager) ensureFileBrowserPublic() {
 	m.ensureOfficeOnFilesNetwork()
 }

+// reportFileBrowserNotLeft says what a failed leave means (R-1323). „No such container" means the CHECKED container was
+// replaced between the check and the leave (the image moved at start, a mounts sync): the line then reads the CURRENT
+// container's networks and says the truth. A false „every app can still reach it" teaches the reader to ignore the real
+// one, which stays: any other failure, a current container on the app network, or one that cannot be read. The new
+// container is checked and published by the next round. Pinned by TestR1323_*.
+func (m *Manager) reportFileBrowserNotLeft(target string, err error) {
+	if !strings.Contains(err.Error(), "No such container") {
+		m.logger.Printf("[WARN] [infra] filebrowser: could not leave %s — every app can still reach it (R-1281): %v", traefikNetwork, err)
+		return
+	}
+	cur := strings.Fields(m.fbStartedAt())
+	switch {
+	case len(cur) == 0:
+		m.logger.Printf("[WARN] [infra] filebrowser: the checked container %s is gone and the current one cannot be read — whether every app can reach it is not known (R-1281, R-1323)", target)
+	case m.fbNetOn(cur[0], traefikNetwork):
+		m.logger.Printf("[WARN] [infra] filebrowser: could not leave %s — every app can still reach it (R-1281): the checked container %s was replaced, and the current one %s IS on it", traefikNetwork, target, cur[0])
+	default:
+		m.logger.Printf("[INFO] [infra] filebrowser: the checked container %s was replaced before it left %s; the current one %s is not on %s (R-1323)", target, traefikNetwork, cur[0], traefikNetwork)
+	}
+}
+
 // traefikContainer is traefik's container (infra/templates/traefik-compose.yml.tmpl container_name).
 const traefikContainer = "traefik"

@@ -254,8 +254,28 @@ func changedInfraFiles(dir string, files map[string]infra.FileSpec) []string {
 	return changed
 }

+// LockFileBrowserStack takes the lock over every `docker compose` on FileBrowser's stack at a boot (R-1351): this file's
+// ensureFileBrowser and web.syncFileBrowserMounts. Returns the unlock. Lock order: infraMu (the base stack) before this;
+// the web sync takes only this one, and starts the gate (infraMu) in a goroutine. Pinned by TestR1351_* and
+// TestR1351_TheWebSyncTakesTheLockBeforeCompose (internal/web).
+func (m *Manager) LockFileBrowserStack() (unlock func()) {
+	m.fbStackMu.Lock()
+	return m.fbStackMu.Unlock
+}
+
+// WithFileBrowserStack runs fn under LockFileBrowserStack.
+func (m *Manager) WithFileBrowserStack(fn func()) {
+	defer m.LockFileBrowserStack()()
+	fn()
+}
+
 func (m *Manager) ensureFileBrowser(dir string) error {
-	if containerRunning("filebrowser") {
+	defer m.LockFileBrowserStack()()
+	running := containerRunning
+	if m.fbRunningFn != nil {
+		running = func(string) bool { return m.fbRunningFn() }
+	}
+	if running("filebrowser") {
 		return m.reconcileFileBrowserImage(dir)
 	}
 	composePath := filepath.Join(dir, "docker-compose.yml")
@@ -263,6 +283,9 @@ func (m *Manager) ensureFileBrowser(dir string) error {
 		// Already provisioned but not running — bring it up WITHOUT regenerating, so the storage
 		// mounts that web.SyncFileBrowserMounts manages are preserved. Just `compose up -d`.
 		m.logger.Printf("[INFO] [infra] filebrowser compose exists — starting without regenerating")
+		if m.fbComposeUpFn != nil {
+			return m.fbComposeUpFn(dir)
+		}
 		return m.composeUp(dir)
 	}
 	m.logger.Printf("[INFO] [infra] deploying filebrowser → %s", dir)
@@ -294,7 +294,12 @@ type Manager struct {
 	fbStarted   func() string
 	fbProbedRun string
 	// fbNetOnFn / fbNetConnectFn replace the docker network reads and connects of the R-1126 gate in tests.
-	fbNetOnFn      func(container, network string) bool
+	fbNetOnFn func(container, network string) bool
+	// fbStackMu (R-1351) is the lock over `docker compose` on FileBrowser's stack: LockFileBrowserStack. fbRunningFn and
+	// fbComposeUpFn replace the docker read and the bring-up of ensureFileBrowser in tests.
+	fbStackMu      sync.Mutex
+	fbRunningFn    func() bool
+	fbComposeUpFn  func(dir string) error
 	fbNetConnectFn func(container, network string) error
 	// R-1281: the disconnect, the network create, and „is OnlyOffice linked to FileBrowser and running" — test seams.
 	fbNetDisconnectFn func(container, network string) error
@@ -0,0 +1,78 @@
+package stacks
+
+import (
+	"bytes"
+	"errors"
+	"log"
+	"net/http/httptest"
+	"strings"
+	"testing"
+)
+
+// R-1323: the checked FileBrowser container can be replaced between the check and the leave (the image moved at start).
+// „No such container" then means the OLD one is gone, not that every app still reaches FileBrowser. The line reads the
+// CURRENT container's networks and says what is true; a current container that IS on the app network keeps the alarm.
+//
+// RED-PROOF (2026-11-02): before the fix the first case logged „could not leave traefik-public — every app can still reach
+// it … No such container" (seen on 9202, 0.346.1-rc.1, 2026-10-27), and this fails on that line.
+func r1323Rig(t *testing.T, currentOnApps bool) (*Manager, *bytes.Buffer) {
+	t.Helper()
+	fb := &fakeFB{password: "admin"}
+	srv := httptest.NewServer(fb.handler())
+	t.Cleanup(srv.Close)
+	m, _, _ := fbManager(t, srv.URL)
+	var buf bytes.Buffer
+	m.logger = log.New(&buf, "", 0)
+	runs := []string{"old111 2026-10-27T14:58:15Z"}
+	m.fbStarted = func() string {
+		r := runs[0]
+		if len(runs) > 1 {
+			runs = runs[1:]
+		}
+		return r
+	}
+	nets := map[string]bool{"old111@traefik-public": true, "new222@traefik-public": currentOnApps}
+	m.fbNetOnFn = func(c, n string) bool { return nets[c+"@"+n] }
+	m.fbNetConnectFn = func(c, n string) error { nets[c+"@"+n] = true; return nil }
+	m.fbNetDisconnectFn = func(c, n string) error {
+		if c == "old111" {
+			runs = []string{"new222 2026-10-27T14:58:17Z"} // the recreate won the race
+			return errors.New("Error response from daemon: No such container: old111")
+		}
+		return nil
+	}
+	m.fbNetEnsureFn = func(string) error { return nil }
+	m.fbOfficeLinkedFn = func() (bool, bool) { return false, false }
+	return m, &buf
+}
+
+func TestR1323_AReplacedContainerIsNotReportedAsExposed(t *testing.T) {
+	m, buf := r1323Rig(t, false)
+	m.fileBrowserGateLocked()
+	got := buf.String()
+	if strings.Contains(got, "every app can still reach it") {
+		t.Fatalf("a false alarm for a container that is gone: %q", got)
+	}
+	if !strings.Contains(got, "new222") || !strings.Contains(got, "not on traefik-public") {
+		t.Fatalf("the line must name the current container and the truth: %q", got)
+	}
+}
+
+func TestR1323_ACurrentContainerOnTheAppNetworkKeepsTheAlarm(t *testing.T) {
+	m, buf := r1323Rig(t, true)
+	m.fileBrowserGateLocked()
+	got := buf.String()
+	if !strings.Contains(got, "every app can still reach it") || !strings.Contains(got, "new222") {
+		t.Fatalf("the replacement IS on the app network: the alarm stays, naming it: %q", got)
+	}
+}
+
+// Any other failure to leave keeps the alarm as before (control).
+func TestR1323_AnotherDisconnectFailureKeepsTheAlarm(t *testing.T) {
+	m, buf := r1323Rig(t, false)
+	m.fbNetDisconnectFn = func(c, n string) error { return errors.New("daemon busy") }
+	m.fileBrowserGateLocked()
+	if got := buf.String(); !strings.Contains(got, "every app can still reach it") || !strings.Contains(got, "daemon busy") {
+		t.Fatalf("a real failure to leave must keep its alarm: %q", got)
+	}
+}
@@ -0,0 +1,40 @@
+package stacks
+
+import (
+	"fmt"
+	"io"
+	"log"
+	"testing"
+	"time"
+)
+
+// R-1347, the consequence: an app whose last update ended held is not watched for a crash loop; once its hold is lifted by
+// a restore and the flag forgotten (backup.clearUpdateHoldAfterRestore → ForgetUpdateHeld) the same restarts trip the
+// watcher again.
+func TestR1347_TheWatcherWatchesAgainOnceTheFlagIsForgotten(t *testing.T) {
+	m := &Manager{logger: log.New(io.Discard, "", 0), stacks: map[string]*Stack{
+		"nextcloud": {Name: "nextcloud", Deployed: true, Containers: []ContainerInfo{{Name: "nextcloud-web-1"}}},
+	}}
+	m.markUpdateHeld("nextcloud")
+	count := int64(0)
+	m.execFn = func(string, ...string) (string, error) { return fmt.Sprintf("/nextcloud-web-1|%d\n", count), nil }
+	t0 := time.Date(2026, 11, 2, 4, 20, 0, 0, time.UTC)
+	sample := func(from int) []UnhealthyVerdict {
+		var got []UnhealthyVerdict
+		for i := from; i < from+12 && len(got) == 0; i++ {
+			count += 3
+			got = m.ObserveUnhealthy(t0.Add(time.Duration(i)*time.Minute), nil)
+		}
+		return got
+	}
+	if got := sample(0); len(got) != 0 {
+		t.Fatalf("control: an app whose update ended held must not be judged: %+v", got)
+	}
+	if !m.ForgetUpdateHeld("nextcloud") {
+		t.Fatal("the flag was set, so it must be forgotten")
+	}
+	got := sample(20)
+	if len(got) != 1 || got[0].Stack != "nextcloud" || got[0].Kind != UnhealthyCrashLoop {
+		t.Fatalf("after the restore lifted the hold the watcher must watch the app again; verdicts %+v", got)
+	}
+}
@@ -0,0 +1,58 @@
+package stacks
+
+import (
+	"io"
+	"log"
+	"os"
+	"path/filepath"
+	"sync"
+	"testing"
+	"time"
+)
+
+// R-1351: at a guest boot EnsureBaseStack's `compose up` and web.SyncFileBrowserMounts' `compose up --force-recreate` ran
+// in the same second; one lost with „Conflict. The container name … is already in use" and a false first-boot WARN. The
+// two paths now share one lock: the base stack's bring-up holds it around its FileBrowser step, the web sync around its
+// compose call (TestFileBrowserSyncRunsInsideTheStackLock pins that side, internal/web).
+//
+// RED-PROOF (2026-11-02): seen failing before the lock existed (build: WithFileBrowserStack); with ensureFileBrowser not
+// taking the lock it fails on „overlap".
+func TestR1351_TheTwoFileBrowserStartPathsNeverOverlap(t *testing.T) {
+	dir := t.TempDir()
+	if err := os.WriteFile(filepath.Join(dir, "docker-compose.yml"), []byte("services: {}\n"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	m := &Manager{logger: log.New(io.Discard, "", 0)}
+	var mu sync.Mutex
+	inside, overlaps := 0, 0
+	enter := func() {
+		mu.Lock()
+		inside++
+		if inside > 1 {
+			overlaps++
+		}
+		mu.Unlock()
+		time.Sleep(30 * time.Millisecond)
+		mu.Lock()
+		inside--
+		mu.Unlock()
+	}
+	m.fbComposeUpFn = func(string) error { enter(); return nil }
+	m.fbRunningFn = func() bool { return false }
+
+	var wg sync.WaitGroup
+	for i := 0; i < 4; i++ {
+		wg.Add(2)
+		go func() {
+			defer wg.Done()
+			if err := m.ensureFileBrowser(dir); err != nil {
+				t.Error(err)
+			}
+		}()
+		go func() { defer wg.Done(); m.WithFileBrowserStack(enter) }()
+	}
+	wg.Wait()
+	if overlaps != 0 {
+		t.Fatalf("overlap: the two start paths ran compose at the same time %d time(s) — the „Conflict” of R-1351", overlaps)
+	}
+}
@@ -214,7 +214,7 @@ func (m *Manager) ClearUpdateState(name string) {
 // (ObserveUnhealthy) and the nightly update leg (legCandidate) skip the app, and keeps the hold's sentence as the
 // update error. Left set, an app whose hold was cleared would stay unobserved and never be updated by night until
 // the controller restarts. A running update is not touched. Returns whether anything was forgotten.
-// Pinned by TestD64_ForgetUpdateHeld (this package) and TestD64_ClearHoldForgetsTheUpdateHeldFlag (cmd/controller).
+// Pinned by TestD64_ForgetUpdateHeld (this package) and TestD64_ClearHoldViaTheHub (cmd/controller).
 func (m *Manager) ForgetUpdateHeld(name string) bool {
 	m.mu.Lock()
 	defer m.mu.Unlock()