From 5b8a3b3728d9760422e9216e765981297ca1893c Mon Sep 17 00:00:00 2001 From: Roberto Villarreal Date: Fri, 2 May 2025 20:32:05 -0600 Subject: [PATCH 1/5] Allow variables to be explicitly typed (and enforced) This allows variables to have explicit types, similar to Terraform variables. It uses HCL's `typeexpr` extension for the specification. For conversion of overrides to complex types (when explicit typing is provided), HCL's native JSON-based unmarshalling is used. Typing is independent of any default, but if a default is provided, it will be validated. Similarly, if an override is provided, it will be converted to that type. When typing is not provided, previous behavior is used, namely passing through as a string when no default, converting to primitives if the default was primitive, and failing otherwise (complex types). For complex types, the happy path is lists of primitives, but in theory any complex/composite type can be used provided they are expressed correctly in JSON. In the interest of simplicity and correctness, there are no shortcuts for lists. There *is* a shortcut for strings as users don't provide them for untyped variables and would be unintuitive. Signed-off-by: Roberto Villarreal --- bake/hcl_test.go | 323 ++++++++++++++++++++++++++++++++++++ bake/hclparser/hclparser.go | 77 +++++++-- 2 files changed, 386 insertions(+), 14 deletions(-) diff --git a/bake/hcl_test.go b/bake/hcl_test.go index 5f899544f..120e28aae 100644 --- a/bake/hcl_test.go +++ b/bake/hcl_test.go @@ -1,6 +1,7 @@ package bake import ( + "fmt" "reflect" "regexp" "testing" @@ -1645,6 +1646,328 @@ func TestHCLIndexOfFunc(t *testing.T) { require.Empty(t, c.Targets[1].Tags[1]) } +func TestVarTypingSpec(t *testing.T) { + templ := ` + variable "FOO" { + type = %s + } + target "default" { + }` + + // not exhaustive, but the common ones + for _, s := range []string{ + "bool", "number", "string", "any", + "list(string)", "set(string)", "tuple([string, number])", + } { + dt := fmt.Sprintf(templ, s) + _, err := ParseFile([]byte(dt), "docker-bake.hcl") + require.NoError(t, err) + } + + for _, s := range []string{ + "boolean", // no synonyms/aliases + "BOOL", // case matters + `lower("bool")`, // must be literals + } { + dt := fmt.Sprintf(templ, s) + _, err := ParseFile([]byte(dt), "docker-bake.hcl") + require.ErrorContains(t, err, "not a valid type") + } +} + +func TestDefaultVarTypeEnforcement(t *testing.T) { + // To help prove a given default doesn't just pass the type check, but *is* that type, + // we use argValue to provide an expression that would work only on that type. + tests := []struct { + name string + varType string + varDefault any + argValue string + wantValue string + wantError bool + }{ + { + name: "number (happy)", + varType: "number", + varDefault: 99, + argValue: "FOO + 1", + wantValue: "100", + }, + { + name: "numeric string compatible with number", + varType: "number", + varDefault: `"99"`, + argValue: "FOO + 1", + wantValue: "100", + }, + { + name: "boolean (happy)", + varType: "bool", + varDefault: true, + argValue: "and(FOO, true)", + wantValue: "true", + }, + { + name: "numeric boolean compatible with boolean", + varType: "bool", + varDefault: `"true"`, + argValue: "and(FOO, true)", + wantValue: "true", + }, + // should be representative of flagrant primitive type mismatches; not worth listing all possibilities? + { + name: "non-numeric string default incompatible with number", + varType: "number", + varDefault: `"oops"`, + wantError: true, + }, + { + name: "list of numbers (happy)", + varType: "list(number)", + varDefault: "[2,3]", + argValue: `join("", [for v in FOO: v + 1])`, + wantValue: "34", + }, + { + name: "list of numbers with numeric strings okay", + varType: "list(number)", + varDefault: `["2","3"]`, + argValue: `join("", [for v in FOO: v + 1])`, + wantValue: "34", + }, + // represent flagrant mismatches for list types + { + name: "non-numeric strings in numeric list rejected", + varType: "list(number)", + varDefault: `["oops"]`, + wantError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + argValue := tt.argValue + if argValue == "" { + argValue = "FOO" + } + dt := fmt.Sprintf(` + variable "FOO" { + type = %s + default = %v + } + + target "default" { + args = { + foo = %s + } + }`, tt.varType, tt.varDefault, argValue) + c, err := ParseFile([]byte(dt), "docker-bake.hcl") + if tt.wantError { + require.ErrorContains(t, err, "invalid type") + } else { + require.NoError(t, err) + if tt.wantValue != "" { + require.Equal(t, 1, len(c.Targets)) + require.Equal(t, ptrstr(tt.wantValue), c.Targets[0].Args["foo"]) + } + } + }) + } +} + +func TestDefaultVarTypeWithAttrValuesEnforcement(t *testing.T) { + tests := []struct { + name string + attrValue any + varType string + wantError bool + }{ + { + name: "attribute literal which matches var type", + attrValue: `"hello"`, + varType: "string", + }, + { + name: "attribute literal which coerces to var type", + attrValue: `"99"`, + varType: "number", + }, + { + name: "mismatch", + attrValue: 99, + varType: "bool", + wantError: true, + }, + { + name: "attribute correctly typed via function", + attrValue: `split(",", "1,2,3")`, + varType: "list(number)", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dt := fmt.Sprintf(` + BAR = %v + variable "FOO" { + type = %s + default = BAR + } + + target "default" { + }`, tt.attrValue, tt.varType) + _, err := ParseFile([]byte(dt), "docker-bake.hcl") + if tt.wantError { + require.ErrorContains(t, err, "invalid type") + require.ErrorContains(t, err, "FOO default value") + } else { + require.NoError(t, err) + } + }) + } +} + +func TestTypedVarOverrides(t *testing.T) { + const convertFailure = "failed to convert FOO" + tests := []struct { + name string + varType string + override string + argValue string + wantValue string + wantErrorMsg string + }{ + { + name: "boolean", + varType: "bool", + override: "true", + wantValue: "true", + }, + { + name: "number", + varType: "number", + override: "99", + wantValue: "99", + }, + // this breaks the rule about needing proper JSON as it would violate + // the principle of least surprise and hinder usability + { + name: "enquoted string accepted", + varType: "string", + override: "hello", + wantValue: "hello", + }, + // similar to above, an environment variable with a quoted string would + // most likely be intended to be a string whose first and last characters + // are quotes + { + name: "quoted string keeps quotes in value", + varType: "string", + override: `"hello"`, + wantValue: `"hello"`, + }, + { + name: "any", + varType: "any", + override: "[1,2]", + wantValue: "[1,2]", + }, + { + name: "any never convert to complex types", + varType: "any", + override: "[1,2]", + argValue: "length(FOO)", + wantErrorMsg: "collection must be a list", + }, + { + name: "proper JSON list of strings", + varType: "list(string)", + override: `["hi","there"]`, + 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, + }, + // ditto above + { + name: "CSV of quoted strings not okay", + varType: "list(string)", + override: `"hi","there"`, + wantErrorMsg: convertFailure, + }, + // ditto above + { + name: "CSV of unquoted strings not okay", + varType: "list(string)", + override: `hi,there`, + wantErrorMsg: convertFailure, + }, + { + name: "JSON 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)", + override: `{"foo": 1, "bar": 2}`, + argValue: `join("-", sort(values(FOO)))`, + wantValue: "1-2", + }, + { + name: "invalid JSON map of numbers", + varType: "map(number)", + override: `{"foo": "oops", "bar": 2}`, + wantErrorMsg: convertFailure, + }, + { + name: "JSON object", + varType: `object({messages: list(string)})`, + override: `{"messages": ["hi", "there"]}`, + argValue: `join("-", FOO["messages"])`, + wantValue: "hi-there", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + argValue := tt.argValue + if argValue == "" { + argValue = "FOO" + } + dt := fmt.Sprintf(` + variable "FOO" { + type = %s + } + + target "default" { + args = { + foo = %s + } + }`, tt.varType, argValue) + t.Setenv("FOO", tt.override) + c, err := ParseFile([]byte(dt), "docker-bake.hcl") + if tt.wantErrorMsg != "" { + require.ErrorContains(t, err, tt.wantErrorMsg) + } else { + require.NoError(t, err) + if tt.wantValue != "" { + require.Equal(t, 1, len(c.Targets)) + require.Equal(t, ptrstr(tt.wantValue), c.Targets[0].Args["foo"]) + } + } + }) + } +} + func ptrstr(s any) *string { var n *string if reflect.ValueOf(s).Kind() == reflect.String { diff --git a/bake/hclparser/hclparser.go b/bake/hclparser/hclparser.go index 1628e7c0d..2ded19216 100644 --- a/bake/hclparser/hclparser.go +++ b/bake/hclparser/hclparser.go @@ -14,9 +14,11 @@ import ( "github.com/docker/buildx/bake/hclparser/gohcl" "github.com/docker/buildx/util/userfunc" "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/hcl/v2/ext/typeexpr" "github.com/pkg/errors" "github.com/zclconf/go-cty/cty" "github.com/zclconf/go-cty/cty/convert" + ctyjson "github.com/zclconf/go-cty/cty/json" ) type Opt struct { @@ -27,6 +29,7 @@ type Opt struct { type variable struct { Name string `json:"-" hcl:"name,label"` + Type hcl.Expression `json:"type,omitempty" hcl:"type,optional"` Default *hcl.Attribute `json:"default,omitempty" hcl:"default,optional"` Description string `json:"description,omitempty" hcl:"description,optional"` Validations []*variableValidation `json:"validation,omitempty" hcl:"validation,block"` @@ -267,38 +270,68 @@ func (p *parser) resolveValue(ectx *hcl.EvalContext, name string) (err error) { } }() + // built-in vars aren't intended to be overridden and are statically typed as strings; + // no sense sending them through type checks or waiting to return them + if val, ok := p.opt.Vars[name]; ok { + vv := cty.StringVal(val) + v = &vv + return + } + + var diags hcl.Diagnostics + varType := cty.DynamicPseudoType def, ok := p.attrs[name] - if _, builtin := p.opt.Vars[name]; !ok && !builtin { + if !ok { vr, ok := p.vars[name] if !ok { return errors.Wrapf(errUndefined{}, "variable %q does not exist", name) } def = vr.Default ectx = p.ectx + varType, diags = typeConstraint(vr.Type) + if diags.HasErrors() { + return diags + } } if def == nil { - val, ok := p.opt.Vars[name] - if !ok { - val, _ = p.opt.LookupVar(name) + // lack of specified value is considered to have an empty string value, + // but any overrides get type checked + if _, ok := p.opt.LookupVar(name); !ok { + vv := cty.StringVal("") + v = &vv + return } - vv := cty.StringVal(val) - v = &vv - return } - if diags := p.loadDeps(ectx, def.Expr, nil, true); diags.HasErrors() { - return diags - } - vv, diags := def.Expr.Value(ectx) - if diags.HasErrors() { - return diags + var vv cty.Value + if def != nil { + if diags := p.loadDeps(ectx, def.Expr, nil, true); diags.HasErrors() { + return diags + } + vv, diags = def.Expr.Value(ectx) + if diags.HasErrors() { + return diags + } + vv, err = convert.Convert(vv, varType) + if err != nil { + return errors.Wrapf(err, "invalid type %s for variable %s default value", varType.FriendlyName(), name) + } } _, isVar := p.vars[name] if envv, ok := p.opt.LookupVar(name); ok && isVar { 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.Equals(cty.DynamicPseudoType): // typing was explicitly specified + vv, err = ctyjson.Unmarshal([]byte(envv), varType) + if err != nil { + return errors.Wrapf(err, "failed to convert %s as required %s", name, varType.FriendlyName()) + } + 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 { @@ -317,7 +350,6 @@ func (p *parser) resolveValue(ectx *hcl.EvalContext, name string) (err error) { } vv = cty.NumberVal(big.NewFloat(n)) default: - // TODO: support lists with csv values return errors.Errorf("unsupported type %s for variable %s", vv.Type().FriendlyName(), name) } } @@ -907,6 +939,23 @@ func Parse(b hcl.Body, opt Opt, val any) (*ParseMeta, hcl.Diagnostics) { }, nil } +// typeConstraint wraps typeexpr.TypeConstraint to differentiate between errors in the +// specification and errors due to being cty.NullVal (not provided). +func typeConstraint(expr hcl.Expression) (cty.Type, hcl.Diagnostics) { + t, diag := typeexpr.TypeConstraint(expr) + if !diag.HasErrors() { + return t, diag + } + // if had errors, it could be because the expression is 'nil', i.e., unspecified + if v, err := expr.Value(nil); err == nil { + if v.IsNull() { + return cty.DynamicPseudoType, nil + } + } + // even if the evaluation resulted in error, the original (error) diagnostics are likely more useful + return t, diag +} + // 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 { From 1f569846268807847fbe8c4449cd821c1306f419 Mon Sep 17 00:00:00 2001 From: Roberto Villarreal Date: Mon, 5 May 2025 23:47:02 -0600 Subject: [PATCH 2/5] 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 --- bake/hcl_test.go | 97 ++++++++++++++++++------ bake/hclparser/hclparser.go | 142 ++++++++++++++++++++++++++++++++---- 2 files changed, 204 insertions(+), 35 deletions(-) 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 { From 956fc0c9eb3cf8cb7a39ea0be6f3a815dfcaf3af Mon Sep 17 00:00:00 2001 From: Roberto Villarreal Date: Wed, 7 May 2025 21:16:00 -0600 Subject: [PATCH 3/5] Use unique environment variables to separate JSON from default parsing The primary intent is to make JSON parsing explicitly opt-in rather than using heuristics to determine intent. With some exceptions, given bake variable `VAR`, an environment variable `VAR_JSON` must be used to provide JSON content. The value in `VAR_JSON` will be ignored when: * a bake built-in of that same name exists * a user-provided variable of that same name exists * typing (attribute `type`) is not present The first is unlikely to happen as built-ins will likely start with `BUILDX_BAKE_`, an unlikely prefix for end users. The second may be a real scenario, where users have `VAR_JSON` dedicated to accepting a string with JSON content and decoding via an HCL function. This will continue to work as-is, but can be simplified by removing the variable from their bake file (`VAR_JSON`) and applying typing (to `VAR`). Signed-off-by: Roberto Villarreal --- bake/hcl_test.go | 433 ++++++++++++++++++++++++++++++------ bake/hclparser/hclparser.go | 65 ++++-- 2 files changed, 420 insertions(+), 78 deletions(-) diff --git a/bake/hcl_test.go b/bake/hcl_test.go index 698de620d..af6dff1b2 100644 --- a/bake/hcl_test.go +++ b/bake/hcl_test.go @@ -1840,6 +1840,8 @@ func TestDefaultVarTypeWithAttrValuesEnforcement(t *testing.T) { func TestTypedVarOverrides(t *testing.T) { const unsuitableValueType = "Unsuitable value type" + const unsupportedType = "unsupported type" + const failedToParseElement = "failed to parse element" tests := []struct { name string varType string @@ -1860,17 +1862,14 @@ func TestTypedVarOverrides(t *testing.T) { override: "99", wantValue: "99", }, - // this breaks the rule about needing proper JSON as it would violate - // the principle of least surprise and hinder usability { - name: "enquoted string accepted", + name: "unquoted string accepted", varType: "string", override: "hello", wantValue: "hello", }, - // similar to above, an environment variable with a quoted string would - // most likely be intended to be a string whose first and last characters - // are quotes + // an environment variable with a quoted string would most likely be intended + // to be a string whose first and last characters are quotes { name: "quoted string keeps quotes in value", varType: "string", @@ -1890,13 +1889,6 @@ func TestTypedVarOverrides(t *testing.T) { argValue: "length(FOO)", wantErrorMsg: "collection must be a list", }, - { - name: "proper JSON list of strings", - varType: "list(string)", - override: `["hi","there"]`, - argValue: `join("-", FOO)`, - wantValue: "hi-there", - }, { name: "proper CSV list of strings", varType: "list(string)", @@ -1904,15 +1896,6 @@ func TestTypedVarOverrides(t *testing.T) { argValue: `join("-", FOO)`, wantValue: "hi-there", }, - // 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: "pseudo-JSON list of unquoted strings", - varType: "list(string)", - override: `[hi,there]`, - argValue: `join("-", FOO)`, - wantValue: "[hi-there]", - }, { name: "CSV of unquoted strings okay", varType: "list(string)", @@ -1920,13 +1903,6 @@ func TestTypedVarOverrides(t *testing.T) { argValue: `join("-", FOO)`, wantValue: "hi-there", }, - { - name: "JSON list of numbers", - varType: "list(number)", - override: "[3, 1, 4]", - argValue: `join("-", [for v in FOO: v + 1])`, - wantValue: "4-2-5", - }, { name: "CSV list of numbers", varType: "list(number)", @@ -1935,11 +1911,12 @@ func TestTypedVarOverrides(t *testing.T) { wantValue: "4-2-5", }, { - name: "JSON map of numbers", - varType: "map(number)", - override: `{"foo": 1, "bar": 2}`, - argValue: `join("-", sort(values(FOO)))`, - wantValue: "1-2", + name: "CSV set of numbers", + varType: "set(number)", + override: "3,1,4", + // anecdotally sets are sorted but may not be guaranteed + argValue: `join("-", [for v in sort(FOO): v + 1])`, + wantValue: "2-4-5", }, { name: "CSV map of numbers", @@ -1948,22 +1925,6 @@ func TestTypedVarOverrides(t *testing.T) { 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])", @@ -1971,12 +1932,6 @@ func TestTypedVarOverrides(t *testing.T) { 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])", @@ -1984,11 +1939,87 @@ func TestTypedVarOverrides(t *testing.T) { wantErrorMsg: unsuitableValueType, }, { - name: "JSON object", - varType: `object({messages: list(string)})`, - override: `{"messages": ["hi", "there"]}`, - argValue: `join("-", FOO["messages"])`, - wantValue: "hi-there", + name: "invalid CSV value", + varType: "list(string)", + override: `"hello,world`, + wantErrorMsg: "from CSV", + }, + { + name: "object not supported", + varType: "object({message: string})", + override: "does not matter", + wantErrorMsg: unsupportedType, + }, + { + name: "list of non-primitives not supported", + varType: "list(list(number))", + override: "1,2", + wantErrorMsg: unsupportedType, + }, + { + name: "set of non-primitives not supported", + varType: "set(set(number))", + override: "1,2", + wantErrorMsg: unsupportedType, + }, + { + name: "tuple of non-primitives not supported", + varType: "tuple([list(number)])", + // Intentionally a different override than other similar tests; tuple is unique in that + // multiple types are involved and length matters. In the real world, it's probably more + // likely a user would accidentally omit or add an item than trying to use non-primitives, + // so the length check comes first. + override: "1", + wantErrorMsg: unsupportedType, + }, + { + name: "map of non-primitives not supported", + varType: "map(list(number))", + override: "foo:1,2", + wantErrorMsg: unsupportedType, + }, + { + name: "invalid map k/v parsing", + varType: "map(string)", + // TODO fragile; will fail in a different manner without first k/v pair + override: `a:b,foo:"bar`, + wantErrorMsg: "as CSV", + }, + { + name: "list with invalidly parsed elements", + varType: "list(number)", + override: "1,1z", + wantErrorMsg: failedToParseElement, + }, + { + name: "set with invalidly parsed elements", + varType: "set(number)", + override: "1,1z", + wantErrorMsg: failedToParseElement, + }, + { + name: "tuple with invalidly parsed elements", + varType: "tuple([number])", + override: "1z", + wantErrorMsg: failedToParseElement, + }, + { + name: "map with invalidly parsed elements", + varType: "map(number)", + override: "foo:1z", + wantErrorMsg: failedToParseElement, + }, + { + name: "map with bad value format", + varType: "map(number)", + override: "foo:1:1", + wantErrorMsg: "expected one k/v pair", + }, + { + name: "primitive with bad value format", + varType: "number", + override: "1z", + wantErrorMsg: "failed to parse", }, } @@ -2016,13 +2047,291 @@ func TestTypedVarOverrides(t *testing.T) { require.NoError(t, err) if tt.wantValue != "" { require.Equal(t, 1, len(c.Targets)) - require.Equal(t, ptrstr(tt.wantValue), c.Targets[0].Args["foo"]) + require.Equal(t, tt.wantValue, *c.Targets[0].Args["foo"]) } } }) } } +func TestTypedVarOverrides_JSON(t *testing.T) { + const unsuitableValueType = "Unsuitable value type" + tests := []struct { + name string + varType string + override string + argValue string + wantValue string + wantErrorMsg string + }{ + { + name: "boolean", + varType: "bool", + override: "true", + wantValue: "true", + }, + { + name: "number", + varType: "number", + override: "99", + wantValue: "99", + }, + // no shortcuts in JSON mode + { + name: "unquoted string is error", + varType: "string", + override: "hello", + wantErrorMsg: "from JSON", + }, + { + name: "string", + varType: "string", + override: `"hello"`, + wantValue: "hello", + }, + { + name: "any", + varType: "any", + override: "[1,2]", + wantValue: "[1,2]", + }, + { + name: "any never convert to complex types", + varType: "any", + override: "[1,2]", + argValue: "length(FOO)", + wantErrorMsg: "collection must be a list", + }, + { + name: "list of strings", + varType: "list(string)", + override: `["hi","there"]`, + argValue: `join("-", FOO)`, + wantValue: "hi-there", + }, + { + name: "list of numbers", + varType: "list(number)", + override: "[3, 1, 4]", + argValue: `join("-", [for v in FOO: v + 1])`, + wantValue: "4-2-5", + }, + { + name: "map of numbers", + varType: "map(number)", + override: `{"foo": 1, "bar": 2}`, + argValue: `join("-", sort(values(FOO)))`, + wantValue: "1-2", + }, + { + 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: "from JSON", + }, + { + name: "tuple", + varType: "tuple([number,string])", + override: `[99, "bottles"]`, + argValue: `format("%d %s", FOO[0], FOO[1])`, + wantValue: "99 bottles", + }, + { + name: "tuple elements with wrong type", + varType: "tuple([number,string])", + override: `[99, 100]`, + wantErrorMsg: unsuitableValueType, + }, + { + name: "JSON object", + varType: `object({messages: list(string)})`, + override: `{"messages": ["hi", "there"]}`, + argValue: `join("-", FOO["messages"])`, + wantValue: "hi-there", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + argValue := tt.argValue + if argValue == "" { + argValue = "FOO" + } + dt := fmt.Sprintf(` + variable "FOO" { + type = %s + } + + target "default" { + args = { + foo = %s + } + }`, tt.varType, argValue) + t.Setenv("FOO_JSON", tt.override) + c, err := ParseFile([]byte(dt), "docker-bake.hcl") + if tt.wantErrorMsg != "" { + require.ErrorContains(t, err, tt.wantErrorMsg) + } else { + require.NoError(t, err) + if tt.wantValue != "" { + require.Equal(t, 1, len(c.Targets)) + require.Equal(t, tt.wantValue, *c.Targets[0].Args["foo"]) + } + } + }) + } +} + +func TestJSONOverridePriority(t *testing.T) { + t.Run("JSON override ignored when same user var exists", func(t *testing.T) { + dt := []byte(` + variable "FOO" { + type = list(number) + } + variable "FOO_JSON" { + type = list(number) + } + + target "default" { + args = { + foo = FOO + } + }`) + // env FOO_JSON is the CSV override of var FOO_JSON, not a JSON override of FOO + t.Setenv("FOO", "[1,2]") + t.Setenv("FOO_JSON", "[3,4]") + _, err := ParseFile(dt, "docker-bake.hcl") + require.ErrorContains(t, err, "failed to convert") + require.ErrorContains(t, err, "from CSV") + }) + + t.Run("JSON override ignored when same builtin var exists", func(t *testing.T) { + dt := []byte(` + variable "FOO" { + type = list(number) + } + + target "default" { + args = { + foo = length(FOO) + } + }`) + t.Setenv("FOO", "1,2") + t.Setenv("FOO_JSON", "[3,4,5]") + c, _, err := ParseFiles( + []File{{Name: "docker-bake.hcl", Data: dt}}, + map[string]string{"FOO_JSON": "whatever"}, + ) + require.NoError(t, err) + require.Equal(t, 1, len(c.Targets)) + require.Equal(t, "2", *c.Targets[0].Args["foo"]) + }) + + // this is implied/exercised in other tests, but repeated for completeness + t.Run("JSON override ignored if var is untyped", func(t *testing.T) { + dt := []byte(` + variable "FOO" { + default = [1, 2] + } + + target "default" { + args = { + foo = length(FOO) + } + }`) + t.Setenv("FOO_JSON", "[3,4]") + _, err := ParseFile(dt, "docker-bake.hcl") + require.ErrorContains(t, err, "unsupported type") + }) + + t.Run("override-ish variable has regular CSV override", func(t *testing.T) { + dt := []byte(` + variable "FOO_JSON" { + type = list(number) + } + + target "default" { + args = { + foo = length(FOO_JSON) + } + }`) + // despite the name, it's still CSV + t.Setenv("FOO_JSON", "10,11,12") + c, err := ParseFile(dt, "docker-bake.hcl") + require.NoError(t, err) + require.Equal(t, 1, len(c.Targets)) + require.Equal(t, "3", *c.Targets[0].Args["foo"]) + + t.Setenv("FOO_JSON", "[10,11,12]") + _, err = ParseFile(dt, "docker-bake.hcl") + require.ErrorContains(t, err, "from CSV") + }) + + t.Run("override-ish variable has own JSON override", func(t *testing.T) { + dt := []byte(` + variable "FOO_JSON" { + type = list(number) + } + + target "default" { + args = { + foo = length(FOO_JSON) + } + }`) + t.Setenv("FOO_JSON_JSON", "[4,5,6]") + c, err := ParseFile(dt, "docker-bake.hcl") + require.NoError(t, err) + require.Equal(t, 1, len(c.Targets)) + require.Equal(t, "3", *c.Targets[0].Args["foo"]) + }) + + t.Run("JSON override trumps CSV when no var name conflict", func(t *testing.T) { + dt := []byte(` + variable "FOO" { + type = list(number) + } + + target "default" { + args = { + foo = length(FOO) + } + }`) + t.Setenv("FOO", "1,2") + t.Setenv("FOO_JSON", "[3,4,5]") + c, err := ParseFile(dt, "docker-bake.hcl") + require.NoError(t, err) + require.Equal(t, 1, len(c.Targets)) + require.Equal(t, "3", *c.Targets[0].Args["foo"]) + }) + + t.Run("JSON override works with lowercase vars", func(t *testing.T) { + dt := []byte(` + variable "foo" { + type = number + } + + target "default" { + args = { + bar = foo + } + }`) + // may seem reasonable, but not supported + t.Setenv("foo_json", "9000") + c, err := ParseFile(dt, "docker-bake.hcl") + require.NoError(t, err) + require.Equal(t, 1, len(c.Targets)) + // a variable with no value has always resulted in an empty string + require.Equal(t, "", *c.Targets[0].Args["bar"]) + + t.Setenv("foo_JSON", "42") + c, err = ParseFile(dt, "docker-bake.hcl") + require.NoError(t, err) + require.Equal(t, 1, len(c.Targets)) + require.Equal(t, "42", *c.Targets[0].Args["bar"]) + }) +} + func ptrstr(s any) *string { var n *string if reflect.ValueOf(s).Kind() == reflect.String { diff --git a/bake/hclparser/hclparser.go b/bake/hclparser/hclparser.go index 13caf8e3f..5dbd0bc78 100644 --- a/bake/hclparser/hclparser.go +++ b/bake/hclparser/hclparser.go @@ -22,6 +22,8 @@ import ( ctyjson "github.com/zclconf/go-cty/cty/json" ) +const jsonEnvOverrideSuffix = "_JSON" + type Opt struct { LookupVar func(string) (string, bool) Vars map[string]string @@ -298,7 +300,7 @@ func (p *parser) resolveValue(ectx *hcl.EvalContext, name string) (err error) { if def == nil { // lack of specified value is considered to have an empty string value, // but any overrides get type checked - if _, ok := p.opt.LookupVar(name); !ok { + if _, ok, _ := p.valueHasOverride(name, false); !ok { vv := cty.StringVal("") v = &vv return @@ -320,32 +322,37 @@ func (p *parser) resolveValue(ectx *hcl.EvalContext, name string) (err error) { } } + // Not entirely true... this doesn't differentiate between a user that specified 'any' + // and a user that specified nothing. But the result is the same; both are treated as strings. + typeSpecified := !varType.Equals(cty.DynamicPseudoType) + envv, hasEnv, jsonEnv := p.valueHasOverride(name, typeSpecified) _, isVar := p.vars[name] - if envv, ok := p.opt.LookupVar(name); ok && isVar { + if hasEnv && isVar { 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 + case typeSpecified && jsonEnv: 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) - } + return errors.Wrapf(err, "failed to convert variable %s from JSON", name) } - case !varType.Equals(cty.DynamicPseudoType): // typing was explicitly specified - vv, err = ctyjson.Unmarshal([]byte(envv), varType) + case supportedCSVType(varType): // typing explicitly specified for selected complex types + vv, err = valueFromCSV(name, envv, varType) if err != nil { - return errors.Wrapf(err, "failed to convert %s as required %s", name, varType.FriendlyName()) + return errors.Wrapf(err, "failed to convert variable %s from CSV", name) } + case typeSpecified && varType.IsPrimitiveType(): + vv, err = convertPrimitive(name, envv, varType) + if err != nil { + return err + } + case typeSpecified: + // e.g., an 'object' not provided as JSON (which can't be expressed in the default CSV format) + return errors.Errorf("unsupported type %s for variable %s", varType.FriendlyName(), name) case def == nil: // no default from which to infer typing vv = cty.StringVal(envv) case vv.Type().Equals(cty.DynamicPseudoType): vv = cty.StringVal(envv) - case vv.Type().Equals(cty.Bool), vv.Type().Equals(cty.String), vv.Type().Equals(cty.Number): + case vv.Type().IsPrimitiveType(): vv, err = convertPrimitive(name, envv, vv.Type()) if err != nil { return err @@ -358,6 +365,27 @@ func (p *parser) resolveValue(ectx *hcl.EvalContext, name string) (err error) { return nil } +// valueHasOverride returns a possible override value if one was specified, and whether it should +// be treated as a JSON value. +// +// A plain/CSV override is the default; this consolidates the logic around how a JSON-specific override +// is specified and when it will be honored when there are naming conflicts or ambiguity. +func (p *parser) valueHasOverride(name string, favorJSON bool) (string, bool, bool) { + jsonEnv := false + envv, hasEnv := p.opt.LookupVar(name) + if !hasEnv || favorJSON { + jsonVarName := name + jsonEnvOverrideSuffix + _, builtin := p.opt.Vars[jsonVarName] + if _, ok := p.vars[jsonVarName]; !ok && !builtin { + if j, ok := p.opt.LookupVar(jsonVarName); ok { + envv = j + hasEnv, jsonEnv = true, true + } + } + } + return envv, hasEnv, jsonEnv +} + // resolveBlock force evaluates a block, storing the result in the parser. If a // target schema is provided, only the attributes and blocks present in the // schema will be evaluated. @@ -982,6 +1010,11 @@ func convertPrimitive(name, value string, target cty.Type) (cty.Value, error) { } } +// supportedCSVType reports whether the given cty.Type might be convertible from a CSV string via valueFromCSV. +func supportedCSVType(t cty.Type) bool { + return t.IsListType() || t.IsSetType() || t.IsTupleType() || t.IsMapType() +} + // valueFromCSV takes CSV value and converts it to cty.Type. // // This currently supports conversion to cty.List and cty.Set. @@ -1060,7 +1093,7 @@ func valueFromCSV(name, value string, target cty.Type) (cty.Value, error) { } 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()) + return cty.NilVal, errors.Wrapf(err, "failed to parse element from type %s", target.FriendlyName()) } m[kvSlice[0]] = v } From 65aea3028fdd66c70cef91ed96c060e03c393381 Mon Sep 17 00:00:00 2001 From: Roberto Villarreal Date: Fri, 9 May 2025 18:29:14 -0600 Subject: [PATCH 4/5] Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Roberto Villarreal --- bake/hclparser/hclparser.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/bake/hclparser/hclparser.go b/bake/hclparser/hclparser.go index 5dbd0bc78..16ffc4546 100644 --- a/bake/hclparser/hclparser.go +++ b/bake/hclparser/hclparser.go @@ -373,6 +373,8 @@ func (p *parser) resolveValue(ectx *hcl.EvalContext, name string) (err error) { func (p *parser) valueHasOverride(name string, favorJSON bool) (string, bool, bool) { jsonEnv := false envv, hasEnv := p.opt.LookupVar(name) + // If no plain override exists (!hasEnv) or JSON overrides are explicitly favored (favorJSON), + // check for a JSON-specific override with the "_JSON" suffix. if !hasEnv || favorJSON { jsonVarName := name + jsonEnvOverrideSuffix _, builtin := p.opt.Vars[jsonVarName] @@ -1086,7 +1088,7 @@ func valueFromCSV(name, value string, target cty.Type) (cty.Value, error) { 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) + return cty.NilVal, errors.Wrapf(err, "failed to parse %s as k/v for variable %s", f, name) } if len(kvSlice) != 2 { return cty.NilVal, errors.Errorf("expected one k/v pair but got %d pieces from %s", len(kvSlice), f) From 56d39e619dfdce9b8205bcbccb7d9267feaf1f3a Mon Sep 17 00:00:00 2001 From: Roberto Villarreal Date: Fri, 9 May 2025 18:20:51 -0600 Subject: [PATCH 5/5] Skip case-sensitive test on Windows Signed-off-by: Roberto Villarreal --- bake/hcl_test.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/bake/hcl_test.go b/bake/hcl_test.go index af6dff1b2..88972e8c6 100644 --- a/bake/hcl_test.go +++ b/bake/hcl_test.go @@ -4,6 +4,7 @@ import ( "fmt" "reflect" "regexp" + "runtime" "testing" hcl "github.com/hashicorp/hcl/v2" @@ -2306,6 +2307,9 @@ func TestJSONOverridePriority(t *testing.T) { }) t.Run("JSON override works with lowercase vars", func(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("Windows case-insensitivity") + } dt := []byte(` variable "foo" { type = number @@ -2316,7 +2320,7 @@ func TestJSONOverridePriority(t *testing.T) { bar = foo } }`) - // may seem reasonable, but not supported + // may seem reasonable, but not supported (on case-sensitive systems) t.Setenv("foo_json", "9000") c, err := ParseFile(dt, "docker-bake.hcl") require.NoError(t, err)