fix(action): crd resources can be empty by manute · Pull Request #31578 · helm/helm

manute · 2025-11-26T15:23:01Z

What this PR does / why we need it:

It checks if the resources are empty to see if this issue #31552 is fixed
Special notes for your reviewer:

If applicable:

this PR contains user facing changes (the docs needed label should be applied if so)
this PR contains unit tests
this PR has been tested for backwards compatibility

benoittgt · 2025-11-26T15:51:05Z

Thanks for the patch. Could you add a test that reproduces the issue in #31552 to validate the fix and prevent future regressions?

manute · 2025-11-26T17:34:57Z

Done @benoittgt , thanks!

banjoh

Changes look OK

TerryHowe

Since you added the error check, can you add a test for nil File?

manute · 2025-11-26T19:00:18Z

@TerryHowe It is already there https://github.com/helm/helm/pull/31578/files#diff-b97a6f3723149f75e7945127b215944ea6a752bcf41a2e1c00b65ae0a409bc7cR1081 or do you refer to other nil File test case?

TerryHowe · 2025-11-27T13:52:00Z

@TerryHowe It is already there https://github.com/helm/helm/pull/31578/files#diff-b97a6f3723149f75e7945127b215944ea6a752bcf41a2e1c00b65ae0a409bc7cR1081 or do you refer to other nil File test case?

Thanks! Sorry I missed that.

TerryHowe

/lgtm

robertsirc · 2025-12-17T20:28:00Z

Hi @manute first thank you for the PR. I pulled it down built it and ran the test and I got the following:

=== RUN TestInstall/install_chart_with_only_crds helpers_test.go:63: running cmd (attempt 1): install crd-test testdata/testcharts/chart-with-only-crds --namespace default helpers_test.go:69: expected no error, got: 'INSTALLATION FAILED: failed to install CRD crds/test-crd.yaml: resources are empty' --- FAIL: TestInstall/install_chart_with_only_crds (0.00s)

manute · 2025-12-22T18:16:20Z

@robertsirc fixed! thanks

Signed-off-by: Manuel Alonso <[email protected]>

TerryHowe

/lgtm

banjoh · 2026-01-08T18:15:20Z

@manute do you mind resolving the current conflicts?

Signed-off-by: Manuel Alonso <[email protected]>

TerryHowe · 2026-01-11T13:25:14Z

Broken tests.

--- FAIL: TestInstalLCRDs (0.00s)
    install_test.go:1100: 
        	Error Trace:	/home/runner/work/helm/helm/pkg/action/install_test.go:1100
        	Error:      	Received unexpected error:
        	            	failed to install CRD crds/foo.yaml: resources are empty
        	Test:       	TestInstalLCRDs
--- FAIL: TestInstalLCRDs_AlreadyExist (0.00s)
    install_test.go:1155: 
        	Error Trace:	/home/runner/work/helm/helm/pkg/action/install_test.go:1155
        	Error:      	Expected nil, but got: &errors.errorString{s:"failed to install CRD crds/foo.yaml: resources are empty"}
        	Test:       	TestInstalLCRDs_AlreadyExist
FAIL

Signed-off-by: Manuel Alonso <[email protected]>

manute · 2026-01-23T07:19:44Z

@TerryHowe fixed, thanks

Signed-off-by: Manuel Alonso <[email protected]>

TerryHowe

/lgtm

Signed-off-by: superShen0916 <[email protected]> Backport the fix from helm#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 helm#31552 Co-Authored-By: Claude Opus 4.6 <[email protected]> Signed-off-by: superShen0916 <[email protected]>

Signed-off-by: shenpeng.sp0916 <[email protected]> Backport the fix from helm#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 helm#31552 Co-Authored-By: Claude Opus 4.6 <[email protected]> Signed-off-by: shenpeng.sp0916 <[email protected]>

pull-request-size Bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Nov 26, 2025

manute mentioned this pull request Nov 26, 2025

Helm 3.19 crashes with a nil pointer dereference #31552

Open

manute force-pushed the main branch from 5bc1de6 to f73554c Compare November 26, 2025 15:25

pull-request-size Bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Nov 26, 2025

banjoh previously approved these changes Nov 26, 2025

View reviewed changes

TerryHowe reviewed Nov 26, 2025

View reviewed changes

TerryHowe previously approved these changes Nov 27, 2025

