diff --git a/codersdk/richparameters.go b/codersdk/richparameters.go index 2ddd5d00f6c41..6fc6b8e0c343f 100644 --- a/codersdk/richparameters.go +++ b/codersdk/richparameters.go @@ -164,7 +164,7 @@ type ParameterResolver struct { // resolves the correct value. It returns the value of the parameter, if valid, and an error if invalid. func (r *ParameterResolver) ValidateResolve(p TemplateVersionParameter, v *WorkspaceBuildParameter) (value string, err error) { prevV := r.findLastValue(p) - if !p.Mutable && v != nil && prevV != nil { + if !p.Mutable && v != nil && prevV != nil && v.Value != prevV.Value { return "", xerrors.Errorf("Parameter %q is not mutable, so it can't be updated after creating a workspace.", p.Name) } if p.Required && v == nil && prevV == nil { diff --git a/codersdk/richparameters_test.go b/codersdk/richparameters_test.go index 16365f7c2f416..5635a82beb6c6 100644 --- a/codersdk/richparameters_test.go +++ b/codersdk/richparameters_test.go @@ -1,6 +1,7 @@ package codersdk_test import ( + "fmt" "testing" "github.com/stretchr/testify/require" @@ -121,20 +122,60 @@ func TestParameterResolver_ValidateResolve_NewOverridesOld(t *testing.T) { func TestParameterResolver_ValidateResolve_Immutable(t *testing.T) { t.Parallel() uut := codersdk.ParameterResolver{ - Rich: []codersdk.WorkspaceBuildParameter{{Name: "n", Value: "5"}}, + Rich: []codersdk.WorkspaceBuildParameter{{Name: "n", Value: "old"}}, } p := codersdk.TemplateVersionParameter{ Name: "n", - Type: "number", + Type: "string", Required: true, Mutable: false, } - v, err := uut.ValidateResolve(p, &codersdk.WorkspaceBuildParameter{ - Name: "n", - Value: "6", - }) - require.Error(t, err) - require.Equal(t, "", v) + + cases := []struct { + name string + newValue string + expectedErr string + }{ + { + name: "mutation", + newValue: "new", // "new" != "old" + expectedErr: fmt.Sprintf("Parameter %q is not mutable", p.Name), + }, + { + // Values are case-sensitive. + name: "case change", + newValue: "Old", // "Old" != "old" + expectedErr: fmt.Sprintf("Parameter %q is not mutable", p.Name), + }, + { + name: "default", + newValue: "", // "" != "old" + expectedErr: fmt.Sprintf("Parameter %q is not mutable", p.Name), + }, + { + name: "no change", + newValue: "old", // "old" == "old" + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + v, err := uut.ValidateResolve(p, &codersdk.WorkspaceBuildParameter{ + Name: "n", + Value: tc.newValue, + }) + + if tc.expectedErr == "" { + require.NoError(t, err) + require.Equal(t, tc.newValue, v) + } else { + require.ErrorContains(t, err, tc.expectedErr) + require.Equal(t, "", v) + } + }) + } } func TestRichParameterValidation(t *testing.T) { diff --git a/enterprise/coderd/prebuilds/claim_test.go b/enterprise/coderd/prebuilds/claim_test.go index 145095e6533e7..ad31d2b4eff1b 100644 --- a/enterprise/coderd/prebuilds/claim_test.go +++ b/enterprise/coderd/prebuilds/claim_test.go @@ -329,9 +329,8 @@ func TestClaimPrebuild(t *testing.T) { require.NoError(t, err) stopBuild, err := userClient.CreateWorkspaceBuild(ctx, workspace.ID, codersdk.CreateWorkspaceBuildRequest{ - TemplateVersionID: version.ID, - Transition: codersdk.WorkspaceTransitionStop, - RichParameterValues: wp, + TemplateVersionID: version.ID, + Transition: codersdk.WorkspaceTransitionStop, }) require.NoError(t, err) coderdtest.AwaitWorkspaceBuildJobCompleted(t, userClient, stopBuild.ID) @@ -369,6 +368,17 @@ func templateWithAgentAndPresetsWithPrebuilds(desiredInstances int32) *echo.Resp }, }, }, + // Make sure immutable params don't break claiming logic + Parameters: []*proto.RichParameter{ + { + Name: "k1", + Description: "immutable param", + Type: "string", + DefaultValue: "", + Required: false, + Mutable: false, + }, + }, Presets: []*proto.Preset{ { Name: "preset-a", diff --git a/enterprise/coderd/prebuilds/reconcile_test.go b/enterprise/coderd/prebuilds/reconcile_test.go index 9783b215f185b..bc886fc0a8231 100644 --- a/enterprise/coderd/prebuilds/reconcile_test.go +++ b/enterprise/coderd/prebuilds/reconcile_test.go @@ -901,6 +901,16 @@ func setupTestDBTemplateVersion( ID: templateID, ActiveVersionID: templateVersion.ID, })) + // Make sure immutable params don't break prebuilt workspace deletion logic + dbgen.TemplateVersionParameter(t, db, database.TemplateVersionParameter{ + TemplateVersionID: templateVersion.ID, + Name: "test", + Description: "required & immutable param", + Type: "string", + DefaultValue: "", + Required: true, + Mutable: false, + }) return templateVersion.ID } @@ -999,7 +1009,7 @@ func setupTestDBWorkspace( OrganizationID: orgID, Error: buildError, }) - dbgen.WorkspaceBuild(t, db, database.WorkspaceBuild{ + workspaceBuild := dbgen.WorkspaceBuild(t, db, database.WorkspaceBuild{ WorkspaceID: workspace.ID, InitiatorID: initiatorID, TemplateVersionID: templateVersionID, @@ -1008,6 +1018,13 @@ func setupTestDBWorkspace( Transition: transition, CreatedAt: clock.Now(), }) + dbgen.WorkspaceBuildParameters(t, db, []database.WorkspaceBuildParameter{ + { + WorkspaceBuildID: workspaceBuild.ID, + Name: "test", + Value: "test", + }, + }) return workspace }