Pass a custom pg connection object

[ivank]

You're using Postgres connection strings, which are more than sufficient for the general use-case, however configuring Slonik to play nice with more special sql offerings is a challenge.

For example connecting to an instance in Google Compute Platform requires this string

socket://username:passwort@/cloudsql/projectId:location:instance-name?db=databse-name

And it gets really complex because https://github.com/iceddev/pg-connection-string does not implement the full specification of https://www.postgresql.org/docs/current/libpq-connect.html#LIBPQ-CONNSTRING

I also haven't found a way to pass TLS options for a socket configuration at all.

My suggestion is to just allow passing of an object that would be used directly by pg.Pool as opposed to passing it through https://github.com/iceddev/pg-connection-string first. I could write a PR but I work mostly with Typescript so I hesitate a bit how to implement it correctly in flow.

I know you are planning on reimplementing pg.Pool yourself but it would seem prudent to keep the same config parameters. That would allow for greater backwards compatibility. I presume you would be keeping Postgres uri syntax as well.

The typescript types are a bit misleading maybe, because they state

export type DatabaseConfigurationType =
    | string
    | {
        database?: string;
        host?: string;
        idleTimeoutMillis?: number;
        max?: number;
        password?: string;
        port?: number;
        user?: string;
    };

but if you pass an object to createPool, you get an error, even leaving aside the fact that there is no TLS config in the types as well.

Slonik Version 19.2.0

[gajus]

These:

export type DatabaseConfigurationType =
    | string
    | {
        database?: string;
        host?: string;
        idleTimeoutMillis?: number;
        max?: number;
        password?: string;
        port?: number;
        user?: string;
    };

are not correct types.

This is the correct configuration, and you can totally use it.

slonik/src/types.js

Lines 75 to 94 in dfd9575

	/**
	* @property captureStackTrace Dictates whether to capture stack trace before executing query. Middlewares access stack trace through query execution context. (Default: true)
	* @property connectionTimeout: Timeout (in milliseconds) after which an error is raised if cannot cannot be established. (Default: 5000)
	* @property idleTimeout Timeout (in milliseconds) after which idle clients are closed. Use 'DISABLE_TIMEOUT' constant to disable the timeout. (Default: 5000)
	* @property interceptors An array of [Slonik interceptors](https://github.com/gajus/slonik#slonik-interceptors).
	* @property maximumPoolSize Do not allow more than this many connections. Use 'DISABLE_TIMEOUT' constant to disable the timeout. (Default: 10)
	* @property minimumPoolSize Add more server connections to pool if below this number. (Default: 1)
	* @property preferNativeBindings Uses libpq bindings when `pg-native` module is installed. (Default: true)
	* @property typeParsers An array of [Slonik type parsers](https://github.com/gajus/slonik#slonik-type-parsers).
	*/
	export type ClientUserConfigurationType = {|
	+captureStackTrace?: boolean,
	+connectionTimeout?: number | 'DISABLE_TIMEOUT',
	+idleTimeout?: number | 'DISABLE_TIMEOUT',
	+interceptors?: $ReadOnlyArray<InterceptorType>,
	+maximumPoolSize?: number,
	+minimumPoolSize?: number,
	+preferNativeBindings?: boolean,
	+typeParsers?: $ReadOnlyArray<TypeParserType>,
	|};

Note, that it is a second parameter.

slonik/src/factories/createPool.js

Lines 23 to 26 in dfd9575

	export default (
	connectionUri: string,
	clientUserConfiguration?: ClientUserConfigurationType
	): DatabasePoolType => {

There is no option to pass anything but DSN as the first parameter.

I am assuming you got that from definitely typed?

It would be worth reporting an issue.

[gajus]

Now with regards of accepting object as connection configuration and extending DSN syntax, I have already raised an issue about it.

#45

I think the proper solution would be to implement utilities createDsn and parseDsn, which would then configure the Pool as needed.

I am opposed to passing a user-provided Pool instance because that highly increases the surface of the API that I need to be concerned with. We are now using only a subset of functionality provided by Pool. Allow to provide a pg Pool instance would imply that we support the entire API as we move forward, which is very unlikely.

[gajus]

With regards to TLS options, depending on if you are using JavaScript driver or native driver, you would configure ?ssl=1 or ?sslmode=require. This inconsistency comes from pg module and it is one of the things that should be made consistent if we implement createDsn/ parseDsn.

#55

If you are talking about configuring a specific private key to use, then no, there is no API for that at all at the moment. It would be added to clientUserConfiguration and it should be used conditionally depending on the presence of the ssl/ sslmode values in the DSN.

[ivank]

10x a lot for the quick response!
createDsn / parseDsn sound like they would definitely help. But looking through https://www.postgresql.org/docs/current/libpq-connect.html#LIBPQ-CONNSTRING specs it sounds way harder to implement all / most of it, rather than support the pg module's config object as well.

And that would give your users a lot more familiarity with Slonik.

The usual flow I've seen for the connection config is "use connection uri first, but when you encounter something unfamiliar, fall back to object", as that is a lot more explicit. That's just a suggestion though :)

