add http proxy support for tornado #29322

mrproper · 2015-12-02T03:42:07Z

the tornado backend lacks support for http proxies from environment variables.

This patch changes the client method to curl if proxies are configured (curl_httpclient is required for proxy_host and proxy_port).

This adds new minion config parameters:
proxy_host
proxy_port

I’m undecided if we should attempt to read in the environment variables for $protocol_proxy or just configure it as a parameter.

This fixes several issues where proxies are in use that use the file.managed module such as:
#23617

It works towards also resolving these issues:
#21985
#8177

cachedout · 2015-12-02T14:39:07Z

Hi @mrproper. It looks like there are a number of lint errors here. Could you please take a look? https://jenkins.saltstack.com/job/salt-pr-lint-n/11348/violations/

jfindlay · 2015-12-02T16:04:12Z

requirements/base.txt

@@ -6,3 +6,4 @@ requests>=1.0.0
 tornado>=4.2.1
 # Required by Tornado to handle threads stuff.
 futures>=2.0
+pycurl>=7.19.3


We need to have a larger discussion here before adding another base dependency to salt.

@thatch45, @meggiebot, @dmurphy18, @UtahDave

There is NO WAY we are going to add another dep, this has been a nightmare

We can certainly find a comprimise here. I recommend that we keep these configuration options while issuing a warning to the user on start that the necessary libraries be installed in order to facilitate this functionality. Salt's http utility library should be made to work in either case, preferring the pycurl implementation if it is installed on the system.

…be optional unless proxy_host/port is set

mrproper · 2015-12-02T22:11:09Z

Interesting, when installing salt on ubuntu, python-tornado has a dep on python-pycurl anyway.

I guess this is a larger question, should we be using curl_httpclient anyway? tornado seems to recommend it over simple?

I have adjusted the pr to have it as an optional dep unless you set proxy_host/port
Im wondering if i also need to add host exclusions (ie dont proxy for these hosts) similar to no_proxy environment variables

jfindlay · 2015-12-03T01:25:27Z

@mrproper, thanks for your updates.

cachedout · 2015-12-03T16:23:43Z

salt/utils/http.py

-        client_argspec = inspect.getargspec(tornado.simple_httpclient.SimpleAsyncHTTPClient.initialize)
+        client_argspec = None
+
+        proxy_host = opts.get('proxy_host', None)


get() will return None by default if the lookup fails, FWIW.

This is true however i was being explicit, can change in another pr if youd like

It's fine. Just an FYI in case you weren't aware. :]

cachedout · 2015-12-03T16:25:47Z

@mrproper Thanks for making these changes. I think this is much improved. Adding host exclusions would be good, but let's do it in an additional PR so we can get this one merged today. Thanks for your hard work here. It's really appreciated.

add http proxy support for tornado

mrproper · 2015-12-03T20:45:53Z

thanks everyone for the comments and the quick turn around!

nlm · 2015-12-04T17:27:50Z

