build: derive default Capabilities.KubeVersion from client-go - #1868
Open
norman-zon wants to merge 2 commits into
Open
build: derive default Capabilities.KubeVersion from client-go#1868norman-zon wants to merge 2 commits into
norman-zon wants to merge 2 commits into
Conversation
Helm's chartutil declares k8sVersionMajor/Minor as build-time overridable variables whose in-source values are "1" and "20". Helm's own Makefile replaces them via -ldflags, deriving them from the k8s.io/client-go version it builds against, so the helm CLI reports a current default. This provider builds with no -ldflags for them, so it inherits the literal source values and DefaultCapabilities.KubeVersion resolves to v1.20.0. That surfaces in data.helm_template, which renders with ClientOnly and therefore falls back to DefaultCapabilities. Any chart declaring a kubeVersion constraint above 1.20 refuses to render, and the error names the bogus version rather than the constraint's real problem: Error running Helm install: chart requires kubeVersion: >=1.29.0-0 which is incompatible with Kubernetes v1.20.0 The same chart renders fine with the helm CLI, because that binary is the one build where the ldflags are set. Set the two flags in both build paths, mirroring Helm's Makefile: the release build in .github/workflows/build.yml, and the GNUmakefile so local builds behave like released ones. client-go v0.x.y corresponds to Kubernetes v1.x.y, hence the +1 on the major. With k8s.io/client-go v0.35.1 the default becomes v1.35.0.
|
Thank you for your submission! We require that all contributors sign our Contributor License Agreement ("CLA") before we can accept the contribution. Read and sign the agreement Learn more about why HashiCorp requires a CLA and what the CLA includes Have you signed the CLA already but the status is still pending? Recheck it. |
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
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.
Description
data.helm_templatereports the rendering cluster as Kubernetes v1.20.0, so any chart declaring akubeVersionconstraint above 1.20 cannot be rendered at all.The cause is a missing pair of linker flags rather than anything in the provider's Go code.
Helm's
pkg/chartutil/capabilities.godeclares the default as build-time overridable variables, with deliberately stale in-source values:Helm's own
Makefilereplaces both at link time, deriving them from thek8s.io/client-goversion it builds against:This provider sets no such flags in either build path, so it links the library with the literal source values and
chartutil.DefaultCapabilities.KubeVersionresolves tov1.20.0.data.helm_templatesetsClientOnly(unlessvalidate = true), which is exactly the path that falls back toDefaultCapabilities— so every offline render claims v1.20.0.The resulting error points at the wrong thing. It names a Kubernetes version that exists nowhere in the user's infrastructure, which makes it read as a cluster or chart problem:
The same chart renders fine with
helm template, because the CLI is the one build where the flags are set. That divergence is what makes this hard to diagnose from the outside — see #1253, where a user hit this on EKS 1.27, was advised to try installing withhelm(which works, for this reason), and the report was eventually closed by the stale bot with the cause never identified.Fix
Set the two flags in both build paths, mirroring Helm's Makefile:
.github/workflows/build.yml— the release build viahashicorp/actions-go-build. This is the one that determines what registry users get.GNUmakefile—buildandpackages, so local and cross builds behave like released ones. Introduces anLDFLAGSvariable, as there was none.k8s.io/client-go v0.x.ycorresponds to Kubernetesv1.x.y, hence the+1on the major. With the currently pinnedk8s.io/client-go v0.35.1the default becomes v1.35.0, and it will track the dependency automatically from here on rather than needing a manual bump.Verification
Default capability, before and after, linking the same tree:
Both build paths resolve the values correctly:
End to end against a real chart with a
kubeVersionfloor (cloudnative-pgdeclareskubeVersion: ">=1.29.0-0"), using this exact config with nokube_versionargument:Error running Helm install: chart requires kubeVersion: >=1.29.0-0 which is incompatible with Kubernetes v1.20.0dev_overrides)Read complete after 1s— all 11 CRDs renderedNotes
kube_version. That argument (added in Add support for setting kube version on data.helm_template. #985, requested in Add kube version parameter to template #917 and helm_template data source does not provide option to set kube version #994) stays the way to pin a specific version deliberately. This only makes the default current instead of leaving it at Kubernetes 1.20, released December 2020.validate = trueis not an alternative. It clearsClientOnlyso Helm discovers real capabilities, but that makes an offline render require cluster connectivity and adds server-side validation of the whole rendered output.go.mod, soreproducible: reportis unaffected.docs/.Relates to #1253, which reported this symptom and was closed without the cause being found.