Skip to content

Commit 7c15a50

Browse files
mtojekammario
mtojek
authored and
ammario
committed
fix: do not skip parameter validation if min or max = 0 (coder#7707)
1 parent d5ffe6c commit 7c15a50

21 files changed

+155
-57
lines changed

‎cli/create_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
"github.com/coder/coder/cli/clitest"
1616
"github.com/coder/coder/coderd/coderdtest"
1717
"github.com/coder/coder/coderd/gitauth"
18+
"github.com/coder/coder/coderd/util/ptr"
1819
"github.com/coder/coder/codersdk"
1920
"github.com/coder/coder/provisioner/echo"
2021
"github.com/coder/coder/provisionersdk/proto"
@@ -494,7 +495,7 @@ func TestCreateValidateRichParameters(t *testing.T) {
494495
)
495496

496497
numberRichParameters := []*proto.RichParameter{
497-
{Name: numberParameterName, Type: "number", Mutable: true, ValidationMin: 3, ValidationMax: 10},
498+
{Name: numberParameterName, Type: "number", Mutable: true, ValidationMin: ptr.Ref(int32(3)), ValidationMax: ptr.Ref(int32(10))},
498499
}
499500

500501
stringRichParameters := []*proto.RichParameter{

‎cli/update_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111

1212
"github.com/coder/coder/cli/clitest"
1313
"github.com/coder/coder/coderd/coderdtest"
14+
"github.com/coder/coder/coderd/util/ptr"
1415
"github.com/coder/coder/codersdk"
1516
"github.com/coder/coder/provisioner/echo"
1617
"github.com/coder/coder/provisionersdk/proto"
@@ -245,7 +246,7 @@ func TestUpdateValidateRichParameters(t *testing.T) {
245246
)
246247

247248
numberRichParameters := []*proto.RichParameter{
248-
{Name: numberParameterName, Type: "number", Mutable: true, ValidationMin: 3, ValidationMax: 10},
249+
{Name: numberParameterName, Type: "number", Mutable: true, ValidationMin: ptr.Ref(int32(3)), ValidationMax: ptr.Ref(int32(10))},
249250
}
250251

251252
stringRichParameters := []*proto.RichParameter{

‎coderd/database/db2sdk/db2sdk.go

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,17 @@ func TemplateVersionParameter(param database.TemplateVersionParameter) (codersdk
4646
if err != nil {
4747
return codersdk.TemplateVersionParameter{}, err
4848
}
49+
50+
var validationMin *int32
51+
if param.ValidationMin.Valid {
52+
validationMin = &param.ValidationMin.Int32
53+
}
54+
55+
var validationMax *int32
56+
if param.ValidationMax.Valid {
57+
validationMax = &param.ValidationMax.Int32
58+
}
59+
4960
return codersdk.TemplateVersionParameter{
5061
Name: param.Name,
5162
DisplayName: param.DisplayName,
@@ -57,8 +68,8 @@ func TemplateVersionParameter(param database.TemplateVersionParameter) (codersdk
5768
Icon: param.Icon,
5869
Options: options,
5970
ValidationRegex: param.ValidationRegex,
60-
ValidationMin: param.ValidationMin,
61-
ValidationMax: param.ValidationMax,
71+
ValidationMin: validationMin,
72+
ValidationMax: validationMax,
6273
ValidationError: param.ValidationError,
6374
ValidationMonotonic: codersdk.ValidationMonotonicOrder(param.ValidationMonotonic),
6475
Required: param.Required,

‎coderd/database/dump.sql

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/migrations/000124_validation_min_max_nullable.down.sql

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
BEGIN;
2+
UPDATE template_version_parameters SET validation_min = 0 WHERE validation_min = NULL;
3+
UPDATE template_version_parameters SET validation_max = 0 WHERE validation_max = NULL;
4+
ALTER TABLE template_version_parameters ALTER COLUMN validation_min SET NOT NULL;
5+
ALTER TABLE template_version_parameters ALTER COLUMN validation_max SET NOT NULL;
6+
COMMIT;

‎coderd/database/migrations/000124_validation_min_max_nullable.up.sql

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
BEGIN;
2+
ALTER TABLE template_version_parameters ALTER COLUMN validation_min DROP NOT NULL;
3+
ALTER TABLE template_version_parameters ALTER COLUMN validation_max DROP NOT NULL;
4+
UPDATE template_version_parameters SET validation_min = NULL WHERE validation_min = 0;
5+
UPDATE template_version_parameters SET validation_max = NULL WHERE validation_max = 0;
6+
COMMIT;

‎coderd/database/models.go

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries.sql.go

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/provisionerdserver/provisionerdserver.go

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -919,6 +919,21 @@ func (server *Server) CompleteJob(ctx context.Context, completed *proto.Complete
919919
if err != nil {
920920
return nil, xerrors.Errorf("marshal parameter options: %w", err)
921921
}
922+
923+
var validationMin, validationMax sql.NullInt32
924+
if richParameter.ValidationMin != nil {
925+
validationMin = sql.NullInt32{
926+
Int32: *richParameter.ValidationMin,
927+
Valid: true,
928+
}
929+
}
930+
if richParameter.ValidationMax != nil {
931+
validationMax = sql.NullInt32{
932+
Int32: *richParameter.ValidationMax,
933+
Valid: true,
934+
}
935+
}
936+
922937
_, err = server.Database.InsertTemplateVersionParameter(ctx, database.InsertTemplateVersionParameterParams{
923938
TemplateVersionID: input.TemplateVersionID,
924939
Name: richParameter.Name,
@@ -931,8 +946,8 @@ func (server *Server) CompleteJob(ctx context.Context, completed *proto.Complete
931946
Options: options,
932947
ValidationRegex: richParameter.ValidationRegex,
933948
ValidationError: richParameter.ValidationError,
934-
ValidationMin: richParameter.ValidationMin,
935-
ValidationMax: richParameter.ValidationMax,
949+
ValidationMin: validationMin,
950+
ValidationMax: validationMax,
936951
ValidationMonotonic: richParameter.ValidationMonotonic,
937952
Required: richParameter.Required,
938953
LegacyVariableName: richParameter.LegacyVariableName,

‎coderd/templateversions.go

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1706,6 +1706,15 @@ func convertTemplateVersionParameter(param database.TemplateVersionParameter) (c
17061706
if err != nil {
17071707
return codersdk.TemplateVersionParameter{}, err
17081708
}
1709+
1710+
var validationMin, validationMax *int32
1711+
if param.ValidationMin.Valid {
1712+
validationMin = &param.ValidationMin.Int32
1713+
}
1714+
if param.ValidationMax.Valid {
1715+
validationMax = &param.ValidationMax.Int32
1716+
}
1717+
17091718
return codersdk.TemplateVersionParameter{
17101719
Name: param.Name,
17111720
DisplayName: param.DisplayName,
@@ -1717,8 +1726,8 @@ func convertTemplateVersionParameter(param database.TemplateVersionParameter) (c
17171726
Icon: param.Icon,
17181727
Options: options,
17191728
ValidationRegex: param.ValidationRegex,
1720-
ValidationMin: param.ValidationMin,
1721-
ValidationMax: param.ValidationMax,
1729+
ValidationMin: validationMin,
1730+
ValidationMax: validationMax,
17221731
ValidationError: param.ValidationError,
17231732
ValidationMonotonic: codersdk.ValidationMonotonicOrder(param.ValidationMonotonic),
17241733
Required: param.Required,

‎coderd/workspaces_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1973,8 +1973,8 @@ func TestWorkspaceWithRichParameters(t *testing.T) {
19731973
DisplayName: secondParameterDisplayName,
19741974
Type: secondParameterType,
19751975
Description: secondParameterDescription,
1976-
ValidationMin: 1,
1977-
ValidationMax: 3,
1976+
ValidationMin: ptr.Ref(int32(1)),
1977+
ValidationMax: ptr.Ref(int32(3)),
19781978
ValidationMonotonic: string(secondParameterValidationMonotonic),
19791979
},
19801980
},

‎codersdk/richparameters.go

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -79,15 +79,23 @@ func ValidateWorkspaceBuildParameter(richParameter TemplateVersionParameter, bui
7979
}
8080

8181
validation := &provider.Validation{
82-
Min: int(richParameter.ValidationMin),
83-
Max: int(richParameter.ValidationMax),
82+
Min: ptrInt(richParameter.ValidationMin),
83+
Max: ptrInt(richParameter.ValidationMax),
8484
Regex: richParameter.ValidationRegex,
8585
Error: richParameter.ValidationError,
8686
Monotonic: string(richParameter.ValidationMonotonic),
8787
}
8888
return validation.Valid(richParameter.Type, value)
8989
}
9090

91+
func ptrInt(number *int32) *int {
92+
if number == nil {
93+
return nil
94+
}
95+
n := int(*number)
96+
return &n
97+
}
98+
9199
func findBuildParameter(params []WorkspaceBuildParameter, parameterName string) (*WorkspaceBuildParameter, bool) {
92100
if params == nil {
93101
return nil, false
@@ -111,7 +119,8 @@ func parameterValuesAsArray(options []TemplateVersionParameterOption) []string {
111119

112120
func validationEnabled(param TemplateVersionParameter) bool {
113121
return len(param.ValidationRegex) > 0 ||
114-
(param.ValidationMin != 0 && param.ValidationMax != 0) ||
122+
param.ValidationMin != nil ||
123+
param.ValidationMax != nil ||
115124
len(param.ValidationMonotonic) > 0 ||
116125
param.Type == "bool" || // boolean type doesn't have any custom validation rules, but the value must be checked (true/false).
117126
param.Type == "list(string)" // list(string) type doesn't have special validation, but we need to check if this is a correct list.

‎codersdk/richparameters_test.go

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55

66
"github.com/stretchr/testify/require"
77

8+
"github.com/coder/coder/coderd/util/ptr"
89
"github.com/coder/coder/codersdk"
910
)
1011

@@ -72,8 +73,8 @@ func TestParameterResolver_ValidateResolve_PrevInvalid(t *testing.T) {
7273
p := codersdk.TemplateVersionParameter{
7374
Name: "n",
7475
Type: "number",
75-
ValidationMax: 10,
76-
ValidationMin: 1,
76+
ValidationMax: ptr.Ref(int32(10)),
77+
ValidationMin: ptr.Ref(int32(1)),
7778
}
7879
v, err := uut.ValidateResolve(p, nil)
7980
require.Error(t, err)
@@ -89,8 +90,8 @@ func TestParameterResolver_ValidateResolve_DefaultInvalid(t *testing.T) {
8990
p := codersdk.TemplateVersionParameter{
9091
Name: "n",
9192
Type: "number",
92-
ValidationMax: 10,
93-
ValidationMin: 1,
93+
ValidationMax: ptr.Ref(int32(10)),
94+
ValidationMin: ptr.Ref(int32(1)),
9495
DefaultValue: "11",
9596
}
9697
v, err := uut.ValidateResolve(p, nil)
@@ -221,19 +222,19 @@ func TestRichParameterValidation(t *testing.T) {
221222

222223
numberRichParameters := []codersdk.TemplateVersionParameter{
223224
{Name: stringParameterName, Type: "string", Mutable: true},
224-
{Name: numberParameterName, Type: "number", Mutable: true, ValidationMin: 3, ValidationMax: 10},
225+
{Name: numberParameterName, Type: "number", Mutable: true, ValidationMin: ptr.Ref(int32(3)), ValidationMax: ptr.Ref(int32(10))},
225226
{Name: boolParameterName, Type: "bool", Mutable: true},
226227
}
227228

228229
monotonicIncreasingNumberRichParameters := []codersdk.TemplateVersionParameter{
229230
{Name: stringParameterName, Type: "string", Mutable: true},
230-
{Name: numberParameterName, Type: "number", Mutable: true, ValidationMin: 3, ValidationMax: 10, ValidationMonotonic: "increasing"},
231+
{Name: numberParameterName, Type: "number", Mutable: true, ValidationMin: ptr.Ref(int32(3)), ValidationMax: ptr.Ref(int32(10)), ValidationMonotonic: "increasing"},
231232
{Name: boolParameterName, Type: "bool", Mutable: true},
232233
}
233234

234235
monotonicDecreasingNumberRichParameters := []codersdk.TemplateVersionParameter{
235236
{Name: stringParameterName, Type: "string", Mutable: true},
236-
{Name: numberParameterName, Type: "number", Mutable: true, ValidationMin: 3, ValidationMax: 10, ValidationMonotonic: "decreasing"},
237+
{Name: numberParameterName, Type: "number", Mutable: true, ValidationMin: ptr.Ref(int32(3)), ValidationMax: ptr.Ref(int32(10)), ValidationMonotonic: "decreasing"},
237238
{Name: boolParameterName, Type: "bool", Mutable: true},
238239
}
239240

‎codersdk/templateversions.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,8 @@ type TemplateVersionParameter struct {
5959
Options []TemplateVersionParameterOption `json:"options"`
6060
ValidationError string `json:"validation_error,omitempty"`
6161
ValidationRegex string `json:"validation_regex,omitempty"`
62-
ValidationMin int32 `json:"validation_min,omitempty"`
63-
ValidationMax int32 `json:"validation_max,omitempty"`
62+
ValidationMin *int32 `json:"validation_min,omitempty"`
63+
ValidationMax *int32 `json:"validation_max,omitempty"`
6464
ValidationMonotonic ValidationMonotonicOrder `json:"validation_monotonic,omitempty" enums:"increasing,decreasing"`
6565
Required bool `json:"required"`
6666
LegacyVariableName string `json:"legacy_variable_name,omitempty"`

‎go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ require (
7575
github.com/codeclysm/extract v2.2.0+incompatible
7676
github.com/coder/flog v1.1.0
7777
github.com/coder/retry v1.3.1-0.20230210155434-e90a2e1e091d
78-
github.com/coder/terraform-provider-coder v0.6.23
78+
github.com/coder/terraform-provider-coder v0.8.1
7979
github.com/coder/wgtunnel v0.1.5
8080
github.com/coreos/go-oidc/v3 v3.6.0
8181
github.com/coreos/go-systemd v0.0.0-20191104093116-d3cd4ed1dbcf

‎go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -336,8 +336,8 @@ github.com/coder/ssh v0.0.0-20230421140225-04bb837133e1 h1:LBw76rEDuhNJyohve11mb
336336
github.com/coder/ssh v0.0.0-20230421140225-04bb837133e1/go.mod h1:ZSS+CUoKHDrqVakTfTWUlKSr9MtMFkC4UvtQKD7O914=
337337
github.com/coder/tailscale v0.0.0-20230522123520-74712221d00f h1:F0Xr1d8h8dAHn7tab1HXuzYFkcjmCydnEfdMbkOhlVk=
338338
github.com/coder/tailscale v0.0.0-20230522123520-74712221d00f/go.mod h1:jpg+77g19FpXL43U1VoIqoSg1K/Vh5CVxycGldQ8KhA=
339-
github.com/coder/terraform-provider-coder v0.6.23 h1:O2Rcj0umez4DfVdGnKZi63z1Xzxd0IQOn9VQDB8YU8g=
340-
github.com/coder/terraform-provider-coder v0.6.23/go.mod h1:UIfU3bYNeSzJJvHyJ30tEKjD6Z9utloI+HUM/7n94CY=
339+
github.com/coder/terraform-provider-coder v0.8.1 h1:i/LhvFi+Ei0gL+h4GItJfwtxjcITTlQhS+R8J+0vRo8=
340+
github.com/coder/terraform-provider-coder v0.8.1/go.mod h1:UIfU3bYNeSzJJvHyJ30tEKjD6Z9utloI+HUM/7n94CY=
341341
github.com/coder/wgtunnel v0.1.5 h1:WP3sCj/3iJ34eKvpMQEp1oJHvm24RYh0NHbj1kfUKfs=
342342
github.com/coder/wgtunnel v0.1.5/go.mod h1:bokoUrHnUFY4lu9KOeSYiIcHTI2MO1KwqumU4DPDyJI=
343343
github.com/containerd/aufs v0.0.0-20200908144142-dab0cbea06f4/go.mod h1:nukgQABAEopAHvB6j7cnP5zJ+/3aVcE7hCYqvIwAHyE=

‎provisioner/terraform/resources.go

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -483,8 +483,8 @@ func ConvertState(modules []*tfjson.StateModule, rawGraph string, rawParameterNa
483483
if len(param.Validation) == 1 {
484484
protoParam.ValidationRegex = param.Validation[0].Regex
485485
protoParam.ValidationError = param.Validation[0].Error
486-
protoParam.ValidationMax = int32(param.Validation[0].Max)
487-
protoParam.ValidationMin = int32(param.Validation[0].Min)
486+
protoParam.ValidationMax = ptrInt32(param.Validation[0].Max)
487+
protoParam.ValidationMin = ptrInt32(param.Validation[0].Min)
488488
protoParam.ValidationMonotonic = param.Validation[0].Monotonic
489489
}
490490
if len(param.Option) > 0 {
@@ -527,6 +527,15 @@ func ConvertState(modules []*tfjson.StateModule, rawGraph string, rawParameterNa
527527
}, nil
528528
}
529529

530+
func ptrInt32(number *int) *int32 {
531+
var n int32
532+
if number == nil {
533+
return &n
534+
}
535+
n = int32(*number)
536+
return &n
537+
}
538+
530539
// convertAddressToLabel returns the Terraform address without the count
531540
// specifier.
532541
// eg. "module.ec2_dev.ec2_instance.dev[0]" becomes "module.ec2_dev.ec2_instance.dev"

0 commit comments

Comments
 (0)