28ba8593b8
gates / gates (push) Successful in 20s
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.
233 lines
8.6 KiB
Go
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)
|
|
}
|