From 48f47bb55a6daaedebf88ca3db08b0e93b094576 Mon Sep 17 00:00:00 2001 From: Robert Fratto Date: Mon, 16 Jun 2025 11:27:54 -0400 Subject: [PATCH] messages: add forward compatibility for gossiped messages Previously, unknown fields received while gossiping would be silently dropped. This can happen if an older version of ckit receives a message from a newer version of ckit (where a new field has been added). This can lead to temporary convergence issues during a rollout: messages will continue to be refuted until all nodes are running the same version. While the State message has not changed since the original version of ckit, not having any support for forwards compatibility prevents us from being able to cleanly extend messages with new fields (see #89). This commit enforces all messages to handle and propagate unknown fields to permit forwards compatibility. Closes #90. --- go.mod | 3 +- go.sum | 6 +-- internal/messages/messages.go | 8 ++- internal/messages/messages_test.go | 80 +++++++++++++++++++++++++++++- internal/messages/state.go | 17 +++++++ node.go | 2 +- 6 files changed, 107 insertions(+), 9 deletions(-) diff --git a/go.mod b/go.mod index 969e4e5..e18c07a 100644 --- a/go.mod +++ b/go.mod @@ -5,7 +5,7 @@ go 1.23.0 require ( github.com/cespare/xxhash/v2 v2.3.0 github.com/go-kit/log v0.2.1 - github.com/hashicorp/go-msgpack v0.5.5 + github.com/hashicorp/go-msgpack/v2 v2.1.3 github.com/hashicorp/go-multierror v1.1.1 github.com/hashicorp/memberlist v0.5.3 github.com/prometheus/client_golang v1.22.0 @@ -23,7 +23,6 @@ require ( github.com/hashicorp/errwrap v1.0.0 // indirect github.com/hashicorp/go-immutable-radix v1.0.0 // indirect github.com/hashicorp/go-metrics v0.5.4 // indirect - github.com/hashicorp/go-msgpack/v2 v2.1.1 // indirect github.com/hashicorp/go-sockaddr v1.0.0 // indirect github.com/hashicorp/go-uuid v1.0.1 // indirect github.com/hashicorp/golang-lru v0.5.1 // indirect diff --git a/go.sum b/go.sum index f4cd5d2..25fc90d 100644 --- a/go.sum +++ b/go.sum @@ -59,10 +59,8 @@ github.com/hashicorp/go-immutable-radix v1.0.0 h1:AKDB1HM5PWEA7i4nhcpwOrO2byshxB github.com/hashicorp/go-immutable-radix v1.0.0/go.mod h1:0y9vanUI8NX6FsYoO3zeMjhV/C5i9g4Q3DwcSNZ4P60= github.com/hashicorp/go-metrics v0.5.4 h1:8mmPiIJkTPPEbAiV97IxdAGNdRdaWwVap1BU6elejKY= github.com/hashicorp/go-metrics v0.5.4/go.mod h1:CG5yz4NZ/AI/aQt9Ucm/vdBnbh7fvmv4lxZ350i+QQI= -github.com/hashicorp/go-msgpack v0.5.5 h1:i9R9JSrqIz0QVLz3sz+i3YJdT7TTSLcfLLzJi9aZTuI= -github.com/hashicorp/go-msgpack v0.5.5/go.mod h1:ahLV/dePpqEmjfWmKiqvPkv/twdG7iPBM1vqhUKIvfM= -github.com/hashicorp/go-msgpack/v2 v2.1.1 h1:xQEY9yB2wnHitoSzk/B9UjXWRQ67QKu5AOm8aFp8N3I= -github.com/hashicorp/go-msgpack/v2 v2.1.1/go.mod h1:upybraOAblm4S7rx0+jeNy+CWWhzywQsSRV5033mMu4= +github.com/hashicorp/go-msgpack/v2 v2.1.3 h1:cB1w4Zrk0O3jQBTcFMKqYQWRFfsSQ/TYKNyUUVyCP2c= +github.com/hashicorp/go-msgpack/v2 v2.1.3/go.mod h1:SjlwKKFnwBXvxD/I1bEcfJIBbEJ+MCUn39TxymNR5ZU= github.com/hashicorp/go-multierror v1.1.1 h1:H5DkEtf6CXdFp0N0Em5UCwQpXMWke8IA0+lD48awMYo= github.com/hashicorp/go-multierror v1.1.1/go.mod h1:iw975J/qwKPdAO1clOe2L8331t/9/fmwbPZ6JB6eMoM= github.com/hashicorp/go-retryablehttp v0.5.3/go.mod h1:9B5zBasrRhHXnJnui7y6sL7es7NDiJgTc6Er0maI1Xs= diff --git a/internal/messages/messages.go b/internal/messages/messages.go index dfd999e..e619b6f 100644 --- a/internal/messages/messages.go +++ b/internal/messages/messages.go @@ -7,7 +7,7 @@ import ( "encoding/binary" "fmt" - "github.com/hashicorp/go-msgpack/codec" + "github.com/hashicorp/go-msgpack/v2/codec" ) // magicHeader is added to the start of every message. @@ -39,6 +39,12 @@ func (t Type) String() string { // Message is a payload that can be gossiped to other peers. type Message interface { + // Messages must implement [codec.MissingFielder] to ensure forwards + // compatibility with newer versions of ckit that add new fields to messages, + // so that older versions of ckit can rebroadcast messages without dropping + // unknown fields. + codec.MissingFielder + // Type returns the Type of the message. Type must be a known, valid type. Type() Type diff --git a/internal/messages/messages_test.go b/internal/messages/messages_test.go index cdbc9e4..a29d261 100644 --- a/internal/messages/messages_test.go +++ b/internal/messages/messages_test.go @@ -49,10 +49,88 @@ func TestMessages_Invalid(t *testing.T) { require.EqualError(t, err, "invalid magic header ddff") } +// TestMessages_ForwardCompatibility ensures that older versions of ckit can +// decode messages from newer versions of ckit (where message introduced new +// files) while also re-encoding those messages with the fields retained. +// +// This ensures that gossiping messages between different versions of ckit does +// not silently drop fields that were added in newer versions. +func TestMessages_ForwardCompatibility(t *testing.T) { + type ( + oldMessage struct { + fakeMessage + + Address string + } + + newMessage struct { + fakeMessage + + Address string + Version int + } + ) + + expect := newMessage{ + fakeMessage: newFakeMessage(TypeState), + Address: "127.0.0.1:80", + Version: 3, + } + + raw, err := Encode(expect) + require.NoError(t, err) + + recv := oldMessage{fakeMessage: newFakeMessage(TypeState)} + { + buf, ty, err := Parse(raw) + require.NoError(t, err) + require.Equal(t, TypeState, ty) + + err = Decode(buf, &recv) + require.NoError(t, err) + } + + // Re-encode the message we received, but include unknown fields that + // oldMessage didn't recognize. + raw, err = Encode(recv) + require.NoError(t, err) + + actual := newMessage{fakeMessage: newFakeMessage(TypeState)} + { + buf, ty, err := Parse(raw) + require.NoError(t, err) + require.Equal(t, TypeState, ty) + + err = Decode(buf, &actual) + require.NoError(t, err) + require.Len(t, actual.CodecMissingFields(), 0) + } + + require.Equal(t, expect, actual) +} + type fakeMessage struct { - ty Type + ty Type + missingFields map[string]any +} + +func newFakeMessage(ty Type) fakeMessage { + return fakeMessage{ + ty: ty, + missingFields: make(map[string]any), + } } func (fm fakeMessage) Type() Type { return fm.ty } func (fm fakeMessage) Name() string { return "" } func (fm fakeMessage) Cache() bool { return false } + +func (fm fakeMessage) CodecMissingField(field []byte, value interface{}) bool { + if fm.missingFields == nil { + return false + } + fm.missingFields[string(field)] = value + return true +} + +func (fm fakeMessage) CodecMissingFields() map[string]interface{} { return fm.missingFields } diff --git a/internal/messages/state.go b/internal/messages/state.go index 25ec3a8..8f7a3f3 100644 --- a/internal/messages/state.go +++ b/internal/messages/state.go @@ -15,6 +15,9 @@ type State struct { NewState peer.State // Time the state was generated. Time lamport.Time + // UnknownFields is used to store any fields that were added in newer + // versions of State. + UnknownFields map[string]any `codec:"-"` } // String returns the string representation of the State message. @@ -32,3 +35,17 @@ func (s *State) Name() string { return s.NodeName } // Cache implements Message. func (s *State) Cache() bool { return true } + +// CodecMissingField registers a missing field into s.UnknownFields. +func (s *State) CodecMissingField(field []byte, value interface{}) bool { + if s.UnknownFields == nil { + s.UnknownFields = make(map[string]interface{}) + } + s.UnknownFields[string(field)] = value + return true +} + +// CodecMissingFields returns the current set of unknown fields. +func (s *State) CodecMissingFields() map[string]interface{} { + return s.UnknownFields +} diff --git a/node.go b/node.go index 94e66b3..4878fd8 100644 --- a/node.go +++ b/node.go @@ -14,7 +14,7 @@ import ( "github.com/go-kit/log" "github.com/go-kit/log/level" - "github.com/hashicorp/go-msgpack/codec" + "github.com/hashicorp/go-msgpack/v2/codec" "github.com/hashicorp/memberlist" "github.com/prometheus/client_golang/prometheus"