Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

2019.2 External file_tree pillar is broken with salt-ssh #53620

Open
kevinhore opened this issue Jun 27, 2019 · 14 comments
Open

2019.2 External file_tree pillar is broken with salt-ssh #53620

kevinhore opened this issue Jun 27, 2019 · 14 comments
Labels
Bug broken, incorrect, or confusing behavior Salt-SSH severity-high 2nd top severity, seen by most users, causes major problems
Projects
Milestone

Comments

@kevinhore
Copy link

kevinhore commented Jun 27, 2019

Description of Issue

Having configured a file tree external pillar (without templating enabled), salt-ssh call results in the following trace:

[ERROR   ] TypeError encountered executing state.apply: Object of type bytes is not JSON serializable
Traceback (most recent call last):
  File "/Users/user1/Library/Python/3.7/lib/python/site-packages/salt/client/ssh/__init__.py", line 1159, in run_wfunc
    result = self.wfuncs[self.fun](*self.args, **self.kwargs)
  File "/Users/user1/Library/Python/3.7/lib/python/site-packages/salt/client/ssh/wrapper/state.py", line 513, in apply_
    return highstate(**kwargs)
  File "/Users/user1/Library/Python/3.7/lib/python/site-packages/salt/client/ssh/wrapper/state.py", line 705, in highstate
    roster_grains)
  File "/Users/user1/Library/Python/3.7/lib/python/site-packages/salt/client/ssh/state.py", line 193, in prep_trans_tar
    salt.utils.json.dump(pillar, fp_)
  File "/Users/user1/Library/Python/3.7/lib/python/site-packages/salt/utils/json.py", line 121, in dump
    return json_module.dump(obj, fp, **kwargs)  # future lint: blacklisted-function
  File "/usr/local/Cellar/python/3.7.3/Frameworks/Python.framework/Versions/3.7/lib/python3.7/json/__init__.py", line 179, in dump
    for chunk in iterable:
  File "/usr/local/Cellar/python/3.7.3/Frameworks/Python.framework/Versions/3.7/lib/python3.7/json/encoder.py", line 431, in _iterencode
    yield from _iterencode_dict(o, _current_indent_level)
  File "/usr/local/Cellar/python/3.7.3/Frameworks/Python.framework/Versions/3.7/lib/python3.7/json/encoder.py", line 405, in _iterencode_dict
    yield from chunks
  File "/usr/local/Cellar/python/3.7.3/Frameworks/Python.framework/Versions/3.7/lib/python3.7/json/encoder.py", line 405, in _iterencode_dict
    yield from chunks
  File "/usr/local/Cellar/python/3.7.3/Frameworks/Python.framework/Versions/3.7/lib/python3.7/json/encoder.py", line 438, in _iterencode
    o = _default(o)
  File "/usr/local/Cellar/python/3.7.3/Frameworks/Python.framework/Versions/3.7/lib/python3.7/json/encoder.py", line 179, in default
    raise TypeError(f'Object of type {o.__class__.__name__} '
TypeError: Object of type bytes is not JSON serializable

This does not occur when using Python2.7.

Setup

Setup pillar.file_tree

ext_pillar:
  - file_tree:
        root_dir: file-tree
        follow_dir_links: False
        keep_newline: True

Place binary file in file_tree.

Steps to Reproduce Issue

Attempt to apply state using salt-ssh.

Versions Report

Salt Version:
           Salt: 2019.2.0

Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: 2.7.3
      docker-py: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
          ioflo: Not Installed
         Jinja2: 2.10.1
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: 1.0.12
   msgpack-pure: Not Installed
 msgpack-python: 0.6.1
   mysql-python: Not Installed
      pycparser: Not Installed
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 3.7.3 (default, Mar 27 2019, 09:23:15)
   python-gnupg: Not Installed
         PyYAML: 3.13
          PyZMQ: 18.0.1
           RAET: Not Installed
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.5.3
            ZMQ: 4.3.1

System Versions:
           dist:
         locale: UTF-8
        machine: x86_64
        release: 18.6.0
         system: Darwin
        version: 10.14.5 x86_64
@Akm0d Akm0d added Bug broken, incorrect, or confusing behavior help-wanted Community help is needed to resolve this P4 Priority 4 Salt-SSH labels Jul 2, 2019
@Akm0d Akm0d modified the milestones: Blocked, Approved Jul 2, 2019
@ghost
Copy link

ghost commented Nov 5, 2019

Hello,

same issue here.
We are using the pillar_ldap external pillar, which explicitly return bytes as per the documentation of the python-ldap library it's using: https://www.python-ldap.org/en/latest/bytes_mode.html#what-s-text-and-what-s-bytes

By default, Salt is using the Python json module.
However, there's code available to use another module instead.

I have a quick and dirty hack that use the ujson module instead in the Salt-SSH client which is working fine with bytestrings.

ret = salt.utils.json.dumps({'local': {'return': result}}, _json_module=ujson)

Maybe a nice way to handle this would be to be able to set a default json module for Salt instead of the standard one (in the configuration file, for example).

Or simply handle more properly bytestring in the JSON Utils module: https://github.com/saltstack/salt/blob/master/salt/utils/json.py

If there's a preferred method, I'm willing to try for a PR :)

