saltstack / salt

Software to automate the management and configuration of any infrastructure or application at scale. Get access to the Salt software package repository here:
https://repo.saltproject.io/
Apache License 2.0
14.16k stars 5.48k forks source link

Data Transport is broken #56166

Open brandon-sling opened 4 years ago

brandon-sling commented 4 years ago

Description of Issue

[WARNING ][2649] Data transport is broken, got: , type: str, exception: str indices must be integers, not str, attempt 1 of 3
[WARNING ][2649] Data transport is broken, got: , type: str, exception: str indices must be integers, not str, attempt 2 of 3
[WARNING ][2649] Data transport is broken, got: , type: str, exception: str indices must be integers, not str, attempt 3 of 3
[WARNING ][2649] Data transport is broken, got: , type: str, exception: str indices must be integers, not str, attempt 4 of 3
[ERROR   ][2649] Data transport is broken, got: , type: str, exception: str indices must be integers, not str, retry attempts exhausted

Also see https://github.com/saltstack/salt/issues/48265

This is only seen from the minion side. Issuing the command to run the state from the master, returns

minion_id:
----------
          ID: /etc/filename.conf
    Function: file.managed
      Result: False
     Comment: Source file 'salt://repo/files/filename.conf' not found
     Started: 22:19:26.668777
    Duration: 135.711 ms
     Changes:   

Setup

I don't know if this matters but the master is running gitfs for its fileserver roots config. The file to transfer should not be in the minion's cache.

Steps to Reproduce Issue

Try transferring using a file.managed on a file that has this line: # This should be at least (number_of_channels × 2) to be able to keep the live If the file is already in the minion's cache there isn't an issue.

Versions Report

No differences except master has some pkgs minion doesn't

minion:
    Salt Version:
               Salt: 2019.2.3

    Dependency Versions:
               cffi: Not Installed
           cherrypy: Not Installed
           dateutil: 2.4.2
          docker-py: 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.6
       mysql-python: Not Installed
          pycparser: Not Installed
           pycrypto: 2.6.1
       pycryptodome: Not Installed
             pygit2: Not Installed
             Python: 3.5.2 (default, Jul  5 2016, 12:43:10)
       python-gnupg: 0.3.8
             PyYAML: 3.11
              PyZMQ: 15.2.0
               RAET: Not Installed
              smmap: Not Installed
            timelib: Not Installed
            Tornado: 4.2.1
                ZMQ: 4.1.4

    System Versions:
               dist: Ubuntu 16.04 xenial
             locale: ANSI_X3.4-1968
            machine: x86_64
            release: 4.4.0-34-generic
             system: Linux
            version: Ubuntu 16.04 xenial
master:
    Salt Version:
               Salt: 2019.2.3

    Dependency Versions:
               cffi: 1.13.2
           cherrypy: 8.7.0
           dateutil: 2.4.2
          docker-py: Not Installed
              gitdb: 0.6.4
          gitpython: 1.0.1
              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.6
       mysql-python: 1.3.7
          pycparser: 2.19
           pycrypto: 2.6.1
       pycryptodome: Not Installed
             pygit2: Not Installed
             Python: 3.5.2 (default, Oct  8 2019, 13:06:37)
       python-gnupg: 0.3.8
             PyYAML: 3.11
              PyZMQ: 15.2.0
               RAET: Not Installed
              smmap: 0.9.0
            timelib: Not Installed
            Tornado: 4.2.1
                ZMQ: 4.1.4

    System Versions:
               dist: Ubuntu 16.04 xenial
             locale: ANSI_X3.4-1968
            machine: x86_64
            release: 4.4.0-34-generic
             system: Linux
            version: Ubuntu 16.04 xenial

I was able to fix the issue by removing the unicode character '×' and replacing it with an ascii 'x'

DmitryKuzmenko commented 4 years ago

@brandon-sling I can't reproduce the exactly your issue but could you try to configure the system locale to use .UTF-8 encoding on both machines and re-run your test? Your local is set to ANSI_X3.4-1968 that is actually a 8-bit ASCII encoding that doesn't support unicode.

