Skip to content

Commit 0ad4eb6

Browse files
EmyrkEmyrk
committed
chore: distinguish unset vs set default and values
1 parent fe4b4d7 commit 0ad4eb6

File tree

2 files changed

+97
-56
lines changed

2 files changed

+97
-56
lines changed

‎provider/parameter.go‎

Lines changed: 88 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,15 @@ import (
2121
"golang.org/x/xerrors"
2222
)
2323

24+
type ValidationMode string
25+
26+
const (
27+
// ValidationModeDefault is used for creating a workspace. It validates the
28+
// final value used for a parameter.
29+
ValidationModeDefault ValidationMode = ""
30+
ValidationModeTemplateImport ValidationMode = "template-import"
31+
)
32+
2433
var (
2534
defaultValuePath = cty.Path{cty.GetAttrStep{Name: "default"}}
2635
)
@@ -56,7 +65,7 @@ type Parameter struct {
5665
Type OptionType
5766
FormType ParameterFormType
5867
Mutable bool
59-
Default string
68+
Default *string
6069
Icon string
6170
Option []Option
6271
Validation []Validation
@@ -105,10 +114,16 @@ func parameterDataSource() *schema.Resource {
105114
Type: rd.Get("type"),
106115
FormType: rd.Get("form_type"),
107116
Mutable: rd.Get("mutable"),
108-
Default: rd.Get("default"),
109-
Icon: rd.Get("icon"),
110-
Option: rd.Get("option"),
111-
Validation: fixedValidation,
117+
Default: func() *string {
118+
if rd.GetRawConfig().AsValueMap()["default"].IsNull() {
119+
return nil
120+
}
121+
val, _ := rd.Get("default").(string)
122+
return &val
123+
}(),
124+
Icon: rd.Get("icon"),
125+
Option: rd.Get("option"),
126+
Validation: fixedValidation,
112127
Optional: func() bool {
113128
// This hack allows for checking if the "default" field is present in the .tf file.
114129
// If "default" is missing or is "null", then it means that this field is required,
@@ -124,25 +139,6 @@ func parameterDataSource() *schema.Resource {
124139
return diag.Errorf("decode parameter: %s", err)
125140
}
126141

127-
var value *string
128-
if !rd.GetRawConfig().AsValueMap()["default"].IsNull() {
129-
value = &parameter.Default
130-
}
131-
132-
envValue, ok := os.LookupEnv(ParameterEnvironmentVariable(parameter.Name))
133-
if ok {
134-
value = &envValue
135-
}
136-
137-
if value != nil {
138-
rd.Set("value", value)
139-
} else {
140-
// Maintaining backwards compatibility. The previous behavior was
141-
// to write back an empty string.
142-
// TODO: Should an empty string exist if no value is set?
143-
rd.Set("value", "")
144-
}
145-
146142
if !parameter.Mutable && parameter.Ephemeral {
147143
return diag.Errorf("parameter can't be immutable and ephemeral")
148144
}
@@ -151,10 +147,17 @@ func parameterDataSource() *schema.Resource {
151147
return diag.Errorf("ephemeral parameter requires the default property")
152148
}
153149

154-
diags := parameter.Valid(value)
150+
var input *string
151+
envValue, ok := os.LookupEnv(ParameterEnvironmentVariable(parameter.Name))
152+
if ok {
153+
input = &envValue
154+
}
155+
156+
value, diags := parameter.Valid(input, ValidationModeDefault)
155157
if diags.HasError() {
156158
return diags
157159
}
160+
rd.Set("value", value)
158161

159162
// Set the form_type as it could have changed in the validation.
160163
rd.Set("form_type", parameter.FormType)
@@ -397,15 +400,20 @@ func valueIsType(typ OptionType, value string) error {
397400
return nil
398401
}
399402

400-
func (v *Parameter) Valid(value *string) diag.Diagnostics {
403+
func (v *Parameter) Valid(input *string, mode ValidationMode) (string, diag.Diagnostics) {
401404
var err error
402405
var optionType OptionType
403406

407+
value := input
408+
if input == nil {
409+
value = v.Default
410+
}
411+
404412
// optionType might differ from parameter.Type. This is ok, and parameter.Type
405413
// should be used for the value type, and optionType for options.
406414
optionType, v.FormType, err = ValidateFormType(v.Type, len(v.Option), v.FormType)
407415
if err != nil {
408-
return diag.Diagnostics{
416+
return "", diag.Diagnostics{
409417
{
410418
Severity: diag.Error,
411419
Summary: "Invalid form_type for parameter",
@@ -417,28 +425,28 @@ func (v *Parameter) Valid(value *string) diag.Diagnostics {
417425

418426
optionValues, diags := v.ValidOptions(optionType)
419427
if diags.HasError() {
420-
return diags
428+
return "", diags
421429
}
422430

423-
// TODO: The default value should also be validated
424-
//if v.Default != "" {
425-
// err := valueIsType(v.Type, v.Default)
426-
// if err != nil {
427-
// return diag.Diagnostics{
428-
// {
429-
// Severity: diag.Error,
430-
// Summary: fmt.Sprintf("Default value is not of type %q", v.Type),
431-
// Detail: err.Error(),
432-
// AttributePath: defaultValuePath,
433-
// },
434-
// }
435-
// }
436-
//
437-
// d := v.validValue(v.Default, optionType, optionValues, defaultValuePath)
438-
// if d.HasError() {
439-
// return d
440-
// }
441-
//}
431+
if mode == ValidationModeTemplateImport && v.Default != nil {
432+
// Template import should validate the default value.
433+
err := valueIsType(v.Type, *v.Default)
434+
if err != nil {
435+
return "", diag.Diagnostics{
436+
{
437+
Severity: diag.Error,
438+
Summary: fmt.Sprintf("Default value is not of type %q", v.Type),
439+
Detail: err.Error(),
440+
AttributePath: defaultValuePath,
441+
},
442+
}
443+
}
444+
445+
d := v.validValue(*v.Default, optionType, optionValues, defaultValuePath)
446+
if d.HasError() {
447+
return "", d
448+
}
449+
}
442450

443451
// TODO: Move this into 'Parameter.validValue'. It exists as another check outside because
444452
// the current behavior is to always apply this validation, regardless if the param is set or not.
@@ -453,7 +461,7 @@ func (v *Parameter) Valid(value *string) diag.Diagnostics {
453461
validCheck := &v.Validation[0]
454462
err := validCheck.Valid(v.Type, *validVal)
455463
if err != nil {
456-
return diag.Diagnostics{
464+
return "", diag.Diagnostics{
457465
{
458466
Severity: diag.Error,
459467
Summary: fmt.Sprintf("Invalid parameter %s according to 'validation' block", strings.ToLower(v.Name)),
@@ -464,17 +472,26 @@ func (v *Parameter) Valid(value *string) diag.Diagnostics {
464472
}
465473
}
466474

475+
// TODO: This is a bit of a hack. The current behavior states if validation
476+
// is given, then apply validation to unset values.
477+
// This should be removed, and all values should be validated. Meaning
478+
// value == nil should not be accepted in the first place.
479+
if len(v.Validation) > 0 && value == nil {
480+
empty := ""
481+
value = &empty
482+
}
483+
467484
// Value is only validated if it is set. If it is unset, validation
468485
// is skipped.
469486
if value != nil {
470487
d := v.validValue(*value, optionType, optionValues, cty.Path{})
471488
if d.HasError() {
472-
return d
489+
return "", d
473490
}
474491

475492
err = valueIsType(v.Type, *value)
476493
if err != nil {
477-
return diag.Diagnostics{
494+
return "", diag.Diagnostics{
478495
{
479496
Severity: diag.Error,
480497
Summary: fmt.Sprintf("Parameter value is not of type %q", v.Type),
@@ -484,7 +501,12 @@ func (v *Parameter) Valid(value *string) diag.Diagnostics {
484501
}
485502
}
486503

487-
return nil
504+
if value == nil {
505+
// The previous behavior is to always write an empty string
506+
return "", nil
507+
}
508+
509+
return *value, nil
488510
}
489511

490512
func (v *Parameter) ValidOptions(optionType OptionType) (map[string]struct{}, diag.Diagnostics) {
@@ -598,6 +620,21 @@ func (v *Parameter) validValue(value string, optionType OptionType, optionValues
598620
}
599621
}
600622

623+
if len(v.Validation) == 1 {
624+
validCheck := &v.Validation[0]
625+
err := validCheck.Valid(v.Type, value)
626+
if err != nil {
627+
return diag.Diagnostics{
628+
{
629+
Severity: diag.Error,
630+
Summary: fmt.Sprintf("Invalid parameter %s according to 'validation' block", strings.ToLower(v.Name)),
631+
Detail: err.Error(),
632+
AttributePath: cty.Path{},
633+
},
634+
}
635+
}
636+
}
637+
601638
return nil
602639
}
603640

‎provider/parameter_test.go‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -786,7 +786,7 @@ func TestParameterValidation(t *testing.T) {
786786
Name: "NotListStringDefault",
787787
Parameter: provider.Parameter{
788788
Type: "list(string)",
789-
Default: "not-a-list",
789+
Default: ptr("not-a-list"),
790790
},
791791
ExpectError: regexp.MustCompile("not a valid list of strings"),
792792
},
@@ -802,7 +802,7 @@ func TestParameterValidation(t *testing.T) {
802802
Name: "DefaultListStringNotInOptions",
803803
Parameter: provider.Parameter{
804804
Type: "list(string)",
805-
Default: `["red", "yellow", "black"]`,
805+
Default: ptr(`["red", "yellow", "black"]`),
806806
Option: opts("red", "blue", "green"),
807807
FormType: provider.ParameterFormTypeMultiSelect,
808808
},
@@ -813,7 +813,7 @@ func TestParameterValidation(t *testing.T) {
813813
Name: "ListStringNotInOptions",
814814
Parameter: provider.Parameter{
815815
Type: "list(string)",
816-
Default: `["red"]`,
816+
Default: ptr(`["red"]`),
817817
Option: opts("red", "blue", "green"),
818818
FormType: provider.ParameterFormTypeMultiSelect,
819819
},
@@ -824,7 +824,7 @@ func TestParameterValidation(t *testing.T) {
824824
Name: "InvalidMiniumum",
825825
Parameter: provider.Parameter{
826826
Type: "number",
827-
Default: "5",
827+
Default: ptr("5"),
828828
Validation: []provider.Validation{{
829829
Min: 10,
830830
Error: "must be greater than 10",
@@ -837,7 +837,7 @@ func TestParameterValidation(t *testing.T) {
837837
t.Run(tc.Name, func(t *testing.T) {
838838
t.Parallel()
839839
value := &tc.Value
840-
diags := tc.Parameter.Valid(value)
840+
_, diags := tc.Parameter.Valid(value, provider.ValidationModeDefault)
841841
if tc.ExpectError != nil {
842842
require.True(t, diags.HasError())
843843
errMsg := fmt.Sprintf("%+v", diags[0]) // close enough
@@ -1255,3 +1255,7 @@ func TestParameterWithManyOptions(t *testing.T) {
12551255
}},
12561256
})
12571257
}
1258+
1259+
func ptr[T any](v T) *T {
1260+
return &v
1261+
}

0 commit comments

Comments
 (0)