bake: reject targets inside target blocks

bake/bake.go

@@ -32,6 +32,7 @@ import (
 	"github.com/moby/buildkit/client/llb"
 	"github.com/moby/buildkit/frontend/dockerfile/dfgitutil"
 	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
 	"github.com/zclconf/go-cty/cty"
 	"github.com/zclconf/go-cty/cty/convert"
 )
@@ -345,6 +346,7 @@ func ParseFiles(files []File, defaults, vars map[string]string) (_ *Config, _ *h
 	var c Config
 	var composeFiles []File
 	var hclFiles []*hcl.File
+	var hclFileNames []string
 	for _, f := range files {
 		isCompose, composeErr := validateComposeFile(f.Data, f.Name, vars)
 		if isCompose {
@@ -360,6 +362,7 @@ func ParseFiles(files []File, defaults, vars map[string]string) (_ *Config, _ *h
 					return nil, nil, err
 				}
 				hclFiles = append(hclFiles, hf)
+				hclFileNames = append(hclFileNames, f.Name)
 			} else if composeErr != nil {
 				return nil, nil, errors.Wrapf(err, "failed to parse %s: parsing yaml: %v, parsing hcl", f.Name, composeErr)
 			} else {
@@ -413,6 +416,10 @@ func ParseFiles(files []File, defaults, vars map[string]string) (_ *Config, _ *h
 		pm = *res
 	}
 
+	if err := validateTargetGroups(hclFiles, hclFileNames); err != nil {
+		return nil, nil, err
+	}
+
 	return &c, &pm, nil
 }
 
@@ -442,6 +449,32 @@ func ParseFile(dt []byte, fn string) (*Config, error) {
 	return c, err
 }
 
+func validateTargetGroups(files []*hcl.File, names []string) error {
+	schema := &hcl.BodySchema{
+		Blocks: []hcl.BlockHeaderSchema{
+			{Type: "target", LabelNames: []string{"name"}},
+		},
+	}
+	for i, f := range files {
+		name := names[i]
+		content, _, diags := f.Body.PartialContent(schema)
+		if diags.HasErrors() {
+			return diags
+		}
+		for _, block := range content.Blocks {
+			attrs, diags := block.Body.JustAttributes()
+			if diags.HasErrors() {
+				return diags
+			}
+			if attr, ok := attrs["targets"]; ok {
+				logrus.Warnf("%s:%d: the \"targets\" argument is not valid inside a target block; use a group block instead",
+					name, attr.NameRange.Start.Line)
+			}
+		}
+	}
+	return nil
+}
+
 type Config struct {
 	Groups  []*Group  `json:"group" hcl:"group,block" cty:"group"`
 	Targets []*Target `json:"target" hcl:"target,block" cty:"target"`

bake/hcl_test.go

@@ -1,13 +1,15 @@
 package bake
 
 import (
+	"bytes"
 	"fmt"
 	"reflect"
 	"regexp"
 	"runtime"
 	"testing"
 
 	hcl "github.com/hashicorp/hcl/v2"
+	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/require"
 )
 
@@ -188,6 +190,181 @@ func TestHCLWithUserDefinedFunctions(t *testing.T) {
 	require.Equal(t, ptrstr("124"), c.Targets[0].Args["buildno"])
 }
 
+func TestTargetWarnsOnGroupTargetsAttributeInTargetBlock(t *testing.T) {
+	assertTargetsInTargetWarning := func(t *testing.T, output string, file string) {
+		t.Helper()
+		require.NotEmpty(t, output)
+		require.Contains(t, output, "targets")
+		require.Contains(t, output, "group block")
+		require.Contains(t, output, file)
+	}
+
+	withWarnings := func(t *testing.T, fn func()) string {
+		t.Helper()
+		var buf bytes.Buffer
+		logrus.SetOutput(&buf)
+		t.Cleanup(func() { logrus.SetOutput(nil) })
+		fn()
+		return buf.String()
+	}
+
+	t.Run("hcl", func(t *testing.T) {
+		dt := []byte(`
+			target "app" {
+				dockerfile = "app.Dockerfile"
+			}
+
+			target "db" {
+				dockerfile = "db.Dockerfile"
+			}
+
+			target "foo" {
+				targets = ["app", "db"]
+			}
+		`)
+
+		output := withWarnings(t, func() {
+			_, err := ParseFile(dt, "docker-bake.hcl")
+			require.NoError(t, err)
+		})
+		assertTargetsInTargetWarning(t, output, "docker-bake.hcl")
+	})
+
+	t.Run("json", func(t *testing.T) {
+		dt := []byte(`{
+			"group": {
+				"default": {
+					"targets": ["foo"]
+				}
+			},
+			"target": {
+				"app": {
+					"dockerfile": "app.Dockerfile"
+				},
+				"db": {
+					"dockerfile": "db.Dockerfile"
+				},
+				"foo": {
+					"targets": ["app", "db"]
+				}
+			}
+		}`)
+
+		output := withWarnings(t, func() {
+			_, err := ParseFile(dt, "docker-bake.json")
+			require.NoError(t, err)
+		})
+		assertTargetsInTargetWarning(t, output, "docker-bake.json")
+	})
+
+	t.Run("merged hcl files", func(t *testing.T) {
+		output := withWarnings(t, func() {
+			_, _, err := ParseFiles([]File{
+				{
+					Name: "base.hcl",
+					Data: []byte(`
+					target "app" {
+						dockerfile = "app.Dockerfile"
+					}
+
+					target "db" {
+						dockerfile = "db.Dockerfile"
+					}
+				`),
+				},
+				{
+					Name: "group-as-target.hcl",
+					Data: []byte(`
+					target "foo" {
+						targets = ["app", "db"]
+					}
+				`),
+				},
+			}, nil, nil)
+			require.NoError(t, err)
+		})
+		assertTargetsInTargetWarning(t, output, "group-as-target.hcl")
+	})
+
+	t.Run("merged hcl and json", func(t *testing.T) {
+		output := withWarnings(t, func() {
+			_, _, err := ParseFiles([]File{
+				{
+					Name: "docker-bake.hcl",
+					Data: []byte(`
+					group "default" {
+						targets = ["foo"]
+					}
+
+					target "app" {
+						dockerfile = "app.Dockerfile"
+					}
+				`),
+				},
+				{
+					Name: "extra.json",
+					Data: []byte(`{
+					"target": {
+						"foo": {
+							"targets": ["app"]
+						}
+					}
+				}`),
+				},
+			}, nil, nil)
+			require.NoError(t, err)
+		})
+		assertTargetsInTargetWarning(t, output, "extra.json")
+	})
+
+	t.Run("group targets still valid", func(t *testing.T) {
+		dt := []byte(`
+			target "app" {
+				dockerfile = "app.Dockerfile"
+			}
+
+			target "db" {
+				dockerfile = "db.Dockerfile"
+			}
+
+			group "foo" {
+				targets = ["app", "db"]
+			}
+		`)
+
+		c, err := ParseFile(dt, "docker-bake.hcl")
+		require.NoError(t, err)
+		require.Equal(t, 1, len(c.Groups))
+		require.Equal(t, "foo", c.Groups[0].Name)
+		require.Equal(t, []string{"app", "db"}, c.Groups[0].Targets)
+	})
+
+	t.Run("merged valid files", func(t *testing.T) {
+		c, _, err := ParseFiles([]File{
+			{
+				Name: "targets.hcl",
+				Data: []byte(`
+					target "app" {
+						dockerfile = "app.Dockerfile"
+					}
+				`),
+			},
+			{
+				Name: "groups.hcl",
+				Data: []byte(`
+					group "foo" {
+						targets = ["app"]
+					}
+				`),
+			},
+		}, nil, nil)
+		require.NoError(t, err)
+		require.Equal(t, 1, len(c.Groups))
+		require.Equal(t, "foo", c.Groups[0].Name)
+		require.Equal(t, []string{"app"}, c.Groups[0].Targets)
+	})
+}
+
 func TestHCLWithVariables(t *testing.T) {
 	dt := []byte(`
 		variable "BUILD_NUMBER" {