Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit29a7313

Browse files
refactor: untangle workspace creation from http logic (#19639)
Coder Tasks requires us to create a workspace, but we want to be able toreturn a `codersdk.Task` instead of a `codersdk.Workspace`. Thisrequires untangling `createWorkspace` from directly writing to`http.ResponseWriter`.
1 parent7365da1 commit29a7313

File tree

3 files changed

+106
-57
lines changed

3 files changed

+106
-57
lines changed

‎coderd/aitasks.go‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
"github.com/coder/coder/v2/coderd/audit"
1818
"github.com/coder/coder/v2/coderd/database"
1919
"github.com/coder/coder/v2/coderd/httpapi"
20+
"github.com/coder/coder/v2/coderd/httpapi/httperror"
2021
"github.com/coder/coder/v2/coderd/httpmw"
2122
"github.com/coder/coder/v2/coderd/rbac"
2223
"github.com/coder/coder/v2/coderd/rbac/policy"
@@ -154,8 +155,9 @@ func (api *API) tasksCreate(rw http.ResponseWriter, r *http.Request) {
154155
// This can be optimized. It exists as it is now for code simplicity.
155156
// The most common case is to create a workspace for 'Me'. Which does
156157
// not enter this code branch.
157-
template,ok:=requestTemplate(ctx,rw,createReq,api.Database)
158-
if!ok {
158+
template,err:=requestTemplate(ctx,createReq,api.Database)
159+
iferr!=nil {
160+
httperror.WriteResponseError(ctx,rw,err)
159161
return
160162
}
161163

@@ -188,7 +190,13 @@ func (api *API) tasksCreate(rw http.ResponseWriter, r *http.Request) {
188190
})
189191

190192
defercommitAudit()
191-
createWorkspace(ctx,aReq,apiKey.UserID,api,owner,createReq,rw,r)
193+
w,err:=createWorkspace(ctx,aReq,apiKey.UserID,api,owner,createReq,r)
194+
iferr!=nil {
195+
httperror.WriteResponseError(ctx,rw,err)
196+
return
197+
}
198+
199+
httpapi.Write(ctx,rw,http.StatusCreated,w)
192200
}
193201

194202
// tasksFromWorkspaces converts a slice of API workspaces into tasks, fetching

‎coderd/httpapi/httperror/responserror.go‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,12 @@
11
package httperror
22

33
import (
4+
"context"
45
"errors"
6+
"fmt"
7+
"net/http"
58

9+
"github.com/coder/coder/v2/coderd/httpapi"
610
"github.com/coder/coder/v2/codersdk"
711
)
812

@@ -17,3 +21,48 @@ func IsResponder(err error) (Responder, bool) {
1721
}
1822
returnnil,false
1923
}
24+
25+
funcNewResponseError(statusint,resp codersdk.Response)error {
26+
return&responseError{
27+
status:status,
28+
response:resp,
29+
}
30+
}
31+
32+
funcWriteResponseError(ctx context.Context,rw http.ResponseWriter,errerror) {
33+
ifresponseErr,ok:=IsResponder(err);ok {
34+
code,resp:=responseErr.Response()
35+
36+
httpapi.Write(ctx,rw,code,resp)
37+
return
38+
}
39+
40+
httpapi.Write(ctx,rw,http.StatusInternalServerError, codersdk.Response{
41+
Message:"Internal server error",
42+
Detail:err.Error(),
43+
})
44+
}
45+
46+
typeresponseErrorstruct {
47+
statusint
48+
response codersdk.Response
49+
}
50+
51+
var (
52+
_error= (*responseError)(nil)
53+
_Responder= (*responseError)(nil)
54+
)
55+
56+
func (e*responseError)Error()string {
57+
returnfmt.Sprintf("%s: %s",e.response.Message,e.response.Detail)
58+
}
59+
60+
func (e*responseError)Status()int {
61+
returne.status
62+
}
63+
64+
func (e*responseError)Response() (int, codersdk.Response) {
65+
returne.status,e.response
66+
}
67+
68+
varErrResourceNotFound=NewResponseError(http.StatusNotFound,httpapi.ResourceNotFoundResponse)

‎coderd/workspaces.go‎

Lines changed: 46 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -388,7 +388,13 @@ func (api *API) postWorkspacesByOrganization(rw http.ResponseWriter, r *http.Req
388388
AvatarURL:member.AvatarURL,
389389
}
390390

