pkg/kube: require namespace in Ctl.List

`client.Client` and `kube.Ctl` treated `namespace` (`client.InNamespace`) as an
optional parameter. If not provided, it default to `"default"`.

This parameter has been missed multiple times across various places in our
codebase and resulted in latent errors, often because tests utilised the
default namespace.

This commit changes the signature of `Ctl.List` to require a `namespace string`
parameter to make it impossible to forget. It additionally adds it to a few
places (lifecycle client) that it was previously forgotten.

Author: chrisseto

charts/console/chart/client_test.go

@@ -72,9 +72,7 @@ func NewClient(ctx context.Context, kubeCtl *kube.Ctl, dot *helmette.Dot) (*Clie
 }
 
 func (c *Client) getConsolePod(ctx context.Context) (*corev1.Pod, error) {
-	deploys, err := kube.List[appsv1.DeploymentList](ctx, c.Ctl,
-		k8sclient.InNamespace(c.Release.Namespace),
-	)
+	deploys, err := kube.List[appsv1.DeploymentList](ctx, c.Ctl, c.Release.Namespace)
 	if err != nil {
 		return nil, err
 	}
@@ -92,7 +90,7 @@ func (c *Client) getConsolePod(ctx context.Context) (*corev1.Pod, error) {
 	}
 
 	pods, err := kube.List[corev1.PodList](ctx, c.Ctl,
-		k8sclient.InNamespace(deployment.Namespace),
+		deployment.Namespace,
 		k8sclient.MatchingLabels(deployment.Spec.Selector.MatchLabels))
 	if err != nil {
 		return nil, err

charts/redpanda/chart_test.go

@@ -168,7 +168,7 @@ func TestIntegrationChart(t *testing.T) {
 			}),
 		})
 
-		pods, err := kube.List[corev1.PodList](ctx, env.Ctl(), client.MatchingLabels{
+		pods, err := kube.List[corev1.PodList](ctx, env.Ctl(), release.Namespace, client.MatchingLabels{
 			"app.kubernetes.io/instance":  release.Name,
 			"app.kubernetes.io/component": release.Chart + "-statefulset",
 		})
@@ -433,7 +433,7 @@ func TestIntegrationChart(t *testing.T) {
 			}),
 		})
 
-		pods, err := kube.List[corev1.PodList](ctx, env.Ctl(), client.MatchingLabels{
+		pods, err := kube.List[corev1.PodList](ctx, env.Ctl(), release.Namespace, client.MatchingLabels{
 			"app.kubernetes.io/instance":  release.Name,
 			"app.kubernetes.io/component": release.Chart + "-statefulset",
 		})
@@ -473,7 +473,7 @@ func TestIntegrationChart(t *testing.T) {
 			}),
 		})
 
