crossplane-contrib / crossplane-contrib/provider-sql

Role: password regenerated on restore can be permanently lost when privilege comparison fails in the same Update() call

Open
#441 0 comments 0 reactions 0 assignees View on GitHub
bug
Dominant language
Go
Stars
154
Forks
119
Avg merge
6d 17h
Merged PRs (30d)
8

Description

### What happened?

The "auto-regenerate role password on restore" feature from [#348](https://github.com/crossplane-contrib/provider-sql/pull/348) (v0.16.1) can regenerate and apply a new password in Postgres, then lose it permanently if a later step in the same `Update()` call fails.

In [`Update()`](https://github.com/crossplane-contrib/provider-sql/blob/fe39828c16548e9d8d9d5b061a29526fad7f4ffc/pkg/controller/cluster/postgresql/role/reconciler.go#L333-L367) (mirrored in `pkg/controller/namespaced/postgresql/role/reconciler.go`): `shouldResetPassword()` detects a restored role (`LastPasswordChange` nil, empty secret), runs `ALTER ROLE ... PASSWORD ...`, and sets [`mg.Status.AtProvider.LastPasswordChange`](https://github.com/crossplane-contrib/provider-sql/blob/fe39828c16548e9d8d9d5b061a29526fad7f4ffc/pkg/controller/cluster/postgresql/role/reconciler.go#L359) before the function has finished. The next step, [`changedPrivs(mg.Status.AtProvider.PrivilegesAsClauses, privs)`](https://github.com/crossplane-contrib/provider-sql/blob/fe39828c16548e9d8d9d5b061a29526fad7f4ffc/pkg/controller/cluster/postgresql/role/reconciler.go#L363), errors with `errComparePrivileges` if `PrivilegesAsClauses` is empty or shorter than desired ([guard](https://github.com/crossplane-contrib/provider-sql/blob/fe39828c16548e9d8d9d5b061a29526fad7f4ffc/pkg/controller/cluster/postgresql/role/reconciler.go#L186)), which happens whenever `.status` hasn't been observed yet. `Update()` returns early with an error, so `PublishConnection()` is never called and the new password never reaches the connection secret. The generic reconciler persists `mg.Status` regardless of the error, so `LastPasswordChange` is saved anyway. `shouldResetPassword()` won't fire again (no longer nil, and nothing sets `PasswordRotationTrigger` automatically), so the role is stuck with a password that's live in Postgres but recorded nowhere.

`PrivilegesAsClauses` arrives empty/short because a plain `client.Update()` earlier in the same reconcile can return a stale `.status` view, from either crossplane-runtime's `AddFinalizer` call (runs unconditionally on any object's first-ever reconcile) or this provider's own late-init persistence. The latter is deterministic: [`lateInit()`](https://github.com/crossplane-contrib/provider-sql/blob/fe39828c16548e9d8d9d5b061a29526fad7f4ffc/pkg/controller/cluster/postgresql/role/reconciler.go#L468-L501) backfills any of `Privileges.SuperUser`, `Inherit`, `CreateDb`, `CreateRole`, `Login`, `Replication`, `BypassRls`, or `ConnectionLimit` left `nil` on the spec, and reports `ResourceLateInitialized` whenever it does. A `Role` spec that doesn't set all of these hits this path on nearly every reconcile.

### How can we reproduce it?

1. Create a `Role` with `managementPolicies` excluding `Delete`, leaving `bypassRls`/`connectionLimit`/the privilege booleans unset (so late-init has something to backfill each reconcile).
2. Wait for it to be `Ready`, then delete and immediately recreate the object (fresh `.status`, external role still exists).
3. Check the connection secret: `password` comes back empty while `status.atProvider.lastPasswordChange` is set.
4. Check events for `cannot update role: cannot compare desired and observed privileges`.

### What environment did it happen in?

provider-sql version: `v0.16.1`
Crossplane version: `v2.2.0`

### Suggested fix

- Don't persist `LastPasswordChange` until `Update()` fully succeeds. Move the assignment to just before the function's final successful return.
- Make `changedPrivs` tolerate an empty/short `existing` instead of erroring. Treat "no prior observation yet" as "apply the full desired list."

Contributor guide

No contributing guide indexed for this repository

Research direction

Start in pkg/controller/cluster/postgresql/role/reconciler.go, especially Update(), shouldResetPassword(), changedPrivs(), and lateInit(); inspect the mirrored namespaced role reconciler as well. Reproduce the restore and stale-status sequence described in the issue. Done means a failed update cannot persist LastPasswordChange prematurely, and missing privilege observations no longer lose the regenerated password or block connection-secret publication.

Written by the indexing model from the issue text.

Assessment

Tech stack
go, postgresql
Domain
backend, databases
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.