From 6501ef490a45e9b7edfed1432702532c5b11c6d2 Mon Sep 17 00:00:00 2001 From: Manuel Alonso Date: Tue, 13 Jan 2026 19:21:16 +0100 Subject: [PATCH] chore(refactor): better testing and functionality for installing crd Signed-off-by: Manuel Alonso --- pkg/action/install.go | 48 ++++++++++++++++++-------------------- pkg/action/install_test.go | 40 +++++++++++-------------------- 2 files changed, 37 insertions(+), 51 deletions(-) diff --git a/pkg/action/install.go b/pkg/action/install.go index b84e9eaf9..f1677c297 100644 --- a/pkg/action/install.go +++ b/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 diff --git a/pkg/action/install_test.go b/pkg/action/install_test.go index 76c35b628..003a29528 100644 --- a/pkg/action/install_test.go +++ b/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}