Skip to content

Commit 9e209ac

Browse files
committed
feat(controller): disable cache CLI flag
In previous commits a divergence between the kube-builder markers to define permissions and final Helm chart RBAC files have been detected. And tests in place did not detect this issue. The reason we hadn't spotted this before is that the missing get verb was being masked because list/watch populates the controller-runtime client cache. Therefore, Get() calls never reach the API server and RBAC is never evaluated for that verb. Other verbs (create, update, delete, patch) always hit the API server and would be caught. list/watch would crash the controller since the informer cache can't start. To allow e2e tests to spot this a new flag is added to the controller to configure the Kubernetes client to disable cache. Therefore, all the request goes directly to the control plane validationg the RBAC rules. Signed-off-by: José Guilherme Vanz <jguilhermevanz@suse.com> Assisted-by: Github Copilot
1 parent fbd42b8 commit 9e209ac

6 files changed

Lines changed: 89 additions & 46 deletions

File tree

charts/kubewarden-controller/templates/deployment.yaml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,9 @@ spec:
5555
- --always-accept-admission-reviews-on-deployments-namespace
5656
{{- end }}
5757
- --zap-log-level={{ .Values.logLevel }}
58+
{{- if .Values.disableClientCache }}
59+
- --disable-client-cache
60+
{{- end }}
5861
{{- if .Values.mTLS.enable }}
5962
- --client-ca-configmap-name={{ .Values.mTLS.configMapName }}
6063
{{- end }}

charts/kubewarden-controller/values.yaml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,11 @@ preDeleteHook:
153153
alwaysAcceptAdmissionReviewsOnDeploymentsNamespace: true
154154
# Verbosity of logging. Can be one of 'debug', 'info', 'error'.
155155
logLevel: info
156+
# Disable the controller-runtime client cache so that all calls go directly to
157+
# the API server. This is used on tests to ensures RBAC is fully evaluated for
158+
# every read operation, including the "get" verb which would otherwise be
159+
# silently served from cache. Should only be enabled in test environments.
160+
disableClientCache: false
156161
# open-telemetry options
157162
telemetry:
158163
# Kubewarden controller telemetry configuration allow two OpenTelemetry

cmd/controller/main.go

