saltstack / saltstack/salt

[BUG] git.latest does not properly handle missing identities on salt file server

Open
#65,199 0 comments 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

bug
Dominant language
Python
Stars
15.7k
Forks
5.6k
Avg merge
2d 44m
Merged PRs (30d)
80

Description

Description
identity option of git.latest supports identities stored on the file server using salt:// style urls. However when the identity does not exist, a cryptic TypeError is thrown (instead of the error message in the code), and the state fails.
This therefore also breaks the functionality where non-existing keys are ignored and the next one is tried.

Setup
Using a salt master to clone a git repo but also with salt-ssh.

Steps to Reproduce the behavior
Trying to clone a repo where one or more keys are available on the file server, but some aren't. Even when only one is configured and doesn't exist like so

clone saltstack:
  git.latest:
    - name: https://github.com/saltstack/salt
    - target: /tmp/salt
    - identity: salt://this_key_does_not_exist

Will result in TypeError: expected str, bytes or os.PathLike object, not bool

[DEBUG   ] compile template: /var/cache/salt/minion/files/base/testgit.sls
[DEBUG   ] Jinja search path: ['/var/cache/salt/minion/files/base']
[PROFILE ] Time (in seconds) to render '/var/cache/salt/minion/files/base/testgit.sls' using 'jinja' renderer: 0.0024216175079345703
[DEBUG   ] Rendered data from file: /var/cache/salt/minion/files/base/testgit.sls:
clone saltstack:
  git.latest:
    - name: https://github.com/saltstack/salt
    - target: /tmp/salt
    - identity: salt://this_key_does_not_exist

