From ba87412508e09db9283ce01f6e6871f4eab90db0 Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Tue, 16 Jun 2026 12:56:51 +0200 Subject: [PATCH] controller: config-apply self-restart + manual restart button (A) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit POST /api/config/apply now takes effect via a graceful SELF-RESTART instead of logging "restart needed" and leaving stale in-process singletons (the CF client is built once at startup, so a rotated Cloudflare token never applied until a manual LXC restart). Container is restart:unless-stopped, so a clean os.Exit(0) auto-restarts with fresh config. - New gracefulSelfRestart helper behind an injectable Restarter seam (Router.restart + SetRestarter) so the exit is unit-testable. - configApply: no-op guard (byte-identical re-push → no write, no restart), else write → 200 (flushed) → restart. Removed stale "restart needed" wording. - Removed the dead OnConfigApplied hook (Phase-1-retired infra-backup push; the self-restart reloads everything and a fresh report is pushed on startup). - New POST /api/selfrestart (auth+CSRF via /api/ mount) + "Vezérlő újraindítása" settings button: confirm → POST → poll GET / every 2s → reload. - Tests: changed→restart once; identical→not called (companion); invalid→not called; selfrestart→restart once. Co-Authored-By: Claude Opus 4.8 (1M context) --- controller/cmd/controller/main.go | 6 -- controller/internal/api/router.go | 58 +++++++++-- controller/internal/api/selfrestart.go | 47 +++++++++ controller/internal/api/selfrestart_test.go | 99 +++++++++++++++++++ .../internal/web/templates/settings.html | 33 +++++++ 5 files changed, 228 insertions(+), 15 deletions(-) create mode 100644 controller/internal/api/selfrestart.go create mode 100644 controller/internal/api/selfrestart_test.go diff --git a/controller/cmd/controller/main.go b/controller/cmd/controller/main.go index 5af754a..8cb301b 100644 --- a/controller/cmd/controller/main.go +++ b/controller/cmd/controller/main.go @@ -573,12 +573,6 @@ func main() { // --- Initialize API router --- apiRouter := api.NewRouter(cfg, *configPath, sett, stackMgr, syncer, cpuCollector, backupMgr, metricsStore, updater, notifier, logger) - if hubPusher != nil { - apiRouter.OnConfigApplied = func() { - // Infra backup push is now the host agent's responsibility; the controller - // only refreshes the Hub report after a config apply. - } - } if assetsSyncer != nil { apiRouter.SetAssetsSyncer(assetsSyncer) } diff --git a/controller/internal/api/router.go b/controller/internal/api/router.go index e80b887..ca92c55 100644 --- a/controller/internal/api/router.go +++ b/controller/internal/api/router.go @@ -1,6 +1,7 @@ package api import ( + "bytes" "context" "encoding/json" "fmt" @@ -41,8 +42,14 @@ type Router struct { notifier *notify.Notifier logger *log.Logger - // OnConfigApplied is called after a successful config apply (e.g., to push infra backup). - OnConfigApplied func() + // restart triggers a graceful self-restart (config-apply + the manual restart button). + // Defaults to a real exit-after-flush so Docker's restart:unless-stopped brings the + // process back with fresh config; tests inject a recorder via SetRestarter. + restart func() + + // triggerReportPush fires an out-of-band, non-blocking hub report push (e.g. after a + // geo settings change so the hub reflects the new state immediately). Nil = no-op. + triggerReportPush func() // OnGeoRelevantChange is called after deploy/remove to re-sync geo rules. OnGeoRelevantChange func() @@ -86,7 +93,25 @@ func (r *Router) SetIntegrationManager(im *integrations.Manager) { } func NewRouter(cfg *config.Config, configPath string, sett *settings.Settings, stackMgr *stacks.Manager, syncer *catalogsync.Syncer, cpuCollector *system.CPUCollector, backupMgr *backup.Manager, metricsStore *metrics.MetricsStore, updater *selfupdate.Updater, notif *notify.Notifier, logger *log.Logger) *Router { - return &Router{cfg: cfg, configPath: configPath, sett: sett, stackMgr: stackMgr, syncer: syncer, cpuCollector: cpuCollector, backupMgr: backupMgr, metricsStore: metricsStore, updater: updater, notifier: notif, logger: logger} + r := &Router{cfg: cfg, configPath: configPath, sett: sett, stackMgr: stackMgr, syncer: syncer, cpuCollector: cpuCollector, backupMgr: backupMgr, metricsStore: metricsStore, updater: updater, notifier: notif, logger: logger} + r.restart = func() { gracefulSelfRestart(r.logger) } + return r +} + +// SetRestarter overrides the graceful-restart action. Tests inject a recorder so the +// process is not actually killed. +func (r *Router) SetRestarter(fn func()) { r.restart = fn } + +// SetReportPushTrigger wires the out-of-band hub report push used after geo changes. +// The provided func MUST be non-blocking (it is called from request handlers). +func (r *Router) SetReportPushTrigger(fn func()) { r.triggerReportPush = fn } + +// reportPushNow fires the report-push trigger if wired. Called after a state change the +// hub should reflect immediately (geo settings/sync) instead of waiting for the next cycle. +func (r *Router) reportPushNow() { + if r.triggerReportPush != nil { + r.triggerReportPush() + } } type apiResponse struct { @@ -146,6 +171,10 @@ func (r *Router) ServeHTTP(w http.ResponseWriter, req *http.Request) { case path == "/config" && req.Method == http.MethodGet: r.configContent(w, req) + // POST /api/selfrestart — customer-facing graceful self-restart (auth + CSRF via /api/ mount) + case path == "/selfrestart" && req.Method == http.MethodPost: + r.selfRestart(w, req) + // --- Integration routes (must be before hasSuffix-based stack cases) --- // GET /api/integrations/{provider} — list integrations for a provider @@ -1058,6 +1087,15 @@ func (r *Router) configApply(w http.ResponseWriter, req *http.Request) { return } + // No-op guard: if the pushed config is byte-identical to what is already on disk, do + // nothing — don't rewrite, don't restart. The hub may re-push the same config + // idempotently, and a self-restart on every push would be a needless flap. + if prior, rerr := os.ReadFile(r.configPath); rerr == nil && bytes.Equal(prior, body) { + r.logger.Printf("[INFO] [api] Config apply: identical to current config (%d bytes) — no change, no restart", len(body)) + writeJSON(w, http.StatusOK, apiResponse{OK: true, Message: "A konfiguráció változatlan — nincs szükség újraindításra."}) + return + } + // Write config 0600: it holds infra credentials (cf_api_token, cf_tunnel_token, hub api_key) in // plaintext (F8), so it must not be world-readable. writeConfig0600 enforces the mode even when the // target file already existed with looser perms (os.WriteFile does not chmod an existing file). @@ -1067,12 +1105,14 @@ func (r *Router) configApply(w http.ResponseWriter, req *http.Request) { return } - r.logger.Printf("[INFO] [api] Config applied from Hub (%d bytes), restart needed to take effect", len(body)) - writeJSON(w, http.StatusOK, apiResponse{OK: true, Message: "Config applied. Restart controller to apply changes."}) - - // Push updated infra backup so Hub has fresh config data immediately - if r.OnConfigApplied != nil { - go r.OnConfigApplied() + // Respond to the hub FIRST (and flush), THEN self-restart. The new config only takes + // effect on restart — singletons such as the Cloudflare client are built once at + // startup, so an in-process write alone would leave e.g. a rotated CF token unused. + r.logger.Printf("[INFO] [api] Config applied from Hub (%d bytes) — self-restarting to take effect", len(body)) + writeJSON(w, http.StatusOK, apiResponse{OK: true, Message: "Konfiguráció alkalmazva — a vezérlő újraindul."}) + flushResponse(w) + if r.restart != nil { + r.restart() } } diff --git a/controller/internal/api/selfrestart.go b/controller/internal/api/selfrestart.go new file mode 100644 index 0000000..d69d8fe --- /dev/null +++ b/controller/internal/api/selfrestart.go @@ -0,0 +1,47 @@ +package api + +import ( + "log" + "net/http" + "os" + "time" +) + +// restartDelay gives the in-flight HTTP response time to flush before the process exits. +const restartDelay = 500 * time.Millisecond + +// gracefulSelfRestart schedules a clean process exit after a short delay. The container +// runs with `restart: unless-stopped`, so exiting 0 makes Docker start a fresh process +// that re-reads controller.yaml. This is how a config-apply (e.g. a rotated Cloudflare +// API token) and the manual restart button actually take effect: singletons such as the +// Cloudflare client are built once at startup and are not reloaded in-process. +func gracefulSelfRestart(logger *log.Logger) { + go func() { + time.Sleep(restartDelay) + if logger != nil { + logger.Println("[INFO] [api] Graceful self-restart: exiting (0) for container restart") + } + os.Exit(0) + }() +} + +// flushResponse flushes buffered output to the client if the writer supports it, so the +// caller receives the response body before a subsequent self-restart kills the process. +func flushResponse(w http.ResponseWriter) { + if f, ok := w.(http.Flusher); ok { + f.Flush() + } +} + +// selfRestart handles POST /api/selfrestart — a customer-facing self-serve restart so a +// user can recover the controller without rebooting the whole guest. Auth + CSRF are +// applied by the /api/ mux mount (same protection as every other state-changing endpoint). +// Responds first, flushes, then triggers the graceful restart. +func (r *Router) selfRestart(w http.ResponseWriter, _ *http.Request) { + r.logger.Println("[INFO] [api] Manual controller restart requested") + writeJSON(w, http.StatusOK, apiResponse{OK: true, Message: "Újraindítás folyamatban…"}) + flushResponse(w) + if r.restart != nil { + r.restart() + } +} diff --git a/controller/internal/api/selfrestart_test.go b/controller/internal/api/selfrestart_test.go new file mode 100644 index 0000000..51f00fb --- /dev/null +++ b/controller/internal/api/selfrestart_test.go @@ -0,0 +1,99 @@ +package api + +import ( + "bytes" + "io" + "log" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "testing" +) + +// newRestartTestRouter builds a minimal Router with a recording restart seam (so the +// process is never actually killed) and a config path seeded with `prior`. +func newRestartTestRouter(t *testing.T, prior []byte) (*Router, *int, string) { + t.Helper() + dir := t.TempDir() + path := filepath.Join(dir, "controller.yaml") + if prior != nil { + if err := os.WriteFile(path, prior, 0o600); err != nil { + t.Fatal(err) + } + } + calls := 0 + r := &Router{configPath: path, logger: log.New(io.Discard, "", 0)} + r.restart = func() { calls++ } + return r, &calls, path +} + +// a minimal config that passes config.LoadFromBytes (customer.id + customer.domain required). +var validConfig = []byte("customer:\n id: test\n domain: test.example\n") + +func TestConfigApply_ChangedConfig_Restarts(t *testing.T) { + prior := []byte("customer:\n id: old\n domain: old.example\n") + r, calls, path := newRestartTestRouter(t, prior) + + req := httptest.NewRequest(http.MethodPost, "/api/config/apply", bytes.NewReader(validConfig)) + rec := httptest.NewRecorder() + r.configApply(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200; body=%s", rec.Code, rec.Body.String()) + } + if *calls != 1 { + t.Fatalf("restart called %d times, want 1 (config changed)", *calls) + } + got, _ := os.ReadFile(path) + if !bytes.Equal(got, validConfig) { + t.Fatalf("config not written: got %q", got) + } +} + +// COMPANION: an identical re-push must NOT restart (the old handler always restarted / +// always ran its post-apply hook regardless of whether anything changed). +func TestConfigApply_IdenticalConfig_NoRestart(t *testing.T) { + r, calls, _ := newRestartTestRouter(t, validConfig) // prior == body + + req := httptest.NewRequest(http.MethodPost, "/api/config/apply", bytes.NewReader(validConfig)) + rec := httptest.NewRecorder() + r.configApply(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200; body=%s", rec.Code, rec.Body.String()) + } + if *calls != 0 { + t.Fatalf("restart called %d times, want 0 (config byte-identical)", *calls) + } +} + +func TestConfigApply_InvalidYAML_NoRestart(t *testing.T) { + r, calls, _ := newRestartTestRouter(t, validConfig) + + req := httptest.NewRequest(http.MethodPost, "/api/config/apply", bytes.NewReader([]byte("customer:\n id: only-id-no-domain\n"))) + rec := httptest.NewRecorder() + r.configApply(rec, req) + + if rec.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400 (missing customer.domain)", rec.Code) + } + if *calls != 0 { + t.Fatalf("restart called %d times, want 0 (validation failed)", *calls) + } +} + +func TestSelfRestart_InvokesRestarter(t *testing.T) { + r, calls, _ := newRestartTestRouter(t, nil) + + req := httptest.NewRequest(http.MethodPost, "/api/selfrestart", nil) + rec := httptest.NewRecorder() + r.selfRestart(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200", rec.Code) + } + if *calls != 1 { + t.Fatalf("restart called %d times, want 1", *calls) + } +} diff --git a/controller/internal/web/templates/settings.html b/controller/internal/web/templates/settings.html index 6e4a900..c50ed75 100644 --- a/controller/internal/web/templates/settings.html +++ b/controller/internal/web/templates/settings.html @@ -1112,9 +1112,42 @@ window.__registeredPaths=[{{range .StoragePaths}}{{if .Path}}"{{.Path}}",{{end}} + + +
+

Vezérlő újraindítása

+

+ Ha a vezérlő hibásan működik, itt biztonságosan újraindíthatja — nem kell az egész szervert újraindítani. + Az alkalmazásai futnak tovább; csak a vezérlő indul újra (néhány másodperc). +

+
+ +
{{end}}