Make image builds reproducible #4142

bjornbugge · 2023-11-17T20:05:26Z

Fixes #4141 🛠️

google-cla · 2023-11-17T20:05:31Z

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

jib-core/src/main/java/com/google/cloud/tools/jib/image/ReproducibleLayerBuilder.java

chanseokoh · 2023-11-17T20:55:09Z

Just so you know, I am not one of the maintainers of this repo.

bjornbugge · 2023-11-17T21:09:39Z

I think this also fixes #4131

glasser · 2023-12-19T22:20:37Z

I don't suppose anybody has published a build with this PR in it somewhere? We really need this in order to use Jib and are having trouble pinning commons-compress in our Gradle build.

jib-core/src/test/java/com/google/cloud/tools/jib/image/ReproducibleLayerBuilderTest.java

mpeddada1 · 2024-01-19T22:05:15Z

@bjornbugge Thanks so much for this fix! It appears that the test jobs are failing with the following error:

Task :jib-core:checkstyleTest FAILED
Build cache key for task ':jib-core:checkstyleTest' is 1e2bdbced948a9c01563c97b77fe87b3
Task ':jib-core:checkstyleTest' is not up-to-date because:
  No history is available.
[ant:checkstyle] Running Checkstyle 8.29 on 98 files
[ant:checkstyle] [WARN] /[tmpfs/src/github/jib/jib-core/src/test/java/com/google/cloud/tools/jib/image/ReproducibleLayerBuilderTest.java:31](https://cs.corp.google.com/piper///depot/google3/tmpfs/src/github/jib/jib-core/src/test/java/com/google/cloud/tools/jib/image/ReproducibleLayerBuilderTest.java?l=31): Extra separation in import group before 'java.io.BufferedOutputStream' [CustomImportOrder]
[ant:xslt] Processing /tmpfs/src/github/jib/jib-core/build/reports/checkstyle/test.xml to /tmpfs/src/github/jib/jib-core/build/reports/checkstyle/test.html
[ant:xslt] Loading stylesheet <xsl:stylesheet	xmlns:xsl="http://www.w3.org/1999/XSL/Transform" version="1.0">

Removing the extra space in the import statements in ReproducibleLayerBuilderTest should hopefully help resolve this.

Co-authored-by: Mridula <[email protected]>

mpeddada1 · 2024-01-24T22:56:47Z

Logging stacktrace:

com.google.cloud.tools.jib.image.ReproducibleLayerBuilderTest > testBuild_timestampDefault FAILED
    expected: 1970-01-01T00:00:01Z
    but was : 1970-01-01T00:00:00Z
        at com.google.cloud.tools.jib.image.ReproducibleLayerBuilderTest.testBuild_timestampDefault(ReproducibleLayerBuilderTest.java:301)

com.google.cloud.tools.jib.image.ReproducibleLayerBuilderTest > testBuild_parentDirBehavior FAILED
    expected: 1970-01-01T00:00:01Z
    but was : 1970-01-01T00:00:00Z
        at com.google.cloud.tools.jib.image.ReproducibleLayerBuilderTest.testBuild_parentDirBehavior(ReproducibleLayerBuilderTest.java:239)

com.google.cloud.tools.jib.image.ReproducibleLayerBuilderTest > testBuild_timestampNonDefault FAILED
    expected: 1970-01-01T00:02:03Z
    but was : 1970-01-01T00:00:00Z

mpeddada1 · 2024-01-24T23:36:52Z

jib-core/src/test/java/com/google/cloud/tools/jib/image/ReproducibleLayerBuilderTest.java

 return new FileEntry(
 source,
 destination,
 FileEntriesLayer.DEFAULT_FILE_PERMISSIONS_PROVIDER.get(source, destination),
- FileEntriesLayer.DEFAULT_MODIFICATION_TIME);
+ Files.getLastModifiedTime(source).toInstant());


Is it possible to leave this in as FileEntriesLayer.DEFAULT_MODIFICATION_TIME? I think this helper method was initially meant to create a FileEntry with fixed permissions and modification time for testing purposes.

The reason that this slipped through the test suite was partly because of this default value, actually. The logic that should reset the modification time header wasn't tested because the tests reset it automatically here. I've added a clarifying comment and also some more helpers to make it easier to add test layers with the actual mod-time of the temp files.

bjornbugge · 2024-01-25T08:17:55Z

@mpeddada1 Thanks for taking a look at this. I've fixed the three tests that failed -- I thought I'd run the entire test suite locally, but clearly I had forgot :P (I guess I was in a hurry to make a local version of jib that we can use in our internal tooling)

