search

theforeman / foreman_hooks

Run custom hook scripts on Foreman events
http://m0dlx.com/blog/Extending_Foreman_quickly_with_hook_scripts.html
GNU General Public License v3.0
56 stars 50 forks source link

fix: remove downcase from hook_type render #62

Closed fionera closed 3 years ago

fionera commented 3 years ago

This fixes #56 by removing the downcase call. Because of this call the tableize function got configreport which then got transformed to configreports. Instead it should have been config_reports.

lzap commented 3 years ago

Thank you, however I am a bit worried merging this - we will completely break all users who already have these hooks implemented :-( The only user-friendly solution would be to trigger two hooks in this case and documenting this behavior in the README too. Would you mind refactoring it that way?

Note we have completely new plugin which aims to deliver integration with Foreman using more user-friendly webhooks, you can customize payload via foreman templating and much more. It is way more capable, there is also a shell hooks smart proxy plugin which allows you to run shell scripts on those events.

We are just about to announce the first release of the plugin, the last missing bit is finalizing documentation. Expect it in few weeks, but the plugin is already in 2.4 release repositories so you can start using that right away.

fionera commented 3 years ago

Thank you, however I am a bit worried merging this - we will completely break all users who already have these hooks implemented :-( The only user-friendly solution would be to trigger two hooks in this case and documenting this behavior in the README too. Would you mind refactoring it that way?

The Issue we had and how I found this in the first place was that we noticed that our prometheus metrics start to disappear after the hook call, because of the Error. I dont know much more about why and how but by fixing this, the error disappeard. I dont know much ruby since I mostly write Go, so I dont really know how to tackle this refactor.

Thanks for the poke with the webhooks :D Will look into it, since thats exactly what our hook does rn

lzap commented 3 years ago

In that case I am going to close this one, we do not want to break existing hooks. Thank you!