Show error on unrecognized configuration keys

[Drugoy]

v2.8.0

...
serving hooks on http://0.0.0.0:9000/hooks/{id}
os signal watcher ready
incoming HTTP POST request from 127.0.0.1:42254
test_webhook got matched
test_webhook hook triggered successfully
200 | 0 B | 124.98us | localhost:9000 | POST /hooks/test_webhook
error in exec "/home/webhook/bin": permission denied

[moorereason]

You have a permissions problem. The process running webhook must be able to access the exec path and execute it.

See also: #516

[Drugoy]

yeah, no The problem is the poor error handling and poor error logging. I've spent last few hours going crazy over the typo in the hooks.yaml I didn't see. Neither -verbose nor -debug were of any help to me.

[moorereason]

The problem is the poor error handling and poor error logging [...] over the typo in the hooks.yaml

How would anyone know that based upon your initial report? So, I guess we're even--our error reporting is at least as good as your bug reporting. :wink:

Please explain what the typo was and how webhook's error reporting could have been more helpful.

[Drugoy]

Now you get my intention (and poorly covered anger over the time I've lost looking for the typo).

What does prometheus do when it starts? It runs a validation against all of its config files. And it errors out (with a meaningful text pointing to the actual error) in case of one.

Now webhook's configuration consists of just 1 file (if I'm not wrong) with a set of rules defining hooks. Those rules could be defined in JSON or YAML and these are pretty common nowadays. To work with JSON decoding there's Unmarshal function and quick googling shows there's more than 1 solution for YAML.

[adnanh]

We'd gladly accept a PR for this feature :-)

[moorereason]

@Drugoy, I get that you're frustrated, but I'd still like to see a detailed report of the problem. What exactly was the typo? What was the fix? What error message or startup validation did you expect to see given this typo?

[Drugoy]

Honestly, you don't need more than I already wrote: The configuration needs to be chopped into 3 parts: necessary parts, optional parts and the rest (should be ignored). Obviously, the necessary part needs to be validated the harshest way. What does the app do when it reads config file? Basically, it constructs an object 'config' with sub-objects like 'a rule', which, in turn, consist of sub-objects like 'a key'/'a param'/'a field', which at some point become atomic (can't be treated as something compound, something that can be divided into smaller parts). And thus, to validate a config means to validate all it's sub-objects recursively: Can a rule have blank id? Can a rule have no execute-command? Can a rule have a declared, but an empty list of pass-environment-to-command? Can a config / a rule / a ${this_particular_list} have some specified keys that are not a part of what's documented? Or should they at very least be a reason for some WARN?

Answers to questions like this should be a code that runs at some config.Read() event that should yield some meaningful (for a human reader) output, like:

• "error: rule №7 has no execute-command specified"
• "warn: rule №4 has an unknown key piss-enbironment-to-command, check for typos"
• "warn: rule №8's env №2 has no name key, this env will be declared as an empty string" and so on.
• "error: rule №1's execute-command points to a non-existent command, check path"
• "error: rule №5's execute-command points to a file with no execute permissions, chmod +x it"
• "error: rule №6's has command-working-directory that the owner of this script's process has no access permissions"

The typo in my case was a misspelled execute-command key and the thing is that at startup the program even listed that rule's name (id) as an item in the list of recognized hooks.

[moorereason]

The typo in my case was a misspelled execute-command key

Thanks for the clarification.

[nopcoder]

We'd gladly accept a PR for this feature :-)

Opened PR to fix this one, happy to get feedback.