Subject: Re: bug#59797: 30.0.50; [wishlist] Using namespaces in Tramp's kubernetes integration

From: Michael Albinus <[email-masked]>
Date: Wed, 24 Jul 2024 18:48:04 +0200

Michael Albinus <[email-masked]> writes:

Hi Eshel,

>> You're absolutely right.  AFAIU from the Kubernetes documentation[0],
>> non-alphanumeric characters other than "-" and "." should be safe.
>> Maybe "@" would be the most natural choice?  "+" could work too I think.
>
> "@" is already used for the "user@host" notation in Tramp. We shouldn't
> confuse users with another meaning in parallel. "+" might be OK, let's
> start with it. It will be a Lisp constant, we can change it any time
> later. So we have "/kubernetes:[CONTAINER.]POD[+NAMESPACE]:/..." now.

I gave it a first try. It turns out, that "+" is a bad choice, because
it doesn't belong to tramp-host-regexp. I would need to rewrite host
name handling in Tramp just for that, which is not desirable.

tramp-host-regexp is "[%._[:alnum:]-]+". So we could take either "%" or
"_" as namespace delimiter. I've decided for "%"; a complete remote file
name for kubernetes is now "/kubernetes:[CONTAINER.]POD[%NAMESPACE]:/path/to/file".

It works OK acc to my basic tests. I have appended the patch; I would
really appreciate if you could give it more testing in your production
environment. Please sync first with the Emacs or Tramp git repo; this
morning I've pushed other changes there.

What's left is file name completion for namespaces. I'm undecided
whether we need this in practice; let's see how it goes as-it-is.

Best regards, Michael.

diff --git a/lisp/tramp-container.el b/lisp/tramp-container.el
index 02512e64..e211a017 100644
--- a/lisp/tramp-container.el
+++ b/lisp/tramp-container.el
@@ -50,18 +50,14 @@
 ;;
 ;; Open file in a Kubernetes container:
 ;;
-;;     C-x C-f /kubernetes:[CONTAINER.]POD:/path/to/file
+;;     C-x C-f /kubernetes:[CONTAINER.]POD[%NAMESPACE]:/path/to/file
 ;;
 ;; Where:
 ;;     POD           is the pod to connect to.
 ;;     CONTAINER     is the container to connect to (optional).
 ;;		     By default, the first container in that pod will
 ;;		     be used.
-;;
-;; Completion for POD and accessing it operate in the current
-;; namespace, use this command to change it:
-;;
-;; "kubectl config set-context --current --namespace=<name>"
+;;     NAMESPACE     is the namespace to be used (optional).
 ;;
 ;;
 ;;
@@ -152,7 +148,9 @@ If it is nil, the default context will be used."
                  (string)))

 (defcustom tramp-kubernetes-namespace "default"
-  "Namespace of Kubernetes."
+  "Namespace of Kubernetes.
+If it is nil, the current namespace will be used.  An explicit NAMESPACE
+in the remote file name host part will override it."
   :group 'tramp
   :version "30.1"
   :type 'string)
@@ -324,10 +322,18 @@ see its function help for a description of the format."
 	    (push (concat elt "." (car line)) names)))
 	(mapcar (lambda (name) (list nil name)) (delq nil names))))))

+;; <https://kubernetes.io/docs/concepts/overview/working-with-objects/names/>
+;; `lower' could also match non-ascii letters.  But since this regexp
+;; is only used for strings matching `tramp-host-regexp', this doesn't
+;; hurt.
+(defconst tramp-kubernetes--name-regexp (rx (** 1 63 (any lower digit "-")))
+  "Regexp matching kubernetes names.")
+
 (defconst tramp-kubernetes--host-name-regexp
-  (rx (? (group (regexp tramp-host-regexp)) ".")
-      (group (regexp tramp-host-regexp)))
-  "The CONTAINER.POD syntax of kubernetes host names in Tramp.")
+  (rx bos (? (group (regexp tramp-kubernetes--name-regexp)) ".")
+      (group (regexp tramp-kubernetes--name-regexp))
+      (? "%" (group (regexp tramp-kubernetes--name-regexp))) eos)
+  "The CONTAINER.POD%NAMESPACE syntax of kubernetes host names in Tramp.")

 ;;;###tramp-autoload
 (defun tramp-kubernetes--container (vec)
@@ -345,6 +351,15 @@ see its function help for a description of the format."
 	     (match-string 2 host)))
       ""))

+;;;###tramp-autoload
+(defun tramp-kubernetes--namespace (vec)
+  "Extract the namespace from a kubernetes host name in VEC."
+  (or (when-let ((_ vec)
+		 (host (tramp-file-name-host vec)))
+	(and (string-match tramp-kubernetes--host-name-regexp host)
+	     (match-string 3 host)))
+      tramp-kubernetes-namespace ""))
+
 ;; We must change `vec' and `default-directory' to the previous hop,
 ;; in order to run `process-file' in a proper environment.
 (defmacro tramp-skeleton-kubernetes-vector (vec &rest body)
@@ -400,8 +415,8 @@ Obey `tramp-kubernetes-context'"
    #'identity
    `(,(when-let ((context (tramp-kubernetes--current-context vec)))
 	(format "--context=%s" context))
-     ,(when tramp-kubernetes-namespace
-	(format "--namespace=%s" tramp-kubernetes-namespace)))
+     ,(when-let ((namespace (tramp-kubernetes--namespace vec)))
+	(format "--namespace=%s" namespace)))
    " "))

 ;;;###tramp-autoload
@@ -617,9 +632,9 @@ see its function help for a description of the format."
      ;; This variable will be eval'ed in `tramp-expand-args'.
      (tramp-extra-expand-args
       . (?a (tramp-kubernetes--container (car tramp-current-connection))
-	    ?h (tramp-kubernetes--pod (car tramp-current-connection))
-	    ?x (tramp-kubernetes--context-namespace
-		(car tramp-current-connection)))))
+	 ?h (tramp-kubernetes--pod (car tramp-current-connection))
+	 ?x (tramp-kubernetes--context-namespace
+	     (car tramp-current-connection)))))
    "Default connection-local variables for remote kubernetes connections.")

  (connection-local-set-profile-variables