saltstack / saltstack/salt

salt.states.git.latest with fetch_tags=False is never idempotent

Open
#55,944 5 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug Confirmed severity-medium
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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.