Versions Report

Salt Version:
           Salt: 2019.2.2
 
Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: 2.6.1
      docker-py: Not Installed
          gitdb: 2.0.3
      gitpython: 2.1.8
          ioflo: Not Installed
         Jinja2: 2.10
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.5.6
   mysql-python: Not Installed
      pycparser: Not Installed
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 3.6.8 (default, Oct  7 2019, 12:59:55)
   python-gnupg: 0.4.1
         PyYAML: 3.12
          PyZMQ: 16.0.2
           RAET: Not Installed
          smmap: 2.0.3
        timelib: Not Installed
        Tornado: 4.5.3
            ZMQ: 4.2.5
 
System Versions:
           dist: Ubuntu 18.04 bionic
         locale: UTF-8
        machine: x86_64
        release: 4.15.0-48-generic
         system: Linux
        version: Ubuntu 18.04 bionic

@stale
Copy link

stale bot commented Jan 7, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.

@stale stale bot added the stale label Jan 7, 2020
@kevinhore
Copy link
Author

Still a problem.

@stale
Copy link

stale bot commented Jan 7, 2020

Thank you for updating this issue. It is no longer marked as stale.

@stale stale bot removed the stale label Jan 7, 2020
@zenitraM
Copy link

This same issue happens when using !!binary YAML syntax on a pillar and then try to use salt-ssh against it.
According to the docs this is not "allowed" on salt-ssh, though - wonder if this issue is one of its causes.

@h0jeZvgoxFepBQ2C
Copy link

This also affects me, raising the error while parsing this:

set-timezone-vienna:
  timezone.system:
    - name: "Europe/Berlin"
    - utc: True

@sagetherage sagetherage removed the P4 Priority 4 label Jun 3, 2020
@djneades
Copy link

djneades commented Jun 26, 2020

Using an ext_pillar of type file_tree appears to be fundamentally broken with salt-ssh running under Python 3, and not just with binary files as implied by the issue summary. The file_tree implementation appears to be loading each file in the file tree as a chunk of bytes, and these objects cannot subsequently be serialized.

Having looked at the file_tree implementation, I believe I have a workaround (for text files, at least). By declaring that templates are allowed in the file tree’s files, and also declaring a default renderer (e.g. jinja), a working code path in the file_tree implementation is used instead of the broken one. Thus, using an ext_pillar definition along the following lines appears to work:

ext_pillar:
  - file_tree:
        root_dir: some-path-here
        follow_dir_links: False
        keep_newline: True
        render_default: jinja
        template: True

Naturally, this workaround requires that the files in the file tree not contain anything that may be inadvertently processed as a Jinja template.

@kevinhore If the above seems plausible to you, perhaps you could update this issue summary accordingly, as it presently may be underplaying the magnitude of the problem? TIA.

@kevinhore kevinhore changed the title Python3 salt-ssh fails to initialize when file_tree pillar contains binary file Python3 salt-ssh fails to initialize when file_tree pillar is configured to not use templating Jun 26, 2020
@ericjmcalvin
Copy link

Hello,

same issue here.
We are using the pillar_ldap external pillar, which explicitly return bytes as per the documentation of the python-ldap library it's using: https://www.python-ldap.org/en/latest/bytes_mode.html#what-s-text-and-what-s-bytes

