luxolus
/
open-consul
2
0
Fork 0
Code Issues 1 Pull Requests Packages Releases Wiki Activity

Merge pull request #1414 from hashicorp/b-nil-slices 

Fixes nil slices leading to null fields in HTTP JSON responses
This commit is contained in:
James Phillips 2015-11-15 16:17:01 -08:00
parent 1a3c523816 763d5ea8a7
commit 10a1bc418c
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) {