Skip to content
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.

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

Add go buildinfo to velero to check toolchain version used. #8398

Closed
wants to merge 1 commit into from

Conversation

kaovilai
Copy link
Member

@kaovilai kaovilai commented Nov 12, 2024

Thank you for contributing to Velero!

Please add a summary of your change

Does your change fix a particular issue?

Facilitates requirements similar to #8397 that may come up in the future.
Helps validate go version used to build given that go.mod or commit hash of a built binary do not tell you this information.

Please indicate you've done the following:

@github-actions github-actions bot added the Dependencies Pull requests that update a dependency file label Nov 12, 2024
@kaovilai kaovilai force-pushed the auto-toolchain branch 4 times, most recently from b0a0cc1 to 106d45f Compare November 12, 2024 21:22
@kaovilai kaovilai force-pushed the auto-toolchain branch 2 times, most recently from 7a82ae4 to 6cc53cf Compare November 12, 2024 21:43
Copy link

codecov bot commented Nov 12, 2024

Codecov Report

Attention: Patch coverage is 10.00000% with 9 lines in your changes missing coverage. Please review.

Project coverage is 58.93%. Comparing base (8e23752) to head (ff8496c).
Report is 26 commits behind head on main.

Files with missing lines Patch % Lines
pkg/buildinfo/buildinfo.go 0.00% 8 Missing ⚠️
pkg/cmd/server/server.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8398      +/-   ##
==========================================
- Coverage   58.96%   58.93%   -0.03%     
==========================================
  Files         367      367              
  Lines       38895    38911      +16     
==========================================
+ Hits        22933    22934       +1     
- Misses      14500    14515      +15     
  Partials     1462     1462              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

@kaovilai kaovilai changed the title Add go buildinfo to velero to assist with #8397 Add go buildinfo to velero to check tool chain version used. Nov 12, 2024
@kaovilai kaovilai changed the title Add go buildinfo to velero to check tool chain version used. Add go buildinfo to velero to check toolchain version used. Nov 13, 2024
@@ -69,6 +69,7 @@ func printVersion(w io.Writer, clientOnly bool, kbClient kbclient.Client, server
fmt.Fprintln(w, "Client:")
fmt.Fprintf(w, "\tVersion: %s\n", buildinfo.Version)
fmt.Fprintf(w, "\tGit commit: %s\n", buildinfo.FormattedGitSHA())
fmt.Fprintf(w, "\tGo version: %s\n", buildinfo.GoVersion())
Copy link
Member Author

Choose a reason for hiding this comment

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

Is this clear enough that it is not user's local go? do we want to say Built by Go Version instead?


var (
// Version is the current version of Velero, set by the go linker's -X flag at build time.
Version string

// goVersion is the version of Go that was used to build Velero
goBuildInfo *debug.BuildInfo
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this necessary?

Copy link
Member Author

Choose a reason for hiding this comment

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

To be able to tell which golang version was used to build the code which can have implications around CVEs verification. ie someone built from a certain velero branch but used a different golang version. ie. #8397

Copy link
Member Author

@kaovilai kaovilai Nov 22, 2024

Choose a reason for hiding this comment

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

@reasonerjt just found
https://github.com/docker/cli/blob/6c76914532de7a39a202b3ef22519da319558560/cli/command/system/version.go#L93 have this :D

Just worth noting that it is a pattern out there that build includes both commit AND golang version that built it.

❯ docker version
Client: Docker Engine - Community
 Version:           27.1.1
 API version:       1.41 (downgraded from 1.46)
+Go version:        go1.22.5
 Git commit:        63125853e3
 Built:             Fri Jul 19 17:35:01 2024
 OS/Arch:           darwin/arm64
 Context:           default

Server: linux/arm64/rhcos-4.17
 Podman Engine:
  Version:          5.2.3
  APIVersion:       5.2.3
  Arch:             arm64
  BuildTime:        2024-10-07T07:47:17Z
  Experimental:     false
  GitCommit:        
+ GoVersion:        go1.22.5 (Red Hat 1.22.5-1.el9)
  KernelVersion:    5.14.0-427.40.1.el9_4.aarch64
  MinAPIVersion:    4.0.0
  Os:               linux
 Conmon:
  Version:          conmon version 2.1.12, commit: 9b3f2e7d010b1b512d88609d3a7b9a913bbaf1e0
  Package:          conmon-2.1.12-4.rhaos4.17.el9.aarch64
 OCI Runtime (crun):
  Version:          crun version 1.17
commit: 000fa0d4eeed8938301f3bcf8206405315bc1017
rundir: /run/crun
spec: 1.0.0
 +SYSTEMD +SELINUX +APPARMOR +CAP +SECCOMP +EBPF +CRIU +YAJL
  Package:          crun-1.17-1.rhaos4.17.el9.aarch64
 Engine:
  Version:          5.2.3
  API version:      1.41 (minimum version 1.24)
+ Go version:       go1.22.5 (Red Hat 1.22.5-1.el9)
  Git commit:       
  Built:            Mon Oct  7 07:47:17 2024
  OS/Arch:          linux/arm64
  Experimental:     false

Leaving this closed for now.

@kaovilai
Copy link
Member Author

Closing as it is deemed not necessary.

@kaovilai kaovilai closed this Nov 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Dependencies Pull requests that update a dependency file has-changelog has-unit-tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants