HTTP agent registration allows proxy to be defined.

agent/agent.go

@@ -2426,9 +2426,15 @@ func (a *Agent) unloadChecks() error {
 // loadProxies will load connect proxy definitions from configuration and
 // persisted definitions on disk, and load them into the local agent.
 func (a *Agent) loadProxies(conf *config.RuntimeConfig) error {
-	for _, proxy := range conf.ConnectProxies {
+	for _, svc := range conf.Services {
-		if err := a.AddProxy(proxy, false); err != nil {
+		if svc.Connect != nil {
-			return fmt.Errorf("failed adding proxy: %s", err)
+			proxy, err := svc.ConnectManagedProxy()
+			if err != nil {
+				return fmt.Errorf("failed adding proxy: %s", err)
+			}
+			if err := a.AddProxy(proxy, false); err != nil {
+				return fmt.Errorf("failed adding proxy: %s", err)
+			}
 		}
 	}
 

agent/agent_endpoint.go

@@ -589,10 +589,24 @@ func (s *HTTPServer) AgentRegisterService(resp http.ResponseWriter, req *http.Re
 		return nil, err
 	}
 
+	// Get any proxy registrations
+	proxy, err := args.ConnectManagedProxy()
+	if err != nil {
+		resp.WriteHeader(http.StatusBadRequest)
+		fmt.Fprintf(resp, err.Error())
+		return nil, nil
+	}
+
 	// Add the service.
 	if err := s.agent.AddService(ns, chkTypes, true, token); err != nil {
 		return nil, err
 	}
+	// Add proxy (which will add proxy service so do it before we trigger sync)
+	if proxy != nil {
+		if err := s.agent.AddProxy(proxy, true); err != nil {
+			return nil, err
+		}
+	}
 	s.syncChanges()
 	return nil, nil
 }

agent/agent_endpoint_test.go

@@ -26,6 +26,7 @@ import (
 	"github.com/hashicorp/serf/serf"
 	"github.com/pascaldekloe/goe/verify"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func makeReadOnlyAgentACL(t *testing.T, srv *HTTPServer) string {
@@ -1369,10 +1370,78 @@ func TestAgent_RegisterService_InvalidAddress(t *testing.T) {
 	}
 }
 
-// This tests local agent service registration of a connect proxy. This
+// This tests local agent service registration with a managed proxy.
-// verifies that it is put in the local state store properly for syncing
+func TestAgent_RegisterService_ManagedConnectProxy(t *testing.T) {
-// later.
+	t.Parallel()
-func TestAgent_RegisterService_ConnectProxy(t *testing.T) {
+
+	assert := assert.New(t)
+	require := require.New(t)
+	a := NewTestAgent(t.Name(), "")
+	defer a.Shutdown()
+
+	// Register a proxy. Note that the destination doesn't exist here on
+	// this agent or in the catalog at all. This is intended and part
+	// of the design.
+	args := &structs.ServiceDefinition{
+		Name: "web",
+		Port: 8000,
+		// This is needed just because empty check struct (not pointer) get json
+		// encoded as object with zero values and then decoded back to object with
+		// zero values _except that the header map is an empty map not a nil map_.
+		// So our check to see if s.Check.Empty() returns false since DeepEqual
+		// considers empty maps and nil maps to be different types. Then the request
+		// fails validation because the Check definition isn't valid... This is jank
+		// we should fix but it's another yak I don't want to shave right now.
+		Check: structs.CheckType{
+			TTL: 15 * time.Second,
+		},
+		Connect: &structs.ServiceDefinitionConnect{
+			Proxy: &structs.ServiceDefinitionConnectProxy{
+				ExecMode: "script",
+				Command:  "proxy.sh",
+				Config: map[string]interface{}{
+					"foo": "bar",
+				},
+			},
+		},
+	}
+
+	req, _ := http.NewRequest("PUT", "/v1/agent/service/register?token=abc123", jsonReader(args))
+	resp := httptest.NewRecorder()
+	obj, err := a.srv.AgentRegisterService(resp, req)
+	assert.NoError(err)
+	assert.Nil(obj)
+	require.Equal(200, resp.Code, "request failed with body: %s",
+		resp.Body.String())
+
+	// Ensure the target service
+	_, ok := a.State.Services()["web"]
+	assert.True(ok, "has service")
+
+	// Ensure the proxy service was registered
+	proxySvc, ok := a.State.Services()["web-proxy"]
+	require.True(ok, "has proxy service")
+	assert.Equal(structs.ServiceKindConnectProxy, proxySvc.Kind)
+	assert.Equal("web", proxySvc.ProxyDestination)
+	assert.NotEmpty(proxySvc.Port, "a port should have been assigned")
+
+	// Ensure proxy itself was registered
+	proxy := a.State.Proxy("web-proxy")
+	require.NotNil(proxy)
+	assert.Equal(structs.ProxyExecModeScript, proxy.ExecMode)
+	assert.Equal("proxy.sh", proxy.Command)
+	assert.Equal(args.Connect.Proxy.Config, proxy.Config)
+
+	// Ensure the token was configured
+	assert.Equal("abc123", a.State.ServiceToken("web"))
+	assert.Equal("abc123", a.State.ServiceToken("web-proxy"))
+}
+
+// This tests local agent service registration of a unmanaged connect proxy.
+// This verifies that it is put in the local state store properly for syncing
+// later. Note that _managed_ connect proxies are registered as part of the
+// target service's registration.
+func TestAgent_RegisterService_UnmanagedConnectProxy(t *testing.T) {
 	t.Parallel()
 
 	assert := assert.New(t)
@@ -1411,7 +1480,7 @@ func TestAgent_RegisterService_ConnectProxy(t *testing.T) {
 // 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.
-func TestAgent_RegisterService_ConnectProxyInvalid(t *testing.T) {
+func TestAgent_RegisterService_UnmanagedConnectProxyInvalid(t *testing.T) {
 	t.Parallel()
 
 	assert := assert.New(t)

agent/config/builder.go

@@ -322,15 +322,8 @@ func (b *Builder) Build() (rt RuntimeConfig, err error) {
 	}
 
 	var services []*structs.ServiceDefinition
-	var proxies []*structs.ConnectManagedProxy
 	for _, service := range c.Services {
 		services = append(services, b.serviceVal(&service))
-		// Register any connect proxies requested
-		if proxy := b.connectManagedProxyVal(&service); proxy != nil {
-			proxies = append(proxies, proxy)
-		}
-		// TODO(banks): support connect-native registrations (v.Connect.Enabled ==
-		// true)
 	}
 	if c.Service != nil {
 		services = append(services, b.serviceVal(c.Service))
@@ -648,7 +641,6 @@ func (b *Builder) Build() (rt RuntimeConfig, err error) {
 		CheckUpdateInterval:         b.durationVal("check_update_interval", c.CheckUpdateInterval),
 		Checks:                      checks,
 		ClientAddrs:                 clientAddrs,
-		ConnectProxies:              proxies,
 		ConnectProxyBindMinPort:     proxyBindMinPort,
 		ConnectProxyBindMaxPort:     proxyBindMaxPort,
 		DataDir:                     b.stringVal(c.DataDir),
@@ -1020,46 +1012,26 @@ func (b *Builder) serviceVal(v *ServiceDefinition) *structs.ServiceDefinition {
 		Token:             b.stringVal(v.Token),
 		EnableTagOverride: b.boolVal(v.EnableTagOverride),
 		Checks:            checks,
+		Connect:           b.serviceConnectVal(v.Connect),
 	}
 }
 
-func (b *Builder) connectManagedProxyVal(v *ServiceDefinition) *structs.ConnectManagedProxy {
+func (b *Builder) serviceConnectVal(v *ServiceConnect) *structs.ServiceDefinitionConnect {
-	if v.Connect == nil || v.Connect.Proxy == nil {
+	if v == nil {
 		return nil
 	}
 
-	p := v.Connect.Proxy
+	var proxy *structs.ServiceDefinitionConnectProxy
-
+	if v.Proxy != nil {
-	targetID := b.stringVal(v.ID)
+		proxy = &structs.ServiceDefinitionConnectProxy{
-	if targetID == "" {
+			ExecMode: b.stringVal(v.Proxy.ExecMode),
-		targetID = b.stringVal(v.Name)
+			Command:  b.stringVal(v.Proxy.Command),
-	}
+			Config:   v.Proxy.Config,
-
-	execMode := structs.ProxyExecModeDaemon
-	if p.ExecMode != nil {
-		switch *p.ExecMode {
-		case "daemon":
-			execMode = structs.ProxyExecModeDaemon
-		case "script":
-			execMode = structs.ProxyExecModeScript
-		default:
-			b.err = multierror.Append(fmt.Errorf(
-				"service[%s]: invalid connect proxy exec_mode: %s", targetID,
-				*p.ExecMode))
-			return nil
 		}
 	}
 
-	return &structs.ConnectManagedProxy{
+	return &structs.ServiceDefinitionConnect{
-		ExecMode: execMode,
+		Proxy: proxy,
-		Command:  b.stringVal(p.Command),
-		Config:   p.Config,
-		// ProxyService will be setup when the agent registers the configured
-		// proxies and starts them etc. We could do it here but we may need to do
-		// things like probe the OS for a free port etc. And we have enough info to
-		// resolve all this later.
-		ProxyService:    nil,
-		TargetServiceID: targetID,
 	}
 }
 

agent/config/runtime.go

@@ -621,10 +621,6 @@ type RuntimeConfig struct {
 	// that.
 	ConnectEnabled bool
 
-	// ConnectProxies is a list of configured proxies taken from the "connect"
-	// block of service registrations.
-	ConnectProxies []*structs.ConnectManagedProxy
-
 	// ConnectProxyBindMinPort is the inclusive start of the range of ports
 	// allocated to the agent for starting proxy listeners on where no explicit
 	// port is specified.

agent/config/runtime_test.go

@@ -3403,21 +3403,8 @@ func TestFullConfig(t *testing.T) {
 				DeregisterCriticalServiceAfter: 13209 * time.Second,
 			},
 		},
-		CheckUpdateInterval: 16507 * time.Second,
+		CheckUpdateInterval:       16507 * time.Second,
-		ClientAddrs:         []*net.IPAddr{ipAddr("93.83.18.19")},
+		ClientAddrs:               []*net.IPAddr{ipAddr("93.83.18.19")},
-		ConnectProxies: []*structs.ConnectManagedProxy{
-			{
-				ExecMode: structs.ProxyExecModeDaemon,
-				Command:  "awesome-proxy",
-				Config: map[string]interface{}{
-					"foo": "qux", // Overriden by service
-					// Note globals are not merged here but on rendering to the proxy
-					// endpoint. That's because proxies can be added later too so merging
-					// at config time is redundant if we have to do it later anyway.
-				},
-				TargetServiceID: "MRHVMZuD",
-			},
-		},
 		ConnectProxyBindMinPort:   2000,
 		ConnectProxyBindMaxPort:   3000,
 		DNSAddrs:                  []net.Addr{tcpAddr("93.95.95.81:7001"), udpAddr("93.95.95.81:7001")},
@@ -3592,6 +3579,15 @@ func TestFullConfig(t *testing.T) {
 						DeregisterCriticalServiceAfter: 68482 * time.Second,
 					},
 				},
+				Connect: &structs.ServiceDefinitionConnect{
+					Proxy: &structs.ServiceDefinitionConnectProxy{
+						ExecMode: "daemon",
+						Command:  "awesome-proxy",
+						Config: map[string]interface{}{
+							"foo": "qux",
+						},
+					},
+				},
 			},
 			{
 				ID:                "dLOXpSCI",
@@ -4082,7 +4078,6 @@ func TestSanitize(t *testing.T) {
     ],
     "ClientAddrs": [],
     "ConnectEnabled": false,
-    "ConnectProxies": [],
     "ConnectProxyBindMaxPort": 0,
     "ConnectProxyBindMinPort": 0,
     "ConnectProxyDefaultConfig": {},
@@ -4219,6 +4214,7 @@ func TestSanitize(t *testing.T) {
                 "Timeout": "0s"
             },
             "Checks": [],
+            "Connect": null,
             "EnableTagOverride": false,
             "ID": "",
             "Kind": "",

agent/structs/service_definition.go

@@ -1,5 +1,9 @@
 package structs
 
+import (
+	"fmt"
+)
+
 // ServiceDefinition is used to JSON decode the Service definitions. For
 // documentation on specific fields see NodeService which is better documented.
 type ServiceDefinition struct {
@@ -15,6 +19,7 @@ type ServiceDefinition struct {
 	Token             string
 	EnableTagOverride bool
 	ProxyDestination  string
+	Connect           *ServiceDefinitionConnect
 }
 
 func (s *ServiceDefinition) NodeService() *NodeService {
@@ -35,6 +40,45 @@ func (s *ServiceDefinition) NodeService() *NodeService {
 	return ns
 }
 
+// ConnectManagedProxy returns a ConnectManagedProxy from the ServiceDefinition
+// if one is configured validly. Note that is may return nil if no proxy is
+// configured and will also return nil error in this case too as it's an
+// expected case. The error returned indicates that there was an attempt to
+// configure a proxy made but that it was invalid input, e.g. invalid
+// "exec_mode".
+func (s *ServiceDefinition) ConnectManagedProxy() (*ConnectManagedProxy, error) {
+	if s.Connect == nil || s.Connect.Proxy == nil {
+		return nil, nil
+	}
+
+	// NodeService performs some simple normalization like copying ID from Name
+	// which we shouldn't hard code ourselves here...
+	ns := s.NodeService()
+
+	execMode := ProxyExecModeDaemon
+	switch s.Connect.Proxy.ExecMode {
+	case "":
+		execMode = ProxyExecModeDaemon
+	case "daemon":
+		execMode = ProxyExecModeDaemon
+	case "script":
+		execMode = ProxyExecModeScript
+	default:
+		return nil, fmt.Errorf("invalid exec mode: %s", s.Connect.Proxy.ExecMode)
+	}
+
+	p := &ConnectManagedProxy{
+		ExecMode: execMode,
+		Command:  s.Connect.Proxy.Command,
+		Config:   s.Connect.Proxy.Config,
+		// ProxyService will be setup when the agent registers the configured
+		// proxies and starts them etc.
+		TargetServiceID: ns.ID,
+	}
+
+	return p, nil
+}
+
 func (s *ServiceDefinition) CheckTypes() (checks CheckTypes, err error) {
 	if !s.Check.Empty() {
 		err := s.Check.Validate()
@@ -51,3 +95,21 @@ func (s *ServiceDefinition) CheckTypes() (checks CheckTypes, err error) {
 	}
 	return checks, nil
 }
+
+// ServiceDefinitionConnect is the connect block within a service registration.
+// Note this is duplicated in config.ServiceConnect and needs to be kept in
+// sync.
+type ServiceDefinitionConnect struct {
+	// TODO(banks) add way to specify that the app is connect-native
+	// Proxy configures a connect proxy instance for the service
+	Proxy *ServiceDefinitionConnectProxy `json:"proxy,omitempty" hcl:"proxy" mapstructure:"proxy"`
+}
+
+// ServiceDefinitionConnectProxy is the connect proxy config  within a service
+// registration. Note this is duplicated in config.ServiceConnectProxy and needs
+// to be kept in sync.
+type ServiceDefinitionConnectProxy struct {
+	Command  string                 `json:"command,omitempty" hcl:"command" mapstructure:"command"`
+	ExecMode string                 `json:"exec_mode,omitempty" hcl:"exec_mode" mapstructure:"exec_mode"`
+	Config   map[string]interface{} `json:"config,omitempty" hcl:"config" mapstructure:"config"`
+}