From 4af8c78f1fea9f37c59846b43a16d3b31ff297b1 Mon Sep 17 00:00:00 2001 From: "R.B. Boyer" Date: Thu, 24 Sep 2020 13:58:52 -0500 Subject: [PATCH] agent: make the json/hcl decoding of ConnectProxyConfig fully work with CamelCase and snake_case (#8741) Fixes #7418 --- .changelog/8741.txt | 3 + agent/structs/connect_proxy_config.go | 60 ++-- agent/structs/connect_proxy_config_test.go | 316 +++++++++++++++++++-- 3 files changed, 340 insertions(+), 39 deletions(-) create mode 100644 .changelog/8741.txt diff --git a/.changelog/8741.txt b/.changelog/8741.txt new file mode 100644 index 000000000..965a32303 --- /dev/null +++ b/.changelog/8741.txt @@ -0,0 +1,3 @@ +```release-note:bug +agent: make the json/hcl decoding of ConnectProxyConfig fully work with CamelCase and snake_case +``` diff --git a/agent/structs/connect_proxy_config.go b/agent/structs/connect_proxy_config.go index bad538781..98f22a4f3 100644 --- a/agent/structs/connect_proxy_config.go +++ b/agent/structs/connect_proxy_config.go @@ -83,7 +83,7 @@ func (c *MeshGatewayConfig) ToAPI() api.MeshGatewayConfig { type ConnectProxyConfig struct { // DestinationServiceName is required and is the name of the service to accept // traffic for. - DestinationServiceName string `json:",omitempty"` + DestinationServiceName string `json:",omitempty" alias:"destination_service_name"` // DestinationServiceID is optional and should only be specified for // "side-car" style proxies where the proxy is in front of just a single @@ -91,19 +91,19 @@ type ConnectProxyConfig struct { // being represented which must be registered to the same agent. It's valid to // provide a service ID that does not yet exist to avoid timing issues when // bootstrapping a service with a proxy. - DestinationServiceID string `json:",omitempty"` + DestinationServiceID string `json:",omitempty" alias:"destination_service_id"` // LocalServiceAddress is the address of the local service instance. It is // optional and should only be specified for "side-car" style proxies. It will // default to 127.0.0.1 if the proxy is a "side-car" (DestinationServiceID is // set) but otherwise will be ignored. - LocalServiceAddress string `json:",omitempty"` + LocalServiceAddress string `json:",omitempty" alias:"local_service_address"` // LocalServicePort is the port of the local service instance. It is optional // and should only be specified for "side-car" style proxies. It will default // to the registered port for the instance if the proxy is a "side-car" // (DestinationServiceID is set) but otherwise will be ignored. - LocalServicePort int `json:",omitempty"` + LocalServicePort int `json:",omitempty" alias:"local_service_port"` // Config is the arbitrary configuration data provided with the proxy // registration. @@ -123,10 +123,11 @@ type ConnectProxyConfig struct { func (t *ConnectProxyConfig) UnmarshalJSON(data []byte) (err error) { type Alias ConnectProxyConfig aux := &struct { - DestinationServiceNameSnake string `json:"destination_service_name"` - DestinationServiceIDSnake string `json:"destination_service_id"` - LocalServiceAddressSnake string `json:"local_service_address"` - LocalServicePortSnake int `json:"local_service_port"` + DestinationServiceNameSnake string `json:"destination_service_name"` + DestinationServiceIDSnake string `json:"destination_service_id"` + LocalServiceAddressSnake string `json:"local_service_address"` + LocalServicePortSnake int `json:"local_service_port"` + MeshGatewaySnake MeshGatewayConfig `json:"mesh_gateway"` *Alias }{ @@ -147,6 +148,9 @@ func (t *ConnectProxyConfig) UnmarshalJSON(data []byte) (err error) { if t.LocalServicePort == 0 { t.LocalServicePort = aux.LocalServicePortSnake } + if t.MeshGateway.Mode == "" { + t.MeshGateway.Mode = aux.MeshGatewaySnake.Mode + } return nil @@ -223,9 +227,9 @@ type Upstream struct { // DestinationType would be better as an int constant but even with custom // JSON marshallers it causes havoc with all the mapstructure mangling we do // on service definitions in various places. - DestinationType string - DestinationNamespace string `json:",omitempty"` - DestinationName string + DestinationType string `alias:"destination_type"` + DestinationNamespace string `json:",omitempty" alias:"destination_namespace"` + DestinationName string `alias:"destination_name"` // Datacenter that the service discovery request should be run against. Note // for prepared queries, the actual results might be from a different @@ -234,19 +238,19 @@ type Upstream struct { // LocalBindAddress is the ip address a side-car proxy should listen on for // traffic destined for this upstream service. Default if empty is 127.0.0.1. - LocalBindAddress string `json:",omitempty"` + LocalBindAddress string `json:",omitempty" alias:"local_bind_address"` // LocalBindPort is the ip address a side-car proxy should listen on for traffic // destined for this upstream service. Required. - LocalBindPort int + LocalBindPort int `alias:"local_bind_port"` // Config is an opaque config that is specific to the proxy process being run. // It can be used to pass arbitrary configuration for this specific upstream // to the proxy. - Config map[string]interface{} `bexpr:"-"` + Config map[string]interface{} `json:",omitempty" bexpr:"-"` // MeshGateway is the configuration for mesh gateway usage of this upstream - MeshGateway MeshGatewayConfig `json:",omitempty"` + MeshGateway MeshGatewayConfig `json:",omitempty" alias:"mesh_gateway"` // IngressHosts are a list of hosts that should route to this upstream from // an ingress gateway. This cannot and should not be set by a user, it is @@ -260,8 +264,11 @@ func (t *Upstream) UnmarshalJSON(data []byte) (err error) { DestinationTypeSnake string `json:"destination_type"` DestinationNamespaceSnake string `json:"destination_namespace"` DestinationNameSnake string `json:"destination_name"` - LocalBindPortSnake int `json:"local_bind_port"` - LocalBindAddressSnake string `json:"local_bind_address"` + + LocalBindAddressSnake string `json:"local_bind_address"` + LocalBindPortSnake int `json:"local_bind_port"` + + MeshGatewaySnake MeshGatewayConfig `json:"mesh_gateway"` *Alias }{ @@ -279,11 +286,14 @@ func (t *Upstream) UnmarshalJSON(data []byte) (err error) { if t.DestinationName == "" { t.DestinationName = aux.DestinationNameSnake } + if t.LocalBindAddress == "" { + t.LocalBindAddress = aux.LocalBindAddressSnake + } if t.LocalBindPort == 0 { t.LocalBindPort = aux.LocalBindPortSnake } - if t.LocalBindAddress == "" { - t.LocalBindAddress = aux.LocalBindAddressSnake + if t.MeshGateway.Mode == "" { + t.MeshGateway.Mode = aux.MeshGatewaySnake.Mode } return nil @@ -412,7 +422,7 @@ type ExposePath struct { // ListenerPort defines the port of the proxy's listener for exposed paths. ListenerPort int `json:",omitempty" alias:"listener_port"` - // ExposePath is the path to expose through the proxy, ie. "/metrics." + // Path is the path to expose through the proxy, ie. "/metrics." Path string `json:",omitempty"` // LocalPathPort is the port that the service is listening on for the given path. @@ -423,14 +433,15 @@ type ExposePath struct { Protocol string `json:",omitempty"` // ParsedFromCheck is set if this path was parsed from a registered check - ParsedFromCheck bool + ParsedFromCheck bool `json:",omitempty" alias:"parsed_from_check"` } func (t *ExposePath) UnmarshalJSON(data []byte) (err error) { type Alias ExposePath aux := &struct { - LocalPathPortSnake int `json:"local_path_port"` - ListenerPortSnake int `json:"listener_port"` + ListenerPortSnake int `json:"listener_port"` + LocalPathPortSnake int `json:"local_path_port"` + ParsedFromCheckSnake bool `json:"parsed_from_check"` *Alias }{ @@ -445,6 +456,9 @@ func (t *ExposePath) UnmarshalJSON(data []byte) (err error) { if t.ListenerPort == 0 { t.ListenerPort = aux.ListenerPortSnake } + if aux.ParsedFromCheckSnake { + t.ParsedFromCheck = true + } return nil } diff --git a/agent/structs/connect_proxy_config_test.go b/agent/structs/connect_proxy_config_test.go index 252111334..7c2c911f8 100644 --- a/agent/structs/connect_proxy_config_test.go +++ b/agent/structs/connect_proxy_config_test.go @@ -108,8 +108,7 @@ func TestUpstream_MarshalJSON(t *testing.T) { "DestinationName": "foo", "Datacenter": "dc1", "LocalBindPort": 1234, - "MeshGateway": {}, - "Config": null + "MeshGateway": {} }`, wantErr: false, }, @@ -126,8 +125,7 @@ func TestUpstream_MarshalJSON(t *testing.T) { "DestinationName": "foo", "Datacenter": "dc1", "LocalBindPort": 1234, - "MeshGateway": {}, - "Config": null + "MeshGateway": {} }`, wantErr: false, }, @@ -148,10 +146,11 @@ func TestUpstream_MarshalJSON(t *testing.T) { func TestUpstream_UnmarshalJSON(t *testing.T) { tests := []struct { - name string - json string - want Upstream - wantErr bool + name string + json string + jsonSnake string + want Upstream + wantErr bool }{ { name: "service", @@ -197,18 +196,303 @@ func TestUpstream_UnmarshalJSON(t *testing.T) { }, wantErr: true, }, + { + name: "kitchen sink", + json: ` + { + "DestinationType": "service", + "DestinationNamespace": "default", + "DestinationName": "bar1", + "Datacenter": "dc1", + "LocalBindAddress": "127.0.0.2", + "LocalBindPort": 6060, + "Config": { + "x": "y", + "z": -2 + }, + "MeshGateway": { + "Mode": "local" + } + } + `, + jsonSnake: ` + { + "destination_type": "service", + "destination_namespace": "default", + "destination_name": "bar1", + "datacenter": "dc1", + "local_bind_address": "127.0.0.2", + "local_bind_port": 6060, + "config": { + "x": "y", + "z": -2 + }, + "mesh_gateway": { + "mode": "local" + } + } + `, + want: Upstream{ + DestinationType: UpstreamDestTypeService, + DestinationNamespace: "default", + DestinationName: "bar1", + Datacenter: "dc1", + LocalBindAddress: "127.0.0.2", + LocalBindPort: 6060, + Config: map[string]interface{}{ + "x": "y", + "z": float64(-2), + }, + MeshGateway: MeshGatewayConfig{ + Mode: "local", + }, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - require := require.New(t) - var got Upstream - err := json.Unmarshal([]byte(tt.json), &got) - if tt.wantErr { - require.Error(err) - return + t.Run("camel", func(t *testing.T) { + var got Upstream + err := json.Unmarshal([]byte(tt.json), &got) + if tt.wantErr { + require.Error(t, err) + } else { + require.NoError(t, err) + require.Equal(t, tt.want, got, "%+v", got) + } + }) + + if tt.jsonSnake != "" { + t.Run("snake", func(t *testing.T) { + var got Upstream + err := json.Unmarshal([]byte(tt.jsonSnake), &got) + if tt.wantErr { + require.Error(t, err) + } else { + require.NoError(t, err) + require.Equal(t, tt.want, got) + } + }) + } + }) + } +} + +func TestConnectProxyConfig_UnmarshalJSON(t *testing.T) { + tests := []struct { + name string + json string + jsonSnake string + want ConnectProxyConfig + wantErr bool + }{ + { + name: "kitchen sink", + json: ` + { + "DestinationServiceName": "foo-name", + "DestinationServiceID": "foo-id", + "LocalServiceAddress": "127.0.0.1", + "LocalServicePort": 5050, + "Config": { + "a": "b", + "v": 42 + }, + "Upstreams": [ + { + "DestinationType": "service", + "DestinationNamespace": "default", + "DestinationName": "bar1", + "Datacenter": "dc1", + "LocalBindAddress": "127.0.0.2", + "LocalBindPort": 6060, + "Config": { + "x": "y", + "z": -2 + }, + "MeshGateway": { + "Mode": "local" + } + }, + { + "DestinationType": "service", + "DestinationNamespace": "default", + "DestinationName": "bar2", + "Datacenter": "dc2", + "LocalBindAddress": "127.0.0.2", + "LocalBindPort": 6161 + } + ], + "MeshGateway": { + "Mode": "remote" + }, + "Expose": { + "Checks": true, + "Paths": [ + { + "ListenerPort": 8080, + "Path": "/foo", + "LocalPathPort": 7070, + "Protocol": "http2", + "ParsedFromCheck": true + }, + { + "ListenerPort": 8181, + "Path": "/foo2", + "LocalPathPort": 7171, + "Protocol": "http", + "ParsedFromCheck": false + } + ] + } + } + `, + jsonSnake: ` + { + "destination_service_name": "foo-name", + "destination_service_id": "foo-id", + "local_service_address": "127.0.0.1", + "local_service_port": 5050, + "config": { + "a": "b", + "v": 42 + }, + "upstreams": [ + { + "destination_type": "service", + "destination_namespace": "default", + "destination_name": "bar1", + "datacenter": "dc1", + "local_bind_address": "127.0.0.2", + "local_bind_port": 6060, + "config": { + "x": "y", + "z": -2 + }, + "mesh_gateway": { + "mode": "local" + } + }, + { + "destination_type": "service", + "destination_namespace": "default", + "destination_name": "bar2", + "datacenter": "dc2", + "local_bind_address": "127.0.0.2", + "local_bind_port": 6161 + } + ], + "mesh_gateway": { + "mode": "remote" + }, + "expose": { + "checks": true, + "paths": [ + { + "listener_port": 8080, + "path": "/foo", + "local_path_port": 7070, + "protocol": "http2", + "parsed_from_check": true + }, + { + "listener_port": 8181, + "path": "/foo2", + "local_path_port": 7171, + "protocol": "http", + "parsed_from_check": false + } + ] + } + } + `, + want: ConnectProxyConfig{ + DestinationServiceName: "foo-name", + DestinationServiceID: "foo-id", + LocalServiceAddress: "127.0.0.1", + LocalServicePort: 5050, + Config: map[string]interface{}{ + "a": "b", + "v": float64(42), + }, + Upstreams: []Upstream{ + { + DestinationType: UpstreamDestTypeService, + DestinationNamespace: "default", + DestinationName: "bar1", + Datacenter: "dc1", + LocalBindAddress: "127.0.0.2", + LocalBindPort: 6060, + Config: map[string]interface{}{ + "x": "y", + "z": float64(-2), + }, + MeshGateway: MeshGatewayConfig{ + Mode: "local", + }, + }, + + { + DestinationType: UpstreamDestTypeService, + DestinationNamespace: "default", + DestinationName: "bar2", + Datacenter: "dc2", + LocalBindAddress: "127.0.0.2", + LocalBindPort: 6161, + }, + }, + + MeshGateway: MeshGatewayConfig{ + Mode: "remote", + }, + Expose: ExposeConfig{ + Checks: true, + Paths: []ExposePath{ + { + ListenerPort: 8080, + Path: "/foo", + LocalPathPort: 7070, + Protocol: "http2", + ParsedFromCheck: true, + }, + { + ListenerPort: 8181, + Path: "/foo2", + LocalPathPort: 7171, + Protocol: "http", + ParsedFromCheck: false, + }, + }, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Run("camel", func(t *testing.T) { + // + var got ConnectProxyConfig + err := json.Unmarshal([]byte(tt.json), &got) + if tt.wantErr { + require.Error(t, err) + } else { + require.NoError(t, err) + require.Equal(t, tt.want, got) + } + }) + if tt.jsonSnake != "" { + t.Run("snake", func(t *testing.T) { + // + var got ConnectProxyConfig + err := json.Unmarshal([]byte(tt.json), &got) + if tt.wantErr { + require.Error(t, err) + } else { + require.NoError(t, err) + require.Equal(t, tt.want, got) + } + }) } - require.NoError(err) - require.Equal(tt.want, got) }) } }