From cc6766e64a2ffb1424259306e0a9697e0191c91a Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 19 Mar 2026 14:05:34 -0500 Subject: [PATCH] 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 --- coderd/dynamicparameters/resolver.go | 36 ++++++++++ coderd/dynamicparameters/resolver_test.go | 83 +++++++++++++++++++++++ 2 files changed, 119 insertions(+) diff --git a/coderd/dynamicparameters/resolver.go b/coderd/dynamicparameters/resolver.go index 7fc67d29a0..b0a5a027c6 100644 --- a/coderd/dynamicparameters/resolver.go +++ b/coderd/dynamicparameters/resolver.go @@ -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. diff --git a/coderd/dynamicparameters/resolver_test.go b/coderd/dynamicparameters/resolver_test.go index e6675e6f4c..4f4785bb02 100644 --- a/coderd/dynamicparameters/resolver_test.go +++ b/coderd/dynamicparameters/resolver_test.go @@ -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) + } + }) + } + }) }