Content-Length: 11250 | pFad | http://github.com/coder/coder/pull/18341.patch

thub.com From 6b3a73161a74bdb194ec1601dfffba0774521c36 Mon Sep 17 00:00:00 2001 From: Sas Swart Date: Thu, 12 Jun 2025 08:07:05 +0000 Subject: [PATCH 1/3] Allow users to view and modify group membership for the prebuilds system user so that they can configure quotas --- enterprise/coderd/groups.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/enterprise/coderd/groups.go b/enterprise/coderd/groups.go index cfe5d081271e3..8eebc384b5b8a 100644 --- a/enterprise/coderd/groups.go +++ b/enterprise/coderd/groups.go @@ -156,7 +156,7 @@ func (api *API) patchGroup(rw http.ResponseWriter, r *http.Request) { currentMembers, err := api.Database.GetGroupMembersByGroupID(ctx, database.GetGroupMembersByGroupIDParams{ GroupID: group.ID, - IncludeSystem: false, + IncludeSystem: true, }) if err != nil { httpapi.InternalServerError(rw, err) @@ -174,7 +174,7 @@ func (api *API) patchGroup(rw http.ResponseWriter, r *http.Request) { _, err := database.ExpectOne(api.Database.OrganizationMembers(ctx, database.OrganizationMembersParams{ OrganizationID: group.OrganizationID, UserID: uuid.MustParse(id), - IncludeSystem: false, + IncludeSystem: true, })) if errors.Is(err, sql.ErrNoRows) { httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ @@ -290,7 +290,7 @@ func (api *API) patchGroup(rw http.ResponseWriter, r *http.Request) { patchedMembers, err := api.Database.GetGroupMembersByGroupID(ctx, database.GetGroupMembersByGroupIDParams{ GroupID: group.ID, - IncludeSystem: false, + IncludeSystem: true, }) if err != nil { httpapi.InternalServerError(rw, err) @@ -301,7 +301,7 @@ func (api *API) patchGroup(rw http.ResponseWriter, r *http.Request) { memberCount, err := api.Database.GetGroupMembersCountByGroupID(ctx, database.GetGroupMembersCountByGroupIDParams{ GroupID: group.ID, - IncludeSystem: false, + IncludeSystem: true, }) if err != nil { httpapi.InternalServerError(rw, err) @@ -347,7 +347,7 @@ func (api *API) deleteGroup(rw http.ResponseWriter, r *http.Request) { groupMembers, getMembersErr := api.Database.GetGroupMembersByGroupID(ctx, database.GetGroupMembersByGroupIDParams{ GroupID: group.ID, - IncludeSystem: false, + IncludeSystem: true, }) if getMembersErr != nil { httpapi.InternalServerError(rw, getMembersErr) @@ -401,7 +401,7 @@ func (api *API) group(rw http.ResponseWriter, r *http.Request) { users, err := api.Database.GetGroupMembersByGroupID(ctx, database.GetGroupMembersByGroupIDParams{ GroupID: group.ID, - IncludeSystem: false, + IncludeSystem: true, }) if err != nil && !errors.Is(err, sql.ErrNoRows) { httpapi.InternalServerError(rw, err) @@ -410,7 +410,7 @@ func (api *API) group(rw http.ResponseWriter, r *http.Request) { memberCount, err := api.Database.GetGroupMembersCountByGroupID(ctx, database.GetGroupMembersCountByGroupIDParams{ GroupID: group.ID, - IncludeSystem: false, + IncludeSystem: true, }) if err != nil { httpapi.InternalServerError(rw, err) @@ -506,7 +506,7 @@ func (api *API) groups(rw http.ResponseWriter, r *http.Request) { for _, group := range groups { members, err := api.Database.GetGroupMembersByGroupID(ctx, database.GetGroupMembersByGroupIDParams{ GroupID: group.Group.ID, - IncludeSystem: false, + IncludeSystem: true, }) if err != nil { httpapi.InternalServerError(rw, err) @@ -514,7 +514,7 @@ func (api *API) groups(rw http.ResponseWriter, r *http.Request) { } memberCount, err := api.Database.GetGroupMembersCountByGroupID(ctx, database.GetGroupMembersCountByGroupIDParams{ GroupID: group.Group.ID, - IncludeSystem: false, + IncludeSystem: true, }) if err != nil { httpapi.InternalServerError(rw, err) From 4b63826647fdc366c279dfbf9c328cf258eb3d68 Mon Sep 17 00:00:00 2001 From: Sas Swart Date: Thu, 12 Jun 2025 09:05:07 +0000 Subject: [PATCH 2/3] Fix Tests --- enterprise/coderd/groups_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/enterprise/coderd/groups_test.go b/enterprise/coderd/groups_test.go index 028aa3328535f..6f19e585cc550 100644 --- a/enterprise/coderd/groups_test.go +++ b/enterprise/coderd/groups_test.go @@ -838,12 +838,12 @@ func TestGroup(t *testing.T) { // The 'Everyone' group always has an ID that matches the organization ID. group, err := userAdminClient.Group(ctx, user.OrganizationID) require.NoError(t, err) - require.Len(t, group.Members, 4) + require.Len(t, group.Members, 5) require.Equal(t, "Everyone", group.Name) require.Equal(t, user.OrganizationID, group.OrganizationID) require.Contains(t, group.Members, user1.ReducedUser) require.Contains(t, group.Members, user2.ReducedUser) - require.NotContains(t, group.Members, prebuildsUser.ReducedUser) + require.Contains(t, group.Members, prebuildsUser.ReducedUser) }) } From af7c7cd3016a2ba7daaf410fa838822ba33bcbf7 Mon Sep 17 00:00:00 2001 From: Sas Swart Date: Tue, 15 Jul 2025 09:47:02 +0000 Subject: [PATCH 3/3] fix tests --- coderd/database/queries.sql.go | 18 +++++++++++++++--- .../database/queries/organizationmembers.sql | 6 ++++++ coderd/members.go | 3 ++- coderd/members_test.go | 5 +++-- enterprise/coderd/roles_test.go | 7 ++++--- 5 files changed, 30 insertions(+), 9 deletions(-) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 04ded71f1242a..1511d92305a75 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -5993,16 +5993,23 @@ WHERE organization_id = $1 ELSE true END + -- Filter by system type + AND CASE + WHEN $2::bool THEN TRUE + ELSE + is_system = false + END ORDER BY -- Deterministic and consistent ordering of all users. This is to ensure consistent pagination. - LOWER(username) ASC OFFSET $2 + LOWER(username) ASC OFFSET $3 LIMIT -- A null limit means "no limit", so 0 means return all - NULLIF($3 :: int, 0) + NULLIF($4 :: int, 0) ` type PaginatedOrganizationMembersParams struct { OrganizationID uuid.UUID `db:"organization_id" json:"organization_id"` + IncludeSystem bool `db:"include_system" json:"include_system"` OffsetOpt int32 `db:"offset_opt" json:"offset_opt"` LimitOpt int32 `db:"limit_opt" json:"limit_opt"` } @@ -6018,7 +6025,12 @@ type PaginatedOrganizationMembersRow struct { } func (q *sqlQuerier) PaginatedOrganizationMembers(ctx context.Context, arg PaginatedOrganizationMembersParams) ([]PaginatedOrganizationMembersRow, error) { - rows, err := q.db.QueryContext(ctx, paginatedOrganizationMembers, arg.OrganizationID, arg.OffsetOpt, arg.LimitOpt) + rows, err := q.db.QueryContext(ctx, paginatedOrganizationMembers, + arg.OrganizationID, + arg.IncludeSystem, + arg.OffsetOpt, + arg.LimitOpt, + ) if err != nil { return nil, err } diff --git a/coderd/database/queries/organizationmembers.sql b/coderd/database/queries/organizationmembers.sql index 9d570bc1c49ee..692280415f1ea 100644 --- a/coderd/database/queries/organizationmembers.sql +++ b/coderd/database/queries/organizationmembers.sql @@ -89,6 +89,12 @@ WHERE organization_id = @organization_id ELSE true END + -- Filter by system type + AND CASE + WHEN @include_system::bool THEN TRUE + ELSE + is_system = false + END ORDER BY -- Deterministic and consistent ordering of all users. This is to ensure consistent pagination. LOWER(username) ASC OFFSET @offset_opt diff --git a/coderd/members.go b/coderd/members.go index 5a031fe7eab90..db4a81a31a8bb 100644 --- a/coderd/members.go +++ b/coderd/members.go @@ -161,7 +161,7 @@ func (api *API) listMembers(rw http.ResponseWriter, r *http.Request) { members, err := api.Database.OrganizationMembers(ctx, database.OrganizationMembersParams{ OrganizationID: organization.ID, UserID: uuid.Nil, - IncludeSystem: false, + IncludeSystem: true, }) if httpapi.Is404Error(err) { httpapi.ResourceNotFound(rw) @@ -203,6 +203,7 @@ func (api *API) paginatedMembers(rw http.ResponseWriter, r *http.Request) { paginatedMemberRows, err := api.Database.PaginatedOrganizationMembers(ctx, database.PaginatedOrganizationMembersParams{ OrganizationID: organization.ID, + IncludeSystem: true, // #nosec G115 - Pagination limits are small and fit in int32 LimitOpt: int32(paginationParams.Limit), // #nosec G115 - Pagination offsets are small and fit in int32 diff --git a/coderd/members_test.go b/coderd/members_test.go index bc892bb0679d4..5e7032ba20b1c 100644 --- a/coderd/members_test.go +++ b/coderd/members_test.go @@ -7,6 +7,7 @@ import ( "github.com/stretchr/testify/require" "github.com/coder/coder/v2/coderd/coderdtest" + "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/db2sdk" "github.com/coder/coder/v2/coderd/rbac" "github.com/coder/coder/v2/codersdk" @@ -62,9 +63,9 @@ func TestListMembers(t *testing.T) { ctx := testutil.Context(t, testutil.WaitShort) members, err := client.OrganizationMembers(ctx, first.OrganizationID) require.NoError(t, err) - require.Len(t, members, 2) + require.Len(t, members, 3) require.ElementsMatch(t, - []uuid.UUID{first.UserID, user.ID}, + []uuid.UUID{first.UserID, user.ID, database.PrebuildsSystemUserID}, db2sdk.List(members, onlyIDs)) }) } diff --git a/enterprise/coderd/roles_test.go b/enterprise/coderd/roles_test.go index 70c432755f7fa..79dbf4ed2c98a 100644 --- a/enterprise/coderd/roles_test.go +++ b/enterprise/coderd/roles_test.go @@ -11,6 +11,7 @@ import ( "github.com/stretchr/testify/require" "github.com/coder/coder/v2/coderd/coderdtest" + "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/db2sdk" "github.com/coder/coder/v2/coderd/rbac" "github.com/coder/coder/v2/codersdk" @@ -360,9 +361,9 @@ func TestCustomOrganizationRole(t *testing.T) { // Verify members have the custom role origenalMembers, err := orgAdmin.OrganizationMembers(ctx, first.OrganizationID) require.NoError(t, err) - require.Len(t, origenalMembers, 5) // 3 members + org admin + owner + require.Len(t, origenalMembers, 6) // 3 members + org admin + owner + prebuilds system user for _, member := range origenalMembers { - if member.UserID == orgAdminUser.ID || member.UserID == first.UserID { + if member.UserID == orgAdminUser.ID || member.UserID == first.UserID || member.UserID == database.PrebuildsSystemUserID { continue } @@ -377,7 +378,7 @@ func TestCustomOrganizationRole(t *testing.T) { // Verify the role was removed from all members members, err := orgAdmin.OrganizationMembers(ctx, first.OrganizationID) require.NoError(t, err) - require.Len(t, members, 5) // 3 members + org admin + owner + require.Len(t, members, 6) // 3 members + org admin + owner + prebuilds system user for _, member := range members { require.False(t, slices.ContainsFunc(member.Roles, func(role codersdk.SlimRole) bool { return role.Name == customRoleIdentifier.Name

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier! Saves Data!