agent/consul: proxy registration and tests

agent/consul/catalog_endpoint.go

@@ -47,6 +47,24 @@ func (c *Catalog) Register(args *structs.RegisterRequest, reply *struct{}) error
 
 	// Handle a service registration.
 	if args.Service != nil {
+		// Connect proxy specific logic
+		if args.Service.Kind == structs.ServiceKindConnectProxy {
+			// Name is optional, if it isn't set, we default to the
+			// proxy name. It actually MUST be this, but the validation
+			// below this will verify.
+			if args.Service.Service == "" {
+				args.Service.Service = fmt.Sprintf(
+					"%s-connect-proxy", args.Service.ProxyDestination)
+			}
+		}
+
+		// Validate the service. This is in addition to the below since
+		// the above just hasn't been moved over yet. We should move it over
+		// in time.
+		if err := args.Service.Validate(); err != nil {
+			return err
+		}
+
 		// If no service id, but service name, use default
 		if args.Service.ID == "" && args.Service.Service != "" {
 			args.Service.ID = args.Service.Service

agent/consul/catalog_endpoint_test.go

@@ -333,6 +333,87 @@ func TestCatalog_Register_ForwardDC(t *testing.T) {
 	}
 }
 
+func TestCatalog_Register_ConnectProxy(t *testing.T) {
+	t.Parallel()
+
+	assert := assert.New(t)
+	dir1, s1 := testServer(t)
+	defer os.RemoveAll(dir1)
+	defer s1.Shutdown()
+	codec := rpcClient(t, s1)
+	defer codec.Close()
+
+	args := structs.TestRegisterRequestProxy(t)
+
+	// Register
+	var out struct{}
+	assert.Nil(msgpackrpc.CallWithCodec(codec, "Catalog.Register", &args, &out))
+
+	// List
+	req := structs.ServiceSpecificRequest{
+		Datacenter:  "dc1",
+		ServiceName: args.Service.Service,
+	}
+	var resp structs.IndexedServiceNodes
+	assert.Nil(msgpackrpc.CallWithCodec(codec, "Catalog.ServiceNodes", &req, &resp))
+	assert.Len(resp.ServiceNodes, 1)
+	v := resp.ServiceNodes[0]
+	assert.Equal(structs.ServiceKindConnectProxy, v.ServiceKind)
+	assert.Equal(args.Service.ProxyDestination, v.ServiceProxyDestination)
+}
+
+// Test an invalid ConnectProxy. We don't need to exhaustively test because
+// this is all tested in structs on the Validate method.
+func TestCatalog_Register_ConnectProxy_invalid(t *testing.T) {
+	t.Parallel()
+
+	assert := assert.New(t)
+	dir1, s1 := testServer(t)
+	defer os.RemoveAll(dir1)
+	defer s1.Shutdown()
+	codec := rpcClient(t, s1)
+	defer codec.Close()
+
+	args := structs.TestRegisterRequestProxy(t)
+	args.Service.ProxyDestination = ""
+
+	// Register
+	var out struct{}
+	err := msgpackrpc.CallWithCodec(codec, "Catalog.Register", &args, &out)
+	assert.NotNil(err)
+	assert.Contains(err.Error(), "ProxyDestination")
+}
+
+// Test registering a proxy with no name set, which should work.
+func TestCatalog_Register_ConnectProxy_noName(t *testing.T) {
+	t.Parallel()
+
+	assert := assert.New(t)
+	dir1, s1 := testServer(t)
+	defer os.RemoveAll(dir1)
+	defer s1.Shutdown()
+	codec := rpcClient(t, s1)
+	defer codec.Close()
+
+	args := structs.TestRegisterRequestProxy(t)
+	args.Service.Service = ""
+
+	// Register
+	var out struct{}
+	assert.Nil(msgpackrpc.CallWithCodec(codec, "Catalog.Register", &args, &out))
+
+	// List
+	req := structs.ServiceSpecificRequest{
+		Datacenter:  "dc1",
+		ServiceName: fmt.Sprintf("%s-connect-proxy", args.Service.ProxyDestination),
+	}
+	var resp structs.IndexedServiceNodes
+	assert.Nil(msgpackrpc.CallWithCodec(codec, "Catalog.ServiceNodes", &req, &resp))
+	assert.Len(resp.ServiceNodes, 1)
+	v := resp.ServiceNodes[0]
+	assert.Equal(structs.ServiceKindConnectProxy, v.ServiceKind)
+}
+
 func TestCatalog_Deregister(t *testing.T) {
 	t.Parallel()
 	dir1, s1 := testServer(t)

agent/structs/structs.go

@@ -12,6 +12,7 @@ import (
 	"github.com/hashicorp/consul/api"
 	"github.com/hashicorp/consul/types"
 	"github.com/hashicorp/go-msgpack/codec"
+	"github.com/hashicorp/go-multierror"
 	"github.com/hashicorp/serf/coordinate"
 )
 
@@ -488,6 +489,31 @@ type NodeService struct {
 	RaftIndex
 }
 
+// Validate validates the node service configuration.
+//
+// NOTE(mitchellh): This currently only validates fields for a ConnectProxy.
+// Historically validation has been directly in the Catalog.Register RPC.
+// ConnectProxy validation was moved here for easier table testing, but
+// other validation still exists in Catalog.Register.
+func (s *NodeService) Validate() error {
+	var result error
+
+	// ConnectProxy validation
+	if s.Kind == ServiceKindConnectProxy {
+		if strings.TrimSpace(s.ProxyDestination) == "" {
+			result = multierror.Append(result, fmt.Errorf(
+				"ProxyDestination must be non-empty for Connect proxy services"))
+		}
+
+		if s.Port == 0 {
+			result = multierror.Append(result, fmt.Errorf(
+				"Port must be set for a Connect proxy"))
+		}
+	}
+
+	return result
+}
+
 // IsSame checks if one NodeService is the same as another, without looking
 // at the Raft information (that's why we didn't call it IsEqual). This is
 // useful for seeing if an update would be idempotent for all the functional

agent/structs/structs_test.go

@@ -8,6 +8,7 @@ import (
 
 	"github.com/hashicorp/consul/api"
 	"github.com/hashicorp/consul/types"
+	"github.com/stretchr/testify/assert"
 )
 
 func TestEncodeDecode(t *testing.T) {
@@ -208,6 +209,60 @@ func TestStructs_ServiceNode_Conversions(t *testing.T) {
 	}
 }
 
+func TestStructs_NodeService_ValidateConnectProxy(t *testing.T) {
+	cases := []struct {
+		Name   string
+		Modify func(*NodeService)
+		Err    string
+	}{
+		{
+			"valid",
+			func(x *NodeService) {},
+			"",
+		},
+
+		{
+			"connect-proxy: no ProxyDestination",
+			func(x *NodeService) { x.ProxyDestination = "" },
+			"ProxyDestination must be",
+		},
+
+		{
+			"connect-proxy: whitespace ProxyDestination",
+			func(x *NodeService) { x.ProxyDestination = "  " },
+			"ProxyDestination must be",
+		},
+
+		{
+			"connect-proxy: valid ProxyDestination",
+			func(x *NodeService) { x.ProxyDestination = "hello" },
+			"",
+		},
+
+		{
+			"connect-proxy: no port set",
+			func(x *NodeService) { x.Port = 0 },
+			"Port must",
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.Name, func(t *testing.T) {
+			assert := assert.New(t)
+			ns := TestNodeServiceProxy(t)
+			tc.Modify(ns)
+
+			err := ns.Validate()
+			assert.Equal(err != nil, tc.Err != "", err)
+			if err == nil {
+				return
+			}
+
+			assert.Contains(strings.ToLower(err.Error()), strings.ToLower(tc.Err))
+		})
+	}
+}
+
 func TestStructs_NodeService_IsSame(t *testing.T) {
 	ns := &NodeService{
 		ID:      "node1",

agent/structs/testing_catalog.go

@@ -11,12 +11,18 @@ func TestRegisterRequestProxy(t testing.T) *RegisterRequest {
 		Datacenter: "dc1",
 		Node:       "foo",
 		Address:    "127.0.0.1",
-		Service: &NodeService{
+		Service:    TestNodeServiceProxy(t),
-			Kind:             ServiceKindConnectProxy,
+	}
-			Service:          ConnectProxyServiceName,
+}
-			Address:          "127.0.0.2",
+
-			Port:             2222,
+// TestNodeServiceProxy returns a *NodeService representing a valid
-			ProxyDestination: "web",
+// Connect proxy.
-		},
+func TestNodeServiceProxy(t testing.T) *NodeService {
+	return &NodeService{
+		Kind:             ServiceKindConnectProxy,
+		Service:          ConnectProxyServiceName,
+		Address:          "127.0.0.2",
+		Port:             2222,
+		ProxyDestination: "web",
 	}
 }