[PROFILE ] Time (in seconds) to render '/var/cache/salt/minion/files/base/testgit.sls' using 'yaml' renderer: 0.0008649826049804688
[DEBUG   ] The functions from module 'config' are being loaded by dir() on the loaded module
[DEBUG   ] LazyLoaded config.option
[DEBUG   ] The functions from module 'git' are being loaded by dir() on the loaded module
[DEBUG   ] LazyLoaded git.version
[TRACE   ] Loaded cmdmod as virtual cmd
[DEBUG   ] The functions from module 'cmd' are being loaded by dir() on the loaded module
[DEBUG   ] LazyLoaded cmd.run_all
[INFO    ] Executing command git in directory '/root'
[DEBUG   ] stdout: git version 2.34.1
[DEBUG   ] The functions from module 'git' are being loaded by dir() on the loaded module
[DEBUG   ] LazyLoaded git.latest
[INFO    ] Running state [https://github.com/saltstack/salt] at time 22:42:52.514924
[INFO    ] Executing state git.latest for [https://github.com/saltstack/salt]
[DEBUG   ] The functions from module 'cp' are being loaded by dir() on the loaded module
[DEBUG   ] LazyLoaded cp.cache_file
[DEBUG   ] Initializing new AsyncAuth for ('/etc/salt/pki/minion', 'salt-master', 'tcp://127.0.0.1:4506')
[TRACE   ] ReqChannel send crypt load={'path': 'this_key_does_not_exist', 'saltenv': 'base', 'cmd': '_file_hash'}
[TRACE   ] ReqChannel send crypt load={'path': 'this_key_does_not_exist', 'saltenv': 'base', 'cmd': '_file_find'}
[DEBUG   ] Could not find file 'salt://this_key_does_not_exist' in saltenv 'base'
[DEBUG   ] Closing AsyncReqChannel instance
[ERROR   ] Unable to cache file 'salt://this_key_does_not_exist' from saltenv 'base'.
[DEBUG   ] An exception occurred in this state: expected str, bytes or os.PathLike object, not bool
Traceback (most recent call last):
  File "/opt/saltstack/salt/lib/python3.10/site-packages/salt/state.py", line 2385, in call
    ret = self.states[cdata["full"]](
  File "/opt/saltstack/salt/lib/python3.10/site-packages/salt/loader/lazy.py", line 149, in __call__
    return self.loader.run(run_func, *args, **kwargs)
  File "/opt/saltstack/salt/lib/python3.10/site-packages/salt/loader/lazy.py", line 1232, in run
    return self._last_context.run(self._run_as, _func_or_method, *args, **kwargs)
  File "/opt/saltstack/salt/lib/python3.10/site-packages/salt/loader/lazy.py", line 1247, in _run_as
    return _func_or_method(*args, **kwargs)
  File "/opt/saltstack/salt/lib/python3.10/site-packages/salt/loader/lazy.py", line 1280, in wrapper
    return f(*args, **kwargs)
  File "/opt/saltstack/salt/lib/python3.10/site-packages/salt/states/git.py", line 662, in latest
    if not os.path.isabs(ident_path):
  File "/opt/saltstack/salt/lib/python3.10/posixpath.py", line 62, in isabs
    s = os.fspath(s)
TypeError: expected str, bytes or os.PathLike object, not bool
[ERROR   ] An exception occurred in this state: Traceback (most recent call last):
  File "/opt/saltstack/salt/lib/python3.10/site-packages/salt/state.py", line 2385, in call
    ret = self.states[cdata["full"]](
  File "/opt/saltstack/salt/lib/python3.10/site-packages/salt/loader/lazy.py", line 149, in __call__
    return self.loader.run(run_func, *args, **kwargs)
  File "/opt/saltstack/salt/lib/python3.10/site-packages/salt/loader/lazy.py", line 1232, in run
    return self._last_context.run(self._run_as, _func_or_method, *args, **kwargs)
  File "/opt/saltstack/salt/lib/python3.10/site-packages/salt/loader/lazy.py", line 1247, in _run_as
    return _func_or_method(*args, **kwargs)
  File "/opt/saltstack/salt/lib/python3.10/site-packages/salt/loader/lazy.py", line 1280, in wrapper
    return f(*args, **kwargs)
  File "/opt/saltstack/salt/lib/python3.10/site-packages/salt/states/git.py", line 662, in latest
    if not os.path.isabs(ident_path):
  File "/opt/saltstack/salt/lib/python3.10/posixpath.py", line 62, in isabs
    s = os.fspath(s)
TypeError: expected str, bytes or os.PathLike object, not bool

[INFO    ] Completed state [https://github.com/saltstack/salt] at time 22:42:52.546763 (duration_in_ms=31.84)

Expected behavior
A message noting that the identity does not exist, as foreseen in the code

https://github.com/saltstack/salt/blob/c5936f92ae441bc1cc5acfd81125f5b9c30063b9/salt/states/git.py#L660

It is also expected that a missing identity is ignored and the next one is tried, if a list is provided. However since an error is thrown, this doesn't happen.

When trying the same but with local paths instead of salt file server urls.

root@salt-master:~# salt-call state.apply testgit
[ERROR   ] identity /srv/salt/this_one_does_not_exist does not exist.
[ERROR   ] identity /srv/salt/this_doesnt_exist_either does not exist.
[ERROR   ] Failed to check remote refs: Unable to authenticate using identity file:

The following identity file(s) were not found: /srv/salt/this_one_does_not_exist, /srv/salt/this_doesnt_exist_either
local:
----------
          ID: clone saltstack
    Function: git.latest
        Name: https://github.com/saltstack/kitchen-salt
      Result: False
     Comment: Failed to check remote refs: Unable to authenticate using identity file:

              The following identity file(s) were not found: /srv/salt/this_one_does_not_exist, /srv/salt/this_doesnt_exist_either
     Started: 23:03:53.568943
    Duration: 17.528 ms
     Changes:

Summary for local
------------
Succeeded: 0
Failed:    1
------------
Total states run:     1
Total run time:  17.528 ms

Versions Report

salt --versions-report 
Salt Version:
          Salt: 3006.1

Python Version:
        Python: 3.10.11 (main, May  5 2023, 02:31:54) [GCC 11.2.0]

Dependency Versions:
          cffi: 1.14.6
      cherrypy: unknown
      dateutil: 2.8.1
     docker-py: Not Installed
         gitdb: 4.0.10
     gitpython: 3.1.31
        Jinja2: 3.1.2
       libgit2: 1.3.0
  looseversion: 1.0.2
      M2Crypto: Not Installed
          Mako: Not Installed
       msgpack: 1.0.2
  msgpack-pure: Not Installed
  mysql-python: Not Installed
     packaging: 22.0
     pycparser: 2.21
      pycrypto: Not Installed
  pycryptodome: 3.9.8
        pygit2: 1.7.0
  python-gnupg: 0.4.8
        PyYAML: 5.4.1
         PyZMQ: 23.2.0
        relenv: 0.12.3
         smmap: 5.0.0
       timelib: 0.2.4
       Tornado: 4.5.3
           ZMQ: 4.3.4

System Versions:
          dist: ubuntu 18.04.3 jammy
        locale: utf-8
       machine: x86_64
       release: 5.15.0-82-generic
        system: Linux
       version: Ubuntu 18.04.3 jammy

Also tested with salt-ssh==3006.2

Additional context
The problem seems to be the caching attempt

https://github.com/saltstack/salt/blob/c5936f92ae441bc1cc5acfd81125f5b9c30063b9/salt/states/git.py#L653-L665

When a salt file server url is provided, ident_path = __salt__["cp.cache_file"](ident_path, __env__) tries to get the cached path. If the file is not present, cp.cache_file will return False, hence the TypeError.

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 around the git.latest identity handling at lines 653-665, and inspect how cp.cache_file returns a value for a missing salt:// identity. Reproduce the provided state with a missing identity, then verify that it reports the intended missing-identity message, avoids the TypeError, and tries subsequent identities when a list is provided.

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
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.