Make host counts optional in "list labels" API (#35831)

<!-- Add the related story/sub-task/bug number, like Resolves #123, or
remove if NA -->
**Related issue:** Resolves #35376 

# Details

This PR updates the "list labels" (`GET /labels`) API by adding an
optional `include_host_counts` parameter, which defaults to `true`. If
explicitly set to `false`, the underlying db code will skip doing an
expensive subquery which returns the number of hosts that are members of
each label. The UI will now default to setting this to `false` in its
calls, because:

1. This is an N+1 query pattern which scales poorly as the # of labels
and hosts increases (see associated ticket as well as
https://github.com/fleetdm/fleet/issues/4890)
1. _We don't use this data anywhere._ At least no where I could find in
the front end or back end (besides a test specifically for this
functionality). So we're doing this work for nothing.

Since this is a public API we can't just [drop the functionality
entirely](https://github.com/fleetdm/fleet/pull/35763) as that would be
a breaking change.

# Checklist for submitter

If some of the following don't apply, delete the relevant line.

- [X] Changes file added for user-visible changes in `changes/`,
`orbit/changes/` or `ee/fleetd-chrome/changes`.
See [Changes
files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/guides/committing-changes.md#changes-files)
for more information.

- [X] Input data is properly validated, `SELECT *` is avoided, SQL
injection is prevented (using placeholders for values in statements)

## Testing

- [X] QA'd all new/changed functionality manually
The only place that I could find that lists host counts for labels is
the Packs UI, which uses a different endpoint and database method (`GET
/targets` and `SearchLabels()`

changes/35763-change-host-counts-default

@@ -0,0 +1 @@
+- Made returning host_count optional in the "List Labels" API, to improve performance.

frontend/services/entities/labels.ts

@@ -7,6 +7,7 @@ import { IDynamicLabelFormData } from "pages/labels/components/DynamicLabelForm/
 import { IManualLabelFormData } from "pages/labels/components/ManualLabelForm/ManualLabelForm";
 import { IHost } from "interfaces/host";
 import { INewLabelFormData } from "pages/labels/NewLabelPage/NewLabelPage";
+import { buildQueryStringFromParams } from "utilities/url";
 
 export interface ILabelsResponse {
   labels: ILabel[];
@@ -109,12 +110,18 @@ export default {
 
     return sendRequest("DELETE", path);
   },
-  // TODO: confirm this still works
-  loadAll: async (): Promise<ILabelsResponse> => {
+  loadAll: async (includeHostCounts = false): Promise<ILabelsResponse> => {
     const { LABELS } = endpoints;
 
+    const queryStringParams = {
+      include_host_counts: includeHostCounts,
+    };
+
+    const queryString = buildQueryStringFromParams(queryStringParams);
+    const path = `${LABELS}?${queryString}`;
+
     try {
-      const response = await sendRequest("GET", LABELS);
+      const response = await sendRequest("GET", path);
       return Promise.resolve({ labels: helpers.formatLabelResponse(response) });
     } catch (error) {
       console.error(error);

server/fleet/service.go

@@ -261,7 +261,7 @@ type Service interface {
 
 	NewLabel(ctx context.Context, p LabelPayload) (label *Label, hostIDs []uint, err error)
 	ModifyLabel(ctx context.Context, id uint, payload ModifyLabelPayload) (*Label, []uint, error)
-	ListLabels(ctx context.Context, opt ListOptions) (labels []*Label, err error)
+	ListLabels(ctx context.Context, opt ListOptions, includeHostCounts bool) (labels []*Label, err error)
 	LabelsSummary(ctx context.Context) (labels []*LabelSummary, err error)
 	GetLabel(ctx context.Context, id uint) (label *Label, hostIDs []uint, err error)
 

server/mock/service/service_mock.go

@@ -148,7 +148,7 @@ type NewLabelFunc func(ctx context.Context, p fleet.LabelPayload) (label *fleet.
 
 type ModifyLabelFunc func(ctx context.Context, id uint, payload fleet.ModifyLabelPayload) (*fleet.Label, []uint, error)
 
-type ListLabelsFunc func(ctx context.Context, opt fleet.ListOptions) (labels []*fleet.Label, err error)
+type ListLabelsFunc func(ctx context.Context, opt fleet.ListOptions, includeHostCounts bool) (labels []*fleet.Label, err error)
 
 type LabelsSummaryFunc func(ctx context.Context) (labels []*fleet.LabelSummary, err error)
 
@@ -2557,11 +2557,11 @@ func (s *Service) ModifyLabel(ctx context.Context, id uint, payload fleet.Modify
 	return s.ModifyLabelFunc(ctx, id, payload)
 }
 
-func (s *Service) ListLabels(ctx context.Context, opt fleet.ListOptions) (labels []*fleet.Label, err error) {
+func (s *Service) ListLabels(ctx context.Context, opt fleet.ListOptions, includeHostCounts bool) (labels []*fleet.Label, err error) {
 	s.mu.Lock()
 	s.ListLabelsFuncInvoked = true
 	s.mu.Unlock()
-	return s.ListLabelsFunc(ctx, opt)
+	return s.ListLabelsFunc(ctx, opt, includeHostCounts)
 }
 
 func (s *Service) LabelsSummary(ctx context.Context) (labels []*fleet.LabelSummary, err error) {

server/service/labels.go

@@ -271,7 +271,8 @@ func (svc *Service) GetLabel(ctx context.Context, id uint) (*fleet.Label, []uint
 ////////////////////////////////////////////////////////////////////////////////
 
 type listLabelsRequest struct {
-	ListOptions fleet.ListOptions `url:"list_options"`
+	ListOptions       fleet.ListOptions `url:"list_options"`
+	IncludeHostCounts *bool             `query:"include_host_counts,optional"`
 }
 
 type listLabelsResponse struct {
@@ -284,7 +285,12 @@ func (r listLabelsResponse) Error() error { return r.Err }
 func listLabelsEndpoint(ctx context.Context, request interface{}, svc fleet.Service) (fleet.Errorer, error) {
 	req := request.(*listLabelsRequest)
 
-	labels, err := svc.ListLabels(ctx, req.ListOptions)
+	includeHostCounts := true
+	if req.IncludeHostCounts != nil {
+		includeHostCounts = *req.IncludeHostCounts
+	}
+
+	labels, err := svc.ListLabels(ctx, req.ListOptions, includeHostCounts)
 	if err != nil {
 		return listLabelsResponse{Err: err}, nil
 	}
@@ -300,15 +306,21 @@ func listLabelsEndpoint(ctx context.Context, request interface{}, svc fleet.Serv
 	return resp, nil
 }
 
-func (svc *Service) ListLabels(ctx context.Context, opt fleet.ListOptions) ([]*fleet.Label, error) {
+func (svc *Service) ListLabels(ctx context.Context, opt fleet.ListOptions, includeHostCounts bool) ([]*fleet.Label, error) {
 	if err := svc.authz.Authorize(ctx, &fleet.Label{}, fleet.ActionRead); err != nil {
 		return nil, err
 	}
+
+	filter := fleet.TeamFilter{}
 	vc, ok := viewer.FromContext(ctx)
 	if !ok {
 		return nil, fleet.ErrNoContext
 	}
-	filter := fleet.TeamFilter{User: vc.User, IncludeObserver: true}
+
+	// Default to including host counts.
+	if includeHostCounts {
+		filter = fleet.TeamFilter{User: vc.User, IncludeObserver: true}
+	}
 
 	// TODO(mna): ListLabels doesn't currently return the hostIDs members of the
 	// label, the quick approach would be an N+1 queries endpoint. Leaving like

server/service/labels_test.go

@@ -114,7 +114,7 @@ func TestLabelsAuth(t *testing.T) {
 			_, err = svc.GetLabelSpec(ctx, "abc")
 			checkAuthErr(t, tt.shouldFailRead, err)
 
-			_, err = svc.ListLabels(ctx, fleet.ListOptions{})
+			_, err = svc.ListLabels(ctx, fleet.ListOptions{}, true)
 			checkAuthErr(t, tt.shouldFailRead, err)
 
 			_, err = svc.LabelsSummary((ctx))
@@ -132,6 +132,33 @@ func TestLabelsAuth(t *testing.T) {
 	}
 }
 
+func TestListLabelsHostCountOptions(t *testing.T) {
+	ds := new(mock.Store)
+	svc, ctx := newTestService(t, ds, nil, nil)
+	user := &fleet.User{GlobalRole: ptr.String(fleet.RoleAdmin)}
+	ctx = viewer.NewContext(ctx, viewer.Viewer{User: user})
+
+	ds.ListLabelsFunc = func(ctx context.Context, filter fleet.TeamFilter, opts fleet.ListOptions) ([]*fleet.Label, error) {
+		// Expect the team filter to be empty, meaning no host counts requested
+		require.Nil(t, filter.User)
+		return nil, nil
+	}
+
+	// Test explicitly setting include_host_counts to false
+	_, err := svc.ListLabels(ctx, fleet.ListOptions{}, false)
+	require.NoError(t, err)
+
+	ds.ListLabelsFunc = func(ctx context.Context, filter fleet.TeamFilter, opts fleet.ListOptions) ([]*fleet.Label, error) {
+		// Expect the team filter to be empty, meaning no host counts requested
+		require.Equal(t, filter.User, user)
+		return nil, nil
+	}
+
+	// Test explicitly setting include_host_counts to true
+	_, err = svc.ListLabels(ctx, fleet.ListOptions{}, true)
+	require.NoError(t, err)
+}
+
 func TestLabelsWithDS(t *testing.T) {
 	ds := mysql.CreateMySQLDS(t)
 
@@ -171,7 +198,7 @@ func testLabelsListLabels(t *testing.T, ds *mysql.Datastore) {
 	svc, ctx := newTestService(t, ds, nil, nil)
 	require.NoError(t, ds.MigrateData(context.Background()))
 
-	labels, err := svc.ListLabels(test.UserContext(ctx, test.UserAdmin), fleet.ListOptions{Page: 0, PerPage: 1000})
+	labels, err := svc.ListLabels(test.UserContext(ctx, test.UserAdmin), fleet.ListOptions{Page: 0, PerPage: 1000}, true)
 	require.NoError(t, err)
 	require.Len(t, labels, 8)