Skip to content

Commit

Permalink
Upgrade go-msgpack to v2 2.1.1
Browse files Browse the repository at this point in the history
And set the `time.Time` option to use the go-msgpack-1.1.5-compatible
encoding in all the places, since that is the (now previous) version in
`go.mod`.

v2 2.1.1 was specifically designed to honor backwards compatibility
with 1.1.5 and 0.5.5, and to clean up the code base to be more
maintainable. There may performance lost with the 1.1.5 to 2.1.1
migration since the fastpath code was removed, but the increased safety
is probably worth it. See
[the release notes for go-msgkack 2.1.0](https://github.com/hashicorp/go-msgpack/releases/tag/v2.1.0)
for more details.

I tested this by running this code, and booting up a cluster with a node
also running Vault 1.15.0 (before the upgrade). Before I made the
changes to set the right `time.Time` option, the previous-version node
would throw a bunch of time-decoding errors. After fixing the option,
the node came up smoothly, even after changing leadership between them.

This relies on
- [ ] hashicorp/raft-boltdb#38
- [ ] hashicorp/raft#577

and will need to be updated after they are merged to get the `go.mod`
fixes removed.
  • Loading branch information
Christopher Swenson committed Oct 19, 2023
1 parent 341aaef commit 3a88232
Show file tree
Hide file tree
Showing 7 changed files with 181 additions and 37 deletions.
19 changes: 13 additions & 6 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ require (
github.com/hashicorp/cap/ldap v0.0.0-20230914221201-c4eecc7e31f7
github.com/hashicorp/consul-template v0.33.0
github.com/hashicorp/consul/api v1.23.0
github.com/hashicorp/consul/sdk v0.14.0
github.com/hashicorp/errwrap v1.1.0
github.com/hashicorp/eventlogger v0.2.5
github.com/hashicorp/go-bexpr v0.1.12
Expand All @@ -95,7 +96,7 @@ require (
github.com/hashicorp/go-kms-wrapping/wrappers/ocikms/v2 v2.0.7
github.com/hashicorp/go-kms-wrapping/wrappers/transit/v2 v2.0.8
github.com/hashicorp/go-memdb v1.3.4
github.com/hashicorp/go-msgpack v1.1.5
github.com/hashicorp/go-msgpack/v2 v2.1.1
github.com/hashicorp/go-multierror v1.1.1
github.com/hashicorp/go-plugin v1.5.2
github.com/hashicorp/go-raftchunking v0.6.3-0.20191002164813-7e9e8525653a
Expand Down Expand Up @@ -123,7 +124,7 @@ require (
github.com/hashicorp/hcp-scada-provider v0.2.1
github.com/hashicorp/hcp-sdk-go v0.23.0
github.com/hashicorp/nomad/api v0.0.0-20230519153805-2275a83cbfdf
github.com/hashicorp/raft v1.3.10
github.com/hashicorp/raft v1.4.0
github.com/hashicorp/raft-autopilot v0.2.0
github.com/hashicorp/raft-boltdb/v2 v2.0.0-20210421194847-a7e34179d62c
github.com/hashicorp/raft-snapshot v1.0.4
Expand Down Expand Up @@ -216,11 +217,11 @@ require (
golang.org/x/exp v0.0.0-20230522175609-2e198f4a06a1
golang.org/x/net v0.17.0
golang.org/x/oauth2 v0.11.0
golang.org/x/sync v0.3.0
golang.org/x/sync v0.4.0
golang.org/x/sys v0.13.0
golang.org/x/term v0.13.0
golang.org/x/text v0.13.0
golang.org/x/tools v0.10.0
golang.org/x/tools v0.14.0
google.golang.org/api v0.138.0
google.golang.org/grpc v1.58.3
google.golang.org/grpc/cmd/protoc-gen-go-grpc v1.1.0
Expand Down Expand Up @@ -297,6 +298,7 @@ require (
github.com/baiyubin/aliyun-sts-go-sdk v0.0.0-20180326062324-cfa1a18b161f // indirect
github.com/beorn7/perks v1.0.1 // indirect
github.com/bgentry/speakeasy v0.1.0 // indirect
github.com/boltdb/bolt v1.3.1 // indirect
github.com/boombuler/barcode v1.0.1 // indirect
github.com/cenkalti/backoff v2.2.1+incompatible // indirect
github.com/cenkalti/backoff/v4 v4.2.0 // indirect
Expand Down Expand Up @@ -384,7 +386,6 @@ require (
github.com/hashicorp/cronexpr v1.1.1 // indirect
github.com/hashicorp/go-immutable-radix v1.3.1 // indirect
github.com/hashicorp/go-metrics v0.5.1 // indirect
github.com/hashicorp/go-msgpack/v2 v2.0.0 // indirect
github.com/hashicorp/go-secure-stdlib/fileutil v0.1.0 // indirect
github.com/hashicorp/go-secure-stdlib/plugincontainer v0.2.2 // indirect
github.com/hashicorp/go-slug v0.12.1 // indirect
Expand Down Expand Up @@ -512,7 +513,7 @@ require (
go.uber.org/multierr v1.7.0 // indirect
go.uber.org/zap v1.19.1 // indirect
golang.org/x/exp/typeparams v0.0.0-20221208152030-732eee02a75a // indirect
golang.org/x/mod v0.12.0 // indirect
golang.org/x/mod v0.13.0 // indirect
golang.org/x/time v0.3.0 // indirect
golang.org/x/xerrors v0.0.0-20220907171357-04be3eba64a2 // indirect
google.golang.org/appengine v1.6.7 // indirect
Expand All @@ -536,3 +537,9 @@ require (
sigs.k8s.io/structured-merge-diff/v4 v4.2.3 // indirect
sigs.k8s.io/yaml v1.3.0 // indirect
)

replace github.com/hashicorp/raft => /Users/swenson/projects/raft

replace github.com/hashicorp/raft-boltdb => /Users/swenson/projects/raft-boltdb

replace github.com/hashicorp/raft-boltdb/v2 => /Users/swenson/projects/raft-boltdb/v2
41 changes: 22 additions & 19 deletions go.sum

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion physical/raft/msgpack.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,5 @@ package raft
// on the library, which allows us to pin the version in go.mod.

import (
_ "github.com/hashicorp/go-msgpack/codec"
_ "github.com/hashicorp/go-msgpack/v2/codec"
)
22 changes: 15 additions & 7 deletions physical/raft/raft.go
Original file line number Diff line number Diff line change
Expand Up @@ -402,8 +402,9 @@ func NewRaftBackend(conf map[string]string, logger log.Logger) (physical.Backend
dbPath := filepath.Join(path, "raft.db")
opts := boltOptions(dbPath)
raftOptions := raftboltdb.Options{
Path: dbPath,
BoltOptions: opts,
Path: dbPath,
BoltOptions: opts,
MsgpackUseNewTimeFormat: true,
}
store, err := raftboltdb.New(raftOptions)
if err != nil {
Expand Down Expand Up @@ -851,6 +852,9 @@ type SetupOpts struct {
// RecoveryModeConfig is the configuration for the raft cluster in recovery
// mode.
RecoveryModeConfig *raft.Configuration

// overrideMsgpackUseNewTimeFormat is used for testing backwards compatability
overrideMsgpackUseNewTimeFormat *bool
}

func (b *RaftBackend) StartRecoveryCluster(ctx context.Context, peer Peer) error {
Expand Down Expand Up @@ -945,11 +949,15 @@ func (b *RaftBackend) SetupCluster(ctx context.Context, opts SetupOpts) error {
return err
}
transConfig := &raft.NetworkTransportConfig{
Stream: streamLayer,
MaxPool: 3,
Timeout: 10 * time.Second,
ServerAddressProvider: b.serverAddressProvider,
Logger: b.logger.Named("raft-net"),
Stream: streamLayer,
MaxPool: 3,
Timeout: 10 * time.Second,
ServerAddressProvider: b.serverAddressProvider,
Logger: b.logger.Named("raft-net"),
MsgpackUseNewTimeFormat: true,
}
if opts.overrideMsgpackUseNewTimeFormat != nil {
transConfig.MsgpackUseNewTimeFormat = *opts.overrideMsgpackUseNewTimeFormat
}
transport := raft.NewNetworkTransportWithConfig(transConfig)

Expand Down
121 changes: 121 additions & 0 deletions physical/raft/raft_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,15 @@ import (
"bytes"
"context"
"crypto/md5"
crand "crypto/rand"
"crypto/tls"
"encoding/base64"
"encoding/hex"
"fmt"
"io"
"io/ioutil"
"math/rand"
"net"
"os"
"path/filepath"
"strings"
Expand All @@ -21,12 +24,14 @@ import (

"github.com/go-test/deep"
"github.com/golang/protobuf/proto"
"github.com/hashicorp/consul/sdk/freeport"
"github.com/hashicorp/go-hclog"
"github.com/hashicorp/go-secure-stdlib/base62"
"github.com/hashicorp/go-uuid"
"github.com/hashicorp/raft"
"github.com/hashicorp/vault/sdk/helper/jsonutil"
"github.com/hashicorp/vault/sdk/physical"
"github.com/hashicorp/vault/vault/cluster"
bolt "go.etcd.io/bbolt"
)

Expand Down Expand Up @@ -763,3 +768,119 @@ type discardCloser struct {

func (d discardCloser) Close() error { return nil }
func (d discardCloser) CloseWithError(error) error { return nil }

// TestRaftNetworkClusterWithMultipleTimeEncodingsSet tests that Raft nodes
// with different msgpack time.Time encodings set will still cluster together.
// However, with go-msgpack 2.1.0+, the decoder is tolerant of both encodings,
// so this could only fail if the decoder drastically changes in the future.
func TestRaftNetworkClusterWithMultipleTimeEncodingsSet(t *testing.T) {
// Create raft node
cipherSuites := []uint16{
// 1.3
tls.TLS_AES_128_GCM_SHA256,
tls.TLS_AES_256_GCM_SHA384,
tls.TLS_CHACHA20_POLY1305_SHA256,
}

port1 := freeport.GetOne(t)
port2 := freeport.GetOne(t)
addr1, err := net.ResolveTCPAddr("tcp", fmt.Sprintf("127.0.0.1:%d", port1))
if err != nil {
t.Fatal(err)
}
addr2, err := net.ResolveTCPAddr("tcp", fmt.Sprintf("127.0.0.1:%d", port2))
if err != nil {
t.Fatal(err)
}
key1, err := GenerateTLSKey(crand.Reader)
if err != nil {
t.Fatal(err)
}
key2, err := GenerateTLSKey(crand.Reader)
if err != nil {
t.Fatal(err)
}
logger1 := hclog.New(&hclog.LoggerOptions{
Name: "raft1",
})
logger2 := hclog.New(&hclog.LoggerOptions{
Name: "raft2",
})
listener1 := cluster.NewListener(
cluster.NewTCPLayer([]*net.TCPAddr{addr1}, logger1), cipherSuites, logger1, time.Minute)
listener2 := cluster.NewListener(
cluster.NewTCPLayer([]*net.TCPAddr{addr2}, logger2), cipherSuites, logger2, time.Minute)
go listener1.Run(context.Background())
go listener2.Run(context.Background())
t.Cleanup(listener1.Stop)
t.Cleanup(listener2.Stop)

raft1, dir1 := GetRaftWithOpts(t, true, true, SetupOpts{
TLSKeyring: &TLSKeyring{
Keys: []*TLSKey{key1, key2},
ActiveKeyID: key1.ID,
},
ClusterListener: listener1,
})

overrideTimeFormatFalse := false
setupOpts2 := SetupOpts{
TLSKeyring: &TLSKeyring{
Keys: []*TLSKey{key2, key1},
ActiveKeyID: key2.ID,
},
ClusterListener: listener2,
overrideMsgpackUseNewTimeFormat: &overrideTimeFormatFalse,
}
raft2, dir2 := GetRaftWithOpts(t, false, true, setupOpts2)
defer os.RemoveAll(dir1)
defer os.RemoveAll(dir2)

// Add raft2 to the cluster
addNetworkPeer(t, raft1, raft2, addr2, setupOpts2)

for i := 0; i < 100; i++ {
err = raft1.Put(context.Background(), &physical.Entry{
Key: "test",
Value: []byte{byte(i)},
})
if err != nil {
t.Error(err)
}
}
for raft2.AppliedIndex() != raft1.AppliedIndex() {
time.Sleep(1 * time.Millisecond)
}
entry, err := raft2.Get(context.Background(), "test")
if err != nil {
t.Fatal(err)
}
if entry == nil {
t.Fatal("Entry from raft secondary is nil")
}
if !bytes.Equal(entry.Value, []byte{99}) {
t.Errorf("Expected {99} but got %+v", entry.Value)
}
}

func addNetworkPeer(t *testing.T, leader, follower *RaftBackend, followerAddr *net.TCPAddr, setupOpts SetupOpts) {
t.Helper()
if err := leader.AddPeer(context.Background(), follower.NodeID(), followerAddr.String()); err != nil {
t.Fatal(err)
}

peers, err := leader.Peers(context.Background())
if err != nil {
t.Fatal(err)
}

err = follower.Bootstrap(peers)
if err != nil {
t.Fatal(err)
}

err = follower.SetupCluster(context.Background(), setupOpts)
if err != nil {
t.Fatal(err)
}
}
10 changes: 7 additions & 3 deletions physical/raft/testing.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,20 @@ import (
)

func GetRaft(t testing.TB, bootstrap bool, noStoreState bool) (*RaftBackend, string) {
return GetRaftWithOpts(t, bootstrap, noStoreState, SetupOpts{})
}

func GetRaftWithOpts(t testing.TB, bootstrap bool, noStoreState bool, setupOpts SetupOpts) (*RaftBackend, string) {
raftDir, err := ioutil.TempDir("", "vault-raft-")
if err != nil {
t.Fatal(err)
}
t.Logf("raft dir: %s", raftDir)

return getRaftWithDir(t, bootstrap, noStoreState, raftDir)
return getRaftWithDir(t, bootstrap, noStoreState, raftDir, setupOpts)
}

func getRaftWithDir(t testing.TB, bootstrap bool, noStoreState bool, raftDir string) (*RaftBackend, string) {
func getRaftWithDir(t testing.TB, bootstrap bool, noStoreState bool, raftDir string, setupOpts SetupOpts) (*RaftBackend, string) {
id, err := uuid.GenerateUUID()
if err != nil {
t.Fatal(err)
Expand Down Expand Up @@ -62,7 +66,7 @@ func getRaftWithDir(t testing.TB, bootstrap bool, noStoreState bool, raftDir str
t.Fatal(err)
}

err = backend.SetupCluster(context.Background(), SetupOpts{})
err = backend.SetupCluster(context.Background(), setupOpts)
if err != nil {
t.Fatal(err)
}
Expand Down
3 changes: 2 additions & 1 deletion vault/cluster/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,8 @@ func (cl *Listener) TLSConfig(ctx context.Context) (*tls.Config, error) {
}

// Run starts the tcp listeners and will accept connections until stop is
// called. This function blocks so should be called in a goroutine.
// called. This function does not block and will start the listeners in
// separate goroutines.
func (cl *Listener) Run(ctx context.Context) error {
// Get our TLS config
tlsConfig, err := cl.TLSConfig(ctx)
Expand Down

0 comments on commit 3a88232

Please sign in to comment.