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 proxy support #29

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

evan-kinney
Copy link

Adds proxy support to the getJSON method.

@ljharb
Copy link
Owner

ljharb commented May 31, 2024

This is a lot of complexity for something that I'd expect should be largely transparent to users of this library.

What does npm itself do here for supporting proxies? What's your use case where you don't have direct access to nodejs.org?

@evan-kinney
Copy link
Author

I agree but when your CI/CD pipelines are behind a corporate proxy what's a guy to do? NPM itself uses keys/values stored in the .npmrc file.

@evan-kinney
Copy link
Author

I would like to add this code is a clone from the get-json library with proxy support and uses the https module instead of phin

@ljharb
Copy link
Owner

ljharb commented May 31, 2024

Would it make more sense to PR this support into get-json itself?

@evan-kinney
Copy link
Author

Yes, but that library hasn't been updated in 6 years...

@ljharb
Copy link
Owner

ljharb commented May 31, 2024

true; i see it's now deprecated in the readme, but using fetch isn't an option for me.

why is your corporate proxy on the machine level and not the network level? could this one URL be allowlisted somehow? just trying to think of other solutions that wouldn't require this much churn.

@evan-kinney
Copy link
Author

Our GitLab runners are all hosted as containers in AWS so the proxy is to allow connecting into certain parts of the corporate network and the internet. I believe their idea is to only allow the containers to connect when required and to block all network traffic otherwise. That way they reduce the security risk of having the network itself be routed through the proxy. I agree this is a lot of churn for what it is. I've already gotten what I need out of my fork to create a new package with proxy support for my use case but wanted to try and fold it into the main project since the work is already done.

@ljharb
Copy link
Owner

ljharb commented Jun 18, 2024

I'm now maintaining get-json, so it'd be ideal to add Proxy support there, and then this PR can be repurposed to make use of it.

@ljharb ljharb marked this pull request as draft June 18, 2024 23:44
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.

2 participants