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
14.15k stars 5.48k forks source link

[BUG] top file parse failure (valid yaml) provides obscure results b/c its not valid for Salt #61088

Open jeffdyke opened 3 years ago

jeffdyke commented 3 years ago

Description From the Yes its my fault, but..... department If top.sls contains valid yaml, but not valid top.sls for salt it fails with 'No Top file or master_tops data matches found. Please see master log for details.'

There is nothing in the log except for a key error in events.py on 899 data["user"] = load["user"]

So this made load leaving the the user key unset and it fails hard.

This is the valid yaml that killed it

  'roles:salt-master':
    match: grain
    - salt-overrides.master
    - mailutils
    - postfix-null-client
    - suricata-update

The valid yaml that fixed it

  'roles:salt-master':
    - match: grain
    - salt-overrides.master
    - mailutils
    - postfix-null-client
    - suricata-update

Note the hyphen before match

I don't think any of the other debugging assistance stuff is required but i am running on both the master and minion.

Salt Version:
          Salt: 3004

Dependency Versions:
          cffi: Not Installed
      cherrypy: Not Installed
      dateutil: 2.8.2
     docker-py: Not Installed
         gitdb: 2.0.6
     gitpython: 3.0.7
        Jinja2: 2.10.1
       libgit2: Not Installed
      M2Crypto: Not Installed
          Mako: Not Installed
       msgpack: 0.6.2
  msgpack-pure: Not Installed
  mysql-python: Not Installed
     pycparser: Not Installed
      pycrypto: Not Installed
  pycryptodome: 3.6.1
        pygit2: Not Installed
        Python: 3.8.10 (default, Sep 28 2021, 16:10:42)
  python-gnupg: 0.4.5
        PyYAML: 5.3.1
         PyZMQ: 18.1.1
         smmap: 2.0.5
       timelib: Not Installed
       Tornado: 4.5.3
           ZMQ: 4.3.2

System Versions:
          dist: ubuntu 20.04 focal
        locale: utf-8
       machine: x86_64
       release: 5.11.0-1019-aws
        system: Linux
       version: Ubuntu 20.04 focal

Thanks!

OrangeDog commented 2 years ago

Better error messages for YAML failures has been requested before (can't find it right now) but it's very difficult given the basic code structure.

garethgreenaway commented 2 years ago

@jeffdyke Thanks for the report. To clarify, the first example appears to be invalid YAML but your message seems to indicate that you believe it is valid and Salt should be able to parse it. Is that the case or is this in fact invalid YAML and Salt is just not providing a friendly error message when the parsing fails.

jeffdyke commented 2 years ago

Hey now, @garethgreenaway - Its been a while, so am re-reading this, i had forgotten about it. I'm not sure why i claimed it was valid yaml...its not. Parse errors[0] that happen after later normally are very specific, so i was surprised when the top file fails it simply told me it does not exist. I understand the merging that needs to occur, and don't dismiss the complexity.

I think, as @OrangeDog properly moved it to feature, its as simple as

The presumed complexity of opening up a PR in this part of the code, seemed silly to me.
Thanks for following up.

[0] - these are jinja errors, so not a great comparison.

Edit: I appreciate that this stayed open. 👍