DmitryKuzmenko commented 4 years ago

Also if you're open for experiments I'll very appreciate if you able to patch your minion with this:

diff --git a/salt/fileclient.py b/salt/fileclient.py
index 718f957c20..57ed5dea3d 100644
--- a/salt/fileclient.py
+++ b/salt/fileclient.py
@@ -1203,6 +1203,8 @@ class RemoteClient(Client):
                     data = data.encode()
                 fn_.write(data)
             except (TypeError, KeyError) as exc:
+                import traceback
+                log.error(traceback.format_exc())
                 try:
                     data_type = type(data).__name__
                 except AttributeError:

Reproduce the issue and share the corresponding log with me.

brandon-sling commented 4 years ago

updating locales on master and minion did not remove the error, but now like the issue I linked it's changed from str to byte in the error Here is the traceback:

[ERROR   ] Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/salt/fileclient.py", line 1165, in get_file
    if not data['data']:
TypeError: byte indices must be integers or slices, not str

[WARNING ] Data transport is broken, got: , type: bytes, exception: byte indices must be integers or slices, not str, attempt 1 of 3
brandon-sling commented 4 years ago

so weirdly I decided to put a log.error(data) just before the try block on line 1164 in fileclient.py and restarted the minion again and this time it worked. It printed the str data and put the file in cache and where it was supposed to go on the system. I restarted the minion after adding the traceback logging lines and still got the error. So some number of restarts after updating locale it finally decided to start working. Trying other minions with the locale not changed yet still has the error some I'm going to chalk it up to that being what the problem was.

stale[bot] commented 4 years ago

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.

ayekat commented 4 years ago

I'm running into this issue with a file that is composed a bit exotically:

The first 400-or-so lines are shell script code, followed by a binary payload. Whenever I attempt to request that file from a minion, I run into the following error on the master:

[ERROR   ] Error in function _serve_file:
Traceback (most recent call last):
  File "/usr/lib/python3.6/site-packages/salt/master.py", line 1839, in run_func
    ret = getattr(self, func)(load)
  File "/usr/lib/python3.6/site-packages/salt/fileserver/__init__.py", line 650, in serve_file
    return self.servers[fstr](load, fnd)
  File "/usr/lib/python3.6/site-packages/salt/fileserver/gitfs.py", line 170, in serve_file
    return _gitfs().serve_file(load, fnd)
  File "/usr/lib/python3.6/site-packages/salt/utils/gitfs.py", line 2857, in serve_file
    data = data.decode(__salt_system_encoding__)
UnicodeDecodeError: 'utf-8' codec can't decode byte 0x8b in position 9447: invalid start byte

The code there looks like this:

if data and six.PY3 and not salt.utils.files.is_binary(fpath):
    data = data.decode(__salt_system_encoding__)

Looking at that is_binary() function then revealed this:

with fopen(path, 'rb') as fp_:
    try:
        data = fp_.read(2048)
        if six.PY3:
            data = data.decode(__salt_system_encoding__)
        return salt.utils.stringutils.is_binary(data)
    except UnicodeDecodeError:
        return True

So essentially, Salt tries to figure out if it's a binary file by only reading the first 2048 bytes (which is by far not sufficient for the file I'm dealing with), and falsely detects it as "not binary" based on that.

Increasing the amount of bytes read there (I've tried with 204800) solves the issue: the function reads enough bytes from the file to correctly determine that it's actually binary.

Is there a way to "force" Salt to treat certain files as binary, and prevent Salt from trying (and failing) to be smart? Something like a binary=true argument, for instance...?

crile commented 3 years ago

Same problem here, it seems that binary file is not properly detected

waynew commented 2 years ago

There are a lot of historical things to go into, but the short answer is unfortunately no(t yet).

Long story short: we should always treat all files as binary data and only consider them text files when we're explicitly told to. Python had a history of conflating text and binary data, but they've fixed that model. Unfortunately (like many other tools) Salt lives on with a lot of that baggage, and it's going to take a concerted effort to correctly fix that across Salt.