saltstack-formulas / nagios-formula

nagios-formula
http://docs.saltstack.com/en/latest/topics/development/conventions/formulas.html
Other
20 stars 73 forks source link

Illegal characters in Pillar file #26

Closed vishal-biyani closed 9 years ago

vishal-biyani commented 9 years ago

I am using the sample Pillar file for setting up Nagios for a small demo. I think there are two entries in Pillar.example which will cause a pillar render error. After removing those two, all works fine. I am using the version: salt 2015.8.1 (Beryllium)

Error:

2015-10-30 05:30:48,910 [salt.pillar      ][CRITICAL][6715] Pillar render error: Rendering SLS 'nagios' failed. Please see master log for details.
2015-10-30 05:30:48,909 [salt.pillar      ][CRITICAL][6717] Rendering SLS 'nagios' failed, render error:
found character '`' that cannot start any token; line 108

---
[...]
    low_host_flap_threshold: 5.0
    high_host_flap_threshold: 20.0
    date_format: iso8601
    enable_embedded_perl: 1
    use_embedded_perl_implicitly: 1
    illegal_object_name_chars: `~!$%^&*|'"<>?,():     <======================
    illegal_macro_output_chars: `~$&|'"<>
    use_regexp_matching: 0
    use_true_regexp_matching: 0
    admin_email: root@localhost
    admin_pager: pageroot@localhost
[...]

---
gravyboat commented 9 years ago

@vishal-biyani Thanks for pointing this out. What happens if you drop those into a different position? so:

    illegal_object_name_chars: ~`!$%^&*|'"<>?,():
    illegal_macro_output_chars: ~`$&|'"<>

@babilen Looks like this was added in https://github.com/saltstack-formulas/nagios-formula/commit/f172cf9e2b6165cf24bdfb862f5c52309dec1c14#diff-aef06cd118a72975a36aa9aeb06e8af6R109 did you ever encounter any issue like this in your testing?

vishal-biyani commented 9 years ago

@gravyboat Still the same:

2015-10-30 08:17:40,280 [salt.pillar      ][CRITICAL][6769] Rendering SLS 'nagios' failed, render error:
mapping values are not allowed here; line 108

---
[...]
    low_host_flap_threshold: 5.0
    high_host_flap_threshold: 20.0
    date_format: iso8601
    enable_embedded_perl: 1
    use_embedded_perl_implicitly: 1
    illegal_object_name_chars: ~`!$%^&*|'"<>?,():    <======================
    illegal_macro_output_chars: ~`$&|'"<>
gravyboat commented 9 years ago

Weird, let's see what Babilen says, it's only 9:30 AM where he lives.

vishal-biyani commented 9 years ago

@gravyboat On a related note (Did not want to create a new issue altogether for such a small issue) - it would be good if you specify if the defaults in pillar.example relate to which flavor of Linux? I spent some time to get a working formula - primarily because of mismatch between service names of the OS I was using and the one stated in Pillar.example

wwentland commented 9 years ago

I guess it would work if you use illegal_object_name_chars: "~!$%^&*|'"<>?,():"` there which quotes it explicitly. Would you mind trying that @vishal-biyani ?

vishal-biyani commented 9 years ago

@babilen No, that also does not work. From what I have gathered so far - in YAML a value can not start with special characters unless they are contained with a ' or " - in our case the matching pairs are not enclosing the whole string as there is a double quote and single quote in the string itself.

I am not a YAML/Python expert but applying concept of escape sequence - following syntax seems to work

"~!$%^&*|'\"<>?,():`"

Here we are enclosing it all in " and the one " we have in middle is not being treated as end due to \

I am not sure if this is the correct way, some YAML expert can comment may be. The only test I carried is simple: I have above string in my pillar file and I run:

sudo salt '*' pillar.items

If it can list all pillar items without error and also list the intended key in proper format (Notice that it has read \" in middle of string as " :) :

         illegal_object_name_chars:
                ~!$%^&*|'"<>?,():`'

Does that sound like a reasonable fix?

wwentland commented 9 years ago

I'd personally prefer "~!$%^&*|'\"<>?,():"as it is more explicitly escaping special characters and therefore more obvious as to *why* it is the way it is. Sorry for missing the pesky"` in my example earlier.

vishal-biyani commented 9 years ago

@babilen Sure - agree. Are you pushing a pull request or shall I go ahead?

vishal-biyani commented 9 years ago

@babilen cool, noticed your pull after commenting. Thanks for fixing.

wwentland commented 9 years ago

Thank you for bringing it to our attention and the debugging!

gravyboat commented 9 years ago

Closing this out, thanks for getting this resolved guys!