Skip to content

Commit a84d752

Browse files
chrissetochrisseto
committed
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. (cherry picked from commit 9e08d14) # Conflicts: # charts/redpanda/chart_test.go # go.work.sum # operator/internal/controller/vectorized/cluster_controller_test.go # operator/internal/lifecycle/client.go # pkg/kube/ctl.go # pkg/kube/syncer.go # pkg/kube/syncer_test.go
1 parent 3c5ad97 commit a84d752

File tree

10 files changed

+21
-88
lines changed

10 files changed

+21
-88
lines changed

‎charts/console/client_test.go‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,7 @@ func NewClient(ctx context.Context, kubeCtl *kube.Ctl, dot *helmette.Dot) (*Clie
7272
}
7373

7474
func (c *Client) getConsolePod(ctx context.Context) (*corev1.Pod, error) {
75-
deploys, err := kube.List[appsv1.DeploymentList](ctx, c.Ctl,
76-
k8sclient.InNamespace(c.Release.Namespace),
77-
)
75+
deploys, err := kube.List[appsv1.DeploymentList](ctx, c.Ctl, c.Release.Namespace)
7876
if err != nil {
7977
return nil, err
8078
}
@@ -92,7 +90,7 @@ func (c *Client) getConsolePod(ctx context.Context) (*corev1.Pod, error) {
9290
}
9391

9492
pods, err := kube.List[corev1.PodList](ctx, c.Ctl,
95-
k8sclient.InNamespace(deployment.Namespace),
93+
deployment.Namespace,
9694
k8sclient.MatchingLabels(deployment.Spec.Selector.MatchLabels))
9795
if err != nil {
9896
return nil, err

‎charts/redpanda/chart_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ func TestIntegrationChart(t *testing.T) {
165165
}),
166166
})
167167

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

