Skip to content

Commit db8592f

Browse files
spikecurtisspikecurtis
authored
chore: refactor workspace conversion to accept ownerName (#10171)
Refactors workspace conversion to accept the ownerName, rather than a slice of users, since all it does is search the slice for the owner and use the username. This is in preparation for a fix to `postWorkspacesByOrganization()` that will remove the need to pass the user object. Also avoids panicing if the required user is not in the slice, since `findUser` could return nil in the old code, which would then get dereferenced for the username.
1 parent 19400d6 commit db8592f

File tree

4 files changed

+90
-48
lines changed

4 files changed

+90
-48
lines changed

‎coderd/users.go

+5-5
Original file line numberDiff line numberDiff line change
@@ -1177,13 +1177,13 @@ func userOrganizationIDs(ctx context.Context, api *API, user database.User) ([]u
11771177
return member.OrganizationIDs, nil
11781178
}
11791179

1180-
func findUser(id uuid.UUID, users []database.User) *database.User {
1181-
for _, u := range users {
1182-
if u.ID == id {
1183-
return &u
1180+
func usernameWithID(id uuid.UUID, users []database.User) (string, bool) {
1181+
for _, user := range users {
1182+
if id == user.ID {
1183+
return user.Username, true
11841184
}
11851185
}
1186-
return nil
1186+
return "", false
11871187
}
11881188

11891189
func convertAPIKey(k database.APIKey) codersdk.APIKey {

‎coderd/workspaceagents.go

+6-6
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ func (api *API) workspaceAgent(rw http.ResponseWriter, r *http.Request) {
121121
}
122122

123123
apiAgent, err := convertWorkspaceAgent(
124-
api.DERPMap(), *api.TailnetCoordinator.Load(), workspaceAgent, convertApps(dbApps, workspaceAgent, owner, workspace), convertScripts(scripts), convertLogSources(logSources), api.AgentInactiveDisconnectTimeout,
124+
api.DERPMap(), *api.TailnetCoordinator.Load(), workspaceAgent, convertApps(dbApps, workspaceAgent, owner.Username, workspace), convertScripts(scripts), convertLogSources(logSources), api.AgentInactiveDisconnectTimeout,
125125
api.DeploymentValues.AgentFallbackTroubleshootingURL.String(),
126126
)
127127
if err != nil {
@@ -235,7 +235,7 @@ func (api *API) workspaceAgentManifest(rw http.ResponseWriter, r *http.Request)
235235

236236
httpapi.Write(ctx, rw, http.StatusOK, agentsdk.Manifest{
237237
AgentID: apiAgent.ID,
238-
Apps: convertApps(dbApps, workspaceAgent, owner, workspace),
238+
Apps: convertApps(dbApps, workspaceAgent, owner.Username, workspace),
239239
Scripts: convertScripts(scripts),
240240
DERPMap: api.DERPMap(),
241241
DERPForceWebSockets: api.DeploymentValues.DERP.Config.ForceWebSockets.Value(),
@@ -1404,14 +1404,14 @@ func (api *API) workspaceAgentClientCoordinate(rw http.ResponseWriter, r *http.R
14041404
// convertProvisionedApps converts applications that are in the middle of provisioning process.
14051405
// It means that they may not have an agent or workspace assigned (dry-run job).
14061406
func convertProvisionedApps(dbApps []database.WorkspaceApp) []codersdk.WorkspaceApp {
1407-
return convertApps(dbApps, database.WorkspaceAgent{}, database.User{}, database.Workspace{})
1407+
return convertApps(dbApps, database.WorkspaceAgent{}, "", database.Workspace{})
14081408
}
14091409

1410-
func convertApps(dbApps []database.WorkspaceApp, agent database.WorkspaceAgent, owner database.User, workspace database.Workspace) []codersdk.WorkspaceApp {
1410+
func convertApps(dbApps []database.WorkspaceApp, agent database.WorkspaceAgent, ownerName string, workspace database.Workspace) []codersdk.WorkspaceApp {
14111411
apps := make([]codersdk.WorkspaceApp, 0)
14121412
for _, dbApp := range dbApps {
14131413
var subdomainName string
1414-
if dbApp.Subdomain && agent.Name != "" && owner.Username != "" && workspace.Name != "" {
1414+
if dbApp.Subdomain && agent.Name != "" && ownerName != "" && workspace.Name != "" {
14151415
appSlug := dbApp.Slug
14161416
if appSlug == "" {
14171417
appSlug = dbApp.DisplayName
@@ -1420,7 +1420,7 @@ func convertApps(dbApps []database.WorkspaceApp, agent database.WorkspaceAgent,
14201420
AppSlugOrPort: appSlug,
14211421
AgentName: agent.Name,
14221422
WorkspaceName: workspace.Name,
1423-
Username: owner.Username,
1423+
Username: ownerName,
14241424
}.String()
14251425
}
14261426

‎coderd/workspacebuilds.go

+35-16
Original file line numberDiff line numberDiff line change
@@ -68,12 +68,20 @@ func (api *API) workspaceBuild(rw http.ResponseWriter, r *http.Request) {
6868
})
6969
return
7070
}
71+
ownerName, ok := usernameWithID(workspace.OwnerID, data.users)
72+
if !ok {
73+
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
74+
Message: "Internal error converting workspace build.",
75+
Detail: "owner not found for workspace",
76+
})
77+
return
78+
}
7179

7280
apiBuild, err := api.convertWorkspaceBuild(
7381
workspaceBuild,
7482
workspace,
7583
data.jobs[0],
76-
data.users,
84+
ownerName,
7785
data.resources,
7886
data.metadata,
7987
data.agents,
@@ -274,12 +282,20 @@ func (api *API) workspaceBuildByBuildNumber(rw http.ResponseWriter, r *http.Requ
274282
})
275283
return
276284
}
285+
ownerName, ok := usernameWithID(workspace.OwnerID, data.users)
286+
if !ok {
287+
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
288+
Message: "Internal error converting workspace build.",
289+
Detail: "owner not found for workspace",
290+
})
291+
return
292+
}
277293

278294
apiBuild, err := api.convertWorkspaceBuild(
279295
workspaceBuild,
280296
workspace,
281297
data.jobs[0],
282-
data.users,
298+
ownerName,
283299
data.resources,
284300
data.metadata,
285301
data.agents,
@@ -398,6 +414,14 @@ func (api *API) postWorkspaceBuilds(rw http.ResponseWriter, r *http.Request) {
398414
})
399415
return
400416
}
417+
ownerName, exists := usernameWithID(workspace.OwnerID, users)
418+
if !exists {
419+
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
420+
Message: "Internal error converting workspace build.",
421+
Detail: "owner not found for workspace",
422+
})
423+
return
424+
}
401425

402426
apiBuild, err := api.convertWorkspaceBuild(
403427
*workspaceBuild,
@@ -406,7 +430,7 @@ func (api *API) postWorkspaceBuilds(rw http.ResponseWriter, r *http.Request) {
406430
ProvisionerJob: *provisionerJob,
407431
QueuePosition: 0,
408432
},
409-
users,
433+
ownerName,
410434
[]database.WorkspaceResource{},
411435
[]database.WorkspaceResourceMetadatum{},
412436
[]database.WorkspaceAgent{},
@@ -807,12 +831,16 @@ func (api *API) convertWorkspaceBuilds(
807831
if !exists {
808832
return nil, xerrors.New("template version not found")
809833
}
834+
ownerName, exists := usernameWithID(workspace.OwnerID, users)
835+
if !exists {
836+
return nil, xerrors.Errorf("owner not found for workspace: %q", workspace.Name)
837+
}
810838

811839
apiBuild, err := api.convertWorkspaceBuild(
812840
build,
813841
workspace,
814842
job,
815-
users,
843+
ownerName,
816844
workspaceResources,
817845
resourceMetadata,
818846
resourceAgents,
@@ -835,7 +863,7 @@ func (api *API) convertWorkspaceBuild(
835863
build database.WorkspaceBuild,
836864
workspace database.Workspace,
837865
job database.GetProvisionerJobsByIDsWithQueuePositionRow,
838-
users []database.User,
866+
ownerName string,
839867
workspaceResources []database.WorkspaceResource,
840868
resourceMetadata []database.WorkspaceResourceMetadatum,
841869
resourceAgents []database.WorkspaceAgent,
@@ -844,10 +872,6 @@ func (api *API) convertWorkspaceBuild(
844872
agentLogSources []database.WorkspaceAgentLogSource,
845873
templateVersion database.TemplateVersion,
846874
) (codersdk.WorkspaceBuild, error) {
847-
userByID := map[uuid.UUID]database.User{}
848-
for _, user := range users {
849-
userByID[user.ID] = user
850-
}
851875
resourcesByJobID := map[uuid.UUID][]database.WorkspaceResource{}
852876
for _, resource := range workspaceResources {
853877
resourcesByJobID[resource.JobID] = append(resourcesByJobID[resource.JobID], resource)
@@ -873,11 +897,6 @@ func (api *API) convertWorkspaceBuild(
873897
logSourcesByAgentID[logSource.WorkspaceAgentID] = append(logSourcesByAgentID[logSource.WorkspaceAgentID], logSource)
874898
}
875899

876-
owner, exists := userByID[workspace.OwnerID]
877-
if !exists {
878-
return codersdk.WorkspaceBuild{}, xerrors.Errorf("owner not found for workspace: %q", workspace.Name)
879-
}
880-
881900
resources := resourcesByJobID[job.ProvisionerJob.ID]
882901
apiResources := make([]codersdk.WorkspaceResource, 0)
883902
for _, resource := range resources {
@@ -888,7 +907,7 @@ func (api *API) convertWorkspaceBuild(
888907
scripts := scriptsByAgentID[agent.ID]
889908
logSources := logSourcesByAgentID[agent.ID]
890909
apiAgent, err := convertWorkspaceAgent(
891-
api.DERPMap(), *api.TailnetCoordinator.Load(), agent, convertApps(apps, agent, owner, workspace), convertScripts(scripts), convertLogSources(logSources), api.AgentInactiveDisconnectTimeout,
910+
api.DERPMap(), *api.TailnetCoordinator.Load(), agent, convertApps(apps, agent, ownerName, workspace), convertScripts(scripts), convertLogSources(logSources), api.AgentInactiveDisconnectTimeout,
892911
api.DeploymentValues.AgentFallbackTroubleshootingURL.String(),
893912
)
894913
if err != nil {
@@ -906,7 +925,7 @@ func (api *API) convertWorkspaceBuild(
906925
CreatedAt: build.CreatedAt,
907926
UpdatedAt: build.UpdatedAt,
908927
WorkspaceOwnerID: workspace.OwnerID,
909-
WorkspaceOwnerName: owner.Username,
928+
WorkspaceOwnerName: ownerName,
910929
WorkspaceID: build.WorkspaceID,
911930
WorkspaceName: workspace.Name,
912931
TemplateVersionID: build.TemplateVersionID,

‎coderd/workspaces.go

+44-21
Original file line numberDiff line numberDiff line change
@@ -92,12 +92,19 @@ func (api *API) workspace(rw http.ResponseWriter, r *http.Request) {
9292
httpapi.Forbidden(rw)
9393
return
9494
}
95-
95+
ownerName, ok := usernameWithID(workspace.OwnerID, data.users)
96+
if !ok {
97+
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
98+
Message: "Internal error fetching workspace resources.",
99+
Detail: "unable to find workspace owner's username",
100+
})
101+
return
102+
}
96103
httpapi.Write(ctx, rw, http.StatusOK, convertWorkspace(
97104
workspace,
98105
data.builds[0],
99106
data.templates[0],
100-
findUser(workspace.OwnerID, data.users),
107+
ownerName,
101108
))
102109
}
103110

@@ -274,12 +281,19 @@ func (api *API) workspaceByOwnerAndName(rw http.ResponseWriter, r *http.Request)
274281
httpapi.ResourceNotFound(rw)
275282
return
276283
}
277-
284+
ownerName, ok := usernameWithID(workspace.OwnerID, data.users)
285+
if !ok {
286+
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
287+
Message: "Internal error fetching workspace resources.",
288+
Detail: "unable to find workspace owner's username",
289+
})
290+
return
291+
}
278292
httpapi.Write(ctx, rw, http.StatusOK, convertWorkspace(
279293
workspace,
280294
data.builds[0],
281295
data.templates[0],
282-
findUser(workspace.OwnerID, data.users),
296+
ownerName,
283297
))
284298
}
285299

@@ -529,29 +543,19 @@ func (api *API) postWorkspacesByOrganization(rw http.ResponseWriter, r *http.Req
529543
}
530544
aReq.New = workspace
531545

532-
initiator, err := api.Database.GetUserByID(ctx, workspaceBuild.InitiatorID)
533-
if err != nil {
534-
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
535-
Message: "Internal error fetching user.",
536-
Detail: err.Error(),
537-
})
538-
return
539-
}
540-
541546
api.Telemetry.Report(&telemetry.Snapshot{
542547
Workspaces: []telemetry.Workspace{telemetry.ConvertWorkspace(workspace)},
543548
WorkspaceBuilds: []telemetry.WorkspaceBuild{telemetry.ConvertWorkspaceBuild(*workspaceBuild)},
544549
})
545550

546-
users := []database.User{user, initiator}
547551
apiBuild, err := api.convertWorkspaceBuild(
548552
*workspaceBuild,
549553
workspace,
550554
database.GetProvisionerJobsByIDsWithQueuePositionRow{
551555
ProvisionerJob: *provisionerJob,
552556
QueuePosition: 0,
553557
},
554-
users,
558+
user.Username,
555559
[]database.WorkspaceResource{},
556560
[]database.WorkspaceResourceMetadatum{},
557561
[]database.WorkspaceAgent{},
@@ -572,7 +576,7 @@ func (api *API) postWorkspacesByOrganization(rw http.ResponseWriter, r *http.Req
572576
workspace,
573577
apiBuild,
574578
template,
575-
findUser(user.ID, users),
579+
user.Username,
576580
))
577581
}
578582

@@ -885,6 +889,14 @@ func (api *API) putWorkspaceDormant(rw http.ResponseWriter, r *http.Request) {
885889
})
886890
return
887891
}
892+
ownerName, ok := usernameWithID(workspace.OwnerID, data.users)
893+
if !ok {
894+
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
895+
Message: "Internal error fetching workspace resources.",
896+
Detail: "unable to find workspace owner's username",
897+
})
898+
return
899+
}
888900