View reviewed changes

maryamsul mentioned this pull request Dec 1, 2025

Fix: Prevent nil pointer dereference bug in otlp_forwarder Helm Chart #31596

Closed

TerryHowe added the Has One Approval This PR has one approval. It still needs a second approval to be merged. label Dec 13, 2025

manute dismissed TerryHowe’s stale review via 69a0a7e December 22, 2025 18:15

manute added 4 commits December 23, 2025 09:36

fix(action): crd resources can be empty

268593b

Signed-off-by: Manuel Alonso <[email protected]>

fix(install): check lenght and file nil, add tests

52235cc

Signed-off-by: Manuel Alonso <[email protected]>

fix(test): no check empty resources

0357e8d

Signed-off-by: Manuel Alonso <[email protected]>

fix(install): add more tests and check nil file data

00f0a48

Signed-off-by: Manuel Alonso <[email protected]>

manute force-pushed the main branch from 35a1c00 to 00f0a48 Compare December 23, 2025 08:36

TerryHowe previously approved these changes Jan 3, 2026

View reviewed changes

manute dismissed banjoh’s stale review via 00f0a48 January 8, 2026 17:09

Merge branch 'main' into main

df8ec77

Signed-off-by: Manuel Alonso <[email protected]>

manute dismissed TerryHowe’s stale review via df8ec77 January 9, 2026 07:32

pull-request-size Bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 9, 2026

banjoh previously approved these changes Jan 11, 2026

View reviewed changes

fix(test): fix tests and check nil for restclient

0f949a9

Signed-off-by: Manuel Alonso <[email protected]>

manute dismissed stale reviews from banjoh and TerryHowe via 0f949a9 January 11, 2026 20:09

pull-request-size Bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 11, 2026

Manuel Alonso and others added 2 commits January 13, 2026 19:21

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

6501ef4

Signed-off-by: Manuel Alonso <[email protected]>

Merge branch 'main' into main

7c089be

Signed-off-by: Manuel Alonso <[email protected]>

joejulian requested changes Feb 6, 2026

View reviewed changes

Comment thread pkg/action/install_test.go Outdated

Comment thread pkg/action/install_test.go Outdated

fix(install): check nil for restClientGetter and fix tests

9817a68

Signed-off-by: Manuel Alonso <[email protected]>

joejulian approved these changes Feb 7, 2026

View reviewed changes

TerryHowe approved these changes Feb 9, 2026

View reviewed changes

joejulian added this to the 4.1.2 milestone Feb 11, 2026

joejulian merged commit 70899a6 into helm:main Feb 11, 2026
5 checks passed

scottrigby modified the milestones: 4.1.2, 4.1.3 Mar 11, 2026

texasich mentioned this pull request May 7, 2026

fix(install): backport nil CRD guard from main to dev-v3 (fixes #31552) #32106

Closed

superShen0916 mentioned this pull request May 9, 2026

fix(action): reject invalid CRD install inputs #32114

Open

rancherbot mentioned this pull request May 14, 2026

rddepman: bump helm from 4.1.4 to 4.2.0 rancher-sandbox/rancher-desktop#10285

Merged

scottrigby mentioned this pull request Jun 12, 2026

[v3 backport] fix(action): guard nil RESTClientGetter in CRD install path #31920

Merged

Uh oh!

Conversation

manute commented Nov 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

benoittgt commented Nov 26, 2025

Uh oh!

manute commented Nov 26, 2025

Uh oh!

banjoh left a comment

Choose a reason for hiding this comment

Uh oh!

TerryHowe left a comment

Choose a reason for hiding this comment

Uh oh!

manute commented Nov 26, 2025

Uh oh!

TerryHowe commented Nov 27, 2025

Uh oh!

TerryHowe left a comment

Choose a reason for hiding this comment

Uh oh!

robertsirc commented Dec 17, 2025

Uh oh!

manute commented Dec 22, 2025

Uh oh!

TerryHowe left a comment

Choose a reason for hiding this comment

Uh oh!

banjoh commented Jan 8, 2026

Uh oh!

TerryHowe commented Jan 11, 2026

Uh oh!

manute commented Jan 23, 2026

Uh oh!

Uh oh!

Uh oh!

TerryHowe left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

manute commented Nov 26, 2025 •

edited

Loading