From b6a7c2956d5280ffc5fa42c2eeb7aa11e9ebdb5a Mon Sep 17 00:00:00 2001 From: Christopher Swenson Date: Thu, 19 Oct 2023 14:21:57 -0700 Subject: [PATCH 1/8] Upgrade go-msgpack to v2 2.1.1 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 - [ ] https://github.com/hashicorp/raft-boltdb/pull/38 - [ ] https://github.com/hashicorp/raft/pull/577 and will need to be updated after they are merged to get the `go.mod` fixes removed. --- go.mod | 15 ++--- go.sum | 19 +++--- physical/raft/msgpack.go | 2 +- physical/raft/raft.go | 22 ++++--- physical/raft/raft_test.go | 121 +++++++++++++++++++++++++++++++++++++ physical/raft/testing.go | 10 ++- vault/cluster/cluster.go | 3 +- 7 files changed, 164 insertions(+), 28 deletions(-) diff --git a/go.mod b/go.mod index fd2cdbf6fe0c..4a89fbcaa168 100644 --- a/go.mod +++ b/go.mod @@ -82,6 +82,7 @@ require ( github.com/hashicorp/cli v1.1.6 github.com/hashicorp/consul-template v0.36.0 github.com/hashicorp/consul/api v1.26.1 + github.com/hashicorp/consul/sdk v0.15.0 github.com/hashicorp/errwrap v1.1.0 github.com/hashicorp/eventlogger v0.2.8 github.com/hashicorp/go-bexpr v0.1.12 @@ -99,7 +100,8 @@ 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-metrics v0.5.1 + github.com/hashicorp/go-msgpack/v2 v2.1.1 github.com/hashicorp/go-multierror v1.1.1 github.com/hashicorp/go-plugin v1.6.0 github.com/hashicorp/go-raftchunking v0.6.3-0.20191002164813-7e9e8525653a @@ -127,7 +129,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 @@ -220,11 +222,11 @@ require ( golang.org/x/exp v0.0.0-20230817173708-d852ddb80c63 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.15.0 golang.org/x/term v0.15.0 golang.org/x/text v0.14.0 - golang.org/x/tools v0.12.1-0.20230815132531-74c255bcf846 + golang.org/x/tools v0.14.0 google.golang.org/api v0.139.0 google.golang.org/grpc v1.58.3 google.golang.org/grpc/cmd/protoc-gen-go-grpc v1.1.0 @@ -386,8 +388,7 @@ require ( github.com/hailocab/go-hostpool v0.0.0-20160125115350-e80d13ce29ed // indirect 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-msgpack v1.1.5 // indirect github.com/hashicorp/go-secure-stdlib/fileutil v0.1.0 // indirect github.com/hashicorp/go-secure-stdlib/plugincontainer v0.3.0 // indirect github.com/hashicorp/go-slug v0.12.1 // indirect @@ -515,7 +516,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 diff --git a/go.sum b/go.sum index 432f86ad0636..7bcaef5c0c75 100644 --- a/go.sum +++ b/go.sum @@ -2205,8 +2205,8 @@ github.com/hashicorp/go-msgpack v0.5.3/go.mod h1:ahLV/dePpqEmjfWmKiqvPkv/twdG7iP github.com/hashicorp/go-msgpack v0.5.5/go.mod h1:ahLV/dePpqEmjfWmKiqvPkv/twdG7iPBM1vqhUKIvfM= github.com/hashicorp/go-msgpack v1.1.5 h1:9byZdVjKTe5mce63pRVNP1L7UAmdHOTEMGehn6KvJWs= github.com/hashicorp/go-msgpack v1.1.5/go.mod h1:gWVc3sv/wbDmR3rQsj1CAktEZzoz1YNK9NfGLXJ69/4= -github.com/hashicorp/go-msgpack/v2 v2.0.0 h1:c1fiLq1LNghmLOry1ipGhvLDi+/zEoaEP2JrE1oFJ9s= -github.com/hashicorp/go-msgpack/v2 v2.0.0/go.mod h1:JIxYkkFJRDDRSoWQBSh7s9QAVThq+82iWmUpmE4jKak= +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-multierror v0.0.0-20161216184304-ed905158d874/go.mod h1:JMRHfdO9jKNzS/+BTlxCjKNQHg/jZAft8U7LloJvN7I= github.com/hashicorp/go-multierror v1.0.0/go.mod h1:dHtQlpGsu+cZNNAkkCN/P3hoUDHhCYQXV3UM06sGGrk= github.com/hashicorp/go-multierror v1.1.0/go.mod h1:spPvp8C1qA32ftKqdAHm4hHTbPw+vmowP0z+KUhOZdA= @@ -2307,8 +2307,8 @@ github.com/hashicorp/raft v1.0.1/go.mod h1:DVSAWItjLjTOkVbSpWQ0j0kUADIvDaCtBxIcb github.com/hashicorp/raft v1.1.0/go.mod h1:4Ak7FSPnuvmb0GV6vgIAJ4vYT4bek9bb6Q+7HVbyzqM= github.com/hashicorp/raft v1.1.2-0.20191002163536-9c6bd3e3eb17/go.mod h1:vPAJM8Asw6u8LxC3eJCUZmRP/E4QmUGE1R7g7k8sG/8= github.com/hashicorp/raft v1.2.0/go.mod h1:vPAJM8Asw6u8LxC3eJCUZmRP/E4QmUGE1R7g7k8sG/8= -github.com/hashicorp/raft v1.3.10 h1:LR5QZX1VQd0DFWZfeCwWawyeKfpS/Tm1yjnJIY5X4Tw= -github.com/hashicorp/raft v1.3.10/go.mod h1:J8naEwc6XaaCfts7+28whSeRvCqTd6e20BlCU3LtEO4= +github.com/hashicorp/raft v1.4.0 h1:tn28S/AWv0BtRQgwZv/1NELu8sCvI0FixqL8C8MYKeY= +github.com/hashicorp/raft v1.4.0/go.mod h1:nz64BIjXphDLATfKGG5RzHtNUPioLeKFsXEm88yTVew= github.com/hashicorp/raft-autopilot v0.2.0 h1:2/R2RPgamgRKgNWGQioULZvjeKXQZmDuw5Ty+6c+H7Y= github.com/hashicorp/raft-autopilot v0.2.0/go.mod h1:q6tZ8UAZ5xio2gv2JvjgmtOlh80M6ic8xQYBe2Egkg8= github.com/hashicorp/raft-boltdb v0.0.0-20171010151810-6e5ba93211ea/go.mod h1:pNv7Wc3ycL6F5oOWn+tPGo2gWD4a5X+yp/ntwdKLjRk= @@ -3551,8 +3551,8 @@ golang.org/x/mod v0.8.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= golang.org/x/mod v0.9.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= golang.org/x/mod v0.10.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= golang.org/x/mod v0.11.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= -golang.org/x/mod v0.12.0 h1:rmsUpXtvNzj340zd98LZ4KntptpfRHwpFOHG188oHXc= -golang.org/x/mod v0.12.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= +golang.org/x/mod v0.13.0 h1:I/DsJXRlw/8l/0c24sM9yb0T4z9liZTduXvdAWYiysY= +golang.org/x/mod v0.13.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c= golang.org/x/net v0.0.0-20170114055629-f2499483f923/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= @@ -3705,8 +3705,9 @@ golang.org/x/sync v0.0.0-20220819030929-7fc1605a5dde/go.mod h1:RxMgew5VJxzue5/jJ golang.org/x/sync v0.0.0-20220929204114-8fcdb60fdcc0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.1.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.2.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sync v0.3.0 h1:ftCYgMx6zT/asHUrPw8BLLscYtGznsLAnjq5RH9P66E= golang.org/x/sync v0.3.0/go.mod h1:FU7BRWz2tNW+3quACPkgCx/L+uEAv1htQ0V83Z9Rj+Y= +golang.org/x/sync v0.4.0 h1:zxkM55ReGkDlKSM+Fu41A+zmbZuaPVbGMzvvdUPznYQ= +golang.org/x/sync v0.4.0/go.mod h1:FU7BRWz2tNW+3quACPkgCx/L+uEAv1htQ0V83Z9Rj+Y= golang.org/x/sys v0.0.0-20170830134202-bb24a47a89ea/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20180823144017-11551d06cbcc/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= @@ -4034,8 +4035,8 @@ golang.org/x/tools v0.7.0/go.mod h1:4pg6aUX35JBAogB10C9AtvVL+qowtN4pT3CGSQex14s= golang.org/x/tools v0.8.0/go.mod h1:JxBZ99ISMI5ViVkT1tr6tdNmXeTrcpVSD3vZ1RsRdN4= golang.org/x/tools v0.9.1/go.mod h1:owI94Op576fPu3cIGQeHs3joujW/2Oc6MtlxbF5dfNc= golang.org/x/tools v0.10.0/go.mod h1:UJwyiVBsOA2uwvK/e5OY3GTpDUJriEd+/YlqAwLPmyM= -golang.org/x/tools v0.12.1-0.20230815132531-74c255bcf846 h1:Vve/L0v7CXXuxUmaMGIEK/dEeq7uiqb5qBgQrZzIE7E= -golang.org/x/tools v0.12.1-0.20230815132531-74c255bcf846/go.mod h1:Sc0INKfu04TlqNoRA1hgpFZbhYXHPr4V5DzpSBTPqQM= +golang.org/x/tools v0.14.0 h1:jvNa2pY0M4r62jkRQ6RwEZZyPcymeL9XZMLBbV7U2nc= +golang.org/x/tools v0.14.0/go.mod h1:uYBEerGOWcJyEORxN+Ek8+TT266gXkNlHdJBwexUsBg= golang.org/x/xerrors v0.0.0-20190410155217-1f06c39b4373/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20190513163551-3ee3066db522/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= diff --git a/physical/raft/msgpack.go b/physical/raft/msgpack.go index b9b4f110486b..4fe8808a656d 100644 --- a/physical/raft/msgpack.go +++ b/physical/raft/msgpack.go @@ -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" ) diff --git a/physical/raft/raft.go b/physical/raft/raft.go index 0a46ff26ae2e..e1cb5f9d0a3a 100644 --- a/physical/raft/raft.go +++ b/physical/raft/raft.go @@ -431,8 +431,9 @@ func NewRaftBackend(conf map[string]string, logger log.Logger) (physical.Backend dbPath := filepath.Join(path, "raft.db") opts := etcdboltOptions(dbPath) raftOptions := raftboltdb.Options{ - Path: dbPath, - BoltOptions: opts, + Path: dbPath, + BoltOptions: opts, + MsgpackUseNewTimeFormat: true, } store, err := raftboltdb.New(raftOptions) if err != nil { @@ -903,6 +904,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 { @@ -997,11 +1001,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) diff --git a/physical/raft/raft_test.go b/physical/raft/raft_test.go index e7532b992a99..e4db89fb9955 100644 --- a/physical/raft/raft_test.go +++ b/physical/raft/raft_test.go @@ -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" @@ -22,12 +25,14 @@ import ( "github.com/go-test/deep" "github.com/golang/protobuf/proto" bolt "github.com/hashicorp-forge/bbolt" + "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" ) func connectPeers(nodes ...*RaftBackend) { @@ -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) + } +} diff --git a/physical/raft/testing.go b/physical/raft/testing.go index 632924950081..89d09747771a 100644 --- a/physical/raft/testing.go +++ b/physical/raft/testing.go @@ -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) @@ -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) } diff --git a/vault/cluster/cluster.go b/vault/cluster/cluster.go index 00967ac3c7a8..ed77acba1e72 100644 --- a/vault/cluster/cluster.go +++ b/vault/cluster/cluster.go @@ -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) From 4400331c77928c6988e5879661a5af2a2e8c362b Mon Sep 17 00:00:00 2001 From: Christopher Swenson Date: Wed, 25 Oct 2023 11:16:25 -0700 Subject: [PATCH 2/8] Update packages to temporary values for msgpack fix --- go.mod | 8 ++++---- go.sum | 16 +++++++++------- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/go.mod b/go.mod index 4a89fbcaa168..e951e32586cd 100644 --- a/go.mod +++ b/go.mod @@ -122,16 +122,16 @@ require ( github.com/hashicorp/go-syslog v1.0.0 github.com/hashicorp/go-uuid v1.0.3 github.com/hashicorp/go-version v1.6.0 - github.com/hashicorp/golang-lru v0.5.4 + github.com/hashicorp/golang-lru v1.0.2 github.com/hashicorp/hcl v1.0.1-vault-5 github.com/hashicorp/hcl/v2 v2.16.2 github.com/hashicorp/hcp-link v0.1.0 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.4.0 + github.com/hashicorp/raft v1.5.1-0.20231024165115-d79654935a01 github.com/hashicorp/raft-autopilot v0.2.0 - github.com/hashicorp/raft-boltdb/v2 v2.0.0-20210421194847-a7e34179d62c + github.com/hashicorp/raft-boltdb/v2 v2.2.3-0.20231024164814-4cd9b96914bd github.com/hashicorp/raft-snapshot v1.0.4 github.com/hashicorp/vault-plugin-auth-alicloud v0.16.0 github.com/hashicorp/vault-plugin-auth-azure v0.16.2 @@ -303,6 +303,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.1 // indirect @@ -388,7 +389,6 @@ require ( github.com/hailocab/go-hostpool v0.0.0-20160125115350-e80d13ce29ed // indirect github.com/hashicorp/cronexpr v1.1.1 // indirect github.com/hashicorp/go-immutable-radix v1.3.1 // indirect - github.com/hashicorp/go-msgpack v1.1.5 // indirect github.com/hashicorp/go-secure-stdlib/fileutil v0.1.0 // indirect github.com/hashicorp/go-secure-stdlib/plugincontainer v0.3.0 // indirect github.com/hashicorp/go-slug v0.12.1 // indirect diff --git a/go.sum b/go.sum index 7bcaef5c0c75..fe6601516dbf 100644 --- a/go.sum +++ b/go.sum @@ -1232,6 +1232,7 @@ github.com/blang/semver v3.5.1+incompatible/go.mod h1:kRBLl5iJ+tD4TcOOxsy/0fnweb github.com/blang/semver/v4 v4.0.0/go.mod h1:IbckMUScFkM3pff0VJDNKRiT6TG/YpiHIM2yvyW5YoQ= github.com/bmizerany/assert v0.0.0-20160611221934-b7ed37b82869 h1:DDGfHa7BWjL4YnC6+E63dPcxHo2sUxDIu8g3QgEJdRY= github.com/bmizerany/assert v0.0.0-20160611221934-b7ed37b82869/go.mod h1:Ekp36dRnpXw/yCqJaO+ZrUyxD+3VXMFFr56k5XYrpB4= +github.com/boltdb/bolt v1.3.1 h1:JQmyP4ZBrce+ZQu0dY660FMfatumYDLun9hBCUVIkF4= github.com/boltdb/bolt v1.3.1/go.mod h1:clJnj/oiGkjum5o1McbSZDSLxVThjynRyGBgiAx27Ps= github.com/boombuler/barcode v1.0.0/go.mod h1:paBWMcWSl3LHKBqUq+rly7CNSldXjb2rDl3JlRe0mD8= github.com/boombuler/barcode v1.0.1-0.20190219062509-6c824513bacc/go.mod h1:paBWMcWSl3LHKBqUq+rly7CNSldXjb2rDl3JlRe0mD8= @@ -2275,8 +2276,9 @@ github.com/hashicorp/go-version v1.6.0/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09 github.com/hashicorp/go.net v0.0.1/go.mod h1:hjKkEWcCURg++eb33jQU7oqQcI9XDCnUzHA0oac0k90= github.com/hashicorp/golang-lru v0.5.0/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ4Ao+sR/qLZy8= github.com/hashicorp/golang-lru v0.5.1/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ4Ao+sR/qLZy8= -github.com/hashicorp/golang-lru v0.5.4 h1:YDjusn29QI/Das2iO9M0BHnIbxPeyuCHsjMW+lJfyTc= github.com/hashicorp/golang-lru v0.5.4/go.mod h1:iADmTwqILo4mZ8BN3D2Q6+9jd8WM5uGBxy+E8yxSoD4= +github.com/hashicorp/golang-lru v1.0.2 h1:dV3g9Z/unq5DpblPpw+Oqcv4dU/1omnb4Ok8iPY6p1c= +github.com/hashicorp/golang-lru v1.0.2/go.mod h1:iADmTwqILo4mZ8BN3D2Q6+9jd8WM5uGBxy+E8yxSoD4= github.com/hashicorp/hcl v1.0.0/go.mod h1:E5yfLk+7swimpb2L/Alb/PJmXilQ/rhwaUYs4T20WEQ= github.com/hashicorp/hcl v1.0.1-vault-5 h1:kI3hhbbyzr4dldA8UdTb7ZlVVlI2DACdCfz31RPDgJM= github.com/hashicorp/hcl v1.0.1-vault-5/go.mod h1:XYhtn6ijBSAj6n4YqAaf7RBPS4I06AItNorpy+MoQNM= @@ -2304,16 +2306,17 @@ github.com/hashicorp/net-rpc-msgpackrpc/v2 v2.0.0/go.mod h1:6pdNz0vo0mF0GvhwDG56 github.com/hashicorp/nomad/api v0.0.0-20230519153805-2275a83cbfdf h1:cKXVf1UJqwdkGiTF3idqCOLApAql0310OSmJxeiaMWg= github.com/hashicorp/nomad/api v0.0.0-20230519153805-2275a83cbfdf/go.mod h1:rb38DqjaaIfhJRiLeCAGgIt+wV7o78rB+liyFE3mVzE= github.com/hashicorp/raft v1.0.1/go.mod h1:DVSAWItjLjTOkVbSpWQ0j0kUADIvDaCtBxIcbNAQLkI= -github.com/hashicorp/raft v1.1.0/go.mod h1:4Ak7FSPnuvmb0GV6vgIAJ4vYT4bek9bb6Q+7HVbyzqM= github.com/hashicorp/raft v1.1.2-0.20191002163536-9c6bd3e3eb17/go.mod h1:vPAJM8Asw6u8LxC3eJCUZmRP/E4QmUGE1R7g7k8sG/8= github.com/hashicorp/raft v1.2.0/go.mod h1:vPAJM8Asw6u8LxC3eJCUZmRP/E4QmUGE1R7g7k8sG/8= -github.com/hashicorp/raft v1.4.0 h1:tn28S/AWv0BtRQgwZv/1NELu8sCvI0FixqL8C8MYKeY= -github.com/hashicorp/raft v1.4.0/go.mod h1:nz64BIjXphDLATfKGG5RzHtNUPioLeKFsXEm88yTVew= +github.com/hashicorp/raft v1.5.1-0.20231024165115-d79654935a01 h1:tVBWxJGIubuWxxTryhTr5n7CrkapQ8E0PN/dkGZass0= +github.com/hashicorp/raft v1.5.1-0.20231024165115-d79654935a01/go.mod h1:Xil5pDgeGwRWuX4uPUmwa+7Vagg4N804dz6mhNi6S7o= github.com/hashicorp/raft-autopilot v0.2.0 h1:2/R2RPgamgRKgNWGQioULZvjeKXQZmDuw5Ty+6c+H7Y= github.com/hashicorp/raft-autopilot v0.2.0/go.mod h1:q6tZ8UAZ5xio2gv2JvjgmtOlh80M6ic8xQYBe2Egkg8= github.com/hashicorp/raft-boltdb v0.0.0-20171010151810-6e5ba93211ea/go.mod h1:pNv7Wc3ycL6F5oOWn+tPGo2gWD4a5X+yp/ntwdKLjRk= -github.com/hashicorp/raft-boltdb/v2 v2.0.0-20210421194847-a7e34179d62c h1:oiKun9QlrOz5yQxMZJ3tf1kWtFYuKSJzxzEDxDPevj4= -github.com/hashicorp/raft-boltdb/v2 v2.0.0-20210421194847-a7e34179d62c/go.mod h1:kiPs9g148eLShc2TYagUAyKDnD+dH9U+CQKsXzlY9xo= +github.com/hashicorp/raft-boltdb v0.0.0-20230125174641-2a8082862702 h1:RLKEcCuKcZ+qp2VlaaZsYZfLOmIiuJNpEi48Rl8u9cQ= +github.com/hashicorp/raft-boltdb v0.0.0-20230125174641-2a8082862702/go.mod h1:nTakvJ4XYq45UXtn0DbwR4aU9ZdjlnIenpbs6Cd+FM0= +github.com/hashicorp/raft-boltdb/v2 v2.2.3-0.20231024164814-4cd9b96914bd h1:oDlwkpj9oMENhrdvpMDfQ60zetCFp0F52Jll0YkpG3M= +github.com/hashicorp/raft-boltdb/v2 v2.2.3-0.20231024164814-4cd9b96914bd/go.mod h1:NBItTzQc6OLdltzc31TibGugcybZWRki30rr4EjRbYs= github.com/hashicorp/raft-snapshot v1.0.4 h1:EuDuayAJPdiDmVk1ygTDnG2zDzrs0/6/yBuma1IYSow= github.com/hashicorp/raft-snapshot v1.0.4/go.mod h1:5sL9eUn72lH5DzsFIJ9jaysITbHksSSszImWSOTC8Ic= github.com/hashicorp/serf v0.8.2/go.mod h1:6hOLApaqBFA1NXqRQAsxw9QxuDEvNxSQRwA/JwenrHc= @@ -3952,7 +3955,6 @@ golang.org/x/tools v0.0.0-20190328211700-ab21143f2384/go.mod h1:LCzVGOaR6xXOjkQ3 golang.org/x/tools v0.0.0-20190329151228-23e29df326fe/go.mod h1:LCzVGOaR6xXOjkQ3onu1FJEFr0SW1gC7cKk1uF8kGRs= golang.org/x/tools v0.0.0-20190416151739-9c9e1878f421/go.mod h1:LCzVGOaR6xXOjkQ3onu1FJEFr0SW1gC7cKk1uF8kGRs= golang.org/x/tools v0.0.0-20190420181800-aa740d480789/go.mod h1:LCzVGOaR6xXOjkQ3onu1FJEFr0SW1gC7cKk1uF8kGRs= -golang.org/x/tools v0.0.0-20190424220101-1e8e1cfdf96b/go.mod h1:RgjU9mgBXZiqYHBnxXauZ1Gv1EHHAz9KjViQ78xBX0Q= golang.org/x/tools v0.0.0-20190425150028-36563e24a262/go.mod h1:RgjU9mgBXZiqYHBnxXauZ1Gv1EHHAz9KjViQ78xBX0Q= golang.org/x/tools v0.0.0-20190425163242-31fd60d6bfdc/go.mod h1:RgjU9mgBXZiqYHBnxXauZ1Gv1EHHAz9KjViQ78xBX0Q= golang.org/x/tools v0.0.0-20190506145303-2d16b83fe98c/go.mod h1:RgjU9mgBXZiqYHBnxXauZ1Gv1EHHAz9KjViQ78xBX0Q= From c7c0c58b98de1549f17a70b1d0be1e0a2ecd73da Mon Sep 17 00:00:00 2001 From: Christopher Swenson Date: Wed, 25 Oct 2023 13:15:55 -0700 Subject: [PATCH 3/8] Don't run raft network test with race detector --- physical/raft/raft_network_test.go | 139 +++++++++++++++++++++++++++++ physical/raft/raft_test.go | 121 ------------------------- 2 files changed, 139 insertions(+), 121 deletions(-) create mode 100644 physical/raft/raft_network_test.go diff --git a/physical/raft/raft_network_test.go b/physical/raft/raft_network_test.go new file mode 100644 index 000000000000..0627698eb4d7 --- /dev/null +++ b/physical/raft/raft_network_test.go @@ -0,0 +1,139 @@ +//go:build !race + +// The raft networking layer tends to reset the TLS keyring, which triggers +// the race detector even though it should be a no-op. + +package raft + +import ( + "bytes" + "context" + "crypto/rand" + "crypto/tls" + "fmt" + "net" + "os" + "testing" + "time" + + "github.com/hashicorp/consul/sdk/freeport" + "github.com/hashicorp/go-hclog" + "github.com/hashicorp/vault/sdk/physical" + "github.com/hashicorp/vault/vault/cluster" +) + +// 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(rand.Reader) + if err != nil { + t.Fatal(err) + } + key2, err := GenerateTLSKey(rand.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) + } +} diff --git a/physical/raft/raft_test.go b/physical/raft/raft_test.go index e4db89fb9955..e7532b992a99 100644 --- a/physical/raft/raft_test.go +++ b/physical/raft/raft_test.go @@ -7,15 +7,12 @@ 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" @@ -25,14 +22,12 @@ import ( "github.com/go-test/deep" "github.com/golang/protobuf/proto" bolt "github.com/hashicorp-forge/bbolt" - "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" ) func connectPeers(nodes ...*RaftBackend) { @@ -768,119 +763,3 @@ 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) - } -} From e754182b8f425933e63841e42e6a64c241a02eba Mon Sep 17 00:00:00 2001 From: Christopher Swenson Date: Wed, 25 Oct 2023 13:36:26 -0700 Subject: [PATCH 4/8] Add copywrite header --- physical/raft/raft_network_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/physical/raft/raft_network_test.go b/physical/raft/raft_network_test.go index 0627698eb4d7..c797512d4bb8 100644 --- a/physical/raft/raft_network_test.go +++ b/physical/raft/raft_network_test.go @@ -1,3 +1,6 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + //go:build !race // The raft networking layer tends to reset the TLS keyring, which triggers From d52ba8be5f0cf089154a7daba17a30f258b1d884 Mon Sep 17 00:00:00 2001 From: Christopher Swenson Date: Thu, 26 Oct 2023 12:00:10 -0700 Subject: [PATCH 5/8] Add benchmarks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I added a simple pair of benchmarks (one with a final sync, one without) and ran them before and after on both my Mac (M2 Max) laptop and my Linux (AMD Threadripper 3970X) desktop. tl;dr There was no performance difference for this benchmark. ``` goos: darwin goarch: arm64 pkg: github.com/hashicorp/vault/physical/raft │ a.txt │ b.txt │ │ sec/op │ sec/op vs base │ RaftWithNetwork-10 58.65m ± 2% 58.62m ± 2% ~ (p=0.937 n=6) ``` ``` goos: linux goarch: amd64 pkg: github.com/hashicorp/vault/physical/raft cpu: AMD Ryzen Threadripper 3970X 32-Core Processor │ c.txt │ d.txt │ │ sec/op │ sec/op vs base │ RaftWithNetwork-64 5.861m ± 1% 5.837m ± 0% ~ (p=0.240 n=6) ``` --- physical/raft/raft_network_test.go | 117 ++++++++++++++++++----------- 1 file changed, 74 insertions(+), 43 deletions(-) diff --git a/physical/raft/raft_network_test.go b/physical/raft/raft_network_test.go index c797512d4bb8..1130d1d0ab0d 100644 --- a/physical/raft/raft_network_test.go +++ b/physical/raft/raft_network_test.go @@ -25,12 +25,64 @@ import ( "github.com/hashicorp/vault/vault/cluster" ) +func BenchmarkRaftWithNetwork(b *testing.B) { + b.StopTimer() + raft1, _ := createRaftNetworkCluster(b, true, false) + b.StartTimer() + for i := 0; i < b.N; i++ { + err := raft1.Put(context.Background(), &physical.Entry{ + Key: "test", + Value: []byte{byte(i)}, + }) + if err != nil { + b.Fatal(err) + } + snapOut, err := os.CreateTemp(b.TempDir(), "bench") + if err != nil { + b.Fatal(err) + } + b.Cleanup(func() { + _ = snapOut.Close() + _ = os.Remove(snapOut.Name()) + }) + err = raft1.Snapshot(snapOut, nil) + if err != nil { + b.Fatal(err) + } + } +} + // 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 + raft1, raft2 := createRaftNetworkCluster(t, true, false) + for i := 0; i < 10; 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 createRaftNetworkCluster(tb testing.TB, overrideTimeFormat1, overrideTimeFormat2 bool) (*RaftBackend, *RaftBackend) { cipherSuites := []uint16{ // 1.3 tls.TLS_AES_128_GCM_SHA256, @@ -38,23 +90,23 @@ func TestRaftNetworkClusterWithMultipleTimeEncodingsSet(t *testing.T) { tls.TLS_CHACHA20_POLY1305_SHA256, } - port1 := freeport.GetOne(t) - port2 := freeport.GetOne(t) + port1 := freeport.GetOne(tb) + port2 := freeport.GetOne(tb) addr1, err := net.ResolveTCPAddr("tcp", fmt.Sprintf("127.0.0.1:%d", port1)) if err != nil { - t.Fatal(err) + tb.Fatal(err) } addr2, err := net.ResolveTCPAddr("tcp", fmt.Sprintf("127.0.0.1:%d", port2)) if err != nil { - t.Fatal(err) + tb.Fatal(err) } key1, err := GenerateTLSKey(rand.Reader) if err != nil { - t.Fatal(err) + tb.Fatal(err) } key2, err := GenerateTLSKey(rand.Reader) if err != nil { - t.Fatal(err) + tb.Fatal(err) } logger1 := hclog.New(&hclog.LoggerOptions{ Name: "raft1", @@ -68,75 +120,54 @@ func TestRaftNetworkClusterWithMultipleTimeEncodingsSet(t *testing.T) { 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) + tb.Cleanup(listener1.Stop) + tb.Cleanup(listener2.Stop) - raft1, dir1 := GetRaftWithOpts(t, true, true, SetupOpts{ + raft1, dir1 := GetRaftWithOpts(tb, true, true, SetupOpts{ TLSKeyring: &TLSKeyring{ Keys: []*TLSKey{key1, key2}, ActiveKeyID: key1.ID, }, - ClusterListener: listener1, + ClusterListener: listener1, + overrideMsgpackUseNewTimeFormat: &overrideTimeFormat1, }) - overrideTimeFormatFalse := false setupOpts2 := SetupOpts{ TLSKeyring: &TLSKeyring{ Keys: []*TLSKey{key2, key1}, ActiveKeyID: key2.ID, }, ClusterListener: listener2, - overrideMsgpackUseNewTimeFormat: &overrideTimeFormatFalse, + overrideMsgpackUseNewTimeFormat: &overrideTimeFormat2, } - raft2, dir2 := GetRaftWithOpts(t, false, true, setupOpts2) + raft2, dir2 := GetRaftWithOpts(tb, false, true, setupOpts2) defer os.RemoveAll(dir1) defer os.RemoveAll(dir2) // Add raft2 to the cluster - addNetworkPeer(t, raft1, raft2, addr2, setupOpts2) + addNetworkPeer(tb, 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) - } + return raft1, raft2 } -func addNetworkPeer(t *testing.T, leader, follower *RaftBackend, followerAddr *net.TCPAddr, setupOpts SetupOpts) { - t.Helper() +func addNetworkPeer(tb testing.TB, leader, follower *RaftBackend, followerAddr *net.TCPAddr, setupOpts SetupOpts) { + tb.Helper() if err := leader.AddPeer(context.Background(), follower.NodeID(), followerAddr.String()); err != nil { - t.Fatal(err) + tb.Fatal(err) } peers, err := leader.Peers(context.Background()) if err != nil { - t.Fatal(err) + tb.Fatal(err) } err = follower.Bootstrap(peers) if err != nil { - t.Fatal(err) + tb.Fatal(err) } err = follower.SetupCluster(context.Background(), setupOpts) if err != nil { - t.Fatal(err) + tb.Fatal(err) } } From dab378ad7081b83a9f2fddeceef15712af5db70b Mon Sep 17 00:00:00 2001 From: Christopher Swenson Date: Thu, 26 Oct 2023 13:39:21 -0700 Subject: [PATCH 6/8] Update test for reduced PUT count --- physical/raft/raft_network_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/physical/raft/raft_network_test.go b/physical/raft/raft_network_test.go index 1130d1d0ab0d..caebe1342e6e 100644 --- a/physical/raft/raft_network_test.go +++ b/physical/raft/raft_network_test.go @@ -77,8 +77,8 @@ func TestRaftNetworkClusterWithMultipleTimeEncodingsSet(t *testing.T) { 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) + if !bytes.Equal(entry.Value, []byte{9}) { + t.Errorf("Expected {9} but got %+v", entry.Value) } } From 12b6ee251994cbcd5017a638b1fc9c849dccfc03 Mon Sep 17 00:00:00 2001 From: Christopher Swenson Date: Wed, 15 Nov 2023 10:03:03 -0800 Subject: [PATCH 7/8] Update raft and raft-boltdb --- go.mod | 4 ++-- go.sum | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/go.mod b/go.mod index e951e32586cd..6d1466cdee07 100644 --- a/go.mod +++ b/go.mod @@ -129,9 +129,9 @@ 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.5.1-0.20231024165115-d79654935a01 + github.com/hashicorp/raft v1.6.0 github.com/hashicorp/raft-autopilot v0.2.0 - github.com/hashicorp/raft-boltdb/v2 v2.2.3-0.20231024164814-4cd9b96914bd + github.com/hashicorp/raft-boltdb/v2 v2.3.0 github.com/hashicorp/raft-snapshot v1.0.4 github.com/hashicorp/vault-plugin-auth-alicloud v0.16.0 github.com/hashicorp/vault-plugin-auth-azure v0.16.2 diff --git a/go.sum b/go.sum index fe6601516dbf..7454c192a323 100644 --- a/go.sum +++ b/go.sum @@ -2308,15 +2308,15 @@ github.com/hashicorp/nomad/api v0.0.0-20230519153805-2275a83cbfdf/go.mod h1:rb38 github.com/hashicorp/raft v1.0.1/go.mod h1:DVSAWItjLjTOkVbSpWQ0j0kUADIvDaCtBxIcbNAQLkI= github.com/hashicorp/raft v1.1.2-0.20191002163536-9c6bd3e3eb17/go.mod h1:vPAJM8Asw6u8LxC3eJCUZmRP/E4QmUGE1R7g7k8sG/8= github.com/hashicorp/raft v1.2.0/go.mod h1:vPAJM8Asw6u8LxC3eJCUZmRP/E4QmUGE1R7g7k8sG/8= -github.com/hashicorp/raft v1.5.1-0.20231024165115-d79654935a01 h1:tVBWxJGIubuWxxTryhTr5n7CrkapQ8E0PN/dkGZass0= -github.com/hashicorp/raft v1.5.1-0.20231024165115-d79654935a01/go.mod h1:Xil5pDgeGwRWuX4uPUmwa+7Vagg4N804dz6mhNi6S7o= +github.com/hashicorp/raft v1.6.0 h1:tkIAORZy2GbJ2Trp5eUSggLXDPOJLXC+JJLNMMqtgtM= +github.com/hashicorp/raft v1.6.0/go.mod h1:Xil5pDgeGwRWuX4uPUmwa+7Vagg4N804dz6mhNi6S7o= github.com/hashicorp/raft-autopilot v0.2.0 h1:2/R2RPgamgRKgNWGQioULZvjeKXQZmDuw5Ty+6c+H7Y= github.com/hashicorp/raft-autopilot v0.2.0/go.mod h1:q6tZ8UAZ5xio2gv2JvjgmtOlh80M6ic8xQYBe2Egkg8= github.com/hashicorp/raft-boltdb v0.0.0-20171010151810-6e5ba93211ea/go.mod h1:pNv7Wc3ycL6F5oOWn+tPGo2gWD4a5X+yp/ntwdKLjRk= github.com/hashicorp/raft-boltdb v0.0.0-20230125174641-2a8082862702 h1:RLKEcCuKcZ+qp2VlaaZsYZfLOmIiuJNpEi48Rl8u9cQ= github.com/hashicorp/raft-boltdb v0.0.0-20230125174641-2a8082862702/go.mod h1:nTakvJ4XYq45UXtn0DbwR4aU9ZdjlnIenpbs6Cd+FM0= -github.com/hashicorp/raft-boltdb/v2 v2.2.3-0.20231024164814-4cd9b96914bd h1:oDlwkpj9oMENhrdvpMDfQ60zetCFp0F52Jll0YkpG3M= -github.com/hashicorp/raft-boltdb/v2 v2.2.3-0.20231024164814-4cd9b96914bd/go.mod h1:NBItTzQc6OLdltzc31TibGugcybZWRki30rr4EjRbYs= +github.com/hashicorp/raft-boltdb/v2 v2.3.0 h1:fPpQR1iGEVYjZ2OELvUHX600VAK5qmdnDEv3eXOwZUA= +github.com/hashicorp/raft-boltdb/v2 v2.3.0/go.mod h1:YHukhB04ChJsLHLJEUD6vjFyLX2L3dsX3wPBZcX4tmc= github.com/hashicorp/raft-snapshot v1.0.4 h1:EuDuayAJPdiDmVk1ygTDnG2zDzrs0/6/yBuma1IYSow= github.com/hashicorp/raft-snapshot v1.0.4/go.mod h1:5sL9eUn72lH5DzsFIJ9jaysITbHksSSszImWSOTC8Ic= github.com/hashicorp/serf v0.8.2/go.mod h1:6hOLApaqBFA1NXqRQAsxw9QxuDEvNxSQRwA/JwenrHc= From 9239f71b6ecfa57733738b4a1057e06559f2f57c Mon Sep 17 00:00:00 2001 From: Christopher Swenson Date: Fri, 22 Dec 2023 10:28:45 -0800 Subject: [PATCH 8/8] Address PR comments --- physical/raft/msgpack.go | 13 --- physical/raft/raft.go | 6 - physical/raft/raft_network_test.go | 173 ----------------------------- physical/raft/testing.go | 10 +- 4 files changed, 3 insertions(+), 199 deletions(-) delete mode 100644 physical/raft/msgpack.go delete mode 100644 physical/raft/raft_network_test.go diff --git a/physical/raft/msgpack.go b/physical/raft/msgpack.go deleted file mode 100644 index 4fe8808a656d..000000000000 --- a/physical/raft/msgpack.go +++ /dev/null @@ -1,13 +0,0 @@ -// Copyright (c) HashiCorp, Inc. -// SPDX-License-Identifier: BUSL-1.1 - -package raft - -// If we downgrade msgpack from v1.1.5 to v0.5.5, everything will still -// work, but any pre-existing raft clusters will break on upgrade. -// This file exists so that the Vault project has an explicit dependency -// on the library, which allows us to pin the version in go.mod. - -import ( - _ "github.com/hashicorp/go-msgpack/v2/codec" -) diff --git a/physical/raft/raft.go b/physical/raft/raft.go index e1cb5f9d0a3a..e937697777ad 100644 --- a/physical/raft/raft.go +++ b/physical/raft/raft.go @@ -904,9 +904,6 @@ 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 { @@ -1008,9 +1005,6 @@ func (b *RaftBackend) SetupCluster(ctx context.Context, opts SetupOpts) error { Logger: b.logger.Named("raft-net"), MsgpackUseNewTimeFormat: true, } - if opts.overrideMsgpackUseNewTimeFormat != nil { - transConfig.MsgpackUseNewTimeFormat = *opts.overrideMsgpackUseNewTimeFormat - } transport := raft.NewNetworkTransportWithConfig(transConfig) b.streamLayer = streamLayer diff --git a/physical/raft/raft_network_test.go b/physical/raft/raft_network_test.go deleted file mode 100644 index caebe1342e6e..000000000000 --- a/physical/raft/raft_network_test.go +++ /dev/null @@ -1,173 +0,0 @@ -// Copyright (c) HashiCorp, Inc. -// SPDX-License-Identifier: BUSL-1.1 - -//go:build !race - -// The raft networking layer tends to reset the TLS keyring, which triggers -// the race detector even though it should be a no-op. - -package raft - -import ( - "bytes" - "context" - "crypto/rand" - "crypto/tls" - "fmt" - "net" - "os" - "testing" - "time" - - "github.com/hashicorp/consul/sdk/freeport" - "github.com/hashicorp/go-hclog" - "github.com/hashicorp/vault/sdk/physical" - "github.com/hashicorp/vault/vault/cluster" -) - -func BenchmarkRaftWithNetwork(b *testing.B) { - b.StopTimer() - raft1, _ := createRaftNetworkCluster(b, true, false) - b.StartTimer() - for i := 0; i < b.N; i++ { - err := raft1.Put(context.Background(), &physical.Entry{ - Key: "test", - Value: []byte{byte(i)}, - }) - if err != nil { - b.Fatal(err) - } - snapOut, err := os.CreateTemp(b.TempDir(), "bench") - if err != nil { - b.Fatal(err) - } - b.Cleanup(func() { - _ = snapOut.Close() - _ = os.Remove(snapOut.Name()) - }) - err = raft1.Snapshot(snapOut, nil) - if err != nil { - b.Fatal(err) - } - } -} - -// 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) { - raft1, raft2 := createRaftNetworkCluster(t, true, false) - for i := 0; i < 10; 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{9}) { - t.Errorf("Expected {9} but got %+v", entry.Value) - } -} - -func createRaftNetworkCluster(tb testing.TB, overrideTimeFormat1, overrideTimeFormat2 bool) (*RaftBackend, *RaftBackend) { - cipherSuites := []uint16{ - // 1.3 - tls.TLS_AES_128_GCM_SHA256, - tls.TLS_AES_256_GCM_SHA384, - tls.TLS_CHACHA20_POLY1305_SHA256, - } - - port1 := freeport.GetOne(tb) - port2 := freeport.GetOne(tb) - addr1, err := net.ResolveTCPAddr("tcp", fmt.Sprintf("127.0.0.1:%d", port1)) - if err != nil { - tb.Fatal(err) - } - addr2, err := net.ResolveTCPAddr("tcp", fmt.Sprintf("127.0.0.1:%d", port2)) - if err != nil { - tb.Fatal(err) - } - key1, err := GenerateTLSKey(rand.Reader) - if err != nil { - tb.Fatal(err) - } - key2, err := GenerateTLSKey(rand.Reader) - if err != nil { - tb.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()) - tb.Cleanup(listener1.Stop) - tb.Cleanup(listener2.Stop) - - raft1, dir1 := GetRaftWithOpts(tb, true, true, SetupOpts{ - TLSKeyring: &TLSKeyring{ - Keys: []*TLSKey{key1, key2}, - ActiveKeyID: key1.ID, - }, - ClusterListener: listener1, - overrideMsgpackUseNewTimeFormat: &overrideTimeFormat1, - }) - - setupOpts2 := SetupOpts{ - TLSKeyring: &TLSKeyring{ - Keys: []*TLSKey{key2, key1}, - ActiveKeyID: key2.ID, - }, - ClusterListener: listener2, - overrideMsgpackUseNewTimeFormat: &overrideTimeFormat2, - } - raft2, dir2 := GetRaftWithOpts(tb, false, true, setupOpts2) - defer os.RemoveAll(dir1) - defer os.RemoveAll(dir2) - - // Add raft2 to the cluster - addNetworkPeer(tb, raft1, raft2, addr2, setupOpts2) - - return raft1, raft2 -} - -func addNetworkPeer(tb testing.TB, leader, follower *RaftBackend, followerAddr *net.TCPAddr, setupOpts SetupOpts) { - tb.Helper() - if err := leader.AddPeer(context.Background(), follower.NodeID(), followerAddr.String()); err != nil { - tb.Fatal(err) - } - - peers, err := leader.Peers(context.Background()) - if err != nil { - tb.Fatal(err) - } - - err = follower.Bootstrap(peers) - if err != nil { - tb.Fatal(err) - } - - err = follower.SetupCluster(context.Background(), setupOpts) - if err != nil { - tb.Fatal(err) - } -} diff --git a/physical/raft/testing.go b/physical/raft/testing.go index 89d09747771a..632924950081 100644 --- a/physical/raft/testing.go +++ b/physical/raft/testing.go @@ -14,20 +14,16 @@ 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, setupOpts) + return getRaftWithDir(t, bootstrap, noStoreState, raftDir) } -func getRaftWithDir(t testing.TB, bootstrap bool, noStoreState bool, raftDir string, setupOpts SetupOpts) (*RaftBackend, string) { +func getRaftWithDir(t testing.TB, bootstrap bool, noStoreState bool, raftDir string) (*RaftBackend, string) { id, err := uuid.GenerateUUID() if err != nil { t.Fatal(err) @@ -66,7 +62,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) }