feat: add services.pgbouncer

[PerchunPak]

Add a module for PgBouncer, a lightweight connection pooler for PostgreSQL.

[PerchunPak]

I couldn't run tests locally, so would appreciate CI (my nix.conf includes a github token)

❯ devenv-run-tests run --only pgbouncer
Running 1 test, 0 skipped

[1/1] Starting: pgbouncer
--------------------------------------------------
Initialized empty Git repository in /tmp/devenv-run-tests-pgbouncerrcfBVl/.git/
Error: Failed to lock inputs: 
       … while updating the flake input 'nixpkgs'

       … while fetching the input 'github:cachix/devenv-nixpkgs/rolling'


       error: connecting to remote 'https://github.com/cachix/devenv-nixpkgs.git': authentication required but no callback set

[domenkozar]

• port option is never declared. basePort = cfg.port; and ports.main.allocate =
  basePort; reference services.pgbouncer.port, but no port option is defined. The
  test sets port = 6666, which throws "option does not exist". See redis.nix for the
  pattern.
• databases type is malformed. types.attrsOf types.str (types.submodule {...})
  passes two args to attrsOf, which takes one. It should be types.attrsOf
  (types.submodule { ... }).
• Submodule options aren't mkOption calls. dbname, host, and port inside the
  submodule are raw attrsets. They need to be wrapped, e.g. dbname = lib.mkOption {
  ... };.
• lib.literalExpression is given attrsets, not strings in both databases.example
  and settings.example. Use a string literal or just example = { ... };.
• Test/module name mismatches: the test sets listen_addresses (module option is
  listen_addr) and port (undefined).

Logic gaps (would fail even after the above):

• databases is declared but never rendered into pgbouncer.ini. The config is built
  only from cfg.settings, so there's no [databases] section and PgBouncer can't route
  to Postgres. The generic pkgs.formats.ini won't produce the key = host=...
  port=... value lines PgBouncer needs here.
• No auth configured. Without auth_type (e.g. trust for the test), connections are
  rejected, so the psql check would fail.
• bind is declared but never used. Either wire it up or drop it.

[PerchunPak]

Oh wow, I really messed this up, sorry. I fixed the tests, addressed all raised issues, and added a custom generator for key=value sections ([databases], [users] and [peers])

[PerchunPak]

Please let me know if I overengineered this

[domenkozar]

yes, there is lib.generators.getINI, we could also adopt our files."...".ini by not creating the file by just referencing the path

[PerchunPak]

I am not sure what you are talking about. I greped nixpkgs, home-manager, and devenv, but couldn't find getINI anywhere

[domenkozar]

Thanks for this — nicely structured module that follows the existing port-allocation conventions, and the option docs are clear. A few things to address before merge:

🔴 Blocking — users option type is wrong

users = lib.mkOption {
  type = types.attrsOf (types.either types.str types.int);
  example = { user1 = { pool_mode = "session"; pool_size = 200; }; };

The type says each user maps to a str/int, but the example and the [users] section semantics map each user to an attrset of settings. So the example itself fails type-checking, and parseKeyValueSections cfg.users calls lib.filterAttrs on a str/int and errors. It should be:

type = types.attrsOf (types.attrsOf (types.either types.str types.int));

The current test only exercises databases, which is why this slipped through — could you add coverage for users and peers too?

🟠 port = 0 (Unix socket) path is unreachable

The port description says port 0 makes PgBouncer listen on a Unix socket only, but ports.main.allocate = cfg.port auto-allocates the first free port at or above the base, so listen_port always ends up non-zero. Either special-case port == 0 (skip allocation, set listen_port = 0) or drop that line from the docs.

🟠 Overlapping config paths

settings is types.anything and any attrs-valued key in it becomes its own ini section, while there are also dedicated databases/users/peers options appended as separate sections. Setting e.g. peers in both produces two [peers] sections. Worth picking one mechanism (the dedicated options) and not advertising section-attrs inside settings, or documenting precedence.

Minor

• .test.sh hardcodes ports 5555/6666, but those are base values — if occupied, the allocator picks different ports and the test breaks. A comment noting this would help.
• Empty databases/users/peers still emit a header + blank line; lib.optionalString (section != "") keeps the output clean.
• The test uses auth_type = "trust" — fine for CI, but maybe show scram-sha-256 in the option example so users aren't nudged toward trust.

Once the users type is fixed (plus a test for it) and the port = 0 doc/behavior is reconciled, this looks good to go. 🙏

[PerchunPak]

settings is types.anything and any attrs-valued key in it becomes its own ini section,

That was intentional, because anything that is not an attrset, technically is in the global section (later it is moved to the [pgbouncer] section). This is just a convenience thing, but I can remove all that logic. Also, changed type to attrsOf anything, which is slightly more accurate. I don't think writing every possible type is necessary, because later all of that gets parsed by pkgs.formats.ini anyway.

could you add coverage for users and peers too?

Added a user and peers definition, but I couldn't think of a practical way to test it through bash (it accepts limited amount of arguments, that are invisible to the client). As for the peers, it is an advanced option, and it would require a lot more setup (including multiple pgbouncer instances).

not advertising section-attrs inside settings, or documenting precedence.

While I did not see any other section, I would also like to try to keep future-compatibility. databases, users and peers are only separated because they use custom syntax. Added an assertion to prevent double defined sections.

.test.sh hardcodes ports 5555/6666, but those are base values — if occupied, the allocator picks different ports and the test breaks. A comment noting this would help.

That is also my concern, devenv sometimes fails to cleanup after failed tests. But this is how every test for postgres works, and a couple of others, so I don't know what to do here. I'd prefer to stick with the existing patterns in the codebase

Empty databases/users/peers still emit a header + blank line; lib.optionalString (section != "") keeps the output clean.

Fixed, thanks for catching

The test uses auth_type = "trust" — fine for CI, but maybe show scram-sha-256 in the option example so users aren't nudged toward trust.

I couldn't make tests work with real auth (it always asks interactively for a password), so had to use trust. scram-sha-256 is already in the option example