889901
if len(data.templates) == 0 {
890902
httpapi.Forbidden(rw)
@@ -896,7 +908,7 @@ func (api *API) putWorkspaceDormant(rw http.ResponseWriter, r *http.Request) {
896908
workspace,
897909
data.builds[0],
898910
data.templates[0],
899-
findUser(workspace.OwnerID, data.users),
911+
ownerName,
900912
))
901913
}
902914

@@ -1111,13 +1123,24 @@ func (api *API) watchWorkspace(rw http.ResponseWriter, r *http.Request) {
11111123
return
11121124
}
11131125

1126+
ownerName, ok := usernameWithID(workspace.OwnerID, data.users)
1127+
if !ok {
1128+
_ = sendEvent(ctx, codersdk.ServerSentEvent{
1129+
Type: codersdk.ServerSentEventTypeError,
1130+
Data: codersdk.Response{
1131+
Message: "Internal error fetching workspace resources.",
1132+
Detail: "unable to find workspace owner's username",
1133+
},
1134+
})
1135+
return
1136+
}
11141137
_ = sendEvent(ctx, codersdk.ServerSentEvent{
11151138
Type: codersdk.ServerSentEventTypeData,
11161139
Data: convertWorkspace(
11171140
workspace,
11181141
data.builds[0],
11191142
data.templates[0],
1120-
findUser(workspace.OwnerID, data.users),
1143+
ownerName,
11211144
),
11221145
})
11231146
}
@@ -1267,7 +1290,7 @@ func convertWorkspaces(workspaces []database.Workspace, data workspaceData) ([]c
12671290
workspace,
12681291
build,
12691292
template,
1270-
&owner,
1293+
owner.Username,
12711294
))
12721295
}
12731296
return apiWorkspaces, nil
@@ -1277,7 +1300,7 @@ func convertWorkspace(
12771300
workspace database.Workspace,
12781301
workspaceBuild codersdk.WorkspaceBuild,
12791302
template database.Template,
1280-
owner *database.User,
1303+
ownerName string,
12811304
) codersdk.Workspace {
12821305
var autostartSchedule *string
12831306
if workspace.AutostartSchedule.Valid {
@@ -1310,7 +1333,7 @@ func convertWorkspace(
13101333
CreatedAt: workspace.CreatedAt,
13111334
UpdatedAt: workspace.UpdatedAt,
13121335
OwnerID: workspace.OwnerID,
1313-
OwnerName: owner.Username,
1336+
OwnerName: ownerName,
13141337
OrganizationID: workspace.OrganizationID,
13151338
TemplateID: workspace.TemplateID,
13161339
LatestBuild: workspaceBuild,

0 commit comments

Comments
 (0)