diff --git a/bake/hcl_test.go b/bake/hcl_test.go index 120e28aae..698de620d 100644 --- a/bake/hcl_test.go +++ b/bake/hcl_test.go @@ -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", diff --git a/bake/hclparser/hclparser.go b/bake/hclparser/hclparser.go index 2ded19216..13caf8e3f 100644 --- a/bake/hclparser/hclparser.go +++ b/bake/hclparser/hclparser.go @@ -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 {