chanseokoh · 2024-01-25T15:19:31Z

jib-core/src/test/java/com/google/cloud/tools/jib/image/ReproducibleLayerBuilderTest.java

+ // Here we make sure to use the actual modification-time here because that's what would happen in
+ // regular use when copying the file from disk into the layer.


No, the regular use is to force setting the file timestamp to epoch+1 by default. The only other alternative for the user is to set a static time. Jib never sets the original timestamp of the source file.

From the Jib plugin doc:

The value should either be EPOCH_PLUS_SECOND to set the timestamps to Epoch + 1 second (default behavior), or an ISO 8601 date-time parsable with DateTimeFormatter.ISO_DATE_TIME such as 2019-07-15T10:15:30+09:00 or 2011-12-03T22:42:05Z.

Yes, I can see my comment was a bit unclear. What I meant was that in normal use, Jib reads in files from disk and they have some "real" modification time. Then Jib is supposed to reset this, but that didn't happen (or at least didn't happen "enough" because there were still PAX headers with the timestamps in them).
Because the test helper created test files where the modtime was already reset, this issue wasn't visible in the test suite.

Then Jib is supposed to reset this, but that didn't happen (or at least didn't happen "enough" because there were still PAX headers with the timestamps in them).

This does happen. To clarify, the ordinary modification time is correctly set to epoch+1. It's just that the newer apache compress library added or set some new PAX headers with timestamp values. AFAICT, the mod time and the time values in PAX headers are independent. Everything worked fine without this PR, except that the new headers had dynamic values hence affected reproducibility. As long as we set the ordinary mod time to epoch+1, I think it shouldn't matter what time you set for the PAX headers. So ideally, I think what we need to test is these two:

Verify that Jib continues to set the ordinary mod time to epoch+1 by default.

Verify that Jib wipes out the time PAX headers. I think the value doesn't matter, as long as people don't complain.

Ad 1: The PAX headers take precedence it seems, so I'll set them to 1 and try to switch the ordering like you suggested below 👍
Ad 2: I believe that testing the wiping of the headers is captured in the existing test case testToBlob_reproducibility, as it compares the raw bytes. This test would fail after I updated the helper function here, hence my point with the old version of this helper "hiding" the issue with PAX headers.

They're set to 1 now and it's asserted (again).

If I disable the PAX header reset then the test testToBlob_reproducibility fails because of mismatched byte arrays. So I'd argue that this is covered as well :)

Ah, yes of course, I forgot that it's user-configurable 👍

Although - I'm a bit at a loss as to where the modTime is supposed to be reset. In line 79 and 167 in ReproducibleLayerBuilder.kt there are calls to .setModTime. In the former case, for directories, it's always set to EPOCH+1 regardless of any user settings. In the latter case, for files, it's always set to the file's real value on disk, which is not reproducible. Are there any other places in the code base where the modification time could've been reset?

167 is the user-configured value. Just set both the PAX headers and setModTime() to the same timestamp. For the (Jib-created) directiories, it is not customizable and always set to epoch+1.

To recap, just set the same value as Jib does now.

Hi @chanseokoh - I've been away from this work for a while due to other things. I've read the code more carefully now and I'll now understand that I was confused earlier as to where the responsibility of (re)setting the datetime lies. I'll set the PAX headers to the same timestamp as setModTime 👍

@bjornbugge too bad that you missed the release 3.4.1.

jib-core/src/test/java/com/google/cloud/tools/jib/image/ReproducibleLayerBuilderTest.java

diegomarquezp · 2024-02-07T15:48:52Z

Looks like the format check is failing

> Task :jib-core:verifyGoogleJavaFormat FAILED
:jib-core:verifyGoogleJavaFormat (Thread[Execution worker for ':' Thread 2,5,main]) completed. Took 5.055 secs.

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':jib-core:verifyGoogleJavaFormat'.
> Problems: formatting style violations

* Try:
Run with --debug option to get more log output. Run with --scan to get full insights.

* Exception is:
org.gradle.api.tasks.TaskExecutionException: Execution failed for task ':jib-core:verifyGoogleJavaFormat'.
	at org.gradle.api.internal.tasks.execution.ExecuteActionsTaskExecuter.lambda$executeIfValid$3(ExecuteActionsTaskExecuter.java:186)
	at org.gradle.internal.Try$Failure.ifSuccessfulOrElse(Try.java:268)

Please see https:/google/google-java-format

mpeddada1 · 2024-02-08T14:57:28Z

Thanks for the observation @diegomarquezp! @bjornbugge try running ./gradlew googleJavaFormat to address the google formatting issues.

