Skip to content

Commit 0fd96ee

Browse files
EmyrkEmyrk
committed
remove validation modes
1 parent f7b9338 commit 0fd96ee

File tree

3 files changed

+121
-207
lines changed

3 files changed

+121
-207
lines changed

‎provider/parameter.go‎

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

24-
type ValidationMode string
25-
26-
const (
27-
ValidationModeEnvVar = "CODER_VALIDATION_MODE"
28-
// ValidationModeDefault is used for creating a workspace. It validates the final
29-
// value used for a parameter. Some allowances for invalid options are tolerated,
30-
// as unused options do not affect the final parameter value. The default value
31-
// is also ignored, assuming a value is provided.
32-
ValidationModeDefault ValidationMode = ""
33-
// ValidationModeTemplateImport tolerates empty values as long as no 'validation'
34-
// block is specified. This allows for importing a template without having to
35-
// specify a value for every parameter. It does not tolerate an invalid default
36-
// or invalid option values.
37-
ValidationModeTemplateImport ValidationMode = "template-import"
38-
)
39-
4024
var (
4125
defaultValuePath = cty.Path{cty.GetAttrStep{Name: "default"}}
4226
)
@@ -160,8 +144,7 @@ func parameterDataSource() *schema.Resource {
160144
input = &envValue
161145
}
162146

163-
mode := os.Getenv(ValidationModeEnvVar)
164-
value, diags := parameter.ValidateInput(input, ValidationMode(mode))
147+
value, diags := parameter.ValidateInput(input)
165148
if diags.HasError() {
166149
return diags
167150
}
@@ -410,17 +393,7 @@ func valueIsType(typ OptionType, value string) error {
410393
return nil
411394
}
412395

