From fb52442b502e31b3dfa5d2f895beef63ecc47f57 Mon Sep 17 00:00:00 2001 From: mesh-tools Date: Tue, 20 Oct 2026 20:47:25 +0000 Subject: [PATCH] Serve a build module's key from its own state, read-only on nox-builder, in the forge account provider (issue 1482), part 5 of 17 --- .../cmd/forge-account-provider/service.go | 203 ++++++++++++++---- .../cmd/forge-account-provider/watch_test.go | 162 ++++++++++++++ 2 files changed, 318 insertions(+), 47 deletions(-) create mode 100644 modules/gitea/cmd/forge-account-provider/watch_test.go diff --git a/modules/gitea/cmd/forge-account-provider/service.go b/modules/gitea/cmd/forge-account-provider/service.go index b9f33717..0eef366c 100644 --- a/modules/gitea/cmd/forge-account-provider/service.go +++ b/modules/gitea/cmd/forge-account-provider/service.go @@ -64,8 +64,11 @@ type Service struct { mu sync.Mutex keys map[string]machineKey - loaded bool // the machines' keys have been read: until then a node with none has not reported none - known string // the forge's host key line, as the machines write it into known_hosts + loaded bool // the machines' keys have been read: until then a node with none has not reported none + // loadedFrom says of each build module's state that its keys have been read, as loaded says of ssh-client's: a build + // module's state that never answers (the module is not installed) holds back its own entries and no other. + loadedFrom map[string]bool + known string // the forge's host key line, as the machines write it into known_hosts bound provider.Bus // the bus the entries are written to (set by Provider) @@ -76,14 +79,40 @@ type Service struct { type machineKey struct { Node, Account, Kind, Public string + // Source is the module whose state the key was read from: ssh-client's for a machine's, a build module's for a + // builder's. Empty is ssh-client's. + Source string } +// SSHClient is the module whose `keys` state holds the machines' keys (kind machine); the build modules' states hold +// the builders' (kind builder, ADR 0466 §2). +const SSHClient = "ssh-client" + +// sourceOf is the module a stored key was read from. +func (k machineKey) sourceOf() string { + if k.Source == "" { + return SSHClient + } + return k.Source +} + +// storageKey is the key a source's key is kept under: a machine's as it is, a builder's under its module's name. +func storageKey(source, key string) string { + if source == "" || source == SSHClient { + return key + } + return source + "/" + key +} + +// kindOf is the kind of key a consumer module's entry is made for. +func kindOf(module string) string { return forgeaccount.KindOfModule(module) } + // NewService is the service over its environment. func NewService(cfg Config) *Service { if cfg.Log == nil { cfg.Log = func(f string, a ...any) { fmt.Fprintf(os.Stderr, "[gitea forge-account] "+f+"\n", a...) } } - return &Service{cfg: cfg, keys: map[string]machineKey{}, letGo: map[string]bool{}, Pause: time.Sleep, + return &Service{cfg: cfg, keys: map[string]machineKey{}, letGo: map[string]bool{}, loadedFrom: map[string]bool{}, Pause: time.Sleep, accounts: &forgeaccount.Accounts{Forge: cfg.Forge, Log: cfg.Log}} } @@ -103,38 +132,67 @@ func (s *Service) Provider(bus provider.Bus) *provider.Provider { } // errNoKey is a consumer whose machine has reported no key yet: nothing is made, and the client says it waits. -type errNoKey struct{ node string } +type errNoKey struct{ node, module string } func (e errNoKey) Error() string { + if kind := kindOf(e.module); kind == "builder" { + return "no key from " + e.node + " yet: its " + e.module + " has not reported a builder key, so there is no " + + "entry to make" + } return "no key from " + e.node + " yet: its ssh-client has not reported a machine key, so there is no account " + "and no entry to make" } +// sourceOfModule is the module whose state holds the keys a consumer module's entry is made from: a build module's +// own, ssh-client's for any other consumer. +func sourceOfModule(module string) string { + if kindOf(module) == "builder" { + return module + } + return SSHClient +} + +// scopeFor is the title prefix of the keys one entry registered: every key of a machine for a machine's entry, one +// build module's key for a builder's (nox-builder holds the keys of every build module of the mesh). +func scopeFor(kind, node, module string) string { + if kind == "builder" { + return forgeaccount.BuilderScope(node, module) + } + return forgeaccount.MachineScope(node) +} + // Make brings one consumer's account, its team membership and its keys into being, as the machine reported them. It // leaves what is as asked alone, so it is asked again freely. func (s *Service) Make(ctx context.Context, c provider.Consumer) error { s.making.Lock() defer s.making.Unlock() node := c.Entry.Consumer - user := forgeaccount.ForgeUser(node, "machine") + module, kind := c.Entry.Module, kindOf(c.Entry.Module) + user := forgeaccount.ForgeUser(node, kind) if user == "" { return fmt.Errorf("%q is not a machine's name an account can be named for", node) } + source := sourceOfModule(module) s.mu.Lock() loaded := s.loaded + if source != SSHClient { + loaded = s.loadedFrom[source] + } s.mu.Unlock() if !loaded { // After a restart the first pass can come before the keys arrive: nothing is removed or deleted on that. - return errors.New("the machines' keys are not read yet, so nothing is made, removed or deleted") + return fmt.Errorf("the keys of %s are not read yet, so nothing is made, removed or deleted", source) } - have := s.keysOf(node) + have := s.keysFrom(source, node, kind) if len(have) == 0 { - if err := s.releasePerNodeUser(ctx, node); err != nil { - return err + if kind == "machine" { + if err := s.releasePerNodeUser(ctx, node); err != nil { + return err + } } // A key the machine no longer reports is no longer wanted (ADR 0400 §2): the mesh's keys go, the operator's // stay, and the entry that served the machine goes with them. - if _, err := s.accounts.RemoveKeysTitled(ctx, user, forgeaccount.MachineScope(node)); err != nil { + if _, err := s.accounts.RemoveKeysTitled(ctx, user, scopeFor(kind, node, module)); err != nil { return err } if s.bound != nil { @@ -142,7 +200,7 @@ func (s *Service) Make(ctx context.Context, c provider.Consumer) error { return err } } - return errNoKey{node} + return errNoKey{node: node, module: module} } if _, err := s.hostName(); err != nil { return err @@ -154,20 +212,23 @@ func (s *Service) Make(ctx context.Context, c provider.Consumer) error { if err := s.accounts.EnsureAccount(ctx, user); err != nil { return err } - if err := s.accounts.EnsureInTeam(ctx, user); err != nil { + if err := s.accounts.EnsureInTeamOf(ctx, forgeaccount.TeamOfUser(user), user); err != nil { return err } var wanted []forgeaccount.WantedKey for _, k := range have { - wanted = append(wanted, forgeaccount.WantedKey{Title: forgeaccount.KeyTitle(k.Node, k.Account), Key: k.Public}) + // A builder's key may read and never push (ADR 0466 §3): the key says it beside the team that does. + wanted = append(wanted, forgeaccount.WantedKey{Title: forgeaccount.KeyTitle(k.Node, k.Account), Key: k.Public, ReadOnly: kind == "builder"}) } // The forge holds one public key on one user only, so the key leaves agent- before it joins nox-agent. That // is done here, after everything that can fail before it, so the window in which the machine's key is on no user // is only the span between these two calls; if the add fails, the next pass (at most ten minutes) puts it back. - if err := s.releasePerNodeUser(ctx, node); err != nil { - return err + if kind == "machine" { + if err := s.releasePerNodeUser(ctx, node); err != nil { + return err + } } - if err := s.accounts.EnsureKeys(ctx, user, forgeaccount.MachineScope(node), wanted); err != nil { + if err := s.accounts.EnsureKeys(ctx, user, scopeFor(kind, node, module), wanted); err != nil { return err } s.mu.Lock() @@ -176,20 +237,25 @@ func (s *Service) Make(ctx context.Context, c provider.Consumer) error { return nil } -// Gone removes the keys the mesh registered for a machine that no longer binds the forge. The account stays: it may +// Gone removes the keys the mesh registered for a machine (or, for a build module, the module's key on that machine) +// that no longer binds the forge. The account stays: it may // hold what the machine's agent wrote, and the keys of the operator's own that were added by hand are not touched // (ADR 0399 §2). func (s *Service) Gone(ctx context.Context, e provider.Entry) error { s.making.Lock() defer s.making.Unlock() - user := forgeaccount.ForgeUser(e.Consumer, "machine") + kind := kindOf(e.Module) + user := forgeaccount.ForgeUser(e.Consumer, kind) if user == "" { return nil } - if err := s.releasePerNodeUser(ctx, e.Consumer); err != nil { - return err + if kind == "machine" { + if err := s.releasePerNodeUser(ctx, e.Consumer); err != nil { + return err + } } - _, err := s.accounts.RemoveKeysTitled(ctx, user, forgeaccount.MachineScope(e.Consumer)) + // A builder's entry gone removes that entry's key only: the user and the team serve the other machines. + _, err := s.accounts.RemoveKeysTitled(ctx, user, scopeFor(kind, e.Consumer, e.Module)) return err } @@ -220,17 +286,24 @@ func (s *Service) releasePerNodeUser(ctx context.Context, node string) error { } // KeyChanged takes in one machine key as the bus keeps it, and tells OnKeys its node. -func (s *Service) KeyChanged(key string, value []byte) { - if node := s.store(key, value); node != "" && s.OnKeys != nil { +func (s *Service) KeyChanged(key string, value []byte) { s.SourceKeyChanged(SSHClient, key, value) } + +// KeyGone forgets one machine key, and tells OnKeys its node. +func (s *Service) KeyGone(key string) { s.SourceKeyGone(SSHClient, key) } + +// SourceKeyChanged takes in one key of the state `keys` of the module source (ssh-client's, or a build module's), and +// tells OnKeys its node. +func (s *Service) SourceKeyChanged(source, key string, value []byte) { + if node := s.storeFrom(source, key, value); node != "" && s.OnKeys != nil { s.OnKeys(node) } } -// KeyGone forgets one machine key, and tells OnKeys its node. -func (s *Service) KeyGone(key string) { +// SourceKeyGone forgets one key of the module source's state, and tells OnKeys its node. +func (s *Service) SourceKeyGone(source, key string) { node, _, _ := strings.Cut(key, ".") s.mu.Lock() - delete(s.keys, key) + delete(s.keys, storageKey(source, key)) s.mu.Unlock() if s.OnKeys != nil { s.OnKeys(node) @@ -240,24 +313,38 @@ func (s *Service) KeyGone(key string) { // store keeps a machine key that reads as one: its node and account the key's own, and a public half that is a // public key line and nothing private. It answers the node, or "" for a value it did not keep (and drops what was // kept under that key, since the key now says something else). -func (s *Service) store(key string, value []byte) string { +func (s *Service) store(key string, value []byte) string { return s.storeFrom(SSHClient, key, value) } + +// storeFrom is store for a key of the module source's state. +func (s *Service) storeFrom(source, key string, value []byte) string { node, account, _ := strings.Cut(key, ".") + stored := storageKey(source, key) var k machineKey var raw struct { Node, Account, Kind, Public string } err := json.Unmarshal(value, &raw) - k = machineKey(raw) + k = machineKey{Node: raw.Node, Account: raw.Account, Kind: raw.Kind, Public: raw.Public} + if source != SSHClient { + k.Source = source + } s.mu.Lock() defer s.mu.Unlock() - if err != nil || k.Node != node || k.Account != account || !publicKeyLine(k.Public) { - if _, had := s.keys[key]; had { - delete(s.keys, key) - } - s.cfg.Log("the key %s is not a machine's public key; it is not registered", key) + if want := kindOf(source); err == nil && k.Kind != want && k.Node == node && k.Account == account { + // Each state holds one kind: ssh-client's the machines', a build module's the builders' (ADR 0466 §2). The other + // kind in either is not this entry's to serve, and is said. + delete(s.keys, stored) + s.cfg.Log("the key %s in the state keys of %s is of kind %q, not %q; it is ignored", key, source, k.Kind, want) return "" } - s.keys[key] = k + if err != nil || k.Node != node || k.Account != account || !publicKeyLine(k.Public) { + if _, had := s.keys[stored]; had { + delete(s.keys, stored) + } + s.cfg.Log("the key %s of %s is not a public key that can be registered; it is not registered", key, source) + return "" + } + s.keys[stored] = k return node } @@ -267,12 +354,15 @@ func publicKeyLine(line string) bool { } // keysOf are the machine keys one node reported, by key. -func (s *Service) keysOf(node string) []machineKey { +func (s *Service) keysOf(node string) []machineKey { return s.keysFrom(SSHClient, node, "machine") } + +// keysFrom are the keys of the kind that one node reported in the module source's state, by key. +func (s *Service) keysFrom(source, node, kind string) []machineKey { s.mu.Lock() defer s.mu.Unlock() var names []string for key, k := range s.keys { - if k.Node == node && k.Kind == "machine" && strings.HasPrefix(key, node+".") { + if k.Node == node && k.Kind == kind && k.sourceOf() == source && strings.HasPrefix(key, storageKey(source, node+".")) { names = append(names, key) } } @@ -284,27 +374,37 @@ func (s *Service) keysOf(node string) []machineKey { return out } -// WatchKeys keeps the machines' keys as the bus holds them, asking again until the runtime answers (the channel to it -// opens as the bundle starts, and the state of ssh-client may arrive after this one). A change after the first -// values is told to OnKeys; the first values are not, since one pass answers them all. +// WatchKeys keeps the machines' keys as the bus holds them (ssh-client's state `keys`), asking again until the runtime +// answers (the channel to it opens as the bundle starts, and the state of ssh-client may arrive after this one). A +// change after the first values is told to OnKeys; the first values are not, since one pass answers them all. func (s *Service) WatchKeys(ctx context.Context, src KeySource) error { + return s.WatchSource(ctx, SSHClient, src) +} + +// WatchSource is WatchKeys for the state `keys` of any module whose keys the provider registers: ssh-client's, or a +// build module's (ADR 0466 §3). Once its first values are in, the entries made from them are no longer held back. +func (s *Service) WatchSource(ctx context.Context, source string, src KeySource) error { waits := []time.Duration{2 * time.Second, 5 * time.Second, 10 * time.Second, 30 * time.Second} for attempt := 0; ; attempt++ { err := src.Watch(func(key, op string, value []byte, current bool) { switch { case op != "put" && current: case op != "put": - s.KeyGone(key) + s.SourceKeyGone(source, key) case current: - s.store(key, value) + s.storeFrom(source, key, value) default: - s.KeyChanged(key, value) + s.SourceKeyChanged(source, key, value) } }) if err == nil { // The watch answers once the current values are all handled (stdio KeptState.Watch). s.mu.Lock() - s.loaded = true + if source == SSHClient { + s.loaded = true + } else { + s.loadedFrom[source] = true + } s.mu.Unlock() return nil } @@ -312,7 +412,7 @@ func (s *Service) WatchKeys(ctx context.Context, src KeySource) error { if attempt < len(waits) { pause = waits[attempt] } - s.cfg.Log("the machines' keys cannot be watched yet (%v); asking again in %s", err, pause) + s.cfg.Log("the keys of %s cannot be watched yet (%v); asking again in %s", source, err, pause) if ctx.Err() != nil { return ctx.Err() } @@ -377,9 +477,18 @@ func knownHostLine(scanned, host string, port int) (string, error) { return "", errors.New("the forge's sshd gave no ed25519 host key") } +// valuesForUser is the forge user a consumer module's entry is served as. +func (s *Service) valuesForUser(node, module string) (string, error) { + user := forgeaccount.ForgeUser(node, kindOf(module)) + if user == "" { + return "", fmt.Errorf("%q is not a machine's name an account can be named for", node) + } + return user, nil +} + // valuesFor are the values served to one machine's consumer (ADR 0400 §3, §4). -func (s *Service) valuesFor(node string) (map[string]any, error) { - user := forgeaccount.ForgeUser(node, "machine") +func (s *Service) valuesFor(node, module string) (map[string]any, error) { + user := forgeaccount.ForgeUser(node, kindOf(module)) host, err := s.hostName() if err != nil { return nil, err @@ -421,7 +530,7 @@ func (b *valuesBus) Put(ctx context.Context, bucket, key string, value []byte) e return err } if e.Refused == "" { - extra, err := b.s.valuesFor(e.Consumer) + extra, err := b.s.valuesFor(e.Consumer, e.Module) if err != nil { return err } diff --git a/modules/gitea/cmd/forge-account-provider/watch_test.go b/modules/gitea/cmd/forge-account-provider/watch_test.go new file mode 100644 index 00000000..e8986788 --- /dev/null +++ b/modules/gitea/cmd/forge-account-provider/watch_test.go @@ -0,0 +1,162 @@ +package main + +import ( + "context" + "encoding/json" + "fmt" + "os" + "slices" + "strings" + "sync" + "testing" + "time" +) + +// Issue 1482 (novox/hq ADR 0466 §3): the provider watches the build modules' state `keys` beside ssh-client's, and +// registers each builder entry read-only on nox-builder. + +func keyJSON(node, account, kind, public string) []byte { + b, _ := json.Marshal(map[string]string{"node": node, "account": account, "kind": kind, "public": public, + "fingerprint": "SHA256:x", "at": "2026-10-20T10:00:00Z"}) + return b +} + +// watched is a team rig whose keys come through watches of the modules' states, as in the running provider. +type watched struct { + *teamRig + sources map[string]*fakeKeys + passes *int + logs *[]string + mu *sync.Mutex +} + +func newWatched(t *testing.T) *watched { + r := newTeamRig(t) + r.svc.loaded, r.svc.loadedFrom = false, map[string]bool{} + r.svc.Pause = func(time.Duration) {} + n, logs, mu := 0, []string{}, &sync.Mutex{} + r.svc.OnKeys = func(string) { n++ } + r.svc.cfg.Log = func(f string, a ...any) { mu.Lock(); logs = append(logs, fmt.Sprintf(f, a...)); mu.Unlock() } + w := &watched{teamRig: r, sources: map[string]*fakeKeys{}, passes: &n, logs: &logs, mu: mu} + for _, module := range []string{"ssh-client", "mesh-build-go", "mesh-build-node"} { + w.sources[module] = &fakeKeys{} + if err := r.svc.WatchSource(context.Background(), module, w.sources[module]); err != nil { + t.Fatal(err) + } + } + return w +} + +func (w *watched) said() string { + w.mu.Lock() + defer w.mu.Unlock() + return strings.Join(*w.logs, "\n") +} + +// 1482/AC1: a mesh-build-go.keys entry ace.mesh-build-go of kind builder: its public half is registered read-only on +// nox-builder titled mesh:ace.mesh-build-go. +func TestABuilderEntryOfABuildModulesStateIsRegisteredReadOnlyOnNoxBuilder(t *testing.T) { + w := newWatched(t) + w.bind("ace", "mesh-build-go") + w.sources["mesh-build-go"].send("ace.mesh-build-go", "put", keyJSON("ace", "mesh-build-go", "builder", pubGo+" mesh-build-go@ace (mesh)"), true) + w.pass() + keys := w.forge.keys["nox-builder"] + if len(keys) != 1 || keys[0].Title != "mesh:ace.mesh-build-go" || !keys[0].ReadOnly || keys[0].Key != pubGo+" mesh-build-go@ace (mesh)" { + t.Errorf("nox-builder's keys = %+v", keys) + } + if !w.svc.loadedFrom["mesh-build-go"] { + t.Errorf("the state was watched and its keys are not counted as read") + } +} + +// 1482/AC2: that entry changed: the old key is removed and the new one registered; deleted: the key is removed and no key +// titled otherwise is touched. +func TestAChangedBuilderEntryReplacesItsKeyAndADeletedOneRemovesIt(t *testing.T) { + w := newWatched(t) + w.bind("ace", "mesh-build-go") + src := w.sources["mesh-build-go"] + src.send("ace.mesh-build-go", "put", keyJSON("ace", "mesh-build-go", "builder", pubGo), true) + w.pass() + w.forge.mu.Lock() + w.forge.keys["nox-builder"] = append(w.forge.keys["nox-builder"], forgeKey{ID: 99, Title: "my own", Key: pubOps}) + w.forge.mu.Unlock() + + before := *w.passes + src.send("ace.mesh-build-go", "put", keyJSON("ace", "mesh-build-go", "builder", pubNode), false) + if *w.passes != before+1 { + t.Errorf("a change asked for %d passes, want 1", *w.passes-before) + } + w.pass() + got := w.forge.keys["nox-builder"] + if len(got) != 2 || got[0].Title != "my own" || got[1].Title != "mesh:ace.mesh-build-go" || got[1].Key != pubNode || !got[1].ReadOnly { + t.Errorf("after the change: %+v", got) + } + + src.send("ace.mesh-build-go", "delete", nil, false) + w.pass() + if got := strings.Join(w.forge.titles("nox-builder"), ","); got != "my own" { + t.Errorf("after the delete: %s, want only the operator's key", got) + } +} + +// 1482/AC3: an entry of kind machine in a build module's state, or of kind builder in ssh-client's, is ignored and +// said in the provider's log. +func TestAKeyOfTheWrongKindForItsStateIsIgnoredAndSaid(t *testing.T) { + w := newWatched(t) + w.bind("ace", "mesh-build-go") + w.bind("g14", "ssh-client") + w.sources["mesh-build-go"].send("ace.mesh-build-go", "put", keyJSON("ace", "mesh-build-go", "machine", pubGo), true) + w.sources["ssh-client"].send("g14.nox", "put", keyJSON("g14", "nox", "builder", pubA), true) + w.pass() + if len(w.forge.users) != 0 || w.bus.has("ace.mesh-build-go.forge-account") || w.bus.has("g14.ssh-client.forge-account") { + t.Errorf("a key of the wrong kind was served: users %v", w.forge.users) + } + said := w.said() + for _, want := range []string{`ace.mesh-build-go in the state keys of mesh-build-go is of kind "machine", not "builder"`, `g14.nox in the state keys of ssh-client is of kind "builder", not "machine"`} { + if !strings.Contains(said, want) { + t.Errorf("the log lacks %q:\n%s", want, said) + } + } +} + +// A build module's state that never answers holds back its own entries, never a machine's. +func TestABuildModulesStateThatNeverAnswersHoldsBackOnlyItsOwnEntries(t *testing.T) { + w := newWatched(t) + w.svc.loadedFrom = map[string]bool{} // mesh-build-go's watch has not answered + w.bind("ace", "mesh-build-go") + w.bind("g14", "ssh-client") + w.sources["ssh-client"].send("g14.nox", "put", keyJSON("g14", "nox", "machine", pubA+" nox@g14 (mesh)"), true) + out := w.pass() + if !w.bus.has("g14.ssh-client.forge-account") { + t.Errorf("a machine's entry waited for a build module's state") + } + waits := 0 + for _, o := range out { + if strings.Contains(o.Key, "mesh-build-go") && strings.Contains(o.Why, "keys of mesh-build-go are not read yet") { + waits++ + } + } + if waits != 1 { + t.Errorf("outcomes = %+v", out) + } +} + +// 1482/AC4 (the provider's half): the manifest reads the states of both build modules beside ssh-client's and the +// controller's assignments; the provider watches exactly the modules the code names as builders. +func TestTheManifestReadsTheBuildModulesKeys(t *testing.T) { + raw, err := os.ReadFile("../../module.json") + if err != nil { + t.Fatal(err) + } + var m struct { + Reads []string `json:"reads"` + } + if err := json.Unmarshal(raw, &m); err != nil { + t.Fatal(err) + } + for _, state := range []string{"mesh-controller.assignments", "ssh-client.keys", "mesh-build-go.keys", "mesh-build-node.keys"} { + if !slices.Contains(m.Reads, state) { + t.Errorf("gitea's reads %v lack %q", m.Reads, state) + } + } +}