fix(lakekeeper): drop database when admin is not the owner#650
Merged
Conversation
#649 wired DropDatabase+DropRole into DeleteForOrg, but the actual teardown failed in mw-dev with: drop database lakekeeper_ben_ext_ice: ERROR: must be owner of database lakekeeper_ben_ext_ice (SQLSTATE 42501) drop role lakekeeper_ben_ext_ice: ERROR: role "lakekeeper_ben_ext_ice" cannot be dropped because some objects depend on it (SQLSTATE 2BP01) EnsureRole runs ALTER DATABASE OWNER TO <role> + ALTER SCHEMA public OWNER TO <role>, so the lakekeeper_<orgid> DB is owned by the per-tenant role, not by the admin. On shared RDS (external metadata case), the admin is a non-superuser tenant of the shared cluster (e.g. ducklingexample), so DROP DATABASE fails with 42501 and the subsequent DROP ROLE fails with 2BP01 because the role still owns the database. DropDatabase: - probe pg_database; no-op if absent - look up the current owner, GRANT it to CURRENT_USER, then ALTER DATABASE OWNER TO CURRENT_USER before DROP DATABASE - GRANT + ALTER OWNER are best-effort so the path stays correct when the admin already is the owner / is a superuser DropRole: - probe pg_roles; no-op if absent - GRANT the role to CURRENT_USER so REASSIGN OWNED + DROP OWNED can run on shared RDS (the operations require role-membership in PG14+) - REASSIGN OWNED first (handles owned database objects), then DROP OWNED CASCADE (cluster-wide grants + default privileges), then DROP ROLE. The first two are best-effort; only DROP ROLE failure is returned.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #649. The drop wired in there hit a real-world ownership
mismatch in mw-dev:
```
drop database lakekeeper_ben_ext_ice: ERROR: must be owner of
database lakekeeper_ben_ext_ice (SQLSTATE 42501)
drop role lakekeeper_ben_ext_ice: ERROR: role
"lakekeeper_ben_ext_ice" cannot be dropped because some objects
depend on it (SQLSTATE 2BP01)
```
`EnsureRole` hands ownership to the per-tenant role
(`ALTER DATABASE OWNER TO ` + `ALTER SCHEMA public OWNER TO`),
so the `lakekeeper_` DB is owned by the tenant, not by the
admin. On shared RDS (external metadata case), the admin is a
non-superuser tenant of the shared cluster
(`ducklingexample`), so:
still owns the database.
Fix
DropDatabase:
`ALTER DATABASE OWNER TO CURRENT_USER` before `DROP DATABASE`
when the admin is already the owner or is a superuser
DropRole:
`DROP OWNED` can run on shared RDS (PG14+ requires role-membership)
`DROP OWNED CASCADE` (cluster-wide grants + default privileges),
then `DROP ROLE`. The first two are best-effort; only the final
`DROP ROLE` failure is returned
🤖 Generated with Claude Code