From 81d697b30a2a7a6484d528833d853901edf8387d Mon Sep 17 00:00:00 2001 From: Justin Chadwell Date: Wed, 14 Dec 2022 14:20:17 +0000 Subject: [PATCH 1/3] 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 f98672353cc0..ae175ea3d164 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -7692,8 +7692,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) @@ -7705,8 +7706,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 b2ff17c3fb1dd8d3888f6be418b63ea7155f94a4 Mon Sep 17 00:00:00 2001 From: Justin Chadwell Date: Wed, 14 Dec 2022 09:55:08 +0000 Subject: [PATCH 2/3] 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. Signed-off-by: Justin Chadwell --- exporter/local/export.go | 21 +++------------------ exporter/local/fs.go | 26 ++++++++++++++++++++++++++ exporter/tar/export.go | 8 +------- 3 files changed, 30 insertions(+), 25 deletions(-) diff --git a/exporter/local/export.go b/exporter/local/export.go index 7d08b172e019..7157c63dac80 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,23 +35,12 @@ 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 - } + _, err := i.opts.Load(opt) + if err != nil { + return nil, err } return i, nil diff --git a/exporter/local/fs.go b/exporter/local/fs.go index c5a524aae32f..9caf6f73af60 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 ( + keyAttestationPrefix = "attestation-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 keyAttestationPrefix: + 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..2f33ceb2da62 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) + _, 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 9ba004c10cd7e4556b832ee0583e545edc0ede80 Mon Sep 17 00:00:00 2001 From: Justin Chadwell Date: Wed, 14 Dec 2022 10:08:35 +0000 Subject: [PATCH 3/3] 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 2f33ceb2da62..7259f6b24a9a 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 {