fix(api-server): service-insights should never return items: null

Our api should never return null instead of an empty array for lists.
This fixes an issue in service-insights

Fix #6639

Signed-off-by: Charly Molter <charly.molter@konghq.com>

pkg/api-server/service_insight_endpoints.go

@@ -44,7 +44,8 @@ func (s *serviceInsightEndpoints) findResource(request *restful.Request, respons
 				Dataplanes: &v1alpha1.ServiceInsight_Service_DataplaneStat{},
 			}
 		}
-		res := rest_unversioned.From.Resource(serviceInsight)
+		out := rest.From.Resource(serviceInsight)
+		res := out.(*rest_unversioned.Resource)
 		res.Meta.Name = service
 		res.Spec = stat
 		if err := response.WriteAsJson(res); err != nil {
@@ -71,8 +72,11 @@ func (s *serviceInsightEndpoints) listResources(request *restful.Request, respon
 		return
 	}
 
-	restList := s.expandInsights(serviceInsightList)
-	restList.Total = uint32(len(restList.Items))
+	items := s.expandInsights(serviceInsightList)
+	restList := rest.ResourceList{
+		Total: uint32(len(items)),
+		Items: items,
+	}
 
 	if err := s.paginateResources(request, &restList); err != nil {
 		rest_errors.HandleError(response, err, "Could not paginate resources")
@@ -90,24 +94,27 @@ func (s *serviceInsightEndpoints) listResources(request *restful.Request, respon
 // 2) Mesh+Name is a key on Universal, but not on Kubernetes, so if there are two services of the same name in different Meshes we would have problems with naming.
 // From the API perspective it's better to provide ServiceInsight per Service, not per Mesh.
 // For this reason, this method expand the one ServiceInsight resource for the mesh to resource per service
-func (s *serviceInsightEndpoints) expandInsights(serviceInsightList *mesh.ServiceInsightResourceList) rest.ResourceList {
-	restItems := []*rest_unversioned.Resource{}
+func (s *serviceInsightEndpoints) expandInsights(serviceInsightList *mesh.ServiceInsightResourceList) []rest.Resource {
+	restItems := []rest.Resource{} // Needs to be set to avoid returning nil and have the api return []
 	for _, insight := range serviceInsightList.Items {
 		for serviceName, stat := range insight.Spec.Services {
-			res := rest_unversioned.From.Resource(insight)
+			out := rest.From.Resource(insight)
+			res := out.(*rest_unversioned.Resource)
 			res.Meta.Name = serviceName
 			res.Spec = stat
-			restItems = append(restItems, res)
+			restItems = append(restItems, out)
 		}
 	}
 
-	sort.Sort(rest_unversioned.ByMeta(restItems))
-
-	restList := rest.ResourceList{}
-	for _, item := range restItems {
-		restList.Items = append(restList.Items, item)
-	}
-	return restList
+	sort.Slice(restItems, func(i, j int) bool {
+		metai := restItems[i].GetMeta()
+		metaj := restItems[j].GetMeta()
+		if metai.Mesh == metaj.Mesh {
+			return metai.Name < metaj.Name
+		}
+		return metai.Mesh < metaj.Mesh
+	})
+	return restItems
 }
 
 // paginateResources paginates resources manually, because we are expanding resources.

pkg/api-server/service_insight_endpoints_test.go

@@ -38,6 +38,9 @@ var _ = Describe("Service Insight Endpoints", func() {
 
 		err = resourceStore.Create(context.Background(), core_mesh.NewMeshResource(), store.CreateByKey("mesh-2", core_model.NoMesh), store.CreatedAt(t1))
 		Expect(err).ToNot(HaveOccurred())
+
+		err = resourceStore.Create(context.Background(), core_mesh.NewMeshResource(), store.CreateByKey("no-services", core_model.NoMesh), store.CreatedAt(t1))
+		Expect(err).ToNot(HaveOccurred())
 	})
 
 	createServiceInsight := func(name, mesh string, serviceInsight *mesh_proto.ServiceInsight) {
@@ -69,7 +72,6 @@ var _ = Describe("Service Insight Endpoints", func() {
 				},
 			},
 		})
-
 		createServiceInsight("all-services-mesh-2", "mesh-2", &mesh_proto.ServiceInsight{
 			Services: map[string]*mesh_proto.ServiceInsight_Service{
 				"db": {
@@ -167,6 +169,25 @@ var _ = Describe("Service Insight Endpoints", func() {
 		})
 	})
 
+	It("should return empty entry (fix #6639)", func() {
+		expected := `
+{
+  "total": 0,
+  "items": [],
+  "next": null
+}`
+
+		// when
+		response, err := http.Get("http://" + apiServer.Address() + "/meshes/no-services/service-insights")
+		Expect(err).ToNot(HaveOccurred())
+
+		// then
+		Expect(response.StatusCode).To(Equal(200))
+		body, err := io.ReadAll(response.Body)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(body).To(MatchJSON(expected))
+	})
+
 	type testCase struct {
 		params   string
 		expected string

pkg/core/resources/model/rest/unversioned/resource.go

@@ -86,16 +86,3 @@ func (r *Resource) ToCore() (core_model.Resource, error) {
 	}
 	return resource, nil
 }
-
-type ByMeta []*Resource
-
-func (a ByMeta) Len() int { return len(a) }
-
-func (a ByMeta) Less(i, j int) bool {
-	if a[i].Meta.Mesh == a[j].Meta.Mesh {
-		return a[i].Meta.Name < a[j].Meta.Name
-	}
-	return a[i].Meta.Mesh < a[j].Meta.Mesh
-}
-
-func (a ByMeta) Swap(i, j int) { a[i], a[j] = a[j], a[i] }