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

Commit98e5611

Browse files
fix: fix for prebuilds claiming and deletion (#17624)
PR contains:- fix for claiming & deleting prebuilds with immutable params- unit test for claiming scenario- unit test for deletion scenarioThe parameter resolver was failing when deleting/claiming prebuildsbecause a value for a previously-used parameter was provided to theresolver, but since the value was unchanged (it's coming from thepreset) it failed in the resolver. The resolver was missing a check tosee if the old value != new value; if the values match then there's nomutation of an immutable parameter.---------Signed-off-by: Danny Kopping <dannykopping@gmail.com>
1 parentc7fc7b9 commit98e5611

File tree

4 files changed

+81
-13
lines changed

4 files changed

+81
-13
lines changed

‎codersdk/richparameters.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ type ParameterResolver struct {
164164
// resolves the correct value. It returns the value of the parameter, if valid, and an error if invalid.
165165
func (r*ParameterResolver)ValidateResolve(pTemplateVersionParameter,v*WorkspaceBuildParameter) (valuestring,errerror) {
166166
prevV:=r.findLastValue(p)
167-
if!p.Mutable&&v!=nil&&prevV!=nil {
167+
if!p.Mutable&&v!=nil&&prevV!=nil&&v.Value!=prevV.Value{
168168
return"",xerrors.Errorf("Parameter %q is not mutable, so it can't be updated after creating a workspace.",p.Name)
169169
}
170170
ifp.Required&&v==nil&&prevV==nil {

‎codersdk/richparameters_test.go

Lines changed: 49 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package codersdk_test
22

33
import (
4+
"fmt"
45
"testing"
56

67
"github.com/stretchr/testify/require"
@@ -121,20 +122,60 @@ func TestParameterResolver_ValidateResolve_NewOverridesOld(t *testing.T) {
121122
funcTestParameterResolver_ValidateResolve_Immutable(t*testing.T) {
122123
t.Parallel()
123124
uut:= codersdk.ParameterResolver{
124-
Rich: []codersdk.WorkspaceBuildParameter{{Name:"n",Value:"5"}},
125+
Rich: []codersdk.WorkspaceBuildParameter{{Name:"n",Value:"old"}},
125126
}
126127
p:= codersdk.TemplateVersionParameter{
127128
Name:"n",
128-
Type:"number",
129+
Type:"string",
129130
Required:true,
130131
Mutable:false,
131132
}
132-
v,err:=uut.ValidateResolve(p,&codersdk.WorkspaceBuildParameter{
133-
Name:"n",
134-
Value:"6",
135-
})
136-
require.Error(t,err)
137-
require.Equal(t,"",v)
133+
134+
cases:= []struct {
135+
namestring
136+
newValuestring
137+
expectedErrstring
138+
}{
139+
{
140+
name:"mutation",
141+
newValue:"new",// "new" != "old"
142+
expectedErr:fmt.Sprintf("Parameter %q is not mutable",p.Name),
143+
},
144+
{
145+
// Values are case-sensitive.
146+
name:"case change",
147+
newValue:"Old",// "Old" != "old"
148+
expectedErr:fmt.Sprintf("Parameter %q is not mutable",p.Name),
149+
},
150+
{
151+
name:"default",
152+
newValue:"",// "" != "old"
153+
expectedErr:fmt.Sprintf("Parameter %q is not mutable",p.Name),
154+
},
155+
{
156+
name:"no change",
157+
newValue:"old",// "old" == "old"
158+
},
159+
}
160+
161+
for_,tc:=rangecases {
162+
t.Run(tc.name,func(t*testing.T) {
163+
t.Parallel()
164+
165+
v,err:=uut.ValidateResolve(p,&codersdk.WorkspaceBuildParameter{
166+
Name:"n",
167+
Value:tc.newValue,
168+
})
169+
170+
iftc.expectedErr=="" {
171+
require.NoError(t,err)
172+
require.Equal(t,tc.newValue,v)
173+
}else {
174+
require.ErrorContains(t,err,tc.expectedErr)
175+
require.Equal(t,"",v)
176+
}
177+
})
178+
}
138179
}
139180

140181
funcTestRichParameterValidation(t*testing.T) {

‎enterprise/coderd/prebuilds/claim_test.go

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -329,9 +329,8 @@ func TestClaimPrebuild(t *testing.T) {
329329
require.NoError(t,err)
330330

331331
stopBuild,err:=userClient.CreateWorkspaceBuild(ctx,workspace.ID, codersdk.CreateWorkspaceBuildRequest{
332-
TemplateVersionID:version.ID,
333-
Transition:codersdk.WorkspaceTransitionStop,
334-
RichParameterValues:wp,
332+
TemplateVersionID:version.ID,
333+
Transition:codersdk.WorkspaceTransitionStop,
335334
})
336335
require.NoError(t,err)
337336
coderdtest.AwaitWorkspaceBuildJobCompleted(t,userClient,stopBuild.ID)
@@ -369,6 +368,17 @@ func templateWithAgentAndPresetsWithPrebuilds(desiredInstances int32) *echo.Resp
369368
},
370369
},
371370
},
371+
// Make sure immutable params don't break claiming logic
372+
Parameters: []*proto.RichParameter{
373+
{
374+
Name:"k1",
375+
Description:"immutable param",
376+
Type:"string",
377+
DefaultValue:"",
378+
Required:false,
379+
Mutable:false,
380+
},
381+
},
372382
Presets: []*proto.Preset{
373383
{
374384
Name:"preset-a",

‎enterprise/coderd/prebuilds/reconcile_test.go

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -901,6 +901,16 @@ func setupTestDBTemplateVersion(
901901
ID:templateID,
902902
ActiveVersionID:templateVersion.ID,
903903
}))
904+
// Make sure immutable params don't break prebuilt workspace deletion logic
905+
dbgen.TemplateVersionParameter(t,db, database.TemplateVersionParameter{
906+
TemplateVersionID:templateVersion.ID,
907+
Name:"test",
908+
Description:"required & immutable param",
909+
Type:"string",
910+
DefaultValue:"",
911+
Required:true,
912+
Mutable:false,
913+
})
904914
returntemplateVersion.ID
905915
}
906916

@@ -999,7 +1009,7 @@ func setupTestDBWorkspace(
9991009
OrganizationID:orgID,
10001010
Error:buildError,
10011011
})
1002-
dbgen.WorkspaceBuild(t,db, database.WorkspaceBuild{
1012+
workspaceBuild:=dbgen.WorkspaceBuild(t,db, database.WorkspaceBuild{
10031013
WorkspaceID:workspace.ID,
10041014
InitiatorID:initiatorID,
10051015
TemplateVersionID:templateVersionID,
@@ -1008,6 +1018,13 @@ func setupTestDBWorkspace(
10081018
Transition:transition,
10091019
CreatedAt:clock.Now(),
10101020
})
1021+
dbgen.WorkspaceBuildParameters(t,db, []database.WorkspaceBuildParameter{
1022+
{
1023+
WorkspaceBuildID:workspaceBuild.ID,
1024+
Name:"test",
1025+
Value:"test",
1026+
},
1027+
})
10111028

10121029
returnworkspace
10131030
}

0 commit comments

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