Additionally, to @chanseokoh's suggestion in #4142 (comment), we haven't heard back from you on this so to reiterate - We learned that clearing the PAX header isn't sufficient and since the mtime represent the modification time of the file, it would make more sense for it to be set to whatever value Jib sets it to. So if we set the modification time (using setModTime()) to the default value, we set the pax header to the same value and if we use a user configured value then we just set the header to that value. Let us know if you have any questions after trying it out!

izogfif · 2024-02-29T10:37:59Z

Could someone review this, please? Having reproducible builds again after 3+ months would be nice.

chanseokoh · 2024-02-29T13:46:36Z

@izogfif it has been reviewed. It's waiting on the PR author.

izogfif · 2024-03-01T03:52:36Z

@chanseokoh I see, this "Review required" message at the bottom of this issue page on GitHub made me misunderstand what's happening.

Previously in this comment you wrote:

167 is the user-configured value. Just set both the PAX headers and setModTime() to the same timestamp. For the (Jib-created) directiories, it is not customizable and always set to epoch+1.

To recap, just set the same value as Jib does now.

Where "167" is the number of line in ReproducibleLayerBuilder.kt file which I was unable to find. If you were actually talking about file jib-core/src/main/java/com/google/cloud/tools/jib/image/ReproducibleLayerBuilder.java, then do I understand you correctly that we need to:

look at method add of com.google.cloud.tools.jib.image.ReproducibleLayerBuilder.UniqueTarArchiveEntries class and take tarArchiveEntry.getModTime().toInstant() from there;
look at method build of com.google.cloud.tools.jib.image.ReproducibleLayerBuilder class and take layerEntry.getModificationTime() from there;

and pass this value into method clearTimeHeaders. This will change this method from this:

  private static void clearTimeHeaders(TarArchiveEntry entry) {
    entry.setModTime(FileEntriesLayer.DEFAULT_MODIFICATION_TIME.toEpochMilli());
    entry.addPaxHeader("mtime", "1");
    entry.addPaxHeader("atime", "1");
    entry.addPaxHeader("ctime", "1");
    entry.addPaxHeader("LIBARCHIVE.creationtime", "1");
  }

to this:

  private static void clearTimeHeaders(TarArchiveEntry entry, Instant modTime) {
    entry.setModTime(modTime.toEpochMilli());

    // PAX headers use <seconds>.<nanoseconds> format
    String headerTime = Long.toString(modTime.getEpochSecond());
    final long nanos = modTime.getNano();
    if (nanos > 0) {
      headerTime += "." + nanos;
    }
    entry.addPaxHeader("mtime", headerTime);
    entry.addPaxHeader("atime", headerTime);
    entry.addPaxHeader("ctime", headerTime);
    entry.addPaxHeader("LIBARCHIVE.creationtime", headerTime);
  }

?
This way:

User specifies modification time in JIB settings.
JIB sets this time to "modification time" property in FileEntry.
clearTimeHeaders propagates this modification time to TarArchiveEntry and corresponding PAX headers.

chanseokoh · 2024-03-02T01:33:59Z

@izogfif I cannot confirm the correctness of the implementation details of yours, but you got the right idea.

izogfif · 2024-03-02T15:08:26Z

@chanseokoh I've made some changes and created a pull request with one extra commit. Please take a look here.

chanseokoh · 2024-03-04T16:58:03Z

This PR is obsolete in favor of #4204.

mpeddada1 · 2024-03-11T17:24:04Z

Closing in favor of #4204. Thanks for getting us started with this fix!

bjornbugge · 2024-03-17T05:46:22Z

Thanks for taking this over for me, @izogfif. Looking forward to a new release with this included :)

Set access, creation, and last-modified time to zero

095343c

chanseokoh mentioned this pull request Nov 17, 2023

Builds not reproducible #4141

Closed

Add small test that ensures that modtime doesn't change layer

6376c32

chanseokoh reviewed Nov 17, 2023

View reviewed changes

jib-core/src/main/java/com/google/cloud/tools/jib/image/ReproducibleLayerBuilder.java Outdated Show resolved Hide resolved

jib-core/src/main/java/com/google/cloud/tools/jib/image/ReproducibleLayerBuilder.java Show resolved Hide resolved

Also clear time-related PAX headers on parent-dir entries

05bd7f8

bjornbugge changed the title ~~Set access, creation, and last-modified time to zero~~ Make image builds reproducible Nov 20, 2023

chanseokoh approved these changes Dec 19, 2023

View reviewed changes

