diff --git a/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/asked_again_test.go b/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/asked_again_test.go index 7901e9f6..fc5dcfae 100644 --- a/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/asked_again_test.go +++ b/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/asked_again_test.go @@ -6,6 +6,8 @@ import ( "fmt" "testing" "time" + + "github.com/nats-io/nats.go/jetstream" ) // scriptedSecrets answers each Get from the next of its errors, then with a value, counting the asks. @@ -104,3 +106,70 @@ func TestANotYetSecretIsAskedAgainOnEveryPass(t *testing.T) { t.Fatalf("value %q after %d asks, want filled on the third", v, fake.asked) } } + +// fakeEntry is one watched key's update, as far as take reads it. +type fakeEntry struct { + jetstream.KeyValueEntry + key string + val []byte + op jetstream.KeyValueOp +} + +func (f fakeEntry) Key() string { return f.key } +func (f fakeEntry) Value() []byte { return f.val } +func (f fakeEntry) Operation() jetstream.KeyValueOp { return f.op } + +// Issue 1945 (ace, 2026-10-25 23:51): a module that leaves the machine and is placed on it again asks the vault +// afresh, whichever way it left or returned, so it is never filled with a generation held from before; a module +// that stays placed keeps using what it holds. +func TestAModulePlacedAgainAsksTheVaultAfreshAndAModuleThatStaysDoesNot(t *testing.T) { + const assignment = `{"node":"g14","module":"build","provisioning":{}}` + own := VaultName("g14", "build", "bus-password") + pair := PairName("g14", "db", "g14", "build", "") + other := VaultName("g14", "gitea", "token") + for _, row := range []struct { + name string + steps func(c *Client, s *state) + again int // asks of the vault after the first three: the module's own secret once more, or none + }{ + {name: "it stays placed: the assignment is put again", again: 0, steps: func(c *Client, s *state) { + c.take(s, update{bucket: AssignmentsBucket, entry: fakeEntry{key: "g14.build", val: []byte(assignment)}}) + }}, + {name: "its assignment is deleted and put again", again: 1, steps: func(c *Client, s *state) { + c.take(s, update{bucket: AssignmentsBucket, entry: fakeEntry{key: "g14.build", op: jetstream.KeyValueDelete}}) + c.take(s, update{bucket: AssignmentsBucket, entry: fakeEntry{key: "g14.build", val: []byte(assignment)}}) + }}, + {name: "it left and the client was away: only the later assignment arrives", again: 1, steps: func(c *Client, s *state) { + delete(s.assigned, "build") + c.take(s, update{bucket: AssignmentsBucket, entry: fakeEntry{key: "g14.build", val: []byte(assignment)}}) + }}, + {name: "it leaves the machine", again: 1, steps: func(c *Client, s *state) { + c.leave(context.Background(), nil, s, "build") + }}, + } { + t.Run(row.name, func(t *testing.T) { + fake := &scriptedSecrets{} + c := &Client{Node: "g14", Secrets: fake, Now: time.Now} + s := &state{secrets: map[string]heldSecret{}, plans: map[string]Plan{}, refused: map[string]string{}, + assigned: map[string]bool{"build": true}, binds: map[string]binding{}, providing: map[string]bool{}, + leftOut: map[string]string{}, needs: map[string][]byte{}, applied: map[string]bool{}, + reported: map[string][]byte{}, items: map[string]map[string]itemValue{}, recorded: map[string][]byte{}, + unplaced: map[string]bool{}} + n := Need{Kind: "secret", Name: "bus-password"} + ask := func(name string) { c.vaultValue(context.Background(), s, n, name) } + ask(own) + ask(pair) + ask(other) + row.steps(c, s) + ask(own) + ask(other) + if got := fake.asked - 3; got != row.again { + t.Errorf("%d asks after the first three, want %d (the module's own once more, if it was placed again)", + got, row.again) + } + if _, held := s.secrets[other]; !held { + t.Error("another module's secret was dropped") + } + }) + } +} diff --git a/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/client.go b/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/client.go index e9745779..beacf539 100644 --- a/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/client.go +++ b/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/client.go @@ -545,6 +545,9 @@ func (c *Client) take(s *state, u update) []string { gone = gone || !provider.Standing(e.Value()) // Whom it binds, whichever machine's it is: a provider here listing its consumers is filled again. var providers []string + if ended, had := s.binds[key]; gone && had { + c.forgetBound(s, ended) + } if takeBinds(s, key, e.Value(), gone) { providers = sortedKeys(s.providing) } @@ -554,6 +557,10 @@ func (c *Client) take(s *state, u update) []string { } delete(s.plans, module) delete(s.refused, module) + if gone || !s.assigned[module] { + // Leaving, or placed here after being absent: nothing held of the vault is served (issue 1945). + c.forgetHeld(s, module) + } if gone { delete(s.assigned, module) return append(providers, module) @@ -670,6 +677,14 @@ func (c *Client) leftModules(s *state) []string { func (c *Client) leave(ctx context.Context, kvs map[string]jetstream.KeyValue, s *state, module string) { prefix := c.Node + "." + module + "." delete(s.unplaced, module) + // What it held of the vault goes with it: placed again, it asks afresh (issue 1945). + c.forgetHeld(s, module) + delete(s.providing, module) + for k := range s.leftOut { + if strings.HasPrefix(k, module+" ") { + delete(s.leftOut, k) + } + } removed := 0 for _, key := range sortedKeys(s.needs) { if strings.HasPrefix(key, prefix) { diff --git a/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/secrets.go b/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/secrets.go index be7473c3..ff2a5e0b 100644 --- a/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/secrets.go +++ b/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/secrets.go @@ -273,6 +273,36 @@ func (c *Client) forgetRefusals(s *state, module string) []string { return dropped } +// forgetHeld drops every secret this run holds for a module of this machine, whatever it is (value or refusal), so +// the next fill asks the vault afresh: a module that left the machine, or is placed on it again, is never served a +// value held from before (ace, 2026-10-25 23:51: a re-assigned module was filled with the generation it held before +// the vault minted a new one, and the bus refused the account; issue 1945). A provider here that lists its consumers +// also loses the credentials of the consumers it held, which no name ties to one provider module. It answers the +// names dropped. +func (c *Client) forgetHeld(s *state, module string) []string { + var dropped []string + for _, name := range sortedKeys(s.secrets) { + own := secretModule(c.Node, name) == module + held := s.providing[module] && strings.HasPrefix(name, "pair/"+c.Node+"/") + if own || held { + delete(s.secrets, name) + dropped = append(dropped, name) + } + } + return dropped +} + +// forgetBound drops the credentials a provider here held for a consumer whose assignment ended: when the consumer +// is assigned again the vault may hold a newer generation of them. +func (c *Client) forgetBound(s *state, b binding) { + for _, name := range sortedKeys(s.secrets) { + parts := strings.Split(name, "/") + if len(parts) >= 5 && parts[0] == "pair" && parts[3] == b.node && parts[4] == b.module { + delete(s.secrets, name) + } + } +} + // secretModule is the module of this machine a vault name is issued for: own///, // provision////, pair////[/]; "" for // none.