Fix env cascading when invoking psql #97
Conversation
Currently we override most PG* environment variables, but there's no good reason for this--if the user has set something, we should not require them to override this via the URL. With this change, they still *can* if they need to do so explicitly, but we shouldn't clobber valid various from the environment with empty ones. We keep PGSSLMODE as a setting users cannot override (to avoid unencrypted connections to remote databases), but account for unset hosts (which defaults to a local connection).
Codecov Report
@@ Coverage Diff @@
## master #97 +/- ##
==========================================
+ Coverage 95.94% 95.96% +0.02%
==========================================
Files 42 42
Lines 986 992 +6
==========================================
+ Hits 946 952 +6
Misses 40 40
Continue to review full report at Codecov.
|
|
I think this fixes #85 as well (you'll be able to either set a |
|
Looks legit! I didn't test it though. +1 |
|
@uhoh-itsmaciek I see at least one use-case where one would need to override The hostname is then considered remote (it's usually a different container than the one running the app, thus usually in Docker virtual network), but cannot be access neither via Unix socket nor with SSL 😢 I'd be happy to provide a PR that makes Unless you have another workaround I didn't think about? |
|
@michaelbaudino yeah, okay, I think that's safe enough. Heroku Postgres will refuse unencrypted connections anyway, so it's not much of an issue. You plan to just move PGSSLMODE here, correct? That works for me... |
|
Woohoo, thanks, here is the PR: #171 😍 |
All `PG*` environment variables can be overridden when using `pg:pull` or `pg:push` since #97, except `PGSSLMODE`, probably because there was no obvious use case to let users decide to insecurely bypass SSL. But when using a PostgreSQL server in Docker Comnpose using the official Postgres image, it's usually hosted on a different container than the user app and can be reached through Docker virtual network, and thus (rightously) considered remote. Unfortunately, the official Postgres image does not support SSL. It seems like a use case where it is safe to bypass SSL, even though the server is considered remote. And it might be a growing use case since even the official Postgres image do not intend to support SSL: docker-library/postgres#152 Thus I'd love to be able to use `pg:pull` from my app container to load a database in a PG server in another container 😍
Currently we override most PG* environment variables, but there's no
good reason for this--if the user has set something, we should not
require them to override this via the URL.
With this change, they still can if they need to do so explicitly,
but we shouldn't clobber valid various from the environment with empty
ones.
We keep PGSSLMODE as a setting users cannot override (to avoid
unencrypted connections to remote databases), but account for unset
hosts (which defaults to a local connection).
Fixes #92