bjornbugge mentioned this pull request Jan 19, 2024

spring boot 3.2 jib build is not reproducible #4163

Closed

mpeddada1 added kokoro:force-run kokoro:run labels Jan 19, 2024

kokoro-team removed kokoro:run kokoro:force-run labels Jan 19, 2024

mpeddada1 reviewed Jan 19, 2024

View reviewed changes

jib-core/src/test/java/com/google/cloud/tools/jib/image/ReproducibleLayerBuilderTest.java Outdated Show resolved Hide resolved

Add blank line for checkstyle

9f048d1

Co-authored-by: Mridula <[email protected]>

mpeddada1 added the kokoro:run label Jan 24, 2024

kokoro-team removed the kokoro:run label Jan 24, 2024

mpeddada1 reviewed Jan 24, 2024

View reviewed changes

bjornbugge added 2 commits January 25, 2024 09:06

Fix the modification-time tests

24162dd

Rename clearTimeHeaders to clearPaxTimeHeaders

8175386

chanseokoh reviewed Jan 25, 2024

View reviewed changes

bjornbugge added 2 commits January 29, 2024 09:15

Set PAX headers to EPOCH plus 1

de4ab25

Rename toclearTimeHeaders and call setModTime in helper

dc3633b

mpeddada1 added the kokoro:run label Jan 31, 2024

kokoro-team removed the kokoro:run label Jan 31, 2024

mpeddada1 mentioned this pull request Feb 21, 2024

problem achieving reproducibility #4131

Closed

izogfif mentioned this pull request Mar 2, 2024

Update apache-commons to 1.26.0 and set PAX headers to address build reproducibility issue #4204

Merged

mpeddada1 closed this Mar 11, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make image builds reproducible #4142

Make image builds reproducible #4142

bjornbugge commented Nov 17, 2023

google-cla bot commented Nov 17, 2023

chanseokoh commented Nov 17, 2023

bjornbugge commented Nov 17, 2023

glasser commented Dec 19, 2023

mpeddada1 commented Jan 19, 2024

mpeddada1 commented Jan 24, 2024

mpeddada1 Jan 24, 2024

bjornbugge Jan 25, 2024

bjornbugge commented Jan 25, 2024

chanseokoh Jan 25, 2024

bjornbugge Jan 26, 2024

chanseokoh Jan 26, 2024 •

edited

Loading

bjornbugge Jan 29, 2024 •

edited

Loading

bjornbugge Jan 29, 2024

bjornbugge Jan 29, 2024

bjornbugge Jan 29, 2024

chanseokoh Jan 29, 2024 •

edited

Loading

bjornbugge Feb 20, 2024

chanseokoh Feb 23, 2024

diegomarquezp commented Feb 7, 2024

mpeddada1 commented Feb 8, 2024

izogfif commented Feb 29, 2024

chanseokoh commented Feb 29, 2024

izogfif commented Mar 1, 2024 •

edited

Loading

chanseokoh commented Mar 2, 2024

izogfif commented Mar 2, 2024

chanseokoh commented Mar 4, 2024

mpeddada1 commented Mar 11, 2024

bjornbugge commented Mar 17, 2024

		// Here we make sure to use the actual modification-time here because that's what would happen in
		// regular use when copying the file from disk into the layer.

Make image builds reproducible #4142

Make image builds reproducible #4142

Conversation

bjornbugge commented Nov 17, 2023

google-cla bot commented Nov 17, 2023

chanseokoh commented Nov 17, 2023

bjornbugge commented Nov 17, 2023

glasser commented Dec 19, 2023

mpeddada1 commented Jan 19, 2024

mpeddada1 commented Jan 24, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bjornbugge commented Jan 25, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chanseokoh Jan 26, 2024 • edited Loading

Choose a reason for hiding this comment

bjornbugge Jan 29, 2024 • edited Loading

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

chanseokoh Jan 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

diegomarquezp commented Feb 7, 2024

mpeddada1 commented Feb 8, 2024

izogfif commented Feb 29, 2024

chanseokoh commented Feb 29, 2024

izogfif commented Mar 1, 2024 • edited Loading

chanseokoh commented Mar 2, 2024

izogfif commented Mar 2, 2024

chanseokoh commented Mar 4, 2024

mpeddada1 commented Mar 11, 2024

bjornbugge commented Mar 17, 2024

chanseokoh Jan 26, 2024 •

edited

Loading

bjornbugge Jan 29, 2024 •

edited

Loading

chanseokoh Jan 29, 2024 •

edited

Loading

izogfif commented Mar 1, 2024 •

edited

Loading