By default, Salt is using the Python json module.
However, there's code available to use another module instead.

I have a quick and dirty hack that use the ujson module instead in the Salt-SSH client which is working fine with bytestrings.

ret = salt.utils.json.dumps({'local': {'return': result}}, _json_module=ujson)

Maybe a nice way to handle this would be to be able to set a default json module for Salt instead of the standard one (in the configuration file, for example).

Or simply handle more properly bytestring in the JSON Utils module: https://github.com/saltstack/salt/blob/master/salt/utils/json.py

If there's a preferred method, I'm willing to try for a PR :)

Versions Report

Salt Version:
           Salt: 2019.2.2
 
Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: 2.6.1
      docker-py: Not Installed
          gitdb: 2.0.3
      gitpython: 2.1.8
          ioflo: Not Installed
         Jinja2: 2.10
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.5.6
   mysql-python: Not Installed
      pycparser: Not Installed
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 3.6.8 (default, Oct  7 2019, 12:59:55)
   python-gnupg: 0.4.1
         PyYAML: 3.12
          PyZMQ: 16.0.2
           RAET: Not Installed
          smmap: 2.0.3
        timelib: Not Installed
        Tornado: 4.5.3
            ZMQ: 4.2.5
 
System Versions:
           dist: Ubuntu 18.04 bionic
         locale: UTF-8
        machine: x86_64
        release: 4.15.0-48-generic
         system: Linux
        version: Ubuntu 18.04 bionic

I was also having this issue with salt-ssh with pillar_ldap so I have tried your hack and it seems to work for me. I did:

  1. install python3-ujson (on the salt-ssh master)
  2. modified /usr/lib/python3.6/site-packages/salt/client/ssh/__init__.py as follows:
@@ -21,6 +21,7 @@
 import binascii
 import sys
 import datetime
+import ujson
 
 # Import salt libs
 import salt.output
@@ -1174,7 +1175,8 @@
         if isinstance(result, dict) and 'local' in result:
             ret = salt.utils.json.dumps({'local': result['local']})
         else:
-            ret = salt.utils.json.dumps({'local': {'return': result}})
+            #ret = salt.utils.json.dumps({'local': {'return': result}})
+            ret = salt.utils.json.dumps({'local': {'return': result}}, _json_module=ujson)
         return ret, retcode
 
     def _cmd_str(self):

Was there any other locations I should be doing this?

@kevinhore kevinhore changed the title Python3 salt-ssh fails to initialize when file_tree pillar is configured to not use templating External file_tree pillar is broken with salt-ssh Jul 9, 2020
@kevinhore
Copy link
Author

#53620 (comment) shows that the current implementation of the file_tree external pillar is attempting to load each file as bytes which can't then be serialized.

Updated title to emphasize that file_tree is fundamentally broken.

@h0jeZvgoxFepBQ2C
Copy link

Please fix this, we can't use salt now for weeks due to this bug :(

@h0jeZvgoxFepBQ2C
Copy link

Here is my stacktrace:

...
[DEBUG   ] Rendered data from file: /etc/salt/tmp/cache/master/files/base/openssh/init.sls:


openssh:

  pkg.latest:
    - name: openssh-server

  service.running:
    - enable: True
    - name: ssh

    - require:
      - pkg: openssh-server

  require:
    - user: deployuser

