bake: recursively resolve groups

Groups that contained other groups were not recursively resolved by
ReadTargets, which prevented output from --print from being useable as a
self-contained bake file.

This patch ensures that all groups that are referenced inside the bake
file are actually defined under the groups field. This has required a
substantial refactor, as previously only a single group was returned
from ReadTargets, notably, returning a map of groups, instead of a
slice.

This does introduce a small behavior change to the behavior of --print -
while previously, passing a group name to bake would return all the
targets of that group back as the default group, now only the name of
that group will be inserted into the default group, keeping the original
group intact. The impact of this can be observed in some of the changes
to the bake_test.go file.

Signed-off-by: Justin Chadwell <me@jedevc.com>
This commit is contained in:
Justin Chadwell 2022-09-12 13:46:48 +01:00
parent aeac42be47
commit 77b33260f8
3 changed files with 148 additions and 116 deletions

View File

@ -84,7 +84,7 @@ func ReadLocalFiles(names []string) ([]File, error) {
return out, nil
}
func ReadTargets(ctx context.Context, files []File, targets, overrides []string, defaults map[string]string) (map[string]*Target, []*Group, error) {
func ReadTargets(ctx context.Context, files []File, targets, overrides []string, defaults map[string]string) (map[string]*Target, map[string]*Group, error) {
c, err := ParseFiles(files, defaults)
if err != nil {
return nil, nil, err
@ -99,42 +99,39 @@ func ReadTargets(ctx context.Context, files []File, targets, overrides []string,
return nil, nil, err
}
m := map[string]*Target{}
for _, n := range targets {
for _, n := range c.ResolveGroup(n) {
t, err := c.ResolveTarget(n, o)
n := map[string]*Group{}
for _, target := range targets {
ts, gs := c.ResolveGroup(target)
for _, tname := range ts {
t, err := c.ResolveTarget(tname, o)
if err != nil {
return nil, nil, err
}
if t != nil {
m[n] = t
m[tname] = t
}
}
for _, gname := range gs {
for _, group := range c.Groups {
if group.Name == gname {
n[gname] = group
break
}
}
}
}
var g []*Group
if len(targets) == 0 || (len(targets) == 1 && targets[0] == "default") {
for _, group := range c.Groups {
if group.Name != "default" {
for _, target := range targets {
if target == "default" {
continue
}
g = []*Group{{Targets: group.Targets}}
if _, ok := n["default"]; !ok {
n["default"] = &Group{Name: "default"}
}
} else {
var gt []string
for _, target := range targets {
isGroup := false
for _, group := range c.Groups {
if target == group.Name {
gt = append(gt, group.Targets...)
isGroup = true
break
n["default"].Targets = append(n["default"].Targets, target)
}
}
if !isGroup {
gt = append(gt, target)
}
}
g = []*Group{{Targets: dedupSlice(gt)}}
if g, ok := n["default"]; ok {
g.Targets = dedupSlice(g.Targets)
}
for name, t := range m {
@ -143,7 +140,7 @@ func ReadTargets(ctx context.Context, files []File, targets, overrides []string,
}
}
return m, g, nil
return m, n, nil
}
func dedupSlice(s []string) []string {
@ -453,13 +450,19 @@ func (c Config) newOverrides(v []string) (map[string]map[string]Override, error)
return m, nil
}
func (c Config) ResolveGroup(name string) []string {
return dedupSlice(c.group(name, map[string][]string{}))
func (c Config) ResolveGroup(name string) ([]string, []string) {
targets, groups := c.group(name, map[string]visit{})
return dedupSlice(targets), dedupSlice(groups)
}
func (c Config) group(name string, visited map[string][]string) []string {
if _, ok := visited[name]; ok {
return visited[name]
type visit struct {
target []string
group []string
}
func (c Config) group(name string, visited map[string]visit) ([]string, []string) {
if v, ok := visited[name]; ok {
return v.target, v.group
}
var g *Group
for _, group := range c.Groups {
@ -469,20 +472,24 @@ func (c Config) group(name string, visited map[string][]string) []string {
}
}
if g == nil {
return []string{name}
return []string{name}, nil
}
visited[name] = []string{}
visited[name] = visit{}
targets := make([]string, 0, len(g.Targets))
groups := []string{name}
for _, t := range g.Targets {
tgroup := c.group(t, visited)
if len(tgroup) > 0 {
targets = append(targets, tgroup...)
ttarget, tgroup := c.group(t, visited)
if len(ttarget) > 0 {
targets = append(targets, ttarget...)
} else {
targets = append(targets, t)
}
if len(tgroup) > 0 {
groups = append(groups, tgroup...)
}
visited[name] = targets
return targets
}
visited[name] = visit{target: targets, group: groups}
return targets, groups
}
func (c Config) ResolveTarget(name string, overrides map[string]map[string]Override) (*Target, error) {

View File

@ -4,6 +4,7 @@ import (
"context"
"os"
"sort"
"strings"
"testing"
"github.com/stretchr/testify/require"
@ -46,7 +47,7 @@ target "webapp" {
require.Nil(t, m["webapp"].Pull)
require.Equal(t, 1, len(g))
require.Equal(t, []string{"webapp"}, g[0].Targets)
require.Equal(t, []string{"webapp"}, g["default"].Targets)
})
t.Run("InvalidTargetOverrides", func(t *testing.T) {
@ -87,7 +88,7 @@ target "webapp" {
require.Equal(t, m["webapp"].Args["VAR_INHERITED"], "override")
require.Equal(t, 1, len(g))
require.Equal(t, []string{"webapp"}, g[0].Targets)
require.Equal(t, []string{"webapp"}, g["default"].Targets)
})
// building leaf but overriding parent fields
@ -101,7 +102,7 @@ target "webapp" {
require.Equal(t, m["webapp"].Args["VAR_INHERITED"], "override")
require.Equal(t, m["webapp"].Args["VAR_BOTH"], "webapp")
require.Equal(t, 1, len(g))
require.Equal(t, []string{"webapp"}, g[0].Targets)
require.Equal(t, []string{"webapp"}, g["default"].Targets)
})
})
@ -113,7 +114,7 @@ target "webapp" {
require.NoError(t, err)
require.Equal(t, "foo", *m["webapp"].Context)
require.Equal(t, 1, len(g))
require.Equal(t, []string{"webapp"}, g[0].Targets)
require.Equal(t, []string{"webapp"}, g["default"].Targets)
})
t.Run("NoCacheOverride", func(t *testing.T) {
@ -121,7 +122,7 @@ target "webapp" {
require.NoError(t, err)
require.Equal(t, false, *m["webapp"].NoCache)
require.Equal(t, 1, len(g))
require.Equal(t, []string{"webapp"}, g[0].Targets)
require.Equal(t, []string{"webapp"}, g["default"].Targets)
})
t.Run("PullOverride", func(t *testing.T) {
@ -129,12 +130,12 @@ target "webapp" {
require.NoError(t, err)
require.Equal(t, false, *m["webapp"].Pull)
require.Equal(t, 1, len(g))
require.Equal(t, []string{"webapp"}, g[0].Targets)
require.Equal(t, []string{"webapp"}, g["default"].Targets)
})
t.Run("PatternOverride", func(t *testing.T) {
// same check for two cases
multiTargetCheck := func(t *testing.T, m map[string]*Target, g []*Group, err error) {
multiTargetCheck := func(t *testing.T, m map[string]*Target, g map[string]*Group, err error) {
require.NoError(t, err)
require.Equal(t, 2, len(m))
require.Equal(t, "foo", *m["webapp"].Dockerfile)
@ -142,15 +143,15 @@ target "webapp" {
require.Equal(t, "foo", *m["webDEP"].Dockerfile)
require.Equal(t, "webDEP", m["webDEP"].Args["VAR_INHERITED"])
require.Equal(t, 1, len(g))
sort.Strings(g[0].Targets)
require.Equal(t, []string{"webDEP", "webapp"}, g[0].Targets)
sort.Strings(g["default"].Targets)
require.Equal(t, []string{"webDEP", "webapp"}, g["default"].Targets)
}
cases := []struct {
name string
targets []string
overrides []string
check func(*testing.T, map[string]*Target, []*Group, error)
check func(*testing.T, map[string]*Target, map[string]*Group, error)
}{
{
name: "multi target single pattern",
@ -168,20 +169,20 @@ target "webapp" {
name: "single target",
targets: []string{"webapp"},
overrides: []string{"web*.dockerfile=foo"},
check: func(t *testing.T, m map[string]*Target, g []*Group, err error) {
check: func(t *testing.T, m map[string]*Target, g map[string]*Group, err error) {
require.NoError(t, err)
require.Equal(t, 1, len(m))
require.Equal(t, "foo", *m["webapp"].Dockerfile)
require.Equal(t, "webDEP", m["webapp"].Args["VAR_INHERITED"])
require.Equal(t, 1, len(g))
require.Equal(t, []string{"webapp"}, g[0].Targets)
require.Equal(t, []string{"webapp"}, g["default"].Targets)
},
},
{
name: "nomatch",
targets: []string{"webapp"},
overrides: []string{"nomatch*.dockerfile=foo"},
check: func(t *testing.T, m map[string]*Target, g []*Group, err error) {
check: func(t *testing.T, m map[string]*Target, g map[string]*Group, err error) {
// NOTE: I am unsure whether failing to match should always error out
// instead of simply skipping that override.
// Let's enforce the error and we can relax it later if users complain.
@ -303,8 +304,8 @@ services:
require.Equal(t, "12", m["webapp"].Args["buildno2"])
require.Equal(t, 1, len(g))
sort.Strings(g[0].Targets)
require.Equal(t, []string{"db", "newservice", "webapp"}, g[0].Targets)
sort.Strings(g["default"].Targets)
require.Equal(t, []string{"db", "newservice", "webapp"}, g["default"].Targets)
}
func TestReadTargetsWithDotCompose(t *testing.T) {
@ -364,8 +365,8 @@ services:
require.Equal(t, "12", m["web_app"].Args["buildno2"])
require.Equal(t, 1, len(g))
sort.Strings(g[0].Targets)
require.Equal(t, []string{"web_app"}, g[0].Targets)
sort.Strings(g["default"].Targets)
require.Equal(t, []string{"web_app"}, g["default"].Targets)
}
func TestHCLCwdPrefix(t *testing.T) {
@ -392,7 +393,7 @@ func TestHCLCwdPrefix(t *testing.T) {
require.Equal(t, "foo", *m["app"].Context)
require.Equal(t, 1, len(g))
require.Equal(t, []string{"app"}, g[0].Targets)
require.Equal(t, []string{"app"}, g["default"].Targets)
}
func TestOverrideMerge(t *testing.T) {
@ -696,7 +697,7 @@ target "image" {
m, g, err := ReadTargets(ctx, []File{f}, []string{"image"}, nil, nil)
require.NoError(t, err)
require.Equal(t, 1, len(g))
require.Equal(t, []string{"image"}, g[0].Targets)
require.Equal(t, []string{"image"}, g["default"].Targets)
require.Equal(t, 1, len(m))
require.Equal(t, "test", *m["image"].Dockerfile)
}
@ -717,8 +718,9 @@ target "image" {
m, g, err := ReadTargets(ctx, []File{f}, []string{"foo"}, nil, nil)
require.NoError(t, err)
require.Equal(t, 1, len(g))
require.Equal(t, []string{"image"}, g[0].Targets)
require.Equal(t, 2, len(g))
require.Equal(t, []string{"foo"}, g["default"].Targets)
require.Equal(t, []string{"image"}, g["foo"].Targets)
require.Equal(t, 1, len(m))
require.Equal(t, "test", *m["image"].Dockerfile)
}
@ -742,15 +744,17 @@ target "image" {
m, g, err := ReadTargets(ctx, []File{f}, []string{"foo"}, nil, nil)
require.NoError(t, err)
require.Equal(t, 1, len(g))
require.Equal(t, []string{"image"}, g[0].Targets)
require.Equal(t, 2, len(g))
require.Equal(t, []string{"foo"}, g["default"].Targets)
require.Equal(t, []string{"image"}, g["foo"].Targets)
require.Equal(t, 1, len(m))
require.Equal(t, "test", *m["image"].Dockerfile)
m, g, err = ReadTargets(ctx, []File{f}, []string{"foo", "foo"}, nil, nil)
require.NoError(t, err)
require.Equal(t, 1, len(g))
require.Equal(t, []string{"image"}, g[0].Targets)
require.Equal(t, 2, len(g))
require.Equal(t, []string{"foo"}, g["default"].Targets)
require.Equal(t, []string{"image"}, g["foo"].Targets)
require.Equal(t, 1, len(m))
require.Equal(t, "test", *m["image"].Dockerfile)
}
@ -829,7 +833,7 @@ services:
m, g, err := ReadTargets(ctx, []File{fhcl}, []string{"default"}, nil, nil)
require.NoError(t, err)
require.Equal(t, 1, len(g))
require.Equal(t, []string{"image"}, g[0].Targets)
require.Equal(t, []string{"image"}, g["default"].Targets)
require.Equal(t, 1, len(m))
require.Equal(t, 1, len(m["image"].Outputs))
require.Equal(t, "type=docker", m["image"].Outputs[0])
@ -837,7 +841,7 @@ services:
m, g, err = ReadTargets(ctx, []File{fhcl}, []string{"image-release"}, nil, nil)
require.NoError(t, err)
require.Equal(t, 1, len(g))
require.Equal(t, []string{"image-release"}, g[0].Targets)
require.Equal(t, []string{"image-release"}, g["default"].Targets)
require.Equal(t, 1, len(m))
require.Equal(t, 1, len(m["image-release"].Outputs))
require.Equal(t, "type=image,push=true", m["image-release"].Outputs[0])
@ -845,7 +849,7 @@ services:
m, g, err = ReadTargets(ctx, []File{fhcl}, []string{"image", "image-release"}, nil, nil)
require.NoError(t, err)
require.Equal(t, 1, len(g))
require.Equal(t, []string{"image", "image-release"}, g[0].Targets)
require.Equal(t, []string{"image", "image-release"}, g["default"].Targets)
require.Equal(t, 2, len(m))
require.Equal(t, ".", *m["image"].Context)
require.Equal(t, 1, len(m["image-release"].Outputs))
@ -854,22 +858,22 @@ services:
m, g, err = ReadTargets(ctx, []File{fyml, fhcl}, []string{"default"}, nil, nil)
require.NoError(t, err)
require.Equal(t, 1, len(g))
require.Equal(t, []string{"image"}, g[0].Targets)
require.Equal(t, []string{"image"}, g["default"].Targets)
require.Equal(t, 1, len(m))
require.Equal(t, ".", *m["image"].Context)
m, g, err = ReadTargets(ctx, []File{fjson}, []string{"default"}, nil, nil)
require.NoError(t, err)
require.Equal(t, 1, len(g))
require.Equal(t, []string{"image"}, g[0].Targets)
require.Equal(t, []string{"image"}, g["default"].Targets)
require.Equal(t, 1, len(m))
require.Equal(t, ".", *m["image"].Context)
m, g, err = ReadTargets(ctx, []File{fyml}, []string{"default"}, nil, nil)
require.NoError(t, err)
require.Equal(t, 1, len(g))
sort.Strings(g[0].Targets)
require.Equal(t, []string{"addon", "aws"}, g[0].Targets)
sort.Strings(g["default"].Targets)
require.Equal(t, []string{"addon", "aws"}, g["default"].Targets)
require.Equal(t, 2, len(m))
require.Equal(t, "./Dockerfile", *m["addon"].Dockerfile)
require.Equal(t, "./aws.Dockerfile", *m["aws"].Dockerfile)
@ -877,8 +881,8 @@ services:
m, g, err = ReadTargets(ctx, []File{fyml, fhcl}, []string{"addon", "aws"}, nil, nil)
require.NoError(t, err)
require.Equal(t, 1, len(g))
sort.Strings(g[0].Targets)
require.Equal(t, []string{"addon", "aws"}, g[0].Targets)
sort.Strings(g["default"].Targets)
require.Equal(t, []string{"addon", "aws"}, g["default"].Targets)
require.Equal(t, 2, len(m))
require.Equal(t, "./Dockerfile", *m["addon"].Dockerfile)
require.Equal(t, "./aws.Dockerfile", *m["aws"].Dockerfile)
@ -886,8 +890,8 @@ services:
m, g, err = ReadTargets(ctx, []File{fyml, fhcl}, []string{"addon", "aws", "image"}, nil, nil)
require.NoError(t, err)
require.Equal(t, 1, len(g))
sort.Strings(g[0].Targets)
require.Equal(t, []string{"addon", "aws", "image"}, g[0].Targets)
sort.Strings(g["default"].Targets)
require.Equal(t, []string{"addon", "aws", "image"}, g["default"].Targets)
require.Equal(t, 3, len(m))
require.Equal(t, ".", *m["image"].Context)
require.Equal(t, "./Dockerfile", *m["addon"].Dockerfile)
@ -913,15 +917,17 @@ target "image" {
m, g, err := ReadTargets(ctx, []File{f}, []string{"foo"}, nil, nil)
require.NoError(t, err)
require.Equal(t, 1, len(g))
require.Equal(t, []string{"foo"}, g[0].Targets)
require.Equal(t, 2, len(g))
require.Equal(t, []string{"foo"}, g["default"].Targets)
require.Equal(t, []string{"foo"}, g["foo"].Targets)
require.Equal(t, 1, len(m))
require.Equal(t, "bar", *m["foo"].Dockerfile)
m, g, err = ReadTargets(ctx, []File{f}, []string{"foo", "foo"}, nil, nil)
require.NoError(t, err)
require.Equal(t, 1, len(g))
require.Equal(t, []string{"foo"}, g[0].Targets)
require.Equal(t, 2, len(g))
require.Equal(t, []string{"foo"}, g["default"].Targets)
require.Equal(t, []string{"foo"}, g["foo"].Targets)
require.Equal(t, 1, len(m))
require.Equal(t, "bar", *m["foo"].Dockerfile)
}
@ -945,16 +951,18 @@ target "image" {
m, g, err := ReadTargets(ctx, []File{f}, []string{"foo"}, nil, nil)
require.NoError(t, err)
require.Equal(t, 1, len(g))
require.Equal(t, []string{"foo", "image"}, g[0].Targets)
require.Equal(t, 2, len(g))
require.Equal(t, []string{"foo"}, g["default"].Targets)
require.Equal(t, []string{"foo", "image"}, g["foo"].Targets)
require.Equal(t, 2, len(m))
require.Equal(t, "bar", *m["foo"].Dockerfile)
require.Equal(t, "type=docker", m["image"].Outputs[0])
m, g, err = ReadTargets(ctx, []File{f}, []string{"foo", "image"}, nil, nil)
require.NoError(t, err)
require.Equal(t, 1, len(g))
require.Equal(t, []string{"foo", "image"}, g[0].Targets)
require.Equal(t, 2, len(g))
require.Equal(t, []string{"foo", "image"}, g["default"].Targets)
require.Equal(t, []string{"foo", "image"}, g["foo"].Targets)
require.Equal(t, 2, len(m))
require.Equal(t, "bar", *m["foo"].Dockerfile)
require.Equal(t, "type=docker", m["image"].Outputs[0])
@ -1015,7 +1023,7 @@ target "d" {
m, g, err := ReadTargets(ctx, []File{f}, []string{"d"}, tt.overrides, nil)
require.NoError(t, err)
require.Equal(t, 1, len(g))
require.Equal(t, []string{"d"}, g[0].Targets)
require.Equal(t, []string{"d"}, g["default"].Targets)
require.Equal(t, 1, len(m))
require.Equal(t, tt.want, m["d"].Args)
})
@ -1087,7 +1095,7 @@ group "default" {
m, g, err := ReadTargets(ctx, []File{f}, []string{"default"}, tt.overrides, nil)
require.NoError(t, err)
require.Equal(t, 1, len(g))
require.Equal(t, []string{"child1", "child2"}, g[0].Targets)
require.Equal(t, []string{"child1", "child2"}, g["default"].Targets)
require.Equal(t, 2, len(m))
require.Equal(t, tt.wantch1, m["child1"].Args)
require.Equal(t, []string{"type=docker"}, m["child1"].Outputs)
@ -1184,44 +1192,67 @@ target "f" {
}`)}
cases := []struct {
name string
names []string
targets []string
ntargets int
groups []string
count int
}{
{
name: "a",
targets: []string{"b", "c"},
ntargets: 1,
names: []string{"a"},
targets: []string{"a"},
groups: []string{"default", "a", "b", "c"},
count: 1,
},
{
name: "b",
targets: []string{"d"},
ntargets: 1,
},
{
name: "c",
names: []string{"b"},
targets: []string{"b"},
ntargets: 1,
groups: []string{"default", "b"},
count: 1,
},
{
name: "d",
names: []string{"c"},
targets: []string{"c"},
groups: []string{"default", "b", "c"},
count: 1,
},
{
names: []string{"d"},
targets: []string{"d"},
ntargets: 1,
groups: []string{"default"},
count: 1,
},
{
name: "e",
targets: []string{"a", "f"},
ntargets: 2,
names: []string{"e"},
targets: []string{"e"},
groups: []string{"default", "a", "b", "c", "e"},
count: 2,
},
{
names: []string{"a", "e"},
targets: []string{"a", "e"},
groups: []string{"default", "a", "b", "c", "e"},
count: 2,
},
}
for _, tt := range cases {
tt := tt
t.Run(tt.name, func(t *testing.T) {
m, g, err := ReadTargets(ctx, []File{f}, []string{tt.name}, nil, nil)
t.Run(strings.Join(tt.names, "+"), func(t *testing.T) {
m, g, err := ReadTargets(ctx, []File{f}, tt.names, nil, nil)
require.NoError(t, err)
require.Equal(t, 1, len(g))
require.Equal(t, tt.targets, g[0].Targets)
require.Equal(t, tt.ntargets, len(m))
var gnames []string
for _, g := range g {
gnames = append(gnames, g.Name)
}
sort.Strings(gnames)
sort.Strings(tt.groups)
require.Equal(t, tt.groups, gnames)
sort.Strings(g["default"].Targets)
sort.Strings(tt.targets)
require.Equal(t, tt.targets, g["default"].Targets)
require.Equal(t, tt.count, len(m))
require.Equal(t, ".", *m["d"].Context)
require.Equal(t, "./testdockerfile", *m["d"].Dockerfile)
})

View File

@ -120,17 +120,11 @@ func runBake(dockerCli command.Cli, targets []string, in bakeOptions) (err error
}
if in.printOnly {
var defg map[string]*bake.Group
if len(grps) == 1 {
defg = map[string]*bake.Group{
"default": grps[0],
}
}
dt, err := json.MarshalIndent(struct {
Group map[string]*bake.Group `json:"group,omitempty"`
Target map[string]*bake.Target `json:"target"`
}{
defg,
grps,
tgts,
}, "", " ")
if err != nil {