diff --git a/agent/consul/config_endpoint_test.go b/agent/consul/config_endpoint_test.go index 7dde5061db..bc035d5a49 100644 --- a/agent/consul/config_endpoint_test.go +++ b/agent/consul/config_endpoint_test.go @@ -6,7 +6,6 @@ import ( "github.com/hashicorp/consul/acl" "github.com/hashicorp/consul/agent/structs" - "github.com/hashicorp/consul/lib" "github.com/hashicorp/consul/testrpc" msgpackrpc "github.com/hashicorp/net-rpc-msgpackrpc" "github.com/stretchr/testify/require" @@ -681,15 +680,6 @@ func TestConfigEntry_ResolveServiceConfig(t *testing.T) { } var out structs.ServiceConfigResponse require.NoError(msgpackrpc.CallWithCodec(codec, "ConfigEntry.ResolveServiceConfig", &args, &out)) - // Hack to fix up the string encoding in the map[string]interface{}. - // msgpackRPC's codec doesn't use RawToString. - var err error - out.ProxyConfig, err = lib.MapWalk(out.ProxyConfig) - require.NoError(err) - for k := range out.UpstreamConfigs { - out.UpstreamConfigs[k], err = lib.MapWalk(out.UpstreamConfigs[k]) - require.NoError(err) - } expected := structs.ServiceConfigResponse{ ProxyConfig: map[string]interface{}{ diff --git a/agent/structs/config_entry.go b/agent/structs/config_entry.go index 1ab8c2b08b..bbe2232c53 100644 --- a/agent/structs/config_entry.go +++ b/agent/structs/config_entry.go @@ -419,6 +419,54 @@ type ServiceConfigResponse struct { QueryMeta } +// MarshalBinary writes ServiceConfigResponse as msgpack encoded. It's only here +// because we need custom decoding of the raw interface{} values. +func (r *ServiceConfigResponse) MarshalBinary() (data []byte, err error) { + // bs will grow if needed but allocate enough to avoid reallocation in common + // case. + bs := make([]byte, 128) + enc := codec.NewEncoderBytes(&bs, msgpackHandle) + + type Alias ServiceConfigResponse + + if err := enc.Encode((*Alias)(r)); err != nil { + return nil, err + } + + return bs, nil +} + +// UnmarshalBinary decodes msgpack encoded ServiceConfigResponse. It used +// default msgpack encoding but fixes up the uint8 strings and other problems we +// have with encoding map[string]interface{}. +func (r *ServiceConfigResponse) UnmarshalBinary(data []byte) error { + dec := codec.NewDecoderBytes(data, msgpackHandle) + + type Alias ServiceConfigResponse + var a Alias + + if err := dec.Decode(&a); err != nil { + return err + } + + *r = ServiceConfigResponse(a) + + var err error + + // Fix strings and maps in the returned maps + r.ProxyConfig, err = lib.MapWalk(r.ProxyConfig) + if err != nil { + return err + } + for k := range r.UpstreamConfigs { + r.UpstreamConfigs[k], err = lib.MapWalk(r.UpstreamConfigs[k]) + if err != nil { + return err + } + } + return nil +} + // ConfigEntryResponse returns a single ConfigEntry type ConfigEntryResponse struct { Entry ConfigEntry diff --git a/agent/structs/config_entry_test.go b/agent/structs/config_entry_test.go index 3fd7ea9553..d34287b58f 100644 --- a/agent/structs/config_entry_test.go +++ b/agent/structs/config_entry_test.go @@ -1,8 +1,10 @@ package structs import ( + "bytes" "testing" + "github.com/hashicorp/go-msgpack/codec" "github.com/stretchr/testify/require" ) @@ -100,3 +102,44 @@ func TestDecodeConfigEntry(t *testing.T) { }) } } + +func TestServiceConfigResponse_MsgPack(t *testing.T) { + // TODO(banks) lib.MapWalker doesn't actually fix the map[interface{}] issue + // it claims to in docs yet. When it does uncomment those cases below. + a := ServiceConfigResponse{ + ProxyConfig: map[string]interface{}{ + "string": "foo", + // "map": map[string]interface{}{ + // "baz": "bar", + // }, + }, + UpstreamConfigs: map[string]map[string]interface{}{ + "a": map[string]interface{}{ + "string": "aaaa", + // "map": map[string]interface{}{ + // "baz": "aa", + // }, + }, + "b": map[string]interface{}{ + "string": "bbbb", + // "map": map[string]interface{}{ + // "baz": "bb", + // }, + }, + }, + } + + var buf bytes.Buffer + + // Encode as msgPack using a regular handle i.e. NOT one with RawAsString + // since our RPC codec doesn't use that. + enc := codec.NewEncoder(&buf, msgpackHandle) + require.NoError(t, enc.Encode(&a)) + + var b ServiceConfigResponse + + dec := codec.NewDecoder(&buf, msgpackHandle) + require.NoError(t, dec.Decode(&b)) + + require.Equal(t, a, b) +} diff --git a/lib/map_walker_test.go b/lib/map_walker_test.go index e15ab98421..cbe8534c8e 100644 --- a/lib/map_walker_test.go +++ b/lib/map_walker_test.go @@ -38,6 +38,20 @@ func TestMapWalk(t *testing.T) { }, unexpected: true, }, + // TODO(banks): despite the doc comment, MapWalker doesn't actually fix + // these cases yet. Do that in a later PR. + // "map iface": tcase{ + // input: map[string]interface{}{ + // "foo": map[interface{}]interface{}{ + // "bar": "baz", + // }, + // }, + // expected: map[string]interface{}{ + // "foo": map[string]interface{}{ + // "bar": "baz", + // }, + // }, + // }, } for name, tcase := range cases {