@mrproper I agree with you on using curl_httpclient in all cases. Now it's a required dependency, it's recommended by the authors (http://www.tornadoweb.org/en/stable/httpclient.html) and it would make the code simpler

mrproper · 2015-12-07T23:48:56Z

It would make sense to use curl_httpclient everywhere that tornado is used, however thats up to the community to discuss a new required dependency, even though debian derivatives have it as a dep due to the debian package python-tornado, its currently not a dep on anything else not using distribution package managers (ie solely from pip etc)

hoonetorg · 2016-03-22T10:51:39Z

@cachedout:
This fix works on 2015.8.7 if proxy settings are made in minion conf and original http.py is overwritten with the one from this commit.
2 questions:
When will this fix be merged into which stable version of salt?

Which is the last stable version of salt where proxy support is not broken (afaiu: which uses libcurl instead of tornado)

hoonetorg · 2016-04-06T12:34:37Z

Copy http.py from this PR over a debian jessie install gave me the following problems:

downloaded file has wrong checksum. checksum is always d41d8cd98f00b204e9800998ecf8427e.

I use this feature mainly for archive.extracted and file managed. Downloaded file has always checksum d41d8cd98f00b204e9800998ecf8427e as said above.

Here is the example output of downloading a warfile from a reposerver via proxy:

[INFO    ] Running state [/var/lib/tomcat7/webapps/warfile.war] at time 12:46:29.450401
[INFO    ] Executing state file.managed for /var/lib/tomcat7/webapps/warfile.war
[DEBUG   ] Reading configuration from /etc/salt/minion
[DEBUG   ] Including configuration from '/etc/salt/minion.d/_schedule.conf'
[DEBUG   ] Reading configuration from /etc/salt/minion.d/_schedule.conf
[DEBUG   ] Including configuration from '/etc/salt/minion.d/f_defaults.conf'
[DEBUG   ] Reading configuration from /etc/salt/minion.d/f_defaults.conf
[DEBUG   ] Using cached minion ID from /etc/salt/minion_id: tserver.example.org
[DEBUG   ] Requesting URL http://reposerver.example.org/warfiles/warfile.war using GET method
[DEBUG   ] GET http://reposerver.example.org/warfiles/warfile.war
[DEBUG   ] Hostname was NOT found in DNS cache
[DEBUG   ] Trying 192.168.1.254...
[DEBUG   ] Connected to 192.168.1.254 (192.168.1.254) port 3128 (#0)
[DEBUG   ] > GET http://reposerver.example.org/warfiles/warfile.war HTTP/1.1
[DEBUG   ] > Host: reposerver.example.org
[DEBUG   ] > Accept: */*
[DEBUG   ] > Accept-Encoding: gzip,deflate
[DEBUG   ] > Proxy-Connection: Keep-Alive
[DEBUG   ] > User-Agent: Salt/2015.8.8.2 http.query()
[DEBUG   ] >
[DEBUG   ] < HTTP/1.1 200 OK
[DEBUG   ] Server nginx is not blacklisted
[DEBUG   ] < Server: nginx
[DEBUG   ] < Date: Wed, 06 Apr 2016 11:46:35 GMT
[DEBUG   ] < Content-Type: application/java-archive
[DEBUG   ] < Content-Length: 35831096
[DEBUG   ] < Last-Modified: Fri, 13 Nov 2015 08:35:00 GMT
[DEBUG   ] < Accept-Ranges: bytes
[DEBUG   ] < X-Cache: MISS from fproxy.example.org
[DEBUG   ] < X-Cache-Lookup: MISS from fproxy.example.org:3128
[DEBUG   ] < X-Cache: MISS from proxy.example.org
[DEBUG   ] < X-Cache-Lookup: MISS from proxy.example.org:3128
[DEBUG   ] < Via: 1.1 fproxy.example.org (squid/3.3.8), 1.1 proxy.example.org (squid/3.4.8)
[DEBUG   ] < Connection: keep-alive
[DEBUG   ] <
[DEBUG   ] Connection #0 to host 192.168.1.254 left intact
[ERROR   ] File changed:
New file
[INFO    ] Completed state [/var/lib/tomcat7/webapps/warfile.war] at time 12:46:38.947663

----------
          ID: app__appwarfile
    Function: file.managed
        Name: /var/lib/tomcat7/webapps/warfile.war
      Result: False
     Comment: File sum set for file /var/lib/tomcat7/webapps/warfile.war of <sum1> does not match real sum of d41d8cd98f00b204e9800998ecf8427e
     Started: 12:46:29.450401
    Duration: 9497.262 ms
     Changes:
              ----------
              diff:
                  New file

Versions:

salt-call --versions-report
Salt Version:
           Salt: 2015.8.8.2

Dependency Versions:
         Jinja2: 2.7.3
       M2Crypto: Not Installed
           Mako: Not Installed
         PyYAML: 3.11
          PyZMQ: 14.4.0
         Python: 2.7.9 (default, Mar  1 2015, 12:57:24)
           RAET: Not Installed
        Tornado: 4.2.1
            ZMQ: 4.0.5
           cffi: 0.8.6
       cherrypy: Not Installed
       dateutil: 2.2
          gitdb: Not Installed
      gitpython: Not Installed
          ioflo: Not Installed
        libgit2: Not Installed
        libnacl: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.4.2
   mysql-python: 1.2.3
      pycparser: 2.10
       pycrypto: 2.6.1
         pygit2: Not Installed
   python-gnupg: Not Installed
          smmap: Not Installed
        timelib: Not Installed

System Versions:
           dist: debian 8.3 
        machine: x86_64
        release: 3.16.0-4-amd64
         system: debian 8.3

jfindlay · 2016-04-21T01:02:26Z

@rallytime, can this be backported to 2015.5?

rallytime · 2016-04-22T19:27:49Z

I don't really want to backport this to 2015.5, but I think 2015.8 is reasonable.

jfindlay · 2016-04-22T19:41:20Z

@rallytime, thanks.

* add http proxy support for tornado * add proxy_username/password and doc for the minion * add pycurl deps for tornado * fix lint issues * remove hard coded requirements for pycurl, change curl_httpclient to be optional unless proxy_host/port is set * fix lint

mrproper added 3 commits December 2, 2015 14:36

add http proxy support for tornado

63a8319

add proxy_username/password and doc for the minion

a2c6592

add pycurl deps for tornado

88aed49

cachedout added the Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged label Dec 2, 2015

jfindlay added Transport Core relates to code central or existential to Salt pending-changes The pull request needs additional changes before it can be merged labels Dec 2, 2015

jfindlay reviewed Dec 2, 2015
View reviewed changes

jfindlay removed the Transport label Dec 2, 2015

mrproper added 2 commits December 3, 2015 07:50

fix lint issues

c5dcad0

remove hard coded requirements for pycurl, change curl_httpclient to …

54ed237

…be optional unless proxy_host/port is set

fix lint

bc34d05

cachedout reviewed Dec 3, 2015
View reviewed changes

cachedout pushed a commit that referenced this pull request Dec 3, 2015

Merge pull request #29322 from nodeintegration/develop

bbf1556

add http proxy support for tornado

cachedout merged commit bbf1556 into saltstack:develop Dec 3, 2015

This was referenced Dec 8, 2015

minion daemon fails to install packages when environment requires http proxy #21985

Closed

Support http_proxy/https_proxy for sources #8177

Closed

rallytime mentioned this pull request Apr 22, 2016

Back-port #29322 to 2015.8 #32785

Merged

rallytime added the ZZZ[Done]-back-ported-bf RETIRED The pull request has been back-ported to an older branch. label Apr 22, 2016

KarlVogel mentioned this pull request Jun 8, 2016

Tornado backend doesn't support 'noproxy' #33849

Closed

mrproper mentioned this pull request Jun 29, 2016

pkgrepo does not support http_proxy when its an apt repository #34360

Closed

The-Loeki mentioned this pull request Oct 18, 2016

file.managed with proxy broken in 2015.5 #23617

Closed

jfindlay mentioned this pull request Oct 18, 2016

Document proxy settings #37076

Merged

Oloremo mentioned this pull request Nov 4, 2019

Tornado minion http proxy connection issues #55192

Closed

Oloremo mentioned this pull request Dec 10, 2019

pip state doesn't support salt global proxy #55593

Closed

jmoffitt mentioned this pull request Nov 16, 2022

[DOCS] No supporting documentation exists for use of http_proxy / https_proxy #63069

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add http proxy support for tornado #29322

add http proxy support for tornado #29322

mrproper commented Dec 2, 2015

cachedout commented Dec 2, 2015

jfindlay Dec 2, 2015

thatch45 Dec 2, 2015

cachedout Dec 2, 2015

mrproper commented Dec 2, 2015

jfindlay commented Dec 3, 2015

cachedout Dec 3, 2015

mrproper Dec 3, 2015

cachedout Dec 3, 2015

cachedout commented Dec 3, 2015

mrproper commented Dec 3, 2015

nlm commented Dec 4, 2015

mrproper commented Dec 7, 2015

hoonetorg commented Mar 22, 2016

hoonetorg commented Apr 6, 2016

jfindlay commented Apr 21, 2016

rallytime commented Apr 22, 2016

jfindlay commented Apr 22, 2016

add http proxy support for tornado #29322

add http proxy support for tornado #29322

Conversation

mrproper commented Dec 2, 2015

cachedout commented Dec 2, 2015

jfindlay Dec 2, 2015

Choose a reason for hiding this comment

thatch45 Dec 2, 2015

Choose a reason for hiding this comment

cachedout Dec 2, 2015

Choose a reason for hiding this comment

mrproper commented Dec 2, 2015

jfindlay commented Dec 3, 2015

cachedout Dec 3, 2015

Choose a reason for hiding this comment

mrproper Dec 3, 2015

Choose a reason for hiding this comment

cachedout Dec 3, 2015

Choose a reason for hiding this comment

cachedout commented Dec 3, 2015

mrproper commented Dec 3, 2015

nlm commented Dec 4, 2015

mrproper commented Dec 7, 2015

hoonetorg commented Mar 22, 2016

hoonetorg commented Apr 6, 2016

jfindlay commented Apr 21, 2016

rallytime commented Apr 22, 2016

jfindlay commented Apr 22, 2016