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

Commit00ba027

Browse files
authored
chore: modify parameter dynamic immutability behavior (#18583)
Immutability behavior is determined by the current build, not affected by the previous
1 parent9c61ef8 commit00ba027

File tree

4 files changed

+174
-26
lines changed

4 files changed

+174
-26
lines changed

‎coderd/dynamicparameters/resolver.go

Lines changed: 16 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -55,19 +55,21 @@ func ResolveParameters(
5555
values[preset.Name]=parameterValue{Source:sourcePreset,Value:preset.Value}
5656
}
5757

58-
//originalValues is going to be used to detect if a user tried to change
58+
//originalInputValues is going to be used to detect if a user tried to change
5959
// an immutable parameter after the first build.
60-
originalValues:=make(map[string]parameterValue,len(values))
60+
// The actual input values are mutated based on attributes like mutability
61+
// and ephemerality.
62+
originalInputValues:=make(map[string]parameterValue,len(values))
6163
forname,value:=rangevalues {
6264
// Store the original values for later use.
63-
originalValues[name]=value
65+
originalInputValues[name]=value
6466
}
6567

6668
// Render the parameters using the values that were supplied to the previous build.
6769
//
6870
// This is how the form should look to the user on their workspace settings page.
6971
// This is the original form truth that our validations should initially be based on.
70-
output,diags:=renderer.Render(ctx,ownerID,values.ValuesMap())
72+
output,diags:=renderer.Render(ctx,ownerID,previousValuesMap)
7173
ifdiags.HasErrors() {
7274
// Top level diagnostics should break the build. Previous values (and new) should
7375
// always be valid. If there is a case where this is not true, then this has to
@@ -91,22 +93,6 @@ func ResolveParameters(
9193
delete(values,parameter.Name)
9294
}
9395
}
94-
95-
// Immutable parameters should also not be allowed to be changed from
96-
// the previous build. Remove any values taken from the preset or
97-
// new build params. This forces the value to be the same as it was before.
98-
//
99-
// We do this so the next form render uses the original immutable value.
100-
if!firstBuild&&!parameter.Mutable {
101-
delete(values,parameter.Name)
102-
prev,ok:=previousValuesMap[parameter.Name]
103-
ifok {
104-
values[parameter.Name]=parameterValue{
105-
Value:prev,
106-
Source:sourcePrevious,
107-
}
108-
}
109-
}
11096
}
11197

11298
// This is the final set of values that will be used. Any errors at this stage
@@ -116,23 +102,28 @@ func ResolveParameters(
116102
returnnil,parameterValidationError(diags)
117103
}
118104

