diff --git a/agent/agent_endpoint.go b/agent/agent_endpoint.go index 307397e616..b29d950c1b 100644 --- a/agent/agent_endpoint.go +++ b/agent/agent_endpoint.go @@ -20,7 +20,6 @@ import ( "github.com/hashicorp/consul/acl" cachetype "github.com/hashicorp/consul/agent/cache-types" "github.com/hashicorp/consul/agent/checks" - "github.com/hashicorp/consul/agent/config" "github.com/hashicorp/consul/agent/debug" "github.com/hashicorp/consul/agent/local" "github.com/hashicorp/consul/agent/structs" @@ -859,7 +858,7 @@ func (s *HTTPServer) AgentRegisterService(resp http.ResponseWriter, req *http.Re // 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{ + lib.TranslateKeys(rawMap, map[string]string{ "enable_tag_override": "EnableTagOverride", // Managed Proxy Config "exec_mode": "ExecMode", diff --git a/agent/config.go b/agent/config.go index bb7d8a2dce..8cb088f03b 100644 --- a/agent/config.go +++ b/agent/config.go @@ -6,7 +6,7 @@ import ( "strings" "time" - "github.com/hashicorp/consul/agent/config" + "github.com/hashicorp/consul/lib" ) var errInvalidHeaderFormat = errors.New("agent: invalid format of 'header' field") @@ -21,7 +21,7 @@ func FixupCheckType(raw interface{}) error { // and why we should get rid of it. In Consul 1.0 we also didn't map // Args correctly, so we ended up exposing (and need to carry forward) // ScriptArgs, see https://github.com/hashicorp/consul/issues/3587. - config.TranslateKeys(rawMap, map[string]string{ + lib.TranslateKeys(rawMap, map[string]string{ "args": "ScriptArgs", "script_args": "ScriptArgs", "deregister_critical_service_after": "DeregisterCriticalServiceAfter", diff --git a/agent/config/builder.go b/agent/config/builder.go index 7961e0d78d..dafc25a9f5 100644 --- a/agent/config/builder.go +++ b/agent/config/builder.go @@ -562,7 +562,7 @@ func (b *Builder) Build() (rt RuntimeConfig, err error) { connectCAProvider := b.stringVal(c.Connect.CAProvider) connectCAConfig := c.Connect.CAConfig if connectCAConfig != nil { - TranslateKeys(connectCAConfig, map[string]string{ + lib.TranslateKeys(connectCAConfig, map[string]string{ // Consul CA config "private_key": "PrivateKey", "root_cert": "RootCert", diff --git a/agent/config/config.go b/agent/config/config.go index 9cbfa2ad32..fdafdb1636 100644 --- a/agent/config/config.go +++ b/agent/config/config.go @@ -5,6 +5,7 @@ import ( "fmt" "strings" + "github.com/hashicorp/consul/lib" multierror "github.com/hashicorp/go-multierror" "github.com/hashicorp/hcl" "github.com/mitchellh/mapstructure" @@ -114,7 +115,7 @@ func Parse(data string, format string) (c Config, err error) { // 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{ + lib.TranslateKeys(m, map[string]string{ "deregistercriticalserviceafter": "deregister_critical_service_after", "dockercontainerid": "docker_container_id", "scriptargs": "args", diff --git a/agent/config/runtime_test.go b/agent/config/runtime_test.go index f285fe7a4e..70e6eb3d96 100644 --- a/agent/config/runtime_test.go +++ b/agent/config/runtime_test.go @@ -2715,7 +2715,7 @@ func TestConfigFlagsAndEdgecases(t *testing.T) { foo = "bar" } }`}, - err: "config_entries.bootstrap[0]: Payload does not contain a kind/Kind", + err: "config_entries.bootstrap[0]: Payload does not contain a Kind", }, { desc: "ConfigEntry bootstrap unknown kind", diff --git a/agent/config_endpoint.go b/agent/config_endpoint.go index c8387c6db0..18b7130ab9 100644 --- a/agent/config_endpoint.go +++ b/agent/config_endpoint.go @@ -44,7 +44,7 @@ func (s *HTTPServer) configGet(resp http.ResponseWriter, req *http.Request) (int } if reply.Entry == nil { - return nil, fmt.Errorf("Config entry not found for %q / %q", pathArgs[0], pathArgs[1]) + return nil, NotFoundError{Reason: fmt.Sprintf("Config entry not found for %q / %q", pathArgs[0], pathArgs[1])} } return reply.Entry, nil @@ -59,9 +59,7 @@ func (s *HTTPServer) configGet(resp http.ResponseWriter, req *http.Request) (int return reply.Entries, nil default: - resp.WriteHeader(http.StatusNotFound) - fmt.Fprintf(resp, "Must provide either a kind or both kind and name") - return nil, nil + return nil, NotFoundError{Reason: "Must provide either a kind or both kind and name"} } } diff --git a/agent/config_endpoint_test.go b/agent/config_endpoint_test.go index 5c2f049456..3c13a4b232 100644 --- a/agent/config_endpoint_test.go +++ b/agent/config_endpoint_test.go @@ -273,7 +273,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/Kind key at the top level", badReq.Reason) + require.Equal(t, "Request decoding failed: Payload does not contain a Kind key at the top level", badReq.Reason) }) t.Run("Kind Not String", func(t *testing.T) { diff --git a/agent/consul/config_endpoint.go b/agent/consul/config_endpoint.go index 99e70c8210..59a0e429f6 100644 --- a/agent/consul/config_endpoint.go +++ b/agent/consul/config_endpoint.go @@ -106,6 +106,10 @@ func (c *ConfigEntry) List(args *structs.ConfigEntryQuery, reply *structs.Indexe return err } + if args.Kind != "" && !structs.ValidateConfigEntryKind(args.Kind) { + return fmt.Errorf("invalid config entry kind: %s", args.Kind) + } + return c.srv.blockingQuery( &args.QueryOptions, &reply.QueryMeta, diff --git a/agent/http.go b/agent/http.go index a7e159d0fb..acbfdb3e4f 100644 --- a/agent/http.go +++ b/agent/http.go @@ -45,6 +45,15 @@ func (e BadRequestError) Error() string { return fmt.Sprintf("Bad request: %s", e.Reason) } +// NotFoundError should be returned by a handler when a resource specified does not exist +type NotFoundError struct { + Reason string +} + +func (e NotFoundError) Error() string { + return e.Reason +} + // CodeWithPayloadError allow returning non HTTP 200 // Error codes while not returning PlainText payload type CodeWithPayloadError struct { @@ -324,6 +333,11 @@ func (s *HTTPServer) wrap(handler endpoint, methods []string) http.HandlerFunc { return ok } + isNotFound := func(err error) bool { + _, ok := err.(NotFoundError) + return ok + } + isTooManyRequests := func(err error) bool { // Sadness net/rpc can't do nice typed errors so this is all we got return err.Error() == consul.ErrRateLimited.Error() @@ -352,6 +366,9 @@ func (s *HTTPServer) wrap(handler endpoint, methods []string) http.HandlerFunc { case isBadRequest(err): resp.WriteHeader(http.StatusBadRequest) fmt.Fprint(resp, err.Error()) + case isNotFound(err): + resp.WriteHeader(http.StatusNotFound) + fmt.Fprintf(resp, err.Error()) case isTooManyRequests(err): resp.WriteHeader(http.StatusTooManyRequests) fmt.Fprint(resp, err.Error()) diff --git a/agent/structs/config_entry.go b/agent/structs/config_entry.go index 3300cfe7ba..059ba35c44 100644 --- a/agent/structs/config_entry.go +++ b/agent/structs/config_entry.go @@ -218,14 +218,20 @@ func (e *ProxyConfigEntry) UnmarshalBinary(data []byte) error { // first decode into a map[string]interface{} and then call this function to decode // into a concrete type. 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", + "Config": "", + }) + var entry 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") + return nil, fmt.Errorf("Payload does not contain a Kind key at the top level") } if kindStr, ok := kindVal.(string); ok { @@ -335,6 +341,15 @@ func MakeConfigEntry(kind, name string) (ConfigEntry, error) { } } +func ValidateConfigEntryKind(kind string) bool { + switch kind { + case ServiceDefaults, ProxyDefaults: + return true + default: + return false + } +} + // ConfigEntryQuery is used when requesting info about a config entry. type ConfigEntryQuery struct { Kind string diff --git a/agent/structs/config_entry_test.go b/agent/structs/config_entry_test.go new file mode 100644 index 0000000000..0ffafe2231 --- /dev/null +++ b/agent/structs/config_entry_test.go @@ -0,0 +1,102 @@ +package structs + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +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, + Config: map[string]interface{}{ + "foo": "bar", + }, + }, + }, + "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, + }, + }, + }, + "service-defaults": tcase{ + input: map[string]interface{}{ + "Kind": ServiceDefaults, + "Name": "foo", + "Protocol": "tcp", + "Connect": map[string]interface{}{ + "SidecarProxy": true, + }, + }, + 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, + }, + }, + expected: &ServiceConfigEntry{ + Kind: ServiceDefaults, + Name: "foo", + Protocol: "tcp", + Connect: ConnectConfiguration{SidecarProxy: true}, + }, + }, + } + + for name, tcase := range cases { + name := name + tcase := tcase + + t.Run(name, func(t *testing.T) { + t.Parallel() + + actual, err := DecodeConfigEntry(tcase.input) + if tcase.expectErr { + require.Error(t, err) + } else { + require.NoError(t, err) + require.Equal(t, tcase.expected, actual) + } + }) + } +} diff --git a/agent/config/translate.go b/lib/translate.go similarity index 99% rename from agent/config/translate.go rename to lib/translate.go index 51752b4527..97a0204ec7 100644 --- a/agent/config/translate.go +++ b/lib/translate.go @@ -1,4 +1,4 @@ -package config +package lib import ( "strings" diff --git a/agent/config/translate_test.go b/lib/translate_test.go similarity index 93% rename from agent/config/translate_test.go rename to lib/translate_test.go index aaa5dad19f..eb04c8cf69 100644 --- a/agent/config/translate_test.go +++ b/lib/translate_test.go @@ -1,10 +1,10 @@ -package config +package lib import ( "encoding/json" "testing" - "github.com/pascaldekloe/goe/verify" + "github.com/stretchr/testify/require" ) func TestTranslateKeys(t *testing.T) { @@ -75,9 +75,7 @@ func TestTranslateKeys(t *testing.T) { 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() - } + require.Equal(t, tt.out, tt.in) }) } }