Merge pull request #1313 from jedevc/bake-group-recurse

bake: recursively resolve groups

bake/bake.go

@@ -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 {
+	n := map[string]*Group{}
-		for _, n := range c.ResolveGroup(n) {
+	for _, target := range targets {
-			t, err := c.ResolveTarget(n, o)
+		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
+	for _, target := range targets {
-	if len(targets) == 0 || (len(targets) == 1 && targets[0] == "default") {
+		if target == "default" {
-		for _, group := range c.Groups {
-			if group.Name != "default" {
 			continue
 		}
-			g = []*Group{{Targets: group.Targets}}
+		if _, ok := n["default"]; !ok {
+			n["default"] = &Group{Name: "default"}
 		}
-	} else {
+		n["default"].Targets = append(n["default"].Targets, target)
-		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
 	}
-			}
+	if g, ok := n["default"]; ok {
-			if !isGroup {
+		g.Targets = dedupSlice(g.Targets)
-				gt = append(gt, target)
-			}
-		}
-		g = []*Group{{Targets: dedupSlice(gt)}}
 	}
 
 	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 {
+func (c Config) ResolveGroup(name string) ([]string, []string) {
-	return dedupSlice(c.group(name, map[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 {
+type visit struct {
-	if _, ok := visited[name]; ok {
+	target []string
-		return visited[name]
+	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)
+		ttarget, tgroup := c.group(t, visited)
-		if len(tgroup) > 0 {
+		if len(ttarget) > 0 {
-			targets = append(targets, tgroup...)
+			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) {

bake/bake_test.go

@@ -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)
+			sort.Strings(g["default"].Targets)
-			require.Equal(t, []string{"webDEP", "webapp"}, g[0].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)
+	sort.Strings(g["default"].Targets)
-	require.Equal(t, []string{"db", "newservice", "webapp"}, g[0].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)
+	sort.Strings(g["default"].Targets)
-	require.Equal(t, []string{"web_app"}, g[0].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, 2, len(g))
-	require.Equal(t, []string{"image"}, g[0].Targets)
+	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, 2, len(g))
-	require.Equal(t, []string{"image"}, g[0].Targets)
+	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, 2, len(g))
-	require.Equal(t, []string{"image"}, g[0].Targets)
+	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)
+	sort.Strings(g["default"].Targets)
-	require.Equal(t, []string{"addon", "aws"}, g[0].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)
+	sort.Strings(g["default"].Targets)
-	require.Equal(t, []string{"addon", "aws"}, g[0].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)
+	sort.Strings(g["default"].Targets)
-	require.Equal(t, []string{"addon", "aws", "image"}, g[0].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, 2, len(g))
-	require.Equal(t, []string{"foo"}, g[0].Targets)
+	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, 2, len(g))
-	require.Equal(t, []string{"foo"}, g[0].Targets)
+	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, 2, len(g))
-	require.Equal(t, []string{"foo", "image"}, g[0].Targets)
+	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, 2, len(g))
-	require.Equal(t, []string{"foo", "image"}, g[0].Targets)
+	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",
+			names:   []string{"a"},
-			targets:  []string{"b", "c"},
+			targets: []string{"a"},
-			ntargets: 1,
+			groups:  []string{"default", "a", "b", "c"},
+			count:   1,
 		},
 		{
-			name:     "b",
+			names:   []string{"b"},
-			targets:  []string{"d"},
-			ntargets: 1,
-		},
-		{
-			name:     "c",
 			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",
+			names:   []string{"e"},
-			targets:  []string{"a", "f"},
+			targets: []string{"e"},
-			ntargets: 2,
+			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) {
+		t.Run(strings.Join(tt.names, "+"), func(t *testing.T) {
-			m, g, err := ReadTargets(ctx, []File{f}, []string{tt.name}, nil, nil)
+			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)
+			var gnames []string
-			require.Equal(t, tt.ntargets, len(m))
+			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)
 		})

commands/bake.go

@@ -127,17 +127,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 {