Change build info date to commit timestamp #3876
Conversation
This change fixes binary reproducibility of builds. Injecting a timestamp of the time-of-build produces unique binaries for every build which is undesirable for security and provenance tracking. If timestamps for builds are desired, it is recommended to inject the timestamp of the commit. This gives a timestamp that is consistent for a build based on that commit and allows checking for binary consistenty across build systems. This change updates BuildDate to use the commit timestamp from git. This change also fixes some builds which include the version package but were not injecting BUILD_INFO. Signed-off-by: Calvin Behling <[email protected]>
There was a problem hiding this comment.
I don't have a particular objection to this change, but it's worth noting that the binary output also depends on the version of Go compiler, which is not controlled by our setup, e.g. we define the compiler version as 1.18.x in GH actions.
And a related question. I see you verified the checksum. Should we be publishing it along with the binaries? I remember it came up before.
| @@ -170,15 +170,15 @@ build-tracegen: | |||
|
|
|||
| .PHONY: build-anonymizer | |||
| build-anonymizer: | |||
| $(GOBUILD) -o ./cmd/anonymizer/anonymizer-$(GOOS)-$(GOARCH) ./cmd/anonymizer/main.go | |||
| $(GOBUILD) -o ./cmd/anonymizer/anonymizer-$(GOOS)-$(GOARCH) $(BUILD_INFO) ./cmd/anonymizer/main.go | |||
There was a problem hiding this comment.
maybe it's better to define GOBUILD to contain BUILD_INFO, to avoid this repetition
There was a problem hiding this comment.
I've limited the scope to those that include `github.com/jaegertracing/jaeger/pkg/version. Including it in GOBUILD does seem like a reasonable improvement for consistency across the binaries but would force changes to the actual go code for some number of binaries.
|
Another possible issue is the UI assets are always generated with some unique number. Your test did not rebuild UI assets. |
Codecov ReportBase: 97.64% // Head: 97.62% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #3876 +/- ##
==========================================
- Coverage 97.64% 97.62% -0.02%
==========================================
Files 293 293
Lines 17064 17064
==========================================
- Hits 16662 16659 -3
- Misses 317 319 +2
- Partials 85 86 +1
☔ View full report at Codecov. |
This is true and reproducibility should be fixed with regard to the UI builds as well. Testing the gzipped files in |
| @@ -42,7 +42,7 @@ IMPORT_LOG=.import.log | |||
|
|
|||
| GIT_SHA=$(shell git rev-parse HEAD) | |||
| GIT_CLOSEST_TAG=$(shell git describe --abbrev=0 --tags) | |||
| DATE=$(shell date -u +'%Y-%m-%dT%H:%M:%SZ') | |||
| DATE=$(shell date -u -d @$(shell git show -s --format=%ct) +'%Y-%m-%dT%H:%M:%SZ') | |||
There was a problem hiding this comment.
date -d does not work on Mac. We can do this directly with git:
$ TZ=UTC0 git show --quiet --date='format-local:%Y-%m-%dT%H:%M:%SZ' --format="%cd" | cat
2022-10-28T15:18:09Z
Which problem is this PR solving?
Binary Reproducibility
See https://reproducible-builds.org for more info on why this is important.
Short description of the changes
This change fixes binary reproducibility of builds. Injecting a
timestamp of the time-of-build produces unique binaries for every build
which is undesirable for security and provenance tracking. If timestamps
for builds are desired, it is recommended to inject the timestamp of the
commit. This gives a timestamp that is consistent for a build based on
that commit and allows checking for binary consistenty across build systems.
This change updates BuildDate to use the commit timestamp from git.
This change also fixes some builds which include the version package but
were not injecting BUILD_INFO.
Manual testing on main
Manual testing on branch