From e8bf10deab2e1eaec01566a59ebabaff1527596d Mon Sep 17 00:00:00 2001 From: darken Date: Tue, 2 Jun 2026 13:26:38 +0200 Subject: [PATCH] feat(server): nudge config-sync after a successful config save A saved feed.env change (e.g. enabling remote configuration) now reaches the server within seconds instead of waiting for the ~60s config-sync timer tick. After an applied write webconfig starts airplanes-config-sync.service --no-block; the unit self-gates. --- files/etc/sudoers.d/010_airplanes-webconfig | 5 + internal/devfakes/runners.go | 3 + internal/server/handlers.go | 77 ++++++++----- internal/server/server.go | 8 ++ internal/server/server_test.go | 118 ++++++++++++++++++++ internal/server/sudoers_parity.go | 1 + 6 files changed, 187 insertions(+), 25 deletions(-) diff --git a/files/etc/sudoers.d/010_airplanes-webconfig b/files/etc/sudoers.d/010_airplanes-webconfig index 4729b2f..8b24221 100644 --- a/files/etc/sudoers.d/010_airplanes-webconfig +++ b/files/etc/sudoers.d/010_airplanes-webconfig @@ -34,6 +34,11 @@ airplanes-webconfig ALL=(root) NOPASSWD: /usr/local/bin/apl-wifi status --json airplanes-webconfig ALL=(root) NOPASSWD: /usr/bin/systemctl reboot airplanes-webconfig ALL=(root) NOPASSWD: /usr/bin/systemctl poweroff airplanes-webconfig ALL=(root) NOPASSWD: /usr/bin/systemctl start --no-block airplanes-claim.service +# Nudge the remote-config syncer after a config save so a change (e.g. enabling +# remote configuration) reaches airplanes.live in seconds instead of waiting for +# the ~60s timer tick. The unit self-gates (ConditionPathExists=feeder-claim-secret +# and the REMOTE_CONFIG_ENABLED opt-in inside apl-feed), so a re-trigger is a no-op. +airplanes-webconfig ALL=(root) NOPASSWD: /usr/bin/systemctl start --no-block airplanes-config-sync.service # Unified update orchestrator (apt → runtime overlay). # The trampoline at /usr/local/lib/airplanes-webconfig/start-orchestrator.sh diff --git a/internal/devfakes/runners.go b/internal/devfakes/runners.go index 9822ccc..e82c7ac 100644 --- a/internal/devfakes/runners.go +++ b/internal/devfakes/runners.go @@ -73,6 +73,7 @@ func StubPrivilegedArgv() server.PrivilegedArgv { Poweroff: []string{"dev-stub", "systemctl", "poweroff"}, StartOrchestrator: []string{"dev-stub", "systemd-run", "airplanes-update-orchestrator"}, RegisterClaim: []string{"dev-stub", "systemctl", "claim-register"}, + SyncConfig: []string{"dev-stub", "systemctl", "config-sync"}, WifiList: []string{"dev-stub", "apl-wifi", "list"}, WifiAdd: []string{"dev-stub", "apl-wifi", "add"}, WifiUpdate: []string{"dev-stub", "apl-wifi", "update"}, @@ -213,6 +214,8 @@ func dispatchStub(state *State, priv server.PrivilegedArgv, argv []string, body if err := state.RegisterClaim(); err != nil { log.Printf("devfakes: RegisterClaim: %v", err) } + case "config-sync": + log.Printf("devfakes: would nudge config-sync") } return wexec.Result{}, nil case "systemd-run": diff --git a/internal/server/handlers.go b/internal/server/handlers.go index 8911fb4..c849094 100644 --- a/internal/server/handlers.go +++ b/internal/server/handlers.go @@ -590,31 +590,7 @@ func (s *Server) handleConfigPost(w http.ResponseWriter, r *http.Request) { } } - s.configMu.Lock() - defer s.configMu.Unlock() - - // Read current feed.env under the same configMu so the "did this - // tracked key actually change" determination is consistent against - // concurrent webconfig writers. A concurrent `apl-feed config sync` - // write is handled by the apply library's flock + LWW gate — the - // race is benign (worst case: one wasted no-op apply round trip). - // - // A read error is not fatal: fall back to a bare-string payload — - // every posted key passes through with no metadata, which matches - // the pre-DEV-383 behavior. We must NOT treat the read failure as - // bootstrap; that would attach metadata to every tracked key the - // form posts, including unchanged ones, and stamp fresh edited_at - // tuples across the sidecar on every save. - current, readErr := s.feedEnv.ReadAll() - var payload map[string]any - if readErr != nil { - log.Printf("config-post: feed.env pre-read for metadata gating failed; falling back to bare-string payload: %v", readErr) - payload = feedmeta.BareStringPayload(req.Updates) - } else { - payload = feedmeta.BuildApplyPayload(current, req.Updates, s.now()) - } - - resp, status, err := s.invokeApplyFeed(r.Context(), payload) + resp, status, err := s.applyConfigLocked(r.Context(), req.Updates) if err != nil { log.Printf("config-post: %v", err) writeJSONError(w, http.StatusInternalServerError, "config write failed") @@ -629,9 +605,60 @@ func (s *Server) handleConfigPost(w http.ResponseWriter, r *http.Request) { writeJSON(w, status, resp) return } + // A real write landed — nudge the remote-config syncer so the change + // reaches airplanes.live on the next sync (seconds) rather than waiting + // for the ~60s config-sync timer tick. Fire-and-forget and outside + // configMu: the unit self-gates, so this never blocks or alters the save + // response. no_change writes skip it (nothing to propagate). + if resp.Status == "applied" { + s.triggerConfigSyncAsync() + } writeJSON(w, http.StatusOK, resp) } +// applyConfigLocked serialises the feed.env metadata-gating read and the apply +// under configMu so both see a consistent view against concurrent webconfig +// writers, then releases the lock. The caller writes the HTTP response and +// fires any follow-up (e.g. the config-sync nudge) outside the lock. +func (s *Server) applyConfigLocked(ctx context.Context, updates map[string]string) (applyFeedResponse, int, error) { + s.configMu.Lock() + defer s.configMu.Unlock() + + // A concurrent `apl-feed config sync` write is handled by the apply + // library's flock + LWW gate — the race is benign (worst case: one + // wasted no-op apply round trip). + // + // A read error is not fatal: fall back to a bare-string payload — every + // posted key passes through with no metadata, which matches the + // pre-DEV-383 behavior. We must NOT treat the read failure as bootstrap; + // that would attach metadata to every tracked key the form posts, + // including unchanged ones, and stamp fresh edited_at tuples across the + // sidecar on every save. + current, readErr := s.feedEnv.ReadAll() + var payload map[string]any + if readErr != nil { + log.Printf("config-post: feed.env pre-read for metadata gating failed; falling back to bare-string payload: %v", readErr) + payload = feedmeta.BareStringPayload(updates) + } else { + payload = feedmeta.BuildApplyPayload(current, updates, s.now()) + } + return s.invokeApplyFeed(ctx, payload) +} + +// triggerConfigSyncAsync starts airplanes-config-sync.service in the background +// so a saved config change is pushed promptly. Best-effort: failures are +// logged, never surfaced on the save response, and the ~60s config-sync timer +// stays the backstop. The unit's own gates (ConditionPathExists on the claim +// secret + the REMOTE_CONFIG_ENABLED opt-in inside apl-feed) decide whether the +// run actually contacts the server, so this carries no remote-config logic. +func (s *Server) triggerConfigSyncAsync() { + go func() { + if err := s.runSudo(context.Background(), s.priv.SyncConfig, systemctlTimeout); err != nil { + log.Printf("config-sync: %v", err) + } + }() +} + // applyFeedResponse mirrors the JSON envelope emitted by // `apl-feed apply --json`. Any subset of fields can be populated // depending on status; the client renders them in priority order diff --git a/internal/server/server.go b/internal/server/server.go index fc0d9b3..852605f 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -82,6 +82,7 @@ type PrivilegedArgv struct { Poweroff []string // sudo -n /usr/bin/systemctl poweroff StartOrchestrator []string // sudo systemd-run --unit=airplanes-update-orchestrator ... RegisterClaim []string // sudo systemctl start --no-block airplanes-claim.service + SyncConfig []string // sudo systemctl start --no-block airplanes-config-sync.service WifiList []string WifiAdd []string WifiUpdate []string @@ -132,6 +133,13 @@ func DefaultPrivilegedArgv() PrivilegedArgv { // immediately. Progress and failures show up in the claim activity // log via the SSE stream the SPA opens after this returns. RegisterClaim: sudo("/usr/bin/systemctl", "start", "--no-block", "airplanes-claim.service"), + // Fired after a successful config save so a change reaches the server + // on the next sync instead of the ~60s timer tick. --no-block: the + // unit is Type=oneshot and `apl-feed config sync` does a network + // round-trip; enqueue and return rather than risk systemctlTimeout. + // The unit self-gates (claim secret + REMOTE_CONFIG_ENABLED opt-in), + // so a redundant trigger is a no-op. + SyncConfig: sudo("/usr/bin/systemctl", "start", "--no-block", "airplanes-config-sync.service"), WifiList: sudo("/usr/local/bin/apl-wifi", "list", "--json"), WifiAdd: sudo("/usr/local/bin/apl-wifi", "add", "--json"), WifiUpdate: sudo("/usr/local/bin/apl-wifi", "update", "--json"), diff --git a/internal/server/server_test.go b/internal/server/server_test.go index 5fc4cef..26d8274 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -106,6 +106,7 @@ func newTestServer(t *testing.T) (*httptest.Server, *Server) { Poweroff: []string{"sudo-stub", "poweroff"}, StartOrchestrator: []string{"sudo-stub", "orchestrator"}, RegisterClaim: []string{"sudo-stub", "systemctl", "start", "--no-block", "airplanes-claim.service"}, + SyncConfig: []string{"sudo-stub", "systemctl", "start", "--no-block", "airplanes-config-sync.service"}, WifiList: []string{"sudo-stub", "apl-wifi", "list", "--json"}, WifiAdd: []string{"sudo-stub", "apl-wifi", "add", "--json"}, WifiUpdate: []string{"sudo-stub", "apl-wifi", "update", "--json"}, @@ -270,6 +271,7 @@ func newWriteHarness(t *testing.T, opts ...harnessOption) *writeHarness { Poweroff: []string{"sudo-stub", "poweroff"}, StartOrchestrator: []string{"sudo-stub", "orchestrator"}, RegisterClaim: []string{"sudo-stub", "systemctl", "start", "--no-block", "airplanes-claim.service"}, + SyncConfig: []string{"sudo-stub", "systemctl", "start", "--no-block", "airplanes-config-sync.service"}, WifiList: []string{"sudo-stub", "apl-wifi", "list", "--json"}, WifiAdd: []string{"sudo-stub", "apl-wifi", "add", "--json"}, WifiUpdate: []string{"sudo-stub", "apl-wifi", "update", "--json"}, @@ -1580,6 +1582,7 @@ func TestConfigPost_AltitudeFtRoundTripsAsBareMetres(t *testing.T) { Privileged: PrivilegedArgv{ ApplyFeed: []string{"sudo-stub", "apl-feed", "apply", "--json", "--lock-timeout", "5"}, SchemaFeed: []string{"apl-feed", "schema", "--json"}, + SyncConfig: []string{"sudo-stub", "systemctl", "start", "--no-block", "airplanes-config-sync.service"}, }, } ts := httptest.NewServer(New(deps)) @@ -1635,3 +1638,118 @@ func extractValueFromApplyPayload(raw json.RawMessage) (string, bool) { } return "", false } + +// TestConfigPost_NudgesConfigSyncOnApplied asserts the post-save config-sync +// nudge fires exactly when apl-feed reports a real write ("applied") and stays +// silent on "no_change". The nudge is async, so the positive case waits on a +// channel and the negative case asserts silence over a short window. +func TestConfigPost_NudgesConfigSyncOnApplied(t *testing.T) { + wantSync := []string{"sudo-stub", "systemctl", "start", "--no-block", "airplanes-config-sync.service"} + + cases := []struct { + name string + applyStatus string + expectNudge bool + }{ + {"applied fires the nudge", "applied", true}, + {"no_change stays silent", "no_change", false}, + } + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + dir := t.TempDir() + hashPath := filepath.Join(dir, "password.hash") + guard, err := auth.NewHashGuard(2) + if err != nil { + t.Fatal(err) + } + feedEnvPath := filepath.Join(dir, "feed.env") + if err := os.WriteFile(feedEnvPath, []byte(`REMOTE_CONFIG_ENABLED="false"`+"\n"), 0o644); err != nil { + t.Fatal(err) + } + + // Apply runner returns the table-driven status without touching disk. + stdinRunner := func(_ context.Context, _ []string, stdin io.Reader) (wexec.Result, error) { + io.Copy(io.Discard, stdin) + return wexec.Result{Stdout: []byte(`{"status":"` + tc.applyStatus + `"}`)}, nil + } + + // Capturing runner signals when the nudge argv fires. + syncCh := make(chan struct{}, 1) + runner := func(_ context.Context, argv []string) (wexec.Result, error) { + if reflect.DeepEqual(argv, wantSync) { + select { + case syncCh <- struct{}{}: + default: + } + } + return wexec.Result{}, nil + } + + deps := Deps{ + Version: "test-sha", + Store: auth.NewPasswordStore(hashPath), + Sessions: auth.NewSessions(time.Hour), + Lockout: auth.NewLockout(5, time.Minute, 15*time.Minute), + Guard: guard, + Argon2Params: fastTestParams, + Identity: identity.NewReader(identity.Paths{FeederIDFile: filepath.Join(dir, "feeder-id")}), + FeedEnv: &feedenv.Reader{Path: feedEnvPath}, + Status: status.NewReader("test-sha", status.Paths{ + SystemctlBinary: "/bin/true", IsActiveTimeout: time.Second, + }, func(_ context.Context, _ []string) (wexec.Result, error) { return wexec.Result{}, nil }), + Logs: logs.NewStreamer(nil), + Schema: schemacache.NewPrepopulated([]string{"REMOTE_CONFIG_ENABLED"}, []string{"REMOTE_CONFIG_ENABLED"}), + Runner: runner, + StdinRunner: stdinRunner, + Privileged: PrivilegedArgv{ + ApplyFeed: []string{"sudo-stub", "apl-feed", "apply", "--json", "--lock-timeout", "5"}, + SchemaFeed: []string{"apl-feed", "schema", "--json"}, + SyncConfig: wantSync, + }, + } + ts := httptest.NewServer(New(deps)) + defer ts.Close() + + c := httpClient(t) + r := postJSON(t, c, ts.URL+"/api/setup", map[string]string{"password": testPassword}) + r.Body.Close() + if r.StatusCode != http.StatusOK { + t.Fatalf("setup status = %d", r.StatusCode) + } + + r = postJSON(t, c, ts.URL+"/api/config", + map[string]any{"updates": map[string]string{"REMOTE_CONFIG_ENABLED": "true"}}) + r.Body.Close() + if r.StatusCode != http.StatusOK { + t.Fatalf("POST /api/config status=%d", r.StatusCode) + } + + if tc.expectNudge { + select { + case <-syncCh: + case <-time.After(2 * time.Second): + t.Fatal("expected config-sync nudge after applied save, got none") + } + } else { + select { + case <-syncCh: + t.Fatal("config-sync nudge fired on no_change save") + case <-time.After(500 * time.Millisecond): + } + } + }) + } +} + +// TestDefaultPrivilegedArgv_SyncConfigPinned pins the exact argv so renaming the +// unit on either the Go or sudoers side trips a test rather than silently +// shipping a no-op nudge. Sudoers parity only proves the two sides agree with +// each other; this proves they agree with the intended unit name. +func TestDefaultPrivilegedArgv_SyncConfigPinned(t *testing.T) { + got := DefaultPrivilegedArgv().SyncConfig + want := []string{"/usr/bin/sudo", "-n", "/usr/bin/systemctl", "start", "--no-block", "airplanes-config-sync.service"} + if !reflect.DeepEqual(got, want) { + t.Fatalf("SyncConfig argv = %v, want %v", got, want) + } +} diff --git a/internal/server/sudoers_parity.go b/internal/server/sudoers_parity.go index 0dc7c20..49ccfd8 100644 --- a/internal/server/sudoers_parity.go +++ b/internal/server/sudoers_parity.go @@ -63,6 +63,7 @@ func privilegedArgvCases(priv PrivilegedArgv) []privilegedArgvCase { {"Poweroff", priv.Poweroff}, {"StartOrchestrator", priv.StartOrchestrator}, {"RegisterClaim", priv.RegisterClaim}, + {"SyncConfig", priv.SyncConfig}, {"WifiList", priv.WifiList}, {"WifiAdd", priv.WifiAdd}, {"WifiUpdate", priv.WifiUpdate},