fix: preserve vendor suffixes in KubeVersion.GitVersion#31528
Merged
fix: preserve vendor suffixes in KubeVersion.GitVersion#31528
Conversation
benoittgt
commented
Nov 18, 2025
| // String implements fmt.Stringer. | ||
| // Returns the normalized version for constraint checking. | ||
| func (kv *KubeVersion) String() string { | ||
| if kv.normalizedVersion != "" { |
Contributor
Author
There was a problem hiding this comment.
That’s the part I’m the least convinced about. But at least is backward compatible and produce something relevant.
Helm 3.19.0 introduced a regression where vendor-specific suffixes (e.g., -gke.1245000, -eks-4096722, +) are stripped from .Capabilities.KubeVersion.GitVersion, breaking charts that detect managed Kubernetes platforms. The root cause was using k8sversion.ParseGeneric().String() which intentionally discards vendor suffixes. The fix stores both the full version (with vendor suffix) and a normalized version. String() returns the normalized version for constraint checking (e.g., ">= 1.21.0"), while Version/GitVersion preserve the full string for template access. Fixes helm#31423 Related to helm#31063, helm#31078 Signed-off-by: Benoit Tigeot <benoit.tigeot@lifen.fr>
0126392 to
ce273ee
Compare
TerryHowe
approved these changes
Nov 20, 2025
Contributor
There was a problem hiding this comment.
/lgtm
Full tests pass for me
Contributor
Author
|
I have two approvals. Is it possible to merge @sabre1041 ? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
Helm 3.19.0 introduced a regression where vendor-specific suffixes (e.g., -gke.1245000, -eks-4096722, +) are stripped from
.Capabilities.KubeVersion.GitVersion, breaking charts that detect managed Kubernetes platforms (#31423).The root cause was using
k8sversion.ParseGeneric().String()which intentionally discards vendor suffixes. It is good thing for comparaison but no when return the fullGitVersion. The fix preserves the original version string while still usingParseGeneric()for validation and Major/Minor extraction.I am not super happy with the fix, especially with
String(), but I was not able to find easier path.Fixes #31423
Related to #31063, #31078
Special notes for your reviewer:
Script I used to check the behavior:
Tiny increase in coverage 🐜 :
If applicable:
docs neededlabel should be applied if so). Not sure it needs to be documented