From 190231999906209392beb9878ecd4dadbdfe5a62 Mon Sep 17 00:00:00 2001 From: Shreyans Nandy Date: Tue, 14 Jul 2026 18:52:39 +0530 Subject: [PATCH 1/2] ISR-11280 Fix unbounded growth of Role status.conditions appendRoleStatusCondition trimmed conditions to the last 5 entries into a local variable (roleStatusConditions) but then appended the new condition onto the original, untrimmed role.Status.Conditions field. The trim was silently discarded every time, so conditions accumulated forever instead of being capped at 5. In production this caused individual Role objects to grow to 8,000+ conditions (900KB+), exceeding etcd's request size limit and making every subsequent status update fail with "etcdserver: request is too large" - which in turn caused the controller to retry rapidly, making the object grow even faster. Fix: append onto the already-trimmed roleStatusConditions instead of the untrimmed role.Status.Conditions, matching the pattern already used correctly in GrantReconciler.appendGrantStatusCondition and GrantStatementReconciler.appendGrantStatementStatusCondition. --- controllers/postgresql/role_controller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/controllers/postgresql/role_controller.go b/controllers/postgresql/role_controller.go index 8f385ed..49ee0d6 100644 --- a/controllers/postgresql/role_controller.go +++ b/controllers/postgresql/role_controller.go @@ -481,7 +481,7 @@ func (r *RoleReconciler) appendRoleStatusCondition(ctx context.Context, role *po getLastItem := roleStatusConditions[len(roleStatusConditions)-1] if getLastItem.Reason != condition.Reason { - role.Status.Conditions = append(role.Status.Conditions, condition) + role.Status.Conditions = append(roleStatusConditions, condition) err := r.Status().Update(ctx, role) if err != nil { roleLogger.Error(err, fmt.Sprintf("Resource status update failed for role `%s`", role.Name)) From 8e881d3f76d8d38c722ed993dc3123c095b29a51 Mon Sep 17 00:00:00 2001 From: Shreyans Nandy Date: Wed, 22 Jul 2026 16:47:26 +0530 Subject: [PATCH 2/2] ISR-11280 Fix GrantStatement controller skipping re-grant after REVOKE ALL The skip-if-unchanged check (isLastConditionSuccess + !hasGrantStatementChanged) was evaluated against the manager's cached client. A reconcile that had just executed REVOKE ALL could be immediately followed by another reconcile reading the pre-revoke cached status, which incorrectly concluded nothing had changed and skipped re-applying the declared GRANT statements - leaving the role with zero privileges until the GrantStatement's spec changed again. Root-caused during a 21 July 2026 production incident: canvas-server-user lost all privileges on common_auth after its GrantStatement controller reconciled for the first time in months (once a stuck connectivity issue was resolved), hitting exactly this REVOKE-then-skip sequence. Fix: read the GrantStatement live via the manager's APIReader (bypassing the cache) before making the skip decision, so it always reflects the latest status. --- .../postgresql/grantstatement_controller.go | 16 ++++++++++++++-- main.go | 5 +++-- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/controllers/postgresql/grantstatement_controller.go b/controllers/postgresql/grantstatement_controller.go index 80d9157..7795069 100644 --- a/controllers/postgresql/grantstatement_controller.go +++ b/controllers/postgresql/grantstatement_controller.go @@ -63,6 +63,13 @@ var ( type GrantStatementReconciler struct { client.Client Scheme *runtime.Scheme + // APIReader bypasses the manager's cache. The skip-if-unchanged check below + // must never evaluate against a status the cache hasn't caught up on yet - + // otherwise a reconcile that just ran REVOKE ALL can be immediately followed + // by another reconcile that reads the pre-revoke cached status, wrongly + // concludes nothing changed, and skips re-granting, leaving the role with + // no privileges. Reading the object live here closes that race. + APIReader client.Reader } //+kubebuilder:rbac:groups=postgresql.facets.cloud,resources=grantstatements,verbs=get;list;watch;create;update;patch;delete @@ -84,9 +91,14 @@ func (r *GrantStatementReconciler) Reconcile(ctx context.Context, req ctrl.Reque panic(err) } - // get grantstatement resource + // get grantstatement resource - read live (not from the cache) since the + // skip-if-unchanged decision below depends on this object's latest status + reader := r.APIReader + if reader == nil { + reader = r.Client + } grantStatement := &postgresqlv1alpha1.GrantStatement{} - err = r.Get(ctx, req.NamespacedName, grantStatement) + err = reader.Get(ctx, req.NamespacedName, grantStatement) if err != nil { return ctrl.Result{}, nil } diff --git a/main.go b/main.go index e16d27d..ccaf0c4 100644 --- a/main.go +++ b/main.go @@ -106,8 +106,9 @@ func main() { os.Exit(1) } if err = (&postgresqlcontrollers.GrantStatementReconciler{ - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + APIReader: mgr.GetAPIReader(), }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "GrantStatement") os.Exit(1)