Make a few config entry endpoints return 404s and allow for snake_case and lowercase key names. (#5748)

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",

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",

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",

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",

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",

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)
+		return nil, NotFoundError{Reason: "Must provide either a kind or both kind and name"}
-		fmt.Fprintf(resp, "Must provide either a kind or both kind and name")
-		return nil, nil
 	}
 }
 

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) {

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,

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())

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"]
+		return nil, fmt.Errorf("Payload does not contain a Kind key at the top level")
-	}
-	if !ok {
-		return nil, fmt.Errorf("Payload does not contain a kind/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

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)
+			}
+		})
+	}
+}

lib/translate.go

@@ -1,4 +1,4 @@
-package config
+package lib
 
 import (
 	"strings"

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) {
+			require.Equal(t, tt.out, tt.in)
-				t.Fail()
-			}
 		})
 	}
 }