Feat/role concurrency safety #440
Merged
Conversation
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
… advisory locks - Add IF NOT EXISTS pre-checks before CREATE ROLE/USER operations - Add IF NOT EXISTS pre-checks before GRANT role membership operations - Add useLocks option (default: false) for optional advisory locking - Harden exception handling for CREATE ROLE/USER: - Handle and ignore: 42710 (duplicate_object), 23505 (unique_violation) - Surface: 42501 (insufficient_privilege) - Harden exception handling for GRANT role membership: - Handle and ignore: 23505 (unique_violation) - Handle gracefully: 42704 (undefined_object) - log notice and continue - Surface: 42501 (insufficient_privilege), 0LP01 (invalid_grant_operation) - Harden exception handling for REVOKE/DROP ROLE: - Ignore: 42704 (undefined_object) - Surface: 2BP01 (dependent_objects_still_exist), 55006 (object_in_use), 42501 (insufficient_privilege) Files modified: - pgpm/core/src/init/client.ts - pgpm/core/src/init/sql/bootstrap-roles.sql - pgpm/core/src/init/sql/bootstrap-test-roles.sql - postgres/pgsql-test/src/admin.ts Co-Authored-By: Dan Lynch <pyramation@gmail.com>
Explains when static SQL files vs dynamic TypeScript helpers are used: - bootstrap-roles.sql: CLI bootstrap of base roles (anonymous, authenticated, administrator) - bootstrap-test-roles.sql: CLI creation of test users (app_user, app_admin) - PgpmInit.bootstrapDbRoles(): CLI creation of custom users with provided credentials - PgpmInit.removeDbRoles(): CLI removal of users - DbAdmin.createUserRole(): Test harness user creation with configurable role names - DbAdmin.grantRole(): Flexible role granting for tests Also documents: - Role model (base roles vs users) - Call paths for each entry point - Role name configuration in pgsql-test - Concurrency safety patterns and error handling Co-Authored-By: Dan Lynch <pyramation@gmail.com>
…onfigurable role names - Add shared role management module in @pgpmjs/core/src/roles/index.ts - Export SQL generators: generateCreateBaseRolesSQL, generateCreateUserSQL, generateCreateTestUsersSQL, generateCreateUserWithGrantsSQL, generateGrantRoleSQL, generateRemoveUserSQL - Update PgpmInit to use shared module with optional RoleMapping parameter - Update DbAdmin to import from @pgpmjs/core instead of duplicating SQL generation - All operations support configurable role names (defaults to canonical: anonymous, authenticated, administrator) - Update ROLES.md to document unified architecture Co-Authored-By: Dan Lynch <pyramation@gmail.com>
- Add sqlLiteral() helper for safe SQL string literal escaping - Declare role names as PL/pgSQL variables at the start of DO blocks - Use variables consistently in EXISTS checks and EXECUTE format() calls - Never concatenate role names directly into SQL identifiers - All role names now properly parameterized via variables Co-Authored-By: Dan Lynch <pyramation@gmail.com>
- Remove DEFAULT_ROLES constant, use pgpmDefaults.db.roles instead - Add TestUserOptions interface for configurable test user credentials - generateCreateTestUsersSQL now accepts optional testUsers parameter - Test user defaults sourced from pgpmDefaults.db.connection - Remove static SQL files (bootstrap-roles.sql, bootstrap-test-roles.sql) - Update ROLES.md documentation to reflect dynamic implementation Co-Authored-By: Dan Lynch <pyramation@gmail.com>
The bootstrap-roles.sql and bootstrap-test-roles.sql files were deleted as role management is now fully dynamic. Remove the copy:sql commands that were copying these files to dist. Co-Authored-By: Dan Lynch <pyramation@gmail.com>
…resolved values - Remove getRoleMapping, getTestUserDefaults, TestUserOptions from core - Add ResolvedRoleMapping and ResolvedTestUserCredentials interfaces - Update SQL generators to accept explicit resolved values (no defaults) - Add TestUserCredentials type with app and admin fields - Add connections field to pgpmDefaults with app/admin credentials - Add env var parsing for DB_CONNECTIONS_APP_* and DB_CONNECTIONS_ADMIN_* - Update CLI commands to use getConnEnvOptions() from @pgpmjs/env - Callers now responsible for merging via deepmerge (env + defaults + overrides) Co-Authored-By: Dan Lynch <pyramation@gmail.com>
… ! assertions - Delete ResolvedRoleMapping and ResolvedTestUserCredentials interfaces - SQL generators now accept RoleMapping and TestUserCredentials with ! assertions - PgpmInit methods accept RoleMapping and TestUserCredentials (optional fields) - CLI commands pass db.roles! and db.connections! directly (no fallbacks) - All ?? 'anonymous' / ?? 'app_user' etc. fallbacks eliminated - Callers use getConnEnvOptions() which merges defaults via deepmerge Co-Authored-By: Dan Lynch <pyramation@gmail.com>
…, remove deprecated connection field - Add useLocks to PgTestConnectionOptions and pgpmDefaults.db - Delete RoleManagementOptions interface from @pgpmjs/core - Update SQL generators to accept useLocks as boolean parameter - Update PgpmInit and DbAdmin methods to use useLocks boolean - Remove deprecated connection field from PgTestConnectionOptions - Update pgsql-test/connect.ts to use connections.app instead of connection - Update supabase-test/connect.ts to remove connection usage Co-Authored-By: Dan Lynch <pyramation@gmail.com>
- Renamed useLocks to useAdvisoryLocks in PgTestConnectionOptions and pgpmDefaults - Updated all SQL generators to use useAdvisoryLocks parameter - Updated PgpmInit and DbAdmin methods to use useAdvisoryLocks - The name clearly describes the PostgreSQL feature being used (pg_advisory_xact_lock) Co-Authored-By: Dan Lynch <pyramation@gmail.com>
- Renamed to useLocksForRoles to make the domain (roles) explicit - Updated all SQL generators, PgpmInit, and DbAdmin methods Co-Authored-By: Dan Lynch <pyramation@gmail.com>
- Restored the app connection config that was deleted when removing the deprecated connection field - Now properly sets connections.app.user/password from PGUSER/PGPASSWORD or Supabase defaults - Preserves user overrides for connections.app Co-Authored-By: Dan Lynch <pyramation@gmail.com>
…test - Added deepmerge as direct dependency - Refactored getConnections() to use deepmerge.all() pattern like pgpm/env - Properly sets connections.app with Supabase credentials (user/password) - Precedence: Supabase defaults -> env vars -> user overrides Co-Authored-By: Dan Lynch <pyramation@gmail.com>
- Removed messy pgEnvOverrides/dbEnvOverrides intermediate objects - pgEnvVars already only includes defined keys, so pass directly to deepmerge - Much cleaner code that matches the pgpm pattern Co-Authored-By: Dan Lynch <pyramation@gmail.com>
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.
No description provided.