413-
func (v *Parameter) ValidateInput(input *string, mode ValidationMode) (string, diag.Diagnostics) {
414-
if mode != ValidationModeDefault && mode != ValidationModeTemplateImport {
415-
return "", diag.Diagnostics{
416-
{
417-
Severity: diag.Error,
418-
Summary: "Invalid validation mode",
419-
Detail: fmt.Sprintf("validation mode %q is not supported, use %q, or %q", mode, ValidationModeDefault, ValidationModeTemplateImport),
420-
},
421-
}
422-
}
423-
396+
func (v *Parameter) ValidateInput(input *string) (string, diag.Diagnostics) {
424397
var err error
425398
var optionType OptionType
426399

@@ -443,31 +416,11 @@ func (v *Parameter) ValidateInput(input *string, mode ValidationMode) (string, d
443416
}
444417
}
445418

446-
optionValues, diags := v.ValidOptions(optionType, mode)
419+
optionValues, diags := v.ValidOptions(optionType)
447420
if diags.HasError() {
448421
return "", diags
449422
}
450423

451-
if mode == ValidationModeTemplateImport && v.Default != nil {
452-
// Template import should validate the default value.
453-
err := valueIsType(v.Type, *v.Default)
454-
if err != nil {
455-
return "", diag.Diagnostics{
456-
{
457-
Severity: diag.Error,
458-
Summary: fmt.Sprintf("Default value is not of type %q", v.Type),
459-
Detail: err.Error(),
460-
AttributePath: defaultValuePath,
461-
},
462-
}
463-
}
464-
465-
d := v.validValue(*v.Default, optionType, optionValues, defaultValuePath)
466-
if d.HasError() {
467-
return "", d
468-
}
469-
}
470-
471424
// TODO: This is a bit of a hack. The current behavior states if validation
472425
// is given, then apply validation to unset values.
473426
// value == nil should not be accepted in the first place.
@@ -502,7 +455,7 @@ func (v *Parameter) ValidateInput(input *string, mode ValidationMode) (string, d
502455
return forcedValue, nil
503456
}
504457

505-
func (v *Parameter) ValidOptions(optionType OptionType, mode ValidationMode) (map[string]struct{}, diag.Diagnostics) {
458+
func (v *Parameter) ValidOptions(optionType OptionType) (map[string]struct{}, diag.Diagnostics) {
506459
optionNames := map[string]struct{}{}
507460
optionValues := map[string]struct{}{}
508461

@@ -538,18 +491,10 @@ func (v *Parameter) ValidOptions(optionType OptionType, mode ValidationMode) (ma
538491
optionValues[option.Value] = struct{}{}
539492
optionNames[option.Name] = struct{}{}
540493

541-
if mode == ValidationModeTemplateImport {
542-
opDiags := v.validValue(option.Value, optionType, nil, cty.Path{})
543-
if opDiags.HasError() {
544-
for i := range opDiags {
545-
opDiags[i].Summary = fmt.Sprintf("Option %q: %s", option.Name, opDiags[i].Summary)
546-
}
547-
diags = append(diags, opDiags...)
548-
}
549-
}
494+
// Option values are assumed to be valid. Do not call validValue on them.
550495
}
551496

552-
if diags.HasError() {
497+
if diags != nil && diags.HasError() {
553498
return nil, diags
554499
}
555500
return optionValues, nil

‎provider/parameter_test.go‎

Lines changed: 35 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -839,7 +839,7 @@ func TestParameterValidation(t *testing.T) {
839839
t.Run(tc.Name, func(t *testing.T) {
840840
t.Parallel()
841841
value := &tc.Value
842-
_, diags := tc.Parameter.ValidateInput(value, provider.ValidationModeDefault)
842+
_, diags := tc.Parameter.ValidateInput(value)
843843
if tc.ExpectError != nil {
844844
require.True(t, diags.HasError())
845845
errMsg := fmt.Sprintf("%+v", diags[0]) // close enough
@@ -881,7 +881,6 @@ func TestParameterValidationEnforcement(t *testing.T) {
881881
OutputValue string
882882
Optional bool
883883
CreateError *regexp.Regexp
884-
ImportError *regexp.Regexp
885884
}
886885

887886
rows := make([]row, 0)
@@ -913,18 +912,6 @@ func TestParameterValidationEnforcement(t *testing.T) {
913912
}
914913
}
915914

916-
var imerr *regexp.Regexp
917-
if columns[10] != "" {
918-
if columns[10] == "=" {
919-
imerr = rerr
920-
} else {
921-
imerr, err = regexp.Compile(columns[10])
922-
if err != nil {
923-
t.Fatalf("failed to parse error column %q: %v", columns[10], err)
924-
}
925-
}
926-
}
927-
928915
var options []string
929916
if columns[4] != "" {
930917
options = strings.Split(columns[4], ",")
@@ -972,7 +959,6 @@ func TestParameterValidationEnforcement(t *testing.T) {
972959
OutputValue: columns[7],
973960
Optional: optional,
974961
CreateError: rerr,
975-
ImportError: imerr,
976962
})
977963
}
978964

@@ -991,10 +977,8 @@ func TestParameterValidationEnforcement(t *testing.T) {
991977
t.Setenv(provider.ParameterEnvironmentVariable("parameter"), row.InputValue)
992978
}
993979

994-
if row.CreateError != nil && row.ImportError != nil {
995-
if row.OutputValue != "" {
996-
t.Errorf("output value %q should not be set if both errors are set", row.OutputValue)
997-
}
980+
if row.CreateError != nil && row.OutputValue != "" {
981+
t.Errorf("output value %q should not be set if both errors are set", row.OutputValue)
998982
}
999983

1000984
var cfg strings.Builder
@@ -1036,57 +1020,42 @@ func TestParameterValidationEnforcement(t *testing.T) {
10361020
}
10371021

10381022
cfg.WriteString("}\n")
1023+
resource.Test(t, resource.TestCase{
1024+
ProviderFactories: coderFactory(),
1025+
IsUnitTest: true,
1026+
Steps: []resource.TestStep{{
1027+
Config: cfg.String(),
1028+
ExpectError: row.CreateError,
1029+
Check: func(state *terraform.State) error {
1030+
require.Len(t, state.Modules, 1)
1031+
require.Len(t, state.Modules[0].Resources, 1)
1032+
param := state.Modules[0].Resources["data.coder_parameter.parameter"]
1033+
require.NotNil(t, param)
10391034

1040-
for _, mode := range []provider.ValidationMode{provider.ValidationModeDefault, provider.ValidationModeTemplateImport} {
1041-
name := string(mode)
1042-
if mode == provider.ValidationModeDefault {
1043-
name = "create"
1044-
}
1045-
t.Run(name, func(t *testing.T) {
1046-
t.Setenv("CODER_VALIDATION_MODE", string(mode))
1047-
rerr := row.CreateError
1048-
if mode == provider.ValidationModeTemplateImport {
1049-
rerr = row.ImportError
1050-
}
1051-
1052-
resource.Test(t, resource.TestCase{
1053-
ProviderFactories: coderFactory(),
1054-
IsUnitTest: true,
1055-
Steps: []resource.TestStep{{
1056-
Config: cfg.String(),
1057-
ExpectError: rerr,
1058-
Check: func(state *terraform.State) error {
1059-
require.Len(t, state.Modules, 1)
1060-
require.Len(t, state.Modules[0].Resources, 1)
1061-
param := state.Modules[0].Resources["data.coder_parameter.parameter"]
1062-
require.NotNil(t, param)
1035+
if row.Default == "" {
1036+
_, ok := param.Primary.Attributes["default"]
1037+
require.False(t, ok, "default should not be set")
1038+
} else {
1039+
require.Equal(t, strings.Trim(row.Default, `"`), param.Primary.Attributes["default"])
1040+
}
10631041

1064-
if row.Default == "" {
1065-
_, ok := param.Primary.Attributes["default"]
1066-
require.False(t, ok, "default should not be set")
1067-
} else {
1068-
require.Equal(t, strings.Trim(row.Default, `"`), param.Primary.Attributes["default"])
1069-
}
1042+
if row.OutputValue == "" {
1043+
_, ok := param.Primary.Attributes["value"]
1044+
require.False(t, ok, "output value should not be set")
1045+
} else {
1046+
require.Equal(t, strings.Trim(row.OutputValue, `"`), param.Primary.Attributes["value"])
1047+
}
10701048

1071-
if row.OutputValue == "" {
1072-
_, ok := param.Primary.Attributes["value"]
1073-
require.False(t, ok, "output value should not be set")
1074-
} else {
1075-
require.Equal(t, strings.Trim(row.OutputValue, `"`), param.Primary.Attributes["value"])
1076-
}
1049+
for key, expected := range map[string]string{
1050+
"optional": strconv.FormatBool(row.Optional),
1051+
} {
1052+
require.Equal(t, expected, param.Primary.Attributes[key], "optional")
1053+
}
10771054

1078-
for key, expected := range map[string]string{
1079-
"optional": strconv.FormatBool(row.Optional),
1080-
} {
1081-
require.Equal(t, expected, param.Primary.Attributes[key], "optional")
1082-
}
1083-
1084-
return nil
1085-
},
1086-
}},
1087-
})
1088-
})
1089-
}
1055+
return nil
1056+
},
1057+
}},
1058+
})
10901059
})
10911060
}
10921061
}

0 commit comments

Comments
 (0)