fix: return empty list instead of nil to prevent panic. Fixes #25189 …

pkg/cache/cluster.go

@@ -282,7 +282,7 @@
 	}
 }
 
 func (c *clusterCache) getResourceUpdatedHandlers() []OnResourceUpdatedHandler {
 	c.handlersLock.Lock()
 	defer c.handlersLock.Unlock()
 	var handlers []OnResourceUpdatedHandler
@@ -306,7 +306,7 @@
 	}
 }
 
 func (c *clusterCache) getEventHandlers() []OnEventHandler {
 	c.handlersLock.Lock()
 	defer c.handlersLock.Unlock()
 	handlers := make([]OnEventHandler, 0, len(c.eventHandlers))
@@ -330,7 +330,7 @@
 	}
 }
 
 func (c *clusterCache) getProcessEventsHandlers() []OnProcessEventsHandler {
 	c.handlersLock.Lock()
 	defer c.handlersLock.Unlock()
 	handlers := make([]OnProcessEventsHandler, 0, len(c.processEventsHandlers))
@@ -341,7 +341,7 @@
 }
 
 // GetServerVersion returns observed cluster version
 func (c *clusterCache) GetServerVersion() string {
 	return c.serverVersion
 }
 
@@ -355,13 +355,13 @@
 }
 
 // GetOpenAPISchema returns open API schema of supported API resources
 func (c *clusterCache) GetOpenAPISchema() openapi.Resources {
 	return c.openAPISchema
 }
 
 // GetGVKParser returns a parser able to build a TypedValue used in
 // structured merge diffs.
 func (c *clusterCache) GetGVKParser() *managedfields.GvkParser {
 	return c.gvkParser
 }
 
@@ -578,7 +578,7 @@
 
 // listResources creates list pager and enforces number of concurrent list requests
 // The callback should not wait on any locks that may be held by other callers.
 func (c *clusterCache) listResources(ctx context.Context, resClient dynamic.ResourceInterface, callback func(*pager.ListPager) error) (string, error) {
 	if err := c.listSemaphore.Acquire(ctx, 1); err != nil {
 		return "", fmt.Errorf("failed to acquire list semaphore: %w", err)
 	}
@@ -606,6 +606,10 @@
 					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
 			}
@@ -630,7 +634,7 @@
 		return listPager.EachListItem(ctx, metav1.ListOptions{}, func(obj runtime.Object) error {
 			if un, ok := obj.(*unstructured.Unstructured); !ok {
 				return fmt.Errorf("object %s/%s has an unexpected type", un.GroupVersionKind().String(), un.GetName())
 			} else {
 				items = append(items, c.newResource(un))
 			}
 			return nil
@@ -938,7 +942,7 @@
 				return listPager.EachListItem(context.Background(), metav1.ListOptions{}, func(obj runtime.Object) error {
 					if un, ok := obj.(*unstructured.Unstructured); !ok {
 						return fmt.Errorf("object %s/%s has an unexpected type", un.GroupVersionKind().String(), un.GetName())
 					} else {
 						newRes := c.newResource(un)
 						lock.Lock()
 						c.setNode(newRes)
@@ -1335,7 +1339,7 @@
 	}
 }
 
 func (c *clusterCache) onNodeUpdated(oldRes *Resource, newRes *Resource) {
 	c.setNode(newRes)
 	for _, h := range c.getResourceUpdatedHandlers() {
 		h(newRes, oldRes, c.nsIndex[newRes.Ref.Namespace])
@@ -1372,7 +1376,7 @@
 }
 
 // GetClusterInfo returns cluster cache statistics
 func (c *clusterCache) GetClusterInfo() ClusterInfo {
 	c.lock.RLock()
 	defer c.lock.RUnlock()
 	c.syncStatus.lock.Lock()

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()")
+}