Skip to content

Commit dc8a2b0

Browse files
jedevcjedevc
committed
bake: avoid early-exit for resolution failures
With changes made to allow lazy evaluation, we were early exiting if an undefined name was detected, either for a variable or a function. This had two key implications: 1. The error messages changed, and became significantly less informative. For example, we went from: > Unknown variable; There is no variable named "FO". Did you mean "FOO"?, and 1 other diagnostic(s) To > Invalid expression; undefined variable "FO" 2. Any issues in our function detection from funcCalls which cause JSON functions to be erroneously detected cause invalid functions to be resolved, which causes new name resolution errors. To avoid the above problems, we can defer the error from an undefined name until HCL evaluation - which produces the more informative errors, and does not suffer from incorrectly detecting JSON functions. Signed-off-by: Justin Chadwell <[email protected]>
1 parent d9780e2 commit dc8a2b0

2 files changed

Lines changed: 35 additions & 6 deletions

File tree

‎bake/hcl_test.go‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -670,6 +670,24 @@ func TestJSONFunctions(t *testing.T) {
670670
require.Equal(t, ptrstr("pre-<FOO-abc>"), c.Targets[0].Args["v1"])
671671
}
672672

673+
func TestJSONInvalidFunctions(t *testing.T) {
674+
dt := []byte(`{
675+
"target": {
676+
"app": {
677+
"args": {
678+
"v1": "myfunc(\"foo\")"
679+
}
680+
}
681+
}}`)
682+
683+
c, err := ParseFile(dt, "docker-bake.json")
684+
require.NoError(t, err)
685+
686+
require.Equal(t, 1, len(c.Targets))
687+
require.Equal(t, c.Targets[0].Name, "app")
688+
require.Equal(t, ptrstr(`myfunc("foo")`), c.Targets[0].Args["v1"])
689+
}
690+
673691
func TestHCLFunctionInAttr(t *testing.T) {
674692
dt := []byte(`
675693
function "brace" {

‎bake/hclparser/hclparser.go‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -62,14 +62,19 @@ type parser struct {
6262
doneB map[*hcl.Block]map[string]struct{}
6363
}
6464

65-
func (p *parser) loadDeps(exp hcl.Expression, exclude map[string]struct{}) hcl.Diagnostics {
65+
var errUndefined = errors.New("undefined")
66+
67+
func (p *parser) loadDeps(exp hcl.Expression, exclude map[string]struct{}, allowMissing bool) hcl.Diagnostics {
6668
fns, hcldiags := funcCalls(exp)
6769
if hcldiags.HasErrors() {
6870
return hcldiags
6971
}
7072

7173
for _, fn := range fns {
7274
if err := p.resolveFunction(fn); err != nil {
75+
if allowMissing && errors.Is(err, errUndefined) {
76+
continue
77+
}
7378
return hcl.Diagnostics{
7479
&hcl.Diagnostic{
7580
Severity: hcl.DiagError,
@@ -128,6 +133,9 @@ func (p *parser) loadDeps(exp hcl.Expression, exclude map[string]struct{}) hcl.D
128133
}
129134
}
130135
if err := p.resolveBlock(blocks[0], target); err != nil {
136+
if allowMissing && errors.Is(err, errUndefined) {
137+
continue
138+
}
131139
return hcl.Diagnostics{
132140
&hcl.Diagnostic{
133141
Severity: hcl.DiagError,
@@ -140,6 +148,9 @@ func (p *parser) loadDeps(exp hcl.Expression, exclude map[string]struct{}) hcl.D
140148
}
141149
} else {
142150
if err := p.resolveValue(v.RootName()); err != nil {
151+
if allowMissing && errors.Is(err, errUndefined) {
152+
continue
153+
}
143154
return hcl.Diagnostics{
144155
&hcl.Diagnostic{
145156
Severity: hcl.DiagError,
@@ -167,7 +178,7 @@ func (p *parser) resolveFunction(name string) error {
167178
if _, ok := p.ectx.Functions[name]; ok {
168179
return nil
169180
}
170-
return errors.Errorf("undefined function %s", name)
181+
return errors.Wrapf(errUndefined, "function %q does not exit", name)
171182
}
172183
if _, ok := p.progressF[name]; ok {
173184
return errors.Errorf("function cycle not allowed for %s", name)
@@ -217,7 +228,7 @@ func (p *parser) resolveFunction(name string) error {
217228
return diags
218229
}
219230

220-
if diags := p.loadDeps(f.Result.Expr, params); diags.HasErrors() {
231+
if diags := p.loadDeps(f.Result.Expr, params, false); diags.HasErrors() {
221232
return diags
222233
}
223234

@@ -255,7 +266,7 @@ func (p *parser) resolveValue(name string) (err error) {
255266
if _, builtin := p.opt.Vars[name]; !ok && !builtin {
256267
vr, ok := p.vars[name]
257268
if !ok {
258-
return errors.Errorf("undefined variable %q", name)
269+
return errors.Wrapf(errUndefined, "variable %q does not exit", name)
259270
}
260271
def = vr.Default
261272
}
@@ -270,7 +281,7 @@ func (p *parser) resolveValue(name string) (err error) {
270281
return
271282
}
272283

273-
if diags := p.loadDeps(def.Expr, nil); diags.HasErrors() {
284+
if diags := p.loadDeps(def.Expr, nil, true); diags.HasErrors() {
274285
return diags
275286
}
276287
vv, diags := def.Expr.Value(p.ectx)
@@ -395,7 +406,7 @@ func (p *parser) resolveBlock(block *hcl.Block, target *hcl.BodySchema) (err err
395406
return diag
396407
}
397408
for _, a := range content.Attributes {
398-
diag := p.loadDeps(a.Expr, nil)
409+
diag := p.loadDeps(a.Expr, nil, true)
399410
if diag.HasErrors() {
400411
return diag
401412
}

0 commit comments

Comments
 (0)