Apply SOURCE_DATE_EPOCH to the files and the whiteouts inside OCI tar layers #3560

AkihiroSuda · 2023-01-31T15:15:34Z

Propagate the build-arg:SOURCE_DATE_EPOCH opt value to the differ, to limit the upper bound of the file timestamps and set the whiteout timestamps.

With this commit, the following workarounds mentioned in docs/build-repro.md are no longer needed for reproducible builds:

# Limit the timestamp upper bound to SOURCE_DATE_EPOCH.
# Workaround for https://github.com/moby/buildkit/issues/3180
ARG SOURCE_DATE_EPOCH
RUN find $( ls / | grep -E -v "^(dev|mnt|proc|sys)$" ) -newermt "@${SOURCE_DATE_EPOCH}" -writable -xdev | xargs touch --date="@${SOURCE_DATE_EPOCH}" --no-dereference

# Squash the entire stage for resetting the whiteout timestamps.
# Workaround for https://github.com/moby/buildkit/issues/3168
FROM scratch
COPY --from=0 / /

Limitations:

containerd 1.7 is needed for the containerd worker mode. (containerd/containerd@9c9f564)

Backup

ac6f97b (withdrew due to dependency on ctx)

cache/blobs.go

tonistiigi · 2023-02-01T00:59:14Z

cache/blobs.go

