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

remoting processing of "no_proxy" env var needs improvement

    XMLWordPrintable

Details

    • Bug
    • Status: Resolved (View Workflow)
    • Major
    • Resolution: Fixed
    • remoting
    • Seen as far back as v3.7 on Jenkins 2.43, and as recent as v3.15 on Jenkins 2.107

    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

          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 jeffret for the final call there.

          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 jeffret for the final call there.
          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.

          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.
          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

          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
          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

          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
          gmontero Gabe Montero added a comment -

          Yep make sense thanks for the confirmation / quick response oleg_nenashev

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

          People

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

            Dates

              Created:
              Updated:
              Resolved: