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 scenario

The parameter resolver was failing when deleting/claiming prebuilds
because a value for a previously-used parameter was provided to the
resolver, but since the value was unchanged (it's coming from the
preset) it failed in the resolver. The resolver was missing a check to
see if the old value != new value; if the values match then there's no
mutation of an immutable parameter.

---------

Signed-off-by: Danny Kopping <dannykopping@gmail.com>
This commit is contained in:
Yevhenii Shcherbina
2025-05-01 08:52:23 +00:00
committed by GitHub
parent c7fc7b91ec
commit 98e5611e16
4 changed files with 81 additions and 13 deletions
+1 -1
View File
@@ -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 {
+49 -8
View File
@@ -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) {