Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add SOCKS proxy support #195

Merged
merged 3 commits into from
May 4, 2022
Merged

Conversation

Tenzer
Copy link
Contributor

@Tenzer Tenzer commented Mar 30, 2022

This copies the implementation from the Redshift provider: brainly/terraform-provider-redshift#19.

It allows users to proxy the requests to the database server through a SOCKS proxy, in case the database server cannot be reached directly.

I have only added it for the "postgres" as it seems to be more cumbersome to do with the AWS and GCP specific drivers, since they wrap the low-level PostgreSQL driver multiple times.

I'd like feedback on if this would be a welcome addition. If so, I'll add some documentation about how to configure this (like the Redshift provider has).

Relates to #184.

@aokomorowski
Copy link

I've managed to test your changes in practice. It works according to the instruction from the Redshift provider. Note that the proxy package doesn't support HTTP_PROXY and HTTPS_PROXY like Terraform does, but with ALL_PROXY and NO_PROXY, it works like a charm 🤌

@Tenzer
Copy link
Contributor Author

Tenzer commented Apr 4, 2022

For those interested, I have pushed a version of the provider with this patch at https://registry.terraform.io/providers/Tenzer/postgresql/1.15.0-proxy. It should make it easier to test out.

I don't intend to maintain a fork and would instead hope this feature could be incorporated here.

Copy link
Owner

@cyrilgdn cyrilgdn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello,

Thanks for the PR, I allowed myself to merge master as it fixes failing golangci-lint.

I don't know by heart the interface for sql/pq but I trust you and the tests 👍

Could you just add in the usage documentation how to use it (more to show that it's possible)?

Thanks in advance.

This copies the implentation from the Redshift provider:
brainly/terraform-provider-redshift#19.

It allows users to proxy the requests to the database server through a SOCKS
proxy, in case the database server cannot be reached directly.
As part of this I found out the format of the values in the environment
variables isn't standardized, so there only place we potentially could link to
about the format of the values would be the source code for
`golang.org/x/net/proxy`. I thought that was a bit much, so I've instead
included a simple example instead.
@Tenzer
Copy link
Contributor Author

Tenzer commented Apr 21, 2022

@cyrilgdn I've added a couple of lines about it to that document now. I rebased the branch on top of master to get the latest fixes in and removed the merge commit.

My workplace has been using this patch since I made the PR, and it's working for us.

@Tenzer Tenzer requested a review from cyrilgdn April 21, 2022 08:28
@cyrilgdn
Copy link
Owner

cyrilgdn commented May 4, 2022

Thanks again for your work 👏

@cyrilgdn cyrilgdn merged commit f0d493d into cyrilgdn:master May 4, 2022
@aokomorowski aokomorowski mentioned this pull request May 4, 2022
@cyrilgdn
Copy link
Owner

cyrilgdn commented May 8, 2022

FYI, this has just been released in v1.16.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants