luxolus
/
open-consul
2
0
Fork 0
Code Issues 1 Pull Requests Packages Releases Wiki Activity

config: add generic method to translate between CamelCase and snake_case (#3557) 

* doc: document discrepancy between id and CheckID

* doc: document enable_tag_override change

* config: add TranslateKeys helper

TranslateKeys makes it easier to map between different representations
of internal structures. It allows to recursively map alias keys to
canonical keys in structured maps.

* config: use TranslateKeys for config file

This also adds support for 'enabletagoverride' and removes
the need for a separate CheckID alias field.

* config: remove dead code

* agent: use TranslateKeys for FixupCheckType

* agent: translate enable_tag_override during service registration

* doc: add '.hcl' as valid extension

* config: map ScriptArgs to args

* config: add comment for TranslateKeys
This commit is contained in:
Frank Schröder 2017-10-11 01:40:59 +02:00 committed by James Phillips
parent 279ade8ef2
commit fa22ad4573
10 changed files with 296 additions and 50 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
agent/agent_endpoint.go
View File

@ -8,6 +8,7 @@ import (
"strings"
"github.com/hashicorp/consul/acl"
"github.com/hashicorp/consul/agent/config"
"github.com/hashicorp/consul/agent/structs"
"github.com/hashicorp/consul/api"
"github.com/hashicorp/consul/ipaddr"
@ -524,6 +525,12 @@ func (s *HTTPServer) AgentRegisterService(resp http.ResponseWriter, req *http.Re
return nil
}
// see https://github.com/hashicorp/consul/pull/3557 why we need this
// and why we should get rid of it.
config.TranslateKeys(rawMap, map[string]string{
"enable_tag_override": "EnableTagOverride",
})
for k, v := range rawMap {
switch strings.ToLower(k) {
case "check":

29
agent/agent_endpoint_test.go
View File

@ -21,6 +21,7 @@ import (
"github.com/hashicorp/consul/testutil/retry"
"github.com/hashicorp/consul/types"
"github.com/hashicorp/serf/serf"
"github.com/pascaldekloe/goe/verify"
)
func makeReadOnlyAgentACL(t *testing.T, srv *HTTPServer) string {
@ -1157,6 +1158,34 @@ func TestAgent_RegisterService(t *testing.T) {
}
}
func TestAgent_RegisterService_TranslateKeys(t *testing.T) {
t.Parallel()
a := NewTestAgent(t.Name(), "")
defer a.Shutdown()
json := `{"name":"test", "port":8000, "enable_tag_override": true}`
req, _ := http.NewRequest("PUT", "/v1/agent/service/register", strings.NewReader(json))
obj, err := a.srv.AgentRegisterService(nil, req)
if err != nil {
t.Fatalf("err: %v", err)
}
if obj != nil {
t.Fatalf("bad: %v", obj)
}
svc := &structs.NodeService{
ID: "test",
Service: "test",
Port: 8000,
EnableTagOverride: true,
}
if got, want := a.state.Services()["test"], svc; !verify.Values(t, "", got, want) {
t.Fail()
}
}
func TestAgent_RegisterService_ACLDeny(t *testing.T) {
t.Parallel()
a := NewTestAgent(t.Name(), TestACLConfig())

36
agent/config.go
View File

@ -5,6 +5,8 @@ import (
"fmt"
"strings"
"time"
"github.com/hashicorp/consul/agent/config"
)
var errInvalidHeaderFormat = errors.New("agent: invalid format of 'header' field")
@ -15,6 +17,15 @@ func FixupCheckType(raw interface{}) error {
return nil
}
// see https://github.com/hashicorp/consul/pull/3557 why we need this
// and why we should get rid of it.
config.TranslateKeys(rawMap, map[string]string{
"deregister_critical_service_after": "DeregisterCriticalServiceAfter",
"docker_container_id": "DockerContainerID",
"tls_skip_verify": "TLSSkipVerify",
"service_id": "ServiceID",
})
parseDuration := func(v interface{}) (time.Duration, error) {
if v == nil {
return 0, nil
@ -56,13 +67,6 @@ func FixupCheckType(raw interface{}) error {
return m, nil
}
replace := func(oldKey, newKey string, val interface{}) {
rawMap[newKey] = val
if oldKey != newKey {
delete(rawMap, oldKey)
}
}
for k, v := range rawMap {
switch strings.ToLower(k) {
case "header":
@ -72,28 +76,12 @@ func FixupCheckType(raw interface{}) error {
}
rawMap[k] = h
case "ttl", "interval", "timeout":
case "ttl", "interval", "timeout", "deregistercriticalserviceafter":
d, err := parseDuration(v)
if err != nil {
return fmt.Errorf("invalid %q: %v", k, err)
}
rawMap[k] = d
case "deregister_critical_service_after", "deregistercriticalserviceafter":
d, err := parseDuration(v)
if err != nil {
return fmt.Errorf("invalid %q: %v", k, err)
}
replace(k, "DeregisterCriticalServiceAfter", d)
case "docker_container_id":
replace(k, "DockerContainerID", v)
case "service_id":
replace(k, "ServiceID", v)
case "tls_skip_verify":
replace(k, "TLSSkipVerify", v)
}
}
return nil

3
agent/config/builder.go
View File

@ -869,9 +869,6 @@ func (b *Builder) checkVal(v *CheckDefinition) *structs.CheckDefinition {
}
id := types.CheckID(b.stringVal(v.ID))
if v.CheckID != nil {
id = types.CheckID(b.stringVal(v.CheckID))
}
return &structs.CheckDefinition{
ID: id,

33
agent/config/config.go
View File

@ -86,15 +86,29 @@ func Parse(data string, format string) (c Config, err error) {
"watches",
})
// toJSON := func(v interface{}) string {
// b, err := json.MarshalIndent(v, "", " ")
// if err != nil {
// panic(err)
// }
// return string(b)
// }
// fmt.Println("raw:", toJSON(raw))
// fmt.Println("patched:", toJSON(m))
// There is a difference of representation of some fields depending on
// where they are used. The HTTP API uses CamelCase whereas the config
// files use snake_case and between the two there is no automatic mapping.
// While the JSON and HCL parsers match keys without case (both `id` and
// `ID` are mapped to an ID field) the same thing does not happen between
// CamelCase and snake_case. Since changing either format would break
// existing setups we have to support both and slowly transition to one of
// the formats. Also, there is at least one case where we use the "wrong"
// key and want to map that to the new key to support deprecation
// (`check.id` vs `service.check.CheckID`) See [GH-3179]. TranslateKeys
// maps potentially CamelCased values to the snake_case that is used in the
// config file parser. If both the CamelCase and snake_case values are set,
// the snake_case value is used and the other value is discarded.
TranslateKeys(m, map[string]string{
"check_id": "id",
"checkid": "id",
"deregistercriticalserviceafter": "deregister_critical_service_after",
"dockercontainerid": "docker_container_id",
"enabletagoverride": "enable_tag_override",
"scriptargs": "args",
"serviceid": "service_id",
"tlsskipverify": "tls_skip_verify",
})
var md mapstructure.Metadata
d, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{
@ -305,7 +319,6 @@ type ServiceDefinition struct {
type CheckDefinition struct {
ID *string `json:"id,omitempty" hcl:"id" mapstructure:"id"`
CheckID *string `json:"check_id,omitempty" hcl:"check_id" mapstructure:"check_id"`
Name *string `json:"name,omitempty" hcl:"name" mapstructure:"name"`
Notes *string `json:"notes,omitempty" hcl:"notes" mapstructure:"notes"`
ServiceID *string `json:"service_id,omitempty" hcl:"service_id" mapstructure:"service_id"`

55
agent/config/runtime_test.go
View File

@ -1773,6 +1773,61 @@ func TestConfigFlagsAndEdgecases(t *testing.T) {
rt.DataDir = dataDir
},
},
{
desc: "translated keys",
flags: []string{
`-data-dir=` + dataDir,
},
json: []string{
`{
"service": {
"name": "a",
"port": 80,
"EnableTagOverride": true,
"check": {
"CheckID": "x",
"name": "y",
"DockerContainerID": "z",
"DeregisterCriticalServiceAfter": "10s",
"ScriptArgs": ["a", "b"]
}
}
}`,
},
hcl: []string{
`service = {
name = "a"
port = 80
EnableTagOverride = true
check = {
CheckID = "x"
name = "y"
DockerContainerID = "z"
DeregisterCriticalServiceAfter = "10s"
ScriptArgs = ["a", "b"]
}
}`,
},
patch: func(rt *RuntimeConfig) {
rt.Services = []*structs.ServiceDefinition{
&structs.ServiceDefinition{
Name: "a",
Port: 80,
EnableTagOverride: true,
Checks: []*structs.CheckType{
&structs.CheckType{
CheckID: types.CheckID("x"),
Name: "y",
DockerContainerID: "z",
DeregisterCriticalServiceAfter: 10 * time.Second,
ScriptArgs: []string{"a", "b"},
},
},
},
}
rt.DataDir = dataDir
},
},
}
testConfig(t, tests, dataDir)

54
agent/config/translate.go Normal file
View File

@ -0,0 +1,54 @@
package config
import "strings"
// TranslateKeys recursively translates all keys from m in-place to their
// canonical form as defined in dict which maps an alias name to the canonical
// name. If m already has a value for the canonical name then that one is used
// and the value for the alias name is discarded. Alias names are matched
// case-insensitive.
//
// Example:
//
// m = TranslateKeys(m, map[string]string{"CamelCase": "snake_case"})
//
func TranslateKeys(v map[string]interface{}, dict map[string]string) {
ck(v, dict)
}
func ck(v interface{}, dict map[string]string) interface{} {
switch x := v.(type) {
case map[string]interface{}:
for k, v := range x {
canonKey := dict[strings.ToLower(k)]
// no canonical key? -> use this key
if canonKey == "" {
x[k] = ck(v, dict)
continue
}
// delete the alias
delete(x, k)
// if there is a value for the canonical key then keep it
if _, ok := x[canonKey]; ok {
continue
}
// otherwise translate to the canonical key
x[canonKey] = ck(v, dict)
}
return x
case []interface{}:
var a []interface{}
for _, xv := range x {
a = append(a, ck(xv, dict))
}
return a
default:
return v
}
}

83
agent/config/translate_test.go Normal file
View File

@ -0,0 +1,83 @@
package config
import (
"encoding/json"
"testing"
"github.com/pascaldekloe/goe/verify"
)
func TestTranslateKeys(t *testing.T) {
fromJSON := func(s string) map[string]interface{} {
var m map[string]interface{}
if err := json.Unmarshal([]byte(s), &m); err != nil {
t.Fatal(err)
}
return m
}
tests := []struct {
desc string
in map[string]interface{}
out map[string]interface{}
dict map[string]string
}{
{
desc: "x->y",
in: map[string]interface{}{"a": "aa", "x": "xx"},
out: map[string]interface{}{"a": "aa", "y": "xx"},
dict: map[string]string{"x": "y"},
},
{
desc: "discard x",
in: map[string]interface{}{"a": "aa", "x": "xx", "y": "yy"},
out: map[string]interface{}{"a": "aa", "y": "yy"},
dict: map[string]string{"x": "y"},
},
{
desc: "b.x->b.y",
in: map[string]interface{}{"a": "aa", "b": map[string]interface{}{"x": "xx"}},
out: map[string]interface{}{"a": "aa", "b": map[string]interface{}{"y": "xx"}},
dict: map[string]string{"x": "y"},
},
{
desc: "json: x->y",
in: fromJSON(`{"a":"aa","x":"xx"}`),
out: fromJSON(`{"a":"aa","y":"xx"}`),
dict: map[string]string{"x": "y"},
},
{
desc: "json: X->y",
in: fromJSON(`{"a":"aa","X":"xx"}`),
out: fromJSON(`{"a":"aa","y":"xx"}`),
dict: map[string]string{"x": "y"},
},
{
desc: "json: discard x",
in: fromJSON(`{"a":"aa","x":"xx","y":"yy"}`),
out: fromJSON(`{"a":"aa","y":"yy"}`),
dict: map[string]string{"x": "y"},
},
{
desc: "json: b.x->b.y",
in: fromJSON(`{"a":"aa","b":{"x":"xx"}}`),
out: fromJSON(`{"a":"aa","b":{"y":"xx"}}`),
dict: map[string]string{"x": "y"},
},
{
desc: "json: b[0].x->b[0].y",
in: fromJSON(`{"a":"aa","b":[{"x":"xx"}]}`),
out: fromJSON(`{"a":"aa","b":[{"y":"xx"}]}`),
dict: map[string]string{"x": "y"},
},
}
for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) {
TranslateKeys(tt.in, tt.dict)
if got, want := tt.in, tt.out; !verify.Values(t, "", got, want) {
t.Fail()
}
})
}
}

15
website/source/docs/agent/checks.html.md
View File

@ -163,10 +163,17 @@ A Docker check:
}
```
Each type of definition must include a `name` and may optionally
provide an `id` and `notes` field. The `id` is set to the `name` if not
provided. It is required that all checks have a unique ID per node: if names
might conflict, unique IDs should be provided.
Each type of definition must include a `name` and may optionally provide an
`id` and `notes` field. The `id` must be unique per _agent_ otherwise only the
last defined check with that `id` will be registered. If the `id` is not set
and the check is embedded within a service definition a unique check id is
generated. Otherwise, `id` will be set to `name`. If names might conflict,
unique IDs should be provided.
-> **Note:** Consul 0.9.3 and before require the optional check ID for a check
that is embedded in a service definition to be configured via the `CheckID`
field. Consul 1.0 accepts both `id` and `CheckID` but the latter is
deprecated and will be removed in Consul 1.1.
The `notes` field is opaque to Consul but can be used to provide a human-readable
description of the current state of the check. With a script check, the field is

31
website/source/docs/agent/services.html.md
View File

@ -26,7 +26,7 @@ A service definition that is a script looks like:
"tags": ["primary"],
"address": "",
"port": 8000,
"enableTagOverride": false,
"enable_tag_override": false,
"checks": [
{
"script": "/usr/local/bin/check_redis.py",
@ -38,11 +38,15 @@ A service definition that is a script looks like:
```
A service definition must include a `name` and may optionally provide an
`id`, `tags`, `address`, `port`, `check`, and `enableTagOverride`. The
`id`, `tags`, `address`, `port`, `check`, and `enable_tag_override`. The
`id` is set to the `name` if not provided. It is required that all
services have a unique ID per node, so if names might conflict then
unique IDs should be provided.
For Consul 0.9.3 and earlier you need to use `enableTagOverride`. Consul 1.0
supports both `enable_tag_override` and `enableTagOverride` but the latter is
deprecated and will be removed in Consul 1.1.
The `tags` property is a list of values that are opaque to Consul but
can be used to distinguish between `primary` or `secondary` nodes,
different versions, or any other service level labels.
@ -75,32 +79,41 @@ from `1`.
-> **Note:** There is more information about [checks here](/docs/agent/checks.html).
The `enableTagOverride` can optionally be specified to disable the
anti-entropy feature for this service. If `enableTagOverride` is set to
-> **Note:** Consul 0.9.3 and before require the optional check ID for a check
that is embedded in a service definition to be configured via the `CheckID`
field. Consul 1.0 accepts both `id` and `CheckID` but the latter is
deprecated and will be removed in Consul 1.1.
The `enable_tag_override` can optionally be specified to disable the
anti-entropy feature for this service. If `enable_tag_override` is set to
`TRUE` then external agents can update this service in the
[catalog](/api/catalog.html) and modify the tags. Subsequent
local sync operations by this agent will ignore the updated tags. For
example, if an external agent modified both the tags and the port for
this service and `enableTagOverride` was set to `TRUE` then after the next
this service and `enable_tag_override` was set to `TRUE` then after the next
sync cycle the service's port would revert to the original value but the
tags would maintain the updated value. As a counter example: If an
external agent modified both the tags and port for this service and
`enableTagOverride` was set to `FALSE` then after the next sync cycle the
`enable_tag_override` was set to `FALSE` then after the next sync cycle the
service's port *and* the tags would revert to the original value and all
modifications would be lost.
It's important to note that this applies only to the locally registered
service. If you have multiple nodes all registering the same service
their `enableTagOverride` configuration and all other service
their `enable_tag_override` configuration and all other service
configuration items are independent of one another. Updating the tags
for the service registered on one node is independent of the same
service (by name) registered on another node. If `enableTagOverride` is
service (by name) registered on another node. If `enable_tag_override` is
not specified the default value is false. See [anti-entropy
syncs](/docs/internals/anti-entropy.html) for more info.
For Consul 0.9.3 and earlier you need to use `enableTagOverride`. Consul 1.0
supports both `enable_tag_override` and `enableTagOverride` but the latter is
deprecated and will be removed in Consul 1.1.
To configure a service, either provide it as a `-config-file` option to
the agent or place it inside the `-config-dir` of the agent. The file
must end in the `.json` extension to be loaded by Consul. Check
must end in the `.json` or `.hcl` extension to be loaded by Consul. Check
definitions can be updated by sending a `SIGHUP` to the agent.
Alternatively, the service can be registered dynamically using the [HTTP
API](/api/index.html).