-
Notifications
You must be signed in to change notification settings - Fork 14
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
feat: custom role capabilities for pgtestdb user (#6)
[kodergarten](https://kodergarten.com/) [forked pgtestdb](https://github.com/kodergarten/pgtestdb) just [to alter the role that is used to create the databases](kodergarten/pgtestdb@12d6d21) to be `SUPERUSER` so that they can install [postgis](https://hub.docker.com/r/postgis/postgis) in a migration, which requires superuser privileges. I [checked with their team](kodergarten/pgtestdb#17) 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.
- Loading branch information
1 parent
8080014
commit 7374056
Showing
16 changed files
with
317 additions
and
87 deletions.
There are no files selected for viewing
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
v0.0.12 | ||
v0.0.13 |
This file contains 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
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
Oops, something went wrong.