From 7d694cd4d1ebbc6f6359129137f508566db321f3 Mon Sep 17 00:00:00 2001 From: Ivan Pedersen Date: Mon, 24 Nov 2025 16:44:15 +0100 Subject: [PATCH] fix: return empty list instead of nil to prevent panic. Fixes #25189 (#25192) Signed-off-by: Ivan Pedersen --- pkg/cache/cluster.go | 4 ++ pkg/cache/cluster_nil_fix_test.go | 109 ++++++++++++++++++++++++++++++ 2 files changed, 113 insertions(+) create mode 100644 pkg/cache/cluster_nil_fix_test.go diff --git a/pkg/cache/cluster.go b/pkg/cache/cluster.go index 38f1e6016..a5ba2e794 100644 --- a/pkg/cache/cluster.go +++ b/pkg/cache/cluster.go @@ -606,6 +606,10 @@ func (c *clusterCache) listResources(ctx context.Context, resClient dynamic.Reso retryCount++ c.log.Info(fmt.Sprintf("Error while listing resources: %v (try %d/%d)", ierr, retryCount, c.listRetryLimit)) } + // Ensure res is never nil even when there's an error + if res == nil { + res = &unstructured.UnstructuredList{} + } //nolint:wrapcheck // wrap outside the retry return ierr } diff --git a/pkg/cache/cluster_nil_fix_test.go b/pkg/cache/cluster_nil_fix_test.go new file mode 100644 index 000000000..d652cc842 --- /dev/null +++ b/pkg/cache/cluster_nil_fix_test.go @@ -0,0 +1,109 @@ +package cache + +import ( + "context" + "errors" + "testing" + + "golang.org/x/sync/semaphore" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/watch" + "k8s.io/client-go/dynamic" + "k8s.io/client-go/tools/pager" + "k8s.io/klog/v2/textlogger" +) + +// mockResourceInterface simulates a dynamic resource interface that returns (nil, error) +type mockResourceInterface struct{} + +func (m *mockResourceInterface) Create(ctx context.Context, obj *unstructured.Unstructured, options metav1.CreateOptions, subresources ...string) (*unstructured.Unstructured, error) { + return nil, errors.New("not implemented") +} + +func (m *mockResourceInterface) Update(ctx context.Context, obj *unstructured.Unstructured, options metav1.UpdateOptions, subresources ...string) (*unstructured.Unstructured, error) { + return nil, errors.New("not implemented") +} + +func (m *mockResourceInterface) UpdateStatus(ctx context.Context, obj *unstructured.Unstructured, options metav1.UpdateOptions) (*unstructured.Unstructured, error) { + return nil, errors.New("not implemented") +} + +func (m *mockResourceInterface) Delete(ctx context.Context, name string, options metav1.DeleteOptions, subresources ...string) error { + return errors.New("not implemented") +} + +func (m *mockResourceInterface) DeleteCollection(ctx context.Context, options metav1.DeleteOptions, listOptions metav1.ListOptions) error { + return errors.New("not implemented") +} + +func (m *mockResourceInterface) Get(ctx context.Context, name string, options metav1.GetOptions, subresources ...string) (*unstructured.Unstructured, error) { + return nil, errors.New("not implemented") +} + +func (m *mockResourceInterface) List(ctx context.Context, opts metav1.ListOptions) (*unstructured.UnstructuredList, error) { + // This simulates the problematic behavior that caused the original panic: + // returning (nil, error) instead of (emptyList, error) + return nil, errors.New("simulated list failure") +} + +func (m *mockResourceInterface) Watch(ctx context.Context, opts metav1.ListOptions) (watch.Interface, error) { + return nil, errors.New("not implemented") +} + +func (m *mockResourceInterface) Patch(ctx context.Context, name string, pt types.PatchType, data []byte, options metav1.PatchOptions, subresources ...string) (*unstructured.Unstructured, error) { + return nil, errors.New("not implemented") +} + +func (m *mockResourceInterface) Apply(ctx context.Context, name string, obj *unstructured.Unstructured, options metav1.ApplyOptions, subresources ...string) (*unstructured.Unstructured, error) { + return nil, errors.New("not implemented") +} + +func (m *mockResourceInterface) ApplyStatus(ctx context.Context, name string, obj *unstructured.Unstructured, options metav1.ApplyOptions) (*unstructured.Unstructured, error) { + return nil, errors.New("not implemented") +} + +func (m *mockResourceInterface) Namespace(string) dynamic.ResourceInterface { + return m +} + +// TestListResourcesNilPointerFix tests that our fix prevents panic when +// resClient.List() returns (nil, error) +func TestListResourcesNilPointerFix(t *testing.T) { + // Create a cluster cache with proper configuration + cache := &clusterCache{ + listSemaphore: semaphore.NewWeighted(1), + listPageSize: 100, + listPageBufferSize: 1, + listRetryLimit: 1, + listRetryUseBackoff: false, + listRetryFunc: ListRetryFuncNever, + log: textlogger.NewLogger(textlogger.NewConfig()), + } + + // Use our mock that returns (nil, error) + mockClient := &mockResourceInterface{} + + // This should not panic even though mockClient.List() returns (nil, error) + _, err := cache.listResources(context.Background(), mockClient, func(listPager *pager.ListPager) error { + // The fix ensures the pager receives a non-nil UnstructuredList + // preventing panic in GetContinue() + return listPager.EachListItem(context.Background(), metav1.ListOptions{}, func(obj runtime.Object) error { + return nil + }) + }) + + // We expect an error (wrapped), but no panic should occur + if err == nil { + t.Fatal("Expected error from listResources due to simulated failure") + } + + // Check that the error is properly wrapped + if err.Error() != "failed to list resources: simulated list failure" { + t.Fatalf("Unexpected error message: %v", err.Error()) + } + + t.Log("Test passed: no panic occurred despite (nil, error) from resClient.List()") +}