Merge pull request #2814 from jsternberg/bake-composable-attributes-phase2

bake: various fixes for composable attributes

Author: tonistiigi

bake/bake.go

@@ -698,30 +698,30 @@ type Target struct {
 	// Inherits is the only field that cannot be overridden with --set
 	Inherits []string `json:"inherits,omitempty" hcl:"inherits,optional" cty:"inherits"`
 
-	Annotations      []string                        `json:"annotations,omitempty" hcl:"annotations,optional" cty:"annotations"`
-	Attest           []string                        `json:"attest,omitempty" hcl:"attest,optional" cty:"attest"`
-	Context          *string                         `json:"context,omitempty" hcl:"context,optional" cty:"context"`
-	Contexts         map[string]string               `json:"contexts,omitempty" hcl:"contexts,optional" cty:"contexts"`
-	Dockerfile       *string                         `json:"dockerfile,omitempty" hcl:"dockerfile,optional" cty:"dockerfile"`
-	DockerfileInline *string                         `json:"dockerfile-inline,omitempty" hcl:"dockerfile-inline,optional" cty:"dockerfile-inline"`
-	Args             map[string]*string              `json:"args,omitempty" hcl:"args,optional" cty:"args"`
-	Labels           map[string]*string              `json:"labels,omitempty" hcl:"labels,optional" cty:"labels"`
-	Tags             []string                        `json:"tags,omitempty" hcl:"tags,optional" cty:"tags"`
-	CacheFrom        []*buildflags.CacheOptionsEntry `json:"cache-from,omitempty"  hcl:"cache-from,optional" cty:"cache-from"`
-	CacheTo          []*buildflags.CacheOptionsEntry `json:"cache-to,omitempty"  hcl:"cache-to,optional" cty:"cache-to"`
-	Target           *string                         `json:"target,omitempty" hcl:"target,optional" cty:"target"`
-	Secrets          []*buildflags.Secret            `json:"secret,omitempty" hcl:"secret,optional" cty:"secret"`
-	SSH              []*buildflags.SSH               `json:"ssh,omitempty" hcl:"ssh,optional" cty:"ssh"`
-	Platforms        []string                        `json:"platforms,omitempty" hcl:"platforms,optional" cty:"platforms"`
-	Outputs          []*buildflags.ExportEntry       `json:"output,omitempty" hcl:"output,optional" cty:"output"`
-	Pull             *bool                           `json:"pull,omitempty" hcl:"pull,optional" cty:"pull"`
-	NoCache          *bool                           `json:"no-cache,omitempty" hcl:"no-cache,optional" cty:"no-cache"`
-	NetworkMode      *string                         `json:"network,omitempty" hcl:"network,optional" cty:"network"`
-	NoCacheFilter    []string                        `json:"no-cache-filter,omitempty" hcl:"no-cache-filter,optional" cty:"no-cache-filter"`
-	ShmSize          *string                         `json:"shm-size,omitempty" hcl:"shm-size,optional"`
-	Ulimits          []string                        `json:"ulimits,omitempty" hcl:"ulimits,optional"`
-	Call             *string                         `json:"call,omitempty" hcl:"call,optional" cty:"call"`
-	Entitlements     []string                        `json:"entitlements,omitempty" hcl:"entitlements,optional" cty:"entitlements"`
+	Annotations      []string                `json:"annotations,omitempty" hcl:"annotations,optional" cty:"annotations"`
+	Attest           []string                `json:"attest,omitempty" hcl:"attest,optional" cty:"attest"`
+	Context          *string                 `json:"context,omitempty" hcl:"context,optional" cty:"context"`
+	Contexts         map[string]string       `json:"contexts,omitempty" hcl:"contexts,optional" cty:"contexts"`
+	Dockerfile       *string                 `json:"dockerfile,omitempty" hcl:"dockerfile,optional" cty:"dockerfile"`
+	DockerfileInline *string                 `json:"dockerfile-inline,omitempty" hcl:"dockerfile-inline,optional" cty:"dockerfile-inline"`
+	Args             map[string]*string      `json:"args,omitempty" hcl:"args,optional" cty:"args"`
+	Labels           map[string]*string      `json:"labels,omitempty" hcl:"labels,optional" cty:"labels"`
+	Tags             []string                `json:"tags,omitempty" hcl:"tags,optional" cty:"tags"`
+	CacheFrom        buildflags.CacheOptions `json:"cache-from,omitempty"  hcl:"cache-from,optional" cty:"cache-from"`
+	CacheTo          buildflags.CacheOptions `json:"cache-to,omitempty"  hcl:"cache-to,optional" cty:"cache-to"`
+	Target           *string                 `json:"target,omitempty" hcl:"target,optional" cty:"target"`
+	Secrets          buildflags.Secrets      `json:"secret,omitempty" hcl:"secret,optional" cty:"secret"`
+	SSH              buildflags.SSHKeys      `json:"ssh,omitempty" hcl:"ssh,optional" cty:"ssh"`
+	Platforms        []string                `json:"platforms,omitempty" hcl:"platforms,optional" cty:"platforms"`
+	Outputs          buildflags.Exports      `json:"output,omitempty" hcl:"output,optional" cty:"output"`
+	Pull             *bool                   `json:"pull,omitempty" hcl:"pull,optional" cty:"pull"`
+	NoCache          *bool                   `json:"no-cache,omitempty" hcl:"no-cache,optional" cty:"no-cache"`
+	NetworkMode      *string                 `json:"network,omitempty" hcl:"network,optional" cty:"network"`
+	NoCacheFilter    []string                `json:"no-cache-filter,omitempty" hcl:"no-cache-filter,optional" cty:"no-cache-filter"`
+	ShmSize          *string                 `json:"shm-size,omitempty" hcl:"shm-size,optional"`
+	Ulimits          []string                `json:"ulimits,omitempty" hcl:"ulimits,optional"`
+	Call             *string                 `json:"call,omitempty" hcl:"call,optional" cty:"call"`
+	Entitlements     []string                `json:"entitlements,omitempty" hcl:"entitlements,optional" cty:"entitlements"`
 	// IMPORTANT: if you add more fields here, do not forget to update newOverrides/AddOverrides and docs/bake-reference.md.
 
 	// linked is a private field to mark a target used as a linked one
@@ -739,12 +739,12 @@ func (t *Target) normalize() {
 	t.Annotations = removeDupesStr(t.Annotations)
 	t.Attest = removeAttestDupes(t.Attest)
 	t.Tags = removeDupesStr(t.Tags)
-	t.Secrets = removeDupes(t.Secrets)
-	t.SSH = removeDupes(t.SSH)
+	t.Secrets = t.Secrets.Normalize()
+	t.SSH = t.SSH.Normalize()
 	t.Platforms = removeDupesStr(t.Platforms)
-	t.CacheFrom = removeDupes(t.CacheFrom)
-	t.CacheTo = removeDupes(t.CacheTo)
-	t.Outputs = removeDupes(t.Outputs)
+	t.CacheFrom = t.CacheFrom.Normalize()
+	t.CacheTo = t.CacheTo.Normalize()
+	t.Outputs = t.Outputs.Normalize()
 	t.NoCacheFilter = removeDupesStr(t.NoCacheFilter)
 	t.Ulimits = removeDupesStr(t.Ulimits)
 
@@ -815,16 +815,16 @@ func (t *Target) Merge(t2 *Target) {
 		t.Attest = removeAttestDupes(t.Attest)
 	}
 	if t2.Secrets != nil { // merge
-		t.Secrets = append(t.Secrets, t2.Secrets...)
+		t.Secrets = t.Secrets.Merge(t2.Secrets)
 	}
 	if t2.SSH != nil { // merge
-		t.SSH = append(t.SSH, t2.SSH...)
+		t.SSH = t.SSH.Merge(t2.SSH)
 	}
 	if t2.Platforms != nil { // no merge
 		t.Platforms = t2.Platforms
 	}
 	if t2.CacheFrom != nil { // merge
-		t.CacheFrom = append(t.CacheFrom, t2.CacheFrom...)
+		t.CacheFrom = t.CacheFrom.Merge(t2.CacheFrom)
 	}
 	if t2.CacheTo != nil { // no merge
 		t.CacheTo = t2.CacheTo
@@ -1333,30 +1333,19 @@ func toBuildOpt(t *Target, inp *Input) (*build.Options, error) {
 	}
 	bo.Platforms = platforms
 
-	secrets := make([]*controllerapi.Secret, len(t.Secrets))
-	for i, s := range t.Secrets {
-		secrets[i] = s.ToPB()
-	}
-	bo.SecretSpecs = secrets
-
-	secretAttachment, err := controllerapi.CreateSecrets(secrets)
+	bo.SecretSpecs = t.Secrets.ToPB()
+	secretAttachment, err := controllerapi.CreateSecrets(bo.SecretSpecs)
 	if err != nil {
 		return nil, err
 	}
 	bo.Session = append(bo.Session, secretAttachment)
 
-	var sshSpecs []*controllerapi.SSH
-	if len(t.SSH) > 0 {
-		sshSpecs := make([]*controllerapi.SSH, len(t.SSH))
-		for i, s := range t.SSH {
-			sshSpecs[i] = s.ToPB()
-		}
-	} else if buildflags.IsGitSSH(bi.ContextPath) || (inp != nil && buildflags.IsGitSSH(inp.URL)) {
-		sshSpecs = []*controllerapi.SSH{{ID: "default"}}
+	bo.SSHSpecs = t.SSH.ToPB()
+	if len(bo.SSHSpecs) == 0 && buildflags.IsGitSSH(bi.ContextPath) || (inp != nil && buildflags.IsGitSSH(inp.URL)) {
+		bo.SSHSpecs = []*controllerapi.SSH{{ID: "default"}}
 	}
-	bo.SSHSpecs = sshSpecs
 
-	sshAttachment, err := controllerapi.CreateSSH(sshSpecs)
+	sshAttachment, err := controllerapi.CreateSSH(bo.SSHSpecs)
 	if err != nil {
 		return nil, err
 	}
@@ -1372,24 +1361,14 @@ func toBuildOpt(t *Target, inp *Input) (*build.Options, error) {
 		}
 	}
 
-	cacheImports := make([]*controllerapi.CacheOptionsEntry, len(t.CacheFrom))
-	for i, ci := range t.CacheFrom {
-		cacheImports[i] = ci.ToPB()
-	}
-	bo.CacheFrom = controllerapi.CreateCaches(cacheImports)
-
-	cacheExports := make([]*controllerapi.CacheOptionsEntry, len(t.CacheTo))
-	for i, ce := range t.CacheTo {
-		cacheExports[i] = ce.ToPB()
+	if t.CacheFrom != nil {
+		bo.CacheFrom = controllerapi.CreateCaches(t.CacheFrom.ToPB())
 	}
-	bo.CacheTo = controllerapi.CreateCaches(cacheExports)
-
-	outputs := make([]*controllerapi.ExportEntry, len(t.Outputs))
-	for i, output := range t.Outputs {
-		outputs[i] = output.ToPB()
+	if t.CacheTo != nil {
+		bo.CacheTo = controllerapi.CreateCaches(t.CacheTo.ToPB())
 	}
 
-	bo.Exports, bo.ExportsLocalPathsTemporary, err = controllerapi.CreateExports(outputs)
+	bo.Exports, bo.ExportsLocalPathsTemporary, err = controllerapi.CreateExports(t.Outputs.ToPB())
 	if err != nil {
 		return nil, err
 	}
@@ -1434,34 +1413,6 @@ func defaultTarget() *Target {
 	return &Target{}
 }
 
-type comparable[E any] interface {
-	Equal(other E) bool
-}
-
-func removeDupes[E comparable[E]](s []E) []E {
-	// Move backwards through the slice.
-	// For each element, any elements after the current element are unique.
-	// If we find our current element conflicts with an existing element,
-	// then we swap the offender with the end of the slice and chop it off.
-
-	// Start at the second to last element.
-	// The last element is always unique.
-	for i := len(s) - 2; i >= 0; i-- {
-		elem := s[i]
-		// Check for duplicates after our current element.
-		for j := i + 1; j < len(s); j++ {
-			if elem.Equal(s[j]) {
-				// Found a duplicate, exchange the
-				// duplicate with the last element.
-				s[j], s[len(s)-1] = s[len(s)-1], s[j]
-				s = s[:len(s)-1]
-				break
-			}
-		}
-	}
-	return s
-}
-
 func removeDupesStr(s []string) []string {
 	i := 0
 	seen := make(map[string]struct{}, len(s))
@@ -1616,6 +1567,10 @@ type arrValue[B any] interface {
 func parseArrValue[T any, PT arrValue[T]](s []string) ([]*T, error) {
 	outputs := make([]*T, 0, len(s))
 	for _, text := range s {
+		if text == "" {
+			continue
+		}
+
 		output := new(T)
 		if err := PT(output).UnmarshalText([]byte(text)); err != nil {
 			return nil, err
@@ -1625,9 +1580,13 @@ func parseArrValue[T any, PT arrValue[T]](s []string) ([]*T, error) {
 	return outputs, nil
 }
 
-func parseCacheArrValues(s []string) ([]*buildflags.CacheOptionsEntry, error) {
-	outs := make([]*buildflags.CacheOptionsEntry, 0, len(s))
+func parseCacheArrValues(s []string) (buildflags.CacheOptions, error) {
+	var outs buildflags.CacheOptions
 	for _, in := range s {
+		if in == "" {
+			continue
+		}
+
 		if !strings.Contains(in, "=") {
 			// This is ref only format. Each field in the CSV is its own entry.
 			fields, err := csvvalue.Fields(in, nil)

bake/bake_test.go

@@ -2019,6 +2019,26 @@ target "app" {
 	})
 }
 
+// https://github.com/docker/buildx/pull/428
+// https://github.com/docker/buildx/issues/2822
+func TestEmptyAttribute(t *testing.T) {
+	fp := File{
+		Name: "docker-bake.hcl",
+		Data: []byte(`
+target "app" {
+  output = [""]
+}
+`),
+	}
+
+	ctx := context.TODO()
+
+	m, _, err := ReadTargets(ctx, []File{fp}, []string{"app"}, nil, nil, &EntitlementConf{})
+	require.Equal(t, 1, len(m))
+	require.Len(t, m["app"].Outputs, 0)
+	require.NoError(t, err)
+}
+
 func stringify[V fmt.Stringer](values []V) []string {
 	s := make([]string, len(values))
 	for i, v := range values {

bake/compose.go

@@ -353,28 +353,28 @@ func (t *Target) composeExtTarget(exts map[string]interface{}) error {
 		if err != nil {
 			return err
 		}
-		t.CacheFrom = removeDupes(append(t.CacheFrom, cacheFrom...))
+		t.CacheFrom = t.CacheFrom.Merge(cacheFrom)
 	}
 	if len(xb.CacheTo) > 0 {
 		cacheTo, err := parseCacheArrValues(xb.CacheTo)
 		if err != nil {
 			return err
 		}
-		t.CacheTo = removeDupes(append(t.CacheTo, cacheTo...))
+		t.CacheTo = t.CacheTo.Merge(cacheTo)
 	}
 	if len(xb.Secrets) > 0 {
 		secrets, err := parseArrValue[buildflags.Secret](xb.Secrets)
 		if err != nil {
 			return err
 		}
-		t.Secrets = removeDupes(append(t.Secrets, secrets...))
+		t.Secrets = t.Secrets.Merge(secrets)
 	}
 	if len(xb.SSH) > 0 {
 		ssh, err := parseArrValue[buildflags.SSH](xb.SSH)
 		if err != nil {
 			return err
 		}
-		t.SSH = removeDupes(append(t.SSH, ssh...))
+		t.SSH = t.SSH.Merge(ssh)
 		slices.SortFunc(t.SSH, func(a, b *buildflags.SSH) int {
 			return a.Less(b)
 		})
@@ -387,7 +387,7 @@ func (t *Target) composeExtTarget(exts map[string]interface{}) error {
 		if err != nil {
 			return err
 		}
-		t.Outputs = removeDupes(append(t.Outputs, outputs...))
+		t.Outputs = t.Outputs.Merge(outputs)
 	}
 	if xb.Pull != nil {
 		t.Pull = xb.Pull

bake/hcl_test.go

@@ -606,7 +606,7 @@ func TestHCLAttrsCapsuleType(t *testing.T) {
 	target "app" {
 		cache-from = [
 			{ type = "registry", ref = "user/app:cache" },
-			{ type = "local", src = "path/to/cache" },
+			"type=local,src=path/to/cache",
 		]
 
 		cache-to = [
@@ -615,6 +615,7 @@ func TestHCLAttrsCapsuleType(t *testing.T) {
 
 		output = [
 			{ type = "oci", dest = "../out.tar" },
+			"type=local,dest=../out",
 		]
 
 		secret = [
@@ -633,7 +634,7 @@ func TestHCLAttrsCapsuleType(t *testing.T) {
 	require.NoError(t, err)
 
 	require.Equal(t, 1, len(c.Targets))
-	require.Equal(t, []string{"type=oci,dest=../out.tar"}, stringify(c.Targets[0].Outputs))
+	require.Equal(t, []string{"type=local,dest=../out", "type=oci,dest=../out.tar"}, stringify(c.Targets[0].Outputs))
 	require.Equal(t, []string{"type=local,src=path/to/cache", "user/app:cache"}, stringify(c.Targets[0].CacheFrom))
 	require.Equal(t, []string{"type=local,dest=path/to/cache"}, stringify(c.Targets[0].CacheTo))
 	require.Equal(t, []string{"id=mysecret,src=/local/secret", "id=mysecret2,env=TOKEN"}, stringify(c.Targets[0].Secrets))
@@ -649,13 +650,14 @@ func TestHCLAttrsCapsuleTypeVars(t *testing.T) {
 	target "app" {
 		cache-from = [
 			{ type = "registry", ref = "user/app:cache" },
-			{ type = "local", src = "path/to/cache" },
+			"type=local,src=path/to/cache",
 		]
 
 		cache-to = [ target.app.cache-from[0] ]
 
 		output = [
 			{ type = "oci", dest = "../out.tar" },
+			"type=local,dest=../out",
 		]
 
 		secret = [
@@ -674,7 +676,7 @@ func TestHCLAttrsCapsuleTypeVars(t *testing.T) {
 		output = [ "type=oci,dest=../${foo}.tar" ]
 
 		secret = [
-			{ id = target.app.output[0].type, src = "/local/secret" },
+			{ id = target.app.output[0].type, src = "/${target.app.cache-from[1].type}/secret" },
 		]
 	}
 	`)
@@ -696,7 +698,7 @@ func TestHCLAttrsCapsuleTypeVars(t *testing.T) {
 	}
 
 	app := findTarget(t, "app")
-	require.Equal(t, []string{"type=oci,dest=../out.tar"}, stringify(app.Outputs))
+	require.Equal(t, []string{"type=local,dest=../out", "type=oci,dest=../out.tar"}, stringify(app.Outputs))
 	require.Equal(t, []string{"type=local,src=path/to/cache", "user/app:cache"}, stringify(app.CacheFrom))
 	require.Equal(t, []string{"user/app:cache"}, stringify(app.CacheTo))
 	require.Equal(t, []string{"id=mysecret,src=/local/secret"}, stringify(app.Secrets))