chore(refactor): better testing and functionality for installing crd

Signed-off-by: Manuel Alonso <m.alonso@Manuels-MacBook-Pro.local>

pkg/action/install.go

@@ -222,34 +222,32 @@ func (i *Install) installCRDs(crds []chart.CRD) error {
 			return err
 		}
 
-		if i.cfg.RESTClientGetter != nil {
-			// 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.Logger().Debug("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 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
 			}
-			if resettable, ok := restMapper.(meta.ResettableRESTMapper); ok {
-				i.cfg.Logger().Debug("clearing REST mapper cache")
-				resettable.Reset()
-			}
+
+			i.cfg.Logger().Debug("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.Logger().Debug("clearing REST mapper cache")
+			resettable.Reset()
 		}
 	}
 	return nil

pkg/action/install_test.go

@@ -35,7 +35,6 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	appsv1 "k8s.io/api/apps/v1"
-	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	kuberuntime "k8s.io/apimachinery/pkg/runtime"
@@ -45,6 +44,7 @@ import (
 	"k8s.io/client-go/rest/fake"
 
 	ci "helm.sh/helm/v4/pkg/chart"
+	"helm.sh/helm/v4/pkg/cli"
 
 	"helm.sh/helm/v4/internal/test"
 	"helm.sh/helm/v4/pkg/chart/common"
@@ -1086,6 +1086,8 @@ func TestInstallSetRegistryClient(t *testing.T) {
 
 func TestInstallCRDs(t *testing.T) {
 	config := actionConfigFixtureWithDummyResources(t, createDummyResourceList(false))
+	config.RESTClientGetter = cli.New().RESTClientGetter()
+
 	instAction := NewInstall(config)
 
 	mockFile := common.File{
@@ -1094,10 +1096,18 @@ func TestInstallCRDs(t *testing.T) {
 	}
 	mockChart := buildChart(withFile(mockFile))
 	crdsToInstall := mockChart.CRDObjects()
-	assert.Len(t, crdsToInstall, 1)
-	assert.Equal(t, crdsToInstall[0].File.Data, mockFile.Data)
 
-	require.NoError(t, instAction.installCRDs(crdsToInstall))
+	t.Run("fresh installation", func(t *testing.T) {
+		assert.Len(t, crdsToInstall, 1)
+		assert.Equal(t, crdsToInstall[0].File.Data, mockFile.Data)
+		require.NoError(t, instAction.installCRDs(crdsToInstall))
+	})
+
+	t.Run("already exist", func(t *testing.T) {
+		assert.Len(t, crdsToInstall, 1)
+		assert.Equal(t, crdsToInstall[0].File.Data, mockFile.Data)
+		require.NoError(t, instAction.installCRDs(crdsToInstall))
+	})
 }
 
 func TestInstallCRDs_KubeClient_BuildError(t *testing.T) {
@@ -1134,28 +1144,6 @@ func TestInstallCRDs_KubeClient_CreateError(t *testing.T) {
 	require.Error(t, instAction.installCRDs(crdsToInstall), "failed to install CRD")
 }
 
-func TestInstallCRDs_AlreadyExist(t *testing.T) {
-	config := actionConfigFixture(t)
-	failingKubeClient := kubefake.FailingKubeClient{PrintingKubeClient: kubefake.PrintingKubeClient{Out: io.Discard}, DummyResources: nil}
-	mockError := &apierrors.StatusError{ErrStatus: metav1.Status{
-		Status: metav1.StatusFailure,
-		Reason: metav1.StatusReasonAlreadyExists,
-	}}
-	failingKubeClient.CreateError = mockError
-	config.KubeClient = &failingKubeClient
-	instAction := NewInstall(config)
-
-	mockFile := common.File{
-		Name: "crds/foo.yaml",
-		Data: []byte("hello"),
-	}
-
-	mockChart := buildChart(withFile(mockFile))
-	crdsToInstall := mockChart.CRDObjects()
-
-	require.Error(t, instAction.installCRDs(crdsToInstall), "failed to install CRD")
-}
-
 func TestInstallCRDs_WaiterError(t *testing.T) {
 	config := actionConfigFixture(t)
 	failingKubeClient := kubefake.FailingKubeClient{PrintingKubeClient: kubefake.PrintingKubeClient{Out: io.Discard}, DummyResources: nil}