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 upstream proxy configuration #80

Merged
merged 2 commits into from
Sep 16, 2021
Merged

Add upstream proxy configuration #80

merged 2 commits into from
Sep 16, 2021

Conversation

NeuronAddict
Copy link
Contributor

In organisations with an internal gitlab, github can't be fetched without use an http proxy.

This MR add some values to support upstream proxy support.

I use the same approch as for the other env params, but feel free to give any feedback.

@andrcuns
Copy link
Owner

Thanks for the PR @NeuronAddict!

Which parts of the app consume these variables? Is it dependabot-core that is able to read http_proxy and https_proxy env values?

@NeuronAddict
Copy link
Contributor Author

Thanks for the feedback.

I think it is the ruby http library that use the http_proxy,

The stacktrace when an error occur (without proxy) look like :

[2021-09-15 14:39:08 +0000 tid=1app] WARN: /usr/lib/ruby/2.7.0/net/http.rb:960:in `initialize'
/usr/lib/ruby/2.7.0/net/http.rb:960:in `open'
/usr/lib/ruby/2.7.0/net/http.rb:960:in `block in connect'
/usr/lib/ruby/2.7.0/timeout.rb:95:in `block in timeout'
/usr/lib/ruby/2.7.0/timeout.rb:105:in `timeout'
/usr/lib/ruby/2.7.0/net/http.rb:958:in `connect'
/usr/lib/ruby/2.7.0/net/http.rb:943:in `do_start'
/home/dependabot/app/vendor/bundle/ruby/2.7.0/gems/sentry-ruby-core-4.6.5/lib/sentry/net/http.rb:45:in `do_start'
/usr/lib/ruby/2.7.0/net/http.rb:932:in `start'
/usr/lib/ruby/2.7.0/net/http.rb:1483:in `request'
/home/dependabot/app/vendor/bundle/ruby/2.7.0/gems/sentry-ruby-core-4.6.5/lib/sentry/net/http.rb:38:in `request'
/home/dependabot/app/vendor/bundle/ruby/2.7.0/gems/httparty-0.18.1/lib/httparty/request.rb:145:in `perform'
/home/dependabot/app/vendor/bundle/ruby/2.7.0/gems/httparty-0.18.1/lib/httparty.rb:594:in `perform_request'
/home/dependabot/app/vendor/bundle/ruby/2.7.0/gems/httparty-0.18.1/lib/httparty.rb:508:in `get'
/home/dependabot/app/vendor/bundle/ruby/2.7.0/bundler/gems/gitlab-cfd0d9a988c8/lib/gitlab/request.rb:51:in `block (2 levels) in <class:Request>'
....

So the container use ruby http.

The documentation on https://ruby-doc.org/stdlib-2.7.1/libdoc/net/http/rdoc/Net/HTTP.html (proxies section) say :

Net::HTTP will automatically create a proxy from the http_proxy environment variable if it is present. To disable use of http_proxy, pass nil for the proxy address.

I don't see any mention about this on dependabot-core, just this issue : dependabot/dependabot-core#3348.

Set the http_proxy work, but indeed I don't know if it's intentional.

@andrcuns
Copy link
Owner

andrcuns commented Sep 15, 2021

Got it, thanks! I pushed a small change to make the pipeline pass since the setup requires running helm-doc to generate compatible readme and update all the chart version mentions.

@andrcuns
Copy link
Owner

One thing to note, I'm not sure how to document this. This will only work for ruby processes and maybe some other tools that use same environment variable.

The dependabot-core uses a lot of native helpers for different package managers, so even when setting this proxy, some of the dependency updates that will not read these environment variables, might still fail.

@NeuronAddict
Copy link
Contributor Author

NeuronAddict commented Sep 15, 2021

Hum, you're right. I just test with the maven dependencies system.

Its worth checking out, let me check with another helper and give a feedback as soon as possible.

May be also open an issue on dependabot-core.

PS: See this case on dependabot-script dependabot/dependabot-script#688, but it doesn't say if it always work.

@andrcuns
Copy link
Owner

I will go ahead and merge it since it would still cover at least some cases if not all and it doesn't really introduce any additional changes if not used

@andrcuns andrcuns merged commit 4ad2d19 into andrcuns:master Sep 16, 2021
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