From 515bab1c8725764dfef264e9d0cba8df61103993 Mon Sep 17 00:00:00 2001 From: Mikhail Mazurskiy Date: Fri, 11 Jul 2025 21:35:16 +1000 Subject: [PATCH 1/2] Use own scheme rather than mutating a global shared one Signed-off-by: Mikhail Mazurskiy --- pkg/chart/common/capabilities.go | 13 ++------- pkg/kube/client.go | 13 --------- pkg/kube/converter.go | 48 ++++++++++++++++---------------- pkg/kube/ready.go | 5 ++-- 4 files changed, 28 insertions(+), 51 deletions(-) diff --git a/pkg/chart/common/capabilities.go b/pkg/chart/common/capabilities.go index 355c3978a..4e7a50931 100644 --- a/pkg/chart/common/capabilities.go +++ b/pkg/chart/common/capabilities.go @@ -20,13 +20,10 @@ import ( "slices" "strconv" - "k8s.io/client-go/kubernetes/scheme" - - apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" - apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" k8sversion "k8s.io/apimachinery/pkg/util/version" helmversion "helm.sh/helm/v4/internal/version" + "helm.sh/helm/v4/pkg/kube" ) var ( @@ -109,13 +106,7 @@ func (v VersionSet) Has(apiVersion string) bool { } func allKnownVersions() VersionSet { - // We should register the built in extension APIs as well so CRDs are - // supported in the default version set. This has caused problems with `helm - // template` in the past, so let's be safe - apiextensionsv1beta1.AddToScheme(scheme.Scheme) - apiextensionsv1.AddToScheme(scheme.Scheme) - - groups := scheme.Scheme.PrioritizedVersionsAllGroups() + groups := kube.NativeScheme.PrioritizedVersionsAllGroups() vs := make(VersionSet, 0, len(groups)) for _, gv := range groups { vs = append(vs, gv.String()) diff --git a/pkg/kube/client.go b/pkg/kube/client.go index 26ba7abfc..91239a7a9 100644 --- a/pkg/kube/client.go +++ b/pkg/kube/client.go @@ -33,7 +33,6 @@ import ( jsonpatch "github.com/evanphx/json-patch/v5" v1 "k8s.io/api/core/v1" - apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" apiextv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" @@ -52,7 +51,6 @@ import ( "k8s.io/cli-runtime/pkg/genericclioptions" "k8s.io/cli-runtime/pkg/resource" "k8s.io/client-go/kubernetes" - "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" "k8s.io/client-go/util/csaupgrade" "k8s.io/client-go/util/retry" @@ -102,17 +100,6 @@ const ( FieldValidationDirectiveStrict FieldValidationDirective = "Strict" ) -func init() { - // Add CRDs to the scheme. They are missing by default. - if err := apiextv1.AddToScheme(scheme.Scheme); err != nil { - // This should never happen. - panic(err) - } - if err := apiextv1beta1.AddToScheme(scheme.Scheme); err != nil { - panic(err) - } -} - func (c *Client) newStatusWatcher() (*statusWaiter, error) { cfg, err := c.Factory.ToRESTConfig() if err != nil { diff --git a/pkg/kube/converter.go b/pkg/kube/converter.go index ac6d95fb4..3d1b6f0d6 100644 --- a/pkg/kube/converter.go +++ b/pkg/kube/converter.go @@ -17,8 +17,6 @@ limitations under the License. package kube // import "helm.sh/helm/v4/pkg/kube" import ( - "sync" - apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" "k8s.io/apimachinery/pkg/api/meta" @@ -28,8 +26,28 @@ import ( "k8s.io/client-go/kubernetes/scheme" ) -var k8sNativeScheme *runtime.Scheme -var k8sNativeSchemeOnce sync.Once +// NativeScheme is a clean *runtime.Scheme with _only_ Kubernetes +// native resources added to it. This is required to break free of custom resources +// that may have been added to scheme.Scheme due to Helm being used as a package in +// combination with e.g. a versioned kube client. If we would not do this, the client +// may attempt to perform e.g. a 3-way-merge strategy patch for custom resources. +// DO NOT MUTATE, this is a shared variable. +var NativeScheme *runtime.Scheme + +func init() { + NativeScheme = runtime.NewScheme() + if err := scheme.AddToScheme(NativeScheme); err != nil { + panic(err) + } + // API extensions are not in the above scheme set, + // and must thus be added separately. + if err := apiextensionsv1beta1.AddToScheme(NativeScheme); err != nil { + panic(err) + } + if err := apiextensionsv1.AddToScheme(NativeScheme); err != nil { + panic(err) + } +} // AsVersioned converts the given info into a runtime.Object with the correct // group and version set @@ -40,30 +58,12 @@ func AsVersioned(info *resource.Info) runtime.Object { // convertWithMapper converts the given object with the optional provided // RESTMapping. If no mapping is provided, the default schema versioner is used func convertWithMapper(obj runtime.Object, mapping *meta.RESTMapping) runtime.Object { - s := kubernetesNativeScheme() - var gv = runtime.GroupVersioner(schema.GroupVersions(s.PrioritizedVersionsAllGroups())) + var gv = runtime.GroupVersioner(schema.GroupVersions(NativeScheme.PrioritizedVersionsAllGroups())) if mapping != nil { gv = mapping.GroupVersionKind.GroupVersion() } - if obj, err := runtime.ObjectConvertor(s).ConvertToVersion(obj, gv); err == nil { + if obj, err := NativeScheme.ConvertToVersion(obj, gv); err == nil { return obj } return obj } - -// kubernetesNativeScheme returns a clean *runtime.Scheme with _only_ Kubernetes -// native resources added to it. This is required to break free of custom resources -// that may have been added to scheme.Scheme due to Helm being used as a package in -// combination with e.g. a versioned kube client. If we would not do this, the client -// may attempt to perform e.g. a 3-way-merge strategy patch for custom resources. -func kubernetesNativeScheme() *runtime.Scheme { - k8sNativeSchemeOnce.Do(func() { - k8sNativeScheme = runtime.NewScheme() - scheme.AddToScheme(k8sNativeScheme) - // API extensions are not in the above scheme set, - // and must thus be added separately. - apiextensionsv1beta1.AddToScheme(k8sNativeScheme) - apiextensionsv1.AddToScheme(k8sNativeScheme) - }) - return k8sNativeScheme -} diff --git a/pkg/kube/ready.go b/pkg/kube/ready.go index 7a06c72f9..bc30a2728 100644 --- a/pkg/kube/ready.go +++ b/pkg/kube/ready.go @@ -31,7 +31,6 @@ import ( "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/cli-runtime/pkg/resource" "k8s.io/client-go/kubernetes" - "k8s.io/client-go/kubernetes/scheme" deploymentutil "helm.sh/helm/v4/internal/third_party/k8s.io/kubernetes/deployment/util" ) @@ -144,7 +143,7 @@ func (c *ReadyChecker) IsReady(ctx context.Context, v *resource.Info) (bool, err return false, err } crd := &apiextv1beta1.CustomResourceDefinition{} - if err := scheme.Scheme.Convert(v.Object, crd, nil); err != nil { + if err := NativeScheme.Convert(v.Object, crd, nil); err != nil { return false, err } if !c.crdBetaReady(*crd) { @@ -155,7 +154,7 @@ func (c *ReadyChecker) IsReady(ctx context.Context, v *resource.Info) (bool, err return false, err } crd := &apiextv1.CustomResourceDefinition{} - if err := scheme.Scheme.Convert(v.Object, crd, nil); err != nil { + if err := NativeScheme.Convert(v.Object, crd, nil); err != nil { return false, err } if !c.crdReady(*crd) { From f04190b91ddc45b245a6a7c8fc3e42bc3fa10648 Mon Sep 17 00:00:00 2001 From: Mikhail Mazurskiy Date: Fri, 12 Sep 2025 10:42:05 +1000 Subject: [PATCH 2/2] Use pre-constructed scheme, do not create a new one for each object Signed-off-by: Mikhail Mazurskiy --- internal/chart/v3/lint/rules/deprecations.go | 7 ++----- pkg/chart/v2/lint/rules/deprecations.go | 7 ++----- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/internal/chart/v3/lint/rules/deprecations.go b/internal/chart/v3/lint/rules/deprecations.go index 6f86bdbbd..8e42c9585 100644 --- a/internal/chart/v3/lint/rules/deprecations.go +++ b/internal/chart/v3/lint/rules/deprecations.go @@ -21,11 +21,11 @@ import ( "strconv" "helm.sh/helm/v4/pkg/chart/common" + "helm.sh/helm/v4/pkg/kube" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apiserver/pkg/endpoints/deprecation" - kscheme "k8s.io/client-go/kubernetes/scheme" ) var ( @@ -93,11 +93,8 @@ func validateNoDeprecations(resource *k8sYamlStruct, kubeVersion *common.KubeVer } func resourceToRuntimeObject(resource *k8sYamlStruct) (runtime.Object, error) { - scheme := runtime.NewScheme() - kscheme.AddToScheme(scheme) - gvk := schema.FromAPIVersionAndKind(resource.APIVersion, resource.Kind) - out, err := scheme.New(gvk) + out, err := kube.NativeScheme.New(gvk) if err != nil { return nil, err } diff --git a/pkg/chart/v2/lint/rules/deprecations.go b/pkg/chart/v2/lint/rules/deprecations.go index 6eba316bc..5365a307e 100644 --- a/pkg/chart/v2/lint/rules/deprecations.go +++ b/pkg/chart/v2/lint/rules/deprecations.go @@ -21,11 +21,11 @@ import ( "strconv" "helm.sh/helm/v4/pkg/chart/common" + "helm.sh/helm/v4/pkg/kube" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apiserver/pkg/endpoints/deprecation" - kscheme "k8s.io/client-go/kubernetes/scheme" ) var ( @@ -93,11 +93,8 @@ func validateNoDeprecations(resource *k8sYamlStruct, kubeVersion *common.KubeVer } func resourceToRuntimeObject(resource *k8sYamlStruct) (runtime.Object, error) { - scheme := runtime.NewScheme() - kscheme.AddToScheme(scheme) - gvk := schema.FromAPIVersionAndKind(resource.APIVersion, resource.Kind) - out, err := scheme.New(gvk) + out, err := kube.NativeScheme.New(gvk) if err != nil { return nil, err }