Anyway if you added { ssl: { ca, cert, key} } or something to clientUserConfiguration that would be awesome, as with the current config there doesn't seem to be any way to pass TLS. This looks like the accepted way to specify them from different nodejs tooling I've seen. For example https://kafka.js.org/docs/configuration or https://nodejs.org/api/https.html

And using cert file paths in the connection string is rather inconvenient. Those things usually come from some secrets management system that just gives you some env variables to work with, saving them to the filesystem and the passing in the filename sounds a lot more work than just passing them directly to an object.

Is there anything I can do to help?

[gajus]

And using cert file paths in the connection string is rather inconvenient. Those things usually come from some secrets management system that just gives you some env variables to work with, saving them to the filesystem and the passing in the filename sounds a lot more work than just passing them directly to an object.

I have a slight problem with adding additional logic to ClientUserConfigurationType. The problem with the latter is that it requires to re-compile the codebase if changes are needed. This is especially painful when working with Docker/ Kubernetes. Therefore, if anything, I am keen to remove as much as possible.

I think a better approach would be to store CA/ CERT/ KEY in an environment variables.

Leave both of these with me. Need to think more about the former. I will provide a solution to the latter in the next 24 hours.

[biern]

Hi, there's a use case where I think it's impossible to use a connection string. Dynamic passwords, supported by node-postgres (brianc/node-postgres#1926). Our precise use case is using RDS IAM Auth with short lived passwords.
I'm testing a monkey patch to skip connection uri parsing on pool creation if object is passed and it seems to "just work" so far.

[lattwood]

@gajus another reason to bump the priority of this up, many distributed tracing libraries do configuration via the connection object passed.

DataDog/dd-trace-js@5a8b92b

[gajus]

Addressed in #240.

[dbousamra]

Hi all. Is the original connection string actually addressed? I am using Cloud SQL, and also need to connect via a socket conn string. In older versions of Slonik, this worked fine. However, with the new parseDsn function, it seems to error, and no combination of fiddling with the string can i get it to work.

[gajus]

To add support for socket connection you would need to update two utilities:

• parseDsn
• stringifyDsn

and then (you might) need to update createPoolConfiguration.ts to map new properties, if any.

[dbousamra]

@gajus Happy to contribute the change.

However, is there any desire for allowing passing in of the actual connection object, instead of a dsn? Or perhaps, allowing us to pass in the connection string itself, as it was in previous versions?

I.e. allow passing in either format. It just seems a bit limiting to have to go through this dsn abstraction, as it now bottlenecks issues

[gajus]

The goal of this change was to create a predictable DSN parser, because earlier version allowed to pass parameters that were quietly ignored. It will be gradually expanded to support other uses cases such as the one being discussed here.

We can allow to pass an object, though it is even bigger chunk of work than the above.

[RP-3]

@dbousamra Did you ever get manage to connect to Cloud SQL using a socket conn string? I've wasted a day fiddling with with the connection string, came across this issue, and am about to decide to go back to Knex if it turns out there's no way to use Slonik with Cloud SQL.

[gajus]

@dbousamra Did you ever get manage to connect to Cloud SQL using a socket conn string? I've wasted a day fiddling with with the connection string, came across this issue, and am about to decide to go back to Knex if it turns out there's no way to use Slonik with Cloud SQL.

Can you please raise a separate issue identify Cloud SQL connection troubles you have?

I will follow up with a solution.

This issue has been largely ignored because the title misidentifies the problem.

[dbousamra]

@dbousamra Did you ever get manage to connect to Cloud SQL using a socket conn string? I've wasted a day fiddling with with the connection string, came across this issue, and am about to decide to go back to Knex if it turns out there's no way to use Slonik with Cloud SQL.

No I didn't. I ended up just staying on an older version of Slonik.