[BUG] ``salt.pillar.get_pillar`` broken since Salt 3003

[mirceaulinic]

Description
A clear and concise description of what the bug is.

I have a custom execution module were I need to compile Pillars given a custom git branch. This is done by invoking salt.pillar.get_pillar. I had no troubles running this across many Salt versions (including 3002.9 currently), but upgrading to anything above 3003 (including 3005rc2), it errors out.
The reasoning why we're needing this call is that we have a gitfs backend, and need on-demand Pillar data compiled for whatever branch the user requires.

Trace:

The minion function caused an exception: Traceback (most recent call last):
  File "/usr/local/lib/python3.9/site-packages/salt/metaproxy/proxy.py", line 479, in thread_return
    return_data = minion_instance.executors[fname](
  File "/usr/local/lib/python3.9/site-packages/salt/loader/lazy.py", line 149, in __call__
    return self.loader.run(run_func, *args, **kwargs)
  File "/usr/local/lib/python3.9/site-packages/salt/loader/lazy.py", line 1228, in run
    return self._last_context.run(self._run_as, _func_or_method, *args, **kwargs)
  File "/usr/local/lib/python3.9/site-packages/salt/loader/lazy.py", line 1243, in _run_as
    return _func_or_method(*args, **kwargs)
  File "/usr/local/lib/python3.9/site-packages/salt/executors/direct_call.py", line 10, in execute
    return func(*args, **kwargs)
  File "/usr/local/lib/python3.9/site-packages/salt/loader/lazy.py", line 149, in __call__
    return self.loader.run(run_func, *args, **kwargs)
  File "/usr/local/lib/python3.9/site-packages/salt/loader/lazy.py", line 1228, in run
    return self._last_context.run(self._run_as, _func_or_method, *args, **kwargs)
  File "/usr/local/lib/python3.9/site-packages/salt/loader/lazy.py", line 1243, in _run_as
    return _func_or_method(*args, **kwargs)
  File "/var/cache/salt/proxy/extmods/some-proxy/modules/do.py", line 862, in config
    master_pillar = get_pillar(ext=_ext_pillar_args(branch) if branch else None)
  File "/var/cache/salt/proxy/extmods/some-proxy/modules/do.py", line 104, in get_pillar
    ret = salt.pillar.get_pillar(
  File "/usr/local/lib/python3.9/site-packages/salt/pillar/__init__.py", line 353, in compile_pillar
    ret_pillar = self.channel.crypted_transfer_decode_dictentry(
  File "/usr/local/lib/python3.9/site-packages/salt/utils/asynchronous.py", line 125, in wrap
    raise exc_info[1].with_traceback(exc_info[2])
  File "/usr/local/lib/python3.9/site-packages/salt/utils/asynchronous.py", line 131, in _target
    result = io_loop.run_sync(lambda: getattr(self.obj, key)(*args, **kwargs))
  File "/usr/local/lib/python3.9/site-packages/salt/ext/tornado/ioloop.py", line 459, in run_sync
    return future_cell[0].result()
  File "/usr/local/lib/python3.9/site-packages/salt/ext/tornado/concurrent.py", line 249, in result
    raise_exc_info(self._exc_info)
  File "<string>", line 4, in raise_exc_info
  File "/usr/local/lib/python3.9/site-packages/salt/ext/tornado/gen.py", line 309, in wrapper
    yielded = next(result)
  File "/usr/local/lib/python3.9/site-packages/salt/channel/client.py", line 172, in crypted_transfer_decode_dictentry
    self._package_load(self.auth.crypticle.dumps(load)),
  File "/usr/local/lib/python3.9/site-packages/salt/crypt.py", line 1518, in dumps
    toencrypt = self.PICKLE_PAD + salt.payload.dumps(obj)
  File "/usr/local/lib/python3.9/site-packages/salt/payload.py", line 163, in dumps
    return salt.utils.msgpack.packb(
  File "/usr/local/lib/python3.9/site-packages/salt/utils/msgpack.py", line 133, in packb
    return msgpack.packb(o, **_sanitize_msgpack_kwargs(kwargs))
  File "/usr/local/lib/python3.9/site-packages/msgpack/__init__.py", line 38, in packb
    return Packer(**kwargs).pack(o)
  File "msgpack/_packer.pyx", line 294, in msgpack._cmsgpack.Packer.pack
  File "msgpack/_packer.pyx", line 300, in msgpack._cmsgpack.Packer.pack
  File "msgpack/_packer.pyx", line 297, in msgpack._cmsgpack.Packer.pack
  File "msgpack/_packer.pyx", line 231, in msgpack._cmsgpack.Packer._pack
  File "msgpack/_packer.pyx", line 285, in msgpack._cmsgpack.Packer._pack
  File "/usr/local/lib/python3.9/site-packages/salt/payload.py", line 158, in ext_type_encoder
    return dict(obj)
  File "/usr/local/lib/python3.9/_collections_abc.py", line 826, in __iter__
    yield from self._mapping
  File "/usr/local/lib/python3.9/site-packages/salt/loader/context.py", line 97, in __iter__
    return self.value().__iter__()
AttributeError: 'NoneType' object has no attribute '__iter__'

Setup

A Salt Master and a Proxy Minion (any flavour) running Salt 3003+, and a custom execution module with these functions:

def _ext_pillar_args(branch):                                                                                                                                                                                  
    repo = "git@github.example.com:org/salt.git"                                                   
    return dict(                                                                                                        
        git=[                                                                                                           
            {                                                                                                           
                "{} {}".format(branch, repo): [                                                                         
                    {"env": "base"},                                                                                    
                    {"root": "salt/pillar"},                                                                            
                    {"pubkey": "/path/to/key.pub"},                                                                   
                    {"privkey": "/path/to/key.priv"},                                                                      
                ]                                                                                                       
            }                                                                                                           
        ]                                                                                                               
    )


def get_pillar(branch=None):
    ext = _ext_pillar_args(branch) if branch else None
    ret = salt.pillar.get_pillar(                                                                                       
        __opts__, __grains__, __opts__["id"], __opts__.get("saltenv"), ext=ext                                              
    ).compile_pillar()
    return ret

Master configuration:

ext_pillar_first: true
ext_pillar:
  - git:
    - git@github.example.com:org/salt.git:
      - root: salt/pillar
      - pubkey: /path/to/key.pub
      - privkey: /path/to/key.priv

This code has been running in production, and currently under 3002.9 perfectly fine.

• on-prem machine
• VM (Virtualbox, KVM, etc. please specify)
• VM running on a cloud service, please be explicit and add details
• container (Kubernetes, Docker, containerd, etc. please specify)
• or a combination, please be explicit
• jails if it is FreeBSD

Steps to Reproduce the behavior

1. Put the code above into a custom execution module.
2. Set up a Master with a gitfs backend and a Proxy Minion of any flavour.
3. Execute salt <tgt> custom_module.get_pillar

Expected behavior

I'd expect to get the pillar not the trace.

Versions Report

salt --versions-report

(Provided by running salt --versions-report. Please also mention any differences in master/minion versions.)

root@salt:~# salt --versions-report
Salt Version:
          Salt: 3004.2
 
Dependency Versions:
          cffi: 1.15.1
      cherrypy: unknown
      dateutil: Not Installed
     docker-py: Not Installed
         gitdb: Not Installed
     gitpython: Not Installed
        Jinja2: 3.0.3
       libgit2: 1.4.3
      M2Crypto: Not Installed
          Mako: Not Installed
       msgpack: 1.0.4
  msgpack-pure: Not Installed
  mysql-python: Not Installed
     pycparser: 2.21
      pycrypto: Not Installed
  pycryptodome: 3.15.0
        pygit2: 1.9.2
        Python: 3.9.13 (main, Jul 12 2022, 12:26:02)
  python-gnupg: 0.4.9
        PyYAML: 6.0
         PyZMQ: 21.0.2
         smmap: Not Installed
       timelib: Not Installed
       Tornado: 4.5.3
           ZMQ: 4.3.3
 
System Versions:
          dist: debian 10 buster
        locale: utf-8
       machine: x86_64
       release: 5.0.0-36-generic
        system: Linux
       version: Debian GNU/Linux 10 buster

Additional context
Add any other context about the problem here.

[mirceaulinic]

More info on this: it seems like the object being sent to msgpack (see the end of the trace)

      File "/usr/local/lib/python3.9/site-packages/msgpack/__init__.py", line 38, in packb
        return Packer(**kwargs).pack(o)

is this:

{'id': 'some-proxy', 'grains': <salt.loader.context.NamedLoaderContext object at 0x7f4fa2700f10>, 'saltenv': 'base', 'pillarenv': None, 'pillar_override': {}, 'extra_minion_data': {}, 'ver': '2', 'cmd': '_pillar', 'nonce': 'supersafe'}

This gets sent to salt.payload.ext_type_encoder and the value of the grains object appears to be None for some reason.

[mirceaulinic]

It turns out that the error is caused by the fact that the Grains are passed in as the NamedLoaderContext object. Changing the snippet above to the following, fixes the issue:

    ret = salt.pillar.get_pillar(                                                                                       
        __opts__, __grains__.value(), __opts__["id"], __opts__.get("saltenv"), ext=ext                                              
    ).compile_pillar()

I am happy to close this one out, but I'd like to see if the core team would find value in checking the data type of the grains argument (into the salt.pillar.get_pillar function) and always call .value() when the object passed in is salt.loader.context.NamedLoaderContext type (I can probably see other places where this confusion would be avoided out of the box).

[jbemmel]

Also happens when passing a simple pillar on the command line (using 3004.2):

salt srl1 cmd.script salt://templates/set_hostname2.j2 template=jinja pillar='{"hostname": "test"}'

Looks like a pretty serious bug to me - no workaround available to command line users

[dmurphy18]

@mirceaulinic Curious, the versions-report states Salt 3004.2, but the python version used is Python 3.9.13 but the default OS version for Debian 10 is Python 3.7.3, and given this is a (classic) package which depends on the OS to supply dependencies (not Onedir - only available with Salt 3005.1), typically they need the default OS python3 version to operate correctly ?

[dwoz]

I wonder if this will fix the issue #64043

[dmurphy18]

Can confirm with

root@Unknown:/srv/pillar# cat /srv/salt/templates/set_hostname.j2 
{# /srv/salt/templates/basic.j2 #} 
{%- set host  = pillar.get('hostname', "UNKNOWN") %}
{%- if host == 'UNKNOWN' %}
  {{ raise("Unsupported UNKNOWN hostname") }}
{%- else %}
    hostnamectl set-hostname {{ host }}
{%- endif %}

The 3002.9 minion on Debian 10 works fine, but 3005.1 minion on Debian 11, has issues, from RHEL 7 salt-master 3002.9

[TRACE   ] _get_event() waited 0 seconds and received nothing
[DEBUG   ] jid 20230410223829757471 found all minions {'td11'}
[DEBUG   ] Closing IPCMessageSubscriber instance
ERROR: Minions returned with non-zero exit code
td11:
    The minion function caused an exception: Traceback (most recent call last):
      File "/usr/lib/python3/dist-packages/salt/minion.py", line 1935, in _thread_return
        return_data = minion_instance._execute_job_function(
      File "/usr/lib/python3/dist-packages/salt/minion.py", line 1894, in _execute_job_function
        return_data = self.executors[fname](opts, data, func, args, kwargs)
      File "/usr/lib/python3/dist-packages/salt/loader/lazy.py", line 149, in __call__
        return self.loader.run(run_func, *args, **kwargs)
      File "/usr/lib/python3/dist-packages/salt/loader/lazy.py", line 1228, in run
        return self._last_context.run(self._run_as, _func_or_method, *args, **kwargs)
      File "/usr/lib/python3/dist-packages/salt/loader/lazy.py", line 1243, in _run_as
        return _func_or_method(*args, **kwargs)
      File "/usr/lib/python3/dist-packages/salt/executors/direct_call.py", line 10, in execute
        return func(*args, **kwargs)
      File "/usr/lib/python3/dist-packages/salt/loader/lazy.py", line 149, in __call__
        return self.loader.run(run_func, *args, **kwargs)
      File "/usr/lib/python3/dist-packages/salt/loader/lazy.py", line 1228, in run
        return self._last_context.run(self._run_as, _func_or_method, *args, **kwargs)
      File "/usr/lib/python3/dist-packages/salt/loader/lazy.py", line 1243, in _run_as
        return _func_or_method(*args, **kwargs)
      File "/usr/lib/python3/dist-packages/salt/modules/cmdmod.py", line 2860, in script
        kwargs["pillar"] = _gather_pillar(pillarenv, kwargs.get("pillar"))
      File "/usr/lib/python3/dist-packages/salt/modules/cmdmod.py", line 226, in _gather_pillar
        ret = pillar.compile_pillar()
      File "/usr/lib/python3/dist-packages/salt/pillar/__init__.py", line 353, in compile_pillar
        ret_pillar = self.channel.crypted_transfer_decode_dictentry(
      File "/usr/lib/python3/dist-packages/salt/utils/asynchronous.py", line 125, in wrap
        raise exc_info[1].with_traceback(exc_info[2])
      File "/usr/lib/python3/dist-packages/salt/utils/asynchronous.py", line 131, in _target
        result = io_loop.run_sync(lambda: getattr(self.obj, key)(*args, **kwargs))
      File "/usr/lib/python3/dist-packages/salt/ext/tornado/ioloop.py", line 459, in run_sync
        return future_cell[0].result()
      File "/usr/lib/python3/dist-packages/salt/ext/tornado/concurrent.py", line 249, in result
        raise_exc_info(self._exc_info)
      File "<string>", line 4, in raise_exc_info
      File "/usr/lib/python3/dist-packages/salt/ext/tornado/gen.py", line 309, in wrapper
        yielded = next(result)
      File "/usr/lib/python3/dist-packages/salt/channel/client.py", line 172, in crypted_transfer_decode_dictentry
        self._package_load(self.auth.crypticle.dumps(load)),
      File "/usr/lib/python3/dist-packages/salt/crypt.py", line 1518, in dumps
        toencrypt = self.PICKLE_PAD + salt.payload.dumps(obj)
      File "/usr/lib/python3/dist-packages/salt/payload.py", line 173, in dumps
        return salt.utils.msgpack.packb(
      File "/usr/lib/python3/dist-packages/salt/utils/msgpack.py", line 133, in packb
        return msgpack.packb(o, **_sanitize_msgpack_kwargs(kwargs))
      File "/usr/lib/python3/dist-packages/msgpack/__init__.py", line 35, in packb
        return Packer(**kwargs).pack(o)
      File "msgpack/_packer.pyx", line 286, in msgpack._cmsgpack.Packer.pack
      File "msgpack/_packer.pyx", line 292, in msgpack._cmsgpack.Packer.pack
      File "msgpack/_packer.pyx", line 289, in msgpack._cmsgpack.Packer.pack
      File "msgpack/_packer.pyx", line 225, in msgpack._cmsgpack.Packer._pack
      File "msgpack/_packer.pyx", line 279, in msgpack._cmsgpack.Packer._pack
      File "/usr/lib/python3/dist-packages/salt/payload.py", line 168, in ext_type_encoder
        return dict(obj)
      File "/usr/lib/python3.9/_collections_abc.py", line 825, in __iter__
        yield from self._mapping
      File "/usr/lib/python3/dist-packages/salt/loader/context.py", line 97, in __iter__
        return self.value().__iter__()
    AttributeError: 'NoneType' object has no attribute '__iter__'
root@Unknown:/srv/salt/templates# 

[dmurphy18]

Upgrade to salt-master with 3005.1 and issue still exists

[dmurphy18]

The problem is here

 49 2023-04-10 17:30:17,532 [salt.payload     :177 ][WARNING ][87528] DGM payload dumps, msg '{'id': 'td11', 'grains': <salt.loader.context.NamedLoaderContext object at 0x7fca3830beb0>, 'saltenv': None, 'pillarenv': None, 'pillar_override': {'hostname':     'test'}, 'extra_minion_data': {}, 'ver': '2', 'cmd': '_pillar', 'nonce': 'd622967d7b384941b09aeec5a8061be7'}', use_bin_type 'False'
 50 2023-04-10 17:30:17,532 [salt.payload     :168 ][WARNING ][87528] DGM payload ext_type_encoder, obj instance of collections.abc.MutableMapping, obj '<salt.loader.context.NamedLoaderContext object at 0x7fca3830beb0>'
 51 2023-04-10 17:30:17,533 [salt.minion      :2013][WARNING ][87528] The minion function caused an exception

trying to do a dict of the obj throws an exception, in salt/payload.py

167         elif isinstance(obj, collections.abc.MutableMapping):
168             log.warning(f"DGM payload ext_type_encoder, obj instance of collections.abc.MutableMapping, obj '{obj}'")
169             dgm_dict = dict(obj)
170             log.warning(f"DGM payload ext_type_encoder, obj instance of collections.abc.MutableMapping, return dict '{dgm_dict}'")
171             return dgm_dict
172             ## return dict(obj)

[dmurphy18]

Simplifying the test which produces the error

root@Unknown:/srv/salt# cat set_hostname.j2 
{%- set host  = pillar.get('hostname', "UNKNOWN") %}
{%- if host == 'UNKNOWN' %}
  {{ raise("Unsupported UNKNOWN hostname") }}
{%- else %}
    hostnamectl set-hostname {{ host }}
{%- endif %}
root@Unknown:/srv/salt# salt td11 cmd.script salt://set_hostname.j2 template=jinja  pillar='{"hostname": "test"}'