Skip to content

Support AWS RDS: read pg_roles instead of pg_authid, and fix role updates for non-superuser admins - #72

Closed
wesselOC wants to merge 3 commits into
aboutbits:mainfrom
ordercloud:main
Closed

Support AWS RDS: read pg_roles instead of pg_authid, and fix role updates for non-superuser admins#72
wesselOC wants to merge 3 commits into
aboutbits:mainfrom
ordercloud:main

Conversation

@wesselOC

Copy link
Copy Markdown

Problem

On AWS RDS, SELECT on pg_authid is denied to every role — including rds_superuser — so the operator failed with permission denied for table pg_authid (SQLSTATE 42501). Non-superuser roles on vanilla
PostgreSQL hit the same wall. Fixing the read alone turned out to be insufficient: two further RDS-specific failures blocked role updates entirely.

Changes

  1. Reads moved off pg_authid to the public pg_roles view (RoleService)
    roleExists, roleLoginMatches, fetchCurrentFlags, and fetchCurrentRoleComment now read pg_roles, which is world-readable and masks only rolpassword. pg_roles was added to the jOOQ codegen and regenerated.

  2. Password verification is privilege-aware (PostgreSQLAuthenticationService)
    checkPassword now probes has_table_privilege('pg_catalog.pg_authid', 'SELECT') (world-readable) before touching pg_authid. When the verifier can't be read it returns UNVERIFIABLE and the reconciler falls
    back to comparing the password Secret's metadata.resourceVersion against Role.status.appliedPasswordSecretVersion. This avoids raising 42501 inside the reconcile transaction, which previously aborted the
    transaction (25P02) and made the subsequent password-rotating ALTER ROLE silently fail.

  3. ALTER ROLE emits privilege-gated attributes only when they change (RoleService.buildAlterRole)
    Previously every attribute token was asserted on each update. Since PG16, merely naming SUPERUSER/CREATEDB/CREATEROLE/REPLICATION/BYPASSRLS requires the executing role to hold it (only a superuser may name
    SUPERUSER), so a non-superuser RDS admin got permission denied to alter role on every update. These five attributes are now emitted only when they differ from the role's current state; INHERIT/CONNECTION
    LIMIT/VALID UNTIL remain asserted (not privilege-gated).

Security notes

  • The tracked value in Role.status is the opaque, non-sensitive Kubernetes resourceVersion — no password-derived material is stored in etcd.
  • Passwords continue to flow as bound parameters; identifiers via jOOQ quoting.

Testing

Adds restrictedConnection_passwordChange_isAppliedThroughTransaction, which rotates a password over a connection that cannot read pg_authid (reproducing RDS). It failed on both bugs above before the fix and
passes now. Full :operator:test is green except the two HelmTest cases, which fail only because the helm CLI isn't installed locally.

wessel and others added 3 commits September 11, 2026 10:00
On AWS RDS, SELECT on pg_authid is denied to all roles (including the
master user and rds_superuser), so every Role reconcile failed with
"permission denied for table pg_authid" (SQLSTATE 42501). The same
applies to any non-superuser role on vanilla PostgreSQL.

- RoleService reads role state from the world-readable pg_roles view
  (roleExists, roleLoginMatches, fetchCurrentFlags); pg_roles added to
  jOOQ codegen and regenerated.
- PostgreSQLAuthenticationService.checkPassword returns
  MATCH/MISMATCH/UNVERIFIABLE, returning UNVERIFIABLE on SQLSTATE 42501
  instead of throwing.
- On UNVERIFIABLE (RDS), the reconciler detects password changes by
  comparing the password Secret's resourceVersion against
  Role.status.appliedPasswordSecretVersion, so no password-derived
  material is stored in etcd. KubernetesService.getSecretRefData returns
  credentials and the Secret resourceVersion together.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Two RDS-specific failures surfaced while verifying the pg_authid handling,
both of which prevented the operator from updating roles on managed
clusters (AWS RDS PG16+) whose admin is not a real superuser.

1. checkPassword read pg_authid inside the reconcile transaction. On RDS
   that SELECT raises 42501, which aborts the whole transaction (25P02),
   so the subsequent ALTER ROLE for a password rotation silently failed
   and the new password was never applied. Probe access with the
   world-readable has_table_privilege() first and fall back to the tracked
   Secret version without touching pg_authid, keeping the transaction clean.

2. buildAlterRole always emitted every attribute token (NOSUPERUSER,
   NOCREATEDB, ...). Since PG16 merely naming a privilege-gated attribute
   requires the executing role to hold it (only a superuser may name
   SUPERUSER at all), so every update failed with "permission denied to
   alter role". Emit SUPERUSER/CREATEDB/CREATEROLE/REPLICATION/BYPASSRLS
   only when they differ from the role's current state; a genuine change
   still correctly requires the matching privilege.

Adds a regression test that rotates a password over a connection lacking
pg_authid SELECT, which reproduced both failures and now passes.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
 Support AWS RDS: read pg_roles instead of pg_authid, and fix role updates for non-superuser admins
@ThoSap

ThoSap commented Sep 11, 2026

Copy link
Copy Markdown
Member

Duplicate of #71

Thanks for opening this PR, but I mentioned already in #70 (comment) that I will have a look at this.

Both PRs found the same two root causes, and they fix them the same way (most likely as you took #70 (comment) as the agent input):
The reads move to pg_roles, and ALTER ROLE names a privilege-gated attribute only when it changes. The two PRs differ in how they detect a password change without a read of pg_authid, and that is where I decided for #71.

What is missing here, or worse than in #71

One idea I liked

The has_table_privilege probe is a good idea. Where pg_authid is readable, it lets the operator compare the real verifier and repair a password that somebody changed directly in PostgreSQL.

I still decided against it for #71. It makes the behaviour depend on the privileges of the admin role: a superuser cluster takes one path, a managed service takes the other. That means two code paths to reason about, to test, and to support forever, and a bug report then needs the privileges of the admin role before anybody can read it. #71 keeps one path, so the operator behaves the same everywhere.

Thanks again for the analysis, tokens and for the time you put into this.

@ThoSap ThoSap closed this Sep 11, 2026
@ThoSap

ThoSap commented Sep 11, 2026

Copy link
Copy Markdown
Member

We should also no longer rely on pg_authid because of the sensitive password hashes, as it requires superuser privileges, and causes permission errors on managed cloud databases as you found out, making pg_roles the safer alternative in the long run and the approach #71 uses.

Using pg_authid was a bad design from my part, and now it is done in a proper way.

@wesselOC

Copy link
Copy Markdown
Author

Thanks for resolving the issue in such quick time!

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