-		pods, err := kube.List[corev1.PodList](ctx, env.Ctl(), client.MatchingLabels{
+		pods, err := kube.List[corev1.PodList](ctx, env.Ctl(), release.Namespace, client.MatchingLabels{
 			"app.kubernetes.io/instance": release.Name,
 			"app.kubernetes.io/name":     "console",
 		})

charts/redpanda/client/client.go

@@ -24,7 +24,6 @@ import (
 	"github.com/twmb/franz-go/pkg/sasl/scram"
 	"github.com/twmb/franz-go/pkg/sr"
 	corev1 "k8s.io/api/core/v1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	"github.com/redpanda-data/redpanda-operator/charts/redpanda/v25"
@@ -328,9 +327,9 @@ func srvLookup(state *redpanda.RenderState, dialer DialContextFunc, service stri
 				// Querying for k8s-app=kube-dns is a generally accepted / safe
 				// way of finding the kube DNS. We could alternatively find the
 				// kube-dns service and use its label selector.
-				pods, err := kube.List[corev1.PodList](ctx, ctl, client.MatchingLabels{
+				pods, err := kube.List[corev1.PodList](ctx, ctl, kube.NamespaceSystem, client.MatchingLabels{
 					"k8s-app": "kube-dns",
-				}, client.InNamespace(metav1.NamespaceSystem))
+				})
 				if err != nil {
 					return nil, err
 				}

go.work.sum

@@ -275,7 +275,7 @@ func normalizeClusterResources(ctx context.Context, testScheme *runtime.Scheme,
 
 	clusterSelector := labels.ForCluster(cluster).AsClientSelector()
 	for _, l := range lists {
-		err := ctl.List(ctx, l)
+		err := ctl.List(ctx, cluster.Namespace, l)
 		if errors.Is(err, &meta.NoKindMatchError{}) {
 			continue
 		}

operator/internal/controller/vectorized/cluster_controller_test.go

@@ -259,7 +259,7 @@ func (r *ResourceClient[T, U]) DeleteAll(ctx context.Context, owner U) (bool, er
 
 // fetchExistingPools fetches the existing pools (StatefulSets) for a given cluster.
 func (r *ResourceClient[T, U]) fetchExistingPools(ctx context.Context, cluster U) ([]*poolWithOrdinals, error) {
-	sets, err := kube.List[appsv1.StatefulSetList](ctx, r.ctl, client.InNamespace(cluster.GetNamespace()), client.MatchingLabels(r.ownershipResolver.GetOwnerLabels(cluster)))
+	sets, err := kube.List[appsv1.StatefulSetList](ctx, r.ctl, cluster.GetNamespace(), client.MatchingLabels(r.ownershipResolver.GetOwnerLabels(cluster)))
 	if err != nil {
 		return nil, errors.Wrapf(err, "listing StatefulSets")
 	}
@@ -288,7 +288,7 @@ func (r *ResourceClient[T, U]) fetchExistingPools(ctx context.Context, cluster U
 		}
 
 		// based on https://github.com/kubernetes/kubernetes/blob/c90a4b16b6aa849ed362ee40997327db09e3a62d/pkg/controller/history/controller_history.go#L222
-		revisions, err := kube.List[appsv1.ControllerRevisionList](ctx, r.ctl, client.MatchingLabelsSelector{
+		revisions, err := kube.List[appsv1.ControllerRevisionList](ctx, r.ctl, cluster.GetNamespace(), client.MatchingLabelsSelector{
 			Selector: selector,
 		})
 		if err != nil {
@@ -303,7 +303,7 @@ func (r *ResourceClient[T, U]) fetchExistingPools(ctx context.Context, cluster U
 			}
 		}
 
-		pods, err := kube.List[corev1.PodList](ctx, r.ctl, client.MatchingLabelsSelector{
+		pods, err := kube.List[corev1.PodList](ctx, r.ctl, cluster.GetNamespace(), client.MatchingLabelsSelector{
 			Selector: selector,
 		})
 		if err != nil {

operator/internal/lifecycle/client.go

@@ -20,6 +20,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/kubernetes/scheme"
@@ -37,8 +38,11 @@ type (
 	Object     = client.Object
 	ObjectList = client.ObjectList
 	ObjectKey  = client.ObjectKey
+)
 
-	InNamespace = client.InNamespace
+const (
+	NamespaceAll    = metav1.NamespaceAll
+	NamespaceSystem = metav1.NamespaceSystem
 )
 
 type Option interface {
@@ -164,11 +168,17 @@ func (c *Ctl) GetAndWait(ctx context.Context, key ObjectKey, obj Object, cond Co
 }
 
 // List fetches a list of objects into `objs` from Kubernetes.
+//
+// Cluster scoped resources should pass `""` as namespace.
+//
 // Usage:
 //
 //	var pods corev1.PodList
 //	ctl.List(ctx, &pods)
-func (c *Ctl) List(ctx context.Context, objs ObjectList, opts ...client.ListOption) error {
+func (c *Ctl) List(ctx context.Context, namespace string, objs ObjectList, opts ...client.ListOption) error {
+	// Top level namespace parameter takes precedence over anything specified
+	// in opts. The other way around is less straightforward.
+	opts = append(opts, client.InNamespace(namespace))
 	if err := c.client.List(ctx, objs, opts...); err != nil {
 		return errors.WithStack(err)
 	}

pkg/kube/ctl.go

@@ -50,7 +50,7 @@ func TestCtl(t *testing.T) {
 	))
 	require.Equal(t, []string{"cm-0", "cm-1", "cm-2"}, seen)
 
-	cms, err := kube.List[corev1.ConfigMapList](ctx, ctl, kube.InNamespace("hello-world"))
+	cms, err := kube.List[corev1.ConfigMapList](ctx, ctl, "hello-world")
 	require.NoError(t, err)
 	require.Len(t, cms.Items, 3)
 

pkg/kube/ctl_test.go

@@ -55,9 +55,9 @@ func ApplyAndWait[T any, PT AddrOfObject[T]](ctx context.Context, ctl *Ctl, obj
 }
 
 // List is a generic equivalent of [Ctl.List].
-func List[T any, L AddrOfObjectList[T]](ctx context.Context, ctl *Ctl, opts ...client.ListOption) (*T, error) {
+func List[T any, L AddrOfObjectList[T]](ctx context.Context, ctl *Ctl, namespace string, opts ...client.ListOption) (*T, error) {
 	var list T
-	if err := ctl.List(ctx, L(&list), opts...); err != nil {
+	if err := ctl.List(ctx, namespace, L(&list), opts...); err != nil {
 		return nil, err
 	}
 	return &list, nil

pkg/kube/generics.go

@@ -170,7 +170,7 @@ func (s *Syncer) listInPurview(ctx context.Context) ([]Object, error) {
 			return nil, err
 		}
 
-		if err := s.Ctl.List(ctx, list, client.InNamespace(s.Namespace), client.MatchingLabels(s.OwnershipLabels)); err != nil {
+		if err := s.Ctl.List(ctx, s.Namespace, list, client.MatchingLabels(s.OwnershipLabels)); err != nil {
 			return nil, err
 		}