Fix: Chart v4 fails on update (#3046)

<!--Thanks for your contribution. See [CONTRIBUTING](CONTRIBUTING.md)
    for Pulumi's contribution guidelines.

    Help us merge your changes more quickly by adding more details such
    as labels, milestones, and reviewers.-->

### Proposed changes

<!--Give us a brief description of what you've done and what it solves.
-->

This PR fixes a problem with how Chart v4 uses the Helm library. The
design goal is to allow for connectivity during template rendering, to
support the lookup function (see helm/helm#9426)
and to provide an accurate [Capabilities
object](https://helm.sh/docs/chart_template_guide/builtin_objects/).
Unfortunately we were slightly too aggressive and caused some of Helm's
"non-template" code to execute.

This fix works by turning off the `helm template --validation` flag, so
that the internal `ClientOnly` flag is true thus avoiding [this block of
code](https://github.com/helm/helm/blob/6f32a8f9d338bacc3c6a1c0c3610012b01edb3d1/pkg/action/install.go#L345-L350)
that causes the unexpected error. A side-effect of `ClientOnly` being
true is that the capabilities aren't automatically set, and so we set
them using the provider's kube client (akin to using `--kube-version`).

Detailed changes:
- (chart.go) use ClientOnly mode, set KubeVersion and APIVersions
- (tool.go) remove redundant KubeVersion and ExtraAPIs 
- (testdata/reference) add a version check
- (chart_test.go) unit tests for `.Capabilities`
- integration test to install cert-manager

### Related issues (optional)

<!--Refer to related PRs or issues: #1234, or 'Fixes #1234' or 'Closes
#1234'.
Or link to full URLs to issues or pull requests in other GitHub
repositories. -->
Closes #3045

CHANGELOG.md

@@ -5,6 +5,7 @@
 - CustomResource for Java SDK (https://github.com/pulumi/pulumi-kubernetes/pull/3020)
 - Fixed spurious diffing for updates when in renderYaml mode (https://github.com/pulumi/pulumi-kubernetes/pull/3030)
 - Kustomize Directory v2 resource (https://github.com/pulumi/pulumi-kubernetes/pull/3036) 
+- Fix: Chart v4 fails on update (https://github.com/pulumi/pulumi-kubernetes/pull/3046)
 
 ## 4.12.0 (May 21, 2024)
 

provider/pkg/clients/fake/clients.go

@@ -25,7 +25,7 @@ import (
 )
 
 var (
-	DefaultServerVersion = kubeversion.Info{Major: "1", Minor: "29"}
+	DefaultServerVersion = kubeversion.Info{Major: "1", Minor: "29", GitVersion: "v1.29.2"}
 )
 
 type NewDynamicClientOption func(*newDynamicClientOptions)

provider/pkg/helm/tool.go

@@ -33,7 +33,6 @@ import (
 	"helm.sh/helm/v3/pkg/action"
 	"helm.sh/helm/v3/pkg/chart"
 	"helm.sh/helm/v3/pkg/chart/loader"
-	"helm.sh/helm/v3/pkg/chartutil"
 	"helm.sh/helm/v3/pkg/cli"
 	"helm.sh/helm/v3/pkg/downloader"
 	"helm.sh/helm/v3/pkg/getter"
@@ -167,8 +166,6 @@ type TemplateCommand struct {
 	Validate    bool
 	IncludeCRDs bool
 	SkipTests   bool
-	KubeVersion string
-	ExtraAPIs   []string
 }
 
 // Template returns a new `helm template` command.
@@ -191,8 +188,6 @@ func (t *Tool) Template() *TemplateCommand {
 	cmd.IncludeCRDs = false
 	cmd.SkipTests = false
 	cmd.Install.IsUpgrade = false
-	cmd.KubeVersion = ""
-	cmd.ExtraAPIs = []string{}
 	cmd.Install.UseReleaseName = false
 	return cmd
 }
@@ -206,15 +201,6 @@ func (cmd *TemplateCommand) Execute(ctx context.Context) (*release.Release, erro
 		return nil, err
 	}
 
-	// https://github.com/helm/helm/blob/635b8cf33d25a86131635c32f35b2a76256e40cb/cmd/helm/template.go#L68-L74
-	if cmd.KubeVersion != "" {
-		parsedKubeVersion, err := chartutil.ParseKubeVersion(cmd.KubeVersion)
-		if err != nil {
-			return nil, fmt.Errorf("invalid kube version '%s': %s", cmd.KubeVersion, err)
-		}
-		client.KubeVersion = parsedKubeVersion
-	}
-
 	// https://github.com/helm/helm/blob/635b8cf33d25a86131635c32f35b2a76256e40cb/cmd/helm/template.go#L76-L81
 	registryClient, err := newRegistryClient(cmd.tool.EnvSettings, client.CertFile, client.KeyFile, client.CaFile,
 		client.InsecureSkipTLSverify, client.PlainHTTP)
@@ -231,7 +217,6 @@ func (cmd *TemplateCommand) Execute(ctx context.Context) (*release.Release, erro
 	// client.ReleaseName = "release-name"
 	client.Replace = true // Skip the name check
 	client.ClientOnly = !cmd.Validate
-	client.APIVersions = chartutil.VersionSet(cmd.ExtraAPIs)
 	client.IncludeCRDs = cmd.IncludeCRDs
 
 	return cmd.runInstall(ctx)

provider/pkg/provider/helm/v4/chart.go

@@ -18,16 +18,20 @@ import (
 	"context"
 	"fmt"
 
+	"github.com/pulumi/pulumi-kubernetes/provider/v4/pkg/clients"
 	kubehelm "github.com/pulumi/pulumi-kubernetes/provider/v4/pkg/helm"
 	providerresource "github.com/pulumi/pulumi-kubernetes/provider/v4/pkg/provider/resource"
 	provideryamlv2 "github.com/pulumi/pulumi-kubernetes/provider/v4/pkg/provider/yaml/v2"
 	helmv4 "github.com/pulumi/pulumi-kubernetes/sdk/v4/go/kubernetes/helm/v4"
 	"github.com/pulumi/pulumi/sdk/v3/go/pulumi"
 	"github.com/pulumi/pulumi/sdk/v3/go/pulumi/internals"
 	pulumiprovider "github.com/pulumi/pulumi/sdk/v3/go/pulumi/provider"
+	"helm.sh/helm/v3/pkg/action"
+	"helm.sh/helm/v3/pkg/chartutil"
 	helmkube "helm.sh/helm/v3/pkg/kube"
 	"helm.sh/helm/v3/pkg/postrender"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/client-go/discovery"
 )
 
 type toolF func() *kubehelm.Tool
@@ -173,10 +177,20 @@ func (r *ChartProvider) Construct(ctx *pulumi.Context, typ, name string, inputs
 	p := tool.AllGetters()
 	cmd := tool.Template()
 
-	cmd.Validate = true
+	// connectivity: use server-side dry-run to enable the lookup function.
+	// i.e. `helm template --dry-run=server --validate=false`.
+	cmd.DisableOpenAPIValidation = true
+	cmd.Validate = false
+	cmd.ClientOnly = true
 	cmd.DryRun = true
 	cmd.DryRunOption = "server"
+	if r.opts.ClientSet.DiscoveryClientCached != nil {
+		if err := setKubeVersionAndAPIVersions(r.opts.ClientSet, cmd); err != nil {
+			return nil, err
+		}
+	}
 
+	// set chart resolution options
 	cmd.Chart = chartArgs.Chart
 	cmd.Version = chartArgs.Version
 	cmd.Devel = chartArgs.Devel
@@ -194,6 +208,7 @@ func (r *ChartProvider) Construct(ctx *pulumi.Context, typ, name string, inputs
 		cmd.Keyring = keyring
 	}
 
+	// set templating options
 	cmd.Values.Values = chartArgs.Values
 	cmd.Values.ValuesFiles = chartArgs.ValuesFiles
 	cmd.IncludeCRDs = !chartArgs.SkipCrds
@@ -279,3 +294,35 @@ func preregister(ctx *pulumi.Context, comp *ChartState, obj *unstructured.Unstru
 
 	return obj, resourceOpts
 }
+
+func setKubeVersionAndAPIVersions(clientSet *clients.DynamicClientSet, cmd *kubehelm.TemplateCommand) error {
+	dc := clientSet.DiscoveryClientCached
+
+	// https://github.com/helm/helm/blob/635b8cf33d25a86131635c32f35b2a76256e40cb/pkg/action/action.go#L246-L285
+
+	// force a discovery cache invalidation to always fetch the latest server version/capabilities.
+	dc.Invalidate()
+	kubeVersion, err := dc.ServerVersion()
+	if err != nil {
+		return fmt.Errorf("could not get server version from Kubernetes: %w", err)
+	}
+	cmd.Install.KubeVersion = &chartutil.KubeVersion{
+		Version: kubeVersion.GitVersion,
+		Major:   kubeVersion.Major,
+		Minor:   kubeVersion.Minor,
+	}
+
+	// Client-Go emits an error when an API service is registered but unimplemented.
+	// Since the discovery client continues building the API object, it is correctly
+	// populated with all valid APIs.
+	// See https://github.com/kubernetes/kubernetes/issues/72051#issuecomment-521157642
+	apiVersions, err := action.GetVersionSet(dc)
+	if err != nil {
+		if !discovery.IsGroupDiscoveryFailedError(err) {
+			return fmt.Errorf("could not get apiVersions from Kubernetes: %w", err)
+		}
+	}
+	cmd.Install.APIVersions = apiVersions
+
+	return nil
+}

provider/pkg/provider/helm/v4/chart_test.go

@@ -128,7 +128,26 @@ var _ = Describe("Construct", func() {
 			Expect(err).ShouldNot(HaveOccurred())
 			Expect(executor.Action().DryRun).To(BeTrue())
 			Expect(executor.Action().DryRunOption).To(Equal("server"))
-			Expect(executor.Action().ClientOnly).To(BeFalse())
+			Expect(executor.Action().ClientOnly).To(BeTrue())
+		})
+		Describe("Capabilities", func() {
+			BeforeEach(func() {
+				inputs["values"] = resource.NewObjectProperty(resource.NewPropertyMapFromMap(map[string]any{
+					"versionCheck": ">=1.21-0",
+				}))
+			})
+			It("should have the correct kubeversion", func(ctx context.Context) {
+				_, err := pulumiprovider.Construct(ctx, req, tc.EngineConn(), k.Construct)
+				Expect(err).ShouldNot(HaveOccurred())
+				v := fake.DefaultServerVersion
+				Expect(executor.Action().KubeVersion).To(PointTo(Equal(
+					chartutil.KubeVersion{Version: v.GitVersion, Major: v.Major, Minor: v.Minor})))
+			})
+			It("should have the correct apiversions", func(ctx context.Context) {
+				_, err := pulumiprovider.Construct(ctx, req, tc.EngineConn(), k.Construct)
+				Expect(err).ShouldNot(HaveOccurred())
+				Expect(executor.Action().APIVersions).To(Not(BeEmpty()))
+			})
 		})
 	})
 

provider/pkg/provider/helm/v4/testdata/reference/templates/versioncheck.yaml

@@ -1,5 +1,5 @@
 {{- if .Values.versionCheck -}}
-{{- if semverCompare .Values.versionCheck .Capabilities.KubeVersion.GitVersion -}}
+{{- if not (semverCompare .Values.versionCheck .Capabilities.KubeVersion.GitVersion) -}}
 {{ fail "Version check failed" }}
 {{- end }}
 {{- end }}

provider/pkg/provider/helm/v4/testdata/reference/values.yaml

@@ -23,4 +23,4 @@ serviceAccount:
   # annotations:
   #   helm.sh/resource-policy: keep
 
-# versionCheck: "<1.21-0"
+# versionCheck: ">=1.21-0"

tests/sdk/java/chartv4_test.go

@@ -0,0 +1,20 @@
+package test
+
+import (
+	"testing"
+
+	"github.com/pulumi/providertest/pulumitest"
+	"github.com/pulumi/pulumi/sdk/v3/go/auto/optup"
+)
+
+// TestChartV4 deploys a complex stack using chart/v4 package.
+func TestChartv4(t *testing.T) {
+	test := pulumitest.NewPulumiTest(t, "testdata/chartv4")
+	t.Logf("into %s", test.Source())
+	t.Cleanup(func() {
+		test.Destroy()
+	})
+	test.Preview()
+	test.Up()
+	test.Up(optup.ExpectNoChanges())
+}

tests/sdk/java/testdata/chartv4/Pulumi.yaml

@@ -0,0 +1,18 @@
+name: chartv4
+runtime: yaml
+description: |
+  Installs cert-manager using Helm Chart v4 resource.
+  Features used:
+  - Chart resource
+variables: {}
+outputs:
+  resources: ${install.resources}
+resources:
+  ns:
+    type: kubernetes:core/v1:Namespace
+  install:
+    type: kubernetes:helm.sh/v4:Chart
+    properties:
+      namespace: ${ns.metadata.name}
+      chart: oci://registry-1.docker.io/bitnamicharts/cert-manager
+      version: "1.3.1"