From a22f0a4c3e246d0b2e772832c76452692da700c5 Mon Sep 17 00:00:00 2001 From: Akihiro Suda Date: Tue, 22 Nov 2022 21:14:07 +0900 Subject: [PATCH] archive: set WithModTimeUpperBound when WithSourceDateEpoch is set WithModTimeUpperBound sets the upper bound value of the ModTime property of the tar entry structs. WithSourceDateEpoch now implies WithModTimeUpperBound too, in addition to WithWhiteoutTime. For moby/buildkit issue 3296 Signed-off-by: Akihiro Suda --- archive/tar.go | 27 +++++++++++++----- archive/tar_opts.go | 5 +++- archive/tar_test.go | 67 ++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 90 insertions(+), 9 deletions(-) diff --git a/archive/tar.go b/archive/tar.go index a78ee3b5f..5007c410a 100644 --- a/archive/tar.go +++ b/archive/tar.go @@ -97,7 +97,9 @@ func WriteDiff(ctx context.Context, w io.Writer, a, b string, opts ...WriteDiffO func writeDiffNaive(ctx context.Context, w io.Writer, a, b string, o WriteDiffOptions) error { var opts []ChangeWriterOpt if o.SourceDateEpoch != nil { - opts = append(opts, WithWhiteoutTime(*o.SourceDateEpoch)) + opts = append(opts, + WithModTimeUpperBound(*o.SourceDateEpoch), + WithWhiteoutTime(*o.SourceDateEpoch)) } cw := NewChangeWriter(w, b, opts...) err := fs.Changes(ctx, a, b, cw.HandleChange) @@ -493,17 +495,25 @@ func mkparent(ctx context.Context, path, root string, parents []string) error { // See also https://github.com/opencontainers/image-spec/blob/main/layer.md for details // about OCI layers type ChangeWriter struct { - tw *tar.Writer - source string - whiteoutT time.Time - inodeSrc map[uint64]string - inodeRefs map[uint64][]string - addedDirs map[string]struct{} + tw *tar.Writer + source string + modTimeUpperBound *time.Time + whiteoutT time.Time + inodeSrc map[uint64]string + inodeRefs map[uint64][]string + addedDirs map[string]struct{} } // ChangeWriterOpt can be specified in NewChangeWriter. type ChangeWriterOpt func(cw *ChangeWriter) +// WithModTimeUpperBound sets the mod time upper bound. +func WithModTimeUpperBound(tm time.Time) ChangeWriterOpt { + return func(cw *ChangeWriter) { + cw.modTimeUpperBound = &tm + } +} + // WithWhiteoutTime sets the whiteout timestamp. func WithWhiteoutTime(tm time.Time) ChangeWriterOpt { return func(cw *ChangeWriter) { @@ -579,6 +589,9 @@ func (cw *ChangeWriter) HandleChange(k fs.ChangeKind, p string, f os.FileInfo, e // truncate timestamp for compatibility. without PAX stdlib rounds timestamps instead hdr.Format = tar.FormatPAX + if cw.modTimeUpperBound != nil && hdr.ModTime.After(*cw.modTimeUpperBound) { + hdr.ModTime = *cw.modTimeUpperBound + } hdr.ModTime = hdr.ModTime.Truncate(time.Second) hdr.AccessTime = time.Time{} hdr.ChangeTime = time.Time{} diff --git a/archive/tar_opts.go b/archive/tar_opts.go index b426c06a5..ac3a03c8d 100644 --- a/archive/tar_opts.go +++ b/archive/tar_opts.go @@ -91,7 +91,10 @@ type WriteDiffOptions struct { writeDiffFunc func(context.Context, io.Writer, string, string, WriteDiffOptions) error - // SourceDateEpoch specifies the timestamp used for whiteouts to provide control for reproducibility. + // SourceDateEpoch specifies the following timestamps to provide control for reproducibility. + // - The upper bound timestamp of the diff contents + // - The timestamp of the whiteouts + // // See also https://reproducible-builds.org/docs/source-date-epoch/ . SourceDateEpoch *time.Time } diff --git a/archive/tar_test.go b/archive/tar_test.go index de757a2e1..78dfbea76 100644 --- a/archive/tar_test.go +++ b/archive/tar_test.go @@ -36,6 +36,7 @@ import ( "github.com/containerd/containerd/pkg/testutil" "github.com/containerd/continuity/fs" "github.com/containerd/continuity/fs/fstest" + "github.com/opencontainers/go-digest" "github.com/stretchr/testify/require" exec "golang.org/x/sys/execabs" ) @@ -1157,20 +1158,36 @@ func TestDiffTar(t *testing.T) { } } -func TestWhiteoutSourceDateEpoch(t *testing.T) { +func TestSourceDateEpoch(t *testing.T) { sourceDateEpoch, err := time.Parse(time.RFC3339, "2022-01-23T12:34:56Z") require.NoError(t, err) + past, err := time.Parse(time.RFC3339, "2022-01-01T00:00:00Z") + require.NoError(t, err) + require.True(t, past.Before(sourceDateEpoch)) + veryRecent := time.Now() + require.True(t, veryRecent.After(sourceDateEpoch)) + opts := []WriteDiffOpt{WithSourceDateEpoch(&sourceDateEpoch)} validators := []tarEntryValidator{ composeValidators(whiteoutEntry("f1"), requireModTime(sourceDateEpoch)), + composeValidators(fileEntry("f2", []byte("content2"), 0644), requireModTime(past)), + composeValidators(fileEntry("f3", []byte("content3"), 0644), requireModTime(sourceDateEpoch)), } a := fstest.Apply( fstest.CreateFile("/f1", []byte("content"), 0644), ) b := fstest.Apply( + // Remove f1; the timestamp of the tar entry will be sourceDateEpoch fstest.RemoveAll("/f1"), + // Create f2 with the past timestamp; the timestamp of the tar entry will be past (< sourceDateEpoch) + fstest.CreateFile("/f2", []byte("content2"), 0644), + fstest.Chtimes("/f2", past, past), + // Create f3 with the veryRecent timestamp; the timestamp of the tar entry will be sourceDateEpoch + fstest.CreateFile("/f3", []byte("content3"), 0644), + fstest.Chtimes("/f3", veryRecent, veryRecent), ) makeDiffTarTest(validators, a, b, opts...)(t) + makeDiffTarReproTest(a, b, opts...)(t) } type tarEntryValidator func(*tar.Header, []byte) error @@ -1303,6 +1320,54 @@ func makeDiffTarTest(validators []tarEntryValidator, a, b fstest.Applier, opts . } } +func makeDiffTar(t *testing.T, a, b fstest.Applier, opts ...WriteDiffOpt) (digest.Digest, []byte) { + ad := t.TempDir() + if err := a.Apply(ad); err != nil { + t.Fatalf("failed to apply a: %v", err) + } + + bd := t.TempDir() + if err := fs.CopyDir(bd, ad); err != nil { + t.Fatalf("failed to copy dir: %v", err) + } + if err := b.Apply(bd); err != nil { + t.Fatalf("failed to apply b: %v", err) + } + + rc := Diff(context.Background(), ad, bd, opts...) + defer rc.Close() + var buf bytes.Buffer + r := io.TeeReader(rc, &buf) + dgst, err := digest.FromReader(r) + if err != nil { + t.Fatal(err) + } + if err = rc.Close(); err != nil { + t.Fatal(err) + } + return dgst, buf.Bytes() +} + +func makeDiffTarReproTest(a, b fstest.Applier, opts ...WriteDiffOpt) func(*testing.T) { + return func(t *testing.T) { + const ( + count = 5 + delay = 100 * time.Millisecond + ) + var lastDigest digest.Digest + for i := 0; i < count; i++ { + dgst, _ := makeDiffTar(t, a, b, opts...) + t.Logf("#%d: digest %s", i, dgst) + if lastDigest == "" { + lastDigest = dgst + } else if dgst != lastDigest { + t.Fatalf("expected digest %s, got %s", lastDigest, dgst) + } + time.Sleep(delay) + } + } +} + type diffApplier struct{} func (d diffApplier) TestContext(ctx context.Context) (context.Context, func(), error) {