guix-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

bug#57473: [PATCH v2 3/4] gnu: fail2ban-service-type: Fix type of enable


From: Maxim Cournoyer
Subject: bug#57473: [PATCH v2 3/4] gnu: fail2ban-service-type: Fix type of enabled? field.
Date: Thu, 01 Sep 2022 09:03:43 -0400
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/28.1 (gnu/linux)

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





reply via email to

[Prev in Thread] Current Thread [Next in Thread]