119-
// parameterNames is going to be used to remove any excess valuesthat wereleft
105+
// parameterNames is going to be used to remove any excess values left
120106
// around without a parameter.
121107
parameterNames:=make(map[string]struct{},len(output.Parameters))
122108
parameterError:=parameterValidationError(nil)
123109
for_,parameter:=rangeoutput.Parameters {
124110
parameterNames[parameter.Name]=struct{}{}
125111

126112
if!firstBuild&&!parameter.Mutable {
127-
originalValue,ok:=originalValues[parameter.Name]
113+
// previousValuesMap should be used over the first render output
114+
// for the previous state of parameters. The previous build
115+
// should emit all values, so the previousValuesMap should be
116+
// complete with all parameter values (user specified and defaults)
117+
originalValue,ok:=previousValuesMap[parameter.Name]
118+
128119
// Immutable parameters should not be changed after the first build.
129-
// If the value matches theoriginal value, that is fine.
120+
// If the value matches theprevious input value, that is fine.
130121
//
131-
// If theoriginal value is not set, that means this is a new parameter. New
122+
// If theprevious value is not set, that means this is a new parameter. New
132123
// immutable parameters are allowed. This is an opinionated choice to prevent
133124
// workspaces failing to update or delete. Ideally we would block this, as
134125
// immutable parameters should only be able to be set at creation time.
135-
ifok&&parameter.Value.AsString()!=originalValue.Value {
126+
ifok&&parameter.Value.AsString()!=originalValue {
136127
varsrc*hcl.Range
137128
ifparameter.Source!=nil {
138129
src=&parameter.Source.HCLBlock().TypeRange

‎coderd/dynamicparameters/resolver_test.go

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"github.com/coder/coder/v2/coderd/database"
1111
"github.com/coder/coder/v2/coderd/dynamicparameters"
1212
"github.com/coder/coder/v2/coderd/dynamicparameters/rendermock"
13+
"github.com/coder/coder/v2/coderd/httpapi/httperror"
1314
"github.com/coder/coder/v2/codersdk"
1415
"github.com/coder/coder/v2/testutil"
1516
"github.com/coder/preview"
@@ -56,4 +57,69 @@ func TestResolveParameters(t *testing.T) {
5657
require.NoError(t,err)
5758
require.Equal(t,map[string]string{"immutable":"foo"},values)
5859
})
60+
61+
// Tests a parameter going from mutable -> immutable
62+
t.Run("BecameImmutable",func(t*testing.T) {
63+
t.Parallel()
64+
65+
ctrl:=gomock.NewController(t)
66+
render:=rendermock.NewMockRenderer(ctrl)
67+
68+
mutable:= previewtypes.ParameterData{
69+
Name:"immutable",
70+
Type:previewtypes.ParameterTypeString,
71+
FormType:provider.ParameterFormTypeInput,
72+
Mutable:true,
73+
DefaultValue:previewtypes.StringLiteral("foo"),
74+
Required:true,
75+
}
76+
immutable:=mutable
77+
immutable.Mutable=false
78+
79+
// A single immutable parameter with no previous value.
80+
render.EXPECT().
81+
Render(gomock.Any(),gomock.Any(),gomock.Any()).
82+
// Return the mutable param first
83+
Return(&preview.Output{
84+
Parameters: []previewtypes.Parameter{
85+
{
86+
ParameterData:mutable,
87+
Value:previewtypes.StringLiteral("foo"),
88+
Diagnostics:nil,
89+
},
90+
},
91+
},nil)
92+
93+
render.EXPECT().
94+
Render(gomock.Any(),gomock.Any(),gomock.Any()).
95+
// Then the immutable param
96+
Return(&preview.Output{
97+
Parameters: []previewtypes.Parameter{
98+
{
99+
ParameterData:immutable,
100+
// The user set the value to bar
101+
Value:previewtypes.StringLiteral("bar"),
102+
Diagnostics:nil,
103+
},
104+
},
105+
},nil)
106+
107+
ctx:=testutil.Context(t,testutil.WaitShort)
108+
_,err:=dynamicparameters.ResolveParameters(ctx,uuid.New(),render,false,
109+
[]database.WorkspaceBuildParameter{
110+
{Name:"immutable",Value:"foo"},// Previous value foo
111+
},
112+
[]codersdk.WorkspaceBuildParameter{
113+
{Name:"immutable",Value:"bar"},// New value
114+
},
115+
[]database.TemplateVersionPresetParameter{},// No preset values
116+
)
117+
require.Error(t,err)
118+
resp,ok:=httperror.IsResponder(err)
119+
require.True(t,ok)
120+
121+
_,respErr:=resp.Response()
122+
require.Len(t,respErr.Validations,1)
123+
require.Contains(t,respErr.Validations[0].Error(),"is not mutable")
124+
})
59125
}

‎enterprise/coderd/dynamicparameters_test.go

Lines changed: 69 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -338,7 +338,6 @@ func TestDynamicParameterBuild(t *testing.T) {
338338
bld,err:=templateAdmin.CreateWorkspaceBuild(ctx,wrk.ID, codersdk.CreateWorkspaceBuildRequest{
339339
TemplateVersionID:immutable.ID,// Use the new template version with the immutable parameter
340340
Transition:codersdk.WorkspaceTransitionDelete,
341-
DryRun:false,
342341
})
343342
require.NoError(t,err)
344343
coderdtest.AwaitWorkspaceBuildJobCompleted(t,templateAdmin,bld.ID)
@@ -354,6 +353,75 @@ func TestDynamicParameterBuild(t *testing.T) {
354353
require.NoError(t,err)
355354
require.Equal(t,wrk.ID,deleted.ID,"workspace should be deleted")
356355
})
356+
357+
t.Run("PreviouslyImmutable",func(t*testing.T) {
358+
// Ok this is a weird test to document how things are working.
359+
// What if a parameter flips it's immutability based on a value?
360+
// The current behavior is to source immutability from the new state.
361+
// So the value is allowed to be changed.
362+
t.Parallel()
363+
364+
ctx:=testutil.Context(t,testutil.WaitShort)
365+
// Start with a new template that has 1 parameter that is immutable
366+
immutable,_:=coderdtest.DynamicParameterTemplate(t,templateAdmin,orgID, coderdtest.DynamicParameterTemplateParams{
367+
MainTF:"# PreviouslyImmutable\n"+string(must(os.ReadFile("testdata/parameters/dynamicimmutable/main.tf"))),
368+
})
369+
370+
// Create the workspace with the immutable parameter
371+
wrk,err:=templateAdmin.CreateUserWorkspace(ctx,codersdk.Me, codersdk.CreateWorkspaceRequest{
372+
TemplateID:immutable.ID,
373+
Name:coderdtest.RandomUsername(t),
374+
RichParameterValues: []codersdk.WorkspaceBuildParameter{
375+
{Name:"isimmutable",Value:"true"},
376+
{Name:"immutable",Value:"coder"},
377+
},
378+
})
379+
require.NoError(t,err)
380+
coderdtest.AwaitWorkspaceBuildJobCompleted(t,templateAdmin,wrk.LatestBuild.ID)
381+
382+
// Try new values
383+
_,err=templateAdmin.CreateWorkspaceBuild(ctx,wrk.ID, codersdk.CreateWorkspaceBuildRequest{
384+
Transition:codersdk.WorkspaceTransitionStart,
385+
RichParameterValues: []codersdk.WorkspaceBuildParameter{
386+
{Name:"isimmutable",Value:"false"},
387+
{Name:"immutable",Value:"not-coder"},
388+
},
389+
})
390+
require.NoError(t,err)
391+
})
392+
393+
t.Run("PreviouslyMutable",func(t*testing.T) {
394+
// The value cannot be changed because it becomes immutable.
395+
t.Parallel()
396+
397+
ctx:=testutil.Context(t,testutil.WaitShort)
398+
immutable,_:=coderdtest.DynamicParameterTemplate(t,templateAdmin,orgID, coderdtest.DynamicParameterTemplateParams{
399+
MainTF:"# PreviouslyMutable\n"+string(must(os.ReadFile("testdata/parameters/dynamicimmutable/main.tf"))),
400+
})
401+
402+
// Create the workspace with the mutable parameter
403+
wrk,err:=templateAdmin.CreateUserWorkspace(ctx,codersdk.Me, codersdk.CreateWorkspaceRequest{
404+
TemplateID:immutable.ID,
405+
Name:coderdtest.RandomUsername(t),
406+
RichParameterValues: []codersdk.WorkspaceBuildParameter{
407+
{Name:"isimmutable",Value:"false"},
408+
{Name:"immutable",Value:"coder"},
409+
},
410+
})
411+
require.NoError(t,err)
412+
coderdtest.AwaitWorkspaceBuildJobCompleted(t,templateAdmin,wrk.LatestBuild.ID)
413+
414+
// Switch it to immutable, which breaks the validation
415+
_,err=templateAdmin.CreateWorkspaceBuild(ctx,wrk.ID, codersdk.CreateWorkspaceBuildRequest{
416+
Transition:codersdk.WorkspaceTransitionStart,
417+
RichParameterValues: []codersdk.WorkspaceBuildParameter{
418+
{Name:"isimmutable",Value:"true"},
419+
{Name:"immutable",Value:"not-coder"},
420+
},
421+
})
422+
require.Error(t,err)
423+
require.ErrorContains(t,err,"is not mutable")
424+
})
357425
})
358426
}
359427

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
terraform {
2+
required_providers {
3+
coder={
4+
source="coder/coder"
5+
}
6+
}
7+
}
8+
9+
data"coder_workspace_owner""me" {}
10+
11+
data"coder_parameter""isimmutable" {
12+
name="isimmutable"
13+
type="bool"
14+
mutable=true
15+
default="true"
16+
}
17+
18+
data"coder_parameter""immutable" {
19+
name="immutable"
20+
type="string"
21+
mutable=data.coder_parameter.isimmutable.value=="false"
22+
default="Hello World"
23+
}

0 commit comments

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