[PATCH] gnu: Use a directory owned by ntpd user for drift file.

[PATCH] gnu: Use a directory owned by ntpd user for drift file.

[John Darrington]

Fixes bug #24366




* gnu/services/networking.scm (ntp-shepherd-service): Create new
directory at startup.
---
 gnu/services/networking.scm | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
index 71f49a0..f983961 100644
--- a/gnu/services/networking.scm
+++ b/gnu/services/networking.scm
@@ -2,6 +2,7 @@
 ;;; Copyright © 2013, 2014, 2015, 2016 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2015 Mark H Weaver <mhw@netris.org>
 ;;; Copyright © 2016 Efraim Flashner <efraim@flashner.co.il>
+;;; Copyright © 2016 John Darrington <jmd@gnu.org>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -272,7 +273,7 @@ Protocol (DHCP) client, on all the non-loopback network interfaces."
      (let ()
        ;; TODO: Add authentication support.
        (define config
-         (string-append "driftfile /var/run/ntp.drift\n"
+         (string-append "driftfile /var/run/ntpd/ntp.drift\n"
                         (string-join (map (cut string-append "server " <>)
                                           servers)
                                      "\n")
@@ -307,13 +308,28 @@ restrict -6 ::1\n"))
          (home-directory "/var/empty")
          (shell #~(string-append #$shadow "/sbin/nologin")))))
 
+
+(define (ntp-service-activation config)
+  "Return the activation gexp for config"
+  #~(begin
+      (use-modules (guix build utils))
+
+      (define %user
+        (getpw "ntpd"))
+
+      (let ((directory "/var/run/ntpd"))
+          (mkdir-p directory)
+          (chown directory (passwd:uid %user) (passwd:gid %user)))))
+
 (define ntp-service-type
   (service-type (name 'ntp)
                 (extensions
                  (list (service-extension shepherd-root-service-type
                                           ntp-shepherd-service)
                        (service-extension account-service-type
-                                          (const %ntp-accounts))))))
+                                          (const %ntp-accounts))
+                       (service-extension activation-service-type
+                                          ntp-service-activation)))))
 
 (define* (ntp-service #:key (ntp ntp)
                       (servers %ntp-servers))
-- 
2.1.4

Re: [PATCH] gnu: Use a directory owned by ntpd user for drift file.

[Ludovic Courtès]

John Darrington <jmd@gnu.org> skribis:

> Fixes bug #24366

Rather:

  Fixes <http://bugs.gnu.org/24366>.

(See ‘git log’ for examples.)

>
>
>
> * gnu/services/networking.scm (ntp-shepherd-service): Create new
> directory at startup.

Please mention the new variables and changes.

> +(define (ntp-service-activation config)
> +  "Return the activation gexp for config"
                                     ^^^^^^^
Capitalize the variable name and add a period.

> +  #~(begin
> +      (use-modules (guix build utils))

Please wrap the gexp in:

  (with-imported-modules '((guix build utils))
    #~(…))

> +      (let ((directory "/var/run/ntpd"))
> +          (mkdir-p directory)
            ^^
Indentation.

OK with these changes.

Thanks for fixing this bug!

Ludo’.

Re: [PATCH] gnu: Use a directory owned by ntpd user for drift file.

[Vincent Legoll]

Hello,

> +(define (ntp-service-activation config)
> +  "Return the activation gexp for config"
> +  #~(begin
> +      (use-modules (guix build utils))
> +
> +      (define %user
> +        (getpw "ntpd"))
> +
> +      (let ((directory "/var/run/ntpd"))
> +          (mkdir-p directory)
> +          (chown directory (passwd:uid %user) (passwd:gid %user)))))
> +

Excuse my scheme-newbie questions, but

- why did you use %user (I thought %s were for kind of global vars) ?
- why did you use define and not put it in the let just below ? Idon't see
it being used elsewhere...

-- 
Vincent Legoll

Re: [PATCH] gnu: Use a directory owned by ntpd user for drift file.

[John Darrington]

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

On Thu, Sep 08, 2016 at 08:57:57AM +0200, Vincent Legoll wrote:
     Hello,
     
     > +(define (ntp-service-activation config)
     > +  "Return the activation gexp for config"
     > +  #~(begin
     > +      (use-modules (guix build utils))
     > +
     > +      (define %user
     > +        (getpw "ntpd"))
     > +
     > +      (let ((directory "/var/run/ntpd"))
     > +          (mkdir-p directory)
     > +          (chown directory (passwd:uid %user) (passwd:gid %user)))))
     > +
     
     - why did you use %user (I thought %s were for kind of global vars) ?
     - why did you use define and not put it in the let just below ? Idon't see
     it being used elsewhere...
     

I think you need to look a little harder, because the answer to both your
(very valid) questions is : I copied it from other, existing examples.

J'

-- 
Avoid eavesdropping.  Send strong encrypted email.
PGP Public key ID: 1024D/2DE827B3 
fingerprint = 8797 A26D 0854 2EAB 0285  A290 8A67 719C 2DE8 27B3
See http://sks-keyservers.net or any PGP keyserver for public key.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH] gnu: Use a directory owned by ntpd user for drift file.

[Vincent Legoll]

> I think you need to look a little harder, because the answer to both your
> (very valid) questions is : I copied it from other, existing examples.

Ah, couldn't have seen it, I only read the patch, not the entire file...

You copied it from : tor-hidden-service-activation ?

So, can an old-schemer tell me if the questions make sense, or is
it equivalent ?

-- 
Vincent Legoll

Re: [PATCH] gnu: Use a directory owned by ntpd user for drift file.

[Ludovic Courtès]

Hi,

Vincent Legoll <vincent.legoll@gmail.com> skribis:

>> +(define (ntp-service-activation config)
>> +  "Return the activation gexp for config"
>> +  #~(begin
>> +      (use-modules (guix build utils))
>> +
>> +      (define %user
>> +        (getpw "ntpd"))
>> +
>> +      (let ((directory "/var/run/ntpd"))
>> +          (mkdir-p directory)
>> +          (chown directory (passwd:uid %user) (passwd:gid %user)))))
>> +
>
> Excuse my scheme-newbie questions, but
>
> - why did you use %user (I thought %s were for kind of global vars) ?

In the context of this gexp it’s a global variable.

> - why did you use define and not put it in the let just below ? Idon't see
> it being used elsewhere...

Both styles are OK, though putting it in the ‘let’ would have made it
more concise, indeed.  :-)

Ludo’.