sous-chefs / fail2ban

Development repository for the fail2ban cookbook

https://supermarket.chef.io/cookbooks/fail2ban

Apache License 2.0

58 stars 61 forks source link

Bugfix/jailresource #91

Closed EZtheOG closed 3 years ago

EZtheOG commented 3 years ago

Description

This change fixes the incorrect property values in the two resources jail and filter. It also adds documentation.

One of the things this also does is allows the ability to pass a "priority" to the jail resource so that files can be named #{new_resource.priority}-#{new_resource.jail}. But, I don't think this is needed? I can't find documentation about Fail2Ban setting jails based on priority.

Issues Resolved

https://github.com/sous-chefs/fail2ban/issues/60 https://github.com/sous-chefs/fail2ban/issues/61

Check List

[X] All tests pass. See TESTING.md for details.
[X] New functionality includes testing.
[X] New functionality has been documented in the README if applicable.

tas50 commented 3 years ago

Nice catches

kitchen-porter commented 3 years ago

	1 Warning
:warning:	This Pull Request is probably missing tests.

Generated by :no_entry_sign: Danger

xorima commented 3 years ago

Hey @EZtheOG

If you can get a changelog entry added for these changes we can get them shipped :+1:

Thanks!

EZtheOG commented 3 years ago

@Xorima - pushed Changelog edits. Sorry, didn't see that originally.

Question - Does Priority matter for jails? E.g. 50-somejailname.conf vs 21-someotherjail.conf. If Jails aren't prioritized by some numerical prefix - then I should remove the priority property from fail2ban_jail resource.

What you think?

kitchen-porter commented 3 years ago

Released as: 6.3.0

djessich commented 3 years ago

@EZtheOG Thanks for implementing the tickets I created 👍 I just wanted to mention that this PR missed setting the priority in delete action in jail resource, currently its hardcoded to 50 there..

ramereth commented 3 years ago

@djessich feel free to create a new PR which addresses that and we'll get it reviewed/merged!