-
Bug
-
Resolution: Fixed
-
Major
-
Powered by SuggestiMate
https://issues.jenkins-ci.org/browse/JENKINS-6167
The fix submitted for this issue does not take into account the excluded proxy hosts unlike the hudson.remoting.Util.openURLConnection method which implicitly excludes hosts.
A fix using the same method as sun.net.www.protocol.http.HttpURLConnection is available here :
https://github.com/hypnoce/remoting/commit/67dbd966e628c42a0aa4d3cd91a56a00b7f95030
Thanks
[JENKINS-28289] JNLP slave should exclude non proxy hosts when connecting via HTTP proxy
Code changed in jenkins
User: Oleg Nenashev
Path:
src/main/java/hudson/remoting/Engine.java
src/main/java/hudson/remoting/Util.java
http://jenkins-ci.org/commit/remoting/a4cc02d33f35bf7ba04b58a36fc3a683097b7a9e
Log:
Merge pull request #55 from hypnoce/master
JENKINS-28289 Ensure proxy exclusion list is used while getting proxy for JNLP clients
Compare: https://github.com/jenkinsci/remoting/compare/bfbcfb3282d9...a4cc02d33f35
Hello
could this fix be included in the incoming LTS version?
Regads
There has not been a release of the library yet, and that release first needs to be in mainline Jenkins to soak before it can be included in LTS.
As the release candidate for 1.625.3 is next Wednesday, this change is ineligible for inclusion in that release unless someone expedites it by pointing to some urgency.
Maybe towards the next LTS base line for release in January.
Assuming this is resolved and just an oversight in the commit message.
I am rejecting this is a candidate against 1.625.3 as #55 was not released (jenkinsci/remoting) and therefore not in Jenkins core. #62 was not merged yet.
danielbeck, Is it really resolved if none of the changes are in core until today?
Good question. In general, resolved does not mean released as it's the commit that matters. Of course with library/framework changes, this means it may never make it into the software as the component version also needs to be increased.
Hi, please ignore #62 as #55 has been redone and merged.
Would appreciate if jenkinsci/remoting could release a new version.
danielbeck Any prospects for this fix to be released via a new jenkinsci/remoting module?
Just for clarification:
how do we set the proxy exclusion list with this patch? Is the environment variable "no_proxy" enought, or do we have to provide a "-Dhttp.nonProxyHosts" java system property when starting the jenkins slave?
Looking forward a new release in a LTS version
geoffroyjabouley, I hope I have not confused you. This was rejected for LTS inclusion again as the component was not released and integrated in Jenkins mainline.
remoting 2.54 including the required fix was released 3 days ago but not yet merged into master
PR to upgrade remoting in master: https://github.com/jenkinsci/jenkins/pull/2054
Another attempt to get this into core https://github.com/jenkinsci/jenkins/pull/2074
Hello
do you think this fix will make its way to a jenkins 1.X LTS release before jenkins 2.0?
geoffroyjabouley Not automatically, as it was only merged into 1.653, and the next LTS line is based on 1.651. But now at least there's a chance.
Code changed in jenkins
User: Kohsuke Kawaguchi
Path:
core/src/main/java/jenkins/slaves/DefaultJnlpSlaveReceiver.java
core/src/main/java/jenkins/slaves/JnlpAgentReceiver.java
core/src/main/java/jenkins/slaves/JnlpSlaveAgentProtocol.java
core/src/main/java/jenkins/slaves/JnlpSlaveAgentProtocol2.java
core/src/main/java/jenkins/slaves/JnlpSlaveAgentProtocol3.java
core/src/main/java/jenkins/slaves/JnlpSlaveHandshake.java
pom.xml
http://jenkins-ci.org/commit/jenkins/af1a53d91c4863f27e4fad295911f131beb64b9a
Log:
Merge pull request #2010 from jenkinsci/jnlp3
[FIXED JENKINS-26580][FIXED JENKINS-28289] Activate JNLP3 support
(cherry picked from commit 6d3e05439643097d2f172761ea82a32a857d058a)
Fixing this on master caused JENKINS-33886, not backporting this into LTS.
What was at the beginning a simple patch supposed to fix a regression introduced in march 2015 (it's been a year now!) still cannot be mainlined because someone has decided instead to include it in a much wider story/task of implementing JNLP3 support.
Why not keeping the scope of this fix simple?
Hi,
We've upgraded to the latest LTS 1.651.2 and still experiencing issue with our proxy. The JNLP agent doesn't take in account the environment variable no_proxy nor the system proprerty http.nonProxyHosts. When http_proxy or http.proxyHost are defined it always use the proxy connection.
To my understanding the bug was introduced as a side-effect by PR27 on remoting which proxy implementation was incomplete (it doesn't support exclusions). It actually added proxy to two methods: Launcher:parseJnlpArguments #1 and Engine:run #2.
PR55 only fixed Engine:run, it didn't modify the Util:openURLConnection method which is now called in Launcher:parseJnlpArguments #3 (instead of URL:openConnection). As this method is immediately called after the agent is started when used with system property jnlpUrl, it also immediately failed when the connection timeouts, throws an Exception and exits at #4.
Moreover PR55 fix only works if you use http.proxyHost property: Java reads for you http.nonProxyHosts and will update the list of proxies returned by ProxySelector.getDefault().select(<URI>). But if you use the environment variable, no_proxy is never used.
1. https://github.com/shirosaki/remoting/blob/8c875cbc81d22df898a57ef86e833b41c50470cb/src/main/java/hudson/remoting/Launcher.java#L247
2. https://github.com/shirosaki/remoting/blob/8c875cbc81d22df898a57ef86e833b41c50470cb/src/main/java/hudson/remoting/Engine.java#L169
3. https://github.com/jenkinsci/remoting/pull/55/files?diff=unified
4. https://github.com/jenkinsci/remoting/blob/master/src/main/java/hudson/remoting/Launcher.java#L275
etiennebec To clarify, this issue is fixed as described, but you're using yet another environment variable, and that one isn't used?
I will give you some background. Our network is behind a corporate proxy and thus we need to go through it to access to Internet. That's why we have defined the two standard environment variables, http_proxy and no_proxy, to configure several tools (curl for instance), to use the proxy. The slaves and the master are on the same network so they can communicate directly, that's why we didn't set the system properties http.proxyHost and so on.
We were previously running Jenkins 1.596.3 which uses remoting 2.47, and this version doesn't include the PR27. And with this PR the trouble begins because the slave started to take in account only the first env var I mentioned: http_proxy. no_proxy is never read. And the PR55 which is related to this issue doesn't fix this either. It just introduced the use of ProxySelector which automatically take in account the exception list ... defined by the system property http.nonProxyHosts. And as in the PR27 it reads the http_proxy env var as a fallback, but doesn't use no_proxy at all!
To be more clear, we don't use the proxy features on our side. It's just that the slave started with the PR27 to read some env var it should not (because that's not how the JVM should be configured). So either we need to take in account no_proxy or we remove from the two methods the part which use http_proxy: https://github.com/jenkinsci/remoting/blob/master/src/main/java/hudson/remoting/Util.java#L114 & https://github.com/jenkinsci/remoting/blob/master/src/main/java/hudson/remoting/Util.java#L114.
I've reopened this issue because to my understanding the goal of this issue wasn't just to add the support for http.nonProxyHosts but to correct PR27. Should we create a new issue?
Looks like it has already been reported: https://issues.jenkins-ci.org/browse/JENKINS-32326
I will mark this issue as resolved.
Hi,
any news on that point ? Is the fix provided suitable for a pull request ?
Thanks
François