From 41f8e5c85c343f697d47ab79e9bf718bf45d8a64 Mon Sep 17 00:00:00 2001 From: Laurent Goderre Date: Thu, 27 Feb 2025 09:56:46 -0500 Subject: [PATCH 1/3] Add attest extra args tests Signed-off-by: Laurent Goderre --- bake/hcl_test.go | 4 ++-- util/buildflags/attests_test.go | 42 +++++++++++++++++++++++++++++++-- 2 files changed, 42 insertions(+), 4 deletions(-) diff --git a/bake/hcl_test.go b/bake/hcl_test.go index ac507a9b..7ee8a259 100644 --- a/bake/hcl_test.go +++ b/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)) diff --git a/util/buildflags/attests_test.go b/util/buildflags/attests_test.go index c18545f2..6e2c942f 100644 --- a/util/buildflags/attests_test.go +++ b/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) From 6da88e1555db601a28291777ab592193fe5c868b Mon Sep 17 00:00:00 2001 From: Laurent Goderre Date: Thu, 27 Feb 2025 10:42:09 -0500 Subject: [PATCH 2/3] Fix handling of attest extra arguments Signed-off-by: Laurent Goderre --- util/buildflags/attests.go | 3 +-- util/buildflags/export.go | 16 +++++++++++++--- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/util/buildflags/attests.go b/util/buildflags/attests.go index e0fe1e2f..54997307 100644 --- a/util/buildflags/attests.go +++ b/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": diff --git a/util/buildflags/export.go b/util/buildflags/export.go index ba364c9f..8460d3e2 100644 --- a/util/buildflags/export.go +++ b/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('=') - w.sb.WriteString(value) + + pair := key + "=" + 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) { From 6019a2b32f3444a282e0540b0492f27b261f826f Mon Sep 17 00:00:00 2001 From: "Jonathan A. Sternberg" Date: Mon, 24 Feb 2025 10:09:02 -0600 Subject: [PATCH 3/3] buildflags: skip empty cache entries when parsing Broken in 11c84973ef104e48eb88a41b5b23d6a559efe868. The section to skip an empty input was accidentally removed when some code was refactored to fix a separate issue. This skips empty cache entries which allows disabling the `cache-from` and `cache-to` entries from the command line overrides. Signed-off-by: Jonathan A. Sternberg --- tests/bake.go | 43 ++++++++++++++++++++++++++++++++++++++++ util/buildflags/cache.go | 4 ++++ 2 files changed, 47 insertions(+) diff --git a/tests/bake.go b/tests/bake.go index f106f21b..4935e115 100644 --- a/tests/bake.go +++ b/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) { diff --git a/util/buildflags/cache.go b/util/buildflags/cache.go index e9256900..f9c99034 100644 --- a/util/buildflags/cache.go +++ b/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)