From 0d5e917ae06457a2313aafcf70749fb5386242ab Mon Sep 17 00:00:00 2001 From: "R.B. Boyer" Date: Fri, 12 Jul 2019 12:20:30 -0500 Subject: [PATCH] handle structs.ConfigEntry decoding similarly to api.ConfigEntry decoding (#6106) Both 'consul config write' and server bootstrap config entries take a decoding detour through mapstructure on the way from HCL to an actual struct. They both may take in snake_case or CamelCase (for consistency) so need very similar handling. Unfortunately since they are operating on mirror universes of structs (api.* vs structs.*) the code cannot be identitical, so try to share the kind-configuration and duplicate the rest for now. --- agent/config/config.go | 7 +- agent/config/runtime_test.go | 453 +++++++++++++++- agent/config_endpoint_test.go | 2 +- agent/structs/config_entry.go | 112 +++- agent/structs/config_entry_test.go | 595 +++++++++++++++++++--- command/config/write/config_write.go | 65 +-- command/config/write/config_write_test.go | 3 + lib/patch_hcl.go | 22 +- lib/patch_hcl_test.go | 51 +- 9 files changed, 1151 insertions(+), 159 deletions(-) diff --git a/agent/config/config.go b/agent/config/config.go index c79a8f833..68a6fc74a 100644 --- a/agent/config/config.go +++ b/agent/config/config.go @@ -98,9 +98,8 @@ func Parse(data string, format string) (c Config, err error) { "services.connect.sidecar_service.checks", "service.connect.sidecar_service.proxy.upstreams", "services.connect.sidecar_service.proxy.upstreams", - - "config_entries.bootstrap", - "config_entries.bootstrap.Splits", + }, []string{ + "config_entries.bootstrap", // completely ignore this tree (fixed elsewhere) }) // There is a difference of representation of some fields depending on @@ -122,6 +121,7 @@ func Parse(data string, format string) (c Config, err error) { "scriptargs": "args", "serviceid": "service_id", "tlsskipverify": "tls_skip_verify", + "config_entries.bootstrap": "", }) var md mapstructure.Metadata @@ -135,6 +135,7 @@ func Parse(data string, format string) (c Config, err error) { if err := d.Decode(m); err != nil { return Config{}, err } + for _, k := range md.Unused { err = multierror.Append(err, fmt.Errorf("invalid config key %s", k)) } diff --git a/agent/config/runtime_test.go b/agent/config/runtime_test.go index 5a18c5cae..462017865 100644 --- a/agent/config/runtime_test.go +++ b/agent/config/runtime_test.go @@ -2796,7 +2796,7 @@ func TestConfigFlagsAndEdgecases(t *testing.T) { foo = "bar" } }`}, - err: "config_entries.bootstrap[0]: Payload does not contain a Kind", + err: "config_entries.bootstrap[0]: Payload does not contain a kind/Kind", }, { desc: "ConfigEntry bootstrap unknown kind", @@ -2850,12 +2850,463 @@ func TestConfigFlagsAndEdgecases(t *testing.T) { }`}, err: "config_entries.bootstrap[0]: invalid name (\"invalid-name\"), only \"global\" is supported", }, + { + desc: "ConfigEntry bootstrap proxy-defaults (snake-case)", + args: []string{`-data-dir=` + dataDir}, + json: []string{`{ + "config_entries": { + "bootstrap": [ + { + "kind": "proxy-defaults", + "name": "global", + "config": { + "bar": "abc", + "moreconfig": { + "moar": "config" + } + }, + "mesh_gateway": { + "mode": "remote" + } + } + ] + } + }`}, + hcl: []string{` + config_entries { + bootstrap { + kind = "proxy-defaults" + name = "global" + config { + "bar" = "abc" + "moreconfig" { + "moar" = "config" + } + } + mesh_gateway { + mode = "remote" + } + } + }`}, + patch: func(rt *RuntimeConfig) { + rt.DataDir = dataDir + rt.ConfigEntryBootstrap = []structs.ConfigEntry{ + &structs.ProxyConfigEntry{ + Kind: structs.ProxyDefaults, + Name: structs.ProxyConfigGlobal, + Config: map[string]interface{}{ + "bar": "abc", + "moreconfig": map[string]interface{}{ + "moar": "config", + }, + }, + MeshGateway: structs.MeshGatewayConfig{ + Mode: structs.MeshGatewayModeRemote, + }, + }, + } + }, + }, + { + desc: "ConfigEntry bootstrap proxy-defaults (camel-case)", + args: []string{`-data-dir=` + dataDir}, + json: []string{`{ + "config_entries": { + "bootstrap": [ + { + "Kind": "proxy-defaults", + "Name": "global", + "Config": { + "bar": "abc", + "moreconfig": { + "moar": "config" + } + }, + "MeshGateway": { + "Mode": "remote" + } + } + ] + } + }`}, + hcl: []string{` + config_entries { + bootstrap { + Kind = "proxy-defaults" + Name = "global" + Config { + "bar" = "abc" + "moreconfig" { + "moar" = "config" + } + } + MeshGateway { + Mode = "remote" + } + } + }`}, + patch: func(rt *RuntimeConfig) { + rt.DataDir = dataDir + rt.ConfigEntryBootstrap = []structs.ConfigEntry{ + &structs.ProxyConfigEntry{ + Kind: structs.ProxyDefaults, + Name: structs.ProxyConfigGlobal, + Config: map[string]interface{}{ + "bar": "abc", + "moreconfig": map[string]interface{}{ + "moar": "config", + }, + }, + MeshGateway: structs.MeshGatewayConfig{ + Mode: structs.MeshGatewayModeRemote, + }, + }, + } + }, + }, + { + desc: "ConfigEntry bootstrap service-defaults (snake-case)", + args: []string{`-data-dir=` + dataDir}, + json: []string{`{ + "config_entries": { + "bootstrap": [ + { + "kind": "service-defaults", + "name": "web", + "protocol": "http", + "mesh_gateway": { + "mode": "remote" + } + } + ] + } + }`}, + hcl: []string{` + config_entries { + bootstrap { + kind = "service-defaults" + name = "web" + protocol = "http" + mesh_gateway { + mode = "remote" + } + } + }`}, + patch: func(rt *RuntimeConfig) { + rt.DataDir = dataDir + rt.ConfigEntryBootstrap = []structs.ConfigEntry{ + &structs.ServiceConfigEntry{ + Kind: structs.ServiceDefaults, + Name: "web", + Protocol: "http", + MeshGateway: structs.MeshGatewayConfig{ + Mode: structs.MeshGatewayModeRemote, + }, + }, + } + }, + }, + { + desc: "ConfigEntry bootstrap service-defaults (camel-case)", + args: []string{`-data-dir=` + dataDir}, + json: []string{`{ + "config_entries": { + "bootstrap": [ + { + "Kind": "service-defaults", + "Name": "web", + "Protocol": "http", + "MeshGateway": { + "Mode": "remote" + } + } + ] + } + }`}, + hcl: []string{` + config_entries { + bootstrap { + Kind = "service-defaults" + Name = "web" + Protocol = "http" + MeshGateway { + Mode = "remote" + } + } + }`}, + patch: func(rt *RuntimeConfig) { + rt.DataDir = dataDir + rt.ConfigEntryBootstrap = []structs.ConfigEntry{ + &structs.ServiceConfigEntry{ + Kind: structs.ServiceDefaults, + Name: "web", + Protocol: "http", + MeshGateway: structs.MeshGatewayConfig{ + Mode: structs.MeshGatewayModeRemote, + }, + }, + } + }, + }, + { + desc: "ConfigEntry bootstrap service-router (snake-case)", + args: []string{`-data-dir=` + dataDir}, + json: []string{`{ + "config_entries": { + "bootstrap": [ + { + "kind": "service-router", + "name": "main", + "routes": [ + { + "match": { + "http": { + "path_exact": "/foo", + "header": [ + { + "name": "debug1", + "present": true + }, + { + "name": "debug2", + "present": false, + "invert": true + }, + { + "name": "debug3", + "exact": "1" + }, + { + "name": "debug4", + "prefix": "aaa" + }, + { + "name": "debug5", + "suffix": "bbb" + }, + { + "name": "debug6", + "regex": "a.*z" + } + ] + } + }, + "destination": { + "service" : "carrot", + "service_subset" : "kale", + "namespace" : "leek", + "prefix_rewrite" : "/alternate", + "request_timeout" : "99s", + "num_retries" : 12345, + "retry_on_connect_failure": true, + "retry_on_status_codes" : [401, 209] + } + }, + { + "match": { + "http": { + "path_prefix": "/foo", + "query_param": [ + { + "name": "hack1" + }, + { + "name": "hack2", + "value": "1" + }, + { + "name": "hack3", + "value": "a.*z", + "regex": true + } + ] + } + } + }, + { + "match": { + "http": { + "path_regex": "/foo" + } + } + } + ] + } + ] + } + }`}, + hcl: []string{` + config_entries { + bootstrap { + kind = "service-router" + name = "main" + routes = [ + { + match { + http { + path_exact = "/foo" + header = [ + { + name = "debug1" + present = true + }, + { + name = "debug2" + present = false + invert = true + }, + { + name = "debug3" + exact = "1" + }, + { + name = "debug4" + prefix = "aaa" + }, + { + name = "debug5" + suffix = "bbb" + }, + { + name = "debug6" + regex = "a.*z" + }, + ] + } + } + destination { + service = "carrot" + service_subset = "kale" + namespace = "leek" + prefix_rewrite = "/alternate" + request_timeout = "99s" + num_retries = 12345 + retry_on_connect_failure = true + retry_on_status_codes = [401, 209] + } + }, + { + match { + http { + path_prefix = "/foo" + query_param = [ + { + name = "hack1" + }, + { + name = "hack2" + value = "1" + }, + { + name = "hack3" + value = "a.*z" + regex = true + }, + ] + } + } + }, + { + match { + http { + path_regex = "/foo" + } + } + }, + ] + } + }`}, + patch: func(rt *RuntimeConfig) { + rt.DataDir = dataDir + rt.ConfigEntryBootstrap = []structs.ConfigEntry{ + &structs.ServiceRouterConfigEntry{ + Kind: structs.ServiceRouter, + Name: "main", + Routes: []structs.ServiceRoute{ + { + Match: &structs.ServiceRouteMatch{ + HTTP: &structs.ServiceRouteHTTPMatch{ + PathExact: "/foo", + Header: []structs.ServiceRouteHTTPMatchHeader{ + { + Name: "debug1", + Present: true, + }, + { + Name: "debug2", + Present: false, + Invert: true, + }, + { + Name: "debug3", + Exact: "1", + }, + { + Name: "debug4", + Prefix: "aaa", + }, + { + Name: "debug5", + Suffix: "bbb", + }, + { + Name: "debug6", + Regex: "a.*z", + }, + }, + }, + }, + Destination: &structs.ServiceRouteDestination{ + Service: "carrot", + ServiceSubset: "kale", + Namespace: "leek", + PrefixRewrite: "/alternate", + RequestTimeout: 99 * time.Second, + NumRetries: 12345, + RetryOnConnectFailure: true, + RetryOnStatusCodes: []uint32{401, 209}, + }, + }, + { + Match: &structs.ServiceRouteMatch{ + HTTP: &structs.ServiceRouteHTTPMatch{ + PathPrefix: "/foo", + QueryParam: []structs.ServiceRouteHTTPMatchQueryParam{ + { + Name: "hack1", + }, + { + Name: "hack2", + Value: "1", + }, + { + Name: "hack3", + Value: "a.*z", + Regex: true, + }, + }, + }, + }, + }, + { + Match: &structs.ServiceRouteMatch{ + HTTP: &structs.ServiceRouteHTTPMatch{ + PathRegex: "/foo", + }, + }, + }, + }, + }, + } + }, + }, } testConfig(t, tests, dataDir) } func testConfig(t *testing.T, tests []configTest, dataDir string) { + t.Helper() for _, tt := range tests { for pass, format := range []string{"json", "hcl"} { // clean data dir before every test diff --git a/agent/config_endpoint_test.go b/agent/config_endpoint_test.go index eb0dda6d9..50f2ce6a5 100644 --- a/agent/config_endpoint_test.go +++ b/agent/config_endpoint_test.go @@ -323,7 +323,7 @@ func TestConfig_Apply_Decoding(t *testing.T) { require.Error(t, err) badReq, ok := err.(BadRequestError) require.True(t, ok) - require.Equal(t, "Request decoding failed: Payload does not contain a Kind key at the top level", badReq.Reason) + require.Equal(t, "Request decoding failed: Payload does not contain a kind/Kind key at the top level", badReq.Reason) }) t.Run("Kind Not String", func(t *testing.T) { diff --git a/agent/structs/config_entry.go b/agent/structs/config_entry.go index 9cda7ec3d..90ecddd9f 100644 --- a/agent/structs/config_entry.go +++ b/agent/structs/config_entry.go @@ -9,6 +9,7 @@ import ( "github.com/hashicorp/consul/agent/cache" "github.com/hashicorp/consul/lib" "github.com/hashicorp/go-msgpack/codec" + "github.com/hashicorp/go-multierror" "github.com/mitchellh/hashstructure" "github.com/mitchellh/mapstructure" ) @@ -227,27 +228,18 @@ func (e *ProxyConfigEntry) UnmarshalBinary(data []byte) error { // the kind so we will not have a concrete type to decode into. In those cases we must // first decode into a map[string]interface{} and then call this function to decode // into a concrete type. +// +// There is an 'api' variation of this in +// command/config/write/config_write.go:newDecodeConfigEntry func DecodeConfigEntry(raw map[string]interface{}) (ConfigEntry, error) { - lib.TranslateKeys(raw, map[string]string{ - "kind": "Kind", - "name": "Name", - "connect": "Connect", - "sidecar_proxy": "SidecarProxy", - "protocol": "Protocol", - "mesh_gateway": "MeshGateway", - "mode": "Mode", - "Config": "", - }) - - // TODO(rb): see if any changes are needed here for the discovery chain - - // TODO(rb): maybe do an initial kind/Kind switch and do kind-specific decoding? - var entry ConfigEntry kindVal, ok := raw["Kind"] if !ok { - return nil, fmt.Errorf("Payload does not contain a Kind key at the top level") + kindVal, ok = raw["kind"] + } + if !ok { + return nil, fmt.Errorf("Payload does not contain a kind/Kind key at the top level") } if kindStr, ok := kindVal.(string); ok { @@ -260,9 +252,23 @@ func DecodeConfigEntry(raw map[string]interface{}) (ConfigEntry, error) { return nil, fmt.Errorf("Kind value in payload is not a string") } + skipWhenPatching, translateKeysDict, err := ConfigEntryDecodeRulesForKind(entry.GetKind()) + if err != nil { + return nil, err + } + + // lib.TranslateKeys doesn't understand []map[string]interface{} so we have + // to do this part first. + raw = lib.PatchSliceOfMaps(raw, skipWhenPatching, nil) + + lib.TranslateKeys(raw, translateKeysDict) + + var md mapstructure.Metadata decodeConf := &mapstructure.DecoderConfig{ - DecodeHook: mapstructure.StringToTimeDurationHookFunc(), - Result: &entry, + DecodeHook: mapstructure.StringToTimeDurationHookFunc(), + Metadata: &md, + Result: &entry, + WeaklyTypedInput: true, } decoder, err := mapstructure.NewDecoder(decodeConf) @@ -270,7 +276,75 @@ func DecodeConfigEntry(raw map[string]interface{}) (ConfigEntry, error) { return nil, err } - return entry, decoder.Decode(raw) + if err := decoder.Decode(raw); err != nil { + return nil, err + } + + for _, k := range md.Unused { + switch k { + case "CreateIndex", "ModifyIndex": + default: + err = multierror.Append(err, fmt.Errorf("invalid config key %q", k)) + } + } + if err != nil { + return nil, err + } + return entry, nil +} + +// ConfigEntryDecodeRulesForKind returns rules for 'fixing' config entry key +// formats by kind. This is shared between the 'structs' and 'api' variations +// of config entries. +func ConfigEntryDecodeRulesForKind(kind string) (skipWhenPatching []string, translateKeysDict map[string]string, err error) { + switch kind { + case ProxyDefaults: + return nil, map[string]string{ + "mesh_gateway": "meshgateway", + "config": "", + }, nil + case ServiceDefaults: + return nil, map[string]string{ + "mesh_gateway": "meshgateway", + }, nil + case ServiceRouter: + return []string{ + "routes", + "Routes", + "routes.match.http.header", + "Routes.Match.HTTP.Header", + "routes.match.http.query_param", + "Routes.Match.HTTP.QueryParam", + }, map[string]string{ + "num_retries": "numretries", + "path_exact": "pathexact", + "path_prefix": "pathprefix", + "path_regex": "pathregex", + "prefix_rewrite": "prefixrewrite", + "query_param": "queryparam", + "request_timeout": "requesttimeout", + "retry_on_connect_failure": "retryonconnectfailure", + "retry_on_status_codes": "retryonstatuscodes", + "service_subset": "servicesubset", + }, nil + case ServiceSplitter: + return []string{ + "splits", + "Splits", + }, map[string]string{ + "service_subset": "servicesubset", + }, nil + case ServiceResolver: + return nil, map[string]string{ + "connect_timeout": "connecttimeout", + "default_subset": "defaultsubset", + "only_passing": "onlypassing", + "overprovisioning_factor": "overprovisioningfactor", + "service_subset": "servicesubset", + }, nil + default: + return nil, nil, fmt.Errorf("kind %q should be explicitly handled here", kind) + } } type ConfigEntryOp string diff --git a/agent/structs/config_entry_test.go b/agent/structs/config_entry_test.go index 583d6933c..e4cfc591e 100644 --- a/agent/structs/config_entry_test.go +++ b/agent/structs/config_entry_test.go @@ -2,103 +2,559 @@ package structs import ( "bytes" + "strings" "testing" + "time" "github.com/hashicorp/go-msgpack/codec" + "github.com/hashicorp/hcl" "github.com/stretchr/testify/require" ) +// TestDecodeConfigEntry is the 'structs' mirror image of +// command/config/write/config_write_test.go:TestParseConfigEntry func TestDecodeConfigEntry(t *testing.T) { t.Parallel() - type tcase struct { - input map[string]interface{} - expected ConfigEntry - expectErr bool - } - cases := map[string]tcase{ - "proxy-defaults": tcase{ - input: map[string]interface{}{ - "Kind": ProxyDefaults, - "Name": ProxyConfigGlobal, - "Config": map[string]interface{}{ - "foo": "bar", - }, - }, - expected: &ProxyConfigEntry{ - Kind: ProxyDefaults, - Name: ProxyConfigGlobal, + for _, tc := range []struct { + name string + camel string + snake string + expect ConfigEntry + expectErr string + }{ + // TODO(rb): test json? + { + name: "proxy-defaults: extra fields or typo", + snake: ` + kind = "proxy-defaults" + name = "main" + cornfig { + "foo" = 19 + } + `, + camel: ` + Kind = "proxy-defaults" + Name = "main" + Cornfig { + "foo" = 19 + } + `, + expectErr: `invalid config key "cornfig"`, + }, + { + name: "proxy-defaults", + snake: ` + kind = "proxy-defaults" + name = "main" + config { + "foo" = 19 + "bar" = "abc" + "moreconfig" { + "moar" = "config" + } + } + mesh_gateway { + mode = "remote" + } + `, + camel: ` + Kind = "proxy-defaults" + Name = "main" + Config { + "foo" = 19 + "bar" = "abc" + "moreconfig" { + "moar" = "config" + } + } + MeshGateway { + Mode = "remote" + } + `, + expect: &ProxyConfigEntry{ + Kind: "proxy-defaults", + Name: "main", Config: map[string]interface{}{ - "foo": "bar", + "foo": 19, + "bar": "abc", + "moreconfig": map[string]interface{}{ + "moar": "config", + }, + }, + MeshGateway: MeshGatewayConfig{ + Mode: MeshGatewayModeRemote, }, }, }, - "proxy-defaults translations": tcase{ - input: map[string]interface{}{ - "kind": ProxyDefaults, - "name": ProxyConfigGlobal, - "config": map[string]interface{}{ - "foo": "bar", - "sidecar_proxy": true, - }, - }, - expected: &ProxyConfigEntry{ - Kind: ProxyDefaults, - Name: ProxyConfigGlobal, - Config: map[string]interface{}{ - "foo": "bar", - "sidecar_proxy": true, + { + name: "service-defaults", + snake: ` + kind = "service-defaults" + name = "main" + protocol = "http" + mesh_gateway { + mode = "remote" + } + `, + camel: ` + Kind = "service-defaults" + Name = "main" + Protocol = "http" + MeshGateway { + Mode = "remote" + } + `, + expect: &ServiceConfigEntry{ + Kind: "service-defaults", + Name: "main", + Protocol: "http", + MeshGateway: MeshGatewayConfig{ + Mode: MeshGatewayModeRemote, }, }, }, - "service-defaults": tcase{ - input: map[string]interface{}{ - "Kind": ServiceDefaults, - "Name": "foo", - "Protocol": "tcp", - "Connect": map[string]interface{}{ - "SidecarProxy": true, + { + name: "service-router: kitchen sink", + snake: ` + kind = "service-router" + name = "main" + routes = [ + { + match { + http { + path_exact = "/foo" + header = [ + { + name = "debug1" + present = true + }, + { + name = "debug2" + present = false + invert = true + }, + { + name = "debug3" + exact = "1" + }, + { + name = "debug4" + prefix = "aaa" + }, + { + name = "debug5" + suffix = "bbb" + }, + { + name = "debug6" + regex = "a.*z" + }, + ] + } + } + destination { + service = "carrot" + service_subset = "kale" + namespace = "leek" + prefix_rewrite = "/alternate" + request_timeout = "99s" + num_retries = 12345 + retry_on_connect_failure = true + retry_on_status_codes = [401, 209] + } + }, + { + match { + http { + path_prefix = "/foo" + query_param = [ + { + name = "hack1" + }, + { + name = "hack2" + value = "1" + }, + { + name = "hack3" + value = "a.*z" + regex = true + }, + ] + } + } + }, + { + match { + http { + path_regex = "/foo" + } + } + }, + ] + `, + camel: ` + Kind = "service-router" + Name = "main" + Routes = [ + { + Match { + HTTP { + PathExact = "/foo" + Header = [ + { + Name = "debug1" + Present = true + }, + { + Name = "debug2" + Present = false + Invert = true + }, + { + Name = "debug3" + Exact = "1" + }, + { + Name = "debug4" + Prefix = "aaa" + }, + { + Name = "debug5" + Suffix = "bbb" + }, + { + Name = "debug6" + Regex = "a.*z" + }, + ] + } + } + Destination { + Service = "carrot" + ServiceSubset = "kale" + Namespace = "leek" + PrefixRewrite = "/alternate" + RequestTimeout = "99s" + NumRetries = 12345 + RetryOnConnectFailure = true + RetryOnStatusCodes = [401, 209] + } + }, + { + Match { + HTTP { + PathPrefix = "/foo" + QueryParam = [ + { + Name = "hack1" + }, + { + Name = "hack2" + Value = "1" + }, + { + Name = "hack3" + Value = "a.*z" + Regex = true + }, + ] + } + } + }, + { + Match { + HTTP { + PathRegex = "/foo" + } + } + }, + ] + `, + expect: &ServiceRouterConfigEntry{ + Kind: "service-router", + Name: "main", + Routes: []ServiceRoute{ + { + Match: &ServiceRouteMatch{ + HTTP: &ServiceRouteHTTPMatch{ + PathExact: "/foo", + Header: []ServiceRouteHTTPMatchHeader{ + { + Name: "debug1", + Present: true, + }, + { + Name: "debug2", + Present: false, + Invert: true, + }, + { + Name: "debug3", + Exact: "1", + }, + { + Name: "debug4", + Prefix: "aaa", + }, + { + Name: "debug5", + Suffix: "bbb", + }, + { + Name: "debug6", + Regex: "a.*z", + }, + }, + }, + }, + Destination: &ServiceRouteDestination{ + Service: "carrot", + ServiceSubset: "kale", + Namespace: "leek", + PrefixRewrite: "/alternate", + RequestTimeout: 99 * time.Second, + NumRetries: 12345, + RetryOnConnectFailure: true, + RetryOnStatusCodes: []uint32{401, 209}, + }, + }, + { + Match: &ServiceRouteMatch{ + HTTP: &ServiceRouteHTTPMatch{ + PathPrefix: "/foo", + QueryParam: []ServiceRouteHTTPMatchQueryParam{ + { + Name: "hack1", + }, + { + Name: "hack2", + Value: "1", + }, + { + Name: "hack3", + Value: "a.*z", + Regex: true, + }, + }, + }, + }, + }, + { + Match: &ServiceRouteMatch{ + HTTP: &ServiceRouteHTTPMatch{ + PathRegex: "/foo", + }, + }, + }, }, }, - expected: &ServiceConfigEntry{ - Kind: ServiceDefaults, - Name: "foo", - Protocol: "tcp", - //Connect: ConnectConfiguration{SidecarProxy: true}, - }, }, - "service-defaults translations": tcase{ - input: map[string]interface{}{ - "kind": ServiceDefaults, - "name": "foo", - "protocol": "tcp", - "connect": map[string]interface{}{ - "sidecar_proxy": true, + { + name: "service-splitter: kitchen sink", + snake: ` + kind = "service-splitter" + name = "main" + splits = [ + { + weight = 99.1 + service_subset = "v1" + }, + { + weight = 0.9 + service = "other" + namespace = "alt" + }, + ] + `, + camel: ` + Kind = "service-splitter" + Name = "main" + Splits = [ + { + Weight = 99.1 + ServiceSubset = "v1" + }, + { + Weight = 0.9 + Service = "other" + Namespace = "alt" + }, + ] + `, + expect: &ServiceSplitterConfigEntry{ + Kind: ServiceSplitter, + Name: "main", + Splits: []ServiceSplit{ + { + Weight: 99.1, + ServiceSubset: "v1", + }, + { + Weight: 0.9, + Service: "other", + Namespace: "alt", + }, }, }, - expected: &ServiceConfigEntry{ - Kind: ServiceDefaults, - Name: "foo", - Protocol: "tcp", - //Connect: ConnectConfiguration{SidecarProxy: true}, + }, + { + name: "service-resolver: subsets with failover", + snake: ` + kind = "service-resolver" + name = "main" + default_subset = "v1" + connect_timeout = "15s" + subsets = { + "v1" = { + filter = "Service.Meta.version == v1" + }, + "v2" = { + filter = "Service.Meta.version == v2" + only_passing = true + }, + } + failover = { + "v2" = { + service = "failcopy" + service_subset = "sure" + namespace = "neighbor" + datacenters = ["dc5", "dc14"] + overprovisioning_factor = 150 + }, + "*" = { + datacenters = ["dc7"] + } + }`, + camel: ` + Kind = "service-resolver" + Name = "main" + DefaultSubset = "v1" + ConnectTimeout = "15s" + Subsets = { + "v1" = { + Filter = "Service.Meta.version == v1" + }, + "v2" = { + Filter = "Service.Meta.version == v2" + OnlyPassing = true + }, + } + Failover = { + "v2" = { + Service = "failcopy" + ServiceSubset = "sure" + Namespace = "neighbor" + Datacenters = ["dc5", "dc14"] + OverprovisioningFactor = 150 + }, + "*" = { + Datacenters = ["dc7"] + } + }`, + expect: &ServiceResolverConfigEntry{ + Kind: "service-resolver", + Name: "main", + DefaultSubset: "v1", + ConnectTimeout: 15 * time.Second, + Subsets: map[string]ServiceResolverSubset{ + "v1": { + Filter: "Service.Meta.version == v1", + }, + "v2": { + Filter: "Service.Meta.version == v2", + OnlyPassing: true, + }, + }, + Failover: map[string]ServiceResolverFailover{ + "v2": { + Service: "failcopy", + ServiceSubset: "sure", + Namespace: "neighbor", + Datacenters: []string{"dc5", "dc14"}, + OverprovisioningFactor: 150, + }, + "*": { + Datacenters: []string{"dc7"}, + }, + }, }, }, - } + { + name: "service-resolver: redirect", + snake: ` + kind = "service-resolver" + name = "main" + redirect { + service = "other" + service_subset = "backup" + namespace = "alt" + datacenter = "dc9" + } + `, + camel: ` + Kind = "service-resolver" + Name = "main" + Redirect { + Service = "other" + ServiceSubset = "backup" + Namespace = "alt" + Datacenter = "dc9" + } + `, + expect: &ServiceResolverConfigEntry{ + Kind: "service-resolver", + Name: "main", + Redirect: &ServiceResolverRedirect{ + Service: "other", + ServiceSubset: "backup", + Namespace: "alt", + Datacenter: "dc9", + }, + }, + }, + { + name: "service-resolver: default", + snake: ` + kind = "service-resolver" + name = "main" + `, + camel: ` + Kind = "service-resolver" + Name = "main" + `, + expect: &ServiceResolverConfigEntry{ + Kind: "service-resolver", + Name: "main", + }, + }, + } { + tc := tc - for name, tcase := range cases { - name := name - tcase := tcase + testbody := func(t *testing.T, body string) { + t.Helper() - t.Run(name, func(t *testing.T) { - t.Parallel() + var raw map[string]interface{} + err := hcl.Decode(&raw, body) + require.NoError(t, err) - actual, err := DecodeConfigEntry(tcase.input) - if tcase.expectErr { + got, err := DecodeConfigEntry(raw) + if tc.expectErr != "" { + require.Nil(t, got) require.Error(t, err) + requireContainsLower(t, err.Error(), tc.expectErr) } else { require.NoError(t, err) - require.Equal(t, tcase.expected, actual) + require.Equal(t, tc.expect, got) } + } + + t.Run(tc.name+" (snake case)", func(t *testing.T) { + testbody(t, tc.snake) + }) + t.Run(tc.name+" (camel case)", func(t *testing.T) { + testbody(t, tc.camel) }) } } @@ -185,3 +641,8 @@ func TestConfigEntryResponseMarshalling(t *testing.T) { }) } } + +func requireContainsLower(t *testing.T, haystack, needle string) { + t.Helper() + require.Contains(t, strings.ToLower(haystack), strings.ToLower(needle)) +} diff --git a/command/config/write/config_write.go b/command/config/write/config_write.go index 593b76eed..8e0c7f77a 100644 --- a/command/config/write/config_write.go +++ b/command/config/write/config_write.go @@ -5,6 +5,7 @@ import ( "fmt" "io" + "github.com/hashicorp/consul/agent/structs" "github.com/hashicorp/consul/api" "github.com/hashicorp/consul/command/flags" "github.com/hashicorp/consul/command/helpers" @@ -108,6 +109,8 @@ func parseConfigEntry(data string) (api.ConfigEntry, error) { return newDecodeConfigEntry(raw) } +// There is a 'structs' variation of this in +// agent/structs/config_entry.go:DecodeConfigEntry func newDecodeConfigEntry(raw map[string]interface{}) (api.ConfigEntry, error) { var entry api.ConfigEntry @@ -129,66 +132,14 @@ func newDecodeConfigEntry(raw map[string]interface{}) (api.ConfigEntry, error) { return nil, fmt.Errorf("Kind value in payload is not a string") } - var ( - // skipWhenPatching should contain anything that legitimately contains a - // slice of structs when decoded. - skipWhenPatching []string - translateKeysDict map[string]string - ) - - switch entry.GetKind() { - case api.ProxyDefaults: - translateKeysDict = map[string]string{ - "mesh_gateway": "meshgateway", - } - case api.ServiceDefaults: - translateKeysDict = map[string]string{ - "mesh_gateway": "meshgateway", - } - case api.ServiceRouter: - skipWhenPatching = []string{ - "routes", - "Routes", - "routes.match.http.header", - "Routes.Match.HTTP.Header", - "routes.match.http.query_param", - "Routes.Match.HTTP.QueryParam", - } - translateKeysDict = map[string]string{ - "num_retries": "numretries", - "path_exact": "pathexact", - "path_prefix": "pathprefix", - "path_regex": "pathregex", - "prefix_rewrite": "prefixrewrite", - "query_param": "queryparam", - "request_timeout": "requesttimeout", - "retry_on_connect_failure": "retryonconnectfailure", - "retry_on_status_codes": "retryonstatuscodes", - "service_subset": "servicesubset", - } - case api.ServiceSplitter: - skipWhenPatching = []string{ - "splits", - "Splits", - } - translateKeysDict = map[string]string{ - "service_subset": "servicesubset", - } - case api.ServiceResolver: - translateKeysDict = map[string]string{ - "connect_timeout": "connecttimeout", - "default_subset": "defaultsubset", - "only_passing": "onlypassing", - "overprovisioning_factor": "overprovisioningfactor", - "service_subset": "servicesubset", - } - default: - return nil, fmt.Errorf("kind %q should be explicitly handled here", entry.GetKind()) + skipWhenPatching, translateKeysDict, err := structs.ConfigEntryDecodeRulesForKind(entry.GetKind()) + if err != nil { + return nil, err } // lib.TranslateKeys doesn't understand []map[string]interface{} so we have - // to do this part first. Any config entry - raw = lib.PatchSliceOfMaps(raw, skipWhenPatching) + // to do this part first. + raw = lib.PatchSliceOfMaps(raw, skipWhenPatching, nil) // CamelCase is the canonical form for these, since this translation // happens in the `consul config write` command and the JSON form is sent diff --git a/command/config/write/config_write_test.go b/command/config/write/config_write_test.go index 3211b8dcf..8e77bb9bc 100644 --- a/command/config/write/config_write_test.go +++ b/command/config/write/config_write_test.go @@ -106,7 +106,10 @@ func TestConfigWrite(t *testing.T) { }) } +// TestParseConfigEntry is the 'api' mirror image of +// agent/structs/config_entry_test.go:TestDecodeConfigEntry func TestParseConfigEntry(t *testing.T) { + t.Parallel() for _, tc := range []struct { name string camel string diff --git a/lib/patch_hcl.go b/lib/patch_hcl.go index ecf5ea6fd..1babf3f13 100644 --- a/lib/patch_hcl.go +++ b/lib/patch_hcl.go @@ -4,11 +4,11 @@ import ( "fmt" ) -func PatchSliceOfMaps(m map[string]interface{}, skip []string) map[string]interface{} { - return patchValue("", m, skip).(map[string]interface{}) +func PatchSliceOfMaps(m map[string]interface{}, skip []string, skipTree []string) map[string]interface{} { + return patchValue("", m, skip, skipTree).(map[string]interface{}) } -func patchValue(name string, v interface{}, skip []string) interface{} { +func patchValue(name string, v interface{}, skip []string, skipTree []string) interface{} { // fmt.Printf("%q: %T\n", name, v) switch x := v.(type) { case map[string]interface{}: @@ -21,7 +21,7 @@ func patchValue(name string, v interface{}, skip []string) interface{} { if name != "" { key = name + "." + k } - mm[k] = patchValue(key, v, skip) + mm[k] = patchValue(key, v, skip, skipTree) } return mm @@ -29,9 +29,12 @@ func patchValue(name string, v interface{}, skip []string) interface{} { if len(x) == 0 { return nil } + if strSliceContains(name, skipTree) { + return x + } if strSliceContains(name, skip) { for i, y := range x { - x[i] = patchValue(name, y, skip) + x[i] = patchValue(name, y, skip, skipTree) } return x } @@ -41,22 +44,25 @@ func patchValue(name string, v interface{}, skip []string) interface{} { if len(x) > 1 { panic(fmt.Sprintf("%s: []map[string]interface{} with more than one element not supported: %s", name, v)) } - return patchValue(name, x[0], skip) + return patchValue(name, x[0], skip, skipTree) case []map[string]interface{}: if len(x) == 0 { return nil } + if strSliceContains(name, skipTree) { + return x + } if strSliceContains(name, skip) { for i, y := range x { - x[i] = patchValue(name, y, skip).(map[string]interface{}) + x[i] = patchValue(name, y, skip, skipTree).(map[string]interface{}) } return x } if len(x) > 1 { panic(fmt.Sprintf("%s: []map[string]interface{} with more than one element not supported: %s", name, v)) } - return patchValue(name, x[0], skip) + return patchValue(name, x[0], skip, skipTree) default: return v diff --git a/lib/patch_hcl_test.go b/lib/patch_hcl_test.go index d5363e2a3..1c3f4844e 100644 --- a/lib/patch_hcl_test.go +++ b/lib/patch_hcl_test.go @@ -17,8 +17,9 @@ func parse(s string) map[string]interface{} { func TestPatchSliceOfMaps(t *testing.T) { tests := []struct { - in, out string - skip []string + in, out string + skip []string + skipTree []string }{ { in: `{"a":{"b":"c"}}`, @@ -64,12 +65,56 @@ func TestPatchSliceOfMaps(t *testing.T) { }`, skip: []string{"services", "services.checks"}, }, + { + // inspired by the 'config_entries.bootstrap.*' structure for configs + in: ` + { + "a": [ + { + "b": [ + { + "c": "val1", + "d": { + "foo": "bar" + }, + "e": [ + { + "super": "duper" + } + ] + } + ] + } + ] + } + `, + out: ` + { + "a": { + "b": [ + { + "c": "val1", + "d": { + "foo": "bar" + }, + "e": [ + { + "super": "duper" + } + ] + } + ] + } + } + `, + skipTree: []string{"a.b"}, + }, } for i, tt := range tests { desc := fmt.Sprintf("%02d: %s -> %s skip: %v", i, tt.in, tt.out, tt.skip) t.Run(desc, func(t *testing.T) { - out := PatchSliceOfMaps(parse(tt.in), tt.skip) + out := PatchSliceOfMaps(parse(tt.in), tt.skip, tt.skipTree) if got, want := out, parse(tt.out); !reflect.DeepEqual(got, want) { t.Fatalf("\ngot %#v\nwant %#v", got, want) }