Skip to content

Commit 4b04642

Browse files
committed
fix(controller): fix e2e failures when running with --disable-client-cache
Two reconciler bugs were exposed when options.Cache is nil: List calls for ReplicaSets and Pods in isPolicyUniquelyReachable had no explicit namespace, becoming cluster-wide requests that the service account is not allowed to make (403). In getPolicies, MatchingFields with a custom field index key is forwarded to the API server as an unsupported CRD field selector (400). Only error check silently swallowed the failure, leaving policy lists empty and policies stuck in pending forever. Fix isPolicyUniquelyReachable by adding client.InNamespace to both List calls. Fix getPolicies by adding a CacheDisabled bool to PolicyServerReconciler: when true, a new getPoliciesWithoutCache method lists all objects and filters by Spec.PolicyServer in Go; when false, the existing MatchingFields path is used unchanged (error check fixed to plain if err != nil). Signed-off-by: José Guilherme Vanz <jguilhermevanz@suse.com> Assisted-by: Github copilot
1 parent 524fa8b commit 4b04642

3 files changed

Lines changed: 81 additions & 20 deletions

File tree

cmd/controller/main.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,7 @@ func main() {
195195
mgrOpts.DeploymentsNamespace,
196196
config,
197197
otelConfiguration,
198+
mgrOpts.DisableClientCache,
198199
); err != nil {
199200
setupLog.Error(err, "unable to create controllers")
200201
retcode = 1
@@ -252,6 +253,8 @@ func setupManager(mgrOpts ManagerOptions) (ctrl.Manager, error) {
252253
// server, bypassing the informer cache. This ensures RBAC is fully evaluated
253254
// for all verbs including "get", which would otherwise be silently served from
254255
// the cache even if the permission is missing in the production RBAC roles.
256+
// Should only be enabled in test/debug environments as it increases API
257+
// server load.
255258
mgrOptions.NewClient = func(config *rest.Config, options client.Options) (client.Client, error) {
256259
options.Cache = nil
257260
return client.New(config, options)
@@ -279,6 +282,7 @@ func setupReconcilers(mgr ctrl.Manager,
279282
deploymentsNamespace string,
280283
config Configuration,
281284
otelConfiguration controller.TelemetryConfiguration,
285+
disableClientCache bool,
282286
) error {
283287
if err := (&controller.PolicyServerReconciler{
284288
Client: mgr.GetClient(),
@@ -289,6 +293,7 @@ func setupReconcilers(mgr ctrl.Manager,
289293
TelemetryConfiguration: otelConfiguration,
290294
ClientCAConfigMapName: config.ClientCAConfigMapName,
291295
ImagePullSecrets: config.ImagePullSecrets,
296+
CacheDisabled: disableClientCache,
292297
}).SetupWithManager(mgr); err != nil {
293298
return errors.Join(errors.New("unable to create PolicyServer controller"), err)
294299
}

internal/controller/policy_subreconciler.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,9 @@ func (r *policySubReconciler) isPolicyUniquelyReachable(ctx context.Context, pol
226226
}
227227

228228
replicaSets := appsv1.ReplicaSetList{}
229-
if err = r.List(ctx, &replicaSets, client.MatchingLabels{constants.PolicyServerLabelKey: policyServerDeployment.Labels[constants.PolicyServerLabelKey]}); err != nil {
229+
if err = r.List(ctx, &replicaSets,
230+
client.InNamespace(policyServerDeployment.Namespace),
231+
client.MatchingLabels{constants.PolicyServerLabelKey: policyServerDeployment.Labels[constants.PolicyServerLabelKey]}); err != nil {
230232
return false
231233
}
232234
podTemplateHash := ""
@@ -240,7 +242,9 @@ func (r *policySubReconciler) isPolicyUniquelyReachable(ctx context.Context, pol
240242
return false
241243
}
242244
pods := corev1.PodList{}
243-
if err = r.List(ctx, &pods, client.MatchingLabels{constants.PolicyServerLabelKey: policyServerDeployment.Labels[constants.PolicyServerLabelKey]}); err != nil {
245+
if err = r.List(ctx, &pods,
246+
client.InNamespace(policyServerDeployment.Namespace),
247+
client.MatchingLabels{constants.PolicyServerLabelKey: policyServerDeployment.Labels[constants.PolicyServerLabelKey]}); err != nil {
244248
return false
245249
}
246250
if len(pods.Items) == 0 {

internal/controller/policyserver_controller.go

Lines changed: 70 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,11 @@ type PolicyServerReconciler struct {
6767
// policy-server Deployment, sourced from the controller's own
6868
// --image-pull-secrets flag.
6969
ImagePullSecrets []corev1.LocalObjectReference
70+
// CacheDisabled signals that the controller is running without the
71+
// informer cache (--disable-client-cache). When true, getPolicies uses a
72+
// plain list+filter instead of MatchingFields, which relies on in-memory
73+
// field indexes that only work through the cache.
74+
CacheDisabled bool
7075
}
7176

7277
// TelemetryConfiguration is a struct that contains the configuration for the
@@ -311,35 +316,34 @@ func (r *PolicyServerReconciler) enqueueClusterAdmissionPolicyGroup(_ context.Co
311316
}
312317
}
313318

314-
// getPolicies returns all admission policies, cluster admission policy,
315-
// admission policies groups and cluster admission policy groups bound to the
319+
// getPolicies returns all admission policies, cluster admission policies,
320+
// admission policy groups and cluster admission policy groups bound to the
316321
// given policyServer.
322+
// When the informer cache is disabled it falls back to listing all objects and
323+
// filtering client-side, because MatchingFields relies on in-memory field
324+
// indexes that are only populated through the cache.
317325
func (r *PolicyServerReconciler) getPolicies(ctx context.Context, policyServer *policiesv1.PolicyServer) ([]policiesv1.Policy, error) {
326+
if r.CacheDisabled {
327+
return r.getPoliciesWithoutCache(ctx, policyServer)
328+
}
329+
318330
var clusterAdmissionPolicies policiesv1.ClusterAdmissionPolicyList
319-
err := r.Client.List(ctx, &clusterAdmissionPolicies, client.MatchingFields{constants.PolicyServerIndexKey: policyServer.Name})
320-
if err != nil && apierrors.IsNotFound(err) {
321-
err = fmt.Errorf("failed obtaining ClusterAdmissionPolicies: %w", err)
322-
return nil, err
331+
if err := r.Client.List(ctx, &clusterAdmissionPolicies, client.MatchingFields{constants.PolicyServerIndexKey: policyServer.Name}); err != nil {
332+
return nil, fmt.Errorf("failed obtaining ClusterAdmissionPolicies: %w", err)
323333
}
324334
var admissionPolicies policiesv1.AdmissionPolicyList
325-
err = r.Client.List(ctx, &admissionPolicies, client.MatchingFields{constants.PolicyServerIndexKey: policyServer.Name})
326-
if err != nil && apierrors.IsNotFound(err) {
327-
err = fmt.Errorf("failed obtaining AdmissionPolicies: %w", err)
328-
return nil, err
335+
if err := r.Client.List(ctx, &admissionPolicies, client.MatchingFields{constants.PolicyServerIndexKey: policyServer.Name}); err != nil {
336+
return nil, fmt.Errorf("failed obtaining AdmissionPolicies: %w", err)
329337
}
330338

331339
var admissionPolicyGroupList policiesv1.AdmissionPolicyGroupList
332-
err = r.Client.List(ctx, &admissionPolicyGroupList, client.MatchingFields{constants.PolicyServerIndexKey: policyServer.Name})
333-
if err != nil && apierrors.IsNotFound(err) {
334-
err = fmt.Errorf("failed obtaining AdmissionPolicyGroups: %w", err)
335-
return nil, err
340+
if err := r.Client.List(ctx, &admissionPolicyGroupList, client.MatchingFields{constants.PolicyServerIndexKey: policyServer.Name}); err != nil {
341+
return nil, fmt.Errorf("failed obtaining AdmissionPolicyGroups: %w", err)
336342
}
337343

338344
var clusterAdmissionPolicyGroupList policiesv1.ClusterAdmissionPolicyGroupList
339-
err = r.Client.List(ctx, &clusterAdmissionPolicyGroupList, client.MatchingFields{constants.PolicyServerIndexKey: policyServer.Name})
340-
if err != nil && apierrors.IsNotFound(err) {
341-
err = fmt.Errorf("failed obtaining ClusterAdmissionPolicyGroups: %w", err)
342-
return nil, err
345+
if err := r.Client.List(ctx, &clusterAdmissionPolicyGroupList, client.MatchingFields{constants.PolicyServerIndexKey: policyServer.Name}); err != nil {
346+
return nil, fmt.Errorf("failed obtaining ClusterAdmissionPolicyGroups: %w", err)
343347
}
344348

345349
policies := make([]policiesv1.Policy, 0)
@@ -358,6 +362,54 @@ func (r *PolicyServerReconciler) getPolicies(ctx context.Context, policyServer *
358362
return policies, nil
359363
}
360364

365+
// getPoliciesWithoutCache lists all policy objects and filters them client-side.
366+
// Used when the informer cache is disabled and MatchingFields would be forwarded
367+
// to the API server as an unsupported CRD field selector.
368+
func (r *PolicyServerReconciler) getPoliciesWithoutCache(ctx context.Context, policyServer *policiesv1.PolicyServer) ([]policiesv1.Policy, error) {
369+
var clusterAdmissionPolicies policiesv1.ClusterAdmissionPolicyList
370+
if err := r.Client.List(ctx, &clusterAdmissionPolicies); err != nil {
371+
return nil, fmt.Errorf("failed obtaining ClusterAdmissionPolicies: %w", err)
372+
}
373+
374+
var admissionPolicies policiesv1.AdmissionPolicyList
375+
if err := r.Client.List(ctx, &admissionPolicies); err != nil {
376+
return nil, fmt.Errorf("failed obtaining AdmissionPolicies: %w", err)
377+
}
378+
379+
var admissionPolicyGroupList policiesv1.AdmissionPolicyGroupList
380+
if err := r.Client.List(ctx, &admissionPolicyGroupList); err != nil {
381+
return nil, fmt.Errorf("failed obtaining AdmissionPolicyGroups: %w", err)
382+
}
383+
384+
var clusterAdmissionPolicyGroupList policiesv1.ClusterAdmissionPolicyGroupList
385+
if err := r.Client.List(ctx, &clusterAdmissionPolicyGroupList); err != nil {
386+
return nil, fmt.Errorf("failed obtaining ClusterAdmissionPolicyGroups: %w", err)
387+
}
388+
389+
policies := make([]policiesv1.Policy, 0)
390+
for _, policy := range clusterAdmissionPolicies.Items {
391+
if policy.Spec.PolicyServer == policyServer.Name {
392+
policies = append(policies, policy.DeepCopy())
393+
}
394+
}
395+
for _, policy := range admissionPolicies.Items {
396+
if policy.Spec.PolicyServer == policyServer.Name {
397+
policies = append(policies, policy.DeepCopy())
398+
}
399+
}
400+
for _, policy := range admissionPolicyGroupList.Items {
401+
if policy.Spec.PolicyServer == policyServer.Name {
402+
policies = append(policies, policy.DeepCopy())
403+
}
404+
}
405+
for _, policy := range clusterAdmissionPolicyGroupList.Items {
406+
if policy.Spec.PolicyServer == policyServer.Name {
407+
policies = append(policies, policy.DeepCopy())
408+
}
409+
}
410+
return policies, nil
411+
}
412+
361413
func (r *PolicyServerReconciler) reconcileDeletion(ctx context.Context, policyServer *policiesv1.PolicyServer, policies []policiesv1.Policy) (ctrl.Result, error) {
362414
if len(policies) != 0 {
363415
// There are still policies scheduled on the PolicyServer, we have to

0 commit comments

Comments
 (0)