From 875b3168350738f0c5b6215f8af8a4dcba63f299 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Mon, 14 Apr 2025 12:56:53 -0500 Subject: [PATCH 01/22] validate input vales --- provider/parameter.go | 126 +++++++++++++++++++++++++++--------------- 1 file changed, 82 insertions(+), 44 deletions(-) diff --git a/provider/parameter.go b/provider/parameter.go index 2f7dc662..05cd57da 100644 --- a/provider/parameter.go +++ b/provider/parameter.go @@ -21,6 +21,10 @@ import ( "golang.org/x/xerrors" ) +var ( + defaultValuePath = cty.Path{cty.GetAttrStep{Name: "default"}} +) + type Option struct { Name string Description string @@ -124,7 +128,7 @@ func parameterDataSource() *schema.Resource { } var value string if parameter.Default != "" { - err := valueIsType(parameter.Type, parameter.Default, cty.Path{cty.GetAttrStep{Name: "default"}}) + err := valueIsType(parameter.Type, parameter.Default, defaultValuePath) if err != nil { return err } @@ -144,34 +148,22 @@ func parameterDataSource() *schema.Resource { return diag.Errorf("ephemeral parameter requires the default property") } - // TODO: Should we move this into the Valid() function on - // Parameter? - if len(parameter.Validation) == 1 { - validation := ¶meter.Validation[0] - err = validation.Valid(parameter.Type, value) - if err != nil { - return diag.FromErr(err) - } + // Do ValidateFormType up front. If there is no error, update the + // 'parameter.FormType' value to the new value. This is to handle default cases, + // since the default logic is more advanced than the sdk provider schema + // supports. + _, newFT, err := ValidateFormType(parameter.Type, len(parameter.Option), parameter.FormType) + if err == nil { + // If there is an error, parameter.Valid will catch it. + parameter.FormType = newFT + + // Set the form_type back in case the value was changed. + // Eg via a default. If a user does not specify, a default value + // is used and saved. + rd.Set("form_type", parameter.FormType) } - // Validate options - _, parameter.FormType, err = ValidateFormType(parameter.Type, len(parameter.Option), parameter.FormType) - if err != nil { - return diag.Diagnostics{ - { - Severity: diag.Error, - Summary: "Invalid form_type for parameter", - Detail: err.Error(), - AttributePath: cty.Path{cty.GetAttrStep{Name: "form_type"}}, - }, - } - } - // Set the form_type back in case the value was changed. - // Eg via a default. If a user does not specify, a default value - // is used and saved. - rd.Set("form_type", parameter.FormType) - - diags := parameter.Valid() + diags := parameter.Valid(value) if diags.HasError() { return diags } @@ -414,10 +406,13 @@ func valueIsType(typ OptionType, value string, attrPath cty.Path) diag.Diagnosti return nil } -func (v *Parameter) Valid() diag.Diagnostics { +func (v *Parameter) Valid(value string) diag.Diagnostics { + var err error + var optionType OptionType + // optionType might differ from parameter.Type. This is ok, and parameter.Type // should be used for the value type, and optionType for options. - optionType, _, err := ValidateFormType(v.Type, len(v.Option), v.FormType) + optionType, v.FormType, err = ValidateFormType(v.Type, len(v.Option), v.FormType) if err != nil { return diag.Diagnostics{ { @@ -458,24 +453,52 @@ func (v *Parameter) Valid() diag.Diagnostics { } optionValues[option.Value] = nil optionNames[option.Name] = nil + + // TODO: Option values should also be validated. + // v.validValue(option.Value, optionType, nil, cty.Path{}) + } + } + + // Validate the default value + if v.Default != "" { + d := v.validValue(v.Default, optionType, optionValues, defaultValuePath) + if d.HasError() { + return d } } - if v.Default != "" && len(v.Option) > 0 { + // Value must always be validated + d := v.validValue(value, optionType, optionValues, cty.Path{}) + if d.HasError() { + return d + } + + return nil +} + +func (v *Parameter) validValue(value string, optionType OptionType, optionValues map[string]any, path cty.Path) diag.Diagnostics { + // name is used for constructing more precise error messages. + name := "Value" + if path.Equals(defaultValuePath) { + name = "Default value" + } + + // First validate if the value is a valid option + if len(optionValues) > 0 { if v.Type == OptionTypeListString && optionType == OptionTypeString { // If the type is list(string) and optionType is string, we have // to ensure all elements of the default exist as options. - defaultValues, diags := valueIsListString(v.Default, cty.Path{cty.GetAttrStep{Name: "default"}}) + listValues, diags := valueIsListString(value, defaultValuePath) if diags.HasError() { return diags } // missing is used to construct a more helpful error message var missing []string - for _, defaultValue := range defaultValues { - _, defaultIsValid := optionValues[defaultValue] - if !defaultIsValid { - missing = append(missing, defaultValue) + for _, listValue := range listValues { + _, isValid := optionValues[listValue] + if !isValid { + missing = append(missing, listValue) } } @@ -483,30 +506,45 @@ func (v *Parameter) Valid() diag.Diagnostics { return diag.Diagnostics{ { Severity: diag.Error, - Summary: "Default values must be a valid option", + Summary: fmt.Sprintf("%ss must be a valid option", name), Detail: fmt.Sprintf( - "default value %q is not a valid option, values %q are missing from the options", - v.Default, strings.Join(missing, ", "), + "%s %q is not a valid option, values %q are missing from the options", + name, value, strings.Join(missing, ", "), ), - AttributePath: cty.Path{cty.GetAttrStep{Name: "default"}}, + AttributePath: defaultValuePath, }, } } } else { - _, defaultIsValid := optionValues[v.Default] - if !defaultIsValid { + _, isValid := optionValues[value] + if !isValid { return diag.Diagnostics{ { Severity: diag.Error, - Summary: "Default value must be a valid option", - Detail: fmt.Sprintf("the value %q must be defined as one of options", v.Default), - AttributePath: cty.Path{cty.GetAttrStep{Name: "default"}}, + Summary: fmt.Sprintf("%s must be a valid option", name), + Detail: fmt.Sprintf("the value %q must be defined as one of options", value), + AttributePath: path, }, } } } } + if len(v.Validation) == 1 { + validCheck := &v.Validation[0] + err := validCheck.Valid(v.Type, value) + if err != nil { + return diag.Diagnostics{ + { + Severity: diag.Error, + Summary: fmt.Sprintf("Invalid parameter %s according to 'validation' block", strings.ToLower(name)), + Detail: err.Error(), + AttributePath: path, + }, + } + } + } + return nil } From 9bb9a1f253a6ace753a87228de4f060f3c9c49dc Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Mon, 14 Apr 2025 16:35:09 -0500 Subject: [PATCH 02/22] begin adding invalid unit tests --- provider/parameter.go | 85 ++++++++++++++++++------------ provider/parameter_test.go | 103 +++++++++++++++++++++++++++++++++++++ 2 files changed, 155 insertions(+), 33 deletions(-) diff --git a/provider/parameter.go b/provider/parameter.go index 05cd57da..f28524e2 100644 --- a/provider/parameter.go +++ b/provider/parameter.go @@ -50,7 +50,6 @@ const ( ) type Parameter struct { - Value string Name string DisplayName string `mapstructure:"display_name"` Description string @@ -86,7 +85,6 @@ func parameterDataSource() *schema.Resource { var parameter Parameter err = mapstructure.Decode(struct { - Value interface{} Name interface{} DisplayName interface{} Description interface{} @@ -101,7 +99,6 @@ func parameterDataSource() *schema.Resource { Order interface{} Ephemeral interface{} }{ - Value: rd.Get("value"), Name: rd.Get("name"), DisplayName: rd.Get("display_name"), Description: rd.Get("description"), @@ -126,14 +123,7 @@ func parameterDataSource() *schema.Resource { if err != nil { return diag.Errorf("decode parameter: %s", err) } - var value string - if parameter.Default != "" { - err := valueIsType(parameter.Type, parameter.Default, defaultValuePath) - if err != nil { - return err - } - value = parameter.Default - } + value := parameter.Default envValue, ok := os.LookupEnv(ParameterEnvironmentVariable(parameter.Name)) if ok { value = envValue @@ -381,27 +371,27 @@ func fixValidationResourceData(rawConfig cty.Value, validation interface{}) (int return vArr, nil } -func valueIsType(typ OptionType, value string, attrPath cty.Path) diag.Diagnostics { +func valueIsType(typ OptionType, value string) error { switch typ { case OptionTypeNumber: _, err := strconv.ParseFloat(value, 64) if err != nil { - return diag.Errorf("%q is not a number", value) + return fmt.Errorf("%q is not a number", value) } case OptionTypeBoolean: _, err := strconv.ParseBool(value) if err != nil { - return diag.Errorf("%q is not a bool", value) + return fmt.Errorf("%q is not a bool", value) } case OptionTypeListString: - _, diags := valueIsListString(value, attrPath) - if diags.HasError() { - return diags + _, err := valueIsListString(value) + if err != nil { + return err } case OptionTypeString: // Anything is a string! default: - return diag.Errorf("invalid type %q", typ) + return fmt.Errorf("invalid type %q", typ) } return nil } @@ -447,9 +437,15 @@ func (v *Parameter) Valid(value string) diag.Diagnostics { }, } } - diags := valueIsType(optionType, option.Value, cty.Path{}) - if diags.HasError() { - return diags + err = valueIsType(optionType, option.Value) + if err != nil { + return diag.Diagnostics{ + { + Severity: diag.Error, + Summary: fmt.Sprintf("Option %q with value=%q is not of type %q", option.Name, option.Value, optionType), + Detail: err.Error(), + }, + } } optionValues[option.Value] = nil optionNames[option.Name] = nil @@ -461,6 +457,18 @@ func (v *Parameter) Valid(value string) diag.Diagnostics { // Validate the default value if v.Default != "" { + err := valueIsType(v.Type, v.Default) + if err != nil { + return diag.Diagnostics{ + { + Severity: diag.Error, + Summary: fmt.Sprintf("Default value is not of type %q", v.Type), + Detail: err.Error(), + AttributePath: defaultValuePath, + }, + } + } + d := v.validValue(v.Default, optionType, optionValues, defaultValuePath) if d.HasError() { return d @@ -473,6 +481,17 @@ func (v *Parameter) Valid(value string) diag.Diagnostics { return d } + err = valueIsType(v.Type, value) + if err != nil { + return diag.Diagnostics{ + { + Severity: diag.Error, + Summary: fmt.Sprintf("Parameter value is not of type %q", v.Type), + Detail: err.Error(), + }, + } + } + return nil } @@ -488,9 +507,16 @@ func (v *Parameter) validValue(value string, optionType OptionType, optionValues if v.Type == OptionTypeListString && optionType == OptionTypeString { // If the type is list(string) and optionType is string, we have // to ensure all elements of the default exist as options. - listValues, diags := valueIsListString(value, defaultValuePath) - if diags.HasError() { - return diags + listValues, err := valueIsListString(value) + if err != nil { + return diag.Diagnostics{ + { + Severity: diag.Error, + Summary: "When using list(string) type, value must be a json encoded list of strings", + Detail: err.Error(), + AttributePath: defaultValuePath, + }, + } } // missing is used to construct a more helpful error message @@ -608,18 +634,11 @@ func (v *Validation) Valid(typ OptionType, value string) error { return nil } -func valueIsListString(value string, path cty.Path) ([]string, diag.Diagnostics) { +func valueIsListString(value string) ([]string, error) { var items []string err := json.Unmarshal([]byte(value), &items) if err != nil { - return nil, diag.Diagnostics{ - { - Severity: diag.Error, - Summary: "When using list(string) type, value must be a json encoded list of strings", - Detail: fmt.Sprintf("value %q is not a valid list of strings", value), - AttributePath: path, - }, - } + return nil, fmt.Errorf("value %q is not a valid list of strings", value) } return items, nil } diff --git a/provider/parameter_test.go b/provider/parameter_test.go index 37a46796..9059acc8 100644 --- a/provider/parameter_test.go +++ b/provider/parameter_test.go @@ -10,6 +10,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/coder/terraform-provider-coder/v2/provider" @@ -688,6 +689,7 @@ data "coder_parameter" "region" { } } +<<<<<<< HEAD // TestParameterValidationEnforcement tests various parameter states and the // validation enforcement that should be applied to them. The table is described // by a markdown table. This is done so that the test cases can be more easily @@ -896,6 +898,107 @@ func TestParameterValidationEnforcement(t *testing.T) { }) }) } +======= +func TestParameterValidation(t *testing.T) { + t.Parallel() + opts := func(vals ...string) []provider.Option { + options := make([]provider.Option, 0, len(vals)) + for _, val := range vals { + options = append(options, provider.Option{ + Name: val, + Value: val, + }) + } + return options + } + + for _, tc := range []struct { + Name string + Parameter provider.Parameter + Value string + ExpectError *regexp.Regexp + }{ + { + Name: "ValidStringParameter", + Parameter: provider.Parameter{ + Type: "string", + }, + Value: "alpha", + }, + // Test invalid states + { + Name: "InvalidFormType", + Parameter: provider.Parameter{ + Type: "string", + Option: opts("alpha", "bravo", "charlie"), + FormType: provider.ParameterFormTypeSlider, + }, + Value: "alpha", + ExpectError: regexp.MustCompile("Invalid form_type for parameter"), + }, + { + Name: "NotInOptions", + Parameter: provider.Parameter{ + Type: "string", + Option: opts("alpha", "bravo", "charlie"), + }, + Value: "delta", // not in option set + ExpectError: regexp.MustCompile("Value must be a valid option"), + }, + { + Name: "NonUniqueOptionNames", + Parameter: provider.Parameter{ + Type: "string", + Option: opts("alpha", "alpha"), + }, + Value: "alpha", + ExpectError: regexp.MustCompile("Option names must be unique"), + }, + { + Name: "NonUniqueOptionValues", + Parameter: provider.Parameter{ + Type: "string", + Option: []provider.Option{ + {Name: "Alpha", Value: "alpha"}, + {Name: "AlphaAgain", Value: "alpha"}, + }, + }, + Value: "alpha", + ExpectError: regexp.MustCompile("Option values must be unique"), + }, + { + Name: "IncorrectValueTypeOption", + Parameter: provider.Parameter{ + Type: "number", + Option: opts("not-a-number"), + }, + Value: "5", + ExpectError: regexp.MustCompile("is not a number"), + }, + { + Name: "IncorrectValueType", + Parameter: provider.Parameter{ + Type: "number", + }, + Value: "not-a-number", + ExpectError: regexp.MustCompile("Parameter value is not of type \"number\""), + }, + } { + tc := tc + t.Run(tc.Name, func(t *testing.T) { + t.Parallel() + diags := tc.Parameter.Valid(tc.Value) + if tc.ExpectError != nil { + require.True(t, diags.HasError()) + errMsg := fmt.Sprintf("%+v", diags[0]) // close enough + require.Truef(t, tc.ExpectError.MatchString(errMsg), "got: %s", errMsg) + } else { + if !assert.False(t, diags.HasError()) { + t.Logf("got: %+v", diags[0]) + } + } + }) +>>>>>>> c640b02 (begin adding invalid unit tests) } } From 6ee8b80682cce8f00234c554588e2aa36dfb6d5d Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Mon, 14 Apr 2025 16:57:39 -0500 Subject: [PATCH 03/22] add more validation tests --- provider/parameter_test.go | 58 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/provider/parameter_test.go b/provider/parameter_test.go index 9059acc8..373be459 100644 --- a/provider/parameter_test.go +++ b/provider/parameter_test.go @@ -945,6 +945,15 @@ func TestParameterValidation(t *testing.T) { Value: "delta", // not in option set ExpectError: regexp.MustCompile("Value must be a valid option"), }, + { + Name: "NumberNotInOptions", + Parameter: provider.Parameter{ + Type: "number", + Option: opts("1", "2", "3"), + }, + Value: "0", // not in option set + ExpectError: regexp.MustCompile("Value must be a valid option"), + }, { Name: "NonUniqueOptionNames", Parameter: provider.Parameter{ @@ -983,6 +992,55 @@ func TestParameterValidation(t *testing.T) { Value: "not-a-number", ExpectError: regexp.MustCompile("Parameter value is not of type \"number\""), }, + { + Name: "NotListStringDefault", + Parameter: provider.Parameter{ + Type: "list(string)", + Default: "not-a-list", + }, + ExpectError: regexp.MustCompile("not a valid list of strings"), + }, + { + Name: "NotListStringDefault", + Parameter: provider.Parameter{ + Type: "list(string)", + }, + Value: "not-a-list", + ExpectError: regexp.MustCompile("not a valid list of strings"), + }, + { + Name: "DefaultListStringNotInOptions", + Parameter: provider.Parameter{ + Type: "list(string)", + Default: `["red", "yellow", "black"]`, + Option: opts("red", "blue", "green"), + FormType: provider.ParameterFormTypeMultiSelect, + }, + ExpectError: regexp.MustCompile("is not a valid option, values \"yellow, black\" are missing from the options"), + }, + { + Name: "ListStringNotInOptions", + Parameter: provider.Parameter{ + Type: "list(string)", + Default: `["red"]`, + Option: opts("red", "blue", "green"), + FormType: provider.ParameterFormTypeMultiSelect, + }, + Value: `["red", "yellow", "black"]`, + ExpectError: regexp.MustCompile("is not a valid option, values \"yellow, black\" are missing from the options"), + }, + { + Name: "InvalidMiniumum", + Parameter: provider.Parameter{ + Type: "number", + Default: "5", + Validation: []provider.Validation{{ + Min: 10, + Error: "must be greater than 10", + }}, + }, + ExpectError: regexp.MustCompile("must be greater than 10"), + }, } { tc := tc t.Run(tc.Name, func(t *testing.T) { From 2cb01ccea99144187dde779d1131808c017f2199 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 15 Apr 2025 10:05:35 -0500 Subject: [PATCH 04/22] add better error message --- provider/parameter.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/provider/parameter.go b/provider/parameter.go index f28524e2..03ca419a 100644 --- a/provider/parameter.go +++ b/provider/parameter.go @@ -544,10 +544,14 @@ func (v *Parameter) validValue(value string, optionType OptionType, optionValues } else { _, isValid := optionValues[value] if !isValid { + extra := "" + if value == "" { + extra = ". The value is empty, did you forget to set it with a default or from user input?" + } return diag.Diagnostics{ { Severity: diag.Error, - Summary: fmt.Sprintf("%s must be a valid option", name), + Summary: fmt.Sprintf("%s must be a valid option%s", name, extra), Detail: fmt.Sprintf("the value %q must be defined as one of options", value), AttributePath: path, }, From 5e19e2a1de7534a48c4a357093bae7b8149dd96f Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 30 Apr 2025 14:38:09 -0500 Subject: [PATCH 05/22] revert test removal' --- provider/parameter_test.go | 320 ++++++++++++++++++------------------- 1 file changed, 160 insertions(+), 160 deletions(-) diff --git a/provider/parameter_test.go b/provider/parameter_test.go index 373be459..f27baffd 100644 --- a/provider/parameter_test.go +++ b/provider/parameter_test.go @@ -689,7 +689,166 @@ data "coder_parameter" "region" { } } -<<<<<<< HEAD +func TestParameterValidation(t *testing.T) { + t.Parallel() + opts := func(vals ...string) []provider.Option { + options := make([]provider.Option, 0, len(vals)) + for _, val := range vals { + options = append(options, provider.Option{ + Name: val, + Value: val, + }) + } + return options + } + + for _, tc := range []struct { + Name string + Parameter provider.Parameter + Value string + ExpectError *regexp.Regexp + }{ + { + Name: "ValidStringParameter", + Parameter: provider.Parameter{ + Type: "string", + }, + Value: "alpha", + }, + // Test invalid states + { + Name: "InvalidFormType", + Parameter: provider.Parameter{ + Type: "string", + Option: opts("alpha", "bravo", "charlie"), + FormType: provider.ParameterFormTypeSlider, + }, + Value: "alpha", + ExpectError: regexp.MustCompile("Invalid form_type for parameter"), + }, + { + Name: "NotInOptions", + Parameter: provider.Parameter{ + Type: "string", + Option: opts("alpha", "bravo", "charlie"), + }, + Value: "delta", // not in option set + ExpectError: regexp.MustCompile("Value must be a valid option"), + }, + { + Name: "NumberNotInOptions", + Parameter: provider.Parameter{ + Type: "number", + Option: opts("1", "2", "3"), + }, + Value: "0", // not in option set + ExpectError: regexp.MustCompile("Value must be a valid option"), + }, + { + Name: "NonUniqueOptionNames", + Parameter: provider.Parameter{ + Type: "string", + Option: opts("alpha", "alpha"), + }, + Value: "alpha", + ExpectError: regexp.MustCompile("Option names must be unique"), + }, + { + Name: "NonUniqueOptionValues", + Parameter: provider.Parameter{ + Type: "string", + Option: []provider.Option{ + {Name: "Alpha", Value: "alpha"}, + {Name: "AlphaAgain", Value: "alpha"}, + }, + }, + Value: "alpha", + ExpectError: regexp.MustCompile("Option values must be unique"), + }, + { + Name: "IncorrectValueTypeOption", + Parameter: provider.Parameter{ + Type: "number", + Option: opts("not-a-number"), + }, + Value: "5", + ExpectError: regexp.MustCompile("is not a number"), + }, + { + Name: "IncorrectValueType", + Parameter: provider.Parameter{ + Type: "number", + }, + Value: "not-a-number", + ExpectError: regexp.MustCompile("Parameter value is not of type \"number\""), + }, + { + Name: "NotListStringDefault", + Parameter: provider.Parameter{ + Type: "list(string)", + Default: "not-a-list", + }, + ExpectError: regexp.MustCompile("not a valid list of strings"), + }, + { + Name: "NotListStringDefault", + Parameter: provider.Parameter{ + Type: "list(string)", + }, + Value: "not-a-list", + ExpectError: regexp.MustCompile("not a valid list of strings"), + }, + { + Name: "DefaultListStringNotInOptions", + Parameter: provider.Parameter{ + Type: "list(string)", + Default: `["red", "yellow", "black"]`, + Option: opts("red", "blue", "green"), + FormType: provider.ParameterFormTypeMultiSelect, + }, + ExpectError: regexp.MustCompile("is not a valid option, values \"yellow, black\" are missing from the options"), + }, + { + Name: "ListStringNotInOptions", + Parameter: provider.Parameter{ + Type: "list(string)", + Default: `["red"]`, + Option: opts("red", "blue", "green"), + FormType: provider.ParameterFormTypeMultiSelect, + }, + Value: `["red", "yellow", "black"]`, + ExpectError: regexp.MustCompile("is not a valid option, values \"yellow, black\" are missing from the options"), + }, + { + Name: "InvalidMiniumum", + Parameter: provider.Parameter{ + Type: "number", + Default: "5", + Validation: []provider.Validation{{ + Min: 10, + Error: "must be greater than 10", + }}, + }, + ExpectError: regexp.MustCompile("must be greater than 10"), + }, + } { + tc := tc + t.Run(tc.Name, func(t *testing.T) { + t.Parallel() + diags := tc.Parameter.Valid(tc.Value) + if tc.ExpectError != nil { + require.True(t, diags.HasError()) + errMsg := fmt.Sprintf("%+v", diags[0]) // close enough + require.Truef(t, tc.ExpectError.MatchString(errMsg), "got: %s", errMsg) + } else { + if !assert.False(t, diags.HasError()) { + t.Logf("got: %+v", diags[0]) + } + } + }) + } +} + // TestParameterValidationEnforcement tests various parameter states and the // validation enforcement that should be applied to them. The table is described // by a markdown table. This is done so that the test cases can be more easily @@ -898,165 +1057,6 @@ func TestParameterValidationEnforcement(t *testing.T) { }) }) } -======= -func TestParameterValidation(t *testing.T) { - t.Parallel() - opts := func(vals ...string) []provider.Option { - options := make([]provider.Option, 0, len(vals)) - for _, val := range vals { - options = append(options, provider.Option{ - Name: val, - Value: val, - }) - } - return options - } - - for _, tc := range []struct { - Name string - Parameter provider.Parameter - Value string - ExpectError *regexp.Regexp - }{ - { - Name: "ValidStringParameter", - Parameter: provider.Parameter{ - Type: "string", - }, - Value: "alpha", - }, - // Test invalid states - { - Name: "InvalidFormType", - Parameter: provider.Parameter{ - Type: "string", - Option: opts("alpha", "bravo", "charlie"), - FormType: provider.ParameterFormTypeSlider, - }, - Value: "alpha", - ExpectError: regexp.MustCompile("Invalid form_type for parameter"), - }, - { - Name: "NotInOptions", - Parameter: provider.Parameter{ - Type: "string", - Option: opts("alpha", "bravo", "charlie"), - }, - Value: "delta", // not in option set - ExpectError: regexp.MustCompile("Value must be a valid option"), - }, - { - Name: "NumberNotInOptions", - Parameter: provider.Parameter{ - Type: "number", - Option: opts("1", "2", "3"), - }, - Value: "0", // not in option set - ExpectError: regexp.MustCompile("Value must be a valid option"), - }, - { - Name: "NonUniqueOptionNames", - Parameter: provider.Parameter{ - Type: "string", - Option: opts("alpha", "alpha"), - }, - Value: "alpha", - ExpectError: regexp.MustCompile("Option names must be unique"), - }, - { - Name: "NonUniqueOptionValues", - Parameter: provider.Parameter{ - Type: "string", - Option: []provider.Option{ - {Name: "Alpha", Value: "alpha"}, - {Name: "AlphaAgain", Value: "alpha"}, - }, - }, - Value: "alpha", - ExpectError: regexp.MustCompile("Option values must be unique"), - }, - { - Name: "IncorrectValueTypeOption", - Parameter: provider.Parameter{ - Type: "number", - Option: opts("not-a-number"), - }, - Value: "5", - ExpectError: regexp.MustCompile("is not a number"), - }, - { - Name: "IncorrectValueType", - Parameter: provider.Parameter{ - Type: "number", - }, - Value: "not-a-number", - ExpectError: regexp.MustCompile("Parameter value is not of type \"number\""), - }, - { - Name: "NotListStringDefault", - Parameter: provider.Parameter{ - Type: "list(string)", - Default: "not-a-list", - }, - ExpectError: regexp.MustCompile("not a valid list of strings"), - }, - { - Name: "NotListStringDefault", - Parameter: provider.Parameter{ - Type: "list(string)", - }, - Value: "not-a-list", - ExpectError: regexp.MustCompile("not a valid list of strings"), - }, - { - Name: "DefaultListStringNotInOptions", - Parameter: provider.Parameter{ - Type: "list(string)", - Default: `["red", "yellow", "black"]`, - Option: opts("red", "blue", "green"), - FormType: provider.ParameterFormTypeMultiSelect, - }, - ExpectError: regexp.MustCompile("is not a valid option, values \"yellow, black\" are missing from the options"), - }, - { - Name: "ListStringNotInOptions", - Parameter: provider.Parameter{ - Type: "list(string)", - Default: `["red"]`, - Option: opts("red", "blue", "green"), - FormType: provider.ParameterFormTypeMultiSelect, - }, - Value: `["red", "yellow", "black"]`, - ExpectError: regexp.MustCompile("is not a valid option, values \"yellow, black\" are missing from the options"), - }, - { - Name: "InvalidMiniumum", - Parameter: provider.Parameter{ - Type: "number", - Default: "5", - Validation: []provider.Validation{{ - Min: 10, - Error: "must be greater than 10", - }}, - }, - ExpectError: regexp.MustCompile("must be greater than 10"), - }, - } { - tc := tc - t.Run(tc.Name, func(t *testing.T) { - t.Parallel() - diags := tc.Parameter.Valid(tc.Value) - if tc.ExpectError != nil { - require.True(t, diags.HasError()) - errMsg := fmt.Sprintf("%+v", diags[0]) // close enough - require.Truef(t, tc.ExpectError.MatchString(errMsg), "got: %s", errMsg) - } else { - if !assert.False(t, diags.HasError()) { - t.Logf("got: %+v", diags[0]) - } - } - }) ->>>>>>> c640b02 (begin adding invalid unit tests) } } From b6787b08c24a1982fd0672403afbbd427963e514 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 30 Apr 2025 16:10:49 -0500 Subject: [PATCH 06/22] WIP --- provider/parameter.go | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/provider/parameter.go b/provider/parameter.go index 03ca419a..df179a6b 100644 --- a/provider/parameter.go +++ b/provider/parameter.go @@ -142,22 +142,25 @@ func parameterDataSource() *schema.Resource { // 'parameter.FormType' value to the new value. This is to handle default cases, // since the default logic is more advanced than the sdk provider schema // supports. - _, newFT, err := ValidateFormType(parameter.Type, len(parameter.Option), parameter.FormType) - if err == nil { - // If there is an error, parameter.Valid will catch it. - parameter.FormType = newFT - - // Set the form_type back in case the value was changed. - // Eg via a default. If a user does not specify, a default value - // is used and saved. - rd.Set("form_type", parameter.FormType) - } + //_, newFT, err := ValidateFormType(parameter.Type, len(parameter.Option), parameter.FormType) + //if err == nil { + // // If there is an error, parameter.Valid will catch it. + // parameter.FormType = newFT + // + // // Set the form_type back in case the value was changed. + // // Eg via a default. If a user does not specify, a default value + // // is used and saved. + // rd.Set("form_type", parameter.FormType) + //} diags := parameter.Valid(value) if diags.HasError() { return diags } + // Set the form_type as it could have changed in the validation. + rd.Set("form_type", parameter.FormType) + return nil }, Schema: map[string]*schema.Schema{ @@ -396,6 +399,11 @@ func valueIsType(typ OptionType, value string) error { return nil } +func (v *Parameter) RelaxedValidation(value string) diag.Diagnostics { + + return nil +} + func (v *Parameter) Valid(value string) diag.Diagnostics { var err error var optionType OptionType From aaf1a175eacc46e508e3af14122c0cf4ebf4a1e5 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 1 May 2025 13:55:48 -0500 Subject: [PATCH 07/22] chore: refactor validation while still keeping it mostly the same Some validation changes: - Invalid options rejected (not in option set) --- provider/parameter.go | 209 ++++++++++++++------------- provider/parameter_test.go | 2 +- provider/testdata/parameter_table.md | 140 +++++++++--------- 3 files changed, 183 insertions(+), 168 deletions(-) diff --git a/provider/parameter.go b/provider/parameter.go index df179a6b..a72b7a91 100644 --- a/provider/parameter.go +++ b/provider/parameter.go @@ -123,12 +123,25 @@ func parameterDataSource() *schema.Resource { if err != nil { return diag.Errorf("decode parameter: %s", err) } - value := parameter.Default + + var value *string + if !rd.GetRawConfig().AsValueMap()["default"].IsNull() { + value = ¶meter.Default + } + envValue, ok := os.LookupEnv(ParameterEnvironmentVariable(parameter.Name)) if ok { - value = envValue + value = &envValue + } + + if value != nil { + rd.Set("value", value) + } else { + // Maintaining backwards compatibility. The previous behavior was + // to write back an empty string. + // TODO: Should an empty string exist if no value is set? + rd.Set("value", "") } - rd.Set("value", value) if !parameter.Mutable && parameter.Ephemeral { return diag.Errorf("parameter can't be immutable and ephemeral") @@ -138,21 +151,6 @@ func parameterDataSource() *schema.Resource { return diag.Errorf("ephemeral parameter requires the default property") } - // Do ValidateFormType up front. If there is no error, update the - // 'parameter.FormType' value to the new value. This is to handle default cases, - // since the default logic is more advanced than the sdk provider schema - // supports. - //_, newFT, err := ValidateFormType(parameter.Type, len(parameter.Option), parameter.FormType) - //if err == nil { - // // If there is an error, parameter.Valid will catch it. - // parameter.FormType = newFT - // - // // Set the form_type back in case the value was changed. - // // Eg via a default. If a user does not specify, a default value - // // is used and saved. - // rd.Set("form_type", parameter.FormType) - //} - diags := parameter.Valid(value) if diags.HasError() { return diags @@ -399,12 +397,7 @@ func valueIsType(typ OptionType, value string) error { return nil } -func (v *Parameter) RelaxedValidation(value string) diag.Diagnostics { - - return nil -} - -func (v *Parameter) Valid(value string) diag.Diagnostics { +func (v *Parameter) Valid(value *string) diag.Diagnostics { var err error var optionType OptionType @@ -422,88 +415,125 @@ func (v *Parameter) Valid(value string) diag.Diagnostics { } } - optionNames := map[string]any{} - optionValues := map[string]any{} - if len(v.Option) > 0 { - for _, option := range v.Option { - _, exists := optionNames[option.Name] - if exists { - return diag.Diagnostics{{ - Severity: diag.Error, - Summary: "Option names must be unique.", - Detail: fmt.Sprintf("multiple options found with the same name %q", option.Name), - }, - } - } - _, exists = optionValues[option.Value] - if exists { - return diag.Diagnostics{ - { - Severity: diag.Error, - Summary: "Option values must be unique.", - Detail: fmt.Sprintf("multiple options found with the same value %q", option.Value), - }, - } - } - err = valueIsType(optionType, option.Value) - if err != nil { - return diag.Diagnostics{ - { - Severity: diag.Error, - Summary: fmt.Sprintf("Option %q with value=%q is not of type %q", option.Name, option.Value, optionType), - Detail: err.Error(), - }, - } - } - optionValues[option.Value] = nil - optionNames[option.Name] = nil - - // TODO: Option values should also be validated. - // v.validValue(option.Value, optionType, nil, cty.Path{}) - } + optionValues, diags := v.ValidOptions(optionType) + if diags.HasError() { + return diags } - // Validate the default value - if v.Default != "" { - err := valueIsType(v.Type, v.Default) + // TODO: The default value should also be validated + //if v.Default != "" { + // err := valueIsType(v.Type, v.Default) + // if err != nil { + // return diag.Diagnostics{ + // { + // Severity: diag.Error, + // Summary: fmt.Sprintf("Default value is not of type %q", v.Type), + // Detail: err.Error(), + // AttributePath: defaultValuePath, + // }, + // } + // } + // + // d := v.validValue(v.Default, optionType, optionValues, defaultValuePath) + // if d.HasError() { + // return d + // } + //} + + // TODO: Move this into 'Parameter.validValue'. It exists as another check outside because + // the current behavior is to always apply this validation, regardless if the param is set or not. + // Other validations are only applied if the parameter is set. + // This behavior should be standardized. + if len(v.Validation) == 1 { + empty := "" + validVal := value + if value == nil { + validVal = &empty + } + validCheck := &v.Validation[0] + err := validCheck.Valid(v.Type, *validVal) if err != nil { return diag.Diagnostics{ { Severity: diag.Error, - Summary: fmt.Sprintf("Default value is not of type %q", v.Type), + Summary: fmt.Sprintf("Invalid parameter %s according to 'validation' block", strings.ToLower(v.Name)), Detail: err.Error(), - AttributePath: defaultValuePath, + AttributePath: cty.Path{}, }, } } + } - d := v.validValue(v.Default, optionType, optionValues, defaultValuePath) + // Value is only validated if it is set. If it is unset, validation + // is skipped. + if value != nil { + d := v.validValue(*value, optionType, optionValues, cty.Path{}) if d.HasError() { return d } - } - // Value must always be validated - d := v.validValue(value, optionType, optionValues, cty.Path{}) - if d.HasError() { - return d + err = valueIsType(v.Type, *value) + if err != nil { + return diag.Diagnostics{ + { + Severity: diag.Error, + Summary: fmt.Sprintf("Parameter value is not of type %q", v.Type), + Detail: err.Error(), + }, + } + } } - err = valueIsType(v.Type, value) - if err != nil { - return diag.Diagnostics{ - { + return nil +} + +func (v *Parameter) ValidOptions(optionType OptionType) (map[string]struct{}, diag.Diagnostics) { + optionNames := map[string]struct{}{} + optionValues := map[string]struct{}{} + + var diags diag.Diagnostics + for _, option := range v.Option { + _, exists := optionNames[option.Name] + if exists { + return nil, diag.Diagnostics{{ Severity: diag.Error, - Summary: fmt.Sprintf("Parameter value is not of type %q", v.Type), + Summary: "Option names must be unique.", + Detail: fmt.Sprintf("multiple options found with the same name %q", option.Name), + }} + } + + _, exists = optionValues[option.Value] + if exists { + return nil, diag.Diagnostics{{ + Severity: diag.Error, + Summary: "Option values must be unique.", + Detail: fmt.Sprintf("multiple options found with the same value %q", option.Value), + }} + } + + err := valueIsType(optionType, option.Value) + if err != nil { + diags = append(diags, diag.Diagnostic{ + Severity: diag.Error, + Summary: fmt.Sprintf("Option %q with value=%q is not of type %q", option.Name, option.Value, optionType), Detail: err.Error(), - }, + }) + continue } + optionValues[option.Value] = struct{}{} + optionNames[option.Name] = struct{}{} + + // TODO: Option values should also be validated. + // v.validValue(option.Value, optionType, nil, cty.Path{}) } - return nil + if diags.HasError() { + return nil, diags + } + return optionValues, nil } -func (v *Parameter) validValue(value string, optionType OptionType, optionValues map[string]any, path cty.Path) diag.Diagnostics { +func (v *Parameter) validValue(value string, optionType OptionType, optionValues map[string]struct{}, path cty.Path) diag.Diagnostics { // name is used for constructing more precise error messages. name := "Value" if path.Equals(defaultValuePath) { @@ -568,21 +598,6 @@ func (v *Parameter) validValue(value string, optionType OptionType, optionValues } } - if len(v.Validation) == 1 { - validCheck := &v.Validation[0] - err := validCheck.Valid(v.Type, value) - if err != nil { - return diag.Diagnostics{ - { - Severity: diag.Error, - Summary: fmt.Sprintf("Invalid parameter %s according to 'validation' block", strings.ToLower(name)), - Detail: err.Error(), - AttributePath: path, - }, - } - } - } - return nil } diff --git a/provider/parameter_test.go b/provider/parameter_test.go index f27baffd..a0b96cbd 100644 --- a/provider/parameter_test.go +++ b/provider/parameter_test.go @@ -835,7 +835,7 @@ func TestParameterValidation(t *testing.T) { tc := tc t.Run(tc.Name, func(t *testing.T) { t.Parallel() - diags := tc.Parameter.Valid(tc.Value) + diags := tc.Parameter.Valid(&tc.Value) if tc.ExpectError != nil { require.True(t, diags.HasError()) errMsg := fmt.Sprintf("%+v", diags[0]) // close enough diff --git a/provider/testdata/parameter_table.md b/provider/testdata/parameter_table.md index 4c9ee458..34c5470a 100644 --- a/provider/testdata/parameter_table.md +++ b/provider/testdata/parameter_table.md @@ -1,70 +1,70 @@ -| Name | Type | Input | Default | Options | Validation | -> | Output | Optional | Error | -|----------------------|---------------|-----------|---------|-------------------|------------|----|--------|----------|--------------| -| | Empty Vals | | | | | | | | | -| Empty | string,number | | | | | | "" | false | | -| EmptyList | list(string) | | | | | | "" | false | | -| EmptyMulti | tag-select | | | | | | "" | false | | -| EmptyOpts | string,number | | | 1,2,3 | | | "" | false | | -| EmptyRegex | string | | | | world | | | | regex error | -| EmptyMin | number | | | | 1-10 | | | | 1 < < 10 | -| EmptyMinOpt | number | | | 1,2,3 | 2-5 | | | | 2 < < 5 | -| EmptyRegexOpt | string | | | "hello","goodbye" | goodbye | | | | regex error | -| EmptyRegexOk | string | | | | .* | | "" | false | | -| | | | | | | | | | | -| | Default Set | No inputs | | | | | | | | -| NumDef | number | | 5 | | | | 5 | true | | -| NumDefVal | number | | 5 | | 3-7 | | 5 | true | | -| NumDefInv | number | | 5 | | 10- | | | | 10 < 5 < 0 | -| NumDefOpts | number | | 5 | 1,3,5,7 | 2-6 | | 5 | true | | -| NumDefNotOpts | number | | 5 | 1,3,7,9 | 2-6 | | | | valid option | -| NumDefInvOpt | number | | 5 | 1,3,5,7 | 6-10 | | | | 6 < 5 < 10 | -| | | | | | | | | | | -| StrDef | string | | hello | | | | hello | true | | -| StrDefInv | string | | hello | | world | | | | regex error | -| StrDefOpts | string | | a | a,b,c | | | a | true | | -| StrDefNotOpts | string | | a | b,c,d | | | | | valid option | -| StrDefValOpts | string | | a | a,b,c,d,e,f | [a-c] | | a | true | | -| StrDefInvOpt | string | | d | a,b,c,d,e,f | [a-c] | | | | regex error | -| | | | | | | | | | | -| LStrDef | list(string) | | ["a"] | | | | ["a"] | true | | -| LStrDefOpts | list(string) | | ["a"] | ["a"], ["b"] | | | ["a"] | true | | -| LStrDefNotOpts | list(string) | | ["a"] | ["b"], ["c"] | | | | | valid option | -| | | | | | | | | | | -| MulDef | tag-select | | ["a"] | | | | ["a"] | true | | -| MulDefOpts | multi-select | | ["a"] | a,b | | | ["a"] | true | | -| MulDefNotOpts | multi-select | | ["a"] | b,c | | | | | valid option | -| | | | | | | | | | | -| | Input Vals | | | | | | | | | -| NumIns | number | 3 | | | | | 3 | false | | -| NumInsDef | number | 3 | 5 | | | | 3 | true | | -| NumIns/DefInv | number | 3 | 5 | | 1-3 | | 3 | true | | -| NumIns=DefInv | number | 5 | 5 | | 1-3 | | | | 1 < 5 < 3 | -| NumInsOpts | number | 3 | 5 | 1,2,3,4,5 | 1-3 | | 3 | true | | -| NumInsNotOptsVal | number | 3 | 5 | 1,2,4,5 | 1-3 | | 3 | true | | -| NumInsNotOptsInv | number | 3 | 5 | 1,2,4,5 | 1-2 | | | true | 1 < 3 < 2 | -| NumInsNotOpts | number | 3 | 5 | 1,2,4,5 | | | 3 | true | | -| NumInsNotOpts/NoDef | number | 3 | | 1,2,4,5 | | | 3 | false | | -| | | | | | | | | | | -| StrIns | string | c | | | | | c | false | | -| StrInsDef | string | c | e | | | | c | true | | -| StrIns/DefInv | string | c | e | | [a-c] | | c | true | | -| StrIns=DefInv | string | e | e | | [a-c] | | | | regex error | -| StrInsOpts | string | c | e | a,b,c,d,e | [a-c] | | c | true | | -| StrInsNotOptsVal | string | c | e | a,b,d,e | [a-c] | | c | true | | -| StrInsNotOptsInv | string | c | e | a,b,d,e | [a-b] | | | | regex error | -| StrInsNotOpts | string | c | e | a,b,d,e | | | c | true | | -| StrInsNotOpts/NoDef | string | c | | a,b,d,e | | | c | false | | -| StrInsBadVal | string | c | | a,b,c,d,e | 1-10 | | | | min cannot | -| | | | | | | | | | | -| | list(string) | | | | | | | | | -| LStrIns | list(string) | ["c"] | | | | | ["c"] | false | | -| LStrInsDef | list(string) | ["c"] | ["e"] | | | | ["c"] | true | | -| LStrIns/DefInv | list(string) | ["c"] | ["e"] | | [a-c] | | | | regex cannot | -| LStrInsOpts | list(string) | ["c"] | ["e"] | ["c"],["d"],["e"] | | | ["c"] | true | | -| LStrInsNotOpts | list(string) | ["c"] | ["e"] | ["d"],["e"] | | | ["c"] | true | | -| LStrInsNotOpts/NoDef | list(string) | ["c"] | | ["d"],["e"] | | | ["c"] | false | | -| | | | | | | | | | | -| MulInsOpts | multi-select | ["c"] | ["e"] | c,d,e | | | ["c"] | true | | -| MulInsNotOpts | multi-select | ["c"] | ["e"] | d,e | | | ["c"] | true | | -| MulInsNotOpts/NoDef | multi-select | ["c"] | | d,e | | | ["c"] | false | | -| MulInsInvOpts | multi-select | ["c"] | ["e"] | c,d,e | [a-c] | | | | regex cannot | \ No newline at end of file +| Name | Type | Input | Default | Options | Validation | -> | Output | Optional | Error | +|----------------------|---------------|-----------|---------|-------------------|------------|----|--------|----------|---------------| +| | Empty Vals | | | | | | | | | +| Empty | string,number | | | | | | "" | false | | +| EmptyList | list(string) | | | | | | "" | false | | +| EmptyMulti | tag-select | | | | | | "" | false | | +| EmptyOpts | string,number | | | 1,2,3 | | | "" | false | | +| EmptyRegex | string | | | | world | | | | regex error | +| EmptyMin | number | | | | 1-10 | | | | 1 < < 10 | +| EmptyMinOpt | number | | | 1,2,3 | 2-5 | | | | 2 < < 5 | +| EmptyRegexOpt | string | | | "hello","goodbye" | goodbye | | | | regex error | +| EmptyRegexOk | string | | | | .* | | "" | false | | +| | | | | | | | | | | +| | Default Set | No inputs | | | | | | | | +| NumDef | number | | 5 | | | | 5 | true | | +| NumDefVal | number | | 5 | | 3-7 | | 5 | true | | +| NumDefInv | number | | 5 | | 10- | | | | 10 < 5 < 0 | +| NumDefOpts | number | | 5 | 1,3,5,7 | 2-6 | | 5 | true | | +| NumDefNotOpts | number | | 5 | 1,3,7,9 | 2-6 | | | | valid option | +| NumDefInvOpt | number | | 5 | 1,3,5,7 | 6-10 | | | | 6 < 5 < 10 | +| | | | | | | | | | | +| StrDef | string | | hello | | | | hello | true | | +| StrDefInv | string | | hello | | world | | | | regex error | +| StrDefOpts | string | | a | a,b,c | | | a | true | | +| StrDefNotOpts | string | | a | b,c,d | | | | | valid option | +| StrDefValOpts | string | | a | a,b,c,d,e,f | [a-c] | | a | true | | +| StrDefInvOpt | string | | d | a,b,c,d,e,f | [a-c] | | | | regex error | +| | | | | | | | | | | +| LStrDef | list(string) | | ["a"] | | | | ["a"] | true | | +| LStrDefOpts | list(string) | | ["a"] | ["a"], ["b"] | | | ["a"] | true | | +| LStrDefNotOpts | list(string) | | ["a"] | ["b"], ["c"] | | | | | valid option | +| | | | | | | | | | | +| MulDef | tag-select | | ["a"] | | | | ["a"] | true | | +| MulDefOpts | multi-select | | ["a"] | a,b | | | ["a"] | true | | +| MulDefNotOpts | multi-select | | ["a"] | b,c | | | | | valid option | +| | | | | | | | | | | +| | Input Vals | | | | | | | | | +| NumIns | number | 3 | | | | | 3 | false | | +| NumInsDef | number | 3 | 5 | | | | 3 | true | | +| NumIns/DefInv | number | 3 | 5 | | 1-3 | | 3 | true | | +| NumIns=DefInv | number | 5 | 5 | | 1-3 | | | | 1 < 5 < 3 | +| NumInsOpts | number | 3 | 5 | 1,2,3,4,5 | 1-3 | | 3 | true | | +| NumInsNotOptsVal | number | 3 | 5 | 1,2,4,5 | 1-3 | | | | valid option | +| NumInsNotOptsInv | number | 3 | 5 | 1,2,4,5 | 1-2 | | | true | 1 < 3 < 2 | +| NumInsNotOpts | number | 3 | 5 | 1,2,4,5 | | | | | valid option | +| NumInsNotOpts/NoDef | number | 3 | | 1,2,4,5 | | | | | valid option | +| | | | | | | | | | | +| StrIns | string | c | | | | | c | false | | +| StrInsDef | string | c | e | | | | c | true | | +| StrIns/DefInv | string | c | e | | [a-c] | | c | true | | +| StrIns=DefInv | string | e | e | | [a-c] | | | | regex error | +| StrInsOpts | string | c | e | a,b,c,d,e | [a-c] | | c | true | | +| StrInsNotOptsVal | string | c | e | a,b,d,e | [a-c] | | c | true | | +| StrInsNotOptsInv | string | c | e | a,b,d,e | [a-b] | | | | regex error | +| StrInsNotOpts | string | c | e | a,b,d,e | | | c | true | | +| StrInsNotOpts/NoDef | string | c | | a,b,d,e | | | c | false | | +| StrInsBadVal | string | c | | a,b,c,d,e | 1-10 | | | | min cannot | +| | | | | | | | | | | +| | list(string) | | | | | | | | | +| LStrIns | list(string) | ["c"] | | | | | ["c"] | false | | +| LStrInsDef | list(string) | ["c"] | ["e"] | | | | ["c"] | true | | +| LStrIns/DefInv | list(string) | ["c"] | ["e"] | | [a-c] | | | | regex cannot | +| LStrInsOpts | list(string) | ["c"] | ["e"] | ["c"],["d"],["e"] | | | ["c"] | true | | +| LStrInsNotOpts | list(string) | ["c"] | ["e"] | ["d"],["e"] | | | ["c"] | true | | +| LStrInsNotOpts/NoDef | list(string) | ["c"] | | ["d"],["e"] | | | ["c"] | false | | +| | | | | | | | | | | +| MulInsOpts | multi-select | ["c"] | ["e"] | c,d,e | | | ["c"] | true | | +| MulInsNotOpts | multi-select | ["c"] | ["e"] | d,e | | | ["c"] | true | | +| MulInsNotOpts/NoDef | multi-select | ["c"] | | d,e | | | ["c"] | false | | +| MulInsInvOpts | multi-select | ["c"] | ["e"] | c,d,e | [a-c] | | | | regex cannot | \ No newline at end of file From 7cf79d8e25d96eecbc30b810b3ae393f5e1255df Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 1 May 2025 14:00:15 -0500 Subject: [PATCH 08/22] fixup test cases --- provider/testdata/parameter_table.md | 140 +++++++++++++-------------- 1 file changed, 70 insertions(+), 70 deletions(-) diff --git a/provider/testdata/parameter_table.md b/provider/testdata/parameter_table.md index 34c5470a..4c6ef99c 100644 --- a/provider/testdata/parameter_table.md +++ b/provider/testdata/parameter_table.md @@ -1,70 +1,70 @@ -| Name | Type | Input | Default | Options | Validation | -> | Output | Optional | Error | -|----------------------|---------------|-----------|---------|-------------------|------------|----|--------|----------|---------------| -| | Empty Vals | | | | | | | | | -| Empty | string,number | | | | | | "" | false | | -| EmptyList | list(string) | | | | | | "" | false | | -| EmptyMulti | tag-select | | | | | | "" | false | | -| EmptyOpts | string,number | | | 1,2,3 | | | "" | false | | -| EmptyRegex | string | | | | world | | | | regex error | -| EmptyMin | number | | | | 1-10 | | | | 1 < < 10 | -| EmptyMinOpt | number | | | 1,2,3 | 2-5 | | | | 2 < < 5 | -| EmptyRegexOpt | string | | | "hello","goodbye" | goodbye | | | | regex error | -| EmptyRegexOk | string | | | | .* | | "" | false | | -| | | | | | | | | | | -| | Default Set | No inputs | | | | | | | | -| NumDef | number | | 5 | | | | 5 | true | | -| NumDefVal | number | | 5 | | 3-7 | | 5 | true | | -| NumDefInv | number | | 5 | | 10- | | | | 10 < 5 < 0 | -| NumDefOpts | number | | 5 | 1,3,5,7 | 2-6 | | 5 | true | | -| NumDefNotOpts | number | | 5 | 1,3,7,9 | 2-6 | | | | valid option | -| NumDefInvOpt | number | | 5 | 1,3,5,7 | 6-10 | | | | 6 < 5 < 10 | -| | | | | | | | | | | -| StrDef | string | | hello | | | | hello | true | | -| StrDefInv | string | | hello | | world | | | | regex error | -| StrDefOpts | string | | a | a,b,c | | | a | true | | -| StrDefNotOpts | string | | a | b,c,d | | | | | valid option | -| StrDefValOpts | string | | a | a,b,c,d,e,f | [a-c] | | a | true | | -| StrDefInvOpt | string | | d | a,b,c,d,e,f | [a-c] | | | | regex error | -| | | | | | | | | | | -| LStrDef | list(string) | | ["a"] | | | | ["a"] | true | | -| LStrDefOpts | list(string) | | ["a"] | ["a"], ["b"] | | | ["a"] | true | | -| LStrDefNotOpts | list(string) | | ["a"] | ["b"], ["c"] | | | | | valid option | -| | | | | | | | | | | -| MulDef | tag-select | | ["a"] | | | | ["a"] | true | | -| MulDefOpts | multi-select | | ["a"] | a,b | | | ["a"] | true | | -| MulDefNotOpts | multi-select | | ["a"] | b,c | | | | | valid option | -| | | | | | | | | | | -| | Input Vals | | | | | | | | | -| NumIns | number | 3 | | | | | 3 | false | | -| NumInsDef | number | 3 | 5 | | | | 3 | true | | -| NumIns/DefInv | number | 3 | 5 | | 1-3 | | 3 | true | | -| NumIns=DefInv | number | 5 | 5 | | 1-3 | | | | 1 < 5 < 3 | -| NumInsOpts | number | 3 | 5 | 1,2,3,4,5 | 1-3 | | 3 | true | | -| NumInsNotOptsVal | number | 3 | 5 | 1,2,4,5 | 1-3 | | | | valid option | -| NumInsNotOptsInv | number | 3 | 5 | 1,2,4,5 | 1-2 | | | true | 1 < 3 < 2 | -| NumInsNotOpts | number | 3 | 5 | 1,2,4,5 | | | | | valid option | -| NumInsNotOpts/NoDef | number | 3 | | 1,2,4,5 | | | | | valid option | -| | | | | | | | | | | -| StrIns | string | c | | | | | c | false | | -| StrInsDef | string | c | e | | | | c | true | | -| StrIns/DefInv | string | c | e | | [a-c] | | c | true | | -| StrIns=DefInv | string | e | e | | [a-c] | | | | regex error | -| StrInsOpts | string | c | e | a,b,c,d,e | [a-c] | | c | true | | -| StrInsNotOptsVal | string | c | e | a,b,d,e | [a-c] | | c | true | | -| StrInsNotOptsInv | string | c | e | a,b,d,e | [a-b] | | | | regex error | -| StrInsNotOpts | string | c | e | a,b,d,e | | | c | true | | -| StrInsNotOpts/NoDef | string | c | | a,b,d,e | | | c | false | | -| StrInsBadVal | string | c | | a,b,c,d,e | 1-10 | | | | min cannot | -| | | | | | | | | | | -| | list(string) | | | | | | | | | -| LStrIns | list(string) | ["c"] | | | | | ["c"] | false | | -| LStrInsDef | list(string) | ["c"] | ["e"] | | | | ["c"] | true | | -| LStrIns/DefInv | list(string) | ["c"] | ["e"] | | [a-c] | | | | regex cannot | -| LStrInsOpts | list(string) | ["c"] | ["e"] | ["c"],["d"],["e"] | | | ["c"] | true | | -| LStrInsNotOpts | list(string) | ["c"] | ["e"] | ["d"],["e"] | | | ["c"] | true | | -| LStrInsNotOpts/NoDef | list(string) | ["c"] | | ["d"],["e"] | | | ["c"] | false | | -| | | | | | | | | | | -| MulInsOpts | multi-select | ["c"] | ["e"] | c,d,e | | | ["c"] | true | | -| MulInsNotOpts | multi-select | ["c"] | ["e"] | d,e | | | ["c"] | true | | -| MulInsNotOpts/NoDef | multi-select | ["c"] | | d,e | | | ["c"] | false | | -| MulInsInvOpts | multi-select | ["c"] | ["e"] | c,d,e | [a-c] | | | | regex cannot | \ No newline at end of file +| Name | Type | Input | Default | Options | Validation | -> | Output | Optional | Error | +|----------------------|---------------|-----------|---------|-------------------|------------|----|--------|----------|--------------| +| | Empty Vals | | | | | | | | | +| Empty | string,number | | | | | | "" | false | | +| EmptyList | list(string) | | | | | | "" | false | | +| EmptyMulti | tag-select | | | | | | "" | false | | +| EmptyOpts | string,number | | | 1,2,3 | | | "" | false | | +| EmptyRegex | string | | | | world | | | | regex error | +| EmptyMin | number | | | | 1-10 | | | | 1 < < 10 | +| EmptyMinOpt | number | | | 1,2,3 | 2-5 | | | | 2 < < 5 | +| EmptyRegexOpt | string | | | "hello","goodbye" | goodbye | | | | regex error | +| EmptyRegexOk | string | | | | .* | | "" | false | | +| | | | | | | | | | | +| | Default Set | No inputs | | | | | | | | +| NumDef | number | | 5 | | | | 5 | true | | +| NumDefVal | number | | 5 | | 3-7 | | 5 | true | | +| NumDefInv | number | | 5 | | 10- | | | | 10 < 5 < 0 | +| NumDefOpts | number | | 5 | 1,3,5,7 | 2-6 | | 5 | true | | +| NumDefNotOpts | number | | 5 | 1,3,7,9 | 2-6 | | | | valid option | +| NumDefInvOpt | number | | 5 | 1,3,5,7 | 6-10 | | | | 6 < 5 < 10 | +| | | | | | | | | | | +| StrDef | string | | hello | | | | hello | true | | +| StrDefInv | string | | hello | | world | | | | regex error | +| StrDefOpts | string | | a | a,b,c | | | a | true | | +| StrDefNotOpts | string | | a | b,c,d | | | | | valid option | +| StrDefValOpts | string | | a | a,b,c,d,e,f | [a-c] | | a | true | | +| StrDefInvOpt | string | | d | a,b,c,d,e,f | [a-c] | | | | regex error | +| | | | | | | | | | | +| LStrDef | list(string) | | ["a"] | | | | ["a"] | true | | +| LStrDefOpts | list(string) | | ["a"] | ["a"], ["b"] | | | ["a"] | true | | +| LStrDefNotOpts | list(string) | | ["a"] | ["b"], ["c"] | | | | | valid option | +| | | | | | | | | | | +| MulDef | tag-select | | ["a"] | | | | ["a"] | true | | +| MulDefOpts | multi-select | | ["a"] | a,b | | | ["a"] | true | | +| MulDefNotOpts | multi-select | | ["a"] | b,c | | | | | valid option | +| | | | | | | | | | | +| | Input Vals | | | | | | | | | +| NumIns | number | 3 | | | | | 3 | false | | +| NumInsDef | number | 3 | 5 | | | | 3 | true | | +| NumIns/DefInv | number | 3 | 5 | | 1-3 | | 3 | true | | +| NumIns=DefInv | number | 5 | 5 | | 1-3 | | | | 1 < 5 < 3 | +| NumInsOpts | number | 3 | 5 | 1,2,3,4,5 | 1-3 | | 3 | true | | +| NumInsNotOptsVal | number | 3 | 5 | 1,2,4,5 | 1-3 | | | | valid option | +| NumInsNotOptsInv | number | 3 | 5 | 1,2,4,5 | 1-2 | | | true | 1 < 3 < 2 | +| NumInsNotOpts | number | 3 | 5 | 1,2,4,5 | | | | | valid option | +| NumInsNotOpts/NoDef | number | 3 | | 1,2,4,5 | | | | | valid option | +| | | | | | | | | | | +| StrIns | string | c | | | | | c | false | | +| StrInsDef | string | c | e | | | | c | true | | +| StrIns/DefInv | string | c | e | | [a-c] | | c | true | | +| StrIns=DefInv | string | e | e | | [a-c] | | | | regex error | +| StrInsOpts | string | c | e | a,b,c,d,e | [a-c] | | c | true | | +| StrInsNotOptsVal | string | c | e | a,b,d,e | [a-c] | | | | valid option | +| StrInsNotOptsInv | string | c | e | a,b,d,e | [a-b] | | | | regex error | +| StrInsNotOpts | string | c | e | a,b,d,e | | | | | valid option | +| StrInsNotOpts/NoDef | string | c | | a,b,d,e | | | | | valid option | +| StrInsBadVal | string | c | | a,b,c,d,e | 1-10 | | | | min cannot | +| | | | | | | | | | | +| | list(string) | | | | | | | | | +| LStrIns | list(string) | ["c"] | | | | | ["c"] | false | | +| LStrInsDef | list(string) | ["c"] | ["e"] | | | | ["c"] | true | | +| LStrIns/DefInv | list(string) | ["c"] | ["e"] | | [a-c] | | | | regex cannot | +| LStrInsOpts | list(string) | ["c"] | ["e"] | ["c"],["d"],["e"] | | | ["c"] | true | | +| LStrInsNotOpts | list(string) | ["c"] | ["e"] | ["d"],["e"] | | | | | valid option | +| LStrInsNotOpts/NoDef | list(string) | ["c"] | | ["d"],["e"] | | | | | valid option | +| | | | | | | | | | | +| MulInsOpts | multi-select | ["c"] | ["e"] | c,d,e | | | ["c"] | true | | +| MulInsNotOpts | multi-select | ["c"] | ["e"] | d,e | | | | | valid option | +| MulInsNotOpts/NoDef | multi-select | ["c"] | | d,e | | | | | valid option | +| MulInsInvOpts | multi-select | ["c"] | ["e"] | c,d,e | [a-c] | | | | regex cannot | \ No newline at end of file From bb16d1ef20e67abda0fd2cdc46e973f94ac87c37 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 1 May 2025 14:07:59 -0500 Subject: [PATCH 09/22] fixup test cases --- provider/parameter_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/provider/parameter_test.go b/provider/parameter_test.go index a0b96cbd..eac26af1 100644 --- a/provider/parameter_test.go +++ b/provider/parameter_test.go @@ -806,6 +806,7 @@ func TestParameterValidation(t *testing.T) { Option: opts("red", "blue", "green"), FormType: provider.ParameterFormTypeMultiSelect, }, + Value: `["red", "yellow", "black"]`, ExpectError: regexp.MustCompile("is not a valid option, values \"yellow, black\" are missing from the options"), }, { @@ -835,7 +836,8 @@ func TestParameterValidation(t *testing.T) { tc := tc t.Run(tc.Name, func(t *testing.T) { t.Parallel() - diags := tc.Parameter.Valid(&tc.Value) + value := &tc.Value + diags := tc.Parameter.Valid(value) if tc.ExpectError != nil { require.True(t, diags.HasError()) errMsg := fmt.Sprintf("%+v", diags[0]) // close enough From 506f1c9764ba4efb4aac5b713c84e947b045a0db Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 1 May 2025 14:11:49 -0500 Subject: [PATCH 10/22] add nan cases --- provider/testdata/parameter_table.md | 143 ++++++++++++++------------- 1 file changed, 73 insertions(+), 70 deletions(-) diff --git a/provider/testdata/parameter_table.md b/provider/testdata/parameter_table.md index 4c6ef99c..fabc30a0 100644 --- a/provider/testdata/parameter_table.md +++ b/provider/testdata/parameter_table.md @@ -1,70 +1,73 @@ -| Name | Type | Input | Default | Options | Validation | -> | Output | Optional | Error | -|----------------------|---------------|-----------|---------|-------------------|------------|----|--------|----------|--------------| -| | Empty Vals | | | | | | | | | -| Empty | string,number | | | | | | "" | false | | -| EmptyList | list(string) | | | | | | "" | false | | -| EmptyMulti | tag-select | | | | | | "" | false | | -| EmptyOpts | string,number | | | 1,2,3 | | | "" | false | | -| EmptyRegex | string | | | | world | | | | regex error | -| EmptyMin | number | | | | 1-10 | | | | 1 < < 10 | -| EmptyMinOpt | number | | | 1,2,3 | 2-5 | | | | 2 < < 5 | -| EmptyRegexOpt | string | | | "hello","goodbye" | goodbye | | | | regex error | -| EmptyRegexOk | string | | | | .* | | "" | false | | -| | | | | | | | | | | -| | Default Set | No inputs | | | | | | | | -| NumDef | number | | 5 | | | | 5 | true | | -| NumDefVal | number | | 5 | | 3-7 | | 5 | true | | -| NumDefInv | number | | 5 | | 10- | | | | 10 < 5 < 0 | -| NumDefOpts | number | | 5 | 1,3,5,7 | 2-6 | | 5 | true | | -| NumDefNotOpts | number | | 5 | 1,3,7,9 | 2-6 | | | | valid option | -| NumDefInvOpt | number | | 5 | 1,3,5,7 | 6-10 | | | | 6 < 5 < 10 | -| | | | | | | | | | | -| StrDef | string | | hello | | | | hello | true | | -| StrDefInv | string | | hello | | world | | | | regex error | -| StrDefOpts | string | | a | a,b,c | | | a | true | | -| StrDefNotOpts | string | | a | b,c,d | | | | | valid option | -| StrDefValOpts | string | | a | a,b,c,d,e,f | [a-c] | | a | true | | -| StrDefInvOpt | string | | d | a,b,c,d,e,f | [a-c] | | | | regex error | -| | | | | | | | | | | -| LStrDef | list(string) | | ["a"] | | | | ["a"] | true | | -| LStrDefOpts | list(string) | | ["a"] | ["a"], ["b"] | | | ["a"] | true | | -| LStrDefNotOpts | list(string) | | ["a"] | ["b"], ["c"] | | | | | valid option | -| | | | | | | | | | | -| MulDef | tag-select | | ["a"] | | | | ["a"] | true | | -| MulDefOpts | multi-select | | ["a"] | a,b | | | ["a"] | true | | -| MulDefNotOpts | multi-select | | ["a"] | b,c | | | | | valid option | -| | | | | | | | | | | -| | Input Vals | | | | | | | | | -| NumIns | number | 3 | | | | | 3 | false | | -| NumInsDef | number | 3 | 5 | | | | 3 | true | | -| NumIns/DefInv | number | 3 | 5 | | 1-3 | | 3 | true | | -| NumIns=DefInv | number | 5 | 5 | | 1-3 | | | | 1 < 5 < 3 | -| NumInsOpts | number | 3 | 5 | 1,2,3,4,5 | 1-3 | | 3 | true | | -| NumInsNotOptsVal | number | 3 | 5 | 1,2,4,5 | 1-3 | | | | valid option | -| NumInsNotOptsInv | number | 3 | 5 | 1,2,4,5 | 1-2 | | | true | 1 < 3 < 2 | -| NumInsNotOpts | number | 3 | 5 | 1,2,4,5 | | | | | valid option | -| NumInsNotOpts/NoDef | number | 3 | | 1,2,4,5 | | | | | valid option | -| | | | | | | | | | | -| StrIns | string | c | | | | | c | false | | -| StrInsDef | string | c | e | | | | c | true | | -| StrIns/DefInv | string | c | e | | [a-c] | | c | true | | -| StrIns=DefInv | string | e | e | | [a-c] | | | | regex error | -| StrInsOpts | string | c | e | a,b,c,d,e | [a-c] | | c | true | | -| StrInsNotOptsVal | string | c | e | a,b,d,e | [a-c] | | | | valid option | -| StrInsNotOptsInv | string | c | e | a,b,d,e | [a-b] | | | | regex error | -| StrInsNotOpts | string | c | e | a,b,d,e | | | | | valid option | -| StrInsNotOpts/NoDef | string | c | | a,b,d,e | | | | | valid option | -| StrInsBadVal | string | c | | a,b,c,d,e | 1-10 | | | | min cannot | -| | | | | | | | | | | -| | list(string) | | | | | | | | | -| LStrIns | list(string) | ["c"] | | | | | ["c"] | false | | -| LStrInsDef | list(string) | ["c"] | ["e"] | | | | ["c"] | true | | -| LStrIns/DefInv | list(string) | ["c"] | ["e"] | | [a-c] | | | | regex cannot | -| LStrInsOpts | list(string) | ["c"] | ["e"] | ["c"],["d"],["e"] | | | ["c"] | true | | -| LStrInsNotOpts | list(string) | ["c"] | ["e"] | ["d"],["e"] | | | | | valid option | -| LStrInsNotOpts/NoDef | list(string) | ["c"] | | ["d"],["e"] | | | | | valid option | -| | | | | | | | | | | -| MulInsOpts | multi-select | ["c"] | ["e"] | c,d,e | | | ["c"] | true | | -| MulInsNotOpts | multi-select | ["c"] | ["e"] | d,e | | | | | valid option | -| MulInsNotOpts/NoDef | multi-select | ["c"] | | d,e | | | | | valid option | -| MulInsInvOpts | multi-select | ["c"] | ["e"] | c,d,e | [a-c] | | | | regex cannot | \ No newline at end of file +| Name | Type | Input | Default | Options | Validation | -> | Output | Optional | Error | +|----------------------|---------------|-----------|---------|-------------------|------------|----|--------|----------|---------------| +| | Empty Vals | | | | | | | | | +| Empty | string,number | | | | | | "" | false | | +| EmptyList | list(string) | | | | | | "" | false | | +| EmptyMulti | tag-select | | | | | | "" | false | | +| EmptyOpts | string,number | | | 1,2,3 | | | "" | false | | +| EmptyRegex | string | | | | world | | | | regex error | +| EmptyMin | number | | | | 1-10 | | | | 1 < < 10 | +| EmptyMinOpt | number | | | 1,2,3 | 2-5 | | | | 2 < < 5 | +| EmptyRegexOpt | string | | | "hello","goodbye" | goodbye | | | | regex error | +| EmptyRegexOk | string | | | | .* | | "" | false | | +| | | | | | | | | | | +| | Default Set | No inputs | | | | | | | | +| NumDef | number | | 5 | | | | 5 | true | | +| NumDefNaN | number | | a | | | | | | type "number" | +| NumDefVal | number | | 5 | | 3-7 | | 5 | true | | +| NumDefInv | number | | 5 | | 10- | | | | 10 < 5 < 0 | +| NumDefOpts | number | | 5 | 1,3,5,7 | 2-6 | | 5 | true | | +| NumDefNotOpts | number | | 5 | 1,3,7,9 | 2-6 | | | | valid option | +| NumDefInvOpt | number | | 5 | 1,3,5,7 | 6-10 | | | | 6 < 5 < 10 | +| | | | | | | | | | | +| StrDef | string | | hello | | | | hello | true | | +| StrDefInv | string | | hello | | world | | | | regex error | +| StrDefOpts | string | | a | a,b,c | | | a | true | | +| StrDefNotOpts | string | | a | b,c,d | | | | | valid option | +| StrDefValOpts | string | | a | a,b,c,d,e,f | [a-c] | | a | true | | +| StrDefInvOpt | string | | d | a,b,c,d,e,f | [a-c] | | | | regex error | +| | | | | | | | | | | +| LStrDef | list(string) | | ["a"] | | | | ["a"] | true | | +| LStrDefOpts | list(string) | | ["a"] | ["a"], ["b"] | | | ["a"] | true | | +| LStrDefNotOpts | list(string) | | ["a"] | ["b"], ["c"] | | | | | valid option | +| | | | | | | | | | | +| MulDef | tag-select | | ["a"] | | | | ["a"] | true | | +| MulDefOpts | multi-select | | ["a"] | a,b | | | ["a"] | true | | +| MulDefNotOpts | multi-select | | ["a"] | b,c | | | | | valid option | +| | | | | | | | | | | +| | Input Vals | | | | | | | | | +| NumIns | number | 3 | | | | | 3 | false | | +| NumInsOptsNaN | number | 3 | 5 | a,1,2,3,4,5 | 1-3 | | 3 | true | type "number" | +| NumInsNaN | number | a | | | | | | | type "number" | +| NumInsDef | number | 3 | 5 | | | | 3 | true | | +| NumIns/DefInv | number | 3 | 5 | | 1-3 | | 3 | true | | +| NumIns=DefInv | number | 5 | 5 | | 1-3 | | | | 1 < 5 < 3 | +| NumInsOpts | number | 3 | 5 | 1,2,3,4,5 | 1-3 | | 3 | true | | +| NumInsNotOptsVal | number | 3 | 5 | 1,2,4,5 | 1-3 | | | | valid option | +| NumInsNotOptsInv | number | 3 | 5 | 1,2,4,5 | 1-2 | | | true | 1 < 3 < 2 | +| NumInsNotOpts | number | 3 | 5 | 1,2,4,5 | | | | | valid option | +| NumInsNotOpts/NoDef | number | 3 | | 1,2,4,5 | | | | | valid option | +| | | | | | | | | | | +| StrIns | string | c | | | | | c | false | | +| StrInsDef | string | c | e | | | | c | true | | +| StrIns/DefInv | string | c | e | | [a-c] | | c | true | | +| StrIns=DefInv | string | e | e | | [a-c] | | | | regex error | +| StrInsOpts | string | c | e | a,b,c,d,e | [a-c] | | c | true | | +| StrInsNotOptsVal | string | c | e | a,b,d,e | [a-c] | | | | valid option | +| StrInsNotOptsInv | string | c | e | a,b,d,e | [a-b] | | | | regex error | +| StrInsNotOpts | string | c | e | a,b,d,e | | | | | valid option | +| StrInsNotOpts/NoDef | string | c | | a,b,d,e | | | | | valid option | +| StrInsBadVal | string | c | | a,b,c,d,e | 1-10 | | | | min cannot | +| | | | | | | | | | | +| | list(string) | | | | | | | | | +| LStrIns | list(string) | ["c"] | | | | | ["c"] | false | | +| LStrInsDef | list(string) | ["c"] | ["e"] | | | | ["c"] | true | | +| LStrIns/DefInv | list(string) | ["c"] | ["e"] | | [a-c] | | | | regex cannot | +| LStrInsOpts | list(string) | ["c"] | ["e"] | ["c"],["d"],["e"] | | | ["c"] | true | | +| LStrInsNotOpts | list(string) | ["c"] | ["e"] | ["d"],["e"] | | | | | valid option | +| LStrInsNotOpts/NoDef | list(string) | ["c"] | | ["d"],["e"] | | | | | valid option | +| | | | | | | | | | | +| MulInsOpts | multi-select | ["c"] | ["e"] | c,d,e | | | ["c"] | true | | +| MulInsNotOpts | multi-select | ["c"] | ["e"] | d,e | | | | | valid option | +| MulInsNotOpts/NoDef | multi-select | ["c"] | | d,e | | | | | valid option | +| MulInsInvOpts | multi-select | ["c"] | ["e"] | c,d,e | [a-c] | | | | regex cannot | \ No newline at end of file From fe4b4d7fe7fcb0bdb54dc6641fe11fa24d0f9fea Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 1 May 2025 14:21:15 -0500 Subject: [PATCH 11/22] remove comment --- provider/parameter_test.go | 4 - provider/testdata/parameter_table.md | 160 +++++++++++++-------------- 2 files changed, 80 insertions(+), 84 deletions(-) diff --git a/provider/parameter_test.go b/provider/parameter_test.go index 84339d76..ea44b2dc 100644 --- a/provider/parameter_test.go +++ b/provider/parameter_test.go @@ -866,10 +866,6 @@ func TestParameterValidationEnforcement(t *testing.T) { // - Validation logic does not apply to the default if a value is given // - [NumIns/DefInv] So the default can be invalid if an input value is valid. // The value is therefore not really optional, but it is marked as such. - // - [NumInsNotOptsVal | NumsInsNotOpts] values do not need to be in the option set? - // - [NumInsNotNum] number params do not require the value to be a number - // - [LStrInsNotList] list(string) do not require the value to be a list(string) - // - Same with [MulInsNotListOpts] table, err := os.ReadFile("testdata/parameter_table.md") require.NoError(t, err) diff --git a/provider/testdata/parameter_table.md b/provider/testdata/parameter_table.md index c9777801..08858bfe 100644 --- a/provider/testdata/parameter_table.md +++ b/provider/testdata/parameter_table.md @@ -1,80 +1,80 @@ -| Name | Type | Input | Default | Options | Validation | -> | Output | Optional | Error | -|----------------------|---------------|-----------|---------|-------------------|------------|----|--------|----------|---------------| -| | Empty Vals | | | | | | | | | -| Empty | string,number | | | | | | "" | false | | -| EmptyDupeOps | string,number | | | 1,1,1 | | | | | unique | -| EmptyList | list(string) | | | | | | "" | false | | -| EmptyListDupeOpts | list(string) | | | ["a"],["a"] | | | | | unique | -| EmptyMulti | tag-select | | | | | | "" | false | | -| EmptyOpts | string,number | | | 1,2,3 | | | "" | false | | -| EmptyRegex | string | | | | world | | | | regex error | -| EmptyMin | number | | | | 1-10 | | | | 1 < < 10 | -| EmptyMinOpt | number | | | 1,2,3 | 2-5 | | | | 2 < < 5 | -| EmptyRegexOpt | string | | | "hello","goodbye" | goodbye | | | | regex error | -| EmptyRegexOk | string | | | | .* | | "" | false | | -| | | | | | | | | | | -| | Default Set | No inputs | | | | | | | | -| NumDef | number | | 5 | | | | 5 | true | | -| NumDefVal | number | | 5 | | 3-7 | | 5 | true | | -| NumDefInv | number | | 5 | | 10- | | | | 10 < 5 < 0 | -| NumDefOpts | number | | 5 | 1,3,5,7 | 2-6 | | 5 | true | | -| NumDefNotOpts | number | | 5 | 1,3,7,9 | 2-6 | | | | valid option | -| NumDefInvOpt | number | | 5 | 1,3,5,7 | 6-10 | | | | 6 < 5 < 10 | -| NumDefNotNum | number | | a | | | | | | type "number" | -| NumDefOptsNotNum | number | | 1 | 1,a,2 | | | | | type "number" | -| | | | | | | | | | | -| StrDef | string | | hello | | | | hello | true | | -| StrDefInv | string | | hello | | world | | | | regex error | -| StrDefOpts | string | | a | a,b,c | | | a | true | | -| StrDefNotOpts | string | | a | b,c,d | | | | | valid option | -| StrDefValOpts | string | | a | a,b,c,d,e,f | [a-c] | | a | true | | -| StrDefInvOpt | string | | d | a,b,c,d,e,f | [a-c] | | | | regex error | -| | | | | | | | | | | -| LStrDef | list(string) | | ["a"] | | | | ["a"] | true | | -| LStrDefOpts | list(string) | | ["a"] | ["a"], ["b"] | | | ["a"] | true | | -| LStrDefNotOpts | list(string) | | ["a"] | ["b"], ["c"] | | | | | valid option | -| | | | | | | | | | | -| MulDef | tag-select | | ["a"] | | | | ["a"] | true | | -| MulDefOpts | multi-select | | ["a"] | a,b | | | ["a"] | true | | -| MulDefNotOpts | multi-select | | ["a"] | b,c | | | | | valid option | -| | | | | | | | | | | -| | Input Vals | | | | | | | | | -| NumIns | number | 3 | | | | | 3 | false | | -| NumInsOptsNaN | number | 3 | 5 | a,1,2,3,4,5 | 1-3 | | 3 | true | type "number" | -| NumInsNotNum | number | a | | | | | | | type "number" | -| NumInsNotNumInv | number | a | | | 1-3 | | | | 1 < a < 3 | -| NumInsDef | number | 3 | 5 | | | | 3 | true | | -| NumIns/DefInv | number | 3 | 5 | | 1-3 | | 3 | true | | -| NumIns=DefInv | number | 5 | 5 | | 1-3 | | | | 1 < 5 < 3 | -| NumInsOpts | number | 3 | 5 | 1,2,3,4,5 | 1-3 | | 3 | true | | -| NumInsNotOptsVal | number | 3 | 5 | 1,2,4,5 | 1-3 | | | | valid option | -| NumInsNotOptsInv | number | 3 | 5 | 1,2,4,5 | 1-2 | | | true | 1 < 3 < 2 | -| NumInsNotOpts | number | 3 | 5 | 1,2,4,5 | | | | | valid option | -| NumInsNotOpts/NoDef | number | 3 | | 1,2,4,5 | | | | | valid option | -| | | | | | | | | | | -| StrIns | string | c | | | | | c | false | | -| StrInsDupeOpts | string | c | | a,b,c,c | | | | | unique | -| StrInsDef | string | c | e | | | | c | true | | -| StrIns/DefInv | string | c | e | | [a-c] | | c | true | | -| StrIns=DefInv | string | e | e | | [a-c] | | | | regex error | -| StrInsOpts | string | c | e | a,b,c,d,e | [a-c] | | c | true | | -| StrInsNotOptsVal | string | c | e | a,b,d,e | [a-c] | | | | valid option | -| StrInsNotOptsInv | string | c | e | a,b,d,e | [a-b] | | | | regex error | -| StrInsNotOpts | string | c | e | a,b,d,e | | | | | valid option | -| StrInsNotOpts/NoDef | string | c | | a,b,d,e | | | | | valid option | -| StrInsBadVal | string | c | | a,b,c,d,e | 1-10 | | | | min cannot | -| | | | | | | | | | | -| | list(string) | | | | | | | | | -| LStrIns | list(string) | ["c"] | | | | | ["c"] | false | | -| LStrInsNotList | list(string) | c | | | | | c | false | | -| LStrInsDef | list(string) | ["c"] | ["e"] | | | | ["c"] | true | | -| LStrIns/DefInv | list(string) | ["c"] | ["e"] | | [a-c] | | | | regex cannot | -| LStrInsOpts | list(string) | ["c"] | ["e"] | ["c"],["d"],["e"] | | | ["c"] | true | | -| LStrInsNotOpts | list(string) | ["c"] | ["e"] | ["d"],["e"] | | | | | valid option | -| LStrInsNotOpts/NoDef | list(string) | ["c"] | | ["d"],["e"] | | | | | valid option | -| | | | | | | | | | | -| MulInsOpts | multi-select | ["c"] | ["e"] | c,d,e | | | ["c"] | true | | -| MulInsNotListOpts | multi-select | c | ["e"] | c,d,e | | | c | true | | -| MulInsNotOpts | multi-select | ["c"] | ["e"] | d,e | | | | | valid option | -| MulInsNotOpts/NoDef | multi-select | ["c"] | | d,e | | | | | valid option | -| MulInsInvOpts | multi-select | ["c"] | ["e"] | c,d,e | [a-c] | | | | regex cannot | \ No newline at end of file +| Name | Type | Input | Default | Options | Validation | -> | Output | Optional | Error | +|----------------------|---------------|-----------|---------|-------------------|------------|----|--------|----------|-----------------| +| | Empty Vals | | | | | | | | | +| Empty | string,number | | | | | | "" | false | | +| EmptyDupeOps | string,number | | | 1,1,1 | | | | | unique | +| EmptyList | list(string) | | | | | | "" | false | | +| EmptyListDupeOpts | list(string) | | | ["a"],["a"] | | | | | unique | +| EmptyMulti | tag-select | | | | | | "" | false | | +| EmptyOpts | string,number | | | 1,2,3 | | | "" | false | | +| EmptyRegex | string | | | | world | | | | regex error | +| EmptyMin | number | | | | 1-10 | | | | 1 < < 10 | +| EmptyMinOpt | number | | | 1,2,3 | 2-5 | | | | 2 < < 5 | +| EmptyRegexOpt | string | | | "hello","goodbye" | goodbye | | | | regex error | +| EmptyRegexOk | string | | | | .* | | "" | false | | +| | | | | | | | | | | +| | Default Set | No inputs | | | | | | | | +| NumDef | number | | 5 | | | | 5 | true | | +| NumDefVal | number | | 5 | | 3-7 | | 5 | true | | +| NumDefInv | number | | 5 | | 10- | | | | 10 < 5 < 0 | +| NumDefOpts | number | | 5 | 1,3,5,7 | 2-6 | | 5 | true | | +| NumDefNotOpts | number | | 5 | 1,3,7,9 | 2-6 | | | | valid option | +| NumDefInvOpt | number | | 5 | 1,3,5,7 | 6-10 | | | | 6 < 5 < 10 | +| NumDefNotNum | number | | a | | | | | | type "number" | +| NumDefOptsNotNum | number | | 1 | 1,a,2 | | | | | type "number" | +| | | | | | | | | | | +| StrDef | string | | hello | | | | hello | true | | +| StrDefInv | string | | hello | | world | | | | regex error | +| StrDefOpts | string | | a | a,b,c | | | a | true | | +| StrDefNotOpts | string | | a | b,c,d | | | | | valid option | +| StrDefValOpts | string | | a | a,b,c,d,e,f | [a-c] | | a | true | | +| StrDefInvOpt | string | | d | a,b,c,d,e,f | [a-c] | | | | regex error | +| | | | | | | | | | | +| LStrDef | list(string) | | ["a"] | | | | ["a"] | true | | +| LStrDefOpts | list(string) | | ["a"] | ["a"], ["b"] | | | ["a"] | true | | +| LStrDefNotOpts | list(string) | | ["a"] | ["b"], ["c"] | | | | | valid option | +| | | | | | | | | | | +| MulDef | tag-select | | ["a"] | | | | ["a"] | true | | +| MulDefOpts | multi-select | | ["a"] | a,b | | | ["a"] | true | | +| MulDefNotOpts | multi-select | | ["a"] | b,c | | | | | valid option | +| | | | | | | | | | | +| | Input Vals | | | | | | | | | +| NumIns | number | 3 | | | | | 3 | false | | +| NumInsOptsNaN | number | 3 | 5 | a,1,2,3,4,5 | 1-3 | | | | type "number" | +| NumInsNotNum | number | a | | | | | | | type "number" | +| NumInsNotNumInv | number | a | | | 1-3 | | | | 1 < a < 3 | +| NumInsDef | number | 3 | 5 | | | | 3 | true | | +| NumIns/DefInv | number | 3 | 5 | | 1-3 | | 3 | true | | +| NumIns=DefInv | number | 5 | 5 | | 1-3 | | | | 1 < 5 < 3 | +| NumInsOpts | number | 3 | 5 | 1,2,3,4,5 | 1-3 | | 3 | true | | +| NumInsNotOptsVal | number | 3 | 5 | 1,2,4,5 | 1-3 | | | | valid option | +| NumInsNotOptsInv | number | 3 | 5 | 1,2,4,5 | 1-2 | | | true | 1 < 3 < 2 | +| NumInsNotOpts | number | 3 | 5 | 1,2,4,5 | | | | | valid option | +| NumInsNotOpts/NoDef | number | 3 | | 1,2,4,5 | | | | | valid option | +| | | | | | | | | | | +| StrIns | string | c | | | | | c | false | | +| StrInsDupeOpts | string | c | | a,b,c,c | | | | | unique | +| StrInsDef | string | c | e | | | | c | true | | +| StrIns/DefInv | string | c | e | | [a-c] | | c | true | | +| StrIns=DefInv | string | e | e | | [a-c] | | | | regex error | +| StrInsOpts | string | c | e | a,b,c,d,e | [a-c] | | c | true | | +| StrInsNotOptsVal | string | c | e | a,b,d,e | [a-c] | | | | valid option | +| StrInsNotOptsInv | string | c | e | a,b,d,e | [a-b] | | | | regex error | +| StrInsNotOpts | string | c | e | a,b,d,e | | | | | valid option | +| StrInsNotOpts/NoDef | string | c | | a,b,d,e | | | | | valid option | +| StrInsBadVal | string | c | | a,b,c,d,e | 1-10 | | | | min cannot | +| | | | | | | | | | | +| | list(string) | | | | | | | | | +| LStrIns | list(string) | ["c"] | | | | | ["c"] | false | | +| LStrInsNotList | list(string) | c | | | | | | | list of strings | +| LStrInsDef | list(string) | ["c"] | ["e"] | | | | ["c"] | true | | +| LStrIns/DefInv | list(string) | ["c"] | ["e"] | | [a-c] | | | | regex cannot | +| LStrInsOpts | list(string) | ["c"] | ["e"] | ["c"],["d"],["e"] | | | ["c"] | true | | +| LStrInsNotOpts | list(string) | ["c"] | ["e"] | ["d"],["e"] | | | | | valid option | +| LStrInsNotOpts/NoDef | list(string) | ["c"] | | ["d"],["e"] | | | | | valid option | +| | | | | | | | | | | +| MulInsOpts | multi-select | ["c"] | ["e"] | c,d,e | | | ["c"] | true | | +| MulInsNotListOpts | multi-select | c | ["e"] | c,d,e | | | | | json encoded | +| MulInsNotOpts | multi-select | ["c"] | ["e"] | d,e | | | | | valid option | +| MulInsNotOpts/NoDef | multi-select | ["c"] | | d,e | | | | | valid option | +| MulInsInvOpts | multi-select | ["c"] | ["e"] | c,d,e | [a-c] | | | | regex cannot | \ No newline at end of file From 0ad4eb660c5d5b381e524581f484aff17193448a Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 1 May 2025 14:43:47 -0500 Subject: [PATCH 12/22] chore: distinguish unset vs set default and values --- provider/parameter.go | 139 +++++++++++++++++++++++-------------- provider/parameter_test.go | 14 ++-- 2 files changed, 97 insertions(+), 56 deletions(-) diff --git a/provider/parameter.go b/provider/parameter.go index a72b7a91..b035c1b4 100644 --- a/provider/parameter.go +++ b/provider/parameter.go @@ -21,6 +21,15 @@ import ( "golang.org/x/xerrors" ) +type ValidationMode string + +const ( + // ValidationModeDefault is used for creating a workspace. It validates the + // final value used for a parameter. + ValidationModeDefault ValidationMode = "" + ValidationModeTemplateImport ValidationMode = "template-import" +) + var ( defaultValuePath = cty.Path{cty.GetAttrStep{Name: "default"}} ) @@ -56,7 +65,7 @@ type Parameter struct { Type OptionType FormType ParameterFormType Mutable bool - Default string + Default *string Icon string Option []Option Validation []Validation @@ -105,10 +114,16 @@ func parameterDataSource() *schema.Resource { Type: rd.Get("type"), FormType: rd.Get("form_type"), Mutable: rd.Get("mutable"), - Default: rd.Get("default"), - Icon: rd.Get("icon"), - Option: rd.Get("option"), - Validation: fixedValidation, + Default: func() *string { + if rd.GetRawConfig().AsValueMap()["default"].IsNull() { + return nil + } + val, _ := rd.Get("default").(string) + return &val + }(), + Icon: rd.Get("icon"), + Option: rd.Get("option"), + Validation: fixedValidation, Optional: func() bool { // This hack allows for checking if the "default" field is present in the .tf file. // If "default" is missing or is "null", then it means that this field is required, @@ -124,25 +139,6 @@ func parameterDataSource() *schema.Resource { return diag.Errorf("decode parameter: %s", err) } - var value *string - if !rd.GetRawConfig().AsValueMap()["default"].IsNull() { - value = ¶meter.Default - } - - envValue, ok := os.LookupEnv(ParameterEnvironmentVariable(parameter.Name)) - if ok { - value = &envValue - } - - if value != nil { - rd.Set("value", value) - } else { - // Maintaining backwards compatibility. The previous behavior was - // to write back an empty string. - // TODO: Should an empty string exist if no value is set? - rd.Set("value", "") - } - if !parameter.Mutable && parameter.Ephemeral { return diag.Errorf("parameter can't be immutable and ephemeral") } @@ -151,10 +147,17 @@ func parameterDataSource() *schema.Resource { return diag.Errorf("ephemeral parameter requires the default property") } - diags := parameter.Valid(value) + var input *string + envValue, ok := os.LookupEnv(ParameterEnvironmentVariable(parameter.Name)) + if ok { + input = &envValue + } + + value, diags := parameter.Valid(input, ValidationModeDefault) if diags.HasError() { return diags } + rd.Set("value", value) // Set the form_type as it could have changed in the validation. rd.Set("form_type", parameter.FormType) @@ -397,15 +400,20 @@ func valueIsType(typ OptionType, value string) error { return nil } -func (v *Parameter) Valid(value *string) diag.Diagnostics { +func (v *Parameter) Valid(input *string, mode ValidationMode) (string, diag.Diagnostics) { var err error var optionType OptionType + value := input + if input == nil { + value = v.Default + } + // optionType might differ from parameter.Type. This is ok, and parameter.Type // should be used for the value type, and optionType for options. optionType, v.FormType, err = ValidateFormType(v.Type, len(v.Option), v.FormType) if err != nil { - return diag.Diagnostics{ + return "", diag.Diagnostics{ { Severity: diag.Error, Summary: "Invalid form_type for parameter", @@ -417,28 +425,28 @@ func (v *Parameter) Valid(value *string) diag.Diagnostics { optionValues, diags := v.ValidOptions(optionType) if diags.HasError() { - return diags + return "", diags } - // TODO: The default value should also be validated - //if v.Default != "" { - // err := valueIsType(v.Type, v.Default) - // if err != nil { - // return diag.Diagnostics{ - // { - // Severity: diag.Error, - // Summary: fmt.Sprintf("Default value is not of type %q", v.Type), - // Detail: err.Error(), - // AttributePath: defaultValuePath, - // }, - // } - // } - // - // d := v.validValue(v.Default, optionType, optionValues, defaultValuePath) - // if d.HasError() { - // return d - // } - //} + if mode == ValidationModeTemplateImport && v.Default != nil { + // Template import should validate the default value. + err := valueIsType(v.Type, *v.Default) + if err != nil { + return "", diag.Diagnostics{ + { + Severity: diag.Error, + Summary: fmt.Sprintf("Default value is not of type %q", v.Type), + Detail: err.Error(), + AttributePath: defaultValuePath, + }, + } + } + + d := v.validValue(*v.Default, optionType, optionValues, defaultValuePath) + if d.HasError() { + return "", d + } + } // TODO: Move this into 'Parameter.validValue'. It exists as another check outside because // 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 { validCheck := &v.Validation[0] err := validCheck.Valid(v.Type, *validVal) if err != nil { - return diag.Diagnostics{ + return "", diag.Diagnostics{ { Severity: diag.Error, 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 { } } + // TODO: This is a bit of a hack. The current behavior states if validation + // is given, then apply validation to unset values. + // This should be removed, and all values should be validated. Meaning + // value == nil should not be accepted in the first place. + if len(v.Validation) > 0 && value == nil { + empty := "" + value = &empty + } + // Value is only validated if it is set. If it is unset, validation // is skipped. if value != nil { d := v.validValue(*value, optionType, optionValues, cty.Path{}) if d.HasError() { - return d + return "", d } err = valueIsType(v.Type, *value) if err != nil { - return diag.Diagnostics{ + return "", diag.Diagnostics{ { Severity: diag.Error, Summary: fmt.Sprintf("Parameter value is not of type %q", v.Type), @@ -484,7 +501,12 @@ func (v *Parameter) Valid(value *string) diag.Diagnostics { } } - return nil + if value == nil { + // The previous behavior is to always write an empty string + return "", nil + } + + return *value, nil } func (v *Parameter) ValidOptions(optionType OptionType) (map[string]struct{}, diag.Diagnostics) { @@ -598,6 +620,21 @@ func (v *Parameter) validValue(value string, optionType OptionType, optionValues } } + if len(v.Validation) == 1 { + validCheck := &v.Validation[0] + err := validCheck.Valid(v.Type, value) + if err != nil { + return diag.Diagnostics{ + { + Severity: diag.Error, + Summary: fmt.Sprintf("Invalid parameter %s according to 'validation' block", strings.ToLower(v.Name)), + Detail: err.Error(), + AttributePath: cty.Path{}, + }, + } + } + } + return nil } diff --git a/provider/parameter_test.go b/provider/parameter_test.go index ea44b2dc..1196f31c 100644 --- a/provider/parameter_test.go +++ b/provider/parameter_test.go @@ -786,7 +786,7 @@ func TestParameterValidation(t *testing.T) { Name: "NotListStringDefault", Parameter: provider.Parameter{ Type: "list(string)", - Default: "not-a-list", + Default: ptr("not-a-list"), }, ExpectError: regexp.MustCompile("not a valid list of strings"), }, @@ -802,7 +802,7 @@ func TestParameterValidation(t *testing.T) { Name: "DefaultListStringNotInOptions", Parameter: provider.Parameter{ Type: "list(string)", - Default: `["red", "yellow", "black"]`, + Default: ptr(`["red", "yellow", "black"]`), Option: opts("red", "blue", "green"), FormType: provider.ParameterFormTypeMultiSelect, }, @@ -813,7 +813,7 @@ func TestParameterValidation(t *testing.T) { Name: "ListStringNotInOptions", Parameter: provider.Parameter{ Type: "list(string)", - Default: `["red"]`, + Default: ptr(`["red"]`), Option: opts("red", "blue", "green"), FormType: provider.ParameterFormTypeMultiSelect, }, @@ -824,7 +824,7 @@ func TestParameterValidation(t *testing.T) { Name: "InvalidMiniumum", Parameter: provider.Parameter{ Type: "number", - Default: "5", + Default: ptr("5"), Validation: []provider.Validation{{ Min: 10, Error: "must be greater than 10", @@ -837,7 +837,7 @@ func TestParameterValidation(t *testing.T) { t.Run(tc.Name, func(t *testing.T) { t.Parallel() value := &tc.Value - diags := tc.Parameter.Valid(value) + _, diags := tc.Parameter.Valid(value, provider.ValidationModeDefault) if tc.ExpectError != nil { require.True(t, diags.HasError()) errMsg := fmt.Sprintf("%+v", diags[0]) // close enough @@ -1255,3 +1255,7 @@ func TestParameterWithManyOptions(t *testing.T) { }}, }) } + +func ptr[T any](v T) *T { + return &v +} From 62f36ea6f5177f378dec0dbee4c3338e4547c1ce Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 1 May 2025 15:08:46 -0500 Subject: [PATCH 13/22] add 2 validation modes --- provider/parameter.go | 25 +++-- provider/parameter_test.go | 101 +++++++++++------ provider/testdata/parameter_table.md | 160 +++++++++++++-------------- 3 files changed, 163 insertions(+), 123 deletions(-) diff --git a/provider/parameter.go b/provider/parameter.go index b035c1b4..2e19bc43 100644 --- a/provider/parameter.go +++ b/provider/parameter.go @@ -54,6 +54,7 @@ type Validation struct { } const ( + ValidationModeEnvVar = "CODER_VALIDATION_MODE" ValidationMonotonicIncreasing = "increasing" ValidationMonotonicDecreasing = "decreasing" ) @@ -153,10 +154,13 @@ func parameterDataSource() *schema.Resource { input = &envValue } - value, diags := parameter.Valid(input, ValidationModeDefault) + mode := os.Getenv(ValidationModeEnvVar) + value, diags := parameter.Valid(input, ValidationMode(mode)) if diags.HasError() { return diags } + + // Always set back the value, as it can be sourced from the default rd.Set("value", value) // Set the form_type as it could have changed in the validation. @@ -423,7 +427,7 @@ func (v *Parameter) Valid(input *string, mode ValidationMode) (string, diag.Diag } } - optionValues, diags := v.ValidOptions(optionType) + optionValues, diags := v.ValidOptions(optionType, mode) if diags.HasError() { return "", diags } @@ -509,7 +513,7 @@ func (v *Parameter) Valid(input *string, mode ValidationMode) (string, diag.Diag return *value, nil } -func (v *Parameter) ValidOptions(optionType OptionType) (map[string]struct{}, diag.Diagnostics) { +func (v *Parameter) ValidOptions(optionType OptionType, mode ValidationMode) (map[string]struct{}, diag.Diagnostics) { optionNames := map[string]struct{}{} optionValues := map[string]struct{}{} @@ -545,8 +549,15 @@ func (v *Parameter) ValidOptions(optionType OptionType) (map[string]struct{}, di optionValues[option.Value] = struct{}{} optionNames[option.Name] = struct{}{} - // TODO: Option values should also be validated. - // v.validValue(option.Value, optionType, nil, cty.Path{}) + if mode == ValidationModeTemplateImport { + opDiags := v.validValue(option.Value, optionType, nil, cty.Path{}) + if opDiags.HasError() { + for i := range opDiags { + opDiags[i].Summary = fmt.Sprintf("Option %q: %s", option.Name, opDiags[i].Summary) + } + diags = append(diags, opDiags...) + } + } } if diags.HasError() { @@ -627,9 +638,9 @@ func (v *Parameter) validValue(value string, optionType OptionType, optionValues return diag.Diagnostics{ { Severity: diag.Error, - Summary: fmt.Sprintf("Invalid parameter %s according to 'validation' block", strings.ToLower(v.Name)), + Summary: fmt.Sprintf("Invalid parameter %s according to 'validation' block", strings.ToLower(name)), Detail: err.Error(), - AttributePath: cty.Path{}, + AttributePath: path, }, } } diff --git a/provider/parameter_test.go b/provider/parameter_test.go index 1196f31c..b2558cb5 100644 --- a/provider/parameter_test.go +++ b/provider/parameter_test.go @@ -878,7 +878,8 @@ func TestParameterValidationEnforcement(t *testing.T) { Validation *provider.Validation OutputValue string Optional bool - Error *regexp.Regexp + CreateError *regexp.Regexp + ImportError *regexp.Regexp } rows := make([]row, 0) @@ -909,6 +910,19 @@ func TestParameterValidationEnforcement(t *testing.T) { t.Fatalf("failed to parse error column %q: %v", columns[9], err) } } + + var imerr *regexp.Regexp + if columns[10] != "" { + if columns[10] == "=" { + imerr = rerr + } else { + imerr, err = regexp.Compile(columns[10]) + if err != nil { + t.Fatalf("failed to parse error column %q: %v", columns[10], err) + } + } + } + var options []string if columns[4] != "" { options = strings.Split(columns[4], ",") @@ -955,7 +969,8 @@ func TestParameterValidationEnforcement(t *testing.T) { Validation: validation, OutputValue: columns[7], Optional: optional, - Error: rerr, + CreateError: rerr, + ImportError: imerr, }) } @@ -974,9 +989,9 @@ func TestParameterValidationEnforcement(t *testing.T) { t.Setenv(provider.ParameterEnvironmentVariable("parameter"), row.InputValue) } - if row.Error != nil { + if row.CreateError != nil && row.ImportError != nil { if row.OutputValue != "" { - t.Errorf("output value %q should not be set if error is set", row.OutputValue) + t.Errorf("output value %q should not be set if both errors are set", row.OutputValue) } } @@ -1020,42 +1035,56 @@ func TestParameterValidationEnforcement(t *testing.T) { cfg.WriteString("}\n") - resource.Test(t, resource.TestCase{ - ProviderFactories: coderFactory(), - IsUnitTest: true, - Steps: []resource.TestStep{{ - Config: cfg.String(), - ExpectError: row.Error, - Check: func(state *terraform.State) error { - require.Len(t, state.Modules, 1) - require.Len(t, state.Modules[0].Resources, 1) - param := state.Modules[0].Resources["data.coder_parameter.parameter"] - require.NotNil(t, param) + for _, mode := range []provider.ValidationMode{provider.ValidationModeDefault, provider.ValidationModeTemplateImport} { + name := string(mode) + if mode == provider.ValidationModeDefault { + name = "create" + } + t.Run(name, func(t *testing.T) { + t.Setenv("CODER_VALIDATION_MODE", string(mode)) + rerr := row.CreateError + if mode == provider.ValidationModeTemplateImport { + rerr = row.ImportError + } - if row.Default == "" { - _, ok := param.Primary.Attributes["default"] - require.False(t, ok, "default should not be set") - } else { - require.Equal(t, strings.Trim(row.Default, `"`), param.Primary.Attributes["default"]) - } + resource.Test(t, resource.TestCase{ + ProviderFactories: coderFactory(), + IsUnitTest: true, + Steps: []resource.TestStep{{ + Config: cfg.String(), + ExpectError: rerr, + Check: func(state *terraform.State) error { + require.Len(t, state.Modules, 1) + require.Len(t, state.Modules[0].Resources, 1) + param := state.Modules[0].Resources["data.coder_parameter.parameter"] + require.NotNil(t, param) - if row.OutputValue == "" { - _, ok := param.Primary.Attributes["value"] - require.False(t, ok, "output value should not be set") - } else { - require.Equal(t, strings.Trim(row.OutputValue, `"`), param.Primary.Attributes["value"]) - } + if row.Default == "" { + _, ok := param.Primary.Attributes["default"] + require.False(t, ok, "default should not be set") + } else { + require.Equal(t, strings.Trim(row.Default, `"`), param.Primary.Attributes["default"]) + } - for key, expected := range map[string]string{ - "optional": strconv.FormatBool(row.Optional), - } { - require.Equal(t, expected, param.Primary.Attributes[key], "optional") - } + if row.OutputValue == "" { + _, ok := param.Primary.Attributes["value"] + require.False(t, ok, "output value should not be set") + } else { + require.Equal(t, strings.Trim(row.OutputValue, `"`), param.Primary.Attributes["value"]) + } - return nil - }, - }}, - }) + for key, expected := range map[string]string{ + "optional": strconv.FormatBool(row.Optional), + } { + require.Equal(t, expected, param.Primary.Attributes[key], "optional") + } + + return nil + }, + }}, + }) + }) + } }) } } diff --git a/provider/testdata/parameter_table.md b/provider/testdata/parameter_table.md index 08858bfe..5049f071 100644 --- a/provider/testdata/parameter_table.md +++ b/provider/testdata/parameter_table.md @@ -1,80 +1,80 @@ -| Name | Type | Input | Default | Options | Validation | -> | Output | Optional | Error | -|----------------------|---------------|-----------|---------|-------------------|------------|----|--------|----------|-----------------| -| | Empty Vals | | | | | | | | | -| Empty | string,number | | | | | | "" | false | | -| EmptyDupeOps | string,number | | | 1,1,1 | | | | | unique | -| EmptyList | list(string) | | | | | | "" | false | | -| EmptyListDupeOpts | list(string) | | | ["a"],["a"] | | | | | unique | -| EmptyMulti | tag-select | | | | | | "" | false | | -| EmptyOpts | string,number | | | 1,2,3 | | | "" | false | | -| EmptyRegex | string | | | | world | | | | regex error | -| EmptyMin | number | | | | 1-10 | | | | 1 < < 10 | -| EmptyMinOpt | number | | | 1,2,3 | 2-5 | | | | 2 < < 5 | -| EmptyRegexOpt | string | | | "hello","goodbye" | goodbye | | | | regex error | -| EmptyRegexOk | string | | | | .* | | "" | false | | -| | | | | | | | | | | -| | Default Set | No inputs | | | | | | | | -| NumDef | number | | 5 | | | | 5 | true | | -| NumDefVal | number | | 5 | | 3-7 | | 5 | true | | -| NumDefInv | number | | 5 | | 10- | | | | 10 < 5 < 0 | -| NumDefOpts | number | | 5 | 1,3,5,7 | 2-6 | | 5 | true | | -| NumDefNotOpts | number | | 5 | 1,3,7,9 | 2-6 | | | | valid option | -| NumDefInvOpt | number | | 5 | 1,3,5,7 | 6-10 | | | | 6 < 5 < 10 | -| NumDefNotNum | number | | a | | | | | | type "number" | -| NumDefOptsNotNum | number | | 1 | 1,a,2 | | | | | type "number" | -| | | | | | | | | | | -| StrDef | string | | hello | | | | hello | true | | -| StrDefInv | string | | hello | | world | | | | regex error | -| StrDefOpts | string | | a | a,b,c | | | a | true | | -| StrDefNotOpts | string | | a | b,c,d | | | | | valid option | -| StrDefValOpts | string | | a | a,b,c,d,e,f | [a-c] | | a | true | | -| StrDefInvOpt | string | | d | a,b,c,d,e,f | [a-c] | | | | regex error | -| | | | | | | | | | | -| LStrDef | list(string) | | ["a"] | | | | ["a"] | true | | -| LStrDefOpts | list(string) | | ["a"] | ["a"], ["b"] | | | ["a"] | true | | -| LStrDefNotOpts | list(string) | | ["a"] | ["b"], ["c"] | | | | | valid option | -| | | | | | | | | | | -| MulDef | tag-select | | ["a"] | | | | ["a"] | true | | -| MulDefOpts | multi-select | | ["a"] | a,b | | | ["a"] | true | | -| MulDefNotOpts | multi-select | | ["a"] | b,c | | | | | valid option | -| | | | | | | | | | | -| | Input Vals | | | | | | | | | -| NumIns | number | 3 | | | | | 3 | false | | -| NumInsOptsNaN | number | 3 | 5 | a,1,2,3,4,5 | 1-3 | | | | type "number" | -| NumInsNotNum | number | a | | | | | | | type "number" | -| NumInsNotNumInv | number | a | | | 1-3 | | | | 1 < a < 3 | -| NumInsDef | number | 3 | 5 | | | | 3 | true | | -| NumIns/DefInv | number | 3 | 5 | | 1-3 | | 3 | true | | -| NumIns=DefInv | number | 5 | 5 | | 1-3 | | | | 1 < 5 < 3 | -| NumInsOpts | number | 3 | 5 | 1,2,3,4,5 | 1-3 | | 3 | true | | -| NumInsNotOptsVal | number | 3 | 5 | 1,2,4,5 | 1-3 | | | | valid option | -| NumInsNotOptsInv | number | 3 | 5 | 1,2,4,5 | 1-2 | | | true | 1 < 3 < 2 | -| NumInsNotOpts | number | 3 | 5 | 1,2,4,5 | | | | | valid option | -| NumInsNotOpts/NoDef | number | 3 | | 1,2,4,5 | | | | | valid option | -| | | | | | | | | | | -| StrIns | string | c | | | | | c | false | | -| StrInsDupeOpts | string | c | | a,b,c,c | | | | | unique | -| StrInsDef | string | c | e | | | | c | true | | -| StrIns/DefInv | string | c | e | | [a-c] | | c | true | | -| StrIns=DefInv | string | e | e | | [a-c] | | | | regex error | -| StrInsOpts | string | c | e | a,b,c,d,e | [a-c] | | c | true | | -| StrInsNotOptsVal | string | c | e | a,b,d,e | [a-c] | | | | valid option | -| StrInsNotOptsInv | string | c | e | a,b,d,e | [a-b] | | | | regex error | -| StrInsNotOpts | string | c | e | a,b,d,e | | | | | valid option | -| StrInsNotOpts/NoDef | string | c | | a,b,d,e | | | | | valid option | -| StrInsBadVal | string | c | | a,b,c,d,e | 1-10 | | | | min cannot | -| | | | | | | | | | | -| | list(string) | | | | | | | | | -| LStrIns | list(string) | ["c"] | | | | | ["c"] | false | | -| LStrInsNotList | list(string) | c | | | | | | | list of strings | -| LStrInsDef | list(string) | ["c"] | ["e"] | | | | ["c"] | true | | -| LStrIns/DefInv | list(string) | ["c"] | ["e"] | | [a-c] | | | | regex cannot | -| LStrInsOpts | list(string) | ["c"] | ["e"] | ["c"],["d"],["e"] | | | ["c"] | true | | -| LStrInsNotOpts | list(string) | ["c"] | ["e"] | ["d"],["e"] | | | | | valid option | -| LStrInsNotOpts/NoDef | list(string) | ["c"] | | ["d"],["e"] | | | | | valid option | -| | | | | | | | | | | -| MulInsOpts | multi-select | ["c"] | ["e"] | c,d,e | | | ["c"] | true | | -| MulInsNotListOpts | multi-select | c | ["e"] | c,d,e | | | | | json encoded | -| MulInsNotOpts | multi-select | ["c"] | ["e"] | d,e | | | | | valid option | -| MulInsNotOpts/NoDef | multi-select | ["c"] | | d,e | | | | | valid option | -| MulInsInvOpts | multi-select | ["c"] | ["e"] | c,d,e | [a-c] | | | | regex cannot | \ No newline at end of file +| Name | Type | Input | Default | Options | Validation | -> | Output | Optional | ErrorCreate | ErrorImport | +|----------------------|---------------|-----------|---------|-------------------|------------|----|--------|----------|-----------------|---------------| +| | Empty Vals | | | | | | | | | | +| Empty | string,number | | | | | | "" | false | | | +| EmptyDupeOps | string,number | | | 1,1,1 | | | | | unique | = | +| EmptyList | list(string) | | | | | | "" | false | | | +| EmptyListDupeOpts | list(string) | | | ["a"],["a"] | | | | | unique | = | +| EmptyMulti | tag-select | | | | | | "" | false | | | +| EmptyOpts | string,number | | | 1,2,3 | | | "" | false | | | +| EmptyRegex | string | | | | world | | | | regex error | = | +| EmptyMin | number | | | | 1-10 | | | | 1 < < 10 | = | +| EmptyMinOpt | number | | | 1,2,3 | 2-5 | | | | 2 < < 5 | 2 < 1 < 5 | +| EmptyRegexOpt | string | | | "hello","goodbye" | goodbye | | | | regex error | = | +| EmptyRegexOk | string | | | | .* | | "" | false | | | +| | | | | | | | | | | | +| | Default Set | No inputs | | | | | | | | | +| NumDef | number | | 5 | | | | 5 | true | | | +| NumDefVal | number | | 5 | | 3-7 | | 5 | true | | | +| NumDefInv | number | | 5 | | 10- | | | | 10 < 5 < 0 | = | +| NumDefOpts | number | | 5 | 1,3,5,7 | 2-6 | | 5 | true | | 2 < 1 < 6 | +| NumDefNotOpts | number | | 5 | 1,3,7,9 | 2-6 | | | | valid option | 2 < 1 < 6 | +| NumDefInvOpt | number | | 5 | 1,3,5,7 | 6-10 | | | | 6 < 5 < 10 | = | +| NumDefNotNum | number | | a | | | | | | type "number" | = | +| NumDefOptsNotNum | number | | 1 | 1,a,2 | | | | | type "number" | = | +| | | | | | | | | | | | +| StrDef | string | | hello | | | | hello | true | | | +| StrDefInv | string | | hello | | world | | | | regex error | = | +| StrDefOpts | string | | a | a,b,c | | | a | true | | | +| StrDefNotOpts | string | | a | b,c,d | | | | | valid option | = | +| StrDefValOpts | string | | a | a,b,c,d,e,f | [a-c] | | a | true | | value "d" | +| StrDefInvOpt | string | | d | a,b,c,d,e,f | [a-c] | | | | regex error | = | +| | | | | | | | | | | | +| LStrDef | list(string) | | ["a"] | | | | ["a"] | true | | | +| LStrDefOpts | list(string) | | ["a"] | ["a"], ["b"] | | | ["a"] | true | | | +| LStrDefNotOpts | list(string) | | ["a"] | ["b"], ["c"] | | | | | valid option | = | +| | | | | | | | | | | | +| MulDef | tag-select | | ["a"] | | | | ["a"] | true | | | +| MulDefOpts | multi-select | | ["a"] | a,b | | | ["a"] | true | | | +| MulDefNotOpts | multi-select | | ["a"] | b,c | | | | | valid option | = | +| | | | | | | | | | | | +| | Input Vals | | | | | | | | | | +| NumIns | number | 3 | | | | | 3 | false | | | +| NumInsOptsNaN | number | 3 | 5 | a,1,2,3,4,5 | 1-3 | | | | type "number" | = | +| NumInsNotNum | number | a | | | | | | | type "number" | = | +| NumInsNotNumInv | number | a | | | 1-3 | | | | 1 < a < 3 | = | +| NumInsDef | number | 3 | 5 | | | | 3 | true | | | +| NumIns/DefInv | number | 3 | 5 | | 1-3 | | 3 | true | | 1 < 5 < 3 | +| NumIns=DefInv | number | 5 | 5 | | 1-3 | | | | 1 < 5 < 3 | = | +| NumInsOpts | number | 3 | 5 | 1,2,3,4,5 | 1-3 | | 3 | true | | 1 < 5 < 3 | +| NumInsNotOptsVal | number | 3 | 5 | 1,2,4,5 | 1-3 | | | | valid option | 1 < 4 < 3 | +| NumInsNotOptsInv | number | 3 | 5 | 1,2,4,5 | 1-2 | | | true | 1 < 3 < 2 | 1 < 4 < 2 | +| NumInsNotOpts | number | 3 | 5 | 1,2,4,5 | | | | | valid option | = | +| NumInsNotOpts/NoDef | number | 3 | | 1,2,4,5 | | | | | valid option | = | +| | | | | | | | | | | | +| StrIns | string | c | | | | | c | false | | | +| StrInsDupeOpts | string | c | | a,b,c,c | | | | | unique | = | +| StrInsDef | string | c | e | | | | c | true | | | +| StrIns/DefInv | string | c | e | | [a-c] | | c | true | | default value | +| StrIns=DefInv | string | e | e | | [a-c] | | | | regex error | = | +| StrInsOpts | string | c | e | a,b,c,d,e | [a-c] | | c | true | | value "d" | +| StrInsNotOptsVal | string | c | e | a,b,d,e | [a-c] | | | | valid option | value "d" | +| StrInsNotOptsInv | string | c | e | a,b,d,e | [a-b] | | | | regex error | = | +| StrInsNotOpts | string | c | e | a,b,d,e | | | | | valid option | = | +| StrInsNotOpts/NoDef | string | c | | a,b,d,e | | | | | valid option | = | +| StrInsBadVal | string | c | | a,b,c,d,e | 1-10 | | | | min cannot | = | +| | | | | | | | | | | | +| | list(string) | | | | | | | | | | +| LStrIns | list(string) | ["c"] | | | | | ["c"] | false | | | +| LStrInsNotList | list(string) | c | | | | | | | list of strings | = | +| LStrInsDef | list(string) | ["c"] | ["e"] | | | | ["c"] | true | | | +| LStrIns/DefInv | list(string) | ["c"] | ["e"] | | [a-c] | | | | regex cannot | = | +| LStrInsOpts | list(string) | ["c"] | ["e"] | ["c"],["d"],["e"] | | | ["c"] | true | | | +| LStrInsNotOpts | list(string) | ["c"] | ["e"] | ["d"],["e"] | | | | | valid option | = | +| LStrInsNotOpts/NoDef | list(string) | ["c"] | | ["d"],["e"] | | | | | valid option | = | +| | | | | | | | | | | | +| MulInsOpts | multi-select | ["c"] | ["e"] | c,d,e | | | ["c"] | true | | | +| MulInsNotListOpts | multi-select | c | ["e"] | c,d,e | | | | | json encoded | = | +| MulInsNotOpts | multi-select | ["c"] | ["e"] | d,e | | | | | valid option | = | +| MulInsNotOpts/NoDef | multi-select | ["c"] | | d,e | | | | | valid option | = | +| MulInsInvOpts | multi-select | ["c"] | ["e"] | c,d,e | [a-c] | | | | regex cannot | = | \ No newline at end of file From 247df4e410c3c9ca159d59a8428975e5e873dc67 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 1 May 2025 15:20:33 -0500 Subject: [PATCH 14/22] add commented block --- provider/parameter.go | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/provider/parameter.go b/provider/parameter.go index 2e19bc43..04f9a4cb 100644 --- a/provider/parameter.go +++ b/provider/parameter.go @@ -24,9 +24,14 @@ import ( type ValidationMode string const ( - // ValidationModeDefault is used for creating a workspace. It validates the - // final value used for a parameter. - ValidationModeDefault ValidationMode = "" + // ValidationModeDefault is used for creating a workspace. It validates the final + // value used for a parameter. Some allowances for invalid options are tolerated, + // as unused options do not affect the final parameter value. The default value + // is also ignored, assuming a value is provided. + ValidationModeDefault ValidationMode = "" + // ValidationModeTemplateImport tolerates empty values, as the value might not be + // available at import. It does not tolerate an invalid default or invalid option + // values. ValidationModeTemplateImport ValidationMode = "template-import" ) @@ -413,6 +418,14 @@ func (v *Parameter) Valid(input *string, mode ValidationMode) (string, diag.Diag value = v.Default } + // TODO: When empty values want to be rejected, uncomment this. + // coder/coder should update to use template import mode first, + // before this is uncommented. + //if value == nil && mode == ValidationModeDefault { + // var empty string + // value = &empty + //} + // optionType might differ from parameter.Type. This is ok, and parameter.Type // should be used for the value type, and optionType for options. optionType, v.FormType, err = ValidateFormType(v.Type, len(v.Option), v.FormType) From dd7af3b73095fee312771088c4f7b997f78d742e Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 1 May 2025 15:30:15 -0500 Subject: [PATCH 15/22] move constant --- provider/parameter.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/provider/parameter.go b/provider/parameter.go index 04f9a4cb..8f01ee46 100644 --- a/provider/parameter.go +++ b/provider/parameter.go @@ -24,6 +24,7 @@ import ( type ValidationMode string const ( + ValidationModeEnvVar = "CODER_VALIDATION_MODE" // ValidationModeDefault is used for creating a workspace. It validates the final // value used for a parameter. Some allowances for invalid options are tolerated, // as unused options do not affect the final parameter value. The default value @@ -59,7 +60,6 @@ type Validation struct { } const ( - ValidationModeEnvVar = "CODER_VALIDATION_MODE" ValidationMonotonicIncreasing = "increasing" ValidationMonotonicDecreasing = "decreasing" ) From fbd613ec2cf41c0138058127426c633fac0da8dc Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 1 May 2025 15:36:50 -0500 Subject: [PATCH 16/22] remove duped validation --- provider/parameter.go | 34 ++++++++-------------------- provider/testdata/parameter_table.md | 8 +++---- 2 files changed, 14 insertions(+), 28 deletions(-) diff --git a/provider/parameter.go b/provider/parameter.go index 8f01ee46..f476e850 100644 --- a/provider/parameter.go +++ b/provider/parameter.go @@ -410,6 +410,16 @@ func valueIsType(typ OptionType, value string) error { } func (v *Parameter) Valid(input *string, mode ValidationMode) (string, diag.Diagnostics) { + if mode != ValidationModeDefault && mode != ValidationModeTemplateImport { + return "", diag.Diagnostics{ + { + Severity: diag.Error, + Summary: "Invalid validation mode", + Detail: fmt.Sprintf("validation mode %q is not supported, use %q, or %q", mode, ValidationModeDefault, ValidationModeTemplateImport), + }, + } + } + var err error var optionType OptionType @@ -465,30 +475,6 @@ func (v *Parameter) Valid(input *string, mode ValidationMode) (string, diag.Diag } } - // TODO: Move this into 'Parameter.validValue'. It exists as another check outside because - // the current behavior is to always apply this validation, regardless if the param is set or not. - // Other validations are only applied if the parameter is set. - // This behavior should be standardized. - if len(v.Validation) == 1 { - empty := "" - validVal := value - if value == nil { - validVal = &empty - } - validCheck := &v.Validation[0] - err := validCheck.Valid(v.Type, *validVal) - if err != nil { - return "", diag.Diagnostics{ - { - Severity: diag.Error, - Summary: fmt.Sprintf("Invalid parameter %s according to 'validation' block", strings.ToLower(v.Name)), - Detail: err.Error(), - AttributePath: cty.Path{}, - }, - } - } - } - // TODO: This is a bit of a hack. The current behavior states if validation // is given, then apply validation to unset values. // This should be removed, and all values should be validated. Meaning diff --git a/provider/testdata/parameter_table.md b/provider/testdata/parameter_table.md index 5049f071..daa56a46 100644 --- a/provider/testdata/parameter_table.md +++ b/provider/testdata/parameter_table.md @@ -9,8 +9,8 @@ | EmptyOpts | string,number | | | 1,2,3 | | | "" | false | | | | EmptyRegex | string | | | | world | | | | regex error | = | | EmptyMin | number | | | | 1-10 | | | | 1 < < 10 | = | -| EmptyMinOpt | number | | | 1,2,3 | 2-5 | | | | 2 < < 5 | 2 < 1 < 5 | -| EmptyRegexOpt | string | | | "hello","goodbye" | goodbye | | | | regex error | = | +| EmptyMinOpt | number | | | 1,2,3 | 2-5 | | | | valid option | 2 < 1 < 5 | +| EmptyRegexOpt | string | | | "hello","goodbye" | goodbye | | | | valid option | regex error | | EmptyRegexOk | string | | | | .* | | "" | false | | | | | | | | | | | | | | | | | Default Set | No inputs | | | | | | | | | @@ -48,7 +48,7 @@ | NumIns=DefInv | number | 5 | 5 | | 1-3 | | | | 1 < 5 < 3 | = | | NumInsOpts | number | 3 | 5 | 1,2,3,4,5 | 1-3 | | 3 | true | | 1 < 5 < 3 | | NumInsNotOptsVal | number | 3 | 5 | 1,2,4,5 | 1-3 | | | | valid option | 1 < 4 < 3 | -| NumInsNotOptsInv | number | 3 | 5 | 1,2,4,5 | 1-2 | | | true | 1 < 3 < 2 | 1 < 4 < 2 | +| NumInsNotOptsInv | number | 3 | 5 | 1,2,4,5 | 1-2 | | | true | valid option | 1 < 4 < 2 | | NumInsNotOpts | number | 3 | 5 | 1,2,4,5 | | | | | valid option | = | | NumInsNotOpts/NoDef | number | 3 | | 1,2,4,5 | | | | | valid option | = | | | | | | | | | | | | | @@ -59,7 +59,7 @@ | StrIns=DefInv | string | e | e | | [a-c] | | | | regex error | = | | StrInsOpts | string | c | e | a,b,c,d,e | [a-c] | | c | true | | value "d" | | StrInsNotOptsVal | string | c | e | a,b,d,e | [a-c] | | | | valid option | value "d" | -| StrInsNotOptsInv | string | c | e | a,b,d,e | [a-b] | | | | regex error | = | +| StrInsNotOptsInv | string | c | e | a,b,d,e | [a-b] | | | | valid option | regex error | | StrInsNotOpts | string | c | e | a,b,d,e | | | | | valid option | = | | StrInsNotOpts/NoDef | string | c | | a,b,d,e | | | | | valid option | = | | StrInsBadVal | string | c | | a,b,c,d,e | 1-10 | | | | min cannot | = | From 5ef834e668561694838ee808510ff574b871997d Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 1 May 2025 15:53:50 -0500 Subject: [PATCH 17/22] Test fixes --- provider/parameter_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/provider/parameter_test.go b/provider/parameter_test.go index b2558cb5..8fa32cfc 100644 --- a/provider/parameter_test.go +++ b/provider/parameter_test.go @@ -85,6 +85,7 @@ func TestParameter(t *testing.T) { data "coder_parameter" "region" { name = "Region" type = "number" + default = 1 option { name = "1" value = "1" @@ -102,6 +103,7 @@ func TestParameter(t *testing.T) { data "coder_parameter" "region" { name = "Region" type = "string" + default = "1" option { name = "1" value = "1" From 3a36fbbc0bfe5a156b47e2902285e1967127a87e Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 2 May 2025 09:22:59 -0500 Subject: [PATCH 18/22] PR fixes --- provider/parameter.go | 60 ++++++++++++++++++------------------------- 1 file changed, 25 insertions(+), 35 deletions(-) diff --git a/provider/parameter.go b/provider/parameter.go index f476e850..a65184b4 100644 --- a/provider/parameter.go +++ b/provider/parameter.go @@ -30,9 +30,10 @@ const ( // as unused options do not affect the final parameter value. The default value // is also ignored, assuming a value is provided. ValidationModeDefault ValidationMode = "" - // ValidationModeTemplateImport tolerates empty values, as the value might not be - // available at import. It does not tolerate an invalid default or invalid option - // values. + // ValidationModeTemplateImport tolerates empty values as long as no 'validation' + // block is specified. This allows for importing a template without having to + // specify a value for every parameter. It does not tolerate an invalid default + // or invalid option values. ValidationModeTemplateImport ValidationMode = "template-import" ) @@ -428,14 +429,6 @@ func (v *Parameter) Valid(input *string, mode ValidationMode) (string, diag.Diag value = v.Default } - // TODO: When empty values want to be rejected, uncomment this. - // coder/coder should update to use template import mode first, - // before this is uncommented. - //if value == nil && mode == ValidationModeDefault { - // var empty string - // value = &empty - //} - // optionType might differ from parameter.Type. This is ok, and parameter.Type // should be used for the value type, and optionType for options. optionType, v.FormType, err = ValidateFormType(v.Type, len(v.Option), v.FormType) @@ -477,39 +470,36 @@ func (v *Parameter) Valid(input *string, mode ValidationMode) (string, diag.Diag // TODO: This is a bit of a hack. The current behavior states if validation // is given, then apply validation to unset values. - // This should be removed, and all values should be validated. Meaning // value == nil should not be accepted in the first place. - if len(v.Validation) > 0 && value == nil { - empty := "" - value = &empty + // To fix this, value should be coerced to an empty string + // if it is nil. Then let the validation logic always apply. + if len(v.Validation) == 0 && value == nil { + return "", nil } - // Value is only validated if it is set. If it is unset, validation - // is skipped. + // forcedValue ensures the value is not-nil. + var forcedValue string if value != nil { - d := v.validValue(*value, optionType, optionValues, cty.Path{}) - if d.HasError() { - return "", d - } + forcedValue = *value + } - err = valueIsType(v.Type, *value) - if err != nil { - return "", diag.Diagnostics{ - { - Severity: diag.Error, - Summary: fmt.Sprintf("Parameter value is not of type %q", v.Type), - Detail: err.Error(), - }, - } - } + d := v.validValue(forcedValue, optionType, optionValues, cty.Path{}) + if d.HasError() { + return "", d } - if value == nil { - // The previous behavior is to always write an empty string - return "", nil + err = valueIsType(v.Type, forcedValue) + if err != nil { + return "", diag.Diagnostics{ + { + Severity: diag.Error, + Summary: fmt.Sprintf("Parameter value is not of type %q", v.Type), + Detail: err.Error(), + }, + } } - return *value, nil + return forcedValue, nil } func (v *Parameter) ValidOptions(optionType OptionType, mode ValidationMode) (map[string]struct{}, diag.Diagnostics) { From 22f12cb7381771e62e7f990a7987aa89ea2d5640 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 2 May 2025 09:26:05 -0500 Subject: [PATCH 19/22] rename Valid -> ValidateInput --- provider/parameter.go | 4 ++-- provider/parameter_test.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/provider/parameter.go b/provider/parameter.go index a65184b4..c14da62c 100644 --- a/provider/parameter.go +++ b/provider/parameter.go @@ -161,7 +161,7 @@ func parameterDataSource() *schema.Resource { } mode := os.Getenv(ValidationModeEnvVar) - value, diags := parameter.Valid(input, ValidationMode(mode)) + value, diags := parameter.ValidateInput(input, ValidationMode(mode)) if diags.HasError() { return diags } @@ -410,7 +410,7 @@ func valueIsType(typ OptionType, value string) error { return nil } -func (v *Parameter) Valid(input *string, mode ValidationMode) (string, diag.Diagnostics) { +func (v *Parameter) ValidateInput(input *string, mode ValidationMode) (string, diag.Diagnostics) { if mode != ValidationModeDefault && mode != ValidationModeTemplateImport { return "", diag.Diagnostics{ { diff --git a/provider/parameter_test.go b/provider/parameter_test.go index 8fa32cfc..e4122f59 100644 --- a/provider/parameter_test.go +++ b/provider/parameter_test.go @@ -839,7 +839,7 @@ func TestParameterValidation(t *testing.T) { t.Run(tc.Name, func(t *testing.T) { t.Parallel() value := &tc.Value - _, diags := tc.Parameter.Valid(value, provider.ValidationModeDefault) + _, diags := tc.Parameter.ValidateInput(value, provider.ValidationModeDefault) if tc.ExpectError != nil { require.True(t, diags.HasError()) errMsg := fmt.Sprintf("%+v", diags[0]) // close enough From 0fd96eeace73cde9d05663b2a470f8de9c8f7599 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Mon, 5 May 2025 11:15:41 -0500 Subject: [PATCH 20/22] remove validation modes --- provider/parameter.go | 67 +---------- provider/parameter_test.go | 101 ++++++----------- provider/testdata/parameter_table.md | 160 +++++++++++++-------------- 3 files changed, 121 insertions(+), 207 deletions(-) diff --git a/provider/parameter.go b/provider/parameter.go index c14da62c..a1023972 100644 --- a/provider/parameter.go +++ b/provider/parameter.go @@ -21,22 +21,6 @@ import ( "golang.org/x/xerrors" ) -type ValidationMode string - -const ( - ValidationModeEnvVar = "CODER_VALIDATION_MODE" - // ValidationModeDefault is used for creating a workspace. It validates the final - // value used for a parameter. Some allowances for invalid options are tolerated, - // as unused options do not affect the final parameter value. The default value - // is also ignored, assuming a value is provided. - ValidationModeDefault ValidationMode = "" - // ValidationModeTemplateImport tolerates empty values as long as no 'validation' - // block is specified. This allows for importing a template without having to - // specify a value for every parameter. It does not tolerate an invalid default - // or invalid option values. - ValidationModeTemplateImport ValidationMode = "template-import" -) - var ( defaultValuePath = cty.Path{cty.GetAttrStep{Name: "default"}} ) @@ -160,8 +144,7 @@ func parameterDataSource() *schema.Resource { input = &envValue } - mode := os.Getenv(ValidationModeEnvVar) - value, diags := parameter.ValidateInput(input, ValidationMode(mode)) + value, diags := parameter.ValidateInput(input) if diags.HasError() { return diags } @@ -410,17 +393,7 @@ func valueIsType(typ OptionType, value string) error { return nil } -func (v *Parameter) ValidateInput(input *string, mode ValidationMode) (string, diag.Diagnostics) { - if mode != ValidationModeDefault && mode != ValidationModeTemplateImport { - return "", diag.Diagnostics{ - { - Severity: diag.Error, - Summary: "Invalid validation mode", - Detail: fmt.Sprintf("validation mode %q is not supported, use %q, or %q", mode, ValidationModeDefault, ValidationModeTemplateImport), - }, - } - } - +func (v *Parameter) ValidateInput(input *string) (string, diag.Diagnostics) { var err error var optionType OptionType @@ -443,31 +416,11 @@ func (v *Parameter) ValidateInput(input *string, mode ValidationMode) (string, d } } - optionValues, diags := v.ValidOptions(optionType, mode) + optionValues, diags := v.ValidOptions(optionType) if diags.HasError() { return "", diags } - if mode == ValidationModeTemplateImport && v.Default != nil { - // Template import should validate the default value. - err := valueIsType(v.Type, *v.Default) - if err != nil { - return "", diag.Diagnostics{ - { - Severity: diag.Error, - Summary: fmt.Sprintf("Default value is not of type %q", v.Type), - Detail: err.Error(), - AttributePath: defaultValuePath, - }, - } - } - - d := v.validValue(*v.Default, optionType, optionValues, defaultValuePath) - if d.HasError() { - return "", d - } - } - // TODO: This is a bit of a hack. The current behavior states if validation // is given, then apply validation to unset values. // value == nil should not be accepted in the first place. @@ -502,7 +455,7 @@ func (v *Parameter) ValidateInput(input *string, mode ValidationMode) (string, d return forcedValue, nil } -func (v *Parameter) ValidOptions(optionType OptionType, mode ValidationMode) (map[string]struct{}, diag.Diagnostics) { +func (v *Parameter) ValidOptions(optionType OptionType) (map[string]struct{}, diag.Diagnostics) { optionNames := map[string]struct{}{} optionValues := map[string]struct{}{} @@ -538,18 +491,10 @@ func (v *Parameter) ValidOptions(optionType OptionType, mode ValidationMode) (ma optionValues[option.Value] = struct{}{} optionNames[option.Name] = struct{}{} - if mode == ValidationModeTemplateImport { - opDiags := v.validValue(option.Value, optionType, nil, cty.Path{}) - if opDiags.HasError() { - for i := range opDiags { - opDiags[i].Summary = fmt.Sprintf("Option %q: %s", option.Name, opDiags[i].Summary) - } - diags = append(diags, opDiags...) - } - } + // Option values are assumed to be valid. Do not call validValue on them. } - if diags.HasError() { + if diags != nil && diags.HasError() { return nil, diags } return optionValues, nil diff --git a/provider/parameter_test.go b/provider/parameter_test.go index e4122f59..21842b6a 100644 --- a/provider/parameter_test.go +++ b/provider/parameter_test.go @@ -839,7 +839,7 @@ func TestParameterValidation(t *testing.T) { t.Run(tc.Name, func(t *testing.T) { t.Parallel() value := &tc.Value - _, diags := tc.Parameter.ValidateInput(value, provider.ValidationModeDefault) + _, diags := tc.Parameter.ValidateInput(value) if tc.ExpectError != nil { require.True(t, diags.HasError()) errMsg := fmt.Sprintf("%+v", diags[0]) // close enough @@ -881,7 +881,6 @@ func TestParameterValidationEnforcement(t *testing.T) { OutputValue string Optional bool CreateError *regexp.Regexp - ImportError *regexp.Regexp } rows := make([]row, 0) @@ -913,18 +912,6 @@ func TestParameterValidationEnforcement(t *testing.T) { } } - var imerr *regexp.Regexp - if columns[10] != "" { - if columns[10] == "=" { - imerr = rerr - } else { - imerr, err = regexp.Compile(columns[10]) - if err != nil { - t.Fatalf("failed to parse error column %q: %v", columns[10], err) - } - } - } - var options []string if columns[4] != "" { options = strings.Split(columns[4], ",") @@ -972,7 +959,6 @@ func TestParameterValidationEnforcement(t *testing.T) { OutputValue: columns[7], Optional: optional, CreateError: rerr, - ImportError: imerr, }) } @@ -991,10 +977,8 @@ func TestParameterValidationEnforcement(t *testing.T) { t.Setenv(provider.ParameterEnvironmentVariable("parameter"), row.InputValue) } - if row.CreateError != nil && row.ImportError != nil { - if row.OutputValue != "" { - t.Errorf("output value %q should not be set if both errors are set", row.OutputValue) - } + if row.CreateError != nil && row.OutputValue != "" { + t.Errorf("output value %q should not be set if both errors are set", row.OutputValue) } var cfg strings.Builder @@ -1036,57 +1020,42 @@ func TestParameterValidationEnforcement(t *testing.T) { } cfg.WriteString("}\n") + resource.Test(t, resource.TestCase{ + ProviderFactories: coderFactory(), + IsUnitTest: true, + Steps: []resource.TestStep{{ + Config: cfg.String(), + ExpectError: row.CreateError, + Check: func(state *terraform.State) error { + require.Len(t, state.Modules, 1) + require.Len(t, state.Modules[0].Resources, 1) + param := state.Modules[0].Resources["data.coder_parameter.parameter"] + require.NotNil(t, param) - for _, mode := range []provider.ValidationMode{provider.ValidationModeDefault, provider.ValidationModeTemplateImport} { - name := string(mode) - if mode == provider.ValidationModeDefault { - name = "create" - } - t.Run(name, func(t *testing.T) { - t.Setenv("CODER_VALIDATION_MODE", string(mode)) - rerr := row.CreateError - if mode == provider.ValidationModeTemplateImport { - rerr = row.ImportError - } - - resource.Test(t, resource.TestCase{ - ProviderFactories: coderFactory(), - IsUnitTest: true, - Steps: []resource.TestStep{{ - Config: cfg.String(), - ExpectError: rerr, - Check: func(state *terraform.State) error { - require.Len(t, state.Modules, 1) - require.Len(t, state.Modules[0].Resources, 1) - param := state.Modules[0].Resources["data.coder_parameter.parameter"] - require.NotNil(t, param) + if row.Default == "" { + _, ok := param.Primary.Attributes["default"] + require.False(t, ok, "default should not be set") + } else { + require.Equal(t, strings.Trim(row.Default, `"`), param.Primary.Attributes["default"]) + } - if row.Default == "" { - _, ok := param.Primary.Attributes["default"] - require.False(t, ok, "default should not be set") - } else { - require.Equal(t, strings.Trim(row.Default, `"`), param.Primary.Attributes["default"]) - } + if row.OutputValue == "" { + _, ok := param.Primary.Attributes["value"] + require.False(t, ok, "output value should not be set") + } else { + require.Equal(t, strings.Trim(row.OutputValue, `"`), param.Primary.Attributes["value"]) + } - if row.OutputValue == "" { - _, ok := param.Primary.Attributes["value"] - require.False(t, ok, "output value should not be set") - } else { - require.Equal(t, strings.Trim(row.OutputValue, `"`), param.Primary.Attributes["value"]) - } + for key, expected := range map[string]string{ + "optional": strconv.FormatBool(row.Optional), + } { + require.Equal(t, expected, param.Primary.Attributes[key], "optional") + } - for key, expected := range map[string]string{ - "optional": strconv.FormatBool(row.Optional), - } { - require.Equal(t, expected, param.Primary.Attributes[key], "optional") - } - - return nil - }, - }}, - }) - }) - } + return nil + }, + }}, + }) }) } } diff --git a/provider/testdata/parameter_table.md b/provider/testdata/parameter_table.md index daa56a46..3df16f06 100644 --- a/provider/testdata/parameter_table.md +++ b/provider/testdata/parameter_table.md @@ -1,80 +1,80 @@ -| Name | Type | Input | Default | Options | Validation | -> | Output | Optional | ErrorCreate | ErrorImport | -|----------------------|---------------|-----------|---------|-------------------|------------|----|--------|----------|-----------------|---------------| -| | Empty Vals | | | | | | | | | | -| Empty | string,number | | | | | | "" | false | | | -| EmptyDupeOps | string,number | | | 1,1,1 | | | | | unique | = | -| EmptyList | list(string) | | | | | | "" | false | | | -| EmptyListDupeOpts | list(string) | | | ["a"],["a"] | | | | | unique | = | -| EmptyMulti | tag-select | | | | | | "" | false | | | -| EmptyOpts | string,number | | | 1,2,3 | | | "" | false | | | -| EmptyRegex | string | | | | world | | | | regex error | = | -| EmptyMin | number | | | | 1-10 | | | | 1 < < 10 | = | -| EmptyMinOpt | number | | | 1,2,3 | 2-5 | | | | valid option | 2 < 1 < 5 | -| EmptyRegexOpt | string | | | "hello","goodbye" | goodbye | | | | valid option | regex error | -| EmptyRegexOk | string | | | | .* | | "" | false | | | -| | | | | | | | | | | | -| | Default Set | No inputs | | | | | | | | | -| NumDef | number | | 5 | | | | 5 | true | | | -| NumDefVal | number | | 5 | | 3-7 | | 5 | true | | | -| NumDefInv | number | | 5 | | 10- | | | | 10 < 5 < 0 | = | -| NumDefOpts | number | | 5 | 1,3,5,7 | 2-6 | | 5 | true | | 2 < 1 < 6 | -| NumDefNotOpts | number | | 5 | 1,3,7,9 | 2-6 | | | | valid option | 2 < 1 < 6 | -| NumDefInvOpt | number | | 5 | 1,3,5,7 | 6-10 | | | | 6 < 5 < 10 | = | -| NumDefNotNum | number | | a | | | | | | type "number" | = | -| NumDefOptsNotNum | number | | 1 | 1,a,2 | | | | | type "number" | = | -| | | | | | | | | | | | -| StrDef | string | | hello | | | | hello | true | | | -| StrDefInv | string | | hello | | world | | | | regex error | = | -| StrDefOpts | string | | a | a,b,c | | | a | true | | | -| StrDefNotOpts | string | | a | b,c,d | | | | | valid option | = | -| StrDefValOpts | string | | a | a,b,c,d,e,f | [a-c] | | a | true | | value "d" | -| StrDefInvOpt | string | | d | a,b,c,d,e,f | [a-c] | | | | regex error | = | -| | | | | | | | | | | | -| LStrDef | list(string) | | ["a"] | | | | ["a"] | true | | | -| LStrDefOpts | list(string) | | ["a"] | ["a"], ["b"] | | | ["a"] | true | | | -| LStrDefNotOpts | list(string) | | ["a"] | ["b"], ["c"] | | | | | valid option | = | -| | | | | | | | | | | | -| MulDef | tag-select | | ["a"] | | | | ["a"] | true | | | -| MulDefOpts | multi-select | | ["a"] | a,b | | | ["a"] | true | | | -| MulDefNotOpts | multi-select | | ["a"] | b,c | | | | | valid option | = | -| | | | | | | | | | | | -| | Input Vals | | | | | | | | | | -| NumIns | number | 3 | | | | | 3 | false | | | -| NumInsOptsNaN | number | 3 | 5 | a,1,2,3,4,5 | 1-3 | | | | type "number" | = | -| NumInsNotNum | number | a | | | | | | | type "number" | = | -| NumInsNotNumInv | number | a | | | 1-3 | | | | 1 < a < 3 | = | -| NumInsDef | number | 3 | 5 | | | | 3 | true | | | -| NumIns/DefInv | number | 3 | 5 | | 1-3 | | 3 | true | | 1 < 5 < 3 | -| NumIns=DefInv | number | 5 | 5 | | 1-3 | | | | 1 < 5 < 3 | = | -| NumInsOpts | number | 3 | 5 | 1,2,3,4,5 | 1-3 | | 3 | true | | 1 < 5 < 3 | -| NumInsNotOptsVal | number | 3 | 5 | 1,2,4,5 | 1-3 | | | | valid option | 1 < 4 < 3 | -| NumInsNotOptsInv | number | 3 | 5 | 1,2,4,5 | 1-2 | | | true | valid option | 1 < 4 < 2 | -| NumInsNotOpts | number | 3 | 5 | 1,2,4,5 | | | | | valid option | = | -| NumInsNotOpts/NoDef | number | 3 | | 1,2,4,5 | | | | | valid option | = | -| | | | | | | | | | | | -| StrIns | string | c | | | | | c | false | | | -| StrInsDupeOpts | string | c | | a,b,c,c | | | | | unique | = | -| StrInsDef | string | c | e | | | | c | true | | | -| StrIns/DefInv | string | c | e | | [a-c] | | c | true | | default value | -| StrIns=DefInv | string | e | e | | [a-c] | | | | regex error | = | -| StrInsOpts | string | c | e | a,b,c,d,e | [a-c] | | c | true | | value "d" | -| StrInsNotOptsVal | string | c | e | a,b,d,e | [a-c] | | | | valid option | value "d" | -| StrInsNotOptsInv | string | c | e | a,b,d,e | [a-b] | | | | valid option | regex error | -| StrInsNotOpts | string | c | e | a,b,d,e | | | | | valid option | = | -| StrInsNotOpts/NoDef | string | c | | a,b,d,e | | | | | valid option | = | -| StrInsBadVal | string | c | | a,b,c,d,e | 1-10 | | | | min cannot | = | -| | | | | | | | | | | | -| | list(string) | | | | | | | | | | -| LStrIns | list(string) | ["c"] | | | | | ["c"] | false | | | -| LStrInsNotList | list(string) | c | | | | | | | list of strings | = | -| LStrInsDef | list(string) | ["c"] | ["e"] | | | | ["c"] | true | | | -| LStrIns/DefInv | list(string) | ["c"] | ["e"] | | [a-c] | | | | regex cannot | = | -| LStrInsOpts | list(string) | ["c"] | ["e"] | ["c"],["d"],["e"] | | | ["c"] | true | | | -| LStrInsNotOpts | list(string) | ["c"] | ["e"] | ["d"],["e"] | | | | | valid option | = | -| LStrInsNotOpts/NoDef | list(string) | ["c"] | | ["d"],["e"] | | | | | valid option | = | -| | | | | | | | | | | | -| MulInsOpts | multi-select | ["c"] | ["e"] | c,d,e | | | ["c"] | true | | | -| MulInsNotListOpts | multi-select | c | ["e"] | c,d,e | | | | | json encoded | = | -| MulInsNotOpts | multi-select | ["c"] | ["e"] | d,e | | | | | valid option | = | -| MulInsNotOpts/NoDef | multi-select | ["c"] | | d,e | | | | | valid option | = | -| MulInsInvOpts | multi-select | ["c"] | ["e"] | c,d,e | [a-c] | | | | regex cannot | = | \ No newline at end of file +| Name | Type | Input | Default | Options | Validation | -> | Output | Optional | ErrorCreate | +|----------------------|---------------|-----------|---------|-------------------|------------|----|--------|----------|-----------------| +| | Empty Vals | | | | | | | | | +| Empty | string,number | | | | | | "" | false | | +| EmptyDupeOps | string,number | | | 1,1,1 | | | | | unique | +| EmptyList | list(string) | | | | | | "" | false | | +| EmptyListDupeOpts | list(string) | | | ["a"],["a"] | | | | | unique | +| EmptyMulti | tag-select | | | | | | "" | false | | +| EmptyOpts | string,number | | | 1,2,3 | | | "" | false | | +| EmptyRegex | string | | | | world | | | | regex error | +| EmptyMin | number | | | | 1-10 | | | | 1 < < 10 | +| EmptyMinOpt | number | | | 1,2,3 | 2-5 | | | | valid option | +| EmptyRegexOpt | string | | | "hello","goodbye" | goodbye | | | | valid option | +| EmptyRegexOk | string | | | | .* | | "" | false | | +| | | | | | | | | | | +| | Default Set | No inputs | | | | | | | | +| NumDef | number | | 5 | | | | 5 | true | | +| NumDefVal | number | | 5 | | 3-7 | | 5 | true | | +| NumDefInv | number | | 5 | | 10- | | | | 10 < 5 < 0 | +| NumDefOpts | number | | 5 | 1,3,5,7 | 2-6 | | 5 | true | | +| NumDefNotOpts | number | | 5 | 1,3,7,9 | 2-6 | | | | valid option | +| NumDefInvOpt | number | | 5 | 1,3,5,7 | 6-10 | | | | 6 < 5 < 10 | +| NumDefNotNum | number | | a | | | | | | type "number" | +| NumDefOptsNotNum | number | | 1 | 1,a,2 | | | | | type "number" | +| | | | | | | | | | | +| StrDef | string | | hello | | | | hello | true | | +| StrDefInv | string | | hello | | world | | | | regex error | +| StrDefOpts | string | | a | a,b,c | | | a | true | | +| StrDefNotOpts | string | | a | b,c,d | | | | | valid option | +| StrDefValOpts | string | | a | a,b,c,d,e,f | [a-c] | | a | true | | +| StrDefInvOpt | string | | d | a,b,c,d,e,f | [a-c] | | | | regex error | +| | | | | | | | | | | +| LStrDef | list(string) | | ["a"] | | | | ["a"] | true | | +| LStrDefOpts | list(string) | | ["a"] | ["a"], ["b"] | | | ["a"] | true | | +| LStrDefNotOpts | list(string) | | ["a"] | ["b"], ["c"] | | | | | valid option | +| | | | | | | | | | | +| MulDef | tag-select | | ["a"] | | | | ["a"] | true | | +| MulDefOpts | multi-select | | ["a"] | a,b | | | ["a"] | true | | +| MulDefNotOpts | multi-select | | ["a"] | b,c | | | | | valid option | +| | | | | | | | | | | +| | Input Vals | | | | | | | | | +| NumIns | number | 3 | | | | | 3 | false | | +| NumInsOptsNaN | number | 3 | 5 | a,1,2,3,4,5 | 1-3 | | | | type "number" | +| NumInsNotNum | number | a | | | | | | | type "number" | +| NumInsNotNumInv | number | a | | | 1-3 | | | | 1 < a < 3 | +| NumInsDef | number | 3 | 5 | | | | 3 | true | | +| NumIns/DefInv | number | 3 | 5 | | 1-3 | | 3 | true | | +| NumIns=DefInv | number | 5 | 5 | | 1-3 | | | | 1 < 5 < 3 | +| NumInsOpts | number | 3 | 5 | 1,2,3,4,5 | 1-3 | | 3 | true | | +| NumInsNotOptsVal | number | 3 | 5 | 1,2,4,5 | 1-3 | | | | valid option | +| NumInsNotOptsInv | number | 3 | 5 | 1,2,4,5 | 1-2 | | | true | valid option | +| NumInsNotOpts | number | 3 | 5 | 1,2,4,5 | | | | | valid option | +| NumInsNotOpts/NoDef | number | 3 | | 1,2,4,5 | | | | | valid option | +| | | | | | | | | | | +| StrIns | string | c | | | | | c | false | | +| StrInsDupeOpts | string | c | | a,b,c,c | | | | | unique | +| StrInsDef | string | c | e | | | | c | true | | +| StrIns/DefInv | string | c | e | | [a-c] | | c | true | | +| StrIns=DefInv | string | e | e | | [a-c] | | | | regex error | +| StrInsOpts | string | c | e | a,b,c,d,e | [a-c] | | c | true | | +| StrInsNotOptsVal | string | c | e | a,b,d,e | [a-c] | | | | valid option | +| StrInsNotOptsInv | string | c | e | a,b,d,e | [a-b] | | | | valid option | +| StrInsNotOpts | string | c | e | a,b,d,e | | | | | valid option | +| StrInsNotOpts/NoDef | string | c | | a,b,d,e | | | | | valid option | +| StrInsBadVal | string | c | | a,b,c,d,e | 1-10 | | | | min cannot | +| | | | | | | | | | | +| | list(string) | | | | | | | | | +| LStrIns | list(string) | ["c"] | | | | | ["c"] | false | | +| LStrInsNotList | list(string) | c | | | | | | | list of strings | +| LStrInsDef | list(string) | ["c"] | ["e"] | | | | ["c"] | true | | +| LStrIns/DefInv | list(string) | ["c"] | ["e"] | | [a-c] | | | | regex cannot | +| LStrInsOpts | list(string) | ["c"] | ["e"] | ["c"],["d"],["e"] | | | ["c"] | true | | +| LStrInsNotOpts | list(string) | ["c"] | ["e"] | ["d"],["e"] | | | | | valid option | +| LStrInsNotOpts/NoDef | list(string) | ["c"] | | ["d"],["e"] | | | | | valid option | +| | | | | | | | | | | +| MulInsOpts | multi-select | ["c"] | ["e"] | c,d,e | | | ["c"] | true | | +| MulInsNotListOpts | multi-select | c | ["e"] | c,d,e | | | | | json encoded | +| MulInsNotOpts | multi-select | ["c"] | ["e"] | d,e | | | | | valid option | +| MulInsNotOpts/NoDef | multi-select | ["c"] | | d,e | | | | | valid option | +| MulInsInvOpts | multi-select | ["c"] | ["e"] | c,d,e | [a-c] | | | | regex cannot | \ No newline at end of file From 9433b31e892a9a27c63b77ded0acdbc7ba87eec2 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 7 May 2025 07:50:44 -0500 Subject: [PATCH 21/22] chore: use default path in error if sourced from default --- provider/parameter.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/provider/parameter.go b/provider/parameter.go index a1023972..e3369019 100644 --- a/provider/parameter.go +++ b/provider/parameter.go @@ -152,7 +152,9 @@ func parameterDataSource() *schema.Resource { // Always set back the value, as it can be sourced from the default rd.Set("value", value) - // Set the form_type as it could have changed in the validation. + // Set the form_type, as if it was unset, a default form_type will be updated on + // the parameter struct. Always set back the updated form_type to be more + // specific than the default empty string. rd.Set("form_type", parameter.FormType) return nil @@ -397,9 +399,13 @@ func (v *Parameter) ValidateInput(input *string) (string, diag.Diagnostics) { var err error var optionType OptionType + valuePath := cty.Path{} value := input if input == nil { value = v.Default + if v.Default != nil { + valuePath = defaultValuePath + } } // optionType might differ from parameter.Type. This is ok, and parameter.Type @@ -436,7 +442,7 @@ func (v *Parameter) ValidateInput(input *string) (string, diag.Diagnostics) { forcedValue = *value } - d := v.validValue(forcedValue, optionType, optionValues, cty.Path{}) + d := v.validValue(forcedValue, optionType, optionValues, valuePath) if d.HasError() { return "", d } @@ -511,7 +517,7 @@ func (v *Parameter) validValue(value string, optionType OptionType, optionValues if len(optionValues) > 0 { if v.Type == OptionTypeListString && optionType == OptionTypeString { // If the type is list(string) and optionType is string, we have - // to ensure all elements of the default exist as options. + // to ensure all elements of the value exist as options. listValues, err := valueIsListString(value) if err != nil { return diag.Diagnostics{ From 79d3bf7fa3cf740ce18ba1386aa6184cb97f0d22 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 7 May 2025 07:52:20 -0500 Subject: [PATCH 22/22] chore: always use the path argument, not default path --- provider/parameter.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/provider/parameter.go b/provider/parameter.go index e3369019..fd484578 100644 --- a/provider/parameter.go +++ b/provider/parameter.go @@ -525,7 +525,7 @@ func (v *Parameter) validValue(value string, optionType OptionType, optionValues Severity: diag.Error, Summary: "When using list(string) type, value must be a json encoded list of strings", Detail: err.Error(), - AttributePath: defaultValuePath, + AttributePath: path, }, } } @@ -548,7 +548,7 @@ func (v *Parameter) validValue(value string, optionType OptionType, optionValues "%s %q is not a valid option, values %q are missing from the options", name, value, strings.Join(missing, ", "), ), - AttributePath: defaultValuePath, + AttributePath: path, }, } }