fix: make role password rotation and verification work on RDS

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>

Author: wessel

operator/src/main/java/it/aboutbits/postgresql/core/PostgreSQLAuthenticationService.java

@@ -22,6 +22,8 @@
 import java.util.Locale;
 
 import static it.aboutbits.postgresql.core.infrastructure.persistence.Tables.PG_AUTHID;
+import static org.jooq.impl.DSL.field;
+import static org.jooq.impl.DSL.val;
 
 @Slf4j
 @Singleton
@@ -54,15 +56,42 @@ public enum PasswordCheck {
     /**
      * Compare the desired password against the verifier PostgreSQL stores in {@code pg_authid}.
      * <p>
-     * Returns {@link PasswordCheck#UNVERIFIABLE} when {@code pg_authid} cannot be read (SQLSTATE
-     * {@code 42501}), so callers can fall back to a tracked hash instead of treating the situation
-     * as a mismatch and rewriting the password on every reconcile.
+     * Returns {@link PasswordCheck#UNVERIFIABLE} when {@code pg_authid} cannot be read (e.g. on AWS
+     * RDS, where SELECT is denied to every role), so callers can fall back to comparing the tracked
+     * Secret version instead of treating the situation as a mismatch and rewriting the password on
+     * every reconcile.
+     * <p>
+     * The privilege is probed with {@code has_table_privilege} <em>before</em> touching
+     * {@code pg_authid}. This matters because {@code checkPassword} runs inside the reconcile
+     * transaction: reading {@code pg_authid} without access raises SQLSTATE {@code 42501}, which
+     * aborts the whole transaction (SQLSTATE {@code 25P02}) and makes every following write — such
+     * as the {@code ALTER ROLE} that rotates the password — fail. {@code has_table_privilege} is
+     * world-readable, so probing it first never poisons the transaction.
      */
     public PasswordCheck checkPassword(
             DSLContext dsl,
             RoleSpec spec,
             String expectedPassword
     ) {
+        // Probe access first: reading pg_authid without permission raises 42501 and aborts the
+        // surrounding transaction. has_table_privilege is world-readable and safe to call here.
+        var canReadAuthid = Boolean.TRUE.equals(
+                dsl.select(field(
+                                "has_table_privilege({0}, 'SELECT')",
+                                Boolean.class,
+                                val("pg_catalog.pg_authid")
+                        ))
+                        .fetchOne(0, Boolean.class)
+        );
+
+        if (!canReadAuthid) {
+            log.debug(
+                    "Cannot read pg_authid to verify the password for role [{}]; falling back to tracked Secret version",
+                    spec.getName()
+            );
+            return PasswordCheck.UNVERIFIABLE;
+        }
+
         String currentPasswordVerifier;
         try {
             currentPasswordVerifier = dsl
@@ -71,9 +100,14 @@ public PasswordCheck checkPassword(
                     .where(PG_AUTHID.ROLNAME.eq(spec.getName()))
                     .fetchSingle(PG_AUTHID.ROLPASSWORD);
         } catch (DataAccessException e) {
+            // Defensive only. The has_table_privilege probe above is the real guard against 42501;
+            // this catch just handles the narrow TOCTOU case where SELECT privilege is revoked
+            // between the probe and this read. Note it cannot un-poison the transaction: if the
+            // SELECT does raise 42501 the transaction is already aborted server-side, so a later
+            // write in the same transaction will still fail with 25P02 and the reconcile retries.
             if (isInsufficientPrivilege(e)) {
                 log.debug(
-                        "Cannot read pg_authid to verify the password for role [{}]; falling back to tracked hash",
+                        "Cannot read pg_authid to verify the password for role [{}]; falling back to tracked Secret version",
                         spec.getName()
                 );
                 return PasswordCheck.UNVERIFIABLE;

operator/src/main/java/it/aboutbits/postgresql/crd/role/RoleReconciler.java

@@ -331,6 +331,7 @@ private UpdateControl<Role> reconcileInTransaction(
             roleService.alterRole(
                     tx,
                     spec,
+                    currentFlags,
                     changePassword,
                     password
             );

operator/src/main/java/it/aboutbits/postgresql/crd/role/RoleService.java

@@ -68,6 +68,7 @@ public void createRole(
     public void alterRole(
             DSLContext tx,
             RoleSpec spec,
+            RoleSpec.Flags currentFlags,
             boolean changePassword,
             @Nullable String password
     ) {
@@ -78,6 +79,7 @@ public void alterRole(
                 buildAlterRole(
                         roleName,
                         flags,
+                        currentFlags,
                         changePassword,
                         password
                 )
@@ -327,9 +329,27 @@ private static Query buildCreateRole(
         );
     }
 
+    /**
+     * Append the token for a privilege-gated role attribute to {@code options} only when the desired
+     * value differs from the current one. See {@link #buildAlterRole} for why naming such attributes
+     * unconditionally breaks on clusters (e.g. AWS RDS) whose admin is not a superuser.
+     */
+    private static void addAttributeIfChanged(
+            ArrayList<QueryPart> options,
+            boolean desired,
+            boolean current,
+            RoleFlag enabled,
+            RoleFlag disabled
+    ) {
+        if (desired != current) {
+            options.add(keyword(desired ? enabled.flag() : disabled.flag()));
+        }
+    }
+
     private static Query buildAlterRole(
             String roleName,
             RoleSpec.Flags flags,
+            RoleSpec.Flags currentFlags,
             boolean changePassword,
             @Nullable String password
     ) {
@@ -353,31 +373,55 @@ private static Query buildAlterRole(
             options.add(val(password));
         }
 
-        // Explicitly set the expected state to make the statement idempotent
-        options.add(keyword(flags.isSuperuser()
-                ? RoleFlag.SUPERUSER.flag()
-                : RoleFlag.NO_SUPERUSER.flag()
-        ));
-        options.add(keyword(flags.isCreatedb()
-                ? RoleFlag.CREATEDB.flag()
-                : RoleFlag.NO_CREATEDB.flag()
-        ));
-        options.add(keyword(flags.isCreaterole()
-                ? RoleFlag.CREATEROLE.flag()
-                : RoleFlag.NO_CREATEROLE.flag()
-        ));
+        // The role attributes below (SUPERUSER, CREATEDB, CREATEROLE, REPLICATION, BYPASSRLS) are
+        // privilege-gated: since PostgreSQL 16, merely *naming* one of these in ALTER ROLE requires
+        // the executing role to hold that attribute itself (and only a superuser may name SUPERUSER
+        // at all) - even when the value is unchanged. On managed clusters like AWS RDS the admin is
+        // not a real superuser, so unconditionally emitting e.g. NOSUPERUSER makes every update fail
+        // with "permission denied to alter role". We therefore emit each of these only when it
+        // actually differs from the role's current state; a genuine change still (correctly)
+        // requires the matching privilege. INHERIT, CONNECTION LIMIT and VALID UNTIL are not
+        // privilege-gated and are always safe to assert.
+        addAttributeIfChanged(
+                options,
+                flags.isSuperuser(),
+                currentFlags.isSuperuser(),
+                RoleFlag.SUPERUSER,
+                RoleFlag.NO_SUPERUSER
+        );
+        addAttributeIfChanged(
+                options,
+                flags.isCreatedb(),
+                currentFlags.isCreatedb(),
+                RoleFlag.CREATEDB,
+                RoleFlag.NO_CREATEDB
+        );
+        addAttributeIfChanged(
+                options,
+                flags.isCreaterole(),
+                currentFlags.isCreaterole(),
+                RoleFlag.CREATEROLE,
+                RoleFlag.NO_CREATEROLE
+        );
+        addAttributeIfChanged(
+                options,
+                flags.isReplication(),
+                currentFlags.isReplication(),
+                RoleFlag.REPLICATION,
+                RoleFlag.NO_REPLICATION
+        );
+        addAttributeIfChanged(
+                options,
+                flags.isBypassrls(),
+                currentFlags.isBypassrls(),
+                RoleFlag.BYPASSRLS,
+                RoleFlag.NO_BYPASSRLS
+        );
+
         options.add(keyword(flags.isInherit()
                 ? RoleFlag.INHERIT.flag()
                 : RoleFlag.NO_INHERIT.flag()
         ));
-        options.add(keyword(flags.isReplication()
-                ? RoleFlag.REPLICATION.flag()
-                : RoleFlag.NO_REPLICATION.flag()
-        ));
-        options.add(keyword(flags.isBypassrls()
-                ? RoleFlag.BYPASSRLS.flag()
-                : RoleFlag.NO_BYPASSRLS.flag()
-        ));
 
         options.add(keyword(RoleFlag.CONNECTION_LIMIT.flag()));
         options.add(val(flags.getConnectionLimit()));

operator/src/test/java/it/aboutbits/postgresql/crd/role/RoleReconcilerTest.java

@@ -279,6 +279,115 @@ void restrictedRole_cannotReadPgAuthid_usesPgRolesAndReportsUnverifiable() {
         }
     }
 
+    @Test
+    @DisplayName(
+            "When reconciling over a connection that cannot read pg_authid (like on RDS), a password change should still be applied"
+    )
+    void restrictedConnection_passwordChange_isAppliedThroughTransaction() {
+        // given: a superuser connection we use to set up and to verify the result
+        var adminClusterConnection = given.one()
+                .clusterConnection()
+                .withName("test-connection-rds-admin")
+                .returnFirst();
+
+        var adminDsl = postgreSQLContextFactory.getDSLContext(adminClusterConnection);
+
+        // and: a non-superuser CREATEROLE login role. As on RDS, it cannot SELECT pg_authid, but it
+        // can create and alter the roles it owns.
+        var limitedAdminName = "test_rds_limited_admin";
+        var limitedAdminPassword = "limited-admin-password";
+
+        adminDsl.execute(query("drop role if exists {0}", role(limitedAdminName)));
+        adminDsl.execute(query(
+                "create role {0} with login createrole password {1}",
+                role(limitedAdminName),
+                val(limitedAdminPassword)
+        ));
+
+        var roleName = "test-role-rds-password-change";
+
+        // and: a ClusterConnection that authenticates as the limited (RDS-like) role
+        var limitedSecretRef = given.one()
+                .secretRef()
+                .withUsername(limitedAdminName)
+                .withPassword(limitedAdminPassword)
+                .returnFirst();
+
+        var limitedClusterConnection = given.one()
+                .clusterConnection()
+                .withName("test-connection-rds-limited")
+                .withAdminSecretRef(limitedSecretRef)
+                .returnFirst();
+
+        var initialPassword = "initial-password";
+        var newPassword = "new-password";
+
+        var passwordSecretRef = given.one()
+                .secretRef()
+                .withPassword(initialPassword)
+                .returnFirst();
+
+        var passwordSecret = kubernetesClient.secrets()
+                .inNamespace(kubernetesClient.getNamespace())
+                .withName(passwordSecretRef.getName())
+                .require();
+
+        // when: create the Role over the limited connection (the create path does not read pg_authid)
+        var role = given.one()
+                .role()
+                .withName(roleName)
+                .withClusterConnectionName(limitedClusterConnection.getMetadata().getName())
+                .withPasswordSecretRef(passwordSecretRef)
+                .returnFirst();
+
+        try {
+            // then: the initial password is applied
+            await().atMost(5, TimeUnit.SECONDS)
+                    .pollInterval(100, TimeUnit.MILLISECONDS)
+                    .until(() -> postgreSQLAuthenticationService.passwordMatches(
+                            adminDsl,
+                            role.getSpec(),
+                            initialPassword
+                    ));
+
+            // when: the password is rotated in the Secret. This triggers a reconcile that must run
+            // ALTER ROLE through the same transaction in which reading pg_authid fails.
+            passwordSecret.getMetadata().setManagedFields(null);
+            var rotatedSecret = new SecretBuilder(passwordSecret)
+                    .addToStringData(SECRET_DATA_BASIC_AUTH_PASSWORD_KEY, newPassword)
+                    .build();
+
+            kubernetesClient.secrets()
+                    .inNamespace(kubernetesClient.getNamespace())
+                    .resource(rotatedSecret)
+                    .serverSideApply();
+
+            // then: the new password should eventually be applied
+            await().atMost(10, TimeUnit.SECONDS)
+                    .pollInterval(200, TimeUnit.MILLISECONDS)
+                    .until(() -> postgreSQLAuthenticationService.passwordMatches(
+                            adminDsl,
+                            role.getSpec(),
+                            newPassword
+                    ));
+        } finally {
+            // Delete the Role CR first so the operator drops the DB role via the still-valid limited
+            // connection, then drop the limited admin role as superuser.
+            kubernetesClient.resources(Role.class)
+                    .inNamespace(role.getMetadata().getNamespace())
+                    .withName(roleName)
+                    .withTimeout(5, TimeUnit.SECONDS)
+                    .delete();
+
+            await().atMost(5, TimeUnit.SECONDS)
+                    .pollInterval(100, TimeUnit.MILLISECONDS)
+                    .until(() -> !roleService.roleExists(adminDsl, role.getSpec()));
+
+            adminDsl.execute(query("drop role if exists {0}", role(roleName)));
+            adminDsl.execute(query("drop role if exists {0}", role(limitedAdminName)));
+        }
+    }
+
     @Test
     @DisplayName("When a Role references a missing ClusterConnection, status should be PENDING with a helpful message")
     void createRole_withMissingClusterConnection_setsPending() {