[DEBUG   ] Results of YAML rendering:
OrderedDict([('openssh', OrderedDict([('pkg.latest', [OrderedDict([('name', 'openssh-server')])]), ('service.running', [OrderedDict([('enable', True)]), OrderedDict([('name', 'ssh')]), OrderedDict([('require', [OrderedDict([('pkg', 'openssh-server')])])])]), ('require', [OrderedDict([('user', 'deployuser')])])]))])
[PROFILE ] Time (in seconds) to render '/etc/salt/tmp/cache/master/files/base/openssh/init.sls' using 'yaml' renderer: 0.00039505958557128906
[ERROR   ] TypeError encountered executing state.apply: Object of type bytes is not JSON serializable
Traceback (most recent call last):
  File "/Users/user/.pyenv/versions/3.7.7/lib/python3.7/site-packages/salt-3001_154_g45efc4c142-py3.7.egg/salt/client/ssh/__init__.py", line 1240, in run_wfunc
    result = self.wfuncs[self.fun](*self.args, **self.kwargs)
  File "/Users/user/.pyenv/versions/3.7.7/lib/python3.7/site-packages/salt-3001_154_g45efc4c142-py3.7.egg/salt/client/ssh/wrapper/state.py", line 504, in apply_
    return sls(mods, **kwargs)
  File "/Users/user/.pyenv/versions/3.7.7/lib/python3.7/site-packages/salt-3001_154_g45efc4c142-py3.7.egg/salt/client/ssh/wrapper/state.py", line 226, in sls
    roster_grains,
  File "/Users/user/.pyenv/versions/3.7.7/lib/python3.7/site-packages/salt-3001_154_g45efc4c142-py3.7.egg/salt/client/ssh/state.py", line 203, in prep_trans_tar
    salt.utils.json.dump(pillar, fp_)
  File "/Users/user/.pyenv/versions/3.7.7/lib/python3.7/site-packages/salt-3001_154_g45efc4c142-py3.7.egg/salt/utils/json.py", line 126, in dump
    return json_module.dump(obj, fp, **kwargs)  # future lint: blacklisted-function
  File "/Users/user/.pyenv/versions/3.7.7/lib/python3.7/json/__init__.py", line 179, in dump
    for chunk in iterable:
  File "/Users/user/.pyenv/versions/3.7.7/lib/python3.7/json/encoder.py", line 431, in _iterencode
    yield from _iterencode_dict(o, _current_indent_level)
  File "/Users/user/.pyenv/versions/3.7.7/lib/python3.7/json/encoder.py", line 405, in _iterencode_dict
    yield from chunks
  File "/Users/user/.pyenv/versions/3.7.7/lib/python3.7/json/encoder.py", line 438, in _iterencode
    o = _default(o)
  File "/Users/user/.pyenv/versions/3.7.7/lib/python3.7/json/encoder.py", line 179, in default
    raise TypeError(f'Object of type {o.__class__.__name__} '
TypeError: Object of type bytes is not JSON serializable
[DEBUG   ] LazyLoaded nested.output
myserver:
    TypeError encountered executing state.apply: Object of type bytes is not JSON serializable

@Rudd-O
Copy link

Rudd-O commented Jan 27, 2021

As of 3002.2, this is broken with regular client/server Salt, it just manifests in a different way. We detected it because we use the Salt API server — salt-call pillar.items returns everything fine, but any program trying to call the exact same thing via the Salt API server receives an HTTP 500 error because json cannot serialize type bytes.

This is a horrible regression and now we must keep using 3001 with the security issues it has.

@Rudd-O
Copy link

Rudd-O commented Jan 27, 2021

Interestingly, note that if I enable rendering of the pillars for file_tree, the issue goes away, because the render process / code stringifies the bytes object read from disk, instead of passing it straight through.

@Rudd-O
Copy link

Rudd-O commented Jan 27, 2021

This is the source of the problem:

https://github.com/saltstack/salt/blob/master/salt/pillar/file_tree.py#L233

            contents = b""
            try:
                with salt.utils.files.fopen(file_path, "rb") as fhr:
                    buf = fhr.read(__opts__["file_buffer_size"])
                    while buf:
                        contents += buf
                        buf = fhr.read(__opts__["file_buffer_size"])
                    if contents.endswith(b"\n") and _check_newline(
                        prefix, file_name, keep_newline
                    ):
                        contents = contents[:-1]

@sagetherage sagetherage changed the title External file_tree pillar is broken with salt-ssh 2019.2 External file_tree pillar is broken with salt-ssh May 18, 2021
@sagetherage sagetherage added severity-high 2nd top severity, seen by most users, causes major problems and removed help-wanted Community help is needed to resolve this labels May 18, 2021
@sagetherage sagetherage added this to To do in Salt-SSH via automation May 18, 2021
@sagetherage sagetherage moved this from To do to Severity-Critical or High in Salt-SSH May 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug broken, incorrect, or confusing behavior Salt-SSH severity-high 2nd top severity, seen by most users, causes major problems
Projects
Salt-SSH
  
Severity-Critical or High
Development

No branches or pull requests

8 participants