[pdm-devel] [PATCH datacenter-manager 1/3] api: remote shell: do not rely on first node being reachable

[Fabian Grünbichler]

currently, there is no way to get to the raw, underlying HTTP or API client
from a PVEClient, and the PVEClient does not support doing the Websocket
upgrade.

by extending the "raw_client" to allow picking a specific endpoint, the remote
shell no longer relies on the first node of a remote being available, if the
target node is directly reachable from the PDM instance. this also fixes an
issue where the `termproxy` request via a regular PVEClient, and the
`vncwebsocket` request via the raw client ended up talking to two different
nodes, which does not work, since neither of those endpoints are proxied to the
target node on the PVE side.

this is just a stop gap measure, ideally we find a way to re-use the
MultiClient for PVE targets for the remote shell.

Signed-off-by: Fabian Grünbichler <f.gruenbichler at proxmox.com>
---
 server/src/api/remote_shell.rs                | 37 +++++++++++++++----
 server/src/connection.rs                      | 10 ++---
 .../src/metric_collection/collection_task.rs  |  6 ++-
 3 files changed, 40 insertions(+), 13 deletions(-)

diff --git a/server/src/api/remote_shell.rs b/server/src/api/remote_shell.rs
index c617b4d..d5e43e9 100644
--- a/server/src/api/remote_shell.rs
+++ b/server/src/api/remote_shell.rs
@@ -7,6 +7,7 @@ use http::{
 };
 use hyper::upgrade::Upgraded;
 use hyper_util::rt::TokioIo;
+use pve_api_types::client::{PveClient, PveClientImpl};
 use serde_json::{json, Value};
 
 use proxmox_auth_api::{
@@ -169,9 +170,17 @@ fn upgrade_to_websocket(
 
             let (remotes, _digest) = pdm_config::remotes::config()?;
             let remote = get_remote(&remotes, &remote)?;
-            let (ticket, port) = match remote.ty {
+            let (ticket, port, endpoint, first_api_url) = match remote.ty {
                 RemoteType::Pve => {
-                    let pve = crate::connection::make_pve_client(&remote)?;
+                    let cache = crate::remote_cache::RemoteMappingCache::get();
+                    let endpoint = match cache.info_by_node_name(&remote.id, &node) {
+                        Some(info) if info.reachable => Some(info.hostname.clone()),
+                        _ => None,
+                    };
+                    let raw_client =
+                        crate::connection::make_raw_client(remote, endpoint.as_deref())?;
+                    let api_url = raw_client.api_url().clone();
+                    let pve = PveClientImpl(*raw_client);
                     let pve_term_ticket = pve
                         .node_shell_termproxy(
                             &node,
@@ -181,21 +190,35 @@ fn upgrade_to_websocket(
                             },
                         )
                         .await?;
-                    (pve_term_ticket.ticket, pve_term_ticket.port)
+                    (
+                        pve_term_ticket.ticket,
+                        pve_term_ticket.port,
+                        endpoint,
+                        Some(api_url),
+                    )
                 }
                 RemoteType::Pbs => {
                     let pbs = crate::connection::make_pbs_client(&remote)?;
                     let pbs_term_ticket = pbs.node_shell_termproxy().await?;
-                    (pbs_term_ticket.ticket, pbs_term_ticket.port as i64)
+                    (
+                        pbs_term_ticket.ticket,
+                        pbs_term_ticket.port as i64,
+                        None,
+                        None,
+                    )
                 }
             };
 
-            let raw_client = crate::connection::make_raw_client(remote)?;
-
+            let raw_client = crate::connection::make_raw_client(remote, endpoint.as_deref())?;
             let ws_key = proxmox_sys::linux::random_data(16)?;
             let ws_key = proxmox_base64::encode(&ws_key);
 
-            let api_url = raw_client.api_url().clone().into_parts();
+            // ensure request above and below end up at the same node
+            let api_url = raw_client.api_url().clone();
+            if first_api_url.is_some() && first_api_url.as_ref() != Some(&api_url) {
+                bail!("termproxy and vncwebsocket API calls must be made to the same node..");
+            }
+            let api_url = api_url.into_parts();
 
             let mut builder = http::uri::Builder::new();
             if let Some(scheme) = api_url.scheme {
diff --git a/server/src/connection.rs b/server/src/connection.rs
index 7e36671..efb5a1d 100644
--- a/server/src/connection.rs
+++ b/server/src/connection.rs
@@ -258,7 +258,7 @@ pub trait ClientFactory {
     async fn make_pbs_client_and_login(&self, remote: &Remote) -> Result<Box<PbsClient>, Error>;
 
     /// Create a new API client for raw acess to the given remote
-    fn make_raw_client(&self, remote: &Remote) -> Result<Box<Client>, Error>;
+    fn make_raw_client(&self, remote: &Remote, node: Option<&str>) -> Result<Box<Client>, Error>;
 }
 
 /// Default production client factory
@@ -353,8 +353,8 @@ impl ClientFactory for DefaultClientFactory {
         ConnectionCache::get().make_pve_client(remote)
     }
 
-    fn make_raw_client(&self, remote: &Remote) -> Result<Box<Client>, Error> {
-        Ok(Box::new(crate::connection::connect(remote, None)?))
+    fn make_raw_client(&self, remote: &Remote, node: Option<&str>) -> Result<Box<Client>, Error> {
+        Ok(Box::new(crate::connection::connect(remote, node)?))
     }
 
     fn make_pbs_client(&self, remote: &Remote) -> Result<Box<PbsClient>, Error> {
@@ -429,8 +429,8 @@ pub fn make_pbs_client(remote: &Remote) -> Result<Box<PbsClient>, Error> {
     instance().make_pbs_client(remote)
 }
 
-pub fn make_raw_client(remote: &Remote) -> Result<Box<Client>, Error> {
-    instance().make_raw_client(remote)
+pub fn make_raw_client(remote: &Remote, node: Option<&str>) -> Result<Box<Client>, Error> {
+    instance().make_raw_client(remote, node)
 }
 
 /// Create a new API client for PVE remotes.
diff --git a/server/src/metric_collection/collection_task.rs b/server/src/metric_collection/collection_task.rs
index cc1a460..7da85de 100644
--- a/server/src/metric_collection/collection_task.rs
+++ b/server/src/metric_collection/collection_task.rs
@@ -431,7 +431,11 @@ pub(super) mod tests {
             bail!("not implemented")
         }
 
-        fn make_raw_client(&self, _remote: &Remote) -> Result<Box<Client>, Error> {
+        fn make_raw_client(
+            &self,
+            _remote: &Remote,
+            _node: Option<&str>,
+        ) -> Result<Box<Client>, Error> {
             bail!("not implemented")
         }
 
-- 
2.47.3