Skip to content

Commit 6abd7fd

Browse files
committed
fix skip label check
Memberlist allows having label and to check them on receiving a message to ensure rings don't collide unexpectedly. The option to skip that checks currently doesn't work and will fail the message unless the labels are exactly the same. This PR change that behaviour to ensure that the option to skip the label check does that. Signed-off-by: Loic Reyreaud <[email protected]>
1 parent 3f82dc1 commit 6abd7fd

File tree

2 files changed

+56
-12
lines changed

2 files changed

+56
-12
lines changed

net.go

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -252,10 +252,6 @@ func (m *Memberlist) handleConn(conn net.Conn) {
252252
}
253253

254254
if m.config.SkipInboundLabelCheck {
255-
if streamLabel != "" {
256-
m.logger.Printf("[ERR] memberlist: unexpected double stream label header: %s", LogConn(conn))
257-
return
258-
}
259255
// Set this from config so that the auth data assertions work below.
260256
streamLabel = m.config.Label
261257
}
@@ -372,10 +368,6 @@ func (m *Memberlist) ingestPacket(buf []byte, from net.Addr, timestamp time.Time
372368
}
373369

374370
if m.config.SkipInboundLabelCheck {
375-
if packetLabel != "" {
376-
m.logger.Printf("[ERR] memberlist: unexpected double packet label header: %s", LogAddress(from))
377-
return
378-
}
379371
// Set this from config so that the auth data assertions work below.
380372
packetLabel = m.config.Label
381373
}
@@ -1118,10 +1110,9 @@ func (m *Memberlist) decryptRemoteState(bufConn io.Reader, streamLabel string) (
11181110

11191111
if moreBytes > maxPushStateBytes {
11201112
return nil, fmt.Errorf("Remote node state is larger than limit (%d)", moreBytes)
1121-
11221113
}
11231114

1124-
//Start reporting the size before you cross the limit
1115+
// Start reporting the size before you cross the limit
11251116
if moreBytes > uint32(math.Floor(.6*maxPushStateBytes)) {
11261117
m.logger.Printf("[WARN] memberlist: Remote node state size is (%d) limit is (%d)", moreBytes, maxPushStateBytes)
11271118
}

transport_test.go

Lines changed: 55 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,6 @@ func TestTransport_Join(t *testing.T) {
5555
if m2.estNumNodes() != 2 {
5656
t.Fatalf("bad: %v", m2.Members())
5757
}
58-
5958
}
6059

6160
func TestTransport_Send(t *testing.T) {
@@ -155,7 +154,6 @@ func (tw testCountingWriter) Write(p []byte) (n int, err error) {
155154
// do not result in a tight loop and spam the log. We verify this here by counting the number
156155
// of entries logged in a given time period.
157156
func TestTransport_TcpListenBackoff(t *testing.T) {
158-
159157
// testTime is the amount of time we will allow NetTransport#tcpListen() to run
160158
// This needs to be long enough that to verify that maxDelay is in force,
161159
// but not so long as to be obnoxious when running the test suite.
@@ -206,3 +204,58 @@ func TestTransport_TcpListenBackoff(t *testing.T) {
206204
// no connections should have been accepted and sent to the channel
207205
require.Equal(t, len(transport.streamCh), 0)
208206
}
207+
208+
func TestTransport_LabelsAndSkip(t *testing.T) {
209+
net := &MockNetwork{}
210+
211+
tests := []struct {
212+
label1 string
213+
label2 string
214+
skip1 bool
215+
skip2 bool
216+
expectSuccess bool
217+
}{
218+
{label1: "label1", label2: "label2"},
219+
{label1: "label1", label2: "label1", expectSuccess: true},
220+
{label1: "label1", label2: "label2", skip1: true, skip2: true, expectSuccess: true},
221+
{label1: "label1", label2: "label1", skip1: true, skip2: true, expectSuccess: true},
222+
}
223+
for _, test := range tests {
224+
t1 := net.NewTransport("node1")
225+
c1 := DefaultLANConfig()
226+
c1.Name = "node1"
227+
c1.Label = test.label1
228+
c1.SkipInboundLabelCheck = test.skip1
229+
c1.Transport = t1
230+
m1, err := Create(c1)
231+
if err != nil {
232+
t.Fatalf("err: %v", err)
233+
}
234+
m1.setAlive()
235+
m1.schedule()
236+
237+
c2 := DefaultLANConfig()
238+
c2.Name = "node2"
239+
c2.Label = test.label2
240+
c2.SkipInboundLabelCheck = test.skip2
241+
c2.Transport = net.NewTransport("node2")
242+
m2, err := Create(c2)
243+
if err != nil {
244+
t.Fatalf("err: %v", err)
245+
}
246+
m2.setAlive()
247+
m2.schedule()
248+
249+
_, err = m2.Join([]string{c1.Name + "/" + t1.addr.String()})
250+
// First shutdown everything so that the next iteration can set it up again
251+
m1.Shutdown()
252+
m2.Shutdown()
253+
254+
// Then check if we expected success or not
255+
if test.expectSuccess && err != nil {
256+
t.Fatalf("unexpected error: %v", err)
257+
} else if !test.expectSuccess && err == nil {
258+
t.Fatalf("expected an error")
259+
}
260+
}
261+
}

0 commit comments

Comments
 (0)