[PATCH] Wireguard: Add autostart? field.

  • Open
  • quality assurance status badge
Details
3 participants
  • Apoorv Singh
  • Maxim Cournoyer
  • Sergey Trofimov
Owner
unassigned
Submitted by
Apoorv Singh
Severity
normal
A
A
Apoorv Singh wrote on 25 Sep 10:37 +0200
(address . guix-patches@gnu.org)
87cyksjfhw.fsf@gmail.com
The following patch adds a record field autostart? which can be
used by the user to configure weather the wireguard service should
start automatically. This field is helpful for people who might
have limited bandwidth and/or they don't want the wireguard
service to start at boot which in turn starts the VPN without them
knowing as it can result in un-desired usage of their bandwidth
etc.

I personally have limited bandwidth on the VPS I am running the wireguard VPN on and don't want to use it all the time, and this options will fix that, as I sometimes forget that I have it turned on
From 378f72413697e418061fe359acddf24d6afe1add Mon Sep 17 00:00:00 2001
From: apoorv569 <apoorvs569@gmail.com>
Date: Wed, 25 Sep 2024 09:10:36 +0530
Subject: [PATCH 2/2] Wireguard add autostart? field

---
gnu/services/vpn.scm | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)

Toggle diff (43 lines)
diff --git a/gnu/services/vpn.scm b/gnu/services/vpn.scm
index 449909e34d..eee7e78c6d 100644
--- a/gnu/services/vpn.scm
+++ b/gnu/services/vpn.scm
@@ -86,6 +86,7 @@ (define-module (gnu services vpn)
wireguard-configuration-pre-down
wireguard-configuration-post-down
wireguard-configuration-table
+ wireguard-configuration-autostart?
wireguard-service-type))
@@ -760,7 +761,9 @@ (define-record-type* <wireguard-configuration>
(post-down wireguard-configuration-post-down ;list of strings
(default '()))
(table wireguard-configuration-table ;string
- (default "auto")))
+ (default "auto"))
+ (autostart? wireguard-configuration-autostart?
+ (default #f)))
(define (wireguard-configuration-file config)
(define (peer->config peer)
@@ -907,7 +910,8 @@ (define (wireguard-shepherd-service config)
(match-record config <wireguard-configuration>
(wireguard interface)
(let ((wg-quick (file-append wireguard "/bin/wg-quick"))
- (config (wireguard-configuration-file config)))
+ (config (wireguard-configuration-file config))
+ (autostart (wireguard-configuration-autostart? config)))
(list (shepherd-service
(requirement '(networking))
(provision (list (wireguard-service-name interface)))
@@ -916,6 +920,7 @@ (define (wireguard-shepherd-service config)
(stop #~(lambda _
(invoke #$wg-quick "down" #$config)
#f)) ;stopped!
+ (auto-start? autostart)
(actions (list (shepherd-configuration-action config)))
(documentation "Run the Wireguard VPN tunnel"))))))
--
2.46.0
.

--
- Apoorv Singh
- Sent from Emacs.
S
S
Sergey Trofimov wrote on 26 Sep 19:45 +0200
(name . Apoorv Singh)(address . apoorvs569@gmail.com)(address . 73467@debbugs.gnu.org)
87ikui1f7t.fsf@sarg.org.ru
Apoorv Singh <apoorvs569@gmail.com> writes:

Toggle quote (3 lines)
> The following patch adds a record field autostart? which can be used by the user
> to configure weather the wireguard service should start automatically.

I generally agree that there should be a way to disable autostart and
I've solved it in a more generic way:

Toggle snippet (28 lines)
(define (no-autostart input-service)
"Augment shepherd extension of INPUT-SERVICE to disable auto-start."
(define (transform-extension ex)
(match ex
(($ (@@ (gnu services) <service-extension>)
(and ($ (@@ (gnu services) <service-type>) 'shepherd-root _) kind)
compute)

(service-extension
kind
(lambda (config)
(let ((orig (car (compute config))))
(list (shepherd-service (inherit orig) (auto-start? #f)))))))

(_ ex)))

(match input-service
(($ (@@ (gnu services) <service>)
(and ($ (@@ (gnu services) <service-type>) _ extensions _) kind)
value)

(service
(service-type
(inherit kind)
(extensions (map transform-extension extensions)))
value))))

Anyway, you need to document the new configuration parameter in the manual.
A
A
Apoorv Singh wrote on 3 Oct 07:30 +0200
Wireguard: Add auto-start? field.
(address . 73467@debbugs.gnu.org)
877capwy5q.fsf@gmail.com
From e17eb73fb9662e44c6cb03405ea87f7e37dbf1e3 Mon Sep 17 00:00:00 2001
From: apoorv569 <apoorvs569@gmail.com>
Date: Thu, 3 Oct 2024 10:51:36 +0530
Subject: [PATCH 3/4] Rename autostart? to auto-start? as other services use
the same.

---
gnu/services/vpn.scm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

Toggle diff (15 lines)
diff --git a/gnu/services/vpn.scm b/gnu/services/vpn.scm
index eee7e78c6d..7e79de48a8 100644
--- a/gnu/services/vpn.scm
+++ b/gnu/services/vpn.scm
@@ -762,7 +762,7 @@ (define-record-type* <wireguard-configuration>
(default '()))
(table wireguard-configuration-table ;string
(default "auto"))
- (autostart? wireguard-configuration-autostart?
+ (auto-start? wireguard-configuration-autostart? ;boolean
(default #f)))
(define (wireguard-configuration-file config)
--
2.46.0
From 2db03437e74407152d4e3cbd6b234baeda670fcf Mon Sep 17 00:00:00 2001
From: apoorv569 <apoorvs569@gmail.com>
Date: Thu, 3 Oct 2024 10:53:05 +0530
Subject: [PATCH 4/4] Document the new auto-start? field for
wireguard-service-type.

---
doc/guix.texi | 4 ++++
1 file changed, 4 insertions(+)

Toggle diff (17 lines)
diff --git a/doc/guix.texi b/doc/guix.texi
index 52e36e4354..50676997e2 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -34396,6 +34396,10 @@ special values: @code{"off"} that disables the creation of routes
altogether, and @code{"auto"} (the default) that adds routes to the
default table and enables special handling of default routes.
+@item @code{auto-start?} (default: @code{#t}) (type: boolean)
+Whether the service should be started automatically. If it
+is @code{#f} the service has to be started manually with @command{herd start}.
+
@end table
@end deftp
--
2.46.0
The following patches rename the autostart? field to auto-start?
to follow what other services name the field and documents the
newly added field in guix.texi file.

--
- Apoorv Singh
- Sent from Emacs.
M
M
Maxim Cournoyer wrote on 3 Oct 14:37 +0200
Re: [bug#73467] [PATCH] Wireguard: Add autostart? field.
(name . Apoorv Singh)(address . apoorvs569@gmail.com)(address . 73467@debbugs.gnu.org)
87o741fjln.fsf@gmail.com
Hello,

Apoorv Singh <apoorvs569@gmail.com> writes:

Toggle quote (11 lines)
> The following patch adds a record field autostart? which can be used
> by the user to configure weather the wireguard service should start
> automatically. This field is helpful for people who might have limited
> bandwidth and/or they don't want the wireguard service to start at
> boot which in turn starts the VPN without them knowing as it can
> result in un-desired usage of their bandwidth etc.
>
> I personally have limited bandwidth on the VPS I am running the
> wireguard VPN on and don't want to use it all the time, and this
> options will fix that, as I sometimes forget that I have it turned on

I guess you are also re-routing all of your traffic to your wireguard
interface? I have such a setup, and I've configured wireguard via
NetworkManager for this case, as it's more conveniently turned on & off,
even from GNOME's UI [0].


Also, by default, a Wireguard tunnel doesn't consume any data (no pings,
nothing) until traffic is sent to it, so it shouldn't be an issue until
you use it.

Toggle quote (30 lines)
>>From 378f72413697e418061fe359acddf24d6afe1add Mon Sep 17 00:00:00 2001
> From: apoorv569 <apoorvs569@gmail.com>
> Date: Wed, 25 Sep 2024 09:10:36 +0530
> Subject: [PATCH 2/2] Wireguard add autostart? field
>
> ---
> gnu/services/vpn.scm | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/gnu/services/vpn.scm b/gnu/services/vpn.scm
> index 449909e34d..eee7e78c6d 100644
> --- a/gnu/services/vpn.scm
> +++ b/gnu/services/vpn.scm
> @@ -86,6 +86,7 @@ (define-module (gnu services vpn)
> wireguard-configuration-pre-down
> wireguard-configuration-post-down
> wireguard-configuration-table
> + wireguard-configuration-autostart?
>
> wireguard-service-type))
>
> @@ -760,7 +761,9 @@ (define-record-type* <wireguard-configuration>
> (post-down wireguard-configuration-post-down ;list of strings
> (default '()))
> (table wireguard-configuration-table ;string
> - (default "auto")))
> + (default "auto"))
> + (autostart? wireguard-configuration-autostart?
> + (default #f)))

IIUC, this would mean the wireguard service would not longer start *by
default*, breaking users configs and more importantly expectations
(since it'd be different to most other services in this respect).

Toggle quote (18 lines)
> (define (wireguard-configuration-file config)
> (define (peer->config peer)
> @@ -907,7 +910,8 @@ (define (wireguard-shepherd-service config)
> (match-record config <wireguard-configuration>
> (wireguard interface)
> (let ((wg-quick (file-append wireguard "/bin/wg-quick"))
> - (config (wireguard-configuration-file config)))
> + (config (wireguard-configuration-file config))
> + (autostart (wireguard-configuration-autostart? config)))
> (list (shepherd-service
> (requirement '(networking))
> (provision (list (wireguard-service-name interface)))
> @@ -916,6 +920,7 @@ (define (wireguard-shepherd-service config)
> (stop #~(lambda _
> (invoke #$wg-quick "down" #$config)
> #f)) ;stopped!
> + (auto-start? autostart)

Like Sergey, I agree it'd be useful to expose an auto-start? value, and
I'd also like to see some way to make this exposed to all services, as
something inherited (though I'm not sure how that could be achieved with
our current structure).

But as a start, if it's really useful (seem my first comment above
regarding bandwidth usage), we could do it this way, as long as it
doesn't change the default behavior (default #f)

It'd also need to be documented in doc/guix.texi.

--
Thanks,
Maxim
M
M
Maxim Cournoyer wrote on 3 Oct 14:47 +0200
Re: [bug#73467] Wireguard: Add auto-start? field.
(name . Apoorv Singh)(address . apoorvs569@gmail.com)(address . 73467@debbugs.gnu.org)
87jzepfj4b.fsf@gmail.com
Hi,

Apoorv Singh <apoorvs569@gmail.com> writes:

Toggle quote (22 lines)
>>From e17eb73fb9662e44c6cb03405ea87f7e37dbf1e3 Mon Sep 17 00:00:00 2001
> From: apoorv569 <apoorvs569@gmail.com>
> Date: Thu, 3 Oct 2024 10:51:36 +0530
> Subject: [PATCH 3/4] Rename autostart? to auto-start? as other services use
> the same.
>
> ---
> gnu/services/vpn.scm | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gnu/services/vpn.scm b/gnu/services/vpn.scm
> index eee7e78c6d..7e79de48a8 100644
> --- a/gnu/services/vpn.scm
> +++ b/gnu/services/vpn.scm
> @@ -762,7 +762,7 @@ (define-record-type* <wireguard-configuration>
> (default '()))
> (table wireguard-configuration-table ;string
> (default "auto"))
> - (autostart? wireguard-configuration-autostart?
> + (auto-start? wireguard-configuration-autostart? ;boolean
> (default #f)))

Instead of sending a diff on top of your original work, please squash
your commits and send it as marked as 'v2' for version 2. Some guidance
is available here for formatting and sending patches [0]


--
Thanks,
Maxim
M
M
Maxim Cournoyer wrote on 3 Oct 14:53 +0200
Re: [bug#73467] [PATCH] Wireguard: Add autostart? field.
(name . Sergey Trofimov)(address . sarg@sarg.org.ru)
87frpdfiu7.fsf@gmail.com
Hi,

Sergey Trofimov <sarg@sarg.org.ru> writes:

Toggle quote (7 lines)
> Apoorv Singh <apoorvs569@gmail.com> writes:
>
>> The following patch adds a record field autostart? which can be used by the user
>> to configure weather the wireguard service should start automatically.
>
> I generally agree that there should be a way to disable autostart

+1. I'm not sure how this could be implemented... seems to me we'd need
some guide of record inheritance or something, with a base configuration
record containing the auto-start? (which should default to #t) option.

--
Thanks,
Maxim
A
A
Apoorv Singh wrote on 3 Oct 19:43 +0200
PATCH V3 for Wireguard: Add auto-start? field
(address . 73467@debbugs.gnu.org)
8734ldw081.fsf@gmail.com
The following patch adds auto-start? field for wireguard-service-type
From 7f4c9783cad8e3574ab43a6dec9e13713ef3311b Mon Sep 17 00:00:00 2001
From: apoorv569 <apoorvs569@gmail.com>
Date: Wed, 25 Sep 2024 09:10:36 +0530
Subject: [PATCH V3] Wireguard: Add auto-start? field

---
doc/guix.texi | 4 ++++
gnu/services/vpn.scm | 9 +++++++--
2 files changed, 11 insertions(+), 2 deletions(-)

Toggle diff (58 lines)
diff --git a/doc/guix.texi b/doc/guix.texi
index 52e36e4354..50676997e2 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -34396,6 +34396,10 @@ special values: @code{"off"} that disables the creation of routes
altogether, and @code{"auto"} (the default) that adds routes to the
default table and enables special handling of default routes.
+@item @code{auto-start?} (default: @code{#t}) (type: boolean)
+Whether the service should be started automatically. If it
+is @code{#f} the service has to be started manually with @command{herd start}.
+
@end table
@end deftp
diff --git a/gnu/services/vpn.scm b/gnu/services/vpn.scm
index 449909e34d..1b0cc4d337 100644
--- a/gnu/services/vpn.scm
+++ b/gnu/services/vpn.scm
@@ -86,6 +86,7 @@ (define-module (gnu services vpn)
wireguard-configuration-pre-down
wireguard-configuration-post-down
wireguard-configuration-table
+ wireguard-configuration-autostart?
wireguard-service-type))
@@ -760,7 +761,9 @@ (define-record-type* <wireguard-configuration>
(post-down wireguard-configuration-post-down ;list of strings
(default '()))
(table wireguard-configuration-table ;string
- (default "auto")))
+ (default "auto"))
+ (auto-start? wireguard-configuration-autostart? ;boolean
+ (default #t)))
(define (wireguard-configuration-file config)
(define (peer->config peer)
@@ -907,7 +910,8 @@ (define (wireguard-shepherd-service config)
(match-record config <wireguard-configuration>
(wireguard interface)
(let ((wg-quick (file-append wireguard "/bin/wg-quick"))
- (config (wireguard-configuration-file config)))
+ (config (wireguard-configuration-file config))
+ (autostart (wireguard-configuration-autostart? config)))
(list (shepherd-service
(requirement '(networking))
(provision (list (wireguard-service-name interface)))
@@ -916,6 +920,7 @@ (define (wireguard-shepherd-service config)
(stop #~(lambda _
(invoke #$wg-quick "down" #$config)
#f)) ;stopped!
+ (auto-start? autostart)
(actions (list (shepherd-configuration-action config)))
(documentation "Run the Wireguard VPN tunnel"))))))
--
2.46.0
.

--
- Apoorv Singh
- Sent from Emacs.
?
Your comment

Commenting via the web interface is currently disabled.

To comment on this conversation send an email to 73467@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 73467
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch