refactor: use Go 1.18 buildinfo #2541
Conversation
Signed-off-by: Sertac Ozercan <[email protected]>
| @@ -40,14 +40,7 @@ BENCHMARK_FILE_NAME ?= benchmarks.txt | |||
| ROOT_DIR := $(shell dirname $(realpath $(firstword $(MAKEFILE_LIST)))) | |||
| BIN_DIR := $(abspath $(ROOT_DIR)/bin) | |||
|
|
|||
| BUILD_COMMIT := $(shell ./build/get-build-commit.sh) | |||
| BUILD_TIMESTAMP := $(shell ./build/get-build-timestamp.sh) | |||
| BUILD_HOSTNAME := $(shell ./build/get-build-hostname.sh) | |||
There was a problem hiding this comment.
removed hostname, i am not sure if there is value but I can re-add if anyone thinks this is important
| COPY go.mod . | ||
|
|
||
| RUN go build -mod vendor -a -ldflags "${LDFLAGS:--X github.com/open-policy-agent/gatekeeper/pkg/version.Version=latest}" -o manager main.go | ||
| COPY . . |
There was a problem hiding this comment.
This runs the risk of copying temp files, such as .output into the system, which may bloat the binary and/or lead to inadvertent disclosures of a publisher's machine.
There was a problem hiding this comment.
this is just the builder image, it is not published
| vcstimestamp := "unknown" | ||
| vcsdirty := "" | ||
|
|
||
| if info, ok := debug.ReadBuildInfo(); ok { |
There was a problem hiding this comment.
I like this PR!
QQ, what happens when !ok to the version here? even this would even happen at all.
There was a problem hiding this comment.
looks like it might be !ok if it's built without mod support or parsing fails https://cs.opensource.google/go/go/+/refs/tags/go1.19.5:src/runtime/debug/mod.go;l=20;drc=178080740c1bc33f2c7f164504eedc24210bbf1e;bpv=1;bpt=1
There was a problem hiding this comment.
from the code it looks like it defaults to "unknown/unknown"
There was a problem hiding this comment.
should the build fail instead of returning unknown/unknown?
There was a problem hiding this comment.
this wouldn't happen in our CI. If someone is randomly building GK with a super old version of Go, do we support it or care that it says "unknown"?
There was a problem hiding this comment.
Since this is mostly for our CI, I'm ok leave it as unknown for now unless someone raises an issue.
There was a problem hiding this comment.
IMO unknown/unknown is fine in order to avoid unnecessary brittleness for people hacking on G8r. If we wanted to ensure our builds aren't affected, we could test that the flag returns an expected result?
Signed-off-by: Sertac Ozercan [email protected]
What this PR does / why we need it:
instead of passing in vcs info through LDFLAGS, migrating to use Go 1.18 buildinfo https://tip.golang.org/doc/go1.18#debug/buildinfo
Which issue(s) this PR fixes (optional, using
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when the PR gets merged):Fixes #
Special notes for your reviewer: