From e88ddab2417069b97fe3fcc14a260dad6fd68d31 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 26 Oct 2026 12:54:06 +0100 Subject: [PATCH] An apply of the bus module waits when the list on the machine cannot be read while the grants part's is followed, so the file is never removed as an orphan (review of the switch, issue 1945) --- cmd/mesh-host/nodeagent.go | 8 +++++++- internal/userlist/userlist.go | 18 ++++++++++++------ internal/userlist/userlist_test.go | 15 ++++++++++++++- 3 files changed, 33 insertions(+), 8 deletions(-) diff --git a/cmd/mesh-host/nodeagent.go b/cmd/mesh-host/nodeagent.go index 895403e..d4e69ab 100644 --- a/cmd/mesh-host/nodeagent.go +++ b/cmd/mesh-host/nodeagent.go @@ -451,7 +451,13 @@ func nodeAgent(ctx context.Context, opts options, signer ed25519.PublicKey, sess actx, done := applyContext(session.WithLink(ctx)) // While the grants part's list is followed, the controller's list in the bus module's composition is not // written: the apply keeps the list on the machine (ADR 0547 clause 8, never both). - res := applyComposition(actx, opts, users.Hold(c), others, sched, say) + held, herr := users.Hold(c) + var res agent.Result + if herr != nil { + res = failedWith(herr.Error()) + } else { + res = applyComposition(actx, opts, held, others, sched, say) + } done() // An apply of the bus module may write an older list than the newest composition holds: looked at again. if module, _ := users.State(); module != "" && module == c.Module { diff --git a/internal/userlist/userlist.go b/internal/userlist/userlist.go index 1a9a878..536da9b 100644 --- a/internal/userlist/userlist.go +++ b/internal/userlist/userlist.go @@ -517,18 +517,24 @@ func (f *Follower) settleGrants(ctx context.Context, again bool) { // Hold is c as the apply may write it (C3 of the switch's review): while the grants part's list is followed, the bus // module's composition from the controller carries the controller's list, so its user-list resource holds the list on the -// machine instead (made 0600), or is left out when none can be read. Any other composition, or the controller's list -// followed, is c unchanged. -func (f *Follower) Hold(c delivery.Composed) delivery.Composed { +// machine instead (made 0600), or is left out when the machine holds none. A list on the machine that cannot be read +// is an error: left out, the apply would remove the file as an orphan and the bus would refuse every connection at its +// next reload, so the bus module's apply waits instead. Any other composition, or the controller's list followed, is c +// unchanged. +func (f *Follower) Hold(c delivery.Composed) (delivery.Composed, error) { if f.Following() != delivery.FollowGrants { - return c + return c, nil } out := c out.Resources = nil for _, raw := range c.Resources { if r, _, _, ok := userList([]json.RawMessage{raw}); ok { var held json.RawMessage - if body, err := f.env.OnDisk(r); err == nil && body != nil { + body, err := f.env.OnDisk(r) + if err != nil { + return c, fmt.Errorf("the user list on the machine could not be read, so the apply waits and the file stays: %w", err) + } + if body != nil { var m map[string]any if json.Unmarshal(r, &m) == nil { delete(m, "digest") @@ -544,7 +550,7 @@ func (f *Follower) Hold(c delivery.Composed) delivery.Composed { } out.Resources = append(out.Resources, raw) } - return out + return out, nil } // mode0600 is a user-list resource made mode 0600, whatever its composition said (ADR 0547 clause 7). diff --git a/internal/userlist/userlist_test.go b/internal/userlist/userlist_test.go index fb25d99..94e89b3 100644 --- a/internal/userlist/userlist_test.go +++ b/internal/userlist/userlist_test.go @@ -444,11 +444,24 @@ func TestTheBusNodeWritesTheNewestSignedUserListAlone(t *testing.T) { if err != nil { t.Fatal(err) } - held := fl.Hold(cmp) + held, err := fl.Hold(cmp) + if err != nil { + t.Fatal(err) + } _, _, _, hasList := userList(held.Resources) if want := c.wantFollows == delivery.FollowController; hasList != want || len(held.Resources) < 1 { t.Fatalf("an apply while following %s holds the controller's list: %v", c.wantFollows, hasList) } + // A list on the machine that cannot be read stops the apply; it is never left out to be removed. + if c.wantFollows == delivery.FollowGrants { + readErr := New(Env{Node: node, Composed: f, OnDisk: func(json.RawMessage) ([]byte, error) { + return nil, errors.New("permission denied") + }}) + readErr.grants = &delivery.GrantsUserList{Follow: delivery.FollowGrants} + if _, err := readErr.Hold(cmp); err == nil || !strings.Contains(err.Error(), "could not be read") { + t.Fatalf("an unreadable list on the machine was held as %v", err) + } + } } if c.wantSource != "" && (st.Source != c.wantSource || st.Key != nodeKey.PublicKey || (c.wantFollows != "" && st.Follows != c.wantFollows)) {