feat: custom role capabilities for pgtestdb user

[peterldowns]

kodergarten forked pgtestdb just to alter the role that is used to create the databases to be SUPERUSER so that they can install postgis in a migration, which requires superuser privileges.

I checked with their team and they'd be interested in having support for this mainlined.

This PR

feature changes

• Adds constants for the username, password, and capabilities that are used to create the pgtestdb role by default.
• Allows defining a custom Role that would be created instead.

test changes

• Tests that this would allow installing postgis by running tests against the postgis/postgis docker image.
• Tests that the default behavior does not change from previous versions of pgtestdb.

docs changes

• Updates the documentation to explain how the default role is used and can be overridden.

implementation changes

• pgtestdb previously assumed a single role would be used, and cached the result of the "get-or-create" sql commands so that it would only ever run them once per test-package. Now, because multiple roles with multiple capabilities can be used in different tests in the same package, it caches the results with the Role.Username as the cache key.
• Functions accept a new TB interface that is a subset of testing.TB to make it easier to "check that a bad migration would cause the tests to fail".

additional improvements

• The nix dev shell is upgraded to include newer versions of all development tools, motivated by wanting to upgrade to a newer version of gopls.
• Subsequent fixes to the linting configuration: depguard has been removed.
• Minor bugfixes in the migrator used in tests, wasn't causing any issues but the new linters detected two places where a session lock was acquired but subsequent statements were executed in separate connection from the one that had acquired the lock. This has no impact on correctness, but helps reduce the number of connections in use when running this package's tests. This has no impact on resource consumption for people who use this library.
• The Justfile test-all command now allows passing arguments, motivated by wanting to pass -count=1 to re-run all the tests and ignore cached results.

What to do about Prepare()?

This work raises the question: shouldn't the Prepare() method from the Migrator interface have allowed for this? It was originally envisioned as the way to run admin-commands and install extensions as a superuser, separately from running migrations. But as kodergarten noticed, and I realized as a result:

• Prepare() connects to the template database with the same role that is used to run the migrations and connect to each test database.
  • This means that if your app runs as NOSUPERUSER in production (which it should), and your pgtestdb config connects to its tests with the same permissions (which it should), you can't install extensions.
  • In the described situation, in production you would install postgis by connecting manually as a superuser and running CREATE EXTENSION postgis;.
  • In the described situation, in tests you just cannot install the postgis extension.
• Prepare() is still useful for installing extensions and running commands that you need to run that haven't been committed to migration files, but it can only ever run commands that could also be run in a migration.
• So, basically, Prepare() is only useful in very select circumstances
  • I added it because for Pipe, there had been some statements we ran manually when we first set up the server, and the migrations depended on those statements having been run, and we didn't want to add a new 0001 migration and re-number all the others.

The two ideas I have for improving this situation are:

• Remove the Prepare() method entirely from the interface. It's current docstrings are misleading, remove it.
  • pro less code more good 🧠 👍
  • con harder (impossible?) to install postgis and other superuser-only extensions.
• Allow specifying capabilities that should be used only when running the Prepare() method.
  • pro this would allow you to run CREATE EXTENSION postgis as a superuser in the Prepare() method
  • pro you could still properly restrict your test connections to be NOSUPERUSER, just like your production connections should be.
  • con this is more configuration, and more complexity to understand, and maybe no one needs this or cares.

For now, the goal is to solve kodergarten's problem, and is worth shipping as long as this works for them. I will continue to think about the role structure and the migrator interface, hopefully I can come up with something nice that solves all concerns.

[timberkeley]

Thanks so much for creating this pull request. Creating a super user TestRole worked fine for us (Kodergarten). We have for now kept the "CREATE EXTENSION postgis;" in Prepare so that we don't need to prepend this into our existing migrations list so I suppose we care about keeping prepare and maybe having a specific role for prepare. I note that this may be supporting an edge case though.

[peterldowns]

@timberkeley Thanks for confirming that this worked for you. I'll merge this shortly so you can use it instead of your custom fork.

We have for now kept the "CREATE EXTENSION postgis;" in Prepare so that we don't need to prepend this into our existing migrations list so I suppose we care about keeping prepare and maybe having a specific role for prepare.

That's also very useful to know. I'll keep Prepare() around, and I'll consider making it possible to give Prepare() a custom role. Not sure when I'll get to this, though, so please let me know if it becomes a more urgent matter.

As an aside, I'm curious, what migration library / Migrator are you using?

Thank you again for being so helpful, for providing great feedback, and for using my library 🙇

[timberkeley]

Hi Peter,
This is the [migrator]((https://bun.uptrace.dev/guide/migrations) we are using. Would you be interested in a pull request for this?

[peterldowns]

@timberkeley very cool, I'm glad I asked because I hadn't heard of Bun before. I'd happily accept a PR, and if you just send me the basics of it I'm happy to help with all the packaging / go.mod stuff necessary to make it look like the other migrators. No rush or urgency on this from my side, and if you don't want to bother just let me know and I can write an implementation myself.