record the password fingerprint in the Role status only after the database commit

The fingerprint was written to the status inside the transaction. When the commit failed, the error handler still patched the status with the new fingerprint, and the next reconcile saw no password change. The fingerprint and the server password are now computed before the transaction, and the status receives the fingerprint after the transaction returns.

The new test covers the state of every `Role` after the upgrade to the version that introduced the fingerprint. It changes the password in PostgreSQL, removes the fingerprint from the status, and asserts that the Secret password is applied once and left alone afterwards.

Author: ThoSap

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

@@ -114,15 +114,32 @@ public UpdateControl<Role> reconcile(
         UpdateControl<Role> updateControl;
 
         try (var dsl = contextFactory.getDSLContext(clusterConnection)) {
+            var passwordEncryption = spec.getPasswordEncryption();
+
+            // The password hash in pg_authid is readable by superusers only.
+            // Managed PostgreSQL services of cloud providers do not grant that,
+            // so the operator tracks a keyed fingerprint of the applied password in the status.
+            var expectedFingerprint = password != null
+                    ? passwordFingerprintService.fingerprint(password, passwordEncryption)
+                    : null;
+            var serverPassword = password != null
+                    ? postgreSQLAuthenticationService.toServerPassword(password, passwordEncryption)
+                    : null;
+
             // Run everything in a single transaction
             updateControl = dsl.transactionResult(
                     cfg -> reconcileInTransaction(
                             cfg.dsl(),
                             resource,
                             status,
-                            password
+                            expectedFingerprint,
+                            serverPassword
                     )
             );
+
+            // Record the fingerprint only after the commit. A failed commit must not leave
+            // the fingerprint of a password that PostgreSQL never stored.
+            status.setPasswordFingerprint(expectedFingerprint);
         } catch (Exception e) {
             return handleError(
                     resource,
@@ -245,30 +262,23 @@ protected RoleStatus newStatus() {
         return new RoleStatus();
     }
 
+    /// @param expectedFingerprint the fingerprint of the Secret password, or `null` for a `NOLOGIN` role;
+    ///                            compared against the fingerprint in the status, and never written here
+    /// @param serverPassword      the password literal to send to PostgreSQL, or `null` for a `NOLOGIN` role
     private UpdateControl<Role> reconcileInTransaction(
             DSLContext tx,
             Role resource,
             RoleStatus status,
-            @Nullable String password
+            @Nullable String expectedFingerprint,
+            @Nullable String serverPassword
     ) {
         var namespace = resource.getMetadata().getNamespace();
         var name = resource.getMetadata().getName();
 
         var spec = resource.getSpec();
         var expectedFlags = spec.getFlags();
 
-        var passwordEncryption = spec.getPasswordEncryption();
-        var loginExpected = password != null;
-
-        // The password hash in pg_authid is readable by superusers only.
-        // Managed PostgreSQL services of cloud providers do not grant that,
-        // so the operator tracks a keyed fingerprint of the applied password in the status.
-        var expectedFingerprint = password != null
-                ? passwordFingerprintService.fingerprint(password, passwordEncryption)
-                : null;
-        var serverPassword = password != null
-                ? postgreSQLAuthenticationService.toServerPassword(password, passwordEncryption)
-                : null;
+        var loginExpected = serverPassword != null;
 
         // Create and return the role if it doesn't exist yet
         if (!roleService.roleExists(tx, spec)) {
@@ -284,8 +294,6 @@ private UpdateControl<Role> reconcileInTransaction(
                     serverPassword
             );
 
-            status.setPasswordFingerprint(expectedFingerprint);
-
             status.setPhase(CRPhase.READY)
                     .setMessage(null);
 
@@ -350,8 +358,6 @@ private UpdateControl<Role> reconcileInTransaction(
             );
         }
 
-        status.setPasswordFingerprint(expectedFingerprint);
-
         status.setPhase(CRPhase.READY)
                 .setMessage(null);
 

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

@@ -35,6 +35,7 @@
 import static it.aboutbits.postgresql.core.infrastructure.persistence.Tables.PG_ROLES;
 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.role;
 
 @QuarkusTest
@@ -365,6 +366,84 @@ void secretRefChange_triggersReconciliation() {
                 ));
     }
 
+    @Test
+    @DisplayName("When an existing Role has no password fingerprint in its status, the Secret password should be applied once")
+    void missingPasswordFingerprint_appliesSecretPasswordOnce() {
+        // given: a Role created by the operator
+        var clusterConnection = given.one()
+                .clusterConnection()
+                .withName("test-connection-role-fingerprint-upgrade")
+                .returnFirst();
+
+        var roleName = "test-role-fingerprint-upgrade";
+        var password = "secret-password";
+
+        var secretRef = given.one()
+                .secretRef()
+                .withPassword(password)
+                .returnFirst();
+
+        var role = given.one()
+                .role()
+                .withName(roleName)
+                .withClusterConnectionName(clusterConnection.getMetadata().getName())
+                .withPasswordSecretRef(secretRef)
+                .returnFirst();
+
+        var dsl = postgreSQLContextFactory.getDSLContext(clusterConnection);
+
+        var initialFingerprint = role.getStatus().getPasswordFingerprint();
+        assertThat(initialFingerprint).isNotBlank();
+        assertThat(PostgreSQLPasswordVerifier.passwordMatches(
+                dsl,
+                roleName,
+                password
+        )).isTrue();
+
+        // given: the password in PostgreSQL differs from the Secret, and the status has no fingerprint,
+        // like a Role that was reconciled by a version of the operator without fingerprints
+        dsl.execute("alter role {0} with password {1}", role(roleName), inline("out-of-band-password"));
+        assertThat(PostgreSQLPasswordVerifier.passwordMatches(
+                dsl,
+                roleName,
+                password
+        )).isFalse();
+
+        kubernetesClient.resources(Role.class)
+                .inNamespace(role.getMetadata().getNamespace())
+                .withName(role.getMetadata().getName())
+                .editStatus(current -> {
+                    current.getStatus().setPasswordFingerprint(null);
+                    return current;
+                });
+
+        // when: a spec change triggers the next reconcile
+        role.getSpec().setComment("triggers a reconcile");
+
+        var updatedRole = applyRole(role);
+
+        // then: the Secret password is applied once and the fingerprint is set again
+        assertThat(updatedRole.getStatus().getPhase()).isEqualTo(CRPhase.READY);
+        assertThat(updatedRole.getStatus().getPasswordFingerprint()).isEqualTo(initialFingerprint);
+        assertThat(PostgreSQLPasswordVerifier.passwordMatches(
+                dsl,
+                roleName,
+                password
+        )).isTrue();
+
+        // when: the next reconcile finds a matching fingerprint and leaves the password alone
+        var verifierAfterUpgrade = PostgreSQLPasswordVerifier.storedVerifier(dsl, roleName);
+
+        updatedRole.getSpec().setComment("triggers another reconcile");
+
+        updatedRole = applyRole(updatedRole);
+
+        // then
+        assertThat(updatedRole.getStatus().getPhase()).isEqualTo(CRPhase.READY);
+        assertThat(updatedRole.getStatus().getPasswordFingerprint()).isEqualTo(initialFingerprint);
+        assertThat(PostgreSQLPasswordVerifier.storedVerifier(dsl, roleName)).isEqualTo(verifierAfterUpgrade);
+    }
+
     @Test
     @DisplayName("When the Secret already contains a SCRAM-SHA-256 verifier, it should be stored verbatim")
     void preHashedPassword_isStoredVerbatim() {