433-
pods, err := kube.List[corev1.PodList](ctx, env.Ctl(), client.MatchingLabels{
433+
pods, err := kube.List[corev1.PodList](ctx, env.Ctl(), release.Namespace, client.MatchingLabels{
434434
"app.kubernetes.io/instance": release.Name,
435435
"app.kubernetes.io/component": release.Chart + "-statefulset",
436436
})

‎charts/redpanda/client/client.go‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ import (
2828
"github.com/twmb/franz-go/pkg/sasl/scram"
2929
"github.com/twmb/franz-go/pkg/sr"
3030
corev1 "k8s.io/api/core/v1"
31-
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3231
"sigs.k8s.io/controller-runtime/pkg/client"
3332

3433
"github.com/redpanda-data/redpanda-operator/charts/redpanda/v25"
@@ -496,9 +495,9 @@ func srvLookup(dot *helmette.Dot, dialer DialContextFunc, service string) ([]*ne
496495
// Querying for k8s-app=kube-dns is a generally accepted / safe
497496
// way of finding the kube DNS. We could alternatively find the
498497
// kube-dns service and use its label selector.
499-
pods, err := kube.List[corev1.PodList](ctx, ctl, client.MatchingLabels{
498+
pods, err := kube.List[corev1.PodList](ctx, ctl, kube.NamespaceSystem, client.MatchingLabels{
500499
"k8s-app": "kube-dns",
501-
}, client.InNamespace(metav1.NamespaceSystem))
500+
})
502501
if err != nil {
503502
return nil, err
504503
}

‎gen/go.mod‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -216,10 +216,7 @@ require (
216216
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.59.0 // indirect
217217
go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.59.0 // indirect
218218
go.opentelemetry.io/otel v1.36.0 // indirect
219-
go.opentelemetry.io/otel/log v0.11.0 // indirect
220219
go.opentelemetry.io/otel/metric v1.36.0 // indirect
221-
go.opentelemetry.io/otel/sdk v1.36.0 // indirect
222-
go.opentelemetry.io/otel/sdk/log v0.11.0 // indirect
223220
go.opentelemetry.io/otel/trace v1.36.0 // indirect
224221
go.uber.org/multierr v1.11.0 // indirect
225222
go.uber.org/zap v1.27.0 // indirect

‎go.work.sum‎

Lines changed: 0 additions & 62 deletions
Large diffs are not rendered by default.

‎harpoon/go.mod‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,6 @@ require (
4747
github.com/go-errors/errors v1.5.1 // indirect
4848
github.com/go-gorp/gorp/v3 v3.1.0 // indirect
4949
github.com/go-logr/logr v1.4.2 // indirect
50-
github.com/go-logr/stdr v1.2.2 // indirect
5150
github.com/go-openapi/jsonpointer v0.21.1 // indirect
5251
github.com/go-openapi/jsonreference v0.21.0 // indirect
5352
github.com/go-openapi/swag v0.23.1 // indirect
@@ -126,13 +125,6 @@ require (
126125
github.com/virtuald/go-ordered-json v0.0.0-20170621173500-b18e6e673d74 // indirect
127126
github.com/x448/float16 v0.8.4 // indirect
128127
github.com/xlab/treeprint v1.2.0 // indirect
129-
go.opentelemetry.io/auto/sdk v1.1.0 // indirect
130-
go.opentelemetry.io/otel v1.36.0 // indirect
131-
go.opentelemetry.io/otel/log v0.11.0 // indirect
132-
go.opentelemetry.io/otel/metric v1.36.0 // indirect
133-
go.opentelemetry.io/otel/sdk v1.36.0 // indirect
134-
go.opentelemetry.io/otel/sdk/log v0.11.0 // indirect
135-
go.opentelemetry.io/otel/trace v1.36.0 // indirect
136128
go.yaml.in/yaml/v2 v2.4.2 // indirect
137129
go.yaml.in/yaml/v3 v3.0.3 // indirect
138130
golang.org/x/crypto v0.40.0 // indirect

‎harpoon/go.sum‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,6 @@ github.com/go-errors/errors v1.5.1 h1:ZwEMSLRCapFLflTpT7NKaAc7ukJ8ZPEjzlxt8rPN8b
126126
github.com/go-errors/errors v1.5.1/go.mod h1:sIVyrIiJhuEF+Pj9Ebtd6P/rEYROXFi3BopGUQ5a5Og=
127127
github.com/go-gorp/gorp/v3 v3.1.0 h1:ItKF/Vbuj31dmV4jxA1qblpSwkl9g1typ24xoe70IGs=
128128
github.com/go-gorp/gorp/v3 v3.1.0/go.mod h1:dLEjIyyRNiXvNZ8PSmzpt1GsWAUK8kjVhEpjH8TixEw=
129-
github.com/go-logr/logr v1.2.2/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A=
130129
github.com/go-logr/logr v1.4.2 h1:6pFjapn8bFcIbiKo3XT4j/BhANplGihG6tvd+8rYgrY=
131130
github.com/go-logr/logr v1.4.2/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY=
132131
github.com/go-logr/stdr v1.2.2 h1:hSWxHoqTgW2S2qGc0LTAI563KZ5YKYRhT3MFKZMbjag=

‎pkg/kube/ctl.go‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"github.com/cockroachdb/errors"
2020
corev1 "k8s.io/api/core/v1"
2121
k8serrors "k8s.io/apimachinery/pkg/api/errors"
22+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2223
"k8s.io/apimachinery/pkg/runtime"
2324
"k8s.io/apimachinery/pkg/runtime/schema"
2425
"k8s.io/client-go/kubernetes/scheme"
@@ -36,8 +37,11 @@ type (
3637
Object = client.Object
3738
ObjectList = client.ObjectList
3839
ObjectKey = client.ObjectKey
40+
)
3941

40-
InNamespace = client.InNamespace
42+
const (
43+
NamespaceAll = metav1.NamespaceAll
44+
NamespaceSystem = metav1.NamespaceSystem
4145
)
4246

4347
type Option interface {
@@ -150,11 +154,17 @@ func (c *Ctl) GetAndWait(ctx context.Context, key ObjectKey, obj Object, cond Co
150154
}
151155

152156
// List fetches a list of objects into `objs` from Kubernetes.
157+
//
158+
// Cluster scoped resources should pass `""` as namespace.
159+
//
153160
// Usage:
154161
//
155162
// var pods corev1.PodList
156163
// ctl.List(ctx, &pods)
157-
func (c *Ctl) List(ctx context.Context, objs client.ObjectList, opts ...client.ListOption) error {
164+
func (c *Ctl) List(ctx context.Context, namespace string, objs ObjectList, opts ...client.ListOption) error {
165+
// Top level namespace parameter takes precedence over anything specified
166+
// in opts. The other way around is less straightforward.
167+
opts = append(opts, client.InNamespace(namespace))
158168
if err := c.client.List(ctx, objs, opts...); err != nil {
159169
return errors.WithStack(err)
160170
}

‎pkg/kube/ctl_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ func TestCtl(t *testing.T) {
5050
))
5151
require.Equal(t, []string{"cm-0", "cm-1", "cm-2"}, seen)
5252

53-
cms, err := kube.List[corev1.ConfigMapList](ctx, ctl, kube.InNamespace("hello-world"))
53+
cms, err := kube.List[corev1.ConfigMapList](ctx, ctl, "hello-world")
5454
require.NoError(t, err)
5555
require.Len(t, cms.Items, 3)
5656
}

‎pkg/kube/generics.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,9 +55,9 @@ func ApplyAndWait[T any, PT AddrOfObject[T]](ctx context.Context, ctl *Ctl, obj
5555
}
5656

5757
// List is a generic equivalent of [Ctl.List].
58-
func List[T any, L AddrOfObjectList[T]](ctx context.Context, ctl *Ctl, opts ...client.ListOption) (*T, error) {
58+
func List[T any, L AddrOfObjectList[T]](ctx context.Context, ctl *Ctl, namespace string, opts ...client.ListOption) (*T, error) {
5959
var list T
60-
if err := ctl.List(ctx, L(&list), opts...); err != nil {
60+
if err := ctl.List(ctx, namespace, L(&list), opts...); err != nil {
6161
return nil, err
6262
}
6363
return &list, nil

0 commit comments

Comments
 (0)