* [pdm-devel] [PATCH datacenter-manager 00/21] improve remote wizard
@ 2025-05-16 13:35 Dominik Csapak
2025-05-16 13:35 ` [pdm-devel] [PATCH datacenter-manager 01/21] server/ui: pve: change 'realm list' api call to GET Dominik Csapak
` (23 more replies)
0 siblings, 24 replies; 33+ messages in thread
From: Dominik Csapak @ 2025-05-16 13:35 UTC (permalink / raw)
To: pdm-devel
# Summary
This series improves the remote wizard in various points:
* improved wording and texts
* moved seperate buttons on pages into the next button
* probing of entered nodes
* confirmation dialog for missing fingerprints
* better realm selector
# Notes
I'm not super sure about the API call structure. I think two seperate
API calls (one for tls probing, one for scanning) might make more sense.
If that's wanted, I'll do so in a v2 of the series.
## Probing on the server side
I tried to determine if we need a fingerprint for nodes inside the
API call, by probing each node. Since we currently only get the
nodenames and not FQDNs in the nodelist, this will currently not
result in a valid connection in most cases and return the fingerprint.
My plan would be to include FQDNs of the nodes on the PVE side API call,
so we can return here a list of nodename, ip and FQDNs, which the user
then can select from. (which of those I'd probe on first check is yet
to be determined)
## Fingerprint confirmation dialogs
Not sure if we want to be able to let the user confirm the fingerprint
so easily. On one hand it's very convenient, but maybe leads to users
simply clicking yes without understanding what's happening.
If it's deemed too dangerous, I'd rework the series without this.
# Future work
The next step for the wizard is to have some kind of quick copy&paste
info. After discussing off-list with Fabian a bit, I think it would be
best for this to contain the hostname (FQDN?) + fingerprint (if just a
self-signed certificate) + a list of nodes with their respective
nodename + FQDNs (maybe requires api change on PVE side to generate
this). The user would then still have to do most of the steps currently
necessary in the wizard, except the manual copy & pasting of
fingerprints and maybe entering of FQDNs.
Dominik Csapak (21):
server/ui: pve: change 'realm list' api call to GET
api types: RemoteType: put default port info to the type
server: connection: add probe_tls_connection helper
server/ui: pve api: extend 'scan' so it can probe the tls connection
pdm-client: add scan_remote and probe_tls methods
ui: remotes: node url list: add placeholder and clear trigger
ui: rmeotes: node url list: make column header clearer
ui: remotes: node url list: handle changing default
ui: pve wizard: rename 'realm' variable to 'info'
ui: pve wizard: summary: add default text for fingerprint
ui: pve wizard: nodes: improve info text
ui: pve wizard: nodes: probe hosts to verify fingerprint settings
ui: pve wizard: info: use pdm_client for scanning
ui: pve wizard: info: detect hostname and fingerprint
ui: pve wizard: info: remove manual scan button
ui: widget: add pve realm selector
ui: pve wizard: info: use pve realm selector
ui: pve wizard: connect: factor out normalize_hostname
ui: pve wizard: connect: move connection logic to next button
ui: pve wizard: connect: use scan api endpoint instead of realms
ui: pve wizard: connect: add certificate confirmation dialog
lib/pdm-api-types/Cargo.toml | 1 +
lib/pdm-api-types/src/lib.rs | 2 +
lib/pdm-api-types/src/remotes.rs | 16 ++
lib/pdm-client/src/lib.rs | 45 ++++
server/src/api/pve/mod.rs | 62 ++++--
server/src/connection.rs | 87 +++++++-
ui/Cargo.toml | 1 +
ui/src/remotes/add_wizard.rs | 8 +-
ui/src/remotes/node_url_list.rs | 33 ++-
ui/src/remotes/wizard_page_connect.rs | 308 ++++++++++++++++----------
ui/src/remotes/wizard_page_info.rs | 127 ++++++-----
ui/src/remotes/wizard_page_nodes.rs | 241 +++++++++++++++++++-
ui/src/remotes/wizard_page_summary.rs | 5 +-
ui/src/widget/mod.rs | 3 +
ui/src/widget/pve_realm_selector.rs | 125 +++++++++++
15 files changed, 853 insertions(+), 211 deletions(-)
create mode 100644 ui/src/widget/pve_realm_selector.rs
--
2.39.5
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 33+ messages in thread
* [pdm-devel] [PATCH datacenter-manager 01/21] server/ui: pve: change 'realm list' api call to GET
2025-05-16 13:35 [pdm-devel] [PATCH datacenter-manager 00/21] improve remote wizard Dominik Csapak
@ 2025-05-16 13:35 ` Dominik Csapak
2025-05-16 13:35 ` [pdm-devel] [PATCH datacenter-manager 02/21] api types: RemoteType: put default port info to the type Dominik Csapak
` (22 subsequent siblings)
23 siblings, 0 replies; 33+ messages in thread
From: Dominik Csapak @ 2025-05-16 13:35 UTC (permalink / raw)
To: pdm-devel
It makes more sense to have this API as a GET method, since we don't
modify any resource, we're simply getting them.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
server/src/api/pve/mod.rs | 2 +-
ui/src/remotes/wizard_page_connect.rs | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/server/src/api/pve/mod.rs b/server/src/api/pve/mod.rs
index dd7cf38..1a3a725 100644
--- a/server/src/api/pve/mod.rs
+++ b/server/src/api/pve/mod.rs
@@ -47,7 +47,7 @@ const SUBDIRS: SubdirMap = &sorted!([
("scan", &Router::new().post(&API_METHOD_SCAN_REMOTE_PVE)),
(
"realms",
- &Router::new().post(&API_METHOD_LIST_REALM_REMOTE_PVE)
+ &Router::new().get(&API_METHOD_LIST_REALM_REMOTE_PVE)
)
]);
diff --git a/ui/src/remotes/wizard_page_connect.rs b/ui/src/remotes/wizard_page_connect.rs
index 28f14bf..f6013f2 100644
--- a/ui/src/remotes/wizard_page_connect.rs
+++ b/ui/src/remotes/wizard_page_connect.rs
@@ -47,7 +47,7 @@ async fn list_realms(
if let Some(fp) = fingerprint {
params["fingerprint"] = fp.into();
}
- let result: Vec<ListRealm> = proxmox_yew_comp::http_post("/pve/realms", Some(params)).await?;
+ let result: Vec<ListRealm> = proxmox_yew_comp::http_get("/pve/realms", Some(params)).await?;
Ok(result)
}
--
2.39.5
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 33+ messages in thread
* [pdm-devel] [PATCH datacenter-manager 02/21] api types: RemoteType: put default port info to the type
2025-05-16 13:35 [pdm-devel] [PATCH datacenter-manager 00/21] improve remote wizard Dominik Csapak
2025-05-16 13:35 ` [pdm-devel] [PATCH datacenter-manager 01/21] server/ui: pve: change 'realm list' api call to GET Dominik Csapak
@ 2025-05-16 13:35 ` Dominik Csapak
2025-05-16 13:35 ` [pdm-devel] [PATCH datacenter-manager 03/21] server: connection: add probe_tls_connection helper Dominik Csapak
` (21 subsequent siblings)
23 siblings, 0 replies; 33+ messages in thread
From: Dominik Csapak @ 2025-05-16 13:35 UTC (permalink / raw)
To: pdm-devel
so we can access that info from a central point instead of having to
duplicate it
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
lib/pdm-api-types/src/remotes.rs | 9 +++++++++
server/src/connection.rs | 8 ++++----
2 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/lib/pdm-api-types/src/remotes.rs b/lib/pdm-api-types/src/remotes.rs
index 65d1081..dca2fa0 100644
--- a/lib/pdm-api-types/src/remotes.rs
+++ b/lib/pdm-api-types/src/remotes.rs
@@ -48,6 +48,15 @@ pub enum RemoteType {
Pbs,
}
+impl RemoteType {
+ pub fn default_port(&self) -> u16 {
+ match self {
+ RemoteType::Pve => 8006,
+ RemoteType::Pbs => 8007,
+ }
+ }
+}
+
serde_plain::derive_display_from_serialize!(RemoteType);
serde_plain::derive_fromstr_from_deserialize!(RemoteType);
diff --git a/server/src/connection.rs b/server/src/connection.rs
index 201d65b..0be9033 100644
--- a/server/src/connection.rs
+++ b/server/src/connection.rs
@@ -36,16 +36,16 @@ struct ConnectInfo {
impl ConnectInfo {
fn for_remote(remote: &Remote) -> Self {
- let (default_port, prefix, perl_compat, pve_compat) = match remote.ty {
- RemoteType::Pve => (8006, "PVEAPIToken".to_string(), true, true),
- RemoteType::Pbs => (8007, "PBSAPIToken".to_string(), false, false),
+ let (prefix, perl_compat, pve_compat) = match remote.ty {
+ RemoteType::Pve => ("PVEAPIToken".to_string(), true, true),
+ RemoteType::Pbs => ("PBSAPIToken".to_string(), false, false),
};
ConnectInfo {
prefix,
perl_compat,
pve_compat,
- default_port,
+ default_port: remote.ty.default_port(),
}
}
}
--
2.39.5
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 33+ messages in thread
* [pdm-devel] [PATCH datacenter-manager 03/21] server: connection: add probe_tls_connection helper
2025-05-16 13:35 [pdm-devel] [PATCH datacenter-manager 00/21] improve remote wizard Dominik Csapak
2025-05-16 13:35 ` [pdm-devel] [PATCH datacenter-manager 01/21] server/ui: pve: change 'realm list' api call to GET Dominik Csapak
2025-05-16 13:35 ` [pdm-devel] [PATCH datacenter-manager 02/21] api types: RemoteType: put default port info to the type Dominik Csapak
@ 2025-05-16 13:35 ` Dominik Csapak
2025-08-19 11:54 ` Lukas Wagner
2025-05-16 13:35 ` [pdm-devel] [PATCH datacenter-manager 04/21] server/ui: pve api: extend 'scan' so it can probe the tls connection Dominik Csapak
` (20 subsequent siblings)
23 siblings, 1 reply; 33+ messages in thread
From: Dominik Csapak @ 2025-05-16 13:35 UTC (permalink / raw)
To: pdm-devel
this is intended to help us probe a remote/host before using it to check
whether the tls connection is working fine, or it returns the
certificate information so we can show it to the user.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
server/src/connection.rs | 79 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 79 insertions(+)
diff --git a/server/src/connection.rs b/server/src/connection.rs
index 0be9033..c7b2558 100644
--- a/server/src/connection.rs
+++ b/server/src/connection.rs
@@ -15,8 +15,10 @@ use std::time::{Duration, SystemTime};
use anyhow::{bail, format_err, Error};
use http::uri::Authority;
use http::Method;
+use openssl::x509::X509StoreContextRef;
use serde::Serialize;
+use proxmox_acme_api::CertificateInfo;
use proxmox_client::{Client, HttpApiClient, HttpApiResponse, HttpApiResponseStream, TlsOptions};
use pdm_api_types::remotes::{NodeUrl, Remote, RemoteType};
@@ -799,3 +801,80 @@ impl HttpApiClient for MultiClient {
try_request! { self, method, path_and_query, params, streaming_request }
}
}
+
+/// Checks TLS connection to the given remote
+///
+/// Returns `Ok(None)` if connecting with the given parameters works
+/// Returns `Ok(Some(cert))` if no fingerprint was given and some certificate could not be validated
+/// Returns `Err(err)` if some other error occurred
+///
+/// # Example
+///
+/// ```
+/// use server::connection::probe_tls_connection;
+/// use pdm_api_types::remotes::RemoteType;
+///
+/// # async fn function() {
+/// let result = probe_tls_connection(RemoteType::Pve, "192.168.2.100".to_string(), None).await;
+/// match result {
+/// Ok(None) => { /* everything ok */ },
+/// Ok(Some(cert)) => { /* do something with cert */ },
+/// Err(err) => { /* do something with error */ },
+/// }
+/// # }
+/// ```
+pub async fn probe_tls_connection(
+ remote_type: RemoteType,
+ hostname: String,
+ fingerprint: Option<String>,
+) -> Result<Option<CertificateInfo>, Error> {
+ let host_port: Authority = hostname.parse()?;
+
+ let uri: http::uri::Uri = format!(
+ "https://{}:{}",
+ host_port.host(),
+ host_port.port_u16().unwrap_or(remote_type.default_port())
+ )
+ .parse()?;
+
+ // to save the invalid cert we find
+ let invalid_cert = Arc::new(StdMutex::new(None));
+
+ let options = if let Some(fp) = &fingerprint {
+ TlsOptions::parse_fingerprint(fp)?
+ } else {
+ TlsOptions::Callback(Box::new({
+ let invalid_cert = invalid_cert.clone();
+ move |valid: bool, chain: &mut X509StoreContextRef| {
+ if let Some(cert) = chain.current_cert() {
+ if !valid {
+ let cert = cert.to_pem().map(|pem| CertificateInfo::from_pem("", &pem));
+ *invalid_cert.lock().unwrap() = Some(cert);
+ }
+ }
+ true
+ }
+ }))
+ };
+ let client = proxmox_client::Client::with_options(uri, options, Default::default())?;
+
+ // set fake auth info. we don't need any, but the proxmox client will return unauthenticated if
+ // none is set.
+ client.set_authentication(proxmox_client::Token {
+ userid: "".to_string(),
+ value: "".to_string(),
+ prefix: "".to_string(),
+ perl_compat: false,
+ });
+
+ client.request(Method::GET, "/", None::<()>).await?;
+
+ let cert = invalid_cert.lock().unwrap().take();
+ if let Some(cert) = cert {
+ let cert = cert?;
+ let cert = cert?;
+ Ok(Some(cert))
+ } else {
+ Ok(None)
+ }
+}
--
2.39.5
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 33+ messages in thread
* [pdm-devel] [PATCH datacenter-manager 04/21] server/ui: pve api: extend 'scan' so it can probe the tls connection
2025-05-16 13:35 [pdm-devel] [PATCH datacenter-manager 00/21] improve remote wizard Dominik Csapak
` (2 preceding siblings ...)
2025-05-16 13:35 ` [pdm-devel] [PATCH datacenter-manager 03/21] server: connection: add probe_tls_connection helper Dominik Csapak
@ 2025-05-16 13:35 ` Dominik Csapak
2025-08-19 11:54 ` Lukas Wagner
2025-05-16 13:35 ` [pdm-devel] [PATCH datacenter-manager 05/21] pdm-client: add scan_remote and probe_tls methods Dominik Csapak
` (19 subsequent siblings)
23 siblings, 1 reply; 33+ messages in thread
From: Dominik Csapak @ 2025-05-16 13:35 UTC (permalink / raw)
To: pdm-devel
Makes the `authid` and `token` parameters optional. If they're omitted,
opens a basic connection to probe the TLS state. If no fingerprint was
given and the certificate is not trusted, the certificate information is
returned (so it can be shown to the user)
Adapt the UI, so it can cope with that change.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
lib/pdm-api-types/Cargo.toml | 1 +
lib/pdm-api-types/src/lib.rs | 2 +
lib/pdm-api-types/src/remotes.rs | 7 ++++
server/src/api/pve/mod.rs | 60 ++++++++++++++++++++----------
ui/src/remotes/wizard_page_info.rs | 9 +++--
5 files changed, 57 insertions(+), 22 deletions(-)
diff --git a/lib/pdm-api-types/Cargo.toml b/lib/pdm-api-types/Cargo.toml
index 6575c03..fbaf5b3 100644
--- a/lib/pdm-api-types/Cargo.toml
+++ b/lib/pdm-api-types/Cargo.toml
@@ -13,6 +13,7 @@ regex.workspace = true
serde.workspace = true
serde_plain.workspace = true
+proxmox-acme-api = { workspace = true, features = [] }
proxmox-auth-api = { workspace = true, features = ["api-types"] }
proxmox-lang.workspace = true
proxmox-config-digest.workspace = true
diff --git a/lib/pdm-api-types/src/lib.rs b/lib/pdm-api-types/src/lib.rs
index 3844907..31420a9 100644
--- a/lib/pdm-api-types/src/lib.rs
+++ b/lib/pdm-api-types/src/lib.rs
@@ -79,6 +79,8 @@ pub use proxmox_dns_api::THIRD_DNS_SERVER_SCHEMA;
pub use proxmox_config_digest::ConfigDigest;
pub use proxmox_config_digest::PROXMOX_CONFIG_DIGEST_SCHEMA;
+pub use proxmox_acme_api::CertificateInfo;
+
#[macro_use]
mod user;
pub use user::*;
diff --git a/lib/pdm-api-types/src/remotes.rs b/lib/pdm-api-types/src/remotes.rs
index dca2fa0..fd20327 100644
--- a/lib/pdm-api-types/src/remotes.rs
+++ b/lib/pdm-api-types/src/remotes.rs
@@ -179,3 +179,10 @@ mod serde_option_uri {
}
}
}
+
+#[allow(clippy::large_enum_variant)]
+#[derive(Clone, PartialEq, Deserialize, Serialize)]
+pub enum ScanResult {
+ TlsResult(Option<proxmox_acme_api::CertificateInfo>),
+ Remote(Remote),
+}
diff --git a/server/src/api/pve/mod.rs b/server/src/api/pve/mod.rs
index 1a3a725..810a38e 100644
--- a/server/src/api/pve/mod.rs
+++ b/server/src/api/pve/mod.rs
@@ -13,7 +13,7 @@ use proxmox_schema::property_string::PropertyString;
use proxmox_section_config::typed::SectionConfigData;
use proxmox_sortable_macro::sortable;
-use pdm_api_types::remotes::{NodeUrl, Remote, RemoteType, REMOTE_ID_SCHEMA};
+use pdm_api_types::remotes::{NodeUrl, Remote, RemoteType, ScanResult, REMOTE_ID_SCHEMA};
use pdm_api_types::resource::PveResource;
use pdm_api_types::{
Authid, RemoteUpid, HOST_OPTIONAL_PORT_FORMAT, PRIV_RESOURCE_AUDIT, PRIV_RESOURCE_DELETE,
@@ -27,8 +27,8 @@ use pve_api_types::{ClusterResourceKind, ClusterResourceType};
use super::resources::{map_pve_lxc, map_pve_node, map_pve_qemu, map_pve_storage};
-use crate::connection;
use crate::connection::PveClient;
+use crate::connection::{self, probe_tls_connection};
use crate::remote_tasks;
mod lxc;
@@ -314,9 +314,11 @@ fn check_guest_delete_perms(
},
"authid": {
type: Authid,
+ optional: true,
},
"token": {
type: String,
+ optional: true,
description: "The token secret or the user password.",
},
},
@@ -326,13 +328,28 @@ fn check_guest_delete_perms(
&Permission::Privilege(&["/"], PRIV_SYS_MODIFY, false),
},
)]
-/// Scans the given connection info for pve cluster information
+/// Scans the given connection info for tls or pve cluster information
+///
+/// Returns the result for tls connection if only hostname (and optionally fingerprint) is given.
+/// If authid and token are also provided, returns pve cluster information.
+///
+/// For each node that is returned, the TLS connection is probed, to check if using
+/// a fingerprint is necessary.
pub async fn scan_remote_pve(
hostname: String,
fingerprint: Option<String>,
- authid: Authid,
- token: String,
-) -> Result<Remote, Error> {
+ authid: Option<Authid>,
+ token: Option<String>,
+) -> Result<ScanResult, Error> {
+ let (authid, token) = match (authid, token) {
+ (Some(authid), Some(token)) => (authid, token),
+ _ => {
+ let res = probe_tls_connection(RemoteType::Pve, hostname.clone(), fingerprint.clone())
+ .await?;
+ return Ok(ScanResult::TlsResult(res));
+ }
+ };
+
let mut remote = Remote {
ty: RemoteType::Pve,
id: String::new(),
@@ -349,18 +366,23 @@ pub async fn scan_remote_pve(
.await
.map_err(|err| format_err!("could not login: {err}"))?;
- let nodes: Vec<_> = client
- .list_nodes()
- .await?
- .into_iter()
- .map(|node| {
- let url = NodeUrl {
- hostname: node.node,
- fingerprint: node.ssl_fingerprint,
- };
- PropertyString::new(url)
- })
- .collect();
+ let mut nodes = Vec::new();
+
+ for node in client.list_nodes().await? {
+ // probe without fingerprint to see if the certificate is trusted
+ // TODO: how can we get the fqdn here?, otherwise it'll fail in most scenarios...
+ let fingerprint = match probe_tls_connection(RemoteType::Pve, node.node.clone(), None).await
+ {
+ Ok(Some(cert)) => cert.fingerprint,
+ Ok(None) => None,
+ Err(_) => node.ssl_fingerprint,
+ };
+
+ nodes.push(PropertyString::new(NodeUrl {
+ hostname: node.node,
+ fingerprint,
+ }));
+ }
if nodes.is_empty() {
bail!("no node list returned");
@@ -383,7 +405,7 @@ pub async fn scan_remote_pve(
.unwrap_or_default();
}
- Ok(remote)
+ Ok(ScanResult::Remote(remote))
}
#[api(
diff --git a/ui/src/remotes/wizard_page_info.rs b/ui/src/remotes/wizard_page_info.rs
index b0a5ba2..9953f42 100644
--- a/ui/src/remotes/wizard_page_info.rs
+++ b/ui/src/remotes/wizard_page_info.rs
@@ -1,6 +1,6 @@
use std::rc::Rc;
-use anyhow::Error;
+use anyhow::{bail, Error};
use html::IntoEventCallback;
use proxmox_schema::property_string::PropertyString;
use serde::{Deserialize, Serialize};
@@ -18,7 +18,7 @@ use pwt::{
AsyncPool,
};
-use pdm_api_types::remotes::{NodeUrl, Remote};
+use pdm_api_types::remotes::{NodeUrl, Remote, ScanResult};
use pwt_macros::builder;
@@ -97,7 +97,10 @@ async fn scan(connection_params: ConnectParams, form_ctx: FormContext) -> Result
let data: ScanParams = serde_json::from_value(data.clone())?;
let params = serde_json::to_value(&data)?;
- let mut result: Remote = proxmox_yew_comp::http_post("/pve/scan", Some(params)).await?;
+ let mut result = match proxmox_yew_comp::http_post("/pve/scan", Some(params)).await? {
+ ScanResult::TlsResult(_) => bail!("Untrusted certificate or invalid fingerprint"),
+ ScanResult::Remote(remote) => remote,
+ };
result.nodes.insert(
0,
PropertyString::new(NodeUrl {
--
2.39.5
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 33+ messages in thread
* [pdm-devel] [PATCH datacenter-manager 05/21] pdm-client: add scan_remote and probe_tls methods
2025-05-16 13:35 [pdm-devel] [PATCH datacenter-manager 00/21] improve remote wizard Dominik Csapak
` (3 preceding siblings ...)
2025-05-16 13:35 ` [pdm-devel] [PATCH datacenter-manager 04/21] server/ui: pve api: extend 'scan' so it can probe the tls connection Dominik Csapak
@ 2025-05-16 13:35 ` Dominik Csapak
2025-08-19 11:55 ` Lukas Wagner
2025-05-16 13:35 ` [pdm-devel] [PATCH datacenter-manager 06/21] ui: remotes: node url list: add placeholder and clear trigger Dominik Csapak
` (18 subsequent siblings)
23 siblings, 1 reply; 33+ messages in thread
From: Dominik Csapak @ 2025-05-16 13:35 UTC (permalink / raw)
To: pdm-devel
so we can use that in the ui instead of making the api calls manually
via the path.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
lib/pdm-client/src/lib.rs | 45 +++++++++++++++++++++++++++++++++++++++
1 file changed, 45 insertions(+)
diff --git a/lib/pdm-client/src/lib.rs b/lib/pdm-client/src/lib.rs
index 61a8ebd..d656ab1 100644
--- a/lib/pdm-client/src/lib.rs
+++ b/lib/pdm-client/src/lib.rs
@@ -3,6 +3,7 @@
use std::collections::HashMap;
use std::time::Duration;
+use pdm_api_types::remotes::ScanResult;
use pdm_api_types::resource::{PveResource, RemoteResources, TopEntities};
use pdm_api_types::rrddata::{
LxcDataPoint, NodeDataPoint, PbsDatastoreDataPoint, PbsNodeDataPoint, QemuDataPoint,
@@ -865,6 +866,50 @@ impl<T: HttpApiClient> PdmClient<T> {
.build();
Ok(self.0.get(&path).await?.expect_json()?.data)
}
+
+ /// uses /pve/scan to probe the tls connection to the given host
+ pub async fn pve_probe_tls(
+ &self,
+ hostname: &str,
+ fingerprint: Option<&str>,
+ ) -> Result<ScanResult, Error> {
+ let mut params = json!({
+ "hostname": hostname,
+ });
+ if let Some(fp) = fingerprint {
+ params["fingerprint"] = fp.into();
+ }
+ Ok(self
+ .0
+ .post("/api2/extjs/pve/scan", ¶ms)
+ .await?
+ .expect_json()?
+ .data)
+ }
+
+ /// Uses /pve/scan to scan the remote cluster for node/fingerprint information
+ pub async fn pve_scan_remote(
+ &self,
+ hostname: &str,
+ fingerprint: Option<&str>,
+ authid: &str,
+ token: &str,
+ ) -> Result<ScanResult, Error> {
+ let mut params = json!({
+ "hostname": hostname,
+ "authid": authid,
+ "token": token,
+ });
+ if let Some(fp) = fingerprint {
+ params["fingerprint"] = fp.into();
+ }
+ Ok(self
+ .0
+ .post("/api2/extjs/pve/scan", ¶ms)
+ .await?
+ .expect_json()?
+ .data)
+ }
}
/// Builder for migration parameters.
--
2.39.5
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 33+ messages in thread
* [pdm-devel] [PATCH datacenter-manager 06/21] ui: remotes: node url list: add placeholder and clear trigger
2025-05-16 13:35 [pdm-devel] [PATCH datacenter-manager 00/21] improve remote wizard Dominik Csapak
` (4 preceding siblings ...)
2025-05-16 13:35 ` [pdm-devel] [PATCH datacenter-manager 05/21] pdm-client: add scan_remote and probe_tls methods Dominik Csapak
@ 2025-05-16 13:35 ` Dominik Csapak
2025-05-16 13:35 ` [pdm-devel] [PATCH datacenter-manager 07/21] ui: rmeotes: node url list: make column header clearer Dominik Csapak
` (17 subsequent siblings)
23 siblings, 0 replies; 33+ messages in thread
From: Dominik Csapak @ 2025-05-16 13:35 UTC (permalink / raw)
To: pdm-devel
so it's easier to
* see what it does mean when no fingerprint is entered
* clear the field of the fingerprint
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
ui/src/remotes/node_url_list.rs | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/ui/src/remotes/node_url_list.rs b/ui/src/remotes/node_url_list.rs
index 54dbfb9..b6dacc7 100644
--- a/ui/src/remotes/node_url_list.rs
+++ b/ui/src/remotes/node_url_list.rs
@@ -10,7 +10,7 @@ use pwt::state::Store;
use pwt::widget::data_table::{DataTable, DataTableColumn, DataTableHeader};
use pwt::widget::form::ManagedField;
use pwt::widget::form::{Field, ManagedFieldContext, ManagedFieldMaster, ManagedFieldState};
-use pwt::widget::{ActionIcon, Button, Column, Container, Fa, Row};
+use pwt::widget::{ActionIcon, Button, Column, Container, Fa, Row, Trigger};
use pwt::{css, prelude::*};
use proxmox_yew_comp::{SchemaValidation, Status};
@@ -267,7 +267,22 @@ fn columns(ctx: &ManagedFieldContext<PdmNodeUrlField>) -> Rc<Vec<DataTableHeader
};
Field::new()
.schema(&CERT_FINGERPRINT_SHA256_SCHEMA)
+ .placeholder(tr!("Use trusted certificate"))
.on_change(link.callback(move |value| Msg::UpdateFingerprint(index, value)))
+ .with_trigger(
+ Trigger::new(
+ (!fingerprint.is_empty())
+ .then_some("fa fa-times")
+ .unwrap_or_default(),
+ )
+ .tip(tr!("Clear"))
+ .on_activate(
+ link.callback(move |_| {
+ Msg::UpdateFingerprint(index, "".to_string())
+ }),
+ ),
+ true,
+ )
.value(fingerprint.to_string())
.into()
}
--
2.39.5
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 33+ messages in thread
* [pdm-devel] [PATCH datacenter-manager 07/21] ui: rmeotes: node url list: make column header clearer
2025-05-16 13:35 [pdm-devel] [PATCH datacenter-manager 00/21] improve remote wizard Dominik Csapak
` (5 preceding siblings ...)
2025-05-16 13:35 ` [pdm-devel] [PATCH datacenter-manager 06/21] ui: remotes: node url list: add placeholder and clear trigger Dominik Csapak
@ 2025-05-16 13:35 ` Dominik Csapak
2025-05-16 13:35 ` [pdm-devel] [PATCH datacenter-manager 08/21] ui: remotes: node url list: handle changing default Dominik Csapak
` (16 subsequent siblings)
23 siblings, 0 replies; 33+ messages in thread
From: Dominik Csapak @ 2025-05-16 13:35 UTC (permalink / raw)
To: pdm-devel
It's either a hostname or IP, so make that clear in the header.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
ui/src/remotes/node_url_list.rs | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ui/src/remotes/node_url_list.rs b/ui/src/remotes/node_url_list.rs
index b6dacc7..d546a94 100644
--- a/ui/src/remotes/node_url_list.rs
+++ b/ui/src/remotes/node_url_list.rs
@@ -239,7 +239,7 @@ fn columns(ctx: &ManagedFieldContext<PdmNodeUrlField>) -> Rc<Vec<DataTableHeader
let link = ctx.link();
Rc::new(vec![
- DataTableColumn::new(tr!("Hostname"))
+ DataTableColumn::new(tr!("Hostname/IP"))
.flex(1)
.render({
let link = link.clone();
--
2.39.5
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 33+ messages in thread
* [pdm-devel] [PATCH datacenter-manager 08/21] ui: remotes: node url list: handle changing default
2025-05-16 13:35 [pdm-devel] [PATCH datacenter-manager 00/21] improve remote wizard Dominik Csapak
` (6 preceding siblings ...)
2025-05-16 13:35 ` [pdm-devel] [PATCH datacenter-manager 07/21] ui: rmeotes: node url list: make column header clearer Dominik Csapak
@ 2025-05-16 13:35 ` Dominik Csapak
2025-05-16 13:35 ` [pdm-devel] [PATCH datacenter-manager 09/21] ui: pve wizard: rename 'realm' variable to 'info' Dominik Csapak
` (15 subsequent siblings)
23 siblings, 0 replies; 33+ messages in thread
From: Dominik Csapak @ 2025-05-16 13:35 UTC (permalink / raw)
To: pdm-devel
in the wizard, the default can change if a user goes back and
starts querying a different host. To show that, we have to manually
update the internal state when the default changes.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
ui/src/remotes/node_url_list.rs | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/ui/src/remotes/node_url_list.rs b/ui/src/remotes/node_url_list.rs
index d546a94..af77f1b 100644
--- a/ui/src/remotes/node_url_list.rs
+++ b/ui/src/remotes/node_url_list.rs
@@ -136,6 +136,20 @@ impl ManagedField for PdmNodeUrlField {
this
}
+ fn changed(&mut self, ctx: &ManagedFieldContext<Self>, old_props: &Self::Properties) -> bool {
+ let props = ctx.props();
+ if old_props.default != props.default {
+ let default: Value = props
+ .default
+ .iter()
+ .filter_map(|n| serde_json::to_value(n).ok())
+ .collect();
+ ctx.link().update_default(default.clone());
+ self.sync_from_value(default);
+ }
+ true
+ }
+
fn value_changed(&mut self, ctx: &ManagedFieldContext<Self>) {
match ctx.state().value {
Value::Null => self.sync_from_value(ctx.state().default.clone()),
--
2.39.5
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 33+ messages in thread
* [pdm-devel] [PATCH datacenter-manager 09/21] ui: pve wizard: rename 'realm' variable to 'info'
2025-05-16 13:35 [pdm-devel] [PATCH datacenter-manager 00/21] improve remote wizard Dominik Csapak
` (7 preceding siblings ...)
2025-05-16 13:35 ` [pdm-devel] [PATCH datacenter-manager 08/21] ui: remotes: node url list: handle changing default Dominik Csapak
@ 2025-05-16 13:35 ` Dominik Csapak
2025-05-16 13:36 ` [pdm-devel] [PATCH datacenter-manager 10/21] ui: pve wizard: summary: add default text for fingerprint Dominik Csapak
` (14 subsequent siblings)
23 siblings, 0 replies; 33+ messages in thread
From: Dominik Csapak @ 2025-05-16 13:35 UTC (permalink / raw)
To: pdm-devel
these are not only realms but general connection parameters, so rename
the variable to a less confusing name.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
ui/src/remotes/add_wizard.rs | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/ui/src/remotes/add_wizard.rs b/ui/src/remotes/add_wizard.rs
index f4bf9a3..27cf9b6 100644
--- a/ui/src/remotes/add_wizard.rs
+++ b/ui/src/remotes/add_wizard.rs
@@ -77,8 +77,8 @@ impl Component for AddWizardState {
Msg::ServerChange(server_info) => {
self.server_info = server_info;
}
- Msg::ConnectChange(realms) => {
- self.connect_info = realms;
+ Msg::ConnectChange(info) => {
+ self.connect_info = info;
}
}
true
@@ -110,11 +110,11 @@ impl Component for AddWizardState {
},
)
.with_page(TabBarItem::new().key("info").label(tr!("Settings")), {
- let realms = self.connect_info.clone();
+ let info = self.connect_info.clone();
let link = ctx.link().clone();
move |p: &WizardPageRenderInfo| {
WizardPageInfo::new(p.clone())
- .connect_info(realms.clone())
+ .connect_info(info.clone())
.on_server_change(link.callback(Msg::ServerChange))
.into()
}
--
2.39.5
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 33+ messages in thread
* [pdm-devel] [PATCH datacenter-manager 10/21] ui: pve wizard: summary: add default text for fingerprint
2025-05-16 13:35 [pdm-devel] [PATCH datacenter-manager 00/21] improve remote wizard Dominik Csapak
` (8 preceding siblings ...)
2025-05-16 13:35 ` [pdm-devel] [PATCH datacenter-manager 09/21] ui: pve wizard: rename 'realm' variable to 'info' Dominik Csapak
@ 2025-05-16 13:36 ` Dominik Csapak
2025-05-16 13:36 ` [pdm-devel] [PATCH datacenter-manager 11/21] ui: pve wizard: nodes: improve info text Dominik Csapak
` (13 subsequent siblings)
23 siblings, 0 replies; 33+ messages in thread
From: Dominik Csapak @ 2025-05-16 13:36 UTC (permalink / raw)
To: pdm-devel
to make it clear what it means when no fingerprint is configured.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
ui/src/remotes/wizard_page_summary.rs | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/ui/src/remotes/wizard_page_summary.rs b/ui/src/remotes/wizard_page_summary.rs
index 9ab06f7..bf46814 100644
--- a/ui/src/remotes/wizard_page_summary.rs
+++ b/ui/src/remotes/wizard_page_summary.rs
@@ -168,7 +168,10 @@ fn columns() -> Vec<DataTableHeader<PropertyString<NodeUrl>>> {
DataTableColumn::new(tr!("Fingerprint"))
.flex(2)
.render(move |item: &PropertyString<NodeUrl>| {
- item.fingerprint.as_deref().unwrap_or_default().into()
+ item.fingerprint
+ .as_deref()
+ .unwrap_or(&tr!("Use trusted certificate"))
+ .into()
})
.into(),
]
--
2.39.5
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 33+ messages in thread
* [pdm-devel] [PATCH datacenter-manager 11/21] ui: pve wizard: nodes: improve info text
2025-05-16 13:35 [pdm-devel] [PATCH datacenter-manager 00/21] improve remote wizard Dominik Csapak
` (9 preceding siblings ...)
2025-05-16 13:36 ` [pdm-devel] [PATCH datacenter-manager 10/21] ui: pve wizard: summary: add default text for fingerprint Dominik Csapak
@ 2025-05-16 13:36 ` Dominik Csapak
2025-05-16 13:36 ` [pdm-devel] [PATCH datacenter-manager 12/21] ui: pve wizard: nodes: probe hosts to verify fingerprint settings Dominik Csapak
` (12 subsequent siblings)
23 siblings, 0 replies; 33+ messages in thread
From: Dominik Csapak @ 2025-05-16 13:36 UTC (permalink / raw)
To: pdm-devel
by making it clear that fingerprints are *only* necessary when the
certificates are not trusted.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
ui/src/remotes/wizard_page_nodes.rs | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ui/src/remotes/wizard_page_nodes.rs b/ui/src/remotes/wizard_page_nodes.rs
index 8f217c5..ce73f6e 100644
--- a/ui/src/remotes/wizard_page_nodes.rs
+++ b/ui/src/remotes/wizard_page_nodes.rs
@@ -51,7 +51,7 @@ impl Component for PdmWizardPageNodes {
.padding(4)
.with_child(Container::new().padding(4).with_child(tr!(
"Define a set of addresses that Proxmox Datacenter Manager can use to reach the \
- cluster or single node. Fingerprints are required for self-signed certificates."
+ cluster or single node. Fingerprints are only required for untrusted certificates."
)))
.with_child(
NodeUrlList::new()
--
2.39.5
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 33+ messages in thread
* [pdm-devel] [PATCH datacenter-manager 12/21] ui: pve wizard: nodes: probe hosts to verify fingerprint settings
2025-05-16 13:35 [pdm-devel] [PATCH datacenter-manager 00/21] improve remote wizard Dominik Csapak
` (10 preceding siblings ...)
2025-05-16 13:36 ` [pdm-devel] [PATCH datacenter-manager 11/21] ui: pve wizard: nodes: improve info text Dominik Csapak
@ 2025-05-16 13:36 ` Dominik Csapak
2025-05-16 13:36 ` [pdm-devel] [PATCH datacenter-manager 13/21] ui: pve wizard: info: use pdm_client for scanning Dominik Csapak
` (11 subsequent siblings)
23 siblings, 0 replies; 33+ messages in thread
From: Dominik Csapak @ 2025-05-16 13:36 UTC (permalink / raw)
To: pdm-devel
when advancing the wizard.
* check each host if the fingerprint is correct
* for hosts without fingerprint configured, will prompt the user to
use the fingerprints if the certificates are not trusted
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
ui/Cargo.toml | 1 +
ui/src/remotes/wizard_page_nodes.rs | 239 ++++++++++++++++++++++++++--
2 files changed, 229 insertions(+), 11 deletions(-)
diff --git a/ui/Cargo.toml b/ui/Cargo.toml
index cf50a32..3de092b 100644
--- a/ui/Cargo.toml
+++ b/ui/Cargo.toml
@@ -32,6 +32,7 @@ pwt-macros = "0.3"
proxmox-yew-comp = { version = "0.4.5", features = ["apt", "dns", "network", "rrd"] }
+proxmox-acme-api = { version = "0.1", features = [] }
proxmox-client = "0.5"
proxmox-human-byte = "0.1.3"
proxmox-login = "0.2"
diff --git a/ui/src/remotes/wizard_page_nodes.rs b/ui/src/remotes/wizard_page_nodes.rs
index ce73f6e..aa7eb94 100644
--- a/ui/src/remotes/wizard_page_nodes.rs
+++ b/ui/src/remotes/wizard_page_nodes.rs
@@ -1,15 +1,22 @@
+use std::collections::HashMap;
use std::rc::Rc;
-use pdm_client::types::Remote;
-use pwt::css::FlexFit;
+use proxmox_schema::property_string::PropertyString;
+use serde_json::Value;
use yew::virtual_dom::{VComp, VNode};
-use pwt::prelude::*;
-use pwt::widget::Container;
+use pwt::css::{FlexFit, FontStyle, JustifyContent, Overflow};
+use pwt::widget::{error_message, Button, Column, Container, Dialog, Mask, Row};
+use pwt::{prelude::*, AsyncAbortGuard};
+use pwt_macros::builder;
-use proxmox_yew_comp::WizardPageRenderInfo;
+use proxmox_yew_comp::{KVGrid, KVGridRow, WizardPageRenderInfo};
-use pwt_macros::builder;
+use pdm_api_types::{
+ remotes::{NodeUrl, ScanResult},
+ CertificateInfo,
+};
+use pdm_client::types::Remote;
use super::NodeUrlList;
@@ -29,14 +36,193 @@ impl WizardPageNodes {
}
}
-pub struct PdmWizardPageNodes {}
+pub enum Msg {
+ Scan,
+ ScanResult(Vec<(String, Result<ScanResult, proxmox_client::Error>)>),
+ ConfirmResult(bool),
+}
+
+pub struct PdmWizardPageNodes {
+ scan_results: Vec<(String, Result<ScanResult, proxmox_client::Error>)>,
+ scan_guard: Option<AsyncAbortGuard>,
+ loading: bool,
+ certificate_rows: Rc<Vec<KVGridRow>>,
+}
+
+impl PdmWizardPageNodes {
+ fn create_certificate_confirmation_dialog(
+ &self,
+ ctx: &Context<Self>,
+ certificates: Vec<(&String, &CertificateInfo)>,
+ ) -> Dialog {
+ let link = ctx.link();
+ Dialog::new(tr!("Connection Certificate"))
+ .on_close(link.callback(|_| Msg::ConfirmResult(false)))
+ .with_child(
+ Column::new()
+ .padding(2)
+ .gap(2)
+ .class(FlexFit)
+ .with_child(Container::new().with_child(tr!(
+ "The following certificates of remote servers are not trusted."
+ )))
+ .with_child(Container::new().with_child(tr!(
+ "Do you want to trust them by saving their fingerprint?"
+ )))
+ .with_child(
+ Column::new()
+ .max_height(400)
+ .gap(1)
+ .padding(2)
+ .class(Overflow::Auto)
+ .children(certificates.into_iter().map(|(hostname, certificate)| {
+ Column::new()
+ .with_child(
+ Container::new().class(FontStyle::TitleSmall).with_child(
+ format!("{}: {hostname}", tr!("Server Address")),
+ ),
+ )
+ .with_child(
+ KVGrid::new()
+ .class(FlexFit)
+ .borderless(true)
+ .striped(false)
+ .rows(self.certificate_rows.clone())
+ .data(Rc::new(
+ serde_json::to_value(certificate)
+ .unwrap_or_default(),
+ )),
+ )
+ .into()
+ })),
+ )
+ .with_child(
+ Row::new()
+ .gap(2)
+ .class(JustifyContent::Center)
+ .with_child(
+ Button::new(tr!("Yes"))
+ .onclick(link.callback(|_| Msg::ConfirmResult(true))),
+ )
+ .with_child(
+ Button::new(tr!("No"))
+ .onclick(link.callback(|_| Msg::ConfirmResult(false))),
+ ),
+ ),
+ )
+ }
+}
impl Component for PdmWizardPageNodes {
- type Message = ();
+ type Message = Msg;
type Properties = WizardPageNodes;
fn create(_ctx: &Context<Self>) -> Self {
- Self {}
+ _ctx.props().info.on_next({
+ let link = _ctx.link().clone();
+ move |_| {
+ link.send_message(Msg::Scan);
+ false
+ }
+ });
+ Self {
+ scan_results: Vec::new(),
+ scan_guard: None,
+ loading: false,
+ certificate_rows: Rc::new(rows()),
+ }
+ }
+
+ fn update(&mut self, ctx: &Context<Self>, msg: Self::Message) -> bool {
+ let props = ctx.props();
+ match msg {
+ Msg::Scan => {
+ self.loading = true;
+ let link = ctx.link().clone();
+ let nodes = props.info.form_ctx.read().get_field_value("nodes");
+ let Some(Value::Array(nodes)) = nodes else {
+ return true;
+ };
+ self.scan_guard = Some(AsyncAbortGuard::spawn(async move {
+ let futures = nodes.into_iter().filter_map(|node| {
+ let node = match serde_json::from_value::<PropertyString<NodeUrl>>(node) {
+ Ok(node) => node.into_inner(),
+ Err(_) => return None,
+ };
+
+ let future = async move {
+ let res = crate::pdm_client()
+ .pve_probe_tls(&node.hostname, node.fingerprint.as_deref())
+ .await;
+ (node.hostname, res)
+ };
+ Some(future)
+ });
+
+ let res = futures::future::join_all(futures).await;
+ link.send_message(Msg::ScanResult(res));
+ }));
+ }
+ Msg::ScanResult(scan_results) => {
+ self.loading = false;
+ self.scan_results = scan_results;
+ let mut success = true;
+ for (_hostname, result) in &self.scan_results {
+ match result {
+ Ok(ScanResult::TlsResult(None)) => {}
+ _ => success = false,
+ }
+ }
+
+ if success {
+ props.info.go_to_next_page();
+ }
+ }
+ Msg::ConfirmResult(confirm) => {
+ if confirm {
+ // update connect information with gathered certificate information
+ // and navigate to next page
+ let mut map = HashMap::new();
+ for (hostname, res) in self.scan_results.drain(..) {
+ if let Ok(ScanResult::TlsResult(Some(cert))) = res {
+ if let Some(fp) = cert.fingerprint {
+ map.insert(hostname, fp);
+ }
+ }
+ }
+
+ let mut form = props.info.form_ctx.write();
+ let value = form
+ .get_field_value("nodes")
+ .unwrap_or(Value::Array(Vec::new()));
+
+ let value = match serde_json::from_value::<Vec<PropertyString<NodeUrl>>>(value)
+ {
+ Ok(mut nodes) => {
+ for node in nodes.iter_mut() {
+ if node.fingerprint.is_none() && map.contains_key(&node.hostname) {
+ node.fingerprint =
+ Some(map.get(&node.hostname).unwrap().to_uppercase());
+ }
+ }
+ // this should never fail
+ serde_json::to_value(nodes).unwrap()
+ }
+ Err(_) => {
+ // data from field is wrong, this should not happen
+ unreachable!("internal data in node field is wrong");
+ }
+ };
+
+ form.set_field_value("nodes", value);
+ drop(form);
+ props.info.go_to_next_page();
+ } else {
+ self.scan_results.clear();
+ }
+ }
+ }
+ true
}
fn view(&self, ctx: &Context<Self>) -> Html {
@@ -46,7 +232,25 @@ impl Component for PdmWizardPageNodes {
.as_ref()
.map(|info| info.nodes.clone())
.unwrap_or_default();
- Container::new()
+
+ let mut errors = Vec::new();
+ let mut certificates = Vec::new();
+
+ for (hostname, result) in &self.scan_results {
+ match result {
+ Ok(ScanResult::TlsResult(Some(cert))) => {
+ certificates.push((hostname, cert));
+ }
+ Ok(_) => {}
+ Err(err) => {
+ errors.push(error_message(&format!("{hostname} - {err}")).into());
+ }
+ }
+ }
+
+ let has_errors = !errors.is_empty();
+
+ let content = Container::new()
.class(FlexFit)
.padding(4)
.with_child(Container::new().padding(4).with_child(tr!(
@@ -61,10 +265,23 @@ impl Component for PdmWizardPageNodes {
.key("nodes")
.required(true),
)
- .into()
+ .with_optional_child((has_errors).then_some(Column::new().children(errors)))
+ .with_optional_child(
+ (!has_errors && !certificates.is_empty())
+ .then_some(self.create_certificate_confirmation_dialog(ctx, certificates)),
+ );
+ Mask::new(content).visible(self.loading).into()
}
}
+fn rows() -> Vec<KVGridRow> {
+ vec![
+ KVGridRow::new("fingerprint", tr!("Fingerprint")),
+ KVGridRow::new("issuer", tr!("Issuer")),
+ KVGridRow::new("subject", tr!("Subject")),
+ ]
+}
+
impl Into<VNode> for WizardPageNodes {
fn into(self) -> VNode {
let comp = VComp::new::<PdmWizardPageNodes>(Rc::new(self), None);
--
2.39.5
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 33+ messages in thread
* [pdm-devel] [PATCH datacenter-manager 13/21] ui: pve wizard: info: use pdm_client for scanning
2025-05-16 13:35 [pdm-devel] [PATCH datacenter-manager 00/21] improve remote wizard Dominik Csapak
` (11 preceding siblings ...)
2025-05-16 13:36 ` [pdm-devel] [PATCH datacenter-manager 12/21] ui: pve wizard: nodes: probe hosts to verify fingerprint settings Dominik Csapak
@ 2025-05-16 13:36 ` Dominik Csapak
2025-05-16 13:36 ` [pdm-devel] [PATCH datacenter-manager 14/21] ui: pve wizard: info: detect hostname and fingerprint Dominik Csapak
` (10 subsequent siblings)
23 siblings, 0 replies; 33+ messages in thread
From: Dominik Csapak @ 2025-05-16 13:36 UTC (permalink / raw)
To: pdm-devel
so that we don't have to use a string as the api path and get better
type safety.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
ui/src/remotes/wizard_page_info.rs | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/ui/src/remotes/wizard_page_info.rs b/ui/src/remotes/wizard_page_info.rs
index 9953f42..882eebd 100644
--- a/ui/src/remotes/wizard_page_info.rs
+++ b/ui/src/remotes/wizard_page_info.rs
@@ -94,18 +94,25 @@ async fn scan(connection_params: ConnectParams, form_ctx: FormContext) -> Result
data["fingerprint"] = fp.into();
}
- let data: ScanParams = serde_json::from_value(data.clone())?;
-
- let params = serde_json::to_value(&data)?;
- let mut result = match proxmox_yew_comp::http_post("/pve/scan", Some(params)).await? {
+ let ScanParams {
+ hostname,
+ authid,
+ token,
+ fingerprint,
+ } = serde_json::from_value(data.clone())?;
+
+ let mut result = match crate::pdm_client()
+ .pve_scan_remote(&hostname, fingerprint.as_deref(), &authid, &token)
+ .await?
+ {
ScanResult::TlsResult(_) => bail!("Untrusted certificate or invalid fingerprint"),
ScanResult::Remote(remote) => remote,
};
result.nodes.insert(
0,
PropertyString::new(NodeUrl {
- hostname: data.hostname,
- fingerprint: data.fingerprint,
+ hostname,
+ fingerprint,
}),
);
result.nodes.sort_by(|a, b| a.hostname.cmp(&b.hostname));
--
2.39.5
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 33+ messages in thread
* [pdm-devel] [PATCH datacenter-manager 14/21] ui: pve wizard: info: detect hostname and fingerprint
2025-05-16 13:35 [pdm-devel] [PATCH datacenter-manager 00/21] improve remote wizard Dominik Csapak
` (12 preceding siblings ...)
2025-05-16 13:36 ` [pdm-devel] [PATCH datacenter-manager 13/21] ui: pve wizard: info: use pdm_client for scanning Dominik Csapak
@ 2025-05-16 13:36 ` Dominik Csapak
2025-08-19 11:55 ` Lukas Wagner
2025-05-16 13:36 ` [pdm-devel] [PATCH datacenter-manager 15/21] ui: pve wizard: info: remove manual scan button Dominik Csapak
` (9 subsequent siblings)
23 siblings, 1 reply; 33+ messages in thread
From: Dominik Csapak @ 2025-05-16 13:36 UTC (permalink / raw)
To: pdm-devel
Instead of always inserting the initial host into the nodelist
(which will always be duplicate since it will be also contained in
the node list), try to match the hostname and or fingerprint with
any of the given node, so we can deduplicate that.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
ui/src/remotes/wizard_page_info.rs | 35 ++++++++++++++++++++++++------
1 file changed, 28 insertions(+), 7 deletions(-)
diff --git a/ui/src/remotes/wizard_page_info.rs b/ui/src/remotes/wizard_page_info.rs
index 882eebd..04b0004 100644
--- a/ui/src/remotes/wizard_page_info.rs
+++ b/ui/src/remotes/wizard_page_info.rs
@@ -108,13 +108,34 @@ async fn scan(connection_params: ConnectParams, form_ctx: FormContext) -> Result
ScanResult::TlsResult(_) => bail!("Untrusted certificate or invalid fingerprint"),
ScanResult::Remote(remote) => remote,
};
- result.nodes.insert(
- 0,
- PropertyString::new(NodeUrl {
- hostname,
- fingerprint,
- }),
- );
+
+ // try to detect inserted hostname/fingerprint
+ let mut found = false;
+ for node in result.nodes.iter_mut() {
+ if node.hostname == hostname {
+ if fingerprint.is_none() {
+ node.fingerprint = None;
+ }
+ found = true;
+ continue;
+ }
+ if node.fingerprint.as_ref().map(|fp| fp.to_uppercase())
+ == fingerprint.as_ref().map(|fp| fp.to_uppercase())
+ {
+ found = true;
+ node.hostname = hostname.clone();
+ continue;
+ }
+ }
+ if !found {
+ result.nodes.insert(
+ 0,
+ PropertyString::new(NodeUrl {
+ hostname,
+ fingerprint: fingerprint.map(|fp| fp.to_uppercase()),
+ }),
+ );
+ }
result.nodes.sort_by(|a, b| a.hostname.cmp(&b.hostname));
Ok(result)
}
--
2.39.5
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 33+ messages in thread
* [pdm-devel] [PATCH datacenter-manager 15/21] ui: pve wizard: info: remove manual scan button
2025-05-16 13:35 [pdm-devel] [PATCH datacenter-manager 00/21] improve remote wizard Dominik Csapak
` (13 preceding siblings ...)
2025-05-16 13:36 ` [pdm-devel] [PATCH datacenter-manager 14/21] ui: pve wizard: info: detect hostname and fingerprint Dominik Csapak
@ 2025-05-16 13:36 ` Dominik Csapak
2025-05-16 13:36 ` [pdm-devel] [PATCH datacenter-manager 16/21] ui: widget: add pve realm selector Dominik Csapak
` (8 subsequent siblings)
23 siblings, 0 replies; 33+ messages in thread
From: Dominik Csapak @ 2025-05-16 13:36 UTC (permalink / raw)
To: pdm-devel
and scan the host when advancing in the wizard instead. This makes the
panel a bit less cluttered.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
ui/src/remotes/wizard_page_info.rs | 30 +++++++++++++++++++-----------
1 file changed, 19 insertions(+), 11 deletions(-)
diff --git a/ui/src/remotes/wizard_page_info.rs b/ui/src/remotes/wizard_page_info.rs
index 04b0004..0d19ebf 100644
--- a/ui/src/remotes/wizard_page_info.rs
+++ b/ui/src/remotes/wizard_page_info.rs
@@ -188,6 +188,14 @@ impl Component for PdmWizardPageInfo {
.form_ctx
.add_listener(ctx.link().callback(|_| Msg::FormChange));
+ props.info.on_next({
+ let link = ctx.link().clone();
+ move |_| {
+ link.send_message(Msg::Connect);
+ false
+ }
+ });
+
Self {
server_info: None,
user_mode: true,
@@ -223,6 +231,7 @@ impl Component for PdmWizardPageInfo {
} else {
self.credentials = None;
}
+ props.info.page_lock(self.credentials.is_none());
}
Msg::Connect => {
let link = ctx.link().clone();
@@ -230,6 +239,7 @@ impl Component for PdmWizardPageInfo {
let form_ctx = props.info.form_ctx.clone();
self.loading = true;
self.last_error = None;
+ props.info.page_lock(true);
if let Some(connection_info) = props.connect_info.clone() {
self.async_pool.spawn(async move {
@@ -242,19 +252,27 @@ impl Component for PdmWizardPageInfo {
}
Msg::ConnectResult(server_info) => {
self.loading = false;
+ props.info.page_lock(false);
match server_info {
Ok(server_info) => {
self.update_server_info(ctx, Some(server_info));
}
Err(err) => {
self.last_error = Some(err);
+ props.info.page_lock(true);
}
}
if let Some(form_ctx) = props.info.lookup_form_context(&Key::from("nodes")) {
- form_ctx.write().reset_form();
+ let mut form = form_ctx.write();
+ form.set_field_value("nodes", serde_json::Value::Null);
+ form.reset_form();
}
props.info.reset_remaining_valid_pages();
+
+ if self.last_error.is_none() {
+ props.info.go_to_next_page();
+ }
}
}
true
@@ -366,16 +384,6 @@ impl Component for PdmWizardPageInfo {
self.last_error
.as_deref()
.map(|err| error_message(&err.to_string())),
- )
- .with_flex_spacer()
- .with_optional_child(
- (self.last_error.is_none() && self.server_info.is_some())
- .then_some(Container::new().with_child(tr!("Scan OK"))),
- )
- .with_child(
- Button::new("Scan")
- .disabled(self.credentials.is_none())
- .onclick(ctx.link().callback(|_| Msg::Connect)),
),
);
Mask::new(content)
--
2.39.5
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 33+ messages in thread
* [pdm-devel] [PATCH datacenter-manager 16/21] ui: widget: add pve realm selector
2025-05-16 13:35 [pdm-devel] [PATCH datacenter-manager 00/21] improve remote wizard Dominik Csapak
` (14 preceding siblings ...)
2025-05-16 13:36 ` [pdm-devel] [PATCH datacenter-manager 15/21] ui: pve wizard: info: remove manual scan button Dominik Csapak
@ 2025-05-16 13:36 ` Dominik Csapak
2025-08-19 11:55 ` Lukas Wagner
2025-05-16 13:36 ` [pdm-devel] [PATCH datacenter-manager 17/21] ui: pve wizard: info: use " Dominik Csapak
` (7 subsequent siblings)
23 siblings, 1 reply; 33+ messages in thread
From: Dominik Csapak @ 2025-05-16 13:36 UTC (permalink / raw)
To: pdm-devel
similar to the general realm selector, but with a hostname/fingerprint
property, so we can query the realms of any PVE instance.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
ui/src/widget/mod.rs | 3 +
ui/src/widget/pve_realm_selector.rs | 125 ++++++++++++++++++++++++++++
2 files changed, 128 insertions(+)
create mode 100644 ui/src/widget/pve_realm_selector.rs
diff --git a/ui/src/widget/mod.rs b/ui/src/widget/mod.rs
index ee9e799..9d7840c 100644
--- a/ui/src/widget/mod.rs
+++ b/ui/src/widget/mod.rs
@@ -13,6 +13,9 @@ pub use pve_storage_selector::PveStorageSelector;
mod pve_migrate_mapping;
pub use pve_migrate_mapping::PveMigrateMap;
+mod pve_realm_selector;
+pub use pve_realm_selector::PveRealmSelector;
+
mod resource_tree;
pub use resource_tree::ResourceTree;
diff --git a/ui/src/widget/pve_realm_selector.rs b/ui/src/widget/pve_realm_selector.rs
new file mode 100644
index 0000000..8025c2d
--- /dev/null
+++ b/ui/src/widget/pve_realm_selector.rs
@@ -0,0 +1,125 @@
+use anyhow::format_err;
+use std::rc::Rc;
+
+use yew::html::IntoPropValue;
+use yew::prelude::*;
+
+use pwt::props::RenderFn;
+use pwt::state::Store;
+use pwt::widget::data_table::{DataTable, DataTableColumn, DataTableHeader};
+use pwt::widget::form::{Selector, SelectorRenderArgs, ValidateFn};
+use pwt::widget::GridPicker;
+
+use proxmox_yew_comp::common_api_types::BasicRealmInfo;
+use proxmox_yew_comp::percent_encoding::percent_encode_component;
+
+use pwt::props::{FieldBuilder, WidgetBuilder};
+use pwt_macros::{builder, widget};
+
+#[widget(comp=PdmPveRealmSelector, @input)]
+#[derive(Clone, Properties, PartialEq)]
+#[builder]
+pub struct PveRealmSelector {
+ /// The default value.
+ #[builder(IntoPropValue, into_prop_value)]
+ #[prop_or_default]
+ pub default: Option<AttrValue>,
+
+ pub hostname: AttrValue,
+
+ pub fingerprint: Option<AttrValue>,
+}
+
+impl PveRealmSelector {
+ pub fn new(
+ hostname: impl IntoPropValue<AttrValue>,
+ fingerprint: impl IntoPropValue<Option<AttrValue>>,
+ ) -> Self {
+ yew::props!(Self {
+ hostname: hostname.into_prop_value(),
+ fingerprint: fingerprint.into_prop_value(),
+ })
+ }
+}
+
+pub struct PdmPveRealmSelector {
+ store: Store<BasicRealmInfo>,
+ validate: ValidateFn<(String, Store<BasicRealmInfo>)>,
+ picker: RenderFn<SelectorRenderArgs<Store<BasicRealmInfo>>>,
+}
+
+impl Component for PdmPveRealmSelector {
+ type Message = ();
+ type Properties = PveRealmSelector;
+
+ fn create(ctx: &Context<Self>) -> Self {
+ let store = Store::new().on_change(ctx.link().callback(|_| ())); // trigger redraw
+
+ let validate = ValidateFn::new(|(realm, store): &(String, Store<BasicRealmInfo>)| {
+ store
+ .read()
+ .data()
+ .iter()
+ .find(|item| &item.realm == realm)
+ .map(drop)
+ .ok_or_else(|| format_err!("no such realm"))
+ });
+
+ let picker = RenderFn::new({
+ let columns = columns();
+ move |args: &SelectorRenderArgs<Store<BasicRealmInfo>>| {
+ let table = DataTable::new(columns.clone(), args.store.clone()).class("pwt-fit");
+
+ GridPicker::new(table)
+ .selection(args.selection.clone())
+ .on_select(args.controller.on_select_callback())
+ .into()
+ }
+ });
+
+ Self {
+ store,
+ validate,
+ picker,
+ }
+ }
+
+ fn view(&self, ctx: &Context<Self>) -> Html {
+ let props = ctx.props();
+
+ let mut url = format!(
+ "/pve/realms?hostname={}",
+ percent_encode_component(&props.hostname)
+ );
+ if let Some(fp) = &props.fingerprint {
+ url.push_str(&format!("&fingerprint={}", percent_encode_component(fp)));
+ }
+
+ Selector::new(self.store.clone(), self.picker.clone())
+ .with_std_props(&props.std_props)
+ .with_input_props(&props.input_props)
+ .required(true)
+ .default(props.default.as_deref().unwrap_or("pam").to_string())
+ .loader(url)
+ .validate(self.validate.clone())
+ .into()
+ }
+}
+
+fn columns() -> Rc<Vec<DataTableHeader<BasicRealmInfo>>> {
+ Rc::new(vec![
+ DataTableColumn::new("Realm")
+ .width("100px")
+ .sort_order(true)
+ .show_menu(false)
+ .get_property(|record: &BasicRealmInfo| &record.realm)
+ .into(),
+ DataTableColumn::new("Comment")
+ .width("300px")
+ .show_menu(false)
+ .get_property_owned(|record: &BasicRealmInfo| {
+ record.comment.clone().unwrap_or_default()
+ })
+ .into(),
+ ])
+}
--
2.39.5
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 33+ messages in thread
* [pdm-devel] [PATCH datacenter-manager 17/21] ui: pve wizard: info: use pve realm selector
2025-05-16 13:35 [pdm-devel] [PATCH datacenter-manager 00/21] improve remote wizard Dominik Csapak
` (15 preceding siblings ...)
2025-05-16 13:36 ` [pdm-devel] [PATCH datacenter-manager 16/21] ui: widget: add pve realm selector Dominik Csapak
@ 2025-05-16 13:36 ` Dominik Csapak
2025-05-16 13:36 ` [pdm-devel] [PATCH datacenter-manager 18/21] ui: pve wizard: connect: factor out normalize_hostname Dominik Csapak
` (6 subsequent siblings)
23 siblings, 0 replies; 33+ messages in thread
From: Dominik Csapak @ 2025-05-16 13:36 UTC (permalink / raw)
To: pdm-devel
instead of using the realm info from the connect page scan.
This enables us to use another api path for the connect page.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
ui/src/remotes/wizard_page_info.rs | 40 ++++++++----------------------
1 file changed, 11 insertions(+), 29 deletions(-)
diff --git a/ui/src/remotes/wizard_page_info.rs b/ui/src/remotes/wizard_page_info.rs
index 0d19ebf..a3834fc 100644
--- a/ui/src/remotes/wizard_page_info.rs
+++ b/ui/src/remotes/wizard_page_info.rs
@@ -2,18 +2,18 @@ use std::rc::Rc;
use anyhow::{bail, Error};
use html::IntoEventCallback;
-use proxmox_schema::property_string::PropertyString;
use serde::{Deserialize, Serialize};
use yew::virtual_dom::{Key, VComp, VNode};
+use proxmox_schema::property_string::PropertyString;
use proxmox_yew_comp::WizardPageRenderInfo;
use pwt::{
css::{self, FlexFit},
prelude::*,
widget::{
error_message,
- form::{Combobox, Field, FormContext, FormContextObserver, InputType, RadioButton},
- Button, Column, Container, InputPanel, Mask, Row,
+ form::{Field, FormContext, FormContextObserver, InputType, RadioButton},
+ Column, Container, InputPanel, Mask, Row,
},
AsyncPool,
};
@@ -23,6 +23,7 @@ use pdm_api_types::remotes::{NodeUrl, Remote, ScanResult};
use pwt_macros::builder;
use super::wizard_page_connect::ConnectParams;
+use crate::widget::PveRealmSelector;
#[derive(Clone, PartialEq, Properties)]
#[builder]
@@ -46,7 +47,6 @@ impl WizardPageInfo {
pub struct PdmWizardPageInfo {
user_mode: bool,
- realms: Rc<Vec<AttrValue>>,
server_info: Option<Remote>,
last_error: Option<Error>,
credentials: Option<(String, String)>,
@@ -72,20 +72,6 @@ pub struct ScanParams {
fingerprint: Option<String>,
}
-fn create_realm_list(props: &WizardPageInfo) -> Rc<Vec<AttrValue>> {
- if let Some(info) = &props.connect_info {
- let realms = Rc::new(
- info.realms
- .iter()
- .map(|realm| AttrValue::from(realm.realm.clone()))
- .collect(),
- );
- realms
- } else {
- Rc::new(Vec::new())
- }
-}
-
async fn scan(connection_params: ConnectParams, form_ctx: FormContext) -> Result<Remote, Error> {
let mut data = form_ctx.get_submit_data();
@@ -199,7 +185,6 @@ impl Component for PdmWizardPageInfo {
Self {
server_info: None,
user_mode: true,
- realms: create_realm_list(props),
_form_observer,
last_error: None,
loading: false,
@@ -278,12 +263,12 @@ impl Component for PdmWizardPageInfo {
true
}
- fn changed(&mut self, ctx: &Context<Self>, _old_props: &Self::Properties) -> bool {
- self.realms = create_realm_list(ctx.props());
- true
- }
-
fn view(&self, ctx: &Context<Self>) -> Html {
+ let props = ctx.props();
+ let (hostname, fingerprint) = match props.connect_info.clone() {
+ Some(info) => (info.hostname, info.fingerprint),
+ None => (Default::default(), None),
+ };
let input_panel = InputPanel::new()
.class(FlexFit)
.padding(4)
@@ -318,12 +303,9 @@ impl Component for PdmWizardPageInfo {
)
.with_field(
tr!("Realm"),
- Combobox::new()
+ PveRealmSelector::new(hostname, fingerprint)
.name("realm")
- .disabled(!self.user_mode)
- .required(self.user_mode)
- .items(self.realms.clone())
- .submit(false),
+ .required(true),
)
.with_field(
tr!("API Token Name"),
--
2.39.5
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 33+ messages in thread
* [pdm-devel] [PATCH datacenter-manager 18/21] ui: pve wizard: connect: factor out normalize_hostname
2025-05-16 13:35 [pdm-devel] [PATCH datacenter-manager 00/21] improve remote wizard Dominik Csapak
` (16 preceding siblings ...)
2025-05-16 13:36 ` [pdm-devel] [PATCH datacenter-manager 17/21] ui: pve wizard: info: use " Dominik Csapak
@ 2025-05-16 13:36 ` Dominik Csapak
2025-05-16 13:36 ` [pdm-devel] [PATCH datacenter-manager 19/21] ui: pve wizard: connect: move connection logic to next button Dominik Csapak
` (5 subsequent siblings)
23 siblings, 0 replies; 33+ messages in thread
From: Dominik Csapak @ 2025-05-16 13:36 UTC (permalink / raw)
To: pdm-devel
we'll use this later again
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
ui/src/remotes/wizard_page_connect.rs | 24 +++++++++++++++---------
1 file changed, 15 insertions(+), 9 deletions(-)
diff --git a/ui/src/remotes/wizard_page_connect.rs b/ui/src/remotes/wizard_page_connect.rs
index f6013f2..bdc6b7f 100644
--- a/ui/src/remotes/wizard_page_connect.rs
+++ b/ui/src/remotes/wizard_page_connect.rs
@@ -65,15 +65,7 @@ pub struct ConnectParams {
async fn connect(form_ctx: FormContext, remote_type: RemoteType) -> Result<ConnectParams, Error> {
let data = form_ctx.get_submit_data();
let mut data: ConnectParams = serde_json::from_value(data.clone())?;
- if let Some(hostname) = data.hostname.strip_prefix("http://") {
- data.hostname = hostname.to_string();
- }
- if let Some(hostname) = data.hostname.strip_prefix("https://") {
- data.hostname = hostname.to_string();
- }
- if let Some(hostname) = data.hostname.strip_suffix("/") {
- data.hostname = hostname.to_string();
- }
+ data.hostname = normalize_hostname(data.hostname);
let realms = match remote_type {
RemoteType::Pve => list_realms(data.hostname.clone(), data.fingerprint.clone()).await?,
@@ -234,6 +226,20 @@ impl Component for PdmWizardPageConnect {
}
}
+fn normalize_hostname(hostname: String) -> String {
+ let mut result = hostname;
+ if let Some(hostname) = result.strip_prefix("http://") {
+ result = hostname.to_string();
+ }
+ if let Some(hostname) = result.strip_prefix("https://") {
+ result = hostname.to_string();
+ }
+ if let Some(hostname) = result.strip_suffix("/") {
+ result = hostname.to_string();
+ }
+ result
+}
+
impl Into<VNode> for WizardPageConnect {
fn into(self) -> VNode {
let comp = VComp::new::<PdmWizardPageConnect>(Rc::new(self), None);
--
2.39.5
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 33+ messages in thread
* [pdm-devel] [PATCH datacenter-manager 19/21] ui: pve wizard: connect: move connection logic to next button
2025-05-16 13:35 [pdm-devel] [PATCH datacenter-manager 00/21] improve remote wizard Dominik Csapak
` (17 preceding siblings ...)
2025-05-16 13:36 ` [pdm-devel] [PATCH datacenter-manager 18/21] ui: pve wizard: connect: factor out normalize_hostname Dominik Csapak
@ 2025-05-16 13:36 ` Dominik Csapak
2025-05-16 13:36 ` [pdm-devel] [PATCH datacenter-manager 20/21] ui: pve wizard: connect: use scan api endpoint instead of realms Dominik Csapak
` (4 subsequent siblings)
23 siblings, 0 replies; 33+ messages in thread
From: Dominik Csapak @ 2025-05-16 13:36 UTC (permalink / raw)
To: pdm-devel
and remove the explicit 'Connect' button
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
ui/src/remotes/wizard_page_connect.rs | 45 ++++++++++-----------------
1 file changed, 17 insertions(+), 28 deletions(-)
diff --git a/ui/src/remotes/wizard_page_connect.rs b/ui/src/remotes/wizard_page_connect.rs
index bdc6b7f..d5d9708 100644
--- a/ui/src/remotes/wizard_page_connect.rs
+++ b/ui/src/remotes/wizard_page_connect.rs
@@ -6,9 +6,9 @@ use serde_json::json;
use yew::html::IntoEventCallback;
use yew::virtual_dom::{Key, VComp, VNode};
-use pwt::css::{AlignItems, FlexFit};
+use pwt::css::FlexFit;
use pwt::widget::form::{Field, FormContext, FormContextObserver};
-use pwt::widget::{error_message, Button, Column, Container, InputPanel, Mask, Row};
+use pwt::widget::{error_message, Column, InputPanel, Mask};
use pwt::{prelude::*, AsyncPool};
use proxmox_yew_comp::{SchemaValidation, WizardPageRenderInfo};
@@ -113,6 +113,13 @@ impl Component for PdmWizardPageConnect {
.add_listener(ctx.link().callback(|_| Msg::FormChange));
props.info.page_lock(true);
+ props.info.on_next({
+ let link = ctx.link().clone();
+ move |_| {
+ link.send_message(Msg::Connect);
+ false
+ }
+ });
Self {
connect_info: None,
@@ -137,6 +144,7 @@ impl Component for PdmWizardPageConnect {
return <Self as yew::Component>::update(self, ctx, Msg::Connect)
}
}
+ props.info.page_lock(!self.form_valid);
}
Msg::Connect => {
let link = ctx.link().clone();
@@ -166,13 +174,16 @@ impl Component for PdmWizardPageConnect {
form_ctx.write().reset_form();
}
props.info.reset_remaining_valid_pages();
+ if self.connect_info.is_some() {
+ props.info.go_to_next_page();
+ }
}
}
true
}
- fn view(&self, ctx: &Context<Self>) -> Html {
- let props = ctx.props();
+ fn view(&self, _ctx: &Context<Self>) -> Html {
+ let error = self.last_error.as_ref();
let input_panel = InputPanel::new()
.class(FlexFit)
// FIXME: input panel css style is not optimal here...
@@ -192,33 +203,11 @@ impl Component for PdmWizardPageConnect {
.placeholder(tr!("Server certificate SHA-256 fingerprint, required for self-signed certificates"))
.schema(&CERT_FINGERPRINT_SHA256_SCHEMA),
);
-
let content = Column::new()
.class(FlexFit)
.with_child(input_panel)
- .with_optional_child(
- (props.remote_type == RemoteType::Pve).then_some(
- Row::new()
- .padding(2)
- .gap(2)
- .class(AlignItems::Center)
- .with_optional_child(
- self.last_error
- .as_deref()
- .map(|err| error_message(&err.to_string())),
- )
- .with_flex_spacer()
- .with_optional_child(
- (self.last_error.is_none() && self.connect_info.is_some())
- .then_some(Container::new().with_child(tr!("Connection OK"))),
- )
- .with_child(
- Button::new("Connect")
- .disabled(!self.form_valid)
- .onclick(ctx.link().callback(|_| Msg::Connect)),
- ),
- ),
- );
+ .with_optional_child(error.map(|err| error_message(&err.to_string())));
+
Mask::new(content)
.class(FlexFit)
.visible(self.loading)
--
2.39.5
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 33+ messages in thread
* [pdm-devel] [PATCH datacenter-manager 20/21] ui: pve wizard: connect: use scan api endpoint instead of realms
2025-05-16 13:35 [pdm-devel] [PATCH datacenter-manager 00/21] improve remote wizard Dominik Csapak
` (18 preceding siblings ...)
2025-05-16 13:36 ` [pdm-devel] [PATCH datacenter-manager 19/21] ui: pve wizard: connect: move connection logic to next button Dominik Csapak
@ 2025-05-16 13:36 ` Dominik Csapak
2025-05-16 13:36 ` [pdm-devel] [PATCH datacenter-manager 21/21] ui: pve wizard: connect: add certificate confirmation dialog Dominik Csapak
` (3 subsequent siblings)
23 siblings, 0 replies; 33+ messages in thread
From: Dominik Csapak @ 2025-05-16 13:36 UTC (permalink / raw)
To: pdm-devel
Since we don't need to query the realms anymore for the next page, we
can now use the scan api endpoint without credentials to probe the
connection to see if TLS works and if the certificate is trusted and/or
if the fingerprint is correct.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
ui/src/remotes/wizard_page_connect.rs | 166 ++++++++++++--------------
1 file changed, 79 insertions(+), 87 deletions(-)
diff --git a/ui/src/remotes/wizard_page_connect.rs b/ui/src/remotes/wizard_page_connect.rs
index d5d9708..9f73779 100644
--- a/ui/src/remotes/wizard_page_connect.rs
+++ b/ui/src/remotes/wizard_page_connect.rs
@@ -2,22 +2,19 @@ use std::rc::Rc;
use anyhow::{bail, Error};
use serde::{Deserialize, Serialize};
-use serde_json::json;
use yew::html::IntoEventCallback;
use yew::virtual_dom::{Key, VComp, VNode};
use pwt::css::FlexFit;
use pwt::widget::form::{Field, FormContext, FormContextObserver};
use pwt::widget::{error_message, Column, InputPanel, Mask};
-use pwt::{prelude::*, AsyncPool};
+use pwt::{prelude::*, AsyncAbortGuard};
+use pwt_macros::builder;
use proxmox_yew_comp::{SchemaValidation, WizardPageRenderInfo};
-use pdm_api_types::remotes::RemoteType;
+use pdm_api_types::remotes::{RemoteType, ScanResult};
use pdm_api_types::CERT_FINGERPRINT_SHA256_SCHEMA;
-use pdm_client::types::ListRealm;
-
-use pwt_macros::builder;
#[derive(Clone, PartialEq, Properties)]
#[builder]
@@ -37,69 +34,46 @@ impl WizardPageConnect {
}
}
-async fn list_realms(
- hostname: String,
- fingerprint: Option<String>,
-) -> Result<Vec<ListRealm>, Error> {
- let mut params = json!({
- "hostname": hostname,
- });
- if let Some(fp) = fingerprint {
- params["fingerprint"] = fp.into();
- }
- let result: Vec<ListRealm> = proxmox_yew_comp::http_get("/pve/realms", Some(params)).await?;
-
- Ok(result)
-}
-
#[derive(PartialEq, Clone, Deserialize, Serialize)]
/// Parameters for connect call.
pub struct ConnectParams {
pub hostname: String,
#[serde(skip_serializing_if = "Option::is_none")]
pub fingerprint: Option<String>,
- #[serde(default)]
- pub realms: Vec<ListRealm>,
}
-async fn connect(form_ctx: FormContext, remote_type: RemoteType) -> Result<ConnectParams, Error> {
- let data = form_ctx.get_submit_data();
- let mut data: ConnectParams = serde_json::from_value(data.clone())?;
- data.hostname = normalize_hostname(data.hostname);
+async fn connect(form_ctx: FormContext, remote_type: RemoteType) -> Result<ScanResult, Error> {
+ match remote_type {
+ RemoteType::Pve => {
+ let hostname = normalize_hostname(form_ctx.read().get_field_text("hostname"));
+ let fingerprint = get_fingerprint(&form_ctx);
+ let res = crate::pdm_client()
+ .pve_probe_tls(&hostname, fingerprint.as_deref())
+ .await
+ .map_err(Error::from);
+
+ if let Ok(ScanResult::TlsResult(Some(_))) = &res {
+ bail!("Untrusted Certificate, please enter fingerprint");
+ }
- let realms = match remote_type {
- RemoteType::Pve => list_realms(data.hostname.clone(), data.fingerprint.clone()).await?,
+ res
+ }
RemoteType::Pbs => bail!("not implemented"),
- };
-
- data.realms = realms;
- Ok(data)
+ }
}
pub enum Msg {
FormChange,
Connect,
- ConnectResult(Result<ConnectParams, Error>),
+ ConnectResult(Result<ScanResult, Error>),
}
pub struct PdmWizardPageConnect {
- connect_info: Option<ConnectParams>,
_form_observer: FormContextObserver,
- form_valid: bool,
loading: bool,
- last_error: Option<Error>,
- async_pool: AsyncPool,
+ scan_result: Option<Result<ScanResult, Error>>,
+ scan_guard: Option<AsyncAbortGuard>,
}
-impl PdmWizardPageConnect {
- fn update_connect_info(&mut self, ctx: &Context<Self>, info: Option<ConnectParams>) {
- let props = ctx.props();
- self.connect_info = info.clone();
- props.info.page_lock(info.is_none());
- if let Some(on_connect_change) = &props.on_connect_change {
- on_connect_change.emit(info);
- }
- }
-}
impl Component for PdmWizardPageConnect {
type Message = Msg;
type Properties = WizardPageConnect;
@@ -122,12 +96,10 @@ impl Component for PdmWizardPageConnect {
});
Self {
- connect_info: None,
_form_observer,
- form_valid: false,
loading: false,
- last_error: None,
- async_pool: AsyncPool::new(),
+ scan_result: None,
+ scan_guard: None,
}
}
@@ -135,47 +107,44 @@ impl Component for PdmWizardPageConnect {
let props = ctx.props();
match msg {
Msg::FormChange => {
- self.form_valid = props.info.form_ctx.read().is_valid();
- match props.remote_type {
- RemoteType::Pve => {
- self.update_connect_info(ctx, None);
- }
- RemoteType::Pbs => {
- return <Self as yew::Component>::update(self, ctx, Msg::Connect)
- }
- }
- props.info.page_lock(!self.form_valid);
+ props.info.page_lock(!props.info.form_ctx.read().is_valid());
+ props.info.reset_remaining_valid_pages();
+ self.scan_result = None;
}
Msg::Connect => {
- let link = ctx.link().clone();
- self.update_connect_info(ctx, None);
- let form_ctx = props.info.form_ctx.clone();
self.loading = true;
- self.last_error = None;
+ props.info.page_lock(true);
+
+ self.scan_guard = Some(AsyncAbortGuard::spawn({
+ let link = ctx.link().clone();
+ let form_ctx = props.info.form_ctx.clone();
+ let remote_type = props.remote_type;
- let remote_type = props.remote_type;
- self.async_pool.spawn(async move {
- let result = connect(form_ctx, remote_type).await;
- link.send_message(Msg::ConnectResult(result));
- });
+ async move {
+ let result = connect(form_ctx, remote_type).await;
+ link.send_message(Msg::ConnectResult(result));
+ }
+ }));
}
- Msg::ConnectResult(server_info) => {
+ Msg::ConnectResult(scan_result) => {
self.loading = false;
- match server_info {
- Ok(connect_info) => {
- self.update_connect_info(ctx, Some(connect_info));
- }
- Err(err) => {
- self.last_error = Some(err);
+ props.info.page_lock(false);
+ self.scan_result = Some(scan_result);
+ match &self.scan_result {
+ Some(Ok(ScanResult::TlsResult(None))) => {
+ call_on_connect_change(props);
+ for page in ["nodes", "info"] {
+ if let Some(form_ctx) = props.info.lookup_form_context(&Key::from(page))
+ {
+ form_ctx.write().reset_form();
+ }
+ }
+ self.scan_result = None;
+ props.info.reset_remaining_valid_pages();
+ props.info.go_to_next_page();
}
- }
-
- if let Some(form_ctx) = props.info.lookup_form_context(&Key::from("nodes")) {
- form_ctx.write().reset_form();
- }
- props.info.reset_remaining_valid_pages();
- if self.connect_info.is_some() {
- props.info.go_to_next_page();
+ Some(Err(_)) => props.info.page_lock(true),
+ _ => {}
}
}
}
@@ -183,7 +152,10 @@ impl Component for PdmWizardPageConnect {
}
fn view(&self, _ctx: &Context<Self>) -> Html {
- let error = self.last_error.as_ref();
+ let error = match &self.scan_result {
+ Some(Err(err)) => Some(err),
+ _ => None,
+ };
let input_panel = InputPanel::new()
.class(FlexFit)
// FIXME: input panel css style is not optimal here...
@@ -215,6 +187,26 @@ impl Component for PdmWizardPageConnect {
}
}
+fn get_fingerprint(form_ctx: &FormContext) -> Option<String> {
+ let fingerprint = form_ctx.read().get_field_text("fingerprint");
+ let fingerprint = if fingerprint.is_empty() {
+ None
+ } else {
+ Some(fingerprint)
+ };
+ fingerprint
+}
+
+fn call_on_connect_change(props: &WizardPageConnect) {
+ if let Some(on_connect_change) = &props.on_connect_change {
+ let fingerprint = get_fingerprint(&props.info.form_ctx);
+ on_connect_change.emit(Some(ConnectParams {
+ hostname: normalize_hostname(props.info.form_ctx.read().get_field_text("hostname")),
+ fingerprint,
+ }));
+ }
+}
+
fn normalize_hostname(hostname: String) -> String {
let mut result = hostname;
if let Some(hostname) = result.strip_prefix("http://") {
--
2.39.5
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 33+ messages in thread
* [pdm-devel] [PATCH datacenter-manager 21/21] ui: pve wizard: connect: add certificate confirmation dialog
2025-05-16 13:35 [pdm-devel] [PATCH datacenter-manager 00/21] improve remote wizard Dominik Csapak
` (19 preceding siblings ...)
2025-05-16 13:36 ` [pdm-devel] [PATCH datacenter-manager 20/21] ui: pve wizard: connect: use scan api endpoint instead of realms Dominik Csapak
@ 2025-05-16 13:36 ` Dominik Csapak
2025-08-18 12:41 ` [pdm-devel] [PATCH datacenter-manager 00/21] improve remote wizard Lukas Wagner
` (2 subsequent siblings)
23 siblings, 0 replies; 33+ messages in thread
From: Dominik Csapak @ 2025-05-16 13:36 UTC (permalink / raw)
To: pdm-devel
In case the user did not enter a fingerprint, but the certificate is
untrusted, show them a dialog with the certificate information and allow
them to progress by confirming it.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
ui/src/remotes/wizard_page_connect.rs | 141 +++++++++++++++++++++-----
1 file changed, 116 insertions(+), 25 deletions(-)
diff --git a/ui/src/remotes/wizard_page_connect.rs b/ui/src/remotes/wizard_page_connect.rs
index 9f73779..ee1ef1b 100644
--- a/ui/src/remotes/wizard_page_connect.rs
+++ b/ui/src/remotes/wizard_page_connect.rs
@@ -2,19 +2,21 @@ use std::rc::Rc;
use anyhow::{bail, Error};
use serde::{Deserialize, Serialize};
+use serde_json::Value;
use yew::html::IntoEventCallback;
use yew::virtual_dom::{Key, VComp, VNode};
-use pwt::css::FlexFit;
+use pwt::css::{FlexFit, JustifyContent};
use pwt::widget::form::{Field, FormContext, FormContextObserver};
-use pwt::widget::{error_message, Column, InputPanel, Mask};
+use pwt::widget::{error_message, Button, Column, Container, Dialog, InputPanel, Mask, Row};
use pwt::{prelude::*, AsyncAbortGuard};
use pwt_macros::builder;
-use proxmox_yew_comp::{SchemaValidation, WizardPageRenderInfo};
+use proxmox_yew_comp::{KVGrid, KVGridRow, SchemaValidation, WizardPageRenderInfo};
use pdm_api_types::remotes::{RemoteType, ScanResult};
use pdm_api_types::CERT_FINGERPRINT_SHA256_SCHEMA;
+use proxmox_acme_api::CertificateInfo;
#[derive(Clone, PartialEq, Properties)]
#[builder]
@@ -47,16 +49,10 @@ async fn connect(form_ctx: FormContext, remote_type: RemoteType) -> Result<ScanR
RemoteType::Pve => {
let hostname = normalize_hostname(form_ctx.read().get_field_text("hostname"));
let fingerprint = get_fingerprint(&form_ctx);
- let res = crate::pdm_client()
+ crate::pdm_client()
.pve_probe_tls(&hostname, fingerprint.as_deref())
.await
- .map_err(Error::from);
-
- if let Ok(ScanResult::TlsResult(Some(_))) = &res {
- bail!("Untrusted Certificate, please enter fingerprint");
- }
-
- res
+ .map_err(Error::from)
}
RemoteType::Pbs => bail!("not implemented"),
}
@@ -64,6 +60,7 @@ async fn connect(form_ctx: FormContext, remote_type: RemoteType) -> Result<ScanR
pub enum Msg {
FormChange,
+ ConfirmResult(bool), // accept or dismiss
Connect,
ConnectResult(Result<ScanResult, Error>),
}
@@ -72,6 +69,58 @@ pub struct PdmWizardPageConnect {
loading: bool,
scan_result: Option<Result<ScanResult, Error>>,
scan_guard: Option<AsyncAbortGuard>,
+ rows: Rc<Vec<KVGridRow>>,
+}
+
+impl PdmWizardPageConnect {
+ fn create_certificate_confirmation_dialog(&self, ctx: &Context<Self>) -> Option<Dialog> {
+ let link = ctx.link();
+ let certificate = match &self.scan_result {
+ Some(Ok(ScanResult::TlsResult(Some(info)))) => info.clone(),
+ _ => return None,
+ };
+ Some(
+ Dialog::new(tr!("Connection Certificate"))
+ .on_close(link.callback(|_| Msg::ConfirmResult(false)))
+ .with_child(
+ Column::new()
+ .padding(2)
+ .gap(2)
+ .class(FlexFit)
+ .with_child(Container::new().with_child(tr!(
+ "The certificate of the remote server is not trusted."
+ )))
+ .with_child(
+ Container::new().with_child(tr!(
+ "Do you want to trust it by saving it's fingerprint?"
+ )),
+ )
+ .with_child(
+ KVGrid::new()
+ .class(FlexFit)
+ .borderless(true)
+ .striped(false)
+ .rows(self.rows.clone())
+ .data(Rc::new(
+ serde_json::to_value(certificate).unwrap_or_default(),
+ )),
+ )
+ .with_child(
+ Row::new()
+ .gap(2)
+ .class(JustifyContent::Center)
+ .with_child(
+ Button::new(tr!("Yes"))
+ .onclick(link.callback(|_| Msg::ConfirmResult(true))),
+ )
+ .with_child(
+ Button::new(tr!("No"))
+ .onclick(link.callback(|_| Msg::ConfirmResult(false))),
+ ),
+ ),
+ ),
+ )
+ }
}
impl Component for PdmWizardPageConnect {
@@ -100,6 +149,7 @@ impl Component for PdmWizardPageConnect {
loading: false,
scan_result: None,
scan_guard: None,
+ rows: Rc::new(rows()),
}
}
@@ -132,26 +182,34 @@ impl Component for PdmWizardPageConnect {
self.scan_result = Some(scan_result);
match &self.scan_result {
Some(Ok(ScanResult::TlsResult(None))) => {
- call_on_connect_change(props);
- for page in ["nodes", "info"] {
- if let Some(form_ctx) = props.info.lookup_form_context(&Key::from(page))
- {
- form_ctx.write().reset_form();
- }
- }
- self.scan_result = None;
- props.info.reset_remaining_valid_pages();
- props.info.go_to_next_page();
+ return <Self as Component>::update(self, ctx, Msg::ConfirmResult(true));
}
Some(Err(_)) => props.info.page_lock(true),
_ => {}
}
}
+ Msg::ConfirmResult(confirm) => {
+ if !confirm {
+ self.scan_result = None;
+ return true;
+ }
+ if let Some(Ok(ScanResult::TlsResult(connection))) = &self.scan_result {
+ call_on_connect_change(props, connection.clone());
+ for page in ["nodes", "info"] {
+ if let Some(form_ctx) = props.info.lookup_form_context(&Key::from(page)) {
+ form_ctx.write().reset_form();
+ }
+ }
+ self.scan_result = None;
+ props.info.reset_remaining_valid_pages();
+ props.info.go_to_next_page();
+ }
+ }
}
true
}
- fn view(&self, _ctx: &Context<Self>) -> Html {
+ fn view(&self, ctx: &Context<Self>) -> Html {
let error = match &self.scan_result {
Some(Err(err)) => Some(err),
_ => None,
@@ -178,7 +236,8 @@ impl Component for PdmWizardPageConnect {
let content = Column::new()
.class(FlexFit)
.with_child(input_panel)
- .with_optional_child(error.map(|err| error_message(&err.to_string())));
+ .with_optional_child(error.map(|err| error_message(&err.to_string())))
+ .with_optional_child(self.create_certificate_confirmation_dialog(ctx));
Mask::new(content)
.class(FlexFit)
@@ -197,12 +256,14 @@ fn get_fingerprint(form_ctx: &FormContext) -> Option<String> {
fingerprint
}
-fn call_on_connect_change(props: &WizardPageConnect) {
+fn call_on_connect_change(props: &WizardPageConnect, certificate_info: Option<CertificateInfo>) {
if let Some(on_connect_change) = &props.on_connect_change {
let fingerprint = get_fingerprint(&props.info.form_ctx);
on_connect_change.emit(Some(ConnectParams {
hostname: normalize_hostname(props.info.form_ctx.read().get_field_text("hostname")),
- fingerprint,
+ fingerprint: certificate_info
+ .and_then(|cert| cert.fingerprint)
+ .or(fingerprint),
}));
}
}
@@ -221,6 +282,36 @@ fn normalize_hostname(hostname: String) -> String {
result
}
+fn rows() -> Vec<KVGridRow> {
+ let render_date = |_name: &str, value: &Value, _record: &Value| -> Html {
+ match value.as_i64() {
+ Some(value) => html! {proxmox_yew_comp::utils::render_epoch(value)},
+ None => html! {value.to_string()},
+ }
+ };
+ let value = vec![
+ KVGridRow::new("fingerprint", tr!("Fingerprint")),
+ KVGridRow::new("issuer", tr!("Issuer")),
+ KVGridRow::new("subject", tr!("Subject")),
+ KVGridRow::new("public-key-type", tr!("Public Key Alogrithm")),
+ KVGridRow::new("public-key-bits", tr!("Public Key Size")),
+ KVGridRow::new("notbefore", tr!("Valid Since")).renderer(render_date),
+ KVGridRow::new("notafter", tr!("Expires")).renderer(render_date),
+ KVGridRow::new("san", tr!("Subject Alternative Names")).renderer(
+ |_name, value, _record| {
+ let list: Result<Vec<String>, _> = serde_json::from_value(value.clone());
+ match list {
+ Ok(value) => {
+ html! {<pre>{&value.join("\n")}</pre>}
+ }
+ _ => html! {value.to_string()},
+ }
+ },
+ ),
+ ];
+ value
+}
+
impl Into<VNode> for WizardPageConnect {
fn into(self) -> VNode {
let comp = VComp::new::<PdmWizardPageConnect>(Rc::new(self), None);
--
2.39.5
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager 00/21] improve remote wizard
2025-05-16 13:35 [pdm-devel] [PATCH datacenter-manager 00/21] improve remote wizard Dominik Csapak
` (20 preceding siblings ...)
2025-05-16 13:36 ` [pdm-devel] [PATCH datacenter-manager 21/21] ui: pve wizard: connect: add certificate confirmation dialog Dominik Csapak
@ 2025-08-18 12:41 ` Lukas Wagner
2025-08-18 12:53 ` Dominik Csapak
2025-08-18 13:48 ` [pdm-devel] superseded: " Dominik Csapak
2025-08-19 12:10 ` [pdm-devel] " Lukas Wagner
23 siblings, 1 reply; 33+ messages in thread
From: Lukas Wagner @ 2025-08-18 12:41 UTC (permalink / raw)
To: Proxmox Datacenter Manager development discussion, Dominik Csapak
On Fri May 16, 2025 at 3:35 PM CEST, Dominik Csapak wrote:
> # Summary
>
> This series improves the remote wizard in various points:
> * improved wording and texts
> * moved seperate buttons on pages into the next button
> * probing of entered nodes
> * confirmation dialog for missing fingerprints
> * better realm selector
>
> # Notes
>
> I'm not super sure about the API call structure. I think two seperate
> API calls (one for tls probing, one for scanning) might make more sense.
> If that's wanted, I'll do so in a v2 of the series.
>
> ## Probing on the server side
>
> I tried to determine if we need a fingerprint for nodes inside the
> API call, by probing each node. Since we currently only get the
> nodenames and not FQDNs in the nodelist, this will currently not
> result in a valid connection in most cases and return the fingerprint.
>
> My plan would be to include FQDNs of the nodes on the PVE side API call,
> so we can return here a list of nodename, ip and FQDNs, which the user
> then can select from. (which of those I'd probe on first check is yet
> to be determined)
>
> ## Fingerprint confirmation dialogs
>
> Not sure if we want to be able to let the user confirm the fingerprint
> so easily. On one hand it's very convenient, but maybe leads to users
> simply clicking yes without understanding what's happening.
>
> If it's deemed too dangerous, I'd rework the series without this.
>
> # Future work
>
> The next step for the wizard is to have some kind of quick copy&paste
> info. After discussing off-list with Fabian a bit, I think it would be
> best for this to contain the hostname (FQDN?) + fingerprint (if just a
> self-signed certificate) + a list of nodes with their respective
> nodename + FQDNs (maybe requires api change on PVE side to generate
> this). The user would then still have to do most of the steps currently
> necessary in the wizard, except the manual copy & pasting of
> fingerprints and maybe entering of FQDNs.
>
>
Seems like these do not apply cleanly any more, could you rebase and
send a v2? Thanks!
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager 00/21] improve remote wizard
2025-08-18 12:41 ` [pdm-devel] [PATCH datacenter-manager 00/21] improve remote wizard Lukas Wagner
@ 2025-08-18 12:53 ` Dominik Csapak
0 siblings, 0 replies; 33+ messages in thread
From: Dominik Csapak @ 2025-08-18 12:53 UTC (permalink / raw)
To: Lukas Wagner, Proxmox Datacenter Manager development discussion
On 8/18/25 14:41, Lukas Wagner wrote:
> On Fri May 16, 2025 at 3:35 PM CEST, Dominik Csapak wrote:
>> # Summary
>>
>> This series improves the remote wizard in various points:
>> * improved wording and texts
>> * moved seperate buttons on pages into the next button
>> * probing of entered nodes
>> * confirmation dialog for missing fingerprints
>> * better realm selector
>>
>> # Notes
>>
>> I'm not super sure about the API call structure. I think two seperate
>> API calls (one for tls probing, one for scanning) might make more sense.
>> If that's wanted, I'll do so in a v2 of the series.
>>
>> ## Probing on the server side
>>
>> I tried to determine if we need a fingerprint for nodes inside the
>> API call, by probing each node. Since we currently only get the
>> nodenames and not FQDNs in the nodelist, this will currently not
>> result in a valid connection in most cases and return the fingerprint.
>>
>> My plan would be to include FQDNs of the nodes on the PVE side API call,
>> so we can return here a list of nodename, ip and FQDNs, which the user
>> then can select from. (which of those I'd probe on first check is yet
>> to be determined)
>>
>> ## Fingerprint confirmation dialogs
>>
>> Not sure if we want to be able to let the user confirm the fingerprint
>> so easily. On one hand it's very convenient, but maybe leads to users
>> simply clicking yes without understanding what's happening.
>>
>> If it's deemed too dangerous, I'd rework the series without this.
>>
>> # Future work
>>
>> The next step for the wizard is to have some kind of quick copy&paste
>> info. After discussing off-list with Fabian a bit, I think it would be
>> best for this to contain the hostname (FQDN?) + fingerprint (if just a
>> self-signed certificate) + a list of nodes with their respective
>> nodename + FQDNs (maybe requires api change on PVE side to generate
>> this). The user would then still have to do most of the steps currently
>> necessary in the wizard, except the manual copy & pasting of
>> fingerprints and maybe entering of FQDNs.
>>
>>
>
> Seems like these do not apply cleanly any more, could you rebase and
> send a v2? Thanks!
>
hi, yep sure, i'm currently working on rebasing all my open series
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 33+ messages in thread
* [pdm-devel] superseded: [PATCH datacenter-manager 00/21] improve remote wizard
2025-05-16 13:35 [pdm-devel] [PATCH datacenter-manager 00/21] improve remote wizard Dominik Csapak
` (21 preceding siblings ...)
2025-08-18 12:41 ` [pdm-devel] [PATCH datacenter-manager 00/21] improve remote wizard Lukas Wagner
@ 2025-08-18 13:48 ` Dominik Csapak
2025-08-19 12:10 ` [pdm-devel] " Lukas Wagner
23 siblings, 0 replies; 33+ messages in thread
From: Dominik Csapak @ 2025-08-18 13:48 UTC (permalink / raw)
To: pdm-devel
superseded by v2:
https://lore.proxmox.com/pdm-devel/20250818133044.2816336-1-d.csapak@proxmox.com/
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager 03/21] server: connection: add probe_tls_connection helper
2025-05-16 13:35 ` [pdm-devel] [PATCH datacenter-manager 03/21] server: connection: add probe_tls_connection helper Dominik Csapak
@ 2025-08-19 11:54 ` Lukas Wagner
0 siblings, 0 replies; 33+ messages in thread
From: Lukas Wagner @ 2025-08-19 11:54 UTC (permalink / raw)
To: Proxmox Datacenter Manager development discussion, Dominik Csapak
Hey,
looks mostly good, two suggestions inline.
On Fri May 16, 2025 at 3:35 PM CEST, Dominik Csapak wrote:
> this is intended to help us probe a remote/host before using it to check
> whether the tls connection is working fine, or it returns the
> certificate information so we can show it to the user.
>
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> server/src/connection.rs | 79 ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 79 insertions(+)
>
> diff --git a/server/src/connection.rs b/server/src/connection.rs
> index 0be9033..c7b2558 100644
> --- a/server/src/connection.rs
> +++ b/server/src/connection.rs
> @@ -15,8 +15,10 @@ use std::time::{Duration, SystemTime};
> use anyhow::{bail, format_err, Error};
> use http::uri::Authority;
> use http::Method;
> +use openssl::x509::X509StoreContextRef;
> use serde::Serialize;
>
> +use proxmox_acme_api::CertificateInfo;
> use proxmox_client::{Client, HttpApiClient, HttpApiResponse, HttpApiResponseStream, TlsOptions};
>
> use pdm_api_types::remotes::{NodeUrl, Remote, RemoteType};
> @@ -799,3 +801,80 @@ impl HttpApiClient for MultiClient {
> try_request! { self, method, path_and_query, params, streaming_request }
> }
> }
> +
> +/// Checks TLS connection to the given remote
> +///
> +/// Returns `Ok(None)` if connecting with the given parameters works
> +/// Returns `Ok(Some(cert))` if no fingerprint was given and some certificate could not be validated
> +/// Returns `Err(err)` if some other error occurred
The intent could be maybe a bit more clear if you used some kind of enum
instead of the Option:
e.g.
enum ProbeOutcome {
Success,
UntrustedCertificate(CertificateInfo),
}
(Maybe there's a better name than 'Success' for the first variant, this
is just what came to mind right now).
> +///
> +/// # Example
> +///
> +/// ```
> +/// use server::connection::probe_tls_connection;
> +/// use pdm_api_types::remotes::RemoteType;
> +///
> +/// # async fn function() {
> +/// let result = probe_tls_connection(RemoteType::Pve, "192.168.2.100".to_string(), None).await;
> +/// match result {
> +/// Ok(None) => { /* everything ok */ },
> +/// Ok(Some(cert)) => { /* do something with cert */ },
> +/// Err(err) => { /* do something with error */ },
> +/// }
> +/// # }
> +/// ```
> +pub async fn probe_tls_connection(
> + remote_type: RemoteType,
> + hostname: String,
> + fingerprint: Option<String>,
> +) -> Result<Option<CertificateInfo>, Error> {
> + let host_port: Authority = hostname.parse()?;
> +
> + let uri: http::uri::Uri = format!(
> + "https://{}:{}",
> + host_port.host(),
> + host_port.port_u16().unwrap_or(remote_type.default_port())
> + )
> + .parse()?;
> +
> + // to save the invalid cert we find
> + let invalid_cert = Arc::new(StdMutex::new(None));
> +
> + let options = if let Some(fp) = &fingerprint {
> + TlsOptions::parse_fingerprint(fp)?
> + } else {
> + TlsOptions::Callback(Box::new({
> + let invalid_cert = invalid_cert.clone();
> + move |valid: bool, chain: &mut X509StoreContextRef| {
> + if let Some(cert) = chain.current_cert() {
> + if !valid {
> + let cert = cert.to_pem().map(|pem| CertificateInfo::from_pem("", &pem));
> + *invalid_cert.lock().unwrap() = Some(cert);
> + }
> + }
> + true
> + }
> + }))
> + };
> + let client = proxmox_client::Client::with_options(uri, options, Default::default())?;
> +
> + // set fake auth info. we don't need any, but the proxmox client will return unauthenticated if
> + // none is set.
> + client.set_authentication(proxmox_client::Token {
> + userid: "".to_string(),
> + value: "".to_string(),
> + prefix: "".to_string(),
> + perl_compat: false,
> + });
> +
> + client.request(Method::GET, "/", None::<()>).await?;
> +
> + let cert = invalid_cert.lock().unwrap().take();
> + if let Some(cert) = cert {
> + let cert = cert?;
> + let cert = cert?;
I think doing a
let cert = cert??;
or
Ok(Some(cert??))
is better here. Otherwise it reads a bit like a copy-paste mistake.
> + Ok(Some(cert))
> + } else {
> + Ok(None)
> + }
> +}
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager 04/21] server/ui: pve api: extend 'scan' so it can probe the tls connection
2025-05-16 13:35 ` [pdm-devel] [PATCH datacenter-manager 04/21] server/ui: pve api: extend 'scan' so it can probe the tls connection Dominik Csapak
@ 2025-08-19 11:54 ` Lukas Wagner
0 siblings, 0 replies; 33+ messages in thread
From: Lukas Wagner @ 2025-08-19 11:54 UTC (permalink / raw)
To: Proxmox Datacenter Manager development discussion, Dominik Csapak
Hi,
some notes inline.
On Fri May 16, 2025 at 3:35 PM CEST, Dominik Csapak wrote:
> Makes the `authid` and `token` parameters optional. If they're omitted,
> opens a basic connection to probe the TLS state. If no fingerprint was
> given and the certificate is not trusted, the certificate information is
> returned (so it can be shown to the user)
>
> Adapt the UI, so it can cope with that change.
>
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> lib/pdm-api-types/Cargo.toml | 1 +
> lib/pdm-api-types/src/lib.rs | 2 +
> lib/pdm-api-types/src/remotes.rs | 7 ++++
> server/src/api/pve/mod.rs | 60 ++++++++++++++++++++----------
> ui/src/remotes/wizard_page_info.rs | 9 +++--
> 5 files changed, 57 insertions(+), 22 deletions(-)
>
> diff --git a/lib/pdm-api-types/Cargo.toml b/lib/pdm-api-types/Cargo.toml
> index 6575c03..fbaf5b3 100644
> --- a/lib/pdm-api-types/Cargo.toml
> +++ b/lib/pdm-api-types/Cargo.toml
> @@ -13,6 +13,7 @@ regex.workspace = true
> serde.workspace = true
> serde_plain.workspace = true
>
> +proxmox-acme-api = { workspace = true, features = [] }
> proxmox-auth-api = { workspace = true, features = ["api-types"] }
> proxmox-lang.workspace = true
> proxmox-config-digest.workspace = true
> diff --git a/lib/pdm-api-types/src/lib.rs b/lib/pdm-api-types/src/lib.rs
> index 3844907..31420a9 100644
> --- a/lib/pdm-api-types/src/lib.rs
> +++ b/lib/pdm-api-types/src/lib.rs
> @@ -79,6 +79,8 @@ pub use proxmox_dns_api::THIRD_DNS_SERVER_SCHEMA;
> pub use proxmox_config_digest::ConfigDigest;
> pub use proxmox_config_digest::PROXMOX_CONFIG_DIGEST_SCHEMA;
>
> +pub use proxmox_acme_api::CertificateInfo;
> +
> #[macro_use]
> mod user;
> pub use user::*;
> diff --git a/lib/pdm-api-types/src/remotes.rs b/lib/pdm-api-types/src/remotes.rs
> index dca2fa0..fd20327 100644
> --- a/lib/pdm-api-types/src/remotes.rs
> +++ b/lib/pdm-api-types/src/remotes.rs
> @@ -179,3 +179,10 @@ mod serde_option_uri {
> }
> }
> }
> +
> +#[allow(clippy::large_enum_variant)]
> +#[derive(Clone, PartialEq, Deserialize, Serialize)]
> +pub enum ScanResult {
> + TlsResult(Option<proxmox_acme_api::CertificateInfo>),
> + Remote(Remote),
> +}
These enum variant names are a bit hard to understand ("what does
TlsResult mean?"). Maybe something like
pub enum ScanResult {
UntrustedCertificate(CertInfo),
TrustedCertificate,
ScanFinished(Remote)
}
(the last one would be left out if you implement my other suggestion
below)
Also some doc-comments would be nice!
> diff --git a/server/src/api/pve/mod.rs b/server/src/api/pve/mod.rs
> index 1a3a725..810a38e 100644
> --- a/server/src/api/pve/mod.rs
> +++ b/server/src/api/pve/mod.rs
> @@ -13,7 +13,7 @@ use proxmox_schema::property_string::PropertyString;
> use proxmox_section_config::typed::SectionConfigData;
> use proxmox_sortable_macro::sortable;
>
> -use pdm_api_types::remotes::{NodeUrl, Remote, RemoteType, REMOTE_ID_SCHEMA};
> +use pdm_api_types::remotes::{NodeUrl, Remote, RemoteType, ScanResult, REMOTE_ID_SCHEMA};
> use pdm_api_types::resource::PveResource;
> use pdm_api_types::{
> Authid, RemoteUpid, HOST_OPTIONAL_PORT_FORMAT, PRIV_RESOURCE_AUDIT, PRIV_RESOURCE_DELETE,
> @@ -27,8 +27,8 @@ use pve_api_types::{ClusterResourceKind, ClusterResourceType};
>
> use super::resources::{map_pve_lxc, map_pve_node, map_pve_qemu, map_pve_storage};
>
> -use crate::connection;
> use crate::connection::PveClient;
> +use crate::connection::{self, probe_tls_connection};
> use crate::remote_tasks;
>
> mod lxc;
> @@ -314,9 +314,11 @@ fn check_guest_delete_perms(
> },
> "authid": {
> type: Authid,
> + optional: true,
> },
> "token": {
> type: String,
> + optional: true,
> description: "The token secret or the user password.",
> },
> },
> @@ -326,13 +328,28 @@ fn check_guest_delete_perms(
> &Permission::Privilege(&["/"], PRIV_SYS_MODIFY, false),
> },
> )]
> -/// Scans the given connection info for pve cluster information
> +/// Scans the given connection info for tls or pve cluster information
> +///
> +/// Returns the result for tls connection if only hostname (and optionally fingerprint) is given.
> +/// If authid and token are also provided, returns pve cluster information.
> +///
> +/// For each node that is returned, the TLS connection is probed, to check if using
> +/// a fingerprint is necessary.
> pub async fn scan_remote_pve(
> hostname: String,
> fingerprint: Option<String>,
> - authid: Authid,
> - token: String,
> -) -> Result<Remote, Error> {
> + authid: Option<Authid>,
> + token: Option<String>,
> +) -> Result<ScanResult, Error> {
> + let (authid, token) = match (authid, token) {
> + (Some(authid), Some(token)) => (authid, token),
> + _ => {
> + let res = probe_tls_connection(RemoteType::Pve, hostname.clone(), fingerprint.clone())
> + .await?;
> + return Ok(ScanResult::TlsResult(res));
> + }
> + };
> +
> let mut remote = Remote {
> ty: RemoteType::Pve,
> id: String::new(),
> @@ -349,18 +366,23 @@ pub async fn scan_remote_pve(
> .await
> .map_err(|err| format_err!("could not login: {err}"))?;
>
> - let nodes: Vec<_> = client
> - .list_nodes()
> - .await?
> - .into_iter()
> - .map(|node| {
> - let url = NodeUrl {
> - hostname: node.node,
> - fingerprint: node.ssl_fingerprint,
> - };
> - PropertyString::new(url)
> - })
> - .collect();
> + let mut nodes = Vec::new();
> +
> + for node in client.list_nodes().await? {
> + // probe without fingerprint to see if the certificate is trusted
> + // TODO: how can we get the fqdn here?, otherwise it'll fail in most scenarios...
> + let fingerprint = match probe_tls_connection(RemoteType::Pve, node.node.clone(), None).await
> + {
> + Ok(Some(cert)) => cert.fingerprint,
> + Ok(None) => None,
> + Err(_) => node.ssl_fingerprint,
> + };
> +
> + nodes.push(PropertyString::new(NodeUrl {
> + hostname: node.node,
> + fingerprint,
> + }));
> + }
>
> if nodes.is_empty() {
> bail!("no node list returned");
> @@ -383,7 +405,7 @@ pub async fn scan_remote_pve(
> .unwrap_or_default();
> }
>
> - Ok(remote)
> + Ok(ScanResult::Remote(remote))
> }
This function feels a bit off to me. Would it maybe be cleaner to have TLS
probing and 'gathering cluster/node info' as two distinct steps that are
just done one after the other by the Web UI?
Sure, the probing is not needed if the certificate is in fact trusted, but
doing a second API call does not really hurt in this case, or does it?
>
> #[api(
> diff --git a/ui/src/remotes/wizard_page_info.rs b/ui/src/remotes/wizard_page_info.rs
> index b0a5ba2..9953f42 100644
> --- a/ui/src/remotes/wizard_page_info.rs
> +++ b/ui/src/remotes/wizard_page_info.rs
> @@ -1,6 +1,6 @@
> use std::rc::Rc;
>
> -use anyhow::Error;
> +use anyhow::{bail, Error};
> use html::IntoEventCallback;
> use proxmox_schema::property_string::PropertyString;
> use serde::{Deserialize, Serialize};
> @@ -18,7 +18,7 @@ use pwt::{
> AsyncPool,
> };
>
> -use pdm_api_types::remotes::{NodeUrl, Remote};
> +use pdm_api_types::remotes::{NodeUrl, Remote, ScanResult};
>
> use pwt_macros::builder;
>
> @@ -97,7 +97,10 @@ async fn scan(connection_params: ConnectParams, form_ctx: FormContext) -> Result
> let data: ScanParams = serde_json::from_value(data.clone())?;
>
> let params = serde_json::to_value(&data)?;
> - let mut result: Remote = proxmox_yew_comp::http_post("/pve/scan", Some(params)).await?;
> + let mut result = match proxmox_yew_comp::http_post("/pve/scan", Some(params)).await? {
> + ScanResult::TlsResult(_) => bail!("Untrusted certificate or invalid fingerprint"),
> + ScanResult::Remote(remote) => remote,
> + };
> result.nodes.insert(
> 0,
> PropertyString::new(NodeUrl {
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager 05/21] pdm-client: add scan_remote and probe_tls methods
2025-05-16 13:35 ` [pdm-devel] [PATCH datacenter-manager 05/21] pdm-client: add scan_remote and probe_tls methods Dominik Csapak
@ 2025-08-19 11:55 ` Lukas Wagner
0 siblings, 0 replies; 33+ messages in thread
From: Lukas Wagner @ 2025-08-19 11:55 UTC (permalink / raw)
To: Proxmox Datacenter Manager development discussion, Dominik Csapak
On Fri May 16, 2025 at 3:35 PM CEST, Dominik Csapak wrote:
> so we can use that in the ui instead of making the api calls manually
> via the path.
>
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> lib/pdm-client/src/lib.rs | 45 +++++++++++++++++++++++++++++++++++++++
> 1 file changed, 45 insertions(+)
>
> diff --git a/lib/pdm-client/src/lib.rs b/lib/pdm-client/src/lib.rs
> index 61a8ebd..d656ab1 100644
> --- a/lib/pdm-client/src/lib.rs
> +++ b/lib/pdm-client/src/lib.rs
> @@ -3,6 +3,7 @@
> use std::collections::HashMap;
> use std::time::Duration;
>
> +use pdm_api_types::remotes::ScanResult;
> use pdm_api_types::resource::{PveResource, RemoteResources, TopEntities};
> use pdm_api_types::rrddata::{
> LxcDataPoint, NodeDataPoint, PbsDatastoreDataPoint, PbsNodeDataPoint, QemuDataPoint,
> @@ -865,6 +866,50 @@ impl<T: HttpApiClient> PdmClient<T> {
> .build();
> Ok(self.0.get(&path).await?.expect_json()?.data)
> }
> +
> + /// uses /pve/scan to probe the tls connection to the given host
> + pub async fn pve_probe_tls(
> + &self,
> + hostname: &str,
> + fingerprint: Option<&str>,
> + ) -> Result<ScanResult, Error> {
> + let mut params = json!({
> + "hostname": hostname,
> + });
> + if let Some(fp) = fingerprint {
> + params["fingerprint"] = fp.into();
> + }
> + Ok(self
> + .0
> + .post("/api2/extjs/pve/scan", ¶ms)
> + .await?
> + .expect_json()?
> + .data)
> + }
> +
> + /// Uses /pve/scan to scan the remote cluster for node/fingerprint information
> + pub async fn pve_scan_remote(
> + &self,
> + hostname: &str,
> + fingerprint: Option<&str>,
> + authid: &str,
> + token: &str,
> + ) -> Result<ScanResult, Error> {
> + let mut params = json!({
> + "hostname": hostname,
> + "authid": authid,
> + "token": token,
> + });
> + if let Some(fp) = fingerprint {
> + params["fingerprint"] = fp.into();
> + }
> + Ok(self
> + .0
> + .post("/api2/extjs/pve/scan", ¶ms)
> + .await?
> + .expect_json()?
> + .data)
> + }
> }
Yep, this also confirms my opinion that the /scan endpoint is a bit odd
right now and should better be split up :)
>
> /// Builder for migration parameters.
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager 14/21] ui: pve wizard: info: detect hostname and fingerprint
2025-05-16 13:36 ` [pdm-devel] [PATCH datacenter-manager 14/21] ui: pve wizard: info: detect hostname and fingerprint Dominik Csapak
@ 2025-08-19 11:55 ` Lukas Wagner
0 siblings, 0 replies; 33+ messages in thread
From: Lukas Wagner @ 2025-08-19 11:55 UTC (permalink / raw)
To: Proxmox Datacenter Manager development discussion, Dominik Csapak
On Fri May 16, 2025 at 3:36 PM CEST, Dominik Csapak wrote:
> Instead of always inserting the initial host into the nodelist
> (which will always be duplicate since it will be also contained in
> the node list), try to match the hostname and or fingerprint with
> any of the given node, so we can deduplicate that.
>
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> ui/src/remotes/wizard_page_info.rs | 35 ++++++++++++++++++++++++------
> 1 file changed, 28 insertions(+), 7 deletions(-)
>
> diff --git a/ui/src/remotes/wizard_page_info.rs b/ui/src/remotes/wizard_page_info.rs
> index 882eebd..04b0004 100644
> --- a/ui/src/remotes/wizard_page_info.rs
> +++ b/ui/src/remotes/wizard_page_info.rs
> @@ -108,13 +108,34 @@ async fn scan(connection_params: ConnectParams, form_ctx: FormContext) -> Result
> ScanResult::TlsResult(_) => bail!("Untrusted certificate or invalid fingerprint"),
> ScanResult::Remote(remote) => remote,
> };
> - result.nodes.insert(
> - 0,
> - PropertyString::new(NodeUrl {
> - hostname,
> - fingerprint,
> - }),
> - );
> +
> + // try to detect inserted hostname/fingerprint
IMO the comment does not really help with understanding what the code
below does. Maybe elaborate a bit here (e.g. similar to the commit
message, that explains it quite well)
> + let mut found = false;
`found` what? (e.g maybe call it `found_probed_node` or sth :) )
> + for node in result.nodes.iter_mut() {
> + if node.hostname == hostname {
> + if fingerprint.is_none() {
> + node.fingerprint = None;
> + }
> + found = true;
> + continue;
> + }
> + if node.fingerprint.as_ref().map(|fp| fp.to_uppercase())
> + == fingerprint.as_ref().map(|fp| fp.to_uppercase())
> + {
> + found = true;
> + node.hostname = hostname.clone();
> + continue;
> + }
> + }
> + if !found {
> + result.nodes.insert(
> + 0,
> + PropertyString::new(NodeUrl {
> + hostname,
> + fingerprint: fingerprint.map(|fp| fp.to_uppercase()),
> + }),
> + );
> + }
> result.nodes.sort_by(|a, b| a.hostname.cmp(&b.hostname));
> Ok(result)
> }
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager 16/21] ui: widget: add pve realm selector
2025-05-16 13:36 ` [pdm-devel] [PATCH datacenter-manager 16/21] ui: widget: add pve realm selector Dominik Csapak
@ 2025-08-19 11:55 ` Lukas Wagner
0 siblings, 0 replies; 33+ messages in thread
From: Lukas Wagner @ 2025-08-19 11:55 UTC (permalink / raw)
To: Proxmox Datacenter Manager development discussion, Dominik Csapak
On Fri May 16, 2025 at 3:36 PM CEST, Dominik Csapak wrote:
> similar to the general realm selector, but with a hostname/fingerprint
> property, so we can query the realms of any PVE instance.
>
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> ui/src/widget/mod.rs | 3 +
> ui/src/widget/pve_realm_selector.rs | 125 ++++++++++++++++++++++++++++
> 2 files changed, 128 insertions(+)
> create mode 100644 ui/src/widget/pve_realm_selector.rs
>
> diff --git a/ui/src/widget/mod.rs b/ui/src/widget/mod.rs
> index ee9e799..9d7840c 100644
> --- a/ui/src/widget/mod.rs
> +++ b/ui/src/widget/mod.rs
> @@ -13,6 +13,9 @@ pub use pve_storage_selector::PveStorageSelector;
> mod pve_migrate_mapping;
> pub use pve_migrate_mapping::PveMigrateMap;
>
> +mod pve_realm_selector;
> +pub use pve_realm_selector::PveRealmSelector;
> +
> mod resource_tree;
> pub use resource_tree::ResourceTree;
>
> diff --git a/ui/src/widget/pve_realm_selector.rs b/ui/src/widget/pve_realm_selector.rs
> new file mode 100644
> index 0000000..8025c2d
> --- /dev/null
> +++ b/ui/src/widget/pve_realm_selector.rs
> @@ -0,0 +1,125 @@
> +use anyhow::format_err;
> +use std::rc::Rc;
> +
> +use yew::html::IntoPropValue;
> +use yew::prelude::*;
> +
> +use pwt::props::RenderFn;
> +use pwt::state::Store;
> +use pwt::widget::data_table::{DataTable, DataTableColumn, DataTableHeader};
> +use pwt::widget::form::{Selector, SelectorRenderArgs, ValidateFn};
> +use pwt::widget::GridPicker;
> +
> +use proxmox_yew_comp::common_api_types::BasicRealmInfo;
> +use proxmox_yew_comp::percent_encoding::percent_encode_component;
> +
> +use pwt::props::{FieldBuilder, WidgetBuilder};
> +use pwt_macros::{builder, widget};
> +
> +#[widget(comp=PdmPveRealmSelector, @input)]
> +#[derive(Clone, Properties, PartialEq)]
> +#[builder]
> +pub struct PveRealmSelector {
> + /// The default value.
> + #[builder(IntoPropValue, into_prop_value)]
> + #[prop_or_default]
> + pub default: Option<AttrValue>,
> +
> + pub hostname: AttrValue,
> +
> + pub fingerprint: Option<AttrValue>,
> +}
> +
> +impl PveRealmSelector {
> + pub fn new(
> + hostname: impl IntoPropValue<AttrValue>,
> + fingerprint: impl IntoPropValue<Option<AttrValue>>,
> + ) -> Self {
> + yew::props!(Self {
> + hostname: hostname.into_prop_value(),
> + fingerprint: fingerprint.into_prop_value(),
> + })
> + }
> +}
> +
> +pub struct PdmPveRealmSelector {
> + store: Store<BasicRealmInfo>,
> + validate: ValidateFn<(String, Store<BasicRealmInfo>)>,
> + picker: RenderFn<SelectorRenderArgs<Store<BasicRealmInfo>>>,
> +}
> +
> +impl Component for PdmPveRealmSelector {
> + type Message = ();
> + type Properties = PveRealmSelector;
> +
> + fn create(ctx: &Context<Self>) -> Self {
> + let store = Store::new().on_change(ctx.link().callback(|_| ())); // trigger redraw
> +
> + let validate = ValidateFn::new(|(realm, store): &(String, Store<BasicRealmInfo>)| {
> + store
> + .read()
> + .data()
> + .iter()
> + .find(|item| &item.realm == realm)
> + .map(drop)
> + .ok_or_else(|| format_err!("no such realm"))
Maybe it's just me, but I find
if store.read().data().iter().any(|item| &item.realm == realm) {
Ok(())
} else {
Err(format_err!("no such realm"))
}
a bit easier to understand here (the `.map(drop)` reads a bit odd to me,
that's not something you see a lot)
> + });
> +
> + let picker = RenderFn::new({
> + let columns = columns();
> + move |args: &SelectorRenderArgs<Store<BasicRealmInfo>>| {
> + let table = DataTable::new(columns.clone(), args.store.clone()).class("pwt-fit");
> +
> + GridPicker::new(table)
> + .selection(args.selection.clone())
> + .on_select(args.controller.on_select_callback())
> + .into()
> + }
> + });
> +
> + Self {
> + store,
> + validate,
> + picker,
> + }
> + }
> +
> + fn view(&self, ctx: &Context<Self>) -> Html {
> + let props = ctx.props();
> +
> + let mut url = format!(
> + "/pve/realms?hostname={}",
> + percent_encode_component(&props.hostname)
> + );
> + if let Some(fp) = &props.fingerprint {
> + url.push_str(&format!("&fingerprint={}", percent_encode_component(fp)));
> + }
> +
> + Selector::new(self.store.clone(), self.picker.clone())
> + .with_std_props(&props.std_props)
> + .with_input_props(&props.input_props)
> + .required(true)
> + .default(props.default.as_deref().unwrap_or("pam").to_string())
> + .loader(url)
> + .validate(self.validate.clone())
> + .into()
> + }
> +}
> +
> +fn columns() -> Rc<Vec<DataTableHeader<BasicRealmInfo>>> {
> + Rc::new(vec![
> + DataTableColumn::new("Realm")
> + .width("100px")
> + .sort_order(true)
> + .show_menu(false)
> + .get_property(|record: &BasicRealmInfo| &record.realm)
> + .into(),
> + DataTableColumn::new("Comment")
> + .width("300px")
> + .show_menu(false)
> + .get_property_owned(|record: &BasicRealmInfo| {
> + record.comment.clone().unwrap_or_default()
> + })
> + .into(),
> + ])
> +}
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager 00/21] improve remote wizard
2025-05-16 13:35 [pdm-devel] [PATCH datacenter-manager 00/21] improve remote wizard Dominik Csapak
` (22 preceding siblings ...)
2025-08-18 13:48 ` [pdm-devel] superseded: " Dominik Csapak
@ 2025-08-19 12:10 ` Lukas Wagner
2025-08-19 12:52 ` Dominik Csapak
23 siblings, 1 reply; 33+ messages in thread
From: Lukas Wagner @ 2025-08-19 12:10 UTC (permalink / raw)
To: Proxmox Datacenter Manager development discussion, Dominik Csapak
On Fri May 16, 2025 at 3:35 PM CEST, Dominik Csapak wrote:
> # Summary
>
> This series improves the remote wizard in various points:
> * improved wording and texts
> * moved seperate buttons on pages into the next button
> * probing of entered nodes
> * confirmation dialog for missing fingerprints
> * better realm selector
>
> # Notes
>
> I'm not super sure about the API call structure. I think two seperate
> API calls (one for tls probing, one for scanning) might make more sense.
> If that's wanted, I'll do so in a v2 of the series.
Yeah, I think two separate API endpoints make a lot of sense, the `scan`
endpoint is a bit weird right now.
>
> ## Probing on the server side
>
> I tried to determine if we need a fingerprint for nodes inside the
> API call, by probing each node. Since we currently only get the
> nodenames and not FQDNs in the nodelist, this will currently not
> result in a valid connection in most cases and return the fingerprint.
>
> My plan would be to include FQDNs of the nodes on the PVE side API call,
> so we can return here a list of nodename, ip and FQDNs, which the user
> then can select from. (which of those I'd probe on first check is yet
> to be determined)
>
> ## Fingerprint confirmation dialogs
>
> Not sure if we want to be able to let the user confirm the fingerprint
> so easily. On one hand it's very convenient, but maybe leads to users
> simply clicking yes without understanding what's happening.
>
> If it's deemed too dangerous, I'd rework the series without this.
Good question. IMO something like this can be okay for a bit more
convenience, but maybe some other people have an opinion on this as
well.
One thing I'm not sure about is the fact that we spawn a dialog from a
dialog here, which (I think) is a first for us.
Some other minor things I've noticed while testing this:
- Entering the remote name on the second stage of the wizard seemed a
bit odd to me (can't really explain why, just a gut feeling I had when
the second stage was shown). But I guess it could make sense in case
we want to autofill the clustername there.
- When you go from "Summary" from "Probe Remote" and then to "Settings"
(all via clicking the tab bar), the 'Trust fingerprint' dialog is
shown again. Maybe we can detect that the IP/host has not been changed
and/or cache which fingerprints we already marked as trusted?
- 'Realm' field should be greyed out when "Use existing Token" has been
selected
All just minor things, all it all these changes looked quite solid to
me!
>
> # Future work
>
> The next step for the wizard is to have some kind of quick copy&paste
> info. After discussing off-list with Fabian a bit, I think it would be
> best for this to contain the hostname (FQDN?) + fingerprint (if just a
> self-signed certificate) + a list of nodes with their respective
> nodename + FQDNs (maybe requires api change on PVE side to generate
> this). The user would then still have to do most of the steps currently
> necessary in the wizard, except the manual copy & pasting of
> fingerprints and maybe entering of FQDNs.
>
>
> Dominik Csapak (21):
> server/ui: pve: change 'realm list' api call to GET
> api types: RemoteType: put default port info to the type
> server: connection: add probe_tls_connection helper
> server/ui: pve api: extend 'scan' so it can probe the tls connection
> pdm-client: add scan_remote and probe_tls methods
> ui: remotes: node url list: add placeholder and clear trigger
> ui: rmeotes: node url list: make column header clearer
> ui: remotes: node url list: handle changing default
> ui: pve wizard: rename 'realm' variable to 'info'
> ui: pve wizard: summary: add default text for fingerprint
> ui: pve wizard: nodes: improve info text
> ui: pve wizard: nodes: probe hosts to verify fingerprint settings
> ui: pve wizard: info: use pdm_client for scanning
> ui: pve wizard: info: detect hostname and fingerprint
> ui: pve wizard: info: remove manual scan button
> ui: widget: add pve realm selector
> ui: pve wizard: info: use pve realm selector
> ui: pve wizard: connect: factor out normalize_hostname
> ui: pve wizard: connect: move connection logic to next button
> ui: pve wizard: connect: use scan api endpoint instead of realms
> ui: pve wizard: connect: add certificate confirmation dialog
>
> lib/pdm-api-types/Cargo.toml | 1 +
> lib/pdm-api-types/src/lib.rs | 2 +
> lib/pdm-api-types/src/remotes.rs | 16 ++
> lib/pdm-client/src/lib.rs | 45 ++++
> server/src/api/pve/mod.rs | 62 ++++--
> server/src/connection.rs | 87 +++++++-
> ui/Cargo.toml | 1 +
> ui/src/remotes/add_wizard.rs | 8 +-
> ui/src/remotes/node_url_list.rs | 33 ++-
> ui/src/remotes/wizard_page_connect.rs | 308 ++++++++++++++++----------
> ui/src/remotes/wizard_page_info.rs | 127 ++++++-----
> ui/src/remotes/wizard_page_nodes.rs | 241 +++++++++++++++++++-
> ui/src/remotes/wizard_page_summary.rs | 5 +-
> ui/src/widget/mod.rs | 3 +
> ui/src/widget/pve_realm_selector.rs | 125 +++++++++++
> 15 files changed, 853 insertions(+), 211 deletions(-)
> create mode 100644 ui/src/widget/pve_realm_selector.rs
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager 00/21] improve remote wizard
2025-08-19 12:10 ` [pdm-devel] " Lukas Wagner
@ 2025-08-19 12:52 ` Dominik Csapak
2025-08-19 13:56 ` Lukas Wagner
0 siblings, 1 reply; 33+ messages in thread
From: Dominik Csapak @ 2025-08-19 12:52 UTC (permalink / raw)
To: Lukas Wagner, Proxmox Datacenter Manager development discussion
On 8/19/25 14:09, Lukas Wagner wrote:
> On Fri May 16, 2025 at 3:35 PM CEST, Dominik Csapak wrote:
>> # Summary
>>
>> This series improves the remote wizard in various points:
>> * improved wording and texts
>> * moved seperate buttons on pages into the next button
>> * probing of entered nodes
>> * confirmation dialog for missing fingerprints
>> * better realm selector
>>
>> # Notes
>>
>> I'm not super sure about the API call structure. I think two seperate
>> API calls (one for tls probing, one for scanning) might make more sense.
>> If that's wanted, I'll do so in a v2 of the series.
>
> Yeah, I think two separate API endpoints make a lot of sense, the `scan`
> endpoint is a bit weird right now.
>
thanks, i also thought so, but only after i did this implementation
already. will change the v3 to include a separate api call for tls
probing. Doing 2 api calls from the ui should be fine
>>
>> ## Probing on the server side
>>
>> I tried to determine if we need a fingerprint for nodes inside the
>> API call, by probing each node. Since we currently only get the
>> nodenames and not FQDNs in the nodelist, this will currently not
>> result in a valid connection in most cases and return the fingerprint.
>>
>> My plan would be to include FQDNs of the nodes on the PVE side API call,
>> so we can return here a list of nodename, ip and FQDNs, which the user
>> then can select from. (which of those I'd probe on first check is yet
>> to be determined)
>>
>> ## Fingerprint confirmation dialogs
>>
>> Not sure if we want to be able to let the user confirm the fingerprint
>> so easily. On one hand it's very convenient, but maybe leads to users
>> simply clicking yes without understanding what's happening.
>>
>> If it's deemed too dangerous, I'd rework the series without this.
>
> Good question. IMO something like this can be okay for a bit more
> convenience, but maybe some other people have an opinion on this as
> well.
>
> One thing I'm not sure about is the fact that we spawn a dialog from a
> dialog here, which (I think) is a first for us.
>
yes it's a first indeed, but especially on the nodes part of the wizard
i found no good way to show that info otherwise. I can try to integrate
the info in the panel itself, but since it requires confirmation
i would change that to a checkbox ("Trust these/this fingerprint(s)") ?
on the nodes part, it would involve scrolling to possibly show all
the fingerprints of the nodes, (which also can happen on the dialog if
there are many).
>
> Some other minor things I've noticed while testing this:
>
> - Entering the remote name on the second stage of the wizard seemed a
> bit odd to me (can't really explain why, just a gut feeling I had when
> the second stage was shown). But I guess it could make sense in case
> we want to autofill the clustername there.
i get what you mean, but yes exactly, the idea initially was to autofill
the cluster name here (so the first panel would not work).
For now I'd leave it on the second page (it's already that way) if
that's ok for you. (We can still move it around later)
>
> - When you go from "Summary" from "Probe Remote" and then to "Settings"
> (all via clicking the tab bar), the 'Trust fingerprint' dialog is
> shown again. Maybe we can detect that the IP/host has not been changed
> and/or cache which fingerprints we already marked as trusted?
Should be possible, I'll see that I do it that way.
>
> - 'Realm' field should be greyed out when "Use existing Token" has been
> selected
oops, that's a bug^^
>
> All just minor things, all it all these changes looked quite solid to
> me!
>
>>
>> # Future work
>>
>> The next step for the wizard is to have some kind of quick copy&paste
>> info. After discussing off-list with Fabian a bit, I think it would be
>> best for this to contain the hostname (FQDN?) + fingerprint (if just a
>> self-signed certificate) + a list of nodes with their respective
>> nodename + FQDNs (maybe requires api change on PVE side to generate
>> this). The user would then still have to do most of the steps currently
>> necessary in the wizard, except the manual copy & pasting of
>> fingerprints and maybe entering of FQDNs.
>>
>>
>> Dominik Csapak (21):
>> server/ui: pve: change 'realm list' api call to GET
>> api types: RemoteType: put default port info to the type
>> server: connection: add probe_tls_connection helper
>> server/ui: pve api: extend 'scan' so it can probe the tls connection
>> pdm-client: add scan_remote and probe_tls methods
>> ui: remotes: node url list: add placeholder and clear trigger
>> ui: rmeotes: node url list: make column header clearer
>> ui: remotes: node url list: handle changing default
>> ui: pve wizard: rename 'realm' variable to 'info'
>> ui: pve wizard: summary: add default text for fingerprint
>> ui: pve wizard: nodes: improve info text
>> ui: pve wizard: nodes: probe hosts to verify fingerprint settings
>> ui: pve wizard: info: use pdm_client for scanning
>> ui: pve wizard: info: detect hostname and fingerprint
>> ui: pve wizard: info: remove manual scan button
>> ui: widget: add pve realm selector
>> ui: pve wizard: info: use pve realm selector
>> ui: pve wizard: connect: factor out normalize_hostname
>> ui: pve wizard: connect: move connection logic to next button
>> ui: pve wizard: connect: use scan api endpoint instead of realms
>> ui: pve wizard: connect: add certificate confirmation dialog
>>
>> lib/pdm-api-types/Cargo.toml | 1 +
>> lib/pdm-api-types/src/lib.rs | 2 +
>> lib/pdm-api-types/src/remotes.rs | 16 ++
>> lib/pdm-client/src/lib.rs | 45 ++++
>> server/src/api/pve/mod.rs | 62 ++++--
>> server/src/connection.rs | 87 +++++++-
>> ui/Cargo.toml | 1 +
>> ui/src/remotes/add_wizard.rs | 8 +-
>> ui/src/remotes/node_url_list.rs | 33 ++-
>> ui/src/remotes/wizard_page_connect.rs | 308 ++++++++++++++++----------
>> ui/src/remotes/wizard_page_info.rs | 127 ++++++-----
>> ui/src/remotes/wizard_page_nodes.rs | 241 +++++++++++++++++++-
>> ui/src/remotes/wizard_page_summary.rs | 5 +-
>> ui/src/widget/mod.rs | 3 +
>> ui/src/widget/pve_realm_selector.rs | 125 +++++++++++
>> 15 files changed, 853 insertions(+), 211 deletions(-)
>> create mode 100644 ui/src/widget/pve_realm_selector.rs
>
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager 00/21] improve remote wizard
2025-08-19 12:52 ` Dominik Csapak
@ 2025-08-19 13:56 ` Lukas Wagner
0 siblings, 0 replies; 33+ messages in thread
From: Lukas Wagner @ 2025-08-19 13:56 UTC (permalink / raw)
To: Dominik Csapak, pdm-devel
On Tue Aug 19, 2025 at 2:52 PM CEST, Dominik Csapak wrote:
>
>
> On 8/19/25 14:09, Lukas Wagner wrote:
>> On Fri May 16, 2025 at 3:35 PM CEST, Dominik Csapak wrote:
>>> # Summary
>>>
>>> This series improves the remote wizard in various points:
>>> * improved wording and texts
>>> * moved seperate buttons on pages into the next button
>>> * probing of entered nodes
>>> * confirmation dialog for missing fingerprints
>>> * better realm selector
>>>
>>> # Notes
>>>
>>> I'm not super sure about the API call structure. I think two seperate
>>> API calls (one for tls probing, one for scanning) might make more sense.
>>> If that's wanted, I'll do so in a v2 of the series.
>>
>> Yeah, I think two separate API endpoints make a lot of sense, the `scan`
>> endpoint is a bit weird right now.
>>
> thanks, i also thought so, but only after i did this implementation
> already. will change the v3 to include a separate api call for tls
> probing. Doing 2 api calls from the ui should be fine
>
>>>
>>> ## Probing on the server side
>>>
>>> I tried to determine if we need a fingerprint for nodes inside the
>>> API call, by probing each node. Since we currently only get the
>>> nodenames and not FQDNs in the nodelist, this will currently not
>>> result in a valid connection in most cases and return the fingerprint.
>>>
>>> My plan would be to include FQDNs of the nodes on the PVE side API call,
>>> so we can return here a list of nodename, ip and FQDNs, which the user
>>> then can select from. (which of those I'd probe on first check is yet
>>> to be determined)
>>>
>>> ## Fingerprint confirmation dialogs
>>>
>>> Not sure if we want to be able to let the user confirm the fingerprint
>>> so easily. On one hand it's very convenient, but maybe leads to users
>>> simply clicking yes without understanding what's happening.
>>>
>>> If it's deemed too dangerous, I'd rework the series without this.
>>
>> Good question. IMO something like this can be okay for a bit more
>> convenience, but maybe some other people have an opinion on this as
>> well.
>>
>> One thing I'm not sure about is the fact that we spawn a dialog from a
>> dialog here, which (I think) is a first for us.
>>
>
> yes it's a first indeed, but especially on the nodes part of the wizard
> i found no good way to show that info otherwise. I can try to integrate
> the info in the panel itself, but since it requires confirmation
> i would change that to a checkbox ("Trust these/this fingerprint(s)") ?
>
> on the nodes part, it would involve scrolling to possibly show all
> the fingerprints of the nodes, (which also can happen on the dialog if
> there are many).
>
Mhmmm, given the security implications I think the dialog is actually
the best choice for now (as it actively seeks the user's attention), so
IMO this can stay as is for now.
>>
>> Some other minor things I've noticed while testing this:
>>
>> - Entering the remote name on the second stage of the wizard seemed a
>> bit odd to me (can't really explain why, just a gut feeling I had when
>> the second stage was shown). But I guess it could make sense in case
>> we want to autofill the clustername there.
>
> i get what you mean, but yes exactly, the idea initially was to autofill
> the cluster name here (so the first panel would not work).
>
> For now I'd leave it on the second page (it's already that way) if
> that's ok for you. (We can still move it around later)
Fine for me :)
>
>>
>> - When you go from "Summary" from "Probe Remote" and then to "Settings"
>> (all via clicking the tab bar), the 'Trust fingerprint' dialog is
>> shown again. Maybe we can detect that the IP/host has not been changed
>> and/or cache which fingerprints we already marked as trusted?
>
> Should be possible, I'll see that I do it that way.
Could you just add the accepted fingerprint into the "fingerprint" field
in the first page? I think this would be a quite simple fix for this.
>
>>
>> - 'Realm' field should be greyed out when "Use existing Token" has been
>> selected
>
> oops, that's a bug^^
>
>>
>> All just minor things, all it all these changes looked quite solid to
>> me!
>>
>>>
>>> # Future work
>>>
>>> The next step for the wizard is to have some kind of quick copy&paste
>>> info. After discussing off-list with Fabian a bit, I think it would be
>>> best for this to contain the hostname (FQDN?) + fingerprint (if just a
>>> self-signed certificate) + a list of nodes with their respective
>>> nodename + FQDNs (maybe requires api change on PVE side to generate
>>> this). The user would then still have to do most of the steps currently
>>> necessary in the wizard, except the manual copy & pasting of
>>> fingerprints and maybe entering of FQDNs.
>>>
>>>
>>> Dominik Csapak (21):
>>> server/ui: pve: change 'realm list' api call to GET
>>> api types: RemoteType: put default port info to the type
>>> server: connection: add probe_tls_connection helper
>>> server/ui: pve api: extend 'scan' so it can probe the tls connection
>>> pdm-client: add scan_remote and probe_tls methods
>>> ui: remotes: node url list: add placeholder and clear trigger
>>> ui: rmeotes: node url list: make column header clearer
>>> ui: remotes: node url list: handle changing default
>>> ui: pve wizard: rename 'realm' variable to 'info'
>>> ui: pve wizard: summary: add default text for fingerprint
>>> ui: pve wizard: nodes: improve info text
>>> ui: pve wizard: nodes: probe hosts to verify fingerprint settings
>>> ui: pve wizard: info: use pdm_client for scanning
>>> ui: pve wizard: info: detect hostname and fingerprint
>>> ui: pve wizard: info: remove manual scan button
>>> ui: widget: add pve realm selector
>>> ui: pve wizard: info: use pve realm selector
>>> ui: pve wizard: connect: factor out normalize_hostname
>>> ui: pve wizard: connect: move connection logic to next button
>>> ui: pve wizard: connect: use scan api endpoint instead of realms
>>> ui: pve wizard: connect: add certificate confirmation dialog
>>>
>>> lib/pdm-api-types/Cargo.toml | 1 +
>>> lib/pdm-api-types/src/lib.rs | 2 +
>>> lib/pdm-api-types/src/remotes.rs | 16 ++
>>> lib/pdm-client/src/lib.rs | 45 ++++
>>> server/src/api/pve/mod.rs | 62 ++++--
>>> server/src/connection.rs | 87 +++++++-
>>> ui/Cargo.toml | 1 +
>>> ui/src/remotes/add_wizard.rs | 8 +-
>>> ui/src/remotes/node_url_list.rs | 33 ++-
>>> ui/src/remotes/wizard_page_connect.rs | 308 ++++++++++++++++----------
>>> ui/src/remotes/wizard_page_info.rs | 127 ++++++-----
>>> ui/src/remotes/wizard_page_nodes.rs | 241 +++++++++++++++++++-
>>> ui/src/remotes/wizard_page_summary.rs | 5 +-
>>> ui/src/widget/mod.rs | 3 +
>>> ui/src/widget/pve_realm_selector.rs | 125 +++++++++++
>>> 15 files changed, 853 insertions(+), 211 deletions(-)
>>> create mode 100644 ui/src/widget/pve_realm_selector.rs
>>
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 33+ messages in thread
end of thread, other threads:[~2025-08-19 13:55 UTC | newest]
Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-05-16 13:35 [pdm-devel] [PATCH datacenter-manager 00/21] improve remote wizard Dominik Csapak
2025-05-16 13:35 ` [pdm-devel] [PATCH datacenter-manager 01/21] server/ui: pve: change 'realm list' api call to GET Dominik Csapak
2025-05-16 13:35 ` [pdm-devel] [PATCH datacenter-manager 02/21] api types: RemoteType: put default port info to the type Dominik Csapak
2025-05-16 13:35 ` [pdm-devel] [PATCH datacenter-manager 03/21] server: connection: add probe_tls_connection helper Dominik Csapak
2025-08-19 11:54 ` Lukas Wagner
2025-05-16 13:35 ` [pdm-devel] [PATCH datacenter-manager 04/21] server/ui: pve api: extend 'scan' so it can probe the tls connection Dominik Csapak
2025-08-19 11:54 ` Lukas Wagner
2025-05-16 13:35 ` [pdm-devel] [PATCH datacenter-manager 05/21] pdm-client: add scan_remote and probe_tls methods Dominik Csapak
2025-08-19 11:55 ` Lukas Wagner
2025-05-16 13:35 ` [pdm-devel] [PATCH datacenter-manager 06/21] ui: remotes: node url list: add placeholder and clear trigger Dominik Csapak
2025-05-16 13:35 ` [pdm-devel] [PATCH datacenter-manager 07/21] ui: rmeotes: node url list: make column header clearer Dominik Csapak
2025-05-16 13:35 ` [pdm-devel] [PATCH datacenter-manager 08/21] ui: remotes: node url list: handle changing default Dominik Csapak
2025-05-16 13:35 ` [pdm-devel] [PATCH datacenter-manager 09/21] ui: pve wizard: rename 'realm' variable to 'info' Dominik Csapak
2025-05-16 13:36 ` [pdm-devel] [PATCH datacenter-manager 10/21] ui: pve wizard: summary: add default text for fingerprint Dominik Csapak
2025-05-16 13:36 ` [pdm-devel] [PATCH datacenter-manager 11/21] ui: pve wizard: nodes: improve info text Dominik Csapak
2025-05-16 13:36 ` [pdm-devel] [PATCH datacenter-manager 12/21] ui: pve wizard: nodes: probe hosts to verify fingerprint settings Dominik Csapak
2025-05-16 13:36 ` [pdm-devel] [PATCH datacenter-manager 13/21] ui: pve wizard: info: use pdm_client for scanning Dominik Csapak
2025-05-16 13:36 ` [pdm-devel] [PATCH datacenter-manager 14/21] ui: pve wizard: info: detect hostname and fingerprint Dominik Csapak
2025-08-19 11:55 ` Lukas Wagner
2025-05-16 13:36 ` [pdm-devel] [PATCH datacenter-manager 15/21] ui: pve wizard: info: remove manual scan button Dominik Csapak
2025-05-16 13:36 ` [pdm-devel] [PATCH datacenter-manager 16/21] ui: widget: add pve realm selector Dominik Csapak
2025-08-19 11:55 ` Lukas Wagner
2025-05-16 13:36 ` [pdm-devel] [PATCH datacenter-manager 17/21] ui: pve wizard: info: use " Dominik Csapak
2025-05-16 13:36 ` [pdm-devel] [PATCH datacenter-manager 18/21] ui: pve wizard: connect: factor out normalize_hostname Dominik Csapak
2025-05-16 13:36 ` [pdm-devel] [PATCH datacenter-manager 19/21] ui: pve wizard: connect: move connection logic to next button Dominik Csapak
2025-05-16 13:36 ` [pdm-devel] [PATCH datacenter-manager 20/21] ui: pve wizard: connect: use scan api endpoint instead of realms Dominik Csapak
2025-05-16 13:36 ` [pdm-devel] [PATCH datacenter-manager 21/21] ui: pve wizard: connect: add certificate confirmation dialog Dominik Csapak
2025-08-18 12:41 ` [pdm-devel] [PATCH datacenter-manager 00/21] improve remote wizard Lukas Wagner
2025-08-18 12:53 ` Dominik Csapak
2025-08-18 13:48 ` [pdm-devel] superseded: " Dominik Csapak
2025-08-19 12:10 ` [pdm-devel] " Lukas Wagner
2025-08-19 12:52 ` Dominik Csapak
2025-08-19 13:56 ` Lukas Wagner
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.