[bug#75145,v3] services: network-manager: Add extra-configuration-files field.
Commit Message
Allow users to specify additional configuration files for
NetworkManager. These files will be added to
`/etc/NetworkManager/conf.d` (NetworkManager's default configuration
directory location).
* gnu/services/networking.scm (<network-manager-configuration>)
[extra-configuration-files]: New field.
(network-manager-activation): Honor the new field.
* doc/guix.texi (Networking Setup): Document the new field.
Change-Id: I07479958e4d0aa318328c666a9630b779230b300
---
doc/guix.texi | 21 +++++++++++++++++++++
gnu/services/networking.scm | 13 +++++++++++--
2 files changed, 32 insertions(+), 2 deletions(-)
base-commit: b85d20e853192a92093cd8d6a5756ec80e94c658
Comments
Hi,
Thank you for the v3. I just have two minor comments.
45mg <45mg.writes@gmail.com> writes:
> @@ -1253,18 +1254,26 @@ (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))
> + (extra-configuration-files network-manager-configuration-extra-configuration-files
> + (default '()))) ;alist of file names to file-like objects
>
> (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 extra-configuration-files)
> #~(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 extra-configuration-files
> + `((symlink
> + ,(file-union "network-manager-configuration-directory"
> + extra-configuration-files)
> + "/etc/NetworkManager/conf.d"))
> '()))))
If I understand, the `if' is here to avoid creating a symbolic link to
an empty directory: if no extra configuration files are provided, do not
symlink. However, in Guile, it seems that (if '() 'yes 'no) evaluates
to 'yes. Therefore, the symlink is always created. A solution is to
use `pair?':
--8<---------------cut here---------------start------------->8---
#$@(if (pair? extra-configuration-files)
`((symlink
,(file-union "network-manager-configuration-directory"
extra-configuration-files)
"/etc/NetworkManager/conf.d"))
'()))))
--8<---------------cut here---------------end--------------->8---
Apologies, I should have realised that before my first review.
> +@item @code{extra-configuration-files} (default: @code{'()})
> +An alist of file names to file-like objects, representing configuration
> +files which will be added to @file{/etc/NetworkManager/conf.d}.
> +NetworkManager will read additional configuration from this directory.
> +For details on configuration file precedence and the configuration file
> +format, see the @command{NetworkManager.conf(5)} man page.
I am not sure that "alist" is the correct term. When I hear alist, I
think of something of the form
((key-1 . val-1)
...
(key-n . val-n))
The documentation of `file-union' uses the term "two-element list" for
its argument. Maybe it would be more precise to do the same (I am not
very opinionated on this).
Best regards,
Hi Arnaud,
Arnaud Daby-Seesaram <ds-ac@nanein.fr> writes:
> Hi,
>
> Thank you for the v3. I just have two minor comments.
>
>
> 45mg <45mg.writes@gmail.com> writes:
>
>> @@ -1253,18 +1254,26 @@ (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))
>> + (extra-configuration-files network-manager-configuration-extra-configuration-files
>> + (default '()))) ;alist of file names to file-like objects
>>
>> (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 extra-configuration-files)
>> #~(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 extra-configuration-files
>> + `((symlink
>> + ,(file-union "network-manager-configuration-directory"
>> + extra-configuration-files)
>> + "/etc/NetworkManager/conf.d"))
>> '()))))
>
> If I understand, the `if' is here to avoid creating a symbolic link to
> an empty directory: if no extra configuration files are provided, do not
> symlink. However, in Guile, it seems that (if '() 'yes 'no) evaluates
> to 'yes. Therefore, the symlink is always created. A solution is to
> use `pair?':
>
> #$@(if (pair? extra-configuration-files)
> `((symlink
> ,(file-union "network-manager-configuration-directory"
> extra-configuration-files)
> "/etc/NetworkManager/conf.d"))
> '()))))
>
> Apologies, I should have realised that before my first review.
That's fine, and good catch!
>> +@item @code{extra-configuration-files} (default: @code{'()})
>> +An alist of file names to file-like objects, representing configuration
>> +files which will be added to @file{/etc/NetworkManager/conf.d}.
>> +NetworkManager will read additional configuration from this directory.
>> +For details on configuration file precedence and the configuration file
>> +format, see the @command{NetworkManager.conf(5)} man page.
>
> I am not sure that "alist" is the correct term. When I hear alist, I
> think of something of the form
> ((key-1 . val-1)
> ...
> (key-n . val-n))
>
> The documentation of `file-union' uses the term "two-element list" for
> its argument. Maybe it would be more precise to do the same (I am not
> very opinionated on this).
Indeed, I think a list of pure lists (rather than pair) cannot be called
an association list (alist), as that shoud be made up of pairs (using
cons or the dot notation).
See info '(guile) Association Lists'.
Thank you for the review!
45mg, would you mind sending a v4?
@@ -21541,6 +21541,27 @@ 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{extra-configuration-files} (default: @code{'()})
+An alist of file names to file-like objects, representing configuration
+files which will be added to @file{/etc/NetworkManager/conf.d}.
+NetworkManager will read additional configuration from this directory.
+For details on configuration file precedence and the configuration file
+format, see the @command{NetworkManager.conf(5)} man page.
+
+For example, to add two files @file{001-basic.conf} and
+@file{002-unmanaged.conf}:
+
+@lisp
+(service network-manager-service-type
+ (network-manager-configuration
+ (extra-configuration-files
+ `(("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
@@ -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 © 2025 45mg <45mg.writes@gmail.com>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -1253,18 +1254,26 @@ (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))
+ (extra-configuration-files network-manager-configuration-extra-configuration-files
+ (default '()))) ;alist of file names to file-like objects
(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 extra-configuration-files)
#~(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 extra-configuration-files
+ `((symlink
+ ,(file-union "network-manager-configuration-directory"
+ extra-configuration-files)
+ "/etc/NetworkManager/conf.d"))
'()))))
(define (vpn-plugin-directory plugins)