From 4c928cb2f766ebcd55067b5a5658729a4f3cf00c Mon Sep 17 00:00:00 2001 From: "Chris S. Kim" Date: Thu, 11 Aug 2022 14:47:10 -0400 Subject: [PATCH] Handle breaking change for ServiceVirtualIP restore (#14149) Consul 1.13.0 changed ServiceVirtualIP to use PeeredServiceName instead of ServiceName which was a breaking change for those using service mesh and wanted to restore their snapshot after upgrading to 1.13.0. This commit handles existing data with older ServiceName and converts it during restore so that there are no issues when restoring from older snapshots. --- .changelog/14149.txt | 3 ++ agent/consul/fsm/snapshot_oss.go | 40 ++++++++++++++++- agent/consul/fsm/snapshot_oss_test.go | 64 +++++++++++++++++++++++++++ agent/structs/structs.go | 4 +- 4 files changed, 107 insertions(+), 4 deletions(-) create mode 100644 .changelog/14149.txt diff --git a/.changelog/14149.txt b/.changelog/14149.txt new file mode 100644 index 0000000000..726861f5a2 --- /dev/null +++ b/.changelog/14149.txt @@ -0,0 +1,3 @@ +```release-note:bug +agent: Fixed a compatibility issue when restoring snapshots from pre-1.13.0 versions of Consul [[GH-14107](https://github.com/hashicorp/consul/issues/14107)] +``` \ No newline at end of file diff --git a/agent/consul/fsm/snapshot_oss.go b/agent/consul/fsm/snapshot_oss.go index 167ffd100b..7fa53381a7 100644 --- a/agent/consul/fsm/snapshot_oss.go +++ b/agent/consul/fsm/snapshot_oss.go @@ -1,8 +1,12 @@ package fsm import ( + "fmt" + "net" + "github.com/hashicorp/consul-net-rpc/go-msgpack/codec" "github.com/hashicorp/raft" + "github.com/mitchellh/mapstructure" "github.com/hashicorp/consul/agent/consul/state" "github.com/hashicorp/consul/agent/structs" @@ -886,11 +890,43 @@ func restoreSystemMetadata(header *SnapshotHeader, restore *state.Restore, decod } func restoreServiceVirtualIP(header *SnapshotHeader, restore *state.Restore, decoder *codec.Decoder) error { - var req state.ServiceVirtualIP + // state.ServiceVirtualIP was changed in a breaking way in 1.13.0 (2e4cb6f77d2be36b02e9be0b289b24e5b0afb794). + // We attempt to reconcile the older type by decoding to a map then decoding that map into + // structs.PeeredServiceName first, and then structs.ServiceName. + var req struct { + Service map[string]interface{} + IP net.IP + + structs.RaftIndex + } if err := decoder.Decode(&req); err != nil { return err } - if err := restore.ServiceVirtualIP(req); err != nil { + + vip := state.ServiceVirtualIP{ + IP: req.IP, + RaftIndex: req.RaftIndex, + } + + // PeeredServiceName is the expected primary key type. + var psn structs.PeeredServiceName + if err := mapstructure.Decode(req.Service, &psn); err != nil { + return fmt.Errorf("cannot decode to structs.PeeredServiceName: %w", err) + } + vip.Service = psn + + // If the expected primary key field is empty, it must be the older ServiceName type. + if vip.Service.ServiceName.Name == "" { + var sn structs.ServiceName + if err := mapstructure.Decode(req.Service, &sn); err != nil { + return fmt.Errorf("cannot decode to structs.ServiceName: %w", err) + } + vip.Service = structs.PeeredServiceName{ + ServiceName: sn, + } + } + + if err := restore.ServiceVirtualIP(vip); err != nil { return err } return nil diff --git a/agent/consul/fsm/snapshot_oss_test.go b/agent/consul/fsm/snapshot_oss_test.go index b893c73bc7..2b2d3e8701 100644 --- a/agent/consul/fsm/snapshot_oss_test.go +++ b/agent/consul/fsm/snapshot_oss_test.go @@ -3,6 +3,7 @@ package fsm import ( "bytes" "fmt" + "net" "testing" "time" @@ -962,3 +963,66 @@ func TestFSM_BadSnapshot_NilCAConfig(t *testing.T) { require.EqualValues(t, 0, idx) require.Nil(t, config) } + +// This test asserts that ServiceVirtualIP, which made a breaking change +// in 1.13.0, can still restore from older snapshots which use the old +// state.ServiceVirtualIP type. +func Test_restoreServiceVirtualIP(t *testing.T) { + psn := structs.PeeredServiceName{ + ServiceName: structs.ServiceName{ + Name: "foo", + }, + } + + run := func(t *testing.T, input interface{}) { + t.Helper() + + var b []byte + buf := bytes.NewBuffer(b) + // Encode input + encoder := codec.NewEncoder(buf, structs.MsgpackHandle) + require.NoError(t, encoder.Encode(input)) + + // Create a decoder + dec := codec.NewDecoder(buf, structs.MsgpackHandle) + + logger := testutil.Logger(t) + fsm, err := New(nil, logger) + require.NoError(t, err) + + restore := fsm.State().Restore() + + // Call restore + require.NoError(t, restoreServiceVirtualIP(nil, restore, dec)) + require.NoError(t, restore.Commit()) + + ip, err := fsm.State().VirtualIPForService(psn) + require.NoError(t, err) + + // 240->224 due to addIPOffset + require.Equal(t, "224.0.0.2", ip) + } + + t.Run("new ServiceVirtualIP with PeeredServiceName", func(t *testing.T) { + run(t, state.ServiceVirtualIP{ + Service: psn, + IP: net.ParseIP("240.0.0.2"), + RaftIndex: structs.RaftIndex{}, + }) + }) + t.Run("pre-1.13.0 ServiceVirtualIP with ServiceName", func(t *testing.T) { + type compatServiceVirtualIP struct { + Service structs.ServiceName + IP net.IP + RaftIndex structs.RaftIndex + } + + run(t, compatServiceVirtualIP{ + Service: structs.ServiceName{ + Name: "foo", + }, + IP: net.ParseIP("240.0.0.2"), + RaftIndex: structs.RaftIndex{}, + }) + }) +} diff --git a/agent/structs/structs.go b/agent/structs/structs.go index 4821b164c3..22fb47ca9d 100644 --- a/agent/structs/structs.go +++ b/agent/structs/structs.go @@ -2211,8 +2211,8 @@ type PeeredServiceName struct { } type ServiceName struct { - Name string - acl.EnterpriseMeta + Name string + acl.EnterpriseMeta `mapstructure:",squash"` } func NewServiceName(name string, entMeta *acl.EnterpriseMeta) ServiceName {