status-im
/
consul
mirror of https://github.com/status-im/consul.git
2
0
Fork 0
Code Issues Projects Releases Wiki Activity

Fixes nil slices from HTTP endpoints. 

These would manifest in the HTTP output as Javascript nulls instead of
empty lists, so we had unintentionally changed the interface while
porting to the new state store. We added code to each HTTP endpoint to
convert nil slices to empty ones so they JSON-ify properly, and we added
tests to catch this in the future.
This commit is contained in:
James Phillips 2015-11-14 21:05:37 -08:00
parent 86f56ee4a1
commit c248b0017a
12 changed files with 398 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
command/agent/acl_endpoint.go
View File

@ -176,6 +176,11 @@ func (s *HTTPServer) ACLGet(resp http.ResponseWriter, req *http.Request) (interf
if err := s.agent.RPC("ACL.Get", &args, &out); err != nil {
return nil, err
}
// Use empty list instead of nil
if out.ACLs == nil {
out.ACLs = make(structs.ACLs, 0)
}
return out.ACLs, nil
}
@ -193,5 +198,10 @@ func (s *HTTPServer) ACLList(resp http.ResponseWriter, req *http.Request) (inter
if err := s.agent.RPC("ACL.List", &args, &out); err != nil {
return nil, err
}
// Use empty list instead of nil
if out.ACLs == nil {
out.ACLs = make(structs.ACLs, 0)
}
return out.ACLs, nil
}

35
command/agent/acl_endpoint_test.go
View File