Lines changed: 27 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -29,18 +29,15 @@ import (
2929
// to ensure that exec-entrypoint and run can make use of them.
3030
_ "k8s.io/client-go/plugin/pkg/client/auth"
3131

32-
appsv1 "k8s.io/api/apps/v1"
33-
corev1 "k8s.io/api/core/v1"
34-
k8spoliciesv1 "k8s.io/api/policy/v1"
3532
metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
3633
"sigs.k8s.io/controller-runtime/pkg/webhook"
3734

38-
"k8s.io/apimachinery/pkg/fields"
35+
corev1 "k8s.io/api/core/v1"
3936
"k8s.io/apimachinery/pkg/runtime"
4037
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
4138
clientgoscheme "k8s.io/client-go/kubernetes/scheme"
39+
"k8s.io/client-go/rest"
4240
ctrl "sigs.k8s.io/controller-runtime"
43-
"sigs.k8s.io/controller-runtime/pkg/cache"
4441
"sigs.k8s.io/controller-runtime/pkg/client"
4542
"sigs.k8s.io/controller-runtime/pkg/healthz"
4643
"sigs.k8s.io/controller-runtime/pkg/log/zap"
@@ -74,6 +71,13 @@ type ManagerOptions struct {
7471
EnableMutualTLS bool
7572
MetricsAddr string
7673
ProbeAddr string
74+
// DisableClientCache forces all calls to bypass the controller-runtime
75+
// informer cache and go directly to the API server. This ensures RBAC is
76+
// evaluated for every read operation, including the "get" verb which would
77+
// otherwise be silently served from cache even if the permission is missing.
78+
// Should only be enabled in test/debug environments as it increases API
79+
// server load.
80+
DisableClientCache bool
7781
}
7882

7983
type Configuration struct {
@@ -135,6 +139,8 @@ func main() {
135139
"image-pull-secrets",
136140
"",
137141
"Comma-separated list of Secret names to use as imagePullSecrets on every policy-server Deployment. The secrets must exist in the deployments namespace.")
142+
flag.BoolVar(&mgrOpts.DisableClientCache, "disable-client-cache", false,
143+
"Disable the controller-runtime client cache for all resource types. Should only be used in test environments.")
138144

139145
opts := zap.Options{}
140146
opts.BindFlags(flag.CommandLine)
@@ -218,10 +224,6 @@ func main() {
218224
}
219225

220226
func setupManager(mgrOpts ManagerOptions) (ctrl.Manager, error) {
221-
namespaceSelector := cache.ByObject{
222-
Field: fields.ParseSelectorOrDie("metadata.namespace=" + mgrOpts.DeploymentsNamespace),
223-
}
224-
225227
clientCAName := ""
226228
if mgrOpts.EnableMutualTLS {
227229
// The WebhookServer shares the same CertDir for both the server
@@ -231,52 +233,32 @@ func setupManager(mgrOpts ManagerOptions) (ctrl.Manager, error) {
231233
clientCAName = filepath.Join("client-ca", constants.ClientCACert)
232234
}
233235

234-
mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{
236+
mgrOptions := ctrl.Options{
235237
Scheme: scheme,
236238
Metrics: metricsserver.Options{
237239
BindAddress: mgrOpts.MetricsAddr,
238240
},
239241
HealthProbeBindAddress: mgrOpts.ProbeAddr,
240242
LeaderElection: mgrOpts.EnableLeaderElection,
241243
LeaderElectionID: "a4ddbf36.kubewarden.io",
242-
// Warning: the manager creates a client, which then uses Watches to monitor
243-
// certain resources. By default, the client is not going to be namespaced,
244-
// it will be able to watch resources across the entire cluster. This is of
245-
// course constrained by the RBAC rules applied to the ServiceAccount that
246-
// runs the controller.
247-
// *However*, even when accessing a resource inside a specific Namespace,
248-
// the default behavior of the cache is to create a Watch that is not namespaced;
249-
// hence requires the privilege to access all the resources of that type inside
250-
// of the cluster. That can cause runtime error if the ServiceAccount lacking
251-
// this privilege.
252-
// For example, when we access a secret inside the `kubewarden`
253-
// namespace, the cache will create a Watch against Secrets, that will require
254-
// privileged to access ALL the secrets of the cluster.
255-
//
256-
// To be able to have stricter RBAC rules, we need to instruct the cache to
257-
// only watch objects inside of the namespace where the controller is running.
258-
// That applies ONLY to the namespaced resources that we know the controller
259-
// is going to own inside of a specific namespace.
260-
// For example, Secret resources are going to be defined by the controller
261-
// only inside of the `kubewarden` namespace; hence their watch can be namespaced.
262-
// On the other hand, AdmissionPolicy resources are namespaced, but the controller
263-
// requires to access them across all the namespaces of the cluster; hence the
264-
// cache must not be namespaced.
265-
Cache: cache.Options{
266-
ByObject: map[client.Object]cache.ByObject{
267-
&appsv1.ReplicaSet{}: namespaceSelector,
268-
&corev1.Secret{}: namespaceSelector,
269-
&corev1.Pod{}: namespaceSelector,
270-
&corev1.Service{}: namespaceSelector,
271-
&k8spoliciesv1.PodDisruptionBudget{}: namespaceSelector,
272-
&corev1.ConfigMap{}: namespaceSelector,
273-
&appsv1.Deployment{}: namespaceSelector,
274-
},
275-
},
244+
Cache: controller.NamespacedCacheOptions(mgrOpts.DeploymentsNamespace),
276245
WebhookServer: webhook.NewServer(webhook.Options{
277246
ClientCAName: clientCAName,
278247
}),
279-
})
248+
}
249+
250+
if mgrOpts.DisableClientCache {
251+
// Override the client so that every call goes directly to the API
252+
// server, bypassing the informer cache. This ensures RBAC is fully evaluated
253+
// for all verbs including "get", which would otherwise be silently served from
254+
// the cache even if the permission is missing in the production RBAC roles.
255+
mgrOptions.NewClient = func(config *rest.Config, options client.Options) (client.Client, error) {
256+
options.Cache = nil
257+
return client.New(config, options)
258+
}
259+
}
260+
261+
mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), mgrOptions)
280262
if err != nil {
281263
return mgr, fmt.Errorf("failed to setup manager: %w", err)
282264
}

e2e/main_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ func TestMain(m *testing.M) {
6969
"--set", "auditScanner.image.tag=dev",
7070
"--set", "logLevel=debug",
7171
"--set", "auditScanner.logLevel=debug",
72+
"--set", "disableClientCache=true",
7273
),
7374
)
7475
if err != nil {
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
/*
2+
Copyright 2026.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package controller
18+
19+
import (
20+
appsv1 "k8s.io/api/apps/v1"
21+
corev1 "k8s.io/api/core/v1"
22+
k8spoliciesv1 "k8s.io/api/policy/v1"
23+
"k8s.io/apimachinery/pkg/fields"
24+
"sigs.k8s.io/controller-runtime/pkg/cache"
25+
"sigs.k8s.io/controller-runtime/pkg/client"
26+
)
27+
28+
// NamespacedCacheOptions returns cache.Options that restrict LIST/WATCH for
29+
// namespace-scoped resources the controller manages exclusively within
30+
// deploymentsNamespace. Without this restriction, controller-runtime creates
31+
// cluster-wide watches, which require privileges the production ServiceAccount
32+
// does not have.
33+
//
34+
// Only resources the controller owns within a single namespace are listed here.
35+
// Resources like AdmissionPolicy that the controller accesses across all
36+
// namespaces must NOT be restricted, so they are intentionally omitted.
37+
func NamespacedCacheOptions(deploymentsNamespace string) cache.Options {
38+
namespaceSelector := cache.ByObject{
39+
Field: fields.ParseSelectorOrDie("metadata.namespace=" + deploymentsNamespace),
40+
}
41+
return cache.Options{
42+
ByObject: map[client.Object]cache.ByObject{
43+
&appsv1.ReplicaSet{}: namespaceSelector,
44+
&corev1.Secret{}: namespaceSelector,
45+
&corev1.Pod{}: namespaceSelector,
46+
&corev1.Service{}: namespaceSelector,
47+
&k8spoliciesv1.PodDisruptionBudget{}: namespaceSelector,
48+
&corev1.ConfigMap{}: namespaceSelector,
49+
&appsv1.Deployment{}: namespaceSelector,
50+
},
51+
}
52+
}

internal/controller/suite_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
Copyright 2021.
2+
Copyright 2026.
33
44
Licensed under the Apache License, Version 2.0 (the "License");
55
you may not use this file except in compliance with the License.

0 commit comments

Comments
 (0)