Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not throw an exception when an invalid requisite is set #35226

Closed
mathieubouchard opened this issue Aug 5, 2016 · 2 comments
Closed

Do not throw an exception when an invalid requisite is set #35226

mathieubouchard opened this issue Aug 5, 2016 · 2 comments
Labels
Bug broken, incorrect, or confusing behavior Core relates to code central or existential to Salt fixed-pls-verify fix is linked, bug author to confirm fix P4 Priority 4 severity-low 4th level, cosemtic problems, work around exists
Milestone

Comments

@mathieubouchard
Copy link
Contributor

Description of Issue/Question

When an invalid require tag is set, there are no safety check in the code and an exception will be raised instead of showing a nice explanation of what is going on. It makes it really difficult to diagnose what's wrong with the SLS file.

Setup

Example SLS :

{% set pil = pillar['git'] %}

git:
  pkg.installed:
    - require:
      - file: {{ pil }}

The following exception will be raised :

    The minion function caused an exception: Traceback (most recent call last):
      File "/usr/lib/python2.6/site-packages/salt/minion.py", line 1320, in _thread_return
        return_data = executor.execute()
      File "/usr/lib/python2.6/site-packages/salt/executors/direct_call.py", line 28, in execute
        return self.func(*self.args, **self.kwargs)
      File "/usr/lib/python2.6/site-packages/salt/modules/state.py", line 722, in highstate
        whitelist=kwargs.get('whitelist')
      File "/usr/lib/python2.6/site-packages/salt/state.py", line 3271, in call_highstate
        os.umask(cumask)
      File "/usr/lib/python2.6/site-packages/salt/state.py", line 2276, in call_high
        return errors
      File "/usr/lib/python2.6/site-packages/salt/state.py", line 1799, in call_chunks
        running = self.call_chunk(low, running, chunks)
      File "/usr/lib/python2.6/site-packages/salt/state.py", line 2066, in call_chunk
        return running
      File "/usr/lib/python2.6/site-packages/salt/state.py", line 1982, in call_chunk
        requisites.append('prerequired')
      File "/usr/lib/python2.6/site-packages/salt/state.py", line 1868, in check_requisite
        if (fnmatch.fnmatch(chunk['name'], req_val) or
      File "/usr/lib64/python2.6/fnmatch.py", line 43, in fnmatch
        return fnmatchcase(name, pat)
      File "/usr/lib64/python2.6/fnmatch.py", line 75, in fnmatchcase
        res = translate(pat)
      File "/usr/lib64/python2.6/fnmatch.py", line 90, in translate
        c = pat[i]
    KeyError: 0

The line 1868 in /usr/lib/python2.6/site-packages/salt/state.py tries to compare two strings. In this case, a dictionary will be passed instead of a string. When the fnmatch method will try to match the values, it will access req_val[0], which is not a valid key for the dictionary.

Steps to Reproduce Issue

Write an invalid SLS with a require on a dictionary.

Versions Report

Salt Version:
Salt: 2016.3.2

Dependency Versions:
cffi: Not Installed
cherrypy: 3.2.2
dateutil: 1.4.1
gitdb: Not Installed
gitpython: Not Installed
ioflo: Not Installed
Jinja2: 2.2.1
libgit2: Not Installed
libnacl: Not Installed
M2Crypto: 0.20.2
Mako: Not Installed
msgpack-pure: Not Installed
msgpack-python: 0.4.6
mysql-python: Not Installed
pycparser: Not Installed
pycrypto: 2.6.1
pygit2: Not Installed
Python: 2.6.6 (r266:84292, May 22 2015, 08:34:51)
python-gnupg: Not Installed
PyYAML: 3.10
PyZMQ: 14.5.0
RAET: Not Installed
smmap: Not Installed
timelib: Not Installed
Tornado: 4.2.1
ZMQ: 4.0.5

System Versions:
dist: redhat 6.7 Santiago
machine: x86_64
release: 2.6.32-573.22.1.el6.x86_64
system: Linux
version: Red Hat Enterprise Linux Server 6.7 Santiago

@Ch3LL
Copy link
Contributor

Ch3LL commented Aug 5, 2016

Agreed we should not be stacktracing here and put in a more useful error.

For future reference I was able to replicate this using the sls file above with the following pillar data:

git:
  test1: test

@Ch3LL Ch3LL added Bug broken, incorrect, or confusing behavior severity-low 4th level, cosemtic problems, work around exists Core relates to code central or existential to Salt P4 Priority 4 labels Aug 5, 2016
@Ch3LL Ch3LL modified the milestones: Blocked, Approved Aug 5, 2016
cachedout pushed a commit to cachedout/salt that referenced this issue Aug 11, 2016
@cachedout
Copy link
Contributor

Please see #35373

@cachedout cachedout added the fixed-pls-verify fix is linked, bug author to confirm fix label Aug 11, 2016
rallytime pushed a commit that referenced this issue Aug 11, 2016
* Raise SaltRenderError on bad requisite

Refs #35226

* Remove extra vals
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug broken, incorrect, or confusing behavior Core relates to code central or existential to Salt fixed-pls-verify fix is linked, bug author to confirm fix P4 Priority 4 severity-low 4th level, cosemtic problems, work around exists
Projects
None yet
Development

No branches or pull requests

3 participants