Skip to content

Commit e03d15f

Browse files
EmyrkEmyrk
committed
fixup test to take an opinion
1 parent ef2a8fd commit e03d15f

File tree

3 files changed

+71
-6
lines changed

3 files changed

+71
-6
lines changed

‎coderd/dynamicparameters/resolver.go

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ func ResolveParameters(
6767
//
6868
// This is how the form should look to the user on their workspace settings page.
6969
// This is the original form truth that our validations should initially be based on.
70-
output, diags := renderer.Render(ctx, ownerID, values.ValuesMap())
70+
output, diags := renderer.Render(ctx, ownerID, previousValuesMap)
7171
if diags.HasErrors() {
7272
// Top level diagnostics should break the build. Previous values (and new) should
7373
// always be valid. If there is a case where this is not true, then this has to
@@ -81,6 +81,7 @@ func ResolveParameters(
8181
//
8282
// To enforce these, the user's input values are trimmed based on the
8383
// mutability and ephemeral parameters defined in the template version.
84+
wasImmutable := make(map[string]struct{})
8485
for _, parameter := range output.Parameters {
8586
// Ephemeral parameters should not be taken from the previous build.
8687
// They must always be explicitly set in every build.
@@ -98,6 +99,7 @@ func ResolveParameters(
9899
//
99100
// We do this so the next form render uses the original immutable value.
100101
if !firstBuild && !parameter.Mutable {
102+
wasImmutable[parameter.Name] = struct{}{}
101103
delete(values, parameter.Name)
102104
prev, ok := previousValuesMap[parameter.Name]
103105
if ok {
@@ -123,7 +125,12 @@ func ResolveParameters(
123125
for _, parameter := range output.Parameters {
124126
parameterNames[parameter.Name] = struct{}{}
125127

126-
if !firstBuild && !parameter.Mutable {
128+
// Immutability is sourced from the current `mutable` argument, and also the
129+
// previous parameter's `mutable` argument. This means you cannot flip an
130+
// `immutable` parameter to `mutable` in a single build. This is to preserve the
131+
// original mutability of the parameter.
132+
_, wi := wasImmutable[parameter.Name]
133+
if !firstBuild && (!parameter.Mutable || wi) {
127134
originalValue, ok := originalValues[parameter.Name]
128135
// Immutable parameters should not be changed after the first build.
129136
// If the value matches the original value, that is fine.
@@ -137,13 +144,19 @@ func ResolveParameters(
137144
if parameter.Source != nil {
138145
src = &parameter.Source.HCLBlock().TypeRange
139146
}
147+
errTitle := "Immutable"
148+
// In the strange case someone flips mutability from `true` to `false`.
149+
// Change the error title to indicate that this was previously immutable.
150+
if wi && parameter.Mutable {
151+
errTitle = "Previously immutable"
152+
}
140153

141154
// An immutable parameter was changed, which is not allowed.
142155
// Add a failed diagnostic to the output.
143156
parameterError.Extend(parameter.Name, hcl.Diagnostics{
144157
&hcl.Diagnostic{
145158
Severity: hcl.DiagError,
146-
Summary: "Immutable parameter changed",
159+
Summary: fmt.Sprintf("%s parameter changed", errTitle),
147160
Detail: fmt.Sprintf("Parameter %q is not mutable, so it can't be updated after creating a workspace.", parameter.Name),
148161
Subject: src,
149162
},

‎coderd/dynamicparameters/resolver_test.go

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,58 @@ import (
2020
func TestResolveParameters(t *testing.T) {
2121
t.Parallel()
2222

23+
t.Run("ChangeImmutable", func(t *testing.T) {
24+
t.Parallel()
25+
26+
ctrl := gomock.NewController(t)
27+
render := rendermock.NewMockRenderer(ctrl)
28+
29+
// The first render takes in the existing values
30+
render.EXPECT().
31+
Render(gomock.Any(), gomock.Any(), gomock.Any()).
32+
Return(&preview.Output{
33+
Parameters: []previewtypes.Parameter{
34+
{
35+
ParameterData: previewtypes.ParameterData{
36+
Name: "immutable",
37+
Type: previewtypes.ParameterTypeString,
38+
FormType: provider.ParameterFormTypeInput,
39+
Mutable: false,
40+
DefaultValue: previewtypes.StringLiteral("foo"),
41+
Required: true,
42+
},
43+
Value: previewtypes.StringLiteral("foo"),
44+
Diagnostics: nil,
45+
},
46+
},
47+
}, nil).
48+
Return(&preview.Output{
49+
Parameters: []previewtypes.Parameter{
50+
{
51+
ParameterData: previewtypes.ParameterData{
52+
Name: "immutable",
53+
Type: previewtypes.ParameterTypeString,
54+
FormType: provider.ParameterFormTypeInput,
55+
Mutable: false,
56+
DefaultValue: previewtypes.StringLiteral("foo"),
57+
Required: true,
58+
},
59+
Value: previewtypes.StringLiteral("foo"),
60+
Diagnostics: nil,
61+
},
62+
},
63+
}, nil)
64+
65+
ctx := testutil.Context(t, testutil.WaitShort)
66+
values, err := dynamicparameters.ResolveParameters(ctx, uuid.New(), render, false,
67+
[]database.WorkspaceBuildParameter{}, // No previous values
68+
[]codersdk.WorkspaceBuildParameter{}, // No new build values
69+
[]database.TemplateVersionPresetParameter{}, // No preset values
70+
)
71+
require.NoError(t, err)
72+
require.Equal(t, map[string]string{"immutable": "foo"}, values)
73+
})
74+
2375
t.Run("NewImmutable", func(t *testing.T) {
2476
t.Parallel()
2577

‎enterprise/coderd/dynamicparameters_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,7 @@ func TestDynamicParameterBuild(t *testing.T) {
357357
t.Run("ImmutableChangeValue", func(t *testing.T) {
358358
// Ok this is a weird test to document how things are working.
359359
// What if a parameter flips it's immutability based on a value?
360+
// The current behavior is to source immutability from the previous build.
360361
t.Parallel()
361362

362363
ctx := testutil.Context(t, testutil.WaitShort)
@@ -378,15 +379,14 @@ func TestDynamicParameterBuild(t *testing.T) {
378379
coderdtest.AwaitWorkspaceBuildJobCompleted(t, templateAdmin, wrk.LatestBuild.ID)
379380

380381
// Try new values
381-
bld, err := templateAdmin.CreateWorkspaceBuild(ctx, wrk.ID, codersdk.CreateWorkspaceBuildRequest{
382+
_, err = templateAdmin.CreateWorkspaceBuild(ctx, wrk.ID, codersdk.CreateWorkspaceBuildRequest{
382383
Transition: codersdk.WorkspaceTransitionStart,
383384
RichParameterValues: []codersdk.WorkspaceBuildParameter{
384385
{Name: "isimmutable", Value: "false"},
385386
{Name: "immutable", Value: "not-coder"},
386387
},
387388
})
388-
require.NoError(t, err)
389-
coderdtest.AwaitWorkspaceBuildJobCompleted(t, templateAdmin, bld.ID)
389+
require.ErrorContains(t, err, `Previously immutable parameter changed`)
390390
})
391391
})
392392
}

0 commit comments

Comments
 (0)