[BUG] string values set with sdb.set come back as bytes from sdb.get

[oeuftete]

Description

String values set with sdb.set (using at least the redis or sqlite3 drivers) are retrieved as bytes. Presumably, they should come back as strings.

Setup

Add the following as /etc/salt/master.d/sdb.conf and /etc/salt/minion.d/sdb.conf:

sdb_redis:
  driver: redis
  host: redis
  port: 6379
  db: 1

sdb_sqlite:
  driver: sqlite3
  database: /tmp/sdb.sqlite
  table: sdb
  create_table: True

/srv/salt/sdb-roundtrip.sls:

{% for sdb in ['sdb_redis', 'sdb_sqlite'] %}
  {% set SDB_URI = 'sdb://{}/foo'.format(sdb) %}
  {% set SDB_VALUE = 'bar' %}

roundtrip_{{ sdb }}:
  test.configurable_test_state:
    - result: {{ salt.sdb.get(SDB_URI) == SDB_VALUE }}

roundtrip_{{ sdb }}-but-value-to-bytes:
  test.configurable_test_state:
    - result: {{ salt.sdb.get(SDB_URI) == SDB_VALUE|to_bytes }}

{% endfor %}{# sdb in ['sdb_redis', 'sdb_sqlite'] #}

Steps to Reproduce the behavior

The states with the original value converted to bytes match.

# salt master1 --state-output terse state.apply zd5757.roundtrip
master1:
  Name: roundtrip_sdb_redis - Function: test.configurable_test_state - Result: Failed Started: - 18:26:49.549289 Duration: 1.759 ms
  Name: roundtrip_sdb_redis-but-value-to-bytes - Function: test.configurable_test_state - Result: Changed Started: - 18:26:49.558864 Duration: 1.129 ms
  Name: roundtrip_sdb_sqlite - Function: test.configurable_test_state - Result: Failed Started: - 18:26:49.566279 Duration: 1.029 ms
  Name: roundtrip_sdb_sqlite-but-value-to-bytes - Function: test.configurable_test_state - Result: Changed Started: - 18:26:49.573589 Duration: 1.012 ms

Or alternately:

[root@master1 /]# salt-call sdb.set sdb://sdb_redis/baz quux
local:
    True
[root@master1 /]# salt-call --out pprint sdb.get sdb://sdb_redis/baz 
{'local': b'quux'}

Expected behavior

sdb.get should return a string when set as a string.

Versions Report

salt --versions-report

(Provided by running salt --versions-report. Please also mention any differences in master/minion versions.) ``` Salt Version: Salt: 3001.1 Dependency Versions: cffi: 1.14.3 cherrypy: Not Installed dateutil: Not Installed docker-py: Not Installed gitdb: Not Installed gitpython: Not Installed Jinja2: 2.11.1 libgit2: Not Installed M2Crypto: 0.35.2 Mako: Not Installed msgpack-pure: Not Installed msgpack-python: 0.6.2 mysql-python: Not Installed pycparser: 2.20 pycrypto: Not Installed pycryptodome: Not Installed pygit2: Not Installed Python: 3.6.8 (default, Apr 2 2020, 13:34:55) python-gnupg: Not Installed PyYAML: 3.13 PyZMQ: 17.0.0 smmap: Not Installed timelib: Not Installed Tornado: 4.5.3 ZMQ: 4.1.4 System Versions: dist: centos 7 Core locale: UTF-8 machine: x86_64 release: 5.4.0-47-generic system: Linux version: CentOS Linux 7 Core ```

Additional context

ZD-5757.

[oeuftete]

ZD-5827.

[oeuftete]

I suspect the fix in #57502 to remove the deprecation warning and default to ~raw=True~ raw=False for msgpack ≥ 0.5.2 will also fix this.

PR #58658, which updates #57502, does appear to fix this.

[amalaguti-mane]

I tried the fix, while it seems to work ok for sdb.get, I think the fact that the string value is converted to byte object by sdb/sqlite3.py set_ function affects the fetching of data by execution module sqlite3.fetch.

def set_(key, value, profile=None): ... if six.PY2:

pylint: disable=undefined-variable

    value = buffer(salt.utils.msgpack.packb(value))
    # pylint: enable=undefined-variable
else:
    value = memoryview(salt.utils.msgpack.packb(value))`

sdb.set converts string value to byte object $ salt-run sdb.set sdb://mysqlite_checks/mysvr_1 'fire alarm' -l info [INFO ] >>>>>>>> value: fire alarm [INFO ] >>>>>>>> memoryview msgpack: <memory at 0x7fab43df1648> <class 'memoryview'> True [INFO ] Runner completed: 20210611215154702435

string value is not stored as plain text $ sqlite3 /tmp/sdb.sqlite SQLite version 3.7.17 2013-05-20 00:56:22 Enter ".help" for instructions Enter SQL statements terminated with a ";" sqlite> .schema alarms CREATE TABLE alarms (key text, value text); CREATE INDEX myidx ON alarms (key); sqlite> select * from alarms; mysvr_1|�fire alarm sqlite>

$ salt-run sdb.get sdb://mysqlite_checks/mysvr_1 fire alarm

sqlite3.fetch fails to show the converted to byte object string value $ salt-call --local sqlite3.fetch /tmp/sdb.sqlite 'SELECT * FROM alarms;' -l info [INFO ] >>>>>> [('mysvr_1', b'\xaafire alarm')] local:

mysvr_1:

[root@ip-172-31-20-217 ~]#

[max-arnold]

I stumbled upon the same issue a while ago while working on a custom module that used sqlite sdb. Here is my workaround

st = __salt__['sdb.get']('sdb://mysdb/test')
return st.decode('utf-8') if isinstance(st, bytes) else st

I spent some time debugging it, and I believe the issue was caused by msgpack serialization/deserialization.

If I remember correctly, I reproduced this on 3002.5 and maybe 3002.6

[jamesbernardi]

With grains.get_or_set_hash being removed from salt 3005 - the logical replacement being sdb.get_or_set_hash this issue needs to be escalated - as this has broken many workflows and not being able to retrieve string values from SDB is a blocker

[OrangeDog]

There's no actual problem with using grains for this in many situations. You can get the function back with a custom module, or do it in jinja.

def get_or_set_hash(name, length=8, chars="abcdefghijklmnopqrstuvwxyz0123456789!@#$%^&*(-_=+)"):
    ret = __salt__["grains.get"](name, None)
    if ret is None:
        ret = __salt__["random.get_str"](length=length, chars=chars)
        __salt__["grains.setval"](name, ret)
    return ret

Double-check the permissions on the /etc/salt/grains file though, as it might be world-readable.

[max-arnold]

Did anyone test whether https://github.com/saltstack/salt/pull/61794 fixes this issue?