Fix ConfigEntryResponse binary marshaller and ensure we watch the cha…

…n in ConfigEntry.Get even when no entry exists. (#5773)
hashicorp · May 2, 2019 · 42d32db · 42d32db
1 parent 1452f4d
commit 42d32db
Show file tree

Hide file tree

Showing 3 changed files with 66 additions and 13 deletions.
diff --git 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
@@ -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
@@ -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)
+		})
+	}
+}