[bug#75145] [PATCH] services: NetworkManager: configuration-directory

[bug#75145] [PATCH] services: NetworkManager: configuration-directory

[45mg]

Give users a way to configure NetworkManager, by specifying a directory
containing configuration files. This directory will then be symlinked to
`/etc/NetworkManager/conf.d` (NetworkManager's default configuration
directory location).

* gnu/services/networking.scm (<network-manager-configuration>)
[configuration-directory]: new option.
* doc/guix.texi: document it.

Change-Id: I243a71593b9235cc11ebf9fea1926a1840d993d2
---
 doc/guix.texi               | 30 ++++++++++++++++++++++++++++++
 gnu/services/networking.scm | 10 ++++++++--
 2 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index da4d2f5ebc..ee0fbf59f9 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -21441,6 +21441,36 @@ Networking Setup
 (VPNs).  An example of this is the @code{network-manager-openvpn}
 package, which allows NetworkManager to manage VPNs @i{via} OpenVPN.
 
+@item @code{configuration-directory} (default: @code{#f})
+A directory (file-like object) that will be symlinked as
+@file{/etc/NetworkManager/conf.d}. NetworkManager will read
+configuration files from this directory. For the configuration file
+format, see the @command{NetworkManager.conf(5)} man page.
+
+For example, you could supply an existing directory:
+
+@lisp
+(service network-manager-service-type
+         (network-manager-configuration
+          (configuration-directory
+           (local-file "files/NetworkManager/conf.d" #:recursive? #t))))
+@end lisp
+
+Or create a directory using @code{file-union}:
+
+@lisp
+(service network-manager-service-type
+         (network-manager-configuration
+          (configuration-directory
+           (file-union "my-configuration-directory"
+                       `(("existing-file" ,(local-file "001-basic.conf"))
+                         ("constructed-file" ,(plain-file "002-unmanaged.conf"
+                                                      "[keyfile]
+unmanaged-devices=interface-name:wlo1_ap
+")))))))
+@end lisp
+
+
 @end table
 @end deftp
 
diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
index 48a86b3694..7152c5a95f 100644
--- a/gnu/services/networking.scm
+++ b/gnu/services/networking.scm
@@ -23,6 +23,7 @@
 ;;; Copyright © 2023 Bruno Victal <mirai@makinata.eu>
 ;;; Copyright © 2023 muradm <mail@muradm.net>
 ;;; Copyright © 2024 Nigko Yerden <nigko.yerden@gmail.com>
+;;; Copyright © 2024 45mg <45mg.writes@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -1262,18 +1263,23 @@ (define-record-type* <network-manager-configuration>
                (default '()))
   (iwd? network-manager-configuration-iwd?  ; TODO: deprecated field, remove.
         (default #f)
-        (sanitize warn-iwd?-field-deprecation)))
+        (sanitize warn-iwd?-field-deprecation))
+  (configuration-directory network-manager-configuration-configuration-directory ;file-like
+                           (default #f)))
 
 (define (network-manager-activation config)
   ;; Activation gexp for NetworkManager
   (match-record config <network-manager-configuration>
-    (network-manager dns vpn-plugins)
+    (network-manager dns vpn-plugins configuration-directory)
     #~(begin
         (use-modules (guix build utils))
         (mkdir-p "/etc/NetworkManager/system-connections")
         #$@(if (equal? dns "dnsmasq")
                ;; create directory to store dnsmasq lease file
                '((mkdir-p "/var/lib/misc"))
+               '())
+        #$@(if configuration-directory
+               `((symlink ,configuration-directory "/etc/NetworkManager/conf.d"))
                '()))))
 
 (define (vpn-plugin-directory plugins)

base-commit: 831b94a1efcea8f793afc949b5123a6235c9bb1a
-- 
2.47.1

[bug#75145] [PATCH v2 0/1] services: network-manager: Add extra-configuration-files field.

[45mg]

Hi Arnaud!

Arnaud Daby-Seesaram <ds-ac@nanein.fr> writes:

> Adding a default configuration could one day be interesting.  However,
> I would rather (personal opinion) see the default value in an exported
> variable %default-networkmanager-files (non-contractual name) if needed.
> This variable could become the default value of the field.
>
> This way, it would be more transparent to users, and would enable them
> to easily opt-out.  Adding their files could be done with the following
> configuration:
>
> --8<---------------cut here---------------start------------->8---
> (extra-configuration-files
>   (cons* `("file1" ,(plain-file ...))
>          ...
>          %default-networkmanager-files))
> --8<---------------cut here---------------end--------------->8---

Yes, I think I agree. Of course, it's a moot point for now since we
don't have any use for such a variable yet, but it seems like a good
approach.

> As you said, you are doing something similar to `etc-service-type'.  Is
> there a reason not to extend [2] it directly (e.g. like
> `greetd-service-type' does in `(gnu services base)')?
>
> You could, for example, prepend "NetworkManager/conf.d/" to file names
> and pass the value to `etc-service-type'.  WDYT?

That's a good suggestion! I don't know why it didn't occur to me.

So I tried it; the problem is that if I pass something like this:
--8<---------------cut here---------------start------------->8---
(service-extension etc-service-type
                   (lambda (config)
                     `(("NetworkManager/conf.d"
                        ,(network-manager-configuration-directory config)))))
--8<---------------cut here---------------end--------------->8---

...then I get this error when creating a container:
--8<---------------cut here---------------start------------->8---
ERROR: In procedure primitive-load:
In procedure mkdir: Read-only file system
--8<---------------cut here---------------end--------------->8---

It looks like this is because of this line in `network-manager-activation`:
--8<---------------cut here---------------start------------->8---
        (mkdir-p "/etc/NetworkManager/system-connections")
--8<---------------cut here---------------end--------------->8---

When we use `etc-service-type`, "/etc/NetworkManager" becomes a symlink
to "/etc/static/NetworkManager"; and "/etc/static" is a symlink to the
result of building the derivation returned by `etc-entry` (see (gnu
services)). And derivations are in the store, which is read-only. So we
can't create "/etc/NetworkManager/system-connections/". And this won't
do, since NetworkManager itself needs to be able to write to that
directory to manage saved connections.

So it looks like that won't work.

> Would it make sense to allow the NetworkManager service to be extended?
> (E.g to allow another service to add a configuration file.)
>
> Note: this is not a blocker for your patch and could be done in a later
>       patch.  It is simply a thought that I wanted to share.

Yeah, this was another reason for this revision. Again, it's not
actually something we need right now; it's just worth leaving open as a
possibility.