saltstack / salt

Software to automate the management and configuration of any infrastructure or application at scale. Get access to the Salt software package repository here:
https://repo.saltproject.io/
Apache License 2.0
13.98k stars 5.47k forks source link

[BUG] Salt-SSH: Accessing modules in Jinja templates via attributes disables wrappers #66600

Open lkubb opened 1 month ago

lkubb commented 1 month ago

Description If - during state template rendering - one accesses execution modules via attributes instead of dict keys, Salt-SSH wrapper modules are ignored.

[This came up in a Discord discussion about performance issues, initiated by @dseomn.]

Setup (Please provide relevant configs and/or SLS files (be sure to remove sensitive info. There is no general set-up of Salt.)

Please be as specific as possible and give set-up details.

Steps to Reproduce the behavior

# /srv/salt/sshtest.sls

foo:
  test.nop:
    - name: {{ salt.config.option("master") | json }}
$ salt-ssh tgt state.show_sls sshtest -l debug
[...]
[DEBUG   ] Performing shimmed, blocking command as follows:
config.option "master"
[...]

Expected behavior Use the config wrapper module instead of executing on the target (the log line above should be absent then).

Screenshots See above.

Versions Report

salt --versions-report (Provided by running salt --versions-report. Please also mention any differences in master/minion versions.) ```yaml Salt Version: Salt: 3006.7 Python Version: Python: 3.10.13 (main, Feb 19 2024, 03:31:20) [GCC 11.2.0] Dependency Versions: cffi: 1.14.6 cherrypy: unknown dateutil: 2.8.1 docker-py: Not Installed gitdb: 4.0.11 gitpython: 3.1.42 Jinja2: 3.1.3 libgit2: Not Installed 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.19.1 pygit2: Not Installed python-gnupg: 0.4.8 PyYAML: 6.0.1 PyZMQ: 23.2.0 relenv: 0.15.1 smmap: 5.0.1 timelib: 0.2.4 Tornado: 4.5.3 ZMQ: 4.3.4 System Versions: dist: rocky 9.3 Blue Onyx locale: utf-8 machine: x86_64 release: 5.14.0-362.13.1.el9_3.x86_64 system: Linux version: Rocky Linux 9.3 Blue Onyx ```

This is valid for 3007 as well.

Additional context The following patch solves the issue:

diff --git a/salt/client/ssh/wrapper/__init__.py b/salt/client/ssh/wrapper/__init__.py
index cfa977be0f..6215e4eb67 100644
--- a/salt/client/ssh/wrapper/__init__.py
+++ b/salt/client/ssh/wrapper/__init__.py
@@ -139,7 +139,7 @@ class FunctionWrapper:
     ):
         super().__init__()
         self.cmd_prefix = cmd_prefix
-        self.wfuncs = wfuncs if isinstance(wfuncs, dict) else {}
+        self.wfuncs = wfuncs if wfuncs is not None else {}
         self.opts = opts
         self.mods = mods if isinstance(mods, dict) else {}
         self.kwargs = {"id_": id_, "host": host}

Additionally, it should be documented that the dict syntax (salt["config.option"]) for accessing loaded modules is the preferred way for several reasons (if it is not already, unsure).

Somewhat related: https://github.com/saltstack/salt/issues/66376 https://github.com/saltstack/salt/issues/41794

OrangeDog commented 1 month ago

This is a well-known issue, and why all examples should be using e.g. salt["config.option"]("master").

I cannot find the original report right now; it may be closed. #41794

lkubb commented 1 month ago

Maybe you mean https://github.com/saltstack/salt/issues/41794 (a different issue, but somewhat related)?

I don't remember reading about wrappers not working in general with this syntax (have at least skimmed most historic Salt-SSH issues at some point in the past year, but maybe it's really old) and couldn't find much that would fit.

To be clear, this issue happens regardless of the specific module or function, the wfuncs dict of the child FunctionWrapper created during the first attribute access is always empty because it receives a NamedLoaderContext, which is a MutableMapping, but not a dict.