fix(postgresql): fix Role password loss on restore when privilege comparison fails

[AGejr]

Description of your changes

Fixes a bug in the password auto-rotation feature where Update() on a Role can regenerate and apply a new password in Postgres, then lose it permanently if a later step in the same call fails.

• changedPrivs now applies the full desired privilege list instead of erroring when the observed clause list is shorter than desired (this happens whenever the role's .status hasn't been observed yet, e.g. right after a restore)
• Update no longer records LastPasswordChange until every step has succeeded, so a later failure can't strand a password that was changed in Postgres but never recorded or published

Both fixes are mirrored in the cluster-scoped and namespaced Role reconcilers.

Also documents the Postgres Role password mechanism, which was previously undocumented: a new docs/postgresql-role-passwords.md covering BYO vs auto-generated passwords, generation parameters, recovery of a lost password, and rotation via passwordRotationTrigger (including the caveat that the trigger is inert until lastPasswordChange is first set). Field godoc on passwordSecretRef and passwordRotationTrigger is expanded in both API groups and links to the new page, and the Role examples gain rotating-role and byo-password-role.

Fixes #441
Fixes #443

I have:

• Read and followed Crossplane's contribution process.
• Run make reviewable to ensure this PR is ready for review.

How has this code been tested

Updated unit tests cover the changedPrivs behavior change (applying the full desired set instead of erroring). The postgres e2e suite (cluster/local/postgresdb_functions.sh) gains coverage for the Role password mechanism in both the cluster and namespaced trees: BYO password publish and update propagation, auto-generated password recovery when the connection secret is lost, rotation via passwordRotationTrigger, and password recovery when the Role object is deleted and recreated while the database role persists. Full suite passes with go test ./....

[fernandezcuesta]

Could you please also fix #443? 🙏

[AGejr]

Sure, no problem. Should I open a new pr or add the documentation alongside the changes in this pr?

[fernandezcuesta]

Just add it in this same PR, I realized all sort of doc was missing while reviewing it (my fault)

[AGejr]

Pushed the docs and tests.

#443: b25eda8 adds docs/postgresql-role-passwords.md covering BYO vs auto-generated passwords, the generation parameters, recovering a lost password, and passwordRotationTrigger (including that it does nothing until lastPasswordChange is set). Expanded the passwordSecretRef and passwordRotationTrigger godoc in both API groups to link the page, and regenerated the Role CRDs so the link shows up on doc.crds.dev and the marketplace.

e2e: 5370c61 adds check_role_passwords to the postgres suite, in both the cluster and namespaced trees:

• BYO password lands in the connection secret, and updates when the source secret changes
• auto-generated password is regenerated when the connection secret is deleted
• passwordRotationTrigger rotates it
• deleting and recreating the Role object while the DB role still exists recovers the password

On changedPrivs: when the observed clause list is shorter than desired, it now applies the full desired set instead of erroring. The thinking is that without a full prior observation to diff against, applying the spec seems a safer default than an error that nothing recovers from - by the time the old path errored, the password had already been changed in Postgres and LastPasswordChange stamped.

[AGejr]

Thanks for running this through your e2e harness @chlunde, both probes are real and I'd like to address them here.

Future-dated passwordRotationTrigger - confirmed. shouldResetPassword compares trigger.After(lastPasswordChange), and since each rotation only stamps lastPasswordChange = now, a trigger in the future stays "after" it and rotates on every reconcile until the wall clock passes it. My own e2e uses the now+1h shape and only asserts a single rotation, so it walked straight past this.

Proposal: record the trigger the provider last acted on in status.atProvider (lastAppliedPasswordRotationTrigger or similar) and rotate only when the spec trigger differs from it, instead of comparing against lastPasswordChange. That makes the semantics "rotate once per distinct trigger value" whether the timestamp is past or future, and the docs/example change to match ("a value you haven't used before" rather than "later than lastPasswordChange"). Does that seem reasonable, or do you see a cleaner approach?

Double rotation on recreate - agree it looks like informer cache lag: the status write recording lastPasswordChange hasn't landed before the next reconcile, so shouldResetPassword still sees a nil LastPasswordChange plus an empty secret and resets again. One extra regenerate on a restore is harmless on its own, but in large-scale setups with many compositions - already reconcile-heavy - the extra ALTER ROLE + secret write + status update per restored role adds up, so I'd rather guard it. Cheapest fix I see is a fresh Get of the role status right before the reset decision so we act on the latest lastPasswordChange. Sound right to you, or would you handle it differently?

Will push both fixes once we've settled on the approach.