Files
felhom-agent/internal/pbsdr/seed_reassert_test.go
T
admin 28ba8593b8
gates / gates (push) Failing after 28s
v0.128.0 — R-221: the escrow seed is asserted every tick, not remembered once
A rebuilt box could not run the escrow ceremony AT ALL, with no way forward from inside the product.
This was the only open item blocking a customer from something we promise them.

MECHANISM, established at file:line rather than assumed. The preflight refuses on
escrow.pbs_storage_id; the pbsdr bridge writes that key; and it wrote it from exactly one place —
finishConverged, reached only on the paths that actually converge.

The marker and the key live in different places and die at different times. The marker is host-side
(<agent-state>/pbsdr/marker.json). The key is in agent.json, which step_agent_config renders from
`base = {}` unless an explicit --preserve-from is given (felhom.eu/scripts/felhom-host-install.sh:
2396 the step, :2449 the render, :2579 the O_TRUNC write; the flag :1246, defaulting empty at :256)
— AND THE RENDER NEVER WRITES AN escrow SECTION AT ALL (grep over the whole heredoc: zero hits). So
a rebuild keeps the marker and takes the key: same descriptor, same hash, early return, and the seed
never runs again into a config that no longer has it.

A rebuild is only the case that was measured. The same hole opens for a hand-edited or restored
config, which is the honest reason this is a seam fix rather than an installer fix: the seed must be
a thing the loop ASSERTS, not a thing it did once.

Apply now re-asserts the seed BEFORE the idempotent early return. seedEscrowStorageID is unchanged
and still never clobbers a different existing value — an operator's own choice outranks the
descriptor's, with a warning naming both.

THE EARLY RETURN IS KEPT. It stops a converged box re-running Proxmox operations every 60s, and
TestSeedReasserted_OnConvergedTick_WithZeroProxmoxCalls asserts ZERO recorded runner calls on that
tick, so a "fix" that simply deleted the return would fail. Cost: one small file read plus a JSON
parse per tick, no exec, no network, early-returning once the value matches.

A seed failure can never un-converge the box: Warn plus a message on the published status, exactly
as finishConverged does it — no marker write, no state change.

Tests drive the REAL Apply with a real temp-dir agent.json and a call-recording runner; calling
seedEscrowStorageID directly cannot see the early return, which IS the defect. Production wiring
(pbsdr.NewManager(..., cfg.SourcePath, ...)) is asserted by walking main.go's AST, not by
strings.Contains, which a commented-out call also satisfies.

Red-proofs, each with the mutation asserted applied: removing the new call makes Scenario A fail on
today's tree (it did, with the intended message); removing the early return makes the
zero-Proxmox-calls assertion fail (it did).

go build / go vet / go test ./... green (29 packages), run separately from this commit.
2026-08-08 16:29:13 +02:00

233 lines
8.6 KiB
Go

