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

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

On 20.10.21 14:11, Oguz Bektas wrote:
> On Tue, Oct 19, 2021 at 03:43:49PM +0200, 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
>>
>> * 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)
> 
>>
>> does that make sense to you?
>>
> 
> well if we hide it like that inside the menu, then surely nobody will
> use it ;)
> 
> separate panel makes it more visible IMO. it shouldn't be hidden 3
> layers deep in the options menu (Host -> Firewall -> Options -> Fail2ban
> -> Enable) for such a simple feature, i think a lot more people would
> use it if it's placed in a visible location (Host -> Fail2ban ->
> Enable). if you really insist on putting it in the firewall options menu
> then i'll have to insist for it to be installed & enabled by default :)

With that arguing every option would need to be its own panel placed at the
top-level.. PVE's interface is complex as is, sensible grouping simple one time
option actually helps in UX and to find stuff, documentation can ensure features
are found, they provide a central searchable place for that, after all.