Skip to content

chore: refactor workspace conversion to accept ownerName #10171

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 10, 2023
Merged

chore: refactor workspace conversion to accept ownerName #10171

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
44aad4b
chore: drop unused redirectToLoginOnMe parameter
spikecurtis Oct 10, 2023
048f310
chore: refactor workspace conversion to accept ownerName
spikecurtis Oct 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions coderd/coderd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -652,7 +652,7 @@ func New(options *Options) *API {
r.Get("/roles", api.assignableOrgRoles)
r.Route("/{user}", func(r chi.Router) {
r.Use(
httpmw.ExtractUserParam(options.Database, false),
httpmw.ExtractUserParam(options.Database),
httpmw.ExtractOrganizationMemberParam(options.Database),
)
r.Put("/roles", api.putMemberRoles)
Expand Down Expand Up @@ -741,7 +741,7 @@ func New(options *Options) *API {
r.Get("/", api.assignableSiteRoles)
})
r.Route("/{user}", func(r chi.Router) {
r.Use(httpmw.ExtractUserParam(options.Database, false))
r.Use(httpmw.ExtractUserParam(options.Database))
r.Post("/convert-login", api.postConvertLoginType)
r.Delete("/", api.deleteUser)
r.Get("/", api.userByName)
Expand Down
4 changes: 2 additions & 2 deletions coderd/httpmw/organizationparam_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ func TestOrganizationParam(t *testing.T) {
DB: db,
RedirectToLogin: false,
}),
httpmw.ExtractUserParam(db, false),
httpmw.ExtractUserParam(db),
httpmw.ExtractOrganizationParam(db),
httpmw.ExtractOrganizationMemberParam(db),
)
Expand Down Expand Up @@ -157,7 +157,7 @@ func TestOrganizationParam(t *testing.T) {
RedirectToLogin: false,
}),
httpmw.ExtractOrganizationParam(db),
httpmw.ExtractUserParam(db, false),
httpmw.ExtractUserParam(db),
httpmw.ExtractOrganizationMemberParam(db),
)
rtr.Get("/", func(rw http.ResponseWriter, r *http.Request) {
Expand Down
15 changes: 3 additions & 12 deletions coderd/httpmw/userparam.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,17 +34,15 @@ func UserParam(r *http.Request) database.User {

// ExtractUserParam extracts a user from an ID/username in the {user} URL
// parameter.
//
//nolint:revive
func ExtractUserParam(db database.Store, redirectToLoginOnMe bool) func(http.Handler) http.Handler {
func ExtractUserParam(db database.Store) func(http.Handler) http.Handler {
return func(next http.Handler) http.Handler {
return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
ctx := r.Context()
// We need to call as SystemRestricted because this middleware is called from
// organizations/{organization}/members/{user}/ paths, and we need to allow
// org-admins to call these paths --- they might not have sitewide read permissions on users.
// nolint:gocritic
user, ok := extractUserContext(dbauthz.AsSystemRestricted(ctx), db, rw, r, redirectToLoginOnMe)
user, ok := extractUserContext(dbauthz.AsSystemRestricted(ctx), db, rw, r)
if !ok {
// response already handled
return
Expand All @@ -56,9 +54,7 @@ func ExtractUserParam(db database.Store, redirectToLoginOnMe bool) func(http.Han
}

// extractUserContext queries the database for the parameterized `{user}` from the request URL.
//
//nolint:revive
func extractUserContext(ctx context.Context, db database.Store, rw http.ResponseWriter, r *http.Request, redirectToLoginOnMe bool) (user database.User, ok bool) {
func extractUserContext(ctx context.Context, db database.Store, rw http.ResponseWriter, r *http.Request) (user database.User, ok bool) {
// userQuery is either a uuid, a username, or 'me'
userQuery := chi.URLParam(r, "user")
if userQuery == "" {
Expand All @@ -71,11 +67,6 @@ func extractUserContext(ctx context.Context, db database.Store, rw http.Response
if userQuery == "me" {
apiKey, ok := APIKeyOptional(r)
if !ok {
if redirectToLoginOnMe {
RedirectToLogin(rw, r, nil, SignedOutErrorMessage)
return database.User{}, false
}

httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
Message: "Cannot use \"me\" without a valid session.",
})
Expand Down
6 changes: 3 additions & 3 deletions coderd/httpmw/userparam_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func TestUserParam(t *testing.T) {
r = returnedRequest
})).ServeHTTP(rw, r)

httpmw.ExtractUserParam(db, false)(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
httpmw.ExtractUserParam(db)(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
rw.WriteHeader(http.StatusOK)
})).ServeHTTP(rw, r)
res := rw.Result()
Expand All @@ -66,7 +66,7 @@ func TestUserParam(t *testing.T) {
routeContext := chi.NewRouteContext()
routeContext.URLParams.Add("user", "ben")
r = r.WithContext(context.WithValue(r.Context(), chi.RouteCtxKey, routeContext))
httpmw.ExtractUserParam(db, false)(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
httpmw.ExtractUserParam(db)(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
rw.WriteHeader(http.StatusOK)
})).ServeHTTP(rw, r)
res := rw.Result()
Expand All @@ -88,7 +88,7 @@ func TestUserParam(t *testing.T) {
routeContext := chi.NewRouteContext()
routeContext.URLParams.Add("user", "me")
r = r.WithContext(context.WithValue(r.Context(), chi.RouteCtxKey, routeContext))
httpmw.ExtractUserParam(db, false)(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
httpmw.ExtractUserParam(db)(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
_ = httpmw.UserParam(r)
rw.WriteHeader(http.StatusOK)
})).ServeHTTP(rw, r)
Expand Down
2 changes: 1 addition & 1 deletion coderd/httpmw/workspaceparam_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ func TestWorkspaceAgentByNameParam(t *testing.T) {
DB: db,
RedirectToLogin: true,
}),
httpmw.ExtractUserParam(db, false),
httpmw.ExtractUserParam(db),
httpmw.ExtractWorkspaceAndAgentParam(db),
)
rtr.Get("/", func(w http.ResponseWriter, r *http.Request) {
Expand Down
10 changes: 5 additions & 5 deletions coderd/users.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
12 changes: 6 additions & 6 deletions coderd/workspaceagents.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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
Expand All @@ -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()
}

Expand Down
51 changes: 35 additions & 16 deletions coderd/workspacebuilds.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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{},
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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)
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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,
Expand Down
Loading