From 68f7b1f1942669126e3290ff809a4dccc6bd0439 Mon Sep 17 00:00:00 2001
From: Hidde Beydals <hidde@hhh.computer>
Date: Tue, 7 Mar 2023 18:52:42 +0100
Subject: [PATCH] Properly invalidate client after CRD install

As the CRDs are installed before the capabilities are gathered, the
current call to invalidate the discovery client is premature and
expensive.

What actually is required is an invalidation of the REST mapper, as
otherwise the Helm install action may later on fail with a `resource
mapping not found` error. More specifically when the caller of the
action is making use of a persisting[1] `RESTClientGetter`.

Which is not something done by the Helm CLI (albeit it could, and this
would potentially save quite some resources?). But is a default
configuration offered by the Helm SDK via `kube.New` when a nil value
is provided as the `getter`.

[1]: https://github.com/kubernetes/cli-runtime/blob/v0.26.2/pkg/genericclioptions/config_flags.go#L118

Signed-off-by: Hidde Beydals <hidde@hhh.computer>
---
 pkg/action/install.go | 39 ++++++++++++++++++++++++++++-----------
 1 file changed, 28 insertions(+), 11 deletions(-)

diff --git a/pkg/action/install.go b/pkg/action/install.go
index 4658c9be8..7ee5b5609 100644
--- a/pkg/action/install.go
+++ b/pkg/action/install.go
@@ -34,6 +34,7 @@ import (
 	"github.com/pkg/errors"
 	v1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/cli-runtime/pkg/resource"
 	"sigs.k8s.io/yaml"
@@ -159,22 +160,38 @@ func (i *Install) installCRDs(crds []chart.CRD) error {
 		totalItems = append(totalItems, res...)
 	}
 	if len(totalItems) > 0 {
-		// Invalidate the local cache, since it will not have the new CRDs
-		// present.
-		discoveryClient, err := i.cfg.RESTClientGetter.ToDiscoveryClient()
-		if err != nil {
-			return err
-		}
-		i.cfg.Log("Clearing discovery cache")
-		discoveryClient.Invalidate()
 		// Give time for the CRD to be recognized.
-
 		if err := i.cfg.KubeClient.Wait(totalItems, 60*time.Second); err != nil {
 			return err
 		}
 
-		// Make sure to force a rebuild of the cache.
-		discoveryClient.ServerGroups()
+		// If we have already gathered the capabilities, we need to invalidate
+		// the cache so that the new CRDs are recognized. This should only be
+		// the case when an action configuration is reused for multiple actions,
+		// as otherwise it is later loaded by ourselves when getCapabilities
+		// is called later on in the installation process.
+		if i.cfg.Capabilities != nil {
+			discoveryClient, err := i.cfg.RESTClientGetter.ToDiscoveryClient()
+			if err != nil {
+				return err
+			}
+
+			i.cfg.Log("Clearing discovery cache")
+			discoveryClient.Invalidate()
+
+			_, _ = discoveryClient.ServerGroups()
+		}
+
+		// Invalidate the REST mapper, since it will not have the new CRDs
+		// present.
+		restMapper, err := i.cfg.RESTClientGetter.ToRESTMapper()
+		if err != nil {
+			return err
+		}
+		if resettable, ok := restMapper.(meta.ResettableRESTMapper); ok {
+			i.cfg.Log("Clearing REST mapper cache")
+			resettable.Reset()
+		}
 	}
 	return nil
 }