Merge pull request #3067 from crazy-max/0.21_picks_0.21.3

[v0.21] cherry-picks for v0.21.3

bake/hcl_test.go

@@ -608,7 +608,7 @@ func TestHCLAttrsCapsuleType(t *testing.T) {
 	target "app" {
 		attest = [
 			{ type = "provenance", mode = "max" },
-			"type=sbom,disabled=true",
+			"type=sbom,disabled=true,generator=foo,\"ENV1=bar,baz\",ENV2=hello",
 		]
 
 		cache-from = [
@@ -641,7 +641,7 @@ func TestHCLAttrsCapsuleType(t *testing.T) {
 	require.NoError(t, err)
 
 	require.Equal(t, 1, len(c.Targets))
-	require.Equal(t, []string{"type=provenance,mode=max", "type=sbom,disabled=true"}, stringify(c.Targets[0].Attest))
+	require.Equal(t, []string{"type=provenance,mode=max", "type=sbom,disabled=true,\"ENV1=bar,baz\",ENV2=hello,generator=foo"}, stringify(c.Targets[0].Attest))
 	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))

commands/bake.go

@@ -66,7 +66,11 @@ type bakeOptions struct {
 func runBake(ctx context.Context, dockerCli command.Cli, targets []string, in bakeOptions, cFlags commonFlags) (err error) {
 	mp := dockerCli.MeterProvider()
 
-	ctx, end, err := tracing.TraceCurrentCommand(ctx, "bake")
+	ctx, end, err := tracing.TraceCurrentCommand(ctx, append([]string{"bake"}, targets...),
+		attribute.String("builder", in.builder),
+		attribute.StringSlice("targets", targets),
+		attribute.StringSlice("files", in.files),
+	)
 	if err != nil {
 		return err
 	}
@@ -283,7 +287,7 @@ func runBake(ctx context.Context, dockerCli command.Cli, targets []string, in ba
 		}
 	}
 
-	if err := saveLocalStateGroup(dockerCli, in, targets, bo, overrides, def); err != nil {
+	if err := saveLocalStateGroup(dockerCli, in, targets, bo); err != nil {
 		return err
 	}
 
@@ -488,7 +492,14 @@ func bakeCmd(dockerCli command.Cli, rootOpts *rootOptions) *cobra.Command {
 	return cmd
 }
 
-func saveLocalStateGroup(dockerCli command.Cli, in bakeOptions, targets []string, bo map[string]build.Options, overrides []string, def any) error {
+func saveLocalStateGroup(dockerCli command.Cli, in bakeOptions, targets []string, bo map[string]build.Options) error {
+	l, err := localstate.New(confutil.NewConfig(dockerCli))
+	if err != nil {
+		return err
+	}
+
+	defer l.MigrateIfNeeded()
+
 	prm := confutil.MetadataProvenance()
 	if len(in.metadataFile) == 0 {
 		prm = confutil.MetadataProvenanceModeDisabled
@@ -508,19 +519,10 @@ func saveLocalStateGroup(dockerCli command.Cli, in bakeOptions, targets []string
 	if len(refs) == 0 {
 		return nil
 	}
-	l, err := localstate.New(confutil.NewConfig(dockerCli))
+
-	if err != nil {
-		return err
-	}
-	dtdef, err := json.MarshalIndent(def, "", "  ")
-	if err != nil {
-		return err
-	}
 	return l.SaveGroup(groupRef, localstate.StateGroup{
-		Definition: dtdef,
+		Refs:    refs,
-		Targets:    targets,
+		Targets: targets,
-		Inputs:     overrides,
-		Refs:       refs,
 	})
 }
 

commands/build.go

@@ -285,7 +285,11 @@ func (o *buildOptionsHash) String() string {
 func runBuild(ctx context.Context, dockerCli command.Cli, options buildOptions) (err error) {
 	mp := dockerCli.MeterProvider()
 
-	ctx, end, err := tracing.TraceCurrentCommand(ctx, "build")
+	ctx, end, err := tracing.TraceCurrentCommand(ctx, []string{"build", options.contextPath},
+		attribute.String("builder", options.builder),
+		attribute.String("context", options.contextPath),
+		attribute.String("dockerfile", options.dockerfileName),
+	)
 	if err != nil {
 		return err
 	}

localstate/localstate.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"strconv"
 	"sync"
 
 	"github.com/docker/buildx/util/confutil"
@@ -14,6 +15,7 @@ import (
 )
 
 const (
+	version  = 2
 	refsDir  = "refs"
 	groupDir = "__group__"
 )
@@ -31,12 +33,8 @@ type State struct {
 }
 
 type StateGroup struct {
-	// Definition is the raw representation of the group (bake definition)
-	Definition []byte
 	// Targets are the targets invoked
 	Targets []string `json:",omitempty"`
-	// Inputs are the user inputs (bake overrides)
-	Inputs []string `json:",omitempty"`
 	// Refs are used to track all the refs that belong to the same group
 	Refs []string
 }
@@ -52,9 +50,7 @@ func New(cfg *confutil.Config) (*LocalState, error) {
 	if err := cfg.MkdirAll(refsDir, 0700); err != nil {
 		return nil, err
 	}
-	return &LocalState{
+	return &LocalState{cfg: cfg}, nil
-		cfg: cfg,
-	}, nil
 }
 
 func (ls *LocalState) ReadRef(builderName, nodeName, id string) (*State, error) {
@@ -87,8 +83,12 @@ func (ls *LocalState) SaveRef(builderName, nodeName, id string, st State) error
 	return ls.cfg.AtomicWriteFile(filepath.Join(refDir, id), dt, 0644)
 }
 
+func (ls *LocalState) GroupDir() string {
+	return filepath.Join(ls.cfg.Dir(), refsDir, groupDir)
+}
+
 func (ls *LocalState) ReadGroup(id string) (*StateGroup, error) {
-	dt, err := os.ReadFile(filepath.Join(ls.cfg.Dir(), refsDir, groupDir, id))
+	dt, err := os.ReadFile(filepath.Join(ls.GroupDir(), id))
 	if err != nil {
 		return nil, err
 	}
@@ -208,7 +208,7 @@ func (ls *LocalState) removeGroup(id string) error {
 	if id == "" {
 		return errors.Errorf("group ref empty")
 	}
-	f := filepath.Join(ls.cfg.Dir(), refsDir, groupDir, id)
+	f := filepath.Join(ls.GroupDir(), id)
 	if _, err := os.Lstat(f); err != nil {
 		if !os.IsNotExist(err) {
 			return err
@@ -230,3 +230,16 @@ func (ls *LocalState) validate(builderName, nodeName, id string) error {
 	}
 	return nil
 }
+
+func (ls *LocalState) readVersion() int {
+	if vdt, err := os.ReadFile(filepath.Join(ls.cfg.Dir(), refsDir, "version")); err == nil {
+		if v, err := strconv.Atoi(string(vdt)); err == nil {
+			return v
+		}
+	}
+	return 1
+}
+
+func (ls *LocalState) writeVersion(version int) error {
+	return ls.cfg.AtomicWriteFile(filepath.Join(refsDir, "version"), []byte(strconv.Itoa(version)), 0600)
+}

localstate/localstate_test.go

@@ -68,10 +68,8 @@ var (
 
 	testStateGroupID = "kvqs0sgly2rmitz84r25u9qd0"
 	testStateGroup   = StateGroup{
-		Definition: []byte(`{"group":{"default":{"targets":["pre-checkin"]},"pre-checkin":{"targets":["vendor-update","format","build"]}},"target":{"build":{"context":".","dockerfile":"dev.Dockerfile","target":"build-update","platforms":["linux/amd64"],"output":["."]},"format":{"context":".","dockerfile":"dev.Dockerfile","target":"format-update","platforms":["linux/amd64"],"output":["."]},"vendor-update":{"context":".","dockerfile":"dev.Dockerfile","target":"vendor-update","platforms":["linux/amd64"],"output":["."]}}}`),
+		Targets: []string{"pre-checkin"},
-		Targets:    []string{"pre-checkin"},
+		Refs:    []string{"builder/builder0/hx2qf1w11qvz1x3k471c5i8xw", "builder/builder0/968zj0g03jmlx0s8qslnvh6rl", "builder/builder0/naf44f9i1710lf7y12lv5hb1z"},
-		Inputs:     []string{"*.platform=linux/amd64"},
-		Refs:       []string{"builder/builder0/hx2qf1w11qvz1x3k471c5i8xw", "builder/builder0/968zj0g03jmlx0s8qslnvh6rl", "builder/builder0/naf44f9i1710lf7y12lv5hb1z"},
 	}
 
 	testStateGroupRef1ID = "hx2qf1w11qvz1x3k471c5i8xw"

localstate/migrate.go

@@ -0,0 +1,56 @@
+package localstate
+
+import (
+	"encoding/json"
+	"os"
+	"path/filepath"
+
+	"github.com/pkg/errors"
+)
+
+func (ls *LocalState) MigrateIfNeeded() error {
+	currentVersion := ls.readVersion()
+	if currentVersion == version {
+		return nil
+	}
+	migrations := map[int]func(*LocalState) error{
+		2: (*LocalState).migration2,
+	}
+	for v := currentVersion + 1; v <= version; v++ {
+		migration, found := migrations[v]
+		if !found {
+			return errors.Errorf("localstate migration v%d not found", v)
+		}
+		if err := migration(ls); err != nil {
+			return errors.Wrapf(err, "localstate migration v%d failed", v)
+		}
+	}
+	return ls.writeVersion(version)
+}
+
+func (ls *LocalState) migration2() error {
+	return filepath.Walk(ls.GroupDir(), func(path string, info os.FileInfo, err error) error {
+		if err != nil {
+			return err
+		}
+		if info.IsDir() {
+			return nil
+		}
+		dt, err := os.ReadFile(path)
+		if err != nil {
+			return err
+		}
+		var stg StateGroup
+		if err := json.Unmarshal(dt, &stg); err != nil {
+			return err
+		}
+		mdt, err := json.Marshal(stg)
+		if err != nil {
+			return err
+		}
+		if err := os.WriteFile(path, mdt, 0600); err != nil {
+			return err
+		}
+		return nil
+	})
+}

tests/bake.go

@@ -38,6 +38,7 @@ func bakeCmd(sb integration.Sandbox, opts ...cmdOpt) (string, error) {
 var bakeTests = []func(t *testing.T, sb integration.Sandbox){
 	testBakePrint,
 	testBakePrintSensitive,
+	testBakePrintOverrideEmpty,
 	testBakeLocal,
 	testBakeLocalMulti,
 	testBakeRemote,
@@ -286,6 +287,47 @@ RUN echo "Hello ${HELLO}"
 	}
 }
 
+func testBakePrintOverrideEmpty(t *testing.T, sb integration.Sandbox) {
+	dockerfile := []byte(`
+FROM scratch
+COPY foo /foo
+	`)
+	bakefile := []byte(`
+target "default" {
+	cache-to = ["type=gha,mode=min,scope=integration-tests"]
+}
+`)
+	dir := tmpdir(
+		t,
+		fstest.CreateFile("docker-bake.hcl", bakefile, 0600),
+		fstest.CreateFile("Dockerfile", dockerfile, 0600),
+		fstest.CreateFile("foo", []byte("foo"), 0600),
+	)
+
+	cmd := buildxCmd(sb, withDir(dir), withArgs("bake", "--print", "--set", "*.cache-to="))
+	stdout := bytes.Buffer{}
+	stderr := bytes.Buffer{}
+	cmd.Stdout = &stdout
+	cmd.Stderr = &stderr
+	require.NoError(t, cmd.Run(), stdout.String(), stderr.String())
+
+	require.JSONEq(t, `{
+	"group": {
+		"default": {
+			"targets": [
+				"default"
+			]
+		}
+	},
+	"target": {
+		"default": {
+			"context": ".",
+			"dockerfile": "Dockerfile"
+		}
+	}
+}`, stdout.String())
+}
+
 func testBakeLocal(t *testing.T, sb integration.Sandbox) {
 	dockerfile := []byte(`
 FROM scratch
@@ -871,6 +913,7 @@ target "default" {
 		})
 	}
 }
+
 func testBakeSetNonExistingOutsideNoParallel(t *testing.T, sb integration.Sandbox) {
 	for _, ent := range []bool{true, false} {
 		t.Run(fmt.Sprintf("ent=%v", ent), func(t *testing.T) {

util/buildflags/attests.go

@@ -148,9 +148,8 @@ func (a *Attest) UnmarshalText(text []byte) error {
 		if !ok {
 			return errors.Errorf("invalid value %s", field)
 		}
-		key = strings.TrimSpace(strings.ToLower(key))
 
-		switch key {
+		switch strings.TrimSpace(strings.ToLower(key)) {
 		case "type":
 			a.Type = value
 		case "disabled":

util/buildflags/attests_test.go

@@ -13,16 +13,21 @@ func TestAttests(t *testing.T) {
 		attests := Attests{
 			{Type: "provenance", Attrs: map[string]string{"mode": "max"}},
 			{Type: "sbom", Disabled: true},
+			{Type: "sbom", Attrs: map[string]string{
+				"generator": "scanner",
+				"ENV1":      `"foo,bar"`,
+				"Env2":      "hello",
+			}},
 		}
 
-		expected := `[{"type":"provenance","mode":"max"},{"type":"sbom","disabled":true}]`
+		expected := `[{"type":"provenance","mode":"max"},{"type":"sbom","disabled":true},{"ENV1":"\"foo,bar\"","Env2":"hello","generator":"scanner","type":"sbom"}]`
 		actual, err := json.Marshal(attests)
 		require.NoError(t, err)
 		require.JSONEq(t, expected, string(actual))
 	})
 
 	t.Run("UnmarshalJSON", func(t *testing.T) {
-		in := `[{"type":"provenance","mode":"max"},{"type":"sbom","disabled":true}]`
+		in := `[{"type":"provenance","mode":"max"},{"type":"sbom","disabled":true},{"ENV1":"\"foo,bar\"","Env2":"hello","generator":"scanner","type":"sbom"}]`
 
 		var actual Attests
 		err := json.Unmarshal([]byte(in), &actual)
@@ -31,6 +36,11 @@ func TestAttests(t *testing.T) {
 		expected := Attests{
 			{Type: "provenance", Attrs: map[string]string{"mode": "max"}},
 			{Type: "sbom", Disabled: true, Attrs: map[string]string{}},
+			{Type: "sbom", Disabled: false, Attrs: map[string]string{
+				"generator": "scanner",
+				"ENV1":      `"foo,bar"`,
+				"Env2":      "hello",
+			}},
 		}
 		require.Equal(t, expected, actual)
 	})
@@ -41,7 +51,14 @@ func TestAttests(t *testing.T) {
 				"type": cty.StringVal("provenance"),
 				"mode": cty.StringVal("max"),
 			}),
+			cty.ObjectVal(map[string]cty.Value{
+				"type":      cty.StringVal("sbom"),
+				"generator": cty.StringVal("scan"),
+				"ENV1":      cty.StringVal(`foo,bar`),
+				"Env2":      cty.StringVal(`hello`),
+			}),
 			cty.StringVal("type=sbom,disabled=true"),
+			cty.StringVal(`type=sbom,generator=scan,"FOO=bar,baz",Hello=World`),
 		})
 
 		var actual Attests
@@ -50,7 +67,17 @@ func TestAttests(t *testing.T) {
 
 		expected := Attests{
 			{Type: "provenance", Attrs: map[string]string{"mode": "max"}},
+			{Type: "sbom", Attrs: map[string]string{
+				"generator": "scan",
+				"ENV1":      "foo,bar",
+				"Env2":      "hello",
+			}},
 			{Type: "sbom", Disabled: true, Attrs: map[string]string{}},
+			{Type: "sbom", Attrs: map[string]string{
+				"generator": "scan",
+				"FOO":       "bar,baz",
+				"Hello":     "World",
+			}},
 		}
 		require.Equal(t, expected, actual)
 	})
@@ -59,6 +86,11 @@ func TestAttests(t *testing.T) {
 		attests := Attests{
 			{Type: "provenance", Attrs: map[string]string{"mode": "max"}},
 			{Type: "sbom", Disabled: true},
+			{Type: "sbom", Attrs: map[string]string{
+				"generator": "scan",
+				"ENV1":      `"foo,bar"`,
+				"Env2":      "hello",
+			}},
 		}
 
 		actual := attests.ToCtyValue()
@@ -71,6 +103,12 @@ func TestAttests(t *testing.T) {
 				"type":     cty.StringVal("sbom"),
 				"disabled": cty.StringVal("true"),
 			}),
+			cty.MapVal(map[string]cty.Value{
+				"type":      cty.StringVal("sbom"),
+				"generator": cty.StringVal("scan"),
+				"ENV1":      cty.StringVal(`"foo,bar"`),
+				"Env2":      cty.StringVal("hello"),
+			}),
 		})
 
 		result := actual.Equals(expected)

util/buildflags/cache.go

@@ -175,6 +175,10 @@ func ParseCacheEntry(in []string) (CacheOptions, error) {
 
 	opts := make(CacheOptions, 0, len(in))
 	for _, in := range in {
+		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)

util/buildflags/entitlements.go

@@ -1,14 +1,11 @@
 package buildflags
 
 import (
-	"log"
-
 	"github.com/moby/buildkit/util/entitlements"
 )
 
 func ParseEntitlements(in []string) ([]string, error) {
 	out := make([]string, 0, len(in))
-	log.Printf("in: %#v", in)
 	for _, v := range in {
 		if v == "" {
 			continue
@@ -19,6 +16,5 @@ func ParseEntitlements(in []string) ([]string, error) {
 		}
 		out = append(out, v)
 	}
-	log.Printf("Parsed entitlements: %v", out)
 	return out, nil
 }

util/buildflags/export.go

@@ -1,6 +1,7 @@
 package buildflags
 
 import (
+	"encoding/csv"
 	"encoding/json"
 	"maps"
 	"regexp"
@@ -259,9 +260,18 @@ func (w *csvBuilder) Write(key, value string) {
 	if w.sb.Len() > 0 {
 		w.sb.WriteByte(',')
 	}
-	w.sb.WriteString(key)
+
-	w.sb.WriteByte('=')
+	pair := key + "=" + value
-	w.sb.WriteString(value)
+	if strings.ContainsRune(pair, ',') || strings.ContainsRune(pair, '"') {
+		var attr strings.Builder
+		writer := csv.NewWriter(&attr)
+		writer.Write([]string{pair})
+		writer.Flush()
+		// Strips the extra newline added by the csv writer
+		pair = strings.TrimSpace(attr.String())
+	}
+
+	w.sb.WriteString(pair)
 }
 
 func (w *csvBuilder) WriteAttributes(attrs map[string]string) {

util/tracing/trace.go

@@ -2,7 +2,6 @@ package tracing
 
 import (
 	"context"
-	"os"
 	"strings"
 
 	"github.com/moby/buildkit/util/tracing/delegated"
@@ -13,7 +12,7 @@ import (
 	"go.opentelemetry.io/otel/trace"
 )
 
-func TraceCurrentCommand(ctx context.Context, name string) (context.Context, func(error), error) {
+func TraceCurrentCommand(ctx context.Context, args []string, attrs ...attribute.KeyValue) (context.Context, func(error), error) {
 	opts := []sdktrace.TracerProviderOption{
 		sdktrace.WithResource(detect.Resource()),
 		sdktrace.WithBatcher(delegated.DefaultExporter),
@@ -25,8 +24,8 @@ func TraceCurrentCommand(ctx context.Context, name string) (context.Context, fun
 	}
 
 	tp := sdktrace.NewTracerProvider(opts...)
-	ctx, span := tp.Tracer("").Start(ctx, name, trace.WithAttributes(
+	ctx, span := tp.Tracer("").Start(ctx, strings.Join(args, " "), trace.WithAttributes(
-		attribute.String("command", strings.Join(os.Args, " ")),
+		attrs...,
 	))
 
 	return ctx, func(err error) {