Skip to content

Commit 8692a56

Browse files
EmyrkEmyrk
authored
feat: enforce monotonicity in terraform provider (#392)
Previous value must come from env var.
1 parent 3c74804 commit 8692a56

File tree

3 files changed

+241
-109
lines changed

3 files changed

+241
-109
lines changed

‎provider/parameter.go

+49-6
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,13 @@ func parameterDataSource() *schema.Resource {
144144
input = &envValue
145145
}
146146

147-
value, diags := parameter.ValidateInput(input)
147+
var previous *string
148+
envPreviousValue, ok := os.LookupEnv(ParameterEnvironmentVariablePrevious(parameter.Name))
149+
if ok {
150+
previous = &envPreviousValue
151+
}
152+
153+
value, diags := parameter.ValidateInput(input, previous)
148154
if diags.HasError() {
149155
return diags
150156
}
@@ -395,7 +401,7 @@ func valueIsType(typ OptionType, value string) error {
395401
return nil
396402
}
397403

398-
func (v *Parameter) ValidateInput(input *string) (string, diag.Diagnostics) {
404+
func (v *Parameter) ValidateInput(input *string, previous *string) (string, diag.Diagnostics) {
399405
var err error
400406
var optionType OptionType
401407

@@ -442,7 +448,7 @@ func (v *Parameter) ValidateInput(input *string) (string, diag.Diagnostics) {
442448
forcedValue = *value
443449
}
444450

445-
d := v.validValue(forcedValue, optionType, optionValues, valuePath)
451+
d := v.validValue(forcedValue, previous, optionType, optionValues, valuePath)
446452
if d.HasError() {
447453
return "", d
448454
}
@@ -506,7 +512,7 @@ func (v *Parameter) ValidOptions(optionType OptionType) (map[string]struct{}, di
506512
return optionValues, nil
507513
}
508514

509-
func (v *Parameter) validValue(value string, optionType OptionType, optionValues map[string]struct{}, path cty.Path) diag.Diagnostics {
515+
func (v *Parameter) validValue(value string, previous *string, optionType OptionType, optionValues map[string]struct{}, path cty.Path) diag.Diagnostics {
510516
// name is used for constructing more precise error messages.
511517
name := "Value"
512518
if path.Equals(defaultValuePath) {
@@ -573,7 +579,7 @@ func (v *Parameter) validValue(value string, optionType OptionType, optionValues
573579

574580
if len(v.Validation) == 1 {
575581
validCheck := &v.Validation[0]
576-
err := validCheck.Valid(v.Type, value)
582+
err := validCheck.Valid(v.Type, value, previous)
577583
if err != nil {
578584
return diag.Diagnostics{
579585
{
@@ -589,7 +595,7 @@ func (v *Parameter) validValue(value string, optionType OptionType, optionValues
589595
return nil
590596
}
591597

592-
func (v *Validation) Valid(typ OptionType, value string) error {
598+
func (v *Validation) Valid(typ OptionType, value string, previous *string) error {
593599
if typ != OptionTypeNumber {
594600
if !v.MinDisabled {
595601
return fmt.Errorf("a min cannot be specified for a %s type", typ)
@@ -639,6 +645,34 @@ func (v *Validation) Valid(typ OptionType, value string) error {
639645
if v.Monotonic != "" && v.Monotonic != ValidationMonotonicIncreasing && v.Monotonic != ValidationMonotonicDecreasing {
640646
return fmt.Errorf("number monotonicity can be either %q or %q", ValidationMonotonicIncreasing, ValidationMonotonicDecreasing)
641647
}
648+
649+
switch v.Monotonic {
650+
case "":
651+
// No monotonicity check
652+
case ValidationMonotonicIncreasing, ValidationMonotonicDecreasing:
653+
if previous != nil { // Only check if previous value exists
654+
previousNum, err := strconv.Atoi(*previous)
655+
if err != nil {
656+
// Do not throw an error for the previous value not being a number. Throwing an
657+
// error here would cause an unrepairable state for the user. This is
658+
// unfortunate, but there is not much we can do at this point.
659+
// TODO: Maybe we should enforce this, and have the calling coderd
660+
// do something to resolve it. Such as doing this check before calling
661+
// terraform apply.
662+
break
663+
}
664+
665+
if v.Monotonic == ValidationMonotonicIncreasing && !(num >= previousNum) {
666+
return fmt.Errorf("parameter value '%d' must be equal or greater than previous value: %d", num, previousNum)
667+
}
668+
669+
if v.Monotonic == ValidationMonotonicDecreasing && !(num <= previousNum) {
670+
return fmt.Errorf("parameter value '%d' must be equal or lower than previous value: %d", num, previousNum)
671+
}
672+
}
673+
default:
674+
return fmt.Errorf("number monotonicity can be either %q or %q", ValidationMonotonicIncreasing, ValidationMonotonicDecreasing)
675+
}
642676
case OptionTypeListString:
643677
var listOfStrings []string
644678
err := json.Unmarshal([]byte(value), &listOfStrings)
@@ -666,6 +700,15 @@ func ParameterEnvironmentVariable(name string) string {
666700
return "CODER_PARAMETER_" + hex.EncodeToString(sum[:])
667701
}
668702

703+
// ParameterEnvironmentVariablePrevious returns the environment variable to
704+
// specify for a parameter's previous value. This is used for workspace
705+
// subsequent builds after the first. Primarily to validate monotonicity in the
706+
// `validation` block.
707+
func ParameterEnvironmentVariablePrevious(name string) string {
708+
sum := sha256.Sum256([]byte(name))
709+
return "CODER_PARAMETER_PREVIOUS_" + hex.EncodeToString(sum[:])
710+
}
711+
669712
func takeFirstError(errs ...error) error {
670713
for _, err := range errs {
671714
if err != nil {

‎provider/parameter_test.go

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

886887
rows := make([]row, 0)
@@ -898,33 +899,44 @@ func TestParameterValidationEnforcement(t *testing.T) {
898899
continue // Skip rows with empty names
899900
}
900901

901-
optional, err := strconv.ParseBool(columns[8])
902-
if columns[8] != "" {
902+
cname, ctype, cprev, cinput, cdefault, coptions, cvalidation, _, coutput, coptional, cerr :=
903+
columns[0], columns[1], columns[2], columns[3], columns[4], columns[5], columns[6], columns[7], columns[8], columns[9], columns[10]
904+
905+
optional, err := strconv.ParseBool(coptional)
906+
if coptional != "" {
903907
// Value does not matter if not specified
904908
require.NoError(t, err)
905909
}
906910

907911
var rerr *regexp.Regexp
908-
if columns[9] != "" {
909-
rerr, err = regexp.Compile(columns[9])
912+
if cerr != "" {
913+
rerr, err = regexp.Compile(cerr)
910914
if err != nil {
911-
t.Fatalf("failed to parse error column %q: %v", columns[9], err)
915+
t.Fatalf("failed to parse error column %q: %v", cerr, err)
912916
}
913917
}
914918

915919
var options []string
916-
if columns[4] != "" {
917-
options = strings.Split(columns[4], ",")
920+
if coptions != "" {
921+
options = strings.Split(coptions, ",")
918922
}
919923

920924
var validation *provider.Validation
921-
if columns[5] != "" {
922-
// Min-Max validation should look like:
923-
// 1-10 :: min=1, max=10
924-
// -10 :: max=10
925-
// 1- :: min=1
926-
if validMinMax.MatchString(columns[5]) {
927-
parts := strings.Split(columns[5], "-")
925+
if cvalidation != "" {
926+
switch {
927+
case cvalidation == provider.ValidationMonotonicIncreasing || cvalidation == provider.ValidationMonotonicDecreasing:
928+
validation = &provider.Validation{
929+
MinDisabled: true,
930+
MaxDisabled: true,
931+
Monotonic: cvalidation,
932+
Error: "monotonicity",
933+
}
934+
case validMinMax.MatchString(cvalidation):
935+
// Min-Max validation should look like:
936+
// 1-10 :: min=1, max=10
937+
// -10 :: max=10
938+
// 1- :: min=1
939+
parts := strings.Split(cvalidation, "-")
928940
min, _ := strconv.ParseInt(parts[0], 10, 64)
929941
max, _ := strconv.ParseInt(parts[1], 10, 64)
930942
validation = &provider.Validation{
@@ -936,29 +948,37 @@ func TestParameterValidationEnforcement(t *testing.T) {
936948
Regex: "",
937949
Error: "{min} < {value} < {max}",
938950
}
939-
} else {
951+
default:
940952
validation = &provider.Validation{
941953
Min: 0,
942954
MinDisabled: true,
943955
Max: 0,
944956
MaxDisabled: true,
945957
Monotonic: "",
946-
Regex: columns[5],
958+
Regex: cvalidation,
947959
Error: "regex error",
948960
}
949961
}
950962
}
951963

964+
var prev *string
965+
if cprev != "" {
966+
prev = ptr(cprev)
967+
if cprev == `""` {
968+
prev = ptr("")
969+
}
970+
}
952971
rows = append(rows, row{
953-
Name: columns[0],
954-
Types: strings.Split(columns[1], ","),
955-
InputValue: columns[2],
956-
Default: columns[3],
972+
Name: cname,
973+
Types: strings.Split(ctype, ","),
974+
InputValue: cinput,
975+
Default: cdefault,
957976
Options: options,
958977
Validation: validation,
959-
OutputValue: columns[7],
978+
OutputValue: coutput,
960979
Optional: optional,
961980
CreateError: rerr,
981+
Previous: prev,
962982
})
963983
}
964984

@@ -976,6 +996,9 @@ func TestParameterValidationEnforcement(t *testing.T) {
976996
if row.InputValue != "" {
977997
t.Setenv(provider.ParameterEnvironmentVariable("parameter"), row.InputValue)
978998
}
999+
if row.Previous != nil {
1000+
t.Setenv(provider.ParameterEnvironmentVariablePrevious("parameter"), *row.Previous)
1001+
}
9791002

9801003
if row.CreateError != nil && row.OutputValue != "" {
9811004
t.Errorf("output value %q should not be set if both errors are set", row.OutputValue)
@@ -1067,6 +1090,7 @@ func TestValueValidatesType(t *testing.T) {
10671090
Name string
10681091
Type provider.OptionType
10691092
Value string
1093+
Previous *string
10701094
Regex string
10711095
RegexError string
10721096
Min int
@@ -1154,6 +1178,56 @@ func TestValueValidatesType(t *testing.T) {
11541178
Min: 0,
11551179
Max: 2,
11561180
Monotonic: "decreasing",
1181+
}, {
1182+
Name: "IncreasingMonotonicityEqual",
1183+
Type: "number",
1184+
Previous: ptr("1"),
1185+
Value: "1",
1186+
Monotonic: "increasing",
1187+
MinDisabled: true,
1188+
MaxDisabled: true,
1189+
}, {
1190+
Name: "DecreasingMonotonicityEqual",
1191+
Type: "number",
1192+
Value: "1",
1193+
Previous: ptr("1"),
1194+
Monotonic: "decreasing",
1195+
MinDisabled: true,
1196+
MaxDisabled: true,
1197+
}, {
1198+
Name: "IncreasingMonotonicityGreater",
1199+
Type: "number",
1200+
Previous: ptr("0"),
1201+
Value: "1",
1202+
Monotonic: "increasing",
1203+
MinDisabled: true,
1204+
MaxDisabled: true,
1205+
}, {
1206+
Name: "DecreasingMonotonicityGreater",
1207+
Type: "number",
1208+
Value: "1",
1209+
Previous: ptr("0"),
1210+
Monotonic: "decreasing",
1211+
MinDisabled: true,
1212+
MaxDisabled: true,
1213+
Error: regexp.MustCompile("must be equal or"),
1214+
}, {
1215+
Name: "IncreasingMonotonicityLesser",
1216+
Type: "number",
1217+
Previous: ptr("2"),
1218+
Value: "1",
1219+
Monotonic: "increasing",
1220+
MinDisabled: true,
1221+
MaxDisabled: true,
1222+
Error: regexp.MustCompile("must be equal or"),
1223+
}, {
1224+
Name: "DecreasingMonotonicityLesser",
1225+
Type: "number",
1226+
Value: "1",
1227+
Previous: ptr("2"),
1228+
Monotonic: "decreasing",
1229+
MinDisabled: true,
1230+
MaxDisabled: true,
11571231
}, {
11581232
Name: "ValidListOfStrings",
11591233
Type: "list(string)",
@@ -1205,7 +1279,7 @@ func TestValueValidatesType(t *testing.T) {
12051279
Regex: tc.Regex,
12061280
Error: tc.RegexError,
12071281
}
1208-
err := v.Valid(tc.Type, tc.Value)
1282+
err := v.Valid(tc.Type, tc.Value, tc.Previous)
12091283
if tc.Error != nil {
12101284
require.Error(t, err)
12111285
require.True(t, tc.Error.MatchString(err.Error()), "got: %s", err.Error())

0 commit comments

Comments
 (0)