-
Notifications
You must be signed in to change notification settings - Fork 1.6k
🐛 fix(cli/version): Insert build info in go tools installation methods #5303
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
base: master
Are you sure you want to change the base?
🐛 fix(cli/version): Insert build info in go tools installation methods #5303
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: vitorfloriano The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
626c690 to
aeb0918
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR modernizes the version resolution logic by extracting build information from Go's built-in debug.ReadBuildInfo() instead of injecting it via ldflags during the build process. This enables go build and go install to produce binaries with complete version information equivalent to those built with make build and goreleaser build.
Key Changes:
- Replaced manual version injection via ldflags with automatic extraction from Go's embedded build metadata
- Refactored version logic into a dedicated
internal/versionpackage with comprehensive unit tests - Removed build-time K8s version parsing from testdata go.mod files; now extracted from binary's dependency list
- Added
KUBEBUILDER_TEST_VERSIONenvironment variable to ensure consistent version output in test environments
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
internal/version/version.go |
New package that implements version resolution using debug.ReadBuildInfo() to extract version, commit, build date, and Kubernetes vendor version from embedded build metadata |
internal/version/version_test.go |
Comprehensive unit tests for the version resolution helper functions |
cmd/cmd.go |
Updated to use the new version package instead of the old version variables |
cmd/version.go |
Removed - replaced by internal/version/version.go with simpler, more maintainable implementation |
cmd/version_test.go |
Removed - complex git-based pseudo-version tests no longer needed with new approach |
Makefile |
Simplified build target by removing ldflags and K8s version extraction logic; now uses go build --trimpath |
build/.goreleaser.yml |
Removed ldflags and KUBERNETES_VERSION environment variable configuration |
test/common.sh |
Added KUBEBUILDER_TEST_VERSION environment variable to force consistent "(devel)" version in tests |
.gitignore |
Added **/dist to ignore goreleaser build output directories |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func New() Version { | ||
| v := Version{ | ||
| KubeBuilderVersion: develVersion, | ||
| KubernetesVendor: unknown, | ||
| GitCommit: unknown, | ||
| BuildDate: unknown, | ||
| GoOs: runtime.GOOS, | ||
| GoArch: runtime.GOARCH, | ||
| } | ||
|
|
||
| if info, ok := debug.ReadBuildInfo(); ok { | ||
| v.KubeBuilderVersion = resolveMainVersion(info.Main) | ||
| v.KubernetesVendor = resolveKubernetesVendor(info.Deps) | ||
| v.applyVCSMetadata(info.Settings) | ||
| } | ||
|
|
||
| if develVersion := os.Getenv("KUBEBUILDER_TEST_VERSION"); develVersion != "" { | ||
| v.KubeBuilderVersion = develVersion | ||
| v.GitCommit = "test-commit" | ||
| v.BuildDate = "2025-01-01T00:00:00Z" | ||
| } | ||
|
|
||
| return v | ||
| } |
Copilot
AI
Dec 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing test coverage for the New() function which integrates all the version resolution logic including the environment variable override behavior. Consider adding tests that verify: 1) the overall version resolution when build info is available, 2) the environment variable override behavior with KUBEBUILDER_TEST_VERSION, and 3) fallback behavior when build info is not available. This would ensure the integration between all the helper functions works correctly.
| // GetKubeBuilderVersion returns only the CLI version string. | ||
| // Used for the cliVersion field in scaffolded PROJECT files. | ||
| func (v Version) GetKubeBuilderVersion() string { | ||
| return strings.TrimPrefix(v.KubeBuilderVersion, "v") | ||
| } |
Copilot
AI
Dec 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing test coverage for the GetKubeBuilderVersion() function. Consider adding tests to verify that it correctly strips the "v" prefix from version strings like "v4.10.4" -> "4.10.4", and handles edge cases like versions without a prefix (e.g., "(devel)" should remain "(devel)").
The go install and go build installation methods now set all the available fields in the version subcommand output. The version resolution logic has been changed to extract build info from the binary itself, rather than inject the info using ldflags at build time. Assisted by: Google/Gemini 3 Pro
f37b773 to
e77b072
Compare
|
ASAP controller-runtime new version support k8s 1.35 be release we should bump and release kb as well |
|
@camilamacedo86 No problem. This one is very delicate and we need to review it with care. If we manage to merge this one we can then encourage users to install Kubebuilder using We could also simplify some scripts that currently use that Anyway, take your time! Thanks! |
|
@vitorfloriano: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
Hey there, I'm a new contributor to open source project, is this issue still pending? can i work with this and solve? |
What:
This PR changes the version resolution logic so that
go buildandgo installproperly populates all the fields in theversionsubcommand, resulting in a similar output to when the binary is built usingmake buildandgoreleaser build:Also, the
cliVersionfield in thePROJECTfile is correctly populated:How:
All the information we need in the
versionoutput is already "stamped" in the go binary by default. We now usedebug.ReadBuildInfoto extract that info and populate the fields in theversionsubcommand output, instead of injecting the build info usingldflagsduring build process.Also, now that the build info is extracted in the build process, not injected, the
Makefileand.goreleaser.ymlfiles do not need to store, update and inject theldflags.The
KubernetesVendorversion is being extracted from thek8s.io/apimachinerymodule in the dependency list that is also stamped in the binary, making it not necessary to parse that info from thego.modfiles intestdata.When we bump theapimachineryversion, we will consequentially update theKubernetesVendorversion.And...
We also added
dist/to.gitignoreto avoid dirty builds when usinggoreleaser build.Closing notes
Now that
go buildandgo installhave a similar behavior tomake buildandgoreleaser build, we can encourage the use of those installation methods as well.Fixes #5302