[bug#56608] [PATCH v2 2/2] gnu: tests: Add fail2ban tests.

[muradm]

Hi,

Squashed patch will come later on.

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

> Hi,
>
> muradm <mail@muradm.net> writes:
>
> [...]
>
> > --- /dev/null
> > +++ b/gnu/tests/security.scm
>
> I'd keep the tests with the introductory commit (squashed in 
> preceding
> one).

Done.

[...]

> > +(define (run-fail2ban-basic-test)
> > +
> > +  (define os
> > +    (marionette-operating-system
> > +     (simple-operating-system
> > +      (service fail2ban-service-type))
> > +     #:imported-modules '((gnu services herd)
> > +                          (guix combinators))))
>                              ^ (guix combinators) seems unused

Done including other places.

> > +  (define vm
> > +    (virtual-machine
> > +     (operating-system os)
> > +     (port-forwardings '())))
>
> (define vm (virtual-machine (operating-system os))) should be
> sufficient.

For me it does not work without specfying port-forwardings.
I get wierd error like following:

gnu/tests/security.scm:47:5: error: os: invalid field specifier

I suppose it is something todo with virtual-machine.

So I'm leaving port-forwardings as is.

[...]

> > +          (define (wait-for-unix-socket-m socket)
> > +            (wait-for-unix-socket socket marionette))
>
> Overkill as used once in scope.

Done including other places.

> > +
> > +          (test-runner-current (system-test-runner #$output))
> > +          (test-begin "fail2ban-basic-test")
> > +
> > +          (test-assert "fail2ban running"
> > +            (marionette-eval
> > +             '(begin
> > +                (use-modules (gnu services herd))
> > +                (start-service 'fail2ban))
> > +             marionette))
>
> I like to test that services can be restarted too, as in my 
> experience
> there can be races and other situations that may cause them to 
> fail
> restarting.

Done.

[...]

> > +          (test-equal "fail2ban sshd jail running"
> > +            '("Status for the jail: sshd"
> > +              "|- Filter"
> > +              "|  |- Currently failed:\t0"
> > +              "|  |- Total failed:\t0"
> > +              "|  `- File list:\t/var/log/secure"
> > +              "`- Actions"
> > +              "   |- Currently banned:\t0"
> > +              "   |- Total banned:\t0"
> > +              "   `- Banned IP list:\t"
> > +              "")
> > +            (marionette-eval
> > +             '(begin
> > +                (use-modules (ice-9 rdelim) (ice-9 popen) 
> > (rnrs io ports))
> > +                (let ((call-command
> > +                       (lambda (cmd)
> > +                         (let* ((err-cons (pipe))
> > +                                (port (with-error-to-port (cdr 
> > err-cons)
> > +                                        (lambda () 
> > (open-input-pipe cmd))))
> > +                                (_ (setvbuf (car err-cons) 
> > 'block
> > +                                            (* 1024 1024 16)))
> > +                                (result (read-delimited "" 
> > port)))
> > +                           (close-port (cdr err-cons))
> > +                           (values result (read-delimited "" 
> > (car err-cons)))))))
> > +                  (string-split
> > +                   (call-command
> > +                    (string-join (list #$%fail2ban-server-cmd 
> > "status" "sshd") " "))
> > +                   #\newline)))
> > +             marionette))
>
> Perhaps this could be turned into an Shepherd action, and the 
> Guile
> procedure could do the above to return the text output; to 
> simplify the
> test and reduce boilerplate, while providing value to the user.

[...]

> > +  (gexp->derivation "fail2ban-extending-test" test))
> > +
> > +(define %test-fail2ban-extending
>
> Perhaps %test-fail2ban-extension ?

Done, s/extending/extension/.

> Otherwise, that last test seems to
> test exactly the same things as the preceding one, so there 
> should be a
> procedure to generate the test, taking the OS as an argument to 
> avoid
> code duplication.

Done, refactored with define-syntax-rule.

> Thanks for working on this!
>
> Maxim

signature.asc
Description: PGP signature