From 3e60bbe30f30b0391cdfe8ee73c30078d2751f4b Mon Sep 17 00:00:00 2001 From: Justin Chadwell Date: Tue, 28 Mar 2023 11:33:10 +0100 Subject: [PATCH 1/3] buildflags: use disabled instead of enabled for attestations Signed-off-by: Justin Chadwell --- util/buildflags/attests.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/util/buildflags/attests.go b/util/buildflags/attests.go index ab1b163ac98b..38370dfa8efa 100644 --- a/util/buildflags/attests.go +++ b/util/buildflags/attests.go @@ -14,7 +14,7 @@ func CanonicalizeAttest(attestType string, in string) string { return "" } if b, err := strconv.ParseBool(in); err == nil { - return fmt.Sprintf("type=%s,enabled=%t", attestType, b) + return fmt.Sprintf("type=%s,disabled=%t", attestType, !b) } return fmt.Sprintf("type=%s,%s", attestType, in) } @@ -23,7 +23,7 @@ func ParseAttests(in []string) (map[string]*string, error) { out := map[string]*string{} for _, in := range in { in := in - attestType, enabled, err := parseAttest(in) + attestType, disabled, err := parseAttest(in) if err != nil { return nil, err } @@ -32,10 +32,10 @@ func ParseAttests(in []string) (map[string]*string, error) { if _, ok := out[k]; ok { return nil, errors.Errorf("duplicate attestation field %s", attestType) } - if enabled { - out[k] = &in - } else { + if disabled { out[k] = nil + } else { + out[k] = &in } } return out, nil @@ -53,7 +53,7 @@ func parseAttest(in string) (string, bool, error) { } attestType := "" - enabled := true + disabled := true for _, field := range fields { key, value, ok := strings.Cut(field, "=") if !ok { @@ -64,8 +64,8 @@ func parseAttest(in string) (string, bool, error) { switch key { case "type": attestType = value - case "enabled": - enabled, err = strconv.ParseBool(value) + case "disabled": + disabled, err = strconv.ParseBool(value) if err != nil { return "", false, err } @@ -75,5 +75,5 @@ func parseAttest(in string) (string, bool, error) { return "", false, errors.Errorf("attestation type not specified") } - return attestType, enabled, nil + return attestType, disabled, nil } From 1a01779e5bc314e4c3b10e35d80a4fb903fe8f28 Mon Sep 17 00:00:00 2001 From: Justin Chadwell Date: Tue, 28 Mar 2023 11:40:43 +0100 Subject: [PATCH 2/3] buildflags: merge attest flags if disabled is set This ensures that `--sbom=` and `--attest type=sbom` can be appropriately merged for build, and `--sbom=` and `target.attest=["type=sbom"]` can be appropriately merged for bake. Signed-off-by: Justin Chadwell --- util/buildflags/attests.go | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/util/buildflags/attests.go b/util/buildflags/attests.go index 38370dfa8efa..aef872c994ee 100644 --- a/util/buildflags/attests.go +++ b/util/buildflags/attests.go @@ -30,9 +30,16 @@ func ParseAttests(in []string) (map[string]*string, error) { k := "attest:" + attestType if _, ok := out[k]; ok { - return nil, errors.Errorf("duplicate attestation field %s", attestType) + if disabled == nil { + return nil, errors.Errorf("duplicate attestation field %s", attestType) + } + if *disabled { + out[k] = nil + } + continue } - if disabled { + + if disabled != nil && *disabled { out[k] = nil } else { out[k] = &in @@ -41,23 +48,23 @@ func ParseAttests(in []string) (map[string]*string, error) { return out, nil } -func parseAttest(in string) (string, bool, error) { +func parseAttest(in string) (string, *bool, error) { if in == "" { - return "", false, nil + return "", nil, nil } csvReader := csv.NewReader(strings.NewReader(in)) fields, err := csvReader.Read() if err != nil { - return "", false, err + return "", nil, err } attestType := "" - disabled := true + var disabled *bool for _, field := range fields { key, value, ok := strings.Cut(field, "=") if !ok { - return "", false, errors.Errorf("invalid value %s", field) + return "", nil, errors.Errorf("invalid value %s", field) } key = strings.TrimSpace(strings.ToLower(key)) @@ -65,14 +72,15 @@ func parseAttest(in string) (string, bool, error) { case "type": attestType = value case "disabled": - disabled, err = strconv.ParseBool(value) + b, err := strconv.ParseBool(value) if err != nil { - return "", false, err + return "", nil, err } + disabled = &b } } if attestType == "" { - return "", false, errors.Errorf("attestation type not specified") + return "", nil, errors.Errorf("attestation type not specified") } return attestType, disabled, nil From 8dcd5d5c022d7241cf0a4367811002a7797a4ab4 Mon Sep 17 00:00:00 2001 From: Justin Chadwell Date: Tue, 28 Mar 2023 11:28:11 +0100 Subject: [PATCH 3/3] bake: add tests for attestation overrides Signed-off-by: Justin Chadwell --- bake/bake_test.go | 107 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 107 insertions(+) diff --git a/bake/bake_test.go b/bake/bake_test.go index 4311b252e8c5..dd62844b0554 100644 --- a/bake/bake_test.go +++ b/bake/bake_test.go @@ -1358,3 +1358,110 @@ func TestJSONNullVars(t *testing.T) { require.NoError(t, err) require.Equal(t, map[string]*string{"bar": ptrstr("baz")}, m["default"].Args) } + +func TestAttestDuplicates(t *testing.T) { + fp := File{ + Name: "docker-bake.hcl", + Data: []byte( + `target "default" { + attest = ["type=sbom", "type=sbom,generator=custom"] + }`), + } + ctx := context.TODO() + m, _, err := ReadTargets(ctx, []File{fp}, []string{"default"}, nil, nil) + require.NoError(t, err) + + _, err = TargetsToBuildOpt(m, &Input{}) + require.Error(t, err) +} + +func TestAttestOverride(t *testing.T) { + ctx := context.TODO() + + // file without attest set + fp := File{ + Name: "docker-bake.hcl", + Data: []byte(`target "default" {}`), + } + + // no override + m, _, err := ReadTargets(ctx, []File{fp}, []string{"default"}, nil, nil) + require.NoError(t, err) + require.Empty(t, m["default"].Attest) + + opts, err := TargetsToBuildOpt(m, &Input{}) + require.NoError(t, err) + require.Empty(t, opts["default"].Attests) + + // with override + m, _, err = ReadTargets(ctx, []File{fp}, []string{"default"}, []string{"*.attest=type=sbom,generator=custom"}, nil) + require.NoError(t, err) + require.Equal(t, []string{"type=sbom,generator=custom"}, m["default"].Attest) + + opts, err = TargetsToBuildOpt(m, &Input{}) + require.NoError(t, err) + require.Equal(t, map[string]*string{"attest:sbom": ptrstr("type=sbom,generator=custom")}, opts["default"].Attests) + + // with disabled=true override + m, _, err = ReadTargets(ctx, []File{fp}, []string{"default"}, []string{"*.attest=type=sbom,disabled=true"}, nil) + require.NoError(t, err) + require.Equal(t, []string{"type=sbom,disabled=true"}, m["default"].Attest) + + opts, err = TargetsToBuildOpt(m, &Input{}) + require.NoError(t, err) + require.Equal(t, map[string]*string{"attest:sbom": nil}, opts["default"].Attests) + + // with disabled=false override + m, _, err = ReadTargets(ctx, []File{fp}, []string{"default"}, []string{"*.attest=type=sbom,disabled=false"}, nil) + require.NoError(t, err) + require.Equal(t, []string{"type=sbom,disabled=false"}, m["default"].Attest) + + opts, err = TargetsToBuildOpt(m, &Input{}) + require.NoError(t, err) + require.Equal(t, map[string]*string{"attest:sbom": ptrstr("type=sbom,disabled=false")}, opts["default"].Attests) + + // file with attest set + fp = File{ + Name: "docker-bake.hcl", + Data: []byte( + `target "default" { + attest = ["type=sbom,generator=custom"] + }`), + } + + // no override + m, _, err = ReadTargets(ctx, []File{fp}, []string{"default"}, nil, nil) + require.NoError(t, err) + require.Equal(t, []string{"type=sbom,generator=custom"}, m["default"].Attest) + + opts, err = TargetsToBuildOpt(m, &Input{}) + require.NoError(t, err) + require.Equal(t, map[string]*string{"attest:sbom": ptrstr("type=sbom,generator=custom")}, opts["default"].Attests) + + // with duplicate override + m, _, err = ReadTargets(ctx, []File{fp}, []string{"default"}, []string{"*.attest=type=sbom,generator=custom"}, nil) + require.NoError(t, err) + require.Equal(t, []string{"type=sbom,generator=custom"}, m["default"].Attest) + + opts, err = TargetsToBuildOpt(m, &Input{}) + require.NoError(t, err) + require.Equal(t, map[string]*string{"attest:sbom": ptrstr("type=sbom,generator=custom")}, opts["default"].Attests) + + // with disabled=true override + m, _, err = ReadTargets(ctx, []File{fp}, []string{"default"}, []string{"*.attest=type=sbom,disabled=true"}, nil) + require.NoError(t, err) + require.Equal(t, []string{"type=sbom,generator=custom", "type=sbom,disabled=true"}, m["default"].Attest) + + opts, err = TargetsToBuildOpt(m, &Input{}) + require.NoError(t, err) + require.Equal(t, map[string]*string{"attest:sbom": nil}, opts["default"].Attests) + + // with disabled=false override + m, _, err = ReadTargets(ctx, []File{fp}, []string{"default"}, []string{"*.attest=type=sbom,disabled=false"}, nil) + require.NoError(t, err) + require.Equal(t, []string{"type=sbom,generator=custom", "type=sbom,disabled=false"}, m["default"].Attest) + + opts, err = TargetsToBuildOpt(m, &Input{}) + require.NoError(t, err) + require.Equal(t, map[string]*string{"attest:sbom": ptrstr("type=sbom,generator=custom")}, opts["default"].Attests) +}