From 9b6eadab770bcdfc65e1762c56766a57c0cfe6de Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 20 May 2026 17:40:50 -0500 Subject: [PATCH] fix: drop N+1 db query on template ACL available (#25465) Fixes [PLAT-149](https://linear.app/codercom/issue/PLAT-149/template-permissions-search-is-extremely-slow-with-many-groups). `/acl/available` ran a db query per group. A deployment with >5,000 groups made this route extremely slow. --- coderd/database/dbauthz/dbauthz.go | 9 ++ coderd/database/dbauthz/dbauthz_test.go | 12 +++ coderd/database/dbmetrics/querymetrics.go | 8 ++ coderd/database/dbmock/dbmock.go | 15 +++ coderd/database/querier.go | 6 ++ coderd/database/queries.sql.go | 63 ++++++++++++ coderd/database/queries/groupmembers.sql | 16 +++ coderd/database/queries/groups.sql | 9 ++ codersdk/templates.go | 16 ++- enterprise/coderd/parameters_test.go | 4 +- enterprise/coderd/templates.go | 68 +++++++++---- enterprise/coderd/templates_test.go | 115 +++++++++++++++++++++- 12 files changed, 315 insertions(+), 26 deletions(-) diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index f8733b410a..b0c2703934 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -3476,6 +3476,15 @@ func (q *querier) GetGroupMembersCountByGroupID(ctx context.Context, arg databas return memberCount, nil } +func (q *querier) GetGroupMembersCountByGroupIDs(ctx context.Context, arg database.GetGroupMembersCountByGroupIDsParams) ([]database.GetGroupMembersCountByGroupIDsRow, error) { + if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceGroup); err != nil { + // Ideally we would check read access on each group ID, but that would be N queries. + // So this function is really only usable by admins. + return nil, err + } + return q.db.GetGroupMembersCountByGroupIDs(ctx, arg) +} + func (q *querier) GetGroups(ctx context.Context, arg database.GetGroupsParams) ([]database.GetGroupsRow, error) { if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceSystem); err == nil { // Optimize this query for system users as it is used in telemetry. diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index 22fb3902f7..8a6d8393b1 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -1820,6 +1820,18 @@ func (s *MethodTestSuite) TestGroup() { check.Args(arg).Asserts(g, policy.ActionRead) })) + s.Run("GetGroupMembersCountByGroupIDs", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) { + g1 := testutil.Fake(s.T(), faker, database.Group{}) + g2 := testutil.Fake(s.T(), faker, database.Group{}) + arg := database.GetGroupMembersCountByGroupIDsParams{GroupIds: []uuid.UUID{g1.ID, g2.ID}, IncludeSystem: false} + rows := []database.GetGroupMembersCountByGroupIDsRow{ + {GroupID: g1.ID, MemberCount: 1}, + {GroupID: g2.ID, MemberCount: 2}, + } + dbm.EXPECT().GetGroupMembersCountByGroupIDs(gomock.Any(), arg).Return(rows, nil).AnyTimes() + check.Args(arg).Asserts(rbac.ResourceGroup, policy.ActionRead).Returns(rows) + })) + s.Run("GetGroupMembers", s.Mocked(func(dbm *dbmock.MockStore, _ *gofakeit.Faker, check *expects) { dbm.EXPECT().GetGroupMembers(gomock.Any(), false).Return([]database.GroupMember{}, nil).AnyTimes() check.Args(false).Asserts(rbac.ResourceSystem, policy.ActionRead) diff --git a/coderd/database/dbmetrics/querymetrics.go b/coderd/database/dbmetrics/querymetrics.go index 2e8cb7c306..fc07c3e6a8 100644 --- a/coderd/database/dbmetrics/querymetrics.go +++ b/coderd/database/dbmetrics/querymetrics.go @@ -1945,6 +1945,14 @@ func (m queryMetricsStore) GetGroupMembersCountByGroupID(ctx context.Context, ar return r0, r1 } +func (m queryMetricsStore) GetGroupMembersCountByGroupIDs(ctx context.Context, arg database.GetGroupMembersCountByGroupIDsParams) ([]database.GetGroupMembersCountByGroupIDsRow, error) { + start := time.Now() + r0, r1 := m.s.GetGroupMembersCountByGroupIDs(ctx, arg) + m.queryLatencies.WithLabelValues("GetGroupMembersCountByGroupIDs").Observe(time.Since(start).Seconds()) + m.queryCounts.WithLabelValues(httpmw.ExtractHTTPRoute(ctx), httpmw.ExtractHTTPMethod(ctx), "GetGroupMembersCountByGroupIDs").Inc() + return r0, r1 +} + func (m queryMetricsStore) GetGroups(ctx context.Context, arg database.GetGroupsParams) ([]database.GetGroupsRow, error) { start := time.Now() r0, r1 := m.s.GetGroups(ctx, arg) diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index 46e61d8cbb..1832525d7c 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -3616,6 +3616,21 @@ func (mr *MockStoreMockRecorder) GetGroupMembersCountByGroupID(ctx, arg any) *go return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetGroupMembersCountByGroupID", reflect.TypeOf((*MockStore)(nil).GetGroupMembersCountByGroupID), ctx, arg) } +// GetGroupMembersCountByGroupIDs mocks base method. +func (m *MockStore) GetGroupMembersCountByGroupIDs(ctx context.Context, arg database.GetGroupMembersCountByGroupIDsParams) ([]database.GetGroupMembersCountByGroupIDsRow, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetGroupMembersCountByGroupIDs", ctx, arg) + ret0, _ := ret[0].([]database.GetGroupMembersCountByGroupIDsRow) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetGroupMembersCountByGroupIDs indicates an expected call of GetGroupMembersCountByGroupIDs. +func (mr *MockStoreMockRecorder) GetGroupMembersCountByGroupIDs(ctx, arg any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetGroupMembersCountByGroupIDs", reflect.TypeOf((*MockStore)(nil).GetGroupMembersCountByGroupIDs), ctx, arg) +} + // GetGroups mocks base method. func (m *MockStore) GetGroups(ctx context.Context, arg database.GetGroupsParams) ([]database.GetGroupsRow, error) { m.ctrl.T.Helper() diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 274c8eb179..32e38750ec 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -473,6 +473,12 @@ type sqlcQuerier interface { // count even if the caller does not have read access to ResourceGroupMember. // They only need ResourceGroup read access. GetGroupMembersCountByGroupID(ctx context.Context, arg GetGroupMembersCountByGroupIDParams) (int64, error) + // Returns the total member count for each of the given group IDs in a + // single query. Used to avoid N+1 lookups when listing many groups. Like + // GetGroupMembersCountByGroupID, the count is returned even when the + // caller does not have read access to individual group members. + GetGroupMembersCountByGroupIDs(ctx context.Context, arg GetGroupMembersCountByGroupIDsParams) ([]GetGroupMembersCountByGroupIDsRow, error) + // A limit of 0 means "no limit". GetGroups(ctx context.Context, arg GetGroupsParams) ([]GetGroupsRow, error) GetHealthSettings(ctx context.Context) (string, error) GetInboxNotificationByID(ctx context.Context, id uuid.UUID) (InboxNotification, error) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 5fbc873c70..4505fe0e35 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -12689,6 +12689,56 @@ func (q *sqlQuerier) GetGroupMembersCountByGroupID(ctx context.Context, arg GetG return count, err } +const getGroupMembersCountByGroupIDs = `-- name: GetGroupMembersCountByGroupIDs :many +SELECT + group_id, + COUNT(*) AS member_count +FROM group_members_expanded +WHERE group_id = ANY($1 :: uuid[]) + AND CASE + WHEN $2::bool THEN TRUE + ELSE user_is_system = false + END +GROUP BY group_id +` + +type GetGroupMembersCountByGroupIDsParams struct { + GroupIds []uuid.UUID `db:"group_ids" json:"group_ids"` + IncludeSystem bool `db:"include_system" json:"include_system"` +} + +type GetGroupMembersCountByGroupIDsRow struct { + GroupID uuid.UUID `db:"group_id" json:"group_id"` + MemberCount int64 `db:"member_count" json:"member_count"` +} + +// Returns the total member count for each of the given group IDs in a +// single query. Used to avoid N+1 lookups when listing many groups. Like +// GetGroupMembersCountByGroupID, the count is returned even when the +// caller does not have read access to individual group members. +func (q *sqlQuerier) GetGroupMembersCountByGroupIDs(ctx context.Context, arg GetGroupMembersCountByGroupIDsParams) ([]GetGroupMembersCountByGroupIDsRow, error) { + rows, err := q.db.QueryContext(ctx, getGroupMembersCountByGroupIDs, pq.Array(arg.GroupIds), arg.IncludeSystem) + if err != nil { + return nil, err + } + defer rows.Close() + var items []GetGroupMembersCountByGroupIDsRow + for rows.Next() { + var i GetGroupMembersCountByGroupIDsRow + if err := rows.Scan(&i.GroupID, &i.MemberCount); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + const insertGroupMember = `-- name: InsertGroupMember :exec INSERT INTO group_members (user_id, group_id) @@ -12906,6 +12956,14 @@ WHERE groups.id = ANY($4) ELSE true END + -- Filter by group name or display name (substring, case-insensitive). + AND CASE WHEN $5 :: text != '' THEN ( + groups.name ILIKE concat('%', $5, '%') + OR groups.display_name ILIKE concat('%', $5, '%') + ) + ELSE true + END +LIMIT NULLIF($6 :: int, 0) ` type GetGroupsParams struct { @@ -12913,6 +12971,8 @@ type GetGroupsParams struct { HasMemberID uuid.UUID `db:"has_member_id" json:"has_member_id"` GroupNames []string `db:"group_names" json:"group_names"` GroupIds []uuid.UUID `db:"group_ids" json:"group_ids"` + Search string `db:"search" json:"search"` + LimitOpt int32 `db:"limit_opt" json:"limit_opt"` } type GetGroupsRow struct { @@ -12921,12 +12981,15 @@ type GetGroupsRow struct { OrganizationDisplayName string `db:"organization_display_name" json:"organization_display_name"` } +// A limit of 0 means "no limit". func (q *sqlQuerier) GetGroups(ctx context.Context, arg GetGroupsParams) ([]GetGroupsRow, error) { rows, err := q.db.QueryContext(ctx, getGroups, arg.OrganizationID, arg.HasMemberID, pq.Array(arg.GroupNames), pq.Array(arg.GroupIds), + arg.Search, + arg.LimitOpt, ) if err != nil { return nil, err diff --git a/coderd/database/queries/groupmembers.sql b/coderd/database/queries/groupmembers.sql index 4e5469317a..fd167d219a 100644 --- a/coderd/database/queries/groupmembers.sql +++ b/coderd/database/queries/groupmembers.sql @@ -142,6 +142,22 @@ WHERE group_id = @group_id user_is_system = false END; +-- name: GetGroupMembersCountByGroupIDs :many +-- Returns the total member count for each of the given group IDs in a +-- single query. Used to avoid N+1 lookups when listing many groups. Like +-- GetGroupMembersCountByGroupID, the count is returned even when the +-- caller does not have read access to individual group members. +SELECT + group_id, + COUNT(*) AS member_count +FROM group_members_expanded +WHERE group_id = ANY(@group_ids :: uuid[]) + AND CASE + WHEN @include_system::bool THEN TRUE + ELSE user_is_system = false + END +GROUP BY group_id; + -- InsertUserGroupsByID adds a user to all provided groups, if they exist. -- name: InsertUserGroupsByID :many WITH groups AS ( diff --git a/coderd/database/queries/groups.sql b/coderd/database/queries/groups.sql index 3413e5832e..39742d5535 100644 --- a/coderd/database/queries/groups.sql +++ b/coderd/database/queries/groups.sql @@ -78,6 +78,15 @@ WHERE groups.id = ANY(@group_ids) ELSE true END + -- Filter by group name or display name (substring, case-insensitive). + AND CASE WHEN @search :: text != '' THEN ( + groups.name ILIKE concat('%', @search, '%') + OR groups.display_name ILIKE concat('%', @search, '%') + ) + ELSE true + END +-- A limit of 0 means "no limit". +LIMIT NULLIF(@limit_opt :: int, 0) ; -- name: InsertGroup :one diff --git a/codersdk/templates.go b/codersdk/templates.go index d0c5276118..87fea25fb9 100644 --- a/codersdk/templates.go +++ b/codersdk/templates.go @@ -375,9 +375,19 @@ func (c *Client) UpdateTemplateACL(ctx context.Context, templateID uuid.UUID, re return nil } -// TemplateACLAvailable returns available users + groups that can be assigned template perms -func (c *Client) TemplateACLAvailable(ctx context.Context, templateID uuid.UUID) (ACLAvailable, error) { - res, err := c.Request(ctx, http.MethodGet, fmt.Sprintf("/api/v2/templates/%s/acl/available", templateID), nil) +// TemplateACLAvailable returns available users + groups that can be assigned +// template perms. The optional req controls the q/limit/offset query +// parameters applied server-side; pass codersdk.UsersRequest{} when no +// filtering is desired. +func (c *Client) TemplateACLAvailable(ctx context.Context, templateID uuid.UUID, req UsersRequest) (ACLAvailable, error) { + res, err := c.Request( + ctx, + http.MethodGet, + fmt.Sprintf("/api/v2/templates/%s/acl/available", templateID), + nil, + req.Pagination.asRequestOption(), + req.asRequestOption(), + ) if err != nil { return ACLAvailable{}, err } diff --git a/enterprise/coderd/parameters_test.go b/enterprise/coderd/parameters_test.go index bda9e3c59e..4522c8ad30 100644 --- a/enterprise/coderd/parameters_test.go +++ b/enterprise/coderd/parameters_test.go @@ -35,7 +35,9 @@ func TestDynamicParametersOwnerGroups(t *testing.T) { _, noGroupUser := coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID) // Create the group to be asserted - group := coderdtest.CreateGroup(t, ownerClient, owner.OrganizationID, "bloob", templateAdminUser) + // Make the group name something after "Everyone" when sorted alphabetically. + // The test wants to check that `Everyone` is the default, which is the first alphabetical group in the test. + group := coderdtest.CreateGroup(t, ownerClient, owner.OrganizationID, "zebra", templateAdminUser) dynamicParametersTerraformSource, err := os.ReadFile("testdata/parameters/groups/main.tf") require.NoError(t, err) diff --git a/enterprise/coderd/templates.go b/enterprise/coderd/templates.go index 62c1b35567..9ef07271fc 100644 --- a/enterprise/coderd/templates.go +++ b/enterprise/coderd/templates.go @@ -9,6 +9,7 @@ import ( "golang.org/x/xerrors" "cdr.dev/slog/v3" + agpl "github.com/coder/coder/v2/coderd" "github.com/coder/coder/v2/coderd/audit" "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/db2sdk" @@ -17,6 +18,7 @@ import ( "github.com/coder/coder/v2/coderd/httpmw" "github.com/coder/coder/v2/coderd/rbac/acl" "github.com/coder/coder/v2/coderd/rbac/policy" + "github.com/coder/coder/v2/coderd/searchquery" "github.com/coder/coder/v2/coderd/util/slice" "github.com/coder/coder/v2/codersdk" ) @@ -50,39 +52,63 @@ func (api *API) templateAvailablePermissions(rw http.ResponseWriter, r *http.Req return } + // Apply the same q/limit semantics to groups as the users half of this response. + // The query semantics are defined for the users, which is awkward. But we can + // just reuse the search part of the query which is a fuzzy match. + userFilter, verr := searchquery.Users(r.URL.Query().Get("q")) + if len(verr) > 0 { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "Invalid user search query.", + Validations: verr, + }) + return + } + groupPagination, ok := agpl.ParsePagination(rw, r) + if !ok { + return + } + // Perm check is the template update check. // nolint:gocritic groups, err := api.Database.GetGroups(dbauthz.AsSystemRestricted(ctx), database.GetGroupsParams{ OrganizationID: template.OrganizationID, + Search: userFilter.Search, + // #nosec G115 - Pagination limits are small and fit in int32 + LimitOpt: int32(groupPagination.Limit), }) if err != nil { httpapi.InternalServerError(rw, err) return } + // Fetch member counts for all groups in a single query to avoid an + // N+1 lookup pattern that was making this endpoint extremely slow on + // deployments with many groups. The per-group member lists are + // intentionally not populated here: callers of this endpoint only + // surface total_member_count (see Group.TotalMemberCount, which is + // already documented as the canonical value). + groupIDs := make([]uuid.UUID, len(groups)) + for i, g := range groups { + groupIDs[i] = g.Group.ID + } + + // nolint:gocritic // Same justification as the GetGroups call above. + countRows, err := api.Database.GetGroupMembersCountByGroupIDs(dbauthz.AsSystemRestricted(ctx), database.GetGroupMembersCountByGroupIDsParams{ + GroupIds: groupIDs, + IncludeSystem: false, + }) + if err != nil { + httpapi.InternalServerError(rw, err) + return + } + countByGroup := make(map[uuid.UUID]int64, len(countRows)) + for _, row := range countRows { + countByGroup[row.GroupID] = row.MemberCount + } + sdkGroups := make([]codersdk.Group, 0, len(groups)) for _, group := range groups { - // nolint:gocritic - members, err := api.Database.GetGroupMembersByGroupID(dbauthz.AsSystemRestricted(ctx), database.GetGroupMembersByGroupIDParams{ - GroupID: group.Group.ID, - IncludeSystem: false, - }) - if err != nil { - httpapi.InternalServerError(rw, err) - return - } - - // nolint:gocritic - memberCount, err := api.Database.GetGroupMembersCountByGroupID(dbauthz.AsSystemRestricted(ctx), database.GetGroupMembersCountByGroupIDParams{ - GroupID: group.Group.ID, - IncludeSystem: false, - }) - if err != nil { - httpapi.InternalServerError(rw, err) - return - } - - sdkGroups = append(sdkGroups, db2sdk.Group(group, members, int(memberCount))) + sdkGroups = append(sdkGroups, db2sdk.Group(group, nil, int(countByGroup[group.Group.ID]))) } httpapi.Write(ctx, rw, http.StatusOK, codersdk.ACLAvailable{ diff --git a/enterprise/coderd/templates_test.go b/enterprise/coderd/templates_test.go index 57c0977be1..57acd59835 100644 --- a/enterprise/coderd/templates_test.go +++ b/enterprise/coderd/templates_test.go @@ -1241,6 +1241,119 @@ func TestTemplateACL(t *testing.T) { }) require.NoError(t, err) }) + + // Regression test for PLAT-149. Previously this endpoint did an N+1 + // fetch of every group's members and member count. Verify that the + // member count is returned correctly for many groups, and that the + // per-group members list is no longer populated (callers should rely + // on TotalMemberCount). + t.Run("AvailableReturnsGroupMemberCounts", func(t *testing.T) { + t.Parallel() + + client, user := coderdenttest.New(t, &coderdenttest.Options{LicenseOptions: &coderdenttest.LicenseOptions{ + Features: license.Features{ + codersdk.FeatureTemplateRBAC: 1, + }, + }}) + admin, _ := coderdtest.CreateAnotherUser(t, client, user.OrganizationID, rbac.RoleTemplateAdmin(), rbac.RoleUserAdmin()) + + // Create a couple of users we can stuff into groups. + _, alice := coderdtest.CreateAnotherUser(t, client, user.OrganizationID) + _, bob := coderdtest.CreateAnotherUser(t, client, user.OrganizationID) + _, carol := coderdtest.CreateAnotherUser(t, client, user.OrganizationID) + + // emptyGroup: zero non-system members. + // singleGroup: alice only. + // fullGroup: alice + bob + carol. + emptyGroup := coderdtest.CreateGroup(t, admin, user.OrganizationID, "empty-group") + singleGroup := coderdtest.CreateGroup(t, admin, user.OrganizationID, "single-group", alice) + fullGroup := coderdtest.CreateGroup(t, admin, user.OrganizationID, "full-group", alice, bob, carol) + + version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) + template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) + + ctx := testutil.Context(t, testutil.WaitLong) + + available, err := admin.TemplateACLAvailable(ctx, template.ID, codersdk.UsersRequest{}) + require.NoError(t, err) + + wantCounts := map[uuid.UUID]int{ + emptyGroup.ID: 0, + singleGroup.ID: 1, + fullGroup.ID: 3, + } + + found := map[uuid.UUID]bool{} + for _, group := range available.Groups { + if want, ok := wantCounts[group.ID]; ok { + found[group.ID] = true + require.Equal(t, want, group.TotalMemberCount, + "unexpected total_member_count for group %q", group.Name) + require.Empty(t, group.Members, + "members must not be populated by the available endpoint for group %q", group.Name) + } + } + for id := range wantCounts { + require.True(t, found[id], "group %s missing from available response", id) + } + }) + + // Companion to the AvailableReturnsGroupMemberCounts test above. Verifies + // that the q query parameter applies a server-side substring filter on + // group name / display_name, and that limit caps the number of groups + // returned. The autocomplete sends both on each keystroke; before + // PLAT-149 both were ignored for groups. + t.Run("AvailableHonorsGroupSearchAndLimit", func(t *testing.T) { + t.Parallel() + + client, user := coderdenttest.New(t, &coderdenttest.Options{LicenseOptions: &coderdenttest.LicenseOptions{ + Features: license.Features{ + codersdk.FeatureTemplateRBAC: 1, + }, + }}) + admin, _ := coderdtest.CreateAnotherUser(t, client, user.OrganizationID, rbac.RoleTemplateAdmin(), rbac.RoleUserAdmin()) + + // Create a handful of groups with predictable names so we can + // pin assertions to specific substrings. + engAlpha := coderdtest.CreateGroup(t, admin, user.OrganizationID, "engineering-alpha") + engBeta := coderdtest.CreateGroup(t, admin, user.OrganizationID, "engineering-beta") + design := coderdtest.CreateGroup(t, admin, user.OrganizationID, "design") + sales := coderdtest.CreateGroup(t, admin, user.OrganizationID, "sales") + + version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) + template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) + + ctx := testutil.Context(t, testutil.WaitLong) + + groupIDs := func(available codersdk.ACLAvailable) []uuid.UUID { + ids := make([]uuid.UUID, 0, len(available.Groups)) + for _, g := range available.Groups { + ids = append(ids, g.ID) + } + return ids + } + + // q filters by group name / display_name substring. + filtered, err := admin.TemplateACLAvailable(ctx, template.ID, codersdk.UsersRequest{ + SearchQuery: "engineering", + }) + require.NoError(t, err) + got := groupIDs(filtered) + require.ElementsMatch(t, []uuid.UUID{engAlpha.ID, engBeta.ID}, got, + "q=engineering should return only engineering-* groups, got %v", got) + require.NotContains(t, got, design.ID) + require.NotContains(t, got, sales.ID) + + // limit caps the number of groups returned. With 4 user-created + // groups plus the implicit Everyone group, asking for 2 must + // return at most 2 groups. + limited, err := admin.TemplateACLAvailable(ctx, template.ID, codersdk.UsersRequest{ + Pagination: codersdk.Pagination{Limit: 2}, + }) + require.NoError(t, err) + require.Len(t, limited.Groups, 2, + "limit=2 should cap groups to 2, got %d", len(limited.Groups)) + }) } func TestUpdateTemplateACL(t *testing.T) { @@ -1626,7 +1739,7 @@ func TestUpdateTemplateACL(t *testing.T) { require.NoError(t, err) // Should be able to see user 3 - available, err := client2.TemplateACLAvailable(ctx, template.ID) + available, err := client2.TemplateACLAvailable(ctx, template.ID, codersdk.UsersRequest{}) require.NoError(t, err) userFound := false for _, avail := range available.Users {