package pbsdr
import (
"context"
"encoding/json"
"go/ast"
"go/parser"
"go/printer"
"go/token"
"io"
"os"
"path/filepath"
"strings"
"testing"
"gitea.dooplex.hu/admin/felhom-agent/internal/hub"
)
// R-221 — the escrow seed must be ASSERTED on every converged tick, not remembered.
//
// These tests drive the real Apply() with a real temp-dir agent.json and a call-recording runner.
// Calling seedEscrowStorageID directly would prove nothing: the defect IS the early return in
// Apply, and a test that steps around it cannot see it.
// convergedMarker writes a marker whose hash matches the block, i.e. puts the manager on exactly
// the idempotent path where the seed used to be skipped.
func convergedMarker(t *testing.T, m *Manager, block *hub.WirePBSDR, state string) {
t.Helper()
if err := m.writeState(m.markerPath(), marker{
Hash: descriptorHash(block), State: state, AppliedAt: "2026-08-08T00:00:00Z",
}); err != nil {
t.Fatalf("write marker: %v", err)
}
}
func escrowStorageID(t *testing.T, cfgPath string) string {
t.Helper()
raw, err := os.ReadFile(cfgPath)
if err != nil {
t.Fatalf("read config: %v", err)
}
var doc struct {
Escrow struct {
PBSStorageID string `json:"pbs_storage_id"`
} `json:"escrow"`
}
if err := json.Unmarshal(raw, &doc); err != nil {
t.Fatalf("parse config: %v", err)
}
return doc.Escrow.PBSStorageID
}
// drBlock mirrors the existing valid fixture (manager_test.go descriptor()) so that validate()
// passes and Apply actually reaches the marker check — the branch these tests are about.
func drBlock(storageID string) *hub.WirePBSDR {
return &hub.WirePBSDR{
Enabled: true, StorageID: storageID, PBSTunnelIP: "10.77.0.1",
Datastore: "felhom-offsite", Namespace: "peti", TokenID: "felhom@pbs!peti",
Fingerprint: testFP,
}
}
// SCENARIO A — the seed is re-asserted on a converged box, and nothing else happens.
//
// THIS IS THE TEST THAT MATTERS. It must fail against the pre-R-221 tree; if it passes there, it is
// not testing the defect and that is the finding.
func TestSeedReasserted_OnConvergedTick_WithZeroProxmoxCalls(t *testing.T) {
r := &fakeRunner{}
st := &fakeStorage{found: true, active: []bool{true}}
c := &fakeConsumer{}
m, cfgPath := newTestManager(t, r, st, c)
block := drBlock("felhom-pbs-dr")
convergedMarker(t, m, block, "applied")
// the shape a rebuild leaves behind: escrow section present, pbs_storage_id GONE
if got := escrowStorageID(t, cfgPath); got != "" {
t.Fatalf("precondition: config already carries a storage id %q", got)
}
m.Apply(context.Background(), true, block)
if got := escrowStorageID(t, cfgPath); got != "felhom-pbs-dr" {
t.Errorf("the converged tick did not re-assert the seed: escrow.pbs_storage_id = %q, want %q.\n"+
"This is R-221: the marker survives a rebuild, the descriptor hash still matches, the early "+
"return fires and the seed never runs into the config that no longer has it — so the customer "+
"cannot run the escrow ceremony at all.", got, "felhom-pbs-dr")
}
// ...and the idempotent path is STILL idempotent. This assertion is not decorative: without it
// a "fix" that simply deletes the early return would pass the line above.
if calls := r.recorded(); len(calls) != 0 {
t.Errorf("a converged tick must execute ZERO Proxmox commands; got %d: %+v", len(calls), calls)
}
}
// SCENARIO B — an operator's own different value survives, and the warning names both.
func TestSeedReasserted_NeverClobbersAnOperatorValue(t *testing.T) {
r := &fakeRunner{}
m, cfgPath := newTestManager(t, r, &fakeStorage{found: true, active: []bool{true}}, &fakeConsumer{})
if err := os.WriteFile(cfgPath, []byte(
`{"log_level":"info","escrow":{"posture":"zero_knowledge","pbs_storage_id":"operator-chosen"},`+
`"custom_unknown":{"keep":1}}`), 0o600); err != nil {
t.Fatal(err)
}
block := drBlock("hub-chosen")
convergedMarker(t, m, block, "applied")
m.Apply(context.Background(), true, block)
if got := escrowStorageID(t, cfgPath); got != "operator-chosen" {
t.Errorf("a value a person put there was overwritten by the descriptor: got %q, want %q", got, "operator-chosen")
}
// unknown keys must still round-trip
raw, _ := os.ReadFile(cfgPath)
if !strings.Contains(string(raw), "custom_unknown") {
t.Error("an unknown config key was dropped by the re-assert")
}
}
// SCENARIO C — the ceremony preflight's live read sees the re-asserted value with NO restart.
//
// The preflight itself lives in internal/localapi and reads the file through config.Load; what this
// asserts is the half that belongs to this package: after a converged tick, THE FILE ON DISK carries
// the id, so any live re-read is green. The daemon is never restarted in this test because it is
// never started — which is the point.
func TestSeedReasserted_IsVisibleOnDiskImmediately(t *testing.T) {
r := &fakeRunner{}
m, cfgPath := newTestManager(t, r, &fakeStorage{found: true, active: []bool{true}}, &fakeConsumer{})
block := drBlock("felhom-pbs-dr")
convergedMarker(t, m, block, "applied")
// preflight's predicate BEFORE: storageID == "" → the row is NOT OK
if escrowStorageID(t, cfgPath) != "" {
t.Fatal("precondition")
}
m.Apply(context.Background(), true, block)
// preflight's predicate AFTER, from the same file the ceremony subprocess loads
if id := escrowStorageID(t, cfgPath); id == "" {
t.Error("the preflight row would still be NOT OK after a converged tick")
}
}
// A seed failure must NEVER un-converge the box: no marker rewrite, no state change, and the status
// still reports the marker's converged state — with the failure surfaced as a message.
func TestSeedReassertFailure_DoesNotUnconverge(t *testing.T) {
r := &fakeRunner{}
m, cfgPath := newTestManager(t, r, &fakeStorage{found: true, active: []bool{true}}, &fakeConsumer{})
block := drBlock("felhom-pbs-dr")
convergedMarker(t, m, block, "applied")
markerBefore, err := os.ReadFile(m.markerPath())
if err != nil {
t.Fatal(err)
}
// make the seed fail in a way it cannot recover from: unparseable config
if err := os.WriteFile(cfgPath, []byte(`{ this is not json`), 0o600); err != nil {
t.Fatal(err)
}
m.Apply(context.Background(), true, block)
after, err := os.ReadFile(m.markerPath())
if err != nil {
t.Fatalf("the marker was removed by a seed failure: %v", err)
}
if string(after) != string(markerBefore) {
t.Error("a seed failure rewrote the convergence marker — it must not touch state")
}
if calls := r.recorded(); len(calls) != 0 {
t.Errorf("a seed failure must not trigger Proxmox work; got %+v", calls)
}
st := m.Status()
if st == nil || st.State != "applied" {
t.Errorf("a seed failure must leave the box converged; status = %+v", st)
}
if st != nil && !strings.Contains(st.Message, "seed failed") {
t.Errorf("a seed failure must be surfaced on the status, got message %q", st.Message)
}
}
// SEAM WIRING — production must construct the manager with the live config path, or the whole seed
// leg is inert. Three shipped defects in this project were fully green while their seam was never
// wired, so this walks main.go's AST for the actual call rather than grepping: a commented-out call
// satisfies strings.Contains, and an AST walk cannot see a comment.
func TestProductionWiring_NewManagerGetsTheLiveConfigPath(t *testing.T) {
path := filepath.Join("..", "..", "cmd", "felhom-agent", "main.go")
fset := token.NewFileSet()
f, err := parser.ParseFile(fset, path, nil, 0) // comments not even collected
if err != nil {
t.Fatalf("parse main.go: %v", err)
}
found := false
ast.Inspect(f, func(n ast.Node) bool {
call, ok := n.(*ast.CallExpr)
if !ok {
return true
}
sel, ok := call.Fun.(*ast.SelectorExpr)
if !ok || sel.Sel.Name != "NewManager" {
return true
}
if pkg, ok := sel.X.(*ast.Ident); !ok || pkg.Name != "pbsdr" {
return true
}
// signature: (runner, px, hubc, stateDir, secretDir, configPath, logger)
if len(call.Args) < 6 {
t.Errorf("pbsdr.NewManager called with %d args, expected >= 6", len(call.Args))
return false
}
var buf strings.Builder
if err := printNode(&buf, fset, call.Args[5]); err != nil {
t.Fatalf("print arg: %v", err)
}
got := buf.String()
if !strings.Contains(got, "SourcePath") {
t.Errorf("pbsdr.NewManager's configPath argument is %q, which is not the live config path.\n"+
"With an empty or wrong path seedEscrowStorageID returns nil immediately and the entire "+
"R-221 fix is inert while every test above still passes.", got)
}
found = true
return false
})
if !found {
t.Error("no pbsdr.NewManager call found in main.go — the manager is not constructed in production")
}
}
func printNode(w io.Writer, fset *token.FileSet, n ast.Node) error {
return printer.Fprint(w, fset, n)
}