Skip to content

Conversation

8tomat8
Copy link
Contributor

@8tomat8 8tomat8 commented Jul 21, 2025

What this PR does / why we need it:

The semver validation is not compatible with what k8s itself uses.
Here is an example of the version response from the kube api:

kubectl version --output json | jq '.serverVersion'
{
  "major": "1",
  "minor": "28+",
  "gitVersion": "v1.28.15-eks-4096722",
  "gitCommit": "79964c34fd537f50bf77579c5bd51cb4426ae5e7",
  "gitTreeState": "clean",
  "buildDate": "2025-04-19T08:21:43Z",
  "goVersion": "go1.23.6",
  "compiler": "gc",
  "platform": "linux/arm64"
}

The version is compiled as 1.28+. This type of version format is used by AWS EKS and considered valid according to the version validator in k8s.

When the same version is parsed into helm template of helm lint, it is being validated with ParseKubeVersion which uses the github.com/Masterminds/semver/v3 package.

The Masterminds/semver package does not support trailing + symbol and upon command run, it throws the Invalid Semantic Version error.

`helm template . --name-template <APP_NAME> --namespace <NAMESPACE> --kube-version 1.28+ --values <TEMP_DIR_1>/values.yaml --values <TEMP_DIR_2> <api versions removed> --include-crds`
failed exit status 1: Error: invalid kube version '1.28+': Invalid Semantic Version

Adresses the #31063

Special notes for your reviewer:

If applicable:

  • this PR contains user facing changes (the docs needed label should be applied if so)
  • this PR contains unit tests
  • this PR has been tested for backwards compatibility

@pull-request-size pull-request-size bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jul 21, 2025
@robertsirc
Copy link
Member

@mattfarina you want this one or you want me to get it?

Copy link
Collaborator

@mattfarina mattfarina left a comment

Choose a reason for hiding this comment

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

LGTM

When Helm initially setup this handling, a long time ago, Kubernetes used semver. Kubernetes has changed so Helm needs to change accordingly.

@mattfarina mattfarina added the Has One Approval This PR has one approval. It still needs a second approval to be merged. label Jul 23, 2025
@mattfarina
Copy link
Collaborator

I think this could be backported for v3, as well.

Copy link
Contributor

@TerryHowe TerryHowe left a comment

Choose a reason for hiding this comment

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

/lgtm

Copy link
Member

@robertsirc robertsirc left a comment

Choose a reason for hiding this comment

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

LGTM

@robertsirc robertsirc added approved Indicates a PR has been approved by the required number of approvers and removed Has One Approval This PR has one approval. It still needs a second approval to be merged. labels Jul 24, 2025
@robertsirc robertsirc merged commit a42b764 into helm:main Jul 24, 2025
5 checks passed
@8tomat8
Copy link
Contributor Author

8tomat8 commented Jul 25, 2025

@mattfarina should I create a separate PR into the dev-v3 branch with the same changes?

@TerryHowe
Copy link
Contributor

@mattfarina should I create a separate PR into the dev-v3 branch with the same changes?

Yes

@scottrigby scottrigby added the bug Categorizes issue or PR as related to a bug. label Sep 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by the required number of approvers bug Categorizes issue or PR as related to a bug. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants