Allow for both snake_case and CamelCase for config entries written with 'consul config write'. (#6044)

This also has the added benefit of fixing an issue with passing
time.Duration fields through config entries.
This commit is contained in:
R.B. Boyer 2019-06-28 11:35:35 -05:00 committed by GitHub
parent 6a52f9f9fb
commit 38d76c624e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 666 additions and 55 deletions

View File

@ -70,7 +70,7 @@ func Parse(data string, format string) (c Config, err error) {
// []map[string]interface{} or are arrays of structs which
// encoding/json will decode to []map[string]interface{}. Therefore,
// we need to be able to specify exceptions for this mapping. The
// patchSliceOfMaps() implements that mapping. All fields of type
// PatchSliceOfMaps() implements that mapping. All fields of type
// []map[string]interface{} are mapped to map[string]interface{} if
// it contains at most one value. If there is more than one value it
// panics. To define exceptions one can specify the nested field
@ -78,7 +78,7 @@ func Parse(data string, format string) (c Config, err error) {
//
// todo(fs): There might be an easier way to achieve the same thing
// todo(fs): but this approach works for now.
m := patchSliceOfMaps(raw, []string{
m := lib.PatchSliceOfMaps(raw, []string{
"checks",
"segments",
"service.checks",

View File

@ -2463,7 +2463,7 @@ func TestConfigFlagsAndEdgecases(t *testing.T) {
{
// This tests that we correct added the nested paths to arrays of objects
// to the exceptions in patchSliceOfMaps in config.go (for single service)
// to the exceptions in PatchSliceOfMaps in config.go (for single service)
desc: "service.connectsidecar_service with checks and upstreams",
args: []string{
`-data-dir=` + dataDir,
@ -2559,7 +2559,7 @@ func TestConfigFlagsAndEdgecases(t *testing.T) {
},
{
// This tests that we correct added the nested paths to arrays of objects
// to the exceptions in patchSliceOfMaps in config.go (for service*s*)
// to the exceptions in PatchSliceOfMaps in config.go (for service*s*)
desc: "services.connect.sidecar_service with checks and upstreams",
args: []string{
`-data-dir=` + dataDir,

View File

@ -155,7 +155,7 @@ func (s *Server) makeUpstreamCluster(upstream structs.Upstream, cfgSnap *proxycf
// arbitrary proto3 json format string or an error if it's invalid.
//
// For now we only support embedding in JSON strings because of the hcl parsing
// pain (see config.go comment above call to patchSliceOfMaps). Until we
// pain (see config.go comment above call to PatchSliceOfMaps). Until we
// refactor config parser a _lot_ user's opaque config that contains arrays will
// be mangled. We could actually fix that up in mapstructure which knows the
// type of the target so could resolve the slices to singletons unambiguously

View File

@ -85,7 +85,7 @@ func makeListener(name, addr string, port int) *envoy.Listener {
// arbitrary proto3 json format string or an error if it's invalid.
//
// For now we only support embedding in JSON strings because of the hcl parsing
// pain (see config.go comment above call to patchSliceOfMaps). Until we
// pain (see config.go comment above call to PatchSliceOfMaps). Until we
// refactor config parser a _lot_ user's opaque config that contains arrays will
// be mangled. We could actually fix that up in mapstructure which knows the
// type of the target so could resolve the slices to singletons unambiguously

View File

@ -84,15 +84,15 @@ type rawEntryListResponse struct {
func makeConfigEntry(kind, name string) (ConfigEntry, error) {
switch kind {
case ServiceDefaults:
return &ServiceConfigEntry{Name: name}, nil
return &ServiceConfigEntry{Kind: kind, Name: name}, nil
case ProxyDefaults:
return &ProxyConfigEntry{Name: name}, nil
return &ProxyConfigEntry{Kind: kind, Name: name}, nil
case ServiceRouter:
return &ServiceRouterConfigEntry{Name: name}, nil
return &ServiceRouterConfigEntry{Kind: kind, Name: name}, nil
case ServiceSplitter:
return &ServiceSplitterConfigEntry{Name: name}, nil
return &ServiceSplitterConfigEntry{Kind: kind, Name: name}, nil
case ServiceResolver:
return &ServiceResolverConfigEntry{Name: name}, nil
return &ServiceResolverConfigEntry{Kind: kind, Name: name}, nil
default:
return nil, fmt.Errorf("invalid config entry kind: %s", kind)
}

View File

@ -100,7 +100,7 @@ type ServiceResolverConfigEntry struct {
ModifyIndex uint64
}
func (e *ServiceResolverConfigEntry) GetKind() string { return ServiceResolver }
func (e *ServiceResolverConfigEntry) GetKind() string { return e.Kind }
func (e *ServiceResolverConfigEntry) GetName() string { return e.Name }
func (e *ServiceResolverConfigEntry) GetCreateIndex() uint64 { return e.CreateIndex }
func (e *ServiceResolverConfigEntry) GetModifyIndex() uint64 { return e.ModifyIndex }

View File

@ -8,8 +8,11 @@ import (
"github.com/hashicorp/consul/api"
"github.com/hashicorp/consul/command/flags"
"github.com/hashicorp/consul/command/helpers"
"github.com/hashicorp/consul/lib"
"github.com/hashicorp/go-multierror"
"github.com/hashicorp/hcl"
"github.com/mitchellh/cli"
"github.com/mitchellh/mapstructure"
)
func New(ui cli.Ui) *cmd {
@ -44,43 +47,6 @@ func (c *cmd) init() {
c.help = flags.Usage(help, c.flags)
}
type genericConfig struct {
Kind1 string `hcl:"Kind"`
Kind2 string `hcl:"kind"`
}
func (c *genericConfig) Kind() string {
if c.Kind1 != "" {
return c.Kind1
}
return c.Kind2
}
func decodeConfigEntryFromHCL(data string) (api.ConfigEntry, error) {
// For why this is necessary see the comment block on api.DecodeConfigEntry.
var generic genericConfig
err := hcl.Decode(&generic, data)
if err != nil {
return nil, err
}
kindVal := generic.Kind()
if kindVal == "" {
return nil, fmt.Errorf("Payload does not contain a kind/Kind key at the top level")
}
entry, err := api.MakeConfigEntry(kindVal, "")
if err != nil {
return nil, err
}
err = hcl.Decode(entry, data)
if err != nil {
return nil, err
}
return entry, nil
}
func (c *cmd) Run(args []string) int {
if err := c.flags.Parse(args); err != nil {
return 1
@ -98,7 +64,7 @@ func (c *cmd) Run(args []string) int {
return 1
}
entry, err := decodeConfigEntryFromHCL(string(data))
entry, err := parseConfigEntry(string(data))
if err != nil {
c.UI.Error(fmt.Sprintf("Failed to decode config entry input: %v", err))
return 1
@ -132,6 +98,124 @@ func (c *cmd) Run(args []string) int {
return 0
}
func parseConfigEntry(data string) (api.ConfigEntry, error) {
// parse the data
var raw map[string]interface{}
if err := hcl.Decode(&raw, data); err != nil {
return nil, fmt.Errorf("Failed to decode config entry input: %v", err)
}
return newDecodeConfigEntry(raw)
}
func newDecodeConfigEntry(raw map[string]interface{}) (api.ConfigEntry, error) {
var entry api.ConfigEntry
kindVal, ok := raw["Kind"]
if !ok {
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 {
newEntry, err := api.MakeConfigEntry(kindStr, "")
if err != nil {
return nil, err
}
entry = newEntry
} else {
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:
case api.ServiceDefaults:
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())
}
// lib.TranslateKeys doesn't understand []map[string]interface{} so we have
// to do this part first. Any config entry
raw = lib.PatchSliceOfMaps(raw, skipWhenPatching)
// CamelCase is the canonical form for these, since this translation
// happens in the `consul config write` command and the JSON form is sent
// off to the server.
lib.TranslateKeys(raw, translateKeysDict)
var md mapstructure.Metadata
decodeConf := &mapstructure.DecoderConfig{
DecodeHook: mapstructure.StringToTimeDurationHookFunc(),
Metadata: &md,
Result: &entry,
WeaklyTypedInput: true,
}
decoder, err := mapstructure.NewDecoder(decodeConf)
if err != nil {
return nil, err
}
if err := decoder.Decode(raw); err != nil {
return nil, err
}
for _, k := range md.Unused {
err = multierror.Append(err, fmt.Errorf("invalid config key %q", k))
}
if err != nil {
return nil, err
}
return entry, nil
}
func (c *cmd) Synopsis() string {
return synopsis
}

View File

@ -3,7 +3,9 @@ package write
import (
"io"
"os"
"strings"
"testing"
"time"
"github.com/hashicorp/consul/agent"
"github.com/hashicorp/consul/api"
@ -102,5 +104,530 @@ func TestConfigWrite(t *testing.T) {
require.NotEqual(t, 0, code)
require.NotEmpty(t, ui.ErrorWriter.String())
})
}
func TestParseConfigEntry(t *testing.T) {
for _, tc := range []struct {
name string
camel string
snake string
expect api.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"
}
}
`,
camel: `
Kind = "proxy-defaults"
Name = "main"
Config {
"foo" = 19
"bar" = "abc"
"moreconfig" {
"moar" = "config"
}
}
`,
expect: &api.ProxyConfigEntry{
Kind: "proxy-defaults",
Name: "main",
Config: map[string]interface{}{
"foo": 19,
"bar": "abc",
"moreconfig": map[string]interface{}{
"moar": "config",
},
},
},
},
{
name: "service-defaults",
snake: `
kind = "service-defaults"
name = "main"
protocol = "http"
`,
camel: `
Kind = "service-defaults"
Name = "main"
Protocol = "http"
`,
expect: &api.ServiceConfigEntry{
Kind: "service-defaults",
Name: "main",
Protocol: "http",
},
},
{
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: &api.ServiceRouterConfigEntry{
Kind: "service-router",
Name: "main",
Routes: []api.ServiceRoute{
{
Match: &api.ServiceRouteMatch{
HTTP: &api.ServiceRouteHTTPMatch{
PathExact: "/foo",
Header: []api.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: &api.ServiceRouteDestination{
Service: "carrot",
ServiceSubset: "kale",
Namespace: "leek",
PrefixRewrite: "/alternate",
RequestTimeout: 99 * time.Second,
NumRetries: 12345,
RetryOnConnectFailure: true,
RetryOnStatusCodes: []uint32{401, 209},
},
},
{
Match: &api.ServiceRouteMatch{
HTTP: &api.ServiceRouteHTTPMatch{
PathPrefix: "/foo",
QueryParam: []api.ServiceRouteHTTPMatchQueryParam{
{
Name: "hack1",
},
{
Name: "hack2",
Value: "1",
},
{
Name: "hack3",
Value: "a.*z",
Regex: true,
},
},
},
},
},
{
Match: &api.ServiceRouteMatch{
HTTP: &api.ServiceRouteHTTPMatch{
PathRegex: "/foo",
},
},
},
},
},
},
{
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: &api.ServiceSplitterConfigEntry{
Kind: api.ServiceSplitter,
Name: "main",
Splits: []api.ServiceSplit{
{
Weight: 99.1,
ServiceSubset: "v1",
},
{
Weight: 0.9,
Service: "other",
Namespace: "alt",
},
},
},
},
{
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: &api.ServiceResolverConfigEntry{
Kind: "service-resolver",
Name: "main",
DefaultSubset: "v1",
ConnectTimeout: 15 * time.Second,
Subsets: map[string]api.ServiceResolverSubset{
"v1": {
Filter: "Service.Meta.version == v1",
},
"v2": {
Filter: "Service.Meta.version == v2",
OnlyPassing: true,
},
},
Failover: map[string]api.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: &api.ServiceResolverConfigEntry{
Kind: "service-resolver",
Name: "main",
Redirect: &api.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: &api.ServiceResolverConfigEntry{
Kind: "service-resolver",
Name: "main",
},
},
} {
tc := tc
testbody := func(t *testing.T, body string) {
t.Helper()
got, err := parseConfigEntry(body)
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, 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)
})
}
}
func requireContainsLower(t *testing.T, haystack, needle string) {
t.Helper()
require.Contains(t, strings.ToLower(haystack), strings.ToLower(needle))
}

View File

@ -1,10 +1,10 @@
package config
package lib
import (
"fmt"
)
func patchSliceOfMaps(m map[string]interface{}, skip []string) map[string]interface{} {
func PatchSliceOfMaps(m map[string]interface{}, skip []string) map[string]interface{} {
return patchValue("", m, skip).(map[string]interface{})
}

View File

@ -1,4 +1,4 @@
package config
package lib
import (
"encoding/json"
@ -69,7 +69,7 @@ func TestPatchSliceOfMaps(t *testing.T) {
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)
if got, want := out, parse(tt.out); !reflect.DeepEqual(got, want) {
t.Fatalf("\ngot %#v\nwant %#v", got, want)
}