391-
createWorkspace(ctx,aReq,apiKey.UserID,api,owner,req,rw,r)
391+
w,err:=createWorkspace(ctx,aReq,apiKey.UserID,api,owner,req,r)
392+
iferr!=nil {
393+
httperror.WriteResponseError(ctx,rw,err)
394+
return
395+
}
396+
397+
httpapi.Write(ctx,rw,http.StatusCreated,w)
392398
}
393399

394400
// Create a new workspace for the currently authenticated user.
@@ -442,8 +448,9 @@ func (api *API) postUserWorkspaces(rw http.ResponseWriter, r *http.Request) {
442448
// This can be optimized. It exists as it is now for code simplicity.
443449
// The most common case is to create a workspace for 'Me'. Which does
444450
// not enter this code branch.
445-
template,ok:=requestTemplate(ctx,rw,req,api.Database)
446-
if!ok {
451+
template,err:=requestTemplate(ctx,req,api.Database)
452+
iferr!=nil {
453+
httperror.WriteResponseError(ctx,rw,err)
447454
return
448455
}
449456

@@ -476,7 +483,14 @@ func (api *API) postUserWorkspaces(rw http.ResponseWriter, r *http.Request) {
476483
})
477484

478485
defercommitAudit()
479-
createWorkspace(ctx,aReq,apiKey.UserID,api,owner,req,rw,r)
486+
487+
w,err:=createWorkspace(ctx,aReq,apiKey.UserID,api,owner,req,r)
488+
iferr!=nil {
489+
httperror.WriteResponseError(ctx,rw,err)
490+
return
491+
}
492+
493+
httpapi.Write(ctx,rw,http.StatusCreated,w)
480494
}
481495

