go-spatial / tegola

Tegola is a Mapbox Vector Tile server written in Go
http://tegola.io/
MIT License
1.3k stars 198 forks source link

Option to avoid setting `default_transaction_read_only` when connecting to PostgreSQL? #966

Closed JackDunnNZ closed 8 months ago

JackDunnNZ commented 10 months ago

I am trying to connect to a postgres instance on AWS RDS that sits behind an RDS Proxy. When I try to connect, I get the following error:

could not register providers: 
failed while creating connection pool: failed to connect to `host=localhost user=postgres database=postgres`: 
server error (FATAL: Feature not supported: RDS Proxy currently doesn’t support the option default_transaction_read_only. (SQLSTATE 0A000))

If I comment out the line that sets this parameter and build, everything works fine.

I am wondering if it might be possible to have a configurable override to avoid setting this parameter so that it's not necessary to patch and build from source to achieve this?

ARolek commented 9 months ago

@JackDunnNZ well that's a bit annoying. Thanks for the report.

We don't currently have a way to config the default RuntimeConfig. The point of this config was to make tegola read only by default as it does not need to make any mutations to the database. Does RDSProxy support a similar query param, or do they just suggest using database roles for this type of control?

JackDunnNZ commented 9 months ago

Thanks, it is super annoying that it doesn't support - I entirely agree the tegola config makes sense!

In certain configurations, RDS Proxy supports marking an entire endpoint as read-only, which provides some of this control. If I had to guess, because the purpose of the proxy is to pool and reuse connections across multiple client connections to the proxy, it cannot set global parameters like default_transaction_read_only on its own connections to the database on a case-by-case basis, since these connections are long-living and might be shared with the next user of the proxy.

ARolek commented 9 months ago

@JackDunnNZ

In certain configurations, RDS Proxy supports marking an entire endpoint as read-only, which provides some of this control. If I had to guess, because the purpose of the proxy is to pool and reuse connections across multiple client connections to the proxy, it cannot set global parameters like default_transaction_read_only on its own connections to the database on a case-by-case basis, since these connections are long-living and might be shared with the next user of the proxy.

I see, yes that would be a direct conflict with the current approach.

Maybe we add a new config option to omit the default runtime params. The main downside in your situation would be the loss of setting the application name on the connection, which I think is really helpful. Looking at the pgx PraseConfig docs it appears the package "ParseConfig closely matches the parsing behavior of libpq". Looking at the libpq docs, 34.1.1. Connection Strings, it appears the param application_name is supported.

Could you do some additional investigation to see if this is a viable solution when working with the RDS Proxy?

JackDunnNZ commented 9 months ago

Could you do some additional investigation to see if this is a viable solution when working with the RDS Proxy?

Sure, happy to. I didn't follow from the message, what exactly would you like me test?

ARolek commented 9 months ago

@JackDunnNZ I'm hoping you can investigate what query params RDS Proxy supports. I suspect they have a whitelist documented somewhere.

JackDunnNZ commented 9 months ago

@ARolek Got it, looks like that whitelist is here: https://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/rds-proxy-setup.html#rds-proxy-connecting-postgresql

ARolek commented 9 months ago

@JackDunnNZ thanks! so it looks like application_name is supported, so I think we can plumb through this config change. Would you want to take a pass at making the code change?

JackDunnNZ commented 9 months ago

@ARolek Normally I would be happy to, but I don't know a single thing about go, so I think it would be better for someone else to handle it sorry.

ARolek commented 9 months ago

@JackDunnNZ fair enough ;-).

iwpnd commented 9 months ago

i gave it a shot and made both application_name and default_transaction_read_only configurable in the provider.

You can either set default_transaction_read_only:"OFF" and it would omit the option from the runtime params of the connection, the option itself can be omitted from the tegola config and we'd resort to the previous default of "TRUE" or it can be set to "FALSE".

while i can test that the runtime params are correctly set, i cannot test their effect. any chance you can checkout my PR, test and let me know? @JackDunnNZ

JackDunnNZ commented 9 months ago

Works perfectly on my end, thanks @iwpnd !