Add SidecarService Syntax sugar to Service Definition (#4686)

* Added new Config for SidecarService in ServiceDefinitions.

* WIP: all the code needed for SidecarService is written... none of it is tested other than config :). Need API updates too.

* Test coverage for the new sidecarServiceFromNodeService method.

* Test API registratrion with SidecarService

* Recursive Key Translation 🤦

* Add tests for nested sidecar defintion arrays to ensure they are translated correctly

* Use dedicated internal state rather than Service Meta for tracking sidecars for deregistration.

Add tests for deregistration.

* API struct for agent register. No other endpoint should be affected yet.

* Additional test cases to cover updates to API registrations

agent/acl.go

@@ -277,6 +277,14 @@ func (a *Agent) vetServiceRegister(token string, service *structs.NodeService) e
 		}
 	}
 
+	// If the service is a proxy, ensure that it has write on the destination too
+	// since it can be discovered as an instance of that service.
+	if service.Kind == structs.ServiceKindConnectProxy {
+		if !rule.ServiceWrite(service.Proxy.DestinationServiceName, nil) {
+			return acl.ErrPermissionDenied
+		}
+	}
+
 	return nil
 }
 

agent/agent.go

@@ -1795,6 +1795,7 @@ func (a *Agent) AddService(service *structs.NodeService, chkTypes []*structs.Che
 			return err
 		}
 	}
+
 	return nil
 }
 
@@ -1839,6 +1840,20 @@ func (a *Agent) RemoveService(serviceID string, persist bool) error {
 	}
 
 	a.logger.Printf("[DEBUG] agent: removed service %q", serviceID)
+
+	// If any Sidecar services exist for the removed service ID, remove them too.
+	if sidecar := a.State.Service(a.sidecarServiceID(serviceID)); sidecar != nil {
+		// Double check that it's not just an ID collision and we actually added
+		// this from a sidecar.
+		if sidecar.LocallyRegisteredAsSidecar {
+			// Remove it!
+			err := a.RemoveService(a.sidecarServiceID(serviceID), persist)
+			if err != nil {
+				return err
+			}
+		}
+	}
+
 	return nil
 }
 
@@ -2718,9 +2733,27 @@ func (a *Agent) loadServices(conf *config.RuntimeConfig) error {
 		if err != nil {
 			return fmt.Errorf("Failed to validate checks for service %q: %v", service.Name, err)
 		}
+
+		// Grab and validate sidecar if there is one too
+		sidecar, sidecarChecks, sidecarToken, err := a.sidecarServiceFromNodeService(ns, service.Token)
+		if err != nil {
+			return fmt.Errorf("Failed to validate sidecar for service %q: %v", service.Name, err)
+		}
+
+		// Remove sidecar from NodeService now it's done it's job it's just a config
+		// syntax sugar and shouldn't be persisted in local or server state.
+		ns.Connect.SidecarService = nil
+
 		if err := a.AddService(ns, chkTypes, false, service.Token); err != nil {
 			return fmt.Errorf("Failed to register service %q: %v", service.Name, err)
 		}
+
+		// If there is a sidecar service, register that too.
+		if sidecar != nil {
+			if err := a.AddService(sidecar, sidecarChecks, false, sidecarToken); err != nil {
+				return fmt.Errorf("Failed to register sidecar for service %q: %v", service.Name, err)
+			}
+		}
 	}
 
 	// Load any persisted services

agent/agent_endpoint.go

@@ -559,30 +559,40 @@ func (s *HTTPServer) AgentRegisterService(resp http.ResponseWriter, req *http.Re
 		// and why we should get rid of it.
 		config.TranslateKeys(rawMap, map[string]string{
 			"enable_tag_override": "EnableTagOverride",
-		})
+			// Managed Proxy Config
+			"exec_mode": "ExecMode",
+			// Proxy Upstreams
+			"destination_name":      "DestinationName",
+			"destination_type":      "DestinationType",
+			"destination_namespace": "DestinationNamespace",
+			"local_bind_port":       "LocalBindPort",
+			"local_bind_address":    "LocalBindAddress",
+			// Proxy Config
+			"destination_service_name": "DestinationServiceName",
+			"destination_service_id":   "DestinationServiceID",
+			"local_service_port":       "LocalServicePort",
+			"local_service_address":    "LocalServiceAddress",
+			// SidecarService
+			"sidecar_service": "SidecarService",
 
-		// Translate upstream keys - we have the same upstream format in two
-		// possible places.
-		translateUpstreams := func(rawMap map[string]interface{}) {
-			var upstreams []interface{}
-			if us, ok := rawMap["upstreams"].([]interface{}); ok {
-				upstreams = us
-			}
-			if us, ok := rawMap["Upstreams"].([]interface{}); ok {
-				upstreams = us
-			}
-			for _, u := range upstreams {
-				if uMap, ok := u.(map[string]interface{}); ok {
-					config.TranslateKeys(uMap, map[string]string{
-						"destination_name":      "DestinationName",
-						"destination_type":      "DestinationType",
-						"destination_namespace": "DestinationNamespace",
-						"local_bind_port":       "LocalBindPort",
-						"local_bind_address":    "LocalBindAddress",
-					})
-				}
-			}
-		}
+			// DON'T Recurse into these opaque config maps or we might mangle user's
+			// keys. Note empty canonical is a special sentinel to prevent recursion.
+			"Meta": "",
+			// upstreams is an array but this prevents recursion into config field of
+			// any item in the array.
+			"Proxy.Config":                   "",
+			"Proxy.Upstreams.Config":         "",
+			"Connect.Proxy.Config":           "",
+			"Connect.Proxy.Upstreams.Config": "",
+
+			// Same exceptions as above, but for a nested sidecar_service note we use
+			// the canonical form SidecarService since that is translated by the time
+			// the lookup here happens. Note that sidecar service doesn't support
+			// managed proxies (connect.proxy).
+			"Connect.SidecarService.Meta":                   "",
+			"Connect.SidecarService.Proxy.Config":           "",
+			"Connect.SidecarService.Proxy.Upstreams.config": "",
+		})
 
 		for k, v := range rawMap {
 			switch strings.ToLower(k) {
@@ -600,32 +610,6 @@ func (s *HTTPServer) AgentRegisterService(resp http.ResponseWriter, req *http.Re
 						return err
 					}
 				}
-			case "proxy":
-				if valMap, ok := v.(map[string]interface{}); ok {
-					config.TranslateKeys(valMap, map[string]string{
-						"destination_service_name": "DestinationServiceName",
-						"destination_service_id":   "DestinationServiceID",
-						"local_service_port":       "LocalServicePort",
-						"local_service_address":    "LocalServiceAddress",
-					})
-					translateUpstreams(valMap)
-				}
-			case "connect":
-				if connectMap, ok := v.(map[string]interface{}); ok {
-					var proxyMap map[string]interface{}
-					if pMap, ok := connectMap["Proxy"].(map[string]interface{}); ok {
-						proxyMap = pMap
-					}
-					if pMap, ok := connectMap["proxy"].(map[string]interface{}); ok {
-						proxyMap = pMap
-					}
-					if proxyMap != nil {
-						config.TranslateKeys(proxyMap, map[string]string{
-							"exec_mode": "ExecMode",
-						})
-						translateUpstreams(proxyMap)
-					}
-				}
 			}
 		}
 		return nil
@@ -689,6 +673,23 @@ func (s *HTTPServer) AgentRegisterService(resp http.ResponseWriter, req *http.Re
 		}
 	}
 
+	// Verify the sidecar check types
+	if args.Connect != nil && args.Connect.SidecarService != nil {
+		chkTypes, err := args.Connect.SidecarService.CheckTypes()
+		if err != nil {
+			return nil, &BadRequestError{
+				Reason: fmt.Sprintf("Invalid check in sidecar_service: %v", err),
+			}
+		}
+		for _, check := range chkTypes {
+			if check.Status != "" && !structs.ValidStatus(check.Status) {
+				return nil, &BadRequestError{
+					Reason: "Status for checks must 'passing', 'warning', 'critical'",
+				}
+			}
+		}
+	}
+
 	// Get the provided token, if any, and vet against any ACL policies.
 	var token string
 	s.parseToken(req, &token)
@@ -696,6 +697,26 @@ func (s *HTTPServer) AgentRegisterService(resp http.ResponseWriter, req *http.Re
 		return nil, err
 	}
 
+	// See if we have a sidecar to register too
+	sidecar, sidecarChecks, sidecarToken, err := s.agent.sidecarServiceFromNodeService(ns, token)
+	if err != nil {
+		return nil, &BadRequestError{
+			Reason: fmt.Sprintf("Invalid SidecarService: %s", err)}
+	}
+	if sidecar != nil {
+		// Make sure we are allowed to register the side car using the token
+		// specified (might be specific to sidecar or the same one as the overall
+		// request).
+		if err := s.agent.vetServiceRegister(sidecarToken, sidecar); err != nil {
+			return nil, err
+		}
+		// We parsed the sidecar registration, now remove it from the NodeService
+		// for the actual service since it's done it's job and we don't want to
+		// persist it in the actual state/catalog. SidecarService is meant to be a
+		// registration syntax sugar so don't propagate it any further.
+		ns.Connect.SidecarService = nil
+	}
+
 	// Get any proxy registrations
 	proxy, err := args.ConnectManagedProxy()
 	if err != nil {
@@ -720,6 +741,12 @@ func (s *HTTPServer) AgentRegisterService(resp http.ResponseWriter, req *http.Re
 			return nil, err
 		}
 	}
+	// Add sidecar.
+	if sidecar != nil {
+		if err := s.agent.AddService(sidecar, sidecarChecks, true, sidecarToken); err != nil {
+			return nil, err
+		}
+	}
 	s.syncChanges()
 	return nil, nil
 }

agent/agent_endpoint_test.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"crypto/tls"
 	"crypto/x509"
