-
Notifications
You must be signed in to change notification settings - Fork 92
feat(ocrvs-10450): legacy user migration #11048
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This comment has been minimized.
This comment has been minimized.
92eaf05 to
639c890
Compare
bfef61b to
6a8ead3
Compare
Basically this would allow us to create endpoints that don't require auth only IF we want. Currently we do not have any yet but after the legacy users are removed, we will be deprecating the |
|
Your environment is deployed to https://ocrvs-10450.e2e-k8s.opencrvs.dev |
Countryconfig will be the entire owner of this service.
|
@makelicious I've put the legacy user migration under the This allows us to easily switch back to the official postgres image once the migrations have run successfully. |
| }) | ||
|
|
||
| /** @knipignore */ | ||
| export const publicProcedure = t.procedure |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Initially I planned on creating a public endpoint to run the migration but afterwards moved the migration to the "migration" package. Still retaining the public procedure as the seed endpoint would potentially need it after the refactor.
| @@ -0,0 +1,43 @@ | |||
| -- Up Migration | |||
| CREATE TYPE status_type AS ENUM( | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea I've been there too. Let's enforce it with zod schema then
| ); | ||
|
|
||
| CREATE TABLE users( | ||
| id uuid PRIMARY KEY DEFAULT gen_random_uuid(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, kinda forgot to add it in
| async function setupPostgresServer() { | ||
| return new PostgreSqlContainer('postgres:17.6') | ||
| return new PostgreSqlContainer( | ||
| 'docker.io/chumaky/postgres_mongo_fdw:17.6_fdw5.5.2' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We mainly need it for our legacy data migration on 2.0. Perhaps we can switch back to official images in 2.1 and remove these mongo related migration files. Do you see any constraints on going with that route?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually yes, they are dependent on the timestamps and I've also had to change the timestamps if newer migrations were merged into develop first. But that only applies if all the migrations are tracked using single migration table. I've created a separate one for the superuser ones:
| --migrations-table="$migrations_table" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
formatting changes, can be ignored
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is basically a duplicate of the one defined in the events migration, to ensure the users table exists no matter which migrations run first, events or migrate-legacy-users.
|
|
||
| GRANT SELECT, INSERT, UPDATE, DELETE ON users TO ${EVENTS_DB_USER}; | ||
|
|
||
| ALTER TABLE users OWNER TO ${EVENTS_MIGRATION_USER}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changing owner to the migration user as this runs as SUPERUSER
makelicious
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! You might get better review regarding .sh changes from someone else.
migrate-legacy-usersopencrvs/opencrvs-countryconfig#1172