Skip to content

ISR-11280 Fix unbounded Role status.conditions growth and GrantStatement REVOKE-without-regrant bug#13

Merged
unni-facets merged 2 commits into
Facets-cloud:mainfrom
shreyansjainl:fix/status-conditions-unbounded-growth
Jul 22, 2026
Merged

ISR-11280 Fix unbounded Role status.conditions growth and GrantStatement REVOKE-without-regrant bug#13
unni-facets merged 2 commits into
Facets-cloud:mainfrom
shreyansjainl:fix/status-conditions-unbounded-growth

Conversation

@shreyansjainl

@shreyansjainl shreyansjainl commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Summary

Two related reliability bugs found and fixed during a production incident investigation on 21-22 July 2026:

1. RoleReconciler - unbounded status.conditions growth

  • appendRoleStatusCondition in role_controller.go computes a trimmed slice (last 5 conditions) into a local variable, but then appends the new condition onto the original, untrimmed role.Status.Conditions field - silently discarding the trim on every call.
  • In production, this let a single Role object accumulate 8,000+ conditions (900KB+), exceeding etcd's request size limit. Every subsequent status write then failed with etcdserver: request is too large, and the resulting error-driven requeue caused the object to grow even faster - a self-reinforcing loop that never recovers on its own.
  • The sibling reconcilers (GrantReconciler.appendGrantStatusCondition, GrantStatementReconciler.appendGrantStatementStatusCondition) already implement this trim correctly - this PR brings RoleReconciler in line with that existing, correct pattern.

2. GrantStatementReconciler - skips re-GRANT after REVOKE ALL on a stale cache read

  • The reconciler's skip-if-unchanged check (isLastConditionSuccess(...) && !hasGrantStatementChanged(...)) was evaluated against the manager's cached client.
  • Once issue Feature: Added postgres user management with database-operator Kubernetes Controller #1 above was fixed and several long-stuck Role/GrantStatement objects finally became reconcilable again, 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 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. The application failed with permission denied for table instances as a direct result.

Fix

  1. One-line change in appendRoleStatusCondition: append onto the already-trimmed roleStatusConditions local variable instead of the untrimmed role.Status.Conditions field.
  2. GrantStatementReconciler now reads the GrantStatement object live via the manager's APIReader (bypassing the cache) before making the skip decision, so the skip check always reflects the latest status rather than a potentially stale, pre-revoke cached copy.

Test plan

  • Confirmed via kubectl get roles.postgresql.facets.cloud -A in a live cluster that multiple Role objects had condition counts in the 8,000-8,900 range and object sizes near 1MB.
  • Manually truncated .status.conditions on the affected objects (via a direct PATCH to the /status subresource) to unblock reconciliation immediately.
  • Reproduced the REVOKE-without-regrant sequence directly in production operator logs (REVOKE ALL ... immediately followed by "already been successfully executed, skipping execution" for the same object) before applying the fix.
  • Verified both changes are type-safe (no local Go toolchain was available to run go build, but client.Reader/client.Client and the []metav1.Condition swap cannot introduce a compile error).
  • Recommend adding a unit test asserting len(role.Status.Conditions) <= 6 after repeated calls to appendRoleStatusCondition with alternating reasons.
  • Recommend adding an envtest-based integration test for GrantStatementReconciler.Reconcile that exercises the REVOKE-then-reconcile-again sequence against a stale cache, once test scaffolding (flag registration, fake DB) exists for this controller.

🤖 Generated with Claude Code

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.
…E 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.
@shreyansjainl shreyansjainl changed the title ISR-11280 Fix unbounded growth of Role status.conditions ISR-11280 Fix unbounded Role status.conditions growth and GrantStatement REVOKE-without-regrant bug Jul 22, 2026
@unni-facets
unni-facets merged commit 4940cca into Facets-cloud:main Jul 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants