Allow installing migrations into existing schema

[joelmukuthu]

Mostly for improved security. One postgres user could be used for creating the schema (and managing database objects, including pgcrypto), while Graphile worker runs with a different user with the appropriate privileges within it's schema. Also might allow for separation of concerns if, for instance, database objects are managed by a separate team.

This was previously requested in https://github.com/graphile/worker/issues/12, but I'm opening a new issue since the schema-name is already configurable, making this more feasible.

[benjie]

Looks like it should already work if you create the schema and the initial version of the migrations table:

create schema graphile_worker;
create table graphile_worker.migrations(
  id int primary key,
  ts timestamptz default now() not null
);

Is this sufficient for your needs? If so, perhaps you'd be good enough to test it works, and if so raise a PR against the README with the instructions/explanation?

[joelmukuthu]

Thanks for the suggestion, that does indeed work. However, I feel like it's too much burden to place on the user especially for this particular use-case. Ideally, I'd like to:

1. Create a postgres user for Graphile worker
2. Create a schema for it
3. Allow the Graphile user to create objects in that schema and that schema only.

Having to know about (the structure of) and add the migrations table feels like it's out of scope for this, especially if the above tasks are handled by a team of database admins while the application specific stuff is handled by a different team of developers.

Do you have reservations against updating the code to do create schema if not exists graphile_worker; instead (along with some docs)?

[benjie]

I sort of agree, though very few people have requested this. CREATE SCHEMA IF NOT EXISTS will fail if the user doesn’t have CREATE permissions (whether or not the schema exists), I think, so the actual solution would have to be a little more complex, and the cost of that solution would be incurred on every single start up, which makes me sad. I guess we could split it into multiple statements and issue a try/catch around create schema if not exists...

[joelmukuthu]

CREATE SCHEMA IF NOT EXISTS will fail if the user doesn’t have CREATE permissions (whether or not the schema exists)

Indeed, I wasn't aware of that. I'm okay running with the earlier solution you proposed to keep things simple and can submit a PR adding some notes about it to the README as well.

We've also considered managing migrations ourselves but that would require "non-breaking" migrations - referring to making database changes through a series of releases. Does this project take this approach currently?

[benjie]

We currently run these migrations in order:

https://github.com/graphile/worker/tree/main/sql

and write the results to the aforementioned migrations table. The full migration code is this small file:

https://github.com/graphile/worker/blob/main/src/migrate.ts

[joelmukuthu]

Thanks for the pointers @benjie. Running the migrations ourselves solves this use-case. Closing.

[benjie]

Awesome; glad you’ve got a solution you’re relatively happy with 👍

[joelmukuthu]

It works quite well indeed. Thanks again :blush:

[Zakinator123]

I sort of agree, though very few people have requested this. CREATE SCHEMA IF NOT EXISTS will fail if the user doesn’t have CREATE permissions (whether or not the schema exists), I think, so the actual solution would have to be a little more complex, and the cost of that solution would be incurred on every single start up, which makes me sad. I guess we could split it into multiple statements and issue a try/catch around create schema if not exists...

I would like to put in a request for this try/catch kind of logic to be added - unfortunately Azure Database for PostgreSQL flexible server does not seem to allow the pseudo-administrator role they provide to create schemas, so my graphile worker is failing I think due to this line: https://github.com/graphile/worker/blob/main/src/migrate.ts#L44C1-L45C1

The failure occurs even when I configure graphile worker to use the public schema, since as you mentioned CREATE SCHEMA IF NOT EXISTS will fail even if the schema already exists if you do not have CREATE SCHEMA permission.

I would be happy to open a PR to resolve this. @benjie

[benjie]

We already have try/catch behavior for this, it's caught here:

https://github.com/graphile/worker/blob/c0a3d814832f3b5f7d49fc51c5ef0e8da70178ea/src/migrate.ts#L153-L163

The issue I guess is that you're getting an error code that we don't specifically allow.

I'd be open to a PR that allows the user to specify error codes to ignore in addition to CLASH_CODES (named something like: additionalInstallSchemaIgnoredErrorCodes). This setting should only exist in the preset:

https://github.com/graphile/worker/blob/c0a3d814832f3b5f7d49fc51c5ef0e8da70178ea/src/index.ts#L119-L120