salt.states.git.latest with fetch_tags=False is never idempotent
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 15.7k
- Forks
- 5.6k
- Avg merge
- 2d 44m
- Merged PRs (30d)
- 80
Description
Description of Issue
When using the git.latest with fetch_tags set to false, the function will always fetch the tags and assign it to the "changes" key resulting in non-idempotency.
This is because the following code at https://github.com/saltstack/salt/blob/master/salt/states/git.py#L1417 is not barrier'd behind a test for fetch_tags being False. This code is conditional only if remote_rev is not None which is at line number 1271. The fetch_tags option is only used at line number 759.
At mark (1), the git.ls_remote function is used with the --tags parameter to list all of the remote tags and converted to a set() (see issue #55708 for a bug related to this type). This set() is used to compare the current local tags with the remote tags. At (2), this set() is then assigned to the "changes" key of the returned result.
remote_tags = set([
(1) x.replace('refs/tags/', '') for x in __salt__['git.ls_remote'](
cwd=target,
remote=remote,
opts="--tags",
user=user,
password=password,
identity=identity,
saltenv=__env__,
ignore_retcode=True,
output_encoding=output_encoding) if '^{}' not in x
])
if all_local_tags != remote_tags:
has_remote_rev = False
new_tags = remote_tags - all_local_tags
deleted_tags = all_local_tags - remote_tags
if new_tags:
(2) ret['changes']['new_tags'] = new_tags
if sync_tags and deleted_tags:
As the fetch_tags parameter intends to never fetch the remote tags into the local repository, this will result in the "changes" at (2) always returning something despite no tags being stored as a result of the fetch_tags parameter.
Setup
Simply write up a state that uses git.latest with "fetch_tags" set to False as one of its parameters. You'll want to specify some git repository with tags to clone from too. I've also been setting sync_tags to False as well.
As issue #55708 is the exact two lines of code causing this problem, you can trigger that issue by using the syslog_returner (or any returner that uses json.dumps or msgpack.dumps which is unable to serialize a set())
Steps to Reproduce Issue
Apply the state the first time and then note the new_tags being stored in "changes". Then do it one more time.
The second time, there shouldn't be any changes as nothing in the repository has changed. However because fetch_tags does not store the remote tags as per its semantics, the remote tags will always be marked as newly introduced changes. This violates the semantics of fetch_tags.
Versions Report
The lines of code describing the bug are from the most recent commit (35646c5ed938a6effbe0a7b7739e1d9143035549) from the master branch.
Salt Version:
Salt: 2019.2.2
Dependency Versions:
cffi: 1.12.3
cherrypy: Not Installed
dateutil: 2.8.0
docker-py: Not Installed
gitdb: 2.0.3
gitpython: 3.0.5
ioflo: 2.0.0
Jinja2: 2.10.1
libgit2: 0.28.2
libnacl: 1.7.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.4
pygit2: 0.28.2
Python: 3.7.6 (default, Dec 19 2019, 22:52:49)
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.86-coreos
system: Linux
version: Fedora 31 Thirty One
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in salt/states/git.py at the git.latest logic around the referenced line and trace the git.ls_remote tag comparison when fetch_tags is false. Reproduce with a tagged repository and fetch_tags=False, applying the state twice; done means the second run reports no new_tags or other changes caused by tags.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- git, python
- Domain
- devops, infrastructure
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 42/100