From 26708570c5b63b6fdec74a8b846e1f2523a58a3a Mon Sep 17 00:00:00 2001 From: Matt Keeler Date: Thu, 2 May 2019 15:25:29 -0400 Subject: [PATCH] Fix ConfigEntryResponse binary marshaller and ensure we watch the chan in ConfigEntry.Get even when no entry exists. (#5773) --- agent/consul/state/config_entry.go | 2 +- agent/structs/config_entry.go | 35 ++++++++++++++++--------- agent/structs/config_entry_test.go | 42 ++++++++++++++++++++++++++++++ 3 files changed, 66 insertions(+), 13 deletions(-) diff --git a/agent/consul/state/config_entry.go b/agent/consul/state/config_entry.go index ad90b1818..e5eb4210b 100644 --- a/agent/consul/state/config_entry.go +++ b/agent/consul/state/config_entry.go @@ -92,10 +92,10 @@ func (s *Store) ConfigEntry(ws memdb.WatchSet, kind, name string) (uint64, struc if err != nil { return 0, nil, fmt.Errorf("failed config entry lookup: %s", err) } + ws.Add(watchCh) if existing == nil { return idx, nil, nil } - ws.Add(watchCh) conf, ok := existing.(structs.ConfigEntry) if !ok { diff --git a/agent/structs/config_entry.go b/agent/structs/config_entry.go index bbe2232c5..1dff93ca8 100644 --- a/agent/structs/config_entry.go +++ b/agent/structs/config_entry.go @@ -479,12 +479,19 @@ func (c *ConfigEntryResponse) MarshalBinary() (data []byte, err error) { bs := make([]byte, 128) enc := codec.NewEncoderBytes(&bs, msgpackHandle) - if err := enc.Encode(c.Entry.GetKind()); err != nil { - return nil, err - } - if err := enc.Encode(c.Entry); err != nil { - return nil, err + if c.Entry != nil { + if err := enc.Encode(c.Entry.GetKind()); err != nil { + return nil, err + } + if err := enc.Encode(c.Entry); err != nil { + return nil, err + } + } else { + if err := enc.Encode(""); err != nil { + return nil, err + } } + if err := enc.Encode(c.QueryMeta); err != nil { return nil, err } @@ -500,15 +507,19 @@ func (c *ConfigEntryResponse) UnmarshalBinary(data []byte) error { return err } - entry, err := MakeConfigEntry(kind, "") - if err != nil { - return err - } + if kind != "" { + entry, err := MakeConfigEntry(kind, "") + if err != nil { + return err + } - if err := dec.Decode(entry); err != nil { - return err + if err := dec.Decode(entry); err != nil { + return err + } + c.Entry = entry + } else { + c.Entry = nil } - c.Entry = entry if err := dec.Decode(&c.QueryMeta); err != nil { return err diff --git a/agent/structs/config_entry_test.go b/agent/structs/config_entry_test.go index d34287b58..583d6933c 100644 --- a/agent/structs/config_entry_test.go +++ b/agent/structs/config_entry_test.go @@ -143,3 +143,45 @@ func TestServiceConfigResponse_MsgPack(t *testing.T) { require.Equal(t, a, b) } + +func TestConfigEntryResponseMarshalling(t *testing.T) { + t.Parallel() + + cases := map[string]ConfigEntryResponse{ + "nil entry": ConfigEntryResponse{}, + "proxy-default entry": ConfigEntryResponse{ + Entry: &ProxyConfigEntry{ + Kind: ProxyDefaults, + Name: ProxyConfigGlobal, + Config: map[string]interface{}{ + "foo": "bar", + }, + }, + }, + "service-default entry": ConfigEntryResponse{ + Entry: &ServiceConfigEntry{ + Kind: ServiceDefaults, + Name: "foo", + Protocol: "tcp", + // Connect: ConnectConfiguration{SideCarProxy: true}, + }, + }, + } + + for name, tcase := range cases { + name := name + tcase := tcase + t.Run(name, func(t *testing.T) { + t.Parallel() + + data, err := tcase.MarshalBinary() + require.NoError(t, err) + require.NotEmpty(t, data) + + var resp ConfigEntryResponse + require.NoError(t, resp.UnmarshalBinary(data)) + + require.Equal(t, tcase, resp) + }) + } +}