Issue a verified sender again only over the membership that was read, so a newer push is not overwritten (issue 930)

cmd/mesh-controller/reissue_verified_sender_test.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"encoding/json"
+	"errors"
 	"strings"
 	"testing"
 
@@ -25,17 +26,28 @@ func verifiedSenderRecords(free map[string]bool) (broker.Records, broker.Declare
 type fakeBus struct {
 	last      map[string][]byte
 	published map[string][]byte
+	seq       map[string]uint64
+	// between runs once, after a read and before the write: a push issuing meanwhile.
+	between func()
 }
 
-func (f *fakeBus) get(node, module string) ([]byte, error) {
+func (f *fakeBus) get(node, module string) ([]byte, uint64, error) {
 	if b, ok := f.last[node+"/"+module]; ok {
-		return b, nil
+		return b, f.seq[node+"/"+module], nil
 	}
-	return nil, nats.ErrMsgNotFound
+	return nil, 0, nats.ErrMsgNotFound
 }
-func (f *fakeBus) put(node, module string, body []byte) error {
+func (f *fakeBus) put(node, module string, body []byte, seq uint64) error {
+	if f.between != nil {
+		f.between()
+		f.between = nil
+	}
+	if seq != f.seq[node+"/"+module] {
+		return errors.New("nats: wrong last sequence")
+	}
 	f.published[node+"/"+module] = body
 	f.last[node+"/"+module] = body
+	f.seq[node+"/"+module]++
 	return nil
 }
 
@@ -59,7 +71,7 @@ func hasVerified(t *testing.T, membership []byte) bool {
 func TestAMembershipThatLostItsVerifiedSenderGetsItBackWhenTheJudgementPasses(t *testing.T) {
 	failed, router := verifiedSenderRecords(nil)
 	whereFailed := broker.PlacementsOf(failed, nil)
-	bus := &fakeBus{last: map[string][]byte{}, published: map[string][]byte{}}
+	bus := &fakeBus{last: map[string][]byte{}, published: map[string][]byte{}, seq: map[string]uint64{}}
 	body, _ := json.Marshal(broker.MembershipFor("novox", router, whereFailed))
 	bus.last["novox/messenger"] = body
 	if hasVerified(t, body) {
@@ -100,7 +112,7 @@ func TestADroppedVerifiedSenderIsSaidWithItsReason(t *testing.T) {
 
 func TestAMembershipNeverIssuedIsLeftToItsOwnSend(t *testing.T) {
 	records, _ := verifiedSenderRecords(map[string]bool{"novox": true})
-	bus := &fakeBus{last: map[string][]byte{}, published: map[string][]byte{}}
+	bus := &fakeBus{last: map[string][]byte{}, published: map[string][]byte{}, seq: map[string]uint64{}}
 	var said []string
 	n := reissueVerifiedSenders(records, broker.PlacementsOf(records, nil), bus.get, bus.put,
 		func(f string, a ...any) { said = append(said, f) })
@@ -156,3 +168,20 @@ func TestNatsAMembershipIssuedWithoutItsVerifiedSenderIsIssuedAgainWhenTheJudgem
 		t.Fatalf("a second pass issued %d memberships", n)
 	}
 }
+
+// A push issuing a newer membership between the self-check's read and its write is not overwritten (issue 930).
+func TestANewerMembershipIssuedMeanwhileIsNotOverwritten(t *testing.T) {
+	failed, router := verifiedSenderRecords(nil)
+	bus := &fakeBus{last: map[string][]byte{}, published: map[string][]byte{}, seq: map[string]uint64{}}
+	old, _ := json.Marshal(broker.MembershipFor("novox", router, broker.PlacementsOf(failed, nil)))
+	bus.last["novox/messenger"], bus.seq["novox/messenger"] = old, 1
+	newer := []byte(`{"node":"novox","module":"messenger","newer":true}`)
+	bus.between = func() { bus.last["novox/messenger"], bus.seq["novox/messenger"] = newer, 2 }
+	passing, _ := verifiedSenderRecords(map[string]bool{"novox": true})
+	if n := reissueVerifiedSenders(passing, broker.PlacementsOf(passing, nil), bus.get, bus.put, func(string, ...any) {}); n != 0 {
+		t.Fatalf("issued %d over a newer membership", n)
+	}
+	if string(bus.last["novox/messenger"]) != string(newer) || len(bus.published) != 0 {
+		t.Fatalf("the newer membership was overwritten: %s", bus.last["novox/messenger"])
+	}
+}