Make image builds reproducible

[bjornbugge]

Fixes #4141 🛠️

[google-cla[bot]]

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.

[chanseokoh]

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

[bjornbugge]

I think this also fixes #4131

[glasser]

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.

[mpeddada1]

@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.

[mpeddada1]

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

[bjornbugge]

@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)

[diegomarquezp]

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://github.com/google/google-java-format

[mpeddada1]

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

Additionally, to @chanseokoh's suggestion in https://github.com/GoogleContainerTools/jib/pull/4142#discussion_r1469684533, 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]

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

[chanseokoh]

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

[izogfif]

@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]

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

[izogfif]

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

[chanseokoh]

This PR is obsolete in favor of #4204.

[mpeddada1]

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

[bjornbugge]

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