From 7d60f23a87ee6f9b2fefd88d18fd9c1d4f8ed36a Mon Sep 17 00:00:00 2001 From: "shenpeng.sp0916" Date: Sun, 10 May 2026 01:21:57 +0800 Subject: [PATCH] fix(action): prevent nil pointer dereference in installCRDs Signed-off-by: shenpeng.sp0916 Backport the fix from #31578 to dev-v3 branch. The installCRDs function previously could panic in several scenarios: - When a CRD object has a nil File field (accessing obj.File.Data) - When a CRD object has nil File.Data (passing nil to KubeClient.Build) - When KubeClient.Build returns an empty resource list (accessing res[0]) - When RESTClientGetter is nil (calling ToDiscoveryClient/ToRESTMapper) Added nil and empty checks to return descriptive errors instead of panicking in all these cases. Closes #31552 Co-Authored-By: Claude Opus 4.6 Signed-off-by: shenpeng.sp0916 --- pkg/action/install.go | 50 ++++++++++++++++++++++++-------------- pkg/action/install_test.go | 39 +++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 18 deletions(-) diff --git a/pkg/action/install.go b/pkg/action/install.go index f8f740005..a4c51e6b4 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -162,12 +162,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 errors.Wrapf(err, "failed to install CRD %s", obj.Name) } + if len(res) == 0 { + return fmt.Errorf("failed to install CRD %s: no Kubernetes resources parsed from CRD data", obj.Name) + } + // Send them to Kube if _, err := i.cfg.KubeClient.Create(res); err != nil { // If the error is CRD already exists, continue. @@ -191,27 +203,29 @@ 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 err != nil { - return err - } + if i.cfg.RESTClientGetter != nil { + if i.cfg.Capabilities != nil { + discoveryClient, err := i.cfg.RESTClientGetter.ToDiscoveryClient() + if err != nil { + return err + } - i.cfg.Log("Clearing discovery cache") - discoveryClient.Invalidate() + i.cfg.Log("Clearing discovery cache") + discoveryClient.Invalidate() - _, _ = discoveryClient.ServerGroups() - } + _, _ = 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() + // 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 diff --git a/pkg/action/install_test.go b/pkg/action/install_test.go index 300091eac..74074e9c2 100644 --- a/pkg/action/install_test.go +++ b/pkg/action/install_test.go @@ -923,3 +923,42 @@ func TestInstallWithSystemLabels(t *testing.T) { is.Equal(fmt.Errorf("user supplied labels contains system reserved label name. System labels: %+v", driver.GetSystemLabels()), err) } + +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: &chart.File{Name: "crds/foo.yaml", Data: nil}}, + }, + }, + { + name: "at least a crd with its file data nil", + input: []chart.CRD{ + {Name: "one", File: &chart.File{Name: "crds/foo.yaml", Data: []byte("data")}}, + {Name: "two", File: &chart.File{Name: "crds/foo2.yaml", Data: nil}}, + {Name: "three", File: &chart.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") + } + }) + } +}