From 3c0f5c5c21155879ae68859a8771601a44e308ff Mon Sep 17 00:00:00 2001 From: Roberto Villarreal Date: Tue, 20 May 2025 13:09:13 -0600 Subject: [PATCH 1/2] Consider typed, value-less variables to have `null` value A variable with a type but no default value or override resulted in an empty string. This matches the legacy behavior of untyped variables, but does not make sense when using types (an empty string is itself a type violation for everything except `string`). All variables defined with a type but with no value are now a typed `null`. A variable explicitly typed `any` was previously treated as if the typing was omitted; with no defined value or override, that resulted in an empty string. The `any` type is now distinguished from an omitted type; these variables, with no default or override, are also `null`. In other respects, the behavior of `any` is unchanged and largely behaves as if the type was omitted. It's not clear whether it should be supported, let alone how it should behave, so these tests were removed. It's being treated as undefined behavior. Signed-off-by: Roberto Villarreal --- bake/hcl_test.go | 87 +++++++++++++++++++++++++------------ bake/hclparser/hclparser.go | 12 +++-- 2 files changed, 64 insertions(+), 35 deletions(-) diff --git a/bake/hcl_test.go b/bake/hcl_test.go index 88972e8c686c..ee52352e6cae 100644 --- a/bake/hcl_test.go +++ b/bake/hcl_test.go @@ -423,6 +423,63 @@ func TestHCLNullVariables(t *testing.T) { require.Equal(t, ptrstr("bar"), c.Targets[0].Args["foo"]) } +func TestHCLTypedNullVariables(t *testing.T) { + types := []string{ + "any", + "string", "number", "bool", + "list(string)", "set(string)", "map(string)", + "tuple([string])", "object({val: string})", + } + for _, varType := range types { + tName := fmt.Sprintf("variable typed %q with null default remains null", varType) + t.Run(tName, func(t *testing.T) { + dt := fmt.Sprintf(` + variable "FOO" { + type = %s + default = null + } + + target "default" { + args = { + foo = equal(FOO, null) + } + }`, varType) + c, err := ParseFile([]byte(dt), "docker-bake.hcl") + require.NoError(t, err) + require.Equal(t, 1, len(c.Targets)) + require.Equal(t, "true", *c.Targets[0].Args["foo"]) + }) + } +} + +func TestHCLTypedValuelessVariables(t *testing.T) { + types := []string{ + "any", + "string", "number", "bool", + "list(string)", "set(string)", "map(string)", + "tuple([string])", "object({val: string})", + } + for _, varType := range types { + tName := fmt.Sprintf("variable typed %q with no default is null", varType) + t.Run(tName, func(t *testing.T) { + dt := fmt.Sprintf(` + variable "FOO" { + type = %s + } + + target "default" { + args = { + foo = equal(FOO, null) + } + }`, varType) + c, err := ParseFile([]byte(dt), "docker-bake.hcl") + require.NoError(t, err) + require.Equal(t, 1, len(c.Targets)) + require.Equal(t, "true", *c.Targets[0].Args["foo"]) + }) + } +} + func TestJSONNullVariables(t *testing.T) { dt := []byte(`{ "variable": { @@ -1877,19 +1934,6 @@ func TestTypedVarOverrides(t *testing.T) { 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 CSV list of strings", varType: "list(string)", @@ -2090,19 +2134,6 @@ func TestTypedVarOverrides_JSON(t *testing.T) { 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)", @@ -2313,6 +2344,7 @@ func TestJSONOverridePriority(t *testing.T) { dt := []byte(` variable "foo" { type = number + default = 101 } target "default" { @@ -2325,8 +2357,7 @@ func TestJSONOverridePriority(t *testing.T) { 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"]) + require.Equal(t, "101", *c.Targets[0].Args["bar"]) t.Setenv("foo_JSON", "42") c, err = ParseFile(dt, "docker-bake.hcl") diff --git a/bake/hclparser/hclparser.go b/bake/hclparser/hclparser.go index d132e96618f9..d8f134590ebd 100644 --- a/bake/hclparser/hclparser.go +++ b/bake/hclparser/hclparser.go @@ -282,7 +282,7 @@ func (p *parser) resolveValue(ectx *hcl.EvalContext, name string) (err error) { } var diags hcl.Diagnostics - varType := cty.DynamicPseudoType + varType, typeSpecified := cty.DynamicPseudoType, false def, ok := p.attrs[name] if !ok { vr, ok := p.vars[name] @@ -295,12 +295,13 @@ func (p *parser) resolveValue(ectx *hcl.EvalContext, name string) (err error) { if diags.HasErrors() { return diags } + typeSpecified = !varType.Equals(cty.DynamicPseudoType) || hcl.ExprAsKeyword(vr.Type) == "any" } if def == nil { - // lack of specified value is considered to have an empty string value, - // but any overrides get type checked - if _, ok, _ := p.valueHasOverride(name, false); !ok { + // Lack of specified value, when untyped is considered to have an empty string value. + // A typed variable with no value will result in (typed) nil. + if _, ok, _ := p.valueHasOverride(name, false); !ok && !typeSpecified { vv := cty.StringVal("") v = &vv return @@ -322,9 +323,6 @@ 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] From 291c3535757c58eb562beb2c1784f3e1a5eea539 Mon Sep 17 00:00:00 2001 From: CrazyMax <1951866+crazy-max@users.noreply.github.com> Date: Wed, 21 May 2025 16:15:30 +0200 Subject: [PATCH 2/2] bake: TestEmptyVariable Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com> --- bake/hcl_test.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/bake/hcl_test.go b/bake/hcl_test.go index ee52352e6cae..db1ca9c6cf73 100644 --- a/bake/hcl_test.go +++ b/bake/hcl_test.go @@ -1622,6 +1622,20 @@ target "two" { require.Equal(t, map[string]*string{"b": ptrstr("pre-jkl")}, c.Targets[1].Args) } +func TestEmptyVariable(t *testing.T) { + dt := []byte(` + variable "FOO" {} + target "default" { + args = { + foo = equal(FOO, "") + } + }`) + c, err := ParseFile(dt, "docker-bake.hcl") + require.NoError(t, err) + require.Equal(t, 1, len(c.Targets)) + require.Equal(t, "true", *c.Targets[0].Args["foo"]) +} + func TestEmptyVariableJSON(t *testing.T) { dt := []byte(`{ "variable": {