bake: additional validation for matrixes

This adds the following constraints to the new features:
- Explicit renaming with the `name` property is *only* permitted when
  used with the `matrix` property.
- Group does not support either `name` or `matrix` (we may choose to
  relax this constraint over time).
- All generated names must be unique.

Signed-off-by: Justin Chadwell <me@jedevc.com>

bake/bake.go

@@ -24,6 +24,7 @@ import (
 	"github.com/moby/buildkit/session/auth/authprovider"
 	"github.com/pkg/errors"
 	"github.com/zclconf/go-cty/cty"
+	"github.com/zclconf/go-cty/cty/convert"
 )
 
 var (
@@ -612,6 +613,11 @@ type Target struct {
 	linked bool
 }
 
+var _ hclparser.WithEvalContexts = &Target{}
+var _ hclparser.WithGetName = &Target{}
+var _ hclparser.WithEvalContexts = &Group{}
+var _ hclparser.WithGetName = &Group{}
+
 func (t *Target) normalize() {
 	t.Attest = removeDupes(t.Attest)
 	t.Tags = removeDupes(t.Tags)
@@ -795,7 +801,20 @@ func (t *Target) AddOverrides(overrides map[string]Override) error {
 	return nil
 }
 
-func (t *Target) EvalContexts(ectx *hcl.EvalContext, block *hcl.Block, loadDeps func(hcl.Expression) hcl.Diagnostics) ([]*hcl.EvalContext, error) {
+func (g *Group) GetEvalContexts(ectx *hcl.EvalContext, block *hcl.Block, loadDeps func(hcl.Expression) hcl.Diagnostics) ([]*hcl.EvalContext, error) {
+	content, _, err := block.Body.PartialContent(&hcl.BodySchema{
+		Attributes: []hcl.AttributeSchema{{Name: "matrix"}},
+	})
+	if err != nil {
+		return nil, err
+	}
+	if _, ok := content.Attributes["matrix"]; ok {
+		return nil, errors.Errorf("matrix is not supported for groups")
+	}
+	return []*hcl.EvalContext{ectx}, nil
+}
+
+func (t *Target) GetEvalContexts(ectx *hcl.EvalContext, block *hcl.Block, loadDeps func(hcl.Expression) hcl.Diagnostics) ([]*hcl.EvalContext, error) {
 	content, _, err := block.Body.PartialContent(&hcl.BodySchema{
 		Attributes: []hcl.AttributeSchema{{Name: "matrix"}},
 	})
@@ -843,6 +862,53 @@ func (t *Target) EvalContexts(ectx *hcl.EvalContext, block *hcl.Block, loadDeps
 	return ectxs, nil
 }
 
+func (g *Group) GetName(ectx *hcl.EvalContext, block *hcl.Block, loadDeps func(hcl.Expression) hcl.Diagnostics) (string, error) {
+	content, _, diags := block.Body.PartialContent(&hcl.BodySchema{
+		Attributes: []hcl.AttributeSchema{{Name: "name"}, {Name: "matrix"}},
+	})
+	if diags != nil {
+		return "", diags
+	}
+
+	if _, ok := content.Attributes["name"]; ok {
+		return "", errors.Errorf("name is not supported for groups")
+	}
+	if _, ok := content.Attributes["matrix"]; ok {
+		return "", errors.Errorf("matrix is not supported for groups")
+	}
+	return block.Labels[0], nil
+}
+
+func (t *Target) GetName(ectx *hcl.EvalContext, block *hcl.Block, loadDeps func(hcl.Expression) hcl.Diagnostics) (string, error) {
+	content, _, diags := block.Body.PartialContent(&hcl.BodySchema{
+		Attributes: []hcl.AttributeSchema{{Name: "name"}, {Name: "matrix"}},
+	})
+	if diags != nil {
+		return "", diags
+	}
+
+	attr, ok := content.Attributes["name"]
+	if !ok {
+		return block.Labels[0], nil
+	}
+	if _, ok := content.Attributes["matrix"]; !ok {
+		return "", errors.Errorf("name requires matrix")
+	}
+	if diags := loadDeps(attr.Expr); diags.HasErrors() {
+		return "", diags
+	}
+	value, diags := attr.Expr.Value(ectx)
+	if diags != nil {
+		return "", diags
+	}
+
+	value, err := convert.Convert(value, cty.String)
+	if err != nil {
+		return "", err
+	}
+	return value.AsString(), nil
+}
+
 func TargetsToBuildOpt(m map[string]*Target, inp *Input) (map[string]build.Options, error) {
 	m2 := make(map[string]build.Options, len(m))
 	for k, v := range m {

bake/hcl_test.go

@@ -660,16 +660,8 @@ func TestHCLRenameTarget(t *testing.T) {
 		}
 		`)
 
-	c, err := ParseFile(dt, "docker-bake.hcl")
+	_, err := ParseFile(dt, "docker-bake.hcl")
-	require.NoError(t, err)
+	require.ErrorContains(t, err, "requires matrix")
-
-	require.Equal(t, 1, len(c.Targets))
-	require.Equal(t, "xyz", c.Targets[0].Name)
-	require.Equal(t, "foo", *c.Targets[0].Dockerfile)
-
-	require.Equal(t, 1, len(c.Groups))
-	require.Equal(t, "abc", c.Groups[0].Name)
-	require.Equal(t, []string{"xyz"}, c.Groups[0].Targets)
 }
 
 func TestHCLRenameGroup(t *testing.T) {
@@ -678,31 +670,28 @@ func TestHCLRenameGroup(t *testing.T) {
 			name = "bar"
 			targets = ["x", "y"]
 		}
+		`)
 
-		target "x" {
+	_, err := ParseFile(dt, "docker-bake.hcl")
-		}
+	require.ErrorContains(t, err, "not supported")
-		target "y" {
+
+	dt = []byte(`
+		group "foo" {
+			matrix = {
+				name = ["x", "y"]
+			}
 		}
 		`)
 
-	c, err := ParseFile(dt, "docker-bake.hcl")
+	_, err = ParseFile(dt, "docker-bake.hcl")
-	require.NoError(t, err)
+	require.ErrorContains(t, err, "not supported")
-
-	require.Equal(t, 2, len(c.Targets))
-	require.Equal(t, "x", c.Targets[0].Name)
-	require.Equal(t, "y", c.Targets[1].Name)
-
-	require.Equal(t, 2, len(c.Groups))
-	require.Equal(t, "bar", c.Groups[0].Name)
-	require.Equal(t, []string{"x", "y"}, c.Groups[0].Targets)
-	require.Equal(t, "foo", c.Groups[1].Name)
-	require.Equal(t, []string{"bar"}, c.Groups[1].Targets)
 }
 
 func TestHCLRenameTargetAttrs(t *testing.T) {
 	dt := []byte(`
 		target "abc" {
 			name = "xyz"
+			matrix = {}
 			dockerfile = "foo"
 		}
 
@@ -726,6 +715,7 @@ func TestHCLRenameTargetAttrs(t *testing.T) {
 
 		target "abc" {
 			name = "xyz"
+			matrix = {}
 			dockerfile = "foo"
 		}
 		`)
@@ -741,6 +731,7 @@ func TestHCLRenameTargetAttrs(t *testing.T) {
 	dt = []byte(`
 		target "abc" {
 			name = "xyz"
+			matrix  = {}
 			dockerfile = "foo"
 		}
 
@@ -750,7 +741,7 @@ func TestHCLRenameTargetAttrs(t *testing.T) {
 		`)
 
 	_, err = ParseFile(dt, "docker-bake.hcl")
-	require.Error(t, err)
+	require.ErrorContains(t, err, "abc")
 
 	dt = []byte(`
 		target "def" {
@@ -759,23 +750,26 @@ func TestHCLRenameTargetAttrs(t *testing.T) {
 
 		target "abc" {
 			name = "xyz"
+			matrix = {}
 			dockerfile = "foo"
 		}
 		`)
 
 	_, err = ParseFile(dt, "docker-bake.hcl")
-	require.Error(t, err)
+	require.ErrorContains(t, err, "abc")
 }
 
 func TestHCLRenameSplit(t *testing.T) {
 	dt := []byte(`
 		target "x" {
 			name = "y"
+			matrix = {}
 			dockerfile = "foo"
 		}
 
 		target "x" {
 			name = "z"
+			matrix = {}
 			dockerfile = "bar"
 		}
 		`)
@@ -798,6 +792,7 @@ func TestHCLRenameMultiFile(t *testing.T) {
 	dt := []byte(`
 		target "foo" {
 			name = "bar"
+			matrix = {}
 			dockerfile = "x"
 		}
 		`)
@@ -979,6 +974,20 @@ func TestHCLMatrixMultipleTargets(t *testing.T) {
 	}
 }
 
+func TestHCLMatrixDuplicateNames(t *testing.T) {
+	dt := []byte(`
+		target "default" {
+			matrix = {
+				foo = ["a", "b"]
+			}
+			name = "c"
+		}
+		`)
+
+	_, err := ParseFile(dt, "docker-bake.hcl")
+	require.Error(t, err)
+}
+
 func TestHCLMatrixArgs(t *testing.T) {
 	dt := []byte(`
 		a = 1
@@ -1033,7 +1042,7 @@ func TestHCLMatrixArgsOverride(t *testing.T) {
 	require.Equal(t, ptrstr("33"), c.Targets[2].Args["foo"])
 }
 
-func TestHCLMatrixErrors(t *testing.T) {
+func TestHCLMatrixBadTypes(t *testing.T) {
 	dt := []byte(`
 		target "default" {
 			matrix = "test"

bake/hclparser/hclparser.go

@@ -54,6 +54,7 @@ type parser struct {
 	blocks       map[string]map[string][]*hcl.Block
 	blockValues  map[*hcl.Block][]reflect.Value
 	blockEvalCtx map[*hcl.Block][]*hcl.EvalContext
+	blockNames   map[*hcl.Block][]string
 	blockTypes   map[string]reflect.Type
 
 	ectx *hcl.EvalContext
@@ -64,6 +65,14 @@ type parser struct {
 	doneB     map[uint64]map[string]struct{}
 }
 
+type WithEvalContexts interface {
+	GetEvalContexts(base *hcl.EvalContext, block *hcl.Block, loadDeps func(hcl.Expression) hcl.Diagnostics) ([]*hcl.EvalContext, error)
+}
+
+type WithGetName interface {
+	GetName(ectx *hcl.EvalContext, block *hcl.Block, loadDeps func(hcl.Expression) hcl.Diagnostics) (string, error)
+}
+
 var errUndefined = errors.New("undefined")
 
 func (p *parser) loadDeps(ectx *hcl.EvalContext, exp hcl.Expression, exclude map[string]struct{}, allowMissing bool) hcl.Diagnostics {
@@ -306,6 +315,11 @@ func (p *parser) resolveValue(ectx *hcl.EvalContext, name string) (err error) {
 // target schema is provided, only the attributes and blocks present in the
 // schema will be evaluated.
 func (p *parser) resolveBlock(block *hcl.Block, target *hcl.BodySchema) (err error) {
+	// prepare the variable map for this type
+	if _, ok := p.ectx.Variables[block.Type]; !ok {
+		p.ectx.Variables[block.Type] = cty.MapValEmpty(cty.Map(cty.String))
+	}
+
 	// prepare the output destination and evaluation context
 	t, ok := p.blockTypes[block.Type]
 	if !ok {
@@ -317,11 +331,8 @@ func (p *parser) resolveBlock(block *hcl.Block, target *hcl.BodySchema) (err err
 		outputs = prev
 		ectxs = p.blockEvalCtx[block]
 	} else {
-		type ectxI interface {
+		if v, ok := reflect.New(t).Interface().(WithEvalContexts); ok {
-			EvalContexts(base *hcl.EvalContext, block *hcl.Block, loadDeps func(hcl.Expression) hcl.Diagnostics) ([]*hcl.EvalContext, error)
+			ectxs, err = v.GetEvalContexts(p.ectx, block, func(expr hcl.Expression) hcl.Diagnostics {
-		}
-		if v, ok := reflect.New(t).Interface().(ectxI); ok {
-			ectxs, err = v.EvalContexts(p.ectx, block, func(expr hcl.Expression) hcl.Diagnostics {
 				return p.loadDeps(p.ectx, expr, nil, true)
 			})
 			if err != nil {
@@ -345,12 +356,9 @@ func (p *parser) resolveBlock(block *hcl.Block, target *hcl.BodySchema) (err err
 	for i, output := range outputs {
 		target := target
 		ectx := ectxs[i]
-		name, _ := getName(output)
+		name := block.Labels[0]
-		if name == "" {
+		if names, ok := p.blockNames[block]; ok {
-			name = block.Labels[0]
+			name = names[i]
-		}
-		if err := p.opt.ValidateLabel(name); err != nil {
-			return err
 		}
 
 		if _, ok := p.doneB[key(block, ectx)]; !ok {
@@ -415,9 +423,6 @@ func (p *parser) resolveBlock(block *hcl.Block, target *hcl.BodySchema) (err err
 
 		// load dependencies from all targeted properties
 		schema, _ := gohcl.ImpliedBodySchema(reflect.New(t).Interface())
-		if nameKey, ok := getNameKey(output); ok {
-			schema.Attributes = append(schema.Attributes, hcl.AttributeSchema{Name: nameKey})
-		}
 		content, _, diag := body().PartialContent(schema)
 		if diag.HasErrors() {
 			return diag
@@ -440,22 +445,6 @@ func (p *parser) resolveBlock(block *hcl.Block, target *hcl.BodySchema) (err err
 		if diag.HasErrors() {
 			return diag
 		}
-		if nameKey, ok := getNameKey(output); ok {
-			for k, v := range content.Attributes {
-				if k == nameKey {
-					var name2 string
-					diag = gohcl.DecodeExpression(v.Expr, ectx, &name)
-					if diag.HasErrors() {
-						return diag
-					}
-					if name2 != "" {
-						name = name2
-					}
-					break
-				}
-			}
-		}
-		setName(output, name)
 
 		// mark all targeted properties as done
 		for _, a := range content.Attributes {
@@ -499,35 +488,49 @@ func (p *parser) resolveBlock(block *hcl.Block, target *hcl.BodySchema) (err err
 // resolveBlockNames returns the names of the block, calling resolveBlock to
 // evaluate any label fields to correctly resolve the name.
 func (p *parser) resolveBlockNames(block *hcl.Block) ([]string, error) {
-	t, ok := p.blockTypes[block.Type]
+	if names, ok := p.blockNames[block]; ok {
-	if !ok {
+		return names, nil
-		return nil, errors.Errorf("internal error: unknown block type %s", block.Type)
 	}
 
-	nameKey, ok := getNameKey(reflect.New(t))
+	if err := p.resolveBlock(block, &hcl.BodySchema{}); err != nil {
-	if ok {
+		return nil, err
-		target := &hcl.BodySchema{
-			Attributes: []hcl.AttributeSchema{{Name: nameKey}},
-			Blocks:     []hcl.BlockHeaderSchema{{Type: nameKey}},
-		}
-		if err := p.resolveBlock(block, target); err != nil {
-			return nil, err
-		}
-	} else {
-		if err := p.resolveBlock(block, &hcl.BodySchema{}); err != nil {
-			return nil, err
-		}
 	}
 
 	names := make([]string, 0, len(p.blockValues[block]))
-	for _, prev := range p.blockValues[block] {
+	for i, val := range p.blockValues[block] {
-		name, ok := getName(prev)
+		ectx := p.blockEvalCtx[block][i]
-		if !ok {
+
-			return nil, errors.New("internal error: failed to get name")
+		name := block.Labels[0]
+		if err := p.opt.ValidateLabel(name); err != nil {
+			return nil, err
 		}
+
+		if v, ok := val.Interface().(WithGetName); ok {
+			var err error
+			name, err = v.GetName(ectx, block, func(expr hcl.Expression) hcl.Diagnostics {
+				return p.loadDeps(ectx, expr, nil, true)
+			})
+			if err != nil {
+				return nil, err
+			}
+			if err := p.opt.ValidateLabel(name); err != nil {
+				return nil, err
+			}
+		}
+
+		setName(val, name)
 		names = append(names, name)
 	}
 
+	found := map[string]struct{}{}
+	for _, name := range names {
+		if _, ok := found[name]; ok {
+			return nil, errors.Errorf("duplicate name %q", name)
+		}
+		found[name] = struct{}{}
+	}
+
+	p.blockNames[block] = names
 	return names, nil
 }
 
@@ -570,6 +573,7 @@ func Parse(b hcl.Body, opt Opt, val interface{}) (map[string]map[string][]string
 		blocks:       map[string]map[string][]*hcl.Block{},
 		blockValues:  map[*hcl.Block][]reflect.Value{},
 		blockEvalCtx: map[*hcl.Block][]*hcl.EvalContext{},
+		blockNames:   map[*hcl.Block][]string{},
 		blockTypes:   map[string]reflect.Type{},
 		ectx: &hcl.EvalContext{
 			Variables: map[string]cty.Value{},
@@ -837,19 +841,6 @@ func getName(v reflect.Value) (string, bool) {
 	return "", false
 }
 
-func getNameKey(v reflect.Value) (string, bool) {
-	numFields := v.Elem().Type().NumField()
-	for i := 0; i < numFields; i++ {
-		parts := strings.Split(v.Elem().Type().Field(i).Tag.Get("hcl"), ",")
-		for _, t := range parts[1:] {
-			if t == "label" {
-				return parts[0], true
-			}
-		}
-	}
-	return "", false
-}
-
 func getNameIndex(v reflect.Value) (int, bool) {
 	numFields := v.Elem().Type().NumField()
 	for i := 0; i < numFields; i++ {