From b1bc63dd3285993b45522ea3cdab827859cf3fba Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Mon, 26 Oct 2026 12:44:25 +0100 Subject: [PATCH] =?UTF-8?q?R-1285:=20the=20new=20agent=20runs=20the=20bund?= =?UTF-8?q?le=20policy=20pass=20once=20after=20its=20self-update=20commits?= =?UTF-8?q?=20=E2=80=94=20in=20the=20normal=20two-job=20delivery=20the=20o?= =?UTF-8?q?ld=20process=20installs=20the=20bundle.=20Test=20in=20the=20rea?= =?UTF-8?q?l=20order=20(commit,=20then=20one=20bundle-policy=20plan);=20re?= =?UTF-8?q?d-proved.=20Commit=20only,=20not=20released.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CHANGELOG.md | 16 ++++ cmd/felhom-agent/main.go | 9 ++ .../r1285_policy_after_commit_test.go | 91 +++++++++++++++++++ internal/osupdate/bundle.go | 13 +++ internal/selfupdate/commit.go | 9 ++ 5 files changed, 138 insertions(+) create mode 100644 cmd/felhom-agent/r1285_policy_after_commit_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index e6f8caa..5d37508 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,19 @@ +## Unreleased (next: v0.167.0) — the BYO SSH clean-up in the SAME update (R-1285) (2026-10-26) — committed, not released + +- **The cause (measured 2026-10-26 on demo-hp, felhom.eu R-1285):** a box gets `agent_update` and `agent_config_update` in + one pass; the bundle job runs in the OLD process (no policy pass in it) and the restart into the new agent comes a + second later — so 0.166.0's pass never ran on the bundle that delivered it. +- **The fix:** `selfupdate.Manager.SetOnCommitted` — a hook run once right after „update committed"; `main.go` + `wireBundlePolicyAfterCommit` hooks `osupdate.ConfigUpdateExecutor.PolicyPassAfterUpdate` (the wrapper's + `bundle-policy` mode, host layer, no envelope) onto it. By the commit the wrapper on disk is the bundle's own, so a BYO + host loses our drop-in in the same update. Idempotent; an appliance removes nothing. The smaller of the two fixes: + agent-only — holding the bundle job until the commit would need new hub queue logic. +- **Test in the real order:** `cmd/felhom-agent/r1285_policy_after_commit_test.go` — a pending marker for the running + version → commit, THEN one `bundle-policy` plan; no marker / a mismatched marker → nothing. Red-proved (the hook call + removed → commit only, no pass — the demo-hp symptom). +- **Which box can show it live:** only one coming FROM 0.164.0 — 0.165.0's wrapper already applies D43 at plan time, so a + 0.165.0 box passes even without this fix. + ## v0.166.0 (2026-10-26) — a signed job's window is at most 24 h (R-1280); the policy pass after each bundle (R-1285); the published gate follows D50 (R-1290) — released in the attended release run; **VOUCHED by the operator 2026-10-26 10:04:55 CET** (binary `3f22c65b…`, bundle `093772c1…`) Released from a clean clone at `aa69114` (tag `v0.166.0`; binary sha256 `3f22c65b78914632fa476292f6b58e5af3ed9e9900f3a5a7a347a9c0bfea9c5c`, diff --git a/cmd/felhom-agent/main.go b/cmd/felhom-agent/main.go index 516a46f..11f6cb4 100644 --- a/cmd/felhom-agent/main.go +++ b/cmd/felhom-agent/main.go @@ -1216,6 +1216,9 @@ func runDaemon(cfg config.Config, logger *slog.Logger, logRing *applog.Ring) int } logger.Warn("osupdate: capability probe after the config bundle", "ok", ok, "total", total, "degraded", strings.Join(names, ",")) }} + // R-1285: the NEW agent runs the bundle policy pass once after its self-update commits — in the normal two-job + // delivery the bundle was installed by the OLD process, which has no such pass (measured 2026-10-26). + wireBundlePolicyAfterCommit(selfUpdateMgr, bundleExec, logger, version) // v0.158.0 (felhom.eu `16` §9): the internal data place on an old box — the root helper, then the mount (late-bound // to the local API server built below). internalPlaceExec := signedjobs.NewInternalPlaceExecutor(suRunner, @@ -4040,3 +4043,9 @@ func agentStateDir(cfg config.Config) string { } return "/var/lib/felhom-agent" } + +// wireBundlePolicyAfterCommit hooks the bundle policy pass onto the self-update commit (R-1285). Pinned by +// TestR1285_TheNewAgentRunsThePolicyPassAfterItsCommit. +func wireBundlePolicyAfterCommit(m *selfupdate.Manager, e osupdate.ConfigUpdateExecutor, lg *slog.Logger, ver string) { + m.SetOnCommitted(func(ctx context.Context) { e.PolicyPassAfterUpdate(ctx, lg, ver) }) +} diff --git a/cmd/felhom-agent/r1285_policy_after_commit_test.go b/cmd/felhom-agent/r1285_policy_after_commit_test.go new file mode 100644 index 0000000..d498f4e --- /dev/null +++ b/cmd/felhom-agent/r1285_policy_after_commit_test.go @@ -0,0 +1,91 @@ +package main + +import ( + "context" + "encoding/json" + "io" + "log/slog" + "os" + "path/filepath" + "sync" + "testing" + "time" + + "gitea.dooplex.hu/admin/felhom-agent/internal/osupdate" + "gitea.dooplex.hu/admin/felhom-agent/internal/selfupdate" +) + +// R-1285, in the REAL order (measured 2026-10-26 on demo-hp): the box gets agent_update and agent_config_update in one +// pass; the OLD process installs the bundle (no policy pass in it), then restarts into the new agent, which dwells and +// commits. The consequence asserted: the NEW agent runs the bundle policy pass exactly once, AFTER its commit — so a +// BYO host loses our drop-in in the same update. No pass without a commit (no marker, a mismatched marker). +// RED-PROOF: drop the wireBundlePolicyAfterCommit call in main → no bundle-policy plan after the commit. + +// orderRunner records, in order, the self-update wrapper's verbs and the os-apply plans' modes. +type orderRunner struct { + mu sync.Mutex + order []string +} + +func (r *orderRunner) Run(_ context.Context, name string, args ...string) ([]byte, []byte, error) { + r.mu.Lock() + defer r.mu.Unlock() + if name == osupdate.WrapperPath && len(args) == 2 && args[0] == "--plan" { + var p map[string]any + b, _ := os.ReadFile(args[1]) + _ = json.Unmarshal(b, &p) + mode, _ := p["mode"].(string) + r.order = append(r.order, "os-apply:"+mode) + return []byte(`OSAPPLY-REPORT {"mode":"bundle-policy","bundle_policy":{"install_mode":"byo","removed":["/etc/ssh/sshd_config.d/10-felhom-root-key-only.conf"]}}` + "\n"), nil, nil + } + if len(args) >= 1 { + r.order = append(r.order, filepath.Base(name)+":"+args[0]) + } + return []byte("ok"), nil, nil +} + +func (r *orderRunner) RunStdin(ctx context.Context, _ io.Reader, name string, args ...string) ([]byte, []byte, error) { + return r.Run(ctx, name, args...) +} + +func r1285Start(t *testing.T, marker *selfupdate.PendingMarker, running string) []string { + t.Helper() + stateDir := t.TempDir() + if marker != nil { + dir := filepath.Join(stateDir, "selfupdate") + if err := os.MkdirAll(dir, 0o750); err != nil { + t.Fatal(err) + } + b, _ := json.Marshal(marker) + if err := os.WriteFile(filepath.Join(dir, "pending.json"), b, 0o644); err != nil { + t.Fatal(err) + } + } + r := &orderRunner{} + lg := slog.New(slog.NewTextHandler(io.Discard, nil)) + mgr := selfupdate.NewManager(selfupdate.ManagerConfig{StateDir: stateDir, RunningVersion: running, Dwell: time.Millisecond, Runner: r, Logger: lg}) + leg := &osupdate.Leg{Runner: r, Logger: lg, PlanDir: t.TempDir(), StatePath: filepath.Join(t.TempDir(), "last")} + exec := osupdate.ConfigUpdateExecutor{Leg: leg} + wireBundlePolicyAfterCommit(mgr, exec, lg, running) + mgr.MaybeCommit(context.Background()) + r.mu.Lock() + defer r.mu.Unlock() + return append([]string(nil), r.order...) +} + +func TestR1285_TheNewAgentRunsThePolicyPassAfterItsCommit(t *testing.T) { + got := r1285Start(t, &selfupdate.PendingMarker{OldVersion: "0.166.0", NewVersion: "0.167.0", SHA256: "ab", AppliedAt: "t"}, "0.167.0") + if len(got) != 2 || got[0] != "felhom-selfupdate-guarded:commit" || got[1] != "os-apply:bundle-policy" { + t.Fatalf("the new agent must commit, then run ONE bundle policy pass: %v", got) + } +} + +func TestR1285_NoCommitNoPolicyPass(t *testing.T) { + if got := r1285Start(t, nil, "0.167.0"); len(got) != 0 { + t.Fatalf("an ordinary start (no pending update) must run nothing: %v", got) + } + got := r1285Start(t, &selfupdate.PendingMarker{OldVersion: "0.166.0", NewVersion: "0.168.0", SHA256: "ab", AppliedAt: "t"}, "0.167.0") + if len(got) != 0 { + t.Fatalf("a mismatched marker is not committed, so no pass: %v", got) + } +} diff --git a/internal/osupdate/bundle.go b/internal/osupdate/bundle.go index 0676316..665509a 100644 --- a/internal/osupdate/bundle.go +++ b/internal/osupdate/bundle.go @@ -126,6 +126,19 @@ func (e ConfigUpdateExecutor) Execute(ctx context.Context, op string, params jso // appliance-only files' policy (mode "bundle-policy"): on a BYO host it removes the SSH drop-in we wrote. The bundle run // itself was planned by the wrapper that was on disk BEFORE it, which may predate the policy (0.164.0). Never fatal: // the bundle is installed either way; an older wrapper refuses the unknown mode and the next bundle does it. +// PolicyPassAfterUpdate runs the wrapper's bundle-policy pass once after a self-update commits (R-1285). In the normal +// delivery the box gets agent_update and agent_config_update in ONE pass: the bundle job runs in the OLD process, which +// has no policy pass, and the restart into the new agent comes a second later (measured 2026-10-26 on demo-hp). By the +// commit the wrapper on disk is the bundle's own, so the pass removes our appliance-only files from a BYO host in the +// SAME update. Idempotent; on an appliance it removes nothing. +func (e ConfigUpdateExecutor) PolicyPassAfterUpdate(ctx context.Context, lg *slog.Logger, ver string) { + if lg == nil { + lg = slog.Default() + } + lg.Info("osupdate: running the bundle policy pass after the self-update commit (R-1285)", "agent_version", ver) + e.policyPass(ctx, lg, ver) +} + func (e ConfigUpdateExecutor) policyPass(ctx context.Context, lg *slog.Logger, ver string) { runID := "bundle-policy-" + e.Leg.now().UTC().Format("20060102T150405Z") wr, err := e.Leg.call(ctx, runID, map[string]any{"release_id": "bundle-policy-" + ver, "layer": LayerHost, "mode": "bundle-policy"}) diff --git a/internal/selfupdate/commit.go b/internal/selfupdate/commit.go index 56bd906..845440b 100644 --- a/internal/selfupdate/commit.go +++ b/internal/selfupdate/commit.go @@ -18,8 +18,14 @@ type Manager struct { logger *slog.Logger now func() time.Time // injectable for tests sleep func(context.Context, time.Duration) + // onCommitted runs once, right after a successful commit (R-1285: the bundle policy pass of the NEW agent — in the + // normal two-job delivery the bundle was installed by the OLD process, which has no such pass). nil = none. + onCommitted func(context.Context) } +// SetOnCommitted wires the after-commit hook (R-1285). INIT-ONLY: call it before MaybeCommit's goroutine starts. +func (m *Manager) SetOnCommitted(fn func(context.Context)) { m.onCommitted = fn } + // ManagerConfig wires the commit manager. type ManagerConfig struct { StateDir string @@ -119,4 +125,7 @@ func (m *Manager) MaybeCommit(ctx context.Context) { return } m.logger.Warn("selfupdate: update committed", "version", m.runningVersion, "wrapper", trim(stdout)) + if m.onCommitted != nil { + m.onCommitted(ctx) + } }