mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
chore: apply monotonic validation to workspace builds (#23180)
Still not applying at the dynamic parameters websocket. The wsbuilder is the source of truth for previous values, so this is the most accurate and still will fail in the synchronous api call to build a workspace. This mirrors how we handle immutable params. Closes https://github.com/coder/coder/issues/19064
This commit is contained in:
@@ -10,6 +10,7 @@ import (
|
||||
"github.com/coder/coder/v2/coderd/database"
|
||||
"github.com/coder/coder/v2/coderd/util/slice"
|
||||
"github.com/coder/coder/v2/codersdk"
|
||||
"github.com/coder/terraform-provider-coder/v2/provider"
|
||||
)
|
||||
|
||||
type parameterValueSource int
|
||||
@@ -109,6 +110,7 @@ func ResolveParameters(
|
||||
for _, parameter := range output.Parameters {
|
||||
parameterNames[parameter.Name] = struct{}{}
|
||||
|
||||
// Validate mutability constraints.
|
||||
if !firstBuild && !parameter.Mutable {
|
||||
// previousValuesMap should be used over the first render output
|
||||
// for the previous state of parameters. The previous build
|
||||
@@ -142,6 +144,40 @@ func ResolveParameters(
|
||||
}
|
||||
}
|
||||
|
||||
// Validate monotonic constraints. Monotonic parameters
|
||||
// require the value to only increase or only decrease
|
||||
// relative to the previous build.
|
||||
if !firstBuild {
|
||||
prevStr, hasPrev := previousValuesMap[parameter.Name]
|
||||
// Only validate on currently valid parameters. Do not load extra diagnostics if
|
||||
// the parameter is already invalid.
|
||||
if hasPrev && parameter.Value.Valid() {
|
||||
MonotonicValidationLoop:
|
||||
for _, v := range parameter.Validations {
|
||||
if v.Monotonic == nil || *v.Monotonic == "" {
|
||||
continue
|
||||
}
|
||||
|
||||
validation := &provider.Validation{
|
||||
Monotonic: *v.Monotonic,
|
||||
MinDisabled: true,
|
||||
MaxDisabled: true,
|
||||
}
|
||||
prev := prevStr
|
||||
if err := validation.Valid(provider.OptionType(parameter.Type), parameter.Value.AsString(), &prev); err != nil {
|
||||
parameterError.Extend(parameter.Name, hcl.Diagnostics{
|
||||
&hcl.Diagnostic{
|
||||
Severity: hcl.DiagError,
|
||||
Summary: fmt.Sprintf("Parameter %q monotonicity", parameter.Name),
|
||||
Detail: err.Error(),
|
||||
},
|
||||
})
|
||||
break MonotonicValidationLoop
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TODO: Fix the `hcl.Diagnostics(...)` type casting. It should not be needed.
|
||||
if hcl.Diagnostics(parameter.Diagnostics).HasErrors() {
|
||||
// All validation errors are raised here for each parameter.
|
||||
|
||||
@@ -11,6 +11,7 @@ import (
|
||||
"github.com/coder/coder/v2/coderd/dynamicparameters"
|
||||
"github.com/coder/coder/v2/coderd/dynamicparameters/rendermock"
|
||||
"github.com/coder/coder/v2/coderd/httpapi/httperror"
|
||||
"github.com/coder/coder/v2/coderd/util/ptr"
|
||||
"github.com/coder/coder/v2/codersdk"
|
||||
"github.com/coder/coder/v2/testutil"
|
||||
"github.com/coder/preview"
|
||||
@@ -122,4 +123,86 @@ func TestResolveParameters(t *testing.T) {
|
||||
require.Len(t, respErr.Validations, 1)
|
||||
require.Contains(t, respErr.Validations[0].Error(), "is not mutable")
|
||||
})
|
||||
|
||||
t.Run("Monotonic", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
monotonic string
|
||||
prev string // empty means no previous value
|
||||
cur string
|
||||
firstBuild bool
|
||||
expectErr string // empty means no error expected
|
||||
}{
|
||||
// Increasing
|
||||
{name: "increasing/increase allowed", monotonic: "increasing", prev: "5", cur: "10"},
|
||||
{name: "increasing/same allowed", monotonic: "increasing", prev: "5", cur: "5"},
|
||||
{name: "increasing/decrease rejected", monotonic: "increasing", prev: "10", cur: "5", expectErr: "must be equal or greater than previous value"},
|
||||
// Decreasing
|
||||
{name: "decreasing/decrease allowed", monotonic: "decreasing", prev: "10", cur: "5"},
|
||||
{name: "decreasing/same allowed", monotonic: "decreasing", prev: "5", cur: "5"},
|
||||
{name: "decreasing/increase rejected", monotonic: "decreasing", prev: "5", cur: "10", expectErr: "must be equal or lower than previous value"},
|
||||
// First build — not enforced
|
||||
{name: "increasing/first build", monotonic: "increasing", cur: "1", firstBuild: true},
|
||||
// No previous value — not enforced
|
||||
{name: "increasing/no previous", monotonic: "increasing", cur: "5"},
|
||||
}
|
||||
|
||||
for _, tc := range tests {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
ctrl := gomock.NewController(t)
|
||||
render := rendermock.NewMockRenderer(ctrl)
|
||||
|
||||
render.EXPECT().
|
||||
Render(gomock.Any(), gomock.Any(), gomock.Any()).
|
||||
AnyTimes().
|
||||
Return(&preview.Output{
|
||||
Parameters: []previewtypes.Parameter{
|
||||
{
|
||||
ParameterData: previewtypes.ParameterData{
|
||||
Name: "param",
|
||||
Type: previewtypes.ParameterTypeNumber,
|
||||
FormType: provider.ParameterFormTypeInput,
|
||||
Mutable: true,
|
||||
Validations: []*previewtypes.ParameterValidation{
|
||||
{Monotonic: ptr.Ref(tc.monotonic)},
|
||||
},
|
||||
},
|
||||
Value: previewtypes.StringLiteral(tc.cur),
|
||||
Diagnostics: nil,
|
||||
},
|
||||
},
|
||||
}, nil)
|
||||
|
||||
var previousValues []database.WorkspaceBuildParameter
|
||||
if tc.prev != "" {
|
||||
previousValues = []database.WorkspaceBuildParameter{
|
||||
{Name: "param", Value: tc.prev},
|
||||
}
|
||||
}
|
||||
|
||||
ctx := testutil.Context(t, testutil.WaitShort)
|
||||
_, err := dynamicparameters.ResolveParameters(ctx, uuid.New(), render, tc.firstBuild,
|
||||
previousValues,
|
||||
[]codersdk.WorkspaceBuildParameter{
|
||||
{Name: "param", Value: tc.cur},
|
||||
},
|
||||
[]database.TemplateVersionPresetParameter{},
|
||||
)
|
||||
if tc.expectErr != "" {
|
||||
require.Error(t, err)
|
||||
resp, ok := httperror.IsResponder(err)
|
||||
require.True(t, ok)
|
||||
_, respErr := resp.Response()
|
||||
require.Len(t, respErr.Validations, 1)
|
||||
require.Contains(t, respErr.Validations[0].Error(), tc.expectErr)
|
||||
} else {
|
||||
require.NoError(t, err)
|
||||
}
|
||||
})
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user