From 36dbd878c93f1cdaa8f6f65d978ba21452dba8b4 Mon Sep 17 00:00:00 2001 From: Paul Banks Date: Fri, 20 Apr 2018 14:24:24 +0100 Subject: [PATCH] Adds `api` client code and tests for new Proxy Config endpoint, registering with proxy and seeing proxy config in /agent/services list. --- GNUmakefile | 12 ++-- agent/agent_endpoint.go | 57 +++++++++++++------ agent/agent_endpoint_test.go | 64 +++++++++++---------- agent/structs/connect.go | 13 ----- agent/structs/service_definition.go | 8 +-- api/agent.go | 72 +++++++++++++++++++++++- api/agent_test.go | 87 ++++++++++++++++++++++++++++- api/api.go | 9 +++ 8 files changed, 251 insertions(+), 71 deletions(-) diff --git a/GNUmakefile b/GNUmakefile index d77342892d..2c412d9e5a 100644 --- a/GNUmakefile +++ b/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. - @awk '/--- SKIP/ {do_print=1} /^[^[:space:]]/ {do_print=0} do_print==1 {print}' test.log || true - @awk '/--- FAIL/ {do_print=1} /^[^[:space:]]/ {do_print=0} do_print==1 {print}' test.log || true + @# helpers often output multiple lines for readability but useless if we can't + @# see them. Un-intuitive order of matches is necessary. No || true because + @# awk always returns true even if there is no match and it breaks non-bash + @# 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 diff --git a/agent/agent_endpoint.go b/agent/agent_endpoint.go index c19b776ac2..c1bf6fbe10 100644 --- a/agent/agent_endpoint.go +++ b/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 { - clone := *s - if s.Tags == nil { - clone.Tags = make([]string, 0) - } else { - clone.Tags = s.Tags - } - if s.Meta == nil { - clone.Meta = make(map[string]string) - } else { - clone.Meta = s.Meta - } - services[id] = &clone + as := &api.AgentService{ + Kind: api.ServiceKind(s.Kind), + ID: s.ID, + Service: s.Service, + Tags: s.Tags, + Port: s.Port, + Address: s.Address, + EnableTagOverride: s.EnableTagOverride, + CreateIndex: s.CreateIndex, + ModifyIndex: s.ModifyIndex, + ProxyDestination: s.ProxyDestination, } + if as.Tags == nil { + as.Tags = []string{} + } + 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, } diff --git a/agent/agent_endpoint_test.go b/agent/agent_endpoint_test.go index b34ac508ae..32cb6ab98f 100644 --- a/agent/agent_endpoint_test.go +++ b/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) - if len(val) != 1 { - t.Fatalf("bad services: %v", obj) - } - if val["mysql"].Port != 5000 { - t.Fatalf("bad service: %v", obj) - } + val := obj.(map[string]*api.AgentService) + assert.Lenf(t, val, 1, "bad services: %v", obj) + assert.Equal(t, 5000, val["mysql"].Port) + assert.NotNil(t, val["mysql"].Connect) + assert.NotNil(t, val["mysql"].Connect.Proxy) + 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 - // 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{ + Connect: &api.AgentServiceConnect{ + Proxy: &api.AgentServiceConnectProxy{ 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 { diff --git a/agent/structs/connect.go b/agent/structs/connect.go index d879718b22..5f907c1ab0 100644 --- a/agent/structs/connect.go +++ b/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{} -} diff --git a/agent/structs/service_definition.go b/agent/structs/service_definition.go index ad77d8e3b2..2ed4241781 100644 --- a/agent/structs/service_definition.go +++ b/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"` - ExecMode string `json:"exec_mode,omitempty" hcl:"exec_mode" mapstructure:"exec_mode"` - Config map[string]interface{} `json:"config,omitempty" hcl:"config" mapstructure:"config"` + Command string + ExecMode string + Config map[string]interface{} } diff --git a/api/agent.go b/api/agent.go index 6b662fa2c5..a81fd96f8c 100644 --- a/api/agent.go +++ b/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 @@ -89,7 +120,8 @@ type AgentServiceRegistration struct { Meta map[string]string `json:",omitempty"` Check *AgentServiceCheck Checks AgentServiceChecks - ProxyDestination string `json:",omitempty"` + 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 { diff --git a/api/agent_test.go b/api/agent_test.go index 6186bffe3c..01d35ae159 100644 --- a/api/agent_test.go +++ b/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) + } +} diff --git a/api/api.go b/api/api.go index 1cdc21e331..6f3034d90a 100644 --- a/api/api.go +++ b/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) }