+	"encoding/json"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -19,6 +20,7 @@ import (
 	"github.com/hashicorp/consul/agent/checks"
 	"github.com/hashicorp/consul/agent/config"
 	"github.com/hashicorp/consul/agent/connect"
+	"github.com/hashicorp/consul/agent/local"
 	"github.com/hashicorp/consul/agent/structs"
 	"github.com/hashicorp/consul/api"
 	"github.com/hashicorp/consul/lib"
@@ -133,6 +135,57 @@ func TestAgent_Services_ExternalConnectProxy(t *testing.T) {
 	assert.Equal(srv1.Proxy.DestinationServiceName, actual.ProxyDestination)
 }
 
+// Thie tests that a sidecar-registered service is returned as expected.
+func TestAgent_Services_Sidecar(t *testing.T) {
+	t.Parallel()
+
+	require := require.New(t)
+	assert := assert.New(t)
+	a := NewTestAgent(t.Name(), "")
+	defer a.Shutdown()
+
+	testrpc.WaitForLeader(t, a.RPC, "dc1")
+	srv1 := &structs.NodeService{
+		Kind:    structs.ServiceKindConnectProxy,
+		ID:      "db-sidecar-proxy",
+		Service: "db-sidecar-proxy",
+		Port:    5000,
+		// Set this internal state that we expect sidecar registrations to have.
+		LocallyRegisteredAsSidecar: true,
+		Proxy: structs.ConnectProxyConfig{
+			DestinationServiceName: "db",
+			Upstreams:              structs.TestUpstreams(t),
+		},
+	}
+	a.State.AddService(srv1, "")
+
+	req, _ := http.NewRequest("GET", "/v1/agent/services", nil)
+	obj, err := a.srv.AgentServices(nil, req)
+	require.NoError(err)
+	val := obj.(map[string]*api.AgentService)
+	assert.Len(val, 1)
+	actual := val["db-sidecar-proxy"]
+	require.NotNil(actual)
+	assert.Equal(api.ServiceKindConnectProxy, actual.Kind)
+	assert.Equal(srv1.Proxy.ToAPI(), actual.Proxy)
+
+	// DEPRECATED (ProxyDestination) - remove the next comment and assertion
+	// Should still have deprecated ProxyDestination filled in until we remove it
+	// completely at a major version bump.
+	assert.Equal(srv1.Proxy.DestinationServiceName, actual.ProxyDestination)
+
+	// Sanity check that LocalRegisteredAsSidecar is not in the output (assuming
+	// JSON encoding). Right now this is not the case becuase the services
+	// endpoint happens to use the api struct which doesn't include that field,
+	// but this test serves as a regression test incase we change the endpoint to
+	// return the internal struct later and accidentally expose some "internal"
+	// state.
+	output, err := json.Marshal(obj)
+	require.NoError(err)
+	assert.NotContains(string(output), "LocallyRegisteredAsSidecar")
+	assert.NotContains(string(output), "locally_registered_as_sidecar")
+}
+
 func TestAgent_Services_ACLFilter(t *testing.T) {
 	t.Parallel()
 	a := NewTestAgent(t.Name(), TestACLConfig())
@@ -1401,36 +1454,80 @@ func TestAgent_RegisterService_TranslateKeys(t *testing.T) {
 		"port":8000, 
 		"enable_tag_override": true, 
 		"meta": {
-			"some": "meta"
+			"some": "meta",
+			"enable_tag_override": "meta is 'opaque' so should not get translated"
 		},
-		"kind": "connect-proxy",
-		"proxy": {
+		"kind": "connect-proxy",` +
+		// Note the uppercase P is important here - it ensures translation works
+		// correctly in case-insensitive way. Without it this test can pass even
+		// when translation is broken for other valid inputs.
+		`"Proxy": {
 			"destination_service_name": "web",
 			"destination_service_id": "web",
 			"local_service_port": 1234,
 			"local_service_address": "127.0.0.1",
+			"config": {
+				"destination_type": "proxy.config is 'opaque' so should not get translated"
+			},
 			"upstreams": [
 				{
 					"destination_type": "service",
 					"destination_namespace": "default",
 					"destination_name": "db",
 		      "local_bind_address": "127.0.0.1",
-		      "local_bind_port": 1234
+		      "local_bind_port": 1234,
+					"config": {
+						"destination_type": "proxy.upstreams.config is 'opaque' so should not get translated"
+					}
 				}
 			]
 		},
 		"connect": {
 			"proxy": {
 				"exec_mode": "script",
+				"config": {
+					"destination_type": "connect.proxy.config is 'opaque' so should not get translated"
+				},
 				"upstreams": [
 					{
 						"destination_type": "service",
 						"destination_namespace": "default",
 						"destination_name": "db",
 						"local_bind_address": "127.0.0.1",
-						"local_bind_port": 1234
+						"local_bind_port": 1234,
+						"config": {
+							"destination_type": "connect.proxy.upstreams.config is 'opaque' so should not get translated"
+						}
 					}
 				]
+			},
+			"sidecar_service": {
+				"name":"test-proxy", 
+				"port":8001, 
+				"enable_tag_override": true, 
+				"meta": {
+					"some": "meta",
+					"enable_tag_override": "sidecar_service.meta is 'opaque' so should not get translated"
+				},
+				"kind": "connect-proxy",
+				"proxy": {
+					"destination_service_name": "test",
+					"destination_service_id": "test",
+					"local_service_port": 4321,
+					"local_service_address": "127.0.0.1",
+					"upstreams": [
+						{
+							"destination_type": "service",
+							"destination_namespace": "default",
+							"destination_name": "db",
+							"local_bind_address": "127.0.0.1",
+							"local_bind_port": 1234,
+							"config": {
+								"destination_type": "sidecar_service.proxy.upstreams.config is 'opaque' so should not get translated"
+							}
+						}
+					]
+				}
 			}
 		},
 		"weights":{
@@ -1446,9 +1543,12 @@ func TestAgent_RegisterService_TranslateKeys(t *testing.T) {
 	require.Equal(t, 200, rr.Code, "body: %s", rr.Body)
 
 	svc := &structs.NodeService{
-		ID:                "test",
-		Service:           "test",
-		Meta:              map[string]string{"some": "meta"},
+		ID:      "test",
+		Service: "test",
+		Meta: map[string]string{
+			"some":                "meta",
+			"enable_tag_override": "meta is 'opaque' so should not get translated",
+		},
 		Port:              8000,
 		EnableTagOverride: true,
 		Weights:           &structs.Weights{Passing: 16, Warning: 0},
@@ -1458,6 +1558,9 @@ func TestAgent_RegisterService_TranslateKeys(t *testing.T) {
 			DestinationServiceID:   "web",
 			LocalServiceAddress:    "127.0.0.1",
 			LocalServicePort:       1234,
+			Config: map[string]interface{}{
+				"destination_type": "proxy.config is 'opaque' so should not get translated",
+			},
 			Upstreams: structs.Upstreams{
 				{
 					DestinationType:      structs.UpstreamDestTypeService,
@@ -1465,12 +1568,18 @@ func TestAgent_RegisterService_TranslateKeys(t *testing.T) {
 					DestinationNamespace: "default",
 					LocalBindAddress:     "127.0.0.1",
 					LocalBindPort:        1234,
+					Config: map[string]interface{}{
+						"destination_type": "proxy.upstreams.config is 'opaque' so should not get translated",
+					},
 				},
 			},
 		},
 		Connect: structs.ServiceConnect{
 			Proxy: &structs.ServiceDefinitionConnectProxy{
 				ExecMode: "script",
+				Config: map[string]interface{}{
+					"destination_type": "connect.proxy.config is 'opaque' so should not get translated",
+				},
 				Upstreams: structs.Upstreams{
 					{
 						DestinationType:      structs.UpstreamDestTypeService,
@@ -1478,6 +1587,36 @@ func TestAgent_RegisterService_TranslateKeys(t *testing.T) {
 						DestinationNamespace: "default",
 						LocalBindAddress:     "127.0.0.1",
 						LocalBindPort:        1234,
+						Config: map[string]interface{}{
+							"destination_type": "connect.proxy.upstreams.config is 'opaque' so should not get translated",
+						},
+					},
+				},
+			},
+			SidecarService: &structs.ServiceDefinition{
+				Name: "test-proxy",
+				Meta: map[string]string{
+					"some":                "meta",
+					"enable_tag_override": "sidecar_service.meta is 'opaque' so should not get translated",
+				}, Port: 8001,
+				EnableTagOverride: true,
+				Kind:              structs.ServiceKindConnectProxy,
+				Proxy: &structs.ConnectProxyConfig{
+					DestinationServiceName: "test",
+					DestinationServiceID:   "test",
+					LocalServiceAddress:    "127.0.0.1",
+					LocalServicePort:       4321,
+					Upstreams: structs.Upstreams{
+						{
+							DestinationType:      structs.UpstreamDestTypeService,
+							DestinationName:      "db",
+							DestinationNamespace: "default",
+							LocalBindAddress:     "127.0.0.1",
+							LocalBindPort:        1234,
+							Config: map[string]interface{}{
+								"destination_type": "sidecar_service.proxy.upstreams.config is 'opaque' so should not get translated",
+							},
+						},
 					},
 				},
 			},
@@ -1815,6 +1954,504 @@ func TestAgent_RegisterService_UnmanagedConnectProxy(t *testing.T) {
 	assert.Equal("abc123", a.State.ServiceToken("connect-proxy"))
 }
 
+func testDefaultSidecar(svc string, port int, fns ...func(*structs.NodeService)) *structs.NodeService {
+	ns := &structs.NodeService{
+		ID:      svc + "-sidecar-proxy",
+		Kind:    structs.ServiceKindConnectProxy,
+		Service: svc + "-sidecar-proxy",
+		Port:    2222,
+		// Note that LocallyRegisteredAsSidecar should be true on the internal
+		// NodeService, but that we never want to see it in the HTTP response as
+		// it's internal only state. This is being compared directly to local state
+		// so should be present here.
+		LocallyRegisteredAsSidecar: true,
+		Proxy: structs.ConnectProxyConfig{
+			DestinationServiceName: svc,
+			DestinationServiceID:   svc,
+			LocalServiceAddress:    "127.0.0.1",
+			LocalServicePort:       port,
+		},
+	}
+	for _, fn := range fns {
+		fn(ns)
+	}
+	return ns
+}
+
+// This tests local agent service registration with a sidecar service. Note we
+// only test simple defaults for the sidecar here since the actual logic for
+// handling sidecar defaults and port assignment is tested thoroughly in
+// TestAgent_sidecarServiceFromNodeService. Note it also tests Deregister
+// explicitly too since setup is identical.
+func TestAgent_RegisterServiceDeregisterService_Sidecar(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name                      string
+		preRegister, preRegister2 *structs.NodeService
+		// Use raw JSON payloads rather than encoding to avoid subtleties with some
+		// internal representations and different ways they encode and decode. We
+		// rely on the payload being Unmarshalable to structs.ServiceDefinition
+		// directly.
+		json                        string
+		enableACL                   bool
+		tokenRules                  string
+		wantNS                      *structs.NodeService
+		wantErr                     string
+		wantSidecarIDLeftAfterDereg bool
+		assertStateFn               func(t *testing.T, state *local.State)
+	}{
+		{
+			name: "sanity check no sidecar case",
+			json: `
+			{
+				"name": "web",
+				"port": 1111
+			}
+			`,
+			wantNS:  nil,
+			wantErr: "",
+		},
+		{
+			name: "default sidecar",
+			json: `
+			{
+				"name": "web",
+				"port": 1111,
+				"connect": {
+					"SidecarService": {}
+				}
+			}
+			`,
+			wantNS:  testDefaultSidecar("web", 1111),
+			wantErr: "",
+		},
+		{
+			name: "ACL OK defaults",
+			json: `
+			{
+				"name": "web",
+				"port": 1111,
+				"connect": {
+					"SidecarService": {}
+				}
+			}
+			`,
+			enableACL: true,
+			tokenRules: `
+			service "web" {
+				policy = "write"
+			}`,
+			wantNS:  testDefaultSidecar("web", 1111),
+			wantErr: "",
+		},
+		{
+			name: "ACL denied",
+			json: `
+			{
+				"name": "web",
+				"port": 1111,
+				"connect": {
+					"SidecarService": {}
+				}
+			}
+			`,
+			enableACL:  true,
+			tokenRules: ``, // No token rules means no valid token
+			wantNS:     nil,
+			wantErr:    "Permission denied",
+		},
+		{
+			name: "ACL OK for service but not for sidecar",
+			json: `
+			{
+				"name": "web",
+				"port": 1111,
+				"connect": {
+					"SidecarService": {}
+				}
+			}
+			`,
+			enableACL: true,
+			// This will become more common/reasonable when ACLs support exact match.
+			tokenRules: `
+			service "web-sidecar-proxy" {
+				policy = "deny"
+			}
+			service "web" {
+				policy = "write"
+			}`,
+			wantNS:  nil,
+			wantErr: "Permission denied",
+		},
+		{
+			name: "ACL OK for service and sidecar but not sidecar's overriden destination",
+			json: `
+			{
+				"name": "web",
+				"port": 1111,
+				"connect": {
+					"SidecarService": {
+						"proxy": {
+							"DestinationServiceName": "foo"
+						}
+					}
+				}
+			}
+			`,
+			enableACL: true,
+			tokenRules: `
+			service "web" {
+				policy = "write"
+			}`,
+			wantNS:  nil,
+			wantErr: "Permission denied",
+		},
+		{
+			name: "ACL OK for service but not for overridden sidecar",
+			json: `
+			{
+				"name": "web",
+				"port": 1111,
+				"connect": {
+					"SidecarService": {
+						"name": "foo-sidecar-proxy"
+					}
+				}
+			}
+			`,
+			enableACL: true,
+			tokenRules: `
+			service "web" {
+				policy = "write"
+			}`,
+			wantNS:  nil,
+			wantErr: "Permission denied",
+		},
+		{
+			name: "ACL OK for service but and overridden for sidecar",
+			// This test ensures that if the sidecar embeds it's own token with
+			// differnt privs from the main request token it will be honoured for the
+			// sidecar registration. We use the test root token since that should have
+			// permission.
+			json: `
+			{
+				"name": "web",
+				"port": 1111,
+				"connect": {
+					"SidecarService": {
+						"name": "foo",
+						"token": "root"
+					}
+				}
+			}
+			`,
+			enableACL: true,
+			tokenRules: `
+			service "web" {
+				policy = "write"
+			}`,
+			wantNS: testDefaultSidecar("web", 1111, func(ns *structs.NodeService) {
+				ns.Service = "foo"
+			}),
+			wantErr: "",
+		},
+		{
+			name: "invalid check definition in sidecar",
+			// Note no interval in the TCP check should fail validation
+			json: `
+			{
+				"name": "web",
+				"port": 1111,
+				"connect": {
+					"SidecarService": {
+						"check": {
+							"TCP": "foo"
+						}
+					}
+				}
+			}
+			`,
+			wantNS:  nil,
+			wantErr: "invalid check in sidecar_service",
+		},
+		{
+			name: "invalid checks definitions in sidecar",
+			// Note no interval in the TCP check should fail validation
+			json: `
+			{
+				"name": "web",
+				"port": 1111,
+				"connect": {
+					"SidecarService": {
+						"checks": [{
+							"TCP": "foo"
+						}]
+					}
+				}
+			}
+			`,
+			wantNS:  nil,
+			wantErr: "invalid check in sidecar_service",
+		},
+		{
+			name: "invalid check status in sidecar",
+			// Note no interval in the TCP check should fail validation
+			json: `
+			{
+				"name": "web",
+				"port": 1111,
+				"connect": {
+					"SidecarService": {
+						"check": {
+							"TCP": "foo",
+							"Interval": 10,
+							"Status": "unsupported-status"
+						}
+					}
+				}
+			}
+			`,
+			wantNS:  nil,
+			wantErr: "Status for checks must 'passing', 'warning', 'critical'",
+		},
+		{
+			name: "invalid checkS status in sidecar",
+			// Note no interval in the TCP check should fail validation
+			json: `
+			{
+				"name": "web",
+				"port": 1111,
+				"connect": {
+					"SidecarService": {
+						"checks": [{
+							"TCP": "foo",
+							"Interval": 10,
+							"Status": "unsupported-status"
+						}]
+					}
+				}
+			}
+			`,
+			wantNS:  nil,
+			wantErr: "Status for checks must 'passing', 'warning', 'critical'",
+		},
+		{
+			name: "another service registered with same ID as a sidecar should not be deregistered",
+			// Add another service with the same ID that a sidecar for web would have
+			preRegister: &structs.NodeService{
+				ID:      "web-sidecar-proxy",
+				Service: "fake-sidecar",
+				Port:    9999,
+			},
+			// Register web with NO SIDECAR
+			json: `
+			{
+				"name": "web",
+				"port": 1111
+			}
+			`,
+			// Note here that although the registration here didn't register it, we
+			// should still see the NodeService we pre-registered here.
+			wantNS: &structs.NodeService{
+				ID:      "web-sidecar-proxy",
+				Service: "fake-sidecar",
+				Port:    9999,
+			},
+			// After we deregister the web service above, the fake sidecar with
+			// clashing ID SHOULD NOT have been removed since it wasn't part of the
+			// original registration.
+			wantSidecarIDLeftAfterDereg: true,
+		},
+		{
+			name: "updates to sidecar should work",
+			// Add a valid sidecar already registered
+			preRegister: &structs.NodeService{
+				ID:                         "web-sidecar-proxy",
+				Service:                    "web-sidecar-proxy",
+				LocallyRegisteredAsSidecar: true,
+				Port:                       9999,
+			},
+			// Register web with Sidecar on different port
+			json: `
+			{
+				"name": "web",
+				"port": 1111,
+				"connect": {
+					"SidecarService": {
+						"Port": 6666
+					}
+				}
+			}
+			`,
+			// Note here that although the registration here didn't register it, we
+			// should still see the NodeService we pre-registered here.
+			wantNS: &structs.NodeService{
+				Kind:                       "connect-proxy",
+				ID:                         "web-sidecar-proxy",
+				Service:                    "web-sidecar-proxy",
+				LocallyRegisteredAsSidecar: true,
+				Port:                       6666,
+				Proxy: structs.ConnectProxyConfig{
+					DestinationServiceName: "web",
+					DestinationServiceID:   "web",
+					LocalServiceAddress:    "127.0.0.1",
+					LocalServicePort:       1111,
+				},
+			},
+		},
+		{
+			name: "update that removes sidecar should NOT deregister it",
+			// Add web with a valid sidecar already registered
+			preRegister: &structs.NodeService{
+				ID:      "web",
+				Service: "web",
+				Port:    1111,
+			},
+			preRegister2: testDefaultSidecar("web", 1111),
+			// Register (update) web and remove sidecar (and port for sanity check)
+			json: `
+			{
+				"name": "web",
+				"port": 2222
+			}
+			`,
+			// Sidecar should still be there such that API can update registration
+			// without accidentally removing a sidecar. This is equivalent to embedded
+			// checks which are not removed by just not being included in an update.
+			// We will document that sidecar registrations via API must be explicitiy
+			// deregistered.
+			wantNS: testDefaultSidecar("web", 1111),
+			// Sanity check the rest of the update happened though.
+			assertStateFn: func(t *testing.T, state *local.State) {
+				svcs := state.Services()
+				svc, ok := svcs["web"]
+				require.True(t, ok)
+				require.Equal(t, 2222, svc.Port)
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			assert := assert.New(t)
+			require := require.New(t)
+
+			// Constrain auto ports to 1 available to make it deterministic
+			hcl := `ports { 
+				sidecar_min_port = 2222
+				sidecar_max_port = 2222
+			}
+			`
+			if tt.enableACL {
+				hcl = hcl + TestACLConfig()
+			}
+
+			a := NewTestAgent(t.Name(), hcl)
+			defer a.Shutdown()
+			testrpc.WaitForLeader(t, a.RPC, "dc1")
+
+			if tt.preRegister != nil {
+				require.NoError(a.AddService(tt.preRegister, nil, false, ""))
+			}
+			if tt.preRegister2 != nil {
+				require.NoError(a.AddService(tt.preRegister2, nil, false, ""))
+			}
+
+			// Create an ACL token with require policy
+			var token string
+			if tt.enableACL && tt.tokenRules != "" {
+				args := map[string]interface{}{
+					"Name":  "User Token",
+					"Type":  "client",
+					"Rules": tt.tokenRules,
+				}
+				req, _ := http.NewRequest("PUT", "/v1/acl/create?token=root", jsonReader(args))
+				resp := httptest.NewRecorder()
+				obj, err := a.srv.ACLCreate(resp, req)
+				require.NoError(err)
+				require.NotNil(obj)
+				aclResp := obj.(aclCreateResponse)
+				token = aclResp.ID
+			}
+
+			br := bytes.NewBufferString(tt.json)
+
+			req, _ := http.NewRequest("PUT", "/v1/agent/service/register?token="+token, br)
+			resp := httptest.NewRecorder()
+			obj, err := a.srv.AgentRegisterService(resp, req)
+			if tt.wantErr != "" {
+				require.Error(err, "response code=%d, body:\n%s",
+					resp.Code, resp.Body.String())
+				require.Contains(strings.ToLower(err.Error()), strings.ToLower(tt.wantErr))
+				return
+			}
+			require.NoError(err)
+			assert.Nil(obj)
+			require.Equal(200, resp.Code, "request failed with body: %s",
+				resp.Body.String())
+
+			// Sanity the target service registration
+			svcs := a.State.Services()
+
+			// Parse the expected definition into a ServiceDefinition
+			var sd structs.ServiceDefinition
+			err = json.Unmarshal([]byte(tt.json), &sd)
+			require.NoError(err)
+			require.NotEmpty(sd.Name)
+
+			svcID := sd.ID
+			if svcID == "" {
+				svcID = sd.Name
+			}
+			svc, ok := svcs[svcID]
+			require.True(ok, "has service "+svcID)
+			assert.Equal(sd.Name, svc.Service)
+			assert.Equal(sd.Port, svc.Port)
+			// Ensure that the actual registered service _doesn't_ still have it's
+			// sidecar info since it's duplicate and we don't want that synced up to
+			// the catalog or included in responses particulary - it's just
+			// registration syntax sugar.
+			assert.Nil(svc.Connect.SidecarService)
+
+			if tt.wantNS == nil {
+				// Sanity check that there was no service registered, we rely on there
+				// being no services at start of test so we can just use the count.
+				assert.Len(svcs, 1, "should be no sidecar registered")
+				return
+			}
+
+			// Ensure sidecar
+			svc, ok = svcs[tt.wantNS.ID]
+			require.True(ok, "no sidecar registered at "+tt.wantNS.ID)
+			assert.Equal(tt.wantNS, svc)
+
+			if tt.assertStateFn != nil {
+				tt.assertStateFn(t, a.State)
+			}
+
+			// Now verify deregistration also removes sidecar (if there was one and it
+			// was added via sidecar not just coincidental ID clash)
+			{
+				req := httptest.NewRequest("PUT",
+					"/v1/agent/service/deregister/"+svcID+"?token="+token, nil)
+				resp := httptest.NewRecorder()
+				obj, err := a.srv.AgentDeregisterService(resp, req)
+				require.NoError(err)
+				require.Nil(obj)
+
+				svcs := a.State.Services()
+				svc, ok = svcs[tt.wantNS.ID]
+				if tt.wantSidecarIDLeftAfterDereg {
+					require.True(ok, "removed non-sidecar service at "+tt.wantNS.ID)
+				} else {
+					require.False(ok, "sidecar not deregistered with service "+svcID)
+				}
+			}
+		})
+	}
+}
+
 // This tests that connect proxy validation is done for local agent
 // registration. This doesn't need to test validation exhaustively since
 // that is done via a table test in the structs package.

agent/agent_test.go

@@ -1987,6 +1987,72 @@ func TestAgent_loadServices_token(t *testing.T) {
 	}
 }
 
+func TestAgent_loadServices_sidecar(t *testing.T) {
+	t.Parallel()
+	a := NewTestAgent(t.Name(), `
+		service = {
+			id = "rabbitmq"
+			name = "rabbitmq"
+			port = 5672
+			token = "abc123"
+			connect = {
+				sidecar_service {}
+			}
+		}
+	`)
+	defer a.Shutdown()
+
+	services := a.State.Services()
+	if _, ok := services["rabbitmq"]; !ok {
+		t.Fatalf("missing service")
+	}
+	if token := a.State.ServiceToken("rabbitmq"); token != "abc123" {
+		t.Fatalf("bad: %s", token)
+	}
+	if _, ok := services["rabbitmq-sidecar-proxy"]; !ok {
+		t.Fatalf("missing service")
+	}
+	if token := a.State.ServiceToken("rabbitmq-sidecar-proxy"); token != "abc123" {
+		t.Fatalf("bad: %s", token)
+	}
+
+	// Sanity check rabbitmq service should NOT have sidecar info in state since
+	// it's done it's job and should be a registration syntax sugar only.
+	assert.Nil(t, services["rabbitmq"].Connect.SidecarService)
+}
+
+func TestAgent_loadServices_sidecarSeparateToken(t *testing.T) {
+	t.Parallel()
+	a := NewTestAgent(t.Name(), `
+		service = {
+			id = "rabbitmq"
+			name = "rabbitmq"
+			port = 5672
+			token = "abc123"
+			connect = {
+				sidecar_service {
+					token = "789xyz"
+				}
+			}
+		}
+	`)
+	defer a.Shutdown()
+
+	services := a.State.Services()
+	if _, ok := services["rabbitmq"]; !ok {
+		t.Fatalf("missing service")
+	}
+	if token := a.State.ServiceToken("rabbitmq"); token != "abc123" {
+		t.Fatalf("bad: %s", token)
+	}
+	if _, ok := services["rabbitmq-sidecar-proxy"]; !ok {
+		t.Fatalf("missing service")
+	}
+	if token := a.State.ServiceToken("rabbitmq-sidecar-proxy"); token != "789xyz" {
+		t.Fatalf("bad: %s", token)
+	}
+}
+
 func TestAgent_unloadServices(t *testing.T) {
 	t.Parallel()
 	a := NewTestAgent(t.Name(), "")
@@ -2194,8 +2260,8 @@ func TestAgent_Service_Reap(t *testing.T) {
 	}
 	chkTypes := []*structs.CheckType{
 		&structs.CheckType{
-			Status: api.HealthPassing,
-			TTL:    25 * time.Millisecond,
+			Status:                         api.HealthPassing,
+			TTL:                            25 * time.Millisecond,
 			DeregisterCriticalServiceAfter: 200 * time.Millisecond,
 		},
 	}

agent/config/builder.go

@@ -344,10 +344,16 @@ func (b *Builder) Build() (rt RuntimeConfig, err error) {
 	serfPortWAN := b.portVal("ports.serf_wan", c.Ports.SerfWAN)
 	proxyMinPort := b.portVal("ports.proxy_min_port", c.Ports.ProxyMinPort)
 	proxyMaxPort := b.portVal("ports.proxy_max_port", c.Ports.ProxyMaxPort)
+	sidecarMinPort := b.portVal("ports.sidecar_min_port", c.Ports.SidecarMinPort)
+	sidecarMaxPort := b.portVal("ports.sidecar_max_port", c.Ports.SidecarMaxPort)
 	if proxyMaxPort < proxyMinPort {
 		return RuntimeConfig{}, fmt.Errorf(
 			"proxy_min_port must be less than proxy_max_port. To disable, set both to zero.")
 	}
+	if sidecarMaxPort < sidecarMinPort {
+		return RuntimeConfig{}, fmt.Errorf(
+			"sidecar_min_port must be less than sidecar_max_port. To disable, set both to zero.")
+	}
 
 	// determine the default bind and advertise address
 	//
@@ -689,6 +695,8 @@ func (b *Builder) Build() (rt RuntimeConfig, err error) {
 		ConnectProxyAllowManagedAPIRegistration: b.boolVal(c.Connect.Proxy.AllowManagedAPIRegistration),
 		ConnectProxyBindMinPort:                 proxyMinPort,
 		ConnectProxyBindMaxPort:                 proxyMaxPort,
+		ConnectSidecarMinPort:                   sidecarMinPort,
+		ConnectSidecarMaxPort:                   sidecarMaxPort,
 		ConnectProxyDefaultExecMode:             proxyDefaultExecMode,
 		ConnectProxyDefaultDaemonCommand:        proxyDefaultDaemonCommand,
 		ConnectProxyDefaultScriptCommand:        proxyDefaultScriptCommand,
@@ -1184,9 +1192,29 @@ func (b *Builder) serviceConnectVal(v *ServiceConnect) *structs.ServiceConnect {
 		}
 	}
 
+	sidecar := b.serviceVal(v.SidecarService)
+	if sidecar != nil {
+		// Sanity checks
+		if sidecar.ID != "" {
+			b.err = multierror.Append(b.err, fmt.Errorf("sidecar_service can't speficy an ID"))
+			sidecar.ID = ""
+		}
+		if sidecar.Connect != nil {
+			if sidecar.Connect.SidecarService != nil {
+				b.err = multierror.Append(b.err, fmt.Errorf("sidecar_service can't have a nested sidecar_service"))
+				sidecar.Connect.SidecarService = nil
+			}
+			if sidecar.Connect.Proxy != nil {
+				b.err = multierror.Append(b.err, fmt.Errorf("sidecar_service can't have a managed proxy"))
+				sidecar.Connect.Proxy = nil
+			}
+		}
+	}
+
 	return &structs.ServiceConnect{
-		Native: b.boolVal(v.Native),
-		Proxy:  proxy,
+		Native:         b.boolVal(v.Native),
+		Proxy:          proxy,
+		SidecarService: sidecar,
 	}
 }
 

agent/config/config.go

@@ -90,6 +90,13 @@ func Parse(data string, format string) (c Config, err error) {
 		"services.connect.proxy.upstreams",
 		"service.proxy.upstreams",
 		"services.proxy.upstreams",
+
+		// Need all the service(s) exceptions also for nested sidecar service except
+		// managed proxy which is explicitly not supported there.
+		"service.connect.sidecar_service.checks",
+		"services.connect.sidecar_service.checks",
+		"service.connect.sidecar_service.proxy.upstreams",
+		"services.connect.sidecar_service.proxy.upstreams",
 	})
 
 	// There is a difference of representation of some fields depending on
@@ -383,6 +390,15 @@ type ServiceConnect struct {
 
 	// Proxy configures a connect proxy instance for the service
 	Proxy *ServiceConnectProxy `json:"proxy,omitempty" hcl:"proxy" mapstructure:"proxy"`
+
+	// SidecarService is a nested Service Definition to register at the same time.
+	// It's purely a convenience mechanism to allow specifying a sidecar service
+	// along with the application service definition. It's nested nature allows
+	// all of the fields to be defaulted which can reduce the amount of
+	// boilerplate needed to register a sidecar service separately, but the end
+	// result is identical to just making a second service registration via any
+	// other means.
+	SidecarService *ServiceDefinition `json:"sidecar_service,omitempty" hcl:"sidecar_service" mapstructure:"sidecar_service"`
 }
 
 type ServiceConnectProxy struct {
@@ -553,14 +569,16 @@ type Telemetry struct {
 }
 
 type Ports struct {
-	DNS          *int `json:"dns,omitempty" hcl:"dns" mapstructure:"dns"`
-	HTTP         *int `json:"http,omitempty" hcl:"http" mapstructure:"http"`
-	HTTPS        *int `json:"https,omitempty" hcl:"https" mapstructure:"https"`
-	SerfLAN      *int `json:"serf_lan,omitempty" hcl:"serf_lan" mapstructure:"serf_lan"`
-	SerfWAN      *int `json:"serf_wan,omitempty" hcl:"serf_wan" mapstructure:"serf_wan"`
-	Server       *int `json:"server,omitempty" hcl:"server" mapstructure:"server"`
-	ProxyMinPort *int `json:"proxy_min_port,omitempty" hcl:"proxy_min_port" mapstructure:"proxy_min_port"`
-	ProxyMaxPort *int `json:"proxy_max_port,omitempty" hcl:"proxy_max_port" mapstructure:"proxy_max_port"`
+	DNS            *int `json:"dns,omitempty" hcl:"dns" mapstructure:"dns"`
+	HTTP           *int `json:"http,omitempty" hcl:"http" mapstructure:"http"`
+	HTTPS          *int `json:"https,omitempty" hcl:"https" mapstructure:"https"`
+	SerfLAN        *int `json:"serf_lan,omitempty" hcl:"serf_lan" mapstructure:"serf_lan"`
+	SerfWAN        *int `json:"serf_wan,omitempty" hcl:"serf_wan" mapstructure:"serf_wan"`
+	Server         *int `json:"server,omitempty" hcl:"server" mapstructure:"server"`
+	ProxyMinPort   *int `json:"proxy_min_port,omitempty" hcl:"proxy_min_port" mapstructure:"proxy_min_port"`
+	ProxyMaxPort   *int `json:"proxy_max_port,omitempty" hcl:"proxy_max_port" mapstructure:"proxy_max_port"`
+	SidecarMinPort *int `json:"sidecar_min_port,omitempty" hcl:"sidecar_min_port" mapstructure:"sidecar_min_port"`
+	SidecarMaxPort *int `json:"sidecar_max_port,omitempty" hcl:"sidecar_max_port" mapstructure:"sidecar_max_port"`
 }
 
 type UnixSocket struct {

agent/config/default.go

@@ -107,6 +107,8 @@ func DefaultSource() Source {
 			server = ` + strconv.Itoa(consul.DefaultRPCPort) + `
 			proxy_min_port = 20000
 			proxy_max_port = 20255
+			sidecar_min_port = 21000
+			sidecar_max_port = 21255
 		}
 		telemetry = {
 			metrics_prefix = "consul"

agent/config/runtime.go

@@ -462,6 +462,16 @@ type RuntimeConfig struct {
 	// port is specified.
 	ConnectProxyBindMaxPort int
 
+	// ConnectSidecarMinPort is the inclusive start of the range of ports
+	// allocated to the agent for asigning to sidecar services where no port is
+	// specified.
+	ConnectSidecarMinPort int
+
+	// ConnectSidecarMaxPort is the inclusive end of the range of ports
+	// allocated to the agent for asigning to sidecar services where no port is
+	// specified
+	ConnectSidecarMaxPort int
+
 	// ConnectProxyAllowManagedRoot is true if Consul can execute managed
 	// proxies when running as root (EUID == 0).
 	ConnectProxyAllowManagedRoot bool

agent/config/runtime_test.go

@@ -1863,6 +1863,103 @@ func TestConfigFlagsAndEdgecases(t *testing.T) {
 				`},
 			err: "Serf Advertise WAN address 10.0.0.1:1000 already configured for RPC Advertise",
 		},
+		{
+			desc: "sidecar_service can't have ID",
+			args: []string{
+				`-data-dir=` + dataDir,
+			},
+			json: []string{`{
+				  "service": {
+						"name": "web",
+						"port": 1234,
+						"connect": {
+							"sidecar_service": {
+								"ID": "random-sidecar-id"
+							}
+						}
+					}
+				}`},
+			hcl: []string{`
+				service {
+					name = "web"
+					port = 1234
+					connect {
+						sidecar_service {
+							ID = "random-sidecar-id"
+						}
+					}
+				}
+			`},
+			err: "sidecar_service can't speficy an ID",
+		},
+		{
+			desc: "sidecar_service can't have nested sidecar",
+			args: []string{
+				`-data-dir=` + dataDir,
+			},
+			json: []string{`{
+				  "service": {
+						"name": "web",
+						"port": 1234,
+						"connect": {
+							"sidecar_service": {
+								"connect": {
+									"sidecar_service": {}
+								}
+							}
+						}
+					}
+				}`},
+			hcl: []string{`
+				service {
+					name = "web"
+					port = 1234
+					connect {
+						sidecar_service {
+							connect {
+								sidecar_service {
+								}
+							}
+						}
+					}
+				}
+			`},
+			err: "sidecar_service can't have a nested sidecar_service",
+		},
+		{
+			desc: "sidecar_service can't have managed proxy",
+			args: []string{
+				`-data-dir=` + dataDir,
+			},
+			json: []string{`{
+				  "service": {
+						"name": "web",
+						"port": 1234,
+						"connect": {
+							"sidecar_service": {
+								"connect": {
+									"proxy": {}
+								}
+							}
+						}
+					}
+				}`},
+			hcl: []string{`
+				service {
+					name = "web"
+					port = 1234
+					connect {
+						sidecar_service {
+							connect {
+								proxy {
+								}
+							}
+						}
+					}
+				}
+			`},
+			err: "sidecar_service can't have a managed proxy",
+		},
 		{
 			desc: "telemetry.prefix_filter cannot be empty",
 			args: []string{
@@ -2308,6 +2405,181 @@ func TestConfigFlagsAndEdgecases(t *testing.T) {
 				rt.ConnectProxyAllowManagedAPIRegistration = true
 			},
 		},
+
+		{
+			// This tests that we correct added the nested paths to arrays of objects
+			// to the exceptions in patchSliceOfMaps in config.go (for single service)
+			desc: "service.connectsidecar_service with checks and upstreams",
+			args: []string{
+				`-data-dir=` + dataDir,
+			},
+			json: []string{`{
+				  "service": {
+						"name": "web",
+						"port": 1234,
+						"connect": {
+							"sidecar_service": {
+								"port": 2345,
+								"checks": [
+									{
+										"TCP": "127.0.0.1:2345",
+										"Interval": "10s"
+									}
+								],
+								"proxy": {
+									"upstreams": [
+										{
+											"destination_name": "db",
+											"local_bind_port": 7000
+										}
+									]
+								}
+							}
+						}
+					}
+				}`},
+			hcl: []string{`
+				service {
+					name = "web"
+					port = 1234
+					connect {
+						sidecar_service {
+							port = 2345
+							checks = [
+								{
+									tcp = "127.0.0.1:2345"
+									interval = "10s"
+								}
+							]
+							proxy {
+								upstreams = [
+									{
+										destination_name = "db"
+										local_bind_port = 7000
+									},
+								]
+							}
+						}
+					}
+				}
+			`},
+			patch: func(rt *RuntimeConfig) {
+				rt.DataDir = dataDir
+				rt.Services = []*structs.ServiceDefinition{
+					{
+						Name: "web",
+						Port: 1234,
+						Connect: &structs.ServiceConnect{
+							SidecarService: &structs.ServiceDefinition{
+								Port: 2345,
+								Checks: structs.CheckTypes{
+									{
+										TCP:      "127.0.0.1:2345",
+										Interval: 10 * time.Second,
+									},
+								},
+								Proxy: &structs.ConnectProxyConfig{
+									Upstreams: structs.Upstreams{
+										structs.Upstream{
+											DestinationType: "service",
+											DestinationName: "db",
+											LocalBindPort:   7000,
+										},
+									},
+								},
+							},
+						},
+					},
+				}
+			},
+		},
+		{
+			// This tests that we correct added the nested paths to arrays of objects
+			// 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,
+			},
+			json: []string{`{
+				  "services": [{
+						"name": "web",
+						"port": 1234,
+						"connect": {
+							"sidecar_service": {
+								"port": 2345,
+								"checks": [
+									{
+										"TCP": "127.0.0.1:2345",
+										"Interval": "10s"
+									}
+								],
+								"proxy": {
+									"upstreams": [
+										{
+											"destination_name": "db",
+											"local_bind_port": 7000
+										}
+									]
+								}
+							}
+						}
+					}]
+				}`},
+			hcl: []string{`
+				services = [{
+					name = "web"
+					port = 1234
+					connect {
+						sidecar_service {
+							port = 2345
+							checks = [
+								{
+									tcp = "127.0.0.1:2345"
+									interval = "10s"
+								}
+							]
+							proxy {
+								upstreams = [
+									{
+										destination_name = "db"
+										local_bind_port = 7000
+									},
+								]
+							}
+						}
+					}
+				}]
+			`},
+			patch: func(rt *RuntimeConfig) {
+				rt.DataDir = dataDir
+				rt.Services = []*structs.ServiceDefinition{
+					{
+						Name: "web",
+						Port: 1234,
+						Connect: &structs.ServiceConnect{
+							SidecarService: &structs.ServiceDefinition{
+								Port: 2345,
+								Checks: structs.CheckTypes{
+									{
+										TCP:      "127.0.0.1:2345",
+										Interval: 10 * time.Second,
+									},
+								},
+								Proxy: &structs.ConnectProxyConfig{
+									Upstreams: structs.Upstreams{
+										structs.Upstream{
+											DestinationType: "service",
+											DestinationName: "db",
+											LocalBindPort:   7000,
+										},
+									},
+								},
+							},
+						},
+					},
+				}
+			},
+		},
 	}
 
 	testConfig(t, tests, dataDir)
@@ -2700,7 +2972,9 @@ func TestFullConfig(t *testing.T) {
 				"https": 15127,
 				"server": 3757,
 				"proxy_min_port": 2000,
-				"proxy_max_port": 3000
+				"proxy_max_port": 3000,
+				"sidecar_min_port": 8888,
+				"sidecar_max_port": 9999
 			},
 			"protocol": 30793,
 			"raft_protocol": 19016,
@@ -2850,6 +3124,9 @@ func TestFullConfig(t *testing.T) {
 						"timeout": "38333s",
 						"ttl": "57201s",
 						"deregister_critical_service_after": "44214s"
+					},
+					"connect": {
+						"sidecar_service": {}
 					}
 				},
 				{
@@ -3224,6 +3501,8 @@ func TestFullConfig(t *testing.T) {
 				server = 3757
 				proxy_min_port = 2000
 				proxy_max_port = 3000
+				sidecar_min_port = 8888
+				sidecar_max_port = 9999
 			}
 			protocol = 30793
 			raft_protocol = 19016
@@ -3374,6 +3653,9 @@ func TestFullConfig(t *testing.T) {
 						ttl = "57201s"
 						deregister_critical_service_after = "44214s"
 					}
+					connect {
+						sidecar_service {}
+					}
 				},
 				{
 					id = "MRHVMZuD"
@@ -3756,6 +4038,8 @@ func TestFullConfig(t *testing.T) {
 		ConnectEnabled:          true,
 		ConnectProxyBindMinPort: 2000,
 		ConnectProxyBindMaxPort: 3000,
+		ConnectSidecarMinPort:   8888,
+		ConnectSidecarMaxPort:   9999,
 		ConnectCAProvider:       "consul",
 		ConnectCAConfig: map[string]interface{}{
 			"RotationPeriod": "90h",
@@ -3896,6 +4180,13 @@ func TestFullConfig(t *testing.T) {
 						DeregisterCriticalServiceAfter: 44214 * time.Second,
 					},
 				},
+				// Note that although this SidecarService is only syntax sugar for
+				// registering another service, that has to happen in the agent code so
+				// it can make intelligent decisions about automatic port assignments
+				// etc. So we expect config just to pass it through verbatim.
+				Connect: &structs.ServiceConnect{
+					SidecarService: &structs.ServiceDefinition{},
+				},
 			},
 			{
 				ID:      "MRHVMZuD",
@@ -4511,6 +4802,8 @@ func TestSanitize(t *testing.T) {
 		"ConnectProxyDefaultDaemonCommand": [],
 		"ConnectProxyDefaultExecMode": "",
 		"ConnectProxyDefaultScriptCommand": [],
+		"ConnectSidecarMaxPort": 0,
+		"ConnectSidecarMinPort": 0,
 		"ConnectTestDisableManagedProxies": false,
 		"ConsulCoordinateUpdateBatchSize": 0,
 		"ConsulCoordinateUpdateMaxBatches": 0,

agent/config/translate.go

@@ -1,6 +1,8 @@
 package config
 
-import "strings"
+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
@@ -10,21 +12,64 @@ import "strings"
 //
 // Example:
 //
-//   m = TranslateKeys(m, map[string]string{"CamelCase": "snake_case"})
+//   m = TranslateKeys(m, map[string]string{"snake_case": "CamelCase"})
 //
+// If the canonical string provided is the empty string, the effect is to stop
+// recursing into any key matching the left hand side. In this case the left
+// hand side must use periods to specify a full path e.g.
+// `connect.proxy.config`. The path must be the canonical key names (i.e.
+// CamelCase) AFTER translation so ExecMode not exec_mode. These are still match
+// in a case-insensitive way.
+//
+// This is needed for example because parts of the Service Definition are
+// "opaque" maps of metadata or config passed to another process or component.
+// If we allow translation to recurse we might mangle the "opaque" keys given
+// where the clash with key names in other parts of the definition (and they do
+// in practice with deprecated managed proxy upstreams) :sob:
+//
+// Example:
+//   m - TranslateKeys(m, map[string]string{
+//     "foo_bar": "FooBar",
+//     "widget.config": "",
+//     // Assume widgets is an array, this will prevent recursing into any
+//     // item's config field
+//     "widgets.config": "",
+//   })
 func TranslateKeys(v map[string]interface{}, dict map[string]string) {
-	ck(v, dict)
+	// Convert all dict keys for exclusions to lower. so we can match against them
+	// unambiguously with a single lookup.
+	for k, v := range dict {
+		if v == "" {
+			dict[strings.ToLower(k)] = ""
+		}
+	}
+	ck(v, dict, "")
 }
 
-func ck(v interface{}, dict map[string]string) interface{} {
+func ck(v interface{}, dict map[string]string, pathPfx string) interface{} {
+	// In array case we don't add a path segment for the item as they are all
+	// assumed to be same which is why we check the prefix doesn't already end in
+	// a .
+	if pathPfx != "" && !strings.HasSuffix(pathPfx, ".") {
+		pathPfx += "."
+	}
 	switch x := v.(type) {
 	case map[string]interface{}:
 		for k, v := range x {
-			canonKey := dict[strings.ToLower(k)]
+			lowerK := strings.ToLower(k)
+
+			// Check if this path has been excluded
+			val, ok := dict[pathPfx+lowerK]
+			if ok && val == "" {
+				// Don't recurse into this key
+				continue
+			}
+
+			canonKey, ok := dict[lowerK]
 
 			// no canonical key? -> use this key
-			if canonKey == "" {
-				x[k] = ck(v, dict)
+			if !ok {
+				x[k] = ck(v, dict, pathPfx+lowerK)
 				continue
 			}
 
@@ -37,14 +82,14 @@ func ck(v interface{}, dict map[string]string) interface{} {
 			}
 
 			// otherwise translate to the canonical key
-			x[canonKey] = ck(v, dict)
+			x[canonKey] = ck(v, dict, pathPfx+strings.ToLower(canonKey))
 		}
 		return x
 
 	case []interface{}:
 		var a []interface{}
 		for _, xv := range x {
-			a = append(a, ck(xv, dict))
+			a = append(a, ck(xv, dict, pathPfx))
 		}
 		return a
 

agent/sidecar_service.go

@@ -0,0 +1,146 @@
+package agent
+
+import (
+	"fmt"
+	"math/rand"
+	"time"
+
+	"github.com/hashicorp/consul/agent/structs"
+)
+
+func (a *Agent) sidecarServiceID(serviceID string) string {
+	return serviceID + "-sidecar-proxy"
+}
+
+// sidecarServiceFromNodeService returns a *structs.NodeService representing a
+// sidecar service with all defaults populated based on the current agent
+// config.
+//
+// It assumes the ns has been validated already which means the nested
+// SidecarService is also already validated.It also assumes that any check
+// definitions within the sidecar service definition have been validated if
+// necessary. If no sidecar service is defined in ns, then nil is returned with
+// nil error.
+//
+// The second return argument is a list of CheckTypes to register along with the
+// service.
+//
+// The third return argument is the effective Token to use for the sidecar
+// registration. This will be the same as the token parameter passed unless the
+// SidecarService definition contains a distint one.
+func (a *Agent) sidecarServiceFromNodeService(ns *structs.NodeService, token string) (*structs.NodeService, []*structs.CheckType, string, error) {
+	if ns.Connect.SidecarService == nil {
+		return nil, nil, "", nil
+	}
+
+	// Start with normal conversion from service definition
+	sidecar := ns.Connect.SidecarService.NodeService()
+
+	// Override the ID which must always be consistent for a given outer service
+	// ID. We rely on this for lifecycle management of the nested definition.
+	sidecar.ID = a.sidecarServiceID(ns.ID)
+
+	// Set some meta we can use to disambiguate between service instances we added
+	// later and are responsible for deregistering.
+	if sidecar.Meta != nil {
+		// Meta is non-nil validate it before we add the special key so we can
+		// enforce that user cannot add a consul- prefix one.
+		if err := structs.ValidateMetadata(sidecar.Meta, false); err != nil {
+			return nil, nil, "", err
+		}
+	}
+
+	// Flag this as a sidecar - this is not persisted in catalog but only needed
+	// in local agent state to disambiguate lineage when deregistereing the parent
+	// service later.
+	sidecar.LocallyRegisteredAsSidecar = true
+
+	// See if there is a more specific token for the sidecar registration
+	if ns.Connect.SidecarService.Token != "" {
+		token = ns.Connect.SidecarService.Token
+	}
+
+	// Setup some sane connect proxy defaults.
+	if sidecar.Kind == "" {
+		sidecar.Kind = structs.ServiceKindConnectProxy
+	}
+	if sidecar.Service == "" {
+		sidecar.Service = ns.Service + "-sidecar-proxy"
+	}
+	if sidecar.Address == "" {
+		// Inherit address from the service if it's provided
+		sidecar.Address = ns.Address
+	}
+	// Proxy defaults
+	if sidecar.Proxy.DestinationServiceName == "" {
+		sidecar.Proxy.DestinationServiceName = ns.Service
+	}
+	if sidecar.Proxy.DestinationServiceID == "" {
+		sidecar.Proxy.DestinationServiceID = ns.ID
+	}
+	if sidecar.Proxy.LocalServiceAddress == "" {
+		sidecar.Proxy.LocalServiceAddress = "127.0.0.1"
+	}
+	if sidecar.Proxy.LocalServicePort < 1 {
+		sidecar.Proxy.LocalServicePort = ns.Port
+	}
+
+	// Allocate port if needed (min and max inclusive).
+	rangeLen := a.config.ConnectSidecarMaxPort - a.config.ConnectSidecarMinPort + 1
+	if sidecar.Port < 1 && a.config.ConnectSidecarMinPort > 0 && rangeLen > 0 {
+		// This should be a really short list so don't bother optimising lookup yet.
+	OUTER:
+		for _, offset := range rand.Perm(rangeLen) {
+			p := a.config.ConnectSidecarMinPort + offset
+			// See if this port was already allocated to another service
+			for _, otherNS := range a.State.Services() {
+				if otherNS.Port == p {
+					// already taken, skip to next random pick in the range
+					continue OUTER
+				}
+			}
+			// We made it through all existing proxies without a match so claim this one
+			sidecar.Port = p
+			break
+		}
+	}
+	// If no ports left (or auto ports disabled) fail
+	if sidecar.Port < 1 {
+		// If ports are set to zero explicitly, config builder switches them to
+		// `-1`. In this case don't show the actual values since we don't know what
+		// was actually in config (zero or negative) and it might be confusing, we
+		// just know they explicitly disabled auto assignment.
+		if a.config.ConnectSidecarMinPort < 1 || a.config.ConnectSidecarMaxPort < 1 {
+			return nil, nil, "", fmt.Errorf("no port provided for sidecar_service " +
+				"and auto-assignement disabled in config")
+		}
+		return nil, nil, "", fmt.Errorf("no port provided for sidecar_service and none "+
+			"left in the configured range [%d, %d]", a.config.ConnectSidecarMinPort,
+			a.config.ConnectSidecarMaxPort)
+	}
+
+	// Setup checks
+	checks, err := ns.Connect.SidecarService.CheckTypes()
+	if err != nil {
+		return nil, nil, "", err
+	}
+
+	// Setup default check if none given
+	if len(checks) < 1 {
+		checks = []*structs.CheckType{
+			&structs.CheckType{
+				Name: "Connect Sidecar Listening",
+				// Default to localhost rather than agent/service public IP. The checks
+				// can always be overridden if a non-loopback IP is needed.
+				TCP:      fmt.Sprintf("127.0.0.1:%d", sidecar.Port),
+				Interval: 10 * time.Second,
+			},
+			&structs.CheckType{
+				Name:         "Connect Sidecar Aliasing " + ns.ID,
+				AliasService: ns.ID,
+			},
+		}
+	}
+
+	return sidecar, checks, token, nil
+}

agent/sidecar_service_test.go

@@ -0,0 +1,248 @@
+package agent
+
+import (
+	"testing"
+	"time"
+
+	"github.com/hashicorp/consul/agent/structs"
+	"github.com/stretchr/testify/require"
+)
+
+func TestAgent_sidecarServiceFromNodeService(t *testing.T) {
+	tests := []struct {
+		name              string
+		preRegister       *structs.ServiceDefinition
+		sd                *structs.ServiceDefinition
+		token             string
+		autoPortsDisabled bool
+		wantNS            *structs.NodeService
+		wantChecks        []*structs.CheckType
+		wantToken         string
+		wantErr           string
+	}{
+		{
+			name: "no sidecar",
+			sd: &structs.ServiceDefinition{
+				Name: "web",
+				Port: 1111,
+			},
+			token:      "foo",
+			wantNS:     nil,
+			wantChecks: nil,
+			wantToken:  "",
+			wantErr:    "", // Should NOT error
+		},
+		{
+			name: "all the defaults",
+			sd: &structs.ServiceDefinition{
+				ID:   "web1",
+				Name: "web",
+				Port: 1111,
+				Connect: &structs.ServiceConnect{
+					SidecarService: &structs.ServiceDefinition{},
+				},
+			},
+			token: "foo",
+			wantNS: &structs.NodeService{
+				Kind:                       structs.ServiceKindConnectProxy,
+				ID:                         "web1-sidecar-proxy",
+				Service:                    "web-sidecar-proxy",
+				Port:                       2222,
+				LocallyRegisteredAsSidecar: true,
+				Proxy: structs.ConnectProxyConfig{
+					DestinationServiceName: "web",
+					DestinationServiceID:   "web1",
+					LocalServiceAddress:    "127.0.0.1",
+					LocalServicePort:       1111,
+				},
+			},
+			wantChecks: []*structs.CheckType{
+				&structs.CheckType{
+					Name:     "Connect Sidecar Listening",
+					TCP:      "127.0.0.1:2222",
+					Interval: 10 * time.Second,
+				},
+				&structs.CheckType{
+					Name:         "Connect Sidecar Aliasing web1",
+					AliasService: "web1",
+				},
+			},
+			wantToken: "foo",
+		},
+		{
+			name: "all the allowed overrides",
+			sd: &structs.ServiceDefinition{
+				ID:   "web1",
+				Name: "web",
+				Port: 1111,
+				Connect: &structs.ServiceConnect{
+					SidecarService: &structs.ServiceDefinition{
+						Name:    "motorbike1",
+						Port:    3333,
+						Tags:    []string{"foo", "bar"},
+						Address: "127.127.127.127",
+						Meta:    map[string]string{"foo": "bar"},
+						Check: structs.CheckType{
+							ScriptArgs: []string{"sleep", "1"},
+							Interval:   999 * time.Second,
+						},
+						Token:             "custom-token",
+						EnableTagOverride: true,
+						Proxy: &structs.ConnectProxyConfig{
+							DestinationServiceName: "web",
+							DestinationServiceID:   "web1",
+							LocalServiceAddress:    "127.0.127.0",
+							LocalServicePort:       9999,
+							Config:                 map[string]interface{}{"baz": "qux"},
+							Upstreams:              structs.TestUpstreams(t),
+						},
+					},
+				},
+			},
+			token: "foo",
+			wantNS: &structs.NodeService{
+				Kind:    structs.ServiceKindConnectProxy,
+				ID:      "web1-sidecar-proxy",
+				Service: "motorbike1",
+				Port:    3333,
+				Tags:    []string{"foo", "bar"},
+				Address: "127.127.127.127",
+				Meta: map[string]string{
+					"foo": "bar",
+				},
+				LocallyRegisteredAsSidecar: true,
+				EnableTagOverride:          true,
+				Proxy: structs.ConnectProxyConfig{
+					DestinationServiceName: "web",
+					DestinationServiceID:   "web1",
+					LocalServiceAddress:    "127.0.127.0",
+					LocalServicePort:       9999,
+					Config:                 map[string]interface{}{"baz": "qux"},
+					Upstreams:              structs.TestAddDefaultsToUpstreams(t, structs.TestUpstreams(t)),
+				},
+			},
+			wantChecks: []*structs.CheckType{
+				&structs.CheckType{
+					ScriptArgs: []string{"sleep", "1"},
+					Interval:   999 * time.Second,
+				},
+			},
+			wantToken: "custom-token",
+		},
+		{
+			name: "no auto ports available",
+			// register another sidecar consuming our 1 and only allocated auto port.
+			preRegister: &structs.ServiceDefinition{
+				Kind: structs.ServiceKindConnectProxy,
+				Name: "api-proxy-sidecar",
+				Port: 2222, // Consume the one available auto-port
+				Proxy: &structs.ConnectProxyConfig{
+					DestinationServiceName: "api",
+				},
+			},
+			sd: &structs.ServiceDefinition{
+				ID:   "web1",
+				Name: "web",
+				Port: 1111,
+				Connect: &structs.ServiceConnect{
+					SidecarService: &structs.ServiceDefinition{},
+				},
+			},
+			token:   "foo",
+			wantErr: "none left in the configured range [2222, 2222]",
+		},
+		{
+			name:              "auto ports disabled",
+			autoPortsDisabled: true,
+			sd: &structs.ServiceDefinition{
+				ID:   "web1",
+				Name: "web",
+				Port: 1111,
+				Connect: &structs.ServiceConnect{
+					SidecarService: &structs.ServiceDefinition{},
+				},
+			},
+			token:   "foo",
+			wantErr: "auto-assignement disabled in config",
+		},
+		{
+			name: "invalid check type",
+			sd: &structs.ServiceDefinition{
+				ID:   "web1",
+				Name: "web",
+				Port: 1111,
+				Connect: &structs.ServiceConnect{
+					SidecarService: &structs.ServiceDefinition{
+						Check: structs.CheckType{
+							TCP: "foo",
+							// Invalid since no interval specified
+						},
+					},
+				},
+			},
+			token:   "foo",
+			wantErr: "Interval must be > 0",
+		},
+		{
+			name: "invalid meta",
+			sd: &structs.ServiceDefinition{
+				ID:   "web1",
+				Name: "web",
+				Port: 1111,
+				Connect: &structs.ServiceConnect{
+					SidecarService: &structs.ServiceDefinition{
+						Meta: map[string]string{
+							"consul-reserved-key-should-be-rejected": "true",
+						},
+					},
+				},
+			},
+			token:   "foo",
+			wantErr: "reserved for internal use",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// Set port range to make it deterministic. This allows a single assigned
+			// port at 2222 thanks to being inclusive at both ends.
+			hcl := `
+			ports {
+				sidecar_min_port = 2222
+				sidecar_max_port = 2222
+			}
+			`
+			if tt.autoPortsDisabled {
+				hcl = `
+				ports {
+					sidecar_min_port = 0
+					sidecar_max_port = 0
+				}
+				`
+			}
+
+			require := require.New(t)
+			a := NewTestAgent("jones", hcl)
+
+			if tt.preRegister != nil {
+				err := a.AddService(tt.preRegister.NodeService(), nil, false, "")
+				require.NoError(err)
+			}
+
+			ns := tt.sd.NodeService()
+			err := ns.Validate()
+			require.NoError(err, "Invalid test case - NodeService must validate")
+
+			gotNS, gotChecks, gotToken, err := a.sidecarServiceFromNodeService(ns, tt.token)
+			if tt.wantErr != "" {
+				require.Error(err)
+				require.Contains(err.Error(), tt.wantErr)
+				return
+			}
+
+			require.NoError(err)
+			require.Equal(tt.wantNS, gotNS)
+			require.Equal(tt.wantChecks, gotChecks)
+			require.Equal(tt.wantToken, gotToken)
+		})
+	}
+}

agent/structs/connect_proxy_config.go

@@ -14,7 +14,7 @@ import (
 type ConnectProxyConfig struct {
 	// DestinationServiceName is required and is the name of the service to accept
 	// traffic for.
-	DestinationServiceName string
+	DestinationServiceName string `json:",omitempty"`
 
 	// DestinationServiceID is optional and should only be specified for
 	// "side-car" style proxies where the proxy is in front of just a single
@@ -22,27 +22,27 @@ type ConnectProxyConfig struct {
 	// being represented which must be registered to the same agent. It's valid to
 	// provide a service ID that does not yet exist to avoid timing issues when
 	// bootstrapping a service with a proxy.
-	DestinationServiceID string
+	DestinationServiceID string `json:",omitempty"`
 
 	// LocalServiceAddress is the address of the local service instance. It is
 	// optional and should only be specified for "side-car" style proxies. It will
 	// default to 127.0.0.1 if the proxy is a "side-car" (DestinationServiceID is
 	// set) but otherwise will be ignored.
-	LocalServiceAddress string
+	LocalServiceAddress string `json:",omitempty"`
 
 	// LocalServicePort is the port of the local service instance. It is optional
 	// and should only be specified for "side-car" style proxies. It will default
 	// to the registered port for the instance if the proxy is a "side-car"
 	// (DestinationServiceID is set) but otherwise will be ignored.
-	LocalServicePort int
+	LocalServicePort int `json:",omitempty"`
 
 	// Config is the arbitrary configuration data provided with the proxy
 	// registration.
-	Config map[string]interface{}
+	Config map[string]interface{} `json:",omitempty"`
 
 	// Upstreams describes any upstream dependencies the proxy instance should
 	// setup.
-	Upstreams Upstreams
+	Upstreams Upstreams `json:",omitempty"`
 }
 
 // ToAPI returns the api struct with the same fields. We have duplicates to

agent/structs/structs.go

@@ -658,6 +658,25 @@ type NodeService struct {
 	// a pointer so that we never have to nil-check this.
 	Connect ServiceConnect
 
+	// LocallyRegisteredAsSidecar is private as it is only used by a local agent
+	// state to track if the service was registered from a nested sidecar_service
+	// block. We need to track that so we can know whether we need to deregister
+	// it automatically too if it's removed from the service definition or if the
+	// parent service is deregistered. Relying only on ID would cause us to
+	// deregister regular services if they happen to be registered using the same
+	// ID scheme as our sidecars do by default. We could use meta but that gets
+	// unpleasant because we can't use the consul- prefix from an agent (reserved
+	// for use internally but in practice that means within the state store or in
+	// responses only), and it leaks the detail publically which people might rely
+	// on which is a bit unpleasant for something that is meant to be config-file
+	// syntax sugar. Note this is not translated to ServiceNode and friends and
+	// may not be set on a NodeService that isn't the one the agent registered and
+	// keeps in it's local state. We never want this rendered in JSON as it's
+	// internal only. Right now our agent endpoints return api structs which don't
+	// include it but this is a safety net incase we change that or there is
+	// somewhere this is used in API output.
+	LocallyRegisteredAsSidecar bool `json:"-"`
+
 	RaftIndex
 }
 
@@ -665,12 +684,21 @@ type NodeService struct {
 // definitions from the agent to the state store.
 type ServiceConnect struct {
 	// Native is true when this service can natively understand Connect.
-	Native bool
+	Native bool `json:",omitempty"`
 
 	// Proxy configures a connect proxy instance for the service. This is
 	// only used for agent service definitions and is invalid for non-agent
 	// (catalog API) definitions.
-	Proxy *ServiceDefinitionConnectProxy
+	Proxy *ServiceDefinitionConnectProxy `json:",omitempty"`
+
+	// SidecarService is a nested Service Definition to register at the same time.
+	// It's purely a convenience mechanism to allow specifying a sidecar service
+	// along with the application service definition. It's nested nature allows
+	// all of the fields to be defaulted which can reduce the amount of
+	// boilerplate needed to register a sidecar service separately, but the end
+	// result is identical to just making a second service registration via any
+	// other means.
+	SidecarService *ServiceDefinition `json:",omitempty"`
 }
 
 // Validate validates the node service configuration.
@@ -708,6 +736,25 @@ func (s *NodeService) Validate() error {
 		}
 	}
 
+	// Nested sidecar validation
+	if s.Connect.SidecarService != nil {
+		if s.Connect.SidecarService.ID != "" {
+			result = multierror.Append(result, fmt.Errorf(
+				"A SidecarService cannot specify an ID as this is managed by the "+
+					"agent"))
+		}
+		if s.Connect.SidecarService.Connect != nil {
+			if s.Connect.SidecarService.Connect.SidecarService != nil {
+				result = multierror.Append(result, fmt.Errorf(
+					"A SidecarService cannot have a nested SidecarService"))
+			}
+			if s.Connect.SidecarService.Connect.Proxy != nil {
+				result = multierror.Append(result, fmt.Errorf(
+					"A SidecarService cannot have a managed proxy"))
+			}
+		}
+	}
+
 	return result
 }
 

agent/structs/structs_test.go

@@ -284,6 +284,62 @@ func TestStructs_NodeService_ValidateConnectProxy(t *testing.T) {
 	}
 }
 
+func TestStructs_NodeService_ValidateSidecarService(t *testing.T) {
+	cases := []struct {
+		Name   string
+		Modify func(*NodeService)
+		Err    string
+	}{
+		{
+			"valid",
+			func(x *NodeService) {},
+			"",
+		},
+
+		{
+			"ID can't be set",
+			func(x *NodeService) { x.Connect.SidecarService.ID = "foo" },
+			"SidecarService cannot specify an ID",
+		},
+
+		{
+			"Nested sidecar can't be set",
+			func(x *NodeService) {
+				x.Connect.SidecarService.Connect = &ServiceConnect{
+					SidecarService: &ServiceDefinition{},
+				}
+			},
+			"SidecarService cannot have a nested SidecarService",
+		},
+
+		{
+			"Sidecar can't have managed proxy",
+			func(x *NodeService) {
+				x.Connect.SidecarService.Connect = &ServiceConnect{
+					Proxy: &ServiceDefinitionConnectProxy{},
+				}
+			},
+			"SidecarService cannot have a managed proxy",
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.Name, func(t *testing.T) {
+			assert := assert.New(t)
+			ns := TestNodeServiceSidecar(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

@@ -48,3 +48,15 @@ func TestNodeServiceProxy(t testing.T) *NodeService {
 		Proxy:   TestConnectProxyConfig(t),
 	}
 }
+
+// TestNodeServiceSidecar returns a *NodeService representing a service
+// registration with a nested Sidecar registration.
+func TestNodeServiceSidecar(t testing.T) *NodeService {
+	return &NodeService{
+		Service: "web",
+		Port:    2222,
+		Connect: ServiceConnect{
+			SidecarService: &ServiceDefinition{},
+		},
+	}
+}

agent/structs/testing_connect_proxy_config.go

@@ -34,3 +34,19 @@ func TestUpstreams(t testing.T) Upstreams {
 		},
 	}
 }
+
+// TestAddDefaultsToUpstreams takes an array of upstreams (such as that from
+// TestUpstreams) and adds default values that are populated during
+// refigistration. Use this for generating the expected Upstreams value after
+// registration.
+func TestAddDefaultsToUpstreams(t testing.T, upstreams []Upstream) []Upstream {
+	ups := make([]Upstream, len(upstreams))
+	for i := range upstreams {
+		ups[i] = upstreams[i]
+		// Fill in default fields we expect to have back explicitly in a response
+		if ups[i].DestinationType == "" {
+			ups[i].DestinationType = UpstreamDestTypeService
+		}
+	}
+	return ups
+}

api/agent.go

@@ -90,8 +90,9 @@ type AgentService struct {
 
 // AgentServiceConnect represents the Connect configuration of a service.
 type AgentServiceConnect struct {
-	Native bool                      `json:",omitempty"`
-	Proxy  *AgentServiceConnectProxy `json:",omitempty"`
+	Native         bool                      `json:",omitempty"`
+	Proxy          *AgentServiceConnectProxy `json:",omitempty"`
+	SidecarService *AgentServiceRegistration `json:",omitempty"`
 }
 
 // AgentServiceConnectProxy represents the Connect Proxy configuration of a

api/agent_test.go

@@ -323,6 +323,52 @@ func TestAPI_AgentServices_ManagedConnectProxyDeprecatedUpstreams(t *testing.T)
 	}
 }
 
+func TestAPI_AgentServices_SidecarService(t *testing.T) {
+	t.Parallel()
+	c, s := makeClient(t)
+	defer s.Stop()
+
+	agent := c.Agent()
+
+	// Register service
+	reg := &AgentServiceRegistration{
+		Name: "foo",
+		Port: 8000,
+		Connect: &AgentServiceConnect{
+			SidecarService: &AgentServiceRegistration{},
+		},
+	}
+	if err := agent.ServiceRegister(reg); err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	services, err := agent.Services()
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	if _, ok := services["foo"]; !ok {
+		t.Fatalf("missing service: %v", services)
+	}
+	if _, ok := services["foo-sidecar-proxy"]; !ok {
+		t.Fatalf("missing sidecar service: %v", services)
+	}
+
+	if err := agent.ServiceDeregister("foo"); err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	// Deregister should have removed both service and it's sidecar
+	services, err = agent.Services()
+	require.NoError(t, err)
+
+	if _, ok := services["foo"]; ok {
+		t.Fatalf("didn't remove service: %v", services)
+	}
+	if _, ok := services["foo-sidecar-proxy"]; ok {
+		t.Fatalf("didn't remove sidecar service: %v", services)
+	}
+}
+
 func TestAPI_AgentServices_ExternalConnectProxy(t *testing.T) {
 	t.Parallel()
 	c, s := makeClient(t)