Skip to content
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

Create default roles on reset if they don't exist #179

Open
4 of 6 tasks
psirenny opened this issue Feb 8, 2023 · 3 comments
Open
4 of 6 tasks

Create default roles on reset if they don't exist #179

psirenny opened this issue Feb 8, 2023 · 3 comments

Comments

@psirenny
Copy link

psirenny commented Feb 8, 2023

Feature description

It would be a great developer experience if the Owner and Authenticator roles could be created on reset if they don't already exist. The tool could create the roles automatically or allow developers to do so themselves in hooks.

If the tool handled it automatically, then it could take additional placeholder variables:

{
  "placeholders": {
    "DATABASE_AUTHENTICATOR": "!ENV",
    "DATABASE_AUTHENTICATOR_PASSWORD": "!ENV",
    "DATABASE_OWNER_PASSWORD": "!ENV"
    // …
  }
}

Motivating example

I'm initializing a database for the first time and trying to avoid writing a one-off setup_db.js script by hooking into the graphile-migrate event system.

I tried to create the default roles in an !afterReset.sql script like so:

BEGIN;

DO $$
BEGIN
  CREATE ROLE :DATABASE_OWNER WITH LOGIN PASSWORD :DATABASE_OWNER_PASSWORD SUPERUSER;
  EXCEPTION WHEN duplicate_object THEN RAISE NOTICE '%, skipping', SQLERRM USING ERRCODE = SQLSTATE;
END;
$$;

DO $$
BEGIN
  CREATE ROLE :DATABASE_AUTHENTICATOR WITH LOGIN PASSWORD :DATABASE_AUTHENTICATOR_PASSWORD NOINHERIT;
  EXCEPTION WHEN duplicate_object THEN RAISE NOTICE '%, skipping', SQLERRM USING ERRCODE = SQLSTATE;
END
$$;

…

However, this fails because graphile-migrate attempts to run the script as the Owner before the Owner is created:

graphile-migrate: dropped database 'my_database'
Error: Failed to create database 'my_database' with owner 'my_database_owner': role "my_database_owner" does not exist

I was surprised by the error because I expected graphile-migrate to connect as the root user.

Alternatively, I tried creating the default roles in a !beforeReset.sql script. However, this fails because the beforeReset hook expects the database to exist before it's been created:

error: database "my_database" does not exist

Creating these roles is something all graphile-migrate users have to do, so all of us could benefit from this feature 😄.

Breaking changes

I don't think so.

Supporting development

I [tick all that apply]:

  • am interested in building this feature myself
  • am interested in collaborating on building this feature
  • am willing to help testing this feature before it's released
  • am willing to write a test-driven test suite for this feature (before it exists)
  • am a Graphile sponsor ❤️
  • have an active support or consultancy contract with Graphile
@benjie
Copy link
Member

benjie commented Feb 9, 2023

One of our "opinions" is:

Roles should be managed outside of migrations (since they can be shared between databases)

Managing them in lifecycle scripts is not unreasonable. I personally handle role creation in a separate process, but I can understand the desire to centralize.

beforeReset was intended to be a chance to dump your old DB (or similar) if you wanted to, before it's deleted and a new one created.

I think maybe we want a new hook beforeDatabaseCreate that runs after beforeReset, after the database is dropped, but before the database is recreated and before afterReset.

This would be a special hook since we know that the database doesn't exist; so it wouldn't make sense to pass it the connection string to the database. Instead it should use the rootConnectionString. I'm thinking a !! prefix to indicate this, such as beforeDatabaseCreate:["!!createRoles.sql"].

Would you like to experiment with this approach?

@aakside
Copy link

aakside commented Feb 20, 2023

@benjie what's your separate process to handle role creation? Is it manual?

@benjie
Copy link
Member

benjie commented Feb 20, 2023

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants