git.latest potentially returning new tags as a set which can cause TypeError: set() is not JSON serializable #55708
Description
Description of Issue
This will be a hard to reproduce issue because its a very particular case and requires setting up a git repo, cloning it with the state locally, adding a tag to the repo, and re-applying the state. If using a returner that calls json.dumps (like cassanda, couchbase, influxdb, etc) then you'll get your exception on the minion.
When git.latest is specified against an already existing repo and the update involves new tags, there is a particular case that results in a set() being returned. When this is sent to a returner, some returners will try and use JSON.dumps to serialize it. In python, a set() is non-serializable which results in a TypeError being thrown and can hide the result from the returner. This exception looks like the following backtrace.
Traceback (most recent call last):
File "/usr/lib/python2.7/dist-packages/salt/minion.py", line 1799, in _thread_return
minion_instance.returners[returner_str](ret)
File "/var/cache/salt/minion/extmods/returners/etcd_return.py", line 217, in returner
data = salt.utils.json.dumps(ret[field])
File "/usr/lib/python2.7/dist-packages/salt/utils/json.py", line 145, in dumps
return json_module.dumps(obj, **kwargs) # future lint: blacklisted-function
File "/usr/lib/python2.7/json/__init__.py", line 251, in dumps
sort_keys=sort_keys, **kw).encode(obj)
File "/usr/lib/python2.7/json/encoder.py", line 207, in encode
chunks = self.iterencode(o, _one_shot=True)
File "/usr/lib/python2.7/json/encoder.py", line 270, in iterencode
return _iterencode(o, 0)
File "/usr/lib/python2.7/json/encoder.py", line 184, in default
raise TypeError(repr(o) + " is not JSON serializable")
TypeError: set([u'v1.0', u'v0.1']) is not JSON serializable
Looking at the implementation of git.latest at line 1417 ([1]), the following code uses git.ls_remote to list the remotes and grab the remote tags. This is stored in the remote_tags local as a set ([2]). This is so the line that follows ([3]), a conditional, can compare it against any local tags. If there's a difference (meaning tags are added/removed during the update), then that subtractions will be performed on the set ([4]) in order to determine whether there's any new_tags or old_tags.
https://github.com/saltstack/salt/blob/master/salt/states/git.py#L1417
def latest(name, # [1]
rev='HEAD',
target=None,
branch=None,
...
remote_tags = set([ # [2]
x.replace('refs/tags/', '') for x in __salt__['git.ls_remote'](
cwd=target,
remote=remote,
opts="--tags",
...
ignore_retcode=True,
output_encoding=output_encoding) if '^{}' not in x
])
if all_local_tags != remote_tags: # [3]
has_remote_rev = False
new_tags = remote_tags - all_local_tags # [4]
deleted_tags = all_local_tags - remote_tags
if new_tags:
ret['changes']['new_tags'] = new_tags # [5]
If any new_tags were found, then this variable is directly assigned into the ret dictionary ([5]) as "changes" in order to stash the result. This dictionary is returned to the caller, and will typically be passed to a returner implementation. A returner will typically transform the contents of this dictionary in order to store it, and typically this data will be serialized depending on the implementation. As a set() is non-serializeable, this can result in the returner being unable to capture this result instead failing w/ an error.
Setup
Create a remote repo that you can clone with git.latest, add some commits, and tag one of them.
Configure your master with a returner that implements a returner() function that uses salt.utils.json.dumps on its ret parameter.
Steps to Reproduce Issue
Use git.latest to clone your repository to a directory.
Add a few commits to your remote repository, and tag them so that there's some new tags.
Re-apply your git.latest state so that the new tags are fetched and the minion uses salt.utils.json.dumps on it.
Look in the minion log for the exception, and confirm that the result of git.latest was not submitted to the returner.
Versions Report
As prior mentioned this is in the source and can be reached in the most recent commit on the master branch. This can be easily fixed by using list() when assigning new_tags into ret['changes']['new_tags'] like the following:
if all_local_tags != remote_tags:
...
if new_tags:
ret['changes']['new_tags'] = list(new_tags)
Salt Version:
Salt: 2019.2.0
Dependency Versions:
cffi: 1.12.3
cherrypy: Not Installed
dateutil: 2.8.0
docker-py: Not Installed
gitdb: 2.0.3
gitpython: 3.0.4
ioflo: 2.0.0
Jinja2: 2.10.1
libgit2: 0.28.2
libnacl: 1.6.1
M2Crypto: Not Installed
Mako: 1.1.0
msgpack-pure: Not Installed
msgpack-python: 0.6.2
mysql-python: Not Installed
pycparser: 2.14
pycrypto: 2.6.1
pycryptodome: 3.9.0
pygit2: 0.28.2
Python: 3.7.5 (default, Oct 17 2019, 12:16:48)
python-gnupg: Not Installed
PyYAML: 5.1.2
PyZMQ: 18.0.2
RAET: Not Installed
smmap: 0.9.0
timelib: Not Installed
Tornado: 4.5.3
ZMQ: 4.3.2
System Versions:
dist: fedora 31 Thirty One
locale: UTF-8
machine: x86_64
release: 4.19.78-coreos
system: Linux
version: Fedora 31 Thirty One