Skip to content

refactor: use Go 1.18 buildinfo #2541

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sozercan merged 2 commits into from
Jan 28, 2023
Merged

refactor: use Go 1.18 buildinfo #2541

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e0c83d3
refactor: use Go 1.18 buildinfo
sozercan Jan 26, 2023
6ade31f
Merge branch 'master' into go118-buildinfo
sozercan Jan 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 2 additions & 12 deletions Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,22 +19,12 @@ ENV GO111MODULE=on \
GOARM=${TARGETVARIANT}

WORKDIR /go/src/github.com/open-policy-agent/gatekeeper

COPY pkg/ pkg/
COPY third_party/ third_party/
COPY vendor/ vendor/
COPY main.go main.go
COPY apis/ apis/
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 . .
Copy link
Copy Markdown
Contributor

@maxsmythe maxsmythe Jan 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Member Author

@sozercan sozercan Jan 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is just the builder image, it is not published

Copy link
Copy Markdown
Contributor

@maxsmythe maxsmythe Jan 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ack, makes sense

RUN go build -mod vendor -a -ldflags "${LDFLAGS:--X github.com/open-policy-agent/gatekeeper/pkg/version.Version=latest}" -o manager

FROM $BASEIMAGE

WORKDIR /

COPY --from=builder /go/src/github.com/open-policy-agent/gatekeeper/manager .

USER 65532:65532

ENTRYPOINT ["/manager"]
7 changes: 0 additions & 7 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Copy Markdown
Member Author

@sozercan sozercan Jan 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed hostname, i am not sure if there is value but I can re-add if anyone thinks this is important


LDFLAGS := "-X github.com/open-policy-agent/gatekeeper/pkg/version.Version=$(VERSION) \
-X github.com/open-policy-agent/gatekeeper/pkg/version.Vcs=$(BUILD_COMMIT) \
-X github.com/open-policy-agent/gatekeeper/pkg/version.Timestamp=$(BUILD_TIMESTAMP) \
-X github.com/open-policy-agent/gatekeeper/pkg/version.Hostname=$(BUILD_HOSTNAME) \
-X main.frameworksVersion=$(FRAMEWORKS_VERSION) \
-X main.opaVersion=$(OPA_VERSION)"

Expand Down
9 changes: 0 additions & 9 deletions build/get-build-commit.sh
View file
Open in desktop

This file was deleted.

3 changes: 0 additions & 3 deletions build/get-build-hostname.sh
View file
Open in desktop

This file was deleted.

3 changes: 0 additions & 3 deletions build/get-build-timestamp.sh
View file
Open in desktop

This file was deleted.

8 changes: 0 additions & 8 deletions build/vendormanifests.go
View file
Open in desktop

This file was deleted.

6 changes: 2 additions & 4 deletions gator.Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,8 @@ ENV GO111MODULE=on \
GOARCH=${TARGETARCH} \
GOARM=${TARGETVARIANT}

COPY . /tmp/gatekeeper

WORKDIR /tmp/gatekeeper/cmd/gator

COPY . /go/src/github.com/open-policy-agent/gatekeeper
WORKDIR /go/src/github.com/open-policy-agent/gatekeeper/cmd/gator
RUN go build -mod vendor -a -ldflags "${LDFLAGS:--X github.com/open-policy-agent/gatekeeper/pkg/version.Version=latest -X main.frameworksVersion=latest -X main.opaVersion=latest}" -o /gator

FROM --platform=$BUILDPLATFORM $BASEIMAGE as build
Expand Down
30 changes: 22 additions & 8 deletions pkg/version/version.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,32 @@ package version
import (
"fmt"
"runtime"
"runtime/debug"
)

// Vcs is is the commit hash for the binary build.
var Vcs string

// Timestamp is the date for the binary build.
var Timestamp string

// Version is the gatekeeper version.
var Version string

// GetUserAgent returns a user agent of the format: gatekeeper/<version> (<goos>/<goarch>) <vcs>/<timestamp>.
// GetUserAgent returns a user agent of the format: gatekeeper/<version> (<goos>/<goarch>) <vcsrevision><-vcsdirty>/<vcstimestamp>.
func GetUserAgent() string {
return fmt.Sprintf("gatekeeper/%s (%s/%s) %s/%s", Version, runtime.GOOS, runtime.GOARCH, Vcs, Timestamp)
vcsrevision := "unknown"
vcstimestamp := "unknown"
vcsdirty := ""

if info, ok := debug.ReadBuildInfo(); ok {
Copy link
Copy Markdown
Contributor

@acpana acpana Jan 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this PR!

QQ, what happens when !ok to the version here? even this would even happen at all.

Copy link
Copy Markdown
Member Author

@sozercan sozercan Jan 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor

@maxsmythe maxsmythe Jan 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from the code it looks like it defaults to "unknown/unknown"

Copy link
Copy Markdown
Member

@ritazh ritazh Jan 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should the build fail instead of returning unknown/unknown?

Copy link
Copy Markdown
Member Author

@sozercan sozercan Jan 28, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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"?

Copy link
Copy Markdown
Member

@ritazh ritazh Jan 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is mostly for our CI, I'm ok leave it as unknown for now unless someone raises an issue.

Copy link
Copy Markdown
Contributor

@maxsmythe maxsmythe Jan 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

for _, v := range info.Settings {
switch v.Key {
case "vcs.revision":
vcsrevision = v.Value
case "vcs.modified":
if v.Value == "true" {
vcsdirty = "-dirty"
}
case "vcs.time":
vcstimestamp = v.Value
}
}
}

return fmt.Sprintf("gatekeeper/%s (%s/%s) %s%s/%s", Version, runtime.GOOS, runtime.GOARCH, vcsrevision, vcsdirty, vcstimestamp)
}