[bug#57473,v2,3/4] gnu: fail2ban-service-type: Fix type of enabled? field.

* gnu/services/security.scm (fail2ban-jail-configuration)
[enabled?]: Should be maybe-boolean, which is how normaly fail2ban
is configured. This also allows sections without enabled field
like @samp{[DEFAULT]}.
* doc/guix.texi: Type of enabled? is maybe-boolean.
---
 doc/guix.texi             | 2 +-
 gnu/services/security.scm | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Hi,

muradm <mail@muradm.net> writes:

> * gnu/services/security.scm (fail2ban-jail-configuration)
> [enabled?]: Should be maybe-boolean, which is how normaly fail2ban
> is configured. This also allows sections without enabled field
> like @samp{[DEFAULT]}.
> * doc/guix.texi: Type of enabled? is maybe-boolean.
> ---
>  doc/guix.texi             | 2 +-
>  gnu/services/security.scm | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/doc/guix.texi b/doc/guix.texi
> index 7199ff6bc2..ebfc8fb3fa 100644
> --- a/doc/guix.texi
> +++ b/doc/guix.texi
> @@ -36656,7 +36656,7 @@ Available @code{fail2ban-jail-configuration} fields are:
>  @item @code{name} (type: string)
>  Required name of this jail configuration.
>  
> -@item @code{enabled?} (default: @code{#t}) (type: boolean)
> +@item @code{enabled?} (type: maybe-boolean)
>  Whether this jail is enabled.
>  
>  @item @code{backend} (type: maybe-symbol)
> diff --git a/gnu/services/security.scm b/gnu/services/security.scm
> index 7f66c462f3..3283a582f2 100644
> --- a/gnu/services/security.scm
> +++ b/gnu/services/security.scm
> @@ -197,7 +197,7 @@ (define-configuration fail2ban-jail-configuration
>     "Required name of this jail configuration."
>     empty-serializer)
>    (enabled?
> -   (boolean #t)
> +   maybe-boolean

I made this change willingly, because it doesn't make sense to me to
define a jail configuration that is disabled by default.

Does that make sense?

Thanks,

Maxim

Hi,

Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

> Hi,
>
> muradm <mail@muradm.net> writes:
>
>> * gnu/services/security.scm (fail2ban-jail-configuration)
>> [enabled?]: Should be maybe-boolean, which is how normaly 
>> fail2ban
>> is configured. This also allows sections without enabled field
>> like @samp{[DEFAULT]}.
>> * doc/guix.texi: Type of enabled? is maybe-boolean.
>> ---

[...]

>> @@ -197,7 +197,7 @@ (define-configuration 
>> fail2ban-jail-configuration
>>     "Required name of this jail configuration."
>>     empty-serializer)
>>    (enabled?
>> -   (boolean #t)
>> +   maybe-boolean
>
> I made this change willingly, because it doesn't make sense to 
> me to
> define a jail configuration that is disabled by default.
>
> Does that make sense?

When I was implementing configuration, I was thinking the same.

However, while trying to find sensible or hardcoded defaults,
looking at code and usage examples, I found that configuration
of fail2ban is quite loose, than one can anticipiate. Variables
can be overriden from many places, so absence of variable also
becomes as important. So I decided to opt-in for most fields
as maybe-.

IMHO, this is why they had to make --d/--dp flags to
dump effective configuration :)

I actually mention that in the docs, where it says that
unspecified values are not serialized in favor of whatever
are defaults and/or values specified elsewhere. Which are
basically hard to track.

And finally there is a [DEFAULT] section which applies to
all jails. But it is not a jail by itself. So if there is
"enabled = false" or "enabled = true", fail2ban fails to
start, because of unexpected variable for that section.

I hope it is clear enough.

Thanks in advance,
muradm

>
> Thanks,
>
> Maxim

Hi muradm,

muradm <mail@muradm.net> writes:

> Hi,
>
> Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:
>
>> Hi,
>>
>> muradm <mail@muradm.net> writes:
>>
>>> * gnu/services/security.scm (fail2ban-jail-configuration)
>>> [enabled?]: Should be maybe-boolean, which is how normaly fail2ban
>>> is configured. This also allows sections without enabled field
>>> like @samp{[DEFAULT]}.
>>> * doc/guix.texi: Type of enabled? is maybe-boolean.
>>> ---
>
> [...]
>
>>> @@ -197,7 +197,7 @@ (define-configuration
>>> fail2ban-jail-configuration
>>>     "Required name of this jail configuration."
>>>     empty-serializer)
>>>    (enabled?
>>> -   (boolean #t)
>>> +   maybe-boolean
>>
>> I made this change willingly, because it doesn't make sense to me to
>> define a jail configuration that is disabled by default.
>>
>> Does that make sense?
>
> When I was implementing configuration, I was thinking the same.
>
> However, while trying to find sensible or hardcoded defaults,
> looking at code and usage examples, I found that configuration
> of fail2ban is quite loose, than one can anticipiate. Variables
> can be overriden from many places, so absence of variable also
> becomes as important. So I decided to opt-in for most fields
> as maybe-.
>
> IMHO, this is why they had to make --d/--dp flags to
> dump effective configuration :)
>
> I actually mention that in the docs, where it says that
> unspecified values are not serialized in favor of whatever
> are defaults and/or values specified elsewhere. Which are
> basically hard to track.

I had looked at how this all works, and while it's true that omitting
values seems the norm, in the context of Guix, it seems OK diverge for
this for the 'enabled' value of extra jail configurations (jail.local).

> And finally there is a [DEFAULT] section which applies to all
> jails. But it is not a jail by itself. So if there is "enabled =
> false" or "enabled = true", fail2ban fails to start, because of
> unexpected variable for that section.

You mean the default section from $(guix build
fail2ban)/etc/fail2ban/jail.conf?  It has 'enabled = false', and we
don't generate such section ourselves, so I don't understand your point.
The test suite also passes; can you think of a scenario that would make
it fail?  We could add a test for it.

I've pushed the other corrections, thanks!

Closing (but we can continue the discussion here).

Maxim