[bug#73467] [PATCH] Wireguard: Add autostart? field.

[bug#73467] [PATCH] Wireguard: Add autostart? field.

[Apoorv Singh]

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

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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Wireguard: Add autostart? field patch. --]
[-- Type: text/x-patch, Size: 2149 bytes --]

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)))
 
 (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


[-- Attachment #3: Type: text/plain, Size: 41 bytes --]

.

-- 
- Apoorv Singh
- Sent from Emacs.

[bug#73467] [PATCH] Wireguard: Add autostart? field.

[Sergey Trofimov]

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 and
I've solved it in a more generic way:

--8<---------------cut here---------------start------------->8---
(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))))
--8<---------------cut here---------------end--------------->8---

Anyway, you need to document the new configuration parameter in the manual.

[bug#73467] Wireguard: Add auto-start? field.

[Apoorv Singh]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Rename field autostart? to auto-start? --]
[-- Type: text/x-patch, Size: 894 bytes --]

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)))
 
 (define (wireguard-configuration-file config)
-- 
2.46.0


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Document the newly added auto-start? field. --]
[-- Type: text/x-patch, Size: 883 bytes --]

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(+)

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


[-- Attachment #3: Type: text/plain, Size: 206 bytes --]

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.

[bug#73467] [PATCH] Wireguard: Add autostart? field.

[Maxim Cournoyer]

Hello,

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. 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].

[0]  https://lists.gnu.org/archive/html/help-guix/2024-09/msg00032.html

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.

>>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).

>  (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

[bug#73467] Wireguard: Add auto-start? field.

[Maxim Cournoyer]

Hi,

Apoorv Singh <apoorvs569@gmail.com> writes:

>>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]

[0]  https://guix.gnu.org/manual/devel/en/html_node/Sending-a-Patch-Series.html

-- 
Thanks,
Maxim

[bug#73467] [PATCH] Wireguard: Add autostart? field.

[Maxim Cournoyer]

Hi,

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

> 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

[bug#73467] PATCH V3 for Wireguard: Add auto-start? field

[Apoorv Singh]

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

The following patch adds auto-start? field for wireguard-service-type

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Wireguard Add auto-start? field --]
[-- Type: text/x-patch, Size: 2768 bytes --]

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(-)

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


[-- Attachment #3: Type: text/plain, Size: 41 bytes --]

.

-- 
- Apoorv Singh
- Sent from Emacs.