@@ -62,6 +63,8 @@ func (sr *immutableRef) computeBlobChain(ctx context.Context, createIfNeeded boo
 }

 func computeBlobChain(ctx context.Context, sr *immutableRef, createIfNeeded bool, comp compression.Config, s session.Group, filter map[string]struct{}) error {


I don't understand how this is safe. If the blob already exists, then just the blob is used without any timestamp check. If the first build uses one epoch and the second uses another or no epoch at all then the second build gets epoch from the first build.

Now validated in cache/blobs.go:blobExistsWithSourceDateEpoch()

ReillyBrogan · 2023-02-23T18:15:03Z

Is this and #3263 still blocked?

AkihiroSuda · 2023-02-24T03:05:06Z

Is this and #3263 still blocked?

Yes, will try to update next week, sorry

AkihiroSuda · 2023-03-11T03:37:14Z

Rebased

AkihiroSuda · 2023-03-14T05:15:23Z

Rebased again

crazy-max · 2023-03-17T16:45:06Z

frontend/dockerfile/dockerfile_test.go

+	if cdAddress := sb.ContainerdAddress(); cdAddress != "" {
+		// https://github.com/containerd/containerd/commit/9c9f564a35df7990870a53a125afbf88ac412753
+		integration.CheckContainerdVersion(t, cdAddress, ">= 1.7.0-beta.1")


It looks good for containerd worker but I'm not sure what we should do for dockerd with containerd snapshotter enabled.

Maybe having a factory in the integration sandbox to return the containerd version depending on the worker if that makes sense?

I guess it can be revisited in a separate PR?
The current code is passing the current CI.

AkihiroSuda · 2023-03-22T08:38:02Z

Rebased

tonistiigi

Iiuc correctly, this also converts the blobs of base images. I don't think that is correct, at least as the default behavior. I think it may be possible to use the DiffIDs in the image config that the frontend passes to the exporter to figure out which layers are from the base image and which are not.

Not for this PR, but we are also assuming that archive/tar and compress/gzip are deterministic. We at least need to add test coverage that would reveal possible changes in Go updates and give us option to vendor old library if needed.

tonistiigi · 2023-03-23T04:24:59Z

cache/blobs.go

 		})
 	}

 	if _, ok := filter[sr.ID()]; ok {
 		eg.Go(func() error {
 			_, err := g.Do(ctx, fmt.Sprintf("%s-%t", sr.ID(), createIfNeeded), func(ctx context.Context) (interface{}, error) {
-				if sr.getBlob() != "" {
+				if blobExistsWithSourceDateEpoch(sr, sourceDateEpoch) {


Does this mean that if there is no blob for epoch but there is one for another one, we will run the differ again? Is it a good idea? I think it would be better to do something similar to compressions there we convert a blob to another one.

tonistiigi · 2023-03-23T04:27:43Z

cache/blobs.go

@@ -305,6 +320,9 @@ func (sr *immutableRef) setBlob(ctx context.Context, desc ocispecs.Descriptor) (
 	sr.queueMediaType(desc.MediaType)
 	sr.queueBlobSize(desc.Size)
 	sr.appendURLs(desc.URLs)
+	if sourceDateEpoch != nil {
+		sr.queueSourceDateEpoch(*sourceDateEpoch)


Not sure I understand how this works. Doesn't it just fix a ref to a specific epoch forever? Instead, it should track multiple variants of blobs that are all associated with a ref (similar to compressions).

it should track multiple variants of blobs that are all associated with a ref (similar to compressions).

Does that require putting the epoch value into an OCI descriptor?

You mean for the remote build-cache or for something else? In build-cache we already have "created time" but not exactly the same(although both would be epoch value). We might need a separate field for that.

If you think for the case of avoiding regenerating blobs for base image layers if they were already generated with the same epoch, then yes, I think saving epoch would be needed. Otoh, I don't think any user would want us to regenerate the layers for the base image, just to change the timestamps in it.

sr.queueSourceDateEpoch() is only called for new diffs, so it seems safe for now

ktock · 2023-03-23T07:48:57Z

Iiuc correctly, this also converts the blobs of base images. I don't think that is correct, at least as the default behavior.

Maybe we can introcue an envvar to experimentally allow this behavour?

Possible another way is managing epoch variants as compression variants by making compression.Type aware of the source date epoch.

AkihiroSuda · 2023-03-30T13:15:43Z

Iiuc correctly, this also converts the blobs of base images. I don't think that is correct, at least as the default behavior. I think it may be possible to use the DiffIDs in the image config that the frontend passes to the exporter to figure out which layers are from the base image and which are not.

Updated PR to avoid converting the base images, by checking len(sr.getImageRefs()) > 0.
Please see cache/blobs.go:computeBlobChain().

Maybe we can introcue an envvar to experimentally allow this behavour?

I don't see a usecase for this, but it can be covered in a follow-up PR if somebody needs it.

ktock · 2023-03-31T06:35:37Z

docs/build-repro.md

@@ -65,9 +65,12 @@ See also the [documentation](/frontend/dockerfile/docs/reference.md#buildkit-bui

 ## Caveats


nit: The limitation described in this PR should be documented somewhere?

Limitations:

containerd 1.7 is needed for the containerd worker mode. (containerd/containerd@9c9f564)

Mentioned in docs/build-repro.md

… layers Propagate the `build-arg:SOURCE_DATE_EPOCH` opt value to the differ, to limit the upper bound of the file timestamps and set the whiteout timestamps. With this commit, the following workarounds mentioned in `docs/build-repro.md` are no longer needed for reproducible builds: > ```dockerfile > # Limit the timestamp upper bound to SOURCE_DATE_EPOCH. > # Workaround for moby#3180 > ARG SOURCE_DATE_EPOCH > RUN find $( ls / | grep -E -v "^(dev|mnt|proc|sys)$" ) -newermt "@${SOURCE_DATE_EPOCH}" -writable -xdev | xargs touch --date="@${SOURCE_DATE_EPOCH}" --no-dereference > ``` > ```dockerfile > # Squash the entire stage for resetting the whiteout timestamps. > # Workaround for moby#3168 > FROM scratch > COPY --from=0 / / > ``` Limitations: * containerd 1.7 is needed for the containerd worker mode. Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>

AkihiroSuda · 2023-05-09T04:43:09Z

@tonistiigi Could you take a look?

AkihiroSuda added the area/reproducible-builds label Jan 31, 2023

AkihiroSuda force-pushed the source-date-epoch-1.7 branch from 7a4ab1e to ac6f97b Compare January 31, 2023 15:52

AkihiroSuda marked this pull request as ready for review January 31, 2023 15:55

tonistiigi reviewed Feb 1, 2023

View reviewed changes

AkihiroSuda marked this pull request as draft February 8, 2023 05:52

AkihiroSuda force-pushed the source-date-epoch-1.7 branch 3 times, most recently from 6155189 to 45acbcd Compare March 7, 2023 10:54

AkihiroSuda marked this pull request as ready for review March 7, 2023 10:54

AkihiroSuda requested a review from tonistiigi March 7, 2023 11:26

AkihiroSuda force-pushed the source-date-epoch-1.7 branch 2 times, most recently from 75276f4 to 7121a39 Compare March 14, 2023 05:15

hermanbanken mentioned this pull request Mar 14, 2023

docker build does not create reproducible output (even with fixed SOURCE_DATE_EPOCH) moby/moby#43124

Open

AkihiroSuda requested a review from crazy-max March 17, 2023 15:47

crazy-max reviewed Mar 17, 2023

View reviewed changes

TBBle mentioned this pull request Mar 20, 2023

WithSourceDateEpoch for Windows archiver containerd/containerd#8289

Open

AkihiroSuda force-pushed the source-date-epoch-1.7 branch from 7121a39 to a274214 Compare March 22, 2023 08:38

tonistiigi reviewed Mar 23, 2023

View reviewed changes

tonistiigi requested a review from ktock March 23, 2023 04:39

AkihiroSuda marked this pull request as draft March 28, 2023 23:01

sbellem mentioned this pull request Mar 29, 2023

[Question] Reproducible builds gramineproject/gramine#153

Open

AkihiroSuda force-pushed the source-date-epoch-1.7 branch 3 times, most recently from 300f800 to 415a7ae Compare March 30, 2023 11:32

AkihiroSuda marked this pull request as ready for review March 30, 2023 13:16

ktock reviewed Mar 31, 2023

View reviewed changes

AkihiroSuda force-pushed the source-date-epoch-1.7 branch from 415a7ae to a7bc2f0 Compare April 3, 2023 05:01

ktock approved these changes Apr 3, 2023

View reviewed changes

AkihiroSuda requested a review from tonistiigi April 13, 2023 02:04

AkihiroSuda mentioned this pull request May 9, 2023

Dockerfile:1.5: SOURCE_DATE_EPOCH not setting mtime of directories created by ADD #3843

Open

Apply SOURCE_DATE_EPOCH to the files and the whiteouts inside OCI tar layers #3560

Apply SOURCE_DATE_EPOCH to the files and the whiteouts inside OCI tar layers #3560

AkihiroSuda commented Jan 31, 2023 •

edited

tonistiigi Feb 1, 2023

AkihiroSuda Mar 7, 2023

ReillyBrogan commented Feb 23, 2023

AkihiroSuda commented Feb 24, 2023

AkihiroSuda commented Mar 11, 2023

AkihiroSuda commented Mar 14, 2023

crazy-max Mar 17, 2023

crazy-max Mar 17, 2023

AkihiroSuda Mar 18, 2023

AkihiroSuda commented Mar 22, 2023

tonistiigi left a comment

tonistiigi Mar 23, 2023

tonistiigi Mar 23, 2023

AkihiroSuda Mar 28, 2023

tonistiigi Mar 28, 2023 •

edited

AkihiroSuda Mar 30, 2023

ktock commented Mar 23, 2023

AkihiroSuda commented Mar 30, 2023

ktock Mar 31, 2023

AkihiroSuda Apr 3, 2023

AkihiroSuda commented May 9, 2023

		@@ -62,6 +63,8 @@ func (sr *immutableRef) computeBlobChain(ctx context.Context, createIfNeeded boo
		}

		func computeBlobChain(ctx context.Context, sr *immutableRef, createIfNeeded bool, comp compression.Config, s session.Group, filter map[string]struct{}) error {

		@@ -65,9 +65,12 @@ See also the [documentation](/frontend/dockerfile/docs/reference.md#buildkit-bui

		## Caveats

Apply SOURCE_DATE_EPOCH to the files and the whiteouts inside OCI tar layers #3560

Are you sure you want to change the base?

Apply SOURCE_DATE_EPOCH to the files and the whiteouts inside OCI tar layers #3560

Conversation

AkihiroSuda commented Jan 31, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ReillyBrogan commented Feb 23, 2023

AkihiroSuda commented Feb 24, 2023

AkihiroSuda commented Mar 11, 2023

AkihiroSuda commented Mar 14, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AkihiroSuda commented Mar 22, 2023

tonistiigi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tonistiigi Mar 28, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ktock commented Mar 23, 2023

AkihiroSuda commented Mar 30, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AkihiroSuda commented May 9, 2023

AkihiroSuda commented Jan 31, 2023 •

edited

tonistiigi Mar 28, 2023 •

edited