From 0d0a3c808450145d61e9f575e4a8e8f58d97950b Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Tue, 30 May 2023 19:45:14 +0200 Subject: [PATCH 01/14] bug: validation min = 0 --- provider/parameter_test.go | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/provider/parameter_test.go b/provider/parameter_test.go index 945820a1..c25fe88a 100644 --- a/provider/parameter_test.go +++ b/provider/parameter_test.go @@ -19,6 +19,34 @@ func TestParameter(t *testing.T) { ExpectError *regexp.Regexp Check func(state *terraform.ResourceState) }{{ + Name: "NumberValidation_Max", + Config: ` + data "coder_parameter" "region" { + name = "Region" + type = "number" + default = 2 + validation { + max = 9 + } + } + `, + Check: func(state *terraform.ResourceState) { + for key, expected := range map[string]string{ + "name": "Region", + "type": "number", + "validation.#": "1", + "default": "2", + "validation.0.max": "9", + } { + require.Equal(t, expected, state.Primary.Attributes[key]) + } + + _, foundDisplayName := state.Primary.Attributes["display_name"] + require.False(t, foundDisplayName, "display_name = "+state.Primary.Attributes["display_name"]) + _, foundValidationMin := state.Primary.Attributes["validation.0.min"] + require.False(t, foundValidationMin, "validation.0.min = "+state.Primary.Attributes["validation.0.min"]) + }, + }, { Name: "FieldsExist", Config: ` data "coder_parameter" "region" { From 5f4b94b799eaefc44b2cdb532ca2a55be4424398 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Wed, 31 May 2023 10:26:30 +0200 Subject: [PATCH 02/14] pseudocode: max_ok, min_ok --- provider/parameter.go | 23 +++++++++++++++++++++-- provider/parameter_test.go | 17 +++++++---------- 2 files changed, 28 insertions(+), 12 deletions(-) diff --git a/provider/parameter.go b/provider/parameter.go index 79ea2480..bec8ac72 100644 --- a/provider/parameter.go +++ b/provider/parameter.go @@ -28,8 +28,11 @@ type Option struct { } type Validation struct { - Min *int - Max *int + Min *int + MinOk bool `mapstructure:"min_ok"` + Max *int + MaxOk bool `mapstructure:"max_ok"` + Monotonic string Regex string @@ -288,11 +291,21 @@ func parameterDataSource() *schema.Resource { Optional: true, Description: "The minimum of a number parameter.", }, + "min_ok": { + Type: schema.TypeBool, + Computed: true, + Description: "Helper field to check if min is present", + }, "max": { Type: schema.TypeInt, Optional: true, Description: "The maximum of a number parameter.", }, + "max_ok": { + Type: schema.TypeBool, + Computed: true, + Description: "Helper field to check if max is present", + }, "monotonic": { Type: schema.TypeString, Optional: true, @@ -366,9 +379,15 @@ func fixValidationResourceData(rawConfig cty.Value, validation interface{}) (int // Fix the resource data if rawValidationRule["min"].IsNull() { validationRule["min"] = nil + validationRule["min_ok"] = false + } else { + validationRule["min_ok"] = true } if rawValidationRule["max"].IsNull() { validationRule["max"] = nil + validationRule["max_ok"] = false + } else { + validationRule["max_ok"] = true } return vArr, nil } diff --git a/provider/parameter_test.go b/provider/parameter_test.go index c25fe88a..1c79330a 100644 --- a/provider/parameter_test.go +++ b/provider/parameter_test.go @@ -32,19 +32,16 @@ func TestParameter(t *testing.T) { `, Check: func(state *terraform.ResourceState) { for key, expected := range map[string]string{ - "name": "Region", - "type": "number", - "validation.#": "1", - "default": "2", - "validation.0.max": "9", + "name": "Region", + "type": "number", + "validation.#": "1", + "default": "2", + "validation.0.max": "9", + "validation.0.min_ok": "false", + "validation.0.max_ok": "true", } { require.Equal(t, expected, state.Primary.Attributes[key]) } - - _, foundDisplayName := state.Primary.Attributes["display_name"] - require.False(t, foundDisplayName, "display_name = "+state.Primary.Attributes["display_name"]) - _, foundValidationMin := state.Primary.Attributes["validation.0.min"] - require.False(t, foundValidationMin, "validation.0.min = "+state.Primary.Attributes["validation.0.min"]) }, }, { Name: "FieldsExist", From f0b3a5ba01d125dd33d793918547870984cf442a Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Wed, 31 May 2023 11:01:47 +0200 Subject: [PATCH 03/14] dont need pointers now --- docs/data-sources/parameter.md | 5 +++++ provider/parameter.go | 16 ++++++++-------- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/docs/data-sources/parameter.md b/docs/data-sources/parameter.md index 6b4b7c09..a8c89e22 100644 --- a/docs/data-sources/parameter.md +++ b/docs/data-sources/parameter.md @@ -63,4 +63,9 @@ Optional: - `monotonic` (String) Number monotonicity, either increasing or decreasing. - `regex` (String) A regex for the input parameter to match against. +Read-Only: + +- `max_ok` (Boolean) Helper field to check if max is present +- `min_ok` (Boolean) Helper field to check if min is present + diff --git a/provider/parameter.go b/provider/parameter.go index bec8ac72..742784e6 100644 --- a/provider/parameter.go +++ b/provider/parameter.go @@ -28,9 +28,9 @@ type Option struct { } type Validation struct { - Min *int + Min int MinOk bool `mapstructure:"min_ok"` - Max *int + Max int MaxOk bool `mapstructure:"max_ok"` Monotonic string @@ -420,10 +420,10 @@ func valueIsType(typ, value string) diag.Diagnostics { func (v *Validation) Valid(typ, value string) error { if typ != "number" { - if v.Min != nil { + if v.MinOk { return fmt.Errorf("a min cannot be specified for a %s type", typ) } - if v.Max != nil { + if v.MaxOk { return fmt.Errorf("a max cannot be specified for a %s type", typ) } } @@ -456,11 +456,11 @@ func (v *Validation) Valid(typ, value string) error { if err != nil { return fmt.Errorf("value %q is not a number", value) } - if v.Min != nil && num < *v.Min { - return fmt.Errorf("value %d is less than the minimum %d", num, *v.Min) + if v.MinOk && num < v.Min { + return fmt.Errorf("value %d is less than the minimum %d", num, v.Min) } - if v.Max != nil && num > *v.Max { - return fmt.Errorf("value %d is more than the maximum %d", num, *v.Max) + if v.MaxOk && num > v.Max { + return fmt.Errorf("value %d is more than the maximum %d", num, v.Max) } if v.Monotonic != "" && v.Monotonic != ValidationMonotonicIncreasing && v.Monotonic != ValidationMonotonicDecreasing { return fmt.Errorf("number monotonicity can be either %q or %q", ValidationMonotonicIncreasing, ValidationMonotonicDecreasing) From 1fcdfa4917521cde8086425cd1f00779b567674a Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Wed, 31 May 2023 11:06:37 +0200 Subject: [PATCH 04/14] fix --- provider/decode_test.go | 7 ++++--- provider/parameter_test.go | 26 +++++++++++--------------- 2 files changed, 15 insertions(+), 18 deletions(-) diff --git a/provider/decode_test.go b/provider/decode_test.go index ca3feccd..c86fc109 100644 --- a/provider/decode_test.go +++ b/provider/decode_test.go @@ -3,10 +3,11 @@ package provider_test import ( "testing" - "github.com/coder/terraform-provider-coder/provider" "github.com/mitchellh/mapstructure" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/coder/terraform-provider-coder/provider" ) func TestDecode(t *testing.T) { @@ -38,6 +39,6 @@ func TestDecode(t *testing.T) { assert.Equal(t, displayName, param.DisplayName) assert.Equal(t, legacyVariable, param.LegacyVariable) assert.Equal(t, legacyVariableName, param.LegacyVariableName) - assert.Equal(t, (*int)(nil), param.Validation[0].Min) - assert.Equal(t, 5, *param.Validation[0].Max) + assert.Equal(t, 5, param.Validation[0].Max) + assert.Equal(t, 0, param.Validation[0].Min) } diff --git a/provider/parameter_test.go b/provider/parameter_test.go index 1c79330a..36cd9910 100644 --- a/provider/parameter_test.go +++ b/provider/parameter_test.go @@ -492,18 +492,18 @@ func TestValueValidatesType(t *testing.T) { Regex, RegexError string Min, - Max *int + Max int Monotonic string Error *regexp.Regexp }{{ Name: "StringWithMin", Type: "string", - Min: ptrNumber(1), + Min: 1, Error: regexp.MustCompile("cannot be specified"), }, { Name: "StringWithMax", Type: "string", - Max: ptrNumber(1), + Max: 1, Error: regexp.MustCompile("cannot be specified"), }, { Name: "NonStringWithRegex", @@ -523,13 +523,13 @@ func TestValueValidatesType(t *testing.T) { Name: "NumberBelowMin", Type: "number", Value: "0", - Min: ptrNumber(1), + Min: 1, Error: regexp.MustCompile("is less than the minimum 1"), }, { Name: "NumberAboveMax", Type: "number", Value: "2", - Max: ptrNumber(1), + Max: 1, Error: regexp.MustCompile("is more than the maximum 1"), }, { Name: "InvalidBool", @@ -547,23 +547,23 @@ func TestValueValidatesType(t *testing.T) { Name: "InvalidMonotonicity", Type: "number", Value: "1", - Min: ptrNumber(0), - Max: ptrNumber(2), + Min: 0, + Max: 2, Monotonic: "foobar", Error: regexp.MustCompile(`number monotonicity can be either "increasing" or "decreasing"`), }, { Name: "IncreasingMonotonicity", Type: "number", Value: "1", - Min: ptrNumber(0), - Max: ptrNumber(2), + Min: 0, + Max: 2, Monotonic: "increasing", }, { Name: "DecreasingMonotonicity", Type: "number", Value: "1", - Min: ptrNumber(0), - Max: ptrNumber(2), + Min: 0, + Max: 2, Monotonic: "decreasing", }, { Name: "ValidListOfStrings", @@ -599,7 +599,3 @@ func TestValueValidatesType(t *testing.T) { }) } } - -func ptrNumber(i int) *int { - return &i -} From 436fd8a3cd7e47042e1037460d0a584a1ec3da80 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Wed, 31 May 2023 11:08:22 +0200 Subject: [PATCH 05/14] fix --- provider/parameter.go | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/provider/parameter.go b/provider/parameter.go index 742784e6..632af4c6 100644 --- a/provider/parameter.go +++ b/provider/parameter.go @@ -379,16 +379,13 @@ func fixValidationResourceData(rawConfig cty.Value, validation interface{}) (int // Fix the resource data if rawValidationRule["min"].IsNull() { validationRule["min"] = nil - validationRule["min_ok"] = false - } else { - validationRule["min_ok"] = true } if rawValidationRule["max"].IsNull() { validationRule["max"] = nil - validationRule["max_ok"] = false - } else { - validationRule["max_ok"] = true } + + validationRule["min_ok"] = !rawValidationRule["min"].IsNull() + validationRule["max_ok"] = !rawValidationRule["max"].IsNull() return vArr, nil } From 22aaa92f61a1a141832dda4f3f05aa4c420a1bf4 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Wed, 31 May 2023 11:10:43 +0200 Subject: [PATCH 06/14] more checks --- provider/decode_test.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/provider/decode_test.go b/provider/decode_test.go index c86fc109..31b13b56 100644 --- a/provider/decode_test.go +++ b/provider/decode_test.go @@ -24,11 +24,12 @@ func TestDecode(t *testing.T) { "display_name": displayName, "legacy_variable": legacyVariable, "legacy_variable_name": legacyVariableName, - "min": nil, "validation": []map[string]interface{}{ { - "min": nil, - "max": 5, + "min": nil, + "min_ok": false, + "max": 5, + "max_ok": true, }, }, } @@ -40,5 +41,7 @@ func TestDecode(t *testing.T) { assert.Equal(t, legacyVariable, param.LegacyVariable) assert.Equal(t, legacyVariableName, param.LegacyVariableName) assert.Equal(t, 5, param.Validation[0].Max) + assert.True(t, param.Validation[0].MaxOk) assert.Equal(t, 0, param.Validation[0].Min) + assert.False(t, param.Validation[0].MinOk) } From 52aff1874794749571824a6fd6b1853680ec3d10 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Wed, 31 May 2023 11:20:44 +0200 Subject: [PATCH 07/14] fix --- provider/parameter_test.go | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/provider/parameter_test.go b/provider/parameter_test.go index 36cd9910..d088d2b8 100644 --- a/provider/parameter_test.go +++ b/provider/parameter_test.go @@ -493,17 +493,20 @@ func TestValueValidatesType(t *testing.T) { RegexError string Min, Max int - Monotonic string - Error *regexp.Regexp + MinOk, MaxOk bool + Monotonic string + Error *regexp.Regexp }{{ Name: "StringWithMin", Type: "string", Min: 1, + MinOk: true, Error: regexp.MustCompile("cannot be specified"), }, { Name: "StringWithMax", Type: "string", Max: 1, + MaxOk: true, Error: regexp.MustCompile("cannot be specified"), }, { Name: "NonStringWithRegex", @@ -524,12 +527,14 @@ func TestValueValidatesType(t *testing.T) { Type: "number", Value: "0", Min: 1, + MinOk: true, Error: regexp.MustCompile("is less than the minimum 1"), }, { Name: "NumberAboveMax", Type: "number", Value: "2", Max: 1, + MaxOk: true, Error: regexp.MustCompile("is more than the maximum 1"), }, { Name: "InvalidBool", @@ -548,7 +553,9 @@ func TestValueValidatesType(t *testing.T) { Type: "number", Value: "1", Min: 0, + MinOk: true, Max: 2, + MaxOk: true, Monotonic: "foobar", Error: regexp.MustCompile(`number monotonicity can be either "increasing" or "decreasing"`), }, { @@ -556,14 +563,18 @@ func TestValueValidatesType(t *testing.T) { Type: "number", Value: "1", Min: 0, + MinOk: true, Max: 2, + MaxOk: true, Monotonic: "increasing", }, { Name: "DecreasingMonotonicity", Type: "number", Value: "1", Min: 0, + MinOk: true, Max: 2, + MaxOk: true, Monotonic: "decreasing", }, { Name: "ValidListOfStrings", @@ -584,7 +595,9 @@ func TestValueValidatesType(t *testing.T) { t.Parallel() v := &provider.Validation{ Min: tc.Min, + MinOk: tc.MinOk, Max: tc.Max, + MaxOk: tc.MaxOk, Monotonic: tc.Monotonic, Regex: tc.Regex, Error: tc.RegexError, From 6df8eab23926671b1846f108199b2b1bdd2778a1 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Wed, 31 May 2023 11:49:15 +0200 Subject: [PATCH 08/14] fix --- provider/decode_test.go | 12 ++-- provider/parameter.go | 24 ++++---- provider/parameter_test.go | 111 ++++++++++++++++++++++--------------- 3 files changed, 83 insertions(+), 64 deletions(-) diff --git a/provider/decode_test.go b/provider/decode_test.go index 31b13b56..de56922c 100644 --- a/provider/decode_test.go +++ b/provider/decode_test.go @@ -26,10 +26,10 @@ func TestDecode(t *testing.T) { "legacy_variable_name": legacyVariableName, "validation": []map[string]interface{}{ { - "min": nil, - "min_ok": false, - "max": 5, - "max_ok": true, + "min": nil, + "min_disabled": false, + "max": 5, + "max_disabled": true, }, }, } @@ -41,7 +41,7 @@ func TestDecode(t *testing.T) { assert.Equal(t, legacyVariable, param.LegacyVariable) assert.Equal(t, legacyVariableName, param.LegacyVariableName) assert.Equal(t, 5, param.Validation[0].Max) - assert.True(t, param.Validation[0].MaxOk) + assert.True(t, param.Validation[0].MaxDisabled) assert.Equal(t, 0, param.Validation[0].Min) - assert.False(t, param.Validation[0].MinOk) + assert.False(t, param.Validation[0].MinDisabled) } diff --git a/provider/parameter.go b/provider/parameter.go index 632af4c6..f6c0ea14 100644 --- a/provider/parameter.go +++ b/provider/parameter.go @@ -28,10 +28,10 @@ type Option struct { } type Validation struct { - Min int - MinOk bool `mapstructure:"min_ok"` - Max int - MaxOk bool `mapstructure:"max_ok"` + Min int + MinDisabled bool `mapstructure:"min_disabled"` + Max int + MaxDisabled bool `mapstructure:"max_disabled"` Monotonic string @@ -291,7 +291,7 @@ func parameterDataSource() *schema.Resource { Optional: true, Description: "The minimum of a number parameter.", }, - "min_ok": { + "min_disabled": { Type: schema.TypeBool, Computed: true, Description: "Helper field to check if min is present", @@ -301,7 +301,7 @@ func parameterDataSource() *schema.Resource { Optional: true, Description: "The maximum of a number parameter.", }, - "max_ok": { + "max_disabled": { Type: schema.TypeBool, Computed: true, Description: "Helper field to check if max is present", @@ -384,8 +384,8 @@ func fixValidationResourceData(rawConfig cty.Value, validation interface{}) (int validationRule["max"] = nil } - validationRule["min_ok"] = !rawValidationRule["min"].IsNull() - validationRule["max_ok"] = !rawValidationRule["max"].IsNull() + validationRule["min_disabled"] = rawValidationRule["min"].IsNull() + validationRule["max_disabled"] = rawValidationRule["max"].IsNull() return vArr, nil } @@ -417,10 +417,10 @@ func valueIsType(typ, value string) diag.Diagnostics { func (v *Validation) Valid(typ, value string) error { if typ != "number" { - if v.MinOk { + if !v.MinDisabled && v.Min != 0 { return fmt.Errorf("a min cannot be specified for a %s type", typ) } - if v.MaxOk { + if !v.MaxDisabled && v.Max != 0 { return fmt.Errorf("a max cannot be specified for a %s type", typ) } } @@ -453,10 +453,10 @@ func (v *Validation) Valid(typ, value string) error { if err != nil { return fmt.Errorf("value %q is not a number", value) } - if v.MinOk && num < v.Min { + if !v.MinDisabled && num < v.Min { return fmt.Errorf("value %d is less than the minimum %d", num, v.Min) } - if v.MaxOk && num > v.Max { + if !v.MaxDisabled && num > v.Max { return fmt.Errorf("value %d is more than the maximum %d", num, v.Max) } if v.Monotonic != "" && v.Monotonic != ValidationMonotonicIncreasing && v.Monotonic != ValidationMonotonicDecreasing { diff --git a/provider/parameter_test.go b/provider/parameter_test.go index d088d2b8..25f6367a 100644 --- a/provider/parameter_test.go +++ b/provider/parameter_test.go @@ -32,13 +32,38 @@ func TestParameter(t *testing.T) { `, Check: func(state *terraform.ResourceState) { for key, expected := range map[string]string{ - "name": "Region", - "type": "number", - "validation.#": "1", - "default": "2", - "validation.0.max": "9", - "validation.0.min_ok": "false", - "validation.0.max_ok": "true", + "name": "Region", + "type": "number", + "validation.#": "1", + "default": "2", + "validation.0.max": "9", + "validation.0.min_disabled": "true", + "validation.0.max_disabled": "false", + } { + require.Equal(t, expected, state.Primary.Attributes[key]) + } + }, + }, { + Name: "NumberValidation_MinZero", + Config: ` + data "coder_parameter" "region" { + name = "Region" + type = "number" + default = 2 + validation { + min = 0 + } + } + `, + Check: func(state *terraform.ResourceState) { + for key, expected := range map[string]string{ + "name": "Region", + "type": "number", + "validation.#": "1", + "default": "2", + "validation.0.min": "0", + "validation.0.min_disabled": "false", + "validation.0.max_disabled": "true", } { require.Equal(t, expected, state.Primary.Attributes[key]) } @@ -493,21 +518,21 @@ func TestValueValidatesType(t *testing.T) { RegexError string Min, Max int - MinOk, MaxOk bool - Monotonic string - Error *regexp.Regexp + MinDisabled, MaxDisabled bool + Monotonic string + Error *regexp.Regexp }{{ - Name: "StringWithMin", - Type: "string", - Min: 1, - MinOk: true, - Error: regexp.MustCompile("cannot be specified"), - }, { - Name: "StringWithMax", - Type: "string", - Max: 1, - MaxOk: true, - Error: regexp.MustCompile("cannot be specified"), + Name: "StringWithMin", + Type: "string", + Min: 1, + MaxDisabled: true, + Error: regexp.MustCompile("cannot be specified"), + }, { + Name: "StringWithMax", + Type: "string", + Max: 1, + MinDisabled: true, + Error: regexp.MustCompile("cannot be specified"), }, { Name: "NonStringWithRegex", Type: "number", @@ -523,19 +548,19 @@ func TestValueValidatesType(t *testing.T) { Value: "hi", Error: regexp.MustCompile("is not a number"), }, { - Name: "NumberBelowMin", - Type: "number", - Value: "0", - Min: 1, - MinOk: true, - Error: regexp.MustCompile("is less than the minimum 1"), + Name: "NumberBelowMin", + Type: "number", + Value: "0", + Min: 1, + MaxDisabled: true, + Error: regexp.MustCompile("is less than the minimum 1"), }, { - Name: "NumberAboveMax", - Type: "number", - Value: "2", - Max: 1, - MaxOk: true, - Error: regexp.MustCompile("is more than the maximum 1"), + Name: "NumberAboveMax", + Type: "number", + Value: "2", + Max: 1, + MinDisabled: true, + Error: regexp.MustCompile("is more than the maximum 1"), }, { Name: "InvalidBool", Type: "bool", @@ -553,9 +578,7 @@ func TestValueValidatesType(t *testing.T) { Type: "number", Value: "1", Min: 0, - MinOk: true, Max: 2, - MaxOk: true, Monotonic: "foobar", Error: regexp.MustCompile(`number monotonicity can be either "increasing" or "decreasing"`), }, { @@ -563,18 +586,14 @@ func TestValueValidatesType(t *testing.T) { Type: "number", Value: "1", Min: 0, - MinOk: true, Max: 2, - MaxOk: true, Monotonic: "increasing", }, { Name: "DecreasingMonotonicity", Type: "number", Value: "1", Min: 0, - MinOk: true, Max: 2, - MaxOk: true, Monotonic: "decreasing", }, { Name: "ValidListOfStrings", @@ -594,13 +613,13 @@ func TestValueValidatesType(t *testing.T) { t.Run(tc.Name, func(t *testing.T) { t.Parallel() v := &provider.Validation{ - Min: tc.Min, - MinOk: tc.MinOk, - Max: tc.Max, - MaxOk: tc.MaxOk, - Monotonic: tc.Monotonic, - Regex: tc.Regex, - Error: tc.RegexError, + Min: tc.Min, + MinDisabled: tc.MinDisabled, + Max: tc.Max, + MaxDisabled: tc.MaxDisabled, + Monotonic: tc.Monotonic, + Regex: tc.Regex, + Error: tc.RegexError, } err := v.Valid(tc.Type, tc.Value) if tc.Error != nil { From 24a1b45e698738c0b8836360eeb0733cd7411ed5 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Wed, 31 May 2023 11:51:58 +0200 Subject: [PATCH 09/14] fix lint --- docs/data-sources/parameter.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/data-sources/parameter.md b/docs/data-sources/parameter.md index a8c89e22..c54e54ed 100644 --- a/docs/data-sources/parameter.md +++ b/docs/data-sources/parameter.md @@ -65,7 +65,7 @@ Optional: Read-Only: -- `max_ok` (Boolean) Helper field to check if max is present -- `min_ok` (Boolean) Helper field to check if min is present +- `max_disabled` (Boolean) Helper field to check if max is present +- `min_disabled` (Boolean) Helper field to check if min is present From 5c82d4ab83ed0bdf27fba945e38d0c9eeea9a567 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Wed, 31 May 2023 11:55:30 +0200 Subject: [PATCH 10/14] more tests --- provider/parameter_test.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/provider/parameter_test.go b/provider/parameter_test.go index 25f6367a..04fdfa48 100644 --- a/provider/parameter_test.go +++ b/provider/parameter_test.go @@ -159,6 +159,20 @@ func TestParameter(t *testing.T) { } } `, + Check: func(state *terraform.ResourceState) { + for key, expected := range map[string]string{ + "name": "Region", + "type": "number", + "validation.#": "1", + "default": "2", + "validation.0.min": "1", + "validation.0.max": "5", + "validation.0.min_disabled": "false", + "validation.0.max_disabled": "false", + } { + require.Equal(t, expected, state.Primary.Attributes[key]) + } + }, }, { Name: "NumberValidation_Min", Config: ` From 0cd0625ab50226f69ddf6af59bc0d019dd9de662 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Wed, 31 May 2023 13:14:08 +0200 Subject: [PATCH 11/14] Address PR comments --- provider/parameter.go | 12 ++------ provider/parameter_test.go | 58 +++++++++++++++++++++++--------------- 2 files changed, 37 insertions(+), 33 deletions(-) diff --git a/provider/parameter.go b/provider/parameter.go index f6c0ea14..f736579b 100644 --- a/provider/parameter.go +++ b/provider/parameter.go @@ -376,14 +376,6 @@ func fixValidationResourceData(rawConfig cty.Value, validation interface{}) (int return nil, xerrors.New("validation rule should be a map") } - // Fix the resource data - if rawValidationRule["min"].IsNull() { - validationRule["min"] = nil - } - if rawValidationRule["max"].IsNull() { - validationRule["max"] = nil - } - validationRule["min_disabled"] = rawValidationRule["min"].IsNull() validationRule["max_disabled"] = rawValidationRule["max"].IsNull() return vArr, nil @@ -417,10 +409,10 @@ func valueIsType(typ, value string) diag.Diagnostics { func (v *Validation) Valid(typ, value string) error { if typ != "number" { - if !v.MinDisabled && v.Min != 0 { + if !v.MinDisabled { return fmt.Errorf("a min cannot be specified for a %s type", typ) } - if !v.MaxDisabled && v.Max != 0 { + if !v.MaxDisabled { return fmt.Errorf("a max cannot be specified for a %s type", typ) } } diff --git a/provider/parameter_test.go b/provider/parameter_test.go index 04fdfa48..3681e38a 100644 --- a/provider/parameter_test.go +++ b/provider/parameter_test.go @@ -553,9 +553,11 @@ func TestValueValidatesType(t *testing.T) { Regex: "banana", Error: regexp.MustCompile("a regex cannot be specified"), }, { - Name: "Bool", - Type: "bool", - Value: "true", + Name: "Bool", + Type: "bool", + Value: "true", + MinDisabled: true, + MaxDisabled: true, }, { Name: "InvalidNumber", Type: "number", @@ -576,17 +578,21 @@ func TestValueValidatesType(t *testing.T) { MinDisabled: true, Error: regexp.MustCompile("is more than the maximum 1"), }, { - Name: "InvalidBool", - Type: "bool", - Value: "cat", - Error: regexp.MustCompile("boolean value can be either"), + Name: "InvalidBool", + Type: "bool", + Value: "cat", + MinDisabled: true, + MaxDisabled: true, + Error: regexp.MustCompile("boolean value can be either"), }, { - Name: "BadStringWithRegex", - Type: "string", - Regex: "banana", - RegexError: "bad fruit", - Value: "apple", - Error: regexp.MustCompile(`bad fruit`), + Name: "BadStringWithRegex", + Type: "string", + Regex: "banana", + RegexError: "bad fruit", + Value: "apple", + MinDisabled: true, + MaxDisabled: true, + Error: regexp.MustCompile(`bad fruit`), }, { Name: "InvalidMonotonicity", Type: "number", @@ -610,18 +616,24 @@ func TestValueValidatesType(t *testing.T) { Max: 2, Monotonic: "decreasing", }, { - Name: "ValidListOfStrings", - Type: "list(string)", - Value: `["first","second","third"]`, + Name: "ValidListOfStrings", + Type: "list(string)", + Value: `["first","second","third"]`, + MinDisabled: true, + MaxDisabled: true, }, { - Name: "InvalidListOfStrings", - Type: "list(string)", - Value: `["first","second","third"`, - Error: regexp.MustCompile("is not valid list of strings"), + Name: "InvalidListOfStrings", + Type: "list(string)", + Value: `["first","second","third"`, + MinDisabled: true, + MaxDisabled: true, + Error: regexp.MustCompile("is not valid list of strings"), }, { - Name: "EmptyListOfStrings", - Type: "list(string)", - Value: `[]`, + Name: "EmptyListOfStrings", + Type: "list(string)", + Value: `[]`, + MinDisabled: true, + MaxDisabled: true, }} { tc := tc t.Run(tc.Name, func(t *testing.T) { From f91fead5ad308cba18448e969073b373195fa266 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Wed, 31 May 2023 13:22:56 +0200 Subject: [PATCH 12/14] More tests --- provider/parameter_test.go | 201 +++++++++++++++++++++++-------------- 1 file changed, 127 insertions(+), 74 deletions(-) diff --git a/provider/parameter_test.go b/provider/parameter_test.go index 3681e38a..b16b4930 100644 --- a/provider/parameter_test.go +++ b/provider/parameter_test.go @@ -19,56 +19,6 @@ func TestParameter(t *testing.T) { ExpectError *regexp.Regexp Check func(state *terraform.ResourceState) }{{ - Name: "NumberValidation_Max", - Config: ` - data "coder_parameter" "region" { - name = "Region" - type = "number" - default = 2 - validation { - max = 9 - } - } - `, - Check: func(state *terraform.ResourceState) { - for key, expected := range map[string]string{ - "name": "Region", - "type": "number", - "validation.#": "1", - "default": "2", - "validation.0.max": "9", - "validation.0.min_disabled": "true", - "validation.0.max_disabled": "false", - } { - require.Equal(t, expected, state.Primary.Attributes[key]) - } - }, - }, { - Name: "NumberValidation_MinZero", - Config: ` - data "coder_parameter" "region" { - name = "Region" - type = "number" - default = 2 - validation { - min = 0 - } - } - `, - Check: func(state *terraform.ResourceState) { - for key, expected := range map[string]string{ - "name": "Region", - "type": "number", - "validation.#": "1", - "default": "2", - "validation.0.min": "0", - "validation.0.min_disabled": "false", - "validation.0.max_disabled": "true", - } { - require.Equal(t, expected, state.Primary.Attributes[key]) - } - }, - }, { Name: "FieldsExist", Config: ` data "coder_parameter" "region" { @@ -173,30 +123,6 @@ func TestParameter(t *testing.T) { require.Equal(t, expected, state.Primary.Attributes[key]) } }, - }, { - Name: "NumberValidation_Min", - Config: ` - data "coder_parameter" "region" { - name = "Region" - type = "number" - default = 2 - validation { - min = 1 - } - } - `, - }, { - Name: "NumberValidation_Max", - Config: ` - data "coder_parameter" "region" { - name = "Region" - type = "number" - default = 2 - validation { - max = 9 - } - } - `, }, { Name: "DefaultNotNumber", Config: ` @@ -494,6 +420,133 @@ data "coder_parameter" "region" { require.Equal(t, expected, attributeValue) } }, + }, { + Name: "NumberValidation_Max", + Config: ` + data "coder_parameter" "region" { + name = "Region" + type = "number" + default = 2 + validation { + max = 9 + } + } + `, + Check: func(state *terraform.ResourceState) { + for key, expected := range map[string]string{ + "name": "Region", + "type": "number", + "validation.#": "1", + "default": "2", + "validation.0.max": "9", + "validation.0.min_disabled": "true", + "validation.0.max_disabled": "false", + } { + require.Equal(t, expected, state.Primary.Attributes[key]) + } + }, + }, { + Name: "NumberValidation_MaxZero", + Config: ` + data "coder_parameter" "region" { + name = "Region" + type = "number" + default = -1 + validation { + max = 0 + } + } + `, + Check: func(state *terraform.ResourceState) { + for key, expected := range map[string]string{ + "name": "Region", + "type": "number", + "validation.#": "1", + "default": "-1", + "validation.0.max": "0", + "validation.0.min_disabled": "true", + "validation.0.max_disabled": "false", + } { + require.Equal(t, expected, state.Primary.Attributes[key]) + } + }, + }, { + Name: "NumberValidation_Min", + Config: ` + data "coder_parameter" "region" { + name = "Region" + type = "number" + default = 2 + validation { + min = 1 + } + } + `, + Check: func(state *terraform.ResourceState) { + for key, expected := range map[string]string{ + "name": "Region", + "type": "number", + "validation.#": "1", + "default": "2", + "validation.0.min": "1", + "validation.0.min_disabled": "false", + "validation.0.max_disabled": "true", + } { + require.Equal(t, expected, state.Primary.Attributes[key]) + } + }, + }, { + Name: "NumberValidation_MinZero", + Config: ` + data "coder_parameter" "region" { + name = "Region" + type = "number" + default = 2 + validation { + min = 0 + } + } + `, + Check: func(state *terraform.ResourceState) { + for key, expected := range map[string]string{ + "name": "Region", + "type": "number", + "validation.#": "1", + "default": "2", + "validation.0.min": "0", + "validation.0.min_disabled": "false", + "validation.0.max_disabled": "true", + } { + require.Equal(t, expected, state.Primary.Attributes[key]) + } + }, + }, { + Name: "NumberValidation_MinMaxZero", + Config: ` + data "coder_parameter" "region" { + name = "Region" + type = "number" + default = 0 + validation { + max = 0 + min = 0 + } + } + `, + Check: func(state *terraform.ResourceState) { + for key, expected := range map[string]string{ + "name": "Region", + "type": "number", + "validation.#": "1", + "default": "0", + "validation.0.min": "0", + "validation.0.max": "0", + "validation.0.min_disabled": "false", + "validation.0.max_disabled": "false", + } { + require.Equal(t, expected, state.Primary.Attributes[key]) + } + }, }} { tc := tc t.Run(tc.Name, func(t *testing.T) { From e25b1c2ad4f67c18fd4bfe3f227cd39a4bec4ed7 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Wed, 31 May 2023 13:34:41 +0200 Subject: [PATCH 13/14] More tests --- provider/parameter_test.go | 40 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/provider/parameter_test.go b/provider/parameter_test.go index b16b4930..aad84b09 100644 --- a/provider/parameter_test.go +++ b/provider/parameter_test.go @@ -547,6 +547,46 @@ data "coder_parameter" "region" { require.Equal(t, expected, state.Primary.Attributes[key]) } }, + }, { + Name: "NumberValidation_LesserThanMin", + Config: ` + data "coder_parameter" "region" { + name = "Region" + type = "number" + default = 5 + validation { + min = 7 + } + } + `, + ExpectError: regexp.MustCompile("is less than the minimum"), + }, { + Name: "NumberValidation_GreaterThanMin", + Config: ` + data "coder_parameter" "region" { + name = "Region" + type = "number" + default = 5 + validation { + max = 3 + } + } + `, + ExpectError: regexp.MustCompile("is more than the maximum"), + }, { + Name: "NumberValidation_NotInRange", + Config: ` + data "coder_parameter" "region" { + name = "Region" + type = "number" + default = 8 + validation { + min = 3 + max = 5 + } + } + `, + ExpectError: regexp.MustCompile("is more than the maximum"), }} { tc := tc t.Run(tc.Name, func(t *testing.T) { From 2a3acf68e1eba96c652bcabe5df2accd865dd34a Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Wed, 31 May 2023 13:38:18 +0200 Subject: [PATCH 14/14] one more --- provider/parameter_test.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/provider/parameter_test.go b/provider/parameter_test.go index aad84b09..d749592b 100644 --- a/provider/parameter_test.go +++ b/provider/parameter_test.go @@ -587,6 +587,19 @@ data "coder_parameter" "region" { } `, ExpectError: regexp.MustCompile("is more than the maximum"), + }, { + Name: "NumberValidation_BoolWithMin", + Config: ` + data "coder_parameter" "region" { + name = "Region" + type = "bool" + default = true + validation { + min = 7 + } + } + `, + ExpectError: regexp.MustCompile("a min cannot be specified for a bool type"), }} { tc := tc t.Run(tc.Name, func(t *testing.T) {