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

Commit817afe8

Browse files
committed
chore: improve dynamic parameter validation errors
1 parent4f52076 commit817afe8

File tree

7 files changed

+160
-55
lines changed

7 files changed

+160
-55
lines changed

‎coderd/dynamicparameters/resolver.go

Lines changed: 65 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,45 @@ type parameterValue struct {
2626
SourceparameterValueSource
2727
}
2828

29+
typeResolverErrorstruct {
30+
Diagnostics hcl.Diagnostics
31+
Parametermap[string]hcl.Diagnostics
32+
}
33+
34+
// Error is a pretty bad format for these errors. Try to avoid using this.
35+
func (e*ResolverError)Error()string {
36+
vardiags hcl.Diagnostics
37+
diags=diags.Extend(e.Diagnostics)
38+
for_,d:=rangee.Parameter {
39+
diags=diags.Extend(d)
40+
}
41+
42+
returndiags.Error()
43+
}
44+
45+
func (e*ResolverError)HasError()bool {
46+
ife.Diagnostics.HasErrors() {
47+
returntrue
48+
}
49+
50+
for_,diags:=rangee.Parameter {
51+
ifdiags.HasErrors() {
52+
returntrue
53+
}
54+
}
55+
returnfalse
56+
}
57+
58+
func (e*ResolverError)Extend(parameterNamestring,diag hcl.Diagnostics) {
59+
ife.Parameter==nil {
60+
e.Parameter=make(map[string]hcl.Diagnostics)
61+
}
62+
if_,ok:=e.Parameter[parameterName];!ok {
63+
e.Parameter[parameterName]= hcl.Diagnostics{}
64+
}
65+
e.Parameter[parameterName]=e.Parameter[parameterName].Extend(diag)
66+
}
67+
2968
//nolint:revive // firstbuild is a control flag to turn on immutable validation
3069
funcResolveParameters(
3170
ctx context.Context,
@@ -35,7 +74,7 @@ func ResolveParameters(
3574
previousValues []database.WorkspaceBuildParameter,
3675
buildValues []codersdk.WorkspaceBuildParameter,
3776
presetValues []database.TemplateVersionPresetParameter,
38-
) (map[string]string,hcl.Diagnostics) {
77+
) (map[string]string,error) {
3978
previousValuesMap:=slice.ToMap(previousValues,func(p database.WorkspaceBuildParameter) (string,string) {
4079
returnp.Name,p.Value
4180
})
@@ -73,7 +112,10 @@ func ResolveParameters(
73112
// always be valid. If there is a case where this is not true, then this has to
74113
// be changed to allow the build to continue with a different set of values.
75114

76-
returnnil,diags
115+
returnnil,&ResolverError{
116+
Diagnostics:diags,
117+
Parameter:nil,
118+
}
77119
}
78120

79121
// The user's input now needs to be validated against the parameters.
@@ -114,12 +156,16 @@ func ResolveParameters(
114156
// are fatal. Additional validation for immutability has to be done manually.
115157
output,diags=renderer.Render(ctx,ownerID,values.ValuesMap())
116158
ifdiags.HasErrors() {
117-
returnnil,diags
159+
returnnil,&ResolverError{
160+
Diagnostics:diags,
161+
Parameter:nil,
162+
}
118163
}
119164

120165
// parameterNames is going to be used to remove any excess values that were left
121166
// around without a parameter.
122167
parameterNames:=make(map[string]struct{},len(output.Parameters))
168+
parameterError:=&ResolverError{}
123169
for_,parameter:=rangeoutput.Parameters {
124170
parameterNames[parameter.Name]=struct{}{}
125171

@@ -133,20 +179,22 @@ func ResolveParameters(
133179
}
134180

135181
// An immutable parameter was changed, which is not allowed.
136-
// Add the failed diagnostic to the output.
137-
diags=diags.Append(&hcl.Diagnostic{
138-
Severity:hcl.DiagError,
139-
Summary:"Immutable parameter changed",
140-
Detail:fmt.Sprintf("Parameter %q is not mutable, so it can't be updated after creating a workspace.",parameter.Name),
141-
Subject:src,
182+
// Add a failed diagnostic to the output.
183+
parameterError.Extend(parameter.Name, hcl.Diagnostics{
184+
&hcl.Diagnostic{
185+
Severity:hcl.DiagError,
186+
Summary:"Immutable parameter changed",
187+
Detail:fmt.Sprintf("Parameter %q is not mutable, so it can't be updated after creating a workspace.",parameter.Name),
188+
Subject:src,
189+
},
142190
})
143191
}
144192
}
145193

146194
// TODO: Fix the `hcl.Diagnostics(...)` type casting. It should not be needed.
147195
ifhcl.Diagnostics(parameter.Diagnostics).HasErrors() {
148-
// All validation errors are raised here.
149-
diags=diags.Extend(hcl.Diagnostics(parameter.Diagnostics))
196+
// All validation errors are raised here for each parameter.
197+
parameterError.Extend(parameter.Name,hcl.Diagnostics(parameter.Diagnostics))
150198
}
151199

152200
// If the parameter has a value, but it was not set explicitly by the user at any
@@ -175,8 +223,13 @@ func ResolveParameters(
175223
}
176224
}
177225

226+
ifparameterError.HasError() {
227+
// If there are any errors, return them.
228+
returnnil,parameterError
229+
}
230+
178231
// Return the values to be saved for the build.
179-
returnvalues.ValuesMap(),diags
232+
returnvalues.ValuesMap(),nil
180233
}
181234

182235
typeparameterValueMapmap[string]parameterValue

‎coderd/httpapi/httperror/doc.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
// Package httperror handles formatting and writing some sentinel errors returned
2+
// within coder to the API.
3+
// This package exists outside httpapi to avoid some cyclic dependencies
4+
package httperror

‎coderd/httpapi/httperror/wsbuild.go

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
package httperror
2+
3+
import (
4+
"context"
5+
"errors"
6+
"fmt"
7+
"net/http"
8+
9+
"github.com/hashicorp/hcl/v2"
10+
11+
"github.com/coder/coder/v2/coderd/dynamicparameters"
12+
"github.com/coder/coder/v2/coderd/httpapi"
13+
"github.com/coder/coder/v2/coderd/wsbuilder"
14+
"github.com/coder/coder/v2/codersdk"
15+
)
16+
17+
funcWriteWorkspaceBuildError(ctx context.Context,rw http.ResponseWriter,errerror) {
18+
varbuildErr wsbuilder.BuildError
19+
iferrors.As(err,&buildErr) {
20+
ifhttpapi.IsUnauthorizedError(err) {
21+
buildErr.Status=http.StatusForbidden
22+
}
23+
24+
httpapi.Write(ctx,rw,buildErr.Status, codersdk.Response{
25+
Message:buildErr.Message,
26+
Detail:buildErr.Error(),
27+
})
28+
return
29+
}
30+
31+
varparameterErr*dynamicparameters.ResolverError
32+
iferrors.As(err,&parameterErr) {
33+
resp:= codersdk.Response{
34+
Message:"Unable to validate parameters",
35+
Validations:nil,
36+
}
37+
38+
forname,diag:=rangeparameterErr.Parameter {
39+
resp.Validations=append(resp.Validations, codersdk.ValidationError{
40+
Field:name,
41+
Detail:DiagnosticsErrorString(diag),
42+
})
43+
}
44+
45+
ifparameterErr.Diagnostics.HasErrors() {
46+
resp.Detail=DiagnosticsErrorString(parameterErr.Diagnostics)
47+
}
48+
49+
httpapi.Write(ctx,rw,http.StatusBadRequest,resp)
50+
return
51+
}
52+
53+
httpapi.Write(ctx,rw,http.StatusInternalServerError, codersdk.Response{
54+
Message:"Internal error creating workspace build.",
55+
Detail:err.Error(),
56+
})
57+
}
58+
59+
funcDiagnosticError(d*hcl.Diagnostic)string {
60+
returnfmt.Sprintf("%s; %s",d.Summary,d.Detail)
61+
}
62+
63+
funcDiagnosticsErrorString(d hcl.Diagnostics)string {
64+
count:=len(d)
65+
switch {
66+
casecount==0:
67+
return"no diagnostics"
68+
casecount==1:
69+
returnDiagnosticError(d[0])
70+
default:
71+
for_,d:=ranged {
72+
// Render the first error diag.
73+
// If there are warnings, do not priority them over errors.
74+
ifd.Severity==hcl.DiagError {
75+
returnfmt.Sprintf("%s, and %d other diagnostic(s)",DiagnosticError(d),count-1)
76+
}
77+
}
78+
79+
// All warnings? ok...
80+
returnfmt.Sprintf("%s, and %d other diagnostic(s)",DiagnosticError(d[0]),count-1)
81+
}
82+
}

‎coderd/workspacebuilds.go

Lines changed: 2 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import (
2727
"github.com/coder/coder/v2/coderd/database/dbtime"
2828
"github.com/coder/coder/v2/coderd/database/provisionerjobs"
2929
"github.com/coder/coder/v2/coderd/httpapi"
30+
"github.com/coder/coder/v2/coderd/httpapi/httperror"
3031
"github.com/coder/coder/v2/coderd/httpmw"
3132
"github.com/coder/coder/v2/coderd/notifications"
3233
"github.com/coder/coder/v2/coderd/provisionerdserver"
@@ -410,28 +411,8 @@ func (api *API) postWorkspaceBuilds(rw http.ResponseWriter, r *http.Request) {
410411
)
411412
returnerr
412413
},nil)
413-
varbuildErr wsbuilder.BuildError
414-
ifxerrors.As(err,&buildErr) {
415-
varauthErr dbauthz.NotAuthorizedError
416-
ifxerrors.As(err,&authErr) {
417-
buildErr.Status=http.StatusForbidden
418-
}
419-
420-
ifbuildErr.Status==http.StatusInternalServerError {
421-
api.Logger.Error(ctx,"workspace build error",slog.Error(buildErr.Wrapped))
422-
}
423-
424-
httpapi.Write(ctx,rw,buildErr.Status, codersdk.Response{
425-
Message:buildErr.Message,
426-
Detail:buildErr.Error(),
427-
})
428-
return
429-
}
430414
iferr!=nil {
431-
httpapi.Write(ctx,rw,http.StatusInternalServerError, codersdk.Response{
432-
Message:"Error posting new build",
433-
Detail:err.Error(),
434-
})
415+
httperror.WriteWorkspaceBuildError(ctx,rw,err)
435416
return
436417
}
437418

‎coderd/workspaces.go

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import (
2727
"github.com/coder/coder/v2/coderd/database/dbtime"
2828
"github.com/coder/coder/v2/coderd/database/provisionerjobs"
2929
"github.com/coder/coder/v2/coderd/httpapi"
30+
"github.com/coder/coder/v2/coderd/httpapi/httperror"
3031
"github.com/coder/coder/v2/coderd/httpmw"
3132
"github.com/coder/coder/v2/coderd/notifications"
3233
"github.com/coder/coder/v2/coderd/prebuilds"
@@ -732,21 +733,11 @@ func createWorkspace(
732733
)
733734
returnerr
734735
},nil)
735-
varbldErr wsbuilder.BuildError
736-
ifxerrors.As(err,&bldErr) {
737-
httpapi.Write(ctx,rw,bldErr.Status, codersdk.Response{
738-
Message:bldErr.Message,
739-
Detail:bldErr.Error(),
740-
})
741-
return
742-
}
743736
iferr!=nil {
744-
httpapi.Write(ctx,rw,http.StatusInternalServerError, codersdk.Response{
745-
Message:"Internal error creating workspace.",
746-
Detail:err.Error(),
747-
})
737+
httperror.WriteWorkspaceBuildError(ctx,rw,err)
748738
return
749739
}
740+
750741
err=provisionerjobs.PostJob(api.Pubsub,*provisionerJob)
751742
iferr!=nil {
752743
// Client probably doesn't care about this error, so just log it.

‎coderd/wsbuilder/wsbuilder.go

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -760,20 +760,13 @@ func (b *Builder) getDynamicParameters() (names, values []string, err error) {
760760
returnnil,nil,BuildError{http.StatusInternalServerError,"failed to check if first build",err}
761761
}
762762

763-
buildValues,diagnostics:=dynamicparameters.ResolveParameters(b.ctx,b.workspace.OwnerID,render,firstBuild,
763+
buildValues,err:=dynamicparameters.ResolveParameters(b.ctx,b.workspace.OwnerID,render,firstBuild,
764764
lastBuildParameters,
765765
b.richParameterValues,
766766
presetParameterValues)
767767

768-
ifdiagnostics.HasErrors() {
769-
// TODO: Improve the error response. The response should include the validations for each failed
770-
// parameter. The response should also indicate it's a validation error or a more general form failure.
771-
// For now, any error is sufficient.
772-
returnnil,nil,BuildError{
773-
Status:http.StatusBadRequest,
774-
Message:fmt.Sprintf("%d errors occurred while resolving parameters",len(diagnostics)),
775-
Wrapped:diagnostics,
776-
}
768+
iferr!=nil {
769+
returnnil,nil,xerrors.Errorf("resolve parameters: %w",err)
777770
}
778771

779772
names=make([]string,0,len(buildValues))
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
package wsbuilderror

0 commit comments

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