salt icon indicating copy to clipboard operation
salt copied to clipboard

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

Open Timmeeh opened this issue 1 year ago • 0 comments

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.

Timmeeh avatar Sep 14 '23 23:09 Timmeeh