opensource/coder
3
0
Fork 0
mirror of https://github.com/coder/coder.git synced 2026-09-24 15:04:27 +08:00
Code Issues Packages Projects Releases Wiki Activity

chore: remove organization requirement from convertGroup() (#12195)

Browse Source 
* feat: convertGroups() no longer requires organization info

Removing role information from some users in the api. This info is
excessive and not required. It is costly to always include
This commit is contained in:
Steven Masley
2024-02-21 15:58:11 -06:00
committed by GitHub
parent 3f65bd14cc
commit c3a7b13690
25 changed files with 350 additions and 278 deletions
Download Patch File Download Diff File Expand all files Collapse all files
enterprise/coderd/groups.go
+5 -64
View File
@@ -11,6 +11,7 @@ import (
"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"
"github.com/coder/coder/v2/coderd/httpapi"
"github.com/coder/coder/v2/coderd/httpmw"
"github.com/coder/coder/v2/coderd/rbac"
@@ -77,7 +78,7 @@ func (api *API) postGroupByOrganization(rw http.ResponseWriter, r *http.Request)
var emptyUsers []database.User
aReq.New = group.Auditable(emptyUsers)
httpapi.Write(ctx, rw, http.StatusCreated, convertGroup(group, nil))
httpapi.Write(ctx, rw, http.StatusCreated, db2sdk.Group(group, nil))
}
// @Summary Update group by name
@@ -281,7 +282,7 @@ func (api *API) patchGroup(rw http.ResponseWriter, r *http.Request) {
aReq.New = group.Auditable(patchedMembers)
httpapi.Write(ctx, rw, http.StatusOK, convertGroup(group, patchedMembers))
httpapi.Write(ctx, rw, http.StatusOK, db2sdk.Group(group, patchedMembers))
}
// @Summary Delete group by name
@@ -365,7 +366,7 @@ func (api *API) group(rw http.ResponseWriter, r *http.Request) {
return
}
httpapi.Write(ctx, rw, http.StatusOK, convertGroup(group, users))
httpapi.Write(ctx, rw, http.StatusOK, db2sdk.Group(group, users))
}
// @Summary Get groups by organization
@@ -410,68 +411,8 @@ func (api *API) groups(rw http.ResponseWriter, r *http.Request) {
return
}
resp = append(resp, convertGroup(group, members))
resp = append(resp, db2sdk.Group(group, members))
}
httpapi.Write(ctx, rw, http.StatusOK, resp)
}
func convertGroup(g database.Group, users []database.User) codersdk.Group {
// It's ridiculous to query all the orgs of a user here
// especially since as of the writing of this comment there
// is only one org. So we pretend everyone is only part of
// the group's organization.
orgs := make(map[uuid.UUID][]uuid.UUID)
for _, user := range users {
orgs[user.ID] = []uuid.UUID{g.OrganizationID}
}
return codersdk.Group{
ID: g.ID,
Name: g.Name,
DisplayName: g.DisplayName,
OrganizationID: g.OrganizationID,
AvatarURL: g.AvatarURL,
QuotaAllowance: int(g.QuotaAllowance),
Members: convertUsers(users, orgs),
Source: codersdk.GroupSource(g.Source),
}
}
func convertUser(user database.User, organizationIDs []uuid.UUID) codersdk.User {
convertedUser := codersdk.User{
ID: user.ID,
Email: user.Email,
CreatedAt: user.CreatedAt,
LastSeenAt: user.LastSeenAt,
Username: user.Username,
Status: codersdk.UserStatus(user.Status),
OrganizationIDs: organizationIDs,
Roles: make([]codersdk.Role, 0, len(user.RBACRoles)),
AvatarURL: user.AvatarURL,
LoginType: codersdk.LoginType(user.LoginType),
}
for _, roleName := range user.RBACRoles {
rbacRole, _ := rbac.RoleByName(roleName)
convertedUser.Roles = append(convertedUser.Roles, convertRole(rbacRole))
}
return convertedUser
}
func convertUsers(users []database.User, organizationIDsByUserID map[uuid.UUID][]uuid.UUID) []codersdk.User {
converted := make([]codersdk.User, 0, len(users))
for _, u := range users {
userOrganizationIDs := organizationIDsByUserID[u.ID]
converted = append(converted, convertUser(u, userOrganizationIDs))
}
return converted
}
func convertRole(role rbac.Role) codersdk.Role {
return codersdk.Role{
DisplayName: role.DisplayName,
Name: role.Name,
}
}
enterprise/coderd/groups_test.go
+20 -19
View File
@@ -236,8 +236,8 @@ func TestPatchGroup(t *testing.T) {
AddUsers: []string{user2.ID.String(), user3.ID.String()},
})
require.NoError(t, err)
require.Contains(t, group.Members, user2)
require.Contains(t, group.Members, user3)
require.Contains(t, group.Members, user2.ReducedUser)
require.Contains(t, group.Members, user3.ReducedUser)
})
t.Run("RemoveUsers", func(t *testing.T) {
@@ -263,16 +263,16 @@ func TestPatchGroup(t *testing.T) {
AddUsers: []string{user2.ID.String(), user3.ID.String(), user4.ID.String()},
})
require.NoError(t, err)
require.Contains(t, group.Members, user2)
require.Contains(t, group.Members, user3)
require.Contains(t, group.Members, user2.ReducedUser)
require.Contains(t, group.Members, user3.ReducedUser)
group, err = userAdminClient.PatchGroup(ctx, group.ID, codersdk.PatchGroupRequest{
RemoveUsers: []string{user2.ID.String(), user3.ID.String()},
})
require.NoError(t, err)
require.NotContains(t, group.Members, user2)
require.NotContains(t, group.Members, user3)
require.Contains(t, group.Members, user4)
require.NotContains(t, group.Members, user2.ReducedUser)
require.NotContains(t, group.Members, user3.ReducedUser)
require.Contains(t, group.Members, user4.ReducedUser)
})
t.Run("Audit", func(t *testing.T) {
@@ -613,8 +613,8 @@ func TestGroup(t *testing.T) {
AddUsers: []string{user2.ID.String(), user3.ID.String()},
})
require.NoError(t, err)
require.Contains(t, group.Members, user2)
require.Contains(t, group.Members, user3)
require.Contains(t, group.Members, user2.ReducedUser)
require.Contains(t, group.Members, user3.ReducedUser)
ggroup, err := userAdminClient.Group(ctx, group.ID)
require.NoError(t, err)
@@ -665,15 +665,15 @@ func TestGroup(t *testing.T) {
AddUsers: []string{user1.ID.String(), user2.ID.String()},
})
require.NoError(t, err)
require.Contains(t, group.Members, user1)
require.Contains(t, group.Members, user2)
require.Contains(t, group.Members, user1.ReducedUser)
require.Contains(t, group.Members, user2.ReducedUser)
err = userAdminClient.DeleteUser(ctx, user1.ID)
require.NoError(t, err)
group, err = userAdminClient.Group(ctx, group.ID)
require.NoError(t, err)
require.NotContains(t, group.Members, user1)
require.NotContains(t, group.Members, user1.ReducedUser)
})
t.Run("IncludeSuspendedAndDormantUsers", func(t *testing.T) {
@@ -700,8 +700,8 @@ func TestGroup(t *testing.T) {
})
require.NoError(t, err)
require.Len(t, group.Members, 2)
require.Contains(t, group.Members, user1)
require.Contains(t, group.Members, user2)
require.Contains(t, group.Members, user1.ReducedUser)
require.Contains(t, group.Members, user2.ReducedUser)
user1, err = userAdminClient.UpdateUserStatus(ctx, user1.ID.String(), codersdk.UserStatusSuspended)
require.NoError(t, err)
@@ -709,8 +709,8 @@ func TestGroup(t *testing.T) {
group, err = userAdminClient.Group(ctx, group.ID)
require.NoError(t, err)
require.Len(t, group.Members, 2)
require.Contains(t, group.Members, user1)
require.Contains(t, group.Members, user2)
require.Contains(t, group.Members, user1.ReducedUser)
require.Contains(t, group.Members, user2.ReducedUser)
// cannot explicitly set a dormant user status so must create a new user
anotherUser, err := userAdminClient.CreateUser(ctx, codersdk.CreateUserRequest{
@@ -731,8 +731,8 @@ func TestGroup(t *testing.T) {
group, err = userAdminClient.Group(ctx, group.ID)
require.NoError(t, err)
require.Len(t, group.Members, 3)
require.Contains(t, group.Members, user1)
require.Contains(t, group.Members, user2)
require.Contains(t, group.Members, user1.ReducedUser)
require.Contains(t, group.Members, user2.ReducedUser)
})
t.Run("everyoneGroupReturnsEmpty", func(t *testing.T) {
@@ -754,7 +754,8 @@ func TestGroup(t *testing.T) {
require.Len(t, group.Members, 4)
require.Equal(t, "Everyone", group.Name)
require.Equal(t, user.OrganizationID, group.OrganizationID)
require.Contains(t, group.Members, user1, user2)
require.Contains(t, group.Members, user1.ReducedUser)
require.Contains(t, group.Members, user2.ReducedUser)
})
}
enterprise/coderd/templates.go
+5 -5
View File
@@ -11,6 +11,7 @@ import (
"github.com/coder/coder/v2/coderd/audit"
"github.com/coder/coder/v2/coderd/database"
"github.com/coder/coder/v2/coderd/database/db2sdk"
"github.com/coder/coder/v2/coderd/database/dbauthz"
"github.com/coder/coder/v2/coderd/httpapi"
"github.com/coder/coder/v2/coderd/httpmw"
@@ -64,15 +65,14 @@ func (api *API) templateAvailablePermissions(rw http.ResponseWriter, r *http.Req
return
}
sdkGroups = append(sdkGroups, convertGroup(group, members))
sdkGroups = append(sdkGroups, db2sdk.Group(group, members))
}
httpapi.Write(ctx, rw, http.StatusOK, codersdk.ACLAvailable{
// No need to pass organization info here.
// TODO: @emyrk we should return a MinimalUser here instead of a full user.
// The FE requires the `email` field, so this cannot be done without
// a UI change.
Users: convertUsers(users, map[uuid.UUID][]uuid.UUID{}),
Users: db2sdk.ReducedUsers(users),
Groups: sdkGroups,
})
}
@@ -134,7 +134,7 @@ func (api *API) templateACL(rw http.ResponseWriter, r *http.Request) {
return
}
groups = append(groups, codersdk.TemplateGroup{
Group: convertGroup(group.Group, members),
Group: db2sdk.Group(group.Group, members),
Role: convertToTemplateRole(group.Actions),
})
}
@@ -287,7 +287,7 @@ func convertTemplateUsers(tus []database.TemplateUser, orgIDsByUserIDs map[uuid.
for _, tu := range tus {
users = append(users, codersdk.TemplateUser{
User: convertUser(tu.User, orgIDsByUserIDs[tu.User.ID]),
User: db2sdk.User(tu.User, orgIDsByUserIDs[tu.User.ID]),
Role: convertToTemplateRole(tu.Actions),
})
}
enterprise/coderd/userauth_test.go
+1 -1
View File
@@ -668,7 +668,7 @@ func TestGroupSync(t *testing.T) {
}
for _, group := range orgGroups {
userInGroup := slice.ContainsCompare(group.Members, codersdk.User{Email: user.Email}, func(a, b codersdk.User) bool {
userInGroup := slice.ContainsCompare(group.Members, codersdk.ReducedUser{Email: user.Email}, func(a, b codersdk.ReducedUser) bool {
return a.Email == b.Email
})
if group.IsEveryone() {