Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-51223

remoting processing of "no_proxy" env var needs improvement

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Resolved (View Workflow)
    • Priority: Major
    • Resolution: Fixed
    • Component/s: remoting
    • Labels:
    • Environment:
      Seen as far back as v3.7 on Jenkins 2.43, and as recent as v3.15 on Jenkins 2.107
    • Similar Issues:

      Description

      The regular expression algorithm in org.jenkinsci.remoting.engine.JnlpAgentEndpointResolver.inNoProxyEnvVar(String)(String) does not match when a single element of a fully qualified hostname and domain is part of the no_proxy setting.

       

      For example, taking a host name of "jenkins.bnd-cd.svc", we tried adding ".svc", "svc", and "*.svc", "*svc", and none of them matched.

       

      We had to specify "bnd-cd.svc" in the no_proxy evn var.

       

      By comparison, in th same env, we were able to run curl with a no_proxy evn var containing ".svc" and it properly matched "jenkins.bnd-cd.svc"

       

      When running in kubernetes/openshift, having to specify "bnd-cd.svc" is problematic as "bnd-cd" corresponds to the project/namespace, and "svc" was the default suffix applied to all service hostnames.  We wanted to have the http_proxy/no_proxy settings be applicable across jenkins running in any project/namespace.

       

      In addition to proving this when running an end to end Jenkins master/agent scenario using the standard jar, we also

      a) took the logic from org.jenkinsci.remoting.engine.JnlpAgentEndpointResolver.inNoProxyEnvVar(String) and put it in a simple main program where we set host and no_proxy accordingly

      b) create debug versions of remoting.jar with additional LOGGER calls to trace the behavior

        Attachments

          Activity

          Hide
          gmontero Gabe Montero added a comment -

          Provided https://github.com/jenkinsci/remoting/pull/269 as a proposal for addressing this.

          Show
          gmontero Gabe Montero added a comment - Provided https://github.com/jenkinsci/remoting/pull/269 as a proposal for addressing this.
          Hide
          scm_issue_link SCM/JIRA link daemon added a comment -

          Code changed in jenkins
          User: gabemontero
          Path:
          src/main/java/hudson/remoting/Util.java
          src/main/java/org/jenkinsci/remoting/engine/JnlpAgentEndpointResolver.java
          src/test/java/hudson/remoting/UtilTest.java
          http://jenkins-ci.org/commit/remoting/0272a4302e5eb3d619530fdb3e1ab4f45663a9b3
          Log:
          JENKINS-51223 catch more domain suffix based no_proxy matches (add curl-like algorithm to existing regexp)

          remove some of the duplicate JnlpAgentEndpointResolver/Util code

          Show
          scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: gabemontero Path: src/main/java/hudson/remoting/Util.java src/main/java/org/jenkinsci/remoting/engine/JnlpAgentEndpointResolver.java src/test/java/hudson/remoting/UtilTest.java http://jenkins-ci.org/commit/remoting/0272a4302e5eb3d619530fdb3e1ab4f45663a9b3 Log: JENKINS-51223 catch more domain suffix based no_proxy matches (add curl-like algorithm to existing regexp) remove some of the duplicate JnlpAgentEndpointResolver/Util code
          Hide
          scm_issue_link SCM/JIRA link daemon added a comment -

          Code changed in jenkins
          User: Jeff Thompson
          Path:
          src/main/java/hudson/remoting/Util.java
          src/main/java/org/jenkinsci/remoting/engine/JnlpAgentEndpointResolver.java
          src/test/java/hudson/remoting/UtilTest.java
          http://jenkins-ci.org/commit/remoting/510dd2b4d1180276060e8c4c3684ae1fc5c0b030
          Log:
          Merge pull request #269 from gabemontero/no-proxy

          JENKINS-51223 catch more domain suffix based no_proxy matches (add …

          Compare: https://github.com/jenkinsci/remoting/compare/3cc5305ac33a...510dd2b4d118
          *NOTE:* This service been marked for deprecation: https://developer.github.com/changes/2018-04-25-github-services-deprecation/

          Functionality will be removed from GitHub.com on January 31st, 2019.

          Show
          scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Jeff Thompson Path: src/main/java/hudson/remoting/Util.java src/main/java/org/jenkinsci/remoting/engine/JnlpAgentEndpointResolver.java src/test/java/hudson/remoting/UtilTest.java http://jenkins-ci.org/commit/remoting/510dd2b4d1180276060e8c4c3684ae1fc5c0b030 Log: Merge pull request #269 from gabemontero/no-proxy JENKINS-51223 catch more domain suffix based no_proxy matches (add … Compare: https://github.com/jenkinsci/remoting/compare/3cc5305ac33a...510dd2b4d118 * NOTE: * This service been marked for deprecation: https://developer.github.com/changes/2018-04-25-github-services-deprecation/ Functionality will be removed from GitHub.com on January 31st, 2019.
          Hide
          scm_issue_link SCM/JIRA link daemon added a comment -

          Code changed in jenkins
          User: Oleg Nenashev
          Path:
          core/src/main/java/hudson/slaves/SlaveComputer.java
          pom.xml
          http://jenkins-ci.org/commit/jenkins/c405ae701c4bca18f7b4edc6e5cd3bc053b7ee60
          Log:
          [JENKINS-51541,JENKINS-51551,...] - Remoting 3.21 + Allow passing custom CommandTransport implementations to SlaveComputer from ComputerLauncher (#3455)

          • JENKINS-51541 - Introduce new SlaveComputer#setChannel() method which takes custom ChannelBuilder and CommandTransport
          • JENKINS-51541 - Listener is nullable according to the documentation
          • [JENKINS-51551,JENKINS-51223,JENKINS-50965] - Update Remoting to 3.21
          • JENKINS-51541 - Restrict SlaveComputer#setChannel(ChannelBuilder cb, …) to Beta-use only

          *NOTE:* This service been marked for deprecation: https://developer.github.com/changes/2018-04-25-github-services-deprecation/

          Functionality will be removed from GitHub.com on January 31st, 2019.

          Show
          scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Oleg Nenashev Path: core/src/main/java/hudson/slaves/SlaveComputer.java pom.xml http://jenkins-ci.org/commit/jenkins/c405ae701c4bca18f7b4edc6e5cd3bc053b7ee60 Log: [JENKINS-51541,JENKINS-51551,...] - Remoting 3.21 + Allow passing custom CommandTransport implementations to SlaveComputer from ComputerLauncher (#3455) JENKINS-51541 - Introduce new SlaveComputer#setChannel() method which takes custom ChannelBuilder and CommandTransport JENKINS-51551 - Pick Remoting version with the API patch JENKINS-51541 - Listener is nullable according to the documentation [JENKINS-51551,JENKINS-51223,JENKINS-50965] - Update Remoting to 3.21 JENKINS-51541 - Restrict SlaveComputer#setChannel(ChannelBuilder cb, …) to Beta-use only * NOTE: * This service been marked for deprecation: https://developer.github.com/changes/2018-04-25-github-services-deprecation/ Functionality will be removed from GitHub.com on January 31st, 2019.
          Hide
          oleg_nenashev Oleg Nenashev added a comment -

          Fixed in Remoting 3.21 and Jenkins 2.127

          Show
          oleg_nenashev Oleg Nenashev added a comment - Fixed in Remoting 3.21 and Jenkins 2.127
          Hide
          oleg_nenashev Oleg Nenashev added a comment -

          Gabe Montero I am not sure it can be backported easily, but feel free to add the lts-candidate label if you want it to be backported to 2.121.x

          Show
          oleg_nenashev Oleg Nenashev added a comment - Gabe Montero I am not sure it can be backported easily, but feel free to add the lts-candidate label if you want it to be backported to 2.121.x
          Hide
          gmontero Gabe Montero added a comment -

          Thanks for the suggestion Oleg Nenashev

           

          I went ahead and added the label.  The sooner we can get the change the better for us (and we would need it in an LTS release).  Based on my work on the PR, where the code path in particular had not changed in a while, I think

          the change should be an easy backport.  But of course I'll defer to Jeff Thompson for the final call there.

          Show
          gmontero Gabe Montero added a comment - Thanks for the suggestion Oleg Nenashev   I went ahead and added the label.  The sooner we can get the change the better for us (and we would need it in an LTS release).  Based on my work on the PR, where the code path in particular had not changed in a while, I think the change should be an easy backport.  But of course I'll defer to Jeff Thompson for the final call there.
          Hide
          jthompson Jeff Thompson added a comment -

          It should be an easy backport. Oleg Nenashev, I'll chat with you about what we need to do.

          Show
          jthompson Jeff Thompson added a comment - It should be an easy backport. Oleg Nenashev , I'll chat with you about what we need to do.
          Hide
          gmontero Gabe Montero added a comment -

          Just saw that Oliver added the 2.121.2 label.

           

          So if I read https://jenkins.io/event-calendar/ correctly, it sounds like a release candidate for that is targeted for today and the LTS release

          is targeted for July 4.  Can anybody in jenkins.io or jenkins-ci.org land confirm that?

           

          thanks

          Show
          gmontero Gabe Montero added a comment - Just saw that Oliver added the 2.121.2 label.   So if I read https://jenkins.io/event-calendar/ correctly, it sounds like a release candidate for that is targeted for today and the LTS release is targeted for July 4.  Can anybody in jenkins.io or jenkins-ci.org land confirm that?   thanks
          Hide
          oleg_nenashev Oleg Nenashev added a comment -

          I would rather expect July 5th. July 4th is a public holiday in US, and I would not anticipate a release on this day

          Show
          oleg_nenashev Oleg Nenashev added a comment - I would rather expect July 5th. July 4th is a public holiday in US, and I would not anticipate a release on this day
          Hide
          gmontero Gabe Montero added a comment -

          Yep make sense thanks for the confirmation / quick response Oleg Nenashev

          Show
          gmontero Gabe Montero added a comment - Yep make sense thanks for the confirmation / quick response Oleg Nenashev

            People

            • Assignee:
              jthompson Jeff Thompson
              Reporter:
              gmontero Gabe Montero
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: