[BUG] top.sls fail to render on empty targeting group

[Nerigal]

Description an empty targeting group / section that has no config declaration will make salt fail to render the top.sls

Steps to Reproduce the behavior define any targeting in the top.sls with no config declaration like

'G@site_domain:ased':
    #- csf.allow.ldap
    #- csf.allow.tss
    #- csf.allow.snmp.prob

the use case was that the configuration was commented

base:
  '*':
    - headers
    - cis
    - requirements
    - sshd
    - sshd.sssd
    - csf
    - chronyd
    - sysctl
    - snmpd
    - sssd
    - spacewalk
    - grains
    - journald
    - postfix

  'G@site:azure':
    - csf.allow.azure
    - csf.allow.nessus.external
    - csf.allow.prtg.prob.mgm.external
    - csf.allow.salt.master.external
    - chronyd.site.azure
    - sshd.site.azure

  'G@site_domain:ased':
    #- csf.allow.ldap
    #- csf.allow.tss
    #- csf.allow.snmp.prob

Expected behavior an empty dictionary should not make salt fail to render the top.sls file, it should just be ignored

Versions Report

Salt Version:
           Salt: 3000.3

Dependency Versions:
           cffi: 1.12.3
       cherrypy: unknown
       dateutil: Not Installed
      docker-py: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
         Jinja2: 2.11.1
        libgit2: 0.28.2
       M2Crypto: 0.35.2
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.6.2
   mysql-python: Not Installed
      pycparser: 2.19
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: 0.28.2
         Python: 3.6.8 (default, Apr  2 2020, 13:34:55)
   python-gnupg: Not Installed
         PyYAML: 3.13
          PyZMQ: 15.3.0
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.5.3
            ZMQ: 4.1.4

System Versions:
           dist: centos 7.8.2003 Core
         locale: UTF-8
        machine: x86_64
        release: 3.10.0-1127.10.1.el7.x86_64
         system: Linux
        version: CentOS Linux 7.8.2003 Core

[cmcmarrow]

Thanks for the report @Nerigal! Do you have a stack trace that you can share please?

[arizvisa]

@Nerigal, I believe that is badly formed yaml as I had attempted to do the same thing.

Although you mentioned a dictionary is being used, it's actually a list of items. If you want to have an empty targeting group, you'll need to use an empty list like [].

However if you use an empty list like I mentioned, then you'll encounter issue #54882.

[Nerigal]

@Nerigal, I believe that is badly formed yaml as I had attempted to do the same thing.

No, yaml lint does not flag it as bad yaml Declaring an empty dictionary should be "supported" or at least not make the rendering crashing

Although you mentioned a dictionary is being used, it's actually a list of items. If you want to have an empty targeting group, you'll need to use an empty list like [].

However if you use an empty list like I mentioned, then you'll encounter issue #54882.

My understanding is more that 'G@site_domain:ased': is a named index of the base dictionary which, of course can contain another "nested" dictionary or a list or a scalar or else ... which itself is in fact a nested dictionary so it is not about the list being empty, it is the list that is being absent which mean the parent dictionary index is empty

[arizvisa]

okay...well, if you parse it with pyyaml which is the yaml library that saltstack is using, you'll see you get the following data structure. so however you feel you understand it, this is how salt is understanding it.

$ python
>>> blah='''
... base:
...   '*':
...     - headers
...     - cis
...     - requirements
...     - sshd
...     - sshd.sssd
...     - csf
...     - chronyd
...     - sysctl
...     - snmpd
...     - sssd
...     - spacewalk
...     - grains
...     - journald
...     - postfix
... 
...   'G@site:azure':
...     - csf.allow.azure
...     - csf.allow.nessus.external
...     - csf.allow.prtg.prob.mgm.external
...     - csf.allow.salt.master.external
...     - chronyd.site.azure
...     - sshd.site.azure
... 
...   'G@site_domain:ased':
...     #- csf.allow.ldap
...     #- csf.allow.tss
...     #- csf.allow.snmp.prob
... '''
>>> __import__('yaml').load(data)
__main__:1: YAMLLoadWarning: calling yaml.load() without Loader=... is deprecated, as the default Loader is unsafe. Please read https://msg.pyyaml.org/load for full details.
{'base': {'*': ['headers', 'cis', 'requirements', 'sshd', 'sshd.sssd', 'csf', 'chronyd', 'sysctl', 'snmpd', 'sssd', 'spacewalk', 'grains', 'journald', 'postfix'], 'G@site:azure': ['csf.allow.azure', 'csf.allow.nessus.external', 'csf.allow.prtg.prob.mgm.external', 'csf.allow.salt.master.external', 'chronyd.site.azure', 'sshd.site.azure'], 'G@site_domain:ased': None}}
>>> 

That None that's appearing where you intend your "emptiness" to be under G@site_domain:ased is not being handled properly which is what that linked issue is referring to. If you include a [] it gets treated as an empty list which corresponds to the data type that salt is actually checking for.

Salt assumes that you're going to give it a list of items under the targetting group. Earlier version of that PR terminated without raising the exception but maintainers decided that they'd prefer to be explicit when validating data structures.

But whatever you want to do to get your problem solved. You know where the code is now, so now you don't need to paste a backtrace.

[Nerigal]

But whatever you want to do to get your problem solved. You know where the code is now, so now you don't need to paste a backtrace.

Thanks for the backtrace

It is not really a problem for me more than the fact that salt rendering crash on the valid yaml definition i mean if the yaml syntax is valid for yaml lint but salt crash on it... make it look more like a bug.

Now, if you think that this behavior is acceptable on a valid yaml syntax well close the request

salt 'test-centos81' pillar.items
test-centos81:
    ----------
    _errors:
        - Error encountered while rendering pillar top file.

To me it is still not acceptable to have like a top.sls file that can potentially have hundreds of lines of config definition where yaml lint claim the file has perfect syntax but in the other hand, salt is crashing for "unknown reason" because the error is not that explicit

it make the troubleshoot very long and not efficient at all.

if salt plan to have its own yaml strict recipe of understanding yaml file, it could be a good idea to have the tools that should come with it to be able to validate the syntax base up on the way salt interpret it. other wise yeah the troubleshoot is hard

Thank you.

[arizvisa]

I'm not a maintainer, so therefore I can't do anything with your ticket. That's up to you to close it if you deem it necessary.

I was only attempting to explain why you were getting the symptoms you were getting. Don't forget that some of us are just members of the community that try to help clean up tickets so the maintainers can focus on merging PRs. They have a huge workload as-is.

[cmcmarrow]

@Nerigal I currently have a ticket assigned to me to improve yaml error messages. SaltStack is definitely aware of this quality of life problem and is planning to take action on it. I get a bad taste in my mouth with yaml errors.

[cmcmarrow]

Thanks @airzvisa for helping out with this issue.

[cmcmarrow]

Closing Issues due to inactivity. Feel free to re-open if deemed necessary.