Merge pull request 'mesh-provisioner-client: a module that leaves or is placed again asks the vault afresh (issue 1945)' (#484) from fix/1945-held-secret-on-leave into main

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")
+			}
+		})
+	}
+}

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) {

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/<node>/<module>/<name>,
 // provision/<provider>/<provision>/<node>/<module>, pair/<provider>/<provision>/<node>/<module>[/<local>]; "" for
 // none.