diff mbox series

[bug#62461,2/3] gnu: openssh-host: Add option match-criteria.

Message ID 20230326140706.32412-2-ngraves@ngraves.fr
State New
Headers show
Series [bug#62461,1/3] gnu: home-openssh-configuration: Add field add-keys-to-agent. | expand

Commit Message

Nicolas Graves March 26, 2023, 2:07 p.m. UTC 
---
 gnu/home/services/ssh.scm | 49 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 43 insertions(+), 6 deletions(-)

Comments

Ludovic Courtès April 1, 2023, 7:59 a.m. UTC | #1 
Nicolas Graves <ngraves@ngraves.fr> skribis:

> ---
>  gnu/home/services/ssh.scm | 49 ++++++++++++++++++++++++++++++++++-----
>  1 file changed, 43 insertions(+), 6 deletions(-)

[...]

> +(define ssh-match-keywords
> +  '("canonical" "final" "exec" "host" "originalhost" "user" "localuser"))

Should it be symbols?

>  (define-configuration openssh-host
>    (name
> -   (string)
> -   "Name of this host declaration.")
> +   maybe-string
> +   "Name of this host declaration.  A @code{openssh-host} must define only
> +@code{name} or @code{match-criteria}.  Use host-name \"*\" for top-level
> +options.")

“Use host name @code{\"*\"} for …”

> +  (match-criteria ;TODO implement stricter match-criteria rules
> +   maybe-match-criteria
> +   "A string where the first element is all or one of
> +@code{ssh-match-keywords}.  The rest of the elements are arguments for the

How about: “When specified, this string denotes the set of hosts to
which the entry applies, superseding the @code{host-name} field.  Its
first element must be all or one of…”

>    (string-append
> -   "Host " (openssh-host-name config) "\n"
> +   (if (maybe-value-set? (openssh-host-name config))
> +       (if (maybe-value-set? (openssh-host-match-criteria config))
> +           (error
> +            "You must either define name or match-criteria, not both.")
> +           (string-append "Host " (openssh-host-name config) "\n"))
> +       (if (maybe-value-set? (openssh-host-match-criteria config))
> +           (string-append
> +            "Match " (string-join (openssh-host-match-criteria config) " ") "\n")
> +           (error
> +            "You must either define name or match-criteria once.")))

Please report errors as lowercase messages with:

  (raise (formatted-message (G_ "…") …))

You can also use ‘&fix-hint’ to provide an additional hint, if needed.

Ludo’.
Nicolas Graves April 17, 2023, 3:08 p.m. UTC | #2 
On 2023-04-01 09:59, Ludovic Courtès wrote:

>> +(define ssh-match-keywords
>> +  '("canonical" "final" "exec" "host" "originalhost" "user" "localuser"))
>
> Should it be symbols?

Could be. But we would then need a conversion from string to symbol
because the original match string which is split can include
spaces. Will switch if it doesn't introduce more complexity. 

Thanks for your other remarks, I'll take them into account and send a
new version.
diff mbox series

Patch

diff --git a/gnu/home/services/ssh.scm b/gnu/home/services/ssh.scm
index 4ab2adb292..0bd79e4322 100644
--- a/gnu/home/services/ssh.scm
+++ b/gnu/home/services/ssh.scm
@@ -45,6 +45,7 @@  (define-module (gnu home services ssh)
 
             openssh-host
             openssh-host-host-name
+            openssh-host-match-criteria
             openssh-host-identity-file
             openssh-host-name
             openssh-host-port
@@ -116,13 +117,39 @@  (define (serialize-string-list field lst)
 
 (define-maybe string-list)
 
+(define ssh-match-keywords
+  '("canonical" "final" "exec" "host" "originalhost" "user" "localuser"))
+
+(define (match-criteria? str)
+  ;; Rule out the case of "all" keyword.
+  (if (member str '("all"
+                    "canonical all"
+                    "final all"))
+      #t
+      (let* ((first (string-take str (string-index str #\ )))
+             (keyword (if (string-prefix? "!" first)
+                          (string-drop first 1)
+                          first)))
+        (member keyword ssh-match-keywords))))
+
+(define-maybe match-criteria)
+
 (define-configuration openssh-host
   (name
-   (string)
-   "Name of this host declaration.")
+   maybe-string
+   "Name of this host declaration.  A @code{openssh-host} must define only
+@code{name} or @code{match-criteria}.  Use host-name \"*\" for top-level
+options.")
   (host-name
    maybe-string
    "Host name---e.g., @code{\"foo.example.org\"} or @code{\"192.168.1.2\"}.")
+  (match-criteria ;TODO implement stricter match-criteria rules
+   maybe-match-criteria
+   "A string where the first element is all or one of
+@code{ssh-match-keywords}.  The rest of the elements are arguments for the
+keyword, or other criteria.  A @code{openssh-host} must define only
+@code{name} or @code{match-criteria}.  Other host configuration options will
+apply to all hosts matching @code{match-criteria}.")
   (address-family
    maybe-address-family
    "Address family to use when connecting to this host: one of
@@ -171,17 +198,27 @@  (define-configuration openssh-host
 @file{~/.ssh/config}."))
 
 (define (serialize-openssh-host config)
-  (define (openssh-host-name-field? field)
-    (eq? (configuration-field-name field) 'name))
+  (define (openssh-host-name-or-match-field? field)
+    (or (eq? (configuration-field-name field) 'name)
+        (eq? (configuration-field-name field) 'match-criteria)))
 
   (string-append
-   "Host " (openssh-host-name config) "\n"
+   (if (maybe-value-set? (openssh-host-name config))
+       (if (maybe-value-set? (openssh-host-match-criteria config))
+           (error
+            "You must either define name or match-criteria, not both.")
+           (string-append "Host " (openssh-host-name config) "\n"))
+       (if (maybe-value-set? (openssh-host-match-criteria config))
+           (string-append
+            "Match " (string-join (openssh-host-match-criteria config) " ") "\n")
+           (error
+            "You must either define name or match-criteria once.")))
    (string-concatenate
     (map (lambda (field)
            ((configuration-field-serializer field)
             (configuration-field-name field)
             ((configuration-field-getter field) config)))
-         (remove openssh-host-name-field?
+         (remove openssh-host-name-or-match-field?
                  openssh-host-fields)))))
 
 (define-record-type* <home-openssh-configuration>