diff --git a/pkg/action/install.go b/pkg/action/install.go index 7857b9035..e29bed580 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -181,12 +181,24 @@ func (i *Install) installCRDs(crds []chart.CRD) error { // We do these one file at a time in the order they were read. totalItems := []*resource.Info{} for _, obj := range crds { + if obj.File == nil { + return fmt.Errorf("failed to install CRD %s: file is empty", obj.Name) + } + + if obj.File.Data == nil { + return fmt.Errorf("failed to install CRD %s: file data is empty", obj.Name) + } + // Read in the resources res, err := i.cfg.KubeClient.Build(bytes.NewBuffer(obj.File.Data), false) if err != nil { return fmt.Errorf("failed to install CRD %s: %w", obj.Name, err) } + if len(res) == 0 { + return fmt.Errorf("failed to install CRD %s: resources are empty", obj.Name) + } + // Send them to Kube if _, err := i.cfg.KubeClient.Create( res, @@ -222,27 +234,30 @@ func (i *Install) installCRDs(crds []chart.CRD) error { // 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 i.cfg.RESTClientGetter != nil { + if i.cfg.Capabilities != nil { + discoveryClient, err := i.cfg.RESTClientGetter.ToDiscoveryClient() + if err != nil { + return err + } + + if discoveryClient != nil { + 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 } - - 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() + 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 47080aef8..52dd83788 100644 --- a/pkg/action/install_test.go +++ b/pkg/action/install_test.go @@ -111,6 +111,54 @@ func createDummyResourceList(owned bool) kube.ResourceList { return resourceList } +func createDummyCRDList(owned bool) kube.ResourceList { + obj := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "dummyName", + Namespace: "spaced", + }, + } + + if owned { + obj.Labels = map[string]string{ + "app.kubernetes.io/managed-by": "Helm", + } + obj.Annotations = map[string]string{ + "meta.helm.sh/release-name": "test-install-release", + "meta.helm.sh/release-namespace": "spaced", + } + } + + resInfo := resource.Info{ + Name: "dummyName", + Namespace: "spaced", + Mapping: &meta.RESTMapping{ + Resource: schema.GroupVersionResource{Group: "test", Version: "v1", Resource: "crd"}, + GroupVersionKind: schema.GroupVersionKind{Group: "test", Version: "v1", Kind: "crd"}, + Scope: meta.RESTScopeNamespace, + }, + Object: obj, + } + body := io.NopCloser(bytes.NewReader([]byte(kuberuntime.EncodeOrDie(appsv1Codec, obj)))) + + resInfo.Client = &fake.RESTClient{ + GroupVersion: schema.GroupVersion{Group: "test", Version: "v1"}, + NegotiatedSerializer: scheme.Codecs.WithoutConversion(), + Client: fake.CreateHTTPClient(func(_ *http.Request) (*http.Response, error) { + header := http.Header{} + header.Set("Content-Type", kuberuntime.ContentTypeJSON) + return &http.Response{ + StatusCode: http.StatusOK, + Header: header, + Body: body, + }, nil + }), + } + var resourceList kube.ResourceList + resourceList.Append(&resInfo) + return resourceList +} + func installActionWithConfig(config *Configuration) *Install { instAction := NewInstall(config) instAction.Namespace = "spaced" @@ -1084,8 +1132,8 @@ func TestInstallSetRegistryClient(t *testing.T) { assert.Equal(t, registryClient, instAction.GetRegistryClient()) } -func TestInstalLCRDs(t *testing.T) { - config := actionConfigFixture(t) +func TestInstallCRDs(t *testing.T) { + config := actionConfigFixtureWithDummyResources(t, createDummyCRDList(false)) instAction := NewInstall(config) mockFile := common.File{ @@ -1094,16 +1142,22 @@ 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)) } -func TestInstalLCRDs_KubeClient_BuildError(t *testing.T) { - config := actionConfigFixture(t) - failingKubeClient := kubefake.FailingKubeClient{PrintingKubeClient: kubefake.PrintingKubeClient{Out: io.Discard}, DummyResources: nil} - failingKubeClient.BuildError = errors.New("build error") +func TestInstallCRDs_AlreadyExist(t *testing.T) { + dummyResources := createDummyCRDList(false) + failingKubeClient := kubefake.FailingKubeClient{PrintingKubeClient: kubefake.PrintingKubeClient{Out: io.Discard}, DummyResources: dummyResources} + mockError := &apierrors.StatusError{ErrStatus: metav1.Status{ + Status: metav1.StatusFailure, + Reason: metav1.StatusReasonAlreadyExists, + }} + failingKubeClient.CreateError = mockError + + config := actionConfigFixtureWithDummyResources(t, dummyResources) config.KubeClient = &failingKubeClient instAction := NewInstall(config) @@ -1114,13 +1168,13 @@ func TestInstalLCRDs_KubeClient_BuildError(t *testing.T) { mockChart := buildChart(withFile(mockFile)) crdsToInstall := mockChart.CRDObjects() - require.Error(t, instAction.installCRDs(crdsToInstall), "failed to install CRD") + assert.Nil(t, instAction.installCRDs(crdsToInstall)) } -func TestInstalLCRDs_KubeClient_CreateError(t *testing.T) { +func TestInstallCRDs_KubeClient_BuildError(t *testing.T) { config := actionConfigFixture(t) failingKubeClient := kubefake.FailingKubeClient{PrintingKubeClient: kubefake.PrintingKubeClient{Out: io.Discard}, DummyResources: nil} - failingKubeClient.CreateError = errors.New("create error") + failingKubeClient.BuildError = errors.New("build error") config.KubeClient = &failingKubeClient instAction := NewInstall(config) @@ -1134,14 +1188,10 @@ func TestInstalLCRDs_KubeClient_CreateError(t *testing.T) { require.Error(t, instAction.installCRDs(crdsToInstall), "failed to install CRD") } -func TestInstalLCRDs_AlreadyExist(t *testing.T) { +func TestInstallCRDs_KubeClient_CreateError(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 + failingKubeClient.CreateError = errors.New("create error") config.KubeClient = &failingKubeClient instAction := NewInstall(config) @@ -1152,10 +1202,10 @@ func TestInstalLCRDs_AlreadyExist(t *testing.T) { mockChart := buildChart(withFile(mockFile)) crdsToInstall := mockChart.CRDObjects() - assert.Nil(t, instAction.installCRDs(crdsToInstall)) + require.Error(t, instAction.installCRDs(crdsToInstall), "failed to install CRD") } -func TestInstalLCRDs_WaiterError(t *testing.T) { +func TestInstallCRDs_WaiterError(t *testing.T) { config := actionConfigFixture(t) failingKubeClient := kubefake.FailingKubeClient{PrintingKubeClient: kubefake.PrintingKubeClient{Out: io.Discard}, DummyResources: nil} failingKubeClient.WaitError = errors.New("wait error") @@ -1187,6 +1237,45 @@ func TestCheckDependencies_MissingDependency(t *testing.T) { assert.ErrorContains(t, CheckDependencies(mockChart, []ci.Dependency{&dependency}), "missing in charts") } +func TestInstallCRDs_CheckNilErrors(t *testing.T) { + tests := []struct { + name string + input []chart.CRD + }{ + { + name: "only one crd with file nil", + input: []chart.CRD{ + {Name: "one", File: nil}, + }, + }, + { + name: "only one crd with its file data nil", + input: []chart.CRD{ + {Name: "one", File: &common.File{Name: "crds/foo.yaml", Data: nil}}, + }, + }, + { + name: "at least a crd with its file data nil", + input: []chart.CRD{ + {Name: "one", File: &common.File{Name: "crds/foo.yaml", Data: []byte("data")}}, + {Name: "two", File: &common.File{Name: "crds/foo2.yaml", Data: nil}}, + {Name: "three", File: &common.File{Name: "crds/foo3.yaml", Data: []byte("data")}}, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + instAction := installAction(t) + + err := instAction.installCRDs(tt.input) + if err == nil { + t.Errorf("got nil expected err") + } + }) + } +} + func TestInstallRelease_WaitOptionsPassedDownstream(t *testing.T) { is := assert.New(t) diff --git a/pkg/cmd/install_test.go b/pkg/cmd/install_test.go index f0f12e4f7..5fa3c1340 100644 --- a/pkg/cmd/install_test.go +++ b/pkg/cmd/install_test.go @@ -240,7 +240,7 @@ func TestInstall(t *testing.T) { // Install chart with only crds { name: "install chart with only crds", - cmd: "install crd-test testdata/testcharts/chart-with-only-crds --namespace default", + cmd: "install crd-test testdata/testcharts/chart-with-only-crds --namespace default --dry-run", }, // Verify the user/pass works {