Salt: cp.get_url reports 'args' parameter should be dict, list or tuple. Not <type 'NoneType'>

[jinm]

Description of Issue/Question

# salt-call --local cp.get_url "http://somehost/managed/afile"
...
Traceback (most recent call last):
  File "/usr/lib/python2.7/site-packages/salt/cli/caller.py", line 197, in call
    ret['return'] = func(*args, **kwargs)
  File "/usr/lib/python2.7/site-packages/salt/modules/cp.py", line 320, in get_url
    result = _client().get_url(path, dest, makedirs, saltenv)
  File "/usr/lib/python2.7/site-packages/salt/fileclient.py", line 625, in get_url
    **get_kwargs
  File "/usr/lib/python2.7/site-packages/salt/utils/http.py", line 181, in query
    url_full = tornado.httputil.url_concat(url, params)
  File "/usr/lib64/python2.7/site-packages/tornado/httputil.py", line 615, in url_concat
    raise TypeError(err)
TypeError: 'args' parameter should be dict, list or tuple. Not <type 'NoneType'>

From this particular host, wget the same url worked. Apache httpd log did not report any errors. This issue only happens on this host only and happens every time. It could be a host configuration issue but just not sure what by looking at the code.

Same issue with 2011.16.{1,2,3}.

Setup

(Please provide relevant configs and/or SLS files (Be sure to remove sensitive info).)

Steps to Reproduce Issue

(Include debug logs if possible and relevant.)

Versions Report

(Provided by running salt --versions-report. Please also mention any differences in master/minion versions.)

# salt-call --versions-report
Salt Version:
           Salt: 2016.11.3

Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
          ioflo: Not Installed
         Jinja2: 2.8
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.4.8
   mysql-python: Not Installed
      pycparser: Not Installed
       pycrypto: 2.6.1
         pygit2: Not Installed
         Python: 2.7.12 (default, Nov  2 2016, 22:29:13)
   python-gnupg: Not Installed
         PyYAML: 3.11
          PyZMQ: 16.0.2
           RAET: Not Installed
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.5b1
            ZMQ: 4.1.6

System Versions:
           dist: centos 7.1.1503 Core
        machine: x86_64
        release: 3.10.0-229.el7.x86_64
         system: Linux
        version: CentOS Linux 7.1.1503 Core

[aaaanderson]

We're having a similar issue, breaking a state used in production.

The cause appears to be https://github.com/tornadoweb/tornado/pull/1899 - some changes in the Tornado URL handling code seem to break assumptions used in Salt.

Using Tornado 4.4.3 works fine; could we freeze Salt on that version as an interim fix? It looks like right now Salt just gets latest.

I haven't looked into the issue enough to understand whether the real fix should be Tornado-side or Salt-side.

[tokiwinter]

This was causing us issues with a file.managed state that used source_hash. I patched this myself with:

# cd /opt/virtualenvs/salt/lib/python2.7/site-packages/salt/utils
# diff -aN http.py.orig http.py
181c181,184
<     url_full = tornado.httputil.url_concat(url, params)
---
>     if params:
>         url_full = tornado.httputil.url_concat(url, params)
>     else:
>         url_full = url

Side-effects? Not sure, would like to hear them though. Fixed my specific issue with a broken state (retrieve a file over http, and also retrieve the source_hash over http) when moving from 2014.1.5 to 2016.11.1 ...

[jcasner]

We're seeing the same error message with pkgrepo.managed.

Configuration

newrelic-sysmond:
  pkgrepo.managed:
    - humanname: newrelic
    - name: deb http://apt.newrelic.com/debian/ newrelic non-free
    - file: /etc/apt/sources.list.d/newrelic.list
    - key_url: https://download.newrelic.com/548C16BF.gpg
    - refresh_db: True
  pkg.installed:
    - pkgs:
      - newrelic-sysmond

Error

----------
          ID: newrelic-sysmond
    Function: pkgrepo.managed
        Name: deb http://apt.newrelic.com/debian/ newrelic non-free
      Result: False
     Comment: Failed to configure repo 'deb http://apt.newrelic.com/debian/ newrelic non-free': 'args' parameter should be dict, list or tuple. Not <type 'NoneType'>
     Started: 00:56:28.722282
    Duration: 86.965 ms
     Changes:
----------

Salt Versions on failing box:

salt --versions-report
Salt Version:
           Salt: 2016.3.1

Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
          ioflo: Not Installed
         Jinja2: 2.8
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: 0.21.1
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.4.6
   mysql-python: Not Installed
      pycparser: Not Installed
       pycrypto: 2.6.1
         pygit2: Not Installed
         Python: 2.7.12 (default, Nov 19 2016, 06:48:10)
   python-gnupg: Not Installed
         PyYAML: 3.11
          PyZMQ: 15.2.0
           RAET: Not Installed
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.5
            ZMQ: 4.1.4

System Versions:
           dist: Ubuntu 16.04 xenial
        machine: x86_64
        release: 4.4.0-31-generic
         system: Linux
        version: Ubuntu 16.04 xenial

[jinm]

The issue is resolved after rolling back python-tornado-4.5b1-1.x86_64 to python-tornado-4.2.1-1.el7.x86_64 on a CentOS 7.x server.

[gtmanfred]

What repository are you getting the python-tornado from?

You could set the priority of the saltstack repo to higher to use our packaged version of python-tornado.

Thanks,
Daniel

[gtmanfred]

I have reproduced the error, we will get it fixed.

Thanks,
Daniel

[sliceofcode]

Please note that the same issue happens on 2015.8.x as well, it was working fine last week.

[gtmanfred]

We won't be patching 2016.3 because that only receives CVE updates now, and 2015.8 has been end of supported.

You will need to make sure to not install the newest version of tornado. Hopefully tornado will fix this API change as it appears 4.5 is just in beta1.

Also, if you install using the packages, and specify in your repositories that python-tornado should be installed from the salt repository, this will work fine.

Thanks,
Daniel

[gtmanfred]

It looks like tornado is fixing this upstream.

https://github.com/tornadoweb/tornado/pull/2018

[aaaanderson]

Note that I'm not "Tornado" - I might have missed something important in that PR; I've never contributed to that project before. I would definitely still move forward with a Salt-side fix.

[gtmanfred]

That is fair.

We are working on a note to salt-users and to put in the topic of the IRC channel.

It seems peculiar that they wouldn't accept your PR, because there is no reason that params couldn't be None.
Thanks for the assist upstream though!

Daniel

[bdarnell]

It seems peculiar that they wouldn't accept your PR, because there is no reason that params couldn't be None.

Well, the function is clearly documented as taking a list or dict. I'll merge that PR because I'm making a 4.5.1 release anyway, but in general I'm not too keen on preserving backward-compatibility with code that passes None to functions that don't expect it and only supported None by accident.

[bdarnell]

There's a fix for this in the master branch of https://github.com/tornadoweb/tornado; I'm planning to release it as Tornado 4.5.1 tomorrow.

[aaaanderson]

Our states are working fine now. Thanks for the quick release, Ben.

[Ch3LL]

Closing since it has been resolved in #40720 on our end and fixed upstream.