Adds `api` client code and tests for new Proxy Config endpoint, registering with proxy and seeing proxy config in /agent/services list.

GNUmakefile

@@ -82,12 +82,14 @@ test: other-consul dev-build vet
 	@echo "Exit code: $$(cat exit-code)" >> test.log
 	@# This prints all the race report between ====== lines
 	@awk '/^WARNING: DATA RACE/ {do_print=1; print "=================="} do_print==1 {print} /^={10,}/ {do_print=0}' test.log || true
-	@grep -A10 'panic: test timed out' test.log || true
+	@grep -A10 'panic: ' test.log || true
 	@# Prints all the failure output until the next non-indented line - testify
 	@# helpers often output multiple lines for readability but useless if we can't
-	@# see them.
+	@# see them. Un-intuitive order of matches is necessary. No || true because
-	@awk '/--- SKIP/ {do_print=1} /^[^[:space:]]/ {do_print=0} do_print==1 {print}' test.log || true
+	@# awk always returns true even if there is no match and it breaks non-bash
-	@awk '/--- FAIL/ {do_print=1} /^[^[:space:]]/ {do_print=0} do_print==1 {print}' test.log || true
+	@# shells locally.
+	@awk '/^[^[:space:]]/ {do_print=0} /--- SKIP/ {do_print=1} do_print==1 {print}' test.log
+	@awk '/^[^[:space:]]/ {do_print=0} /--- FAIL/ {do_print=1} do_print==1 {print}' test.log
 	@grep '^FAIL' test.log || true
 	@if [ "$$(cat exit-code)" == "0" ] ; then echo "PASS" ; exit 0 ; else exit 1 ; fi
 

agent/agent_endpoint.go

@@ -162,25 +162,48 @@ func (s *HTTPServer) AgentServices(resp http.ResponseWriter, req *http.Request)
 		return nil, err
 	}
 
