shishantbiswas/coder
1
0
Fork 0
mirror of https://github.com/coder/coder.git synced 2026-06-02 20:48:20 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor: untangle workspace creation from http logic (#19639)

Browse Source 
Coder Tasks requires us to create a workspace, but we want to be able to
return a `codersdk.Task` instead of a `codersdk.Workspace`. This
requires untangling `createWorkspace` from directly writing to
`http.ResponseWriter`.
This commit is contained in:
Danielle Maywood
2025-08-29 14:17:33 +01:00
committed by GitHub
parent 7365da1110
commit 29a731375e
3 changed files with 106 additions and 57 deletions
Download Patch File Download Diff File Expand all files Collapse all files
coderd/aitasks.go
+11 -3
View File
@@ -17,6 +17,7 @@ import (
"github.com/coder/coder/v2/coderd/audit"
"github.com/coder/coder/v2/coderd/database"
"github.com/coder/coder/v2/coderd/httpapi"
"github.com/coder/coder/v2/coderd/httpapi/httperror"
"github.com/coder/coder/v2/coderd/httpmw"
"github.com/coder/coder/v2/coderd/rbac"
"github.com/coder/coder/v2/coderd/rbac/policy"
@@ -154,8 +155,9 @@ func (api *API) tasksCreate(rw http.ResponseWriter, r *http.Request) {
// This can be optimized. It exists as it is now for code simplicity.
// The most common case is to create a workspace for 'Me'. Which does
// not enter this code branch.
template, ok := requestTemplate(ctx, rw, createReq, api.Database)
if !ok {
template, err := requestTemplate(ctx, createReq, api.Database)
if err != nil {
httperror.WriteResponseError(ctx, rw, err)
return
}
@@ -188,7 +190,13 @@ func (api *API) tasksCreate(rw http.ResponseWriter, r *http.Request) {
})
defer commitAudit()
createWorkspace(ctx, aReq, apiKey.UserID, api, owner, createReq, rw, r)
w, err := createWorkspace(ctx, aReq, apiKey.UserID, api, owner, createReq, r)
if err != nil {
httperror.WriteResponseError(ctx, rw, err)
return
}
httpapi.Write(ctx, rw, http.StatusCreated, w)
}
// tasksFromWorkspaces converts a slice of API workspaces into tasks, fetching
coderd/httpapi/httperror/responserror.go
+49
View File
@@ -1,8 +1,12 @@
package httperror
import (
"context"
"errors"
"fmt"
"net/http"
"github.com/coder/coder/v2/coderd/httpapi"
"github.com/coder/coder/v2/codersdk"
)
@@ -17,3 +21,48 @@ func IsResponder(err error) (Responder, bool) {
}
return nil, false
}
func NewResponseError(status int, resp codersdk.Response) error {
return &responseError{
status: status,
response: resp,
}
}
func WriteResponseError(ctx context.Context, rw http.ResponseWriter, err error) {
if responseErr, ok := IsResponder(err); ok {
code, resp := responseErr.Response()
httpapi.Write(ctx, rw, code, resp)
return
}
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
Message: "Internal server error",
Detail: err.Error(),
})
}
type responseError struct {
status int
response codersdk.Response
}
var (
_ error = (*responseError)(nil)
_ Responder = (*responseError)(nil)
)
func (e *responseError) Error() string {
return fmt.Sprintf("%s: %s", e.response.Message, e.response.Detail)
}
func (e *responseError) Status() int {
return e.status
}
func (e *responseError) Response() (int, codersdk.Response) {
return e.status, e.response
}
var ErrResourceNotFound = NewResponseError(http.StatusNotFound, httpapi.ResourceNotFoundResponse)
coderd/workspaces.go
+46 -54
View File
@@ -388,7 +388,13 @@ func (api *API) postWorkspacesByOrganization(rw http.ResponseWriter, r *http.Req
AvatarURL: member.AvatarURL,
}
createWorkspace(ctx, aReq, apiKey.UserID, api, owner, req, rw, r)
w, err := createWorkspace(ctx, aReq, apiKey.UserID, api, owner, req, r)
if err != nil {
httperror.WriteResponseError(ctx, rw, err)
return
}
httpapi.Write(ctx, rw, http.StatusCreated, w)
}
// Create a new workspace for the currently authenticated user.
@@ -442,8 +448,9 @@ func (api *API) postUserWorkspaces(rw http.ResponseWriter, r *http.Request) {
// This can be optimized. It exists as it is now for code simplicity.
// The most common case is to create a workspace for 'Me'. Which does
// not enter this code branch.
template, ok := requestTemplate(ctx, rw, req, api.Database)
if !ok {
template, err := requestTemplate(ctx, req, api.Database)
if err != nil {
httperror.WriteResponseError(ctx, rw, err)
return
}
@@ -476,7 +483,14 @@ func (api *API) postUserWorkspaces(rw http.ResponseWriter, r *http.Request) {
})
defer commitAudit()
createWorkspace(ctx, aReq, apiKey.UserID, api, owner, req, rw, r)
w, err := createWorkspace(ctx, aReq, apiKey.UserID, api, owner, req, r)
if err != nil {
httperror.WriteResponseError(ctx, rw, err)
return
}
httpapi.Write(ctx, rw, http.StatusCreated, w)
}
type workspaceOwner struct {
@@ -492,12 +506,11 @@ func createWorkspace(
api *API,
owner workspaceOwner,
req codersdk.CreateWorkspaceRequest,
rw http.ResponseWriter,
r *http.Request,
) {
template, ok := requestTemplate(ctx, rw, req, api.Database)
if !ok {
return
) (codersdk.Workspace, error) {
template, err := requestTemplate(ctx, req, api.Database)
if err != nil {
return codersdk.Workspace{}, err
}
// This is a premature auth check to avoid doing unnecessary work if the user
@@ -506,14 +519,12 @@ func createWorkspace(
rbac.ResourceWorkspace.InOrg(template.OrganizationID).WithOwner(owner.ID.String())) {
// If this check fails, return a proper unauthorized error to the user to indicate
// what is going on.
httpapi.Write(ctx, rw, http.StatusForbidden, codersdk.Response{
return codersdk.Workspace{}, httperror.NewResponseError(http.StatusForbidden, codersdk.Response{
Message: "Unauthorized to create workspace.",
Detail: "You are unable to create a workspace in this organization. " +
"It is possible to have access to the template, but not be able to create a workspace. " +
"Please contact an administrator about your permissions if you feel this is an error.",
Validations: nil,
})
return
}
// Update audit log's organization
@@ -523,49 +534,42 @@ func createWorkspace(
// would be wasted.
if !api.Authorize(r, policy.ActionCreate,
rbac.ResourceWorkspace.InOrg(template.OrganizationID).WithOwner(owner.ID.String())) {
httpapi.ResourceNotFound(rw)
return
return codersdk.Workspace{}, httperror.ErrResourceNotFound
}
// The user also needs permission to use the template. At this point they have
// read perms, but not necessarily "use". This is also checked in `db.InsertWorkspace`.
// Doing this up front can save some work below if the user doesn't have permission.
if !api.Authorize(r, policy.ActionUse, template) {
httpapi.Write(ctx, rw, http.StatusForbidden, codersdk.Response{
return codersdk.Workspace{}, httperror.NewResponseError(http.StatusForbidden, codersdk.Response{
Message: fmt.Sprintf("Unauthorized access to use the template %q.", template.Name),
Detail: "Although you are able to view the template, you are unable to create a workspace using it. " +
"Please contact an administrator about your permissions if you feel this is an error.",
Validations: nil,
})
return
}
templateAccessControl := (*(api.AccessControlStore.Load())).GetTemplateAccessControl(template)
if templateAccessControl.IsDeprecated() {
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
return codersdk.Workspace{}, httperror.NewResponseError(http.StatusBadRequest, codersdk.Response{
Message: fmt.Sprintf("Template %q has been deprecated, and cannot be used to create a new workspace.", template.Name),
// Pass the deprecated message to the user.
Detail: templateAccessControl.Deprecated,
Validations: nil,
Detail: templateAccessControl.Deprecated,
})
return
}
dbAutostartSchedule, err := validWorkspaceSchedule(req.AutostartSchedule)
if err != nil {
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
return codersdk.Workspace{}, httperror.NewResponseError(http.StatusBadRequest, codersdk.Response{
Message: "Invalid Autostart Schedule.",
Validations: []codersdk.ValidationError{{Field: "schedule", Detail: err.Error()}},
})
return
}
templateSchedule, err := (*api.TemplateScheduleStore.Load()).Get(ctx, api.Database, template.ID)
if err != nil {
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
return codersdk.Workspace{}, httperror.NewResponseError(http.StatusInternalServerError, codersdk.Response{
Message: "Internal error fetching template schedule.",
Detail: err.Error(),
})
return
}
nextStartAt := sql.NullTime{}
@@ -578,11 +582,10 @@ func createWorkspace(
dbTTL, err := validWorkspaceTTLMillis(req.TTLMillis, templateSchedule.DefaultTTL)
if err != nil {
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
return codersdk.Workspace{}, httperror.NewResponseError(http.StatusBadRequest, codersdk.Response{
Message: "Invalid Workspace Time to Shutdown.",
Validations: []codersdk.ValidationError{{Field: "ttl_ms", Detail: err.Error()}},
})
return
}
// back-compatibility: default to "never" if not included.
@@ -590,11 +593,10 @@ func createWorkspace(
if req.AutomaticUpdates != "" {
dbAU, err = validWorkspaceAutomaticUpdates(req.AutomaticUpdates)
if err != nil {
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
return codersdk.Workspace{}, httperror.NewResponseError(http.StatusBadRequest, codersdk.Response{
Message: "Invalid Workspace Automatic Updates setting.",
Validations: []codersdk.ValidationError{{Field: "automatic_updates", Detail: err.Error()}},
})
return
}
}
@@ -607,20 +609,18 @@ func createWorkspace(
})
if err == nil {
// If the workspace already exists, don't allow creation.
httpapi.Write(ctx, rw, http.StatusConflict, codersdk.Response{
return codersdk.Workspace{}, httperror.NewResponseError(http.StatusConflict, codersdk.Response{
Message: fmt.Sprintf("Workspace %q already exists.", req.Name),
Validations: []codersdk.ValidationError{{
Field: "name",
Detail: "This value is already in use and should be unique.",
}},
})
return
} else if !errors.Is(err, sql.ErrNoRows) {
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
return codersdk.Workspace{}, httperror.NewResponseError(http.StatusInternalServerError, codersdk.Response{
Message: fmt.Sprintf("Internal error fetching workspace by name %q.", req.Name),
Detail: err.Error(),
})
return
}
var (
@@ -759,8 +759,7 @@ func createWorkspace(
return err
}, nil)
if err != nil {
httperror.WriteWorkspaceBuildError(ctx, rw, err)
return
return codersdk.Workspace{}, err
}
err = provisionerjobs.PostJob(api.Pubsub, *provisionerJob)
@@ -809,11 +808,10 @@ func createWorkspace(
provisionerDaemons,
)
if err != nil {
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
return codersdk.Workspace{}, httperror.NewResponseError(http.StatusInternalServerError, codersdk.Response{
Message: "Internal error converting workspace build.",
Detail: err.Error(),
})
return
}
w, err := convertWorkspace(
@@ -825,40 +823,38 @@ func createWorkspace(
codersdk.WorkspaceAppStatus{},
)
if err != nil {
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
return codersdk.Workspace{}, httperror.NewResponseError(http.StatusInternalServerError, codersdk.Response{
Message: "Internal error converting workspace.",
Detail: err.Error(),
})
return
}
httpapi.Write(ctx, rw, http.StatusCreated, w)
return w, nil
}
func requestTemplate(ctx context.Context, rw http.ResponseWriter, req codersdk.CreateWorkspaceRequest, db database.Store) (database.Template, bool) {
func requestTemplate(ctx context.Context, req codersdk.CreateWorkspaceRequest, db database.Store) (database.Template, error) {
// If we were given a `TemplateVersionID`, we need to determine the `TemplateID` from it.
templateID := req.TemplateID
if templateID == uuid.Nil {
templateVersion, err := db.GetTemplateVersionByID(ctx, req.TemplateVersionID)
if httpapi.Is404Error(err) {
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
return database.Template{}, httperror.NewResponseError(http.StatusBadRequest, codersdk.Response{
Message: fmt.Sprintf("Template version %q doesn't exist.", req.TemplateVersionID),
Validations: []codersdk.ValidationError{{
Field: "template_version_id",
Detail: "template not found",
}},
})
return database.Template{}, false
}
if err != nil {
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
return database.Template{}, httperror.NewResponseError(http.StatusInternalServerError, codersdk.Response{
Message: "Internal error fetching template version.",
Detail: err.Error(),
})
return database.Template{}, false
}
if templateVersion.Archived {
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
return database.Template{}, httperror.NewResponseError(http.StatusInternalServerError, codersdk.Response{
Message: "Archived template versions cannot be used to make a workspace.",
Validations: []codersdk.ValidationError{
{
@@ -867,7 +863,6 @@ func requestTemplate(ctx context.Context, rw http.ResponseWriter, req codersdk.C
},
},
})
return database.Template{}, false
}
templateID = templateVersion.TemplateID.UUID
@@ -875,29 +870,26 @@ func requestTemplate(ctx context.Context, rw http.ResponseWriter, req codersdk.C
template, err := db.GetTemplateByID(ctx, templateID)
if httpapi.Is404Error(err) {
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
return database.Template{}, httperror.NewResponseError(http.StatusBadRequest, codersdk.Response{
Message: fmt.Sprintf("Template %q doesn't exist.", templateID),
Validations: []codersdk.ValidationError{{
Field: "template_id",
Detail: "template not found",
}},
})
return database.Template{}, false
}
if err != nil {
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
return database.Template{}, httperror.NewResponseError(http.StatusInternalServerError, codersdk.Response{
Message: "Internal error fetching template.",
Detail: err.Error(),
})
return database.Template{}, false
}
if template.Deleted {
httpapi.Write(ctx, rw, http.StatusNotFound, codersdk.Response{
return database.Template{}, httperror.NewResponseError(http.StatusNotFound, codersdk.Response{
Message: fmt.Sprintf("Template %q has been deleted!", template.Name),
})
return database.Template{}, false
}
return template, true
return template, nil
}
func claimPrebuild(