@ -94,15 +94,30 @@ func TestACLUpdate_Upsert(t *testing.T) {
func TestACLDestroy(t *testing.T) {
httpTest(t, func(srv *HTTPServer) {
id := makeTestACL(t, srv)
req, err := http.NewRequest("PUT", "/v1/session/destroy/"+id+"?token=root", nil)
req, err := http.NewRequest("PUT", "/v1/acl/destroy/"+id+"?token=root", nil)
resp := httptest.NewRecorder()
obj, err := srv.ACLDestroy(resp, req)
if err != nil {
t.Fatalf("err: %v", err)
}
if resp := obj.(bool); !resp {
if resp, ok := obj.(bool); !ok || !resp {
t.Fatalf("should work")
}
req, err = http.NewRequest("GET",
"/v1/acl/info/"+id, nil)
resp = httptest.NewRecorder()
obj, err = srv.ACLGet(resp, req)
if err != nil {
t.Fatalf("err: %v", err)
}
respObj, ok := obj.(structs.ACLs)
if !ok {
t.Fatalf("should work")
}
if len(respObj) != 0 {
t.Fatalf("bad: %v", respObj)
}
})
}
@ -143,6 +158,22 @@ func TestACLClone(t *testing.T) {
}
func TestACLGet(t *testing.T) {
httpTest(t, func(srv *HTTPServer) {
req, err := http.NewRequest("GET", "/v1/acl/info/nope", nil)
resp := httptest.NewRecorder()
obj, err := srv.ACLGet(resp, req)
if err != nil {
t.Fatalf("err: %v", err)
}
respObj, ok := obj.(structs.ACLs)
if !ok {
t.Fatalf("should work")
}
if respObj == nil || len(respObj) != 0 {
t.Fatalf("bad: %v", respObj)
}
})
httpTest(t, func(srv *HTTPServer) {
id := makeTestACL(t, srv)

10
command/agent/catalog_endpoint.go
View File

@ -70,6 +70,11 @@ func (s *HTTPServer) CatalogNodes(resp http.ResponseWriter, req *http.Request) (
if err := s.agent.RPC("Catalog.ListNodes", &args, &out); err != nil {
return nil, err
}
// Use empty list instead of nil
if out.Nodes == nil {
out.Nodes = make(structs.Nodes, 0)
}
return out.Nodes, nil
}
@ -117,6 +122,11 @@ func (s *HTTPServer) CatalogServiceNodes(resp http.ResponseWriter, req *http.Req
if err := s.agent.RPC("Catalog.ServiceNodes", &args, &out); err != nil {
return nil, err
}
// Use empty list instead of nil
if out.ServiceNodes == nil {
out.ServiceNodes = make(structs.ServiceNodes, 0)
}
return out.ServiceNodes, nil
}

21
command/agent/catalog_endpoint_test.go
View File

@ -351,6 +351,27 @@ func TestCatalogServiceNodes(t *testing.T) {
testutil.WaitForLeader(t, srv.agent.RPC, "dc1")
// Make sure an empty list is returned, not a nil
{
req, err := http.NewRequest("GET", "/v1/catalog/service/api?tag=a", nil)
if err != nil {
t.Fatalf("err: %v", err)
}
resp := httptest.NewRecorder()
obj, err := srv.CatalogServiceNodes(resp, req)
if err != nil {
t.Fatalf("err: %v", err)
}
assertIndex(t, resp)
nodes := obj.(structs.ServiceNodes)
if nodes == nil || len(nodes) != 0 {
t.Fatalf("bad: %v", obj)
}
}
// Register node
args := &structs.RegisterRequest{
Datacenter: "dc1",

17
command/agent/coordinate_endpoint.go
View File

@ -45,6 +45,18 @@ func (s *HTTPServer) CoordinateDatacenters(resp http.ResponseWriter, req *http.R
}
return nil, err
}
// Use empty list instead of nil (these aren't really possible because
// Serf will give back a default coordinate and there's always one DC,
// but it's better to be explicit about what we want here).
for i, _ := range out {
if out[i].Coordinates == nil {
out[i].Coordinates = make(structs.Coordinates, 0)
}
}
if out == nil {
out = make([]structs.DatacenterMap, 0)
}
return out, nil
}
@ -62,5 +74,10 @@ func (s *HTTPServer) CoordinateNodes(resp http.ResponseWriter, req *http.Request
sort.Sort(&sorter{out.Coordinates})
return nil, err
}
// Use empty list instead of nil.
if out.Coordinates == nil {
out.Coordinates = make(structs.Coordinates, 0)
}
return out.Coordinates, nil
}

25
command/agent/coordinate_endpoint_test.go
View File

@ -48,6 +48,23 @@ func TestCoordinate_Nodes(t *testing.T) {
testutil.WaitForLeader(t, srv.agent.RPC, "dc1")
// Make sure an empty list is non-nil.
req, err := http.NewRequest("GET", "/v1/coordinate/nodes?dc=dc1", nil)
if err != nil {
t.Fatalf("err: %v", err)
}
resp := httptest.NewRecorder()
obj, err := srv.CoordinateNodes(resp, req)
if err != nil {
t.Fatalf("err: %v", err)
}
coordinates := obj.(structs.Coordinates)
if coordinates == nil || len(coordinates) != 0 {
t.Fatalf("bad: %v", coordinates)
}
// Register the nodes.
nodes := []string{"foo", "bar"}
for _, node := range nodes {
@ -85,18 +102,18 @@ func TestCoordinate_Nodes(t *testing.T) {
time.Sleep(200 * time.Millisecond)
// Query back and check the nodes are present and sorted correctly.
req, err := http.NewRequest("GET", "/v1/coordinate/nodes?dc=dc1", nil)
req, err = http.NewRequest("GET", "/v1/coordinate/nodes?dc=dc1", nil)
if err != nil {
t.Fatalf("err: %v", err)
}
resp := httptest.NewRecorder()
obj, err := srv.CoordinateNodes(resp, req)
resp = httptest.NewRecorder()
obj, err = srv.CoordinateNodes(resp, req)
if err != nil {
t.Fatalf("err: %v", err)
}
coordinates := obj.(structs.Coordinates)
coordinates = obj.(structs.Coordinates)
if len(coordinates) != 2 ||
coordinates[0].Node != "bar" ||
coordinates[1].Node != "foo" {

28
command/agent/health_endpoint.go
View File

@ -28,6 +28,11 @@ func (s *HTTPServer) HealthChecksInState(resp http.ResponseWriter, req *http.Req
if err := s.agent.RPC("Health.ChecksInState", &args, &out); err != nil {
return nil, err
}
// Use empty list instead of nil
if out.HealthChecks == nil {
out.HealthChecks = make(structs.HealthChecks, 0)
}
return out.HealthChecks, nil
}
@ -52,6 +57,11 @@ func (s *HTTPServer) HealthNodeChecks(resp http.ResponseWriter, req *http.Reques
if err := s.agent.RPC("Health.NodeChecks", &args, &out); err != nil {
return nil, err
}
// Use empty list instead of nil
if out.HealthChecks == nil {
out.HealthChecks = make(structs.HealthChecks, 0)
}
return out.HealthChecks, nil
}
@ -77,6 +87,11 @@ func (s *HTTPServer) HealthServiceChecks(resp http.ResponseWriter, req *http.Req
if err := s.agent.RPC("Health.ServiceChecks", &args, &out); err != nil {
return nil, err
}
// Use empty list instead of nil
if out.HealthChecks == nil {
out.HealthChecks = make(structs.HealthChecks, 0)
}
return out.HealthChecks, nil
}
@ -114,6 +129,19 @@ func (s *HTTPServer) HealthServiceNodes(resp http.ResponseWriter, req *http.Requ
if _, ok := params["passing"]; ok {
out.Nodes = filterNonPassing(out.Nodes)
}
// Use empty list instead of nil
for i, _ := range out.Nodes {
// TODO (slackpad) It's lame that this isn't a slice of pointers
// but it's not a well-scoped change to fix this. We should
// change this at the next opportunity.
if out.Nodes[i].Checks == nil {
out.Nodes[i].Checks = make(structs.HealthChecks, 0)
}
}
if out.Nodes == nil {
out.Nodes = make(structs.CheckServiceNodes, 0)
}
return out.Nodes, nil
}

130
command/agent/health_endpoint_test.go
View File

@ -15,6 +15,31 @@ import (
)
func TestHealthChecksInState(t *testing.T) {
httpTest(t, func(srv *HTTPServer) {
req, err := http.NewRequest("GET", "/v1/health/state/warning?dc=dc1", nil)
if err != nil {
t.Fatalf("err: %v", err)
}
testutil.WaitForResult(func() (bool, error) {
resp := httptest.NewRecorder()
obj, err := srv.HealthChecksInState(resp, req)
if err != nil {
return false, err
}
if err := checkIndex(resp); err != nil {
return false, err
}
// Should be a non-nil empty list
nodes := obj.(structs.HealthChecks)
if nodes == nil || len(nodes) != 0 {
return false, fmt.Errorf("bad: %v", obj)
}
return true, nil
}, func(err error) { t.Fatalf("err: %v", err) })
})
httpTest(t, func(srv *HTTPServer) {
req, err := http.NewRequest("GET", "/v1/health/state/passing?dc=dc1", nil)
if err != nil {
@ -130,8 +155,7 @@ func TestHealthNodeChecks(t *testing.T) {
testutil.WaitForLeader(t, srv.agent.RPC, "dc1")
req, err := http.NewRequest("GET",
fmt.Sprintf("/v1/health/node/%s?dc=dc1", srv.agent.config.NodeName), nil)
req, err := http.NewRequest("GET", "/v1/health/node/nope?dc=dc1", nil)
if err != nil {
t.Fatalf("err: %v", err)
}
@ -143,8 +167,27 @@ func TestHealthNodeChecks(t *testing.T) {
}
assertIndex(t, resp)
// Should be 1 health check for the server
// Should be a non-nil empty list
nodes := obj.(structs.HealthChecks)
if nodes == nil || len(nodes) != 0 {
t.Fatalf("bad: %v", obj)
}
req, err = http.NewRequest("GET",
fmt.Sprintf("/v1/health/node/%s?dc=dc1", srv.agent.config.NodeName), nil)
if err != nil {
t.Fatalf("err: %v", err)
}
resp = httptest.NewRecorder()
obj, err = srv.HealthNodeChecks(resp, req)
if err != nil {
t.Fatalf("err: %v", err)
}
assertIndex(t, resp)
// Should be 1 health check for the server
nodes = obj.(structs.HealthChecks)
if len(nodes) != 1 {
t.Fatalf("bad: %v", obj)
}
@ -158,6 +201,24 @@ func TestHealthServiceChecks(t *testing.T) {
testutil.WaitForLeader(t, srv.agent.RPC, "dc1")
req, err := http.NewRequest("GET", "/v1/health/checks/consul?dc=dc1", nil)
if err != nil {
t.Fatalf("err: %v", err)
}
resp := httptest.NewRecorder()
obj, err := srv.HealthServiceChecks(resp, req)
if err != nil {
t.Fatalf("err: %v", err)
}
assertIndex(t, resp)
// Should be a non-nil empty list
nodes := obj.(structs.HealthChecks)
if nodes == nil || len(nodes) != 0 {
t.Fatalf("bad: %v", obj)
}
// Create a service check
args := &structs.RegisterRequest{
Datacenter: "dc1",
@ -171,24 +232,24 @@ func TestHealthServiceChecks(t *testing.T) {
}
var out struct{}
if err := srv.agent.RPC("Catalog.Register", args, &out); err != nil {
if err = srv.agent.RPC("Catalog.Register", args, &out); err != nil {
t.Fatalf("err: %v", err)
}
req, err := http.NewRequest("GET", "/v1/health/checks/consul?dc=dc1", nil)
req, err = http.NewRequest("GET", "/v1/health/checks/consul?dc=dc1", nil)
if err != nil {
t.Fatalf("err: %v", err)
}
resp := httptest.NewRecorder()
obj, err := srv.HealthServiceChecks(resp, req)
resp = httptest.NewRecorder()
obj, err = srv.HealthServiceChecks(resp, req)
if err != nil {
t.Fatalf("err: %v", err)
}
assertIndex(t, resp)
// Should be 1 health check for consul
nodes := obj.(structs.HealthChecks)
nodes = obj.(structs.HealthChecks)
if len(nodes) != 1 {
t.Fatalf("bad: %v", obj)
}
@ -306,6 +367,59 @@ func TestHealthServiceNodes(t *testing.T) {
if len(nodes) != 1 {
t.Fatalf("bad: %v", obj)
}
req, err = http.NewRequest("GET", "/v1/health/service/nope?dc=dc1", nil)
if err != nil {
t.Fatalf("err: %v", err)
}
resp = httptest.NewRecorder()
obj, err = srv.HealthServiceNodes(resp, req)
if err != nil {
t.Fatalf("err: %v", err)
}
assertIndex(t, resp)
// Should be a non-nil empty list
nodes = obj.(structs.CheckServiceNodes)
if nodes == nil || len(nodes) != 0 {
t.Fatalf("bad: %v", obj)
}
args := &structs.RegisterRequest{
Datacenter: "dc1",
Node: "bar",
Address: "127.0.0.1",
Service: &structs.NodeService{
ID: "test",
Service: "test",
},
}
var out struct{}
if err := srv.agent.RPC("Catalog.Register", args, &out); err != nil {
t.Fatalf("err: %v", err)
}
req, err = http.NewRequest("GET", "/v1/health/service/test?dc=dc1", nil)
if err != nil {
t.Fatalf("err: %v", err)
}
resp = httptest.NewRecorder()
obj, err = srv.HealthServiceNodes(resp, req)
if err != nil {
t.Fatalf("err: %v", err)
}
assertIndex(t, resp)
// Should be a non-nil empty list for checks
nodes = obj.(structs.CheckServiceNodes)
if len(nodes) != 1 || nodes[0].Checks == nil || len(nodes[0].Checks) != 0 {
t.Fatalf("bad: %v", obj)
}
}
func TestHealthServiceNodes_DistanceSort(t *testing.T) {

15
command/agent/session_endpoint.go
View File

@ -185,6 +185,11 @@ func (s *HTTPServer) SessionGet(resp http.ResponseWriter, req *http.Request) (in
if err := s.agent.RPC("Session.Get", &args, &out); err != nil {
return nil, err
}
// Use empty list instead of nil
if out.Sessions == nil {
out.Sessions = make(structs.Sessions, 0)
}
return out.Sessions, nil
}
@ -200,6 +205,11 @@ func (s *HTTPServer) SessionList(resp http.ResponseWriter, req *http.Request) (i
if err := s.agent.RPC("Session.List", &args, &out); err != nil {
return nil, err
}
// Use empty list instead of nil
if out.Sessions == nil {
out.Sessions = make(structs.Sessions, 0)
}
return out.Sessions, nil
}
@ -223,5 +233,10 @@ func (s *HTTPServer) SessionsForNode(resp http.ResponseWriter, req *http.Request
if err := s.agent.RPC("Session.NodeSessions", &args, &out); err != nil {
return nil, err
}
// Use empty list instead of nil
if out.Sessions == nil {
out.Sessions = make(structs.Sessions, 0)
}
return out.Sessions, nil
}

49
command/agent/session_endpoint_test.go
View File

@ -349,6 +349,22 @@ func TestSessionTTLRenew(t *testing.T) {
}
func TestSessionGet(t *testing.T) {
httpTest(t, func(srv *HTTPServer) {
req, err := http.NewRequest("GET", "/v1/session/info/adf4238a-882b-9ddc-4a9d-5b6758e4159e", nil)
resp := httptest.NewRecorder()
obj, err := srv.SessionGet(resp, req)
if err != nil {
t.Fatalf("err: %v", err)
}
respObj, ok := obj.(structs.Sessions)
if !ok {
t.Fatalf("should work")
}
if respObj == nil || len(respObj) != 0 {
t.Fatalf("bad: %v", respObj)
}
})
httpTest(t, func(srv *HTTPServer) {
id := makeTestSession(t, srv)
@ -370,6 +386,22 @@ func TestSessionGet(t *testing.T) {
}
func TestSessionList(t *testing.T) {
httpTest(t, func(srv *HTTPServer) {
req, err := http.NewRequest("GET", "/v1/session/list", nil)
resp := httptest.NewRecorder()
obj, err := srv.SessionList(resp, req)
if err != nil {
t.Fatalf("err: %v", err)
}
respObj, ok := obj.(structs.Sessions)
if !ok {
t.Fatalf("should work")
}
if respObj == nil || len(respObj) != 0 {
t.Fatalf("bad: %v", respObj)
}
})
httpTest(t, func(srv *HTTPServer) {
var ids []string
for i := 0; i < 10; i++ {
@ -393,6 +425,23 @@ func TestSessionList(t *testing.T) {
}
func TestSessionsForNode(t *testing.T) {
httpTest(t, func(srv *HTTPServer) {
req, err := http.NewRequest("GET",
"/v1/session/node/"+srv.agent.config.NodeName, nil)
resp := httptest.NewRecorder()
obj, err := srv.SessionsForNode(resp, req)
if err != nil {
t.Fatalf("err: %v", err)
}
respObj, ok := obj.(structs.Sessions)
if !ok {
t.Fatalf("should work")
}
if respObj == nil || len(respObj) != 0 {
t.Fatalf("bad: %v", respObj)
}
})
httpTest(t, func(srv *HTTPServer) {
var ids []string
for i := 0; i < 10; i++ {

22
command/agent/ui_endpoint.go
View File

@ -38,6 +38,19 @@ RPC:
}
return nil, err
}
// Use empty list instead of nil
for _, info := range out.Dump {
if info.Services == nil {
info.Services = make([]*structs.NodeService, 0)
}
if info.Checks == nil {
info.Checks = make([]*structs.HealthCheck, 0)
}
}
if out.Dump == nil {
out.Dump = make(structs.NodeDump, 0)
}
return out.Dump, nil
}
@ -73,7 +86,14 @@ RPC:
// Return only the first entry
if len(out.Dump) > 0 {
return out.Dump[0], nil
info := out.Dump[0]
if info.Services == nil {
info.Services = make([]*structs.NodeService, 0)
}
if info.Checks == nil {
info.Checks = make([]*structs.HealthCheck, 0)
}
return info, nil
}
return nil, nil
}

53
command/agent/ui_endpoint_test.go
View File

@ -65,6 +65,17 @@ func TestUiNodes(t *testing.T) {
testutil.WaitForLeader(t, srv.agent.RPC, "dc1")
args := &structs.RegisterRequest{
Datacenter: "dc1",
Node: "test",
Address: "127.0.0.1",
}
var out struct{}
if err := srv.agent.RPC("Catalog.Register", args, &out); err != nil {
t.Fatalf("err: %v", err)
}
req, err := http.NewRequest("GET", "/v1/internal/ui/nodes/dc1", nil)
if err != nil {
t.Fatalf("err: %v", err)
@ -77,9 +88,15 @@ func TestUiNodes(t *testing.T) {
}
assertIndex(t, resp)
// Should be 1 node for the server
// Should be 2 nodes, and all the empty lists should be non-nil
nodes := obj.(structs.NodeDump)
if len(nodes) != 1 {
if len(nodes) != 2 ||
nodes[0].Node != srv.agent.config.NodeName ||
nodes[0].Services == nil || len(nodes[0].Services) != 1 ||
nodes[0].Checks == nil || len(nodes[0].Checks) != 1 ||
nodes[1].Node != "test" ||
nodes[1].Services == nil || len(nodes[1].Services) != 0 ||
nodes[1].Checks == nil || len(nodes[1].Checks) != 0 {
t.Fatalf("bad: %v", obj)
}
}
@ -111,6 +128,38 @@ func TestUiNodeInfo(t *testing.T) {
if node.Node != srv.agent.config.NodeName {
t.Fatalf("bad: %v", node)
}
args := &structs.RegisterRequest{
Datacenter: "dc1",
Node: "test",
Address: "127.0.0.1",
}
var out struct{}
if err := srv.agent.RPC("Catalog.Register", args, &out); err != nil {
t.Fatalf("err: %v", err)
}
req, err = http.NewRequest("GET", "/v1/internal/ui/node/test", nil)
if err != nil {
t.Fatalf("err: %v", err)
}
resp = httptest.NewRecorder()
obj, err = srv.UINodeInfo(resp, req)
if err != nil {
t.Fatalf("err: %v", err)
}
assertIndex(t, resp)
// Should be non-nil empty lists for services and checks
node = obj.(*structs.NodeInfo)
if node.Node != "test" ||
node.Services == nil || len(node.Services) != 0 ||
node.Checks == nil || len(node.Checks) != 0 {
t.Fatalf("bad: %v", node)
}
}
func TestSummarizeServices(t *testing.T) {