+	proxies := s.agent.State.Proxies()
+
+	// Convert into api.AgentService since that includes Connect config but so far
+	// NodeService doesn't need to internally. They are otherwise identical since
+	// that is the struct used in client for reading the one we output here
+	// anyway.
+	agentSvcs := make(map[string]*api.AgentService)
+
 	// Use empty list instead of nil
 	for id, s := range services {
-		if s.Tags == nil || s.Meta == nil {
+		as := &api.AgentService{
-			clone := *s
+			Kind:              api.ServiceKind(s.Kind),
-			if s.Tags == nil {
+			ID:                s.ID,
-				clone.Tags = make([]string, 0)
+			Service:           s.Service,
-			} else {
+			Tags:              s.Tags,
-				clone.Tags = s.Tags
+			Port:              s.Port,
+			Address:           s.Address,
+			EnableTagOverride: s.EnableTagOverride,
+			CreateIndex:       s.CreateIndex,
+			ModifyIndex:       s.ModifyIndex,
+			ProxyDestination:  s.ProxyDestination,
 		}
-			if s.Meta == nil {
+		if as.Tags == nil {
-				clone.Meta = make(map[string]string)
+			as.Tags = []string{}
-			} else {
-				clone.Meta = s.Meta
 		}
-			services[id] = &clone
+		if as.Meta == nil {
+			as.Meta = map[string]string{}
 		}
+		// Attach Connect configs if the exist
+		if proxy, ok := proxies[id+"-proxy"]; ok {
+			as.Connect = &api.AgentServiceConnect{
+				Proxy: &api.AgentServiceConnectProxy{
+					ExecMode: api.ProxyExecMode(proxy.Proxy.ExecMode.String()),
+					Command:  proxy.Proxy.Command,
+					Config:   proxy.Proxy.Config,
+				},
+			}
+		}
+		agentSvcs[id] = as
 	}
 
-	return services, nil
+	return agentSvcs, nil
 }
 
 func (s *HTTPServer) AgentChecks(resp http.ResponseWriter, req *http.Request) (interface{}, error) {
@@ -904,7 +927,7 @@ func (s *HTTPServer) AgentConnectCALeafCert(resp http.ResponseWriter, req *http.
 //
 // Returns the local proxy config for the identified proxy. Requires token=
 // param with the correct local ProxyToken (not ACL token).
-func (s *HTTPServer) AgentConnectProxyConfig(resp http.ResponseWriter, req *http.Request) (interface{}, error) {
+func (s *HTTPServer) ConnectProxyConfig(resp http.ResponseWriter, req *http.Request) (interface{}, error) {
 	// Get the proxy ID. Note that this is the ID of a proxy's service instance.
 	id := strings.TrimPrefix(req.URL.Path, "/v1/agent/connect/proxy/")
 
@@ -949,12 +972,12 @@ func (s *HTTPServer) AgentConnectProxyConfig(resp http.ResponseWriter, req *http
 			}
 			contentHash := fmt.Sprintf("%x", hash)
 
-			reply := &structs.ConnectManageProxyResponse{
+			reply := &api.ConnectProxyConfig{
 				ProxyServiceID:    proxy.Proxy.ProxyService.ID,
 				TargetServiceID:   target.ID,
 				TargetServiceName: target.Service,
 				ContentHash:       contentHash,
-				ExecMode:          proxy.Proxy.ExecMode.String(),
+				ExecMode:          api.ProxyExecMode(proxy.Proxy.ExecMode.String()),
 				Command:           proxy.Proxy.Command,
 				Config:            proxy.Proxy.Config,
 			}

agent/agent_endpoint_test.go

@@ -57,25 +57,39 @@ func TestAgent_Services(t *testing.T) {
 		Tags:    []string{"master"},
 		Port:    5000,
 	}
-	a.State.AddService(srv1, "")
+	require.NoError(t, a.State.AddService(srv1, ""))
+
+	// Add a managed proxy for that service
+	prxy1 := &structs.ConnectManagedProxy{
+		ExecMode: structs.ProxyExecModeScript,
+		Command:  "proxy.sh",
+		Config: map[string]interface{}{
+			"bind_port": 1234,
+			"foo":       "bar",
+		},
+		TargetServiceID: "mysql",
+	}
+	_, err := a.State.AddProxy(prxy1, "")
+	require.NoError(t, err)
 
 	req, _ := http.NewRequest("GET", "/v1/agent/services", nil)
 	obj, err := a.srv.AgentServices(nil, req)
 	if err != nil {
 		t.Fatalf("Err: %v", err)
 	}
-	val := obj.(map[string]*structs.NodeService)
+	val := obj.(map[string]*api.AgentService)
-	if len(val) != 1 {
+	assert.Lenf(t, val, 1, "bad services: %v", obj)
-		t.Fatalf("bad services: %v", obj)
+	assert.Equal(t, 5000, val["mysql"].Port)
-	}
+	assert.NotNil(t, val["mysql"].Connect)
-	if val["mysql"].Port != 5000 {
+	assert.NotNil(t, val["mysql"].Connect.Proxy)
-		t.Fatalf("bad service: %v", obj)
+	assert.Equal(t, prxy1.ExecMode.String(), string(val["mysql"].Connect.Proxy.ExecMode))
-	}
+	assert.Equal(t, prxy1.Command, val["mysql"].Connect.Proxy.Command)
+	assert.Equal(t, prxy1.Config, val["mysql"].Connect.Proxy.Config)
 }
 
 // This tests that the agent services endpoint (/v1/agent/services) returns
 // Connect proxies.
-func TestAgent_Services_ConnectProxy(t *testing.T) {
+func TestAgent_Services_ExternalConnectProxy(t *testing.T) {
 	t.Parallel()
 
 	assert := assert.New(t)
@@ -94,10 +108,10 @@ func TestAgent_Services_ConnectProxy(t *testing.T) {
 	req, _ := http.NewRequest("GET", "/v1/agent/services", nil)
 	obj, err := a.srv.AgentServices(nil, req)
 	assert.Nil(err)
-	val := obj.(map[string]*structs.NodeService)
+	val := obj.(map[string]*api.AgentService)
 	assert.Len(val, 1)
 	actual := val["db-proxy"]
-	assert.Equal(structs.ServiceKindConnectProxy, actual.Kind)
+	assert.Equal(api.ServiceKindConnectProxy, actual.Kind)
 	assert.Equal("db", actual.ProxyDestination)
 }
 
@@ -120,7 +134,7 @@ func TestAgent_Services_ACLFilter(t *testing.T) {
 		if err != nil {
 			t.Fatalf("Err: %v", err)
 		}
-		val := obj.(map[string]*structs.NodeService)
+		val := obj.(map[string]*api.AgentService)
 		if len(val) != 0 {
 			t.Fatalf("bad: %v", obj)
 		}
@@ -132,7 +146,7 @@ func TestAgent_Services_ACLFilter(t *testing.T) {
 		if err != nil {
 			t.Fatalf("Err: %v", err)
 		}
-		val := obj.(map[string]*structs.NodeService)
+		val := obj.(map[string]*api.AgentService)
 		if len(val) != 1 {
 			t.Fatalf("bad: %v", obj)
 		}
@@ -1383,21 +1397,11 @@ func TestAgent_RegisterService_ManagedConnectProxy(t *testing.T) {
 	// 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{
+	args := &api.AgentServiceRegistration{
 		Name: "web",
 		Port: 8000,
-		// This is needed just because empty check struct (not pointer) get json
+		Connect: &api.AgentServiceConnect{
-		// encoded as object with zero values and then decoded back to object with
+			Proxy: &api.AgentServiceConnectProxy{
-		// 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{}{
@@ -2233,7 +2237,7 @@ func TestAgentConnectProxy(t *testing.T) {
 		},
 	}
 
-	expectedResponse := &structs.ConnectManageProxyResponse{
+	expectedResponse := &api.ConnectProxyConfig{
 		ProxyServiceID:    "test-proxy",
 		TargetServiceID:   "test",
 		TargetServiceName: "test",
@@ -2254,7 +2258,7 @@ func TestAgentConnectProxy(t *testing.T) {
 
 	ur, err := copystructure.Copy(expectedResponse)
 	require.NoError(t, err)
-	updatedResponse := ur.(*structs.ConnectManageProxyResponse)
+	updatedResponse := ur.(*api.ConnectProxyConfig)
 	updatedResponse.ContentHash = "22bc9233a52c08fd"
 	upstreams := updatedResponse.Config["upstreams"].([]interface{})
 	upstreams = append(upstreams,
@@ -2271,7 +2275,7 @@ func TestAgentConnectProxy(t *testing.T) {
 		wantWait   time.Duration
 		wantCode   int
 		wantErr    bool
-		wantResp   *structs.ConnectManageProxyResponse
+		wantResp   *api.ConnectProxyConfig
 	}{
 		{
 			name:     "simple fetch",
@@ -2338,7 +2342,7 @@ func TestAgentConnectProxy(t *testing.T) {
 				go tt.updateFunc()
 			}
 			start := time.Now()
-			obj, err := a.srv.AgentConnectProxyConfig(resp, req)
+			obj, err := a.srv.ConnectProxyConfig(resp, req)
 			elapsed := time.Now().Sub(start)
 
 			if tt.wantErr {

agent/structs/connect.go

@@ -103,16 +103,3 @@ func (p *ConnectManagedProxy) ParseConfig() (*ConnectManagedProxyConfig, error)
 	}
 	return &cfg, nil
 }
-
-// ConnectManageProxyResponse is the public response object we return for
-// queries on local proxy config state. It's similar to ConnectManagedProxy but
-// with some fields re-arranged.
-type ConnectManageProxyResponse struct {
-	ProxyServiceID    string
-	TargetServiceID   string
-	TargetServiceName string
-	ContentHash       string
-	ExecMode          string
-	Command           string
-	Config            map[string]interface{}
-}

agent/structs/service_definition.go

@@ -102,14 +102,14 @@ func (s *ServiceDefinition) CheckTypes() (checks CheckTypes, err error) {
 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"`
+	Proxy *ServiceDefinitionConnectProxy
 }
 
 // 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"`
+	Command  string
-	ExecMode string                 `json:"exec_mode,omitempty" hcl:"exec_mode" mapstructure:"exec_mode"`
+	ExecMode string
-	Config   map[string]interface{} `json:"config,omitempty" hcl:"config" mapstructure:"config"`
+	Config   map[string]interface{}
 }

api/agent.go

@@ -21,6 +21,23 @@ const (
 	ServiceKindConnectProxy ServiceKind = "connect-proxy"
 )
 
+// ProxyExecMode is the execution mode for a managed Connect proxy.
+type ProxyExecMode string
+
+const (
+	// ProxyExecModeDaemon indicates that the proxy command should be long-running
+	// and should be started and supervised by the agent until it's target service
+	// is deregistered.
+	ProxyExecModeDaemon ProxyExecMode = "daemon"
+
+	// ProxyExecModeScript indicates that the proxy command should be invoke to
+	// completion on each change to the configuration of lifecycle event. The
+	// script typically fetches the config and certificates from the agent API and
+	// then configures an externally managed daemon, perhaps starting and stopping
+	// it if necessary.
+	ProxyExecModeScript ProxyExecMode = "script"
+)
+
 // AgentCheck represents a check known to the agent
 type AgentCheck struct {
 	Node        string
@@ -47,6 +64,20 @@ type AgentService struct {
 	CreateIndex       uint64
 	ModifyIndex       uint64
 	ProxyDestination  string
+	Connect           *AgentServiceConnect
+}
+
+// AgentServiceConnect represents the Connect configuration of a service.
+type AgentServiceConnect struct {
+	Proxy *AgentServiceConnectProxy
+}
+
+// AgentServiceConnectProxy represents the Connect Proxy configuration of a
+// service.
+type AgentServiceConnectProxy struct {
+	ExecMode ProxyExecMode
+	Command  string
+	Config   map[string]interface{}
 }
 
 // AgentMember represents a cluster member known to the agent
@@ -90,6 +121,7 @@ type AgentServiceRegistration struct {
 	Check             *AgentServiceCheck
 	Checks            AgentServiceChecks
 	ProxyDestination  string               `json:",omitempty"`
+	Connect           *AgentServiceConnect `json:",omitempty"`
 }
 
 // AgentCheckRegistration is used to register a new check
@@ -185,6 +217,18 @@ type AgentAuthorize struct {
 	Reason     string
 }
 
+// ConnectProxyConfig is the response structure for agent-local proxy
+// configuration.
+type ConnectProxyConfig struct {
+	ProxyServiceID    string
+	TargetServiceID   string
+	TargetServiceName string
+	ContentHash       string
+	ExecMode          ProxyExecMode
+	Command           string
+	Config            map[string]interface{}
+}
+
 // Agent can be used to query the Agent endpoints
 type Agent struct {
 	c *Client
@@ -286,6 +330,7 @@ func (a *Agent) Services() (map[string]*AgentService, error) {
 	if err := decodeBody(resp, &out); err != nil {
 		return nil, err
 	}
+
 	return out, nil
 }
 
@@ -587,6 +632,31 @@ func (a *Agent) ConnectCALeaf(serviceID string, q *QueryOptions) (*LeafCert, *Qu
 	return &out, qm, nil
 }
 
+// ConnectProxyConfig gets the configuration for a local managed proxy instance.
+//
+// Note that this uses an unconventional blocking mechanism since it's
+// agent-local state. That means there is no persistent raft index so we block
+// based on object hash instead.
+func (a *Agent) ConnectProxyConfig(proxyServiceID string, q *QueryOptions) (*ConnectProxyConfig, *QueryMeta, error) {
+	r := a.c.newRequest("GET", "/v1/agent/connect/proxy/"+proxyServiceID)
+	r.setQueryOptions(q)
+	rtt, resp, err := requireOK(a.c.doRequest(r))
+	if err != nil {
+		return nil, nil, err
+	}
+	defer resp.Body.Close()
+
+	qm := &QueryMeta{}
+	parseQueryMeta(resp, qm)
+	qm.RequestTime = rtt
+
+	var out ConnectProxyConfig
+	if err := decodeBody(resp, &out); err != nil {
+		return nil, nil, err
+	}
+	return &out, qm, nil
+}
+
 // EnableServiceMaintenance toggles service maintenance mode on
 // for the given service ID.
 func (a *Agent) EnableServiceMaintenance(serviceID, reason string) error {

api/agent_test.go

@@ -186,7 +186,64 @@ func TestAPI_AgentServices(t *testing.T) {
 	}
 }
 
-func TestAPI_AgentServices_ConnectProxy(t *testing.T) {
+func TestAPI_AgentServices_ManagedConnectProxy(t *testing.T) {
+	t.Parallel()
+	c, s := makeClient(t)
+	defer s.Stop()
+
+	agent := c.Agent()
+
+	reg := &AgentServiceRegistration{
+		Name: "foo",
+		Tags: []string{"bar", "baz"},
+		Port: 8000,
+		Check: &AgentServiceCheck{
+			TTL: "15s",
+		},
+		Connect: &AgentServiceConnect{
+			Proxy: &AgentServiceConnectProxy{
+				ExecMode: ProxyExecModeScript,
+				Command:  "foo.rb",
+				Config: map[string]interface{}{
+					"foo": "bar",
+				},
+			},
+		},
+	}
+	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)
+	}
+	checks, err := agent.Checks()
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	chk, ok := checks["service:foo"]
+	if !ok {
+		t.Fatalf("missing check: %v", checks)
+	}
+
+	// Checks should default to critical
+	if chk.Status != HealthCritical {
+		t.Fatalf("Bad: %#v", chk)
+	}
+
+	// Proxy config should be present in response
+	require.Equal(t, reg.Connect, services["foo"].Connect)
+
+	if err := agent.ServiceDeregister("foo"); err != nil {
+		t.Fatalf("err: %v", err)
+	}
+}
+
+func TestAPI_AgentServices_ExternalConnectProxy(t *testing.T) {
 	t.Parallel()
 	c, s := makeClient(t)
 	defer s.Stop()
@@ -1019,3 +1076,31 @@ func TestAPI_AgentConnectAuthorize(t *testing.T) {
 	require.True(auth.Authorized)
 	require.Equal(auth.Reason, "ACLs disabled, access is allowed by default")
 }
+
+func TestAPI_AgentConnectProxyConfig(t *testing.T) {
+	t.Parallel()
+	c, s := makeClient(t)
+	defer s.Stop()
+
+	agent := c.Agent()
+	reg := &AgentServiceRegistration{
+		Name: "foo",
+		Tags: []string{"bar", "baz"},
+		Port: 8000,
+		Check: &AgentServiceCheck{
+			CheckID: "foo-ttl",
+			TTL:     "15s",
+		},
+	}
+	if err := agent.ServiceRegister(reg); err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	checks, err := agent.Checks()
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	if _, ok := checks["foo-ttl"]; !ok {
+		t.Fatalf("missing check: %v", checks)
+	}
+}

api/api.go

@@ -82,6 +82,12 @@ type QueryOptions struct {
 	// until the timeout or the next index is reached
 	WaitIndex uint64
 
+	// WaitHash is used by some endpoints instead of WaitIndex to perform blocking
+	// on state based on a hash of the response rather than a monotonic index.
+	// This is required when the state being blocked on is not stored in Raft, for
+	// example agent-local proxy configuration.
+	WaitHash string
+
 	// WaitTime is used to bound the duration of a wait.
 	// Defaults to that of the Config, but can be overridden.
 	WaitTime time.Duration
@@ -533,6 +539,9 @@ func (r *request) setQueryOptions(q *QueryOptions) {
 	if q.WaitTime != 0 {
 		r.params.Set("wait", durToMsec(q.WaitTime))
 	}
+	if q.WaitHash != "" {
+		r.params.Set("hash", q.WaitHash)
+	}
 	if q.Token != "" {
 		r.header.Set("X-Consul-Token", q.Token)
 	}