From 91324b53774540cbc65695a7fadbb958276a480c Mon Sep 17 00:00:00 2001 From: Justin Chadwell Date: Wed, 14 Dec 2022 14:20:17 +0000 Subject: [PATCH 1/5] tests: fixup attestation tar to not panic when file not found Signed-off-by: Justin Chadwell --- client/client_test.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/client/client_test.go b/client/client_test.go index 81f6186d898f..21e8bf4a896e 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -7469,8 +7469,9 @@ func testExportAttestations(t *testing.T, sb integration.Sandbox) { for _, p := range ps { var attest intoto.Statement - dt := m[path.Join(strings.ReplaceAll(platforms.Format(p), "/", "_"), "test.attestation.json")].Data - require.NoError(t, json.Unmarshal(dt, &attest)) + item := m[path.Join(strings.ReplaceAll(platforms.Format(p), "/", "_"), "test.attestation.json")] + require.NotNil(t, item) + require.NoError(t, json.Unmarshal(item.Data, &attest)) require.Equal(t, "https://in-toto.io/Statement/v0.1", attest.Type) require.Equal(t, "https://example.com/attestations/v1.0", attest.PredicateType) @@ -7482,8 +7483,9 @@ func testExportAttestations(t *testing.T, sb integration.Sandbox) { }}, attest.Subject) var attest2 intoto.Statement - dt = m[path.Join(strings.ReplaceAll(platforms.Format(p), "/", "_"), "test.attestation2.json")].Data - require.NoError(t, json.Unmarshal(dt, &attest2)) + item = m[path.Join(strings.ReplaceAll(platforms.Format(p), "/", "_"), "test.attestation2.json")] + require.NotNil(t, item) + require.NoError(t, json.Unmarshal(item.Data, &attest2)) require.Equal(t, "https://in-toto.io/Statement/v0.1", attest2.Type) require.Equal(t, "https://example.com/attestations2/v1.0", attest2.PredicateType) From a9ed1f161e3e9028c5cebf057c4a04c478fefa75 Mon Sep 17 00:00:00 2001 From: Justin Chadwell Date: Wed, 14 Dec 2022 09:55:08 +0000 Subject: [PATCH 2/5] exporter: move fs opt parsing to method This allows one implementation for all the opts parsing, similar to how we do today for the ImageCommitOpts. Additionally, we rename attestation-prefix to attestations-prefix (pluralized) to prepare for the new attestations option. Signed-off-by: Justin Chadwell --- client/client_test.go | 4 ++-- exporter/local/export.go | 22 ++++------------------ exporter/local/fs.go | 26 ++++++++++++++++++++++++++ exporter/tar/export.go | 8 +------- 4 files changed, 33 insertions(+), 27 deletions(-) diff --git a/client/client_test.go b/client/client_test.go index 21e8bf4a896e..8e99247cfd9c 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -7402,7 +7402,7 @@ func testExportAttestations(t *testing.T, sb integration.Sandbox) { Type: ExporterLocal, OutputDir: dir, Attrs: map[string]string{ - "attestation-prefix": "test.", + "attestations-prefix": "test.", }, }, }, @@ -7454,7 +7454,7 @@ func testExportAttestations(t *testing.T, sb integration.Sandbox) { Type: ExporterTar, Output: fixedWriteCloser(outW), Attrs: map[string]string{ - "attestation-prefix": "test.", + "attestations-prefix": "test.", }, }, }, diff --git a/exporter/local/export.go b/exporter/local/export.go index 7d08b172e019..7f9dce675977 100644 --- a/exporter/local/export.go +++ b/exporter/local/export.go @@ -20,10 +20,6 @@ import ( "golang.org/x/time/rate" ) -const ( - keyAttestationPrefix = "attestation-prefix" -) - type Opt struct { SessionManager *session.Manager } @@ -39,24 +35,14 @@ func New(opt Opt) (exporter.Exporter, error) { } func (e *localExporter) Resolve(ctx context.Context, opt map[string]string) (exporter.ExporterInstance, error) { - tm, _, err := epoch.ParseExporterAttrs(opt) - if err != nil { - return nil, err - } - i := &localExporterInstance{ localExporter: e, - opts: CreateFSOpts{ - Epoch: tm, - }, } - - for k, v := range opt { - switch k { - case keyAttestationPrefix: - i.opts.AttestationPrefix = v - } + opt, err := i.opts.Load(opt) + if err != nil { + return nil, err } + _ = opt return i, nil } diff --git a/exporter/local/fs.go b/exporter/local/fs.go index c5a524aae32f..be57111f8f96 100644 --- a/exporter/local/fs.go +++ b/exporter/local/fs.go @@ -15,6 +15,7 @@ import ( "github.com/moby/buildkit/cache" "github.com/moby/buildkit/exporter" "github.com/moby/buildkit/exporter/attestation" + "github.com/moby/buildkit/exporter/util/epoch" "github.com/moby/buildkit/session" "github.com/moby/buildkit/snapshot" "github.com/moby/buildkit/solver/result" @@ -25,11 +26,36 @@ import ( fstypes "github.com/tonistiigi/fsutil/types" ) +const ( + keyAttestationsPrefix = "attestations-prefix" +) + type CreateFSOpts struct { Epoch *time.Time AttestationPrefix string } +func (c *CreateFSOpts) Load(opt map[string]string) (map[string]string, error) { + rest := make(map[string]string) + + var err error + c.Epoch, opt, err = epoch.ParseExporterAttrs(opt) + if err != nil { + return nil, err + } + + for k, v := range opt { + switch k { + case keyAttestationsPrefix: + c.AttestationPrefix = v + default: + rest[k] = v + } + } + + return rest, nil +} + func CreateFS(ctx context.Context, sessionID string, k string, ref cache.ImmutableRef, attestations []exporter.Attestation, defaultTime time.Time, opt CreateFSOpts) (fsutil.FS, func() error, error) { var cleanup func() error var src string diff --git a/exporter/tar/export.go b/exporter/tar/export.go index 4d136c89c1ca..7c195ee99266 100644 --- a/exporter/tar/export.go +++ b/exporter/tar/export.go @@ -21,8 +21,6 @@ import ( ) const ( - attestationPrefixKey = "attestation-prefix" - // preferNondistLayersKey is an exporter option which can be used to mark a layer as non-distributable if the layer reference was // already found to use a non-distributable media type. // When this option is not set, the exporter will change the media type of the layer to a distributable one. @@ -45,12 +43,10 @@ func New(opt Opt) (exporter.Exporter, error) { func (e *localExporter) Resolve(ctx context.Context, opt map[string]string) (exporter.ExporterInstance, error) { li := &localExporterInstance{localExporter: e} - - tm, opt, err := epoch.ParseExporterAttrs(opt) + opt, err := li.opts.Load(opt) if err != nil { return nil, err } - li.opts.Epoch = tm for k, v := range opt { switch k { @@ -60,8 +56,6 @@ func (e *localExporter) Resolve(ctx context.Context, opt map[string]string) (exp return nil, errors.Wrapf(err, "non-bool value for %s: %s", preferNondistLayersKey, v) } li.preferNonDist = b - case attestationPrefixKey: - li.opts.AttestationPrefix = v } } From ab91151881f049e0fcf6c0f74525fa16cca6f47f Mon Sep 17 00:00:00 2001 From: Justin Chadwell Date: Wed, 14 Dec 2022 10:08:35 +0000 Subject: [PATCH 3/5] exporter: remove non dist options from tar exporter This option looks like a mistake added in 45fc3ed51048e15cb1e1e2ba419bc17fd084eb6d. These options aren't ever used, so we don't need to parse them, we can just silently discard them. Signed-off-by: Justin Chadwell --- exporter/tar/export.go | 23 ++--------------------- 1 file changed, 2 insertions(+), 21 deletions(-) diff --git a/exporter/tar/export.go b/exporter/tar/export.go index 7c195ee99266..1aa0fd4fe5f1 100644 --- a/exporter/tar/export.go +++ b/exporter/tar/export.go @@ -3,7 +3,6 @@ package local import ( "context" "os" - "strconv" "strings" "time" @@ -20,13 +19,6 @@ import ( fstypes "github.com/tonistiigi/fsutil/types" ) -const ( - // preferNondistLayersKey is an exporter option which can be used to mark a layer as non-distributable if the layer reference was - // already found to use a non-distributable media type. - // When this option is not set, the exporter will change the media type of the layer to a distributable one. - preferNondistLayersKey = "prefer-nondist-layers" -) - type Opt struct { SessionManager *session.Manager } @@ -47,25 +39,14 @@ func (e *localExporter) Resolve(ctx context.Context, opt map[string]string) (exp if err != nil { return nil, err } - - for k, v := range opt { - switch k { - case preferNondistLayersKey: - b, err := strconv.ParseBool(v) - if err != nil { - return nil, errors.Wrapf(err, "non-bool value for %s: %s", preferNondistLayersKey, v) - } - li.preferNonDist = b - } - } + _ = opt return li, nil } type localExporterInstance struct { *localExporter - opts local.CreateFSOpts - preferNonDist bool + opts local.CreateFSOpts } func (e *localExporterInstance) Name() string { From 602d9f8f2476ecee90aaeaaa663b31fd58671282 Mon Sep 17 00:00:00 2001 From: Justin Chadwell Date: Wed, 14 Dec 2022 10:19:24 +0000 Subject: [PATCH 4/5] exporter: add attestations key to allow disabling attestation output Signed-off-by: Justin Chadwell --- exporter/containerimage/export.go | 3 ++- exporter/containerimage/opts.go | 4 ++++ exporter/containerimage/writer.go | 11 ++++++----- exporter/local/export.go | 3 +++ exporter/local/fs.go | 19 ++++++++++++++----- exporter/oci/export.go | 5 +++-- exporter/tar/export.go | 7 ++++++- 7 files changed, 38 insertions(+), 14 deletions(-) diff --git a/exporter/containerimage/export.go b/exporter/containerimage/export.go index 9c5262377d3f..6762460e4633 100644 --- a/exporter/containerimage/export.go +++ b/exporter/containerimage/export.go @@ -78,7 +78,8 @@ func (e *imageExporter) Resolve(ctx context.Context, opt map[string]string) (exp RefCfg: cacheconfig.RefConfig{ Compression: compression.New(compression.Default), }, - BuildInfo: true, + BuildInfo: true, + Attestations: true, }, store: true, } diff --git a/exporter/containerimage/opts.go b/exporter/containerimage/opts.go index 057bd299e4f2..7537ca2741f6 100644 --- a/exporter/containerimage/opts.go +++ b/exporter/containerimage/opts.go @@ -19,6 +19,7 @@ const ( keyOCITypes = "oci-mediatypes" keyBuildInfo = "buildinfo" keyBuildInfoAttrs = "buildinfo-attrs" + keyAttestations = "attestations" // preferNondistLayersKey is an exporter option which can be used to mark a layer as non-distributable if the layer reference was // already found to use a non-distributable media type. @@ -34,6 +35,7 @@ type ImageCommitOpts struct { BuildInfoAttrs bool Annotations AnnotationsGroup Epoch *time.Time + Attestations bool } func (c *ImageCommitOpts) Load(opt map[string]string) (map[string]string, error) { @@ -73,6 +75,8 @@ func (c *ImageCommitOpts) Load(opt map[string]string) (map[string]string, error) err = parseBoolWithDefault(&c.BuildInfo, k, v, true) case keyBuildInfoAttrs: err = parseBoolWithDefault(&c.BuildInfoAttrs, k, v, false) + case keyAttestations: + err = parseBool(&c.Attestations, k, v) case keyPreferNondistLayers: err = parseBool(&c.RefCfg.PreferNonDistributable, k, v) default: diff --git a/exporter/containerimage/writer.go b/exporter/containerimage/writer.go index c9b5d48b804e..52ea36ccfbb9 100644 --- a/exporter/containerimage/writer.go +++ b/exporter/containerimage/writer.go @@ -69,19 +69,20 @@ func (ic *ImageWriter) Commit(ctx context.Context, inp *exporter.Source, session return nil, err } - requiredAttestations := false + hasAttestations := false for _, p := range ps.Platforms { if atts, ok := inp.Attestations[p.ID]; ok { atts = attestation.Filter(atts, nil, map[string][]byte{ result.AttestationInlineOnlyKey: []byte(strconv.FormatBool(true)), }) if len(atts) > 0 { - requiredAttestations = true + hasAttestations = true break } } } - if requiredAttestations { + hasAttestations = opts.Attestations && hasAttestations + if hasAttestations { isMap = true } @@ -108,7 +109,7 @@ func (ic *ImageWriter) Commit(ctx context.Context, inp *exporter.Source, session if len(ps.Platforms) > 1 { return nil, errors.Errorf("cannot export multiple platforms without multi-platform enabled") } - if requiredAttestations { + if hasAttestations { return nil, errors.Errorf("cannot export attestations without multi-platform enabled") } @@ -159,7 +160,7 @@ func (ic *ImageWriter) Commit(ctx context.Context, inp *exporter.Source, session return mfstDesc, nil } - if len(inp.Attestations) > 0 { + if hasAttestations { opts.EnableOCITypes("attestations") } diff --git a/exporter/local/export.go b/exporter/local/export.go index 7f9dce675977..b70007546605 100644 --- a/exporter/local/export.go +++ b/exporter/local/export.go @@ -37,6 +37,9 @@ func New(opt Opt) (exporter.Exporter, error) { func (e *localExporter) Resolve(ctx context.Context, opt map[string]string) (exporter.ExporterInstance, error) { i := &localExporterInstance{ localExporter: e, + opts: CreateFSOpts{ + Attestations: true, + }, } opt, err := i.opts.Load(opt) if err != nil { diff --git a/exporter/local/fs.go b/exporter/local/fs.go index be57111f8f96..9f032ef0fa13 100644 --- a/exporter/local/fs.go +++ b/exporter/local/fs.go @@ -27,11 +27,13 @@ import ( ) const ( + keyAttestations = "attestations" keyAttestationsPrefix = "attestations-prefix" ) type CreateFSOpts struct { Epoch *time.Time + Attestations bool AttestationPrefix string } @@ -46,6 +48,12 @@ func (c *CreateFSOpts) Load(opt map[string]string) (map[string]string, error) { for k, v := range opt { switch k { + case keyAttestations: + b, err := strconv.ParseBool(v) + if err != nil { + return nil, errors.Wrapf(err, "non-bool value for %s: %s", keyAttestations, v) + } + c.Attestations = b case keyAttestationsPrefix: c.AttestationPrefix = v default: @@ -118,11 +126,12 @@ func CreateFS(ctx context.Context, sessionID string, k string, ref cache.Immutab attestations = attestation.Filter(attestations, nil, map[string][]byte{ result.AttestationInlineOnlyKey: []byte(strconv.FormatBool(true)), }) - attestations, err = attestation.Unbundle(ctx, session.NewGroup(sessionID), attestations) - if err != nil { - return nil, nil, err - } - if len(attestations) > 0 { + if opt.Attestations && len(attestations) > 0 { + attestations, err = attestation.Unbundle(ctx, session.NewGroup(sessionID), attestations) + if err != nil { + return nil, nil, err + } + subjects := []intoto.Subject{} err = outputFS.Walk(ctx, func(path string, info fs.FileInfo, err error) error { if err != nil { diff --git a/exporter/oci/export.go b/exporter/oci/export.go index 60982f4daf3c..f9ed39bbb436 100644 --- a/exporter/oci/export.go +++ b/exporter/oci/export.go @@ -67,8 +67,9 @@ func (e *imageExporter) Resolve(ctx context.Context, opt map[string]string) (exp RefCfg: cacheconfig.RefConfig{ Compression: compression.New(compression.Default), }, - BuildInfo: true, - OCITypes: e.opt.Variant == VariantOCI, + BuildInfo: true, + OCITypes: e.opt.Variant == VariantOCI, + Attestations: true, }, } diff --git a/exporter/tar/export.go b/exporter/tar/export.go index 1aa0fd4fe5f1..758348f8be0d 100644 --- a/exporter/tar/export.go +++ b/exporter/tar/export.go @@ -34,7 +34,12 @@ func New(opt Opt) (exporter.Exporter, error) { } func (e *localExporter) Resolve(ctx context.Context, opt map[string]string) (exporter.ExporterInstance, error) { - li := &localExporterInstance{localExporter: e} + li := &localExporterInstance{ + localExporter: e, + opts: local.CreateFSOpts{ + Attestations: true, + }, + } opt, err := li.opts.Load(opt) if err != nil { return nil, err From 24881670eee34fe2d772dbd258ae4028cebe0d5b Mon Sep 17 00:00:00 2001 From: Justin Chadwell Date: Wed, 14 Dec 2022 11:55:02 +0000 Subject: [PATCH 5/5] exporter: allow attestation options to contain list of reasons Instead of just the boolean true/false values, we allow the attestation option for exporters to contain an arbitrary list of "attestation reasons". Only attestations that have a reason matching the list will actually be output. This allows clients to completely detach the concepts of "what attestations to generate" and "what attestations to output". Signed-off-by: Justin Chadwell --- exporter/attestation/filter.go | 64 ++++++++++++++++--------------- exporter/containerimage/opts.go | 23 ++++++----- exporter/containerimage/writer.go | 7 ++-- exporter/local/fs.go | 21 +++++----- 4 files changed, 63 insertions(+), 52 deletions(-) diff --git a/exporter/attestation/filter.go b/exporter/attestation/filter.go index 5abc234b875e..0ecc8416735b 100644 --- a/exporter/attestation/filter.go +++ b/exporter/attestation/filter.go @@ -1,45 +1,49 @@ package attestation import ( - "bytes" + "strconv" "github.com/moby/buildkit/exporter" + "github.com/moby/buildkit/solver/result" ) -func Filter(attestations []exporter.Attestation, include map[string][]byte, exclude map[string][]byte) []exporter.Attestation { - if len(include) == 0 && len(exclude) == 0 { - return attestations - } - - result := []exporter.Attestation{} +func FilterInline(attestations []exporter.Attestation) (matching []exporter.Attestation, nonMatching []exporter.Attestation) { for _, att := range attestations { - meta := att.Metadata - if meta == nil { - meta = map[string][]byte{} - } - - match := true - for k, v := range include { - if !bytes.Equal(meta[k], v) { - match = false - break + v, ok := att.Metadata[result.AttestationInlineOnlyKey] + if ok { + b, err := strconv.ParseBool(string(v)) + if b && err == nil { + matching = append(matching, att) + continue } } - if !match { - continue - } + nonMatching = append(nonMatching, att) + } + return matching, nonMatching +} - for k, v := range exclude { - if bytes.Equal(meta[k], v) { - match = false - break +func FilterReasons(attestations []exporter.Attestation, reasons []string) (matching []exporter.Attestation, nonMatching []exporter.Attestation) { + if reasons == nil { + // don't filter if no filter provided + return attestations, nil + } + + for _, att := range attestations { + target, ok := att.Metadata[result.AttestationReasonKey] + if ok { + matched := false + for _, reason := range reasons { + if string(target) == reason { + matched = true + break + } + } + if matched { + matching = append(matching, att) + continue } } - if !match { - continue - } - - result = append(result, att) + nonMatching = append(nonMatching, att) } - return result + return matching, nonMatching } diff --git a/exporter/containerimage/opts.go b/exporter/containerimage/opts.go index 7537ca2741f6..6d975bdb46f0 100644 --- a/exporter/containerimage/opts.go +++ b/exporter/containerimage/opts.go @@ -2,6 +2,7 @@ package containerimage import ( "strconv" + "strings" "time" cacheconfig "github.com/moby/buildkit/cache/config" @@ -28,14 +29,15 @@ const ( ) type ImageCommitOpts struct { - ImageName string - RefCfg cacheconfig.RefConfig - OCITypes bool - BuildInfo bool - BuildInfoAttrs bool - Annotations AnnotationsGroup - Epoch *time.Time - Attestations bool + ImageName string + RefCfg cacheconfig.RefConfig + OCITypes bool + BuildInfo bool + BuildInfoAttrs bool + Annotations AnnotationsGroup + Epoch *time.Time + Attestations bool + AttestationsFilter []string } func (c *ImageCommitOpts) Load(opt map[string]string) (map[string]string, error) { @@ -76,7 +78,10 @@ func (c *ImageCommitOpts) Load(opt map[string]string) (map[string]string, error) case keyBuildInfoAttrs: err = parseBoolWithDefault(&c.BuildInfoAttrs, k, v, false) case keyAttestations: - err = parseBool(&c.Attestations, k, v) + if parseBool(&c.Attestations, k, v) != nil { + c.Attestations = true + c.AttestationsFilter = strings.Split(v, ",") + } case keyPreferNondistLayers: err = parseBool(&c.RefCfg.PreferNonDistributable, k, v) default: diff --git a/exporter/containerimage/writer.go b/exporter/containerimage/writer.go index 52ea36ccfbb9..f2d40cd9c305 100644 --- a/exporter/containerimage/writer.go +++ b/exporter/containerimage/writer.go @@ -5,7 +5,6 @@ import ( "context" "encoding/json" "fmt" - "strconv" "strings" "time" @@ -72,9 +71,8 @@ func (ic *ImageWriter) Commit(ctx context.Context, inp *exporter.Source, session hasAttestations := false for _, p := range ps.Platforms { if atts, ok := inp.Attestations[p.ID]; ok { - atts = attestation.Filter(atts, nil, map[string][]byte{ - result.AttestationInlineOnlyKey: []byte(strconv.FormatBool(true)), - }) + _, atts = attestation.FilterInline(atts) + atts, _ = attestation.FilterReasons(atts, opts.AttestationsFilter) if len(atts) > 0 { hasAttestations = true break @@ -239,6 +237,7 @@ func (ic *ImageWriter) Commit(ctx context.Context, inp *exporter.Source, session labels[fmt.Sprintf("containerd.io/gc.ref.content.%d", i)] = desc.Digest.String() if attestations, ok := inp.Attestations[p.ID]; ok { + attestations, _ = attestation.FilterReasons(attestations, opts.AttestationsFilter) attestations, err := attestation.Unbundle(ctx, session.NewGroup(sessionID), attestations) if err != nil { return nil, err diff --git a/exporter/local/fs.go b/exporter/local/fs.go index 9f032ef0fa13..e9cbe38fe24a 100644 --- a/exporter/local/fs.go +++ b/exporter/local/fs.go @@ -8,6 +8,7 @@ import ( "os" "path" "strconv" + "strings" "time" "github.com/docker/docker/pkg/idtools" @@ -32,9 +33,10 @@ const ( ) type CreateFSOpts struct { - Epoch *time.Time - Attestations bool - AttestationPrefix string + Epoch *time.Time + Attestations bool + AttestationsFilter []string + AttestationPrefix string } func (c *CreateFSOpts) Load(opt map[string]string) (map[string]string, error) { @@ -50,10 +52,12 @@ func (c *CreateFSOpts) Load(opt map[string]string) (map[string]string, error) { switch k { case keyAttestations: b, err := strconv.ParseBool(v) - if err != nil { - return nil, errors.Wrapf(err, "non-bool value for %s: %s", keyAttestations, v) + if err == nil { + c.Attestations = b + } else { + c.Attestations = true + c.AttestationsFilter = strings.Split(v, ",") } - c.Attestations = b case keyAttestationsPrefix: c.AttestationPrefix = v default: @@ -123,9 +127,8 @@ func CreateFS(ctx context.Context, sessionID string, k string, ref cache.Immutab } outputFS := fsutil.NewFS(src, walkOpt) - attestations = attestation.Filter(attestations, nil, map[string][]byte{ - result.AttestationInlineOnlyKey: []byte(strconv.FormatBool(true)), - }) + _, attestations = attestation.FilterInline(attestations) + attestations, _ = attestation.FilterReasons(attestations, opt.AttestationsFilter) if opt.Attestations && len(attestations) > 0 { attestations, err = attestation.Unbundle(ctx, session.NewGroup(sessionID), attestations) if err != nil {