[bug#57473] [PATCH] gnu: fail2ban-service-type: Fix field name serialization.

[bug#57473] [PATCH] gnu: fail2ban-service-type: Fix field name serialization.

[muradm]

* gnu/services/security.scm: Wrong condition in string-filter
function in fail2ban-jail-configuration-serialize-field-name.
---
 gnu/services/security.scm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gnu/services/security.scm b/gnu/services/security.scm
index 1e0360c07f..9ab23ffb46 100644
--- a/gnu/services/security.scm
+++ b/gnu/services/security.scm
@@ -127,7 +127,7 @@ (define (fail2ban-jail-configuration-serialize-field-name name)
           (string-append "bantime." (substring name 9))))
         ((string-contains name "-")
          (fail2ban-jail-configuration-serialize-field-name
-          (string-filter (lambda (c) (equal? c #\-)) name)))
+          (string-filter (lambda (c) (not (equal? c #\-))) name)))
         (else name)))
 
 (define (fail2ban-jail-configuration-serialize-string field-name value)
-- 
2.37.2

[bug#57473] [PATCH v2 1/4] gnu: fail2ban-service-type: Fix field name serialization.

[muradm]

* gnu/services/security.scm: Wrong condition in string-filter
function in fail2ban-jail-configuration-serialize-field-name.
---
 gnu/services/security.scm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gnu/services/security.scm b/gnu/services/security.scm
index 1e0360c07f..9ab23ffb46 100644
--- a/gnu/services/security.scm
+++ b/gnu/services/security.scm
@@ -127,7 +127,7 @@ (define (fail2ban-jail-configuration-serialize-field-name name)
           (string-append "bantime." (substring name 9))))
         ((string-contains name "-")
          (fail2ban-jail-configuration-serialize-field-name
-          (string-filter (lambda (c) (equal? c #\-)) name)))
+          (string-filter (lambda (c) (not (equal? c #\-))) name)))
         (else name)))
 
 (define (fail2ban-jail-configuration-serialize-string field-name value)
-- 
2.37.2

[bug#57473] [PATCH v2 2/4] gnu: fail2ban-service-type: Improve jail configuration serialization.

[muradm]

* gnu/services/security.scm (fail2ban-jail-configuration)
[name]: Should use empty-serializer.
---
 gnu/services/security.scm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/gnu/services/security.scm b/gnu/services/security.scm
index 9ab23ffb46..7f66c462f3 100644
--- a/gnu/services/security.scm
+++ b/gnu/services/security.scm
@@ -194,7 +194,8 @@ (define-maybe fail2ban-jail-filter-configuration (prefix fail2ban-jail-configura
 (define-configuration fail2ban-jail-configuration
   (name
    string
-   "Required name of this jail configuration.")
+   "Required name of this jail configuration."
+   empty-serializer)
   (enabled?
    (boolean #t)
    "Whether this jail is enabled.")
-- 
2.37.2

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

[muradm]

* 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
    "Whether this jail is enabled.")
   (backend
    maybe-symbol
-- 
2.37.2

[bug#57473] [PATCH v2 4/4] gnu: fail2ban-service-type: Fix documentation typos.

[muradm]

---
 doc/guix.texi             | 6 +++---
 gnu/services/security.scm | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index ebfc8fb3fa..25eed72d30 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -36555,7 +36555,7 @@ is an example of a basic, explicit configuration:
              (list
               (fail2ban-jail-configuration
                (name "sshd")
-               (enabled #t))))))
+               (enabled? #t))))))
   ;; There is no implicit dependency on an actual SSH
   ;; service, so you need to provide one.
   (service openssh-service-type))
@@ -36580,7 +36580,7 @@ For example:
     openssh-service-type
     (fail2ban-jail-configuration
      (name "sshd")
-     (enabled #t)))
+     (enabled? #t)))
    (openssh-configuration ...))))
 @end lisp
 @end deffn
@@ -36660,7 +36660,7 @@ Required name of this jail configuration.
 Whether this jail is enabled.
 
 @item @code{backend} (type: maybe-symbol)
-Backend to use to detect changes in the @code{ogpath}.  The default is
+Backend to use to detect changes in the @code{log-path}.  The default is
 'auto.  To consult the defaults of the jail configuration, refer to the
 @file{/etc/fail2ban/jail.conf} file of the @code{fail2ban} package.
 
diff --git a/gnu/services/security.scm b/gnu/services/security.scm
index 3283a582f2..949f332333 100644
--- a/gnu/services/security.scm
+++ b/gnu/services/security.scm
@@ -201,7 +201,7 @@ (define-configuration fail2ban-jail-configuration
    "Whether this jail is enabled.")
   (backend
    maybe-symbol
-   "Backend to use to detect changes in the @code{ogpath}.  The default is
+   "Backend to use to detect changes in the @code{log-path}.  The default is
 'auto.  To consult the defaults of the jail configuration, refer to the
 @file{/etc/fail2ban/jail.conf} file of the @code{fail2ban} package."
 fail2ban-jail-configuration-serialize-backend)
-- 
2.37.2

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

[Maxim Cournoyer]

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

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

[muradm]

[-- Attachment #1: Type: text/plain, Size: 1756 bytes --]


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


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

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

[Maxim Cournoyer]

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