Skip to content

fix(cli): check for existing tenant roles before seeding - #9573

Open
ryanchou1994 wants to merge 2 commits into
logto-io:masterfrom
ryanchou1994:ryanchou1994-fix-seed-role-conflict
Open

fix(cli): check for existing tenant roles before seeding#9573
ryanchou1994 wants to merge 2 commits into
logto-io:masterfrom
ryanchou1994:ryanchou1994-fix-seed-role-conflict

Conversation

@ryanchou1994

Copy link
Copy Markdown

Summary

Dropping a Logto database leaves its PostgreSQL roles in the cluster. Seeding a new database with the same name then fails with a role-already-exists error.

Check the three tenant roles needed by the seed before creating any tables. If any already exist, report their names and explain why they survived the database deletion so an administrator can inspect them before retrying. Existing roles and permissions are left unchanged.

Fixes #9555.

Testing

  • Unit tests: all 20 CLI tests passed, including the role query and conflict message.
  • Integration tests: tested locally with the built CLI and an isolated PostgreSQL 17 instance. Covered a clean seed, dropping and recreating the database, and leftover default-only or admin-only roles. Conflict cases stopped before creating tables and left existing roles unchanged.
  • The full monorepo integration suite was not run.

Checklist

  • .changeset
  • unit tests
  • integration tests
  • necessary TSDoc comments

Copilot AI lite review requested due to automatic review settings September 9, 2026 07:19

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions github-actions Bot added the bugfix label Sep 9, 2026
@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown

COMPARE TO master

Total Size Diff 📈 +4.93 KB

Diff by File
Name Diff
.changeset/quiet-roles-seed.md 📈 +414 Bytes
packages/cli/src/commands/database/seed/index.ts 📈 +331 Bytes
packages/cli/src/queries/database.test.ts 📈 +2.45 KB
packages/cli/src/queries/database.ts 📈 +1.75 KB

Comment thread packages/cli/src/queries/database.ts Outdated
Comment thread packages/cli/src/commands/database/seed/tables.ts Outdated
@ryanchou1994

Copy link
Copy Markdown
Author

Thanks, both make sense. I'll derive the role names from createTenantDatabaseMetadata() (one call per tenant, to avoid the extra throwaway password) and move the pre-check up into seedByPool() before pool.transaction(...), so tables.ts goes back to how it was. Pushing shortly.

…e transaction

- derive the role names from `createTenantDatabaseMetadata()` so the check keeps
  matching if the naming scheme ever changes
- run the check at the top of `seedByPool()` before opening the transaction;
  `createTables()` goes back to its original shape
Copilot AI review requested due to automatic review settings September 10, 2026 10:42

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions github-actions Bot added size/m and removed size/m labels Sep 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

bug: db seed fails with error "logto_tenant_logto" already exists

5 participants