diff --git a/command/agent/acl_endpoint.go b/command/agent/acl_endpoint.go index 72ae4c20af..3bc054611b 100644 --- a/command/agent/acl_endpoint.go +++ b/command/agent/acl_endpoint.go @@ -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 } diff --git a/command/agent/acl_endpoint_test.go b/command/agent/acl_endpoint_test.go index b0dea0c9ba..361d661ac6 100644 --- a/command/agent/acl_endpoint_test.go +++ b/command/agent/acl_endpoint_test.go @@ -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) diff --git a/command/agent/catalog_endpoint.go b/command/agent/catalog_endpoint.go index 447822a875..3a5bd98134 100644 --- a/command/agent/catalog_endpoint.go +++ b/command/agent/catalog_endpoint.go @@ -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 } diff --git a/command/agent/catalog_endpoint_test.go b/command/agent/catalog_endpoint_test.go index 5e4d75aab3..251d288df0 100644 --- a/command/agent/catalog_endpoint_test.go +++ b/command/agent/catalog_endpoint_test.go @@ -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", diff --git a/command/agent/coordinate_endpoint.go b/command/agent/coordinate_endpoint.go index 0cc5824cec..92453225d7 100644 --- a/command/agent/coordinate_endpoint.go +++ b/command/agent/coordinate_endpoint.go @@ -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 } diff --git a/command/agent/coordinate_endpoint_test.go b/command/agent/coordinate_endpoint_test.go index aee7bc18cd..8da0a5b397 100644 --- a/command/agent/coordinate_endpoint_test.go +++ b/command/agent/coordinate_endpoint_test.go @@ -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" { diff --git a/command/agent/health_endpoint.go b/command/agent/health_endpoint.go index f95e4c5e72..e4a36f6a6a 100644 --- a/command/agent/health_endpoint.go +++ b/command/agent/health_endpoint.go @@ -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 } diff --git a/command/agent/health_endpoint_test.go b/command/agent/health_endpoint_test.go index 317203065b..02330a37c0 100644 --- a/command/agent/health_endpoint_test.go +++ b/command/agent/health_endpoint_test.go @@ -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) { diff --git a/command/agent/session_endpoint.go b/command/agent/session_endpoint.go index f3d8db22bc..0f2685465f 100644 --- a/command/agent/session_endpoint.go +++ b/command/agent/session_endpoint.go @@ -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 } diff --git a/command/agent/session_endpoint_test.go b/command/agent/session_endpoint_test.go index 960fdeec18..c7f744c84a 100644 --- a/command/agent/session_endpoint_test.go +++ b/command/agent/session_endpoint_test.go @@ -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++ { diff --git a/command/agent/ui_endpoint.go b/command/agent/ui_endpoint.go index 425e392ce9..c240879876 100644 --- a/command/agent/ui_endpoint.go +++ b/command/agent/ui_endpoint.go @@ -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 } diff --git a/command/agent/ui_endpoint_test.go b/command/agent/ui_endpoint_test.go index 76e3f379ba..c37d7c0214 100644 --- a/command/agent/ui_endpoint_test.go +++ b/command/agent/ui_endpoint_test.go @@ -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) {