[pve-devel] [PATCH v4 firewall 1/2] implement fail2ban backend and API

[Thomas Lamprecht]

On 19.10.21 15:43, Dominik Csapak wrote:
> while the code looks ok IMHO, i have some general questions:
> * does it really make sense to hard depend on fail2ban?
>   could it not also make sense to have it as 'recommends' or 'suggests'?
>   setting enabled to 1 could then check if its installed and
>   raise an error

fwiw, it does not make sense to me to have a hard dependency here, as I pointed
out in pretty much every revision of this series, that and most other things (e.g.,
trying if we can simply generate the rules here ourself) where rather ignored so
after the third iteration I went "tit for tat" and ignored the whole thing..

> 
> * if we do not plan to add more fail2ban options in our config,
>   i would rather see a combined fail2ban option (propertystring?)
>   that would go into the general host firewall options
> 
>   that way we would not have to c&p the whole config parsing/setting api
>   and could have a single new option line in the gui instead
>   of a whole new panel with only 3 options (i think the majority of our
>   users will not use fail2ban)

would make much more sense, it's an simple option and bringing down UX by
crowding the interface for a simple an option that one sets one-time only
anyway seems not ideal to me..