diff mbox series

[bug#36239] opensmtpd-service-type to extend pam

Message ID 87k1dgsjs9.fsf@nckx
State Accepted
Headers show
Series [bug#36239] opensmtpd-service-type to extend pam | expand

Checks

Context Check Description
cbaines/applying patch fail Apply failed

Commit Message

Tobias Geerinckx-Rice June 20, 2019, 9:11 a.m. UTC
Kristofer, Ludo',

Kristofer Buffington wrote:
> The opensmtpd package includes --with-pam at build time, but the 
> service
> does not extend pam. As a result, an administrator would need to
> maintain a seperate passwd or other authentication
> mechanism instead of using PAM.

Thanks for the patch!  Looks good to me, assuming you've 
battle-tested the thing.  I've Guixed up the commit message, but 
let's give Ludo' a chance to respond.

> walletsworth

I feel all, like, personally offended.

Kind regards,

T G-R

Comments

Tobias Geerinckx-Rice June 20, 2019, 9:23 a.m. UTC | #1
Ludo',

> +(define %opensmtpd-pam-services
> +  (list (unix-pam-service "smtpd")))

Hm.  This is consistent with the (inconsistent) use of % in (gnu 
services mail) even for unexported variables, but that doesn't 
match my understanding of %.

I'd like to remove it here and elsewhere in the file:

  $ grep define.*-activation gnu/services/mail.scm
  (define (%dovecot-activation config) ← local
  (define opensmtpd-activation
  (define exim-activation

Would that be noise?

Kind regards,

T G-R
Ludovic Courtès June 20, 2019, 11:52 a.m. UTC | #2
Hello,

Tobias Geerinckx-Rice <me@tobias.gr> skribis:

> From 60f3fd35ac4925e86556781ccc2620f54dccacd9 Mon Sep 17 00:00:00 2001
> From: Kristofer Buffington <kristoferbuffington@gmail.com>
> Date: Wed, 12 Jun 2019 16:34:30 -0400
> Subject: [PATCH] services: opensmtpd: Extend the PAM service.
>
> * gnu/services/mail.scm (%opensmtpd-pam-services): New variable.
> (opensmtpd-service-type)[extensions]: Add it, extending
> PAM-ROOT-SERVICE-TYPE.
>
> Signed-off-by: Tobias Geerinckx-Rice <me@tobias.gr>

LGTM, please push Tobias!

Thanks,
Ludo’.
Ludovic Courtès June 20, 2019, 11:54 a.m. UTC | #3
Tobias Geerinckx-Rice <me@tobias.gr> skribis:

>> +(define %opensmtpd-pam-services
>> +  (list (unix-pam-service "smtpd")))
>
> Hm.  This is consistent with the (inconsistent) use of % in (gnu
> services mail) even for unexported variables, but that doesn't match
> my understanding of %.

I think I’d use it here; that’s consistent with the rest of the file
actually, no?  To me everything looks OK as it is.

Thanks,
Ludo’.
Tobias Geerinckx-Rice June 20, 2019, 12:18 p.m. UTC | #4
Ludovic Courtès wrote:
> I think I’d use it here; that’s consistent with the rest of the 
> file
> actually, no?  To me everything looks OK as it is.

Then I'm not sure I grok % (which is likely as I've never read a 
description, only induced its use).

What's the difference between:

 (define (%dovecot-activation config) …

and

 (define opensmtpd-activation …
 (define exim-activation …

?  I thought it had something to do with being exports/globals but 
here (%opensmtpd-accounts, etc.) it is used simply to say ‘this is 
a list’.

Will push,

T G-R
Ludovic Courtès June 20, 2019, 12:49 p.m. UTC | #5
Tobias Geerinckx-Rice <me@tobias.gr> skribis:

> Ludovic Courtès wrote:
>> I think I’d use it here; that’s consistent with the rest of the file
>> actually, no?  To me everything looks OK as it is.
>
> Then I'm not sure I grok % (which is likely as I've never read a
> description, only induced its use).
>
> What's the difference between:
>
> (define (%dovecot-activation config) …
>
> and
>
> (define opensmtpd-activation …
> (define exim-activation …
>
> ?  I thought it had something to do with being exports/globals but
> here (%opensmtpd-accounts, etc.) it is used simply to say ‘this is a
> list’.

It’s a long story.  :-)

Initially in Guile the ‘%’ prefix was used for “system” bindings, thing
you don’t normally use as a normal user.  Then I think I started using
it for things that are meant to be “internal” or variables meant to be
read-only (not procedures).  In Guix we sort of follow that but somewhat
extend the meaning.

In the example above, I’d rather write ‘dovecot-activation’ (without
prefix, because it’s a procedure), but I’d probably write
‘%exim-activation’, depending on the phase of the moon.

HTH!

Ludo’.
Tobias Geerinckx-Rice June 20, 2019, 1:54 p.m. UTC | #6
Ludo',

Ludovic Courtès wrote:
>> I thought [%] had something to do with being exports/globals 
>> but
>> here (%opensmtpd-accounts, etc.) it is used simply to say ‘this 
>> is a
>> list’.
>
> It’s a long story.  :-)

Thanks for taking the time to tell it.

> Initially in Guile the ‘%’ prefix was used for “system” 
> bindings, thing
> you don’t normally use as a normal user.  Then I think I started 
> using
> it for things that are meant to be “internal” or variables meant 
> to be
> read-only (not procedures).  In Guix we sort of follow that but 
> somewhat
> extend the meaning.

s/extend/invert/ if I were feeling sassy.  I'd be lying if I said 
I understood it clearly now, but it'll be a lot easier to placate 
my obsessive side with a tasty bag of hysterical raisins than to 
keep searching for a pattern that's just not there.

> In the example above, I’d rather write ‘dovecot-activation’ 
> (without
> prefix, because it’s a procedure), but I’d probably write
> ‘%exim-activation’, depending on the phase of the moon.

Now you're just trollin'.

Thanks,

T G-R
Tobias Geerinckx-Rice June 20, 2019, 2:01 p.m. UTC | #7
Ludovic Courtès wrote:
> LGTM, please push Tobias!

Done with ace8e269a40b31a9c6f413c0674529880da6a48e.

Thanks, Kristofer!

Kind regards,

T G-R
diff mbox series

Patch

From 60f3fd35ac4925e86556781ccc2620f54dccacd9 Mon Sep 17 00:00:00 2001
From: Kristofer Buffington <kristoferbuffington@gmail.com>
Date: Wed, 12 Jun 2019 16:34:30 -0400
Subject: [PATCH] services: opensmtpd: Extend the PAM service.

* gnu/services/mail.scm (%opensmtpd-pam-services): New variable.
(opensmtpd-service-type)[extensions]: Add it, extending
PAM-ROOT-SERVICE-TYPE.

Signed-off-by: Tobias Geerinckx-Rice <me@tobias.gr>
---
 gnu/services/mail.scm | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/gnu/services/mail.scm b/gnu/services/mail.scm
index 0e85ada3d0..3de0b4c2f3 100644
--- a/gnu/services/mail.scm
+++ b/gnu/services/mail.scm
@@ -3,6 +3,7 @@ 
 ;;; Copyright © 2017, 2018 Clément Lassieur <clement@lassieur.org>
 ;;; Copyright © 2017 Carlo Zancanaro <carlo@zancanaro.id.au>
 ;;; Copyright © 2017 Tobias Geerinckx-Rice <me@tobias.gr>
+;;; Copyright © 2019 Kristofer Buffington <kristoferbuffington@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -1667,6 +1668,9 @@  accept from local for any relay
            (mkdir-p "/var/spool/smtpd")
            (chmod "/var/spool/smtpd" #o711))))))
 
+(define %opensmtpd-pam-services
+  (list (unix-pam-service "smtpd")))
+
 (define opensmtpd-service-type
   (service-type
    (name 'opensmtpd)
@@ -1675,6 +1679,8 @@  accept from local for any relay
                              (const %opensmtpd-accounts))
           (service-extension activation-service-type
                              opensmtpd-activation)
+          (service-extension pam-root-service-type
+                             (const %opensmtpd-pam-services))
           (service-extension profile-service-type
                              (compose list opensmtpd-configuration-package))
           (service-extension shepherd-root-service-type
-- 
2.22.0