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.
This commit is contained in:
Spike Curtis 2023-10-10 16:55:28 +04:00 committed by GitHub
parent 19400d6794
commit db8592fa93
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 90 additions and 48 deletions

View File

@ -1177,13 +1177,13 @@ func userOrganizationIDs(ctx context.Context, api *API, user database.User) ([]u
return member.OrganizationIDs, nil
}
func findUser(id uuid.UUID, users []database.User) *database.User {
for _, u := range users {
if u.ID == id {
return &u
func usernameWithID(id uuid.UUID, users []database.User) (string, bool) {
for _, user := range users {
if id == user.ID {
return user.Username, true
}
}
return nil
return "", false
}
func convertAPIKey(k database.APIKey) codersdk.APIKey {

View File

@ -121,7 +121,7 @@ func (api *API) workspaceAgent(rw http.ResponseWriter, r *http.Request) {
}
apiAgent, err := convertWorkspaceAgent(
api.DERPMap(), *api.TailnetCoordinator.Load(), workspaceAgent, convertApps(dbApps, workspaceAgent, owner, workspace), convertScripts(scripts), convertLogSources(logSources), api.AgentInactiveDisconnectTimeout,
api.DERPMap(), *api.TailnetCoordinator.Load(), workspaceAgent, convertApps(dbApps, workspaceAgent, owner.Username, workspace), convertScripts(scripts), convertLogSources(logSources), api.AgentInactiveDisconnectTimeout,
api.DeploymentValues.AgentFallbackTroubleshootingURL.String(),
)
if err != nil {
@ -235,7 +235,7 @@ func (api *API) workspaceAgentManifest(rw http.ResponseWriter, r *http.Request)
httpapi.Write(ctx, rw, http.StatusOK, agentsdk.Manifest{
AgentID: apiAgent.ID,
Apps: convertApps(dbApps, workspaceAgent, owner, workspace),
Apps: convertApps(dbApps, workspaceAgent, owner.Username, workspace),
Scripts: convertScripts(scripts),
DERPMap: api.DERPMap(),
DERPForceWebSockets: api.DeploymentValues.DERP.Config.ForceWebSockets.Value(),
@ -1404,14 +1404,14 @@ func (api *API) workspaceAgentClientCoordinate(rw http.ResponseWriter, r *http.R
// convertProvisionedApps converts applications that are in the middle of provisioning process.
// It means that they may not have an agent or workspace assigned (dry-run job).
func convertProvisionedApps(dbApps []database.WorkspaceApp) []codersdk.WorkspaceApp {
return convertApps(dbApps, database.WorkspaceAgent{}, database.User{}, database.Workspace{})
return convertApps(dbApps, database.WorkspaceAgent{}, "", database.Workspace{})
}
func convertApps(dbApps []database.WorkspaceApp, agent database.WorkspaceAgent, owner database.User, workspace database.Workspace) []codersdk.WorkspaceApp {
func convertApps(dbApps []database.WorkspaceApp, agent database.WorkspaceAgent, ownerName string, workspace database.Workspace) []codersdk.WorkspaceApp {
apps := make([]codersdk.WorkspaceApp, 0)
for _, dbApp := range dbApps {
var subdomainName string
if dbApp.Subdomain && agent.Name != "" && owner.Username != "" && workspace.Name != "" {
if dbApp.Subdomain && agent.Name != "" && ownerName != "" && workspace.Name != "" {
appSlug := dbApp.Slug
if appSlug == "" {
appSlug = dbApp.DisplayName
@ -1420,7 +1420,7 @@ func convertApps(dbApps []database.WorkspaceApp, agent database.WorkspaceAgent,
AppSlugOrPort: appSlug,
AgentName: agent.Name,
WorkspaceName: workspace.Name,
Username: owner.Username,
Username: ownerName,
}.String()
}

View File

@ -68,12 +68,20 @@ func (api *API) workspaceBuild(rw http.ResponseWriter, r *http.Request) {
})
return
}
ownerName, ok := usernameWithID(workspace.OwnerID, data.users)
if !ok {
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
Message: "Internal error converting workspace build.",
Detail: "owner not found for workspace",
})
return
}
apiBuild, err := api.convertWorkspaceBuild(
workspaceBuild,
workspace,
data.jobs[0],
data.users,
ownerName,
data.resources,
data.metadata,
data.agents,
@ -274,12 +282,20 @@ func (api *API) workspaceBuildByBuildNumber(rw http.ResponseWriter, r *http.Requ
})
return
}
ownerName, ok := usernameWithID(workspace.OwnerID, data.users)
if !ok {
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
Message: "Internal error converting workspace build.",
Detail: "owner not found for workspace",
})
return
}
apiBuild, err := api.convertWorkspaceBuild(
workspaceBuild,
workspace,
data.jobs[0],
data.users,
ownerName,
data.resources,
data.metadata,
data.agents,
@ -398,6 +414,14 @@ func (api *API) postWorkspaceBuilds(rw http.ResponseWriter, r *http.Request) {
})
return
}
ownerName, exists := usernameWithID(workspace.OwnerID, users)
if !exists {
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
Message: "Internal error converting workspace build.",
Detail: "owner not found for workspace",
})
return
}
apiBuild, err := api.convertWorkspaceBuild(
*workspaceBuild,
@ -406,7 +430,7 @@ func (api *API) postWorkspaceBuilds(rw http.ResponseWriter, r *http.Request) {
ProvisionerJob: *provisionerJob,
QueuePosition: 0,
},
users,
ownerName,
[]database.WorkspaceResource{},
[]database.WorkspaceResourceMetadatum{},
[]database.WorkspaceAgent{},
@ -807,12 +831,16 @@ func (api *API) convertWorkspaceBuilds(
if !exists {
return nil, xerrors.New("template version not found")
}
ownerName, exists := usernameWithID(workspace.OwnerID, users)
if !exists {
return nil, xerrors.Errorf("owner not found for workspace: %q", workspace.Name)
}
apiBuild, err := api.convertWorkspaceBuild(
build,
workspace,
job,
users,
ownerName,
workspaceResources,
resourceMetadata,
resourceAgents,
@ -835,7 +863,7 @@ func (api *API) convertWorkspaceBuild(
build database.WorkspaceBuild,
workspace database.Workspace,
job database.GetProvisionerJobsByIDsWithQueuePositionRow,
users []database.User,
ownerName string,
workspaceResources []database.WorkspaceResource,
resourceMetadata []database.WorkspaceResourceMetadatum,
resourceAgents []database.WorkspaceAgent,
@ -844,10 +872,6 @@ func (api *API) convertWorkspaceBuild(
agentLogSources []database.WorkspaceAgentLogSource,
templateVersion database.TemplateVersion,
) (codersdk.WorkspaceBuild, error) {
userByID := map[uuid.UUID]database.User{}
for _, user := range users {
userByID[user.ID] = user
}
resourcesByJobID := map[uuid.UUID][]database.WorkspaceResource{}
for _, resource := range workspaceResources {
resourcesByJobID[resource.JobID] = append(resourcesByJobID[resource.JobID], resource)
@ -873,11 +897,6 @@ func (api *API) convertWorkspaceBuild(
logSourcesByAgentID[logSource.WorkspaceAgentID] = append(logSourcesByAgentID[logSource.WorkspaceAgentID], logSource)
}
owner, exists := userByID[workspace.OwnerID]
if !exists {
return codersdk.WorkspaceBuild{}, xerrors.Errorf("owner not found for workspace: %q", workspace.Name)
}
resources := resourcesByJobID[job.ProvisionerJob.ID]
apiResources := make([]codersdk.WorkspaceResource, 0)
for _, resource := range resources {
@ -888,7 +907,7 @@ func (api *API) convertWorkspaceBuild(
scripts := scriptsByAgentID[agent.ID]
logSources := logSourcesByAgentID[agent.ID]
apiAgent, err := convertWorkspaceAgent(
api.DERPMap(), *api.TailnetCoordinator.Load(), agent, convertApps(apps, agent, owner, workspace), convertScripts(scripts), convertLogSources(logSources), api.AgentInactiveDisconnectTimeout,
api.DERPMap(), *api.TailnetCoordinator.Load(), agent, convertApps(apps, agent, ownerName, workspace), convertScripts(scripts), convertLogSources(logSources), api.AgentInactiveDisconnectTimeout,
api.DeploymentValues.AgentFallbackTroubleshootingURL.String(),
)
if err != nil {
@ -906,7 +925,7 @@ func (api *API) convertWorkspaceBuild(
CreatedAt: build.CreatedAt,
UpdatedAt: build.UpdatedAt,
WorkspaceOwnerID: workspace.OwnerID,
WorkspaceOwnerName: owner.Username,
WorkspaceOwnerName: ownerName,
WorkspaceID: build.WorkspaceID,
WorkspaceName: workspace.Name,
TemplateVersionID: build.TemplateVersionID,

View File

@ -92,12 +92,19 @@ func (api *API) workspace(rw http.ResponseWriter, r *http.Request) {
httpapi.Forbidden(rw)
return
}
ownerName, ok := usernameWithID(workspace.OwnerID, data.users)
if !ok {
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
Message: "Internal error fetching workspace resources.",
Detail: "unable to find workspace owner's username",
})
return
}
httpapi.Write(ctx, rw, http.StatusOK, convertWorkspace(
workspace,
data.builds[0],
data.templates[0],
findUser(workspace.OwnerID, data.users),
ownerName,
))
}
@ -274,12 +281,19 @@ func (api *API) workspaceByOwnerAndName(rw http.ResponseWriter, r *http.Request)
httpapi.ResourceNotFound(rw)
return
}
ownerName, ok := usernameWithID(workspace.OwnerID, data.users)
if !ok {
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
Message: "Internal error fetching workspace resources.",
Detail: "unable to find workspace owner's username",
})
return
}
httpapi.Write(ctx, rw, http.StatusOK, convertWorkspace(
workspace,
data.builds[0],
data.templates[0],
findUser(workspace.OwnerID, data.users),
ownerName,
))
}
@ -529,21 +543,11 @@ func (api *API) postWorkspacesByOrganization(rw http.ResponseWriter, r *http.Req
}
aReq.New = workspace
initiator, err := api.Database.GetUserByID(ctx, workspaceBuild.InitiatorID)
if err != nil {
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
Message: "Internal error fetching user.",
Detail: err.Error(),
})
return
}
api.Telemetry.Report(&telemetry.Snapshot{
Workspaces: []telemetry.Workspace{telemetry.ConvertWorkspace(workspace)},
WorkspaceBuilds: []telemetry.WorkspaceBuild{telemetry.ConvertWorkspaceBuild(*workspaceBuild)},
})
users := []database.User{user, initiator}
apiBuild, err := api.convertWorkspaceBuild(
*workspaceBuild,
workspace,
@ -551,7 +555,7 @@ func (api *API) postWorkspacesByOrganization(rw http.ResponseWriter, r *http.Req
ProvisionerJob: *provisionerJob,
QueuePosition: 0,
},
users,
user.Username,
[]database.WorkspaceResource{},
[]database.WorkspaceResourceMetadatum{},
[]database.WorkspaceAgent{},
@ -572,7 +576,7 @@ func (api *API) postWorkspacesByOrganization(rw http.ResponseWriter, r *http.Req
workspace,
apiBuild,
template,
findUser(user.ID, users),
user.Username,
))
}
@ -885,6 +889,14 @@ func (api *API) putWorkspaceDormant(rw http.ResponseWriter, r *http.Request) {
})
return
}
ownerName, ok := usernameWithID(workspace.OwnerID, data.users)
if !ok {
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
Message: "Internal error fetching workspace resources.",
Detail: "unable to find workspace owner's username",
})
return
}
if len(data.templates) == 0 {
httpapi.Forbidden(rw)
@ -896,7 +908,7 @@ func (api *API) putWorkspaceDormant(rw http.ResponseWriter, r *http.Request) {
workspace,
data.builds[0],
data.templates[0],
findUser(workspace.OwnerID, data.users),
ownerName,
))
}
@ -1111,13 +1123,24 @@ func (api *API) watchWorkspace(rw http.ResponseWriter, r *http.Request) {
return
}
ownerName, ok := usernameWithID(workspace.OwnerID, data.users)
if !ok {
_ = sendEvent(ctx, codersdk.ServerSentEvent{
Type: codersdk.ServerSentEventTypeError,
Data: codersdk.Response{
Message: "Internal error fetching workspace resources.",
Detail: "unable to find workspace owner's username",
},
})
return
}
_ = sendEvent(ctx, codersdk.ServerSentEvent{
Type: codersdk.ServerSentEventTypeData,
Data: convertWorkspace(
workspace,
data.builds[0],
data.templates[0],
findUser(workspace.OwnerID, data.users),
ownerName,
),
})
}
@ -1267,7 +1290,7 @@ func convertWorkspaces(workspaces []database.Workspace, data workspaceData) ([]c
workspace,
build,
template,
&owner,
owner.Username,
))
}
return apiWorkspaces, nil
@ -1277,7 +1300,7 @@ func convertWorkspace(
workspace database.Workspace,
workspaceBuild codersdk.WorkspaceBuild,
template database.Template,
owner *database.User,
ownerName string,
) codersdk.Workspace {
var autostartSchedule *string
if workspace.AutostartSchedule.Valid {
@ -1310,7 +1333,7 @@ func convertWorkspace(
CreatedAt: workspace.CreatedAt,
UpdatedAt: workspace.UpdatedAt,
OwnerID: workspace.OwnerID,
OwnerName: owner.Username,
OwnerName: ownerName,
OrganizationID: workspace.OrganizationID,
TemplateID: workspace.TemplateID,
LatestBuild: workspaceBuild,