Implement CSV-based overrides for list-like variables

Though CSV is favored for 'simple' lists, a JSON value will be used if it parses without error. This assumes that it is extremely unlikely that something that parses as JSON would be intended to be parsed as CSV, e.g. `["a"` and `"b"]`, as opposed to `a` and `b`. If parsing/conversion fails, it is treated as if it was a CSV. Since the CSV approach required processing of each element, code was refactored to reuse the same logic used for individual non-typed variables. Signed-off-by: Roberto Villarreal <rrjjvv@yahoo.com>
2025-05-09 18:36:11 -06:00
parent 5b8a3b3728
commit 1f56984626
2 changed files with 204 additions and 35 deletions
@@ -1792,6 +1792,17 @@ func TestDefaultVarTypeWithAttrValuesEnforcement(t *testing.T) {
 			attrValue: `"99"`,
 			varType:   "number",
 		},
+		{
+			name:      "attribute from function which coerces to var type",
+			attrValue: `substr("99 bottles", 0, 2)`,
+			varType:   "number",
+		},
+		{
+			name:      "attribute from function returning non-coercible value",
+			attrValue: `split(",", "1,2,3foo")`,
+			varType:   "list(number)",
+			wantError: true,
+		},
 		{
 			name:      "mismatch",
 			attrValue: 99,
@@ -1828,7 +1839,7 @@ func TestDefaultVarTypeWithAttrValuesEnforcement(t *testing.T) {
 }

 func TestTypedVarOverrides(t *testing.T) {
-	const convertFailure = "failed to convert FOO"
+	const unsuitableValueType = "Unsuitable value type"
 	tests := []struct {
 		name         string
 		varType      string
@@ -1886,27 +1897,28 @@ func TestTypedVarOverrides(t *testing.T) {
 			argValue:  `join("-", FOO)`,
 			wantValue: "hi-there",
 		},
-		// not that this *should* be an error, but pseudo-documentation that this is
-		// a scenario that might be expected to work, but doesn't (yet) for simplicity
 		{
-			name:         "JSON list of unquoted strings not okay",
-			varType:      "list(string)",
-			override:     `[hi,there]`,
-			wantErrorMsg: convertFailure,
+			name:      "proper CSV list of strings",
+			varType:   "list(string)",
+			override:  "hi,there",
+			argValue:  `join("-", FOO)`,
+			wantValue: "hi-there",
 		},
-		// ditto above
+		// pseudo-documentation that this is a scenario that might be expected to work,
+		// but will parse as a valid CSV in (usually) an undesirable way
 		{
-			name:         "CSV of quoted strings not okay",
-			varType:      "list(string)",
-			override:     `"hi","there"`,
-			wantErrorMsg: convertFailure,
+			name:      "pseudo-JSON list of unquoted strings",
+			varType:   "list(string)",
+			override:  `[hi,there]`,
+			argValue:  `join("-", FOO)`,
+			wantValue: "[hi-there]",
 		},
-		// ditto above
 		{
-			name:         "CSV of unquoted strings not okay",
-			varType:      "list(string)",
-			override:     `hi,there`,
-			wantErrorMsg: convertFailure,
+			name:      "CSV of unquoted strings okay",
+			varType:   "list(string)",
+			override:  `hi,there`,
+			argValue:  `join("-", FOO)`,
+			wantValue: "hi-there",
 		},
 		{
 			name:      "JSON list of numbers",
@@ -1915,6 +1927,13 @@ func TestTypedVarOverrides(t *testing.T) {
 			argValue:  `join("-", [for v in FOO: v + 1])`,
 			wantValue: "4-2-5",
 		},
+		{
+			name:      "CSV list of numbers",
+			varType:   "list(number)",
+			override:  "3,1,4",
+			argValue:  `join("-", [for v in FOO: v + 1])`,
+			wantValue: "4-2-5",
+		},
 		{
 			name:      "JSON map of numbers",
 			varType:   "map(number)",
@@ -1923,10 +1942,46 @@ func TestTypedVarOverrides(t *testing.T) {
 			wantValue: "1-2",
 		},
 		{
-			name:         "invalid JSON map of numbers",
-			varType:      "map(number)",
-			override:     `{"foo": "oops", "bar": 2}`,
-			wantErrorMsg: convertFailure,
+			name:      "CSV map of numbers",
+			varType:   "map(number)",
+			override:  "foo:1,bar:2",
+			argValue:  `join("-", sort(values(FOO)))`,
+			wantValue: "1-2",
+		},
+		// though a JSON payload, any failure (types in this case) defers to
+		// CSV parsing and its error; not ideal and could be improved
+		{
+			name:     "invalid JSON map of numbers",
+			varType:  "map(number)",
+			override: `{"foo": "oops", "bar": 2}`,
+			// in lieu of something like ErrorMatches, this is the best single phrase
+			wantErrorMsg: "as CSV",
+		},
+		{
+			name:      "JSON tuple",
+			varType:   "tuple([number,string])",
+			override:  `[99, "bottles"]`,
+			argValue:  `format("%d %s", FOO[0], FOO[1])`,
+			wantValue: "99 bottles",
+		},
+		{
+			name:      "CSV tuple",
+			varType:   "tuple([number,string])",
+			override:  `99,bottles`,
+			argValue:  `format("%d %s", FOO[0], FOO[1])`,
+			wantValue: "99 bottles",
+		},
+		{
+			name:         "JSON tuple elements with wrong type",
+			varType:      "tuple([number,string])",
+			override:     `[99, 100]`,
+			wantErrorMsg: unsuitableValueType,
+		},
+		{
+			name:         "CSV tuple elements with wrong type",
+			varType:      "tuple([number,string])",
+			override:     `99,100`,
+			wantErrorMsg: unsuitableValueType,
 		},
 		{
 			name:      "JSON object",
@@ -16,6 +16,7 @@ import (
 	"github.com/hashicorp/hcl/v2"
 	"github.com/hashicorp/hcl/v2/ext/typeexpr"
 	"github.com/pkg/errors"
+	"github.com/tonistiigi/go-csvvalue"
 	"github.com/zclconf/go-cty/cty"
 	"github.com/zclconf/go-cty/cty/convert"
 	ctyjson "github.com/zclconf/go-cty/cty/json"
@@ -325,6 +326,16 @@ func (p *parser) resolveValue(ectx *hcl.EvalContext, name string) (err error) {
 		switch {
 		case varType.Equals(cty.String): // don't parse as JSON; users don't expect to have to quote strings
 			vv = cty.StringVal(envv)
+		case varType.IsListType(), varType.IsSetType(), varType.IsTupleType(), varType.IsMapType(): // typing explicitly specified
+			// since CSV is being treated as the officially supported way, throw away (for now) any JSON errors
+			// in favor of CSV behavior and leave it the user to figure it out if they intended JSON
+			vv, err = ctyjson.Unmarshal([]byte(envv), varType)
+			if err != nil {
+				vv, err = valueFromCSV(name, envv, varType)
+				if err != nil {
+					return errors.Wrapf(err, "failed to convert variable %s", name)
+				}
+			}
 		case !varType.Equals(cty.DynamicPseudoType): // typing was explicitly specified
 			vv, err = ctyjson.Unmarshal([]byte(envv), varType)
 			if err != nil {
@@ -332,23 +343,13 @@ func (p *parser) resolveValue(ectx *hcl.EvalContext, name string) (err error) {
 			}
 		case def == nil: // no default from which to infer typing
 			vv = cty.StringVal(envv)
-		case vv.Type().Equals(cty.Bool):
-			b, err := strconv.ParseBool(envv)
-			if err != nil {
-				return errors.Wrapf(err, "failed to parse %s as bool", name)
-			}
-			vv = cty.BoolVal(b)
-		case vv.Type().Equals(cty.String), vv.Type().Equals(cty.DynamicPseudoType):
+		case vv.Type().Equals(cty.DynamicPseudoType):
 			vv = cty.StringVal(envv)
-		case vv.Type().Equals(cty.Number):
-			n, err := strconv.ParseFloat(envv, 64)
-			if err == nil && (math.IsNaN(n) || math.IsInf(n, 0)) {
-				err = errors.Errorf("invalid number value")
-			}
+		case vv.Type().Equals(cty.Bool), vv.Type().Equals(cty.String), vv.Type().Equals(cty.Number):
+			vv, err = convertPrimitive(name, envv, vv.Type())
 			if err != nil {
-				return errors.Wrapf(err, "failed to parse %s as number", name)
+				return err
 			}
-			vv = cty.NumberVal(big.NewFloat(n))
 		default:
 			return errors.Errorf("unsupported type %s for variable %s", vv.Type().FriendlyName(), name)
 		}
@@ -956,6 +957,119 @@ func typeConstraint(expr hcl.Expression) (cty.Type, hcl.Diagnostics) {
 	return t, diag
 }

+// convertPrimitive converts a single string primitive value to a given cty.Type.
+func convertPrimitive(name, value string, target cty.Type) (cty.Value, error) {
+	switch {
+	case target.Equals(cty.String):
+		return cty.StringVal(value), nil
+	case target.Equals(cty.Bool):
+		b, err := strconv.ParseBool(value)
+		if err != nil {
+			return cty.NilVal, errors.Wrapf(err, "failed to parse %s as bool", name)
+		}
+		return cty.BoolVal(b), nil
+	case target.Equals(cty.Number):
+		n, err := strconv.ParseFloat(value, 64)
+		if err == nil && (math.IsNaN(n) || math.IsInf(n, 0)) {
+			err = errors.Errorf("invalid number value")
+		}
+		if err != nil {
+			return cty.NilVal, errors.Wrapf(err, "failed to parse %s as number", name)
+		}
+		return cty.NumberVal(big.NewFloat(n)), nil
+	default:
+		return cty.NilVal, errors.Errorf("%s of type %s is not a primitive", name, target.FriendlyName())
+	}
+}
+
+// valueFromCSV takes CSV value and converts it to cty.Type.
+//
+// This currently supports conversion to cty.List and cty.Set.
+// It also contains preliminary support for cty.Map (the other collection type).
+// While not considered a collection type, it also tentatively supports cty.Tuple.
+func valueFromCSV(name, value string, target cty.Type) (cty.Value, error) {
+	fields, err := csvvalue.Fields(value, nil)
+	if err != nil {
+		return cty.NilVal, errors.Wrapf(err, "failed to parse %s as CSV", value)
+	}
+
+	// used for lists and set, which require identical processing and differ only in return type
+	singleTypeConvert := func(t cty.Type) ([]cty.Value, error) {
+		var elems []cty.Value
+		for _, f := range fields {
+			v, err := convertPrimitive(name, f, t)
+			if err != nil {
+				return nil, errors.Wrapf(err, "failed to parse element of type %s", target.FriendlyName())
+			}
+			elems = append(elems, v)
+		}
+		return elems, nil
+	}
+
+	switch {
+	case target.IsListType():
+		if !target.ElementType().IsPrimitiveType() {
+			return cty.NilVal, errors.Errorf("unsupported type %s for CSV specification", target.FriendlyName())
+		}
+		elems, err := singleTypeConvert(target.ElementType())
+		if err != nil {
+			return cty.NilVal, err
+		}
+		return cty.ListVal(elems), nil
+	case target.IsSetType():
+		if !target.ElementType().IsPrimitiveType() {
+			return cty.NilVal, errors.Errorf("unsupported type %s for CSV specification", target.FriendlyName())
+		}
+		elems, err := singleTypeConvert(target.ElementType())
+		if err != nil {
+			return cty.NilVal, err
+		}
+		return cty.SetVal(elems), nil
+	case target.IsTupleType():
+		tupleTypes := target.TupleElementTypes()
+		if len(tupleTypes) != len(fields) {
+			return cty.NilVal, errors.Errorf("%s expects %d elements but only %d provided", target.FriendlyName(), len(tupleTypes), len(fields))
+		}
+		var elems []cty.Value
+		for i, f := range fields {
+			tt := tupleTypes[i]
+			if !tt.IsPrimitiveType() {
+				return cty.NilVal, errors.Errorf("unsupported type %s for CSV specification", target.FriendlyName())
+			}
+			v, err := convertPrimitive(name, f, tt)
+			if err != nil {
+				return cty.NilVal, errors.Wrapf(err, "failed to parse element of type %s", target.FriendlyName())
+			}
+			elems = append(elems, v)
+		}
+		return cty.TupleVal(elems), nil
+	case target.IsMapType():
+		if !target.ElementType().IsPrimitiveType() {
+			return cty.NilVal, errors.Errorf("unsupported type %s for CSV specification", target.FriendlyName())
+		}
+		p := csvvalue.Parser{Comma: ':'}
+		var kvSlice []string
+		m := make(map[string]cty.Value)
+		for _, f := range fields {
+			kvSlice, err = p.Fields(f, kvSlice)
+			if err != nil {
+				return cty.NilVal, errors.Wrapf(err, "failed to parse %s as k/v", f)
+			}
+			if len(kvSlice) != 2 {
+				return cty.NilVal, errors.Errorf("expected one k/v pair but got %d pieces from %s", len(kvSlice), f)
+			}
+			v, err := convertPrimitive(name, kvSlice[1], target.ElementType())
+			if err != nil {
+				return cty.NilVal, errors.Wrapf(err, "failed to parse value from type %s", target.FriendlyName())
+			}
+			m[kvSlice[0]] = v
+		}
+		return cty.MapVal(m), nil
+	default:
+		return cty.NilVal, errors.Errorf("unsupported type %s for CSV specification", target.FriendlyName())
+	}
+}
+
 // wrapErrorDiagnostic wraps an error into a hcl.Diagnostics object.
 // If the error is already an hcl.Diagnostics object, it is returned as is.
 func wrapErrorDiagnostic(message string, err error, subject *hcl.Range, context *hcl.Range) hcl.Diagnostics {