clear the password of a NOLOGIN role that still holds one

`buildAlterRole` added `PASSWORD NULL` only on the transition from `LOGIN` to
`NOLOGIN`. A role that was already `NOLOGIN` but still held a password kept it.

`pg_roles` masks `rolpassword` with a constant, so the operator cannot tell
whether such a role has a password. It therefore always clears the password when
the spec expects no login.

This costs nothing in the steady state. `RoleReconciler` calls `alterRole` only
when the login state, the flags, or the password differ from the spec.

Author: ThoSap

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

@@ -374,12 +374,17 @@ private static Optional<Query> buildAlterRole(
         }
 
         // Password handling
-        // - if the role loses LOGIN, remove the password
+        // - if no login is expected, remove the password
         // - if LOGIN and the password changed, set the new password
-        if (!loginExpected && currentCanLogin) {
+        //
+        // `PASSWORD NULL` is not conditional on the current state. `pg_roles` masks `rolpassword`
+        // with a constant, so the operator cannot tell whether a `NOLOGIN` role still holds a
+        // password. The reconciler calls this method only when the role differs from the spec,
+        // so the statement does not run on every reconcile.
+        if (!loginExpected) {
             options.add(keyword(RoleFlag.PASSWORD.flag()));
             options.add(keyword("NULL"));
-        } else if (loginExpected && changePassword) {
+        } else if (changePassword) {
             options.add(keyword(RoleFlag.PASSWORD.flag()));
             options.add(val(password));
         }

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

@@ -36,7 +36,9 @@
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.awaitility.Awaitility.await;
 import static org.jooq.impl.DSL.inline;
+import static org.jooq.impl.DSL.query;
 import static org.jooq.impl.DSL.role;
+import static org.jooq.impl.DSL.val;
 
 @QuarkusTest
 @RequiredArgsConstructor
@@ -196,6 +198,48 @@ void toggleRoleLogin_updatesCorrectly() {
         assertThat(getRoleFlagValue(dsl, roleName, PG_ROLES.ROLCANLOGIN)).isFalse();
     }
 
+    @Test
+    @DisplayName("When a NOLOGIN Role still holds a password, the next update should clear it")
+    void noLoginRole_withLeftoverPassword_clearsPassword() {
+        // given: a reconciled NOLOGIN role
+        var clusterConnection = given.one()
+                .clusterConnection()
+                .withName("test-connection-role-leftover-password")
+                .returnFirst();
+
+        var roleName = "test-role-leftover-password";
+
+        var role = given.one()
+                .role()
+                .withName(roleName)
+                .withClusterConnectionName(clusterConnection.getMetadata().getName())
+                .returnFirst();
+
+        var dsl = postgreSQLContextFactory.getDSLContext(clusterConnection);
+
+        assertThat(getRoleFlagValue(dsl, roleName, PG_ROLES.ROLCANLOGIN)).isFalse();
+
+        // and: somebody sets a password directly in PostgreSQL.
+        // The role keeps NOLOGIN, so the login state still matches the spec.
+        // `pg_roles` masks `rolpassword`, so the operator cannot see that password.
+        dsl.execute(query(
+                "alter role {0} with password {1}",
+                role(roleName),
+                val("leftover-password")
+        ));
+
+        assertThat(PostgreSQLPasswordVerifier.storedVerifier(dsl, roleName)).isNotNull();
+
+        // when: an unrelated flag changes, so the operator alters the role
+        role.getSpec().getFlags().setCreatedb(true);
+
+        applyRole(role);
+
+        // then: the operator cleared the leftover password
+        assertThat(getRoleFlagValue(dsl, roleName, PG_ROLES.ROLCREATEDB)).isTrue();
+        assertThat(PostgreSQLPasswordVerifier.storedVerifier(dsl, roleName)).isNull();
+    }
+
     @Test
     @DisplayName("When a Role references a missing ClusterConnection, status should be PENDING with a helpful message")
     void createRole_withMissingClusterConnection_setsPending() {