482496
typeworkspaceOwnerstruct {
@@ -492,12 +506,11 @@ func createWorkspace(
492506
api*API,
493507
ownerworkspaceOwner,
494508
req codersdk.CreateWorkspaceRequest,
495-
rw http.ResponseWriter,
496509
r*http.Request,
497-
) {
498-
template,ok:=requestTemplate(ctx,rw,req,api.Database)
499-
if!ok {
500-
return
510+
)(codersdk.Workspace,error){
511+
template,err:=requestTemplate(ctx,req,api.Database)
512+
iferr!=nil {
513+
return codersdk.Workspace{},err
501514
}
502515

503516
// This is a premature auth check to avoid doing unnecessary work if the user
@@ -506,14 +519,12 @@ func createWorkspace(
506519
rbac.ResourceWorkspace.InOrg(template.OrganizationID).WithOwner(owner.ID.String())) {
507520
// If this check fails, return a proper unauthorized error to the user to indicate
508521
// what is going on.
509-
httpapi.Write(ctx,rw,http.StatusForbidden, codersdk.Response{
522+
return codersdk.Workspace{},httperror.NewResponseError(http.StatusForbidden, codersdk.Response{
510523
Message:"Unauthorized to create workspace.",
511524
Detail:"You are unable to create a workspace in this organization. "+
512525
"It is possible to have access to the template, but not be able to create a workspace. "+
513526
"Please contact an administrator about your permissions if you feel this is an error.",
514-
Validations:nil,
515527
})
516-
return
517528
}
518529

519530
// Update audit log's organization
@@ -523,49 +534,42 @@ func createWorkspace(
523534
// would be wasted.
524535
if!api.Authorize(r,policy.ActionCreate,
525536
rbac.ResourceWorkspace.InOrg(template.OrganizationID).WithOwner(owner.ID.String())) {
526-
httpapi.ResourceNotFound(rw)
527-
return
537+
return codersdk.Workspace{},httperror.ErrResourceNotFound
528538
}
529539
// The user also needs permission to use the template. At this point they have
530540
// read perms, but not necessarily "use". This is also checked in `db.InsertWorkspace`.
531541
// Doing this up front can save some work below if the user doesn't have permission.
532542
if!api.Authorize(r,policy.ActionUse,template) {
533-
httpapi.Write(ctx,rw,http.StatusForbidden, codersdk.Response{
543+
return codersdk.Workspace{},httperror.NewResponseError(http.StatusForbidden, codersdk.Response{
534544
Message:fmt.Sprintf("Unauthorized access to use the template %q.",template.Name),
535545
Detail:"Although you are able to view the template, you are unable to create a workspace using it. "+
536546
"Please contact an administrator about your permissions if you feel this is an error.",
537-
Validations:nil,
538547
})
539-
return
540548
}
541549

542550
templateAccessControl:= (*(api.AccessControlStore.Load())).GetTemplateAccessControl(template)
543551
iftemplateAccessControl.IsDeprecated() {
544-
httpapi.Write(ctx,rw,http.StatusBadRequest, codersdk.Response{
552+
return codersdk.Workspace{},httperror.NewResponseError(http.StatusBadRequest, codersdk.Response{
545553
Message:fmt.Sprintf("Template %q has been deprecated, and cannot be used to create a new workspace.",template.Name),
546554
// Pass the deprecated message to the user.
547-
Detail:templateAccessControl.Deprecated,
548-
Validations:nil,
555+
Detail:templateAccessControl.Deprecated,
549556
})
550-
return
551557
}
552558

553559
dbAutostartSchedule,err:=validWorkspaceSchedule(req.AutostartSchedule)
554560
iferr!=nil {
555-
httpapi.Write(ctx,rw,http.StatusBadRequest, codersdk.Response{
561+
return codersdk.Workspace{},httperror.NewResponseError(http.StatusBadRequest, codersdk.Response{
556562
Message:"Invalid Autostart Schedule.",
557563
Validations: []codersdk.ValidationError{{Field:"schedule",Detail:err.Error()}},
558564
})
559-
return
560565
}
561566

562567
templateSchedule,err:= (*api.TemplateScheduleStore.Load()).Get(ctx,api.Database,template.ID)
563568
iferr!=nil {
564-
httpapi.Write(ctx,rw,http.StatusInternalServerError, codersdk.Response{
569+
return codersdk.Workspace{},httperror.NewResponseError(http.StatusInternalServerError, codersdk.Response{
565570
Message:"Internal error fetching template schedule.",
566571
Detail:err.Error(),
567572
})
568-
return
569573
}
570574

571575
nextStartAt:= sql.NullTime{}
@@ -578,23 +582,21 @@ func createWorkspace(
578582

579583
dbTTL,err:=validWorkspaceTTLMillis(req.TTLMillis,templateSchedule.DefaultTTL)
580584
iferr!=nil {
581-
httpapi.Write(ctx,rw,http.StatusBadRequest, codersdk.Response{
585+
return codersdk.Workspace{},httperror.NewResponseError(http.StatusBadRequest, codersdk.Response{
582586
Message:"Invalid Workspace Time to Shutdown.",
583587
Validations: []codersdk.ValidationError{{Field:"ttl_ms",Detail:err.Error()}},
584588
})
585-
return
586589
}
587590

588591
// back-compatibility: default to "never" if not included.
589592
dbAU:=database.AutomaticUpdatesNever
590593
ifreq.AutomaticUpdates!="" {
591594
dbAU,err=validWorkspaceAutomaticUpdates(req.AutomaticUpdates)
592595
iferr!=nil {
593-
httpapi.Write(ctx,rw,http.StatusBadRequest, codersdk.Response{
596+
return codersdk.Workspace{},httperror.NewResponseError(http.StatusBadRequest, codersdk.Response{
594597
Message:"Invalid Workspace Automatic Updates setting.",
595598
Validations: []codersdk.ValidationError{{Field:"automatic_updates",Detail:err.Error()}},
596599
})
597-
return
598600
}
599601
}
600602

@@ -607,20 +609,18 @@ func createWorkspace(
607609
})
608610
iferr==nil {
609611
// If the workspace already exists, don't allow creation.
610-
httpapi.Write(ctx,rw,http.StatusConflict, codersdk.Response{
612+
return codersdk.Workspace{},httperror.NewResponseError(http.StatusConflict, codersdk.Response{
611613
Message:fmt.Sprintf("Workspace %q already exists.",req.Name),
612614
Validations: []codersdk.ValidationError{{
613615
Field:"name",
614616
Detail:"This value is already in use and should be unique.",
615617
}},
616618
})
617-
return
618619
}elseif!errors.Is(err,sql.ErrNoRows) {
619-
httpapi.Write(ctx,rw,http.StatusInternalServerError, codersdk.Response{
620+
return codersdk.Workspace{},httperror.NewResponseError(http.StatusInternalServerError, codersdk.Response{
620621
Message:fmt.Sprintf("Internal error fetching workspace by name %q.",req.Name),
621622
Detail:err.Error(),
622623
})
623-
return
624624
}
625625

626626
var (
@@ -759,8 +759,7 @@ func createWorkspace(
759759
returnerr
760760
},nil)
761761
iferr!=nil {
762-
httperror.WriteWorkspaceBuildError(ctx,rw,err)
763-
return
762+
return codersdk.Workspace{},err
764763
}
765764

766765
err=provisionerjobs.PostJob(api.Pubsub,*provisionerJob)
@@ -809,11 +808,10 @@ func createWorkspace(
809808
provisionerDaemons,
810809
)
811810
iferr!=nil {
812-
httpapi.Write(ctx,rw,http.StatusInternalServerError, codersdk.Response{
811+
return codersdk.Workspace{},httperror.NewResponseError(http.StatusInternalServerError, codersdk.Response{
813812
Message:"Internal error converting workspace build.",
814813
Detail:err.Error(),
815814
})
816-
return
817815
}
818816

819817
w,err:=convertWorkspace(
@@ -825,40 +823,38 @@ func createWorkspace(
825823
codersdk.WorkspaceAppStatus{},
826824
)
827825
iferr!=nil {
828-
httpapi.Write(ctx,rw,http.StatusInternalServerError, codersdk.Response{
826+
return codersdk.Workspace{},httperror.NewResponseError(http.StatusInternalServerError, codersdk.Response{
829827
Message:"Internal error converting workspace.",
830828
Detail:err.Error(),
831829
})
832-
return
833830
}
834-
httpapi.Write(ctx,rw,http.StatusCreated,w)
831+
832+
returnw,nil
835833
}
836834

837-
funcrequestTemplate(ctx context.Context,rw http.ResponseWriter,req codersdk.CreateWorkspaceRequest,db database.Store) (database.Template,bool) {
835+
funcrequestTemplate(ctx context.Context,req codersdk.CreateWorkspaceRequest,db database.Store) (database.Template,error) {
838836
// If we were given a `TemplateVersionID`, we need to determine the `TemplateID` from it.
839837
templateID:=req.TemplateID
840838

841839
iftemplateID==uuid.Nil {
842840
templateVersion,err:=db.GetTemplateVersionByID(ctx,req.TemplateVersionID)
843841
ifhttpapi.Is404Error(err) {
844-
httpapi.Write(ctx,rw,http.StatusBadRequest, codersdk.Response{
842+
return database.Template{},httperror.NewResponseError(http.StatusBadRequest, codersdk.Response{
845843
Message:fmt.Sprintf("Template version %q doesn't exist.",req.TemplateVersionID),
846844
Validations: []codersdk.ValidationError{{
847845
Field:"template_version_id",
848846
Detail:"template not found",
849847
}},
850848
})
851-
return database.Template{},false
852849
}
853850
iferr!=nil {
854-
httpapi.Write(ctx,rw,http.StatusInternalServerError, codersdk.Response{
851+
return database.Template{},httperror.NewResponseError(http.StatusInternalServerError, codersdk.Response{
855852
Message:"Internal error fetching template version.",
856853
Detail:err.Error(),
857854
})
858-
return database.Template{},false
859855
}
860856
iftemplateVersion.Archived {
861-
httpapi.Write(ctx,rw,http.StatusInternalServerError, codersdk.Response{
857+
return database.Template{},httperror.NewResponseError(http.StatusInternalServerError, codersdk.Response{
862858
Message:"Archived template versions cannot be used to make a workspace.",
863859
Validations: []codersdk.ValidationError{
864860
{
@@ -867,37 +863,33 @@ func requestTemplate(ctx context.Context, rw http.ResponseWriter, req codersdk.C
867863
},
868864
},
869865
})
870-
return database.Template{},false
871866
}
872867

873868
templateID=templateVersion.TemplateID.UUID
874869
}
875870

876871
template,err:=db.GetTemplateByID(ctx,templateID)
877872
ifhttpapi.Is404Error(err) {
878-
httpapi.Write(ctx,rw,http.StatusBadRequest, codersdk.Response{
873+
return database.Template{},httperror.NewResponseError(http.StatusBadRequest, codersdk.Response{
879874
Message:fmt.Sprintf("Template %q doesn't exist.",templateID),
880875
Validations: []codersdk.ValidationError{{
881876
Field:"template_id",
882877
Detail:"template not found",
883878
}},
884879
})
885-
return database.Template{},false
886880
}
887881
iferr!=nil {
888-
httpapi.Write(ctx,rw,http.StatusInternalServerError, codersdk.Response{
882+
return database.Template{},httperror.NewResponseError(http.StatusInternalServerError, codersdk.Response{
889883
Message:"Internal error fetching template.",
890884
Detail:err.Error(),
891885
})
892-
return database.Template{},false
893886
}
894887
iftemplate.Deleted {
895-
httpapi.Write(ctx,rw,http.StatusNotFound, codersdk.Response{
888+
return database.Template{},httperror.NewResponseError(http.StatusNotFound, codersdk.Response{
896889
Message:fmt.Sprintf("Template %q has been deleted!",template.Name),
897890
})
898-
return database.Template{},false
899891
}
900-
returntemplate,true
892+
returntemplate,nil
901893
}
902894

903895
funcclaimPrebuild(

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp